[PATCH 00/11] drm/i915: Patches cherry-picked from drm_atomic

[PATCH 00/11] drm/i915: Patches cherry-picked from drm_atomic

[ville.syrjala]

Here's a bunch of changes that were sitting in my drm_atomic tree, but
not really related to that work.

I quickly modified the fb offset handling patch to avoid having to
write test cases :), and the stride patch to avoid open questions
about <=Gen4 stride limits.

The execbuffer patch is an open question because IIRC Chris said
he doesn't like the whole concept. So if we don't want that patch,
then I think we should either remove the whole mechanism, or at least
disable it on IVB. Currently it seems capable of adding MI_WAIT_EVENT
commands into the ring with undefined wait bits enabled.

[PATCH 01/11] drm/i915: Fix display pixel format handling

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Fix support for all RGB/BGR pixel formats (except the 16:16:16:16 float
format).

Fix intel_init_framebuffer() to match hardware and driver limitations:
* RGB332 is not supported at all
* CI8 is supported
* XRGB1555 & co. are supported on Gen3 and earlier
* XRGB210101010 & co. are supported from Gen4 onwards
* BGR formats are supported from Gen4 onwards
* YUV formats are supported from Gen5 onwards (driver limitation)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |   17 ++++--
 drivers/gpu/drm/i915/intel_display.c |   98 ++++++++++++++++++++++------------
 2 files changed, 76 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index be22aeb..da8400a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3008,12 +3008,19 @@
 #define   DISPPLANE_GAMMA_ENABLE		(1<<30)
 #define   DISPPLANE_GAMMA_DISABLE		0
 #define   DISPPLANE_PIXFORMAT_MASK		(0xf<<26)
+#define   DISPPLANE_YUV422			(0x0<<26)
 #define   DISPPLANE_8BPP			(0x2<<26)
-#define   DISPPLANE_15_16BPP			(0x4<<26)
-#define   DISPPLANE_16BPP			(0x5<<26)
-#define   DISPPLANE_32BPP_NO_ALPHA		(0x6<<26)
-#define   DISPPLANE_32BPP			(0x7<<26)
-#define   DISPPLANE_32BPP_30BIT_NO_ALPHA	(0xa<<26)
+#define   DISPPLANE_BGRA555			(0x3<<26)
+#define   DISPPLANE_BGRX555			(0x4<<26)
+#define   DISPPLANE_BGRX565			(0x5<<26)
+#define   DISPPLANE_BGRX888			(0x6<<26)
+#define   DISPPLANE_BGRA888			(0x7<<26)
+#define   DISPPLANE_RGBX101010			(0x8<<26)
+#define   DISPPLANE_RGBA101010			(0x9<<26)
+#define   DISPPLANE_BGRX101010			(0xa<<26)
+#define   DISPPLANE_RGBX161616			(0xc<<26)
+#define   DISPPLANE_RGBX888			(0xe<<26)
+#define   DISPPLANE_RGBA888			(0xf<<26)
 #define   DISPPLANE_STEREO_ENABLE		(1<<25)
 #define   DISPPLANE_STEREO_DISABLE		0
 #define   DISPPLANE_SEL_PIPE_SHIFT		24
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a5be346..6d3feea 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1998,24 +1998,38 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	dspcntr = I915_READ(reg);
 	/* Mask out pixel format bits in case we change it */
 	dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
-	switch (fb->bits_per_pixel) {
-	case 8:
+	switch (fb->pixel_format) {
+	case DRM_FORMAT_C8:
 		dspcntr |= DISPPLANE_8BPP;
 		break;
-	case 16:
-		if (fb->depth == 15)
-			dspcntr |= DISPPLANE_15_16BPP;
-		else
-			dspcntr |= DISPPLANE_16BPP;
-		break;
-	case 24:
-	case 32:
-		dspcntr |= DISPPLANE_32BPP_NO_ALPHA;
+	case DRM_FORMAT_XRGB1555:
+	case DRM_FORMAT_ARGB1555:
+		dspcntr |= DISPPLANE_BGRX555;
+		break;
+	case DRM_FORMAT_RGB565:
+		dspcntr |= DISPPLANE_BGRX565;
+		break;
+	case DRM_FORMAT_XRGB8888:
+	case DRM_FORMAT_ARGB8888:
+		dspcntr |= DISPPLANE_BGRX888;
+		break;
+	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_ABGR8888:
+		dspcntr |= DISPPLANE_RGBX888;
+		break;
+	case DRM_FORMAT_XRGB2101010:
+	case DRM_FORMAT_ARGB2101010:
+		dspcntr |= DISPPLANE_BGRX101010;
+		break;
+	case DRM_FORMAT_XBGR2101010:
+	case DRM_FORMAT_ABGR2101010:
+		dspcntr |= DISPPLANE_RGBX101010;
 		break;
 	default:
-		DRM_ERROR("Unknown color depth %d\n", fb->bits_per_pixel);
+		DRM_ERROR("Unknown pixel format 0x%08x\n", fb->pixel_format);
 		return -EINVAL;
 	}
+
 	if (INTEL_INFO(dev)->gen >= 4) {
 		if (obj->tiling_mode != I915_TILING_NONE)
 			dspcntr |= DISPPLANE_TILED;
@@ -2082,27 +2096,31 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
 	dspcntr = I915_READ(reg);
 	/* Mask out pixel format bits in case we change it */
 	dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
-	switch (fb->bits_per_pixel) {
-	case 8:
+	switch (fb->pixel_format) {
+	case DRM_FORMAT_C8:
 		dspcntr |= DISPPLANE_8BPP;
 		break;
-	case 16:
-		if (fb->depth != 16)
-			return -EINVAL;
-
-		dspcntr |= DISPPLANE_16BPP;
-		break;
-	case 24:
-	case 32:
-		if (fb->depth == 24)
-			dspcntr |= DISPPLANE_32BPP_NO_ALPHA;
-		else if (fb->depth == 30)
-			dspcntr |= DISPPLANE_32BPP_30BIT_NO_ALPHA;
-		else
-			return -EINVAL;
+	case DRM_FORMAT_RGB565:
+		dspcntr |= DISPPLANE_BGRX565;
+		break;
+	case DRM_FORMAT_XRGB8888:
+	case DRM_FORMAT_ARGB8888:
+		dspcntr |= DISPPLANE_BGRX888;
+		break;
+	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_ABGR8888:
+		dspcntr |= DISPPLANE_RGBX888;
+		break;
+	case DRM_FORMAT_XRGB2101010:
+	case DRM_FORMAT_ARGB2101010:
+		dspcntr |= DISPPLANE_BGRX101010;
+		break;
+	case DRM_FORMAT_XBGR2101010:
+	case DRM_FORMAT_ABGR2101010:
+		dspcntr |= DISPPLANE_RGBX101010;
 		break;
 	default:
-		DRM_ERROR("Unknown color depth %d\n", fb->bits_per_pixel);
+		DRM_ERROR("Unknown pixel format 0x%08x\n", fb->pixel_format);
 		return -EINVAL;
 	}
 
@@ -8195,24 +8213,36 @@ int intel_framebuffer_init(struct drm_device *dev,
 	if (mode_cmd->pitches[0] & 63)
 		return -EINVAL;
 
+	/* Reject formats not supported by any plane early. */
 	switch (mode_cmd->pixel_format) {
-	case DRM_FORMAT_RGB332:
+	case DRM_FORMAT_C8:
 	case DRM_FORMAT_RGB565:
 	case DRM_FORMAT_XRGB8888:
-	case DRM_FORMAT_XBGR8888:
 	case DRM_FORMAT_ARGB8888:
+		break;
+	case DRM_FORMAT_XRGB1555:
+	case DRM_FORMAT_ARGB1555:
+		if (INTEL_INFO(dev)->gen > 3)
+			return -EINVAL;
+		break;
+	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_ABGR8888:
 	case DRM_FORMAT_XRGB2101010:
 	case DRM_FORMAT_ARGB2101010:
-		/* RGB formats are common across chipsets */
+	case DRM_FORMAT_XBGR2101010:
+	case DRM_FORMAT_ABGR2101010:
+		if (INTEL_INFO(dev)->gen < 4)
+			return -EINVAL;
 		break;
 	case DRM_FORMAT_YUYV:
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_VYUY:
+		if (INTEL_INFO(dev)->gen < 6)
+			return -EINVAL;
 		break;
 	default:
-		DRM_DEBUG_KMS("unsupported pixel format %u\n",
-				mode_cmd->pixel_format);
+		DRM_DEBUG_KMS("unsupported pixel format 0x%08x\n", mode_cmd->pixel_format);
 		return -EINVAL;
 	}
 
-- 
1.7.8.6

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

Re: [PATCH 01/11] drm/i915: Fix display pixel format handling

[Jesse Barnes]

On Wed, 31 Oct 2012 17:50:14 +0200
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Fix support for all RGB/BGR pixel formats (except the 16:16:16:16 float
> format).
> 
> Fix intel_init_framebuffer() to match hardware and driver limitations:
> * RGB332 is not supported at all
> * CI8 is supported
> * XRGB1555 & co. are supported on Gen3 and earlier
> * XRGB210101010 & co. are supported from Gen4 onwards
> * BGR formats are supported from Gen4 onwards
> * YUV formats are supported from Gen5 onwards (driver limitation)

Yeah looks nice.  I love how our definitions have the component order
reversed relative to the fourcc-ish codes.

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

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/11] drm/i915: Fix display pixel format handling

[Ville Syrjälä]

On Wed, Oct 31, 2012 at 01:20:23PM -0700, Jesse Barnes wrote:
> On Wed, 31 Oct 2012 17:50:14 +0200
> ville.syrjala@linux.intel.com wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Fix support for all RGB/BGR pixel formats (except the 16:16:16:16 float
> > format).
> > 
> > Fix intel_init_framebuffer() to match hardware and driver limitations:
> > * RGB332 is not supported at all
> > * CI8 is supported
> > * XRGB1555 & co. are supported on Gen3 and earlier
> > * XRGB210101010 & co. are supported from Gen4 onwards
> > * BGR formats are supported from Gen4 onwards
> > * YUV formats are supported from Gen5 onwards (driver limitation)
> 
> Yeah looks nice.  I love how our definitions have the component order
> reversed relative to the fourcc-ish codes.

Yeah. I just copied the names more or less blindly from the spec. I
usually prefer to keep register definitons and specs in sync, since
it makes it easier to search for stuff in the spec. But if people
are bothered by this mess, I won't object to changing the defns to
match the fourcc stuff.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 01/11] drm/i915: Fix display pixel format handling

[Chris Wilson]

[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]

On Thu, 1 Nov 2012 16:03:13 +0200, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Oct 31, 2012 at 01:20:23PM -0700, Jesse Barnes wrote:
> > On Wed, 31 Oct 2012 17:50:14 +0200
> > ville.syrjala@linux.intel.com wrote:
> > 
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Fix support for all RGB/BGR pixel formats (except the 16:16:16:16 float
> > > format).
> > > 
> > > Fix intel_init_framebuffer() to match hardware and driver limitations:
> > > * RGB332 is not supported at all
> > > * CI8 is supported
> > > * XRGB1555 & co. are supported on Gen3 and earlier
> > > * XRGB210101010 & co. are supported from Gen4 onwards
> > > * BGR formats are supported from Gen4 onwards
> > > * YUV formats are supported from Gen5 onwards (driver limitation)
> > 
> > Yeah looks nice.  I love how our definitions have the component order
> > reversed relative to the fourcc-ish codes.
> 
> Yeah. I just copied the names more or less blindly from the spec. I
> usually prefer to keep register definitons and specs in sync, since
> it makes it easier to search for stuff in the spec. But if people
> are bothered by this mess, I won't object to changing the defns to
> match the fourcc stuff.

We have the same issue in userspace, where we follow the spec counter to
the common convention. It is just one of those things you get used to.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH 02/11] drm/i915: Add SURFLIVE register definitions

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index da8400a..2555986 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3041,6 +3041,7 @@
 #define _DSPASIZE		0x70190
 #define _DSPASURF		0x7019C /* 965+ only */
 #define _DSPATILEOFF		0x701A4 /* 965+ only */
+#define _DSPASURFLIVE		0x701AC
 
 #define DSPCNTR(plane) _PIPE(plane, _DSPACNTR, _DSPBCNTR)
 #define DSPADDR(plane) _PIPE(plane, _DSPAADDR, _DSPBADDR)
@@ -3050,6 +3051,7 @@
 #define DSPSURF(plane) _PIPE(plane, _DSPASURF, _DSPBSURF)
 #define DSPTILEOFF(plane) _PIPE(plane, _DSPATILEOFF, _DSPBTILEOFF)
 #define DSPLINOFF(plane) DSPADDR(plane)
+#define DSPSURFLIVE(plane) _PIPE(plane, _DSPASURFLIVE, _DSPBSURFLIVE)
 
 /* Display/Sprite base address macros */
 #define DISP_BASEADDR_MASK	(0xfffff000)
@@ -3095,6 +3097,7 @@
 #define _DSPBSIZE		0x71190
 #define _DSPBSURF		0x7119C
 #define _DSPBTILEOFF		0x711A4
+#define _DSPBSURFLIVE		0x711AC
 
 /* Sprite A control */
 #define _DVSACNTR		0x72180
@@ -3160,6 +3163,7 @@
 #define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF)
 #define DVSKEYVAL(pipe) _PIPE(pipe, _DVSAKEYVAL, _DVSBKEYVAL)
 #define DVSKEYMSK(pipe) _PIPE(pipe, _DVSAKEYMSK, _DVSBKEYMSK)
+#define DVSSURFLIVE(pipe) _PIPE(pipe, _DVSASURFLIVE, _DVSBSURFLIVE)
 
 #define _SPRA_CTL		0x70280
 #define   SPRITE_ENABLE			(1<<31)
@@ -3194,6 +3198,7 @@
 #define _SPRA_SURF		0x7029c
 #define _SPRA_KEYMAX		0x702a0
 #define _SPRA_TILEOFF		0x702a4
+#define _SPRA_SURFLIVE		0x702ac
 #define _SPRA_SCALE		0x70304
 #define   SPRITE_SCALE_ENABLE	(1<<31)
 #define   SPRITE_FILTER_MASK	(3<<29)
@@ -3214,6 +3219,7 @@
 #define _SPRB_SURF		0x7129c
 #define _SPRB_KEYMAX		0x712a0
 #define _SPRB_TILEOFF		0x712a4
+#define _SPRB_SURFLIVE		0x712ac
 #define _SPRB_SCALE		0x71304
 #define _SPRB_GAMC		0x71400
 
@@ -3229,6 +3235,7 @@
 #define SPRTILEOFF(pipe) _PIPE(pipe, _SPRA_TILEOFF, _SPRB_TILEOFF)
 #define SPRSCALE(pipe) _PIPE(pipe, _SPRA_SCALE, _SPRB_SCALE)
 #define SPRGAMC(pipe) _PIPE(pipe, _SPRA_GAMC, _SPRB_GAMC)
+#define SPRSURFLIVE(pipe) _PIPE(pipe, _SPRA_SURFLIVE, _SPRB_SURFLIVE)
 
 /* VBIOS regs */
 #define VGACNTRL		0x71400
-- 
1.7.8.6

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

Re: [PATCH 02/11] drm/i915: Add SURFLIVE register definitions

[Jesse Barnes]

On Wed, 31 Oct 2012 17:50:15 +0200
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index da8400a..2555986 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3041,6 +3041,7 @@
>  #define _DSPASIZE		0x70190
>  #define _DSPASURF		0x7019C /* 965+ only */
>  #define _DSPATILEOFF		0x701A4 /* 965+ only */
> +#define _DSPASURFLIVE		0x701AC
>  
>  #define DSPCNTR(plane) _PIPE(plane, _DSPACNTR, _DSPBCNTR)
>  #define DSPADDR(plane) _PIPE(plane, _DSPAADDR, _DSPBADDR)
> @@ -3050,6 +3051,7 @@
>  #define DSPSURF(plane) _PIPE(plane, _DSPASURF, _DSPBSURF)
>  #define DSPTILEOFF(plane) _PIPE(plane, _DSPATILEOFF, _DSPBTILEOFF)
>  #define DSPLINOFF(plane) DSPADDR(plane)
> +#define DSPSURFLIVE(plane) _PIPE(plane, _DSPASURFLIVE, _DSPBSURFLIVE)
>  
>  /* Display/Sprite base address macros */
>  #define DISP_BASEADDR_MASK	(0xfffff000)
> @@ -3095,6 +3097,7 @@
>  #define _DSPBSIZE		0x71190
>  #define _DSPBSURF		0x7119C
>  #define _DSPBTILEOFF		0x711A4
> +#define _DSPBSURFLIVE		0x711AC
>  
>  /* Sprite A control */
>  #define _DVSACNTR		0x72180
> @@ -3160,6 +3163,7 @@
>  #define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF)
>  #define DVSKEYVAL(pipe) _PIPE(pipe, _DVSAKEYVAL, _DVSBKEYVAL)
>  #define DVSKEYMSK(pipe) _PIPE(pipe, _DVSAKEYMSK, _DVSBKEYMSK)
> +#define DVSSURFLIVE(pipe) _PIPE(pipe, _DVSASURFLIVE, _DVSBSURFLIVE)
>  
>  #define _SPRA_CTL		0x70280
>  #define   SPRITE_ENABLE			(1<<31)
> @@ -3194,6 +3198,7 @@
>  #define _SPRA_SURF		0x7029c
>  #define _SPRA_KEYMAX		0x702a0
>  #define _SPRA_TILEOFF		0x702a4
> +#define _SPRA_SURFLIVE		0x702ac
>  #define _SPRA_SCALE		0x70304
>  #define   SPRITE_SCALE_ENABLE	(1<<31)
>  #define   SPRITE_FILTER_MASK	(3<<29)
> @@ -3214,6 +3219,7 @@
>  #define _SPRB_SURF		0x7129c
>  #define _SPRB_KEYMAX		0x712a0
>  #define _SPRB_TILEOFF		0x712a4
> +#define _SPRB_SURFLIVE		0x712ac
>  #define _SPRB_SCALE		0x71304
>  #define _SPRB_GAMC		0x71400
>  
> @@ -3229,6 +3235,7 @@
>  #define SPRTILEOFF(pipe) _PIPE(pipe, _SPRA_TILEOFF, _SPRB_TILEOFF)
>  #define SPRSCALE(pipe) _PIPE(pipe, _SPRA_SCALE, _SPRB_SCALE)
>  #define SPRGAMC(pipe) _PIPE(pipe, _SPRA_GAMC, _SPRB_GAMC)
> +#define SPRSURFLIVE(pipe) _PIPE(pipe, _SPRA_SURFLIVE, _SPRB_SURFLIVE)
>  
>  /* VBIOS regs */
>  #define VGACNTRL		0x71400

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

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/11] drm/i915: Add SURFLIVE register definitions

[Daniel Vetter]

On Wed, Oct 31, 2012 at 01:23:05PM -0700, Jesse Barnes wrote:
> On Wed, 31 Oct 2012 17:50:15 +0200
> ville.syrjala@linux.intel.com wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Fails to apply here somehow. Also, this thing is base64 encoded, which
confused my normal workflow for a bit ... Dunno what exactly caused this
havoc.
-Daniel

> > ---
> >  drivers/gpu/drm/i915/i915_reg.h |    7 +++++++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index da8400a..2555986 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3041,6 +3041,7 @@
> >  #define _DSPASIZE		0x70190
> >  #define _DSPASURF		0x7019C /* 965+ only */
> >  #define _DSPATILEOFF		0x701A4 /* 965+ only */
> > +#define _DSPASURFLIVE		0x701AC
> >  
> >  #define DSPCNTR(plane) _PIPE(plane, _DSPACNTR, _DSPBCNTR)
> >  #define DSPADDR(plane) _PIPE(plane, _DSPAADDR, _DSPBADDR)
> > @@ -3050,6 +3051,7 @@
> >  #define DSPSURF(plane) _PIPE(plane, _DSPASURF, _DSPBSURF)
> >  #define DSPTILEOFF(plane) _PIPE(plane, _DSPATILEOFF, _DSPBTILEOFF)
> >  #define DSPLINOFF(plane) DSPADDR(plane)
> > +#define DSPSURFLIVE(plane) _PIPE(plane, _DSPASURFLIVE, _DSPBSURFLIVE)
> >  
> >  /* Display/Sprite base address macros */
> >  #define DISP_BASEADDR_MASK	(0xfffff000)
> > @@ -3095,6 +3097,7 @@
> >  #define _DSPBSIZE		0x71190
> >  #define _DSPBSURF		0x7119C
> >  #define _DSPBTILEOFF		0x711A4
> > +#define _DSPBSURFLIVE		0x711AC
> >  
> >  /* Sprite A control */
> >  #define _DVSACNTR		0x72180
> > @@ -3160,6 +3163,7 @@
> >  #define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF)
> >  #define DVSKEYVAL(pipe) _PIPE(pipe, _DVSAKEYVAL, _DVSBKEYVAL)
> >  #define DVSKEYMSK(pipe) _PIPE(pipe, _DVSAKEYMSK, _DVSBKEYMSK)
> > +#define DVSSURFLIVE(pipe) _PIPE(pipe, _DVSASURFLIVE, _DVSBSURFLIVE)
> >  
> >  #define _SPRA_CTL		0x70280
> >  #define   SPRITE_ENABLE			(1<<31)
> > @@ -3194,6 +3198,7 @@
> >  #define _SPRA_SURF		0x7029c
> >  #define _SPRA_KEYMAX		0x702a0
> >  #define _SPRA_TILEOFF		0x702a4
> > +#define _SPRA_SURFLIVE		0x702ac
> >  #define _SPRA_SCALE		0x70304
> >  #define   SPRITE_SCALE_ENABLE	(1<<31)
> >  #define   SPRITE_FILTER_MASK	(3<<29)
> > @@ -3214,6 +3219,7 @@
> >  #define _SPRB_SURF		0x7129c
> >  #define _SPRB_KEYMAX		0x712a0
> >  #define _SPRB_TILEOFF		0x712a4
> > +#define _SPRB_SURFLIVE		0x712ac
> >  #define _SPRB_SCALE		0x71304
> >  #define _SPRB_GAMC		0x71400
> >  
> > @@ -3229,6 +3235,7 @@
> >  #define SPRTILEOFF(pipe) _PIPE(pipe, _SPRA_TILEOFF, _SPRB_TILEOFF)
> >  #define SPRSCALE(pipe) _PIPE(pipe, _SPRA_SCALE, _SPRB_SCALE)
> >  #define SPRGAMC(pipe) _PIPE(pipe, _SPRA_GAMC, _SPRB_GAMC)
> > +#define SPRSURFLIVE(pipe) _PIPE(pipe, _SPRA_SURFLIVE, _SPRB_SURFLIVE)
> >  
> >  /* VBIOS regs */
> >  #define VGACNTRL		0x71400
> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> -- 
> Jesse Barnes, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 02/11] drm/i915: Add SURFLIVE register definitions

[Ville Syrjälä]

On Wed, Oct 31, 2012 at 11:57:00PM +0100, Daniel Vetter wrote:
> On Wed, Oct 31, 2012 at 01:23:05PM -0700, Jesse Barnes wrote:
> > On Wed, 31 Oct 2012 17:50:15 +0200
> > ville.syrjala@linux.intel.com wrote:
> > 
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Fails to apply here somehow. Also, this thing is base64 encoded, which
> confused my normal workflow for a bit ... Dunno what exactly caused this
> havoc.

Strange. I had it sitting on top of drm-intel-next when I sent it.

The whole series seems to be base64 after I got it back from the list.
Maybe it's out wonderful mail system trying to do something clever.
Base64 itself shouldn't disturb git-am & co.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 02/11] drm/i915: Add SURFLIVE register definitions

[Daniel Vetter]

On Thu, Nov 1, 2012 at 3:16 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, Oct 31, 2012 at 11:57:00PM +0100, Daniel Vetter wrote:
>> On Wed, Oct 31, 2012 at 01:23:05PM -0700, Jesse Barnes wrote:
>> > On Wed, 31 Oct 2012 17:50:15 +0200
>> > ville.syrjala@linux.intel.com wrote:
>> >
>> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > >
>> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Fails to apply here somehow. Also, this thing is base64 encoded, which
>> confused my normal workflow for a bit ... Dunno what exactly caused this
>> havoc.
>
> Strange. I had it sitting on top of drm-intel-next when I sent it.
>
> The whole series seems to be base64 after I got it back from the list.
> Maybe it's out wonderful mail system trying to do something clever.
> Base64 itself shouldn't disturb git-am & co.

Yeah, git am coped fine, but patch was a bit upset. I've then feed it
the decoded text/plain part, which resulted in some strange conflicts.
Dunno what has happened there. Can you simply resend a new patch
rebased on top of latest dinq?

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

Re: [PATCH 02/11] drm/i915: Add SURFLIVE register definitions

[Ville Syrjälä]

On Thu, Nov 01, 2012 at 03:19:34PM +0100, Daniel Vetter wrote:
> On Thu, Nov 1, 2012 at 3:16 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Wed, Oct 31, 2012 at 11:57:00PM +0100, Daniel Vetter wrote:
> >> On Wed, Oct 31, 2012 at 01:23:05PM -0700, Jesse Barnes wrote:
> >> > On Wed, 31 Oct 2012 17:50:15 +0200
> >> > ville.syrjala@linux.intel.com wrote:
> >> >
> >> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > >
> >> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> Fails to apply here somehow. Also, this thing is base64 encoded, which
> >> confused my normal workflow for a bit ... Dunno what exactly caused this
> >> havoc.
> >
> > Strange. I had it sitting on top of drm-intel-next when I sent it.
> >
> > The whole series seems to be base64 after I got it back from the list.
> > Maybe it's out wonderful mail system trying to do something clever.
> > Base64 itself shouldn't disturb git-am & co.
> 
> Yeah, git am coped fine, but patch was a bit upset. I've then feed it
> the decoded text/plain part, which resulted in some strange conflicts.
> Dunno what has happened there. Can you simply resend a new patch
> rebased on top of latest dinq?

