From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 6/6] drm/i915: Really wait for pending flips in intel_pipe_set_base() Date: Wed, 13 Feb 2013 19:26:31 +0200 Message-ID: <20130213172631.GR9135@intel.com> References: <1359476018-31274-1-git-send-email-ville.syrjala@linux.intel.com> <1359476018-31274-7-git-send-email-ville.syrjala@linux.intel.com> <20130213154935.GL5813@phenom.ffwll.local> <20130213170623.GQ9135@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id 91A60E606B for ; Wed, 13 Feb 2013 09:26:35 -0800 (PST) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Feb 13, 2013 at 06:11:00PM +0100, Daniel Vetter wrote: > On Wed, Feb 13, 2013 at 6:06 PM, Ville Syrj=E4l=E4 > wrote: > >> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/= i915/intel_display.c > >> > index a2e04f7..7e047c1 100644 > >> > --- a/drivers/gpu/drm/i915/intel_display.c > >> > +++ b/drivers/gpu/drm/i915/intel_display.c > >> > @@ -2330,8 +2330,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int= x, int y, > >> > return ret; > >> > } > >> > > >> > - if (crtc->fb) > >> > - intel_finish_fb(crtc->fb); > >> > + intel_crtc_wait_for_pending_flips_locked(crtc); > >> > >> Ah, I see now why you grab dev->struct_mutex and need to kick waiters = from > >> the pending flip queue. I think grabbing the mutex twice isn't a major > >> offense, since both the crtc disable code and set_base are slowpaths u= sed > >> rarely. So what about simply calling wait_for_pending_flips before > >> grabbing the mutex in intel_pipe_set_base? We could then also inline > >> finish_fb into it's only callsite ... > > > > I didn't want to slow down intel_pipe_set_base() too much. If we wait > > for pending flips before pinning the new fb, we can never achieve any > > parallelism there. But if no-one cares about that, we can reorder. > = > I'm confused here - where can we extract parallelism in set_base > between waiting for pending flips and the pinning? And imo set_base > isn't really critical: It's officially a synchronous thing (we have a > vblank wait in there), and if we want to fix that imo the nuclear > pageflip should be the answer. The flip is making progress on the GPU side, and at the same time the CPU side can make some progress with the pin operation. At least that was my theory. -- = Ville Syrj=E4l=E4 Intel OTC