Re: [PATCH v2 2/7] drm/connector: hdmi: Add support for YUV420 format verification

[Maxime Ripard <mripard@kernel.org>]

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

On Tue, Mar 11, 2025 at 12:57:34PM +0200, Cristian Ciocaltea wrote:
> Provide the necessary constraints verification in
> sink_supports_format_bpc() in order to support handling of YUV420
> output format.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 40 +++++++++++++++++++++++--
>  1 file changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index 6bc96d5d1ab9115989e208d9899e16cd22254fb6..e99d868edc1854eddc5ebf8692ccffb9e2338268 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> @@ -3,6 +3,7 @@
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_connector.h>
>  #include <drm/drm_edid.h>
> +#include <drm/drm_modes.h>
>  #include <drm/drm_print.h>
>  
>  #include <drm/display/drm_hdmi_audio_helper.h>
> @@ -115,6 +116,12 @@ sink_supports_format_bpc(const struct drm_connector *connector,
>  		return false;
>  	}
>  
> +	if (drm_mode_is_420_only(info, mode) && format != HDMI_COLORSPACE_YUV420) {
> +		drm_dbg_kms(dev, "%s format unsupported by the sink for VIC%u.\n",
> +			    drm_hdmi_connector_get_output_format_name(format), vic);

We don't necessarily have a VIC for the mode we pass, so it's not super
useful to pass it. I'd rather mention that the mode is supposed to be
YUV420 only, but the format isn't YUV420.

> +		return false;
> +	}
> +
>  	switch (format) {
>  	case HDMI_COLORSPACE_RGB:
>  		drm_dbg_kms(dev, "RGB Format, checking the constraints.\n");
> @@ -145,9 +152,36 @@ sink_supports_format_bpc(const struct drm_connector *connector,
>  		return true;
>  
>  	case HDMI_COLORSPACE_YUV420:
> -		/* TODO: YUV420 is unsupported at the moment. */
> -		drm_dbg_kms(dev, "YUV420 format isn't supported yet.\n");
> -		return false;
> +		drm_dbg_kms(dev, "YUV420 format, checking the constraints.\n");
> +
> +		if (!(info->color_formats & DRM_COLOR_FORMAT_YCBCR420)) {
> +			drm_dbg_kms(dev, "Sink doesn't support YUV420.\n");
> +			return false;
> +		}
> +
> +		if (!drm_mode_is_420(info, mode)) {
> +			drm_dbg_kms(dev, "Sink doesn't support YUV420 for VIC%u.\n", vic);

Again, we shouldn't print the VIC here. There's a printk format we can
use to print drm_display_mode if you want to, but we should keep things
consistent.

Maxime

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