Will do.

-- 
Ville Syrjälä
Intel OTC

[PATCH 03/11] drm/i915: Implement execbuffer wait for all planes

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add the MI_WAIT_FOR_EVENT bits for sprites, and fix up the whole thing
for IVB which moved most of the bits around.

Not tested at all!

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   44 +++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_reg.h            |    9 ++++++
 2 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 91d43d5..2b56354 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -625,10 +625,46 @@ i915_gem_execbuffer_wait_for_flips(struct intel_ring_buffer *ring, u32 flips)
 		if (((flips >> plane) & 1) == 0)
 			continue;
 
-		if (plane)
-			flip_mask = MI_WAIT_FOR_PLANE_B_FLIP;
-		else
-			flip_mask = MI_WAIT_FOR_PLANE_A_FLIP;
+		if (IS_GEN7(ring->dev)) {
+			switch (plane) {
+			case 0:
+				flip_mask = MI_WAIT_FOR_PLANE_A_FLIP_IVB;
+				break;
+			case 1:
+				flip_mask = MI_WAIT_FOR_PLANE_B_FLIP_IVB;
+				break;
+			case 2:
+				flip_mask = MI_WAIT_FOR_PLANE_C_FLIP_IVB;
+				break;
+			case 16:
+				flip_mask = MI_WAIT_FOR_SPRITE_A_FLIP_IVB;
+				break;
+			case 17:
+				flip_mask = MI_WAIT_FOR_SPRITE_B_FLIP_IVB;
+				break;
+			case 18:
+				flip_mask = MI_WAIT_FOR_SPRITE_C_FLIP_IVB;
+				break;
+			}
+		} else {
+			switch (plane) {
+			case 0:
+				flip_mask = MI_WAIT_FOR_PLANE_A_FLIP;
+				break;
+			case 1:
+				flip_mask = MI_WAIT_FOR_PLANE_B_FLIP;
+				break;
+			case 2:
+				flip_mask = MI_WAIT_FOR_PLANE_C_FLIP;
+				break;
+			case 16:
+				flip_mask = MI_WAIT_FOR_SPRITE_A_FLIP;
+				break;
+			case 17:
+				flip_mask = MI_WAIT_FOR_SPRITE_B_FLIP;
+				break;
+			}
+		}
 
 		ret = intel_ring_begin(ring, 2);
 		if (ret)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2555986..99894ff 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -189,7 +189,16 @@
 #define MI_NOOP			MI_INSTR(0, 0)
 #define MI_USER_INTERRUPT	MI_INSTR(0x02, 0)
 #define MI_WAIT_FOR_EVENT       MI_INSTR(0x03, 0)
+#define   MI_WAIT_FOR_SPRITE_C_FLIP_IVB	(1<<20)
+#define   MI_WAIT_FOR_PLANE_C_FLIP_IVB	(1<<15)
+#define   MI_WAIT_FOR_SPRITE_B_FLIP_IVB	(1<<10)
+#define   MI_WAIT_FOR_PLANE_B_FLIP_IVB	(1<<9)
+#define   MI_WAIT_FOR_SPRITE_A_FLIP_IVB	(1<<2)
+#define   MI_WAIT_FOR_PLANE_A_FLIP_IVB	(1<<1)
 #define   MI_WAIT_FOR_OVERLAY_FLIP	(1<<16)
+#define   MI_WAIT_FOR_SPRITE_B_FLIP	(1<<16)
+#define   MI_WAIT_FOR_SPRITE_A_FLIP	(1<<8)
+#define   MI_WAIT_FOR_PLANE_C_FLIP	(1<<8)
 #define   MI_WAIT_FOR_PLANE_B_FLIP      (1<<6)
 #define   MI_WAIT_FOR_PLANE_A_FLIP      (1<<2)
 #define   MI_WAIT_FOR_PLANE_A_SCANLINES (1<<1)
-- 
1.7.8.6

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

Re: [PATCH 03/11] drm/i915: Implement execbuffer wait for all planes

[Chris Wilson]

[-- Attachment #1: Type: text/plain, Size: 414 bytes --]

On Wed, 31 Oct 2012 17:50:16 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add the MI_WAIT_FOR_EVENT bits for sprites, and fix up the whole thing
> for IVB which moved most of the bits around.
> 
Please just kill this code. Everyone will be happier and there will be
rejoicing in the streets.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH 04/11] drm/i915: Factor out i9xx_compute_clocks() like ironlake_compute_clocks()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Split the i9xx clock stuff out from i9xx_compute_clocks().

Only compile tested!

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  128 ++++++++++++++++++++--------------
 1 files changed, 75 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6d3feea..b42637b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4557,6 +4557,76 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc,
 		   ((mode->hdisplay - 1) << 16) | (mode->vdisplay - 1));
 }
 
+static bool i9xx_compute_clocks(struct drm_crtc *crtc,
+				struct drm_display_mode *adjusted_mode,
+				intel_clock_t *clock,
+				bool *has_reduced_clock,
+				intel_clock_t *reduced_clock,
+				int *refclk, int *num_connectors, bool *is_dp)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_encoder *encoder;
+	const intel_limit_t *limit;
+	bool ok, is_sdvo = false, is_tv = false, is_lvds = false;
+
+	*num_connectors = 0;
+
+	for_each_encoder_on_crtc(dev, crtc, encoder) {
+		switch (encoder->type) {
+		case INTEL_OUTPUT_LVDS:
+			is_lvds = true;
+			break;
+		case INTEL_OUTPUT_SDVO:
+		case INTEL_OUTPUT_HDMI:
+			is_sdvo = true;
+			if (encoder->needs_tv_clock)
+				is_tv = true;
+			break;
+		case INTEL_OUTPUT_TVOUT:
+			is_tv = true;
+			break;
+		case INTEL_OUTPUT_DISPLAYPORT:
+			*is_dp = true;
+			break;
+		}
+
+		(*num_connectors)++;
+	}
+
+	*refclk = i9xx_get_refclk(crtc, *num_connectors);
+
+	/*
+	 * Returns a set of divisors for the desired target clock with the given
+	 * refclk, or FALSE.  The returned values represent the clock equation:
+	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
+	 */
+	limit = intel_limit(crtc, *refclk);
+	ok = limit->find_pll(limit, crtc, adjusted_mode->clock, *refclk, NULL,
+			      clock);
+	if (!ok)
+		return false;
+
+	if (is_lvds && dev_priv->lvds_downclock_avail) {
+		/*
+		 * Ensure we match the reduced clock's P to the target clock.
+		 * If the clocks don't match, we can't switch the display clock
+		 * by using the FP0/FP1. In such case we will disable the LVDS
+		 * downclock feature.
+		*/
+		*has_reduced_clock = limit->find_pll(limit, crtc,
+						     dev_priv->lvds_downclock,
+						     *refclk,
+						     clock,
+						     reduced_clock);
+	}
+
+	if (is_sdvo && is_tv)
+		i9xx_adjust_sdvo_tv_clock(adjusted_mode, clock);
+
+	return true;
+}
+
 static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 			      struct drm_display_mode *mode,
 			      struct drm_display_mode *adjusted_mode,
@@ -4571,44 +4641,13 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	int refclk, num_connectors = 0;
 	intel_clock_t clock, reduced_clock;
 	u32 dspcntr, pipeconf;
-	bool ok, has_reduced_clock = false, is_sdvo = false;
-	bool is_lvds = false, is_tv = false, is_dp = false;
-	struct intel_encoder *encoder;
-	const intel_limit_t *limit;
+	bool ok, has_reduced_clock = false;
+	bool is_dp = false;
 	int ret;
 
-	for_each_encoder_on_crtc(dev, crtc, encoder) {
-		switch (encoder->type) {
-		case INTEL_OUTPUT_LVDS:
-			is_lvds = true;
-			break;
-		case INTEL_OUTPUT_SDVO:
-		case INTEL_OUTPUT_HDMI:
-			is_sdvo = true;
-			if (encoder->needs_tv_clock)
-				is_tv = true;
-			break;
-		case INTEL_OUTPUT_TVOUT:
-			is_tv = true;
-			break;
-		case INTEL_OUTPUT_DISPLAYPORT:
-			is_dp = true;
-			break;
-		}
-
-		num_connectors++;
-	}
-
-	refclk = i9xx_get_refclk(crtc, num_connectors);
-
-	/*
-	 * Returns a set of divisors for the desired target clock with the given
-	 * refclk, or FALSE.  The returned values represent the clock equation:
-	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
-	 */
-	limit = intel_limit(crtc, refclk);
-	ok = limit->find_pll(limit, crtc, adjusted_mode->clock, refclk, NULL,
-			     &clock);
+	ok = i9xx_compute_clocks(crtc, adjusted_mode, &clock,
+				 &has_reduced_clock, &reduced_clock,
+				 &refclk, &num_connectors, &is_dp);
 	if (!ok) {
 		DRM_ERROR("Couldn't find PLL settings for mode!\n");
 		return -EINVAL;
@@ -4617,23 +4656,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	/* Ensure that the cursor is valid for the new mode before changing... */
 	intel_crtc_update_cursor(crtc, true);
 
-	if (is_lvds && dev_priv->lvds_downclock_avail) {
-		/*
-		 * Ensure we match the reduced clock's P to the target clock.
-		 * If the clocks don't match, we can't switch the display clock
-		 * by using the FP0/FP1. In such case we will disable the LVDS
-		 * downclock feature.
-		*/
-		has_reduced_clock = limit->find_pll(limit, crtc,
-						    dev_priv->lvds_downclock,
-						    refclk,
-						    &clock,
-						    &reduced_clock);
-	}
-
-	if (is_sdvo && is_tv)
-		i9xx_adjust_sdvo_tv_clock(adjusted_mode, &clock);
-
 	if (IS_GEN2(dev))
 		i8xx_update_pll(crtc, adjusted_mode, &clock,
 				has_reduced_clock ? &reduced_clock : NULL,
-- 
1.7.8.6

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

Re: [PATCH 04/11] drm/i915: Factor out i9xx_compute_clocks() like ironlake_compute_clocks()

[Daniel Vetter]

On Wed, Oct 31, 2012 at 05:50:17PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Split the i9xx clock stuff out from i9xx_compute_clocks().
> 
> Only compile tested!
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I'm working on a massive overhaul of the clock computation madness, so
that we do all that stuff before we start to touch the hw (and so would
finally have a reasonable chance at getting global bw issues right).
Current wip pile is at:

http://cgit.freedesktop.org/~danvet/drm/log/?h=modeset-rework

Until we've figured out what's the best approach I prefere if we keep the
churn in this (rather convoluted code) low.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c |  128 ++++++++++++++++++++--------------
>  1 files changed, 75 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6d3feea..b42637b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4557,6 +4557,76 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc,
>  		   ((mode->hdisplay - 1) << 16) | (mode->vdisplay - 1));
>  }
>  
> +static bool i9xx_compute_clocks(struct drm_crtc *crtc,
> +				struct drm_display_mode *adjusted_mode,
> +				intel_clock_t *clock,
> +				bool *has_reduced_clock,
> +				intel_clock_t *reduced_clock,
> +				int *refclk, int *num_connectors, bool *is_dp)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_encoder *encoder;
> +	const intel_limit_t *limit;
> +	bool ok, is_sdvo = false, is_tv = false, is_lvds = false;
> +
> +	*num_connectors = 0;
> +
> +	for_each_encoder_on_crtc(dev, crtc, encoder) {
> +		switch (encoder->type) {
> +		case INTEL_OUTPUT_LVDS:
> +			is_lvds = true;
> +			break;
> +		case INTEL_OUTPUT_SDVO:
> +		case INTEL_OUTPUT_HDMI:
> +			is_sdvo = true;
> +			if (encoder->needs_tv_clock)
> +				is_tv = true;
> +			break;
> +		case INTEL_OUTPUT_TVOUT:
> +			is_tv = true;
> +			break;
> +		case INTEL_OUTPUT_DISPLAYPORT:
> +			*is_dp = true;
> +			break;
> +		}
> +
> +		(*num_connectors)++;
> +	}
> +
> +	*refclk = i9xx_get_refclk(crtc, *num_connectors);
> +
> +	/*
> +	 * Returns a set of divisors for the desired target clock with the given
> +	 * refclk, or FALSE.  The returned values represent the clock equation:
> +	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
> +	 */
> +	limit = intel_limit(crtc, *refclk);
> +	ok = limit->find_pll(limit, crtc, adjusted_mode->clock, *refclk, NULL,
> +			      clock);
> +	if (!ok)
> +		return false;
> +
> +	if (is_lvds && dev_priv->lvds_downclock_avail) {
> +		/*
> +		 * Ensure we match the reduced clock's P to the target clock.
> +		 * If the clocks don't match, we can't switch the display clock
> +		 * by using the FP0/FP1. In such case we will disable the LVDS
> +		 * downclock feature.
> +		*/
> +		*has_reduced_clock = limit->find_pll(limit, crtc,
> +						     dev_priv->lvds_downclock,
> +						     *refclk,
> +						     clock,
> +						     reduced_clock);
> +	}
> +
> +	if (is_sdvo && is_tv)
> +		i9xx_adjust_sdvo_tv_clock(adjusted_mode, clock);
> +
> +	return true;
> +}
> +
>  static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  			      struct drm_display_mode *mode,
>  			      struct drm_display_mode *adjusted_mode,
> @@ -4571,44 +4641,13 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  	int refclk, num_connectors = 0;
>  	intel_clock_t clock, reduced_clock;
>  	u32 dspcntr, pipeconf;
> -	bool ok, has_reduced_clock = false, is_sdvo = false;
> -	bool is_lvds = false, is_tv = false, is_dp = false;
> -	struct intel_encoder *encoder;
> -	const intel_limit_t *limit;
> +	bool ok, has_reduced_clock = false;
> +	bool is_dp = false;
>  	int ret;
>  
> -	for_each_encoder_on_crtc(dev, crtc, encoder) {
> -		switch (encoder->type) {
> -		case INTEL_OUTPUT_LVDS:
> -			is_lvds = true;
> -			break;
> -		case INTEL_OUTPUT_SDVO:
> -		case INTEL_OUTPUT_HDMI:
> -			is_sdvo = true;
> -			if (encoder->needs_tv_clock)
> -				is_tv = true;
> -			break;
> -		case INTEL_OUTPUT_TVOUT:
> -			is_tv = true;
> -			break;
> -		case INTEL_OUTPUT_DISPLAYPORT:
> -			is_dp = true;
> -			break;
> -		}
> -
> -		num_connectors++;
> -	}
> -
> -	refclk = i9xx_get_refclk(crtc, num_connectors);
> -
> -	/*
> -	 * Returns a set of divisors for the desired target clock with the given
> -	 * refclk, or FALSE.  The returned values represent the clock equation:
> -	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
> -	 */
> -	limit = intel_limit(crtc, refclk);
> -	ok = limit->find_pll(limit, crtc, adjusted_mode->clock, refclk, NULL,
> -			     &clock);
> +	ok = i9xx_compute_clocks(crtc, adjusted_mode, &clock,
> +				 &has_reduced_clock, &reduced_clock,
> +				 &refclk, &num_connectors, &is_dp);
>  	if (!ok) {
>  		DRM_ERROR("Couldn't find PLL settings for mode!\n");
>  		return -EINVAL;
> @@ -4617,23 +4656,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  	/* Ensure that the cursor is valid for the new mode before changing... */
>  	intel_crtc_update_cursor(crtc, true);
>  
> -	if (is_lvds && dev_priv->lvds_downclock_avail) {
> -		/*
> -		 * Ensure we match the reduced clock's P to the target clock.
> -		 * If the clocks don't match, we can't switch the display clock
> -		 * by using the FP0/FP1. In such case we will disable the LVDS
> -		 * downclock feature.
> -		*/
> -		has_reduced_clock = limit->find_pll(limit, crtc,
> -						    dev_priv->lvds_downclock,
> -						    refclk,
> -						    &clock,
> -						    &reduced_clock);
> -	}
> -
> -	if (is_sdvo && is_tv)
> -		i9xx_adjust_sdvo_tv_clock(adjusted_mode, &clock);
> -
>  	if (IS_GEN2(dev))
>  		i8xx_update_pll(crtc, adjusted_mode, &clock,
>  				has_reduced_clock ? &reduced_clock : NULL,
> -- 
> 1.7.8.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 04/11] drm/i915: Factor out i9xx_compute_clocks() like ironlake_compute_clocks()

[Ville Syrjälä]

On Wed, Oct 31, 2012 at 05:28:40PM +0100, Daniel Vetter wrote:
> On Wed, Oct 31, 2012 at 05:50:17PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Split the i9xx clock stuff out from i9xx_compute_clocks().
> > 
> > Only compile tested!
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I'm working on a massive overhaul of the clock computation madness, so
> that we do all that stuff before we start to touch the hw (and so would
> finally have a reasonable chance at getting global bw issues right).

I was sure that the compute_clock() funcs already satisfied that.
Perhaps I didn't look hard enough.

The reason for this patch actually was that I'm already using that
approach in the atomic modeset code. Ie. I call compute_clock() for
all modified CRTCs before touching the hardware.

Or is the problem simply that multiple calls to compute_clock() would
depend on some bit of state that is changed somewhere later in
crtc_mode_set()? So that when doing a multi-CRTC modeset, the final
state is never seen by any of the compute_clock() funcs when used this
way?

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 04/11] drm/i915: Factor out i9xx_compute_clocks() like ironlake_compute_clocks()

[Daniel Vetter]

On Wed, Oct 31, 2012 at 07:04:29PM +0200, Ville Syrjälä wrote:
> On Wed, Oct 31, 2012 at 05:28:40PM +0100, Daniel Vetter wrote:
> > On Wed, Oct 31, 2012 at 05:50:17PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Split the i9xx clock stuff out from i9xx_compute_clocks().
> > > 
> > > Only compile tested!
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > I'm working on a massive overhaul of the clock computation madness, so
> > that we do all that stuff before we start to touch the hw (and so would
> > finally have a reasonable chance at getting global bw issues right).
> 
> I was sure that the compute_clock() funcs already satisfied that.
> Perhaps I didn't look hard enough.
> 
> The reason for this patch actually was that I'm already using that
> approach in the atomic modeset code. Ie. I call compute_clock() for
> all modified CRTCs before touching the hardware.
> 
> Or is the problem simply that multiple calls to compute_clock() would
> depend on some bit of state that is changed somewhere later in
> crtc_mode_set()? So that when doing a multi-CRTC modeset, the final
> state is never seen by any of the compute_clock() funcs when used this
> way?

Well the plan is to compute all this state into struct intel_crtc_config
and then switch the code that changes the hw state to only use that
precomputed configuration instead of recomputing it. The reason for that
intermediate state keeping is twofold:
- We need to have some much better notion of a pipe configuration for
  fastboot anyway (including things like pll sharing and exact dotclocks).
  Using the same data structure for both the hw readout code needed for
  fastboot and in our own modeset code should allow for some nice
  consistency checks.
- We have a bunch of rippling dependencies in our state computation, e.g.
  depending upon global configuration we have different amounts of
  bandwidth available, which (should) affect the pipe bpp and so the
  clocks we select, in turn having effects on how we can share plls. If we
  try to run these computations twice, once in the ->check callback and
  once when changing the hw state we'll never get a perfect match. Hence I
  want to precompute and store all values into that pipe_config.

Essentially that branch is trying to implement all the atomic modeset
stuff without actually having an atomic modeset ioctl ;-) My aim is that
in the end I can detect fdi b/c link sharing constraints, dither the other
pipe to a lower pipe_bpp, add it to the array of pipe that need a full
modeset and then enable everything. Also, while I'm at it a want to kill
all these messy layering inversions where the crtc code pokes around in
various encoders ... It's not quite there yet ;-)

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

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

[ville.syrjala]

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:
-- 
1.7.8.6

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

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

[Jesse Barnes]

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>

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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.

-- 
Ville Syrjälä
Intel OTC

[PATCH 06/11] drm/i915: Check the framebuffer offset

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The current code can't deal with framebuffers with an offset. Return an
error when trying to create such a framebuffer until the rest of the
code is fixed to handle them.

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

I had an earlier version that actually added the handling for the offsets,
but as I still haven't managed to write test cases for that, I decided
that just refusing any offset is a good enough solution for now.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f431f2a..a3496f5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8276,6 +8276,10 @@ int intel_framebuffer_init(struct drm_device *dev,
 		return -EINVAL;
 	}
 
+	/* FIXME need to adjust LINOFF/TILEOFF accordingly. */
+	if (mode_cmd->offsets[0] != 0)
+		return -EINVAL;
+
 	ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs);
 	if (ret) {
 		DRM_ERROR("framebuffer init failed %d\n", ret);
-- 
1.7.8.6

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

Re: [PATCH 06/11] drm/i915: Check the framebuffer offset

[Jesse Barnes]

On Wed, 31 Oct 2012 17:50:19 +0200
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The current code can't deal with framebuffers with an offset. Return an
> error when trying to create such a framebuffer until the rest of the
> code is fixed to handle them.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> 
> I had an earlier version that actually added the handling for the offsets,
> but as I still haven't managed to write test cases for that, I decided
> that just refusing any offset is a good enough solution for now.
> 
>  drivers/gpu/drm/i915/intel_display.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f431f2a..a3496f5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8276,6 +8276,10 @@ int intel_framebuffer_init(struct drm_device *dev,
>  		return -EINVAL;
>  	}
>  
> +	/* FIXME need to adjust LINOFF/TILEOFF accordingly. */
> +	if (mode_cmd->offsets[0] != 0)
> +		return -EINVAL;
> +
>  	ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs);
>  	if (ret) {
>  		DRM_ERROR("framebuffer init failed %d\n", ret);

Userspace doesn't use this today at all even in the panning case?  I
know it worked at one point at least, but that may have been back in
the UMS days...

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/11] drm/i915: Check the framebuffer offset

[Ville Syrjälä]

On Wed, Oct 31, 2012 at 01:26:12PM -0700, Jesse Barnes wrote:
> On Wed, 31 Oct 2012 17:50:19 +0200
> ville.syrjala@linux.intel.com wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The current code can't deal with framebuffers with an offset. Return an
> > error when trying to create such a framebuffer until the rest of the
> > code is fixed to handle them.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > 
> > I had an earlier version that actually added the handling for the offsets,
> > but as I still haven't managed to write test cases for that, I decided
> > that just refusing any offset is a good enough solution for now.
> > 
> >  drivers/gpu/drm/i915/intel_display.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index f431f2a..a3496f5 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8276,6 +8276,10 @@ int intel_framebuffer_init(struct drm_device *dev,
> >  		return -EINVAL;
> >  	}
> >  
> > +	/* FIXME need to adjust LINOFF/TILEOFF accordingly. */
> > +	if (mode_cmd->offsets[0] != 0)
> > +		return -EINVAL;
> > +
> >  	ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs);
> >  	if (ret) {
> >  		DRM_ERROR("framebuffer init failed %d\n", ret);
> 
> Userspace doesn't use this today at all even in the panning case?

If it does, then the user is going to be upset when nothing happens.
Only the x/y offsets are effective with the current code.

> I
> know it worked at one point at least, but that may have been back in
> the UMS days...

Before my time.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 06/11] drm/i915: Check the framebuffer offset

[Daniel Vetter]

On Thu, Nov 1, 2012 at 3:09 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>> Userspace doesn't use this today at all even in the panning case?
>
> If it does, then the user is going to be upset when nothing happens.
> Only the x/y offsets are effective with the current code.
>
>> I
>> know it worked at one point at least, but that may have been back in
>> the UMS days...
>
> Before my time.

Afaik the fb->offsets have been added to support planar formats with
all planes smashed into the same buffer object (where the fourcc alone
doesn't specify anything about inter-plane offsets). We don't support
planar buffers, so enforcing an offset of 0 is imo totally ok.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 06/11] drm/i915: Check the framebuffer offset

[Jesse Barnes]

On Thu, 1 Nov 2012 15:18:04 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Thu, Nov 1, 2012 at 3:09 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >> Userspace doesn't use this today at all even in the panning case?
> >
> > If it does, then the user is going to be upset when nothing happens.
> > Only the x/y offsets are effective with the current code.
> >
> >> I
> >> know it worked at one point at least, but that may have been back in
> >> the UMS days...
> >
> > Before my time.
> 
> Afaik the fb->offsets have been added to support planar formats with
> all planes smashed into the same buffer object (where the fourcc alone
> doesn't specify anything about inter-plane offsets). We don't support
> planar buffers, so enforcing an offset of 0 is imo totally ok.

Ok yeah was confusing it with the x/y offsets.

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/11] drm/i915: Check the framebuffer offset

[Ville Syrjälä]

On Thu, Nov 01, 2012 at 03:18:04PM +0100, Daniel Vetter wrote:
> On Thu, Nov 1, 2012 at 3:09 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >> Userspace doesn't use this today at all even in the panning case?
> >
> > If it does, then the user is going to be upset when nothing happens.
> > Only the x/y offsets are effective with the current code.
> >
> >> I
> >> know it worked at one point at least, but that may have been back in
> >> the UMS days...
> >
> > Before my time.
> 
> Afaik the fb->offsets have been added to support planar formats with
> all planes smashed into the same buffer object (where the fourcc alone
> doesn't specify anything about inter-plane offsets). We don't support
> planar buffers, so enforcing an offset of 0 is imo totally ok.

There's also another use case for it. That is, if you specify a source
rectangle for a plane, which is smaller than the full fb, then it's
perfectly legal for the plane to access the pixels outside the source
rectangle to produce good looking filtered edges. But when the source
rectangle edge matches the fb edge, then it's clearly not OK to do
that. So if you want to cut the edges of your source rectangle sharply,
then you can do it through fb->offsets[].

It's similar to texturing w/ GL_CLAMP_TO_EDGE. The texture coordinates
are clamped so that nothing outside the texture is sampled, but
when sampling inside the texture, the filter can blend in texels that
are not inside the area specified by the texture coordinates.

But I supose doing this could be easier if we just added a property
to the plane which specifies how the filtering is done at the edges
of the source rectangle. Then you at least wouldn't need to create
a new fb every time the source rectangle changes.

-- 
Ville Syrjälä
Intel OTC

[PATCH 07/11] drm/i915: pixel_size == cpp

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use drm_format_plane_cpp() to get 'pixel_size' in the sprite code.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c |   19 +++----------------
 1 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index e69035d..a4fe7f5 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -48,7 +48,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	int pipe = intel_plane->pipe;
 	u32 sprctl, sprscale = 0;
-	int pixel_size;
+	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 
 	sprctl = I915_READ(SPRCTL(pipe));
 
@@ -61,32 +61,25 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 	switch (fb->pixel_format) {
 	case DRM_FORMAT_XBGR8888:
 		sprctl |= SPRITE_FORMAT_RGBX888 | SPRITE_RGB_ORDER_RGBX;
-		pixel_size = 4;
 		break;
 	case DRM_FORMAT_XRGB8888:
 		sprctl |= SPRITE_FORMAT_RGBX888;
-		pixel_size = 4;
 		break;
 	case DRM_FORMAT_YUYV:
 		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_YUYV;
-		pixel_size = 2;
 		break;
 	case DRM_FORMAT_YVYU:
 		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_YVYU;
-		pixel_size = 2;
 		break;
 	case DRM_FORMAT_UYVY:
 		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_UYVY;
-		pixel_size = 2;
 		break;
 	case DRM_FORMAT_VYUY:
 		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_VYUY;
-		pixel_size = 2;
 		break;
 	default:
 		DRM_DEBUG_DRIVER("bad pixel format, assuming RGBX888\n");
 		sprctl |= SPRITE_FORMAT_RGBX888;
-		pixel_size = 4;
 		break;
 	}
 
@@ -227,8 +220,9 @@ ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
-	int pipe = intel_plane->pipe, pixel_size;
+	int pipe = intel_plane->pipe;
 	u32 dvscntr, dvsscale;
+	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 
 	dvscntr = I915_READ(DVSCNTR(pipe));
 
@@ -241,32 +235,25 @@ ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 	switch (fb->pixel_format) {
 	case DRM_FORMAT_XBGR8888:
 		dvscntr |= DVS_FORMAT_RGBX888 | DVS_RGB_ORDER_XBGR;
-		pixel_size = 4;
 		break;
 	case DRM_FORMAT_XRGB8888:
 		dvscntr |= DVS_FORMAT_RGBX888;
-		pixel_size = 4;
 		break;
 	case DRM_FORMAT_YUYV:
 		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_YUYV;
-		pixel_size = 2;
 		break;
 	case DRM_FORMAT_YVYU:
 		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_YVYU;
-		pixel_size = 2;
 		break;
 	case DRM_FORMAT_UYVY:
 		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_UYVY;
-		pixel_size = 2;
 		break;
 	case DRM_FORMAT_VYUY:
 		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_VYUY;
-		pixel_size = 2;
 		break;
 	default:
 		DRM_DEBUG_DRIVER("bad pixel format, assuming RGBX888\n");
 		dvscntr |= DVS_FORMAT_RGBX888;
-		pixel_size = 4;
 		break;
 	}
 
-- 
1.7.8.6

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

Re: [PATCH 07/11] drm/i915: pixel_size == cpp

[Jesse Barnes]

On Wed, 31 Oct 2012 17:50:20 +0200
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use drm_format_plane_cpp() to get 'pixel_size' in the sprite code.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sprite.c |   19 +++----------------
>  1 files changed, 3 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index e69035d..a4fe7f5 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -48,7 +48,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
>  	int pipe = intel_plane->pipe;
>  	u32 sprctl, sprscale = 0;
> -	int pixel_size;
> +	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>  
>  	sprctl = I915_READ(SPRCTL(pipe));
>  
> @@ -61,32 +61,25 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>  	switch (fb->pixel_format) {
>  	case DRM_FORMAT_XBGR8888:
>  		sprctl |= SPRITE_FORMAT_RGBX888 | SPRITE_RGB_ORDER_RGBX;
> -		pixel_size = 4;
>  		break;
>  	case DRM_FORMAT_XRGB8888:
>  		sprctl |= SPRITE_FORMAT_RGBX888;
> -		pixel_size = 4;
>  		break;
>  	case DRM_FORMAT_YUYV:
>  		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_YUYV;
> -		pixel_size = 2;
>  		break;
>  	case DRM_FORMAT_YVYU:
>  		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_YVYU;
> -		pixel_size = 2;
>  		break;
>  	case DRM_FORMAT_UYVY:
>  		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_UYVY;
> -		pixel_size = 2;
>  		break;
>  	case DRM_FORMAT_VYUY:
>  		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_VYUY;
> -		pixel_size = 2;
>  		break;
>  	default:
>  		DRM_DEBUG_DRIVER("bad pixel format, assuming RGBX888\n");
>  		sprctl |= SPRITE_FORMAT_RGBX888;
> -		pixel_size = 4;
>  		break;
>  	}
>  
> @@ -227,8 +220,9 @@ ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>  	struct drm_device *dev = plane->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
> -	int pipe = intel_plane->pipe, pixel_size;
> +	int pipe = intel_plane->pipe;
>  	u32 dvscntr, dvsscale;
> +	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>  
>  	dvscntr = I915_READ(DVSCNTR(pipe));
>  
> @@ -241,32 +235,25 @@ ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>  	switch (fb->pixel_format) {
>  	case DRM_FORMAT_XBGR8888:
>  		dvscntr |= DVS_FORMAT_RGBX888 | DVS_RGB_ORDER_XBGR;
> -		pixel_size = 4;
>  		break;
>  	case DRM_FORMAT_XRGB8888:
>  		dvscntr |= DVS_FORMAT_RGBX888;
> -		pixel_size = 4;
>  		break;
>  	case DRM_FORMAT_YUYV:
>  		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_YUYV;
> -		pixel_size = 2;
>  		break;
>  	case DRM_FORMAT_YVYU:
>  		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_YVYU;
> -		pixel_size = 2;
>  		break;
>  	case DRM_FORMAT_UYVY:
>  		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_UYVY;
> -		pixel_size = 2;
>  		break;
>  	case DRM_FORMAT_VYUY:
>  		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_VYUY;
> -		pixel_size = 2;
>  		break;
>  	default:
>  		DRM_DEBUG_DRIVER("bad pixel format, assuming RGBX888\n");
>  		dvscntr |= DVS_FORMAT_RGBX888;
> -		pixel_size = 4;
>  		break;
>  	}
>  

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

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 08/11] drm/i915: Bad pixel formats can't reach the sprite code

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The framebuffer pixel format is already checked by the common code.
So there's no way an invalid format could reach the driver. So instead
of falling back to a default format, call BUG().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index a4fe7f5..7959e75 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -78,9 +78,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_VYUY;
 		break;
 	default:
-		DRM_DEBUG_DRIVER("bad pixel format, assuming RGBX888\n");
-		sprctl |= SPRITE_FORMAT_RGBX888;
-		break;
+		BUG();
 	}
 
 	if (obj->tiling_mode != I915_TILING_NONE)
@@ -252,9 +250,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_VYUY;
 		break;
 	default:
-		DRM_DEBUG_DRIVER("bad pixel format, assuming RGBX888\n");
-		dvscntr |= DVS_FORMAT_RGBX888;
-		break;
+		BUG();
 	}
 
 	if (obj->tiling_mode != I915_TILING_NONE)
-- 
1.7.8.6

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

Re: [PATCH 08/11] drm/i915: Bad pixel formats can't reach the sprite code

[Jesse Barnes]

On Wed, 31 Oct 2012 17:50:21 +0200
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The framebuffer pixel format is already checked by the common code.
> So there's no way an invalid format could reach the driver. So instead
> of falling back to a default format, call BUG().
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sprite.c |    8 ++------
>  1 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index a4fe7f5..7959e75 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -78,9 +78,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>  		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_VYUY;
>  		break;
>  	default:
> -		DRM_DEBUG_DRIVER("bad pixel format, assuming RGBX888\n");
> -		sprctl |= SPRITE_FORMAT_RGBX888;
> -		break;
> +		BUG();
>  	}
>  
>  	if (obj->tiling_mode != I915_TILING_NONE)
> @@ -252,9 +250,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>  		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_VYUY;
>  		break;
>  	default:
> -		DRM_DEBUG_DRIVER("bad pixel format, assuming RGBX888\n");
> -		dvscntr |= DVS_FORMAT_RGBX888;
> -		break;
> +		BUG();
>  	}
>  
>  	if (obj->tiling_mode != I915_TILING_NONE)

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

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 09/11] drm/i915: Consitify adjusted_mode parameter

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

i9xx_adjust_sdvo_tv_clock(), i9xx_compute_clocks() and
ironlake_compute_clocks() do not modify the adjusted_mode passed in, so
pass it as const.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a3496f5..074667a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4123,7 +4123,7 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors)
 	return refclk;
 }
 
-static void i9xx_adjust_sdvo_tv_clock(struct drm_display_mode *adjusted_mode,
+static void i9xx_adjust_sdvo_tv_clock(const struct drm_display_mode *adjusted_mode,
 				      intel_clock_t *clock)
 {
 	/* SDVO TV has fixed PLL values depend on its clock range,
@@ -4558,7 +4558,7 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc,
 }
 
 static bool i9xx_compute_clocks(struct drm_crtc *crtc,
-				struct drm_display_mode *adjusted_mode,
+				const struct drm_display_mode *adjusted_mode,
 				intel_clock_t *clock,
 				bool *has_reduced_clock,
 				intel_clock_t *reduced_clock,
@@ -4975,7 +4975,7 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc,
 }
 
 static bool ironlake_compute_clocks(struct drm_crtc *crtc,
-				    struct drm_display_mode *adjusted_mode,
+				    const struct drm_display_mode *adjusted_mode,
 				    intel_clock_t *clock,
 				    bool *has_reduced_clock,
 				    intel_clock_t *reduced_clock)
-- 
1.7.8.6

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

[PATCH 10/11] drm/i915: Constify mode argument to intel_modeset_adjusted_mode()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

intel_modeset_adjusted_mode() doesn't modify the passed display mode. So
pass it as const.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 074667a..e75156a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7221,7 +7221,7 @@ static void intel_modeset_commit_output_state(struct drm_device *dev)
 
 static struct drm_display_mode *
 intel_modeset_adjusted_mode(struct drm_crtc *crtc,
-			    struct drm_display_mode *mode)
+			    const struct drm_display_mode *mode)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_display_mode *adjusted_mode;
-- 
1.7.8.6

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

[PATCH 11/11] drm/i915: Introduce intel_crtc_update_sarea_pos()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Refactor the code that stores the panning x/y position into the sarea.

This also changes the code so that it won't mistakenly update
sareaB_x/y for pipe >= C.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   43 ++++++++++++++++++++++------------
 1 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e75156a..cbc0035 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2195,13 +2195,39 @@ intel_finish_fb(struct drm_framebuffer *old_fb)
 	return ret;
 }
 
+static void intel_crtc_update_sarea_pos(struct drm_crtc *crtc, int x, int y)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_master_private *master_priv;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	if (!dev->primary->master)
+		return;
+
+	master_priv = dev->primary->master->driver_priv;
+	if (!master_priv->sarea_priv)
+		return;
+
+	switch (intel_crtc->pipe) {
+	case 0:
+		master_priv->sarea_priv->pipeA_x = x;
+		master_priv->sarea_priv->pipeA_y = y;
+		break;
+	case 1:
+		master_priv->sarea_priv->pipeB_x = x;
+		master_priv->sarea_priv->pipeB_y = y;
+		break;
+	default:
+		break;
+	}
+}
+
 static int
 intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 		    struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_i915_master_private *master_priv;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_framebuffer *old_fb;
 	int ret;
@@ -2253,20 +2279,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 	intel_update_fbc(dev);
 	mutex_unlock(&dev->struct_mutex);
 
-	if (!dev->primary->master)
-		return 0;
-
-	master_priv = dev->primary->master->driver_priv;
-	if (!master_priv->sarea_priv)
-		return 0;
-
-	if (intel_crtc->pipe) {
-		master_priv->sarea_priv->pipeB_x = x;
-		master_priv->sarea_priv->pipeB_y = y;
-	} else {
-		master_priv->sarea_priv->pipeA_x = x;
-		master_priv->sarea_priv->pipeA_y = y;
-	}
+	intel_crtc_update_sarea_pos(crtc, x, y);
 
 	return 0;
 }
-- 
1.7.8.6

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

Re: [PATCH 11/11] drm/i915: Introduce intel_crtc_update_sarea_pos()

[Jesse Barnes]

On Wed, 31 Oct 2012 17:50:24 +0200
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Refactor the code that stores the panning x/y position into the sarea.
> 
> This also changes the code so that it won't mistakenly update
> sareaB_x/y for pipe >= C.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   43 ++++++++++++++++++++++------------
>  1 files changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e75156a..cbc0035 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2195,13 +2195,39 @@ intel_finish_fb(struct drm_framebuffer *old_fb)
>  	return ret;
>  }
>  
> +static void intel_crtc_update_sarea_pos(struct drm_crtc *crtc, int x, int y)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_master_private *master_priv;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> +	if (!dev->primary->master)
> +		return;
> +
> +	master_priv = dev->primary->master->driver_priv;
> +	if (!master_priv->sarea_priv)
> +		return;
> +
> +	switch (intel_crtc->pipe) {
> +	case 0:
> +		master_priv->sarea_priv->pipeA_x = x;
> +		master_priv->sarea_priv->pipeA_y = y;
> +		break;
> +	case 1:
> +		master_priv->sarea_priv->pipeB_x = x;
> +		master_priv->sarea_priv->pipeB_y = y;
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
>  static int
>  intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
>  		    struct drm_framebuffer *fb)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct drm_i915_master_private *master_priv;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct drm_framebuffer *old_fb;
>  	int ret;
> @@ -2253,20 +2279,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
>  	intel_update_fbc(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  
> -	if (!dev->primary->master)
> -		return 0;
> -
> -	master_priv = dev->primary->master->driver_priv;
> -	if (!master_priv->sarea_priv)
> -		return 0;
> -
> -	if (intel_crtc->pipe) {
> -		master_priv->sarea_priv->pipeB_x = x;
> -		master_priv->sarea_priv->pipeB_y = y;
> -	} else {
> -		master_priv->sarea_priv->pipeA_x = x;
> -		master_priv->sarea_priv->pipeA_y = y;
> -	}
> +	intel_crtc_update_sarea_pos(crtc, x, y);
>  
>  	return 0;
>  }

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

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 11/11] drm/i915: Introduce intel_crtc_update_sarea_pos()

[Daniel Vetter]

On Wed, Oct 31, 2012 at 01:27:48PM -0700, Jesse Barnes wrote:
> On Wed, 31 Oct 2012 17:50:24 +0200
> ville.syrjala@linux.intel.com wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Refactor the code that stores the panning x/y position into the sarea.
> > 
> > This also changes the code so that it won't mistakenly update
> > sareaB_x/y for pipe >= C.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I've slurped in the patches that Jesse reviewed from this series, safe for
the 2nd patch which somehow fails to apply. To be a good maintainer, I've
fixed up a conflict with the sprite offset adjustment code from Damien
though ;-)

Thanks, Daniel

> > ---
> >  drivers/gpu/drm/i915/intel_display.c |   43 ++++++++++++++++++++++------------
> >  1 files changed, 28 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index e75156a..cbc0035 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2195,13 +2195,39 @@ intel_finish_fb(struct drm_framebuffer *old_fb)
> >  	return ret;
> >  }
> >  
> > +static void intel_crtc_update_sarea_pos(struct drm_crtc *crtc, int x, int y)
> > +{
> > +	struct drm_device *dev = crtc->dev;
> > +	struct drm_i915_master_private *master_priv;
> > +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > +
> > +	if (!dev->primary->master)
> > +		return;
> > +
> > +	master_priv = dev->primary->master->driver_priv;
> > +	if (!master_priv->sarea_priv)
> > +		return;
> > +
> > +	switch (intel_crtc->pipe) {
> > +	case 0:
> > +		master_priv->sarea_priv->pipeA_x = x;
> > +		master_priv->sarea_priv->pipeA_y = y;
> > +		break;
> > +	case 1:
> > +		master_priv->sarea_priv->pipeB_x = x;
> > +		master_priv->sarea_priv->pipeB_y = y;
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +}
> > +
> >  static int
> >  intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
> >  		    struct drm_framebuffer *fb)
> >  {
> >  	struct drm_device *dev = crtc->dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	struct drm_i915_master_private *master_priv;
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >  	struct drm_framebuffer *old_fb;
> >  	int ret;
> > @@ -2253,20 +2279,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
> >  	intel_update_fbc(dev);
> >  	mutex_unlock(&dev->struct_mutex);
> >  
> > -	if (!dev->primary->master)
> > -		return 0;
> > -
> > -	master_priv = dev->primary->master->driver_priv;
> > -	if (!master_priv->sarea_priv)
> > -		return 0;
> > -
> > -	if (intel_crtc->pipe) {
> > -		master_priv->sarea_priv->pipeB_x = x;
> > -		master_priv->sarea_priv->pipeB_y = y;
> > -	} else {
> > -		master_priv->sarea_priv->pipeA_x = x;
> > -		master_priv->sarea_priv->pipeA_y = y;
> > -	}
> > +	intel_crtc_update_sarea_pos(crtc, x, y);
> >  
> >  	return 0;
> >  }
> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> -- 
> Jesse Barnes, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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