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/vc4: crtc: Reduce PV fifo threshold on hvs4 #4207

Merged
merged 1 commit into from
Mar 12, 2021

Conversation

popcornmix
Copy link
Collaborator

@popcornmix popcornmix commented Mar 12, 2021

Experimentally have found PV on hv4 reports fifo full
error with expected settings and does not with one less

This appears as:
[drm:drm_atomic_helper_wait_for_flip_done] *ERROR* [CRTC:82:crtc-3] flip_done timed out

with bit 10 of PV_STAT set "HVS driving pixels when the PV FIFO is full"

Signed-off-by: Dom Cobley [email protected]

@popcornmix
Copy link
Collaborator Author

@mripard I believe this is the fix for the flip_done issue on Pi3.
Waiting for @HiassofT to confirm.

My test has got to 738 reboots of test where kodi is started and osd is brought up.
Without this patch it typically fails after a few reboots.

It's hard to say if any other conditions in this function would benefit from a threshold one lower.
We've never seen it on Pi4 and it's so hard to provoke reliably (heisenbug) that looking for it in other situations is unlikely to be successful.

Just note here that if flip_done error with bit 10 of PVSTAT set is found again then reducing fifo threshold for that case should be the first thing to try.

@HiassofT
Copy link
Contributor

I've been testing with this PR on RPi3B+ this whole afternoon and didn't get a single flip_done timeout - without the PR I had about a 50:50 chance if flip_done/screen cutout would hit me which made kodi testing nearly impossible.

I'd vote for getting this merged, it's a huge improvement for me (and other users as well)

@@ -238,6 +241,13 @@ static u32 vc4_get_fifo_full_level(struct vc4_crtc *vc4_crtc, u32 format)
if (crtc_data->hvs_output == 5)
return 32;

/*
* Experimentally have found PV on hv4 reports fifo full
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/hv4/hvs4

@@ -210,6 +210,9 @@ static u32 vc4_get_fifo_full_level(struct vc4_crtc *vc4_crtc, u32 format)
{
const struct vc4_crtc_data *crtc_data = vc4_crtc_to_vc4_crtc_data(vc4_crtc);
const struct vc4_pv_data *pv_data = vc4_crtc_to_vc4_pv_data(vc4_crtc);
struct drm_crtc *crtc = &vc4_crtc->base;
struct drm_device *dev = crtc->dev;
struct vc4_dev *vc4 = to_vc4_dev(dev);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slight nitpicking but you don't need crtc, so you may as well do dev = vc4_crtc->base.dev (if I've got my indirections right). Or as dev isn't actually needed either,

struct vc4_dev *vc4 = to_vc4_dev(vc4_crtc->base.dev);

Experimentally have found PV on hvs4 reports fifo full
error with expected settings and does not with one less

This appears as:
[drm:drm_atomic_helper_wait_for_flip_done] *ERROR* [CRTC:82:crtc-3] flip_done timed out

with bit 10 of PV_STAT set "HVS driving pixels when the PV FIFO is full"

Signed-off-by: Dom Cobley <[email protected]>
@popcornmix popcornmix marked this pull request as ready for review March 12, 2021 19:06
@popcornmix
Copy link
Collaborator Author

Updated to address @6by9's comments.

@pelwell pelwell merged commit b2625fe into raspberrypi:rpi-5.10.y Mar 12, 2021
@popcornmix popcornmix deleted the pvfifosize branch March 14, 2021 12:13
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Mar 15, 2021
kernel: drm/vc4: crtc: Reduce PV fifo threshold on hvs4
See: raspberrypi/linux#4207

kernel: vc4/drm: Adjustments to hdmi audio dma to reduce glitches
See: raspberrypi/linux#4208

kernel: overlays: gpio-led: new overlay
See: raspberrypi/linux#4206

kernel: bcm2835-codec tweaks
See: raspberrypi/linux#4113

kernel: Assign crypto aliases to different AES implementation modules
See: raspberrypi/linux#4198

kernel: media: bcm2835-unicam: Fix bug in buffer swapping logic
See: raspberrypi/linux#4189

kernel: configs: Add CONFIG_RTS_HCTOSYS=y
See: raspberrypi/linux#4205

kernel: overlays: Improve the i2c-rtc,i2c_csi_dsi option

firmware: video_decode: For VC1/WMV with no signalled header bytes, use start of 1st buffer
See: raspberrypi/linux#4113
popcornmix added a commit to Hexxeh/rpi-firmware that referenced this pull request Mar 15, 2021
kernel: drm/vc4: crtc: Reduce PV fifo threshold on hvs4
See: raspberrypi/linux#4207

kernel: vc4/drm: Adjustments to hdmi audio dma to reduce glitches
See: raspberrypi/linux#4208

kernel: overlays: gpio-led: new overlay
See: raspberrypi/linux#4206

kernel: bcm2835-codec tweaks
See: raspberrypi/linux#4113

kernel: Assign crypto aliases to different AES implementation modules
See: raspberrypi/linux#4198

kernel: media: bcm2835-unicam: Fix bug in buffer swapping logic
See: raspberrypi/linux#4189

kernel: configs: Add CONFIG_RTS_HCTOSYS=y
See: raspberrypi/linux#4205

kernel: overlays: Improve the i2c-rtc,i2c_csi_dsi option

firmware: video_decode: For VC1/WMV with no signalled header bytes, use start of 1st buffer
See: raspberrypi/linux#4113
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.

4 participants