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

From: Thomas Zimmermann <tzimmermann@suse.de>
To: "Noralf Trønnes" <noralf@tronnes.org>, dri-devel@lists.freedesktop.org
Cc: peter@stuge.se, linus.walleij@linaro.org
Subject: Re: [PATCH v2 7/7] drm/gud: Add module parameter to control emulation: xrgb8888
Date: Wed, 8 Sep 2021 20:10:12 +0200	[thread overview]
Message-ID: <3428e1f0-7586-a0e2-6334-b7362cdb75b4@suse.de> (raw)
In-Reply-To: <20210907115752.6181-8-noralf@tronnes.org>

[-- Attachment #1.1: Type: text/plain, Size: 3170 bytes --]

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.

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

Best regards
Thomas

> 
> 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;
>   			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) {
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]