mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] drm/bridge: dw-hdmi: Sync comments with actual bus formats order
@ 2024-11-29 23:04 Cristian Ciocaltea
  2024-12-02 15:45 ` Neil Armstrong
  0 siblings, 1 reply; 4+ messages in thread
From: Cristian Ciocaltea @ 2024-11-29 23:04 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: kernel, dri-devel, linux-kernel

Commit d3d6b1bf85ae ("drm: bridge: dw_hdmi: fix preference of RGB modes
over YUV420") changed the order of the output bus formats, but missed to
update accordingly the affected comment blocks related to
dw_hdmi_bridge_atomic_get_output_bus_fmts().

Fix the misleading comments and a context related typo.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 996733ed2c004c83a989cb8da54d8b630d9f2c02..d76aede757175d7ad5873c5d7623abf2d12da73c 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2621,6 +2621,7 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
  * - MEDIA_BUS_FMT_UYYVYY12_0_5X36,
  * - MEDIA_BUS_FMT_UYYVYY10_0_5X30,
  * - MEDIA_BUS_FMT_UYYVYY8_0_5X24,
+ * - MEDIA_BUS_FMT_RGB888_1X24,
  * - MEDIA_BUS_FMT_YUV16_1X48,
  * - MEDIA_BUS_FMT_RGB161616_1X48,
  * - MEDIA_BUS_FMT_UYVY12_1X24,
@@ -2631,7 +2632,6 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
  * - MEDIA_BUS_FMT_RGB101010_1X30,
  * - MEDIA_BUS_FMT_UYVY8_1X16,
  * - MEDIA_BUS_FMT_YUV8_1X24,
- * - MEDIA_BUS_FMT_RGB888_1X24,
  */
 
 /* Can return a maximum of 11 possible output formats for a mode/connector */
@@ -2669,7 +2669,7 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
 	}
 
 	/*
-	 * If the current mode enforces 4:2:0, force the output but format
+	 * If the current mode enforces 4:2:0, force the output bus format
 	 * to 4:2:0 and do not add the YUV422/444/RGB formats
 	 */
 	if (conn->ycbcr_420_allowed &&
@@ -2698,14 +2698,14 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
 		}
 	}
 
+	/* Default 8bit RGB fallback */
+	output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+
 	/*
 	 * Order bus formats from 16bit to 8bit and from YUV422 to RGB
-	 * if supported. In any case the default RGB888 format is added
+	 * if supported.
 	 */
 
-	/* Default 8bit RGB fallback */
-	output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
-
 	if (max_bpc >= 16 && info->bpc == 16) {
 		if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
 			output_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;

---
base-commit: f486c8aa16b8172f63bddc70116a0c897a7f3f02
change-id: 20241130-dw-hdmi-bus-fmt-order-7f6db5db2f0a


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/bridge: dw-hdmi: Sync comments with actual bus formats order
  2024-11-29 23:04 [PATCH] drm/bridge: dw-hdmi: Sync comments with actual bus formats order Cristian Ciocaltea
@ 2024-12-02 15:45 ` Neil Armstrong
  2024-12-02 17:32   ` Cristian Ciocaltea
  0 siblings, 1 reply; 4+ messages in thread
From: Neil Armstrong @ 2024-12-02 15:45 UTC (permalink / raw)
  To: Cristian Ciocaltea, Andrzej Hajda, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: kernel, dri-devel, linux-kernel

Hi,

On 30/11/2024 00:04, Cristian Ciocaltea wrote:
> Commit d3d6b1bf85ae ("drm: bridge: dw_hdmi: fix preference of RGB modes
> over YUV420") changed the order of the output bus formats, but missed to
> update accordingly the affected comment blocks related to
> dw_hdmi_bridge_atomic_get_output_bus_fmts().
> 
> Fix the misleading comments and a context related typo.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 996733ed2c004c83a989cb8da54d8b630d9f2c02..d76aede757175d7ad5873c5d7623abf2d12da73c 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2621,6 +2621,7 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
>    * - MEDIA_BUS_FMT_UYYVYY12_0_5X36,
>    * - MEDIA_BUS_FMT_UYYVYY10_0_5X30,
>    * - MEDIA_BUS_FMT_UYYVYY8_0_5X24,
> + * - MEDIA_BUS_FMT_RGB888_1X24,
>    * - MEDIA_BUS_FMT_YUV16_1X48,
>    * - MEDIA_BUS_FMT_RGB161616_1X48,
>    * - MEDIA_BUS_FMT_UYVY12_1X24,
> @@ -2631,7 +2632,6 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
>    * - MEDIA_BUS_FMT_RGB101010_1X30,
>    * - MEDIA_BUS_FMT_UYVY8_1X16,
>    * - MEDIA_BUS_FMT_YUV8_1X24,
> - * - MEDIA_BUS_FMT_RGB888_1X24,
>    */
>   
>   /* Can return a maximum of 11 possible output formats for a mode/connector */
> @@ -2669,7 +2669,7 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>   	}
>   
>   	/*
> -	 * If the current mode enforces 4:2:0, force the output but format
> +	 * If the current mode enforces 4:2:0, force the output bus format
>   	 * to 4:2:0 and do not add the YUV422/444/RGB formats
>   	 */
>   	if (conn->ycbcr_420_allowed &&
> @@ -2698,14 +2698,14 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>   		}
>   	}
>   
> +	/* Default 8bit RGB fallback */
> +	output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;

Why did you move this ? the following comment mentions it

> +
>   	/*
>   	 * Order bus formats from 16bit to 8bit and from YUV422 to RGB
> -	 * if supported. In any case the default RGB888 format is added
> +	 * if supported.
>   	 */
>   
> -	/* Default 8bit RGB fallback */
> -	output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> -
>   	if (max_bpc >= 16 && info->bpc == 16) {
>   		if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
>   			output_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
> 
> ---
> base-commit: f486c8aa16b8172f63bddc70116a0c897a7f3f02
> change-id: 20241130-dw-hdmi-bus-fmt-order-7f6db5db2f0a
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/bridge: dw-hdmi: Sync comments with actual bus formats order
  2024-12-02 15:45 ` Neil Armstrong
