All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915: Actually respect DSPSURF alignment restrictions
@ 2015-06-11 13:31 ville.syrjala
  2015-06-11 13:31 ` [PATCH 2/3] drm/i915: Align DSPSURF to 128k on VLV/CHV ville.syrjala
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: ville.syrjala @ 2015-06-11 13:31 UTC (permalink / raw)
  To: intel-gfx

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>
---
 drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
 drivers/gpu/drm/i915/intel_sprite.c  |  9 ++++++---
 3 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5cc2263..0ac213a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2325,6 +2325,18 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb,
 	return 0;
 }
 
+static unsigned int intel_linear_alignment(struct drm_i915_private *dev_priv)
+{
+	if (INTEL_INFO(dev_priv)->gen >= 9)
+		return 256 * 1024;
+	else if (IS_BROADWATER(dev_priv) || IS_CRESTLINE(dev_priv))
+		return 128 * 1024;
+	else if (INTEL_INFO(dev_priv)->gen >= 4)
+		return 4 * 1024;
+	else
+		return 64 * 1024;
+}
+
 int
 intel_pin_and_fence_fb_obj(struct drm_plane *plane,
 			   struct drm_framebuffer *fb,
@@ -2342,14 +2354,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
 
 	switch (fb->modifier[0]) {
 	case DRM_FORMAT_MOD_NONE:
-		if (INTEL_INFO(dev)->gen >= 9)
-			alignment = 256 * 1024;
-		else if (IS_BROADWATER(dev) || IS_CRESTLINE(dev))
-			alignment = 128 * 1024;
-		else if (INTEL_INFO(dev)->gen >= 4)
-			alignment = 4 * 1024;
-		else
-			alignment = 64 * 1024;
+		alignment = intel_linear_alignment(dev_priv);
 		break;
 	case I915_FORMAT_MOD_X_TILED:
 		if (INTEL_INFO(dev)->gen >= 9)
@@ -2439,7 +2444,8 @@ static void intel_unpin_fb_obj(struct drm_framebuffer *fb,
 
 /* Computes the linear offset to the base tile and adjusts x, y. bytes per pixel
  * is assumed to be a power-of-two. */
-unsigned long intel_gen4_compute_page_offset(int *x, int *y,
+unsigned long intel_gen4_compute_page_offset(struct drm_i915_private *dev_priv,
+					     int *x, int *y,
 					     unsigned int tiling_mode,
 					     unsigned int cpp,
 					     unsigned int pitch)
@@ -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;
 	}
 }
 
@@ -2729,7 +2736,8 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 
 	if (INTEL_INFO(dev)->gen >= 4) {
 		intel_crtc->dspaddr_offset =
-			intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
+			intel_gen4_compute_page_offset(dev_priv,
+						       &x, &y, obj->tiling_mode,
 						       pixel_size,
 						       fb->pitches[0]);
 		linear_offset -= intel_crtc->dspaddr_offset;
@@ -2830,7 +2838,8 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 
 	linear_offset = y * fb->pitches[0] + x * pixel_size;
 	intel_crtc->dspaddr_offset =
-		intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
+		intel_gen4_compute_page_offset(dev_priv,
+					       &x, &y, obj->tiling_mode,
 					       pixel_size,
 					       fb->pitches[0]);
 	linear_offset -= intel_crtc->dspaddr_offset;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b28029a..c28d117 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1113,7 +1113,8 @@ void assert_fdi_rx_pll(struct drm_i915_private *dev_priv,
 void assert_pipe(struct drm_i915_private *dev_priv, enum pipe pipe, bool state);
 #define assert_pipe_enabled(d, p) assert_pipe(d, p, true)
 #define assert_pipe_disabled(d, p) assert_pipe(d, p, false)
-unsigned long intel_gen4_compute_page_offset(int *x, int *y,
+unsigned long intel_gen4_compute_page_offset(struct drm_i915_private *dev_priv,
+					     int *x, int *y,
 					     unsigned int tiling_mode,
 					     unsigned int bpp,
 					     unsigned int pitch);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 8193a35..28f6972 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -411,7 +411,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 	crtc_h--;
 
 	linear_offset = y * fb->pitches[0] + x * pixel_size;
-	sprsurf_offset = intel_gen4_compute_page_offset(&x, &y,
+	sprsurf_offset = intel_gen4_compute_page_offset(dev_priv,
+							&x, &y,
 							obj->tiling_mode,
 							pixel_size,
 							fb->pitches[0]);
@@ -546,7 +547,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 
 	linear_offset = y * fb->pitches[0] + x * pixel_size;
 	sprsurf_offset =
-		intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
+		intel_gen4_compute_page_offset(dev_priv,
+					       &x, &y, obj->tiling_mode,
 					       pixel_size, fb->pitches[0]);
 	linear_offset -= sprsurf_offset;
 
@@ -682,7 +684,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 
 	linear_offset = y * fb->pitches[0] + x * pixel_size;
 	dvssurf_offset =
-		intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
+		intel_gen4_compute_page_offset(dev_priv,
+					       &x, &y, obj->tiling_mode,
 					       pixel_size, fb->pitches[0]);
 	linear_offset -= dvssurf_offset;
 
-- 
2.3.6

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] drm/i915: Align DSPSURF to 128k on VLV/CHV
  2015-06-11 13:31 [PATCH 1/3] drm/i915: Actually respect DSPSURF alignment restrictions ville.syrjala
@ 2015-06-11 13:31 ` ville.syrjala
  2015-06-12  0:46   ` Clint Taylor
  2015-06-12  4:47   ` Arun R Murthy
  2015-06-11 13:31 ` [PATCH 3/3] drm/i915: Drop the 64k linear scanout alignment on gen2/3 ville.syrjala
  2015-06-11 13:51 ` [PATCH 1/3] drm/i915: Actually respect DSPSURF alignment restrictions Chris Wilson
  2 siblings, 2 replies; 10+ messages in thread
From: ville.syrjala @ 2015-06-11 13:31 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

VLV/CHV have problems with 4k aligned linear scanout buffers. The VLV
docs got updated at some point to say that we need to align them to
128k, just like we do on gen4.

So far I've seen the problem manifest when the stride is an odd multiple
of 512 bytes, and the surface address meets the following pattern
'(addr & 0xf000) == 0x1000' (also == 0x2000 is problematic on VLV). The
result is a starcase effect (so some pages get dropped maybe?), with a
few pages here and there clearly getting scannout out at the wrong position.

I've not actually been able to reproduce this problem on gen4, so it's
not clear of the issue is any way related to the 128k restrictions
supposedly inherited from gen4. But let's hope the 128k alignment is
sufficient to hide it all.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0ac213a..2221323 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2329,7 +2329,8 @@ static unsigned int intel_linear_alignment(struct drm_i915_private *dev_priv)
 {
 	if (INTEL_INFO(dev_priv)->gen >= 9)
 		return 256 * 1024;
-	else if (IS_BROADWATER(dev_priv) || IS_CRESTLINE(dev_priv))
+	else if (IS_BROADWATER(dev_priv) || IS_CRESTLINE(dev_priv) ||
+		 IS_VALLEYVIEW(dev_priv))
 		return 128 * 1024;
 	else if (INTEL_INFO(dev_priv)->gen >= 4)
 		return 4 * 1024;
-- 
2.3.6

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] drm/i915: Drop the 64k linear scanout alignment on gen2/3
  2015-06-11 13:31 [PATCH 1/3] drm/i915: Actually respect DSPSURF alignment restrictions ville.syrjala
  2015-06-11 13:31 ` [PATCH 2/3] drm/i915: Align DSPSURF to 128k on VLV/CHV ville.syrjala
@ 2015-06-11 13:31 ` ville.syrjala
  2015-06-11 13:52   ` Chris Wilson
  2015-06-14 23:18   ` shuang.he
  2015-06-11 13:51 ` [PATCH 1/3] drm/i915: Actually respect DSPSURF alignment restrictions Chris Wilson
  2 siblings, 2 replies; 10+ messages in thread
From: ville.syrjala @ 2015-06-11 13:31 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The docs don't support the 64k linear scanout alignment we impose
on gen2/3. And it really makes no sense since we have no DSPSURF
register, so the only thing that the hardware will see is the linear
offset which will be just pixel aligned anyway.

There is one case where 64k comes into the picture, and that's FBC.
The start of the line length buffer corresponds to a 64k aligned
address of the uncompressed framebuffer. So if the uncompressed fb is
not 64k aligned, the first actually used entry in the line length
buffer will not be byte 0. There are 32 extra entries in the line
length buffer to account for this extra alignment so we shouldn't
have to worry about it when mapping the uncompressed fb to the GTT.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2221323..b48c237 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2335,7 +2335,7 @@ static unsigned int intel_linear_alignment(struct drm_i915_private *dev_priv)
 	else if (INTEL_INFO(dev_priv)->gen >= 4)
 		return 4 * 1024;
 	else
-		return 64 * 1024;
+		return 0;
 }
 
 int
-- 
2.3.6

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] drm/i915: Actually respect DSPSURF alignment restrictions
  2015-06-11 13:31 [PATCH 1/3] drm/i915: Actually respect DSPSURF alignment restrictions ville.syrjala
  2015-06-11 13:31 ` [PATCH 2/3] drm/i915: Align DSPSURF to 128k on VLV/CHV ville.syrjala
  2015-06-11 13:31 ` [PATCH 3/3] drm/i915: Drop the 64k linear scanout alignment on gen2/3 ville.syrjala
@ 2015-06-11 13:51 ` Chris Wilson
  2015-06-12 10:50   ` Ville Syrjälä
  2 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2015-06-11 13:51 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

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?

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] drm/i915: Drop the 64k linear scanout alignment on gen2/3
  2015-06-11 13:31 ` [PATCH 3/3] drm/i915: Drop the 64k linear scanout alignment on gen2/3 ville.syrjala
@ 2015-06-11 13:52   ` Chris Wilson
  2015-06-14 23:18   ` shuang.he
  1 sibling, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2015-06-11 13:52 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Thu, Jun 11, 2015 at 04:31:16PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The docs don't support the 64k linear scanout alignment we impose
> on gen2/3. And it really makes no sense since we have no DSPSURF
> register, so the only thing that the hardware will see is the linear
> offset which will be just pixel aligned anyway.
> 
> There is one case where 64k comes into the picture, and that's FBC.
> The start of the line length buffer corresponds to a 64k aligned
> address of the uncompressed framebuffer. So if the uncompressed fb is
> not 64k aligned, the first actually used entry in the line length
> buffer will not be byte 0. There are 32 extra entries in the line
> length buffer to account for this extra alignment so we shouldn't
> have to worry about it when mapping the uncompressed fb to the GTT.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I could not find any rationale either, and it worked this way ~10 years
ago, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] drm/i915: Align DSPSURF to 128k on VLV/CHV
  2015-06-11 13:31 ` [PATCH 2/3] drm/i915: Align DSPSURF to 128k on VLV/CHV ville.syrjala
@ 2015-06-12  0:46   ` Clint Taylor
  2015-06-12  4:47   ` Arun R Murthy
  1 sibling, 0 replies; 10+ messages in thread
From: Clint Taylor @ 2015-06-12  0:46 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On 06/11/2015 06:31 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> VLV/CHV have problems with 4k aligned linear scanout buffers. The VLV
> docs got updated at some point to say that we need to align them to
> 128k, just like we do on gen4.
>
> So far I've seen the problem manifest when the stride is an odd multiple
> of 512 bytes, and the surface address meets the following pattern
> '(addr & 0xf000) == 0x1000' (also == 0x2000 is problematic on VLV). The
> result is a starcase effect (so some pages get dropped maybe?), with a
> few pages here and there clearly getting scannout out at the wrong position.
>
> I've not actually been able to reproduce this problem on gen4, so it's
> not clear of the issue is any way related to the 128k restrictions
> supposedly inherited from gen4. But let's hope the 128k alignment is
> sufficient to hide it all.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0ac213a..2221323 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2329,7 +2329,8 @@ static unsigned int intel_linear_alignment(struct drm_i915_private *dev_priv)
>   {
>   	if (INTEL_INFO(dev_priv)->gen >= 9)
>   		return 256 * 1024;
> -	else if (IS_BROADWATER(dev_priv) || IS_CRESTLINE(dev_priv))
> +	else if (IS_BROADWATER(dev_priv) || IS_CRESTLINE(dev_priv) ||
> +		 IS_VALLEYVIEW(dev_priv))
>   		return 128 * 1024;
>   	else if (INTEL_INFO(dev_priv)->gen >= 4)
>   		return 4 * 1024;
>

Fairly straight forward now that we know VLV and CHV require the 128k 
alignment.

Reviewed-by: Clint Taylor <Clinton.A.Taylor@intel.com>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] drm/i915: Align DSPSURF to 128k on VLV/CHV
  2015-06-11 13:31 ` [PATCH 2/3] drm/i915: Align DSPSURF to 128k on VLV/CHV ville.syrjala
  2015-06-12  0:46   ` Clint Taylor
@ 2015-06-12  4:47   ` Arun R Murthy
  1 sibling, 0 replies; 10+ messages in thread
From: Arun R Murthy @ 2015-06-12  4:47 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx


On Thursday 11 June 2015 07:01 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> VLV/CHV have problems with 4k aligned linear scanout buffers. The VLV
> docs got updated at some point to say that we need to align them to
> 128k, just like we do on gen4.
>
> So far I've seen the problem manifest when the stride is an odd multiple
> of 512 bytes, and the surface address meets the following pattern
> '(addr & 0xf000) == 0x1000' (also == 0x2000 is problematic on VLV). The
> result is a starcase effect (so some pages get dropped maybe?), with a
> few pages here and there clearly getting scannout out at the wrong position.
>
> I've not actually been able to reproduce this problem on gen4, so it's
> not clear of the issue is any way related to the 128k restrictions
> supposedly inherited from gen4. But let's hope the 128k alignment is
> sufficient to hide it all.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
-------------------

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] drm/i915: Actually respect DSPSURF alignment restrictions
  2015-06-11 13:51 ` [PATCH 1/3] drm/i915: Actually respect DSPSURF alignment restrictions Chris Wilson
@ 2015-06-12 10:50   ` Ville Syrjälä
  2015-06-15 16:04     ` Daniel Vetter
  0 siblings, 1 reply; 10+ messages in thread
From: Ville Syrjälä @ 2015-06-12 10:50 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

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.

> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] drm/i915: Drop the 64k linear scanout alignment on gen2/3
  2015-06-11 13:31 ` [PATCH 3/3] drm/i915: Drop the 64k linear scanout alignment on gen2/3 ville.syrjala
  2015-06-11 13:52   ` Chris Wilson
@ 2015-06-14 23:18   ` shuang.he
  1 sibling, 0 replies; 10+ messages in thread
From: shuang.he @ 2015-06-14 23:18 UTC (permalink / raw)
  To: shuang.he, lei.a.liu, intel-gfx, ville.syrjala

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6570
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  303/303              303/303
SNB                                  312/312              312/312
IVB                                  343/343              343/343
BYT                                  287/287              287/287
BDW                                  321/321              321/321
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] drm/i915: Actually respect DSPSURF alignment restrictions
  2015-06-12 10:50   ` Ville Syrjälä
@ 2015-06-15 16:04     ` Daniel Vetter
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2015-06-15 16:04 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-06-15 16:01 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-11 13:31 [PATCH 1/3] drm/i915: Actually respect DSPSURF alignment restrictions ville.syrjala
2015-06-11 13:31 ` [PATCH 2/3] drm/i915: Align DSPSURF to 128k on VLV/CHV ville.syrjala
2015-06-12  0:46   ` Clint Taylor
2015-06-12  4:47   ` Arun R Murthy
2015-06-11 13:31 ` [PATCH 3/3] drm/i915: Drop the 64k linear scanout alignment on gen2/3 ville.syrjala
2015-06-11 13:52   ` Chris Wilson
2015-06-14 23:18   ` shuang.he
2015-06-11 13:51 ` [PATCH 1/3] drm/i915: Actually respect DSPSURF alignment restrictions Chris Wilson
2015-06-12 10:50   ` Ville Syrjälä
2015-06-15 16:04     ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.