Re: [PATCH 1/3] drm/i915: Actually respect DSPSURF alignment restrictions

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Jun 12, 2015 at 01:50:13PM +0300, Ville Syrjälä wrote:
> On Thu, Jun 11, 2015 at 02:51:19PM +0100, Chris Wilson wrote:
> > On Thu, Jun 11, 2015 at 04:31:14PM +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Currently intel_gen4_compute_page_offset() simply picks the closest
> > > page boundary below the linear offset. That however may not be suitably
> > > aligned to satisfy any hardware specific restrictions. So let's make
> > > sure the page boundary we choose is properly aligned.
> > > 
> > > Also to play it a bit safer lets split the remaining linear offset into
> > > x and y values instead of just x. This should make no difference for
> > > most platforms since we convert the x and y offsets back into a linear
> > > offset before feeding them to the hardware. HSW+ are different however
> > > and use x and y offsets even with linear buffers, so they might have
> > > trouble if either the x or y get too big.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > 
> > > @@ -2455,12 +2461,13 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y,
> > >  
> > >  		return tile_rows * pitch * 8 + tiles * 4096;
> > >  	} else {
> > > +		unsigned int alignment = intel_linear_alignment(dev_priv) - 1;
> > >  		unsigned int offset;
> > >  
> > >  		offset = *y * pitch + *x * cpp;
> > > -		*y = 0;
> > > -		*x = (offset & 4095) / cpp;
> > > -		return offset & -4096;
> > > +		*y = (offset & alignment) / pitch;
> > > +		*x = ((offset & alignment) - *y * pitch) / cpp;
> > > +		return offset & ~alignment;
> > 
> > Calculation looks solid. I presume we have a igt/kms test that combines
> > linear/tiled, large surfaces and large offsets?
> 
> kms_plane has some kind of panning tests. Probably not as good as it
> could/should be. I have a few custom tests I created to hunt for the
> VLV/CHV bug, but those aren't really useable as regular igt tests as
> is. Would take a bit of extra effort to turn them into such.

We'd need a crc based testcase where we first draw a test picture at (0,0)
and then with a hilariously large buffer at some offset. Evil values seems
to be anything > 2048 iirc.

Testcase for this would be really good since I think skl is all broken
too.
-Daniel
-- 
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