[PATCH v2 7/7] drm/gud: Add module parameter to control emulation: xrgb8888

[Noralf Trønnes]

Den 08.09.2021 20.10, skrev Thomas Zimmermann:
> Hi
> 
> Am 07.09.21 um 13:57 schrieb Noralf Trønnes:
>> For devices that don't support XRGB8888 give the user the ability to
>> choose what's most important: Color depth or frames per second.
>>
>> Add an 'xrgb8888' module parameter to override the emulation format.
>>
>> Assume the user wants full control if xrgb8888 is set and don't set
>> DRM_CAP_DUMB_PREFERRED_DEPTH if RGB565 is supported (AFAIK only X.org
>> supports this).
> 
> More of a general statement: wouldn't it make more sense to auto-detect
> this entirely? The GUD protocol could order the list of supported
> formats by preference (maybe it does already). Or you could take the
> type of USB connection into account.
> 

There is preference in that the driver will use xrgb8888 emulation on
the first format returned by the device. Yeah USB speed could be used as
a metric for preferred depth, but I don't know of anyone else than X.org
using it so I'm unsure about the value.

Daniel mentioned that this parameter targets all devices, so it's not a
very good solution.

Maybe I should drop this patch and gather more user experience with the
depth/speed challenge before trying to come up with a solution.

> Additionally, xrgb8888 is really a fall-back for lazy userspace
> programs, but userspace should do better IMHO.
> 

I don't think even the most agile userspace supports monochrome,
greyscale or even RGB332 ;)

Noralf.

> Best regards
> Thomas
> 
>>
>> Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
>> ---
>>   drivers/gpu/drm/gud/gud_drv.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/gud/gud_drv.c
>> b/drivers/gpu/drm/gud/gud_drv.c
>> index 3f9d4b9a1e3d..60d27ee5ddbd 100644
>> --- a/drivers/gpu/drm/gud/gud_drv.c
>> +++ b/drivers/gpu/drm/gud/gud_drv.c
>> @@ -30,6 +30,10 @@
>>     #include "gud_internal.h"
>>   +static int gud_xrgb8888;
>> +module_param_named(xrgb8888, gud_xrgb8888, int, 0644);
>> +MODULE_PARM_DESC(xrgb8888, "XRGB8888 emulation format:
>> GUD_PIXEL_FORMAT_* value, 0=auto, -1=disable [default=auto]");
>> +
>>   /* Only used internally */
>>   static const struct drm_format_info gud_drm_format_r1 = {
>>       .format = GUD_DRM_FORMAT_R1,
>> @@ -530,12 +534,12 @@ static int gud_probe(struct usb_interface *intf,
>> const struct usb_device_id *id)
>>           case DRM_FORMAT_RGB332:
>>               fallthrough;
>>           case DRM_FORMAT_RGB888:
>> -            if (!xrgb8888_emulation_format)
>> +            if (!gud_xrgb8888 && !xrgb8888_emulation_format)
>>                   xrgb8888_emulation_format = info;
>>               break;
>>           case DRM_FORMAT_RGB565:
>>               rgb565_supported = true;
>> -            if (!xrgb8888_emulation_format)
>> +            if (!gud_xrgb8888 && !xrgb8888_emulation_format)
>>                   xrgb8888_emulation_format = info;
>>               break;
>>           case DRM_FORMAT_XRGB8888:
>> @@ -543,6 +547,9 @@ static int gud_probe(struct usb_interface *intf,
>> const struct usb_device_id *id)
>>               break;
>>           }
>>   +        if (gud_xrgb8888 == formats_dev[i])
>> +            xrgb8888_emulation_format = info;
>> +
>>           fmt_buf_size = drm_format_info_min_pitch(info, 0,
>> drm->mode_config.max_width) *
>>                      drm->mode_config.max_height;
>>           max_buffer_size = max(max_buffer_size, fmt_buf_size);
>> @@ -559,7 +566,7 @@ static int gud_probe(struct usb_interface *intf,
>> const struct usb_device_id *id)
>>       }
>>         /* Prefer speed over color depth */
>> -    if (rgb565_supported)
>> +    if (!gud_xrgb8888 && rgb565_supported)
>>           drm->mode_config.preferred_depth = 16;
>>         if (!xrgb8888_supported && xrgb8888_emulation_format) {
>>
>