From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/4] drm/i915: Suppress redundant syncs with mmio page flips Date: Tue, 23 Sep 2014 10:13:46 +0200 Message-ID: <20140923081346.GS15734@phenom.ffwll.local> References: <1403090597-26210-1-git-send-email-chris@chris-wilson.co.uk> <20140919175858.GQ12416@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-wi0-f176.google.com (mail-wi0-f176.google.com [209.85.212.176]) by gabe.freedesktop.org (Postfix) with ESMTP id 29A166E0B9 for ; Tue, 23 Sep 2014 01:13:51 -0700 (PDT) Received: by mail-wi0-f176.google.com with SMTP id fb4so4322378wid.15 for ; Tue, 23 Sep 2014 01:13:50 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140919175858.GQ12416@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Sep 19, 2014 at 08:58:58PM +0300, Ville Syrj=E4l=E4 wrote: > 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: Yeah, patch doesn't apply with the s/seqno/request/ patch, which I don't have here. And the conflict looks messy enough that I don't want to fix it up myself. Can you please rebase? Thanks, Daniel > = > 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/i91= 5/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 > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch