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

["Noralf Trønnes" <noralf@tronnes.org>]

Den 17.08.2021 15.49, skrev Daniel Vetter:
> On Tue, Aug 17, 2021 at 02:29:17PM +0200, Noralf Trønnes wrote:
>> 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).
>>
>> Signed-off-by: Noralf Trønnes <noralf@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;
> 
> Shouldn't the emulation format be per drm_device instance?

Ideally yes, this happens during probe so I can only use a module
parameter and I don't know how to differenciate the devices since the
DRM minor is unknown at this point and implementing filtering on the
various USB properties (VID:PID, serial number,..) will involve a lot of
code. So I've kept it simple. It can be expanded on later should someone
come up with a clever idea.

Noralf.

> -Daniel
> 
>>  			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) {
>> -- 
>> 2.32.0
>>
>