Re: [PATCH 4/6] drm/i915: set ILK_DPFC_FENCE_YOFF to 0 on SNB

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: Re: [PATCH 4/6] drm/i915: set ILK_DPFC_FENCE_YOFF to 0 on SNB
Date: Thu, 9 Jul 2015 20:22:32 +0300	[thread overview]
Message-ID: <20150709172232.GK5176@intel.com> (raw)
In-Reply-To: <20150709171004.GA7410@phenom.ffwll.local>

On Thu, Jul 09, 2015 at 07:10:04PM +0200, Daniel Vetter wrote:
> On Wed, Jul 08, 2015 at 05:58:57PM -0300, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > 
> > The doc is pretty clear that this register should be set to 0 on SNB.
> > We already write y_offset to DPFC_CPU_FENCE_OFFSET a few lines below.
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Hm, do we have testcases where we have a sufficiently big y offset? We can
> just allocate 128 lines more and use that as the offset, that should be
> big enough everywhere. Actually make that 129 lines to check the tile-size
> rounding ;-)
> 
> Ofc this means we need to have two sets of testcases for all the affected
> tests (i.e. everything that tries to test the gtt hw tracking).
> 
> Another funny corner case (which we're getting wrong on skl even without
> fbc) is x offsets > 2048 pixels (since x/y offset registers don't hold
> bigger values and then it wraps).
> 
> I.e. I'd like this patch (and the others) to be augmented with a Testcase:
> tag.

I think the entire Y offset thing is currently being misprogrammed.
IIRC the offset is from the display base address but we program in
the offset from the start of the FB.

> 
> Cheers, Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_fbc.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> > index 0373cbc..0a24e96 100644
> > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > @@ -216,7 +216,12 @@ static void ilk_fbc_enable(struct intel_crtc *crtc)
> >  		dpfc_ctl |= obj->fence_reg;
> >  
> >  	y_offset = get_crtc_fence_y_offset(crtc);
> > -	I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset);
> > +
> > +	if (IS_GEN5(dev_priv))
> > +		I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset);
> > +	else
> > +		I915_WRITE(ILK_DPFC_FENCE_YOFF, 0);
> > +
> >  	I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID);
> >  	/* enable it... */
> >  	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
> > -- 
> > 2.1.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx