[PATCH v3 0/4] drm/vkms: add overlay plane support

[PATCH v3 0/4] drm/vkms: add overlay plane support

[Melissa Wen]

Adding support to overlay type in addition to primary and cursor plane.
The planes composition relies on the z order of the active planes and
only occurs if there is a primary plane (as in the current behavior).

The first patch switches the function of initializing planes from
drm_universal_plane_init to drmm_universal_plane_alloc. It aims to
improve aspects of allocation and cleanup operations, leaving it to the
DRM infrastructure.

The second patch generalizes variables and functions names to refer to
any kind of plane, not only cursor. The goal is to reuse them for
blending overlay and cursor planes to primary.

The third patch enables the plane composition to selct the correct pixel
blending operation according to the plane format (XRGB8888 or ARGB8888).

The last patch creates a module option to enable overlay, and includes
overlay to supported types of plane. When the overlay option is enabled,
one overlay plane is initialized (plus primary and cursor) and it is
included in the planes composition.

This work preserves the current results of IGT tests: kms_cursor_crc;
kms_flip and kms_writeback. In addition, subtests related to overlay in
kms_atomic and kms_plane_cursor start to pass (pointed out in the commit
message).

---
v2:

- Drop unnecessary changes that init crtc without cursor (Daniel)
- Replace function to initialize planes (Daniel)
- Add proper pixel blending op according to the plane format (Daniel)

v3:

- Proper use of the variable funcs (kernel bot)
- Adjust the patch series format

Melissa Wen (4):
  drm/vkms: init plane using drmm_universal_plane_alloc
  drm/vkms: rename cursor to plane on ops of planes composition
  drm/vkms: add XRGB planes composition
  drm/vkms: add overlay support

 drivers/gpu/drm/vkms/vkms_composer.c | 67 ++++++++++++++++++----------
 drivers/gpu/drm/vkms/vkms_drv.c      |  5 +++
 drivers/gpu/drm/vkms/vkms_drv.h      |  9 +++-
 drivers/gpu/drm/vkms/vkms_output.c   | 28 ++++++------
 drivers/gpu/drm/vkms/vkms_plane.c    | 50 +++++++++++----------
 5 files changed, 96 insertions(+), 63 deletions(-)

-- 
2.30.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 1/4] drm/vkms: init plane using drmm_universal_plane_alloc

[Melissa Wen]

By using drmm_universal_plane_alloc instead of
drm_universal_plane_init, we let the DRM infrastructure handles
resource allocation and cleanup. We can also get rid of some
code repetitions for plane cleanup, improving code maintainability
in vkms.

Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
---
 drivers/gpu/drm/vkms/vkms_drv.h    |  8 ++++++--
 drivers/gpu/drm/vkms/vkms_output.c | 19 +++++--------------
 drivers/gpu/drm/vkms/vkms_plane.c  | 29 +++++++++++------------------
 3 files changed, 22 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 35540c7c4416..70fb79621617 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -37,6 +37,10 @@ struct vkms_plane_state {
 	struct vkms_composer *composer;
 };
 
+struct vkms_plane {
+	struct drm_plane base;
+};
+
 /**
  * vkms_crtc_state - Driver specific CRTC state
  * @base: base CRTC state
@@ -114,8 +118,8 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
 
 int vkms_output_init(struct vkms_device *vkmsdev, int index);
 
-struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,
-				  enum drm_plane_type type, int index);
+struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
+				   enum drm_plane_type type, int index);
 
 /* CRC Support */
 const char *const *vkms_get_crc_sources(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index f5f6f15c362c..6979fbc7f821 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -39,7 +39,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
 	struct drm_connector *connector = &output->connector;
 	struct drm_encoder *encoder = &output->encoder;
 	struct drm_crtc *crtc = &output->crtc;
-	struct drm_plane *primary, *cursor = NULL;
+	struct vkms_plane *primary, *cursor = NULL;
 	int ret;
 	int writeback;
 
@@ -49,15 +49,13 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
 
 	if (vkmsdev->config->cursor) {
 		cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index);
-		if (IS_ERR(cursor)) {
-			ret = PTR_ERR(cursor);
-			goto err_cursor;
-		}
+		if (IS_ERR(cursor))
+			return PTR_ERR(cursor);
 	}
 
-	ret = vkms_crtc_init(dev, crtc, primary, cursor);
+	ret = vkms_crtc_init(dev, crtc, &primary->base, &cursor->base);
 	if (ret)
-		goto err_crtc;
+		return ret;
 
 	ret = drm_connector_init(dev, connector, &vkms_connector_funcs,
 				 DRM_MODE_CONNECTOR_VIRTUAL);
@@ -100,12 +98,5 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
 err_connector:
 	drm_crtc_cleanup(crtc);
 
-err_crtc:
-	if (vkmsdev->config->cursor)
-		drm_plane_cleanup(cursor);
-
-err_cursor:
-	drm_plane_cleanup(primary);
-
 	return ret;
 }
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
index 6d310d31b75d..135140f8e87a 100644
--- a/drivers/gpu/drm/vkms/vkms_plane.c
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -86,7 +86,6 @@ static void vkms_plane_reset(struct drm_plane *plane)
 static const struct drm_plane_funcs vkms_plane_funcs = {
 	.update_plane		= drm_atomic_helper_update_plane,
 	.disable_plane		= drm_atomic_helper_disable_plane,
-	.destroy		= drm_plane_cleanup,
 	.reset			= vkms_plane_reset,
 	.atomic_duplicate_state = vkms_plane_duplicate_state,
 	.atomic_destroy_state	= vkms_plane_destroy_state,
@@ -191,18 +190,14 @@ static const struct drm_plane_helper_funcs vkms_primary_helper_funcs = {
 	.cleanup_fb		= vkms_cleanup_fb,
 };
 
-struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,
-				  enum drm_plane_type type, int index)
+struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
+				   enum drm_plane_type type, int index)
 {
 	struct drm_device *dev = &vkmsdev->drm;
 	const struct drm_plane_helper_funcs *funcs;
-	struct drm_plane *plane;
+	struct vkms_plane *plane;
 	const u32 *formats;
-	int ret, nformats;
-
-	plane = kzalloc(sizeof(*plane), GFP_KERNEL);
-	if (!plane)
-		return ERR_PTR(-ENOMEM);
+	int nformats;
 
 	if (type == DRM_PLANE_TYPE_CURSOR) {
 		formats = vkms_cursor_formats;
@@ -214,16 +209,14 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,
 		funcs = &vkms_primary_helper_funcs;
 	}
 
-	ret = drm_universal_plane_init(dev, plane, 1 << index,
-				       &vkms_plane_funcs,
-				       formats, nformats,
-				       NULL, type, NULL);
-	if (ret) {
-		kfree(plane);
-		return ERR_PTR(ret);
-	}
+	plane = drmm_universal_plane_alloc(dev, struct vkms_plane, base, 1 << index,
+					   &vkms_plane_funcs,
+					   formats, nformats,
+					   NULL, type, NULL);
+	if (IS_ERR(plane))
+		return plane;
 
-	drm_plane_helper_add(plane, funcs);
+	drm_plane_helper_add(&plane->base, funcs);
 
 	return plane;
 }
-- 
2.30.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 0/4] drm/vkms: add overlay plane support

[Melissa Wen]

Adding support to overlay type in addition to primary and cursor plane.
The planes composition relies on the z order of the active planes and
only occurs if there is a primary plane (as in the current behavior).

The first patch switches the function of initializing planes from
drm_universal_plane_init to drmm_universal_plane_alloc. It aims to
improve aspects of allocation and cleanup operations, leaving it to the
DRM infrastructure.

The second patch generalizes variables and functions names to refer to
any kind of plane, not only cursor. The goal is to reuse them for
blending overlay and cursor planes to primary.

The third patch enables the plane composition to selct the correct pixel
blending operation according to the plane format (XRGB8888 or ARGB8888).

The last patch creates a module option to enable overlay, and includes
overlay to supported types of plane. When the overlay option is enabled,
one overlay plane is initialized (plus primary and cursor) and it is
included in the planes composition.

This work preserves the current results of IGT tests: kms_cursor_crc;
kms_flip and kms_writeback. In addition, subtests related to overlay in
kms_atomic and kms_plane_cursor start to pass (pointed out in the commit
message).

---
v2:

- Drop unnecessary changes that init crtc without cursor (Daniel)
- Replace function to initialize planes (Daniel)
- Add proper pixel blending op according to the plane format (Daniel)

v3:

- Proper use of the variable funcs (kernel bot)
- Adjust the patch series format

Melissa Wen (4):
  drm/vkms: init plane using drmm_universal_plane_alloc
  drm/vkms: rename cursor to plane on ops of planes composition
  drm/vkms: add XRGB planes composition
  drm/vkms: add overlay support

 drivers/gpu/drm/vkms/vkms_composer.c | 67 ++++++++++++++++++----------
 drivers/gpu/drm/vkms/vkms_drv.c      |  5 +++
 drivers/gpu/drm/vkms/vkms_drv.h      |  9 +++-
 drivers/gpu/drm/vkms/vkms_output.c   | 28 ++++++------
 drivers/gpu/drm/vkms/vkms_plane.c    | 50 +++++++++++----------
 5 files changed, 96 insertions(+), 63 deletions(-)

-- 
2.30.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 2/4] drm/vkms: rename cursor to plane on ops of planes composition

[Melissa Wen]

Generalize variables and function names used for planes composition
(from cursor to plane), since we will reuse the operations for both
cursor and overlay types.

No functional change.

Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
---
 drivers/gpu/drm/vkms/vkms_composer.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index 66c6842d70db..be8f1d33c645 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -125,26 +125,26 @@ static void blend(void *vaddr_dst, void *vaddr_src,
 	}
 }
 
-static void compose_cursor(struct vkms_composer *cursor_composer,
-			   struct vkms_composer *primary_composer,
+static void compose_planes(struct vkms_composer *primary_composer,
+			   struct vkms_composer *plane_composer,
 			   void *vaddr_out)
 {
-	struct drm_gem_object *cursor_obj;
-	struct drm_gem_shmem_object *cursor_shmem_obj;
+	struct drm_gem_object *plane_obj;
+	struct drm_gem_shmem_object *plane_shmem_obj;
 
-	cursor_obj = drm_gem_fb_get_obj(&cursor_composer->fb, 0);
-	cursor_shmem_obj = to_drm_gem_shmem_obj(cursor_obj);
+	plane_obj = drm_gem_fb_get_obj(&plane_composer->fb, 0);
+	plane_shmem_obj = to_drm_gem_shmem_obj(plane_obj);
 
-	if (WARN_ON(!cursor_shmem_obj->vaddr))
+	if (WARN_ON(!plane_shmem_obj->vaddr))
 		return;
 
-	blend(vaddr_out, cursor_shmem_obj->vaddr,
-	      primary_composer, cursor_composer);
+	blend(vaddr_out, plane_shmem_obj->vaddr,
+	      primary_composer, plane_composer);
 }
 
-static int compose_planes(void **vaddr_out,
-			  struct vkms_composer *primary_composer,
-			  struct vkms_composer *cursor_composer)
+static int composite(void **vaddr_out,
+		     struct vkms_composer *primary_composer,
+		     struct vkms_composer *cursor_composer)
 {
 	struct drm_framebuffer *fb = &primary_composer->fb;
 	struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0);
@@ -164,7 +164,7 @@ static int compose_planes(void **vaddr_out,
 	memcpy(*vaddr_out, shmem_obj->vaddr, shmem_obj->base.size);
 
 	if (cursor_composer)
-		compose_cursor(cursor_composer, primary_composer, *vaddr_out);
+		compose_planes(primary_composer, cursor_composer, *vaddr_out);
 
 	return 0;
 }
@@ -222,7 +222,7 @@ void vkms_composer_worker(struct work_struct *work)
 	if (wb_pending)
 		vaddr_out = crtc_state->active_writeback;
 
-	ret = compose_planes(&vaddr_out, primary_composer, cursor_composer);
+	ret = composite(&vaddr_out, primary_composer, cursor_composer);
 	if (ret) {
 		if (ret == -EINVAL && !wb_pending)
 			kfree(vaddr_out);
-- 
2.30.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 3/4] drm/vkms: add XRGB planes composition

[Melissa Wen]

Add support for composing XRGB888 planes in addition to the
ARGB8888 format. In the case of an XRGB plane at the top, the
composition consists of just copying the RGB values of a
pixel from src to dst, without the need for alpha blending
operations for each pixel.

Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
---
 drivers/gpu/drm/vkms/vkms_composer.c | 22 ++++++++++++++++++----
 drivers/gpu/drm/vkms/vkms_plane.c    |  7 ++++---
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index be8f1d33c645..7fe1fdb3af39 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -4,6 +4,7 @@
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_fourcc.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_gem_shmem_helper.h>
 #include <drm/drm_vblank.h>
@@ -76,6 +77,11 @@ static void alpha_blending(const u8 *argb_src, u8 *argb_dst)
 	argb_dst[3] = 0xFF;
 }
 
+static void x_blending(const u8 *xrgb_src, u8 *xrgb_dst)
+{
+	memcpy(xrgb_dst, xrgb_src, sizeof(u8) * 3);
+}
+
 /**
  * blend - blend value at vaddr_src with value at vaddr_dst
  * @vaddr_dst: destination address
@@ -91,7 +97,8 @@ static void alpha_blending(const u8 *argb_src, u8 *argb_dst)
  */
 static void blend(void *vaddr_dst, void *vaddr_src,
 		  struct vkms_composer *dst_composer,
-		  struct vkms_composer *src_composer)
+		  struct vkms_composer *src_composer,
+		  void (*pixel_blend)(const u8 *, u8 *))
 {
 	int i, j, j_dst, i_dst;
 	int offset_src, offset_dst;
@@ -119,7 +126,7 @@ static void blend(void *vaddr_dst, void *vaddr_src,
 
 			pixel_src = (u8 *)(vaddr_src + offset_src);
 			pixel_dst = (u8 *)(vaddr_dst + offset_dst);
-			alpha_blending(pixel_src, pixel_dst);
+			pixel_blend(pixel_src, pixel_dst);
 		}
 		i_dst++;
 	}
@@ -131,6 +138,8 @@ static void compose_planes(struct vkms_composer *primary_composer,
 {
 	struct drm_gem_object *plane_obj;
 	struct drm_gem_shmem_object *plane_shmem_obj;
+	struct drm_framebuffer *fb = &plane_composer->fb;
+	void (*pixel_blend)(const u8 *p_src, u8 *p_dst);
 
 	plane_obj = drm_gem_fb_get_obj(&plane_composer->fb, 0);
 	plane_shmem_obj = to_drm_gem_shmem_obj(plane_obj);
@@ -138,8 +147,13 @@ static void compose_planes(struct vkms_composer *primary_composer,
 	if (WARN_ON(!plane_shmem_obj->vaddr))
 		return;
 
-	blend(vaddr_out, plane_shmem_obj->vaddr,
-	      primary_composer, plane_composer);
+	if (fb->format->format == DRM_FORMAT_ARGB8888)
+		pixel_blend = &alpha_blending;
+	else
+		pixel_blend = &x_blending;
+
+	blend(vaddr_out, plane_shmem_obj->vaddr, primary_composer,
+	      plane_composer, pixel_blend);
 }
 
 static int composite(void **vaddr_out,
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
index 135140f8e87a..da4251aff67f 100644
--- a/drivers/gpu/drm/vkms/vkms_plane.c
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -16,8 +16,9 @@ static const u32 vkms_formats[] = {
 	DRM_FORMAT_XRGB8888,
 };
 
-static const u32 vkms_cursor_formats[] = {
+static const u32 vkms_plane_formats[] = {
 	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_XRGB8888
 };
 
 static struct drm_plane_state *
@@ -200,8 +201,8 @@ struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
 	int nformats;
 
 	if (type == DRM_PLANE_TYPE_CURSOR) {
-		formats = vkms_cursor_formats;
-		nformats = ARRAY_SIZE(vkms_cursor_formats);
+		formats = vkms_plane_formats;
+		nformats = ARRAY_SIZE(vkms_plane_formats);
 		funcs = &vkms_primary_helper_funcs;
 	} else {
 		formats = vkms_formats;
-- 
2.30.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 4/4] drm/vkms: add overlay support

[Melissa Wen]

Add support to overlay plane, in addition to primary and cursor
planes. In this approach, the plane composition still requires an
active primary plane and planes are composed associatively in the
order: (primary <- overlay) <- cursor

It enables to run the following IGT tests successfully:
- kms_plane_cursor:
  - pipe-A-[overlay, primary, viewport]-size-[64, 128, 256]
- kms_atomic:
  - plane-overlay-legacy
and preserves the successful execution of kms_cursor_crc,
kms_writeback and kms_flip

Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
---
 drivers/gpu/drm/vkms/vkms_composer.c | 27 +++++++++++++++++----------
 drivers/gpu/drm/vkms/vkms_drv.c      |  5 +++++
 drivers/gpu/drm/vkms/vkms_drv.h      |  1 +
 drivers/gpu/drm/vkms/vkms_output.c   | 11 ++++++++++-
 drivers/gpu/drm/vkms/vkms_plane.c    | 14 +++++++++++---
 5 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index 7fe1fdb3af39..73ce1d381737 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -158,11 +158,12 @@ static void compose_planes(struct vkms_composer *primary_composer,
 
 static int composite(void **vaddr_out,
 		     struct vkms_composer *primary_composer,
-		     struct vkms_composer *cursor_composer)
+		     struct vkms_crtc_state *crtc_state)
 {
 	struct drm_framebuffer *fb = &primary_composer->fb;
 	struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0);
 	struct drm_gem_shmem_object *shmem_obj = to_drm_gem_shmem_obj(gem_obj);
+	int i;
 
 	if (!*vaddr_out) {
 		*vaddr_out = kzalloc(shmem_obj->base.size, GFP_KERNEL);
@@ -177,8 +178,14 @@ static int composite(void **vaddr_out,
 
 	memcpy(*vaddr_out, shmem_obj->vaddr, shmem_obj->base.size);
 
-	if (cursor_composer)
-		compose_planes(primary_composer, cursor_composer, *vaddr_out);
+	/* If there are other planes besides primary, we consider the active
+	 * planes should be in z-order and compose them associatively:
+	 * ((primary <- overlay) <- cursor)
+	 */
+	for (i = 1; i < crtc_state->num_active_planes; i++)
+		compose_planes(primary_composer,
+			       crtc_state->active_planes[i]->composer,
+			       *vaddr_out);
 
 	return 0;
 }
@@ -200,7 +207,7 @@ void vkms_composer_worker(struct work_struct *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_composer *cursor_composer = NULL;
+	struct vkms_plane_state *act_plane = NULL;
 	bool crc_pending, wb_pending;
 	void *vaddr_out = NULL;
 	u32 crc32 = 0;
@@ -224,11 +231,11 @@ void vkms_composer_worker(struct work_struct *work)
 	if (!crc_pending)
 		return;
 
-	if (crtc_state->num_active_planes >= 1)
-		primary_composer = crtc_state->active_planes[0]->composer;
-
-	if (crtc_state->num_active_planes == 2)
-		cursor_composer = crtc_state->active_planes[1]->composer;
+	if (crtc_state->num_active_planes >= 1) {
+		act_plane = crtc_state->active_planes[0];
+		if (act_plane->base.plane->type == DRM_PLANE_TYPE_PRIMARY)
+			primary_composer = act_plane->composer;
+	}
 
 	if (!primary_composer)
 		return;
@@ -236,7 +243,7 @@ void vkms_composer_worker(struct work_struct *work)
 	if (wb_pending)
 		vaddr_out = crtc_state->active_writeback;
 
-	ret = composite(&vaddr_out, primary_composer, cursor_composer);
+	ret = composite(&vaddr_out, primary_composer, crtc_state);
 	if (ret) {
 		if (ret == -EINVAL && !wb_pending)
 			kfree(vaddr_out);
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 2173b82606f6..027ffe759440 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -44,6 +44,10 @@ static bool enable_writeback = true;
 module_param_named(enable_writeback, enable_writeback, bool, 0444);
 MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector support");
 
+static bool enable_overlay;
+module_param_named(enable_overlay, enable_overlay, bool, 0444);
+MODULE_PARM_DESC(enable_overlay, "Enable/Disable overlay support");
+
 DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
 
 static void vkms_release(struct drm_device *dev)
@@ -198,6 +202,7 @@ static int __init vkms_init(void)
 
 	config->cursor = enable_cursor;
 	config->writeback = enable_writeback;
+	config->overlay = enable_overlay;
 
 	return vkms_create(config);
 }
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 70fb79621617..ac8c9c2fa4ed 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -89,6 +89,7 @@ struct vkms_device;
 struct vkms_config {
 	bool writeback;
 	bool cursor;
+	bool overlay;
 	/* only set when instantiated */
 	struct vkms_device *dev;
 };
diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index 6979fbc7f821..04406bd3ff02 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -39,7 +39,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
 	struct drm_connector *connector = &output->connector;
 	struct drm_encoder *encoder = &output->encoder;
 	struct drm_crtc *crtc = &output->crtc;
-	struct vkms_plane *primary, *cursor = NULL;
+	struct vkms_plane *primary, *cursor = NULL, *overlay = NULL;
 	int ret;
 	int writeback;
 
@@ -47,6 +47,15 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
 	if (IS_ERR(primary))
 		return PTR_ERR(primary);
 
+	if (vkmsdev->config->overlay) {
+		overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY, index);
+		if (IS_ERR(overlay))
+			return PTR_ERR(overlay);
+
+		if (!overlay->base.possible_crtcs)
+			overlay->base.possible_crtcs = drm_crtc_mask(crtc);
+	}
+
 	if (vkmsdev->config->cursor) {
 		cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index);
 		if (IS_ERR(cursor))
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
index da4251aff67f..8be9eab41ea0 100644
--- a/drivers/gpu/drm/vkms/vkms_plane.c
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -133,7 +133,7 @@ static int vkms_plane_atomic_check(struct drm_plane *plane,
 	if (IS_ERR(crtc_state))
 		return PTR_ERR(crtc_state);
 
-	if (plane->type == DRM_PLANE_TYPE_CURSOR)
+	if (plane->type != DRM_PLANE_TYPE_PRIMARY)
 		can_position = true;
 
 	ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
@@ -200,11 +200,19 @@ struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
 	const u32 *formats;
 	int nformats;
 
-	if (type == DRM_PLANE_TYPE_CURSOR) {
+	switch (type) {
+	case DRM_PLANE_TYPE_PRIMARY:
+		formats = vkms_formats;
+		nformats = ARRAY_SIZE(vkms_formats);
+		funcs = &vkms_primary_helper_funcs;
+		break;
+	case DRM_PLANE_TYPE_CURSOR:
+	case DRM_PLANE_TYPE_OVERLAY:
 		formats = vkms_plane_formats;
 		nformats = ARRAY_SIZE(vkms_plane_formats);
 		funcs = &vkms_primary_helper_funcs;
-	} else {
+		break;
+	default:
 		formats = vkms_formats;
 		nformats = ARRAY_SIZE(vkms_formats);
 		funcs = &vkms_primary_helper_funcs;
-- 
2.30.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/4] drm/vkms: init plane using drmm_universal_plane_alloc

[Daniel Vetter]

On Tue, Apr 13, 2021 at 04:50:08AM -0300, Melissa Wen wrote:
> By using drmm_universal_plane_alloc instead of
> drm_universal_plane_init, we let the DRM infrastructure handles
> resource allocation and cleanup. We can also get rid of some
> code repetitions for plane cleanup, improving code maintainability
> in vkms.
> 
> Signed-off-by: Melissa Wen <melissa.srw@gmail.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/vkms/vkms_drv.h    |  8 ++++++--
>  drivers/gpu/drm/vkms/vkms_output.c | 19 +++++--------------
>  drivers/gpu/drm/vkms/vkms_plane.c  | 29 +++++++++++------------------
>  3 files changed, 22 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 35540c7c4416..70fb79621617 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -37,6 +37,10 @@ struct vkms_plane_state {
>  	struct vkms_composer *composer;
>  };
>  
> +struct vkms_plane {
> +	struct drm_plane base;
> +};
> +
>  /**
>   * vkms_crtc_state - Driver specific CRTC state
>   * @base: base CRTC state
> @@ -114,8 +118,8 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>  
>  int vkms_output_init(struct vkms_device *vkmsdev, int index);
>  
> -struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> -				  enum drm_plane_type type, int index);
> +struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> +				   enum drm_plane_type type, int index);
>  
>  /* CRC Support */
>  const char *const *vkms_get_crc_sources(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index f5f6f15c362c..6979fbc7f821 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -39,7 +39,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
>  	struct drm_connector *connector = &output->connector;
>  	struct drm_encoder *encoder = &output->encoder;
>  	struct drm_crtc *crtc = &output->crtc;
> -	struct drm_plane *primary, *cursor = NULL;
> +	struct vkms_plane *primary, *cursor = NULL;
>  	int ret;
>  	int writeback;
>  
> @@ -49,15 +49,13 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
>  
>  	if (vkmsdev->config->cursor) {
>  		cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index);
> -		if (IS_ERR(cursor)) {
> -			ret = PTR_ERR(cursor);
> -			goto err_cursor;
> -		}
> +		if (IS_ERR(cursor))
> +			return PTR_ERR(cursor);
>  	}
>  
> -	ret = vkms_crtc_init(dev, crtc, primary, cursor);
> +	ret = vkms_crtc_init(dev, crtc, &primary->base, &cursor->base);
>  	if (ret)
> -		goto err_crtc;
> +		return ret;
>  
>  	ret = drm_connector_init(dev, connector, &vkms_connector_funcs,
>  				 DRM_MODE_CONNECTOR_VIRTUAL);
> @@ -100,12 +98,5 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
>  err_connector:
>  	drm_crtc_cleanup(crtc);
>  
> -err_crtc:
> -	if (vkmsdev->config->cursor)
> -		drm_plane_cleanup(cursor);
> -
> -err_cursor:
> -	drm_plane_cleanup(primary);
> -
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> index 6d310d31b75d..135140f8e87a 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -86,7 +86,6 @@ static void vkms_plane_reset(struct drm_plane *plane)
>  static const struct drm_plane_funcs vkms_plane_funcs = {
>  	.update_plane		= drm_atomic_helper_update_plane,
>  	.disable_plane		= drm_atomic_helper_disable_plane,
> -	.destroy		= drm_plane_cleanup,
>  	.reset			= vkms_plane_reset,
>  	.atomic_duplicate_state = vkms_plane_duplicate_state,
>  	.atomic_destroy_state	= vkms_plane_destroy_state,
> @@ -191,18 +190,14 @@ static const struct drm_plane_helper_funcs vkms_primary_helper_funcs = {
>  	.cleanup_fb		= vkms_cleanup_fb,
>  };
>  
> -struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> -				  enum drm_plane_type type, int index)
> +struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> +				   enum drm_plane_type type, int index)
>  {
>  	struct drm_device *dev = &vkmsdev->drm;
>  	const struct drm_plane_helper_funcs *funcs;
> -	struct drm_plane *plane;
> +	struct vkms_plane *plane;
>  	const u32 *formats;
> -	int ret, nformats;
> -
> -	plane = kzalloc(sizeof(*plane), GFP_KERNEL);
> -	if (!plane)
> -		return ERR_PTR(-ENOMEM);
> +	int nformats;
>  
>  	if (type == DRM_PLANE_TYPE_CURSOR) {
>  		formats = vkms_cursor_formats;
> @@ -214,16 +209,14 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,
>  		funcs = &vkms_primary_helper_funcs;
>  	}
>  
> -	ret = drm_universal_plane_init(dev, plane, 1 << index,
> -				       &vkms_plane_funcs,
> -				       formats, nformats,
> -				       NULL, type, NULL);
> -	if (ret) {
> -		kfree(plane);
> -		return ERR_PTR(ret);
> -	}
> +	plane = drmm_universal_plane_alloc(dev, struct vkms_plane, base, 1 << index,
> +					   &vkms_plane_funcs,
> +					   formats, nformats,
> +					   NULL, type, NULL);
> +	if (IS_ERR(plane))
> +		return plane;
>  
> -	drm_plane_helper_add(plane, funcs);
> +	drm_plane_helper_add(&plane->base, funcs);
>  
>  	return plane;
>  }
> -- 
> 2.30.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/4] drm/vkms: rename cursor to plane on ops of planes composition

[Daniel Vetter]

On Tue, Apr 13, 2021 at 04:53:43AM -0300, Melissa Wen wrote:
> Generalize variables and function names used for planes composition
> (from cursor to plane), since we will reuse the operations for both
> cursor and overlay types.
> 
> No functional change.
> 
> Signed-off-by: Melissa Wen <melissa.srw@gmail.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/vkms/vkms_composer.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
> index 66c6842d70db..be8f1d33c645 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -125,26 +125,26 @@ static void blend(void *vaddr_dst, void *vaddr_src,
>  	}
>  }
>  
> -static void compose_cursor(struct vkms_composer *cursor_composer,
> -			   struct vkms_composer *primary_composer,
> +static void compose_planes(struct vkms_composer *primary_composer,
> +			   struct vkms_composer *plane_composer,
>  			   void *vaddr_out)
>  {
> -	struct drm_gem_object *cursor_obj;
> -	struct drm_gem_shmem_object *cursor_shmem_obj;
> +	struct drm_gem_object *plane_obj;
> +	struct drm_gem_shmem_object *plane_shmem_obj;
>  
> -	cursor_obj = drm_gem_fb_get_obj(&cursor_composer->fb, 0);
> -	cursor_shmem_obj = to_drm_gem_shmem_obj(cursor_obj);
> +	plane_obj = drm_gem_fb_get_obj(&plane_composer->fb, 0);
> +	plane_shmem_obj = to_drm_gem_shmem_obj(plane_obj);
>  
> -	if (WARN_ON(!cursor_shmem_obj->vaddr))
> +	if (WARN_ON(!plane_shmem_obj->vaddr))
>  		return;
>  
> -	blend(vaddr_out, cursor_shmem_obj->vaddr,
> -	      primary_composer, cursor_composer);
> +	blend(vaddr_out, plane_shmem_obj->vaddr,
> +	      primary_composer, plane_composer);
>  }
>  
> -static int compose_planes(void **vaddr_out,
> -			  struct vkms_composer *primary_composer,
> -			  struct vkms_composer *cursor_composer)
> +static int composite(void **vaddr_out,
> +		     struct vkms_composer *primary_composer,
> +		     struct vkms_composer *cursor_composer)
>  {
>  	struct drm_framebuffer *fb = &primary_composer->fb;
>  	struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0);
> @@ -164,7 +164,7 @@ static int compose_planes(void **vaddr_out,
>  	memcpy(*vaddr_out, shmem_obj->vaddr, shmem_obj->base.size);
>  
>  	if (cursor_composer)
> -		compose_cursor(cursor_composer, primary_composer, *vaddr_out);
> +		compose_planes(primary_composer, cursor_composer, *vaddr_out);
>  
>  	return 0;
>  }
> @@ -222,7 +222,7 @@ void vkms_composer_worker(struct work_struct *work)
>  	if (wb_pending)
>  		vaddr_out = crtc_state->active_writeback;
>  
> -	ret = compose_planes(&vaddr_out, primary_composer, cursor_composer);
> +	ret = composite(&vaddr_out, primary_composer, cursor_composer);
>  	if (ret) {
>  		if (ret == -EINVAL && !wb_pending)
>  			kfree(vaddr_out);
> -- 
> 2.30.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 3/4] drm/vkms: add XRGB planes composition

[Daniel Vetter]

On Tue, Apr 13, 2021 at 04:54:52AM -0300, Melissa Wen wrote:
> Add support for composing XRGB888 planes in addition to the
> ARGB8888 format. In the case of an XRGB plane at the top, the
> composition consists of just copying the RGB values of a
> pixel from src to dst, without the need for alpha blending
> operations for each pixel.
> 
> Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
> ---
>  drivers/gpu/drm/vkms/vkms_composer.c | 22 ++++++++++++++++++----
>  drivers/gpu/drm/vkms/vkms_plane.c    |  7 ++++---
>  2 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
> index be8f1d33c645..7fe1fdb3af39 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -4,6 +4,7 @@
>  
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_fourcc.h>
>  #include <drm/drm_gem_framebuffer_helper.h>
>  #include <drm/drm_gem_shmem_helper.h>
>  #include <drm/drm_vblank.h>
> @@ -76,6 +77,11 @@ static void alpha_blending(const u8 *argb_src, u8 *argb_dst)
>  	argb_dst[3] = 0xFF;
>  }
>  
> +static void x_blending(const u8 *xrgb_src, u8 *xrgb_dst)
> +{
> +	memcpy(xrgb_dst, xrgb_src, sizeof(u8) * 3);

Don't we need to clear the alpha channel, or is that done already when we
allocate the buffer? Looking at the code we have I think we have a few
more bugs there to fix.

I also just realized that we don't support when the primary plane isn't
exactly matching our output size. So that's another thing to maybe fix -
with your new x_blending here what we could do is essentially blend the
primary plane onto the black background (allocated with all zeros) like we
blend the other planes.

Minimally I'd add a comment above that we rely on alpha being cleared in
the target buffer or something like that for stable crc values.

Anyway, work for another patch set.

> +}
> +
>  /**
>   * blend - blend value at vaddr_src with value at vaddr_dst
>   * @vaddr_dst: destination address
> @@ -91,7 +97,8 @@ static void alpha_blending(const u8 *argb_src, u8 *argb_dst)
>   */
>  static void blend(void *vaddr_dst, void *vaddr_src,
>  		  struct vkms_composer *dst_composer,
> -		  struct vkms_composer *src_composer)
> +		  struct vkms_composer *src_composer,
> +		  void (*pixel_blend)(const u8 *, u8 *))
>  {
>  	int i, j, j_dst, i_dst;
>  	int offset_src, offset_dst;
> @@ -119,7 +126,7 @@ static void blend(void *vaddr_dst, void *vaddr_src,
>  
>  			pixel_src = (u8 *)(vaddr_src + offset_src);
>  			pixel_dst = (u8 *)(vaddr_dst + offset_dst);
> -			alpha_blending(pixel_src, pixel_dst);
> +			pixel_blend(pixel_src, pixel_dst);

Since it's all one file, did you check whether gcc inlines the two
functions calls here? If we do actual function calls for each blend it's
going to be a lot slower. Maybe worth checking, and perhaps we need to
throw some ineline hints (like always_inline or something like that) on
both the blend function and the alpha_blending and x_blending helpers.

Cursor is generally tiny, but when we start "blending" the primary plane
into the black background, then performance really starts to matter.

Anyway that's all stuff for later tuning.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  		}
>  		i_dst++;
>  	}
> @@ -131,6 +138,8 @@ static void compose_planes(struct vkms_composer *primary_composer,
>  {
>  	struct drm_gem_object *plane_obj;
>  	struct drm_gem_shmem_object *plane_shmem_obj;
> +	struct drm_framebuffer *fb = &plane_composer->fb;
> +	void (*pixel_blend)(const u8 *p_src, u8 *p_dst);
>  
>  	plane_obj = drm_gem_fb_get_obj(&plane_composer->fb, 0);
>  	plane_shmem_obj = to_drm_gem_shmem_obj(plane_obj);
> @@ -138,8 +147,13 @@ static void compose_planes(struct vkms_composer *primary_composer,
>  	if (WARN_ON(!plane_shmem_obj->vaddr))
>  		return;
>  
> -	blend(vaddr_out, plane_shmem_obj->vaddr,
> -	      primary_composer, plane_composer);
> +	if (fb->format->format == DRM_FORMAT_ARGB8888)
> +		pixel_blend = &alpha_blending;
> +	else
> +		pixel_blend = &x_blending;
> +
> +	blend(vaddr_out, plane_shmem_obj->vaddr, primary_composer,
> +	      plane_composer, pixel_blend);
>  }
>  
>  static int composite(void **vaddr_out,
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> index 135140f8e87a..da4251aff67f 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -16,8 +16,9 @@ static const u32 vkms_formats[] = {
>  	DRM_FORMAT_XRGB8888,
>  };
>  
> -static const u32 vkms_cursor_formats[] = {
> +static const u32 vkms_plane_formats[] = {
>  	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_XRGB8888
>  };
>  
>  static struct drm_plane_state *
> @@ -200,8 +201,8 @@ struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
>  	int nformats;
>  
>  	if (type == DRM_PLANE_TYPE_CURSOR) {
> -		formats = vkms_cursor_formats;
> -		nformats = ARRAY_SIZE(vkms_cursor_formats);
> +		formats = vkms_plane_formats;
> +		nformats = ARRAY_SIZE(vkms_plane_formats);
>  		funcs = &vkms_primary_helper_funcs;
>  	} else {
>  		formats = vkms_formats;
> -- 
> 2.30.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 4/4] drm/vkms: add overlay support

[Daniel Vetter]

On Tue, Apr 13, 2021 at 04:56:02AM -0300, Melissa Wen wrote:
> Add support to overlay plane, in addition to primary and cursor
> planes. In this approach, the plane composition still requires an
> active primary plane and planes are composed associatively in the
> order: (primary <- overlay) <- cursor
> 
> It enables to run the following IGT tests successfully:
> - kms_plane_cursor:
>   - pipe-A-[overlay, primary, viewport]-size-[64, 128, 256]
> - kms_atomic:
>   - plane-overlay-legacy
> and preserves the successful execution of kms_cursor_crc,
> kms_writeback and kms_flip
> 
> Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
> ---
>  drivers/gpu/drm/vkms/vkms_composer.c | 27 +++++++++++++++++----------
>  drivers/gpu/drm/vkms/vkms_drv.c      |  5 +++++
>  drivers/gpu/drm/vkms/vkms_drv.h      |  1 +
>  drivers/gpu/drm/vkms/vkms_output.c   | 11 ++++++++++-
>  drivers/gpu/drm/vkms/vkms_plane.c    | 14 +++++++++++---
>  5 files changed, 44 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
> index 7fe1fdb3af39..73ce1d381737 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -158,11 +158,12 @@ static void compose_planes(struct vkms_composer *primary_composer,
>  
>  static int composite(void **vaddr_out,

Ok this was done in patch 2, but I think the names here need a bit
improvement. composite is a noun, not a verb, but this function does
stuff, so we need a verb. Also I feel like compose_planes (i.e. the
original name) reflects better what it actually does.

>  		     struct vkms_composer *primary_composer,
> -		     struct vkms_composer *cursor_composer)
> +		     struct vkms_crtc_state *crtc_state)
>  {
>  	struct drm_framebuffer *fb = &primary_composer->fb;
>  	struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0);
>  	struct drm_gem_shmem_object *shmem_obj = to_drm_gem_shmem_obj(gem_obj);
> +	int i;
>  
>  	if (!*vaddr_out) {
>  		*vaddr_out = kzalloc(shmem_obj->base.size, GFP_KERNEL);
> @@ -177,8 +178,14 @@ static int composite(void **vaddr_out,
>  
>  	memcpy(*vaddr_out, shmem_obj->vaddr, shmem_obj->base.size);
>  
> -	if (cursor_composer)
> -		compose_planes(primary_composer, cursor_composer, *vaddr_out);
> +	/* If there are other planes besides primary, we consider the active
> +	 * planes should be in z-order and compose them associatively:
> +	 * ((primary <- overlay) <- cursor)
> +	 */
> +	for (i = 1; i < crtc_state->num_active_planes; i++)
> +		compose_planes(primary_composer,

Ofc that then clashes with compose_planes here, but this function only
composes a single plane. So the plural plane_s_ is kinda wrong, and we
could just call this function compose_plane. I think with that bikeshed
this reads a bit better. So please adjust that in patch 2 (keep the r-b)
and this here also lgtm.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +			       crtc_state->active_planes[i]->composer,
> +			       *vaddr_out);
>  
>  	return 0;
>  }
> @@ -200,7 +207,7 @@ void vkms_composer_worker(struct work_struct *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_composer *cursor_composer = NULL;
> +	struct vkms_plane_state *act_plane = NULL;
>  	bool crc_pending, wb_pending;
>  	void *vaddr_out = NULL;
>  	u32 crc32 = 0;
> @@ -224,11 +231,11 @@ void vkms_composer_worker(struct work_struct *work)
>  	if (!crc_pending)
>  		return;
>  
> -	if (crtc_state->num_active_planes >= 1)
> -		primary_composer = crtc_state->active_planes[0]->composer;
> -
> -	if (crtc_state->num_active_planes == 2)
> -		cursor_composer = crtc_state->active_planes[1]->composer;
> +	if (crtc_state->num_active_planes >= 1) {
> +		act_plane = crtc_state->active_planes[0];
> +		if (act_plane->base.plane->type == DRM_PLANE_TYPE_PRIMARY)
> +			primary_composer = act_plane->composer;
> +	}
>  
>  	if (!primary_composer)
>  		return;
> @@ -236,7 +243,7 @@ void vkms_composer_worker(struct work_struct *work)
>  	if (wb_pending)
>  		vaddr_out = crtc_state->active_writeback;
>  
> -	ret = composite(&vaddr_out, primary_composer, cursor_composer);
> +	ret = composite(&vaddr_out, primary_composer, crtc_state);
>  	if (ret) {
>  		if (ret == -EINVAL && !wb_pending)
>  			kfree(vaddr_out);
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 2173b82606f6..027ffe759440 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -44,6 +44,10 @@ static bool enable_writeback = true;
>  module_param_named(enable_writeback, enable_writeback, bool, 0444);
>  MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector support");
>  
> +static bool enable_overlay;
> +module_param_named(enable_overlay, enable_overlay, bool, 0444);
> +MODULE_PARM_DESC(enable_overlay, "Enable/Disable overlay support");
> +
>  DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
>  
>  static void vkms_release(struct drm_device *dev)
> @@ -198,6 +202,7 @@ static int __init vkms_init(void)
>  
>  	config->cursor = enable_cursor;
>  	config->writeback = enable_writeback;
> +	config->overlay = enable_overlay;
>  
>  	return vkms_create(config);
>  }
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 70fb79621617..ac8c9c2fa4ed 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -89,6 +89,7 @@ struct vkms_device;
>  struct vkms_config {
>  	bool writeback;
>  	bool cursor;
> +	bool overlay;
>  	/* only set when instantiated */
>  	struct vkms_device *dev;
>  };
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index 6979fbc7f821..04406bd3ff02 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -39,7 +39,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
>  	struct drm_connector *connector = &output->connector;
>  	struct drm_encoder *encoder = &output->encoder;
>  	struct drm_crtc *crtc = &output->crtc;
> -	struct vkms_plane *primary, *cursor = NULL;
> +	struct vkms_plane *primary, *cursor = NULL, *overlay = NULL;
>  	int ret;
>  	int writeback;
>  
> @@ -47,6 +47,15 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
>  	if (IS_ERR(primary))
>  		return PTR_ERR(primary);
>  
> +	if (vkmsdev->config->overlay) {
> +		overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY, index);
> +		if (IS_ERR(overlay))
> +			return PTR_ERR(overlay);
> +
> +		if (!overlay->base.possible_crtcs)
> +			overlay->base.possible_crtcs = drm_crtc_mask(crtc);
> +	}
> +
>  	if (vkmsdev->config->cursor) {
>  		cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index);
>  		if (IS_ERR(cursor))
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> index da4251aff67f..8be9eab41ea0 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -133,7 +133,7 @@ static int vkms_plane_atomic_check(struct drm_plane *plane,
>  	if (IS_ERR(crtc_state))
>  		return PTR_ERR(crtc_state);
>  
> -	if (plane->type == DRM_PLANE_TYPE_CURSOR)
> +	if (plane->type != DRM_PLANE_TYPE_PRIMARY)
>  		can_position = true;
>  
>  	ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
> @@ -200,11 +200,19 @@ struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
>  	const u32 *formats;
>  	int nformats;
>  
> -	if (type == DRM_PLANE_TYPE_CURSOR) {
> +	switch (type) {
> +	case DRM_PLANE_TYPE_PRIMARY:
> +		formats = vkms_formats;
> +		nformats = ARRAY_SIZE(vkms_formats);
> +		funcs = &vkms_primary_helper_funcs;
> +		break;
> +	case DRM_PLANE_TYPE_CURSOR:
> +	case DRM_PLANE_TYPE_OVERLAY:
>  		formats = vkms_plane_formats;
>  		nformats = ARRAY_SIZE(vkms_plane_formats);
>  		funcs = &vkms_primary_helper_funcs;
> -	} else {
> +		break;
> +	default:
>  		formats = vkms_formats;
>  		nformats = ARRAY_SIZE(vkms_formats);
>  		funcs = &vkms_primary_helper_funcs;
> -- 
> 2.30.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel