On 07/10, Daniel Vetter wrote: > On Mon, Jul 01, 2019 at 12:23:39AM -0300, Rodrigo Siqueira wrote: > > This patchset introduces the support for configfs in vkms by adding a > > primary structure for handling the vkms subsystem and exposing > > connectors as a use case. This series allows enabling/disabling virtual > > and writeback connectors on the fly. The first patch of this series > > reworks the initialization and cleanup code of each type of connector, > > with this change, the second patch adds the configfs support for vkms. > > It is important to highlight that this patchset depends on > > https://patchwork.freedesktop.org/series/61738/. > > > > After applying this series, the user can utilize these features with the > > following steps: > > > > 1. Load vkms without parameter > > > > modprobe vkms > > > > 2. Mount a configfs filesystem > > > > mount -t configfs none /mnt/ > > > > After that, the vkms subsystem will look like this: > > > > vkms/ > > |__connectors > > |__Virtual > > |__ enable > > > > The connectors directories have information related to connectors, and > > as can be seen, the virtual connector is enabled by default. Inside a > > connector directory (e.g., Virtual) has an attribute named ‘enable’ > > which is used to enable and disable the target connector. For example, > > the Virtual connector has the enable attribute set to 1. If the user > > wants to enable the writeback connector it is required to use the mkdir > > command, as follows: > > > > cd /mnt/vkms/connectors > > mkdir Writeback > > > > After the above command, the writeback connector will be enabled, and > > the user could see the following tree: > > > > vkms/ > > |__connectors > > |__Virtual > > | |__ enable > > |__Writeback > > |__ enable > > > > If the user wants to remove the writeback connector, it is required to > > use the command rmdir, for example > > > > rmdir Writeback > > > > Another way to enable and disable a connector it is by using the enable > > attribute, for example, we can disable the Virtual connector with: > > > > echo 0 > /mnt/vkms/connectors/Virtual/enable > > > > And enable it again with: > > > > echo 1 > /mnt/vkms/connectors/Virtual/enable > > > > It is important to highlight that configfs 'obey' the parameters used > > during the vkms load and does not allow users to remove a connector > > directory if it was load via module parameter. For example: > > > > modprobe vkms enable_writeback=1 > > > > vkms/ > > |__connectors > > |__Virtual > > | |__ enable > > |__Writeback > > |__ enable > > > > If the user tries to remove the Writeback connector with “rmdir > > Writeback”, the operation will be not permitted because the Writeback > > connector was loaded with the modules. However, the user may disable the > > writeback connector with: > > > > echo 0 > /mnt/vkms/connectors/Writeback/enable Thanks for detail this issue, I just have some few questions inline. > I guess I should have put a warning into that task that step one is > designing the interface. Here's the fundamental thoughts: > > - The _only_ thing we can hotplug after drm_dev_register() is a > drm_connector. That's an interesting use-case, but atm not really > supported by the vkms codebase. So we can't just enable/disable > writeback like this. We also can't change _anything_ else in the drm > driver like this. In the first patch of this series, I tried to decouple enable/disable for virtual and writeback connectors; I tried to take advantage of drm_connector_[register/unregister] in each connector. Can we use the first patch or it doesn't make sense? I did not understand why writeback connectors should not be registered and unregister by calling drm_connector_[register/unregister], is it a writeback or vkms limitation? Could you detail why we cannot change connectors as I did? Additionally, below you said "enable going from 1 -> 0, needs to be treated like a physical hotunplug", do you mean that we first have to add support for drm_dev_plug and drm_dev_unplug in vkms? > - The other bit we want is support multiple vkms instances, to simulate > multi-gpus and fun stuff like that. Do you mean something like this: configfs/vkms/instance1 |_enable_device |_more_stuff configfs/vkms/instance2 |_enable_device |_more_stuff configfs/vkms/instanceN |_enable_device |_more_stuff Will each instance work like a totally independent device? What is the main benefit of this? I can think about some use case for testing prime, but I cannot see other things. > - Therefore vkms configs should be at the drm_device level, so a > directory under configfs/vkms/ represents an entire device. > > - We need a special config item to control > drm_dev_register/drm_dev_unregister. While a drm_device is registers, > all other config items need to fail if userspace tries to change them. > Maybe this should be a top-level "enable" property. > > - Every time enable goes from 0 -> 1 we need to create a completely new > vkms instance. The old one might still be around, waiting for the last > few references to disappear. > > - enable going from 1 -> 0 needs to be treated like a physical hotunplug, > i.e. not drm_dev_unregister but drm_dev_unplug. We also need to annotate > all the vkms code with drm_dev_enter/exit() as the kerneldoc of > drm_dev_unplug explains. > > - rmdir should be treated like enable going from 1 -> 0. Or maybe we > should disable enable every going from 1 -> 0, would propably simplify > everything. > > - The initial instance created at module load also neeeds to be removable > like this. > > Once we have all this, then can we start to convert driver module options > over to configs and add cool features. But lots of infrastructure needed > first. > > Also, we probably want some nasty testcases which races an rmdir in > configfs against userspace still doing ioctl calls against vkms. This is > ideal for validation the hotunplug infrastructure we have in drm. > > An alternative is adding connector hotplugging. But I think before we do > that we need to have support for more crtc and more connectors as static > module options. So maybe not a good starting point for configfs. So, probably the first set of tasks should be: 1. Enable multiple CRTC via module parameters. For example: modprobe vkms crtcs=13 2. Enable multiple connectors via module parameters. For example: modprobe vkms crtcs=3 connector=3 // 3 virtual connectors per crtc Thanks again, > The above text should probably be added to the vkms.rst todo item ... > -Daniel > > > > > > > Rodrigo Siqueira (2): > > drm/vkms: Add enable/disable functions per connector > > drm/vkms: Introduce configfs for enabling/disabling connectors > > > > drivers/gpu/drm/vkms/Makefile | 3 +- > > drivers/gpu/drm/vkms/vkms_configfs.c | 229 ++++++++++++++++++++++++++ > > drivers/gpu/drm/vkms/vkms_drv.c | 6 + > > drivers/gpu/drm/vkms/vkms_drv.h | 17 ++ > > drivers/gpu/drm/vkms/vkms_output.c | 84 ++++++---- > > drivers/gpu/drm/vkms/vkms_writeback.c | 31 +++- > > 6 files changed, 332 insertions(+), 38 deletions(-) > > create mode 100644 drivers/gpu/drm/vkms/vkms_configfs.c > > > > -- > > 2.21.0 > > > > > > -- > > Rodrigo Siqueira > > https://siqueira.tech > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Rodrigo Siqueira https://siqueira.tech
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
