Hi Sumera, Thanks for your patch. Please see some comments below. On 01/02, Sumera Priyadarsini wrote: > Currently, data for the device instance is held by vkms_device. > Add a separate type, vkms_config to contain configuration details > for the device and various modes to be later used by configfs. > This config data stays constant once the device is created. > > Accordingly, add vkms_create and vkms_destroy to initialize/destroy > device through configfs. Currently, they are being called from vkms_init > and vkms_exit, but will be evoked from configfs later on. When configfs > is added, device configuration- enabling/disabling features will > be tracked by configfs and only vkms device lifetime will be kept track of > by vkms_init and vkms_exit functions. I didn't quite understand this very last sentence. > > Modify usage of enable_cursor feature to reflect the changes in > relevant files. Since the goal is also enabling/disabling writeback and vblank via a future configfs support, it would be nice to see here this same cursor adaptation applied to others, initially as a module parameter. For writeback, it's really straightforward... > > Signed-off-by: Sumera Priyadarsini <sylphrenadin@xxxxxxxxx> > Co-developed-by: Daniel Vetter <danvet.vetter@xxxxxxx> There are some formatting details for credits, for example, including the co-developer s-o-b. See here: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by > --- > Changes in v2: > - add Co-developed-by tag > > --- > drivers/gpu/drm/vkms/vkms_drv.c | 40 ++++++++++++++++++++++++------ > drivers/gpu/drm/vkms/vkms_drv.h | 11 +++++--- > drivers/gpu/drm/vkms/vkms_output.c | 4 +-- > 3 files changed, 42 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > index aef29393b811..6b33975a5cb2 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.c > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > @@ -34,9 +34,9 @@ > #define DRIVER_MAJOR 1 > #define DRIVER_MINOR 0 > > -static struct vkms_device *vkms_device; > +static struct vkms_config *default_config; > > -bool enable_cursor = true; > +static bool enable_cursor = true; > module_param_named(enable_cursor, enable_cursor, bool, 0444); > MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support"); > > @@ -122,10 +122,11 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > return vkms_output_init(vkmsdev, 0); > } > > -static int __init vkms_init(void) > +static int vkms_create(struct vkms_config *config) > { > int ret; > struct platform_device *pdev; > + struct vkms_device *vkms_device; > > pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0); > if (IS_ERR(pdev)) > @@ -143,6 +144,8 @@ static int __init vkms_init(void) > goto out_devres; > } > vkms_device->platform = pdev; > + vkms_device->config = config; > + config->dev = vkms_device; > > ret = dma_coerce_mask_and_coherent(vkms_device->drm.dev, > DMA_BIT_MASK(64)); > @@ -179,21 +182,42 @@ static int __init vkms_init(void) > return ret; > } > > -static void __exit vkms_exit(void) > +static int __init vkms_init(void) > +{ > + struct vkms_config *config = kmalloc(sizeof(*config), GFP_KERNEL); > + > + default_config = config; > + > + config->cursor = enable_cursor; > + > + return vkms_create(config); > +} > + > +static void vkms_destroy(struct vkms_config *config) > { > struct platform_device *pdev; > > - if (!vkms_device) { > + if (!config->dev) { > DRM_INFO("vkms_device is NULL.\n"); > return; > } > > - pdev = vkms_device->platform; > + pdev = config->dev->platform; > > - drm_dev_unregister(&vkms_device->drm); > - drm_atomic_helper_shutdown(&vkms_device->drm); > + drm_dev_unregister(&config->dev->drm); > + drm_atomic_helper_shutdown(&config->dev->drm); > devres_release_group(&pdev->dev, NULL); > platform_device_unregister(pdev); > + > + config->dev = NULL; > +} > + > +static void __exit vkms_exit(void) > +{ > + if (default_config->dev) > + vkms_destroy(default_config); > + > + kfree(default_config); > } > > module_init(vkms_init); > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > index 5ed91ff08cb3..2fa0c52f1dd8 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.h > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > @@ -19,8 +19,6 @@ > #define XRES_MAX 8192 > #define YRES_MAX 8192 > > -extern bool enable_cursor; > - > struct vkms_composer { > struct drm_framebuffer fb; > struct drm_rect src, dst; > @@ -82,10 +80,17 @@ struct vkms_output { > spinlock_t composer_lock; > }; > > +struct vkms_device; > +struct vkms_config { > + bool cursor; > + /* only set when instantiated */ > + struct vkms_device *dev; > +}; > struct vkms_device { > struct drm_device drm; > struct platform_device *platform; > struct vkms_output output; > + const struct vkms_config *config; > }; > > #define drm_crtc_to_vkms_output(target) \ > @@ -123,4 +128,4 @@ void vkms_set_composer(struct vkms_output *out, bool enabled); > /* Writeback */ > int vkms_enable_writeback_connector(struct vkms_device *vkmsdev); > > -#endif /* _VKMS_DRV_H_ */ > +#endif /* _VKMS_DRV_H_ */ > \ No newline at end of file > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c > index 4a1848b0318f..8f3ffb28b9d1 100644 > --- a/drivers/gpu/drm/vkms/vkms_output.c > +++ b/drivers/gpu/drm/vkms/vkms_output.c > @@ -46,7 +46,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index) > if (IS_ERR(primary)) > return PTR_ERR(primary); > > - if (enable_cursor) { > + if (vkmsdev->config->cursor) { > cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index); > if (IS_ERR(cursor)) { > ret = PTR_ERR(cursor); > @@ -98,7 +98,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index) > drm_crtc_cleanup(crtc); > > err_crtc: > - if (enable_cursor) > + if (vkmsdev->config->cursor) > drm_plane_cleanup(cursor); > > err_cursor: > -- > 2.25.1 > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel