2022-01-07 18:28:16

by José Expósito

[permalink] [raw]
Subject: [PATCH RESEND v2 1/3] drm/vkms: refactor overlay plane creation

Move the logic to create an overlay plane to its own function.
Refactor, no functional changes.

Signed-off-by: José Expósito <[email protected]>
---
drivers/gpu/drm/vkms/vkms_output.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index 04406bd3ff02..2e805b2d36ae 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -32,6 +32,21 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = {
.get_modes = vkms_conn_get_modes,
};

+static int vkms_add_overlay_plane(struct vkms_device *vkmsdev, int index,
+ struct drm_crtc *crtc)
+{
+ struct vkms_plane *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);
+
+ return 0;
+}
+
int vkms_output_init(struct vkms_device *vkmsdev, int index)
{
struct vkms_output *output = &vkmsdev->output;
@@ -39,7 +54,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, *overlay = NULL;
+ struct vkms_plane *primary, *cursor = NULL;
int ret;
int writeback;

@@ -48,12 +63,9 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
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);
+ ret = vkms_add_overlay_plane(vkmsdev, index, crtc);
+ if (ret)
+ return ret;
}

if (vkmsdev->config->cursor) {
--
2.25.1



2022-01-07 18:28:18

by José Expósito

[permalink] [raw]
Subject: [PATCH RESEND v2 2/3] drm/vkms: add support for multiple overlay planes

Create 8 overlay planes instead of 1 when the "enable_overlay" module
parameter is set.

The following igt-gpu-tools tests were executed without finding
regressions. Notice than the numbers are identical:

| master branch | this patch |
| SUCCESS | SKIP | FAIL | SUCCESS | SKIP | FAIL |
kms_atomic | 10 | 02 | 00 | 10 | 02 | 00 |
kms_plane_cursor | 09 | 45 | 00 | 09 | 45 | 00 |
kms_plane_multiple | 01 | 23 | 00 | 01 | 23 | 00 |
kms_writeback | 04 | 00 | 00 | 04 | 00 | 00 |

Signed-off-by: José Expósito <[email protected]>

---

v2:

- Set the number of overlay planes as a constant instead of as a
module parameter (Melissa Wen)

- Add a test results in the commit message (Melissa Wen)
---
drivers/gpu/drm/vkms/vkms_drv.h | 2 ++
drivers/gpu/drm/vkms/vkms_output.c | 9 ++++++---
2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index d48c23d40ce5..9496fdc900b8 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -20,6 +20,8 @@
#define XRES_MAX 8192
#define YRES_MAX 8192

+#define NUM_OVERLAY_PLANES 8
+
struct vkms_writeback_job {
struct dma_buf_map map[DRM_FORMAT_MAX_PLANES];
struct dma_buf_map data[DRM_FORMAT_MAX_PLANES];
diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index 2e805b2d36ae..ba0e82ae549a 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -57,15 +57,18 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
struct vkms_plane *primary, *cursor = NULL;
int ret;
int writeback;
+ unsigned int n;

primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index);
if (IS_ERR(primary))
return PTR_ERR(primary);

if (vkmsdev->config->overlay) {
- ret = vkms_add_overlay_plane(vkmsdev, index, crtc);
- if (ret)
- return ret;
+ for (n = 0; n < NUM_OVERLAY_PLANES; n++) {
+ ret = vkms_add_overlay_plane(vkmsdev, index, crtc);
+ if (ret)
+ return ret;
+ }
}

if (vkmsdev->config->cursor) {
--
2.25.1


2022-01-07 18:28:21

by José Expósito

[permalink] [raw]
Subject: [PATCH RESEND v2 3/3] drm/vkms: drop "Multiple overlay planes" TODO

Remove the task from the TODO list.

Signed-off-by: José Expósito <[email protected]>
---
Documentation/gpu/vkms.rst | 2 --
1 file changed, 2 deletions(-)

diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index 941f0e7e5eef..9c873c3912cc 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -124,8 +124,6 @@ Add Plane Features

There's lots of plane features we could add support for:

-- Multiple overlay planes. [Good to get started]
-
- Clearing primary plane: clear primary plane before plane composition (at the
start) for correctness of pixel blend ops. It also guarantees alpha channel
is cleared in the target buffer for stable crc. [Good to get started]
--
2.25.1


2022-01-09 16:51:18

by Melissa Wen

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 1/3] drm/vkms: refactor overlay plane creation

On 01/07, Jos? Exp?sito wrote:
> Move the logic to create an overlay plane to its own function.
> Refactor, no functional changes.
>
> Signed-off-by: Jos? Exp?sito <[email protected]>
> ---
> drivers/gpu/drm/vkms/vkms_output.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index 04406bd3ff02..2e805b2d36ae 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -32,6 +32,21 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = {
> .get_modes = vkms_conn_get_modes,
> };
>
> +static int vkms_add_overlay_plane(struct vkms_device *vkmsdev, int index,
> + struct drm_crtc *crtc)
> +{
> + struct vkms_plane *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);
> +
> + return 0;
> +}
> +
> int vkms_output_init(struct vkms_device *vkmsdev, int index)
> {
> struct vkms_output *output = &vkmsdev->output;
> @@ -39,7 +54,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, *overlay = NULL;
> + struct vkms_plane *primary, *cursor = NULL;
> int ret;
> int writeback;
>
> @@ -48,12 +63,9 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
> 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);
> + ret = vkms_add_overlay_plane(vkmsdev, index, crtc);
> + if (ret)
> + return ret;
lgtm, thanks!

Reviewed-by: Melissa Wen <[email protected]>
> }
>
> if (vkmsdev->config->cursor) {
> --
> 2.25.1
>


Attachments:
(No filename) (2.13 kB)
signature.asc (833.00 B)
Download all attachments

2022-01-09 17:00:13

by Melissa Wen

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 2/3] drm/vkms: add support for multiple overlay planes

On 01/07, Jos? Exp?sito wrote:
> Create 8 overlay planes instead of 1 when the "enable_overlay" module
> parameter is set.
>
> The following igt-gpu-tools tests were executed without finding
> regressions. Notice than the numbers are identical:
>
> | master branch | this patch |
> | SUCCESS | SKIP | FAIL | SUCCESS | SKIP | FAIL |
> kms_atomic | 10 | 02 | 00 | 10 | 02 | 00 |
> kms_plane_cursor | 09 | 45 | 00 | 09 | 45 | 00 |
> kms_plane_multiple | 01 | 23 | 00 | 01 | 23 | 00 |
> kms_writeback | 04 | 00 | 00 | 04 | 00 | 00 |

I checked that several testcases from kms_cursor_crc are failing after
recent changes in the test. However, this regression is not caused by
these changes and need futher investigation for fixing.

That said, this patch lgtm.

Reviewed-by: Melissa Wen <[email protected]>
>
> Signed-off-by: Jos? Exp?sito <[email protected]>
>
> ---
>
> v2:
>
> - Set the number of overlay planes as a constant instead of as a
> module parameter (Melissa Wen)
>
> - Add a test results in the commit message (Melissa Wen)
> ---
> drivers/gpu/drm/vkms/vkms_drv.h | 2 ++
> drivers/gpu/drm/vkms/vkms_output.c | 9 ++++++---
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index d48c23d40ce5..9496fdc900b8 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -20,6 +20,8 @@
> #define XRES_MAX 8192
> #define YRES_MAX 8192
>
> +#define NUM_OVERLAY_PLANES 8
> +
> struct vkms_writeback_job {
> struct dma_buf_map map[DRM_FORMAT_MAX_PLANES];
> struct dma_buf_map data[DRM_FORMAT_MAX_PLANES];
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index 2e805b2d36ae..ba0e82ae549a 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -57,15 +57,18 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
> struct vkms_plane *primary, *cursor = NULL;
> int ret;
> int writeback;
> + unsigned int n;
>
> primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index);
> if (IS_ERR(primary))
> return PTR_ERR(primary);
>
> if (vkmsdev->config->overlay) {
> - ret = vkms_add_overlay_plane(vkmsdev, index, crtc);
> - if (ret)
> - return ret;
> + for (n = 0; n < NUM_OVERLAY_PLANES; n++) {
> + ret = vkms_add_overlay_plane(vkmsdev, index, crtc);
> + if (ret)
> + return ret;
> + }
> }
>
> if (vkmsdev->config->cursor) {
> --
> 2.25.1
>


Attachments:
(No filename) (2.61 kB)
signature.asc (833.00 B)
Download all attachments

2022-01-09 17:04:08

by Melissa Wen

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 3/3] drm/vkms: drop "Multiple overlay planes" TODO

On 01/07, Jos? Exp?sito wrote:
> Remove the task from the TODO list.
>
> Signed-off-by: Jos? Exp?sito <[email protected]>
> ---
> Documentation/gpu/vkms.rst | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
> index 941f0e7e5eef..9c873c3912cc 100644
> --- a/Documentation/gpu/vkms.rst
> +++ b/Documentation/gpu/vkms.rst
> @@ -124,8 +124,6 @@ Add Plane Features
>
> There's lots of plane features we could add support for:
>
> -- Multiple overlay planes. [Good to get started]
> -
Hi Jos?,

Thanks for your patches.
I'll apply to drm-misc-next.

Reviewed-by: Melissa Wen <[email protected]>

> - Clearing primary plane: clear primary plane before plane composition (at the
> start) for correctness of pixel blend ops. It also guarantees alpha channel
> is cleared in the target buffer for stable crc. [Good to get started]
> --
> 2.25.1
>


Attachments:
(No filename) (924.00 B)
signature.asc (833.00 B)
Download all attachments

2022-01-09 17:59:46

by José Expósito

[permalink] [raw]
Subject: Re: [PATCH RESEND v2 2/3] drm/vkms: add support for multiple overlay planes

Hi Melissa,

Thanks for reviewing and applying the series :)

Now [1] should apply cleanly on drm-misc-next. Do you want me to resend
it with the appropriate prefix?

On Sun, Jan 09, 2022 at 03:59:55PM -0100, Melissa Wen wrote:
> I checked that several testcases from kms_cursor_crc are failing after
> recent changes in the test. However, this regression is not caused by
> these changes and need futher investigation for fixing.

I didn't include the results for kms_cursor_crc because some of the
tests cause issues on my PC. However, if you want to share the list of
regressions, I'll have a look into the test as well.

Thanks again,
Jos? Exp?sito

[1] https://lore.kernel.org/all/[email protected]/