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.exposito89@xxxxxxxxx> > --- > 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 >
Attachment:
signature.asc
Description: PGP signature