| Message ID | 20201009232156.3916879-3-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/vkms: Set preferred depth correctly | expand |
Hi On Sat, 10 Oct 2020 01:21:56 +0200 Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > Hooray for generic fbdev support, making this a oneliner. We just > needed to fix preferred_depth fixed and the vmap support added first. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > Cc: Melissa Wen <melissa.srw@gmail.com> > Cc: Haneen Mohammed <hamohammed.sa@gmail.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > --- > drivers/gpu/drm/vkms/vkms_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > index 6221e5040264..cc09e2df5cb1 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.c > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > @@ -169,6 +169,8 @@ static int __init vkms_init(void) > if (ret) > goto out_devres; > > + drm_fbdev_generic_setup(&vkms_device->drm, 0); > + It feels strange to have console support in a driver for non-interactive systems. But OK, why not. I guess it helps with testing? Acked-by: Thomas Zimmermann <tzimmermann@suse.de> Best regards Thomas > return 0; > > out_devres:
Hi, On 12/10/2020 13:24, Thomas Zimmermann wrote: > Hi > > On Sat, 10 Oct 2020 01:21:56 +0200 Daniel Vetter <daniel.vetter@ffwll.ch> > wrote: > >> Hooray for generic fbdev support, making this a oneliner. We just >> needed to fix preferred_depth fixed and the vmap support added first. >> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> >> Cc: Melissa Wen <melissa.srw@gmail.com> >> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> >> Cc: Daniel Vetter <daniel@ffwll.ch> >> --- >> drivers/gpu/drm/vkms/vkms_drv.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c >> index 6221e5040264..cc09e2df5cb1 100644 >> --- a/drivers/gpu/drm/vkms/vkms_drv.c >> +++ b/drivers/gpu/drm/vkms/vkms_drv.c >> @@ -169,6 +169,8 @@ static int __init vkms_init(void) >> if (ret) >> goto out_devres; >> >> + drm_fbdev_generic_setup(&vkms_device->drm, 0); >> + > > It feels strange to have console support in a driver for non-interactive > systems. But OK, why not. I guess it helps with testing? It's weird because it the kernel is misconfigured and no console is specified on the cmdline this console could become the main console... It's a great feature, but couldn't this be a module parameter ? Neil > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > > Best regards > Thomas > >> return 0; >> >> out_devres: > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
On Mon, Oct 12, 2020 at 02:40:58PM +0200, Neil Armstrong wrote: > Hi, > > On 12/10/2020 13:24, Thomas Zimmermann wrote: > > Hi > > > > On Sat, 10 Oct 2020 01:21:56 +0200 Daniel Vetter <daniel.vetter@ffwll.ch> > > wrote: > > > >> Hooray for generic fbdev support, making this a oneliner. We just > >> needed to fix preferred_depth fixed and the vmap support added first. > >> > >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > >> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > >> Cc: Melissa Wen <melissa.srw@gmail.com> > >> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> > >> Cc: Daniel Vetter <daniel@ffwll.ch> > >> --- > >> drivers/gpu/drm/vkms/vkms_drv.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > >> index 6221e5040264..cc09e2df5cb1 100644 > >> --- a/drivers/gpu/drm/vkms/vkms_drv.c > >> +++ b/drivers/gpu/drm/vkms/vkms_drv.c > >> @@ -169,6 +169,8 @@ static int __init vkms_init(void) > >> if (ret) > >> goto out_devres; > >> > >> + drm_fbdev_generic_setup(&vkms_device->drm, 0); > >> + > > > > It feels strange to have console support in a driver for non-interactive > > systems. But OK, why not. I guess it helps with testing? Yeah I want to polish the igt fbdev testcase a bit, need a victim for fb device selction. vkms was the quickest way to get two fbdev instances into one box :-) Plus more code we could test using igt & vkms in gitlab CI, whenever we get around to that ... > It's weird because it the kernel is misconfigured and no console is specified on the cmdline > this console could become the main console... > > It's a great feature, but couldn't this be a module parameter ? If you have vkms enabled in a distro, you're doing it wrong. Also, if you accidentally load vkms, then Xorg will happily bind to it (maybe in preference to the real fbdev driver that's there as a simplefb). So imo this problem isn't really new. -Daniel > > Neil > > > > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > > > > Best regards > > Thomas > > > >> return 0; > >> > >> out_devres: > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > >
On Mon, 12 Oct 2020 16:23:35 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, Oct 12, 2020 at 02:40:58PM +0200, Neil Armstrong wrote: > > Hi, > > ... > > It's weird because it the kernel is misconfigured and no console is specified on the cmdline > > this console could become the main console... > > > > It's a great feature, but couldn't this be a module parameter ? > > If you have vkms enabled in a distro, you're doing it wrong. That's really not a great position to take. I would prefer that if a random contributor writes a Weston patch and runs 'meson test', it will use VKMS to run Weston's DRM-backend tests on his machine automatically, maybe save for some seat and device node access permissions bits which distributions could be delivering as well. Just put the VKMS device node into a non-default seat, and Xorg etc. will happily ignore it. For the fbdev device node, I don't know. Maybe a module parameter really is a good choice there, defaulting to off. I have no interest in testing anything against fbdev, but other people might disagree of course. Why? Gitlab CI is still not running tests for every commit, just per MR, and it might even be infeasible too. I am also hoping for a future where I don't have to build my own kernel just to be able to run Weston DRM tests with VKMS. That means I want to be able to run my machine with VKMS loaded and active at all times, without affecting the normal desktop. I already have such a setup with an extra AMD card, but you can't run most KMS tests against real hardware drivers. Thanks, pq
On Tue, Oct 13, 2020 at 8:14 AM Pekka Paalanen <ppaalanen@gmail.com> wrote: > > On Mon, 12 Oct 2020 16:23:35 +0200 > Daniel Vetter <daniel@ffwll.ch> wrote: > > > On Mon, Oct 12, 2020 at 02:40:58PM +0200, Neil Armstrong wrote: > > > Hi, > > > > > ... > > > > It's weird because it the kernel is misconfigured and no console is specified on the cmdline > > > this console could become the main console... > > > > > > It's a great feature, but couldn't this be a module parameter ? > > > > If you have vkms enabled in a distro, you're doing it wrong. > > That's really not a great position to take. I would prefer that > if a random contributor writes a Weston patch and runs 'meson test', it > will use VKMS to run Weston's DRM-backend tests on his machine > automatically, maybe save for some seat and device node access > permissions bits which distributions could be delivering as well. > > Just put the VKMS device node into a non-default seat, and Xorg etc. > will happily ignore it. > > For the fbdev device node, I don't know. Maybe a module parameter > really is a good choice there, defaulting to off. I have no interest in > testing anything against fbdev, but other people might disagree of > course. > > Why? Gitlab CI is still not running tests for every commit, just per > MR, and it might even be infeasible too. > > I am also hoping for a future where I don't have to build my own kernel > just to be able to run Weston DRM tests with VKMS. That means I want to > be able to run my machine with VKMS loaded and active at all times, > without affecting the normal desktop. I already have such a setup with > an extra AMD card, but you can't run most KMS tests against real > hardware drivers. I just realized that building vkms is no problem, since it doesn't auto-load. And if our Grand Plans with configurability come true, then your test-harness will want to do that loading and setup itself anyway. With that there also shouldn't be any problems with fbcon, since presumably you already have that bound to the real gpu. So I think we're all fine here, for everyone. Now if you built-in vkms, that's a different thing. And for that I really think a "don't do that" is the right choice. -Daniel
On Tue, 13 Oct 2020 09:53:44 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > On Tue, Oct 13, 2020 at 8:14 AM Pekka Paalanen <ppaalanen@gmail.com> wrote: > > > > On Mon, 12 Oct 2020 16:23:35 +0200 > > Daniel Vetter <daniel@ffwll.ch> wrote: > > > > > On Mon, Oct 12, 2020 at 02:40:58PM +0200, Neil Armstrong wrote: > > > > Hi, > > > > > > > > ... > > > > > > It's weird because it the kernel is misconfigured and no console is specified on the cmdline > > > > this console could become the main console... > > > > > > > > It's a great feature, but couldn't this be a module parameter ? > > > > > > If you have vkms enabled in a distro, you're doing it wrong. > > > > That's really not a great position to take. I would prefer that > > if a random contributor writes a Weston patch and runs 'meson test', it > > will use VKMS to run Weston's DRM-backend tests on his machine > > automatically, maybe save for some seat and device node access > > permissions bits which distributions could be delivering as well. > > > > Just put the VKMS device node into a non-default seat, and Xorg etc. > > will happily ignore it. > > > > For the fbdev device node, I don't know. Maybe a module parameter > > really is a good choice there, defaulting to off. I have no interest in > > testing anything against fbdev, but other people might disagree of > > course. > > > > Why? Gitlab CI is still not running tests for every commit, just per > > MR, and it might even be infeasible too. > > > > I am also hoping for a future where I don't have to build my own kernel > > just to be able to run Weston DRM tests with VKMS. That means I want to > > be able to run my machine with VKMS loaded and active at all times, > > without affecting the normal desktop. I already have such a setup with > > an extra AMD card, but you can't run most KMS tests against real > > hardware drivers. > > I just realized that building vkms is no problem, since it doesn't > auto-load. And if our Grand Plans with configurability come true, then > your test-harness will want to do that loading and setup itself > anyway. With that there also shouldn't be any problems with fbcon, > since presumably you already have that bound to the real gpu. > > So I think we're all fine here, for everyone. > > Now if you built-in vkms, that's a different thing. And for that I > really think a "don't do that" is the right choice. Very good. My remaining wish is that VKMS would be fully configurable and usable by an ordinary user, but I suppose that should be solved with a privileged userspace daemon somewhat similar to logind that hands out VKMS "sessions" somehow. Not sure configfs is the best choice for VKMS configuration, unless maybe unprivileged userspace could ask for a VKMS instance with its own configfs tree it can access without CAP_ADMIN... Thanks, pq
On Tue, Oct 13, 2020 at 01:19:38PM +0300, Pekka Paalanen wrote: > On Tue, 13 Oct 2020 09:53:44 +0200 > Daniel Vetter <daniel@ffwll.ch> wrote: > > > On Tue, Oct 13, 2020 at 8:14 AM Pekka Paalanen <ppaalanen@gmail.com> wrote: > > > > > > On Mon, 12 Oct 2020 16:23:35 +0200 > > > Daniel Vetter <daniel@ffwll.ch> wrote: > > > > > > > On Mon, Oct 12, 2020 at 02:40:58PM +0200, Neil Armstrong wrote: > > > > > Hi, > > > > > > > > > > > ... > > > > > > > > It's weird because it the kernel is misconfigured and no console is specified on the cmdline > > > > > this console could become the main console... > > > > > > > > > > It's a great feature, but couldn't this be a module parameter ? > > > > > > > > If you have vkms enabled in a distro, you're doing it wrong. > > > > > > That's really not a great position to take. I would prefer that > > > if a random contributor writes a Weston patch and runs 'meson test', it > > > will use VKMS to run Weston's DRM-backend tests on his machine > > > automatically, maybe save for some seat and device node access > > > permissions bits which distributions could be delivering as well. > > > > > > Just put the VKMS device node into a non-default seat, and Xorg etc. > > > will happily ignore it. > > > > > > For the fbdev device node, I don't know. Maybe a module parameter > > > really is a good choice there, defaulting to off. I have no interest in > > > testing anything against fbdev, but other people might disagree of > > > course. > > > > > > Why? Gitlab CI is still not running tests for every commit, just per > > > MR, and it might even be infeasible too. > > > > > > I am also hoping for a future where I don't have to build my own kernel > > > just to be able to run Weston DRM tests with VKMS. That means I want to > > > be able to run my machine with VKMS loaded and active at all times, > > > without affecting the normal desktop. I already have such a setup with > > > an extra AMD card, but you can't run most KMS tests against real > > > hardware drivers. > > > > I just realized that building vkms is no problem, since it doesn't > > auto-load. And if our Grand Plans with configurability come true, then > > your test-harness will want to do that loading and setup itself > > anyway. With that there also shouldn't be any problems with fbcon, > > since presumably you already have that bound to the real gpu. > > > > So I think we're all fine here, for everyone. > > > > Now if you built-in vkms, that's a different thing. And for that I > > really think a "don't do that" is the right choice. > > Very good. > > My remaining wish is that VKMS would be fully configurable and usable > by an ordinary user, but I suppose that should be solved with a > privileged userspace daemon somewhat similar to logind that hands out > VKMS "sessions" somehow. > > Not sure configfs is the best choice for VKMS configuration, unless > maybe unprivileged userspace could ask for a VKMS instance with its own > configfs tree it can access without CAP_ADMIN... My idea for testing would be to build a gitlab docker image and run that under usermode linux, with vkms built-in. Given how few people work on this, I don't think you get a fancy logind configuration session thing for it anytime soon. -Daniel
Hi, Thanks for this improvement. I could see that it increased the IGT test coverage, including now the fbdev test cases. On 10/10, Daniel Vetter wrote: > Hooray for generic fbdev support, making this a oneliner. We just > needed to fix preferred_depth fixed and the vmap support added first. I consider that including in the msg that, with this patch, both fbdev test cases [info and mmap] are passing would be interesting for future debugs. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > Cc: Melissa Wen <melissa.srw@gmail.com> > Cc: Haneen Mohammed <hamohammed.sa@gmail.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > --- > drivers/gpu/drm/vkms/vkms_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > index 6221e5040264..cc09e2df5cb1 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.c > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > @@ -169,6 +169,8 @@ static int __init vkms_init(void) > if (ret) > goto out_devres; > > + drm_fbdev_generic_setup(&vkms_device->drm, 0); > + > return 0; > > out_devres: > -- > 2.28.0 > Looks good to me, Reviewed-by: Melissa Wen <melissa.srw@gmail.com>
On Fri, Oct 16, 2020 at 07:10:56AM -0300, Melissa Wen wrote: > Hi, > > Thanks for this improvement. > > I could see that it increased the IGT test coverage, including now the > fbdev test cases. > > On 10/10, Daniel Vetter wrote: > > Hooray for generic fbdev support, making this a oneliner. We just > > needed to fix preferred_depth fixed and the vmap support added first. > > I consider that including in the msg that, with this patch, both fbdev > test cases [info and mmap] are passing would be interesting for future > debugs. Done and patch applied, thanks for taking a look. I'll also follow up with a quick patch to paper over the fbdev vs preferred_depth issue, until we have a proper solution for all that. -Daniel > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > Cc: Melissa Wen <melissa.srw@gmail.com> > > Cc: Haneen Mohammed <hamohammed.sa@gmail.com> > > Cc: Daniel Vetter <daniel@ffwll.ch> > > --- > > drivers/gpu/drm/vkms/vkms_drv.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > > index 6221e5040264..cc09e2df5cb1 100644 > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > @@ -169,6 +169,8 @@ static int __init vkms_init(void) > > if (ret) > > goto out_devres; > > > > + drm_fbdev_generic_setup(&vkms_device->drm, 0); > > + > > return 0; > > > > out_devres: > > -- > > 2.28.0 > > > > Looks good to me, > > Reviewed-by: Melissa Wen <melissa.srw@gmail.com>
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c index 6221e5040264..cc09e2df5cb1 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.c +++ b/drivers/gpu/drm/vkms/vkms_drv.c @@ -169,6 +169,8 @@ static int __init vkms_init(void) if (ret) goto out_devres; + drm_fbdev_generic_setup(&vkms_device->drm, 0); + return 0; out_devres:
Hooray for generic fbdev support, making this a oneliner. We just needed to fix preferred_depth fixed and the vmap support added first. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Melissa Wen <melissa.srw@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> --- drivers/gpu/drm/vkms/vkms_drv.c | 2 ++ 1 file changed, 2 insertions(+)