Nouveau - Jan 2014 - [PATCH] drm/nouveau: set irq_enabled manually

Since commit 0fa9061ae8c ("drm/nouveau/mc: handle irq-related setup
ourselves"), drm_device->irq_enabled remained unset. This is needed in
order to properly wait for a vblank event in the generic drm code.

See https://bugs.freedesktop.org/show_bug.cgi?id=74195

Reported-by: Jan Janecek <janjanjanx at gmail.com>
Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
Cc: stable at vger.kernel.org # 3.10+
---

TBH, not sure why this fixes things, as irq_enabled == false should have
caused the vblank wait to not wait, since the condition would be
immediately true.

Jan, mind double-checking that this version of the patch fixes things
for you? Not 100% sure where you stuck the irq_enabled=true line when you
tried it out.

 drivers/gpu/drm/nouveau/nouveau_drm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index bfd02410..3ba7b62 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -376,6 +376,8 @@ nouveau_drm_load(struct drm_device *dev, unsigned long
flags)
 	if (ret)
 		goto fail_device;
 
+	dev->irq_enabled = true;
+
 	/* workaround an odd issue on nvc1 by disabling the device's
 	 * nosnoop capability.  hopefully won't cause issues until a
 	 * better fix is found - assuming there is one...
@@ -475,6 +477,7 @@ nouveau_drm_remove(struct pci_dev *pdev)
 	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct nouveau_object *device;
 
+	dev->irq_enabled = false;
 	device = drm->client.base.device;
 	drm_put_dev(dev);
 
-- 
1.8.3.2

I can confirm that this patch fixes the problem. (had to change spaces
to tabs, but that was probably just screwed up by mail)

2014-01-30, Ilia Mirkin <imirkin at alum.mit.edu>:
> Since commit 0fa9061ae8c ("drm/nouveau/mc: handle irq-related setup
> ourselves"), drm_device->irq_enabled remained unset. This is needed
in
> order to properly wait for a vblank event in the generic drm code.
>
> See https://bugs.freedesktop.org/show_bug.cgi?id=74195
>
> Reported-by: Jan Janecek <janjanjanx at gmail.com>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: stable at vger.kernel.org # 3.10+
> ---
>
> TBH, not sure why this fixes things, as irq_enabled == false should have
> caused the vblank wait to not wait, since the condition would be
> immediately true.
>
> Jan, mind double-checking that this version of the patch fixes things
> for you? Not 100% sure where you stuck the irq_enabled=true line when you
> tried it out.
>
>  drivers/gpu/drm/nouveau/nouveau_drm.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
> b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index bfd02410..3ba7b62 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -376,6 +376,8 @@ nouveau_drm_load(struct drm_device *dev, unsigned long
> flags)
>  	if (ret)
>  		goto fail_device;
>
> +	dev->irq_enabled = true;
> +
>  	/* workaround an odd issue on nvc1 by disabling the device's
>  	 * nosnoop capability.  hopefully won't cause issues until a
>  	 * better fix is found - assuming there is one...
> @@ -475,6 +477,7 @@ nouveau_drm_remove(struct pci_dev *pdev)
>  	struct nouveau_drm *drm = nouveau_drm(dev);
>  	struct nouveau_object *device;
>
> +	dev->irq_enabled = false;
>  	device = drm->client.base.device;
>  	drm_put_dev(dev);
>
> --
> 1.8.3.2
>
>

On Thu, Jan 30, 2014 at 1:53 AM, Ilia Mirkin <imirkin at alum.mit.edu>
wrote:
> Since commit 0fa9061ae8c ("drm/nouveau/mc: handle irq-related setup
> ourselves"), drm_device->irq_enabled remained unset. This is needed
in
> order to properly wait for a vblank event in the generic drm code.
>
> See https://bugs.freedesktop.org/show_bug.cgi?id=74195
>
> Reported-by: Jan Janecek <janjanjanx at gmail.com>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: stable at vger.kernel.org # 3.10+
> ---
>
> TBH, not sure why this fixes things, as irq_enabled == false should have
> caused the vblank wait to not wait, since the condition would be
> immediately true.
>
> Jan, mind double-checking that this version of the patch fixes things
> for you? Not 100% sure where you stuck the irq_enabled=true line when you
> tried it out.
The core drm vblank code bails out if dev->irq_enabled isn't set. So
if you opt to not use the drm irq helpers and instead roll your own
you still need to set this to allow vblank wait ioctls and related
stuff. It's even documented in the drm DocBook ;-) So

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

if you will.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

On Thu, Jan 30, 2014 at 3:33 AM, Daniel Vetter <daniel at ffwll.ch>
wrote:
> On Thu, Jan 30, 2014 at 1:53 AM, Ilia Mirkin <imirkin at
alum.mit.edu> wrote:
>> Since commit 0fa9061ae8c ("drm/nouveau/mc: handle irq-related
setup
>> ourselves"), drm_device->irq_enabled remained unset. This is
needed in
>> order to properly wait for a vblank event in the generic drm code.
>>
>> See https://bugs.freedesktop.org/show_bug.cgi?id=74195
>>
>> Reported-by: Jan Janecek <janjanjanx at gmail.com>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> Cc: stable at vger.kernel.org # 3.10+
>> ---
>>
>> TBH, not sure why this fixes things, as irq_enabled == false should
have
>> caused the vblank wait to not wait, since the condition would be
>> immediately true.
>>
>> Jan, mind double-checking that this version of the patch fixes things
>> for you? Not 100% sure where you stuck the irq_enabled=true line when
you
>> tried it out.
>
> The core drm vblank code bails out if dev->irq_enabled isn't set. So
Right. And what I'm unclear on is how does bailing out on vblank wait
cause the originally reported issue -- sluggishness. That seems to
imply that one is waiting too long rather than not waiting enough.
> if you opt to not use the drm irq helpers and instead roll your own
> you still need to set this to allow vblank wait ioctls and related
> stuff. It's even documented in the drm DocBook ;-) So
>
> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>
> if you will.
>
> Cheers, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch