[PATCH] drm/i915: Detect invalid scanout pitches

[PATCH] drm/i915: Detect invalid scanout pitches

[Chris Wilson]

Report back the user error of attempting to setup a CRTC with an invalid
framebuffer pitch. This is trickier than it should be as on gen4, there
is a restriction that tiled surfaces must have a stride less than 16k -
which is less than the largest supported CRTC size.

References: https://bugs.freedesktop.org/show_bug.cgi?id=65099
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 39e7b1b..cdb768a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1871,6 +1871,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	struct drm_i915_gem_object *obj;
 	int plane = intel_crtc->plane;
 	unsigned long linear_offset;
+	int pitch_limit;
 	u32 dspcntr;
 	u32 reg;
 
@@ -1886,6 +1887,21 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
+	if (IS_VLV(dev)) {
+		pitch_limit = 32*1024;
+	} else if (IS_GEN4(dev)) {
+		if (obj->tiling_mode)
+			pitch_limit = 16*1024;
+		else
+			pitch_limit = 32*1024;
+	} else
+		pitch_limit = 8*1024;
+
+	if (fb->pitches[0] > pitch_limit) {
+		DRM_DEBUG_KMS("Invalid pitch (%d) for scanout\n", fb->pitches[0]);
+		return -EINVAL;
+	}
+
 	reg = DSPCNTR(plane);
 	dspcntr = I915_READ(reg);
 	/* Mask out pixel format bits in case we change it */
@@ -1983,6 +1999,11 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
 		return -EINVAL;
 	}
 
+	if (fb->pitches[0] > 32*1024) {
+		DRM_DEBUG_KMS("Invalid pitch (%d) for scanout\n", fb->pitches[0]);
+		return -EINVAL;
+	}
+
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
-- 
1.8.3.1

[PATCH] drm/i915: Detect invalid scanout pitches

[Chris Wilson]

Report back the user error of attempting to setup a CRTC with an invalid
framebuffer pitch. This is trickier than it should be as on gen4, there
is a restriction that tiled surfaces must have a stride less than 16k -
which is less than the largest supported CRTC size.

v2: Fix the limits for gen3

References: https://bugs.freedesktop.org/show_bug.cgi?id=65099
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 39e7b1b..68cab9f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1871,6 +1871,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	struct drm_i915_gem_object *obj;
 	int plane = intel_crtc->plane;
 	unsigned long linear_offset;
+	int pitch_limit;
 	u32 dspcntr;
 	u32 reg;
 
@@ -1886,6 +1887,27 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
+	if (IS_VLV(dev)) {
+		pitch_limit = 32*1024;
+	} else if (IS_GEN4(dev)) {
+		if (obj->tiling_mode)
+			pitch_limit = 16*1024;
+		else
+			pitch_limit = 32*1024;
+	} else if (IS_GEN3(dev)) {
+		if (obj->tiling_mode)
+			pitch_limit = 8*1024;
+		else
+			pitch_limit = 16*1024;
+	} else
+		pitch_limit = 8*1024;
+
+
+	if (fb->pitches[0] > pitch_limit) {
+		DRM_DEBUG_KMS("Invalid pitch (%d) for scanout\n", fb->pitches[0]);
+		return -EINVAL;
+	}
+
 	reg = DSPCNTR(plane);
 	dspcntr = I915_READ(reg);
 	/* Mask out pixel format bits in case we change it */
@@ -1983,6 +2005,11 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
 		return -EINVAL;
 	}
 
+	if (fb->pitches[0] > 32*1024) {
+		DRM_DEBUG_KMS("Invalid pitch (%d) for scanout\n", fb->pitches[0]);
+		return -EINVAL;
+	}
+
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
-- 
1.8.3.1

Re: [PATCH] drm/i915: Detect invalid scanout pitches

[Ville Syrjälä]

On Wed, Jun 19, 2013 at 04:50:34PM +0100, Chris Wilson wrote:
> Report back the user error of attempting to setup a CRTC with an invalid
> framebuffer pitch. This is trickier than it should be as on gen4, there
> is a restriction that tiled surfaces must have a stride less than 16k -
> which is less than the largest supported CRTC size.
> 
> v2: Fix the limits for gen3
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=65099
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 39e7b1b..68cab9f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1871,6 +1871,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  	struct drm_i915_gem_object *obj;
>  	int plane = intel_crtc->plane;
>  	unsigned long linear_offset;
> +	int pitch_limit;
>  	u32 dspcntr;
>  	u32 reg;
>  
> @@ -1886,6 +1887,27 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  	intel_fb = to_intel_framebuffer(fb);
>  	obj = intel_fb->obj;
>  
> +	if (IS_VLV(dev)) {

That won't compile.

> +		pitch_limit = 32*1024;
> +	} else if (IS_GEN4(dev)) {
> +		if (obj->tiling_mode)
> +			pitch_limit = 16*1024;
> +		else
> +			pitch_limit = 32*1024;
> +	} else if (IS_GEN3(dev)) {
> +		if (obj->tiling_mode)
> +			pitch_limit = 8*1024;
> +		else
> +			pitch_limit = 16*1024;
> +	} else
> +		pitch_limit = 8*1024;
> +
> +
> +	if (fb->pitches[0] > pitch_limit) {
> +		DRM_DEBUG_KMS("Invalid pitch (%d) for scanout\n", fb->pitches[0]);
> +		return -EINVAL;
> +	}

We already have a bit of pitch checking in intel_framebuffer_init().
In fact there's a FIXME about pre-ilk limits there.

Assuming all the planes on a specific piece of hardware have the same
pitch limits, I'd like the checks to be live in
intel_framebuffer_init() so that the issue gets caught as early as
possible. For stricter per-plane limits we obviously need the checks
in update_plane.

What I can gather from BSpec is this:
gen2: linear/tiled 8k, (maybe DSPC tiled max 4k?)
gen3: linear ?, tiled 8k
gen4: linear ?, tiled 16k
ctg: linear ?, tiled 16k
ilk+: 32k all the way

Looking at your patch you have 16k,32k,32k for the ?s in my list.
Otherwise your numbers seem to agree with my findings.

So, to me it looks like all the planes do share the same limits (DSPC on
gen2 might be a minor exception), so I think we could move all of these
checks to intel_framebuffer_init().

> +
>  	reg = DSPCNTR(plane);
>  	dspcntr = I915_READ(reg);
>  	/* Mask out pixel format bits in case we change it */
> @@ -1983,6 +2005,11 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
>  		return -EINVAL;
>  	}
>  
> +	if (fb->pitches[0] > 32*1024) {
> +		DRM_DEBUG_KMS("Invalid pitch (%d) for scanout\n", fb->pitches[0]);
> +		return -EINVAL;
> +	}
> +
>  	intel_fb = to_intel_framebuffer(fb);
>  	obj = intel_fb->obj;
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: Detect invalid scanout pitches

[Chris Wilson]

On Thu, Jun 20, 2013 at 11:17:16AM +0300, Ville Syrjälä wrote:
> We already have a bit of pitch checking in intel_framebuffer_init().
> In fact there's a FIXME about pre-ilk limits there.

It looks tidier to fix that check. We still need to double check the
values though as the tiling mode is independent of the fb config and may
be changed by the user.
 
> Assuming all the planes on a specific piece of hardware have the same
> pitch limits, I'd like the checks to be live in
> intel_framebuffer_init() so that the issue gets caught as early as
> possible. For stricter per-plane limits we obviously need the checks
> in update_plane.
> 
> What I can gather from BSpec is this:
> gen2: linear/tiled 8k, (maybe DSPC tiled max 4k?)
> gen3: linear ?, tiled 8k
> gen4: linear ?, tiled 16k
> ctg: linear ?, tiled 16k
> ilk+: 32k all the way
> 
> Looking at your patch you have 16k,32k,32k for the ?s in my list.
> Otherwise your numbers seem to agree with my findings.

The only one I didn't check was the VLV addendum.
 
> So, to me it looks like all the planes do share the same limits (DSPC on
> gen2 might be a minor exception), so I think we could move all of these
> checks to intel_framebuffer_init().

Except for the rare cases where the limits may change between init and
set-base...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Detect invalid scanout pitches

[Ville Syrjälä]

On Thu, Jun 20, 2013 at 10:14:36AM +0100, Chris Wilson wrote:
> On Thu, Jun 20, 2013 at 11:17:16AM +0300, Ville Syrjälä wrote:
> > We already have a bit of pitch checking in intel_framebuffer_init().
> > In fact there's a FIXME about pre-ilk limits there.
> 
> It looks tidier to fix that check. We still need to double check the
> values though as the tiling mode is independent of the fb config and may
> be changed by the user.

True, some checking needs to be done after pinning.

I guess we could have one function that has the checks, and just call it
from both places.

>  
> > Assuming all the planes on a specific piece of hardware have the same
> > pitch limits, I'd like the checks to be live in
> > intel_framebuffer_init() so that the issue gets caught as early as
> > possible. For stricter per-plane limits we obviously need the checks
> > in update_plane.
> > 
> > What I can gather from BSpec is this:
> > gen2: linear/tiled 8k, (maybe DSPC tiled max 4k?)
> > gen3: linear ?, tiled 8k
> > gen4: linear ?, tiled 16k
> > ctg: linear ?, tiled 16k
> > ilk+: 32k all the way
> > 
> > Looking at your patch you have 16k,32k,32k for the ?s in my list.
> > Otherwise your numbers seem to agree with my findings.
> 
> The only one I didn't check was the VLV addendum.
>  
> > So, to me it looks like all the planes do share the same limits (DSPC on
> > gen2 might be a minor exception), so I think we could move all of these
> > checks to intel_framebuffer_init().
> 
> Except for the rare cases where the limits may change between init and
> set-base...
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: Detect invalid scanout pitches

[Chris Wilson]

On Thu, Jun 20, 2013 at 01:29:10PM +0300, Ville Syrjälä wrote:
> On Thu, Jun 20, 2013 at 10:14:36AM +0100, Chris Wilson wrote:
> > On Thu, Jun 20, 2013 at 11:17:16AM +0300, Ville Syrjälä wrote:
> > > We already have a bit of pitch checking in intel_framebuffer_init().
> > > In fact there's a FIXME about pre-ilk limits there.
> > 
> > It looks tidier to fix that check. We still need to double check the
> > values though as the tiling mode is independent of the fb config and may
> > be changed by the user.
> 
> True, some checking needs to be done after pinning.
> 
> I guess we could have one function that has the checks, and just call it
> from both places.

I'm thinking we note the tiling (and anything else of significance)
during framebuffer_init() and reject the set-base if the object changes.
I don't think any userspace depends upon being able to change the object
after AddFB, and I don't think we want to allow the fb/obj to so easily
become inconsistent.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Detect invalid scanout pitches

[Ville Syrjälä]

On Thu, Jun 20, 2013 at 01:27:14PM +0100, Chris Wilson wrote:
> On Thu, Jun 20, 2013 at 01:29:10PM +0300, Ville Syrjälä wrote:
> > On Thu, Jun 20, 2013 at 10:14:36AM +0100, Chris Wilson wrote:
> > > On Thu, Jun 20, 2013 at 11:17:16AM +0300, Ville Syrjälä wrote:
> > > > We already have a bit of pitch checking in intel_framebuffer_init().
> > > > In fact there's a FIXME about pre-ilk limits there.
> > > 
> > > It looks tidier to fix that check. We still need to double check the
> > > values though as the tiling mode is independent of the fb config and may
> > > be changed by the user.
> > 
> > True, some checking needs to be done after pinning.
> > 
> > I guess we could have one function that has the checks, and just call it
> > from both places.
> 
> I'm thinking we note the tiling (and anything else of significance)
> during framebuffer_init() and reject the set-base if the object changes.
> I don't think any userspace depends upon being able to change the object
> after AddFB, and I don't think we want to allow the fb/obj to so easily
> become inconsistent.

An alternative idea I've been tossing around is that we'd reject tiling
changes while we have drm_framebuffers pointing at the object. So some
kind of fb_refcount on the object. That would make buggy user space trip
over a bit earlier. But either way seems reasonable to me.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: Detect invalid scanout pitches

[Chris Wilson]

On Thu, Jun 20, 2013 at 03:44:56PM +0300, Ville Syrjälä wrote:
> On Thu, Jun 20, 2013 at 01:27:14PM +0100, Chris Wilson wrote:
> > On Thu, Jun 20, 2013 at 01:29:10PM +0300, Ville Syrjälä wrote:
> > > On Thu, Jun 20, 2013 at 10:14:36AM +0100, Chris Wilson wrote:
> > > > On Thu, Jun 20, 2013 at 11:17:16AM +0300, Ville Syrjälä wrote:
> > > > > We already have a bit of pitch checking in intel_framebuffer_init().
> > > > > In fact there's a FIXME about pre-ilk limits there.
> > > > 
> > > > It looks tidier to fix that check. We still need to double check the
> > > > values though as the tiling mode is independent of the fb config and may
> > > > be changed by the user.
> > > 
> > > True, some checking needs to be done after pinning.
> > > 
> > > I guess we could have one function that has the checks, and just call it
> > > from both places.
> > 
> > I'm thinking we note the tiling (and anything else of significance)
> > during framebuffer_init() and reject the set-base if the object changes.
> > I don't think any userspace depends upon being able to change the object
> > after AddFB, and I don't think we want to allow the fb/obj to so easily
> > become inconsistent.
> 
> An alternative idea I've been tossing around is that we'd reject tiling
> changes while we have drm_framebuffers pointing at the object. So some
> kind of fb_refcount on the object. That would make buggy user space trip
> over a bit earlier. But either way seems reasonable to me.

That's viable; we need obj->fb tracking for frontbuffer write detection
as well. We can prevent such changes - as it is semantically equivalent
to EINVAL for pinned objects - once that tracking lands.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Detect invalid scanout pitches

[Ville Syrjälä]

On Thu, Jun 20, 2013 at 10:14:36AM +0100, Chris Wilson wrote:
> On Thu, Jun 20, 2013 at 11:17:16AM +0300, Ville Syrjälä wrote:
> > Assuming all the planes on a specific piece of hardware have the same
> > pitch limits, I'd like the checks to be live in
> > intel_framebuffer_init() so that the issue gets caught as early as
> > possible. For stricter per-plane limits we obviously need the checks
> > in update_plane.
> > 
> > What I can gather from BSpec is this:
> > gen2: linear/tiled 8k, (maybe DSPC tiled max 4k?)
> > gen3: linear ?, tiled 8k
> > gen4: linear ?, tiled 16k
> > ctg: linear ?, tiled 16k
> > ilk+: 32k all the way
> > 
> > Looking at your patch you have 16k,32k,32k for the ?s in my list.
> > Otherwise your numbers seem to agree with my findings.
> 
> The only one I didn't check was the VLV addendum.

My VLV doc says 16K for tiled.

-- 
Ville Syrjälä
Intel OTC

[PATCH] drm/i915: Detect invalid scanout pitches

[Chris Wilson]

Report back the user error of attempting to setup a CRTC with an invalid
framebuffer pitch. This is trickier than it should be as on gen4, there
is a restriction that tiled surfaces must have a stride less than 16k -
which is less than the largest supported CRTC size.

v2: Fix the limits for gen3
v3: Move check into intel_framebuffer_init() and fix VLV limits. (vsyrjala)

