Jocelyn Falempe <jfale...@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 <jfale...@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 <javi...@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
