| Message ID | cover.1561950553.git.rodrigosiqueiramelo@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/vkms: Introduce basic support for configfs | expand |
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 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. - The other bit we want is support multiple vkms instances, to simulate multi-gpus and fun stuff like that. - 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. 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
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
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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Jul 15, 2019 at 11:09:24AM +0000, Vasilev, Oleg wrote: > 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? No, because you also add the drm_encoder. That cannot be hotadded/removed right now. So you'd need to add a drm_encoder preemptively for all writeback connectors you might need, which doesn't make much 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()? writeback is generally a SoC feature. I don't think anyone expects that you can hotplug a writeback connector. > > > > > 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 virtual should be a top-level property. I'm not aware of any real driver where vblank works on some connector/crtc, but not on another one. > echo 1 > instanceN/connector/virtual0/enable For the initial connectors those don't need separate enable properties, they get enabled with the global enable knob. > 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. For hotplugged connectors we need a separate enable knob. But we're still _very_ far away from making that possible I think. > > 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. Also hotunplug behaviour. We can take out an entire drm_device, and make sure userspace and the kernel copes correctly. This is _really_ hard to get right on both sides. And it's a real thing with stuff like usb display link. > > > - 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? Imo we should phase out module options, or at least not add more. Then you can just have a small script which sets up the vkms device you want, and then run the tests. I'd go further even, and say we should have default_device=0 knob to start out with nothing when you load the driver. -Daniel > > > > > 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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > -- > Oleg Vasilev <oleg.vasilev@intel.com> > Intel Corporation > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
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 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