@ 2024-12-02 17:32   ` Cristian Ciocaltea
  2024-12-23 16:30     ` Cristian Ciocaltea
  0 siblings, 1 reply; 4+ messages in thread
From: Cristian Ciocaltea @ 2024-12-02 17:32 UTC (permalink / raw)
  To: neil.armstrong, Andrzej Hajda, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: kernel, dri-devel, linux-kernel

On 12/2/24 5:45 PM, Neil Armstrong wrote:
> Hi,
> 
> On 30/11/2024 00:04, Cristian Ciocaltea wrote:
>> Commit d3d6b1bf85ae ("drm: bridge: dw_hdmi: fix preference of RGB modes
>> over YUV420") changed the order of the output bus formats, but missed to
>> update accordingly the affected comment blocks related to
>> dw_hdmi_bridge_atomic_get_output_bus_fmts().
>>
>> Fix the misleading comments and a context related typo.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 12 ++++++------
>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/
>> drm/bridge/synopsys/dw-hdmi.c
>> index
>> 996733ed2c004c83a989cb8da54d8b630d9f2c02..d76aede757175d7ad5873c5d7623abf2d12da73c 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -2621,6 +2621,7 @@ static int dw_hdmi_connector_create(struct
>> dw_hdmi *hdmi)
>>    * - MEDIA_BUS_FMT_UYYVYY12_0_5X36,
>>    * - MEDIA_BUS_FMT_UYYVYY10_0_5X30,
>>    * - MEDIA_BUS_FMT_UYYVYY8_0_5X24,
>> + * - MEDIA_BUS_FMT_RGB888_1X24,
>>    * - MEDIA_BUS_FMT_YUV16_1X48,
>>    * - MEDIA_BUS_FMT_RGB161616_1X48,
>>    * - MEDIA_BUS_FMT_UYVY12_1X24,
>> @@ -2631,7 +2632,6 @@ static int dw_hdmi_connector_create(struct
>> dw_hdmi *hdmi)
>>    * - MEDIA_BUS_FMT_RGB101010_1X30,
>>    * - MEDIA_BUS_FMT_UYVY8_1X16,
>>    * - MEDIA_BUS_FMT_YUV8_1X24,
>> - * - MEDIA_BUS_FMT_RGB888_1X24,
>>    */
>>     /* Can return a maximum of 11 possible output formats for a mode/
>> connector */
>> @@ -2669,7 +2669,7 @@ static u32
>> *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>>       }
>>         /*
>> -     * If the current mode enforces 4:2:0, force the output but format
>> +     * If the current mode enforces 4:2:0, force the output bus format
>>        * to 4:2:0 and do not add the YUV422/444/RGB formats
>>        */
>>       if (conn->ycbcr_420_allowed &&
>> @@ -2698,14 +2698,14 @@ static u32
>> *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>>           }
>>       }
>>   +    /* Default 8bit RGB fallback */
>> +    output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> 
> Why did you move this ? the following comment mentions it

