Hi

Am 18.08.24 um 22:07 schrieb Laurent Pinchart:
Hi Thomas,

Thank you for the patch.

On Fri, Aug 16, 2024 at 02:22:30PM +0200, 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.
I really like the direction this is taking :-)

Some drivers handle the color mode for clients internally. Provide the
helper drm_client_setup_with_color_mode() for them.

Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
---
  drivers/gpu/drm/Makefile           |  1 +
  drivers/gpu/drm/drm_client_setup.c | 55 ++++++++++++++++++++++++++++++
  include/drm/drm_client_setup.h     | 12 +++++++
  3 files changed, 68 insertions(+)
  create mode 100644 drivers/gpu/drm/drm_client_setup.c
  create mode 100644 include/drm/drm_client_setup.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 0beb55d028a8..e7fc77d1d573 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -129,6 +129,7 @@ drm_kms_helper-y := \
        drm_atomic_helper.o \
        drm_atomic_state_helper.o \
        drm_bridge_connector.o \
+       drm_client_setup.o \
        drm_crtc_helper.o \
        drm_damage_helper.o \
        drm_encoder_slave.o \
diff --git a/drivers/gpu/drm/drm_client_setup.c 
b/drivers/gpu/drm/drm_client_setup.c
new file mode 100644
index 000000000000..2e3315f5c3e2
--- /dev/null
+++ b/drivers/gpu/drm/drm_client_setup.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: MIT
+
+#include <drm/drm_client_setup.h>
+#include <drm/drm_device.h>
+#include <drm/drm_fbdev_client.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_print.h>
+
+/**
+ * drm_client_setup() - Setup in-kernel DRM clients
+ * @dev: DRM device
+ * @format: Preferred color format for the device. Use NULL, unless
s/color format/pixel format/

Ah, ok.


+ *          there is clearly a driver-preferred format.
+ *
+ * This function sets up the in-kernel DRM clients. Restore, hotplug
+ * events and teardown are all taken care of.
+ *
+ * Drivers should call drm_client_setup() after registering the new
+ * DRM device with drm_dev_register(). This function is safe to call
+ * even when there are no connectors present. Setup will be retried
+ * on the next hotplug event.
+ *
+ * The clients are destroyed by drm_dev_unregister().
+ */
+void drm_client_setup(struct drm_device *dev, const struct drm_format_info 
*format)
+{
+       int ret;
+
+       if (!format)
+               format = drm_format_info(DRM_FORMAT_XRGB8888);
+
+       ret = drm_fbdev_client_setup(dev, format);
+       if (ret)
+               drm_warn(dev, "Failed to set up DRM client; error %d\n", ret);
+}
+EXPORT_SYMBOL(drm_client_setup);
+
+/**
+ * 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
+ * format of NULL.
+ */
+void drm_client_setup_with_color_mode(struct drm_device *dev, unsigned int 
color_mode)
+{
+       uint32_t fmt = drm_driver_color_mode_format(dev, color_mode);
+
+       drm_client_setup(dev, drm_format_info(fmt));
+}
+EXPORT_SYMBOL(drm_client_setup_with_color_mode);
diff --git a/include/drm/drm_client_setup.h b/include/drm/drm_client_setup.h
new file mode 100644
index 000000000000..f5fd1fabd4b1
--- /dev/null
+++ b/include/drm/drm_client_setup.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: MIT */
+
+#ifndef DRM_CLIENT_SETUP_H
+#define DRM_CLIENT_SETUP_H
+
+struct drm_device;
+struct drm_format_info;
+
+void drm_client_setup(struct drm_device *dev, const struct drm_format_info 
*format);
+void drm_client_setup_with_color_mode(struct drm_device *dev, unsigned int 
color_mode);
This is not common in DRM, so we may not want to introduce it here, but
_Generic() gives a nice function overloading syntax:

void drm_client_setup_with_format(struct drm_device *dev, const struct 
drm_format_info *format);
void drm_client_setup_with_color_mode(struct drm_device *dev, unsigned int 
color_mode);

#define drm_client_setup_(dev, format_or_color_mode)                            
                         \
        _Generic(format_or_color_mode,                                          
                         \
                const struct drm_format_info *: 
drm_client_setup_with_format(dev, format_or_color_mode), \
                unsigned int: drm_client_setup_with_color_mode(dev, 
format_or_color_mode)                \
        )

Up to you.

The color format is really a user interface. I'd rather like to remove it from the implementation where possible.

Best regards
Thomas


Reviewed-by: Laurent Pinchart <laurent.pinchart+rene...@ideasonboard.com>

+
+#endif

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

Reply via email to