Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DRM: rp1: rp1-dsi: Fix escape clock divider and timeouts. #6120

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

njhollinghurst
Copy link
Contributor

Escape clock divider was fixed at 5, which is correct at 800Mbps/lane but increasingly out of spec for higher rates. Compute it correctly.

High speed timeout was fixed at 5*512 == 2560 byte-clocks per lane. Compute it conservatively to be 8/7 times the line period (assuming there will be a transition to LP some time during each scanline?) keeping the old value as a lower bound. Increase LPRX TO to 1024, and BTA TO to 0xb00 (same value as in bridge/synopsys/dw-mipi-dsi).

(No change to LP_CMD_TIM. To do: compute this correctly.)

Escape clock divider was fixed at 5, which is correct at 800Mbps/lane
but increasingly out of spec for higher rates. Compute it correctly.

High speed timeout was fixed at 5*512 == 2560 byte-clocks per lane.
Compute it conservatively to be 8/7 times the line period (assuming
there will be a transition to LP some time during each scanline?)
keeping the old value as a lower bound. Increase LPRX TO to 1024,
and BTA TO to 0xb00 (same value as in bridge/synopsys/dw-mipi-dsi).

(No change to LP_CMD_TIM. To do: compute this correctly.)

Signed-off-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>
@njhollinghurst
Copy link
Contributor Author

I think this fixes wide-scanline DSI flakiness, and perhaps setup issues at high bitrates. Some loose ends remain:

  • The HSTX timeout is computed assuming that there will be a HS->LP transition every scanline. If that's not the case then neither this nor the fixed value (1000) from bridge/synopsys/dw-mipi-dsi would be sufficient.
  • The LPRX and BTA timeouts are still arbitrary magic numbers.
  • LP_CMD_TIM is not calculated: we assume that short commands can be accommodated during VBLANK (both here and in bridge/synopsys; but unlike the latter I have not allowed them in HBLANK).
  • Nothing here explains why 4-lane 1920x1200 might misbehave or run slowly?

And there's the question of whether we should be using the bridge/synopsys driver instead! I avoided this because it requires a great deal of adaptation / boilerplate (since PHY config-bus, clock and power arrangements can vary so much between chips).

@6by9
Copy link
Contributor

6by9 commented Apr 22, 2024

Seems to work for 2 lane mode.

I've added a line to the overlay to allow overriding mode, and both sync events and sync pulse modes do the expected thing.
I've been testing with a number of timings, but mainly
Crtc 0/@33: 1280x800@18.19 30.000 1280/136/200/336/? 800/3/6/36/? 18 (18.19) P|D (360Mbits/s/lane)
Crtc 0/@33: 1920x1080@33.37 83.333 1920/100/100/100/+ 1080/3/6/36/+ 0 (33.37) (1000Mbit/s/lane)

4 lane mode is odd.

  • Sync events mode is giving me 2 HSYNC_START (0x21) packets per line, with 1 immediately after the RGB data packet. The image data appears scrambled, and 1280x800 is being read as 1280x267. kmstest --flip is also reporting 58.2fps, when the clock setup should be giving us 18.19fps, and I've got two vertical white bars across the width of the image. The 1920x1080 mode is being detected as 1920x540, and the expected 33.37fps is coming through as 38.44 (reporting as 500Mbit/s/lane).
  • Sync pulse mode with the either timing above is only giving sync pulses (with blanking packet between), no image data.

@6by9
Copy link
Contributor

6by9 commented Apr 22, 2024

Tell a lie - we get RGB, blanking packet, and then HSync start. Drop to LP, wait a bit, back to HS with a HSync start, back to LP. Then repeat.

@njhollinghurst
Copy link
Contributor Author

I'll try to reproduce this in the sim. I expect it's unrelated to the timeout(?) but it'd be good to fix it while we're here.

@njhollinghurst
Copy link
Contributor Author

njhollinghurst commented Apr 23, 2024

Ahhh -- I suspect it's another driver silliness and we most likely have never correctly supported 4 lanes.

Currently we make the DPI clock by dividing down the DSI byte clock. That is not going to work in cases where bpp< 8*lanes.

Will have to think about another way to route clocks to synchronize them. I'm not yet completely sure how synchronized they need to be for correct operation in non-burst mode... The DSI PLL has finite precision and doesn't produce exactly the requested frequency. Everything is coming from the same crystal so it should be tractable.

@njhollinghurst
Copy link
Contributor Author

I'm inclined to push this change and raise a new issue for 4 lanes.

We may need to investigate what pixel-clock precision can be achieved while keeping DPI and DSI in effective lock-step; or alternatively what happens when they're not quite aligned (hopefully the worst that will happen is that HFP duration becomes slightly irregular).

It's a bit of an odd design that we have these two clock / video timing domains, one of them purely internal. Unlike in 2712 there is no pixel-valve (there is an eDPI flow-control signal, but I think it's only for command-mode displays).

@6by9
Copy link
Contributor

6by9 commented Apr 24, 2024

I'm inclined to push this change and raise a new issue for 4 lanes.

Absolutely. Feel free to drop the Draft tag from this PR.

@njhollinghurst njhollinghurst marked this pull request as ready for review April 24, 2024 13:46
@pelwell pelwell merged commit 85cc26d into raspberrypi:rpi-6.6.y Apr 24, 2024
12 checks passed
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Apr 26, 2024
See: raspberrypi/linux#6113

kernel: DRM: rp1: rp1-dsi: Fix escape clock divider and timeouts
See: raspberrypi/linux#6120

kernel: vc4/hdmi: Ignore hotplug interrupt with force_hotplug
See: raspberrypi/linux#6123

kernel: drivers: media: cfe: Add remap entries for mono formats
See: raspberrypi/linux#6122

kernel: dw-axi-dmac-platform: Avoid trampling with zero length buffer
See: raspberrypi/linux#6118

kernel: Add a strict_gpiod option
See: raspberrypi/linux#6117

kernel: Unicam FS/FE timing GPIO
See: raspberrypi/linux#6088
popcornmix added a commit to raspberrypi/rpi-firmware that referenced this pull request Apr 26, 2024
See: raspberrypi/linux#6113

kernel: DRM: rp1: rp1-dsi: Fix escape clock divider and timeouts
See: raspberrypi/linux#6120

kernel: vc4/hdmi: Ignore hotplug interrupt with force_hotplug
See: raspberrypi/linux#6123

kernel: drivers: media: cfe: Add remap entries for mono formats
See: raspberrypi/linux#6122

kernel: dw-axi-dmac-platform: Avoid trampling with zero length buffer
See: raspberrypi/linux#6118

kernel: Add a strict_gpiod option
See: raspberrypi/linux#6117

kernel: Unicam FS/FE timing GPIO
See: raspberrypi/linux#6088
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants