From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 3/3] drm/i915/bdw: Use the GEN8 SRM when qeueing a flip Date: Wed, 9 Apr 2014 14:59:51 +0200 Message-ID: <20140409125951.GM9262@phenom.ffwll.local> References: <1396898674-22510-1-git-send-email-damien.lespiau@intel.com> <1396898674-22510-4-git-send-email-damien.lespiau@intel.com> <20140407205917.GD18726@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f45.google.com (mail-ee0-f45.google.com [74.125.83.45]) by gabe.freedesktop.org (Postfix) with ESMTP id C4B516EB5F for ; Wed, 9 Apr 2014 05:59:54 -0700 (PDT) Received: by mail-ee0-f45.google.com with SMTP id d17so1870826eek.4 for ; Wed, 09 Apr 2014 05:59:53 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140407205917.GD18726@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Ben Widawsky Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Apr 07, 2014 at 01:59:17PM -0700, Ben Widawsky wrote: > On Mon, Apr 07, 2014 at 08:24:34PM +0100, Damien Lespiau wrote: > > Signed-off-by: Damien Lespiau > > --- > > drivers/gpu/drm/i915/intel_display.c | 22 +++++++++++++++++++--- > > 1 file changed, 19 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 3697433..a646ed4 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -8821,8 +8821,16 @@ static int intel_gen7_queue_flip(struct drm_device *dev, > > } > > > > len = 4; > > - if (ring->id == RCS) > > + if (ring->id == RCS) { > > len += 6; > > + /* > > + * On Gen 8, SRM is now taking an extra dword to accommodate > > + * 48bits addresses, and we need a NOOP for the batch size to > > + * stay even. > > + */ > > + if (IS_GEN8(dev)) > > + len += 2; > > + } > > > > /* > > * BSpec MI_DISPLAY_FLIP for IVB: > > @@ -8857,10 +8865,18 @@ static int intel_gen7_queue_flip(struct drm_device *dev, > > intel_ring_emit(ring, ~(DERRMR_PIPEA_PRI_FLIP_DONE | > > DERRMR_PIPEB_PRI_FLIP_DONE | > > DERRMR_PIPEC_PRI_FLIP_DONE)); > > - intel_ring_emit(ring, MI_STORE_REGISTER_MEM(1) | > > - MI_SRM_LRM_GLOBAL_GTT); > > + if (IS_GEN8(dev)) > > + intel_ring_emit(ring, MI_STORE_REGISTER_MEM_GEN8(1) | > > + MI_SRM_LRM_GLOBAL_GTT); > > + else > > + intel_ring_emit(ring, MI_STORE_REGISTER_MEM(1) | > > + MI_SRM_LRM_GLOBAL_GTT); > > intel_ring_emit(ring, DERRMR); > > intel_ring_emit(ring, ring->scratch.gtt_offset + 256); > > + if (IS_GEN8(dev)) { > > + intel_ring_emit(ring, 0); > > + intel_ring_emit(ring, MI_NOOP); > > + } > > } > > > > intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane_bit); > > Cool. This explains the bad DERRMR values I was seeing in in error > states. I'm honestly didn't check if we actually need an SRM for BDW > still, but I'll assume you did check. > > I also think it's worth to make a intel_gen8_queue_flip, but since I > don't touch this code much, I'll leave it to you to decide. Yeah, gen8_queue_flip would kinda look neat. Anyway pull in the entire series, thanks. -Daniel > > The series is: > Reviewed-by: Ben Widawsky > > minus the above mentioned fact of whether it's needed. > > -- > Ben Widawsky, Intel Open Source Technology Center > _______________________________________________ > 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