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: Skip writes to disabled packet RAM #4770

Merged
merged 1 commit into from
Dec 17, 2021

Conversation

popcornmix
Copy link
Collaborator

This path actually occurs when audio is started during a hdmi mode set.
As the data will be written by vc4_hdmi_set_infoframes when packet RAM
is enabled again, don't treat as an error

Signed-off-by: Dom Cobley popcornmix@gmail.com

@popcornmix
Copy link
Collaborator Author

@HiassofT does this help your issue from #4759 (comment) ?

@6by9
Copy link
Contributor

6by9 commented Dec 14, 2021

Having the WARN in vc4_hdmi_write_infoframe is useful for ensuring we don't try to write it when powered down.

It seems better to make the vc4_hdmi_set_audio_infoframe call in vc4_hdmi_audio_prepare conditional on the video side being enabled (that would be the vc4_hdmi->output_enabled flag we just removed, but set in vc4_hdmi_encoder_post_crtc_enable before we call vc4_hdmi_set_infoframes).

Your current patch is also accessing registers without having acquired vc4_hdmi->hw_lock first.

@popcornmix popcornmix force-pushed the skip_packet branch 2 times, most recently from 925c12d to 435f4f6 Compare December 14, 2021 15:13
This path actually occurs when audio is started during a hdmi mode set.
As the data will be written by vc4_hdmi_set_infoframes when packet RAM
is enabled again, don't treat as an error

Signed-off-by: Dom Cobley <popcornmix@gmail.com>
@popcornmix
Copy link
Collaborator Author

@6by9 updated - was that what you were after?

@6by9
Copy link
Contributor

6by9 commented Dec 14, 2021

Yes, that was what I was thinking of.

Github seems to have messed up the indentation when viewing the patch, but I'll trust that it's GH at fault.

@pelwell
Copy link
Contributor

pelwell commented Dec 14, 2021

Github seems to have messed up the indentation when viewing the patch, but I'll trust that it's GH at fault.

Strange - it looks OK here.

@6by9
Copy link
Contributor

6by9 commented Dec 14, 2021

I'd expanded file before and after the 3rd hunk, and it's differently indented the unmodified bit vs the patch.

@popcornmix
Copy link
Collaborator Author

Github is doing something funny when expanding this part:

		vc4_hdmi_set_infoframes(encoder);
	}
	vc4_hdmi_recenter_fifo(vc4_hdmi);
	vc4_hdmi_enable_scrambling(encoder);

Seems like they are using different tab widths for the new code and old code,
although it looks fine when I've copied it and in editor it is fine (all initial white space is tabs).

@popcornmix popcornmix marked this pull request as ready for review December 17, 2021 10:54
@popcornmix
Copy link
Collaborator Author

Removed draft status as tested by @HiassofT (and me for a few days).

@popcornmix
Copy link
Collaborator Author

@pelwell okay to merge?

@pelwell pelwell merged commit 72b58ab into raspberrypi:rpi-5.10.y Dec 17, 2021
@popcornmix popcornmix deleted the skip_packet branch December 17, 2021 15:08
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Dec 17, 2021
kernel: drm/vc4: Skip writes to disabled packet RAM
See: raspberrypi/linux#4770

kernel: BCM2711 KMS YUV Support
See: raspberrypi/linux#4777

kernel: Revert media: bcm2835-codec: Limit video callbacks
See: raspberrypi/linux#4773

kernel: staging/bcm2835-isp: Fix cleanup after init fail
See: raspberrypi/linux#4774
popcornmix added a commit to raspberrypi/rpi-firmware that referenced this pull request Dec 17, 2021
kernel: drm/vc4: Skip writes to disabled packet RAM
See: raspberrypi/linux#4770

kernel: BCM2711 KMS YUV Support
See: raspberrypi/linux#4777

kernel: Revert media: bcm2835-codec: Limit video callbacks
See: raspberrypi/linux#4773

kernel: staging/bcm2835-isp: Fix cleanup after init fail
See: raspberrypi/linux#4774
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.

3 participants