Re: [PATCH v5 06/17] drm/display: hdmi-state-helper: Try subsampling in mode_valid

[Maxime Ripard <mripard@kernel.org>]

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

On Fri, Nov 28, 2025 at 10:05:42PM +0100, Nicolas Frattaroli wrote:
> drm_hdmi_connector_mode_valid assumes modes are only valid if they work
> with RGB. The reality is more complex however: YCbCr 4:2:0
> chroma-subsampled modes only require half the pixel clock that the same
> mode would require in RGB.
> 
> This leads to drm_hdmi_connector_mode_valid rejecting perfectly valid
> 420-only modes.
> 
> Fix this by checking whether the mode is 420-only first. If so, then
> proceed by checking it with HDMI_COLORSPACE_YUV420 so long as the
> connector has legalized 420, otherwise error out. If the mode is not
> 420-only, check with RGB as was previously always the case.
> 
> Fixes: 47368ab437fd ("drm/display: hdmi: add generic mode_valid helper")
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index 5da956bdd68c..1800e00b30c5 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> @@ -892,8 +892,18 @@ drm_hdmi_connector_mode_valid(struct drm_connector *connector,
>  			      const struct drm_display_mode *mode)
>  {
>  	unsigned long long clock;
> +	enum hdmi_colorspace fmt;
> +
> +	if (drm_mode_is_420_only(&connector->display_info, mode)) {
> +		if (connector->ycbcr_420_allowed)
> +			fmt = HDMI_COLORSPACE_YUV420;
> +		else
> +			return MODE_NO_420;
> +	} else {
> +		fmt = HDMI_COLORSPACE_RGB;
> +	}
>  
> -	clock = drm_hdmi_compute_mode_clock(mode, 8, HDMI_COLORSPACE_RGB);
> +	clock = drm_hdmi_compute_mode_clock(mode, 8, fmt);

I agree on principle, but we need to have a test for this.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]