On Fri, 2019-07-12 at 00:07 -0300, Rodrigo Siqueira wrote: > 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? Hi, I guess, some more stuff should happen during the hotplug, like drm_kms_helper_hotplug_event()? > > 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 I think it would be a good idea. This way the creation of new device could look like this: mkdir -p instanceN/connector/virtual0 echo "virtual" > instanceN/connector/virtual0/type echo 1 > instanceN/connector/virtual0/enable mkdir -p instanceN/crtc/crtc0 ... echo 1 > instanceN/enable Once the last command is executed, the whole instanceN/ becomes immutable, except for - instanceN/enable, so we can later disable it - instanceN/connector, where we can create new connectors, it would be treated as a hotplug. > > 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. We can test that userspace always select the right device to display. > > > - 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 But do we want to have those parameters as module options, if we then plan to replace those with configfs? > > 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 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Oleg Vasilev <oleg.vasilev@xxxxxxxxx> Intel Corporation
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
