Re: [PATCH v3 04/81] drm: Add client-agnostic setup helper

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 30.08.24 um 11:45 schrieb Geert Uytterhoeven:
>     Hi Thomas,
>
> On Fri, 30 Aug 2024, Thomas Zimmermann wrote:
>> DRM may support multiple in-kernel clients that run as soon as a DRM
>> driver has been registered. To select the client(s) in a single place,
>> introduce drm_client_setup().
>>
>> Drivers that call the new helper automatically instantiate the kernel's
>> configured default clients. Only fbdev emulation is currently supported.
>> Later versions can add support for DRM-based logging, a boot logo or 
>> even
>> a console.
>>
>> Some drivers handle the color mode for clients internally. Provide the
>> helper drm_client_setup_with_color_mode() for them.
>>
>> v3:
>> - fix build error
>> v2:
>> - add drm_client_setup_with_fourcc() (Laurent)
>> - push default-format handling into actual clients
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Reviewed-by: Laurent Pinchart 
>> <laurent.pinchart+renesas@ideasonboard.com>
>
> Thanks for your patch!
>
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_client_setup.c
>
>> +/**
>> + * drm_client_setup_with_fourcc() - Setup in-kernel DRM clients for 
>> color mode
>> + * @dev: DRM device
>> + * @fourcc: Preferred pixel format as 4CC code for the device
>> + *
>> + * This function sets up the in-kernel DRM clients. It is equivalent
>> + * to drm_client_setup(), but expects a 4CC code as second argument.
>> + *
>> + * Do not use this function in new drivers. Prefer 
>> drm_client_setup() with a
>> + * format of NULL.
>
> Why? To me this looks like the right function to call on hardware
> that does not support ARGB8888 natively.

Sorry, that needs to be fixed. the _fourcc() helper is ok-ish.

Ideally, the client would select the format automatically. It could also 
look at the preferred_depth in struct drm_mode_config. But some drivers 
still want a different format for fbdev emulation. See the _fourcc() 
helper as a fallback for this scenario.

>
> BTW, once this series is applied, I plan to check again how to wire up
> native fbcon support for monochrome (DRM_FORMAT_R1) and grayscale
> (DRM_FORMAT_R8), as used by the Solomon driver.

The internals of fbdev emulation still use a color-mode value (see 
drm_fbdev_client_setup()). This would require fixing first. It's 
probably not hard.

I know that you've been waiting for the format parameter for some time. 
We're getting there. :)

Best regards
Thomas

>
>> + */
>> +void drm_client_setup_with_fourcc(struct drm_device *dev, u32 fourcc)
>> +{
>> +    drm_client_setup(dev, drm_format_info(fourcc));
>> +}
>> +EXPORT_SYMBOL(drm_client_setup_with_fourcc);
>> +
>> +/**
>> + * drm_client_setup_with_color_mode() - Setup in-kernel DRM clients 
>> for color mode
>> + * @dev: DRM device
>> + * @color_mode: Preferred color mode for the device
>> + *
>> + * This function sets up the in-kernel DRM clients. It is equivalent
>> + * to drm_client_setup(), but expects a color mode as second argument.
>> + *
>> + * Do not use this function in new drivers. Prefer 
>> drm_client_setup() with a
>
>
> Yeah, this is definitely not to be used in new drivers, as color_mode is
> ambiguous.
>
>> + * format of NULL.
>
> or drm_client_setup_with_fourcc().
>
>> + */
>> +void drm_client_setup_with_color_mode(struct drm_device *dev, 
>> unsigned int color_mode)
>> +{
>> +    u32 fourcc = drm_driver_color_mode_format(dev, color_mode);
>> +
>> +    drm_client_setup_with_fourcc(dev, fourcc);
>> +}
>> +EXPORT_SYMBOL(drm_client_setup_with_color_mode);
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a 
> hacker. But
> when I'm talking to journalists I just say "programmer" or something 
> like that.
>                                 -- Linus Torvalds

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)