[PATCH v2] drm/i915: Error out when trying to set a y-tiled as a sprite

[PATCH v2] drm/i915: Error out when trying to set a y-tiled as a sprite

[Damien Lespiau]

From: Damien Lespiau <damien.lespiau@intel.com>

v2: Use a switch for consistency (Chris Wilson)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 87c8f1b..03307be 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -466,6 +466,15 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	if (intel_plane->pipe != intel_crtc->pipe)
 		return -EINVAL;
 
+	/* Sprite planes can be linear or x-tiled surfaces */
+	switch (obj->tiling_mode) {
+		case I915_TILING_NONE:
+		case I915_TILING_X:
+			break;
+		default:
+			return -EINVAL;
+	}
+
 	/*
 	 * Clamp the width & height into the visible area.  Note we don't
 	 * try to scale the source if part of the visible region is offscreen.
-- 
1.7.7.5

Re: [PATCH v2] drm/i915: Error out when trying to set a y-tiled as a sprite

[Jesse Barnes]

On Mon, 29 Oct 2012 15:14:51 +0000
Damien Lespiau <damien.lespiau@gmail.com> wrote:

> From: Damien Lespiau <damien.lespiau@intel.com>
> 
> v2: Use a switch for consistency (Chris Wilson)
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sprite.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 87c8f1b..03307be 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -466,6 +466,15 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	if (intel_plane->pipe != intel_crtc->pipe)
>  		return -EINVAL;
>  
> +	/* Sprite planes can be linear or x-tiled surfaces */
> +	switch (obj->tiling_mode) {
> +		case I915_TILING_NONE:
> +		case I915_TILING_X:
> +			break;
> +		default:
> +			return -EINVAL;
> +	}
> +
>  	/*
>  	 * Clamp the width & height into the visible area.  Note we don't
>  	 * try to scale the source if part of the visible region is offscreen.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH v2] drm/i915: Error out when trying to set a y-tiled as a sprite

[Daniel Vetter]

On Mon, Oct 29, 2012 at 09:33:24AM -0700, Jesse Barnes wrote:
> On Mon, 29 Oct 2012 15:14:51 +0000
> Damien Lespiau <damien.lespiau@gmail.com> wrote:
> 
> > From: Damien Lespiau <damien.lespiau@intel.com>
> > 
> > v2: Use a switch for consistency (Chris Wilson)
> > 
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_sprite.c |    9 +++++++++
> >  1 files changed, 9 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 87c8f1b..03307be 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -466,6 +466,15 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> >  	if (intel_plane->pipe != intel_crtc->pipe)
> >  		return -EINVAL;
> >  
> > +	/* Sprite planes can be linear or x-tiled surfaces */
> > +	switch (obj->tiling_mode) {
> > +		case I915_TILING_NONE:
> > +		case I915_TILING_X:
> > +			break;
> > +		default:
> > +			return -EINVAL;
> > +	}
> > +
> >  	/*
> >  	 * Clamp the width & height into the visible area.  Note we don't
> >  	 * try to scale the source if part of the visible region is offscreen.
> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

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

Re: [PATCH v2] drm/i915: Error out when trying to set a y-tiled as a sprite

[Ville Syrjälä]

On Mon, Oct 29, 2012 at 03:14:51PM +0000, Damien Lespiau wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
> 
> v2: Use a switch for consistency (Chris Wilson)
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sprite.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 87c8f1b..03307be 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -466,6 +466,15 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	if (intel_plane->pipe != intel_crtc->pipe)
>  		return -EINVAL;
>  
> +	/* Sprite planes can be linear or x-tiled surfaces */
> +	switch (obj->tiling_mode) {
> +		case I915_TILING_NONE:
> +		case I915_TILING_X:
> +			break;
> +		default:
> +			return -EINVAL;
> +	}

I'm confused why this is necessary. intel_pin_and_fence_fb_obj() already
has the check, so things should never go much further than this.

Futhermore someone can still go and change the tiling mode after this
point. Or indeed even at the same time as we're not holding
struct_mutex here (but this would be caught by
intel_pin_and_fence_fb_obj()).

-- 
Ville Syrjälä
Intel OTC