All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Fix sprite offset on HSW
@ 2012-10-25 16:10 Damien Lespiau
  2012-10-26  7:33 ` Mika Kuoppala
  2012-10-26 13:18 ` Paulo Zanoni
  0 siblings, 2 replies; 5+ messages in thread
From: Damien Lespiau @ 2012-10-25 16:10 UTC (permalink / raw)
  To: intel-gfx

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

HSW consolidates SPRTILEOFF and SPRLINOFF into a single SPROFFSET
register.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h     |    3 +++
 drivers/gpu/drm/i915/intel_sprite.c |   18 +++++++++++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index be22aeb..2a6c0b6 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3187,6 +3187,7 @@
 #define _SPRA_SURF		0x7029c
 #define _SPRA_KEYMAX		0x702a0
 #define _SPRA_TILEOFF		0x702a4
+#define _SPRA_OFFSET		0x702a4
 #define _SPRA_SCALE		0x70304
 #define   SPRITE_SCALE_ENABLE	(1<<31)
 #define   SPRITE_FILTER_MASK	(3<<29)
@@ -3207,6 +3208,7 @@
 #define _SPRB_SURF		0x7129c
 #define _SPRB_KEYMAX		0x712a0
 #define _SPRB_TILEOFF		0x712a4
+#define _SPRB_OFFSET		0x712a4
 #define _SPRB_SCALE		0x71304
 #define _SPRB_GAMC		0x71400
 
@@ -3220,6 +3222,7 @@
 #define SPRSURF(pipe) _PIPE(pipe, _SPRA_SURF, _SPRB_SURF)
 #define SPRKEYMAX(pipe) _PIPE(pipe, _SPRA_KEYMAX, _SPRB_KEYMAX)
 #define SPRTILEOFF(pipe) _PIPE(pipe, _SPRA_TILEOFF, _SPRB_TILEOFF)
+#define SPROFFSET(pipe) _PIPE(pipe, _SPRA_OFFSET, _SPRB_OFFSET)
 #define SPRSCALE(pipe) _PIPE(pipe, _SPRA_SCALE, _SPRB_SCALE)
 #define SPRGAMC(pipe) _PIPE(pipe, _SPRA_GAMC, _SPRB_GAMC)
 
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 176c462..24b8231 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -127,13 +127,21 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 
 	I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
 	I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
-	if (obj->tiling_mode != I915_TILING_NONE) {
-		I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
+
+	if (IS_HASWELL(dev)) {
+		/* HSW consolidates SPRTILEOFF and SPRLINOFF into a single
+		 * SPROFFSET register */
+		I915_WRITE(SPROFFSET(pipe), (y << 16) | x);
 	} else {
-		unsigned long offset;
+		if (obj->tiling_mode != I915_TILING_NONE) {
+			I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
+		} else {
+			unsigned long offset;
 
-		offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
-		I915_WRITE(SPRLINOFF(pipe), offset);
+			offset = y * fb->pitches[0] +
+				 x * (fb->bits_per_pixel / 8);
+			I915_WRITE(SPRLINOFF(pipe), offset);
+		}
 	}
 	I915_WRITE(SPRSIZE(pipe), (crtc_h << 16) | crtc_w);
 	if (intel_plane->can_scale)
-- 
1.7.7.5

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

* Re: [PATCH] drm/i915: Fix sprite offset on HSW
  2012-10-25 16:10 [PATCH] drm/i915: Fix sprite offset on HSW Damien Lespiau
@ 2012-10-26  7:33 ` Mika Kuoppala
  2012-10-26  9:48   ` Chris Wilson
  2012-10-26  9:49   ` Daniel Vetter
  2012-10-26 13:18 ` Paulo Zanoni
  1 sibling, 2 replies; 5+ messages in thread
From: Mika Kuoppala @ 2012-10-26  7:33 UTC (permalink / raw)
  To: Damien Lespiau, intel-gfx

On Thu, 25 Oct 2012 17:10:01 +0100, Damien Lespiau <damien.lespiau@gmail.com> wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
> 
> HSW consolidates SPRTILEOFF and SPRLINOFF into a single SPROFFSET
> register.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h     |    3 +++
>  drivers/gpu/drm/i915/intel_sprite.c |   18 +++++++++++++-----
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index be22aeb..2a6c0b6 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3187,6 +3187,7 @@
>  #define _SPRA_SURF		0x7029c
>  #define _SPRA_KEYMAX		0x702a0
>  #define _SPRA_TILEOFF		0x702a4
> +#define _SPRA_OFFSET		0x702a4
>  #define _SPRA_SCALE		0x70304
>  #define   SPRITE_SCALE_ENABLE	(1<<31)
>  #define   SPRITE_FILTER_MASK	(3<<29)
> @@ -3207,6 +3208,7 @@
>  #define _SPRB_SURF		0x7129c
>  #define _SPRB_KEYMAX		0x712a0
>  #define _SPRB_TILEOFF		0x712a4
> +#define _SPRB_OFFSET		0x712a4
>  #define _SPRB_SCALE		0x71304
>  #define _SPRB_GAMC		0x71400
>  
> @@ -3220,6 +3222,7 @@
>  #define SPRSURF(pipe) _PIPE(pipe, _SPRA_SURF, _SPRB_SURF)
>  #define SPRKEYMAX(pipe) _PIPE(pipe, _SPRA_KEYMAX, _SPRB_KEYMAX)
>  #define SPRTILEOFF(pipe) _PIPE(pipe, _SPRA_TILEOFF, _SPRB_TILEOFF)
> +#define SPROFFSET(pipe) _PIPE(pipe, _SPRA_OFFSET, _SPRB_OFFSET)
>  #define SPRSCALE(pipe) _PIPE(pipe, _SPRA_SCALE, _SPRB_SCALE)
>  #define SPRGAMC(pipe) _PIPE(pipe, _SPRA_GAMC, _SPRB_GAMC)
>  
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 176c462..24b8231 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -127,13 +127,21 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>  
>  	I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
>  	I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
> -	if (obj->tiling_mode != I915_TILING_NONE) {
> -		I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
> +
> +	if (IS_HASWELL(dev)) {
> +		/* HSW consolidates SPRTILEOFF and SPRLINOFF into a single
> +		 * SPROFFSET register */

I don't know if upper layers sanitize already but
x should be < 8192 and y < 4096 in here, and both <4096 for ivb.
Emit warning and clamp if they are not in range?

> +		I915_WRITE(SPROFFSET(pipe), (y << 16) | x);
>  	} else {
> -		unsigned long offset;
> +		if (obj->tiling_mode != I915_TILING_NONE) {
> +			I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
> +		} else {
> +			unsigned long offset;
>  
> -		offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
> -		I915_WRITE(SPRLINOFF(pipe), offset);
> +			offset = y * fb->pitches[0] +
> +				 x * (fb->bits_per_pixel / 8);
> +			I915_WRITE(SPRLINOFF(pipe), offset);
> +		}
>  	}
>  	I915_WRITE(SPRSIZE(pipe), (crtc_h << 16) | crtc_w);
>  	if (intel_plane->can_scale)
> -- 
> 1.7.7.5

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

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

* Re: [PATCH] drm/i915: Fix sprite offset on HSW
  2012-10-26  7:33 ` Mika Kuoppala
@ 2012-10-26  9:48   ` Chris Wilson
  2012-10-26  9:49   ` Daniel Vetter
  1 sibling, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2012-10-26  9:48 UTC (permalink / raw)
  To: Mika Kuoppala, Damien Lespiau, intel-gfx

On Fri, 26 Oct 2012 10:33:00 +0300, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> On Thu, 25 Oct 2012 17:10:01 +0100, Damien Lespiau <damien.lespiau@gmail.com> wrote:
> > @@ -127,13 +127,21 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
> >  
> >  	I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
> >  	I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
> > -	if (obj->tiling_mode != I915_TILING_NONE) {
> > -		I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
> > +
> > +	if (IS_HASWELL(dev)) {
> > +		/* HSW consolidates SPRTILEOFF and SPRLINOFF into a single
> > +		 * SPROFFSET register */
> 
> I don't know if upper layers sanitize already but
> x should be < 8192 and y < 4096 in here, and both <4096 for ivb.
> Emit warning and clamp if they are not in range?

Should be -EINVAL on entry, and BUG_ON here if truly paranoid -
knowledge of the hardware is already spread across the layers.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH] drm/i915: Fix sprite offset on HSW
  2012-10-26  7:33 ` Mika Kuoppala
  2012-10-26  9:48   ` Chris Wilson
@ 2012-10-26  9:49   ` Daniel Vetter
  1 sibling, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2012-10-26  9:49 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

On Fri, Oct 26, 2012 at 9:33 AM, Mika Kuoppala
<mika.kuoppala@linux.intel.com> wrote:
> I don't know if upper layers sanitize already but
> x should be < 8192 and y < 4096 in here, and both <4096 for ivb.
> Emit warning and clamp if they are not in range?

We probably need to do the same trick we're already doing in the main
plane code by adjusting the the base address, see:

commit c2c75131244507c93f812862fdbd4f3a37139401
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu Jul 5 12:17:30 2012 +0200

    drm/i915: adjust framebuffer base address on gen4+

Damien, could you throw this in on top?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

* Re: [PATCH] drm/i915: Fix sprite offset on HSW
  2012-10-25 16:10 [PATCH] drm/i915: Fix sprite offset on HSW Damien Lespiau
  2012-10-26  7:33 ` Mika Kuoppala
@ 2012-10-26 13:18 ` Paulo Zanoni
  1 sibling, 0 replies; 5+ messages in thread
From: Paulo Zanoni @ 2012-10-26 13:18 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

Hi

2012/10/25 Damien Lespiau <damien.lespiau@gmail.com>:
> From: Damien Lespiau <damien.lespiau@intel.com>
>
> HSW consolidates SPRTILEOFF and SPRLINOFF into a single SPROFFSET
> register.
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h     |    3 +++
>  drivers/gpu/drm/i915/intel_sprite.c |   18 +++++++++++++-----
>  2 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index be22aeb..2a6c0b6 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3187,6 +3187,7 @@
>  #define _SPRA_SURF             0x7029c
>  #define _SPRA_KEYMAX           0x702a0
>  #define _SPRA_TILEOFF          0x702a4
> +#define _SPRA_OFFSET           0x702a4
>  #define _SPRA_SCALE            0x70304
>  #define   SPRITE_SCALE_ENABLE  (1<<31)
>  #define   SPRITE_FILTER_MASK   (3<<29)
> @@ -3207,6 +3208,7 @@
>  #define _SPRB_SURF             0x7129c
>  #define _SPRB_KEYMAX           0x712a0
>  #define _SPRB_TILEOFF          0x712a4
> +#define _SPRB_OFFSET           0x712a4
>  #define _SPRB_SCALE            0x71304
>  #define _SPRB_GAMC             0x71400
>
> @@ -3220,6 +3222,7 @@
>  #define SPRSURF(pipe) _PIPE(pipe, _SPRA_SURF, _SPRB_SURF)
>  #define SPRKEYMAX(pipe) _PIPE(pipe, _SPRA_KEYMAX, _SPRB_KEYMAX)
>  #define SPRTILEOFF(pipe) _PIPE(pipe, _SPRA_TILEOFF, _SPRB_TILEOFF)
> +#define SPROFFSET(pipe) _PIPE(pipe, _SPRA_OFFSET, _SPRB_OFFSET)
>  #define SPRSCALE(pipe) _PIPE(pipe, _SPRA_SCALE, _SPRB_SCALE)
>  #define SPRGAMC(pipe) _PIPE(pipe, _SPRA_GAMC, _SPRB_GAMC)
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 176c462..24b8231 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -127,13 +127,21 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>
>         I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
>         I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
> -       if (obj->tiling_mode != I915_TILING_NONE) {
> -               I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
> +
> +       if (IS_HASWELL(dev)) {
> +               /* HSW consolidates SPRTILEOFF and SPRLINOFF into a single
> +                * SPROFFSET register */
> +               I915_WRITE(SPROFFSET(pipe), (y << 16) | x);
>         } else {
> -               unsigned long offset;
> +               if (obj->tiling_mode != I915_TILING_NONE) {
> +                       I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
> +               } else {
> +                       unsigned long offset;
>
> -               offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
> -               I915_WRITE(SPRLINOFF(pipe), offset);
> +                       offset = y * fb->pitches[0] +
> +                                x * (fb->bits_per_pixel / 8);
> +                       I915_WRITE(SPRLINOFF(pipe), offset);
> +               }

<bikeshed>
Doing "if (is_hsw) { code; } else if (obj->tiling_mode) { code; } else
{ code; }" would probably make reading the patch easier :)
But I don't really think you need to resend just because of this.
</bikeshed>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I'd love to see the same fix for the "Primary Plane" registers since
this will get us rid of many "Unclaimed write" messages.

>         }
>         I915_WRITE(SPRSIZE(pipe), (crtc_h << 16) | crtc_w);
>         if (intel_plane->can_scale)
> --
> 1.7.7.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

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

end of thread, other threads:[~2012-10-26 13:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-25 16:10 [PATCH] drm/i915: Fix sprite offset on HSW Damien Lespiau
2012-10-26  7:33 ` Mika Kuoppala
2012-10-26  9:48   ` Chris Wilson
2012-10-26  9:49   ` Daniel Vetter
2012-10-26 13:18 ` Paulo Zanoni

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.