[v3] drm/connector: hdmi: Allow using the YUV420 output format

[PATCH v3 06/15] drm/connector: hdmi: Factor out bpc and format computation logic

Posted by Cristian Ciocaltea 8 months, 4 weeks ago

In preparation to support fallback to an alternative output format, e.g.
YUV420, when RGB cannot be used for any of the available color depths,
move the bpc try loop out of hdmi_compute_config() and, instead, make it
part of hdmi_compute_format_bpc().  Additionally, add a new parameter to
the latter holding the output format to be checked and eventually set.

This improves code reusability and further extensibility.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/display/drm_hdmi_state_helper.c | 50 ++++++++++++-------------
 1 file changed, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index 160964190d82ac233fdbe34ac54024a007a19872..6de0abb15ecb36fd4eb98725e2a3835e5e0db134 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -608,42 +608,19 @@ static int
 hdmi_compute_format_bpc(const struct drm_connector *connector,
 			struct drm_connector_state *conn_state,
 			const struct drm_display_mode *mode,
-			unsigned int bpc)
+			unsigned int max_bpc, enum hdmi_colorspace fmt)
 {
 	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)
-{
-	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) {
-		ret = hdmi_compute_format_bpc(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;
 
 		drm_dbg_kms(dev,
 			    "Mode %ux%u @ %uHz: Found configuration: bpc: %u, fmt: %s, clock: %llu\n",
@@ -655,9 +632,28 @@ hdmi_compute_config(const struct drm_connector *connector,
 		return 0;
 	}
 
+	drm_dbg_kms(dev, "Failed. %s output format not supported for any bpc count.\n",
+		    drm_hdmi_connector_get_output_format_name(fmt));
+
 	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_compute_format_bpc(connector, conn_state, mode, max_bpc,
+				      HDMI_COLORSPACE_RGB);
+
+	return ret;
+}
+
 static int hdmi_generate_avi_infoframe(const struct drm_connector *connector,
 				       struct drm_connector_state *conn_state)
 {

-- 
2.49.0

Re: [PATCH v3 06/15] drm/connector: hdmi: Factor out bpc and format computation logic

Posted by Maxime Ripard 8 months, 1 week ago

Hi,

On Wed, Mar 26, 2025 at 12:19:55PM +0200, Cristian Ciocaltea wrote:
> In preparation to support fallback to an alternative output format, e.g.
> YUV420, when RGB cannot be used for any of the available color depths,
> move the bpc try loop out of hdmi_compute_config() and, instead, make it
> part of hdmi_compute_format_bpc().  Additionally, add a new parameter to
> the latter holding the output format to be checked and eventually set.
>
> This improves code reusability and further extensibility.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

I think patch 5 could be squashed into this one.

> ---
>  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 50 ++++++++++++-------------
>  1 file changed, 23 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index 160964190d82ac233fdbe34ac54024a007a19872..6de0abb15ecb36fd4eb98725e2a3835e5e0db134 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> @@ -608,42 +608,19 @@ static int
>  hdmi_compute_format_bpc(const struct drm_connector *connector,
>  			struct drm_connector_state *conn_state,
>  			const struct drm_display_mode *mode,
> -			unsigned int bpc)
> +			unsigned int max_bpc, enum hdmi_colorspace fmt)
>  {
>  	struct drm_device *dev = connector->dev;
> -
> -	/*
> -	 * TODO: Add support for YCbCr420 output for HDMI 2.0 capable
> -	 * devices, for modes that only support YCbCr420.
> -	 */

And we should fix that comment for now.

Once fixed,
Reviewed-by: Maxime Ripard <mripard@kernel.org>

Maxime

Re: [PATCH v3 06/15] drm/connector: hdmi: Factor out bpc and format computation logic

Posted by Cristian Ciocaltea 8 months, 1 week ago

Hi Maxime,

On 4/9/25 6:02 PM, Maxime Ripard wrote:
> Hi,
> 
> On Wed, Mar 26, 2025 at 12:19:55PM +0200, Cristian Ciocaltea wrote:
>> In preparation to support fallback to an alternative output format, e.g.
>> YUV420, when RGB cannot be used for any of the available color depths,
>> move the bpc try loop out of hdmi_compute_config() and, instead, make it
>> part of hdmi_compute_format_bpc().  Additionally, add a new parameter to
>> the latter holding the output format to be checked and eventually set.
>>
>> This improves code reusability and further extensibility.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> 
> I think patch 5 could be squashed into this one.

Ack.

>> ---
>>  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 50 ++++++++++++-------------
>>  1 file changed, 23 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>> index 160964190d82ac233fdbe34ac54024a007a19872..6de0abb15ecb36fd4eb98725e2a3835e5e0db134 100644
>> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>> @@ -608,42 +608,19 @@ static int
>>  hdmi_compute_format_bpc(const struct drm_connector *connector,
>>  			struct drm_connector_state *conn_state,
>>  			const struct drm_display_mode *mode,
>> -			unsigned int bpc)
>> +			unsigned int max_bpc, enum hdmi_colorspace fmt)
>>  {
>>  	struct drm_device *dev = connector->dev;
>> -
>> -	/*
>> -	 * TODO: Add support for YCbCr420 output for HDMI 2.0 capable
>> -	 * devices, for modes that only support YCbCr420.
>> -	 */
> 
> And we should fix that comment for now.

Sorry, I missed to move this hunk to the next patch.

> 
> Once fixed,
> Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks,
Cristian