Re: [PATCH v2 4/6] drm/vkms: Add ConfigFS scaffolding to VKMS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jim,

On 6/23/23 19:23, Jim Shargo wrote:
This change adds the basic scaffolding for ConfigFS, including setting
up the default directories. It does not allow for the registration of
configfs-backed devices, which is complex and provided in a follow-up
commit.

This CL includes docs about using ConfigFS with VKMS, but I'll summarize
in brief here as well (assuming ConfigFS is mounted at /config/):

To create a new device, you can do so via `mkdir
/config/vkms/my-device`.

This will create a number of directories and files automatically:

	/config
	`-- vkms
	    `-- my-device
		|-- connectors
		|-- crtcs
		|-- encoders
		|-- planes
		`-- enabled

You can then configure objects by mkdir'ing in each of the directories.

When you're satisfied, you can `echo 1 > /config/vkms/my-device/enabled`.
This will create a new device according to your configuration.

For now, this will fail, but the next change will add support for it.

Signed-off-by: Jim Shargo <jshargo@xxxxxxxxxxxx>
---
  Documentation/gpu/vkms.rst           |  17 +-
  drivers/gpu/drm/Kconfig              |   1 +
  drivers/gpu/drm/vkms/Makefile        |   1 +
  drivers/gpu/drm/vkms/vkms_configfs.c | 646 +++++++++++++++++++++++++++
  drivers/gpu/drm/vkms/vkms_drv.c      |  52 ++-
  drivers/gpu/drm/vkms/vkms_drv.h      |  92 +++-
  drivers/gpu/drm/vkms/vkms_output.c   |   5 +
  7 files changed, 799 insertions(+), 15 deletions(-)
  create mode 100644 drivers/gpu/drm/vkms/vkms_configfs.c

diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index ba04ac7c2167..2c342ef0fb7b 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -51,6 +51,12 @@ To disable the driver, use ::
sudo modprobe -r vkms +Configuration With ConfigFS
+===========================
+
+.. kernel-doc:: drivers/gpu/drm/vkms/vkms_configfs.c
+   :doc: ConfigFS Support for VKMS
+
  Testing With IGT
  ================
@@ -135,22 +141,15 @@ project.
  Runtime Configuration
  ---------------------
-We want to be able to reconfigure vkms instance without having to reload the
-module. Use/Test-cases:
+We want to vkms instances without having to reload the module. Such

I believe that there is a verb missing here.

+configuration can be added as extensions to vkms's ConfigFS support. Use-cases:
- Hotplug/hotremove connectors on the fly (to be able to test DP MST handling
    of compositors).
-- Configure planes/crtcs/connectors (we'd need some code to have more than 1 of
-  them first).
-
  - Change output configuration: Plug/unplug screens, change EDID, allow changing
    the refresh rate.
-The currently proposed solution is to expose vkms configuration through
-configfs. All existing module options should be supported through configfs
-too.
-
  Writeback support
  -----------------
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index afb3b2f5f425..71eb913b378f 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -262,6 +262,7 @@ config DRM_VKMS
  	depends on DRM && MMU
  	select DRM_KMS_HELPER
  	select DRM_GEM_SHMEM_HELPER
+	select CONFIGFS_FS
  	select CRC32
  	default n
  	help
diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
index 1b28a6a32948..6b83907ad554 100644
--- a/drivers/gpu/drm/vkms/Makefile
+++ b/drivers/gpu/drm/vkms/Makefile
@@ -1,5 +1,6 @@
  # SPDX-License-Identifier: GPL-2.0-only
  vkms-y := \
+	vkms_configfs.o \
  	vkms_drv.o \
  	vkms_plane.o \
  	vkms_output.o \
diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
new file mode 100644
index 000000000000..544024735d19
--- /dev/null
+++ b/drivers/gpu/drm/vkms/vkms_configfs.c
@@ -0,0 +1,646 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <linux/configfs.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+
+#include <drm/drm_plane.h>
+#include <drm/drm_print.h>
+
+#include "vkms_drv.h"
+
+/**
+ * DOC: ConfigFS Support for VKMS
+ *
+ * VKMS is instrumented with support for configuration via :doc:`ConfigFS
+ * <../filesystems/configfs>`.
+ *
+ * With VKMS installed, you can mount ConfigFS at ``/config/`` like so::
+ *
+ *   mkdir -p /config/
+ *   sudo mount -t configfs none /config
+ *
+ * This allows you to configure multiple virtual devices in addition to an
+ * immutable "default" device created by the driver at initialization time. Note
+ * that the default device is immutable because we cannot pre-populate ConfigFS
+ * directories with normal files.
+ *
+ * To set up a new device, create a new directory under the VKMS configfs
+ * directory::
+ *
+ *   mkdir /config/vkms/test
+ *
+ * With your device created you'll find an new directory ready to be
+ * configured::
+ *
+ *   /config
+ *   `-- vkms
+ *       |-- default
+ *       |   `-- enabled
+ *       `-- test
+ *           |-- connectors
+ *           |-- crtcs
+ *           |-- encoders
+ *           |-- planes
+ *           `-- enabled

I followed this steps and ended up with the following tree:

[root@fedora config]# tree vkms
vkms
└── test
    ├── connectors
    ├── crtcs
    ├── enabled
    ├── encoders
    ├── id
    └── planes

I'm not sure if I did something wrong or if the folder "default" really
shouldn't show. If the folder "default" shouldn't show, it would be nice
to remove it from the docs.

+ *
+ * Each directory you add within the connectors, crtcs, encoders, and planes
+ * directories will let you configure a new object of that type. Adding new
+ * objects will automatically create a set of default files and folders you can
+ * use to configure that object.
+ *
+ * For instance, we can set up a two-output device like so::
+ *
+ *   DRM_PLANE_TYPE_PRIMARY=1
+ *   DRM_PLANE_TYPE_CURSOR=2
+ *   DRM_PLANE_TYPE_OVERLAY=0
+ *
+ *   mkdir /config/vkms/test/planes/primary
+ *   echo $DRM_PLANE_TYPE_PRIMARY > /config/vkms/test/planes/primary/type
+ *
+ *   mkdir /config/vkms/test/planes/other_primary
+ *   echo $DRM_PLANE_TYPE_PRIMARY > /config/vkms/test/planes/other_primary/type
+ *
+ *   mkdir /config/vkms/test/planes/cursor
+ *   echo $DRM_PLANE_TYPE_CURSOR > /config/vkms/test/planes/cursor/type
+ *
+ *   mkdir /config/vkms/test/planes/overlay
+ *   echo $DRM_PLANE_TYPE_OVERLAY > /config/vkms/test/planes/overlay/type
+ *
+ *   mkdir /config/vkms/test/crtcs/crtc
+ *   mkdir /config/vkms/test/crtcs/crtc_other
+ *   mkdir /config/vkms/test/encoders/encoder
+ *   mkdir /config/vkms/test/connectors/connector
+ *
+ * You can see that specific attributes, such as ``.../<plane>/type``, can be
+ * configured by writing into them. Associating objects together can be done via
+ * symlinks::
+ *
+ *   ln -s /config/vkms/test/encoders/encoder /config/vkms/test/connectors/connector/possible_encoders
+ *
+ *   ln -s /config/vkms/test/crtcs/crtc /config/vkms/test/encoders/encoder/possible_crtcs/
+ *   ln -s /config/vkms/test/crtcs/crtc /config/vkms/test/planes/primary/possible_crtcs/
+ *   ln -s /config/vkms/test/crtcs/crtc /config/vkms/test/planes/cursor/possible_crtcs/
+ *   ln -s /config/vkms/test/crtcs/crtc /config/vkms/test/planes/overlay/possible_crtcs/
+ *
+ *   ln -s /config/vkms/test/crtcs/crtc_other /config/vkms/test/planes/overlay/possible_crtcs/
+ *   ln -s /config/vkms/test/crtcs/crtc_other /config/vkms/test/planes/other_primary/possible_crtcs/
+ *
+ * Finally, to enable your configured device, just write 1 to the ``enabled``
+ * file::
+ *
+ *   echo 1 > /config/vkms/test/enabled
+ *
+ * When you're done with the virtual device, you can clean up the device like
+ * so::
+ *
+ *   echo 0 > /config/vkms/test/enabled
+ *
+ *   rm /config/vkms/test/connectors/connector/possible_encoders/encoder
+ *   rm /config/vkms/test/encoders/encoder/possible_crtcs/crtc
+ *   rm /config/vkms/test/planes/primary/possible_crtcs/crtc
+ *   rm /config/vkms/test/planes/cursor/possible_crtcs/crtc
+ *   rm /config/vkms/test/planes/overlay/possible_crtcs/crtc
+ *   rm /config/vkms/test/planes/overlay/possible_crtcs/crtc_other
+ *   rm /config/vkms/test/planes/other_primary/possible_crtcs/crtc_other
+ *
+ *   rmdir /config/vkms/test/planes/primary
+ *   rmdir /config/vkms/test/planes/other_primary
+ *   rmdir /config/vkms/test/planes/cursor
+ *   rmdir /config/vkms/test/planes/overlay
+ *   rmdir /config/vkms/test/crtcs/crtc
+ *   rmdir /config/vkms/test/crtcs/crtc_other
+ *   rmdir /config/vkms/test/encoders/encoder
+ *   rmdir /config/vkms/test/connectors/connector
+ *
+ *   rmdir /config/vkms/test
+ */
+
+/*

[...]

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index d43121addf66..1b5b7143792f 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -9,7 +9,10 @@
   * the GPU in DRM API tests.
   */
+#include <linux/configfs.h>
  #include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
  #include <linux/module.h>
  #include <linux/platform_device.h>
  #include <linux/dma-mapping.h>
@@ -152,8 +155,8 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev)
  	dev->mode_config.preferred_depth = 0;
  	dev->mode_config.helper_private = &vkms_mode_config_helpers;
- return vkmsdev->is_default ? vkms_output_init_default(vkmsdev) :
-				     -ENOSYS;

Is this middle step really needed?

Best Regards,
- Maíra

+	return vkmsdev->configfs ? vkms_output_init(vkmsdev) :
+				   vkms_output_init_default(vkmsdev);
  }
static int vkms_platform_probe(struct platform_device *pdev)
@@ -165,6 +168,7 @@ static int vkms_platform_probe(struct platform_device *pdev)
grp = devres_open_group(&pdev->dev, NULL, GFP_KERNEL);
  	if (!grp) {
+		DRM_ERROR("Could not open devres group\n");
  		return -ENOMEM;
  	}
@@ -179,7 +183,7 @@ static int vkms_platform_probe(struct platform_device *pdev)
  	vkms_device->config.cursor = enable_cursor;
  	vkms_device->config.writeback = enable_writeback;
  	vkms_device->config.overlay = enable_overlay;
-	vkms_device->is_default = vkms_device_setup->is_default;
+	vkms_device->configfs = vkms_device_setup->configfs;
ret = dma_coerce_mask_and_coherent(vkms_device->drm.dev,
  					   DMA_BIT_MASK(64));
@@ -239,12 +243,43 @@ static struct platform_driver vkms_platform_driver = {
  	.driver.name = DRIVER_NAME,
  };
+struct vkms_device *vkms_add_device(struct vkms_configfs *configfs)
+{
+	struct device *dev = NULL;
+	struct platform_device *pdev;
+	int max_id = 1;
+	struct vkms_device_setup vkms_device_setup = {
+		.configfs = configfs,
+	};
+
+	while ((dev = platform_find_device_by_driver(
+			dev, &vkms_platform_driver.driver))) {
+		pdev = to_platform_device(dev);
+		max_id = max(max_id, pdev->id);
+	}
+
+	pdev = platform_device_register_data(NULL, DRIVER_NAME, max_id + 1,
+					     &vkms_device_setup,
+					     sizeof(vkms_device_setup));
+	if (IS_ERR(pdev)) {
+		DRM_ERROR("Unable to register vkms device'\n");
+		return ERR_PTR(PTR_ERR(pdev));
+	}
+
+	return platform_get_drvdata(pdev);
+}
+
+void vkms_remove_device(struct vkms_device *vkms_device)
+{
+	platform_device_unregister(vkms_device->platform);
+}
+
  static int __init vkms_init(void)
  {
  	int ret;
  	struct platform_device *pdev;
  	struct vkms_device_setup vkms_device_setup = {
-		.is_default = true,
+		.configfs = NULL,
  	};
ret = platform_driver_register(&vkms_platform_driver);
@@ -262,6 +297,13 @@ static int __init vkms_init(void)
  		return PTR_ERR(pdev);
  	}
+ ret = vkms_init_configfs();
+	if (ret) {
+		DRM_ERROR("Unable to initialize configfs\n");
+		platform_device_unregister(pdev);
+		platform_driver_unregister(&vkms_platform_driver);
+	}
+
  	return 0;
  }
@@ -269,6 +311,8 @@ static void __exit vkms_exit(void)
  {
  	struct device *dev;
+ vkms_unregister_configfs();
+
  	while ((dev = platform_find_device_by_driver(
  			NULL, &vkms_platform_driver.driver))) {
  		// platform_find_device_by_driver increments the refcount. Drop
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 84b35aa1dc5a..3634eeeb4548 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -3,6 +3,7 @@
  #ifndef _VKMS_DRV_H_
  #define _VKMS_DRV_H_
+#include <linux/configfs.h>
  #include <linux/hrtimer.h>
#include <drm/drm.h>
@@ -10,6 +11,7 @@
  #include <drm/drm_gem.h>
  #include <drm/drm_gem_atomic_helper.h>
  #include <drm/drm_encoder.h>
+#include <drm/drm_plane.h>
  #include <drm/drm_writeback.h>
#define XRES_MIN 10
@@ -127,14 +129,65 @@ struct vkms_config {
  	bool overlay;
  };
+struct vkms_config_links {
+	struct config_group group;
+	unsigned long linked_object_bitmap;
+};
+
+struct vkms_config_connector {
+	struct config_group config_group;
+	struct vkms_config_links possible_encoders;
+};
+
+struct vkms_config_crtc {
+	struct config_group config_group;
+	unsigned long crtc_config_idx;
+};
+
+struct vkms_config_encoder {
+	struct config_group config_group;
+	struct vkms_config_links possible_crtcs;
+	unsigned long encoder_config_idx;
+};
+
+struct vkms_config_plane {
+	struct vkms_configfs *configfs;
+	struct config_group config_group;
+	struct vkms_config_links possible_crtcs;
+	enum drm_plane_type type;
+};
+
+struct vkms_configfs {
+	/* Directory group containing connector configs, e.g. /config/vkms/device/ */
+	struct config_group device_group;
+	/* Directory group containing connector configs, e.g. /config/vkms/device/connectors/ */
+	struct config_group connectors_group;
+	/* Directory group containing CRTC configs, e.g. /config/vkms/device/crtcs/ */
+	struct config_group crtcs_group;
+	/* Directory group containing encoder configs, e.g. /config/vkms/device/encoders/ */
+	struct config_group encoders_group;
+	/* Directory group containing plane configs, e.g. /config/vkms/device/planes/ */
+	struct config_group planes_group;
+
+	unsigned long allocated_crtcs;
+	unsigned long allocated_encoders;
+
+	struct mutex lock;
+
+	/* The platform device if this is registered, otherwise NULL */
+	struct vkms_device *vkms_device;
+};
+
  struct vkms_device_setup {
-	bool is_default;
+	// Is NULL in the case of the default card.
+	struct vkms_configfs *configfs;
  };
struct vkms_device {
  	struct drm_device drm;
  	struct platform_device *platform;
-	bool is_default;
+	// Is NULL in the case of the default card.
+	struct vkms_configfs *configfs;
  	struct vkms_output output;
  	struct vkms_config config;
  };
@@ -153,11 +206,42 @@ struct vkms_device {
  #define to_vkms_plane_state(target)\
  	container_of(target, struct vkms_plane_state, base.base)
+#define item_to_configfs(item) \
+	container_of(to_config_group(item), struct vkms_configfs, device_group)
+
+#define item_to_config_connector(item)                                    \
+	container_of(to_config_group(item), struct vkms_config_connector, \
+		     config_group)
+
+#define item_to_config_crtc(item)                                    \
+	container_of(to_config_group(item), struct vkms_config_crtc, \
+		     config_group)
+
+#define item_to_config_encoder(item)                                    \
+	container_of(to_config_group(item), struct vkms_config_encoder, \
+		     config_group)
+
+#define item_to_config_plane(item)                                    \
+	container_of(to_config_group(item), struct vkms_config_plane, \
+		     config_group)
+
+#define item_to_config_links(item) \
+	container_of(to_config_group(item), struct vkms_config_links, group)
+
+#define plane_item_to_configfs(item)                                         \
+	container_of(to_config_group(item->ci_parent), struct vkms_configfs, \
+		     planes_group)
+
+/* Devices */
+struct vkms_device *vkms_add_device(struct vkms_configfs *configfs);
+void vkms_remove_device(struct vkms_device *vkms_device);
+
  /* CRTC */
  struct vkms_crtc *vkms_crtc_init(struct vkms_device *vkmsdev,
  				 struct drm_plane *primary,
  				 struct drm_plane *cursor);
+int vkms_output_init(struct vkms_device *vkmsdev);
  int vkms_output_init_default(struct vkms_device *vkmsdev);
struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
@@ -179,4 +263,8 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state
  int vkms_enable_writeback_connector(struct vkms_device *vkmsdev,
  				    struct vkms_crtc *vkms_crtc);
+/* ConfigFS Support */
+int vkms_init_configfs(void);
+void vkms_unregister_configfs(void);
+
  #endif /* _VKMS_DRV_H_ */
diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index 3b5e272a8b33..c9e6c653de19 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -177,3 +177,8 @@ int vkms_output_init_default(struct vkms_device *vkmsdev)
return ret;
  }
+
+int vkms_output_init(struct vkms_device *vkmsdev)
+{
+	return -ENOTSUPP;
+}



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux