Re: [PATCH V2] drm/vkms: Decouple config data for configfs

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@gmail.com Co-developed-by: Daniel Vetter danvet.vetter@ffwl.ch

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-...

---

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