Before d3d6b1bf85ae ("drm: bridge: dw_hdmi: fix preference of RGB modes
over YUV420"), this was the last format added to the list.  Hence I
interpreted the comment below as referring to this particular last entry
as a fallback, which is not the case anymore.

If you still prefer to keep the original comment, then maybe we should
simply drop the "Default 8bit RGB fallback" one, as it's pretty
redundant now.

Thanks,
Cristian

>> +
>>       /*
>>        * Order bus formats from 16bit to 8bit and from YUV422 to RGB
>> -     * if supported. In any case the default RGB888 format is added
>> +     * if supported.
>>        */
>>   -    /* Default 8bit RGB fallback */
>> -    output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
>> -
>>       if (max_bpc >= 16 && info->bpc == 16) {
>>           if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
>>               output_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
>>
>> ---
>> base-commit: f486c8aa16b8172f63bddc70116a0c897a7f3f02
>> change-id: 20241130-dw-hdmi-bus-fmt-order-7f6db5db2f0a
>>
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/bridge: dw-hdmi: Sync comments with actual bus formats order
  2024-12-02 17:32   ` Cristian Ciocaltea
@ 2024-12-23 16:30     ` Cristian Ciocaltea
  0 siblings, 0 replies; 4+ messages in thread
From: Cristian Ciocaltea @ 2024-12-23 16:30 UTC (permalink / raw)
  To: neil.armstrong, Andrzej Hajda, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: kernel, dri-devel, linux-kernel

Hi Neil,

On 12/2/24 7:32 PM, Cristian Ciocaltea wrote:
> On 12/2/24 5:45 PM, Neil Armstrong wrote:
>> Hi,
>>
>> On 30/11/2024 00:04, Cristian Ciocaltea wrote:
>>> Commit d3d6b1bf85ae ("drm: bridge: dw_hdmi: fix preference of RGB modes
>>> over YUV420") changed the order of the output bus formats, but missed to
>>> update accordingly the affected comment blocks related to
>>> dw_hdmi_bridge_atomic_get_output_bus_fmts().
>>>
>>> Fix the misleading comments and a context related typo.
>>>
>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>> ---
>>>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 12 ++++++------
>>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/
>>> drm/bridge/synopsys/dw-hdmi.c
>>> index
>>> 996733ed2c004c83a989cb8da54d8b630d9f2c02..d76aede757175d7ad5873c5d7623abf2d12da73c 100644
>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> @@ -2621,6 +2621,7 @@ static int dw_hdmi_connector_create(struct
>>> dw_hdmi *hdmi)
>>>    * - MEDIA_BUS_FMT_UYYVYY12_0_5X36,
>>>    * - MEDIA_BUS_FMT_UYYVYY10_0_5X30,
>>>    * - MEDIA_BUS_FMT_UYYVYY8_0_5X24,
>>> + * - MEDIA_BUS_FMT_RGB888_1X24,
>>>    * - MEDIA_BUS_FMT_YUV16_1X48,
>>>    * - MEDIA_BUS_FMT_RGB161616_1X48,
>>>    * - MEDIA_BUS_FMT_UYVY12_1X24,
>>> @@ -2631,7 +2632,6 @@ static int dw_hdmi_connector_create(struct
>>> dw_hdmi *hdmi)
>>>    * - MEDIA_BUS_FMT_RGB101010_1X30,
>>>    * - MEDIA_BUS_FMT_UYVY8_1X16,
>>>    * - MEDIA_BUS_FMT_YUV8_1X24,
>>> - * - MEDIA_BUS_FMT_RGB888_1X24,
>>>    */
>>>     /* Can return a maximum of 11 possible output formats for a mode/
>>> connector */
>>> @@ -2669,7 +2669,7 @@ static u32
>>> *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>>>       }
>>>         /*
>>> -     * If the current mode enforces 4:2:0, force the output but format
>>> +     * If the current mode enforces 4:2:0, force the output bus format
>>>        * to 4:2:0 and do not add the YUV422/444/RGB formats
>>>        */
>>>       if (conn->ycbcr_420_allowed &&
>>> @@ -2698,14 +2698,14 @@ static u32
>>> *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>>>           }
>>>       }
>>>   +    /* Default 8bit RGB fallback */
>>> +    output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
>>
>> Why did you move this ? the following comment mentions it
> 
> Before d3d6b1bf85ae ("drm: bridge: dw_hdmi: fix preference of RGB modes
> over YUV420"), this was the last format added to the list.  Hence I
> interpreted the comment below as referring to this particular last entry
> as a fallback, which is not the case anymore.
> 
> If you still prefer to keep the original comment, then maybe we should
> simply drop the "Default 8bit RGB fallback" one, as it's pretty
> redundant now.

Please let me know what is your preference here and I'll send a new
revision if necessary.

Thanks,
Cristian

>>> +
>>>       /*
>>>        * Order bus formats from 16bit to 8bit and from YUV422 to RGB
>>> -     * if supported. In any case the default RGB888 format is added
>>> +     * if supported.
>>>        */
>>>   -    /* Default 8bit RGB fallback */
>>> -    output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
>>> -
>>>       if (max_bpc >= 16 && info->bpc == 16) {
>>>           if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
>>>               output_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
>>>
>>> ---
>>> base-commit: f486c8aa16b8172f63bddc70116a0c897a7f3f02
>>> change-id: 20241130-dw-hdmi-bus-fmt-order-7f6db5db2f0a
>>>
>>
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-12-23 16:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-29 23:04 [PATCH] drm/bridge: dw-hdmi: Sync comments with actual bus formats order Cristian Ciocaltea
2024-12-02 15:45 ` Neil Armstrong
2024-12-02 17:32   ` Cristian Ciocaltea
2024-12-23 16:30     ` Cristian Ciocaltea

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®