[PATCH 00/14] drm/i915: Moar plane update optimizations

[PATCH 00/14] drm/i915: Moar plane update optimizations

[ville.syrjala]

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

The plane updates are still taking far too long, at least with
heavy kernel debug knobs turned on. Using kms_atomic_transitions
I'm currently seeing numbers as high as 170 usec on my VLV.

To combat lockdep the most beneficial thing is taking the
uncore.lock around the entire pipe update. Combined with all
the other optimizations here I was able to push the max
duration below 60 usec with my debug kernel.

The pre-compute stuff isn't supremely important with lockdep/etc.
turned on, but once those are turned off the few usec saved by
the other optimizations start to matter. With all the optimization
and a less debug heavy kernel I was able to get the max duration
to around 15 usec. It was around 25 usec with the current code.

Mika was saying that he's still seeing huge numbers (as high
as 400 usec) with the current drm-tip, and that wasn't even with
a particularly debug heavy kernel. One theory is that there's
contention on the uncore.lock. So I'm thinking we may want to
split the lock into two; one for gt, the other for display. Not
starving the GPU by hogging the shared lock for display stuff
might also be a good thing. I've not tried playing with more
GPU heavy scenarios yet

Anyways, running out of time to play with this more today so
I figured I'd post what I have now.

Series available here:
git://github.com/vsyrjala/linux.git plane_update_optimization

Ville Syrjälä (14):
  drm/i915: Extract skl_plane_ctl()
  drm/i915: Use skl_plane_ctl() for the SKL "sprite" planes
  drm/i915: Extract i9xx_plane_ctl()
  drm/i915: Extract vlv_sprite_ctl()
  drm/i915: Extract ivb_sprite_ctl()
  drm/i915: Extract ilk_sprite_ctl()
  drm/i915: Extract i845_cursor_ctl() and i9xx_cursor_ctl()
  drm/i915: Pre-compute plane control register value
  drm/i915: Introduce i9xx_check_plane_surface()
  drm/i915: Eliminate ironlake_update_primary_plane()
  drm/i915: Use i9xx_check_plane_surface() for sprite planes as well
  drm/i915: Keep vblanks enabled during the entire pipe update
  WIP/drm/i915: Protect the entire pipe update with uncore.lock
  WIP/drm/i915: Nuke posting reads from plane updates

 drivers/gpu/drm/i915/i915_irq.c      |  66 ++++--
 drivers/gpu/drm/i915/i915_trace.h    |  28 +--
 drivers/gpu/drm/i915/intel_display.c | 436 +++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h     |  16 +-
 drivers/gpu/drm/i915/intel_pm.c      |  11 +-
 drivers/gpu/drm/i915/intel_sprite.c  | 355 ++++++++++++----------------
 6 files changed, 440 insertions(+), 472 deletions(-)

-- 
2.10.2

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

[PATCH 01/14] drm/i915: Extract skl_plane_ctl()

[ville.syrjala]

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

Pull the code to calculate the SKL plane control register value into
a separate function. Allows us to pre-compute it in the future.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 010e5ddb198a..6ddf5c90d15c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3335,6 +3335,31 @@ u32 skl_plane_ctl_rotation(unsigned int rotation)
 	return 0;
 }
 
+static u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
+			 const struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	unsigned int rotation = plane_state->base.rotation;
+	u32 plane_ctl;
+
+	plane_ctl = PLANE_CTL_ENABLE;
+
+	if (!IS_GEMINILAKE(dev_priv)) {
+		plane_ctl |=
+			PLANE_CTL_PIPE_GAMMA_ENABLE |
+			PLANE_CTL_PIPE_CSC_ENABLE |
+			PLANE_CTL_PLANE_GAMMA_DISABLE;
+	}
+
+	plane_ctl |= skl_plane_ctl_format(fb->format->format);
+	plane_ctl |= skl_plane_ctl_tiling(fb->modifier);
+	plane_ctl |= skl_plane_ctl_rotation(rotation);
+
+	return plane_ctl;
+}
+
 static void skylake_update_primary_plane(struct drm_plane *plane,
 					 const struct intel_crtc_state *crtc_state,
 					 const struct intel_plane_state *plane_state)
@@ -3360,18 +3385,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	int dst_h = drm_rect_height(&plane_state->base.dst);
 	unsigned long irqflags;
 
-	plane_ctl = PLANE_CTL_ENABLE;
-
-	if (!IS_GEMINILAKE(dev_priv)) {
-		plane_ctl |=
-			PLANE_CTL_PIPE_GAMMA_ENABLE |
-			PLANE_CTL_PIPE_CSC_ENABLE |
-			PLANE_CTL_PLANE_GAMMA_DISABLE;
-	}
-
-	plane_ctl |= skl_plane_ctl_format(fb->format->format);
-	plane_ctl |= skl_plane_ctl_tiling(fb->modifier);
-	plane_ctl |= skl_plane_ctl_rotation(rotation);
+	plane_ctl = skl_plane_ctl(crtc_state, plane_state);
 
 	/* Sizes are 0 based */
 	src_w--;
-- 
2.10.2

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

[PATCH 02/14] drm/i915: Use skl_plane_ctl() for the SKL "sprite" planes

[ville.syrjala]

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

On SKL the planes are uniform so the "sprites" can use the
primary plane code perfectly fine. The only difference we
have is the color key handling, but since we never enable that
for the primary plane the same code works just fine.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6ddf5c90d15c..8a9f1bd21e0e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3254,7 +3254,7 @@ u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
 	return stride;
 }
 
-u32 skl_plane_ctl_format(uint32_t pixel_format)
+static u32 skl_plane_ctl_format(uint32_t pixel_format)
 {
 	switch (pixel_format) {
 	case DRM_FORMAT_C8:
@@ -3295,7 +3295,7 @@ u32 skl_plane_ctl_format(uint32_t pixel_format)
 	return 0;
 }
 
-u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
+static u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
 {
 	switch (fb_modifier) {
 	case DRM_FORMAT_MOD_NONE:
@@ -3313,7 +3313,7 @@ u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
 	return 0;
 }
 
-u32 skl_plane_ctl_rotation(unsigned int rotation)
+static u32 skl_plane_ctl_rotation(unsigned int rotation)
 {
 	switch (rotation) {
 	case DRM_ROTATE_0:
@@ -3335,13 +3335,14 @@ u32 skl_plane_ctl_rotation(unsigned int rotation)
 	return 0;
 }
 
-static u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
-			 const struct intel_plane_state *plane_state)
+u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
+		  const struct intel_plane_state *plane_state)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(plane_state->base.plane->dev);
 	const struct drm_framebuffer *fb = plane_state->base.fb;
 	unsigned int rotation = plane_state->base.rotation;
+	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	u32 plane_ctl;
 
 	plane_ctl = PLANE_CTL_ENABLE;
@@ -3357,6 +3358,11 @@ static u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
 	plane_ctl |= skl_plane_ctl_tiling(fb->modifier);
 	plane_ctl |= skl_plane_ctl_rotation(rotation);
 
+	if (key->flags & I915_SET_COLORKEY_DESTINATION)
+		plane_ctl |= PLANE_CTL_KEY_ENABLE_DESTINATION;
+	else if (key->flags & I915_SET_COLORKEY_SOURCE)
+		plane_ctl |= PLANE_CTL_KEY_ENABLE_SOURCE;
+
 	return plane_ctl;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 51228fe4283b..45e55b45e772 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1445,9 +1445,8 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
 	return i915_ggtt_offset(state->vma);
 }
 
-u32 skl_plane_ctl_format(uint32_t pixel_format);
-u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
-u32 skl_plane_ctl_rotation(unsigned int rotation);
+u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
+		  const struct intel_plane_state *plane_state);
 u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
 		     unsigned int rotation);
 int skl_check_plane_surface(struct intel_plane_state *plane_state);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index b931d0bd7a64..455a9ed44204 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -232,23 +232,7 @@ skl_update_plane(struct drm_plane *drm_plane,
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
 	unsigned long irqflags;
 
-	plane_ctl = PLANE_CTL_ENABLE;
-
-	if (!IS_GEMINILAKE(dev_priv)) {
-		plane_ctl |=
-			PLANE_CTL_PIPE_GAMMA_ENABLE |
-			PLANE_CTL_PIPE_CSC_ENABLE |
-			PLANE_CTL_PLANE_GAMMA_DISABLE;
-	}
-
-	plane_ctl |= skl_plane_ctl_format(fb->format->format);
-	plane_ctl |= skl_plane_ctl_tiling(fb->modifier);
-	plane_ctl |= skl_plane_ctl_rotation(rotation);
-
-	if (key->flags & I915_SET_COLORKEY_DESTINATION)
-		plane_ctl |= PLANE_CTL_KEY_ENABLE_DESTINATION;
-	else if (key->flags & I915_SET_COLORKEY_SOURCE)
-		plane_ctl |= PLANE_CTL_KEY_ENABLE_SOURCE;
+	plane_ctl = skl_plane_ctl(crtc_state, plane_state);
 
 	/* Sizes are 0 based */
 	src_w--;
-- 
2.10.2

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

[PATCH 03/14] drm/i915: Extract i9xx_plane_ctl()

[ville.syrjala]

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

Pull the code to calculate the pre-SKL primary plane control register
value into a separate function. Allows us to pre-compute it in the
future.

We can also share the code between the i9xx and ilk codepaths as the
differences are minimal. Actually there are no differences between
g4x and ilk, so the current split doesn't really make any sense.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8a9f1bd21e0e..99b72c4219a8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2962,28 +2962,27 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
 	return 0;
 }
 
-static void i9xx_update_primary_plane(struct drm_plane *primary,
-				      const struct intel_crtc_state *crtc_state,
-				      const struct intel_plane_state *plane_state)
+static u32 i9xx_plane_ctl(const struct intel_crtc_state *crtc_state,
+			  const struct intel_plane_state *plane_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(primary->dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_framebuffer *fb = plane_state->base.fb;
-	int plane = intel_crtc->plane;
-	u32 linear_offset;
-	u32 dspcntr;
-	i915_reg_t reg = DSPCNTR(plane);
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
 	unsigned int rotation = plane_state->base.rotation;
-	int x = plane_state->base.src.x1 >> 16;
-	int y = plane_state->base.src.y1 >> 16;
-	unsigned long irqflags;
+	u32 dspcntr;
 
-	dspcntr = DISPPLANE_GAMMA_ENABLE;
+	dspcntr = DISPLAY_PLANE_ENABLE | DISPPLANE_GAMMA_ENABLE;
 
-	dspcntr |= DISPLAY_PLANE_ENABLE;
+	if (IS_G4X(dev_priv) || IS_GEN5(dev_priv) ||
+	    IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv))
+		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
+
+	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+		dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
 
 	if (INTEL_GEN(dev_priv) < 4) {
-		if (intel_crtc->pipe == PIPE_B)
+		if (crtc->pipe == PIPE_B)
 			dspcntr |= DISPPLANE_SEL_PIPE_B;
 	}
 
@@ -3010,7 +3009,8 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 		dspcntr |= DISPPLANE_RGBX101010;
 		break;
 	default:
-		BUG();
+		MISSING_CASE(fb->format->format);
+		return 0;
 	}
 
 	if (INTEL_GEN(dev_priv) >= 4 &&
@@ -3023,8 +3023,26 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	if (rotation & DRM_REFLECT_X)
 		dspcntr |= DISPPLANE_MIRROR;
 
-	if (IS_G4X(dev_priv))
-		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
+	return dspcntr;
+}
+
+static void i9xx_update_primary_plane(struct drm_plane *primary,
+				      const struct intel_crtc_state *crtc_state,
+				      const struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(primary->dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_framebuffer *fb = plane_state->base.fb;
+	int plane = intel_crtc->plane;
+	u32 linear_offset;
+	u32 dspcntr;
+	i915_reg_t reg = DSPCNTR(plane);
+	unsigned int rotation = plane_state->base.rotation;
+	int x = plane_state->base.src.x1 >> 16;
+	int y = plane_state->base.src.y1 >> 16;
+	unsigned long irqflags;
+
+	dspcntr = i9xx_plane_ctl(crtc_state, plane_state);
 
 	intel_add_fb_offsets(&x, &y, plane_state, 0);
 
@@ -3122,43 +3140,7 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
 	int y = plane_state->base.src.y1 >> 16;
 	unsigned long irqflags;
 
-	dspcntr = DISPPLANE_GAMMA_ENABLE;
-	dspcntr |= DISPLAY_PLANE_ENABLE;
-
-	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
-
-	switch (fb->format->format) {
-	case DRM_FORMAT_C8:
-		dspcntr |= DISPPLANE_8BPP;
-		break;
-	case DRM_FORMAT_RGB565:
-		dspcntr |= DISPPLANE_BGRX565;
-		break;
-	case DRM_FORMAT_XRGB8888:
-		dspcntr |= DISPPLANE_BGRX888;
-		break;
-	case DRM_FORMAT_XBGR8888:
-		dspcntr |= DISPPLANE_RGBX888;
-		break;
-	case DRM_FORMAT_XRGB2101010:
-		dspcntr |= DISPPLANE_BGRX101010;
-		break;
-	case DRM_FORMAT_XBGR2101010:
-		dspcntr |= DISPPLANE_RGBX101010;
-		break;
-	default:
-		BUG();
-	}
-
-	if (fb->modifier == I915_FORMAT_MOD_X_TILED)
-		dspcntr |= DISPPLANE_TILED;
-
-	if (rotation & DRM_ROTATE_180)
-		dspcntr |= DISPPLANE_ROTATE_180;
-
-	if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv))
-		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
+	dspcntr = i9xx_plane_ctl(crtc_state, plane_state);
 
 	intel_add_fb_offsets(&x, &y, plane_state, 0);
 
-- 
2.10.2

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

[PATCH 04/14] drm/i915: Extract vlv_sprite_ctl()

[ville.syrjala]

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

Pull the code to calculate the VLV/CHV sprite control register value
into a separate function. Allows us to pre-compute it in the future.

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

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 455a9ed44204..ed4db9276c59 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -345,32 +345,15 @@ chv_update_csc(struct intel_plane *intel_plane, uint32_t format)
 	I915_WRITE_FW(SPCSCCROCLAMP(plane_id), SPCSC_OMAX(1023) | SPCSC_OMIN(0));
 }
 
-static void
-vlv_update_plane(struct drm_plane *dplane,
-		 const struct intel_crtc_state *crtc_state,
-		 const struct intel_plane_state *plane_state)
+static u32 vlv_sprite_ctl(const struct intel_crtc_state *crtc_state,
+			  const struct intel_plane_state *plane_state)
 {
-	struct drm_device *dev = dplane->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_plane *intel_plane = to_intel_plane(dplane);
-	struct drm_framebuffer *fb = plane_state->base.fb;
-	enum pipe pipe = intel_plane->pipe;
-	enum plane_id plane_id = intel_plane->id;
-	u32 sprctl;
-	u32 sprsurf_offset, linear_offset;
+	const struct drm_framebuffer *fb = plane_state->base.fb;
 	unsigned int rotation = plane_state->base.rotation;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
-	int crtc_x = plane_state->base.dst.x1;
-	int crtc_y = plane_state->base.dst.y1;
-	uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
-	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
-	uint32_t x = plane_state->base.src.x1 >> 16;
-	uint32_t y = plane_state->base.src.y1 >> 16;
-	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
-	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
-	unsigned long irqflags;
+	u32 sprctl;
 
-	sprctl = SP_ENABLE;
+	sprctl = SP_ENABLE | SP_GAMMA_ENABLE;
 
 	switch (fb->format->format) {
 	case DRM_FORMAT_YUYV:
@@ -407,20 +390,10 @@ vlv_update_plane(struct drm_plane *dplane,
 		sprctl |= SP_FORMAT_RGBA8888;
 		break;
 	default:
-		/*
-		 * If we get here one of the upper layers failed to filter
-		 * out the unsupported plane formats
-		 */
-		BUG();
-		break;
+		MISSING_CASE(fb->format->format);
+		return 0;
 	}
 
-	/*
-	 * Enable gamma to match primary/cursor plane behaviour.
-	 * FIXME should be user controllable via propertiesa.
-	 */
-	sprctl |= SP_GAMMA_ENABLE;
-
 	if (fb->modifier == I915_FORMAT_MOD_X_TILED)
 		sprctl |= SP_TILED;
 
@@ -433,6 +406,36 @@ vlv_update_plane(struct drm_plane *dplane,
 	if (key->flags & I915_SET_COLORKEY_SOURCE)
 		sprctl |= SP_SOURCE_KEY;
 
+	return sprctl;
+}
+
+static void
+vlv_update_plane(struct drm_plane *dplane,
+		 const struct intel_crtc_state *crtc_state,
+		 const struct intel_plane_state *plane_state)
+{
+	struct drm_device *dev = dplane->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_plane *intel_plane = to_intel_plane(dplane);
+	struct drm_framebuffer *fb = plane_state->base.fb;
+	enum pipe pipe = intel_plane->pipe;
+	enum plane_id plane_id = intel_plane->id;
+	u32 sprctl;
+	u32 sprsurf_offset, linear_offset;
+	unsigned int rotation = plane_state->base.rotation;
+	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
+	int crtc_x = plane_state->base.dst.x1;
+	int crtc_y = plane_state->base.dst.y1;
+	uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
+	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
+	uint32_t x = plane_state->base.src.x1 >> 16;
+	uint32_t y = plane_state->base.src.y1 >> 16;
+	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
+	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
+	unsigned long irqflags;
+
+	sprctl = vlv_sprite_ctl(crtc_state, plane_state);
+
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
-- 
2.10.2

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

[PATCH 05/14] drm/i915: Extract ivb_sprite_ctl()

[ville.syrjala]

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

Pull the code to calculate the IVB-BDW sprite control register value
into a separate function. Allows us to pre-compute it in the future.

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

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index ed4db9276c59..a3a847a3b39f 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -503,31 +503,23 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-static void
-ivb_update_plane(struct drm_plane *plane,
-		 const struct intel_crtc_state *crtc_state,
-		 const struct intel_plane_state *plane_state)
+static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
+			  const struct intel_plane_state *plane_state)
 {
-	struct drm_device *dev = plane->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_plane *intel_plane = to_intel_plane(plane);
-	struct drm_framebuffer *fb = plane_state->base.fb;
-	enum pipe pipe = intel_plane->pipe;
-	u32 sprctl, sprscale = 0;
-	u32 sprsurf_offset, linear_offset;
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
 	unsigned int rotation = plane_state->base.rotation;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
-	int crtc_x = plane_state->base.dst.x1;
-	int crtc_y = plane_state->base.dst.y1;
-	uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
-	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
-	uint32_t x = plane_state->base.src.x1 >> 16;
-	uint32_t y = plane_state->base.src.y1 >> 16;
-	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
-	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
-	unsigned long irqflags;
+	u32 sprctl;
 
-	sprctl = SPRITE_ENABLE;
+	sprctl = SPRITE_ENABLE | SPRITE_GAMMA_ENABLE;
+
+	if (IS_IVYBRIDGE(dev_priv))
+		sprctl |= SPRITE_TRICKLE_FEED_DISABLE;
+
+	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+		sprctl |= SPRITE_PIPE_CSC_ENABLE;
 
 	switch (fb->format->format) {
 	case DRM_FORMAT_XBGR8888:
@@ -549,34 +541,50 @@ ivb_update_plane(struct drm_plane *plane,
 		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_VYUY;
 		break;
 	default:
-		BUG();
+		MISSING_CASE(fb->format->format);
+		return 0;
 	}
 
-	/*
-	 * Enable gamma to match primary/cursor plane behaviour.
-	 * FIXME should be user controllable via propertiesa.
-	 */
-	sprctl |= SPRITE_GAMMA_ENABLE;
-
 	if (fb->modifier == I915_FORMAT_MOD_X_TILED)
 		sprctl |= SPRITE_TILED;
 
 	if (rotation & DRM_ROTATE_180)
 		sprctl |= SPRITE_ROTATE_180;
 
-	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		sprctl &= ~SPRITE_TRICKLE_FEED_DISABLE;
-	else
-		sprctl |= SPRITE_TRICKLE_FEED_DISABLE;
-
-	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		sprctl |= SPRITE_PIPE_CSC_ENABLE;
-
 	if (key->flags & I915_SET_COLORKEY_DESTINATION)
 		sprctl |= SPRITE_DEST_KEY;
 	else if (key->flags & I915_SET_COLORKEY_SOURCE)
 		sprctl |= SPRITE_SOURCE_KEY;
 
+	return sprctl;
+}
+
+static void
+ivb_update_plane(struct drm_plane *plane,
+		 const struct intel_crtc_state *crtc_state,
+		 const struct intel_plane_state *plane_state)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct drm_framebuffer *fb = plane_state->base.fb;
+	enum pipe pipe = intel_plane->pipe;
+	u32 sprctl, sprscale = 0;
+	u32 sprsurf_offset, linear_offset;
+	unsigned int rotation = plane_state->base.rotation;
+	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
+	int crtc_x = plane_state->base.dst.x1;
+	int crtc_y = plane_state->base.dst.y1;
+	uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
+	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
+	uint32_t x = plane_state->base.src.x1 >> 16;
+	uint32_t y = plane_state->base.src.y1 >> 16;
+	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
+	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
+	unsigned long irqflags;
+
+	sprctl = ivb_sprite_ctl(crtc_state, plane_state);
+
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
-- 
2.10.2

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

[PATCH 06/14] drm/i915: Extract ilk_sprite_ctl()

[ville.syrjala]

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

Pull the code to calculate the ILK-SNB sprite control register value
into a separate function. Allows us to pre-compute it in the future.

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

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index a3a847a3b39f..12e33bc149e4 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -659,31 +659,20 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-static void
-ilk_update_plane(struct drm_plane *plane,
-		 const struct intel_crtc_state *crtc_state,
-		 const struct intel_plane_state *plane_state)
+static u32 ilk_sprite_ctl(const struct intel_crtc_state *crtc_state,
+			  const struct intel_plane_state *plane_state)
 {
-	struct drm_device *dev = plane->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_plane *intel_plane = to_intel_plane(plane);
-	struct drm_framebuffer *fb = plane_state->base.fb;
-	int pipe = intel_plane->pipe;
-	u32 dvscntr, dvsscale;
-	u32 dvssurf_offset, linear_offset;
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
 	unsigned int rotation = plane_state->base.rotation;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
-	int crtc_x = plane_state->base.dst.x1;
-	int crtc_y = plane_state->base.dst.y1;
-	uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
-	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
-	uint32_t x = plane_state->base.src.x1 >> 16;
-	uint32_t y = plane_state->base.src.y1 >> 16;
-	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
-	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
-	unsigned long irqflags;
+	u32 dvscntr;
+
+	dvscntr = DVS_ENABLE | DVS_GAMMA_ENABLE;
 
-	dvscntr = DVS_ENABLE;
+	if (IS_GEN6(dev_priv))
+		dvscntr |= DVS_TRICKLE_FEED_DISABLE;
 
 	switch (fb->format->format) {
 	case DRM_FORMAT_XBGR8888:
@@ -705,29 +694,50 @@ ilk_update_plane(struct drm_plane *plane,
 		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_VYUY;
 		break;
 	default:
-		BUG();
+		MISSING_CASE(fb->format->format);
+		return 0;
 	}
 
-	/*
-	 * Enable gamma to match primary/cursor plane behaviour.
-	 * FIXME should be user controllable via propertiesa.
-	 */
-	dvscntr |= DVS_GAMMA_ENABLE;
-
 	if (fb->modifier == I915_FORMAT_MOD_X_TILED)
 		dvscntr |= DVS_TILED;
 
 	if (rotation & DRM_ROTATE_180)
 		dvscntr |= DVS_ROTATE_180;
 
-	if (IS_GEN6(dev_priv))
-		dvscntr |= DVS_TRICKLE_FEED_DISABLE; /* must disable */
-
 	if (key->flags & I915_SET_COLORKEY_DESTINATION)
 		dvscntr |= DVS_DEST_KEY;
 	else if (key->flags & I915_SET_COLORKEY_SOURCE)
 		dvscntr |= DVS_SOURCE_KEY;
 
+	return dvscntr;
+}
+
+static void
+ilk_update_plane(struct drm_plane *plane,
+		 const struct intel_crtc_state *crtc_state,
+		 const struct intel_plane_state *plane_state)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct drm_framebuffer *fb = plane_state->base.fb;
+	int pipe = intel_plane->pipe;
+	u32 dvscntr, dvsscale;
+	u32 dvssurf_offset, linear_offset;
+	unsigned int rotation = plane_state->base.rotation;
+	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
+	int crtc_x = plane_state->base.dst.x1;
+	int crtc_y = plane_state->base.dst.y1;
+	uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
+	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
+	uint32_t x = plane_state->base.src.x1 >> 16;
+	uint32_t y = plane_state->base.src.y1 >> 16;
+	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
+	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
+	unsigned long irqflags;
+
+	dvscntr = ilk_sprite_ctl(crtc_state, plane_state);
+
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
-- 
2.10.2

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

[PATCH 07/14] drm/i915: Extract i845_cursor_ctl() and i9xx_cursor_ctl()

[ville.syrjala]

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

Pull the code to calculate the cursor control register value into
separate functions. Allows us to pre-compute them in the future.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 99b72c4219a8..040978e704f3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9161,7 +9161,33 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	return active;
 }
 
+static u32 i845_cursor_ctl(const struct intel_crtc_state *crtc_state,
+			   const struct intel_plane_state *plane_state)
+{
+	unsigned int width = plane_state->base.crtc_w;
+	unsigned int stride = roundup_pow_of_two(width) * 4;
+
+	switch (stride) {
+	default:
+		WARN_ONCE(1, "Invalid cursor width/stride, width=%u, stride=%u\n",
+			  width, stride);
+		stride = 256;
+		/* fallthrough */
+	case 256:
+	case 512:
+	case 1024:
+	case 2048:
+		break;
+	}
+
+	return CURSOR_ENABLE |
+		CURSOR_GAMMA_ENABLE |
+		CURSOR_FORMAT_ARGB |
+		CURSOR_STRIDE(stride);
+}
+
 static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
+			       const struct intel_crtc_state *crtc_state,
 			       const struct intel_plane_state *plane_state)
 {
 	struct drm_device *dev = crtc->dev;
@@ -9172,26 +9198,8 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
 	if (plane_state && plane_state->base.visible) {
 		unsigned int width = plane_state->base.crtc_w;
 		unsigned int height = plane_state->base.crtc_h;
-		unsigned int stride = roundup_pow_of_two(width) * 4;
-
-		switch (stride) {
-		default:
-			WARN_ONCE(1, "Invalid cursor width/stride, width=%u, stride=%u\n",
-				  width, stride);
-			stride = 256;
-			/* fallthrough */
-		case 256:
-		case 512:
-		case 1024:
-		case 2048:
-			break;
-		}
-
-		cntl |= CURSOR_ENABLE |
-			CURSOR_GAMMA_ENABLE |
-			CURSOR_FORMAT_ARGB |
-			CURSOR_STRIDE(stride);
 
+		cntl = i845_cursor_ctl(crtc_state, plane_state);
 		size = (height << 12) | width;
 	}
 
@@ -9224,7 +9232,45 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
 	}
 }
 
+static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
+			   const struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	enum pipe pipe = crtc->pipe;
+	uint32_t cntl = 0;
+
+	cntl = MCURSOR_GAMMA_ENABLE;
+
+	if (HAS_DDI(dev_priv))
+		cntl |= CURSOR_PIPE_CSC_ENABLE;
+
+	cntl |= pipe << 28; /* Connect to correct pipe */
+
+	switch (plane_state->base.crtc_w) {
+	case 64:
+		cntl |= CURSOR_MODE_64_ARGB_AX;
+		break;
+	case 128:
+		cntl |= CURSOR_MODE_128_ARGB_AX;
+		break;
+	case 256:
+		cntl |= CURSOR_MODE_256_ARGB_AX;
+		break;
+	default:
+		MISSING_CASE(plane_state->base.crtc_w);
+		return 0;
+	}
+
+	if (plane_state->base.rotation & DRM_ROTATE_180)
+		cntl |= CURSOR_ROTATE_180;
+
+	return cntl;
+}
+
 static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
+			       const struct intel_crtc_state *crtc_state,
 			       const struct intel_plane_state *plane_state)
 {
 	struct drm_device *dev = crtc->dev;
@@ -9233,30 +9279,8 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
 	int pipe = intel_crtc->pipe;
 	uint32_t cntl = 0;
 
-	if (plane_state && plane_state->base.visible) {
-		cntl = MCURSOR_GAMMA_ENABLE;
-		switch (plane_state->base.crtc_w) {
-			case 64:
-				cntl |= CURSOR_MODE_64_ARGB_AX;
-				break;
-			case 128:
-				cntl |= CURSOR_MODE_128_ARGB_AX;
-				break;
-			case 256:
-				cntl |= CURSOR_MODE_256_ARGB_AX;
-				break;
-			default:
-				MISSING_CASE(plane_state->base.crtc_w);
-				return;
-		}
-		cntl |= pipe << 28; /* Connect to correct pipe */
-
-		if (HAS_DDI(dev_priv))
-			cntl |= CURSOR_PIPE_CSC_ENABLE;
-
-		if (plane_state->base.rotation & DRM_ROTATE_180)
-			cntl |= CURSOR_ROTATE_180;
-	}
+	if (plane_state && plane_state->base.visible)
+		cntl = i9xx_cursor_ctl(crtc_state, plane_state);
 
 	if (intel_crtc->cursor_cntl != cntl) {
 		I915_WRITE_FW(CURCNTR(pipe), cntl);
@@ -9273,6 +9297,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
 
 /* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
 static void intel_crtc_update_cursor(struct drm_crtc *crtc,
+				     const struct intel_crtc_state *crtc_state,
 				     const struct intel_plane_state *plane_state)
 {
 	struct drm_device *dev = crtc->dev;
@@ -9312,9 +9337,9 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 	I915_WRITE_FW(CURPOS(pipe), pos);
 
 	if (IS_I845G(dev_priv) || IS_I865G(dev_priv))
-		i845_update_cursor(crtc, base, plane_state);
+		i845_update_cursor(crtc, base, crtc_state, plane_state);
 	else
-		i9xx_update_cursor(crtc, base, plane_state);
+		i9xx_update_cursor(crtc, base, crtc_state, plane_state);
 
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
@@ -13744,7 +13769,7 @@ intel_disable_cursor_plane(struct drm_plane *plane,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
 	intel_crtc->cursor_addr = 0;
-	intel_crtc_update_cursor(crtc, NULL);
+	intel_crtc_update_cursor(crtc, NULL, NULL);
 }
 
 static void
@@ -13766,7 +13791,7 @@ intel_update_cursor_plane(struct drm_plane *plane,
 		addr = obj->phys_handle->busaddr;
 
 	intel_crtc->cursor_addr = addr;
-	intel_crtc_update_cursor(crtc, state);
+	intel_crtc_update_cursor(crtc, crtc_state, state);
 }
 
 static struct intel_plane *
-- 
2.10.2

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

[PATCH 08/14] drm/i915: Pre-compute plane control register value

[ville.syrjala]

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

Computing the plane control register value is branchy so moving it out
from the plane commit hook seems prudent. Let's pre-compute it during
the atomic check phase and store the result in the plane state.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h     |  3 +++
 drivers/gpu/drm/i915/intel_sprite.c  | 24 ++++++++++-----------
 3 files changed, 36 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 040978e704f3..2e0106a11f8f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3035,15 +3035,13 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	struct drm_framebuffer *fb = plane_state->base.fb;
 	int plane = intel_crtc->plane;
 	u32 linear_offset;
-	u32 dspcntr;
+	u32 dspcntr = plane_state->ctl;
 	i915_reg_t reg = DSPCNTR(plane);
 	unsigned int rotation = plane_state->base.rotation;
 	int x = plane_state->base.src.x1 >> 16;
 	int y = plane_state->base.src.y1 >> 16;
 	unsigned long irqflags;
 
-	dspcntr = i9xx_plane_ctl(crtc_state, plane_state);
-
 	intel_add_fb_offsets(&x, &y, plane_state, 0);
 
 	if (INTEL_GEN(dev_priv) >= 4)
@@ -3133,15 +3131,13 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
 	struct drm_framebuffer *fb = plane_state->base.fb;
 	int plane = intel_crtc->plane;
 	u32 linear_offset;
-	u32 dspcntr;
+	u32 dspcntr = plane_state->ctl;
 	i915_reg_t reg = DSPCNTR(plane);
 	unsigned int rotation = plane_state->base.rotation;
 	int x = plane_state->base.src.x1 >> 16;
 	int y = plane_state->base.src.y1 >> 16;
 	unsigned long irqflags;
 
-	dspcntr = i9xx_plane_ctl(crtc_state, plane_state);
-
 	intel_add_fb_offsets(&x, &y, plane_state, 0);
 
 	intel_crtc->dspaddr_offset =
@@ -3358,7 +3354,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	struct drm_framebuffer *fb = plane_state->base.fb;
 	enum plane_id plane_id = to_intel_plane(plane)->id;
 	enum pipe pipe = to_intel_plane(plane)->pipe;
-	u32 plane_ctl;
+	u32 plane_ctl = plane_state->ctl;
 	unsigned int rotation = plane_state->base.rotation;
 	u32 stride = skl_plane_stride(fb, 0, rotation);
 	u32 surf_addr = plane_state->main.offset;
@@ -3373,8 +3369,6 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	int dst_h = drm_rect_height(&plane_state->base.dst);
 	unsigned long irqflags;
 
-	plane_ctl = skl_plane_ctl(crtc_state, plane_state);
-
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -9187,7 +9181,6 @@ static u32 i845_cursor_ctl(const struct intel_crtc_state *crtc_state,
 }
 
 static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
-			       const struct intel_crtc_state *crtc_state,
 			       const struct intel_plane_state *plane_state)
 {
 	struct drm_device *dev = crtc->dev;
@@ -9199,7 +9192,7 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
 		unsigned int width = plane_state->base.crtc_w;
 		unsigned int height = plane_state->base.crtc_h;
 
-		cntl = i845_cursor_ctl(crtc_state, plane_state);
+		cntl = plane_state->ctl;
 		size = (height << 12) | width;
 	}
 
@@ -9270,7 +9263,6 @@ static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
 }
 
 static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
-			       const struct intel_crtc_state *crtc_state,
 			       const struct intel_plane_state *plane_state)
 {
 	struct drm_device *dev = crtc->dev;
@@ -9280,7 +9272,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
 	uint32_t cntl = 0;
 
 	if (plane_state && plane_state->base.visible)
-		cntl = i9xx_cursor_ctl(crtc_state, plane_state);
+		cntl = plane_state->ctl;
 
 	if (intel_crtc->cursor_cntl != cntl) {
 		I915_WRITE_FW(CURCNTR(pipe), cntl);
@@ -9297,7 +9289,6 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
 
 /* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
 static void intel_crtc_update_cursor(struct drm_crtc *crtc,
-				     const struct intel_crtc_state *crtc_state,
 				     const struct intel_plane_state *plane_state)
 {
 	struct drm_device *dev = crtc->dev;
@@ -9337,9 +9328,9 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 	I915_WRITE_FW(CURPOS(pipe), pos);
 
 	if (IS_I845G(dev_priv) || IS_I865G(dev_priv))
-		i845_update_cursor(crtc, base, crtc_state, plane_state);
+		i845_update_cursor(crtc, base, plane_state);
 	else
-		i9xx_update_cursor(crtc, base, crtc_state, plane_state);
+		i9xx_update_cursor(crtc, base, plane_state);
 
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
@@ -13371,6 +13362,10 @@ intel_check_primary_plane(struct drm_plane *plane,
 		ret = skl_check_plane_surface(state);
 		if (ret)
 			return ret;
+
+		state->ctl = skl_plane_ctl(crtc_state, state);
+	} else {
+		state->ctl = i9xx_plane_ctl(crtc_state, state);
 	}
 
 	return 0;
@@ -13706,6 +13701,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
 			 struct intel_crtc_state *crtc_state,
 			 struct intel_plane_state *state)
 {
+	struct drm_i915_private *dev_priv = to_i915(plane->dev);
 	struct drm_framebuffer *fb = state->base.fb;
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	enum pipe pipe = to_intel_plane(plane)->pipe;
@@ -13725,7 +13721,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
 		return 0;
 
 	/* Check for which cursor types we support */
-	if (!cursor_size_ok(to_i915(plane->dev), state->base.crtc_w,
+	if (!cursor_size_ok(dev_priv, state->base.crtc_w,
 			    state->base.crtc_h)) {
 		DRM_DEBUG("Cursor dimension %dx%d not supported\n",
 			  state->base.crtc_w, state->base.crtc_h);
@@ -13753,12 +13749,17 @@ intel_check_cursor_plane(struct drm_plane *plane,
 	 * display power well must be turned off and on again.
 	 * Refuse the put the cursor into that compromised position.
 	 */
-	if (IS_CHERRYVIEW(to_i915(plane->dev)) && pipe == PIPE_C &&
+	if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_C &&
 	    state->base.visible && state->base.crtc_x < 0) {
 		DRM_DEBUG_KMS("CHV cursor C not allowed to straddle the left screen edge\n");
 		return -EINVAL;
 	}
 
+	if (IS_I845G(dev_priv) || IS_I865G(dev_priv))
+		state->ctl = i845_cursor_ctl(crtc_state, state);
+	else
+		state->ctl = i9xx_cursor_ctl(crtc_state, state);
+
 	return 0;
 }
 
@@ -13769,7 +13770,7 @@ intel_disable_cursor_plane(struct drm_plane *plane,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
 	intel_crtc->cursor_addr = 0;
-	intel_crtc_update_cursor(crtc, NULL, NULL);
+	intel_crtc_update_cursor(crtc, NULL);
 }
 
 static void
@@ -13791,7 +13792,7 @@ intel_update_cursor_plane(struct drm_plane *plane,
 		addr = obj->phys_handle->busaddr;
 
 	intel_crtc->cursor_addr = addr;
-	intel_crtc_update_cursor(crtc, crtc_state, state);
+	intel_crtc_update_cursor(crtc, state);
 }
 
 static struct intel_plane *
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 45e55b45e772..a35442eadd36 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -398,6 +398,9 @@ struct intel_plane_state {
 		int x, y;
 	} aux;
 
+	/* plane control register */
+	u32 ctl;
+
 	/*
 	 * scaler_id
 	 *    = -1 : not using a scaler
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 12e33bc149e4..6b76012ded1a 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -217,7 +217,7 @@ skl_update_plane(struct drm_plane *drm_plane,
 	struct drm_framebuffer *fb = plane_state->base.fb;
 	enum plane_id plane_id = intel_plane->id;
 	enum pipe pipe = intel_plane->pipe;
-	u32 plane_ctl;
+	u32 plane_ctl = plane_state->ctl;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	u32 surf_addr = plane_state->main.offset;
 	unsigned int rotation = plane_state->base.rotation;
@@ -232,8 +232,6 @@ skl_update_plane(struct drm_plane *drm_plane,
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
 	unsigned long irqflags;
 
-	plane_ctl = skl_plane_ctl(crtc_state, plane_state);
-
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -420,7 +418,7 @@ vlv_update_plane(struct drm_plane *dplane,
 	struct drm_framebuffer *fb = plane_state->base.fb;
 	enum pipe pipe = intel_plane->pipe;
 	enum plane_id plane_id = intel_plane->id;
-	u32 sprctl;
+	u32 sprctl = plane_state->ctl;
 	u32 sprsurf_offset, linear_offset;
 	unsigned int rotation = plane_state->base.rotation;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
@@ -434,8 +432,6 @@ vlv_update_plane(struct drm_plane *dplane,
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
 	unsigned long irqflags;
 
-	sprctl = vlv_sprite_ctl(crtc_state, plane_state);
-
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -569,7 +565,7 @@ ivb_update_plane(struct drm_plane *plane,
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct drm_framebuffer *fb = plane_state->base.fb;
 	enum pipe pipe = intel_plane->pipe;
-	u32 sprctl, sprscale = 0;
+	u32 sprctl = plane_state->ctl, sprscale = 0;
 	u32 sprsurf_offset, linear_offset;
 	unsigned int rotation = plane_state->base.rotation;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
@@ -583,8 +579,6 @@ ivb_update_plane(struct drm_plane *plane,
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
 	unsigned long irqflags;
 
-	sprctl = ivb_sprite_ctl(crtc_state, plane_state);
-
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -722,7 +716,7 @@ ilk_update_plane(struct drm_plane *plane,
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct drm_framebuffer *fb = plane_state->base.fb;
 	int pipe = intel_plane->pipe;
-	u32 dvscntr, dvsscale;
+	u32 dvscntr = plane_state->ctl, dvsscale;
 	u32 dvssurf_offset, linear_offset;
 	unsigned int rotation = plane_state->base.rotation;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
@@ -736,8 +730,6 @@ ilk_update_plane(struct drm_plane *plane,
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
 	unsigned long irqflags;
 
-	dvscntr = ilk_sprite_ctl(crtc_state, plane_state);
-
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -986,6 +978,14 @@ intel_check_sprite_plane(struct drm_plane *plane,
 		ret = skl_check_plane_surface(state);
 		if (ret)
 			return ret;
+
+		state->ctl = skl_plane_ctl(crtc_state, state);
+	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+		state->ctl = vlv_sprite_ctl(crtc_state, state);
+	} else if (INTEL_GEN(dev_priv) >= 7) {
+		state->ctl = ivb_sprite_ctl(crtc_state, state);
+	} else {
+		state->ctl = ilk_sprite_ctl(crtc_state, state);
 	}
 
 	return 0;
-- 
2.10.2

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

[PATCH 09/14] drm/i915: Introduce i9xx_check_plane_surface()

[ville.syrjala]

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

Extract the primary plane surfae offset/x/y calculations for
pre-SKL platforms into a common function, and call it during the
atomic check phase to reduce the amount of stuff we have to do
during the commit phase. SKL is already doing this.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2e0106a11f8f..024614cb47b6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3026,6 +3026,43 @@ static u32 i9xx_plane_ctl(const struct intel_crtc_state *crtc_state,
 	return dspcntr;
 }
 
+static int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
+	int src_x = plane_state->base.src.x1 >> 16;
+	int src_y = plane_state->base.src.y1 >> 16;
+	u32 offset;
+
+	intel_add_fb_offsets(&src_x, &src_y, plane_state, 0);
+
+	if (INTEL_GEN(dev_priv) >= 4)
+		offset = intel_compute_tile_offset(&src_x, &src_y,
+						   plane_state, 0);
+	else
+		offset = 0;
+
+	/* HSW+ does this automagically in hardware */
+	if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv)) {
+		unsigned int rotation = plane_state->base.rotation;
+		int src_w = drm_rect_width(&plane_state->base.src) >> 16;
+		int src_h = drm_rect_height(&plane_state->base.src) >> 16;
+
+		if (rotation & DRM_ROTATE_180) {
+			src_x += src_w - 1;
+			src_y += src_h - 1;
+		} else if (rotation & DRM_REFLECT_X) {
+			src_x += src_w - 1;
+		}
+	}
+
+	plane_state->main.offset = offset;
+	plane_state->main.x = src_x;
+	plane_state->main.y = src_y;
+
+	return 0;
+}
+
 static void i9xx_update_primary_plane(struct drm_plane *primary,
 				      const struct intel_crtc_state *crtc_state,
 				      const struct intel_plane_state *plane_state)
@@ -3037,27 +3074,15 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	u32 linear_offset;
 	u32 dspcntr = plane_state->ctl;
 	i915_reg_t reg = DSPCNTR(plane);
-	unsigned int rotation = plane_state->base.rotation;
-	int x = plane_state->base.src.x1 >> 16;
-	int y = plane_state->base.src.y1 >> 16;
+	int x = plane_state->main.x;
+	int y = plane_state->main.y;
 	unsigned long irqflags;
 
-	intel_add_fb_offsets(&x, &y, plane_state, 0);
-
-	if (INTEL_GEN(dev_priv) >= 4)
-		intel_crtc->dspaddr_offset =
-			intel_compute_tile_offset(&x, &y, plane_state, 0);
-
-	if (rotation & DRM_ROTATE_180) {
-		x += crtc_state->pipe_src_w - 1;
-		y += crtc_state->pipe_src_h - 1;
-	} else if (rotation & DRM_REFLECT_X) {
-		x += crtc_state->pipe_src_w - 1;
-	}
-
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
-	if (INTEL_GEN(dev_priv) < 4)
+	if (INTEL_GEN(dev_priv) >= 4)
+		intel_crtc->dspaddr_offset = plane_state->main.offset;
+	else
 		intel_crtc->dspaddr_offset = linear_offset;
 
 	intel_crtc->adjusted_x = x;
@@ -3133,25 +3158,14 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
 	u32 linear_offset;
 	u32 dspcntr = plane_state->ctl;
 	i915_reg_t reg = DSPCNTR(plane);
-	unsigned int rotation = plane_state->base.rotation;
-	int x = plane_state->base.src.x1 >> 16;
-	int y = plane_state->base.src.y1 >> 16;
+	int x = plane_state->main.x;
+	int y = plane_state->main.y;
 	unsigned long irqflags;
 
-	intel_add_fb_offsets(&x, &y, plane_state, 0);
-
-	intel_crtc->dspaddr_offset =
-		intel_compute_tile_offset(&x, &y, plane_state, 0);
-
-	/* HSW+ does this automagically in hardware */
-	if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv) &&
-	    rotation & DRM_ROTATE_180) {
-		x += crtc_state->pipe_src_w - 1;
-		y += crtc_state->pipe_src_h - 1;
-	}
-
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
+	intel_crtc->dspaddr_offset = plane_state->main.offset;
+
 	intel_crtc->adjusted_x = x;
 	intel_crtc->adjusted_y = y;
 
@@ -13365,6 +13379,10 @@ intel_check_primary_plane(struct drm_plane *plane,
 
 		state->ctl = skl_plane_ctl(crtc_state, state);
 	} else {
+		ret = i9xx_check_plane_surface(state);
+		if (ret)
+			return ret;
+
 		state->ctl = i9xx_plane_ctl(crtc_state, state);
 	}
 
-- 
2.10.2

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

[PATCH 10/14] drm/i915: Eliminate ironlake_update_primary_plane()

[ville.syrjala]

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

The effective difference between i9xx_update_primary_plane()
and ironlake_update_primary_plane() is only the HSW/BDW
DSPOFFSET special case. So bring that over into
i9xx_update_primary_plane() and eliminate the duplicated code.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 024614cb47b6..c7f6bc4e605a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3109,7 +3109,12 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	I915_WRITE_FW(reg, dspcntr);
 
 	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