References: https://bugs.freedesktop.org/show_bug.cgi?id=65099
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 03a5ed0..ed84c57 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9223,6 +9223,7 @@ int intel_framebuffer_init(struct drm_device *dev,
 			   struct drm_mode_fb_cmd2 *mode_cmd,
 			   struct drm_i915_gem_object *obj)
 {
+	int pitch_limit;
 	int ret;
 
 	if (obj->tiling_mode == I915_TILING_Y) {
@@ -9236,10 +9237,26 @@ int intel_framebuffer_init(struct drm_device *dev,
 		return -EINVAL;
 	}
 
-	/* FIXME <= Gen4 stride limits are bit unclear */
-	if (mode_cmd->pitches[0] > 32768) {
-		DRM_DEBUG("pitch (%d) must be at less than 32768\n",
-			  mode_cmd->pitches[0]);
+	if (INTEL_INFO(dev)->gen > 4 && !IS_VALLEYVIEW(dev)) {
+		pitch_limit = 32*1024;
+	} else if (INTEL_INFO(dev)->gen > 3) {
+		if (obj->tiling_mode)
+			pitch_limit = 16*1024;
+		else
+			pitch_limit = 32*1024;
+	} else if (INTEL_INFO(dev)->gen > 2) {
+		if (obj->tiling_mode)
+			pitch_limit = 8*1024;
+		else
+			pitch_limit = 16*1024;
+	} else
+		/* XXX DSPC is limited to 4k tiled */
+		pitch_limit = 8*1024;
+
+	if (mode_cmd->pitches[0] > pitch_limit) {
+		DRM_DEBUG("%s pitch (%d) must be at less than %d\n",
+			  obj->tiling_mode ? "tiled" : "linear",
+			  mode_cmd->pitches[0], pitch_limit);
 		return -EINVAL;
 	}
 
-- 
1.8.3.1

Re: [PATCH] drm/i915: Detect invalid scanout pitches

[Ville Syrjälä]

On Thu, Jun 20, 2013 at 05:14:20PM +0100, Chris Wilson wrote:
> Report back the user error of attempting to setup a CRTC with an invalid
> framebuffer pitch. This is trickier than it should be as on gen4, there
> is a restriction that tiled surfaces must have a stride less than 16k -
> which is less than the largest supported CRTC size.
> 
> v2: Fix the limits for gen3
> v3: Move check into intel_framebuffer_init() and fix VLV limits. (vsyrjala)
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=65099
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 03a5ed0..ed84c57 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9223,6 +9223,7 @@ int intel_framebuffer_init(struct drm_device *dev,
>  			   struct drm_mode_fb_cmd2 *mode_cmd,
>  			   struct drm_i915_gem_object *obj)
>  {
> +	int pitch_limit;
>  	int ret;
>  
>  	if (obj->tiling_mode == I915_TILING_Y) {
> @@ -9236,10 +9237,26 @@ int intel_framebuffer_init(struct drm_device *dev,
>  		return -EINVAL;
>  	}
>  
> -	/* FIXME <= Gen4 stride limits are bit unclear */
> -	if (mode_cmd->pitches[0] > 32768) {
> -		DRM_DEBUG("pitch (%d) must be at less than 32768\n",
> -			  mode_cmd->pitches[0]);
> +	if (INTEL_INFO(dev)->gen > 4 && !IS_VALLEYVIEW(dev)) {

I believe Daniel wants to use >= for such gen checks, and I tend to
agree that it would make things a bit easier to parse.

> +		pitch_limit = 32*1024;
> +	} else if (INTEL_INFO(dev)->gen > 3) {
> +		if (obj->tiling_mode)
> +			pitch_limit = 16*1024;
> +		else
> +			pitch_limit = 32*1024;
> +	} else if (INTEL_INFO(dev)->gen > 2) {
> +		if (obj->tiling_mode)
> +			pitch_limit = 8*1024;
> +		else
> +			pitch_limit = 16*1024;
> +	} else
> +		/* XXX DSPC is limited to 4k tiled */
> +		pitch_limit = 8*1024;
> +
> +	if (mode_cmd->pitches[0] > pitch_limit) {
> +		DRM_DEBUG("%s pitch (%d) must be at less than %d\n",
> +			  obj->tiling_mode ? "tiled" : "linear",
> +			  mode_cmd->pitches[0], pitch_limit);
>  		return -EINVAL;
>  	}
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH] drm/i915: Detect invalid scanout pitches

[Chris Wilson]

Report back the user error of attempting to setup a CRTC with an invalid
framebuffer pitch. This is trickier than it should be as on gen4, there
is a restriction that tiled surfaces must have a stride less than 16k -
which is less than the largest supported CRTC size.

v2: Fix the limits for gen3
v3: Move check into intel_framebuffer_init() and fix VLV limits. (vsyrjala)
v4: Use idiomatic '>=' for generation checks

References: https://bugs.freedesktop.org/show_bug.cgi?id=65099
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0d76595..ae6f71f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9224,6 +9224,7 @@ int intel_framebuffer_init(struct drm_device *dev,
 			   struct drm_mode_fb_cmd2 *mode_cmd,
 			   struct drm_i915_gem_object *obj)
 {
+	int pitch_limit;
 	int ret;
 
 	if (obj->tiling_mode == I915_TILING_Y) {
@@ -9237,10 +9238,26 @@ int intel_framebuffer_init(struct drm_device *dev,
 		return -EINVAL;
 	}
 
-	/* FIXME <= Gen4 stride limits are bit unclear */
-	if (mode_cmd->pitches[0] > 32768) {
-		DRM_DEBUG("pitch (%d) must be at less than 32768\n",
-			  mode_cmd->pitches[0]);
+	if (INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev)) {
+		pitch_limit = 32*1024;
+	} else if (INTEL_INFO(dev)->gen >= 4) {
+		if (obj->tiling_mode)
+			pitch_limit = 16*1024;
+		else
+			pitch_limit = 32*1024;
+	} else if (INTEL_INFO(dev)->gen >= 3) {
+		if (obj->tiling_mode)
+			pitch_limit = 8*1024;
+		else
+			pitch_limit = 16*1024;
+	} else
+		/* XXX DSPC is limited to 4k tiled */
+		pitch_limit = 8*1024;
+
+	if (mode_cmd->pitches[0] > pitch_limit) {
+		DRM_DEBUG("%s pitch (%d) must be at less than %d\n",
+			  obj->tiling_mode ? "tiled" : "linear",
+			  mode_cmd->pitches[0], pitch_limit);
 		return -EINVAL;
 	}
 
-- 
1.8.3.1

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

Re: [PATCH] drm/i915: Detect invalid scanout pitches

[Daniel Vetter]

On Tue, Jun 25, 2013 at 05:26:45PM +0100, Chris Wilson wrote:
> Report back the user error of attempting to setup a CRTC with an invalid
> framebuffer pitch. This is trickier than it should be as on gen4, there
> is a restriction that tiled surfaces must have a stride less than 16k -
> which is less than the largest supported CRTC size.
> 
> v2: Fix the limits for gen3
> v3: Move check into intel_framebuffer_init() and fix VLV limits. (vsyrjala)
> v4: Use idiomatic '>=' for generation checks
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=65099
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch