| 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
| ||
|
Message-ID: <20250311-hypersonic-mature-leopard-d3afdc@houat>
Date: Tue, 11 Mar 2025 16:55:17 +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>, Dave Stevenson <dave.stevenson@...pberrypi.com>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, kernel@...labora.com, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/7] drm/connector: hdmi: Use YUV420 output format as
an RGB fallback
Hi,
I think the first thing we need to address is that we will need to
differentiate between HDMI 1.4 devices and HDMI 2.0.
It applies to YUV420, which is HDMI 2.0-only, and I guess your patches
are good enough if you consider YUV420 support only, but scrambler setup
for example is a thing we want to support in that infrastructure
eventually, and is conditioned on HDMI 2.0 as well.
On Tue, Mar 11, 2025 at 12:57:36PM +0200, Cristian Ciocaltea wrote:
> Try to make use of YUV420 when computing the best output format and
> RGB cannot be supported for any of the available color depths.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
> ---
> drivers/gpu/drm/display/drm_hdmi_state_helper.c | 69 +++++++++++++------------
> 1 file changed, 35 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index a70e204a8df3ac1c2d7318e81cde87a83267dd21..f2052781b797dd09b41127e33d98fe25408a9b23 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> @@ -287,8 +287,9 @@ hdmi_try_format_bpc(const struct drm_connector *connector,
> struct drm_device *dev = connector->dev;
> int ret;
>
> - drm_dbg_kms(dev, "Trying %s output format\n",
> - drm_hdmi_connector_get_output_format_name(fmt));
> + drm_dbg_kms(dev, "Trying %s output format with %u bpc\n",
> + drm_hdmi_connector_get_output_format_name(fmt),
> + bpc);
That part should be in a separate patch, it's independant of the rest.
> if (!sink_supports_format_bpc(connector, info, mode, fmt, bpc)) {
> drm_dbg_kms(dev, "%s output format not supported with %u bpc\n",
> @@ -313,47 +314,22 @@ hdmi_try_format_bpc(const struct drm_connector *connector,
> }
>
> static int
> -hdmi_compute_format(const struct drm_connector *connector,
> - struct drm_connector_state *conn_state,
> - const struct drm_display_mode *mode,
> - unsigned int bpc)
> -{
> - struct drm_device *dev = connector->dev;
> -
> - /*
> - * TODO: Add support for YCbCr420 output for HDMI 2.0 capable
> - * devices, for modes that only support YCbCr420.
> - */
> - if (hdmi_try_format_bpc(connector, conn_state, mode, bpc, HDMI_COLORSPACE_RGB)) {
> - conn_state->hdmi.output_format = HDMI_COLORSPACE_RGB;
> - return 0;
> - }
> -
> - drm_dbg_kms(dev, "Failed. No Format Supported for that bpc count.\n");
> -
> - return -EINVAL;
> -}
> -
> -static int
> -hdmi_compute_config(const struct drm_connector *connector,
> - struct drm_connector_state *conn_state,
> - const struct drm_display_mode *mode)
> +hdmi_try_format(const struct drm_connector *connector,
> + struct drm_connector_state *conn_state,
> + const struct drm_display_mode *mode,
> + unsigned int max_bpc, enum hdmi_colorspace fmt)
> {
> struct drm_device *dev = connector->dev;
> - unsigned int max_bpc = clamp_t(unsigned int,
> - conn_state->max_bpc,
> - 8, connector->max_bpc);
> unsigned int bpc;
> int ret;
>
> for (bpc = max_bpc; bpc >= 8; bpc -= 2) {
> - drm_dbg_kms(dev, "Trying with a %d bpc output\n", bpc);
> -
> - ret = hdmi_compute_format(connector, conn_state, mode, bpc);
> - if (ret)
> + ret = hdmi_try_format_bpc(connector, conn_state, mode, bpc, fmt);
> + if (!ret)
> continue;
>
> conn_state->hdmi.output_bpc = bpc;
> + conn_state->hdmi.output_format = fmt;
I guess it's a matter of semantics, but if it sets the value in the
state, it doesn't try. Maybe the function should be named
hdmi_compute_format_bpc then?
That renaming should be in a separate patch too (possibly several).
> drm_dbg_kms(dev,
> "Mode %ux%u @ %uHz: Found configuration: bpc: %u, fmt: %s, clock: %llu\n",
> @@ -368,6 +344,31 @@ hdmi_compute_config(const struct drm_connector *connector,
> return -EINVAL;
> }
>
> +static int
> +hdmi_compute_config(const struct drm_connector *connector,
> + struct drm_connector_state *conn_state,
> + const struct drm_display_mode *mode)
> +{
> + unsigned int max_bpc = clamp_t(unsigned int,
> + conn_state->max_bpc,
> + 8, connector->max_bpc);
> + int ret;
> +
> + ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
> + HDMI_COLORSPACE_RGB);
> + if (!ret)
> + return 0;
> +
> + if (connector->ycbcr_420_allowed)
> + ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
> + HDMI_COLORSPACE_YUV420);
I think that's conditioned on a few more things:
- That the driver supports HDMI 2.0
- That the display is an HDMI output
- That the mode is allowed YUV420 by the sink EDIDs
> + else
> + drm_dbg_kms(connector->dev,
> + "%s output format not allowed for connector\n",
> + drm_hdmi_connector_get_output_format_name(HDMI_COLORSPACE_YUV420));
And I think we should keep the catch-all failure message we had.
Maxime
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists