diff mbox

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

Message ID 1351523691-21106-1-git-send-email-damien.lespiau@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Damien Lespiau Oct. 29, 2012, 3:14 p.m. UTC
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(-)

Comments

Jesse Barnes Oct. 29, 2012, 4:33 p.m. UTC | #1
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>
Daniel Vetter Oct. 29, 2012, 8:29 p.m. UTC | #2
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
Ville Syrjala Oct. 31, 2012, 5:28 p.m. UTC | #3
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()).
diff mbox

Patch

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.