linux-kernel - Re: [PATCH] drm/bridge: dw-hdmi: Sync comments with actual bus formats order

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a215731f-7edd-4662-9c67-420c4e60b039@collabora.com>
Date: Mon, 23 Dec 2024 18:30:45 +0200
From: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
To: neil.armstrong@...aro.org, Andrzej Hajda <andrzej.hajda@...el.com>,
 Robert Foss <rfoss@...nel.org>,
 Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
 Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>
Cc: kernel@...labora.com, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/bridge: dw-hdmi: Sync comments with actual bus
 formats order

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@...labora.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
>>>
>>
>