On 12/13, José Expósito wrote: > Add a new module parameter to allow to set the number of overlay planes > to create. Set it to 1 by default in order to keep the "enable_overlay" > backwards compatible. > Hi José,
in general, lgtm. However, I think we need some limits for this number
of planes. I would suggest to just expand the enable_overlay option to
expose a predefined number of planes and to avoid passing "crazy"
numbers by another module option.
Afaik, we are also limited to 32, as you can see in this commit:
2a8d3eac3d6e1
drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
I don't have a strong opinion on an exact/practical number. I took a
quick look at other drivers and exposing 8 planes seems reasonable to
me. Also, changing this number in the future would be pretty
straightfoward.
Thanks,
Melissa
> Signed-off-by: José Expósito <jose.exposit...@gmail.com>
> ---
> drivers/gpu/drm/vkms/vkms_drv.c | 5 +++++
> drivers/gpu/drm/vkms/vkms_drv.h | 1 +
> drivers/gpu/drm/vkms/vkms_output.c | 9 ++++++---
> 3 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 0ffe5f0e33f7..bb98f6c6c561 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 unsigned int num_overlay_planes = 1;
> +module_param_named(num_overlay_planes, num_overlay_planes, uint, 0444);
> +MODULE_PARM_DESC(num_overlay_planes, "Number of overlay planes to create");
> +
> DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
>
> static void vkms_release(struct drm_device *dev)
> @@ -229,6 +233,7 @@ static int __init vkms_init(void)
> config->cursor = enable_cursor;
> config->writeback = enable_writeback;
> config->overlay = enable_overlay;
> + config->num_overlay_planes = num_overlay_planes;
>
> return vkms_create(config);
> }
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index d48c23d40ce5..33bdf717e3cd 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -97,6 +97,7 @@ struct vkms_config {
> bool writeback;
> bool cursor;
> bool overlay;
> + unsigned int num_overlay_planes;
> /* 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 2e805b2d36ae..6f26998fdb7e 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 < vkmsdev->config->num_overlay_planes; n++) {
> + ret = vkms_add_overlay_plane(vkmsdev, index, crtc);
> + if (ret)
> + return ret;
> + }
> }
>
> if (vkmsdev->config->cursor) {
> --
> 2.25.1
>
signature.asc
Description: PGP signature
