public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH v2 3b/3] drm/i915: Kill i915_gem_execbuffer_wait_for_flips()
Date: Fri, 02 Nov 2012 18:44:38 +0000	[thread overview]
Message-ID: <84c8a8$6ck6iq@orsmga001.jf.intel.com> (raw)
In-Reply-To: <CAOeoa-dkGi2RCK01GTEvcZSPEN=xyznixvMeVtuS1PkCUs6hKg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2275 bytes --]

On Fri, 2 Nov 2012 14:24:12 -0400, Kristian Høgsberg <krh@bitplanet.net> wrote:
> On Fri, Nov 2, 2012 at 9:29 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Thu,  1 Nov 2012 20:06:03 +0200, ville.syrjala@linux.intel.com wrote:
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> As per Chris Wilson's suggestion make
> >> i915_gem_execbuffer_wait_for_flips() go away.
> >>
> >> This was used to stall the GPU ring while there are pending
> >> page flips involving the relevant BO. Ie. while the BO is still
> >> being scanned out by the display controller.
> >>
> >> The recommended alternative is to use the page flip events to
> >> wait for the page flips to fully complete before reusing the BO
> >> of the old front buffer. Or use more buffers.
> >>
> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> >
> > Needs an ack from either Jesse or Kristian.
> 
> I wouldn't mind seeing this go; we don't rely on this behavior in
> Weston, we use the events.  However, this is a user visible change in
> behavior of the pageflip ioctl.  I don't remember if X needs this for
> the case where it's pageflipping to fullscreen client buffers... I
> think it might (or at least might have), since we did it this way so
> that the client wouldn't have to wait for a protocol event before
> starting the next frame.  With this the client can start rendering and
> submit the first batch before it has to block.  It also minimizes the
> time from pageflip done to the client can start rendering, since the
> kernel now just unblocks the client directly.  Of course you can argue
> that we can fix that with more buffers, and maybe it's fixed in newer
> servers / ddx drivers, but it doesn't change that this is how it used
> to work.  Without this, the  client doesn't know when it's new
> backbuffer done scanning out.

We never relied on this in userspace, as you provided the flip-completed
event at the same time as you introduced pageflipping and that was used
in the original ddx to throttle the client. And from experience, I can say
that this blocking is not sufficient to prevent userspace from being
stupid...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

      reply	other threads:[~2012-11-02 18:45 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-01 18:05 [PATCH 0/3] drm/i915: i915_gem_execbuffer_wait_for_flips and other flip stuff ville.syrjala
2012-11-01 18:06 ` [PATCH 1/3] drm/i915: Wait for pending flips in intel_pipe_set_base() ville.syrjala
2012-11-02 13:26   ` Chris Wilson
2012-11-02 14:02     ` Ville Syrjälä
2012-11-02 14:28       ` Chris Wilson
2012-11-02 15:25   ` Chris Wilson
2012-11-02 17:26     ` Ville Syrjälä
2012-11-02 17:34       ` Chris Wilson
2012-11-01 18:06 ` [PATCH 2/3] drm/i915: Wake up pending flip waiters when the GPU hangs ville.syrjala
2012-11-02 13:27   ` Chris Wilson
2012-11-01 18:06 ` [PATCH 3a/3] drm/i915: Avoid i915_gem_execbuffer_wait_for_flips() on SNB+ ville.syrjala
2012-11-01 18:06 ` [PATCH v2 3b/3] drm/i915: Kill i915_gem_execbuffer_wait_for_flips() ville.syrjala
2012-11-02 13:29   ` Chris Wilson
2012-11-02 18:09     ` Jesse Barnes
2012-11-02 18:24     ` Kristian Høgsberg
2012-11-02 18:44       ` Chris Wilson [this message]

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='84c8a8$6ck6iq@orsmga001.jf.intel.com' \
    --to=chris@chris-wilson.co.uk \
    --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