diff mbox series

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

Message ID 20240830084456.77630-5-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series drm: Provide client setup helper and convert drivers | expand

Commit Message

Thomas Zimmermann Aug. 30, 2024, 8:39 a.m. UTC 
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>
---
 drivers/gpu/drm/Makefile           |  1 +
 drivers/gpu/drm/drm_client_setup.c | 69 ++++++++++++++++++++++++++++++
 include/drm/drm_client_setup.h     | 15 +++++++
 3 files changed, 85 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_client_setup.c
 create mode 100644 include/drm/drm_client_setup.h

Comments

Geert Uytterhoeven Aug. 30, 2024, 9:45 a.m. UTC | #1 
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.

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.

> + */
> +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 Aug. 30, 2024, 10:30 a.m. UTC | #2 
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
diff mbox series

Patch

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..5373a892f097
--- /dev/null
+++ b/drivers/gpu/drm/drm_client_setup.c
@@ -0,0 +1,69 @@ 
+// 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 pixel format for the device. Use NULL, unless
+ *          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;
+
+	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_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.
+ */
+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
+ * format of NULL.
+ */
+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);
diff --git a/include/drm/drm_client_setup.h b/include/drm/drm_client_setup.h
new file mode 100644
index 000000000000..6c0396b4f815
--- /dev/null
+++ b/include/drm/drm_client_setup.h
@@ -0,0 +1,15 @@ 
+/* SPDX-License-Identifier: MIT */
+
+#ifndef DRM_CLIENT_SETUP_H
+#define DRM_CLIENT_SETUP_H
+
+#include <linux/types.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_fourcc(struct drm_device *dev, u32 fourcc);
+void drm_client_setup_with_color_mode(struct drm_device *dev, unsigned int color_mode);
+
+#endif