diff mbox

[05/11] drm/i915: Check framebuffer stride more thoroughly

Message ID 1351698624-26626-6-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Oct. 31, 2012, 3:50 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Make sure the the framebuffer stride is smaller than 32k. That
seems to be the limit on recent hardware. Not quite sure if
<=Gen4 has smaller limits.

Also when using a tiled memory make sure the object stride matches
the framebuffer stride.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---

I had an earlier version a long time ago that tried to use smaller stride limits
on <=Gen4, but as there isn't clear information what those limits are, I decided
to just check for the 32K limit everywhere. It's definitely an upper bound for
the older hardware as well.

 drivers/gpu/drm/i915/intel_display.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

Comments

Jesse Barnes Oct. 31, 2012, 8:25 p.m. UTC | #1
On Wed, 31 Oct 2012 17:50:18 +0200
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Make sure the the framebuffer stride is smaller than 32k. That
> seems to be the limit on recent hardware. Not quite sure if
> <=Gen4 has smaller limits.
> 
> Also when using a tiled memory make sure the object stride matches
> the framebuffer stride.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> 
> I had an earlier version a long time ago that tried to use smaller stride limits
> on <=Gen4, but as there isn't clear information what those limits are, I decided
> to just check for the 32K limit everywhere. It's definitely an upper bound for
> the older hardware as well.
> 
>  drivers/gpu/drm/i915/intel_display.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b42637b..f431f2a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8235,6 +8235,14 @@ int intel_framebuffer_init(struct drm_device *dev,
>  	if (mode_cmd->pitches[0] & 63)
>  		return -EINVAL;
>  
> +	/* FIXME <= Gen4 stride limits are bit unclear */
> +	if (mode_cmd->pitches[0] > 32768)
> +		return -EINVAL;
> +
> +	if (obj->tiling_mode != I915_TILING_NONE &&
> +	    mode_cmd->pitches[0] != obj->stride)
> +		return -EINVAL;
> +
>  	/* Reject formats not supported by any plane early. */
>  	switch (mode_cmd->pixel_format) {
>  	case DRM_FORMAT_C8:

The bspec archive should have the pre-gen4 info, but this is a good
start.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Ville Syrjälä Nov. 1, 2012, 2:06 p.m. UTC | #2
On Wed, Oct 31, 2012 at 01:25:11PM -0700, Jesse Barnes wrote:
> On Wed, 31 Oct 2012 17:50:18 +0200
> ville.syrjala@linux.intel.com wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Make sure the the framebuffer stride is smaller than 32k. That
> > seems to be the limit on recent hardware. Not quite sure if
> > <=Gen4 has smaller limits.
> > 
> > Also when using a tiled memory make sure the object stride matches
> > the framebuffer stride.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > 
> > I had an earlier version a long time ago that tried to use smaller stride limits
> > on <=Gen4, but as there isn't clear information what those limits are, I decided
> > to just check for the 32K limit everywhere. It's definitely an upper bound for
> > the older hardware as well.
> > 
> >  drivers/gpu/drm/i915/intel_display.c |    8 ++++++++
> >  1 files changed, 8 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index b42637b..f431f2a 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8235,6 +8235,14 @@ int intel_framebuffer_init(struct drm_device *dev,
> >  	if (mode_cmd->pitches[0] & 63)
> >  		return -EINVAL;
> >  
> > +	/* FIXME <= Gen4 stride limits are bit unclear */
> > +	if (mode_cmd->pitches[0] > 32768)
> > +		return -EINVAL;
> > +
> > +	if (obj->tiling_mode != I915_TILING_NONE &&
> > +	    mode_cmd->pitches[0] != obj->stride)
> > +		return -EINVAL;
> > +
> >  	/* Reject formats not supported by any plane early. */
> >  	switch (mode_cmd->pixel_format) {
> >  	case DRM_FORMAT_C8:
> 
> The bspec archive should have the pre-gen4 info, but this is a good
> start.

I trawled through them when I made the original patch, but the
information in the specs was lacking. Eg. for some registers they
just state something like "for tiled surfaces the stride is limited to
N", with no mention what the untiled limit is. So if someone wants to
figure out the real limits, the best option would be to test on real
HW. Currently I have access only to Gen5+ HW, so I can't do it myself.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b42637b..f431f2a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8235,6 +8235,14 @@  int intel_framebuffer_init(struct drm_device *dev,
 	if (mode_cmd->pitches[0] & 63)
 		return -EINVAL;
 
+	/* FIXME <= Gen4 stride limits are bit unclear */
+	if (mode_cmd->pitches[0] > 32768)
+		return -EINVAL;
+
+	if (obj->tiling_mode != I915_TILING_NONE &&
+	    mode_cmd->pitches[0] != obj->stride)
+		return -EINVAL;
+
 	/* Reject formats not supported by any plane early. */
 	switch (mode_cmd->pixel_format) {
 	case DRM_FORMAT_C8: