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
