From: Ben Skeggs <bskeggs@redhat.com>
To: "Michel Dänzer" <michel@daenzer.net>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm: make sure drm_vblank_init() has been called before touching vbl_lock
Date: Thu, 07 Jul 2011 09:10:04 +1000 [thread overview]
Message-ID: <1309993804.3763.18.camel@clockmaker-el6> (raw)
In-Reply-To: <1309937939.3713.153.camel@thor.local>
On Wed, 2011-07-06 at 09:38 +0200, Michel Dänzer wrote:
> On Mon, 2011-07-04 at 13:16 +1000, skeggsb@gmail.com wrote:
> > From: Ben Skeggs <bskeggs@redhat.com>
> >
> > Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
> > ---
> > drivers/gpu/drm/drm_irq.c | 15 +++++++++------
> > 1 files changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index 2022a5c..c3c87a1 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -397,13 +397,16 @@ int drm_irq_uninstall(struct drm_device *dev)
> > /*
> > * Wake up any waiters so they don't hang.
> > */
> > - spin_lock_irqsave(&dev->vbl_lock, irqflags);
> > - for (i = 0; i < dev->num_crtcs; i++) {
> > - DRM_WAKEUP(&dev->vbl_queue[i]);
> > - dev->vblank_enabled[i] = 0;
> > - dev->last_vblank[i] = dev->driver->get_vblank_counter(dev, i);
> > + if (dev->num_crtcs) {
> > + spin_lock_irqsave(&dev->vbl_lock, irqflags);
> > + for (i = 0; i < dev->num_crtcs; i++) {
> > + DRM_WAKEUP(&dev->vbl_queue[i]);
> > + dev->vblank_enabled[i] = 0;
> > + dev->last_vblank[i] =
> > + dev->driver->get_vblank_counter(dev, i);
> > + }
> > + spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
> > }
> > - spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
> >
> > if (!irq_enabled)
> > return -EINVAL;
>
> Makes sense, but according to commit
> 7a1c2f6c8d8485af5ac6c2a313f6a7162207a4af ('vmwgfx: Enable use of the
> vblank system'), the vblank ioctls will also access uninitialized data
> if drm_vblank_init() hasn't been called. So, is this just for cases
> where drm_irq_uninstall() is called before the driver gets a chance to
> call drm_vblank_init()?
Heh, ouch.
Yes, it was specifically that case I was aiming for however. If
nouveau's running on a board without any outputs, it won't call
drm_vblank_init(), but it *will* call drm_irq_*. Just avoiding lockdep
complaints on module unload.
Ben.
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2011-07-06 23:10 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-04 3:16 [PATCH] drm: make sure drm_vblank_init() has been called before touching vbl_lock skeggsb
2011-07-06 7:38 ` Michel Dänzer
2011-07-06 23:10 ` Ben Skeggs [this message]
2011-07-07 6:05 ` Michel Dänzer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1309993804.3763.18.camel@clockmaker-el6 \
--to=bskeggs@redhat.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=michel@daenzer.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.