Re: [PATCH 05/18] drm/irq: remove cargo-culted locking from irq_install/unistall

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1584 bytes --]

On Fri, Apr 11, 2014 at 11:36:02PM +0200, Daniel Vetter wrote:
> The dev->struct_mutex locking in drm_irq.c only protects
> dev->irq_enabled. Which isn't really much at all and only prevents
> especially nasty ums userspace from concurrently installing the
> interrupt handling a few times. Or at least trying.
> 
> There are tons of unlocked readers of dev->irqs_enabled in the vblank
> wait code (and by extension also in the pageflip code since that uses
> the same vblank timestamp engine).
> 
> Real modesetting drivers should ensure that nothing can go haywire
> with a sane setup teardown sequence. So we only really need this for
> the drm_control ioctl, everywhere else this will just paper over
> nastiness.
> 
> Note that drm/i915 is a bit specially due to the gem+ums combination.
> So there we also need to properly protect the entervt and leavevt
> ioctls. But it's definitely saner to do everything in one go than to
> drop the lock in-between.
> 
> Finally there's the gpu reset code in drm/i915. That one's just race
> (concurrent userspace calls to for vblank waits of pageflips could
> spuriously fail). So wrap it up in with a nice comment since fixing
> this is more involved.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_irq.c       | 30 +++++++++++++-----------------
>  drivers/gpu/drm/i915/i915_drv.c |  5 +++++
>  drivers/gpu/drm/i915/i915_gem.c |  5 +++--
>  3 files changed, 21 insertions(+), 19 deletions(-)

Looks good to me:

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel