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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>

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@xxxxxxxxxxxxxx

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)




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux