* [RFC PATCH 0/1] drm/vkms: ConfigFS support @ 2022-05-06 0:18 Jim Shargo 2022-05-06 0:18 ` [RFC PATCH 1/1] drm/vkms: Add basic support for ConfigFS to VKMS Jim Shargo 2022-05-09 15:10 ` [RFC PATCH 0/1] drm/vkms: ConfigFS support Daniel Vetter 0 siblings, 2 replies; 6+ messages in thread From: Jim Shargo @ 2022-05-06 0:18 UTC (permalink / raw) To: Rodrigo Siqueira, Melissa Wen Cc: Haneen Mohammed, Thomas Zimmermann, David Airlie, dri-devel, Jim Shargo Hi! I wanted to send this patch out early to get some feedback on the layout of the code and new ConfigFS directory. I intend to follow this up with a more complete patch set that uses this to, for instance, add more connectors and toggle feature support. A few questions I had as someone new to kernel dev: 1. Would you prefer laying out all the objects right now or add them as-needed? IMO it’s nice to lay things out now to make future work that much easier. 2. Is the layout of /config/vkms/<type>s/<id>/<attributes> OK? If VKMS would eventually want to support installing multiple devices, we could do something like /config/vkms/card<N>/<type>s/<id>/<attributes>. 3. Should I split out the documention into a separate change? 4. Any other style / design thoughts? Thanks! I am excited to be reaching out and contributing. Jim Shargo (1): drm/vkms: Add basic support for ConfigFS to VKMS Documentation/gpu/vkms.rst | 23 +++++ drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/vkms/Makefile | 1 + drivers/gpu/drm/vkms/vkms_configfs.c | 129 +++++++++++++++++++++++++++ drivers/gpu/drm/vkms/vkms_drv.c | 10 +++ drivers/gpu/drm/vkms/vkms_drv.h | 25 ++++++ drivers/gpu/drm/vkms/vkms_output.c | 2 + drivers/gpu/drm/vkms/vkms_plane.c | 2 + 8 files changed, 193 insertions(+) create mode 100644 drivers/gpu/drm/vkms/vkms_configfs.c -- 2.36.0.512.ge40c2bad7a-goog ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH 1/1] drm/vkms: Add basic support for ConfigFS to VKMS 2022-05-06 0:18 [RFC PATCH 0/1] drm/vkms: ConfigFS support Jim Shargo @ 2022-05-06 0:18 ` Jim Shargo 2022-05-09 15:10 ` [RFC PATCH 0/1] drm/vkms: ConfigFS support Daniel Vetter 1 sibling, 0 replies; 6+ messages in thread From: Jim Shargo @ 2022-05-06 0:18 UTC (permalink / raw) To: Rodrigo Siqueira, Melissa Wen Cc: Haneen Mohammed, Thomas Zimmermann, David Airlie, dri-devel, Jim Shargo This change adds boilerplate setup for ConfigFS, resulting in a config directory that looks like this (assuming it's mounted under /config/): /config `-- vkms |-- connectors | `-- 52 |-- crtcs | `-- 51 |-- encoders | `-- 53 `-- planes |-- 31 |-- 33 |-- 35 |-- 37 |-- 39 |-- 41 |-- 43 |-- 45 |-- 47 `-- 49 Notes on the ConfigFS setup: - Each `vkms/<type>s/<id>/` is a config group/item based directory that can have readable and writable attributes set within them. - Each `vkms/<type>s` directory can have ConfigFS groupops set on them to respond to mkdir calls, allowing for the creation of new objects within vkms (such as hotplugging a new connector). --- Documentation/gpu/vkms.rst | 23 +++++ drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/vkms/Makefile | 1 + drivers/gpu/drm/vkms/vkms_configfs.c | 129 +++++++++++++++++++++++++++ drivers/gpu/drm/vkms/vkms_drv.c | 10 +++ drivers/gpu/drm/vkms/vkms_drv.h | 25 ++++++ drivers/gpu/drm/vkms/vkms_output.c | 2 + drivers/gpu/drm/vkms/vkms_plane.c | 2 + 8 files changed, 193 insertions(+) create mode 100644 drivers/gpu/drm/vkms/vkms_configfs.c diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst index 9c873c3912cc..13233fc87b53 100644 --- a/Documentation/gpu/vkms.rst +++ b/Documentation/gpu/vkms.rst @@ -51,6 +51,29 @@ To disable the driver, use :: sudo modprobe -r vkms +Configuration With ConfigFS +=========================== + +VKMS is instrumented with support for configuration via `ConfigFS`. With VKMS +installed, you can mount ConfigFS at /config/ like so:: + + mkdir -p /config/ + sudo mount -t configfs none /config + +This will create a directory tree that looks something like this:: + + - /config/vkms/ + - connectors/ -- a list of all the planes available in the VKMS driver + - N/ -- the connector with ID=N + - crtc/ -- a list of all the crtcs available in the VKMS driver + - N/ -- the crtc with ID=N + - encoders/ -- a list of all the encoders available in the VKMS driver + - N/ -- the encoder with ID=N + - planes/ -- a list of all the planes available in the VKMS driver + - N/ -- the plane with ID=N. + +Settings for each object will appear in the ``/config/vkms/<type>/<N>/`` directory. + Testing With IGT ================ diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index f1422bee3dcc..5c90c82fab6d 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -307,6 +307,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 72f779cbfedd..3efb7b0f5319 100644 --- a/drivers/gpu/drm/vkms/Makefile +++ b/drivers/gpu/drm/vkms/Makefile @@ -3,6 +3,7 @@ vkms-y := \ vkms_drv.o \ vkms_plane.o \ vkms_output.o \ + vkms_configfs.o \ vkms_crtc.o \ vkms_composer.o \ vkms_writeback.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..edaa041a830c --- /dev/null +++ b/drivers/gpu/drm/vkms/vkms_configfs.c @@ -0,0 +1,129 @@ +// SPDX-License-Identifier: GPL-2.0+ + +#include <linux/configfs.h> +#include <linux/export.h> +#include <linux/mutex.h> + +#include "vkms_drv.h" + +static void setup_configfs_object(uint32_t id, struct config_group *object, + struct config_group *parent, + struct config_item_type *type) +{ + char name[CONFIGFS_ITEM_NAME_LEN]; + + snprintf(name, CONFIGFS_ITEM_NAME_LEN, "%d", id); + config_group_init_type_name(object, name, type); + configfs_add_default_group(object, parent); +} + +/* connector item, e.g. /config/vkms/connectors/ID */ + +static struct config_item_type connector_type = { + .ct_owner = THIS_MODULE, +}; + +/* crtc item, e.g. /config/vkms/crtc/ID */ + +static struct config_item_type crtc_type = { + .ct_owner = THIS_MODULE, +}; + +/* encoder item, e.g. /config/vkms/encoder/ID */ + +static struct config_item_type encoder_type = { + .ct_owner = THIS_MODULE, +}; + +void vkms_init_output_configfs(struct vkms_device *vkmsdev, + struct vkms_output *output) +{ + setup_configfs_object(output->connector.base.id, + &output->connector_config_group, + &vkmsdev->configfs.connectors_group, + &connector_type); + + setup_configfs_object(output->crtc.base.id, &output->crtc_config_group, + &vkmsdev->configfs.crtcs_group, &crtc_type); + + setup_configfs_object(output->encoder.base.id, + &output->encoder_config_group, + &vkmsdev->configfs.encoders_group, &encoder_type); +} + +/* Plane item, e.g. /config/vkms/planes/ID */ + +static struct config_item_type plane_type = { + .ct_owner = THIS_MODULE, +}; + +void vkms_init_plane_configfs(struct vkms_device *vkmsdev, + struct vkms_plane *plane) +{ + setup_configfs_object(plane->base.base.id, &plane->config_group, + &vkmsdev->configfs.planes_group, &plane_type); +} + +/* Directory groups, e.g. /config/vkms/planes */ + +static struct config_item_type connectors_group_type = { + .ct_owner = THIS_MODULE, +}; + +static struct config_item_type crtcs_group_type = { + .ct_owner = THIS_MODULE, +}; + +static struct config_item_type encoders_group_type = { + .ct_owner = THIS_MODULE, +}; + +static struct config_item_type planes_group_type = { + .ct_owner = THIS_MODULE, +}; + +/* Root directory group, e.g. /config/vkms/ */ + +static struct config_item_type vkms_type = { + .ct_owner = THIS_MODULE, +}; + +static struct configfs_subsystem vkms_subsys = { + .su_group = { + .cg_item = { + .ci_name = "vkms", + .ci_type = &vkms_type, + }, + }, +}; + +void vkms_init_configfs(struct vkms_device *vkmsdev) +{ + config_group_init(&vkms_subsys.su_group); + mutex_init(&vkms_subsys.su_mutex); + + config_group_init_type_name(&vkmsdev->configfs.connectors_group, + "connectors", &connectors_group_type); + configfs_add_default_group(&vkmsdev->configfs.connectors_group, + &vkms_subsys.su_group); + + config_group_init_type_name(&vkmsdev->configfs.crtcs_group, "crtcs", + &crtcs_group_type); + configfs_add_default_group(&vkmsdev->configfs.crtcs_group, + &vkms_subsys.su_group); + + config_group_init_type_name(&vkmsdev->configfs.encoders_group, + "encoders", &encoders_group_type); + configfs_add_default_group(&vkmsdev->configfs.encoders_group, + &vkms_subsys.su_group); + + config_group_init_type_name(&vkmsdev->configfs.planes_group, "planes", + &planes_group_type); + configfs_add_default_group(&vkmsdev->configfs.planes_group, + &vkms_subsys.su_group); +} + +int vkms_register_configfs(void) +{ + return configfs_register_subsystem(&vkms_subsys); +} diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c index 0ffe5f0e33f7..92ca8cf2d104 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.c +++ b/drivers/gpu/drm/vkms/vkms_drv.c @@ -9,6 +9,7 @@ * the GPU in DRM API tests. */ +#include <linux/configfs.h> #include <linux/module.h> #include <linux/platform_device.h> #include <linux/dma-mapping.h> @@ -191,6 +192,8 @@ static int vkms_create(struct vkms_config *config) goto out_devres; } + vkms_init_configfs(vkms_device); + ret = drm_vblank_init(&vkms_device->drm, 1); if (ret) { DRM_ERROR("Failed to vblank\n"); @@ -207,8 +210,15 @@ static int vkms_create(struct vkms_config *config) drm_fbdev_generic_setup(&vkms_device->drm, 0); + ret = vkms_register_configfs(); + if (ret) + goto out_drmres; + + return 0; +out_drmres: + drm_dev_unregister(&vkms_device->drm); out_devres: devres_release_group(&pdev->dev, NULL); out_unregister: diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index 91e63b12f60f..873b91f63309 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> @@ -48,6 +49,7 @@ struct vkms_plane_state { struct vkms_plane { struct drm_plane base; + struct config_group config_group; }; /** @@ -86,6 +88,10 @@ struct vkms_output { /* protects concurrent access to composer */ spinlock_t lock; + struct config_group crtc_config_group; + struct config_group encoder_config_group; + struct config_group connector_config_group; + /* protected by @lock */ bool composer_enabled; struct vkms_crtc_state *composer_state; @@ -103,10 +109,22 @@ struct vkms_config { struct vkms_device *dev; }; +struct vkms_configfs { + /* Directory group containing connector configs, e.g. /config/vkms/connectors/ */ + struct config_group connectors_group; + /* Directory group containing CRTC configs, e.g. /config/vkms/crtcs/ */ + struct config_group crtcs_group; + /* Directory group containing encoder configs, e.g. /config/vkms/encoders/ */ + struct config_group encoders_group; + /* Directory group containing plane configs, e.g. /config/vkms/planes/ */ + struct config_group planes_group; +}; + struct vkms_device { struct drm_device drm; struct platform_device *platform; struct vkms_output output; + struct vkms_configfs configfs; const struct vkms_config *config; }; @@ -145,4 +163,11 @@ void vkms_set_composer(struct vkms_output *out, bool enabled); /* Writeback */ int vkms_enable_writeback_connector(struct vkms_device *vkmsdev); +/* ConfigFS Support */ +void vkms_init_configfs(struct vkms_device *vkmsdev); +int vkms_register_configfs(void); + +void vkms_init_plane_configfs(struct vkms_device *vkmsdev, struct vkms_plane *plane); +void vkms_init_output_configfs(struct vkms_device *vkmsdev, struct vkms_output *output); + #endif /* _VKMS_DRV_H_ */ diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index ba0e82ae549a..fa80d27118c8 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -111,6 +111,8 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index) drm_mode_config_reset(dev); + vkms_init_output_configfs(vkmsdev, output); + return 0; err_attach: diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c index d8eb674b49a6..4b93cfa1bb19 100644 --- a/drivers/gpu/drm/vkms/vkms_plane.c +++ b/drivers/gpu/drm/vkms/vkms_plane.c @@ -195,5 +195,7 @@ struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev, drm_plane_helper_add(&plane->base, funcs); + vkms_init_plane_configfs(vkmsdev, plane); + return plane; } -- 2.36.0.512.ge40c2bad7a-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 0/1] drm/vkms: ConfigFS support 2022-05-06 0:18 [RFC PATCH 0/1] drm/vkms: ConfigFS support Jim Shargo 2022-05-06 0:18 ` [RFC PATCH 1/1] drm/vkms: Add basic support for ConfigFS to VKMS Jim Shargo @ 2022-05-09 15:10 ` Daniel Vetter 2022-06-06 21:59 ` Jim Shargo 1 sibling, 1 reply; 6+ messages in thread From: Daniel Vetter @ 2022-05-09 15:10 UTC (permalink / raw) To: Jim Shargo Cc: Haneen Mohammed, Thomas Zimmermann, Rodrigo Siqueira, David Airlie, Melissa Wen, dri-devel On Fri, May 06, 2022 at 12:18:20AM +0000, Jim Shargo wrote: > Hi! > > I wanted to send this patch out early to get some feedback on the layout > of the code and new ConfigFS directory. I intend to follow this up with > a more complete patch set that uses this to, for instance, add more > connectors and toggle feature support. > > A few questions I had as someone new to kernel dev: > > 1. Would you prefer laying out all the objects right now or add them > as-needed? IMO it’s nice to lay things out now to make future work that > much easier. > > 2. Is the layout of /config/vkms/<type>s/<id>/<attributes> OK? If VKMS > would eventually want to support installing multiple devices, we could > do something like /config/vkms/card<N>/<type>s/<id>/<attributes>. > > 3. Should I split out the documention into a separate change? > > 4. Any other style / design thoughts? > > Thanks! I am excited to be reaching out and contributing. So the overall idea here is that a lot of these things cannot be changed once the vkms_device instance is created, due to how drm works. This is why struct vmks_config has been extracted. The rough flow would be: 1. you create a new directory in the vkms configfs directory when creating a new instance. This then gets populated with all the properties from vkms_config 2. user sets these properts through configfs 3. There is a special property called "registered" or similar, which starts out as 0/false. When set to 1/true the vkms_device will be registered with the vkms_config that's been prepared. After that point further changes to vkms_config are not allowed anymore at all (this might change later on for connector hotplug, which really is the only thing a drm_device can change at runtime). 4. removal of the vkms_device could perhaps be done simply be deleting the entire directory? I think that would be a nice clean interface. So in other words, basing the configfs interface on drm objects doesn't work, because once the drm objects exist you cannot change the configuration anymore. Cheers, Daniel > > > Jim Shargo (1): > drm/vkms: Add basic support for ConfigFS to VKMS > > Documentation/gpu/vkms.rst | 23 +++++ > drivers/gpu/drm/Kconfig | 1 + > drivers/gpu/drm/vkms/Makefile | 1 + > drivers/gpu/drm/vkms/vkms_configfs.c | 129 +++++++++++++++++++++++++++ > drivers/gpu/drm/vkms/vkms_drv.c | 10 +++ > drivers/gpu/drm/vkms/vkms_drv.h | 25 ++++++ > drivers/gpu/drm/vkms/vkms_output.c | 2 + > drivers/gpu/drm/vkms/vkms_plane.c | 2 + > 8 files changed, 193 insertions(+) > create mode 100644 drivers/gpu/drm/vkms/vkms_configfs.c > > -- > 2.36.0.512.ge40c2bad7a-goog > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 0/1] drm/vkms: ConfigFS support 2022-05-09 15:10 ` [RFC PATCH 0/1] drm/vkms: ConfigFS support Daniel Vetter @ 2022-06-06 21:59 ` Jim Shargo 2022-06-08 15:45 ` Daniel Vetter 0 siblings, 1 reply; 6+ messages in thread From: Jim Shargo @ 2022-06-06 21:59 UTC (permalink / raw) To: Daniel Vetter Cc: Haneen Mohammed, Rodrigo Siqueira, David Airlie, Melissa Wen, dri-devel, Thomas Zimmermann, Jim Shargo Thanks for taking a look! Sorry for the delay in response, I've been moving house and prototyping locally. On Mon, May 9, 2022 at 11:10 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > On Fri, May 06, 2022 at 12:18:20AM +0000, Jim Shargo wrote: > > Hi! > > > > I wanted to send this patch out early to get some feedback on the layout > > of the code and new ConfigFS directory. I intend to follow this up with > > a more complete patch set that uses this to, for instance, add more > > connectors and toggle feature support. > > > > A few questions I had as someone new to kernel dev: > > > > 1. Would you prefer laying out all the objects right now or add them > > as-needed? IMO it’s nice to lay things out now to make future work that > > much easier. > > > > 2. Is the layout of /config/vkms/<type>s/<id>/<attributes> OK? If VKMS > > would eventually want to support installing multiple devices, we could > > do something like /config/vkms/card<N>/<type>s/<id>/<attributes>. > > > > 3. Should I split out the documention into a separate change? > > > > 4. Any other style / design thoughts? > > > > Thanks! I am excited to be reaching out and contributing. > > So the overall idea here is that a lot of these things cannot be changed > once the vkms_device instance is created, due to how drm works. This is > why struct vmks_config has been extracted. The rough flow would be: > > 1. you create a new directory in the vkms configfs directory when creating > a new instance. This then gets populated with all the properties from > vkms_config > > 2. user sets these properts through configfs > > 3. There is a special property called "registered" or similar, which > starts out as 0/false. When set to 1/true the vkms_device will be > registered with the vkms_config that's been prepared. After that point > further changes to vkms_config are not allowed anymore at all (this might > change later on for connector hotplug, which really is the only thing a > drm_device can change at runtime). I think this allows for a slightly easier initialization, too, where we don't keep a half-built drm device around or need to manage ids in any special way. I'll get things working and send out a new patch set. I'm also thinking that, to make life easier, we'll create the regular default device during init and register it automatically, so unless someone starts actively adding virtual devices things behavior remains the same. > > 4. removal of the vkms_device could perhaps be done simply be deleting the > entire directory? I think that would be a nice clean interface. Yep! I just got that wired up locally. > > So in other words, basing the configfs interface on drm objects doesn't > work, because once the drm objects exist you cannot change the > configuration anymore. I wasn't 100% sure how much trouble we'd get into if we tried to set DRM device properties at run time, but with this confirmation I think that this staging/registration approach is the best. > Cheers, Daniel > > > > > > Jim Shargo (1): > > drm/vkms: Add basic support for ConfigFS to VKMS > > > > Documentation/gpu/vkms.rst | 23 +++++ > > drivers/gpu/drm/Kconfig | 1 + > > drivers/gpu/drm/vkms/Makefile | 1 + > > drivers/gpu/drm/vkms/vkms_configfs.c | 129 +++++++++++++++++++++++++++ > > drivers/gpu/drm/vkms/vkms_drv.c | 10 +++ > > drivers/gpu/drm/vkms/vkms_drv.h | 25 ++++++ > > drivers/gpu/drm/vkms/vkms_output.c | 2 + > > drivers/gpu/drm/vkms/vkms_plane.c | 2 + > > 8 files changed, 193 insertions(+) > > create mode 100644 drivers/gpu/drm/vkms/vkms_configfs.c > > > > -- > > 2.36.0.512.ge40c2bad7a-goog > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 0/1] drm/vkms: ConfigFS support 2022-06-06 21:59 ` Jim Shargo @ 2022-06-08 15:45 ` Daniel Vetter 0 siblings, 0 replies; 6+ messages in thread From: Daniel Vetter @ 2022-06-08 15:45 UTC (permalink / raw) To: Jim Shargo Cc: Haneen Mohammed, Thomas Zimmermann, Rodrigo Siqueira, David Airlie, Melissa Wen, dri-devel, Jim Shargo On Mon, Jun 06, 2022 at 05:59:37PM -0400, Jim Shargo wrote: > Thanks for taking a look! Sorry for the delay in response, I've been > moving house and prototyping locally. > > On Mon, May 9, 2022 at 11:10 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > > > On Fri, May 06, 2022 at 12:18:20AM +0000, Jim Shargo wrote: > > > Hi! > > > > > > I wanted to send this patch out early to get some feedback on the layout > > > of the code and new ConfigFS directory. I intend to follow this up with > > > a more complete patch set that uses this to, for instance, add more > > > connectors and toggle feature support. > > > > > > A few questions I had as someone new to kernel dev: > > > > > > 1. Would you prefer laying out all the objects right now or add them > > > as-needed? IMO it’s nice to lay things out now to make future work that > > > much easier. > > > > > > 2. Is the layout of /config/vkms/<type>s/<id>/<attributes> OK? If VKMS > > > would eventually want to support installing multiple devices, we could > > > do something like /config/vkms/card<N>/<type>s/<id>/<attributes>. > > > > > > 3. Should I split out the documention into a separate change? > > > > > > 4. Any other style / design thoughts? > > > > > > Thanks! I am excited to be reaching out and contributing. > > > > So the overall idea here is that a lot of these things cannot be changed > > once the vkms_device instance is created, due to how drm works. This is > > why struct vmks_config has been extracted. The rough flow would be: > > > > 1. you create a new directory in the vkms configfs directory when creating > > a new instance. This then gets populated with all the properties from > > vkms_config > > > > 2. user sets these properts through configfs > > > > 3. There is a special property called "registered" or similar, which > > starts out as 0/false. When set to 1/true the vkms_device will be > > registered with the vkms_config that's been prepared. After that point > > further changes to vkms_config are not allowed anymore at all (this might > > change later on for connector hotplug, which really is the only thing a > > drm_device can change at runtime). > > I think this allows for a slightly easier initialization, too, where > we don't keep a half-built drm device around or need to manage ids in > any special way. > > I'll get things working and send out a new patch set. > > I'm also thinking that, to make life easier, we'll create the regular > default device during init and register it automatically, so unless > someone starts actively adding virtual devices things behavior remains > the same. Yup, I think keeping the regular default device is a good idea. Also I think initializing a new instance's vkms_config with the module options we have would probably also make sense. Of course for new complex features (like special plane features or attaching ebpf to implement atomic_check restrictions) are best done only through configfs, so that we can slowly deprecate the module options. But for the handful of existing knobs I think it's fine to just keep it all as-is. > > 4. removal of the vkms_device could perhaps be done simply be deleting the > > entire directory? I think that would be a nice clean interface. > > Yep! I just got that wired up locally. > > > > > So in other words, basing the configfs interface on drm objects doesn't > > work, because once the drm objects exist you cannot change the > > configuration anymore. > > I wasn't 100% sure how much trouble we'd get into if we tried to set > DRM device properties at run time, but with this confirmation I think > that this staging/registration approach is the best. Looking forward to your next round! Cheers, Daniel > > > Cheers, Daniel > > > > > > > > > Jim Shargo (1): > > > drm/vkms: Add basic support for ConfigFS to VKMS > > > > > > Documentation/gpu/vkms.rst | 23 +++++ > > > drivers/gpu/drm/Kconfig | 1 + > > > drivers/gpu/drm/vkms/Makefile | 1 + > > > drivers/gpu/drm/vkms/vkms_configfs.c | 129 +++++++++++++++++++++++++++ > > > drivers/gpu/drm/vkms/vkms_drv.c | 10 +++ > > > drivers/gpu/drm/vkms/vkms_drv.h | 25 ++++++ > > > drivers/gpu/drm/vkms/vkms_output.c | 2 + > > > drivers/gpu/drm/vkms/vkms_plane.c | 2 + > > > 8 files changed, 193 insertions(+) > > > create mode 100644 drivers/gpu/drm/vkms/vkms_configfs.c > > > > > > -- > > > 2.36.0.512.ge40c2bad7a-goog > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/1] drm/vkms: Add basic support for ConfigFS to VKMS
@ 2022-05-07 2:57 kernel test robot
0 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2022-05-07 2:57 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 2409 bytes --]
CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220506001822.890772-2-jshargo@chromium.org>
References: <20220506001822.890772-2-jshargo@chromium.org>
TO: Jim Shargo <jshargo@chromium.org>
Hi Jim,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on drm/drm-next]
[also build test WARNING on v5.18-rc5 next-20220506]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Jim-Shargo/drm-vkms-ConfigFS-support/20220506-182056
base: git://anongit.freedesktop.org/drm/drm drm-next
:::::: branch date: 17 hours ago
:::::: commit date: 17 hours ago
compiler: mipsel-linux-gcc (GCC) 11.3.0
reproduce (cppcheck warning):
# apt-get install cppcheck
git checkout e32fa3dd59fa0c63d6c7b007bddbc78a53cb40bc
cppcheck --quiet --enable=style,performance,portability --template=gcc FILE
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
cppcheck warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/vkms/vkms_configfs.c:15:2: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
snprintf(name, CONFIGFS_ITEM_NAME_LEN, "%d", id);
^
vim +15 drivers/gpu/drm/vkms/vkms_configfs.c
e32fa3dd59fa0c Jim Shargo 2022-05-06 8
e32fa3dd59fa0c Jim Shargo 2022-05-06 9 static void setup_configfs_object(uint32_t id, struct config_group *object,
e32fa3dd59fa0c Jim Shargo 2022-05-06 10 struct config_group *parent,
e32fa3dd59fa0c Jim Shargo 2022-05-06 11 struct config_item_type *type)
e32fa3dd59fa0c Jim Shargo 2022-05-06 12 {
e32fa3dd59fa0c Jim Shargo 2022-05-06 13 char name[CONFIGFS_ITEM_NAME_LEN];
e32fa3dd59fa0c Jim Shargo 2022-05-06 14
e32fa3dd59fa0c Jim Shargo 2022-05-06 @15 snprintf(name, CONFIGFS_ITEM_NAME_LEN, "%d", id);
e32fa3dd59fa0c Jim Shargo 2022-05-06 16 config_group_init_type_name(object, name, type);
e32fa3dd59fa0c Jim Shargo 2022-05-06 17 configfs_add_default_group(object, parent);
e32fa3dd59fa0c Jim Shargo 2022-05-06 18 }
e32fa3dd59fa0c Jim Shargo 2022-05-06 19
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 6+ messages in threadend of thread, other threads:[~2022-06-08 15:46 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-06 0:18 [RFC PATCH 0/1] drm/vkms: ConfigFS support Jim Shargo 2022-05-06 0:18 ` [RFC PATCH 1/1] drm/vkms: Add basic support for ConfigFS to VKMS Jim Shargo 2022-05-09 15:10 ` [RFC PATCH 0/1] drm/vkms: ConfigFS support Daniel Vetter 2022-06-06 21:59 ` Jim Shargo 2022-06-08 15:45 ` Daniel Vetter 2022-05-07 2:57 [RFC PATCH 1/1] drm/vkms: Add basic support for ConfigFS to VKMS kernel test robot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.