[PATCH] drm: Don't block HDR_OUTPUT_METADATA on unknown EOTF

[PATCH] drm: Don't block HDR_OUTPUT_METADATA on unknown EOTF

[Harry Wentland]

The supported EOTFs are defined in eotf_supported in drm_edid
but userspace has no way of knowing what is and isn't supported
when creating an HDR_OUTPUT_METADATA and will only know
something is wrong when the atomic commit fails.

Since it is expected that userspace reads the EDID to understand
what the display supports it doesn't make sense for DRM to block
an HDR_OUTPUT_METADATA if it contains an EOTF the kernel doesn't
understand.

This comes with the added benefit of future-proofing metadata
support. If the spec defines a new EOTF there is no need to
update DRM and an compositor can immediately make use of it.

Fixes: https://gitlab.freedesktop.org/wayland/weston/-/issues/609

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
Cc: ppaalanen@gmail.com
Cc: sebastian.wick@redhat.com
Cc: vprosyak@amd.com
Cc: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 12893e7be89b..223f96a72064 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5691,10 +5691,8 @@ drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
 
 	/* Sink EOTF is Bit map while infoframe is absolute values */
 	if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf,
-	    connector->hdr_sink_metadata.hdmi_type1.eotf)) {
-		DRM_DEBUG_KMS("EOTF Not Supported\n");
-		return -EINVAL;
-	}
+	    connector->hdr_sink_metadata.hdmi_type1.eotf))
+		DRM_DEBUG_KMS("Unknown EOTF %d\n", hdr_metadata->hdmi_metadata_type1.eotf);
 
 	err = hdmi_drm_infoframe_init(frame);
 	if (err < 0)
-- 
2.36.1

[PATCH] drm: Don't block HDR_OUTPUT_METADATA on unknown EOTF

[Harry Wentland]

The supported EOTFs are defined in eotf_supported in drm_edid
but userspace has no way of knowing what is and isn't supported
when creating an HDR_OUTPUT_METADATA and will only know
something is wrong when the atomic commit fails.

Since it is expected that userspace reads the EDID to understand
what the display supports it doesn't make sense for DRM to block
an HDR_OUTPUT_METADATA if it contains an EOTF the kernel doesn't
understand.

This comes with the added benefit of future-proofing metadata
support. If the spec defines a new EOTF there is no need to
update DRM and an compositor can immediately make use of it.

Fixes: https://gitlab.freedesktop.org/wayland/weston/-/issues/609

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
Cc: ppaalanen@gmail.com
Cc: sebastian.wick@redhat.com
Cc: vprosyak@amd.com
Cc: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 12893e7be89b..223f96a72064 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5691,10 +5691,8 @@ drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
 
 	/* Sink EOTF is Bit map while infoframe is absolute values */
 	if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf,
-	    connector->hdr_sink_metadata.hdmi_type1.eotf)) {
-		DRM_DEBUG_KMS("EOTF Not Supported\n");
-		return -EINVAL;
-	}
+	    connector->hdr_sink_metadata.hdmi_type1.eotf))
+		DRM_DEBUG_KMS("Unknown EOTF %d\n", hdr_metadata->hdmi_metadata_type1.eotf);
 
 	err = hdmi_drm_infoframe_init(frame);
 	if (err < 0)
-- 
2.36.1

Re: [PATCH] drm: Don't block HDR_OUTPUT_METADATA on unknown EOTF

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 2118 bytes --]

On Tue, 24 May 2022 14:33:20 -0400
Harry Wentland <harry.wentland@amd.com> wrote:

> The supported EOTFs are defined in eotf_supported in drm_edid
> but userspace has no way of knowing what is and isn't supported
> when creating an HDR_OUTPUT_METADATA and will only know
> something is wrong when the atomic commit fails.

Hi Harry,

maybe clarify here the two different sets of "supported": EDID data vs.
kernel code? I think the paragraph uses the same word for both meanings.

> 
> Since it is expected that userspace reads the EDID to understand
> what the display supports it doesn't make sense for DRM to block
> an HDR_OUTPUT_METADATA if it contains an EOTF the kernel doesn't
> understand.
> 
> This comes with the added benefit of future-proofing metadata
> support. If the spec defines a new EOTF there is no need to
> update DRM and an compositor can immediately make use of it.
> 
> Fixes: https://gitlab.freedesktop.org/wayland/weston/-/issues/609
> 
> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> Cc: ppaalanen@gmail.com
> Cc: sebastian.wick@redhat.com
> Cc: vprosyak@amd.com
> Cc: Uma Shankar <uma.shankar@intel.com>

Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>


Thanks,
pq

> ---
>  drivers/gpu/drm/drm_edid.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 12893e7be89b..223f96a72064 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -5691,10 +5691,8 @@ drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame,
>  
>  	/* Sink EOTF is Bit map while infoframe is absolute values */
>  	if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf,
> -	    connector->hdr_sink_metadata.hdmi_type1.eotf)) {
> -		DRM_DEBUG_KMS("EOTF Not Supported\n");
> -		return -EINVAL;
> -	}
> +	    connector->hdr_sink_metadata.hdmi_type1.eotf))
> +		DRM_DEBUG_KMS("Unknown EOTF %d\n", hdr_metadata->hdmi_metadata_type1.eotf);
>  
>  	err = hdmi_drm_infoframe_init(frame);
>  	if (err < 0)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]