From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 1/4] drm/i915: Suppress redundant syncs with mmio page flips Date: Fri, 19 Sep 2014 20:58:58 +0300 Message-ID: <20140919175858.GQ12416@intel.com> References: <1403090597-26210-1-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 9678B6E174 for ; Fri, 19 Sep 2014 11:07:13 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1403090597-26210-1-git-send-email-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Jun 18, 2014 at 12:23:14PM +0100, Chris Wilson wrote: > Since mmio-flips do not occur on the suggested ring, we are introducing > an extra sync operation where none is required. Pass the current > obj->ring, which is what mmio flip will use, to pin_to_display_plane so > that we emit the appropriate synchronisation (none). > = > Signed-off-by: Chris Wilson I had a vague recollection that I had seen something like this and now I found it... The patch could use a rebase I think, and perhaps it could use a boolean to avoid duplicating the buffer pinning in two place? I was thinking something like: ring =3D whatever; bool mmio_flip =3D use_mmio_flip(ring); if (mmio_flip) ring =3D obj->ring; pin_and_fence(ring); ... if (mmio_flip) ... else ... But either way this gets Reviewed-by: Ville Syrj=E4l=E4 > --- > drivers/gpu/drm/i915/intel_display.c | 29 ++++++++++++++++++++--------- > 1 file changed, 20 insertions(+), 9 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index 5e8e711..55cb343 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9492,21 +9492,32 @@ static int intel_crtc_page_flip(struct drm_crtc *= crtc, > ring =3D &dev_priv->ring[RCS]; > } > = > - ret =3D intel_pin_and_fence_fb_obj(dev, obj, ring); > - if (ret) > - goto cleanup_pending; > + if (use_mmio_flip(ring, obj)) { > + ret =3D intel_pin_and_fence_fb_obj(dev, obj, obj->ring); > + if (ret) > + goto cleanup_pending; > = > - work->gtt_offset =3D > - i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset; > + work->gtt_offset =3D > + i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset; > = > - if (use_mmio_flip(ring, obj)) > ret =3D intel_queue_mmio_flip(dev, crtc, fb, obj, ring, > page_flip_flags); > - else > + if (ret) > + goto cleanup_unpin; > + > + } else { > + ret =3D intel_pin_and_fence_fb_obj(dev, obj, ring); > + if (ret) > + goto cleanup_pending; > + > + work->gtt_offset =3D > + i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset; > + > ret =3D dev_priv->display.queue_flip(dev, crtc, fb, obj, ring, > page_flip_flags); > - if (ret) > - goto cleanup_unpin; > + if (ret) > + goto cleanup_unpin; > + } > = > intel_disable_fbc(dev); > intel_mark_fb_busy(obj, NULL); > -- = > 1.9.1 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC