| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20190712025718.wdlafaujpxhpupj7@smtp.gmail.com>
Date: Thu, 11 Jul 2019 23:57:18 -0300
From: Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>
To: Haneen Mohammed <hamohammed.sa@...il.com>,
David Airlie <airlied@...ux.ie>,
Simon Ser <contact@...rsion.fr>,
Oleg Vasilev <oleg.vasilev@...el.com>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V3] drm/vkms: Add support for vkms work without vblank
On 07/10, Daniel Vetter wrote:
> On Tue, Jul 09, 2019 at 10:55:14PM -0300, Rodrigo Siqueira wrote:
> > Currently, vkms only work with enabled VBlank. This patch adds another
> > operation model that allows vkms to work without VBlank support. In this
> > scenario, vblank signaling is faked by calling drm_send_vblank_event()
> > in vkms_crtc_atomic_flush(); this approach works due to the
> > drm_vblank_get() == 0 checking.
> >
> > Changes since V2:
> > - Rebase
> >
> > Changes since V1:
> > Daniel Vetter:
> > - Change module parameter name from disablevblank to virtual_hw
> > - Improve parameter description
> > - Improve commit message
> >
> > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>
> > ---
> > drivers/gpu/drm/vkms/vkms_crtc.c | 10 ++++++++++
> > drivers/gpu/drm/vkms/vkms_drv.c | 13 +++++++++++--
> > drivers/gpu/drm/vkms/vkms_drv.h | 2 ++
> > 3 files changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> > index 49a8ec2cb1c1..a0c75b8c4335 100644
> > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > @@ -207,12 +207,22 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
> > static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
> > struct drm_crtc_state *old_state)
> > {
> > + struct vkms_output *vkms_out = drm_crtc_to_vkms_output(crtc);
> > +
> > + if (vkms_out->disable_vblank)
> > + return;
> > +
> > drm_crtc_vblank_on(crtc);
> > }
> >
> > static void vkms_crtc_atomic_disable(struct drm_crtc *crtc,
> > struct drm_crtc_state *old_state)
> > {
> > + struct vkms_output *vkms_out = drm_crtc_to_vkms_output(crtc);
> > +
> > + if (vkms_out->disable_vblank)
> > + return;
> > +
> > drm_crtc_vblank_off(crtc);
> > }
> >
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> > index 152d7de24a76..542a002ef9d5 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > @@ -34,6 +34,11 @@ bool enable_writeback;
> > module_param_named(enable_writeback, enable_writeback, bool, 0444);
> > MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector");
> >
> > +bool virtual_hw;
>
> Can be static, you only use this in vkms_drv.c.
>
> > +module_param_named(virtual_hw, virtual_hw, bool, 0444);
> > +MODULE_PARM_DESC(virtual_hw,
> > + "Enable virtual hardware mode (disables vblanks and immediately completes flips)");
> > +
> > static const struct file_operations vkms_driver_fops = {
> > .owner = THIS_MODULE,
> > .open = drm_open,
> > @@ -154,9 +159,13 @@ static int __init vkms_init(void)
> > if (ret)
> > goto out_unregister;
> >
> > - vkms_device->drm.irq_enabled = true;
> > + vkms_device->output.disable_vblank = virtual_hw;
> > + vkms_device->drm.irq_enabled = !virtual_hw;
> > +
> > + if (virtual_hw)
> > + DRM_INFO("Virtual hardware mode enabled");
> >
> > - ret = drm_vblank_init(&vkms_device->drm, 1);
> > + ret = (virtual_hw) ? 0 : drm_vblank_init(&vkms_device->drm, 1);
> > if (ret) {
> > DRM_ERROR("Failed to vblank\n");
> > goto out_fini;
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> > index 9ff2cd4ebf81..256e5e65c947 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > @@ -21,6 +21,7 @@
> >
> > extern bool enable_cursor;
> > extern bool enable_writeback;
> > +extern bool virtual_hw;
> >
> > struct vkms_composer {
> > struct drm_framebuffer fb;
> > @@ -69,6 +70,7 @@ struct vkms_output {
> > struct drm_connector connector;
> > struct drm_writeback_connector wb_connector;
> > struct hrtimer vblank_hrtimer;
> > + bool disable_vblank;
> > ktime_t period_ns;
> > struct drm_pending_vblank_event *event;
> > /* ordered wq for composer_work */
>
> I'm kinda wondering how this works at all ... does writeback/crc still
> work if you set virtual mode? Writeback since this seems based on the
> writeback series ...
Hi,
I tested this patch with kms_flip with the "drm/drm_vblank: Change
EINVAL by the correct errno" patch [1]. However, you’re right about the
writeback/crc tests, they does not pass because this patch disables
vblank which in turn does not invoke "vkms_vblank_simulate()".
In this sense, I’m a little bit confused about how should I handle this
issue. If I correctly understood, without vblank support we have to
trigger vkms_composer_worker() when the userspace invoke
drmModePageFlip(), am I right? If so, an approach could add page_flip()
callback to vkms and invoke vkms_composer_worker(); however, the
documentation says that page_flip is a legacy entry point. Do you have a
suggestion?
1. https://patchwork.freedesktop.org/patch/314399/?series=50697&rev=7
Thanks
> btw if you send out patches that need other patches, point at that other
> series in the cover letter or patch. Gets confusing fast otherwise.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Rodrigo Siqueira
https://siqueira.tech
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists