linux-kernel - Re: [PATCH 3/3] drm/connector: hdmi: Use YUV420 output format as an RGB fallback

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241202-tangible-ibis-from-sirius-464b60@houat>
Date: Mon, 2 Dec 2024 12:11:41 +0100
From: Maxime Ripard <mripard@...nel.org>
To: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, kernel@...labora.com, dri-devel@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] drm/connector: hdmi: Use YUV420 output format as an
 RGB fallback

On Sat, Nov 30, 2024 at 01:56:34AM +0200, Cristian Ciocaltea wrote:
> Introduce the switch to YUV420 when computing the best output format and
> RGB cannot be supported for a given color depth.
> 
> While at it, add a minor improvement to the debug message indicating the
> supported format.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
> ---
>  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index 3a55881a544a519bb1254968db891c814f831a0f..b4e865e0680f35fd2d849536789f6c1f98a48258 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> @@ -304,7 +304,7 @@ hdmi_try_format_bpc(const struct drm_connector *connector,
>  		return false;
>  	}
>  
> -	drm_dbg_kms(dev, "%s output format supported with %u (TMDS char rate: %llu Hz)\n",
> +	drm_dbg_kms(dev, "%s output format supported with %u bpc (TMDS char rate: %llu Hz)\n",
>  		    drm_hdmi_connector_get_output_format_name(fmt),
>  		    bpc, conn_state->hdmi.tmds_char_rate);
>  
> @@ -319,15 +319,16 @@ hdmi_compute_format(const struct drm_connector *connector,
>  {
>  	struct drm_device *dev = connector->dev;
>  
> -	/*
> -	 * TODO: Add support for YCbCr420 output for HDMI 2.0 capable
> -	 * devices, for modes that only support YCbCr420.
> -	 */

It's something that I had in the back of my mind for a while, but we're
at the point where we need to discuss this I guess :)

Not all HDMI controllers are HDMI2.0+ compliant, and we need to gatekeep
this to the fact the controller supports it.

This will also be useful for things like scrambling support. And
probably to provide some TMDS rate check based on the standard a given
controller supports, since most of the drivers have that check
duplicated everywhere.

I don't really have an opinion on how to do this, so I guess it's really
up for debate. The alternatives I could think of are either to add a new
parameter to the init function, or to create a new callback to query the
driver for its capabilities.

The former doesn't seem great since the parameters set is pretty
extensive already. The latter doesn't seem super idiomatic in KMS, but
it's a common pattern in the rest of the kernel, so maybe it's a good
idea still.

>  	if (hdmi_try_format_bpc(connector, conn_state, mode, bpc, HDMI_COLORSPACE_RGB)) {
>  		conn_state->hdmi.output_format = HDMI_COLORSPACE_RGB;
>  		return 0;
>  	}
>  
> +	if (hdmi_try_format_bpc(connector, conn_state, mode, bpc, HDMI_COLORSPACE_YUV420)) {
> +		conn_state->hdmi.output_format = HDMI_COLORSPACE_YUV420;
> +		return 0;
> +	}
> +

During our discussions when we merged this infrastructure, the goal was
to align our behaviour to Intel's. The discussion also pointed out that
we want to degrade the bpc before falling back to a YUV format.

So we need to first try RGB with any bpc, and then try YUV with any BPC
if it didn't work.

We also need plenty of tests based on whether the source supports
YUV420, the sink has YUV420-only modes, that the fallback occurs
properly, etc.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)