Re: [PATCH 1/1] drm/fourcc: Add documentation about software color conversion.

[Javier Martinez Canillas <javierm@redhat.com>]

Jocelyn Falempe <jfalempe@redhat.com> writes:

Hello Jocelyn,

> On 10/08/2023 09:45, Maxime Ripard wrote:
>> Hi
>> 
>> On Mon, Aug 07, 2023 at 03:45:15PM +0200, Jocelyn Falempe wrote:
>>> After discussions on IRC, the consensus is that the DRM drivers should
>>> not do software color conversion, and only advertise the supported formats.
>>> Update the doc accordingly so that the rule and exceptions are clear for
>>> everyone.
>>>
>>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>>> ---
>>>   include/uapi/drm/drm_fourcc.h | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>>> index 8db7fd3f743e..00a29152da9f 100644
>>> --- a/include/uapi/drm/drm_fourcc.h
>>> +++ b/include/uapi/drm/drm_fourcc.h
>>> @@ -38,6 +38,13 @@ extern "C" {
>>>    * fourcc code, a Format Modifier may optionally be provided, in order to
>>>    * further describe the buffer's format - for example tiling or compression.
>>>    *
>>> + * DRM drivers should not do software color conversion, and only advertise the
>>> + * format they support in hardware. But there are two exceptions:
>> 
>> I would do a bullet list here:
>> https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#lists-and-quote-like-blocks
>> 
> ok, that would look better.
>
>>> + * The first is to support XRGB8888 if the hardware doesn't support it, because
>>> + * it's the de facto standard for userspace applications.
>> 
>> We can also provide a bit more context here, something like:
>> 
>> All drivers must support XRGB8888, even if the hardware cannot support
>> it. This has become the de-facto standard and a lot of user-space assume
>> it will be present.
>
> ok, I can add this before the first paragraph.
>>

Agreed with Maxime's suggestion but I would also mention that if XRGB8888
is not supported, the native formats should be the default and XRGB8888 be
only used as a fallback for user-space compatiblity. In other words, that
XRGB8888 must not be the default, e.g: mode_config.preferred_depth = 24 or
drm_fbdev_generic_setup(drm, 32) only if is a supported native format.

I agree with the general content of the patch, so if you post a v2 feel
free to add my

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat