diff mbox

[RFC,v3,11/12] drm/client: Add bootsplash client

Message ID 20180222200653.19453-12-noralf@tronnes.org (mailing list archive)
State New, archived
Headers show

Commit Message

Noralf Trønnes Feb. 22, 2018, 8:06 p.m. UTC
Just a hack to test the client API.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/client/Kconfig          |   5 +
 drivers/gpu/drm/client/Makefile         |   1 +
 drivers/gpu/drm/client/drm_bootsplash.c | 205 ++++++++++++++++++++++++++++++++
 3 files changed, 211 insertions(+)
 create mode 100644 drivers/gpu/drm/client/drm_bootsplash.c

Comments

Daniel Vetter March 6, 2018, 9:12 a.m. UTC | #1
On Thu, Feb 22, 2018 at 09:06:52PM +0100, Noralf Trønnes wrote:
> Just a hack to test the client API.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Adding the suse folks who submitted the bootsplash a while ago, would be
great if they could pick this up and run with it.

> ---
>  drivers/gpu/drm/client/Kconfig          |   5 +
>  drivers/gpu/drm/client/Makefile         |   1 +
>  drivers/gpu/drm/client/drm_bootsplash.c | 205 ++++++++++++++++++++++++++++++++
>  3 files changed, 211 insertions(+)
>  create mode 100644 drivers/gpu/drm/client/drm_bootsplash.c
> 
> diff --git a/drivers/gpu/drm/client/Kconfig b/drivers/gpu/drm/client/Kconfig
> index 73902ab44c75..16cf1e14620a 100644
> --- a/drivers/gpu/drm/client/Kconfig
> +++ b/drivers/gpu/drm/client/Kconfig
> @@ -17,4 +17,9 @@ config DRM_CLIENT_FBDEV
>  	help
>  	  Generic fbdev emulation
>  
> +config DRM_CLIENT_BOOTSPLASH
> +	tristate "DRM Bootsplash"
> +	help
> +	  DRM Bootsplash
> +
>  endmenu
> diff --git a/drivers/gpu/drm/client/Makefile b/drivers/gpu/drm/client/Makefile
> index 3ff694429dec..8660530e4646 100644
> --- a/drivers/gpu/drm/client/Makefile
> +++ b/drivers/gpu/drm/client/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
>  obj-$(CONFIG_DRM_CLIENT_FBDEV) += drm_fbdev.o
> +obj-$(CONFIG_DRM_CLIENT_BOOTSPLASH) += drm_bootsplash.o
> diff --git a/drivers/gpu/drm/client/drm_bootsplash.c b/drivers/gpu/drm/client/drm_bootsplash.c
> new file mode 100644
> index 000000000000..43c703606e74
> --- /dev/null
> +++ b/drivers/gpu/drm/client/drm_bootsplash.c
> @@ -0,0 +1,205 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/workqueue.h>
> +
> +#include <drm/drm_client.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_fourcc.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_print.h>
> +
> +struct drm_bootsplash {
> +	struct drm_client_dev *client;
> +	struct drm_client_display *display;
> +	struct drm_client_buffer *buffer[2];
> +	struct work_struct worker;
> +	bool stop;
> +};
> +
> +static u32 drm_bootsplash_color_table[3] = {
> +	0x00ff0000, 0x0000ff00, 0x000000ff,
> +};
> +
> +/* Draw a box with changing colors */
> +static void
> +drm_bootsplash_draw(struct drm_client_buffer *buffer, unsigned int sequence)
> +{
> +	unsigned int x, y;
> +	u32 *pix;
> +
> +	pix = buffer->vaddr;
> +	pix += ((buffer->height / 2) - 50) * buffer->width;
> +	pix += (buffer->width / 2) - 50;
> +
> +	for (y = 0; y < 100; y++) {
> +		for (x = 0; x < 100; x++)
> +			*pix++ = drm_bootsplash_color_table[sequence];
> +		pix += buffer->width - 100;
> +	}
> +}
> +
> +static void drm_bootsplash_worker(struct work_struct *work)
> +{
> +	struct drm_bootsplash *splash = container_of(work, struct drm_bootsplash,
> +						     worker);
> +	struct drm_event *event;
> +	unsigned int i = 0, sequence = 0, fb_id;
> +	int ret;
> +
> +	while (!splash->stop) {
> +		/* Are we still in charge? */
> +		fb_id = drm_client_display_current_fb(splash->display);
> +		if (fb_id != splash->buffer[i]->fb_ids[0])
> +			break;
> +
> +		/*
> +		 * We can race with userspace here between checking and doing
> +		 * the page flip, so double buffering isn't such a good idea.
> +		 * Tearing probably isn't a problem on a presumably small splash
> +		 * animation. I've kept it to test the page flip code.
> +		 */

I think a much cleaner way to solve all this is to tie it into our master
tracking. If there is a master (even if it's not displaying anything),
then none of the in-kernel clients should display anything.

If there is not, then we grab a temporary/weak master reference which
blocks other masters for the time being, but only until we've complete our
drawing operation. Then the in-kernel client drops that weak reference
again. A very simply solution would be to simply hold the device's master
lock (but I'm not sure whether that would result in deadlocks).

That would mean something like

		if (!drm_master_try_internal_acquire())
			/* someone else is master */
			break;

here instead of your fb id check.
> +
> +		i = !i;
> +		drm_bootsplash_draw(splash->buffer[i], sequence++);
> +		if (sequence == 3)
> +			sequence = 0;
> +
> +		ret = drm_client_display_page_flip(splash->display,
> +						   splash->buffer[i]->fb_ids[0],
> +						   true);
> +		if (!ret) {
> +			event = drm_client_read_event(splash->client, true);
> +			if (!IS_ERR(event))
> +				kfree(event);
> +		}

And
		drm_master_interal_relase()

here before we sleep again. Similar for all the fbdev ioctls and all that
stuff.

Just an idea, names definitely need improvements, and I have no idea
wheter it'll work. But using the same logic in the existing fbdev
emulation code would be really good (since that's used a lot more, at
least right now).
-Daniel

> +		msleep(500);
> +	}
> +
> +	for (i = 0; i < 2; i++)
> +		drm_client_framebuffer_delete(splash->buffer[i]);
> +	drm_client_display_free(splash->display);
> +}
> +
> +static int drm_bootsplash_setup(struct drm_bootsplash *splash)
> +{
> +	struct drm_client_dev *client = splash->client;
> +	struct drm_client_buffer *buffer[2];
> +	struct drm_client_display *display;
> +	struct drm_mode_modeinfo *mode;
> +	int ret, i;
> +
> +	display = drm_client_display_get_first_enabled(client, false);
> +	if (IS_ERR(display))
> +		return PTR_ERR(display);
> +	if (!display)
> +		return -ENOENT;
> +
> +	mode = drm_client_display_first_mode(display);
> +	if (!mode) {
> +		ret = -EINVAL;
> +		goto err_free_display;
> +	}
> +
> +	for (i = 0; i < 2; i++) {
> +		buffer[i] = drm_client_framebuffer_create(client, mode,
> +							  DRM_FORMAT_XRGB8888);
> +		if (IS_ERR(buffer[i])) {
> +			ret = PTR_ERR(buffer[i]);
> +			goto err_free_buffer;
> +		}
> +	}
> +
> +	ret = drm_client_display_commit_mode(display, buffer[0]->fb_ids[0], mode);
> +	if (ret)
> +		goto err_free_buffer;
> +
> +	splash->display = display;
> +	splash->buffer[0] = buffer[0];
> +	splash->buffer[1] = buffer[1];
> +
> +	schedule_work(&splash->worker);
> +
> +	return 0;
> +
> +err_free_buffer:
> +	for (i--; i >= 0; i--)
> +		drm_client_framebuffer_delete(buffer[i]);
> +err_free_display:
> +	drm_client_display_free(display);
> +
> +	return ret;
> +}
> +
> +static int drm_bootsplash_client_hotplug(struct drm_client_dev *client)
> +{
> +	struct drm_bootsplash *splash = client->private;
> +	int ret = 0;
> +
> +	if (!splash->display)
> +		ret = drm_bootsplash_setup(splash);
> +
> +	return ret;
> +}
> +
> +static int drm_bootsplash_client_new(struct drm_client_dev *client)
> +{
> +	struct drm_bootsplash *splash;
> +
> +	splash = kzalloc(sizeof(*splash), GFP_KERNEL);
> +	if (!splash)
> +		return -ENOMEM;
> +
> +	INIT_WORK(&splash->worker, drm_bootsplash_worker);
> +
> +	splash->client = client;
> +	client->private = splash;
> +
> +	/*
> +	 * vc4 isn't done with it's setup when drm_dev_register() is called.
> +	 * It should have shouldn't it?
> +	 * So to keep it from crashing defer setup to hotplug...
> +	 */
> +	if (client->dev->mode_config.max_width)
> +		drm_bootsplash_client_hotplug(client);
> +
> +	return 0;
> +}
> +
> +static int drm_bootsplash_client_remove(struct drm_client_dev *client)
> +{
> +	struct drm_bootsplash *splash = client->private;
> +
> +	if (splash->display) {
> +		splash->stop = true;
> +		flush_work(&splash->worker);
> +	}
> +
> +	kfree(splash);
> +
> +	return 0;
> +}
> +
> +static const struct drm_client_funcs drm_bootsplash_client_funcs = {
> +	.name		= "drm_bootsplash",
> +	.new		= drm_bootsplash_client_new,
> +	.remove		= drm_bootsplash_client_remove,
> +	.hotplug	= drm_bootsplash_client_hotplug,
> +};
> +
> +static int __init drm_bootsplash_init(void)
> +{
> +	return drm_client_register(&drm_bootsplash_client_funcs);
> +}
> +module_init(drm_bootsplash_init);
> +
> +static void __exit drm_bootsplash_exit(void)
> +{
> +	drm_client_unregister(&drm_bootsplash_client_funcs);
> +}
> +module_exit(drm_bootsplash_exit);
> +
> +MODULE_LICENSE("GPL");
> -- 
> 2.15.1
>
'Max Staudt March 6, 2018, 3:21 p.m. UTC | #2
Thanks for CCing!

I like the idea of this patchset. As far as I understand, this multiplexing is exactly what I would have had to write in order to port the bootsplash to DRM. And we finally get rid of the driver-specific FB emulation hacks, too. Good riddance.

Thanks for the initiative, Noralf!

I've stopped working on the bootsplash since there seemed to be no more interest in it - but if there is, I can port it to such an in-kernel DRM client architecture once it's in.


Max
diff mbox

Patch

diff --git a/drivers/gpu/drm/client/Kconfig b/drivers/gpu/drm/client/Kconfig
index 73902ab44c75..16cf1e14620a 100644
--- a/drivers/gpu/drm/client/Kconfig
+++ b/drivers/gpu/drm/client/Kconfig
@@ -17,4 +17,9 @@  config DRM_CLIENT_FBDEV
 	help
 	  Generic fbdev emulation
 
+config DRM_CLIENT_BOOTSPLASH
+	tristate "DRM Bootsplash"
+	help
+	  DRM Bootsplash
+
 endmenu
diff --git a/drivers/gpu/drm/client/Makefile b/drivers/gpu/drm/client/Makefile
index 3ff694429dec..8660530e4646 100644
--- a/drivers/gpu/drm/client/Makefile
+++ b/drivers/gpu/drm/client/Makefile
@@ -1,3 +1,4 @@ 
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_DRM_CLIENT_FBDEV) += drm_fbdev.o
+obj-$(CONFIG_DRM_CLIENT_BOOTSPLASH) += drm_bootsplash.o
diff --git a/drivers/gpu/drm/client/drm_bootsplash.c b/drivers/gpu/drm/client/drm_bootsplash.c
new file mode 100644
index 000000000000..43c703606e74
--- /dev/null
+++ b/drivers/gpu/drm/client/drm_bootsplash.c
@@ -0,0 +1,205 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+
+#include <drm/drm_client.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_print.h>
+
+struct drm_bootsplash {
+	struct drm_client_dev *client;
+	struct drm_client_display *display;
+	struct drm_client_buffer *buffer[2];
+	struct work_struct worker;
+	bool stop;
+};
+
+static u32 drm_bootsplash_color_table[3] = {
+	0x00ff0000, 0x0000ff00, 0x000000ff,
+};
+
+/* Draw a box with changing colors */
+static void
+drm_bootsplash_draw(struct drm_client_buffer *buffer, unsigned int sequence)
+{
+	unsigned int x, y;
+	u32 *pix;
+
+	pix = buffer->vaddr;
+	pix += ((buffer->height / 2) - 50) * buffer->width;
+	pix += (buffer->width / 2) - 50;
+
+	for (y = 0; y < 100; y++) {
+		for (x = 0; x < 100; x++)
+			*pix++ = drm_bootsplash_color_table[sequence];
+		pix += buffer->width - 100;
+	}
+}
+
+static void drm_bootsplash_worker(struct work_struct *work)
+{
+	struct drm_bootsplash *splash = container_of(work, struct drm_bootsplash,
+						     worker);
+	struct drm_event *event;
+	unsigned int i = 0, sequence = 0, fb_id;
+	int ret;
+
+	while (!splash->stop) {
+		/* Are we still in charge? */
+		fb_id = drm_client_display_current_fb(splash->display);
+		if (fb_id != splash->buffer[i]->fb_ids[0])
+			break;
+
+		/*
+		 * We can race with userspace here between checking and doing
+		 * the page flip, so double buffering isn't such a good idea.
+		 * Tearing probably isn't a problem on a presumably small splash
+		 * animation. I've kept it to test the page flip code.
+		 */
+
+		i = !i;
+		drm_bootsplash_draw(splash->buffer[i], sequence++);
+		if (sequence == 3)
+			sequence = 0;
+
+		ret = drm_client_display_page_flip(splash->display,
+						   splash->buffer[i]->fb_ids[0],
+						   true);
+		if (!ret) {
+			event = drm_client_read_event(splash->client, true);
+			if (!IS_ERR(event))
+				kfree(event);
+		}
+		msleep(500);
+	}
+
+	for (i = 0; i < 2; i++)
+		drm_client_framebuffer_delete(splash->buffer[i]);
+	drm_client_display_free(splash->display);
+}
+
+static int drm_bootsplash_setup(struct drm_bootsplash *splash)
+{
+	struct drm_client_dev *client = splash->client;
+	struct drm_client_buffer *buffer[2];
+	struct drm_client_display *display;
+	struct drm_mode_modeinfo *mode;
+	int ret, i;
+
+	display = drm_client_display_get_first_enabled(client, false);
+	if (IS_ERR(display))
+		return PTR_ERR(display);
+	if (!display)
+		return -ENOENT;
+
+	mode = drm_client_display_first_mode(display);
+	if (!mode) {
+		ret = -EINVAL;
+		goto err_free_display;
+	}
+
+	for (i = 0; i < 2; i++) {
+		buffer[i] = drm_client_framebuffer_create(client, mode,
+							  DRM_FORMAT_XRGB8888);
+		if (IS_ERR(buffer[i])) {
+			ret = PTR_ERR(buffer[i]);
+			goto err_free_buffer;
+		}
+	}
+
+	ret = drm_client_display_commit_mode(display, buffer[0]->fb_ids[0], mode);
+	if (ret)
+		goto err_free_buffer;
+
+	splash->display = display;
+	splash->buffer[0] = buffer[0];
+	splash->buffer[1] = buffer[1];
+
+	schedule_work(&splash->worker);
+
+	return 0;
+
+err_free_buffer:
+	for (i--; i >= 0; i--)
+		drm_client_framebuffer_delete(buffer[i]);
+err_free_display:
+	drm_client_display_free(display);
+
+	return ret;
+}
+
+static int drm_bootsplash_client_hotplug(struct drm_client_dev *client)
+{
+	struct drm_bootsplash *splash = client->private;
+	int ret = 0;
+
+	if (!splash->display)
+		ret = drm_bootsplash_setup(splash);
+
+	return ret;
+}
+
+static int drm_bootsplash_client_new(struct drm_client_dev *client)
+{
+	struct drm_bootsplash *splash;
+
+	splash = kzalloc(sizeof(*splash), GFP_KERNEL);
+	if (!splash)
+		return -ENOMEM;
+
+	INIT_WORK(&splash->worker, drm_bootsplash_worker);
+
+	splash->client = client;
+	client->private = splash;
+
+	/*
+	 * vc4 isn't done with it's setup when drm_dev_register() is called.
+	 * It should have shouldn't it?
+	 * So to keep it from crashing defer setup to hotplug...
+	 */
+	if (client->dev->mode_config.max_width)
+		drm_bootsplash_client_hotplug(client);
+
+	return 0;
+}
+
+static int drm_bootsplash_client_remove(struct drm_client_dev *client)
+{
+	struct drm_bootsplash *splash = client->private;
+
+	if (splash->display) {
+		splash->stop = true;
+		flush_work(&splash->worker);
+	}
+
+	kfree(splash);
+
+	return 0;
+}
+
+static const struct drm_client_funcs drm_bootsplash_client_funcs = {
+	.name		= "drm_bootsplash",
+	.new		= drm_bootsplash_client_new,
+	.remove		= drm_bootsplash_client_remove,
+	.hotplug	= drm_bootsplash_client_hotplug,
+};
+
+static int __init drm_bootsplash_init(void)
+{
+	return drm_client_register(&drm_bootsplash_client_funcs);
+}
+module_init(drm_bootsplash_init);
+
+static void __exit drm_bootsplash_exit(void)
+{
+	drm_client_unregister(&drm_bootsplash_client_funcs);
+}
+module_exit(drm_bootsplash_exit);
+
+MODULE_LICENSE("GPL");