public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: "Michel Dänzer" <michel@daenzer.net>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 5/5] drm/i915: Allow vblank interrupts during modeset and eliminate some vblank races
Date: Wed, 28 May 2014 18:12:54 +0900	[thread overview]
Message-ID: <5385A896.9070600@daenzer.net> (raw)
In-Reply-To: <20140304091343.GP17001@phenom.ffwll.local>


Digging out an ooold post of Daniel's...

On 04.03.2014 18:13, Daniel Vetter wrote:
> On Tue, Feb 25, 2014 at 11:58:26AM +0900, Michel Dänzer wrote:
>> 
>> When the pre/post-modeset hooks were originally added, it worked like
>> this: the pre-modeset hook enabled the vblank interrupt, which updated
>> the DRM vblank counter from the driver/HW counter. The post-modeset hook
>> disabled the vblank interrupt again, which recorded the post-modeset
>> driver/HW counter value.
>>
>> But the vblank code has changed a lot since then, not sure it still
>> works like that.
> 
> It still works like that, but there's two fundamental issues with this
> trick:
> - There's a race where the vblank state is fubar right between the
>   completion of the modeset and before the first vblank happened.

Can you provide more details about that? You mentioned on IRC that
sometimes 'bogus' DRM vblank counter values are returned to userspace.
The most likely cause of that would be drm_vblank_pre_modeset() being
called too late, i.e. after the hardware counter was reset. (Or if
you're reducing / eliminating the vblank disable timer, possibly the
vblank interrupt getting disabled too early, i.e. before the hardware
counter was reset)


Speaking of reducing or disabling the vblank disable timer, that should
be possible with drm_vblank_pre/post_modeset() as well.


> - It doesn't work across suspend/resume since no one re-enables the vblank
>   interrupt.

That sounds like a driver bug to me. The driver should re-enable the
hardware interrupt on resume if the vblank interrupt is currently
enabled by the DRM core. This seems to work fine for me with radeon.


So, I'm afraid I'm still not convinced that the new vblank_off/on()
functions and the ever-increasing series of fixes for problems related
to them are the right (let alone only) solution for your problems.


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer

  reply	other threads:[~2014-05-28  9:12 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-21 19:03 [PATCH 0/5] drm: Allow vblank interrupts during modeset and make the code less racy ville.syrjala
2014-02-21 19:03 ` [PATCH 1/5] drm: Use correct spinlock flavor in drm_vblank_get() ville.syrjala
2014-02-26 19:24   ` [Intel-gfx] " Jesse Barnes
2014-02-21 19:03 ` [PATCH 2/5] drm: Make the vblank disable timer per-crtc ville.syrjala
2014-02-26 19:32   ` Jesse Barnes
2014-02-21 19:03 ` [PATCH 3/5] drm: Allow the driver to reject vblank requests only when it really has the vblank interrupts disabled ville.syrjala
2014-02-26 19:41   ` Jesse Barnes
2014-03-04  9:24   ` Daniel Vetter
2014-03-05 12:38     ` Ville Syrjälä
2014-03-05 13:55       ` Daniel Vetter
2014-02-21 19:03 ` [PATCH 4/5] drm: Allow reenabling of vblank interrupts even if refcount>0 ville.syrjala
2014-02-26 19:44   ` Jesse Barnes
2014-03-04  9:16   ` Daniel Vetter
2014-03-05 12:33     ` Ville Syrjälä
2014-02-21 19:03 ` [PATCH 5/5] drm/i915: Allow vblank interrupts during modeset and eliminate some vblank races ville.syrjala
2014-02-24  3:48   ` Michel Dänzer
2014-02-24 12:11     ` Ville Syrjälä
2014-02-25  2:58       ` Michel Dänzer
2014-02-26 19:48         ` Jesse Barnes
2014-03-04  9:13         ` Daniel Vetter
2014-05-28  9:12           ` Michel Dänzer [this message]
2014-05-28 11:19             ` [Intel-gfx] " Ville Syrjälä
2014-05-29  4:11               ` Michel Dänzer
2014-05-29 10:56                 ` Daniel Vetter
2014-05-30  3:13                   ` Michel Dänzer
2014-02-28  8:56   ` Ville Syrjälä
2014-03-04  9:15     ` Daniel Vetter

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=5385A896.9070600@daenzer.net \
    --to=michel@daenzer.net \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox