dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Alex Deucher <alexdeucher@gmail.com>
Cc: "Michel Dänzer" <michel@daenzer.net>
Subject: Re: Funky new vblank counter regressions in Linux 4.4-rc1
Date: Fri, 27 Nov 2015 16:55:56 +0200	[thread overview]
Message-ID: <20151127145556.GO4437@intel.com> (raw)
In-Reply-To: <CADnq5_MMfaVYLNLOoMtCEg=ymh25yVA5me8Ch+nwmb5R4=37mQ@mail.gmail.com>

On Wed, Nov 25, 2015 at 02:38:04PM -0500, Alex Deucher wrote:
> On Wed, Nov 25, 2015 at 1:21 PM, Mario Kleiner
> <mario.kleiner.de@gmail.com> wrote:
> > On 11/25/2015 06:58 PM, Ville Syrjälä wrote:
> >>
> >> On Wed, Nov 25, 2015 at 06:24:13PM +0100, Mario Kleiner wrote:
> >>>
> >>> On 11/23/2015 09:24 PM, Ville Syrjälä wrote:
> >>>>
> >>>> On Mon, Nov 23, 2015 at 06:58:34PM +0100, Mario Kleiner wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>> On 11/23/2015 04:51 PM, Ville Syrjälä wrote:
> >>>>>>
> >>>>>> On Mon, Nov 23, 2015 at 04:23:21PM +0100, Mario Kleiner wrote:
> >>>>>>>
> >>>>>>> On 11/20/2015 04:34 PM, Ville Syrjälä wrote:
> >>>>>>>>
> >>>>>>>> On Fri, Nov 20, 2015 at 04:24:50PM +0100, Mario Kleiner wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> ...
> >>>>>>> Ok, but why would that be a bad thing? I think we want it to think it
> >>>>>>> is
> >>>>>>> in the previous frame if it is called outside the vblank irq context.
> >>>>>>> The only reason we fudge it to the next frames vblank if i vblank irq
> >>>>>>> is
> >>>>>>> because we know the vblank irq handler we are executing atm. was
> >>>>>>> meant
> >>>>>>> to execute within the upcoming vblank for the next frame, so we fudge
> >>>>>>> the scanout positions and thereby timestamp to correspond to that new
> >>>>>>> frame. But if something called outside irq context it should get a
> >>>>>>> scanout position/timestamp that corresponds to "reality".
> >>>>>>
> >>>>>>
> >>>>>> It would be a bad thing since it would cause the timestamp to jump
> >>>>>> backwards, and that would also cause the frame count guesstimate to go
> >>>>>> backwards.
> >>>>>>
> >>>>>
> >>>>> But only if we don't use the dev->driver->get_vblank_counter() method,
> >>>>> which we try to use on AMD.
> >>>>
> >>>>
> >>>> Well, if you do it that way then you have the problem of the hw counter
> >>>> seeming to jump forward by one after crossing the start of vblank (when
> >>>> compared to the value you sampled when you processed the early vblank
> >>>> interrupt).
> >>>>
> >>>
> >>> Ok, finally i see the bad scenario that wouldn't get prevented by our
> >>> current locking with the new vblank counting in the core. The vblank
> >>> enable path is safe due to locking and discounting of redundant
> >>> timestamps etc. But the disable path could go wrong:
> >>>
> >>> 1. Vblank irq fires, drm_handle_vblank() -> drm_update_vblank_count(),
> >>> updates timestamps and counts "as if" in vblank -> incremented vblank
> >>> count and timestamp now set in the future.
> >>>
> >>> 2. After vblank irq finishes, but just before leading edge of vblank,
> >>> vblank_disable_and_save() executes, doesn't get bumped timestamp or
> >>> count because before vblank and not in vblank irq. Now
> >>> drm_update_vblank_count() would process a
> >>> "new" timestamp and count from the past and we'd have time and counts
> >>> going backwards, and bad things would happen.
> >>>
> >>> I haven't observed such a thing happening during testing so far,
> >>> probably because the time window in which it could happen is tiny, but
> >>> given how awfully bad it would be, it needs to be prevented.
> >>>
> >>> I had a look at the description of the Vblank irq in the "M76 Register
> >>> Reference Guide" for older asics and the description suggests that the
> >>> vblank irq fires when the crtc's line buffer is finished reading pixel
> >>> data from the scanout buffer in memory for a frame, ie., when the line
> >>> buffer read "enters" vblank.
> >>
> >>
> >> Hmm. Does that mean there's always at least one fullscreen plane enabled
> >> in the hw? As in you can't turn off the primary plane or make it smaller
> >> than the active video area? Othwewise it sounds like you'd could either
> >> not get it at all, or get it somewhere in the middle of the screen.
> >>
> >
> > It says "Interrupt that can be programmed to be generated by the
> > primary display controller's line buffer logic either when the
> > source image line counter is not requesting any active
> > display data (i.e. in the vertical blank) or the output CRTC
> > timing generator is within the vertical blanking region."
> >
> > So my statements were my interpretation of this quote, so i can make some
> > sense out of the vblank irq behaviour. I guess Alex or Harry would know? The
> > M76 reference refers to some older asics, i just assume it is the same for
> > the current ones, given that observed behaviour would be consistent with the
> > line buffer causing this lead of a couple of scanlines. I see about 2
> > scanlines on DCE4 and about 3 scanlines on DCE3. I don't know how big the
> > line buffer is, how quickly it refills etc., but it sounds reasonable.
> 
> The size of the line buffer varies by generation, but the LB logic is
> still responsible for generating the vblank interrupt even on newer
> hw.

So there's no actual vblank interrupt available from the timing
generator?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2015-11-30 20:14 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-19 17:46 Funky new vblank counter regressions in Linux 4.4-rc1 Mario Kleiner
2015-11-19 18:20 ` Ville Syrjälä
2015-11-19 19:12   ` Mario Kleiner
2015-11-19 19:45     ` Ville Syrjälä
2015-11-20 15:24       ` Mario Kleiner
2015-11-20 15:34         ` Ville Syrjälä
2015-11-23 15:23           ` Mario Kleiner
2015-11-23 15:51             ` Ville Syrjälä
2015-11-23 17:58               ` Mario Kleiner
2015-11-23 20:24                 ` Ville Syrjälä
2015-11-25 17:24                   ` Mario Kleiner
2015-11-25 17:46                     ` Ville Syrjälä
2015-11-25 19:04                       ` Mario Kleiner
2015-11-25 19:36                         ` Ville Syrjälä
2015-11-27 14:09                           ` Mario Kleiner
2015-11-25 17:58                     ` Ville Syrjälä
2015-11-25 18:21                       ` Mario Kleiner
2015-11-25 19:38                         ` Alex Deucher
2015-11-27 14:36                           ` Mario Kleiner
2015-11-27 14:55                           ` Ville Syrjälä [this message]
2015-11-20  3:42 ` Alex Deucher
2015-11-23 16:02   ` Mario Kleiner
2015-11-23 20:04     ` Harry Wentland
2015-11-23 21:20       ` Mario Kleiner

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=20151127145556.GO4437@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=alexdeucher@gmail.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox