LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH V5 0/2] drm/vkms: Add virtual hardware module
@ 2021-08-01 16:01 Sumera Priyadarsini
  2021-08-01 16:03 ` [PATCH V5 1/2] drm/vkms: Refactor vkms_composer_worker() to prep for virtual_hw mode Sumera Priyadarsini
  2021-08-01 16:03 ` [PATCH V5 2/2] drm/vkms: Add support for virtual hardware mode Sumera Priyadarsini
  0 siblings, 2 replies; 4+ messages in thread
From: Sumera Priyadarsini @ 2021-08-01 16:01 UTC (permalink / raw)
  To: melissa.srw
  Cc: rodrigosiqueiramelo, hamohammed.sa, daniel, airlied, dri-devel,
	linux-kernel

This patchset adds support for emulating virtual hardware with VKMS.
The virtual hardware mode can be enabled by using the following command
while loading the module:
        sudo modprobe vkms enable_virtual_hw=1

The first patch is prep work for adding virtual_hw mode and refactors
the plane composition in vkms by adding a helper function vkms_composer_common()
which can be used for both vblank mode and virtual mode.

The second patch adds virtual hardware support as a module option. It
adds new atomic helper functions for the virtual mode
and modifies the existing atomic helpers for usage by the vblank mode
This gives us two sets of drm_crtc_helper_funcs struct for both modes,
making the code flow cleaner and easier to debug.

This patchset has been tested with the igt tests- kms_writeback, kms_atomic,
kms_lease, kms_flip, kms_pipe_get_crc and preserves results except for
subtests related to crc reads and skips tests that rely on vertical
blanking.

Sumera Priyadarsini (2):
  drm/vkms: Refactor vkms_composer_worker() to prep for virtual_hw mode
  drm/vkms: Add support for virtual hardware mode

 drivers/gpu/drm/vkms/vkms_composer.c | 93 ++++++++++++++++++----------
 drivers/gpu/drm/vkms/vkms_crtc.c     | 51 ++++++++++-----
 drivers/gpu/drm/vkms/vkms_drv.c      | 16 +++--
 drivers/gpu/drm/vkms/vkms_drv.h      |  4 ++
 4 files changed, 112 insertions(+), 52 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH V5 1/2] drm/vkms: Refactor vkms_composer_worker() to prep for virtual_hw mode
  2021-08-01 16:01 [PATCH V5 0/2] drm/vkms: Add virtual hardware module Sumera Priyadarsini
@ 2021-08-01 16:03 ` Sumera Priyadarsini
  2021-08-01 16:03 ` [PATCH V5 2/2] drm/vkms: Add support for virtual hardware mode Sumera Priyadarsini
  1 sibling, 0 replies; 4+ messages in thread
From: Sumera Priyadarsini @ 2021-08-01 16:03 UTC (permalink / raw)
  To: melissa.srw
  Cc: rodrigosiqueiramelo, hamohammed.sa, daniel, airlied, dri-devel,
	linux-kernel

Add a new function vkms_composer_common(). The actual plane
composition work has been moved to the helper function,
vkms_composer_common() which is called by vkms_composer_worker()
and will be called in the implementation of virtual_hw mode
as well.

Signed-off-by: Sumera Priyadarsini <sylphrenadin@gmail.com>
---
Changes in V5:
- Move vkms_crtc_composer() to the patch that introduces
virtual_hw mode (Melissa)
- Fix checkpatch errors(Melissa)
Changes in V4:
- Fix warning
Changes in V3:
- Refactor patchset (Melissa)
Change in V2:
- Add vkms_composer_common() (Daniel)
---
 drivers/gpu/drm/vkms/vkms_composer.c | 76 ++++++++++++++++------------
 drivers/gpu/drm/vkms/vkms_drv.h      |  2 +
 2 files changed, 45 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index ead8fff81f30..bf3d576db225 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -206,6 +206,47 @@ static int compose_active_planes(void **vaddr_out,
 	return 0;
 }
 
+int vkms_composer_common(struct vkms_crtc_state *crtc_state,
+			 struct vkms_output *out, bool wb_pending, uint32_t *crc32)
+{
+	struct vkms_composer *primary_composer = NULL;
+	struct vkms_plane_state *act_plane = NULL;
+	void *vaddr_out = NULL;
+	int ret;
+
+	if (crtc_state->num_active_planes >= 1) {
+		act_plane = crtc_state->active_planes[0];
+		if (act_plane->base.base.plane->type == DRM_PLANE_TYPE_PRIMARY)
+			primary_composer = act_plane->composer;
+	}
+
+	if (!primary_composer)
+		return -EINVAL;
+	if (wb_pending)
+		vaddr_out = crtc_state->active_writeback;
+
+	ret = compose_active_planes(&vaddr_out, primary_composer, crtc_state);
+
+	if (ret) {
+		if ((ret == -EINVAL || ret == -ENOMEM) && !wb_pending)
+			kfree(vaddr_out);
+		return ret;
+	}
+
+	*crc32 = compute_crc(vaddr_out, primary_composer);
+
+	if (wb_pending) {
+		drm_writeback_signal_completion(&out->wb_connector, 0);
+		spin_lock_irq(&out->composer_lock);
+		crtc_state->wb_pending = false;
+		spin_unlock_irq(&out->composer_lock);
+	} else {
+		kfree(vaddr_out);
+	}
+
+	return 0;
+}
+
 /**
  * vkms_composer_worker - ordered work_struct to compute CRC
  *
@@ -222,10 +263,7 @@ void vkms_composer_worker(struct work_struct *work)
 						composer_work);
 	struct drm_crtc *crtc = crtc_state->base.crtc;
 	struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
-	struct vkms_composer *primary_composer = NULL;
-	struct vkms_plane_state *act_plane = NULL;
 	bool crc_pending, wb_pending;
-	void *vaddr_out = NULL;
 	u32 crc32 = 0;
 	u64 frame_start, frame_end;
 	int ret;
@@ -247,37 +285,9 @@ void vkms_composer_worker(struct work_struct *work)
 	if (!crc_pending)
 		return;
 
-	if (crtc_state->num_active_planes >= 1) {
-		act_plane = crtc_state->active_planes[0];
-		if (act_plane->base.base.plane->type == DRM_PLANE_TYPE_PRIMARY)
-			primary_composer = act_plane->composer;
-	}
-
-	if (!primary_composer)
-		return;
-
-	if (wb_pending)
-		vaddr_out = crtc_state->active_writeback;
-
-	ret = compose_active_planes(&vaddr_out, primary_composer,
-				    crtc_state);
-	if (ret) {
-		if (ret == -EINVAL && !wb_pending)
-			kfree(vaddr_out);
+	ret = vkms_composer_common(crtc_state, out, wb_pending, &crc32);
+	if (ret == -EINVAL)
 		return;
-	}
-
-	crc32 = compute_crc(vaddr_out, primary_composer);
-
-	if (wb_pending) {
-		drm_writeback_signal_completion(&out->wb_connector, 0);
-		spin_lock_irq(&out->composer_lock);
-		crtc_state->wb_pending = false;
-		spin_unlock_irq(&out->composer_lock);
-	} else {
-		kfree(vaddr_out);
-	}
-
 	/*
 	 * The worker can fall behind the vblank hrtimer, make sure we catch up.
 	 */
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 8c731b6dcba7..01beba424f18 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -132,6 +132,8 @@ int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
 			   size_t *values_cnt);
 
 /* Composer Support */
+int vkms_composer_common(struct vkms_crtc_state *crtc_state, struct vkms_output *out,
+			 bool wb_pending, uint32_t *crcs);
 void vkms_composer_worker(struct work_struct *work);
 void vkms_set_composer(struct vkms_output *out, bool enabled);
 
-- 
2.31.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH V5 2/2] drm/vkms: Add support for virtual hardware mode
  2021-08-01 16:01 [PATCH V5 0/2] drm/vkms: Add virtual hardware module Sumera Priyadarsini
  2021-08-01 16:03 ` [PATCH V5 1/2] drm/vkms: Refactor vkms_composer_worker() to prep for virtual_hw mode Sumera Priyadarsini
@ 2021-08-01 16:03 ` Sumera Priyadarsini
  2021-08-03 19:35   ` Melissa Wen
  1 sibling, 1 reply; 4+ messages in thread
From: Sumera Priyadarsini @ 2021-08-01 16:03 UTC (permalink / raw)
  To: melissa.srw
  Cc: rodrigosiqueiramelo, hamohammed.sa, daniel, airlied, dri-devel,
	linux-kernel

Add a virtual hardware or vblank-less mode as a module
to enable VKMS to emulate virtual hardware drivers. This means
no vertical blanking events occur and pageflips are completed
arbitrarily and when required for updating the frame.

Add a new drm_crtc_helper_funcs struct,
vkms_virtual_crtc_helper_funcs() which holds the atomic helpers
for virtual hardware mode. Rename the existing
vkms_crtc_helper_funcs struct to vkms_vblank_crtc_helper_funcs
which holds atomic helpers for the vblank mode.
This makes the code flow clearer and testing
virtual hardware mode.

Add a function vkms_crtc_composer() which calls the helper function,
vkms_composer_common() for plane composition in vblank-less mode.
vkms_crtc_composer() is directly called in the atomic hook in
vkms_crtc_atomic_begin().

However, some crc captures still use vblanks which causes the crc-based
igt tests to crash. Currently, I am unsure about how to approach the
one-shot implementation of crc reads so I am still working on that.

This patchset has been tested with the igt tests- kms_writeback, kms_atomic
, kms_lease, kms_flip, kms_pipe_get_crc and preserves results except for
subtests related to crc reads and skips tests that rely on vertical
blanking.

The patch is based on Rodrigo Siqueira's
patch(https://patchwork.freedesktop.org/patch/316851/?series=48469&rev=3)
and the ensuing review.

Signed-off-by: Sumera Priyadarsini <sylphrenadin@gmail.com>
---
Changes in V5:
- Move vkms_crtc_composer() to this patch(Melissa)
- Add more clarification for "vblank-less" mode(Pekka)
- Replace kzalloc() with kvmalloc() in compose_active_planes()
to fix memory allocation error for output frame
- Fix checkpatch warnings (Melissa)
Changes in V3:
- Refactor patchset(Melissa)
Changes in V2:
- Add atomic helper functions in a separate struct for virtual hardware
mode (Daniel)
- Remove spinlock across 'vkms_output->lock' in vkms_crtc.c(Daniel)
- Add vkms_composer_common() (Daniel)
---
 drivers/gpu/drm/vkms/vkms_composer.c | 21 ++++++++++--
 drivers/gpu/drm/vkms/vkms_crtc.c     | 51 ++++++++++++++++++++--------
 drivers/gpu/drm/vkms/vkms_drv.c      | 16 ++++++---
 drivers/gpu/drm/vkms/vkms_drv.h      |  2 ++
 4 files changed, 69 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index bf3d576db225..2988d5b49eb6 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -176,11 +176,12 @@ static int compose_active_planes(void **vaddr_out,
 {
 	struct drm_framebuffer *fb = &primary_composer->fb;
 	struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0);
+
 	const void *vaddr;
 	int i;
 
 	if (!*vaddr_out) {
-		*vaddr_out = kzalloc(gem_obj->size, GFP_KERNEL);
+		*vaddr_out = kvmalloc(gem_obj->size, GFP_KERNEL);
 		if (!*vaddr_out) {
 			DRM_ERROR("Cannot allocate memory for output frame.");
 			return -ENOMEM;
@@ -229,7 +230,7 @@ int vkms_composer_common(struct vkms_crtc_state *crtc_state,
 
 	if (ret) {
 		if ((ret == -EINVAL || ret == -ENOMEM) && !wb_pending)
-			kfree(vaddr_out);
+			kvfree(vaddr_out);
 		return ret;
 	}
 
@@ -241,7 +242,7 @@ int vkms_composer_common(struct vkms_crtc_state *crtc_state,
 		crtc_state->wb_pending = false;
 		spin_unlock_irq(&out->composer_lock);
 	} else {
-		kfree(vaddr_out);
+		kvfree(vaddr_out);
 	}
 
 	return 0;
@@ -295,6 +296,20 @@ void vkms_composer_worker(struct work_struct *work)
 		drm_crtc_add_crc_entry(crtc, true, frame_start++, &crc32);
 }
 
+void vkms_crtc_composer(struct vkms_crtc_state *crtc_state)
+{
+	struct drm_crtc *crtc = crtc_state->base.crtc;
+	struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
+	u32 crc32 = 0;
+	int ret;
+
+	ret = vkms_composer_common(crtc_state, out, crtc_state->wb_pending, &crc32);
+	if (ret == -EINVAL)
+		return;
+
+	drm_crtc_add_crc_entry(crtc, true, 0, &crc32);
+}
+
 static const char * const pipe_crc_sources[] = {"auto"};
 
 const char *const *vkms_get_crc_sources(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 57bbd32e9beb..8477b33c4d09 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -222,20 +222,20 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
 	return 0;
 }
 
-static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
-				    struct drm_atomic_state *state)
+static void vkms_vblank_crtc_atomic_enable(struct drm_crtc *crtc,
+					   struct drm_atomic_state *state)
 {
 	drm_crtc_vblank_on(crtc);
 }
 
-static void vkms_crtc_atomic_disable(struct drm_crtc *crtc,
-				     struct drm_atomic_state *state)
+static void vkms_vblank_crtc_atomic_disable(struct drm_crtc *crtc,
+					    struct drm_atomic_state *state)
 {
 	drm_crtc_vblank_off(crtc);
 }
 
-static void vkms_crtc_atomic_begin(struct drm_crtc *crtc,
-				   struct drm_atomic_state *state)
+static void vkms_vblank_crtc_atomic_begin(struct drm_crtc *crtc,
+					  struct drm_atomic_state *state)
 {
 	struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
 
@@ -245,8 +245,8 @@ static void vkms_crtc_atomic_begin(struct drm_crtc *crtc,
 	spin_lock_irq(&vkms_output->lock);
 }
 
-static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
-				   struct drm_atomic_state *state)
+static void vkms_vblank_crtc_atomic_flush(struct drm_crtc *crtc,
+					  struct drm_atomic_state *state)
 {
 	struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
 
@@ -268,18 +268,38 @@ static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
 	spin_unlock_irq(&vkms_output->lock);
 }
 
-static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
+/*
+ * Crtc functions for virtual hardware/vblankless mode
+ */
+static void vkms_virtual_crtc_atomic_flush(struct drm_crtc *crtc,
+					   struct drm_atomic_state *state)
+{
+	struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
+	struct vkms_crtc_state *vkms_state = to_vkms_crtc_state(crtc->state);
+
+	vkms_crtc_composer(vkms_state);
+
+	vkms_output->composer_state = to_vkms_crtc_state(crtc->state);
+}
+
+static const struct drm_crtc_helper_funcs vkms_vblank_crtc_helper_funcs = {
 	.atomic_check	= vkms_crtc_atomic_check,
-	.atomic_begin	= vkms_crtc_atomic_begin,
-	.atomic_flush	= vkms_crtc_atomic_flush,
-	.atomic_enable	= vkms_crtc_atomic_enable,
-	.atomic_disable	= vkms_crtc_atomic_disable,
+	.atomic_begin	= vkms_vblank_crtc_atomic_begin,
+	.atomic_flush	= vkms_vblank_crtc_atomic_flush,
+	.atomic_enable	= vkms_vblank_crtc_atomic_enable,
+	.atomic_disable	= vkms_vblank_crtc_atomic_disable,
+};
+
+static const struct drm_crtc_helper_funcs vkms_virtual_crtc_helper_funcs = {
+	.atomic_check	= vkms_crtc_atomic_check,
+	.atomic_flush	= vkms_virtual_crtc_atomic_flush,
 };
 
 int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
 		   struct drm_plane *primary, struct drm_plane *cursor)
 {
 	struct vkms_output *vkms_out = drm_crtc_to_vkms_output(crtc);
+	struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
 	int ret;
 
 	ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor,
@@ -289,7 +309,10 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
 		return ret;
 	}
 
-	drm_crtc_helper_add(crtc, &vkms_crtc_helper_funcs);
+	if (vkmsdev->config->virtual_hw)
+		drm_crtc_helper_add(crtc, &vkms_virtual_crtc_helper_funcs);
+	else
+		drm_crtc_helper_add(crtc, &vkms_vblank_crtc_helper_funcs);
 
 	spin_lock_init(&vkms_out->lock);
 	spin_lock_init(&vkms_out->composer_lock);
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 0ffe5f0e33f7..ee78f5eef653 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -51,6 +51,10 @@ static bool enable_overlay;
 module_param_named(enable_overlay, enable_overlay, bool, 0444);
 MODULE_PARM_DESC(enable_overlay, "Enable/Disable overlay support");
 
+static bool enable_virtual_hw;
+module_param_named(enable_virtual_hw, enable_virtual_hw, bool, 0444);
+MODULE_PARM_DESC(enable_virtual_hw, "Enable/Disable virtual hardware mode(vblank-less mode)");
+
 DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
 
 static void vkms_release(struct drm_device *dev)
@@ -98,6 +102,7 @@ static int vkms_config_show(struct seq_file *m, void *data)
 	seq_printf(m, "writeback=%d\n", vkmsdev->config->writeback);
 	seq_printf(m, "cursor=%d\n", vkmsdev->config->cursor);
 	seq_printf(m, "overlay=%d\n", vkmsdev->config->overlay);
+	seq_printf(m, "virtual_hw=%d\n", vkmsdev->config->virtual_hw);
 
 	return 0;
 }
@@ -191,10 +196,12 @@ static int vkms_create(struct vkms_config *config)
 		goto out_devres;
 	}
 
-	ret = drm_vblank_init(&vkms_device->drm, 1);
-	if (ret) {
-		DRM_ERROR("Failed to vblank\n");
-		goto out_devres;
+	if (!vkms_device->config->virtual_hw) {
+		ret = drm_vblank_init(&vkms_device->drm, 1);
+		if (ret) {
+			DRM_ERROR("Failed to vblank\n");
+			goto out_devres;
+		}
 	}
 
 	ret = vkms_modeset_init(vkms_device);
@@ -229,6 +236,7 @@ static int __init vkms_init(void)
 	config->cursor = enable_cursor;
 	config->writeback = enable_writeback;
 	config->overlay = enable_overlay;
+	config->virtual_hw = enable_virtual_hw;
 
 	return vkms_create(config);
 }
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 01beba424f18..770594e07f0e 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -92,6 +92,7 @@ struct vkms_config {
 	bool writeback;
 	bool cursor;
 	bool overlay;
+	bool virtual_hw;
 	/* only set when instantiated */
 	struct vkms_device *dev;
 };
@@ -136,6 +137,7 @@ int vkms_composer_common(struct vkms_crtc_state *crtc_state, struct vkms_output
 			 bool wb_pending, uint32_t *crcs);
 void vkms_composer_worker(struct work_struct *work);
 void vkms_set_composer(struct vkms_output *out, bool enabled);
+void vkms_crtc_composer(struct vkms_crtc_state *crtc_state);
 
 /* Writeback */
 int vkms_enable_writeback_connector(struct vkms_device *vkmsdev);
-- 
2.31.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH V5 2/2] drm/vkms: Add support for virtual hardware mode
  2021-08-01 16:03 ` [PATCH V5 2/2] drm/vkms: Add support for virtual hardware mode Sumera Priyadarsini
@ 2021-08-03 19:35   ` Melissa Wen
  0 siblings, 0 replies; 4+ messages in thread
From: Melissa Wen @ 2021-08-03 19:35 UTC (permalink / raw)
  To: Sumera Priyadarsini
  Cc: melissa.srw, rodrigosiqueiramelo, hamohammed.sa, daniel, airlied,
	dri-devel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 11270 bytes --]

On 08/01, Sumera Priyadarsini wrote:
> Add a virtual hardware or vblank-less mode as a module
> to enable VKMS to emulate virtual hardware drivers. This means
> no vertical blanking events occur and pageflips are completed
> arbitrarily and when required for updating the frame.
> 
> Add a new drm_crtc_helper_funcs struct,
> vkms_virtual_crtc_helper_funcs() which holds the atomic helpers
> for virtual hardware mode. Rename the existing
> vkms_crtc_helper_funcs struct to vkms_vblank_crtc_helper_funcs
> which holds atomic helpers for the vblank mode.
> This makes the code flow clearer and testing
> virtual hardware mode.
> 
Hi Sumera,

Thanks for working to enable it.

> Add a function vkms_crtc_composer() which calls the helper function,
> vkms_composer_common() for plane composition in vblank-less mode.
> vkms_crtc_composer() is directly called in the atomic hook in
> vkms_crtc_atomic_begin().
> 
> However, some crc captures still use vblanks which causes the crc-based
> igt tests to crash. Currently, I am unsure about how to approach the
> one-shot implementation of crc reads so I am still working on that.
>
We've briefly discussed on irc that we could go ahead without covering
crc-based tests. However, we should not introduce errors in the driver
when trying to set crc source. 
For this, we need to avoid that vblank_get/put calls in
vkms_set_crc_source (vkms_composer.c) Can you check it, please?

> This patchset has been tested with the igt tests- kms_writeback, kms_atomic
> , kms_lease, kms_flip, kms_pipe_get_crc and preserves results except for
> subtests related to crc reads and skips tests that rely on vertical
> blanking.
> 
> The patch is based on Rodrigo Siqueira's
> patch(https://patchwork.freedesktop.org/patch/316851/?series=48469&rev=3)
> and the ensuing review.
> 
> Signed-off-by: Sumera Priyadarsini <sylphrenadin@gmail.com>
> ---
> Changes in V5:
> - Move vkms_crtc_composer() to this patch(Melissa)
> - Add more clarification for "vblank-less" mode(Pekka)
> - Replace kzalloc() with kvmalloc() in compose_active_planes()
> to fix memory allocation error for output frame
> - Fix checkpatch warnings (Melissa)
> Changes in V3:
> - Refactor patchset(Melissa)
> Changes in V2:
> - Add atomic helper functions in a separate struct for virtual hardware
> mode (Daniel)
> - Remove spinlock across 'vkms_output->lock' in vkms_crtc.c(Daniel)
> - Add vkms_composer_common() (Daniel)
> ---
>  drivers/gpu/drm/vkms/vkms_composer.c | 21 ++++++++++--
>  drivers/gpu/drm/vkms/vkms_crtc.c     | 51 ++++++++++++++++++++--------
>  drivers/gpu/drm/vkms/vkms_drv.c      | 16 ++++++---
>  drivers/gpu/drm/vkms/vkms_drv.h      |  2 ++
>  4 files changed, 69 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
> index bf3d576db225..2988d5b49eb6 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -176,11 +176,12 @@ static int compose_active_planes(void **vaddr_out,
>  {
>  	struct drm_framebuffer *fb = &primary_composer->fb;
>  	struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0);
> +
>  	const void *vaddr;
>  	int i;
>  
>  	if (!*vaddr_out) {
> -		*vaddr_out = kzalloc(gem_obj->size, GFP_KERNEL);
> +		*vaddr_out = kvmalloc(gem_obj->size, GFP_KERNEL);

Is this change necessary to enable vblank-less mode? Or is it treating
another problem?

>  		if (!*vaddr_out) {
>  			DRM_ERROR("Cannot allocate memory for output frame.");
>  			return -ENOMEM;
> @@ -229,7 +230,7 @@ int vkms_composer_common(struct vkms_crtc_state *crtc_state,
>  
>  	if (ret) {
>  		if ((ret == -EINVAL || ret == -ENOMEM) && !wb_pending)
> -			kfree(vaddr_out);
> +			kvfree(vaddr_out);
>  		return ret;
>  	}
>  
> @@ -241,7 +242,7 @@ int vkms_composer_common(struct vkms_crtc_state *crtc_state,
>  		crtc_state->wb_pending = false;
>  		spin_unlock_irq(&out->composer_lock);
>  	} else {
> -		kfree(vaddr_out);
> +		kvfree(vaddr_out);
>  	}
>  
>  	return 0;
> @@ -295,6 +296,20 @@ void vkms_composer_worker(struct work_struct *work)
>  		drm_crtc_add_crc_entry(crtc, true, frame_start++, &crc32);
>  }
>  
> +void vkms_crtc_composer(struct vkms_crtc_state *crtc_state)
> +{
> +	struct drm_crtc *crtc = crtc_state->base.crtc;
> +	struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
> +	u32 crc32 = 0;
> +	int ret;
> +
> +	ret = vkms_composer_common(crtc_state, out, crtc_state->wb_pending, &crc32);
> +	if (ret == -EINVAL)
> +		return;
> +
> +	drm_crtc_add_crc_entry(crtc, true, 0, &crc32);
> +}
> +
>  static const char * const pipe_crc_sources[] = {"auto"};
>  
>  const char *const *vkms_get_crc_sources(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 57bbd32e9beb..8477b33c4d09 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -222,20 +222,20 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
>  	return 0;
>  }
>  
> -static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
> -				    struct drm_atomic_state *state)
> +static void vkms_vblank_crtc_atomic_enable(struct drm_crtc *crtc,
> +					   struct drm_atomic_state *state)
>  {
>  	drm_crtc_vblank_on(crtc);
>  }
>  
> -static void vkms_crtc_atomic_disable(struct drm_crtc *crtc,
> -				     struct drm_atomic_state *state)
> +static void vkms_vblank_crtc_atomic_disable(struct drm_crtc *crtc,
> +					    struct drm_atomic_state *state)
>  {
>  	drm_crtc_vblank_off(crtc);
>  }
>  
> -static void vkms_crtc_atomic_begin(struct drm_crtc *crtc,
> -				   struct drm_atomic_state *state)
> +static void vkms_vblank_crtc_atomic_begin(struct drm_crtc *crtc,
> +					  struct drm_atomic_state *state)
>  {
>  	struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
>  
> @@ -245,8 +245,8 @@ static void vkms_crtc_atomic_begin(struct drm_crtc *crtc,
>  	spin_lock_irq(&vkms_output->lock);
>  }
>  
> -static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
> -				   struct drm_atomic_state *state)
> +static void vkms_vblank_crtc_atomic_flush(struct drm_crtc *crtc,
> +					  struct drm_atomic_state *state)
>  {
>  	struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
>  
> @@ -268,18 +268,38 @@ static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
>  	spin_unlock_irq(&vkms_output->lock);
>  }
>  
> -static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
> +/*
> + * Crtc functions for virtual hardware/vblankless mode
> + */
> +static void vkms_virtual_crtc_atomic_flush(struct drm_crtc *crtc,
> +					   struct drm_atomic_state *state)
> +{
> +	struct vkms_output *vkms_output = drm_crtc_to_vkms_output(crtc);
> +	struct vkms_crtc_state *vkms_state = to_vkms_crtc_state(crtc->state);
> +
> +	vkms_crtc_composer(vkms_state);
> +
> +	vkms_output->composer_state = to_vkms_crtc_state(crtc->state);
> +}
> +
> +static const struct drm_crtc_helper_funcs vkms_vblank_crtc_helper_funcs = {
>  	.atomic_check	= vkms_crtc_atomic_check,
> -	.atomic_begin	= vkms_crtc_atomic_begin,
> -	.atomic_flush	= vkms_crtc_atomic_flush,
> -	.atomic_enable	= vkms_crtc_atomic_enable,
> -	.atomic_disable	= vkms_crtc_atomic_disable,
> +	.atomic_begin	= vkms_vblank_crtc_atomic_begin,
> +	.atomic_flush	= vkms_vblank_crtc_atomic_flush,
> +	.atomic_enable	= vkms_vblank_crtc_atomic_enable,
> +	.atomic_disable	= vkms_vblank_crtc_atomic_disable,
> +};
> +
> +static const struct drm_crtc_helper_funcs vkms_virtual_crtc_helper_funcs = {
> +	.atomic_check	= vkms_crtc_atomic_check,
> +	.atomic_flush	= vkms_virtual_crtc_atomic_flush,
>  };
>  
>  int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>  		   struct drm_plane *primary, struct drm_plane *cursor)
>  {
>  	struct vkms_output *vkms_out = drm_crtc_to_vkms_output(crtc);
> +	struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
>  	int ret;
>  
>  	ret = drm_crtc_init_with_planes(dev, crtc, primary, cursor,
> @@ -289,7 +309,10 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>  		return ret;
>  	}
>  
> -	drm_crtc_helper_add(crtc, &vkms_crtc_helper_funcs);
> +	if (vkmsdev->config->virtual_hw)
> +		drm_crtc_helper_add(crtc, &vkms_virtual_crtc_helper_funcs);
> +	else
> +		drm_crtc_helper_add(crtc, &vkms_vblank_crtc_helper_funcs);
>  
>  	spin_lock_init(&vkms_out->lock);
>  	spin_lock_init(&vkms_out->composer_lock);
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 0ffe5f0e33f7..ee78f5eef653 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -51,6 +51,10 @@ static bool enable_overlay;
>  module_param_named(enable_overlay, enable_overlay, bool, 0444);
>  MODULE_PARM_DESC(enable_overlay, "Enable/Disable overlay support");
>  
> +static bool enable_virtual_hw;
> +module_param_named(enable_virtual_hw, enable_virtual_hw, bool, 0444);
> +MODULE_PARM_DESC(enable_virtual_hw, "Enable/Disable virtual hardware mode(vblank-less mode)");
> +
>  DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
>  
>  static void vkms_release(struct drm_device *dev)
> @@ -98,6 +102,7 @@ static int vkms_config_show(struct seq_file *m, void *data)
>  	seq_printf(m, "writeback=%d\n", vkmsdev->config->writeback);
>  	seq_printf(m, "cursor=%d\n", vkmsdev->config->cursor);
>  	seq_printf(m, "overlay=%d\n", vkmsdev->config->overlay);
> +	seq_printf(m, "virtual_hw=%d\n", vkmsdev->config->virtual_hw);
>  
>  	return 0;
>  }
> @@ -191,10 +196,12 @@ static int vkms_create(struct vkms_config *config)
>  		goto out_devres;
>  	}
>  
> -	ret = drm_vblank_init(&vkms_device->drm, 1);
> -	if (ret) {
> -		DRM_ERROR("Failed to vblank\n");
> -		goto out_devres;
> +	if (!vkms_device->config->virtual_hw) {
> +		ret = drm_vblank_init(&vkms_device->drm, 1);
> +		if (ret) {
> +			DRM_ERROR("Failed to vblank\n");
> +			goto out_devres;
> +		}
>  	}
>  
>  	ret = vkms_modeset_init(vkms_device);
> @@ -229,6 +236,7 @@ static int __init vkms_init(void)
>  	config->cursor = enable_cursor;
>  	config->writeback = enable_writeback;
>  	config->overlay = enable_overlay;
> +	config->virtual_hw = enable_virtual_hw;
>  
>  	return vkms_create(config);
>  }
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 01beba424f18..770594e07f0e 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -92,6 +92,7 @@ struct vkms_config {
>  	bool writeback;
>  	bool cursor;
>  	bool overlay;
> +	bool virtual_hw;
>  	/* only set when instantiated */
>  	struct vkms_device *dev;
>  };
> @@ -136,6 +137,7 @@ int vkms_composer_common(struct vkms_crtc_state *crtc_state, struct vkms_output
>  			 bool wb_pending, uint32_t *crcs);
>  void vkms_composer_worker(struct work_struct *work);
>  void vkms_set_composer(struct vkms_output *out, bool enabled);
> +void vkms_crtc_composer(struct vkms_crtc_state *crtc_state);
>  
>  /* Writeback */
>  int vkms_enable_writeback_connector(struct vkms_device *vkmsdev);
> -- 
> 2.31.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-08-03 20:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-01 16:01 [PATCH V5 0/2] drm/vkms: Add virtual hardware module Sumera Priyadarsini
2021-08-01 16:03 ` [PATCH V5 1/2] drm/vkms: Refactor vkms_composer_worker() to prep for virtual_hw mode Sumera Priyadarsini
2021-08-01 16:03 ` [PATCH V5 2/2] drm/vkms: Add support for virtual hardware mode Sumera Priyadarsini
2021-08-03 19:35   ` Melissa Wen

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox