| Message ID | 20241202181939.724011-1-derek.foreman@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/connector: Allow clearing HDMI infoframes | expand |
On Mon, Dec 02, 2024 at 12:19:39PM -0600, Derek Foreman wrote: > Our infoframe setting code currently lacks the ability to clear > infoframes. For some of the infoframes, we only need to replace them, > so if an error occurred when generating a new infoframe we would leave > a stale frame instead of clearing the frame. > > However, the Dynamic Range and Mastering (DRM) infoframe should only > be present when displaying HDR content (ie: the HDR_OUTPUT_METADATA blob > is set). If we can't clear infoframes, the stale DRM infoframe will > remain and we can never set the display back to SDR mode. > > With this change, we clear infoframes when they can not, or should not, > be generated. This fixes switching to an SDR mode from an HDR one. > > Fixes: f378b77227bc4 ("drm/connector: hdmi: Add Infoframes generation") > Signed-off-by: Derek Foreman <derek.foreman@collabora.com> > --- > drivers/gpu/drm/display/drm_hdmi_state_helper.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Il 02/12/24 19:19, Derek Foreman ha scritto: > Our infoframe setting code currently lacks the ability to clear > infoframes. For some of the infoframes, we only need to replace them, > so if an error occurred when generating a new infoframe we would leave > a stale frame instead of clearing the frame. > > However, the Dynamic Range and Mastering (DRM) infoframe should only > be present when displaying HDR content (ie: the HDR_OUTPUT_METADATA blob > is set). If we can't clear infoframes, the stale DRM infoframe will > remain and we can never set the display back to SDR mode. > > With this change, we clear infoframes when they can not, or should not, > be generated. This fixes switching to an SDR mode from an HDR one. > > Fixes: f378b77227bc4 ("drm/connector: hdmi: Add Infoframes generation") > Signed-off-by: Derek Foreman <derek.foreman@collabora.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Just a ping - is there anything further I need to do here? On 2024-12-03 03:45, AngeloGioacchino Del Regno wrote: > Il 02/12/24 19:19, Derek Foreman ha scritto: >> Our infoframe setting code currently lacks the ability to clear >> infoframes. For some of the infoframes, we only need to replace them, >> so if an error occurred when generating a new infoframe we would leave >> a stale frame instead of clearing the frame. >> >> However, the Dynamic Range and Mastering (DRM) infoframe should only >> be present when displaying HDR content (ie: the HDR_OUTPUT_METADATA blob >> is set). If we can't clear infoframes, the stale DRM infoframe will >> remain and we can never set the display back to SDR mode. >> >> With this change, we clear infoframes when they can not, or should not, >> be generated. This fixes switching to an SDR mode from an HDR one. >> >> Fixes: f378b77227bc4 ("drm/connector: hdmi: Add Infoframes generation") >> Signed-off-by: Derek Foreman <derek.foreman@collabora.com> > > Reviewed-by: AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> > >
On Mon, Dec 16, 2024 at 02:50:13PM +0000, Derek Foreman wrote: > Just a ping - is there anything further I need to do here? I expected that the patch should have been reviewed by Maxime. I'll check if it's okay to push it with Angelo's and mine reviews. > > On 2024-12-03 03:45, AngeloGioacchino Del Regno wrote: > > Il 02/12/24 19:19, Derek Foreman ha scritto: > > > Our infoframe setting code currently lacks the ability to clear > > > infoframes. For some of the infoframes, we only need to replace them, > > > so if an error occurred when generating a new infoframe we would leave > > > a stale frame instead of clearing the frame. > > > > > > However, the Dynamic Range and Mastering (DRM) infoframe should only > > > be present when displaying HDR content (ie: the HDR_OUTPUT_METADATA blob > > > is set). If we can't clear infoframes, the stale DRM infoframe will > > > remain and we can never set the display back to SDR mode. > > > > > > With this change, we clear infoframes when they can not, or should not, > > > be generated. This fixes switching to an SDR mode from an HDR one. > > > > > > Fixes: f378b77227bc4 ("drm/connector: hdmi: Add Infoframes generation") > > > Signed-off-by: Derek Foreman <derek.foreman@collabora.com> > > > > Reviewed-by: AngeloGioacchino Del Regno > > <angelogioacchino.delregno@collabora.com> > > > >
On Mon, 02 Dec 2024 12:19:39 -0600, Derek Foreman wrote: > Our infoframe setting code currently lacks the ability to clear > infoframes. For some of the infoframes, we only need to replace them, > so if an error occurred when generating a new infoframe we would leave > a stale frame instead of clearing the frame. > > However, the Dynamic Range and Mastering (DRM) infoframe should only > be present when displaying HDR content (ie: the HDR_OUTPUT_METADATA blob > is set). If we can't clear infoframes, the stale DRM infoframe will > remain and we can never set the display back to SDR mode. > > [...] Applied to misc/kernel.git (drm-misc-next). Thanks! Maxime
diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c index feb7a3a75981..936a8f95d80f 100644 --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c @@ -347,6 +347,8 @@ static int hdmi_generate_avi_infoframe(const struct drm_connector *connector, is_limited_range ? HDMI_QUANTIZATION_RANGE_LIMITED : HDMI_QUANTIZATION_RANGE_FULL; int ret; + infoframe->set = false; + ret = drm_hdmi_avi_infoframe_from_display_mode(frame, connector, mode); if (ret) return ret; @@ -376,6 +378,8 @@ static int hdmi_generate_spd_infoframe(const struct drm_connector *connector, &infoframe->data.spd; int ret; + infoframe->set = false; + ret = hdmi_spd_infoframe_init(frame, connector->hdmi.vendor, connector->hdmi.product); @@ -398,6 +402,8 @@ static int hdmi_generate_hdr_infoframe(const struct drm_connector *connector, &infoframe->data.drm; int ret; + infoframe->set = false; + if (connector->max_bpc < 10) return 0; @@ -425,6 +431,8 @@ static int hdmi_generate_hdmi_vendor_infoframe(const struct drm_connector *conne &infoframe->data.vendor.hdmi; int ret; + infoframe->set = false; + if (!info->has_hdmi_infoframe) return 0;
Our infoframe setting code currently lacks the ability to clear infoframes. For some of the infoframes, we only need to replace them, so if an error occurred when generating a new infoframe we would leave a stale frame instead of clearing the frame. However, the Dynamic Range and Mastering (DRM) infoframe should only be present when displaying HDR content (ie: the HDR_OUTPUT_METADATA blob is set). If we can't clear infoframes, the stale DRM infoframe will remain and we can never set the display back to SDR mode. With this change, we clear infoframes when they can not, or should not, be generated. This fixes switching to an SDR mode from an HDR one. Fixes: f378b77227bc4 ("drm/connector: hdmi: Add Infoframes generation") Signed-off-by: Derek Foreman <derek.foreman@collabora.com> --- drivers/gpu/drm/display/drm_hdmi_state_helper.c | 8 ++++++++ 1 file changed, 8 insertions(+)