[PATCH 0/2] drm/vkms: Introduce basic support for configfs
Vasilev, Oleg
oleg.vasilev at intel.com
Mon Jul 15 11:09:24 UTC 2019
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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Oleg Vasilev <oleg.vasilev at intel.com>
Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3261 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20190715/5caf856b/attachment-0001.bin>
More information about the dri-devel
mailing list