[PATCH] drm/vkms: Add support for vkms work without vblank

[PATCH] drm/vkms: Add support for vkms work without vblank

[Rodrigo Siqueira]

Currently, vkms needs VBlank to work well. This patch adds another
operation model that make vkms works without VBlank support.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 drivers/gpu/drm/vkms/vkms_crtc.c | 10 ++++++++++
 drivers/gpu/drm/vkms/vkms_drv.c  | 12 ++++++++++--
 drivers/gpu/drm/vkms/vkms_drv.h  |  1 +
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index bfe6e0312cc4..001fa5c1d326 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -145,12 +145,22 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
 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 5d78bd97e69c..f30c4113d4c4 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -21,6 +21,10 @@
 
 static struct vkms_device *vkms_device;
 
+static bool disable_vblank;
+module_param_named(disablevblank, disable_vblank, bool, 0444);
+MODULE_PARM_DESC(disablevblank, "Disable vblank interrupt");
+
 static const struct file_operations vkms_driver_fops = {
 	.owner		= THIS_MODULE,
 	.open		= drm_open,
@@ -105,9 +109,13 @@ static int __init vkms_init(void)
 		goto out_fini;
 	}
 
-	vkms_device->drm.irq_enabled = true;
+	vkms_device->output.disable_vblank = disable_vblank;
+	vkms_device->drm.irq_enabled = !disable_vblank ? true : false;
+
+	if (disable_vblank)
+		DRM_INFO("Virtual Hardware mode enabled");
 
-	ret = drm_vblank_init(&vkms_device->drm, 1);
+	ret = (disable_vblank) ? 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 f156c930366a..9ee862e6f7e5 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -52,6 +52,7 @@ struct vkms_output {
 	struct drm_encoder encoder;
 	struct drm_connector connector;
 	struct hrtimer vblank_hrtimer;
+	bool disable_vblank;
 	ktime_t period_ns;
 	struct drm_pending_vblank_event *event;
 	bool crc_enabled;
-- 
2.18.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/vkms: Add support for vkms work without vblank

[Daniel Vetter]

On Mon, Aug 20, 2018 at 08:55:44PM -0300, Rodrigo Siqueira wrote:
> Currently, vkms needs VBlank to work well. This patch adds another
> operation model that make vkms works without VBlank support.
> 
> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> ---
>  drivers/gpu/drm/vkms/vkms_crtc.c | 10 ++++++++++
>  drivers/gpu/drm/vkms/vkms_drv.c  | 12 ++++++++++--
>  drivers/gpu/drm/vkms/vkms_drv.h  |  1 +
>  3 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index bfe6e0312cc4..001fa5c1d326 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -145,12 +145,22 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
>  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 5d78bd97e69c..f30c4113d4c4 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -21,6 +21,10 @@
>  
>  static struct vkms_device *vkms_device;
>  
> +static bool disable_vblank;
> +module_param_named(disablevblank, disable_vblank, bool, 0444);
> +MODULE_PARM_DESC(disablevblank, "Disable vblank interrupt");

I think the info output is much clearer, I'd rename this to virtual_hw,
with 0 as default, and a help text along of "Enable virtual HW mode
(disables vblanks and immediately completes flips)."

> +
>  static const struct file_operations vkms_driver_fops = {
>  	.owner		= THIS_MODULE,
>  	.open		= drm_open,
> @@ -105,9 +109,13 @@ static int __init vkms_init(void)
>  		goto out_fini;
>  	}
>  
> -	vkms_device->drm.irq_enabled = true;
> +	vkms_device->output.disable_vblank = disable_vblank;
> +	vkms_device->drm.irq_enabled = !disable_vblank ? true : false;
> +
> +	if (disable_vblank)
> +		DRM_INFO("Virtual Hardware mode enabled");
>  
> -	ret = drm_vblank_init(&vkms_device->drm, 1);
> +	ret = (disable_vblank) ? 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 f156c930366a..9ee862e6f7e5 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -52,6 +52,7 @@ struct vkms_output {
>  	struct drm_encoder encoder;
>  	struct drm_connector connector;
>  	struct hrtimer vblank_hrtimer;
> +	bool disable_vblank;
>  	ktime_t period_ns;
>  	struct drm_pending_vblank_event *event;
>  	bool crc_enabled;

The vblank handling parts look correct to me.

Does kms_flip work with this? Virtual hw immediately completes the
drm_crtc_state->event in the atomic_update callback, that part seems to be
missing from your patch.

Also I like the idea of switching away from the generic -EINVAL to -ENOTTY
for when vblanks aren't supported. Do you have that somewhere (as a
separate patch).
-Daniel
-- 
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

Re: [PATCH] drm/vkms: Add support for vkms work without vblank

[Rodrigo Siqueira]

On 08/21, Daniel Vetter wrote:
> On Mon, Aug 20, 2018 at 08:55:44PM -0300, Rodrigo Siqueira wrote:
> > Currently, vkms needs VBlank to work well. This patch adds another
> > operation model that make vkms works without VBlank support.
> > 
> > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > ---
> >  drivers/gpu/drm/vkms/vkms_crtc.c | 10 ++++++++++
> >  drivers/gpu/drm/vkms/vkms_drv.c  | 12 ++++++++++--
> >  drivers/gpu/drm/vkms/vkms_drv.h  |  1 +
> >  3 files changed, 21 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> > index bfe6e0312cc4..001fa5c1d326 100644
> > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > @@ -145,12 +145,22 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
> >  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 5d78bd97e69c..f30c4113d4c4 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > @@ -21,6 +21,10 @@
> >  
> >  static struct vkms_device *vkms_device;
> >  
> > +static bool disable_vblank;
> > +module_param_named(disablevblank, disable_vblank, bool, 0444);
> > +MODULE_PARM_DESC(disablevblank, "Disable vblank interrupt");
> 
> I think the info output is much clearer, I'd rename this to virtual_hw,
> with 0 as default, and a help text along of "Enable virtual HW mode
> (disables vblanks and immediately completes flips)."

You right, virtual_hw is much better :)
I will fix it.
 
> > +
> >  static const struct file_operations vkms_driver_fops = {
> >  	.owner		= THIS_MODULE,
> >  	.open		= drm_open,
> > @@ -105,9 +109,13 @@ static int __init vkms_init(void)
> >  		goto out_fini;
> >  	}
> >  
> > -	vkms_device->drm.irq_enabled = true;
> > +	vkms_device->output.disable_vblank = disable_vblank;
> > +	vkms_device->drm.irq_enabled = !disable_vblank ? true : false;
> > +
> > +	if (disable_vblank)
> > +		DRM_INFO("Virtual Hardware mode enabled");
> >  
> > -	ret = drm_vblank_init(&vkms_device->drm, 1);
> > +	ret = (disable_vblank) ? 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 f156c930366a..9ee862e6f7e5 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > @@ -52,6 +52,7 @@ struct vkms_output {
> >  	struct drm_encoder encoder;
> >  	struct drm_connector connector;
> >  	struct hrtimer vblank_hrtimer;
> > +	bool disable_vblank;
> >  	ktime_t period_ns;
> >  	struct drm_pending_vblank_event *event;
> >  	bool crc_enabled;
> 
> The vblank handling parts look correct to me.
> 
> Does kms_flip work with this? Virtual hw immediately completes the
> drm_crtc_state->event in the atomic_update callback, that part seems to be
> missing from your patch.

I tested with patched version of kms_flip to skip vblank. Here is the patch:

https://patchwork.kernel.org/patch/10570861/

About the drm_crtc_state->event, afaiu, the code in the function
vkms_crtc_atomic_flush immediately send the vblank event to the user
space via drm_crtc_send_vblank_event which make things work in the user
space. Make sense? Or I missed something?

> Also I like the idea of switching away from the generic -EINVAL to -ENOTTY
> for when vblanks aren't supported. Do you have that somewhere (as a
> separate patch).

I already started to investigate if the change between -EINVAL to
-ENOTTY will break an existing kms userspace compositors. I already
checked:

* kwin
* sway
* wayland-core
* weston

I still have to check the X drivers, chromium, and drm_hwc.

Thanks for the feedback.

> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Rodrigo Siqueira
http://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/vkms: Add support for vkms work without vblank

[Daniel Vetter]

On Tue, Aug 21, 2018 at 4:52 PM, Rodrigo Siqueira
<rodrigosiqueiramelo@gmail.com> wrote:
> On 08/21, Daniel Vetter wrote:
>> On Mon, Aug 20, 2018 at 08:55:44PM -0300, Rodrigo Siqueira wrote:
>> > Currently, vkms needs VBlank to work well. This patch adds another
>> > operation model that make vkms works without VBlank support.
>> >
>> > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
>> > ---
>> >  drivers/gpu/drm/vkms/vkms_crtc.c | 10 ++++++++++
>> >  drivers/gpu/drm/vkms/vkms_drv.c  | 12 ++++++++++--
>> >  drivers/gpu/drm/vkms/vkms_drv.h  |  1 +
>> >  3 files changed, 21 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
>> > index bfe6e0312cc4..001fa5c1d326 100644
>> > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
>> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
>> > @@ -145,12 +145,22 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
>> >  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 5d78bd97e69c..f30c4113d4c4 100644
>> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
>> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
>> > @@ -21,6 +21,10 @@
>> >
>> >  static struct vkms_device *vkms_device;
>> >
>> > +static bool disable_vblank;
>> > +module_param_named(disablevblank, disable_vblank, bool, 0444);
>> > +MODULE_PARM_DESC(disablevblank, "Disable vblank interrupt");
>>
>> I think the info output is much clearer, I'd rename this to virtual_hw,
>> with 0 as default, and a help text along of "Enable virtual HW mode
>> (disables vblanks and immediately completes flips)."
>
> You right, virtual_hw is much better :)
> I will fix it.
>
>> > +
>> >  static const struct file_operations vkms_driver_fops = {
>> >     .owner          = THIS_MODULE,
>> >     .open           = drm_open,
>> > @@ -105,9 +109,13 @@ static int __init vkms_init(void)
>> >             goto out_fini;
>> >     }
>> >
>> > -   vkms_device->drm.irq_enabled = true;
>> > +   vkms_device->output.disable_vblank = disable_vblank;
>> > +   vkms_device->drm.irq_enabled = !disable_vblank ? true : false;
>> > +
>> > +   if (disable_vblank)
>> > +           DRM_INFO("Virtual Hardware mode enabled");
>> >
>> > -   ret = drm_vblank_init(&vkms_device->drm, 1);
>> > +   ret = (disable_vblank) ? 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 f156c930366a..9ee862e6f7e5 100644
>> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
>> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
>> > @@ -52,6 +52,7 @@ struct vkms_output {
>> >     struct drm_encoder encoder;
>> >     struct drm_connector connector;
>> >     struct hrtimer vblank_hrtimer;
>> > +   bool disable_vblank;
>> >     ktime_t period_ns;
>> >     struct drm_pending_vblank_event *event;
>> >     bool crc_enabled;
>>
>> The vblank handling parts look correct to me.
>>
>> Does kms_flip work with this? Virtual hw immediately completes the
>> drm_crtc_state->event in the atomic_update callback, that part seems to be
>> missing from your patch.
>
> I tested with patched version of kms_flip to skip vblank. Here is the patch:
>
> https://patchwork.kernel.org/patch/10570861/
>
> About the drm_crtc_state->event, afaiu, the code in the function
> vkms_crtc_atomic_flush immediately send the vblank event to the user
> space via drm_crtc_send_vblank_event which make things work in the user
> space. Make sense? Or I missed something?

Ah, the magic of the vblank_get() == 0 check. A bit tricky, but yes
this indeed works. I think it'd be good to explain this in the commit
message.

>> Also I like the idea of switching away from the generic -EINVAL to -ENOTTY
>> for when vblanks aren't supported. Do you have that somewhere (as a
>> separate patch).
>
> I already started to investigate if the change between -EINVAL to
> -ENOTTY will break an existing kms userspace compositors. I already
> checked:
>
> * kwin
> * sway
> * wayland-core
> * weston
>
> I still have to check the X drivers, chromium, and drm_hwc.

Awesome, keep up the great work.

Cheers, Daniel

>
> Thanks for the feedback.
>
>> -Daniel
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>
> --
> Rodrigo Siqueira
> http://siqueira.tech
> Graduate Student
> Department of Computer Science
> University of São Paulo



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/vkms: Add support for vkms work without vblank

[Rodrigo Siqueira]

Thanks for the feedbacks,

I will prepare a V2 based on your suggestions.

On 08/21, Daniel Vetter wrote:
> On Tue, Aug 21, 2018 at 4:52 PM, Rodrigo Siqueira
> <rodrigosiqueiramelo@gmail.com> wrote:
> > On 08/21, Daniel Vetter wrote:
> >> On Mon, Aug 20, 2018 at 08:55:44PM -0300, Rodrigo Siqueira wrote:
> >> > Currently, vkms needs VBlank to work well. This patch adds another
> >> > operation model that make vkms works without VBlank support.
> >> >
> >> > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> >> > ---
> >> >  drivers/gpu/drm/vkms/vkms_crtc.c | 10 ++++++++++
> >> >  drivers/gpu/drm/vkms/vkms_drv.c  | 12 ++++++++++--
> >> >  drivers/gpu/drm/vkms/vkms_drv.h  |  1 +
> >> >  3 files changed, 21 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> >> > index bfe6e0312cc4..001fa5c1d326 100644
> >> > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> >> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> >> > @@ -145,12 +145,22 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
> >> >  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 5d78bd97e69c..f30c4113d4c4 100644
> >> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> >> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> >> > @@ -21,6 +21,10 @@
> >> >
> >> >  static struct vkms_device *vkms_device;
> >> >
> >> > +static bool disable_vblank;
> >> > +module_param_named(disablevblank, disable_vblank, bool, 0444);
> >> > +MODULE_PARM_DESC(disablevblank, "Disable vblank interrupt");
> >>
> >> I think the info output is much clearer, I'd rename this to virtual_hw,
> >> with 0 as default, and a help text along of "Enable virtual HW mode
> >> (disables vblanks and immediately completes flips)."
> >
> > You right, virtual_hw is much better :)
> > I will fix it.
> >
> >> > +
> >> >  static const struct file_operations vkms_driver_fops = {
> >> >     .owner          = THIS_MODULE,
> >> >     .open           = drm_open,
> >> > @@ -105,9 +109,13 @@ static int __init vkms_init(void)
> >> >             goto out_fini;
> >> >     }
> >> >
> >> > -   vkms_device->drm.irq_enabled = true;
> >> > +   vkms_device->output.disable_vblank = disable_vblank;
> >> > +   vkms_device->drm.irq_enabled = !disable_vblank ? true : false;
> >> > +
> >> > +   if (disable_vblank)
> >> > +           DRM_INFO("Virtual Hardware mode enabled");
> >> >
> >> > -   ret = drm_vblank_init(&vkms_device->drm, 1);
> >> > +   ret = (disable_vblank) ? 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 f156c930366a..9ee862e6f7e5 100644
> >> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> >> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> >> > @@ -52,6 +52,7 @@ struct vkms_output {
> >> >     struct drm_encoder encoder;
> >> >     struct drm_connector connector;
> >> >     struct hrtimer vblank_hrtimer;
> >> > +   bool disable_vblank;
> >> >     ktime_t period_ns;
> >> >     struct drm_pending_vblank_event *event;
> >> >     bool crc_enabled;
> >>
> >> The vblank handling parts look correct to me.
> >>
> >> Does kms_flip work with this? Virtual hw immediately completes the
> >> drm_crtc_state->event in the atomic_update callback, that part seems to be
> >> missing from your patch.
> >
> > I tested with patched version of kms_flip to skip vblank. Here is the patch:
> >
> > https://patchwork.kernel.org/patch/10570861/
> >
> > About the drm_crtc_state->event, afaiu, the code in the function
> > vkms_crtc_atomic_flush immediately send the vblank event to the user
> > space via drm_crtc_send_vblank_event which make things work in the user
> > space. Make sense? Or I missed something?
> 
> Ah, the magic of the vblank_get() == 0 check. A bit tricky, but yes
> this indeed works. I think it'd be good to explain this in the commit
> message.
> 
> >> Also I like the idea of switching away from the generic -EINVAL to -ENOTTY
> >> for when vblanks aren't supported. Do you have that somewhere (as a
> >> separate patch).
> >
> > I already started to investigate if the change between -EINVAL to
> > -ENOTTY will break an existing kms userspace compositors. I already
> > checked:
> >
> > * kwin
> > * sway
> > * wayland-core
> > * weston
> >
> > I still have to check the X drivers, chromium, and drm_hwc.
> 
> Awesome, keep up the great work.
> 
> Cheers, Daniel
> 
> >
> > Thanks for the feedback.
> >
> >> -Daniel
> >> --
> >> Daniel Vetter
> >> Software Engineer, Intel Corporation
> >> http://blog.ffwll.ch
> >
> > --
> > Rodrigo Siqueira
> > http://siqueira.tech
> > Graduate Student
> > Department of Computer Science
> > University of São Paulo
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Rodrigo Siqueira
http://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel