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 <tzimmermann@xxxxxxx>
> ---
> 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/
> + * 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.
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> +
> +#endif
--
Regards,
Laurent Pinchart