-	if (INTEL_GEN(dev_priv) >= 4) {
+	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
+		I915_WRITE_FW(DSPSURF(plane),
+			      intel_plane_ggtt_offset(plane_state) +
+			      intel_crtc->dspaddr_offset);
+		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
+	} else if (INTEL_GEN(dev_priv) >= 4) {
 		I915_WRITE_FW(DSPSURF(plane),
 			      intel_plane_ggtt_offset(plane_state) +
 			      intel_crtc->dspaddr_offset);
@@ -3146,48 +3151,6 @@ static void i9xx_disable_primary_plane(struct drm_plane *primary,
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-static void ironlake_update_primary_plane(struct drm_plane *primary,
-					  const struct intel_crtc_state *crtc_state,
-					  const struct intel_plane_state *plane_state)
-{
-	struct drm_device *dev = primary->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_framebuffer *fb = plane_state->base.fb;
-	int plane = intel_crtc->plane;
-	u32 linear_offset;
-	u32 dspcntr = plane_state->ctl;
-	i915_reg_t reg = DSPCNTR(plane);
-	int x = plane_state->main.x;
-	int y = plane_state->main.y;
-	unsigned long irqflags;
-
-	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
-
-	intel_crtc->dspaddr_offset = plane_state->main.offset;
-
-	intel_crtc->adjusted_x = x;
-	intel_crtc->adjusted_y = y;
-
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-
-	I915_WRITE_FW(reg, dspcntr);
-
-	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
-	I915_WRITE_FW(DSPSURF(plane),
-		      intel_plane_ggtt_offset(plane_state) +
-		      intel_crtc->dspaddr_offset);
-	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
-		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
-	} else {
-		I915_WRITE_FW(DSPTILEOFF(plane), (y << 16) | x);
-		I915_WRITE_FW(DSPLINOFF(plane), linear_offset);
-	}
-	POSTING_READ_FW(reg);
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
-}
-
 static u32
 intel_fb_stride_alignment(const struct drm_framebuffer *fb, int plane)
 {
@@ -13642,12 +13605,6 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 		primary->update_plane = skylake_update_primary_plane;
 		primary->disable_plane = skylake_disable_primary_plane;
-	} else if (HAS_PCH_SPLIT(dev_priv)) {
-		intel_primary_formats = i965_primary_formats;
-		num_formats = ARRAY_SIZE(i965_primary_formats);
-
-		primary->update_plane = ironlake_update_primary_plane;
-		primary->disable_plane = i9xx_disable_primary_plane;
 	} else if (INTEL_GEN(dev_priv) >= 4) {
 		intel_primary_formats = i965_primary_formats;
 		num_formats = ARRAY_SIZE(i965_primary_formats);
-- 
2.10.2

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

[PATCH 11/14] drm/i915: Use i9xx_check_plane_surface() for sprite planes as well

[ville.syrjala]

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

All the pre-SKL sprite planes compute the x/y/tile offsets in a
similar way. There are a couple of minor differences but the primary
planes have those as well. Thus i9xx_check_plane_surface()
already does what we need, so let's use it.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c7f6bc4e605a..34d833975b32 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3026,7 +3026,7 @@ static u32 i9xx_plane_ctl(const struct intel_crtc_state *crtc_state,
 	return dspcntr;
 }
 
-static int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
+int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(plane_state->base.plane->dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a35442eadd36..15e3eb2824e3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1453,6 +1453,7 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
 u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
 		     unsigned int rotation);
 int skl_check_plane_surface(struct intel_plane_state *plane_state);
+int i9xx_check_plane_surface(struct intel_plane_state *plane_state);
 
 /* intel_csr.c */
 void intel_csr_ucode_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 6b76012ded1a..26c8bdcd7e72 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -419,35 +419,21 @@ vlv_update_plane(struct drm_plane *dplane,
 	enum pipe pipe = intel_plane->pipe;
 	enum plane_id plane_id = intel_plane->id;
 	u32 sprctl = plane_state->ctl;
-	u32 sprsurf_offset, linear_offset;
-	unsigned int rotation = plane_state->base.rotation;
+	u32 sprsurf_offset = plane_state->main.offset;
+	u32 linear_offset;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	int crtc_x = plane_state->base.dst.x1;
 	int crtc_y = plane_state->base.dst.y1;
 	uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
 	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
-	uint32_t x = plane_state->base.src.x1 >> 16;
-	uint32_t y = plane_state->base.src.y1 >> 16;
-	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
-	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
+	uint32_t x = plane_state->main.x;
+	uint32_t y = plane_state->main.y;
 	unsigned long irqflags;
 
 	/* Sizes are 0 based */
-	src_w--;
-	src_h--;
 	crtc_w--;
 	crtc_h--;
 
-	intel_add_fb_offsets(&x, &y, plane_state, 0);
-	sprsurf_offset = intel_compute_tile_offset(&x, &y, plane_state, 0);
-
-	if (rotation & DRM_ROTATE_180) {
-		x += src_w;
-		y += src_h;
-	} else if (rotation & DRM_REFLECT_X) {
-		x += src_w;
-	}
-
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
@@ -566,15 +552,15 @@ ivb_update_plane(struct drm_plane *plane,
 	struct drm_framebuffer *fb = plane_state->base.fb;
 	enum pipe pipe = intel_plane->pipe;
 	u32 sprctl = plane_state->ctl, sprscale = 0;
-	u32 sprsurf_offset, linear_offset;
-	unsigned int rotation = plane_state->base.rotation;
+	u32 sprsurf_offset = plane_state->main.offset;
+	u32 linear_offset;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	int crtc_x = plane_state->base.dst.x1;
 	int crtc_y = plane_state->base.dst.y1;
 	uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
 	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
-	uint32_t x = plane_state->base.src.x1 >> 16;
-	uint32_t y = plane_state->base.src.y1 >> 16;
+	uint32_t x = plane_state->main.x;
+	uint32_t y = plane_state->main.y;
 	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
 	unsigned long irqflags;
@@ -588,16 +574,6 @@ ivb_update_plane(struct drm_plane *plane,
 	if (crtc_w != src_w || crtc_h != src_h)
 		sprscale = SPRITE_SCALE_ENABLE | (src_w << 16) | src_h;
 
-	intel_add_fb_offsets(&x, &y, plane_state, 0);
-	sprsurf_offset = intel_compute_tile_offset(&x, &y, plane_state, 0);
-
-	/* HSW+ does this automagically in hardware */
-	if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv) &&
-	    rotation & DRM_ROTATE_180) {
-		x += src_w;
-		y += src_h;
-	}
-
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
@@ -716,16 +692,16 @@ ilk_update_plane(struct drm_plane *plane,
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct drm_framebuffer *fb = plane_state->base.fb;
 	int pipe = intel_plane->pipe;
-	u32 dvscntr = plane_state->ctl, dvsscale;
-	u32 dvssurf_offset, linear_offset;
-	unsigned int rotation = plane_state->base.rotation;
+	u32 dvscntr = plane_state->ctl, dvsscale = 0;
+	u32 dvssurf_offset = plane_state->main.offset;
+	u32 linear_offset;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	int crtc_x = plane_state->base.dst.x1;
 	int crtc_y = plane_state->base.dst.y1;
 	uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
 	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
-	uint32_t x = plane_state->base.src.x1 >> 16;
-	uint32_t y = plane_state->base.src.y1 >> 16;
+	uint32_t x = plane_state->main.x;
+	uint32_t y = plane_state->main.y;
 	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
 	unsigned long irqflags;
@@ -736,18 +712,9 @@ ilk_update_plane(struct drm_plane *plane,
 	crtc_w--;
 	crtc_h--;
 
-	dvsscale = 0;
 	if (crtc_w != src_w || crtc_h != src_h)
 		dvsscale = DVS_SCALE_ENABLE | (src_w << 16) | src_h;
 
-	intel_add_fb_offsets(&x, &y, plane_state, 0);
-	dvssurf_offset = intel_compute_tile_offset(&x, &y, plane_state, 0);
-
-	if (rotation & DRM_ROTATE_180) {
-		x += src_w;
-		y += src_h;
-	}
-
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
@@ -981,10 +948,21 @@ intel_check_sprite_plane(struct drm_plane *plane,
 
 		state->ctl = skl_plane_ctl(crtc_state, state);
 	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+		ret = i9xx_check_plane_surface(state);
+		if (ret)
+			return ret;
+
 		state->ctl = vlv_sprite_ctl(crtc_state, state);
 	} else if (INTEL_GEN(dev_priv) >= 7) {
+		ret = i9xx_check_plane_surface(state);
+		if (ret)
+			return ret;
 		state->ctl = ivb_sprite_ctl(crtc_state, state);
 	} else {
+		ret = i9xx_check_plane_surface(state);
+		if (ret)
+			return ret;
+
 		state->ctl = ilk_sprite_ctl(crtc_state, state);
 	}
 
-- 
2.10.2

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

[PATCH 12/14] drm/i915: Keep vblanks enabled during the entire pipe update

[ville.syrjala]

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

We currently hold a vblank referenced while trying to evade the
vblank, but we drop it as soon as we've done that. After all the
planes have been committed we are quite likely to grab a new vblank
reference for delivering the flip event. This causes the vblank
interrupt to do a enable->enable->disable ping-pong during many
commits. If we instead hang on to the original vblank reference
across the entire commit we can eliminate that ping-pong.

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

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 26c8bdcd7e72..337e884252e5 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -137,8 +137,6 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
 
 	finish_wait(wq, &wait);
 
-	drm_crtc_vblank_put(&crtc->base);
-
 	crtc->debug.scanline_start = scanline;
 	crtc->debug.start_vbl_time = ktime_get();
 	crtc->debug.start_vbl_count = intel_crtc_get_vblank_counter(crtc);
@@ -185,6 +183,15 @@ void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work
 		crtc->base.state->event = NULL;
 	}
 
+	/*
+	 * The reference was taken in intel_pipe_update_start(). It could
+	 * have been dropped as soon as the vblank was evaded, but we hold
+	 * on to it until this time to avoid the extra vblank interrupt
+	 * enable->disable->enable ping-pong whenever we have to deliver
+	 * an event.
+	 */
+	drm_crtc_vblank_put(&crtc->base);
+
 	local_irq_enable();
 
 	if (intel_vgpu_active(dev_priv))
-- 
2.10.2

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

[RFC][PATCH 13/14] WIP/drm/i915: Protect the entire pipe update with uncore.lock

[ville.syrjala]

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

I'm thinking we'll want to actually spliut the uncore.lock into
two locks (display vs. gt). But this is just a proof of concept and
it is indeed quote effective, especially with lockdep and whatnot
enabled.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c      | 66 ++++++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/i915_trace.h    | 28 +++++++--------
 drivers/gpu/drm/i915/intel_display.c | 51 ++++++++++++----------------
 drivers/gpu/drm/i915/intel_drv.h     |  7 ++--
 drivers/gpu/drm/i915/intel_pm.c      | 11 +++---
 drivers/gpu/drm/i915/intel_sprite.c  | 57 ++++++-------------------------
 6 files changed, 104 insertions(+), 116 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index cb20c9408b12..113de4309b41 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -715,15 +715,13 @@ static void i915_enable_asle_pipestat(struct drm_i915_private *dev_priv)
 /* Called from drm generic code, passed a 'crtc', which
  * we use as a pipe index
  */
-static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
+static u32 __i915_get_vblank_counter(struct intel_crtc *crtc)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
 	i915_reg_t high_frame, low_frame;
 	u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal;
-	struct intel_crtc *intel_crtc = intel_get_crtc_for_pipe(dev_priv,
-								pipe);
-	const struct drm_display_mode *mode = &intel_crtc->base.hwmode;
-	unsigned long irqflags;
+	const struct drm_display_mode *mode = &crtc->base.hwmode;
 
 	htotal = mode->crtc_htotal;
 	hsync_start = mode->crtc_hsync_start;
@@ -740,8 +738,6 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
 	high_frame = PIPEFRAME(pipe);
 	low_frame = PIPEFRAMEPIXEL(pipe);
 
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-
 	/*
 	 * High & low register fields aren't synchronized, so make sure
 	 * we get a low value that's stable across two reads of the high
@@ -753,8 +749,6 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
 		high2 = I915_READ_FW(high_frame) & PIPE_FRAME_HIGH_MASK;
 	} while (high1 != high2);
 
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
-
 	high1 >>= PIPE_FRAME_HIGH_SHIFT;
 	pixel = low & PIPE_PIXEL_MASK;
 	low >>= PIPE_FRAME_LOW_SHIFT;
@@ -767,15 +761,57 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
 	return (((high1 << 8) | low) + (pixel >= vbl_start)) & 0xffffff;
 }
 
+static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
+	unsigned long irqflags;
+	u32 ret;
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+	ret = __i915_get_vblank_counter(crtc);
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+
+	return ret;
+}
+
+static u32 __g4x_get_vblank_counter(struct intel_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+
+	return I915_READ_FW(PIPE_FRMCOUNT_G4X(pipe));
+}
+
 static u32 g4x_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
+	unsigned long irqflags;
+	u32 ret;
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+	ret = __g4x_get_vblank_counter(crtc);
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+
+	return ret;
+}
 
-	return I915_READ(PIPE_FRMCOUNT_G4X(pipe));
+u32 __intel_crtc_get_vblank_counter(struct intel_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+
+	if (IS_GEN2(dev_priv)) {
+		return 0;
+	} else if (IS_G4X(dev_priv) || INTEL_GEN(dev_priv) >= 5) {
+		return __g4x_get_vblank_counter(crtc);
+	} else {
+		return __i915_get_vblank_counter(crtc);
+	}
 }
 
 /* I915_READ_FW, only for fast reads of display block, no need for forcewake etc. */
-static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
+int __intel_crtc_get_scanline(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -878,7 +914,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
 		/* No obvious pixelcount register. Only query vertical
 		 * scanout position from Display scan line register.
 		 */
-		position = __intel_get_crtc_scanline(intel_crtc);
+		position = __intel_crtc_get_scanline(intel_crtc);
 	} else {
 		/* Have access to pixelcount since start of frame.
 		 * We can split this into vertical and horizontal
@@ -951,14 +987,14 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe,
 	return ret;
 }
 
-int intel_get_crtc_scanline(struct intel_crtc *crtc)
+int intel_crtc_get_scanline(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	unsigned long irqflags;
 	int position;
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-	position = __intel_get_crtc_scanline(crtc);
+	position = __intel_crtc_get_scanline(crtc);
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 
 	return position;
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 66404c5aee82..66491e3db344 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -29,7 +29,7 @@ TRACE_EVENT(intel_cpu_fifo_underrun,
 	    TP_fast_assign(
 			   __entry->pipe = pipe;
 			   __entry->frame = dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, pipe);
-			   __entry->scanline = intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
+			   __entry->scanline = intel_crtc_get_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
 			   ),
 
 	    TP_printk("pipe %c, frame=%u, scanline=%u",
@@ -51,7 +51,7 @@ TRACE_EVENT(intel_pch_fifo_underrun,
 			   enum pipe pipe = (enum pipe)pch_transcoder;
 			   __entry->pipe = pipe;
 			   __entry->frame = dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, pipe);
-			   __entry->scanline = intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
+			   __entry->scanline = intel_crtc_get_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
 			   ),
 
 	    TP_printk("pch transcoder %c, frame=%u, scanline=%u",
@@ -76,7 +76,7 @@ TRACE_EVENT(intel_memory_cxsr,
 				   __entry->frame[pipe] =
 					   dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, pipe);
 				   __entry->scanline[pipe] =
-					   intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
+					   intel_crtc_get_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
 			   }
 			   __entry->old = old;
 			   __entry->new = new;
@@ -111,7 +111,7 @@ TRACE_EVENT(vlv_wm,
 			   __entry->pipe = crtc->pipe;
 			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
 										       crtc->pipe);
-			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   __entry->scanline = intel_crtc_get_scanline(crtc);
 			   __entry->level = wm->level;
 			   __entry->cxsr = wm->cxsr;
 			   __entry->primary = wm->pipe[crtc->pipe].plane[PLANE_PRIMARY];
@@ -144,9 +144,8 @@ TRACE_EVENT(vlv_fifo_size,
 
 	    TP_fast_assign(
 			   __entry->pipe = crtc->pipe;
-			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-										       crtc->pipe);
-			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   __entry->frame = __intel_crtc_get_vblank_counter(crtc);
+			   __entry->scanline = __intel_crtc_get_scanline(crtc);
 			   __entry->sprite0_start = sprite0_start;
 			   __entry->sprite1_start = sprite1_start;
 			   __entry->fifo_size = fifo_size;
@@ -176,9 +175,8 @@ TRACE_EVENT(intel_update_plane,
 	    TP_fast_assign(
 			   __entry->pipe = crtc->pipe;
 			   __entry->name = plane->name;
-			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-										       crtc->pipe);
-			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   __entry->frame = __intel_crtc_get_vblank_counter(crtc);
+			   __entry->scanline = __intel_crtc_get_scanline(crtc);
 			   memcpy(__entry->src, &plane->state->src, sizeof(__entry->src));
 			   memcpy(__entry->dst, &plane->state->dst, sizeof(__entry->dst));
 			   ),
@@ -204,9 +202,8 @@ TRACE_EVENT(intel_disable_plane,
 	    TP_fast_assign(
 			   __entry->pipe = crtc->pipe;
 			   __entry->name = plane->name;
-			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-										       crtc->pipe);
-			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   __entry->frame = __intel_crtc_get_vblank_counter(crtc);
+			   __entry->scanline = __intel_crtc_get_scanline(crtc);
 			   ),
 
 	    TP_printk("pipe %c, plane %s, frame=%u, scanline=%u",
@@ -230,9 +227,8 @@ TRACE_EVENT(i915_pipe_update_start,
 
 	    TP_fast_assign(
 			   __entry->pipe = crtc->pipe;
-			   __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
-										       crtc->pipe);
-			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   __entry->frame = __intel_crtc_get_vblank_counter(crtc);
+			   __entry->scanline = __intel_crtc_get_scanline(crtc);
 			   __entry->min = crtc->debug.min_vbl;
 			   __entry->max = crtc->debug.max_vbl;
 			   ),
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 34d833975b32..c35818b42762 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1931,7 +1931,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 	 * drm_crtc_vblank_on()
 	 */
 	if (dev->max_vblank_count == 0 &&
-	    wait_for(intel_get_crtc_scanline(crtc) != crtc->scanline_offset, 50))
+	    wait_for(intel_crtc_get_scanline(crtc) != crtc->scanline_offset, 50))
 		DRM_ERROR("pipe %c didn't start\n", pipe_name(pipe));
 }
 
@@ -2749,8 +2749,11 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 				to_intel_plane_state(plane_state),
 				false);
 	intel_pre_disable_primary_noatomic(&intel_crtc->base);
+
+	spin_lock_irq(&dev_priv->uncore.lock);
 	trace_intel_disable_plane(primary, intel_crtc);
 	intel_plane->disable_plane(primary, &intel_crtc->base);
+	spin_unlock_irq(&dev_priv->uncore.lock);
 
 	return;
 
@@ -3076,7 +3079,6 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	i915_reg_t reg = DSPCNTR(plane);
 	int x = plane_state->main.x;
 	int y = plane_state->main.y;
-	unsigned long irqflags;
 
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
@@ -3088,8 +3090,6 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	intel_crtc->adjusted_x = x;
 	intel_crtc->adjusted_y = y;
 
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-
 	if (INTEL_GEN(dev_priv) < 4) {
 		/* pipesrc and dspsize control the size that is scaled from,
 		 * which should always be the user's requested size.
@@ -3126,8 +3126,6 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 			      intel_crtc->dspaddr_offset);
 	}
 	POSTING_READ_FW(reg);
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void i9xx_disable_primary_plane(struct drm_plane *primary,
@@ -3137,9 +3135,6 @@ static void i9xx_disable_primary_plane(struct drm_plane *primary,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int plane = intel_crtc->plane;
-	unsigned long irqflags;
-
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
 	I915_WRITE_FW(DSPCNTR(plane), 0);
 	if (INTEL_INFO(dev_priv)->gen >= 4)
@@ -3147,8 +3142,6 @@ static void i9xx_disable_primary_plane(struct drm_plane *primary,
 	else
 		I915_WRITE_FW(DSPADDR(plane), 0);
 	POSTING_READ_FW(DSPCNTR(plane));
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static u32
@@ -3344,7 +3337,6 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	int dst_y = plane_state->base.dst.y1;
 	int dst_w = drm_rect_width(&plane_state->base.dst);
 	int dst_h = drm_rect_height(&plane_state->base.dst);
-	unsigned long irqflags;
 
 	/* Sizes are 0 based */
 	src_w--;
@@ -3357,8 +3349,6 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	intel_crtc->adjusted_x = src_x;
 	intel_crtc->adjusted_y = src_y;
 
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-
 	if (IS_GEMINILAKE(dev_priv)) {
 		I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
 			      PLANE_COLOR_PIPE_GAMMA_ENABLE |
@@ -3390,8 +3380,6 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 		      intel_plane_ggtt_offset(plane_state) + surf_addr);
 
 	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void skylake_disable_primary_plane(struct drm_plane *primary,
@@ -3401,15 +3389,10 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	enum plane_id plane_id = to_intel_plane(primary)->id;
 	enum pipe pipe = to_intel_plane(primary)->pipe;
-	unsigned long irqflags;
-
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
 	I915_WRITE_FW(PLANE_CTL(pipe, plane_id), 0);
 	I915_WRITE_FW(PLANE_SURF(pipe, plane_id), 0);
 	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 /* Assume fb object is pinned & idle & fenced and just update base pointers */
@@ -3433,8 +3416,11 @@ static void intel_complete_page_flips(struct drm_i915_private *dev_priv)
 
 static void intel_update_primary_planes(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_crtc *crtc;
 
+	spin_lock_irq(&dev_priv->uncore.lock);
+
 	for_each_crtc(dev, crtc) {
 		struct intel_plane *plane = to_intel_plane(crtc->primary);
 		struct intel_plane_state *plane_state =
@@ -3449,6 +3435,8 @@ static void intel_update_primary_planes(struct drm_device *dev)
 					    plane_state);
 		}
 	}
+
+	spin_unlock_irq(&dev_priv->uncore.lock);
 }
 
 static int
@@ -5089,22 +5077,24 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
 
 static void intel_crtc_disable_planes(struct drm_crtc *crtc, unsigned plane_mask)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_plane *p;
 	int pipe = intel_crtc->pipe;
 
 	intel_crtc_dpms_overlay_disable(intel_crtc);
 
-	drm_for_each_plane_mask(p, dev, plane_mask)
+	spin_lock_irq(&dev_priv->uncore.lock);
+	drm_for_each_plane_mask(p, &dev_priv->drm, plane_mask)
 		to_intel_plane(p)->disable_plane(p, crtc);
+	spin_unlock_irq(&dev_priv->uncore.lock);
 
 	/*
 	 * FIXME: Once we grow proper nuclear flip support out of this we need
 	 * to compute the mask of flip planes precisely. For the time being
 	 * consider this a flip to a NULL plane.
 	 */
-	intel_frontbuffer_flip(to_i915(dev), INTEL_FRONTBUFFER_ALL_MASK(pipe));
+	intel_frontbuffer_flip(dev_priv, INTEL_FRONTBUFFER_ALL_MASK(pipe));
 }
 
 static void intel_encoders_pre_pll_enable(struct drm_crtc *crtc,
@@ -9273,7 +9263,6 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_crtc->pipe;
 	u32 base = intel_crtc->cursor_addr;
-	unsigned long irqflags;
 	u32 pos = 0;
 
 	if (plane_state) {
@@ -9300,16 +9289,12 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 		}
 	}
 
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-
 	I915_WRITE_FW(CURPOS(pipe), pos);
 
 	if (IS_I845G(dev_priv) || IS_I865G(dev_priv))
 		i845_update_cursor(crtc, base, plane_state);
 	else
 		i9xx_update_cursor(crtc, base, plane_state);
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static bool cursor_size_ok(struct drm_i915_private *dev_priv,
@@ -13521,6 +13506,8 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 	new_plane_state->fb = old_fb;
 	to_intel_plane_state(new_plane_state)->vma = old_vma;
 
+	spin_lock_irq(&dev_priv->uncore.lock);
+
 	if (plane->state->visible) {
 		trace_intel_update_plane(plane, to_intel_crtc(crtc));
 		intel_plane->update_plane(plane,
@@ -13531,6 +13518,8 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 		intel_plane->disable_plane(plane, crtc);
 	}
 
+	spin_unlock_irq(&dev_priv->uncore.lock);
+
 	intel_cleanup_plane_fb(plane, new_plane_state);
 
 out_unlock:
@@ -15174,6 +15163,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 
 		drm_crtc_vblank_on(&crtc->base);
 
+		spin_lock_irq(&dev_priv->uncore.lock);
+
 		/* Disable everything but the primary plane */
 		for_each_intel_plane_on_crtc(dev, crtc, plane) {
 			if (plane->base.type == DRM_PLANE_TYPE_PRIMARY)
@@ -15182,6 +15173,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 			trace_intel_disable_plane(&plane->base, crtc);
 			plane->disable_plane(&plane->base, &crtc->base);
 		}
+
+		spin_unlock_irq(&dev_priv->uncore.lock);
 	}
 
 	/* We need to sanitize the plane -> pipe mapping first because this will
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 15e3eb2824e3..8c6bf32244fb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1209,7 +1209,10 @@ static inline bool intel_irqs_enabled(struct drm_i915_private *dev_priv)
 	return dev_priv->pm.irqs_enabled;
 }
 
-int intel_get_crtc_scanline(struct intel_crtc *crtc);
+int __intel_crtc_get_scanline(struct intel_crtc *crtc);
+int intel_crtc_get_scanline(struct intel_crtc *crtc);
+u32 __intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
+u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
 void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
 				     unsigned int pipe_mask);
 void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
@@ -1350,8 +1353,6 @@ intel_wait_for_vblank_if_active(struct drm_i915_private *dev_priv, int pipe)
 		intel_wait_for_vblank(dev_priv, pipe);
 }
 
-u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
-
 int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp);
 void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
 			 struct intel_digital_port *dport,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index aece0ff88a5d..622ae89f3be1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1367,7 +1367,6 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 	 * intel_pipe_update_start() has already disabled interrupts
 	 * for us, so a plain spin_lock() is sufficient here.
 	 */
-	spin_lock(&dev_priv->uncore.lock);
 
 	switch (crtc->pipe) {
 		uint32_t dsparb, dsparb2, dsparb3;
@@ -1427,8 +1426,6 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 	}
 
 	POSTING_READ_FW(DSPARB);
-
-	spin_unlock(&dev_priv->uncore.lock);
 }
 
 #undef VLV_FIFO
@@ -4003,9 +4000,9 @@ static void skl_ddb_entry_write(struct drm_i915_private *dev_priv,
 				const struct skl_ddb_entry *entry)
 {
 	if (entry->end)
-		I915_WRITE(reg, (entry->end - 1) << 16 | entry->start);
+		I915_WRITE_FW(reg, (entry->end - 1) << 16 | entry->start);
 	else
-		I915_WRITE(reg, 0);
+		I915_WRITE_FW(reg, 0);
 }
 
 static void skl_write_wm_level(struct drm_i915_private *dev_priv,
@@ -4020,7 +4017,7 @@ static void skl_write_wm_level(struct drm_i915_private *dev_priv,
 		val |= level->plane_res_l << PLANE_WM_LINES_SHIFT;
 	}
 
-	I915_WRITE(reg, val);
+	I915_WRITE_FW(reg, val);
 }
 
 static void skl_write_plane_wm(struct intel_crtc *intel_crtc,
@@ -4376,7 +4373,7 @@ static void skl_atomic_update_crtc_wm(struct intel_atomic_state *state,
 	if (!(state->wm_results.dirty_pipes & drm_crtc_mask(&crtc->base)))
 		return;
 
-	I915_WRITE(PIPE_WM_LINETIME(pipe), pipe_wm->linetime);
+	I915_WRITE_FW(PIPE_WM_LINETIME(pipe), pipe_wm->linetime);
 
 	for_each_plane_id_on_crtc(crtc, plane_id) {
 		if (plane_id != PLANE_CURSOR)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 337e884252e5..06c89deca36b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -83,6 +83,7 @@ int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
  */
 void intel_pipe_update_start(struct intel_crtc *crtc)
 {
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
 	long timeout = msecs_to_jiffies_timeout(1);
 	int scanline, min, max, vblank_start;
@@ -106,6 +107,8 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
 	if (WARN_ON(drm_crtc_vblank_get(&crtc->base)))
 		return;
 
+	spin_lock(&dev_priv->uncore.lock);
+
 	crtc->debug.min_vbl = min;
 	crtc->debug.max_vbl = max;
 	trace_i915_pipe_update_start(crtc);
@@ -118,7 +121,7 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
 		 */
 		prepare_to_wait(wq, &wait, TASK_UNINTERRUPTIBLE);
 
-		scanline = intel_get_crtc_scanline(crtc);
+		scanline = __intel_crtc_get_scanline(crtc);
 		if (scanline < min || scanline > max)
 			break;
 
@@ -128,18 +131,18 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
 			break;
 		}
 
-		local_irq_enable();
+		spin_unlock_irq(&dev_priv->uncore.lock);
 
 		timeout = schedule_timeout(timeout);
 
-		local_irq_disable();
+		spin_lock_irq(&dev_priv->uncore.lock);
 	}
 
 	finish_wait(wq, &wait);
 
 	crtc->debug.scanline_start = scanline;
 	crtc->debug.start_vbl_time = ktime_get();
-	crtc->debug.start_vbl_count = intel_crtc_get_vblank_counter(crtc);
+	crtc->debug.start_vbl_count = __intel_crtc_get_vblank_counter(crtc);
 
 	trace_i915_pipe_update_vblank_evaded(crtc);
 }
@@ -156,8 +159,8 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
 void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work)
 {
 	enum pipe pipe = crtc->pipe;
-	int scanline_end = intel_get_crtc_scanline(crtc);
-	u32 end_vbl_count = intel_crtc_get_vblank_counter(crtc);
+	int scanline_end = __intel_crtc_get_scanline(crtc);
+	u32 end_vbl_count = __intel_crtc_get_vblank_counter(crtc);
 	ktime_t end_vbl_time = ktime_get();
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 
@@ -169,6 +172,8 @@ void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work
 
 	trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end);
 
+	spin_unlock(&dev_priv->uncore.lock);
+
 	/* We're still in the vblank-evade critical section, this can't race.
 	 * Would be slightly nice to just grab the vblank count and arm the
 	 * event outside of the critical section - the spinlock might spin for a
@@ -237,7 +242,6 @@ skl_update_plane(struct drm_plane *drm_plane,
 	uint32_t y = plane_state->main.y;
 	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
-	unsigned long irqflags;
 
 	/* Sizes are 0 based */
 	src_w--;
@@ -245,8 +249,6 @@ skl_update_plane(struct drm_plane *drm_plane,
 	crtc_w--;
 	crtc_h--;
 
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-
 	if (IS_GEMINILAKE(dev_priv)) {
 		I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
 			      PLANE_COLOR_PIPE_GAMMA_ENABLE |
@@ -287,8 +289,6 @@ skl_update_plane(struct drm_plane *drm_plane,
 	I915_WRITE_FW(PLANE_SURF(pipe, plane_id),
 		      intel_plane_ggtt_offset(plane_state) + surf_addr);
 	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void
@@ -299,16 +299,11 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 	struct intel_plane *intel_plane = to_intel_plane(dplane);
 	enum plane_id plane_id = intel_plane->id;
 	enum pipe pipe = intel_plane->pipe;
-	unsigned long irqflags;
-
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
 	I915_WRITE_FW(PLANE_CTL(pipe, plane_id), 0);
 
 	I915_WRITE_FW(PLANE_SURF(pipe, plane_id), 0);
 	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void
@@ -435,7 +430,6 @@ vlv_update_plane(struct drm_plane *dplane,
 	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
 	uint32_t x = plane_state->main.x;
 	uint32_t y = plane_state->main.y;
-	unsigned long irqflags;
 
 	/* Sizes are 0 based */
 	crtc_w--;
@@ -443,8 +437,6 @@ vlv_update_plane(struct drm_plane *dplane,
 
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-
 	if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B)
 		chv_update_csc(intel_plane, fb->format->format);
 
@@ -468,8 +460,6 @@ vlv_update_plane(struct drm_plane *dplane,
 	I915_WRITE_FW(SPSURF(pipe, plane_id),
 		      intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
 	POSTING_READ_FW(SPSURF(pipe, plane_id));
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void
@@ -480,16 +470,11 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 	struct intel_plane *intel_plane = to_intel_plane(dplane);
 	enum pipe pipe = intel_plane->pipe;
 	enum plane_id plane_id = intel_plane->id;
-	unsigned long irqflags;
-
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
 	I915_WRITE_FW(SPCNTR(pipe, plane_id), 0);
 
 	I915_WRITE_FW(SPSURF(pipe, plane_id), 0);
 	POSTING_READ_FW(SPSURF(pipe, plane_id));
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
@@ -570,7 +555,6 @@ ivb_update_plane(struct drm_plane *plane,
 	uint32_t y = plane_state->main.y;
 	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
-	unsigned long irqflags;
 
 	/* Sizes are 0 based */
 	src_w--;
@@ -583,8 +567,6 @@ ivb_update_plane(struct drm_plane *plane,
 
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-
 	if (key->flags) {
 		I915_WRITE_FW(SPRKEYVAL(pipe), key->min_value);
 		I915_WRITE_FW(SPRKEYMAX(pipe), key->max_value);
@@ -610,8 +592,6 @@ ivb_update_plane(struct drm_plane *plane,
 	I915_WRITE_FW(SPRSURF(pipe),
 		      intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
 	POSTING_READ_FW(SPRSURF(pipe));
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void
@@ -621,9 +601,6 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	int pipe = intel_plane->pipe;
-	unsigned long irqflags;
-
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
 	I915_WRITE_FW(SPRCTL(pipe), 0);
 	/* Can't leave the scaler enabled... */
@@ -632,8 +609,6 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 
 	I915_WRITE_FW(SPRSURF(pipe), 0);
 	POSTING_READ_FW(SPRSURF(pipe));
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static u32 ilk_sprite_ctl(const struct intel_crtc_state *crtc_state,
@@ -711,7 +686,6 @@ ilk_update_plane(struct drm_plane *plane,
 	uint32_t y = plane_state->main.y;
 	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
-	unsigned long irqflags;
 
 	/* Sizes are 0 based */
 	src_w--;
@@ -724,8 +698,6 @@ ilk_update_plane(struct drm_plane *plane,
 
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-
 	if (key->flags) {
 		I915_WRITE_FW(DVSKEYVAL(pipe), key->min_value);
 		I915_WRITE_FW(DVSKEYMAX(pipe), key->max_value);
@@ -746,8 +718,6 @@ ilk_update_plane(struct drm_plane *plane,
 	I915_WRITE_FW(DVSSURF(pipe),
 		      intel_plane_ggtt_offset(plane_state) + dvssurf_offset);
 	POSTING_READ_FW(DVSSURF(pipe));
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static void
@@ -757,9 +727,6 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	int pipe = intel_plane->pipe;
-	unsigned long irqflags;
-
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
 	I915_WRITE_FW(DVSCNTR(pipe), 0);
 	/* Disable the scaler */
@@ -767,8 +734,6 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 
 	I915_WRITE_FW(DVSSURF(pipe), 0);
 	POSTING_READ_FW(DVSSURF(pipe));
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
 static int
-- 
2.10.2

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

[RFC][PATCH 14/14] WIP/drm/i915: Nuke posting reads from plane updates

[ville.syrjala]

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

We'll maybe want just one posting read at the end? This does seem to
potentially shave a few usec off from the update.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c35818b42762..d9ec654db5a3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3125,7 +3125,6 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 			      intel_plane_ggtt_offset(plane_state) +
 			      intel_crtc->dspaddr_offset);
 	}
-	POSTING_READ_FW(reg);
 }
 
 static void i9xx_disable_primary_plane(struct drm_plane *primary,
@@ -3141,7 +3140,6 @@ static void i9xx_disable_primary_plane(struct drm_plane *primary,
 		I915_WRITE_FW(DSPSURF(plane), 0);
 	else
 		I915_WRITE_FW(DSPADDR(plane), 0);
-	POSTING_READ_FW(DSPCNTR(plane));
 }
 
 static u32
@@ -3379,7 +3377,6 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	I915_WRITE_FW(PLANE_SURF(pipe, plane_id),
 		      intel_plane_ggtt_offset(plane_state) + surf_addr);
 
-	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
 }
 
 static void skylake_disable_primary_plane(struct drm_plane *primary,
@@ -3392,7 +3389,6 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
 
 	I915_WRITE_FW(PLANE_CTL(pipe, plane_id), 0);
 	I915_WRITE_FW(PLANE_SURF(pipe, plane_id), 0);
-	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
 }
 
 /* Assume fb object is pinned & idle & fenced and just update base pointers */
@@ -9171,7 +9167,6 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
 		 * whilst the cursor is disabled.
 		 */
 		I915_WRITE_FW(CURCNTR(PIPE_A), 0);
-		POSTING_READ_FW(CURCNTR(PIPE_A));
 		intel_crtc->cursor_cntl = 0;
 	}
 
@@ -9187,7 +9182,6 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
 
 	if (intel_crtc->cursor_cntl != cntl) {
 		I915_WRITE_FW(CURCNTR(PIPE_A), cntl);
-		POSTING_READ_FW(CURCNTR(PIPE_A));
 		intel_crtc->cursor_cntl = cntl;
 	}
 }
@@ -9243,13 +9237,11 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
 
 	if (intel_crtc->cursor_cntl != cntl) {
 		I915_WRITE_FW(CURCNTR(pipe), cntl);
-		POSTING_READ_FW(CURCNTR(pipe));
 		intel_crtc->cursor_cntl = cntl;
 	}
 
 	/* and commit changes on next vblank */
 	I915_WRITE_FW(CURBASE(pipe), base);
-	POSTING_READ_FW(CURBASE(pipe));
 
 	intel_crtc->cursor_base = base;
 }
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 06c89deca36b..5cf88f95c23e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -288,7 +288,6 @@ skl_update_plane(struct drm_plane *drm_plane,
 	I915_WRITE_FW(PLANE_CTL(pipe, plane_id), plane_ctl);
 	I915_WRITE_FW(PLANE_SURF(pipe, plane_id),
 		      intel_plane_ggtt_offset(plane_state) + surf_addr);
-	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
 }
 
 static void
@@ -303,7 +302,6 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 	I915_WRITE_FW(PLANE_CTL(pipe, plane_id), 0);
 
 	I915_WRITE_FW(PLANE_SURF(pipe, plane_id), 0);
-	POSTING_READ_FW(PLANE_SURF(pipe, plane_id));
 }
 
 static void
@@ -459,7 +457,6 @@ vlv_update_plane(struct drm_plane *dplane,
 	I915_WRITE_FW(SPCNTR(pipe, plane_id), sprctl);
 	I915_WRITE_FW(SPSURF(pipe, plane_id),
 		      intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
-	POSTING_READ_FW(SPSURF(pipe, plane_id));
 }
 
 static void
@@ -474,7 +471,6 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 	I915_WRITE_FW(SPCNTR(pipe, plane_id), 0);
 
 	I915_WRITE_FW(SPSURF(pipe, plane_id), 0);
-	POSTING_READ_FW(SPSURF(pipe, plane_id));
 }
 
 static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
@@ -591,7 +587,6 @@ ivb_update_plane(struct drm_plane *plane,
 	I915_WRITE_FW(SPRCTL(pipe), sprctl);
 	I915_WRITE_FW(SPRSURF(pipe),
 		      intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
-	POSTING_READ_FW(SPRSURF(pipe));
 }
 
 static void
@@ -608,7 +603,6 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 		I915_WRITE_FW(SPRSCALE(pipe), 0);
 
 	I915_WRITE_FW(SPRSURF(pipe), 0);
-	POSTING_READ_FW(SPRSURF(pipe));
 }
 
 static u32 ilk_sprite_ctl(const struct intel_crtc_state *crtc_state,
@@ -717,7 +711,6 @@ ilk_update_plane(struct drm_plane *plane,
 	I915_WRITE_FW(DVSCNTR(pipe), dvscntr);
 	I915_WRITE_FW(DVSSURF(pipe),
 		      intel_plane_ggtt_offset(plane_state) + dvssurf_offset);
-	POSTING_READ_FW(DVSSURF(pipe));
 }
 
 static void
@@ -733,7 +726,6 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	I915_WRITE_FW(DVSSCALE(pipe), 0);
 
 	I915_WRITE_FW(DVSSURF(pipe), 0);
-	POSTING_READ_FW(DVSSURF(pipe));
 }
 
 static int
-- 
2.10.2

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

Re: [PATCH 12/14] drm/i915: Keep vblanks enabled during the entire pipe update

[Chris Wilson]

On Fri, Mar 17, 2017 at 11:18:06PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We currently hold a vblank referenced while trying to evade the
> vblank, but we drop it as soon as we've done that. After all the
> planes have been committed we are quite likely to grab a new vblank
> reference for delivering the flip event. This causes the vblank
> interrupt to do a enable->enable->disable ping-pong during many
> commits. If we instead hang on to the original vblank reference
> across the entire commit we can eliminate that ping-pong.

Does it make sense to take an an extra vblank_get() only if we have an
event to deliver?

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 26c8bdcd7e72..337e884252e5 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -137,8 +137,6 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
>  
>  	finish_wait(wq, &wait);
>  
	if (crtc->base.state->event)
		drm_crtc_vblank_get(&crtc->base);

	drm_crtc_vblank_put(&crtc->base);

>  	crtc->debug.scanline_start = scanline;
>  	crtc->debug.start_vbl_time = ktime_get();
>  	crtc->debug.start_vbl_count = intel_crtc_get_vblank_counter(crtc);

> @@ -185,6 +183,15 @@ void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work
>  		crtc->base.state->event = NULL;
>  
> +	/*
> +	 * The reference was taken in intel_pipe_update_start(). It could
> +	 * have been dropped as soon as the vblank was evaded, but we hold
> +	 * on to it until this time to avoid the extra vblank interrupt
> +	 * enable->disable->enable ping-pong whenever we have to deliver
> +	 * an event.
> +	 */
> +	drm_crtc_vblank_put(&crtc->base);

	}

>  	local_irq_enable();

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC][PATCH 13/14] WIP/drm/i915: Protect the entire pipe update with uncore.lock

[Chris Wilson]

On Fri, Mar 17, 2017 at 11:18:07PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I'm thinking we'll want to actually spliut the uncore.lock into
> two locks (display vs. gt).

Yes, a single lock for display seems reasonable, esp. given the proof of
principle here.

Hmm, I was thinking we would need a separate one for fw_domains handling,
but that's actually just GT. Splitting into two seems doable.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Moar plane update optimizations

[Patchwork]

== Series Details ==

Series: drm/i915: Moar plane update optimizations
URL   : https://patchwork.freedesktop.org/series/21475/
State : success

== Summary ==

Series 21475v1 drm/i915: Moar plane update optimizations
https://patchwork.freedesktop.org/api/1.0/series/21475/revisions/1/mbox/

Test kms_cursor_legacy:
        Subgroup basic-flip-before-cursor-varying-size:
                dmesg-warn -> PASS       (fi-byt-n2820) fdo#100094

fdo#100094 https://bugs.freedesktop.org/show_bug.cgi?id=100094

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 462s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 585s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 502s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 504s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 440s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 436s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 438s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 517s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 496s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 549s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 419s

ca3e896e9cf4e4c3319bbb51843daa45902927a7 drm-tip: 2017y-03m-17d-18h-26m-58s UTC integration manifest
01176d4 WIP/drm/i915: Nuke posting reads from plane updates
eda4237 WIP/drm/i915: Protect the entire pipe update with uncore.lock
859d84b drm/i915: Keep vblanks enabled during the entire pipe update
d2b9e89 drm/i915: Use i9xx_check_plane_surface() for sprite planes as well
db96ecb drm/i915: Eliminate ironlake_update_primary_plane()
44bdc9a drm/i915: Introduce i9xx_check_plane_surface()
1f3f57d drm/i915: Pre-compute plane control register value
68c38ec drm/i915: Extract i845_cursor_ctl() and i9xx_cursor_ctl()
00b8e71 drm/i915: Extract ilk_sprite_ctl()
bc22047 drm/i915: Extract ivb_sprite_ctl()
61284a8 drm/i915: Extract vlv_sprite_ctl()
d9d52a0 drm/i915: Extract i9xx_plane_ctl()
2db4f23 drm/i915: Use skl_plane_ctl() for the SKL "sprite" planes
d9063a1 drm/i915: Extract skl_plane_ctl()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4223/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/14] drm/i915: Extract skl_plane_ctl()

[Chris Wilson]

On Fri, Mar 17, 2017 at 11:17:55PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Pull the code to calculate the SKL plane control register value into
> a separate function. Allows us to pre-compute it in the future.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/14] drm/i915: Use skl_plane_ctl() for the SKL "sprite" planes

[Chris Wilson]

On Fri, Mar 17, 2017 at 11:17:56PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On SKL the planes are uniform so the "sprites" can use the
> primary plane code perfectly fine. The only difference we
> have is the color key handling, but since we never enable that
> for the primary plane the same code works just fine.

Rationale works for me. Does setting the color key report an -EINVAL on
the primary? Even via the legacy setcolorkey ioctl?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/14] drm/i915: Extract i9xx_plane_ctl()

[Chris Wilson]

On Fri, Mar 17, 2017 at 11:17:57PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Pull the code to calculate the pre-SKL primary plane control register
> value into a separate function. Allows us to pre-compute it in the
> future.
> 
> We can also share the code between the i9xx and ilk codepaths as the
> differences are minimal. Actually there are no differences between
> g4x and ilk, so the current split doesn't really make any sense.

Could you do this as 2 passes? I started to freak out seeing hsw/bdw
when checking for no changes from the i9xx_update_primary_plane.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/14] drm/i915: Extract vlv_sprite_ctl()

[Chris Wilson]

On Fri, Mar 17, 2017 at 11:17:58PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Pull the code to calculate the VLV/CHV sprite control register value
> into a separate function. Allows us to pre-compute it in the future.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/14] drm/i915: Extract ivb_sprite_ctl()

[Chris Wilson]

On Fri, Mar 17, 2017 at 11:17:59PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Pull the code to calculate the IVB-BDW sprite control register value
> into a separate function. Allows us to pre-compute it in the future.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

> +static void
> +ivb_update_plane(struct drm_plane *plane,
> +		 const struct intel_crtc_state *crtc_state,
> +		 const struct intel_plane_state *plane_state)
> +{
> +	struct drm_device *dev = plane->dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	struct drm_framebuffer *fb = plane_state->base.fb;
> +	enum pipe pipe = intel_plane->pipe;
> +	u32 sprctl, sprscale = 0;
> +	u32 sprsurf_offset, linear_offset;
> +	unsigned int rotation = plane_state->base.rotation;
> +	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> +	int crtc_x = plane_state->base.dst.x1;
> +	int crtc_y = plane_state->base.dst.y1;
> +	uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
> +	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
> +	uint32_t x = plane_state->base.src.x1 >> 16;
> +	uint32_t y = plane_state->base.src.y1 >> 16;
> +	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
> +	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;

struct crtc_rectangle { int x, y; u32 w, h; } crtc_r = crtc_rectangle(plane_state);
struct source_rectangle { int x, y; u32 w, h; } src_r = source_rectangle(plane_state);

c_rect / s_rect?
crtc_rect / src_rect.

Just idly mentioning the large blocks of repeated code.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/14] drm/i915: Extract ilk_sprite_ctl()

[Chris Wilson]

On Fri, Mar 17, 2017 at 11:18:00PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Pull the code to calculate the ILK-SNB sprite control register value
> into a separate function. Allows us to pre-compute it in the future.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 07/14] drm/i915: Extract i845_cursor_ctl() and i9xx_cursor_ctl()

[Chris Wilson]

On Fri, Mar 17, 2017 at 11:18:01PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Pull the code to calculate the cursor control register value into
> separate functions. Allows us to pre-compute them in the future.

Want to comment on the benefit of passing struct intel_crtc_state?
 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 119 +++++++++++++++++++++--------------
>  1 file changed, 72 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 99b72c4219a8..040978e704f3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9161,7 +9161,33 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	return active;
>  }
>  
> +static u32 i845_cursor_ctl(const struct intel_crtc_state *crtc_state,
> +			   const struct intel_plane_state *plane_state)
> +{

>  static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
> +			       const struct intel_crtc_state *crtc_state,
>  			       const struct intel_plane_state *plane_state)
>  {

Odd mishmap of parameters as both functions seem to use the same state
in the end. (i845/i915_cursor_ctl recovering crtc, dev etc).

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/14] drm/i915: Pre-compute plane control register value

[Chris Wilson]

On Fri, Mar 17, 2017 at 11:18:02PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Computing the plane control register value is branchy so moving it out
> from the plane commit hook seems prudent. Let's pre-compute it during
> the atomic check phase and store the result in the plane state.

i845_cursor_ctl parameters starting to look more sensible.

> @@ -986,6 +978,14 @@ intel_check_sprite_plane(struct drm_plane *plane,

This function still doesn't do what it says on the tin!!!

>  		ret = skl_check_plane_surface(state);
>  		if (ret)
>  			return ret;
> +
> +		state->ctl = skl_plane_ctl(crtc_state, state);
> +	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> +		state->ctl = vlv_sprite_ctl(crtc_state, state);
> +	} else if (INTEL_GEN(dev_priv) >= 7) {
> +		state->ctl = ivb_sprite_ctl(crtc_state, state);
> +	} else {
> +		state->ctl = ilk_sprite_ctl(crtc_state, state);
>  	}

Precomputing these is a very good idea.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/14] drm/i915: Introduce i9xx_check_plane_surface()

[Chris Wilson]

On Fri, Mar 17, 2017 at 11:18:03PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Extract the primary plane surfae offset/x/y calculations for
> pre-SKL platforms into a common function, and call it during the
> atomic check phase to reduce the amount of stuff we have to do
> during the commit phase. SKL is already doing this.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 82 ++++++++++++++++++++++--------------
>  1 file changed, 50 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2e0106a11f8f..024614cb47b6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3026,6 +3026,43 @@ static u32 i9xx_plane_ctl(const struct intel_crtc_state *crtc_state,
>  	return dspcntr;
>  }
>  
> +static int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
> +{
> +	struct drm_i915_private *dev_priv =
> +		to_i915(plane_state->base.plane->dev);
> +	int src_x = plane_state->base.src.x1 >> 16;
> +	int src_y = plane_state->base.src.y1 >> 16;
> +	u32 offset;
> +
> +	intel_add_fb_offsets(&src_x, &src_y, plane_state, 0);
> +
> +	if (INTEL_GEN(dev_priv) >= 4)
> +		offset = intel_compute_tile_offset(&src_x, &src_y,
> +						   plane_state, 0);
> +	else
> +		offset = 0;
> +
> +	/* HSW+ does this automagically in hardware */
> +	if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv)) {

if (INTEL_GEN() <= 7 && !IS_HASWELL()) {

would match the comment better.

> +		unsigned int rotation = plane_state->base.rotation;
> +		int src_w = drm_rect_width(&plane_state->base.src) >> 16;
> +		int src_h = drm_rect_height(&plane_state->base.src) >> 16;
> +
> +		if (rotation & DRM_ROTATE_180) {
> +			src_x += src_w - 1;
> +			src_y += src_h - 1;
> +		} else if (rotation & DRM_REFLECT_X) {
> +			src_x += src_w - 1;
> +		}
> +	}
> +
> +	plane_state->main.offset = offset;
> +	plane_state->main.x = src_x;
> +	plane_state->main.y = src_y;

plane_state->actual.offset, ->actual.x, ->actual.y ?
plane_state->commit.offset, ->commit.x, ->commit.y ?

Movement looks fine,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/14] drm/i915: Eliminate ironlake_update_primary_plane()

[Chris Wilson]

On Fri, Mar 17, 2017 at 11:18:04PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The effective difference between i9xx_update_primary_plane()
> and ironlake_update_primary_plane() is only the HSW/BDW
> DSPOFFSET special case. So bring that over into
> i9xx_update_primary_plane() and eliminate the duplicated code.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 55 ++++--------------------------------
>  1 file changed, 6 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 024614cb47b6..c7f6bc4e605a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3109,7 +3109,12 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
>  	I915_WRITE_FW(reg, dspcntr);
>  
>  	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
> -	if (INTEL_GEN(dev_priv) >= 4) {
> +	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> +		I915_WRITE_FW(DSPSURF(plane),
> +			      intel_plane_ggtt_offset(plane_state) +
> +			      intel_crtc->dspaddr_offset);
> +		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
> +	} else if (INTEL_GEN(dev_priv) >= 4) {
>  		I915_WRITE_FW(DSPSURF(plane),
>  			      intel_plane_ggtt_offset(plane_state) +
>  			      intel_crtc->dspaddr_offset);
> @@ -3146,48 +3151,6 @@ static void i9xx_disable_primary_plane(struct drm_plane *primary,
>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>  }
>  
> -static void ironlake_update_primary_plane(struct drm_plane *primary,
> -					  const struct intel_crtc_state *crtc_state,
> -					  const struct intel_plane_state *plane_state)
> -{
> -	struct drm_device *dev = primary->dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> -	struct drm_framebuffer *fb = plane_state->base.fb;
> -	int plane = intel_crtc->plane;
> -	u32 linear_offset;
> -	u32 dspcntr = plane_state->ctl;
> -	i915_reg_t reg = DSPCNTR(plane);
> -	int x = plane_state->main.x;
> -	int y = plane_state->main.y;
> -	unsigned long irqflags;
> -
> -	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
> -
> -	intel_crtc->dspaddr_offset = plane_state->main.offset;
> -
> -	intel_crtc->adjusted_x = x;
> -	intel_crtc->adjusted_y = y;
> -
> -	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> -
> -	I915_WRITE_FW(reg, dspcntr);
> -
> -	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
> -	I915_WRITE_FW(DSPSURF(plane),
> -		      intel_plane_ggtt_offset(plane_state) +
> -		      intel_crtc->dspaddr_offset);
> -	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> -		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
> -	} else {
> -		I915_WRITE_FW(DSPTILEOFF(plane), (y << 16) | x);
> -		I915_WRITE_FW(DSPLINOFF(plane), linear_offset);
> -	}
> -	POSTING_READ_FW(reg);
> -
> -	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> -}
> -
>  static u32
>  intel_fb_stride_alignment(const struct drm_framebuffer *fb, int plane)
>  {
> @@ -13642,12 +13605,6 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>  
>  		primary->update_plane = skylake_update_primary_plane;
>  		primary->disable_plane = skylake_disable_primary_plane;
> -	} else if (HAS_PCH_SPLIT(dev_priv)) {
> -		intel_primary_formats = i965_primary_formats;
> -		num_formats = ARRAY_SIZE(i965_primary_formats);
> -
> -		primary->update_plane = ironlake_update_primary_plane;
> -		primary->disable_plane = i9xx_disable_primary_plane;
>  	} else if (INTEL_GEN(dev_priv) >= 4) {

Oh, that looks quite odd, but it checks out.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC][PATCH 14/14] WIP/drm/i915: Nuke posting reads from plane updates

[Chris Wilson]

On Fri, Mar 17, 2017 at 11:18:08PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We'll maybe want just one posting read at the end? This does seem to
> potentially shave a few usec off from the update.

They are all superfluous until proven otherwise. The only cases where I
would (quietly) argue for a post is before a latch (to document the
serialisation of the latch with the controls that go before) and before
completing the transaction and waiting for the hw/irq.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/14] drm/i915: Use skl_plane_ctl() for the SKL "sprite" planes

[Ville Syrjälä]

On Fri, Mar 17, 2017 at 09:48:26PM +0000, Chris Wilson wrote:
> On Fri, Mar 17, 2017 at 11:17:56PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > On SKL the planes are uniform so the "sprites" can use the
> > primary plane code perfectly fine. The only difference we
> > have is the color key handling, but since we never enable that
> > for the primary plane the same code works just fine.
> 
> Rationale works for me. Does setting the color key report an -EINVAL on
> the primary?

Yes.

> Even via the legacy setcolorkey ioctl?

That's actually the only way to set it. We never got around to defining
a property for this stuff.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/14] drm/i915: Use skl_plane_ctl() for the SKL "sprite" planes

[Chris Wilson]

On Mon, Mar 20, 2017 at 06:09:44PM +0200, Ville Syrjälä wrote:
> On Fri, Mar 17, 2017 at 09:48:26PM +0000, Chris Wilson wrote:
> > On Fri, Mar 17, 2017 at 11:17:56PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > On SKL the planes are uniform so the "sprites" can use the
> > > primary plane code perfectly fine. The only difference we
> > > have is the color key handling, but since we never enable that
> > > for the primary plane the same code works just fine.
> > 
> > Rationale works for me. Does setting the color key report an -EINVAL on
> > the primary?
> 
> Yes.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 03/14] drm/i915: Extract i9xx_plane_ctl() and ironlake_plane_ctl()

[ville.syrjala]

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

Pull the code to calculate the pre-SKL primary plane control register
value into separate functions. Allows us to pre-compute it in the
future.

v2: Split the pre-ilk vs. ilk+ unification to a separate patch (Chris)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 104 ++++++++++++++++++++++-------------
 1 file changed, 66 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8a9f1bd21e0e..bd6c1c5469e4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2962,28 +2962,23 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
 	return 0;
 }
 
-static void i9xx_update_primary_plane(struct drm_plane *primary,
-				      const struct intel_crtc_state *crtc_state,
-				      const struct intel_plane_state *plane_state)
+static u32 i9xx_plane_ctl(const struct intel_crtc_state *crtc_state,
+			  const struct intel_plane_state *plane_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(primary->dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_framebuffer *fb = plane_state->base.fb;
-	int plane = intel_crtc->plane;
-	u32 linear_offset;
-	u32 dspcntr;
-	i915_reg_t reg = DSPCNTR(plane);
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
 	unsigned int rotation = plane_state->base.rotation;
-	int x = plane_state->base.src.x1 >> 16;
-	int y = plane_state->base.src.y1 >> 16;
-	unsigned long irqflags;
+	u32 dspcntr;
 
-	dspcntr = DISPPLANE_GAMMA_ENABLE;
+	dspcntr = DISPLAY_PLANE_ENABLE | DISPPLANE_GAMMA_ENABLE;
 
-	dspcntr |= DISPLAY_PLANE_ENABLE;
+	if (IS_G4X(dev_priv))
+		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 
 	if (INTEL_GEN(dev_priv) < 4) {
-		if (intel_crtc->pipe == PIPE_B)
+		if (crtc->pipe == PIPE_B)
 			dspcntr |= DISPPLANE_SEL_PIPE_B;
 	}
 
@@ -3010,7 +3005,8 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 		dspcntr |= DISPPLANE_RGBX101010;
 		break;
 	default:
-		BUG();
+		MISSING_CASE(fb->format->format);
+		return 0;
 	}
 
 	if (INTEL_GEN(dev_priv) >= 4 &&
@@ -3023,8 +3019,26 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	if (rotation & DRM_REFLECT_X)
 		dspcntr |= DISPPLANE_MIRROR;
 
-	if (IS_G4X(dev_priv))
-		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
+	return dspcntr;
+}
+
+static void i9xx_update_primary_plane(struct drm_plane *primary,
+				      const struct intel_crtc_state *crtc_state,
+				      const struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(primary->dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_framebuffer *fb = plane_state->base.fb;
+	int plane = intel_crtc->plane;
+	u32 linear_offset;
+	u32 dspcntr;
+	i915_reg_t reg = DSPCNTR(plane);
+	unsigned int rotation = plane_state->base.rotation;
+	int x = plane_state->base.src.x1 >> 16;
+	int y = plane_state->base.src.y1 >> 16;
+	unsigned long irqflags;
+
+	dspcntr = i9xx_plane_ctl(crtc_state, plane_state);
 
 	intel_add_fb_offsets(&x, &y, plane_state, 0);
 
@@ -3105,25 +3119,19 @@ static void i9xx_disable_primary_plane(struct drm_plane *primary,
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-static void ironlake_update_primary_plane(struct drm_plane *primary,
-					  const struct intel_crtc_state *crtc_state,
-					  const struct intel_plane_state *plane_state)
+static u32 ironlake_plane_ctl(const struct intel_crtc_state *crtc_state,
+			      const struct intel_plane_state *plane_state)
 {
-	struct drm_device *dev = primary->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_framebuffer *fb = plane_state->base.fb;
-	int plane = intel_crtc->plane;
-	u32 linear_offset;
-	u32 dspcntr;
-	i915_reg_t reg = DSPCNTR(plane);
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
 	unsigned int rotation = plane_state->base.rotation;
-	int x = plane_state->base.src.x1 >> 16;
-	int y = plane_state->base.src.y1 >> 16;
-	unsigned long irqflags;
+	u32 dspcntr;
 
-	dspcntr = DISPPLANE_GAMMA_ENABLE;
-	dspcntr |= DISPLAY_PLANE_ENABLE;
+	dspcntr = DISPLAY_PLANE_ENABLE | DISPPLANE_GAMMA_ENABLE;
+
+	if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv))
+		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
 		dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
@@ -3148,7 +3156,8 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
 		dspcntr |= DISPPLANE_RGBX101010;
 		break;
 	default:
-		BUG();
+		MISSING_CASE(fb->format->format);
+		return 0;
 	}
 
 	if (fb->modifier == I915_FORMAT_MOD_X_TILED)
@@ -3157,8 +3166,27 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
 	if (rotation & DRM_ROTATE_180)
 		dspcntr |= DISPPLANE_ROTATE_180;
 
-	if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv))
-		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
+	return dspcntr;
+}
+
+static void ironlake_update_primary_plane(struct drm_plane *primary,
+					  const struct intel_crtc_state *crtc_state,
+					  const struct intel_plane_state *plane_state)
+{
+	struct drm_device *dev = primary->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_framebuffer *fb = plane_state->base.fb;
+	int plane = intel_crtc->plane;
+	u32 linear_offset;
+	u32 dspcntr;
+	i915_reg_t reg = DSPCNTR(plane);
+	unsigned int rotation = plane_state->base.rotation;
+	int x = plane_state->base.src.x1 >> 16;
+	int y = plane_state->base.src.y1 >> 16;
+	unsigned long irqflags;
+
+	dspcntr = ironlake_plane_ctl(crtc_state, plane_state);
 
 	intel_add_fb_offsets(&x, &y, plane_state, 0);
 
-- 
2.10.2

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

[PATCH 03.5/14] drm/i915: Nuke ironlake_plane_ctl()

[ville.syrjala]

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

Share the code to compute the primary plane control register value
between the i9xx and ilk codepaths as the differences are minimal.
Actually there are no differences between g4x and ilk, so the
current split doesn't really make any sense.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 58 ++++--------------------------------
 1 file changed, 6 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bd6c1c5469e4..99b72c4219a8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2974,9 +2974,13 @@ static u32 i9xx_plane_ctl(const struct intel_crtc_state *crtc_state,
 
 	dspcntr = DISPLAY_PLANE_ENABLE | DISPPLANE_GAMMA_ENABLE;
 
-	if (IS_G4X(dev_priv))
+	if (IS_G4X(dev_priv) || IS_GEN5(dev_priv) ||
+	    IS_GEN6(dev_priv) || IS_IVYBRIDGE(dev_priv))
 		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 
+	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+		dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
+
 	if (INTEL_GEN(dev_priv) < 4) {
 		if (crtc->pipe == PIPE_B)
 			dspcntr |= DISPPLANE_SEL_PIPE_B;
@@ -3119,56 +3123,6 @@ static void i9xx_disable_primary_plane(struct drm_plane *primary,
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-static u32 ironlake_plane_ctl(const struct intel_crtc_state *crtc_state,
-			      const struct intel_plane_state *plane_state)
-{
-	struct drm_i915_private *dev_priv =
-		to_i915(plane_state->base.plane->dev);
-	const struct drm_framebuffer *fb = plane_state->base.fb;
-	unsigned int rotation = plane_state->base.rotation;
-	u32 dspcntr;
-
-	dspcntr = DISPLAY_PLANE_ENABLE | DISPPLANE_GAMMA_ENABLE;
-
-	if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv))
-		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
-
-	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-		dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
-
-	switch (fb->format->format) {
-	case DRM_FORMAT_C8:
-		dspcntr |= DISPPLANE_8BPP;
-		break;
-	case DRM_FORMAT_RGB565:
-		dspcntr |= DISPPLANE_BGRX565;
-		break;
-	case DRM_FORMAT_XRGB8888:
-		dspcntr |= DISPPLANE_BGRX888;
-		break;
-	case DRM_FORMAT_XBGR8888:
-		dspcntr |= DISPPLANE_RGBX888;
-		break;
-	case DRM_FORMAT_XRGB2101010:
-		dspcntr |= DISPPLANE_BGRX101010;
-		break;
-	case DRM_FORMAT_XBGR2101010:
-		dspcntr |= DISPPLANE_RGBX101010;
-		break;
-	default:
-		MISSING_CASE(fb->format->format);
-		return 0;
-	}
-
-	if (fb->modifier == I915_FORMAT_MOD_X_TILED)
-		dspcntr |= DISPPLANE_TILED;
-
-	if (rotation & DRM_ROTATE_180)
-		dspcntr |= DISPPLANE_ROTATE_180;
-
-	return dspcntr;
-}
-
 static void ironlake_update_primary_plane(struct drm_plane *primary,
 					  const struct intel_crtc_state *crtc_state,
 					  const struct intel_plane_state *plane_state)
@@ -3186,7 +3140,7 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
 	int y = plane_state->base.src.y1 >> 16;
 	unsigned long irqflags;
 
-	dspcntr = ironlake_plane_ctl(crtc_state, plane_state);
+	dspcntr = i9xx_plane_ctl(crtc_state, plane_state);
 
 	intel_add_fb_offsets(&x, &y, plane_state, 0);
 
-- 
2.10.2

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

✗ Fi.CI.BAT: failure for drm/i915: Moar plane update optimizations (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Moar plane update optimizations (rev3)
URL   : https://patchwork.freedesktop.org/series/21475/
State : failure

== Summary ==

  LD [M]  drivers/net/ethernet/broadcom/genet/genet.o
  LD      drivers/gpu/drm/drm.o
  LD      drivers/thermal/thermal_sys.o
  LD      drivers/misc/built-in.o
  LD      drivers/thermal/built-in.o
  LD      drivers/acpi/acpica/built-in.o
  LD      lib/raid6/raid6_pq.o
  LD [M]  drivers/mmc/core/mmc_block.o
  LD      drivers/pci/pcie/pcieportdrv.o
  LD      drivers/mmc/built-in.o
  LD      net/xfrm/built-in.o
  LD      lib/raid6/built-in.o
  LD      drivers/acpi/built-in.o
  LD      kernel/sched/built-in.o
  LD      drivers/iommu/built-in.o
  LD      kernel/built-in.o
  LD [M]  drivers/usb/serial/usbserial.o
  LD [M]  drivers/net/ethernet/intel/igbvf/igbvf.o
  LD      drivers/base/power/built-in.o
  LD      drivers/spi/built-in.o
  LD      drivers/video/fbdev/core/fb.o
  LD [M]  sound/pci/hda/snd-hda-codec-generic.o
  LD      drivers/tty/serial/8250/8250.o
  LD      drivers/video/fbdev/core/built-in.o
  LD      sound/pci/built-in.o
  LD      drivers/pci/pcie/aer/aerdriver.o
  LD      drivers/usb/gadget/libcomposite.o
  LD      drivers/pci/pcie/aer/built-in.o
drivers/gpu/drm/i915/intel_display.c: In function ‘ironlake_update_primary_plane’:
drivers/gpu/drm/i915/intel_display.c:3187:1: error: expected expression before ‘<<’ token
 <<<<<<< acaa0c0b22fe8c7bf6e7adb2337ba23bce6f9147
 ^
drivers/gpu/drm/i915/intel_display.c:3190:1: error: expected expression before ‘==’ token
 =======
 ^
  LD      net/ipv6/ipv6.o
  LD      drivers/pci/pcie/built-in.o
  LD      sound/built-in.o
  LD      drivers/base/regmap/built-in.o
  LD      net/ipv6/built-in.o
  LD      drivers/scsi/scsi_mod.o
  LD      drivers/base/built-in.o
drivers/gpu/drm/i915/intel_display.c: At top level:
drivers/gpu/drm/i915/intel_display.c:3120:12: error: ‘ironlake_plane_ctl’ defined but not used [-Werror=unused-function]
 static u32 ironlake_plane_ctl(const struct intel_crtc_state *crtc_state,
            ^
  LD      drivers/usb/gadget/udc/udc-core.o
  LD [M]  drivers/net/ethernet/intel/e1000/e1000.o
cc1: all warnings being treated as errors
scripts/Makefile.build:294: recipe for target 'drivers/gpu/drm/i915/intel_display.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_display.o] Error 1
make[4]: *** Waiting for unfinished jobs....
  LD      drivers/usb/gadget/udc/built-in.o
  LD      drivers/usb/gadget/built-in.o
  LD      drivers/video/fbdev/built-in.o
  LD      drivers/pci/built-in.o
  AR      lib/lib.a
  EXPORTS lib/lib-ksyms.o
  LD      net/ipv4/built-in.o
  LD      lib/built-in.o
  LD      drivers/tty/serial/8250/8250_base.o
  LD      drivers/tty/serial/8250/built-in.o
  LD      drivers/video/console/built-in.o
  LD      drivers/video/built-in.o
  LD      drivers/tty/serial/built-in.o
  LD [M]  drivers/net/ethernet/intel/igb/igb.o
  LD      drivers/scsi/sd_mod.o
  LD      fs/btrfs/btrfs.o
  LD      drivers/scsi/built-in.o
  LD      fs/btrfs/built-in.o
  LD      drivers/usb/core/usbcore.o
  LD      drivers/usb/core/built-in.o
  CC      arch/x86/kernel/cpu/capflags.o
  LD      arch/x86/kernel/cpu/built-in.o
  LD      arch/x86/kernel/built-in.o
  LD      drivers/md/md-mod.o
  LD      arch/x86/built-in.o
  LD      drivers/md/built-in.o
  LD      drivers/usb/host/xhci-hcd.o
  LD      fs/ext4/ext4.o
  LD      drivers/tty/vt/built-in.o
  LD      drivers/tty/built-in.o
  LD      fs/ext4/built-in.o
  LD      fs/built-in.o
  LD      drivers/usb/host/built-in.o
  LD      drivers/usb/built-in.o
  LD      net/core/built-in.o
  LD [M]  drivers/net/ethernet/intel/e1000e/e1000e.o
  LD      net/built-in.o
scripts/Makefile.build:553: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:553: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:553: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
make[1]: *** Waiting for unfinished jobs....
  LD      drivers/net/ethernet/built-in.o
  LD      drivers/net/built-in.o
Makefile:1002: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

Re: [PATCH 09/14] drm/i915: Introduce i9xx_check_plane_surface()

[Ville Syrjälä]

On Fri, Mar 17, 2017 at 10:04:32PM +0000, Chris Wilson wrote:
> On Fri, Mar 17, 2017 at 11:18:03PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Extract the primary plane surfae offset/x/y calculations for
> > pre-SKL platforms into a common function, and call it during the
> > atomic check phase to reduce the amount of stuff we have to do
> > during the commit phase. SKL is already doing this.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 82 ++++++++++++++++++++++--------------
> >  1 file changed, 50 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 2e0106a11f8f..024614cb47b6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3026,6 +3026,43 @@ static u32 i9xx_plane_ctl(const struct intel_crtc_state *crtc_state,
> >  	return dspcntr;
> >  }
> >  
> > +static int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
> > +{
> > +	struct drm_i915_private *dev_priv =
> > +		to_i915(plane_state->base.plane->dev);
> > +	int src_x = plane_state->base.src.x1 >> 16;
> > +	int src_y = plane_state->base.src.y1 >> 16;
> > +	u32 offset;
> > +
> > +	intel_add_fb_offsets(&src_x, &src_y, plane_state, 0);
> > +
> > +	if (INTEL_GEN(dev_priv) >= 4)
> > +		offset = intel_compute_tile_offset(&src_x, &src_y,
> > +						   plane_state, 0);
> > +	else
> > +		offset = 0;
> > +
> > +	/* HSW+ does this automagically in hardware */
> > +	if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv)) {
> 
> if (INTEL_GEN() <= 7 && !IS_HASWELL()) {
> 
> would match the comment better.

That would leave out CHV.

I think 'HAS_GMCH || IS_GEN5 || IS_GEN6 || IS_IVB' might be
a semi-decent way to put this. But it's still not quite as
succinct as '!HSW && !BDW'.

What about if I just change the comment to "HSW/BDW do this ..."?

> 
> > +		unsigned int rotation = plane_state->base.rotation;
> > +		int src_w = drm_rect_width(&plane_state->base.src) >> 16;
> > +		int src_h = drm_rect_height(&plane_state->base.src) >> 16;
> > +
> > +		if (rotation & DRM_ROTATE_180) {
> > +			src_x += src_w - 1;
> > +			src_y += src_h - 1;
> > +		} else if (rotation & DRM_REFLECT_X) {
> > +			src_x += src_w - 1;
> > +		}
> > +	}
> > +
> > +	plane_state->main.offset = offset;
> > +	plane_state->main.x = src_x;
> > +	plane_state->main.y = src_y;
> 
> plane_state->actual.offset, ->actual.x, ->actual.y ?
> plane_state->commit.offset, ->commit.x, ->commit.y ?
> 
> Movement looks fine,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/14] drm/i915: Introduce i9xx_check_plane_surface()

[Chris Wilson]

On Mon, Mar 20, 2017 at 07:07:55PM +0200, Ville Syrjälä wrote:
> On Fri, Mar 17, 2017 at 10:04:32PM +0000, Chris Wilson wrote:
> > On Fri, Mar 17, 2017 at 11:18:03PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Extract the primary plane surfae offset/x/y calculations for
> > > pre-SKL platforms into a common function, and call it during the
> > > atomic check phase to reduce the amount of stuff we have to do
> > > during the commit phase. SKL is already doing this.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 82 ++++++++++++++++++++++--------------
> > >  1 file changed, 50 insertions(+), 32 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 2e0106a11f8f..024614cb47b6 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3026,6 +3026,43 @@ static u32 i9xx_plane_ctl(const struct intel_crtc_state *crtc_state,
> > >  	return dspcntr;
> > >  }
> > >  
> > > +static int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
> > > +{
> > > +	struct drm_i915_private *dev_priv =
> > > +		to_i915(plane_state->base.plane->dev);
> > > +	int src_x = plane_state->base.src.x1 >> 16;
> > > +	int src_y = plane_state->base.src.y1 >> 16;
> > > +	u32 offset;
> > > +
> > > +	intel_add_fb_offsets(&src_x, &src_y, plane_state, 0);
> > > +
> > > +	if (INTEL_GEN(dev_priv) >= 4)
> > > +		offset = intel_compute_tile_offset(&src_x, &src_y,
> > > +						   plane_state, 0);
> > > +	else
> > > +		offset = 0;
> > > +
> > > +	/* HSW+ does this automagically in hardware */
> > > +	if (!IS_HASWELL(dev_priv) && !IS_BROADWELL(dev_priv)) {
> > 
> > if (INTEL_GEN() <= 7 && !IS_HASWELL()) {
> > 
> > would match the comment better.
> 
> That would leave out CHV.
> 
> I think 'HAS_GMCH || IS_GEN5 || IS_GEN6 || IS_IVB' might be
> a semi-decent way to put this. But it's still not quite as
> succinct as '!HSW && !BDW'.
> 
> What about if I just change the comment to "HSW/BDW do this ..."?

Prevents me showing my ignorance in that chv isn't include in that set.
Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC][PATCH 14/14] WIP/drm/i915: Nuke posting reads from plane updates

[Maarten Lankhorst]

Op 17-03-17 om 23:12 schreef Chris Wilson:
> On Fri, Mar 17, 2017 at 11:18:08PM +0200, ville.syrjala@linux.intel.com wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> We'll maybe want just one posting read at the end? This does seem to
>> potentially shave a few usec off from the update.
> They are all superfluous until proven otherwise. The only cases where I
> would (quietly) argue for a post is before a latch (to document the
> serialisation of the latch with the controls that go before) and before
> completing the transaction and waiting for the hw/irq.
> -Chris
>
Patch series look sane, but didn't review extensively for errors.

Was considering doing patch 13 myself, but didn't have the courage to.

Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

Re: [PATCH 00/14] drm/i915: Moar plane update optimizations

[Ville Syrjälä]

On Fri, Mar 17, 2017 at 11:17:54PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The plane updates are still taking far too long, at least with
> heavy kernel debug knobs turned on. Using kms_atomic_transitions
> I'm currently seeing numbers as high as 170 usec on my VLV.
> 
> To combat lockdep the most beneficial thing is taking the
> uncore.lock around the entire pipe update. Combined with all
> the other optimizations here I was able to push the max
> duration below 60 usec with my debug kernel.
> 
> The pre-compute stuff isn't supremely important with lockdep/etc.
> turned on, but once those are turned off the few usec saved by
> the other optimizations start to matter. With all the optimization
> and a less debug heavy kernel I was able to get the max duration
> to around 15 usec. It was around 25 usec with the current code.
> 
> Mika was saying that he's still seeing huge numbers (as high
> as 400 usec) with the current drm-tip, and that wasn't even with
> a particularly debug heavy kernel. One theory is that there's
> contention on the uncore.lock. So I'm thinking we may want to
> split the lock into two; one for gt, the other for display. Not
> starving the GPU by hogging the shared lock for display stuff
> might also be a good thing. I've not tried playing with more
> GPU heavy scenarios yet
> 
> Anyways, running out of time to play with this more today so
> I figured I'd post what I have now.
> 
> Series available here:
> git://github.com/vsyrjala/linux.git plane_update_optimization
> 
> Ville Syrjälä (14):
>   drm/i915: Extract skl_plane_ctl()
>   drm/i915: Use skl_plane_ctl() for the SKL "sprite" planes
>   drm/i915: Extract vlv_sprite_ctl()
>   drm/i915: Extract ivb_sprite_ctl()
>   drm/i915: Extract ilk_sprite_ctl()
>   drm/i915: Extract i845_cursor_ctl() and i9xx_cursor_ctl()

I've pushed these to dinq. Thanks for the review.

>   drm/i915: Extract i9xx_plane_ctl()
>   drm/i915: Pre-compute plane control register value
>   drm/i915: Introduce i9xx_check_plane_surface()
>   drm/i915: Eliminate ironlake_update_primary_plane()
>   drm/i915: Use i9xx_check_plane_surface() for sprite planes as well

I'll repost these as new series.

>   drm/i915: Keep vblanks enabled during the entire pipe update
>   WIP/drm/i915: Protect the entire pipe update with uncore.lock
>   WIP/drm/i915: Nuke posting reads from plane updates

And these I'll probably leave out from the next series, and repost
separately later.

> 
>  drivers/gpu/drm/i915/i915_irq.c      |  66 ++++--
>  drivers/gpu/drm/i915/i915_trace.h    |  28 +--
>  drivers/gpu/drm/i915/intel_display.c | 436 +++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  16 +-
>  drivers/gpu/drm/i915/intel_pm.c      |  11 +-
>  drivers/gpu/drm/i915/intel_sprite.c  | 355 ++++++++++++----------------
>  6 files changed, 440 insertions(+), 472 deletions(-)
> 
> -- 
> 2.10.2

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx