[PATCH v2 00/15] drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ (v2)

[PATCH v2 00/15] drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ (v2)

[ville.syrjala]

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

Another iteration of my cursor stuff. I ended up expanding it quite a bit
because I decided that I need to handle the fb/src offsets correctly.

I also tried to address some of Chris's review feedback, mainly about
sharing some of the code between the i9xx and i845/i865 codepaths.

The mess with the conditional register writes was also starting to
bother me, so I tried to make it less convoluted, but spliting it
between just updating CURPOS vs. updating everything. This even seemed
improve the performance a little bit on my old 830.

Entire series is available here:
git://github.com/vsyrjala/linux.git cursor_improvements_5

Ville Syrjälä (15):
  drm/i915: Parametrize cursor/primary pipe select bits
  drm/i915: Pass intel_plane and intel_crtc to plane hooks
  drm/i915: Refactor CURBASE calculation
  drm/i915: Clean up cursor junk from intel_crtc
  drm/i915: Refactor CURPOS calculation
  drm/i915: Move cursor position and base handling into the platform
    specific functions
  drm/i915: Drop useless posting reads from cursor commit
  drm/i915: Split cursor check_plane into i845 and i9xx variants
  drm/i915: Generalize cursor size checks a bit
  drm/i915: Use fb->pitches[0] in cursor code
  drm/i915: Support variable cursor height on ivb+
  drm/i915: Fix gen3 physical cursor alignment requirements
  drm/i915: Handle fb offset and src coordinates for cursors
  drm/i915: Relax 845/865 CURBASE alignemnt requirement to 32 bytes
  drm/i915: Simplify cursor register write sequence

 drivers/gpu/drm/i915/i915_debugfs.c       |  48 +--
 drivers/gpu/drm/i915/i915_drv.h           |   1 +
 drivers/gpu/drm/i915/i915_reg.h           |  12 +-
 drivers/gpu/drm/i915/intel_atomic_plane.c |   6 +-
 drivers/gpu/drm/i915/intel_display.c      | 661 +++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_drv.h          |  17 +-
 drivers/gpu/drm/i915/intel_sprite.c       | 110 +++--
 7 files changed, 454 insertions(+), 401 deletions(-)

-- 
2.10.2

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

[PATCH 01/15] drm/i915: Parametrize cursor/primary pipe select bits

[ville.syrjala]

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

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_reg.h      | 7 ++-----
 drivers/gpu/drm/i915/intel_display.c | 9 +++------
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 04c8f69fcc62..4d97d9fb2ad0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5437,9 +5437,7 @@ enum {
 #define   CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX)
 #define   CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX)
 #define   CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
-#define   MCURSOR_PIPE_SELECT	(1 << 28)
-#define   MCURSOR_PIPE_A	0x00
-#define   MCURSOR_PIPE_B	(1 << 28)
+#define   MCURSOR_PIPE_SELECT(pipe)	((pipe) << 28)
 #define   MCURSOR_GAMMA_ENABLE  (1 << 26)
 #define   CURSOR_ROTATE_180	(1<<15)
 #define   CURSOR_TRICKLE_FEED_DISABLE	(1 << 14)
@@ -5497,8 +5495,7 @@ enum {
 #define   DISPPLANE_PIPE_CSC_ENABLE		(1<<24)
 #define   DISPPLANE_SEL_PIPE_SHIFT		24
 #define   DISPPLANE_SEL_PIPE_MASK		(3<<DISPPLANE_SEL_PIPE_SHIFT)
-#define   DISPPLANE_SEL_PIPE_A			0
-#define   DISPPLANE_SEL_PIPE_B			(1<<DISPPLANE_SEL_PIPE_SHIFT)
+#define   DISPPLANE_SEL_PIPE(pipe)		((pipe)<<DISPPLANE_SEL_PIPE_SHIFT)
 #define   DISPPLANE_SRC_KEY_ENABLE		(1<<22)
 #define   DISPPLANE_SRC_KEY_DISABLE		0
 #define   DISPPLANE_LINE_DOUBLE			(1<<20)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6dc75552c753..4d7bb55ceee3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2981,10 +2981,8 @@ static u32 i9xx_plane_ctl(const struct intel_crtc_state *crtc_state,
 	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;
-	}
+	if (INTEL_GEN(dev_priv) < 4)
+		dspcntr |= DISPPLANE_SEL_PIPE(crtc->pipe);
 
 	switch (fb->format->format) {
 	case DRM_FORMAT_C8:
@@ -9208,7 +9206,6 @@ static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_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;
 	u32 cntl;
 
 	cntl = MCURSOR_GAMMA_ENABLE;
@@ -9216,7 +9213,7 @@ static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
 	if (HAS_DDI(dev_priv))
 		cntl |= CURSOR_PIPE_CSC_ENABLE;
 
-	cntl |= pipe << 28; /* Connect to correct pipe */
+	cntl |= MCURSOR_PIPE_SELECT(crtc->pipe);
 
 	switch (plane_state->base.crtc_w) {
 	case 64:
-- 
2.10.2

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

[PATCH 02/15] drm/i915: Pass intel_plane and intel_crtc to plane hooks

[ville.syrjala]

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

Streamline things by passing intel_plane and intel_crtc instead of
the drm types to our plane hooks.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c |   6 +-
 drivers/gpu/drm/i915/intel_display.c      | 109 ++++++++++++++---------------
 drivers/gpu/drm/i915/intel_drv.h          |   8 +--
 drivers/gpu/drm/i915/intel_sprite.c       | 110 +++++++++++++-----------------
 4 files changed, 104 insertions(+), 129 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index cfb47293fd53..182dc2a36ed1 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -185,7 +185,7 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
 	}
 
 	intel_state->base.visible = false;
-	ret = intel_plane->check_plane(plane, crtc_state, intel_state);
+	ret = intel_plane->check_plane(intel_plane, crtc_state, intel_state);
 	if (ret)
 		return ret;
 
@@ -235,14 +235,14 @@ static void intel_plane_atomic_update(struct drm_plane *plane,
 		trace_intel_update_plane(plane,
 					 to_intel_crtc(crtc));
 
-		intel_plane->update_plane(plane,
+		intel_plane->update_plane(intel_plane,
 					  to_intel_crtc_state(crtc->state),
 					  intel_state);
 	} else {
 		trace_intel_disable_plane(plane,
 					  to_intel_crtc(crtc));
 
-		intel_plane->disable_plane(plane, crtc);
+		intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc));
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4d7bb55ceee3..f737a743bdf6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2750,7 +2750,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 				false);
 	intel_pre_disable_primary_noatomic(&intel_crtc->base);
 	trace_intel_disable_plane(primary, intel_crtc);
-	intel_plane->disable_plane(primary, &intel_crtc->base);
+	intel_plane->disable_plane(intel_plane, intel_crtc);
 
 	return;
 
@@ -3061,14 +3061,14 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
 	return 0;
 }
 
-static void i9xx_update_primary_plane(struct drm_plane *primary,
+static void i9xx_update_primary_plane(struct intel_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;
+	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	enum plane plane = primary->plane;
 	u32 linear_offset;
 	u32 dspcntr = plane_state->ctl;
 	i915_reg_t reg = DSPCNTR(plane);
@@ -3079,12 +3079,12 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
 	if (INTEL_GEN(dev_priv) >= 4)
-		intel_crtc->dspaddr_offset = plane_state->main.offset;
+		crtc->dspaddr_offset = plane_state->main.offset;
 	else
-		intel_crtc->dspaddr_offset = linear_offset;
+		crtc->dspaddr_offset = linear_offset;
 
-	intel_crtc->adjusted_x = x;
-	intel_crtc->adjusted_y = y;
+	crtc->adjusted_x = x;
+	crtc->adjusted_y = y;
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
@@ -3110,31 +3110,29 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
 		I915_WRITE_FW(DSPSURF(plane),
 			      intel_plane_ggtt_offset(plane_state) +
-			      intel_crtc->dspaddr_offset);
+			      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);
+			      crtc->dspaddr_offset);
 		I915_WRITE_FW(DSPTILEOFF(plane), (y << 16) | x);
 		I915_WRITE_FW(DSPLINOFF(plane), linear_offset);
 	} else {
 		I915_WRITE_FW(DSPADDR(plane),
 			      intel_plane_ggtt_offset(plane_state) +
-			      intel_crtc->dspaddr_offset);
+			      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,
-				       struct drm_crtc *crtc)
+static void i9xx_disable_primary_plane(struct intel_plane *primary,
+				       struct intel_crtc *crtc)
 {
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int plane = intel_crtc->plane;
+	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
+	enum plane plane = primary->plane;
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
@@ -3319,16 +3317,15 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
 	return plane_ctl;
 }
 
-static void skylake_update_primary_plane(struct drm_plane *plane,
+static void skylake_update_primary_plane(struct intel_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_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
-	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;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = plane->pipe;
 	u32 plane_ctl = plane_state->ctl;
 	unsigned int rotation = plane_state->base.rotation;
 	u32 stride = skl_plane_stride(fb, 0, rotation);
@@ -3350,10 +3347,10 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	dst_w--;
 	dst_h--;
 
-	intel_crtc->dspaddr_offset = surf_addr;
+	crtc->dspaddr_offset = surf_addr;
 
-	intel_crtc->adjusted_x = src_x;
-	intel_crtc->adjusted_y = src_y;
+	crtc->adjusted_x = src_x;
+	crtc->adjusted_y = src_y;
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
@@ -3392,13 +3389,12 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-static void skylake_disable_primary_plane(struct drm_plane *primary,
-					  struct drm_crtc *crtc)
+static void skylake_disable_primary_plane(struct intel_plane *primary,
+					  struct intel_crtc *crtc)
 {
-	struct drm_device *dev = crtc->dev;
-	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;
+	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
+	enum plane_id plane_id = primary->id;
+	enum pipe pipe = primary->pipe;
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
@@ -3442,7 +3438,7 @@ static void intel_update_primary_planes(struct drm_device *dev)
 			trace_intel_update_plane(&plane->base,
 						 to_intel_crtc(crtc));
 
-			plane->update_plane(&plane->base,
+			plane->update_plane(plane,
 					    to_intel_crtc_state(crtc->state),
 					    plane_state);
 		}
@@ -5095,7 +5091,7 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc, unsigned plane_mask
 	intel_crtc_dpms_overlay_disable(intel_crtc);
 
 	drm_for_each_plane_mask(p, dev, plane_mask)
-		to_intel_plane(p)->disable_plane(p, crtc);
+		to_intel_plane(p)->disable_plane(to_intel_plane(p), intel_crtc);
 
 	/*
 	 * FIXME: Once we grow proper nuclear flip support out of this we need
@@ -13302,11 +13298,11 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state
 }
 
 static int
-intel_check_primary_plane(struct drm_plane *plane,
+intel_check_primary_plane(struct intel_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_i915_private *dev_priv = to_i915(plane->base.dev);
 	struct drm_crtc *crtc = state->base.crtc;
 	int min_scale = DRM_PLANE_HELPER_NO_SCALING;
 	int max_scale = DRM_PLANE_HELPER_NO_SCALING;
@@ -13520,12 +13516,12 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 
 	if (plane->state->visible) {
 		trace_intel_update_plane(plane, to_intel_crtc(crtc));
-		intel_plane->update_plane(plane,
+		intel_plane->update_plane(intel_plane,
 					  to_intel_crtc_state(crtc->state),
 					  to_intel_plane_state(plane->state));
 	} else {
 		trace_intel_disable_plane(plane, to_intel_crtc(crtc));
-		intel_plane->disable_plane(plane, crtc);
+		intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc));
 	}
 
 	intel_cleanup_plane_fb(plane, new_plane_state);
@@ -13669,14 +13665,14 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 }
 
 static int
-intel_check_cursor_plane(struct drm_plane *plane,
+intel_check_cursor_plane(struct intel_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_private *dev_priv = to_i915(plane->base.dev);
+	const 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;
+	enum pipe pipe = plane->pipe;
 	unsigned stride;
 	int ret;
 
@@ -13736,23 +13732,20 @@ intel_check_cursor_plane(struct drm_plane *plane,
 }
 
 static void
-intel_disable_cursor_plane(struct drm_plane *plane,
-			   struct drm_crtc *crtc)
+intel_disable_cursor_plane(struct intel_plane *plane,
+			   struct intel_crtc *crtc)
 {
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-	intel_crtc->cursor_addr = 0;
-	intel_crtc_update_cursor(crtc, NULL);
+	crtc->cursor_addr = 0;
+	intel_crtc_update_cursor(&crtc->base, NULL);
 }
 
 static void
-intel_update_cursor_plane(struct drm_plane *plane,
+intel_update_cursor_plane(struct intel_plane *plane,
 			  const struct intel_crtc_state *crtc_state,
 			  const struct intel_plane_state *state)
 {
-	struct drm_crtc *crtc = crtc_state->base.crtc;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct drm_i915_private *dev_priv = to_i915(plane->dev);
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
 	uint32_t addr;
 
@@ -13763,8 +13756,8 @@ intel_update_cursor_plane(struct drm_plane *plane,
 	else
 		addr = obj->phys_handle->busaddr;
 
-	intel_crtc->cursor_addr = addr;
-	intel_crtc_update_cursor(crtc, state);
+	crtc->cursor_addr = addr;
+	intel_crtc_update_cursor(&crtc->base, state);
 }
 
 static struct intel_plane *
@@ -15177,7 +15170,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 				continue;
 
 			trace_intel_disable_plane(&plane->base, crtc);
-			plane->disable_plane(&plane->base, &crtc->base);
+			plane->disable_plane(plane, crtc);
 		}
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 15e3eb2824e3..a3de17f3058b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -811,12 +811,12 @@ struct intel_plane {
 	 * the intel_plane_state structure and accessed via plane_state.
 	 */
 
-	void (*update_plane)(struct drm_plane *plane,
+	void (*update_plane)(struct intel_plane *plane,
 			     const struct intel_crtc_state *crtc_state,
 			     const struct intel_plane_state *plane_state);
-	void (*disable_plane)(struct drm_plane *plane,
-			      struct drm_crtc *crtc);
-	int (*check_plane)(struct drm_plane *plane,
+	void (*disable_plane)(struct intel_plane *plane,
+			      struct intel_crtc *crtc);
+	int (*check_plane)(struct intel_plane *plane,
 			   struct intel_crtc_state *crtc_state,
 			   struct intel_plane_state *state);
 };
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index f7d431427115..8bf26b34e193 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -207,16 +207,14 @@ void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work
 }
 
 static void
-skl_update_plane(struct drm_plane *drm_plane,
+skl_update_plane(struct intel_plane *plane,
 		 const struct intel_crtc_state *crtc_state,
 		 const struct intel_plane_state *plane_state)
 {
-	struct drm_device *dev = drm_plane->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_plane *intel_plane = to_intel_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;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = plane->pipe;
 	u32 plane_ctl = plane_state->ctl;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	u32 surf_addr = plane_state->main.offset;
@@ -285,13 +283,11 @@ skl_update_plane(struct drm_plane *drm_plane,
 }
 
 static void
-skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
+skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 {
-	struct drm_device *dev = dplane->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_plane *intel_plane = to_intel_plane(dplane);
-	enum plane_id plane_id = intel_plane->id;
-	enum pipe pipe = intel_plane->pipe;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = plane->pipe;
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
@@ -305,10 +301,10 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 }
 
 static void
-chv_update_csc(struct intel_plane *intel_plane, uint32_t format)
+chv_update_csc(struct intel_plane *plane, uint32_t format)
 {
-	struct drm_i915_private *dev_priv = to_i915(intel_plane->base.dev);
-	enum plane_id plane_id = intel_plane->id;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum plane_id plane_id = plane->id;
 
 	/* Seems RGB data bypasses the CSC always */
 	if (!format_is_yuv(format))
@@ -408,16 +404,14 @@ static u32 vlv_sprite_ctl(const struct intel_crtc_state *crtc_state,
 }
 
 static void
-vlv_update_plane(struct drm_plane *dplane,
+vlv_update_plane(struct intel_plane *plane,
 		 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;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	enum pipe pipe = plane->pipe;
+	enum plane_id plane_id = plane->id;
 	u32 sprctl = plane_state->ctl;
 	u32 sprsurf_offset = plane_state->main.offset;
 	u32 linear_offset;
@@ -439,7 +433,7 @@ vlv_update_plane(struct drm_plane *dplane,
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
 	if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B)
-		chv_update_csc(intel_plane, fb->format->format);
+		chv_update_csc(plane, fb->format->format);
 
 	if (key->flags) {
 		I915_WRITE_FW(SPKEYMINVAL(pipe, plane_id), key->min_value);
@@ -466,13 +460,11 @@ vlv_update_plane(struct drm_plane *dplane,
 }
 
 static void
-vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
+vlv_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 {
-	struct drm_device *dev = dplane->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_plane *intel_plane = to_intel_plane(dplane);
-	enum pipe pipe = intel_plane->pipe;
-	enum plane_id plane_id = intel_plane->id;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum pipe pipe = plane->pipe;
+	enum plane_id plane_id = plane->id;
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
@@ -542,15 +534,13 @@ static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
 }
 
 static void
-ivb_update_plane(struct drm_plane *plane,
+ivb_update_plane(struct intel_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;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	enum pipe pipe = plane->pipe;
 	u32 sprctl = plane_state->ctl, sprscale = 0;
 	u32 sprsurf_offset = plane_state->main.offset;
 	u32 linear_offset;
@@ -597,7 +587,7 @@ ivb_update_plane(struct drm_plane *plane,
 		I915_WRITE_FW(SPRLINOFF(pipe), linear_offset);
 
 	I915_WRITE_FW(SPRSIZE(pipe), (crtc_h << 16) | crtc_w);
-	if (intel_plane->can_scale)
+	if (plane->can_scale)
 		I915_WRITE_FW(SPRSCALE(pipe), sprscale);
 	I915_WRITE_FW(SPRCTL(pipe), sprctl);
 	I915_WRITE_FW(SPRSURF(pipe),
@@ -608,19 +598,17 @@ ivb_update_plane(struct drm_plane *plane,
 }
 
 static void
-ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
+ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 {
-	struct drm_device *dev = plane->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_plane *intel_plane = to_intel_plane(plane);
-	int pipe = intel_plane->pipe;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum pipe pipe = 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... */
-	if (intel_plane->can_scale)
+	if (plane->can_scale)
 		I915_WRITE_FW(SPRSCALE(pipe), 0);
 
 	I915_WRITE_FW(SPRSURF(pipe), 0);
@@ -683,15 +671,13 @@ static u32 ilk_sprite_ctl(const struct intel_crtc_state *crtc_state,
 }
 
 static void
-ilk_update_plane(struct drm_plane *plane,
+ilk_update_plane(struct intel_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;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	enum pipe pipe = plane->pipe;
 	u32 dvscntr = plane_state->ctl, dvsscale = 0;
 	u32 dvssurf_offset = plane_state->main.offset;
 	u32 linear_offset;
@@ -744,12 +730,10 @@ ilk_update_plane(struct drm_plane *plane,
 }
 
 static void
-ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
+ilk_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 {
-	struct drm_device *dev = plane->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_plane *intel_plane = to_intel_plane(plane);
-	int pipe = intel_plane->pipe;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum pipe pipe = plane->pipe;
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
@@ -765,14 +749,12 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 }
 
 static int
-intel_check_sprite_plane(struct drm_plane *plane,
+intel_check_sprite_plane(struct intel_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_crtc *crtc = state->base.crtc;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_framebuffer *fb = state->base.fb;
 	int crtc_x, crtc_y;
 	unsigned int crtc_w, crtc_h;
@@ -794,7 +776,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
 	}
 
 	/* Don't modify another pipe's plane */
-	if (intel_plane->pipe != intel_crtc->pipe) {
+	if (plane->pipe != crtc->pipe) {
 		DRM_DEBUG_KMS("Wrong plane <-> crtc mapping\n");
 		return -EINVAL;
 	}
@@ -811,16 +793,16 @@ intel_check_sprite_plane(struct drm_plane *plane,
 		if (state->ckey.flags == I915_SET_COLORKEY_NONE) {
 			can_scale = 1;
 			min_scale = 1;
-			max_scale = skl_max_scale(intel_crtc, crtc_state);
+			max_scale = skl_max_scale(crtc, crtc_state);
 		} else {
 			can_scale = 0;
 			min_scale = DRM_PLANE_HELPER_NO_SCALING;
 			max_scale = DRM_PLANE_HELPER_NO_SCALING;
 		}
 	} else {
-		can_scale = intel_plane->can_scale;
-		max_scale = intel_plane->max_downscale << 16;
-		min_scale = intel_plane->can_scale ? 1 : (1 << 16);
+		can_scale = plane->can_scale;
+		max_scale = plane->max_downscale << 16;
+		min_scale = plane->can_scale ? 1 : (1 << 16);
 	}
 
 	/*
-- 
2.10.2

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

[PATCH v2 03/15] drm/i915: Refactor CURBASE calculation

[ville.syrjala]

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

The remaining cursor base address calculations are spread
around into several different locations. Just pull it all
into one place.

v2: Don't pass intel_plane as we don't really need it

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f737a743bdf6..5e04f64a0f76 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9126,6 +9126,31 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	return active;
 }
 
+static u32 intel_cursor_base(struct intel_crtc *crtc,
+			     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;
+	const struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+	u32 base;
+
+	if (INTEL_INFO(dev_priv)->cursor_needs_physical)
+		base = obj->phys_handle->busaddr;
+	else
+		base = intel_plane_ggtt_offset(plane_state);
+
+	crtc->cursor_addr = base;
+
+	/* ILK+ do this automagically */
+	if (HAS_GMCH_DISPLAY(dev_priv) &&
+	    plane_state->base.rotation & DRM_ROTATE_180)
+		base += (plane_state->base.crtc_h *
+			 plane_state->base.crtc_w - 1) * fb->format->cpp[0];
+
+	return base;
+}
+
 static u32 i845_cursor_ctl(const struct intel_crtc_state *crtc_state,
 			   const struct intel_plane_state *plane_state)
 {
@@ -9265,9 +9290,8 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_crtc->pipe;
-	u32 base = intel_crtc->cursor_addr;
+	u32 pos = 0, base = 0;
 	unsigned long irqflags;
-	u32 pos = 0;
 
 	if (plane_state) {
 		int x = plane_state->base.crtc_x;
@@ -9285,12 +9309,9 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 		}
 		pos |= y << CURSOR_Y_SHIFT;
 
-		/* ILK+ do this automagically */
-		if (HAS_GMCH_DISPLAY(dev_priv) &&
-		    plane_state->base.rotation & DRM_ROTATE_180) {
-			base += (plane_state->base.crtc_h *
-				 plane_state->base.crtc_w - 1) * 4;
-		}
+		base = intel_cursor_base(intel_crtc, plane_state);
+	} else {
+		intel_crtc->cursor_addr = 0;
 	}
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
@@ -13735,7 +13756,6 @@ static void
 intel_disable_cursor_plane(struct intel_plane *plane,
 			   struct intel_crtc *crtc)
 {
-	crtc->cursor_addr = 0;
 	intel_crtc_update_cursor(&crtc->base, NULL);
 }
 
@@ -13744,19 +13764,8 @@ intel_update_cursor_plane(struct intel_plane *plane,
 			  const struct intel_crtc_state *crtc_state,
 			  const struct intel_plane_state *state)
 {
-	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
-	uint32_t addr;
-
-	if (!obj)
-		addr = 0;
-	else if (!INTEL_INFO(dev_priv)->cursor_needs_physical)
-		addr = intel_plane_ggtt_offset(state);
-	else
-		addr = obj->phys_handle->busaddr;
 
-	crtc->cursor_addr = addr;
 	intel_crtc_update_cursor(&crtc->base, state);
 }
 
-- 
2.10.2

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

[PATCH 04/15] drm/i915: Clean up cursor junk from intel_crtc

[ville.syrjala]

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

Move cursor_base, cursor_cntl, and cursor_size from intel_crtc
into intel_plane so that we don't need the crtc for cursor stuff
so much.

Also entirely nuke cursor_addr which IMO doesn't provide any benefit
since it's not actually used by the cursor code itself. I'm not 100%
sure what the SKL+ DDB is code is after by looking at cursor_addr so
I just make it do its checks unconditionally. If that's not correct
then we should likely replace it with somehting like
plane_state->visible.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  | 48 +++++-----------------
 drivers/gpu/drm/i915/intel_display.c | 80 +++++++++++++++---------------------
 drivers/gpu/drm/i915/intel_drv.h     |  9 ++--
 3 files changed, 48 insertions(+), 89 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 316bc47a8eea..b9410cb845f3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3040,36 +3040,6 @@ static void intel_connector_info(struct seq_file *m,
 		intel_seq_print_mode(m, 2, mode);
 }
 
-static bool cursor_active(struct drm_i915_private *dev_priv, int pipe)
-{
-	u32 state;
-
-	if (IS_I845G(dev_priv) || IS_I865G(dev_priv))
-		state = I915_READ(CURCNTR(PIPE_A)) & CURSOR_ENABLE;
-	else
-		state = I915_READ(CURCNTR(pipe)) & CURSOR_MODE;
-
-	return state;
-}
-
-static bool cursor_position(struct drm_i915_private *dev_priv,
-			    int pipe, int *x, int *y)
-{
-	u32 pos;
-
-	pos = I915_READ(CURPOS(pipe));
-
-	*x = (pos >> CURSOR_X_SHIFT) & CURSOR_POS_MASK;
-	if (pos & (CURSOR_POS_SIGN << CURSOR_X_SHIFT))
-		*x = -*x;
-
-	*y = (pos >> CURSOR_Y_SHIFT) & CURSOR_POS_MASK;
-	if (pos & (CURSOR_POS_SIGN << CURSOR_Y_SHIFT))
-		*y = -*y;
-
-	return cursor_active(dev_priv, pipe);
-}
-
 static const char *plane_type(enum drm_plane_type type)
 {
 	switch (type) {
@@ -3191,9 +3161,7 @@ static int i915_display_info(struct seq_file *m, void *unused)
 	seq_printf(m, "CRTC info\n");
 	seq_printf(m, "---------\n");
 	for_each_intel_crtc(dev, crtc) {
-		bool active;
 		struct intel_crtc_state *pipe_config;
-		int x, y;
 
 		drm_modeset_lock(&crtc->base.mutex, NULL);
 		pipe_config = to_intel_crtc_state(crtc->base.state);
@@ -3205,14 +3173,18 @@ static int i915_display_info(struct seq_file *m, void *unused)
 			   yesno(pipe_config->dither), pipe_config->pipe_bpp);
 
 		if (pipe_config->base.active) {
+			struct intel_plane *cursor =
+				to_intel_plane(crtc->base.cursor);
+
 			intel_crtc_info(m, crtc);
 
-			active = cursor_position(dev_priv, crtc->pipe, &x, &y);
-			seq_printf(m, "\tcursor visible? %s, position (%d, %d), size %dx%d, addr 0x%08x, active? %s\n",
-				   yesno(crtc->cursor_base),
-				   x, y, crtc->base.cursor->state->crtc_w,
-				   crtc->base.cursor->state->crtc_h,
-				   crtc->cursor_addr, yesno(active));
+			seq_printf(m, "\tcursor visible? %s, position (%d, %d), size %dx%d, addr 0x%08x\n",
+				   yesno(cursor->base.state->visible),
+				   cursor->base.state->crtc_x,
+				   cursor->base.state->crtc_y,
+				   cursor->base.state->crtc_w,
+				   cursor->base.state->crtc_h,
+				   cursor->cursor.base);
 			intel_scaler_info(m, crtc);
 			intel_plane_info(m, crtc);
 		}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5e04f64a0f76..1d55fac397ad 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9126,8 +9126,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	return active;
 }
 
-static u32 intel_cursor_base(struct intel_crtc *crtc,
-			     const struct intel_plane_state *plane_state)
+static u32 intel_cursor_base(const struct intel_plane_state *plane_state)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(plane_state->base.plane->dev);
@@ -9140,8 +9139,6 @@ static u32 intel_cursor_base(struct intel_crtc *crtc,
 	else
 		base = intel_plane_ggtt_offset(plane_state);
 
-	crtc->cursor_addr = base;
-
 	/* ILK+ do this automagically */
 	if (HAS_GMCH_DISPLAY(dev_priv) &&
 	    plane_state->base.rotation & DRM_ROTATE_180)
@@ -9176,12 +9173,10 @@ static u32 i845_cursor_ctl(const struct intel_crtc_state *crtc_state,
 		CURSOR_STRIDE(stride);
 }
 
-static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
+static void i845_update_cursor(struct intel_plane *plane, u32 base,
 			       const struct intel_plane_state *plane_state)
 {
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	uint32_t cntl = 0, size = 0;
 
 	if (plane_state && plane_state->base.visible) {
@@ -9192,32 +9187,32 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
 		size = (height << 12) | width;
 	}
 
-	if (intel_crtc->cursor_cntl != 0 &&
-	    (intel_crtc->cursor_base != base ||
-	     intel_crtc->cursor_size != size ||
-	     intel_crtc->cursor_cntl != cntl)) {
+	if (plane->cursor.cntl != 0 &&
+	    (plane->cursor.base != base ||
+	     plane->cursor.size != size ||
+	     plane->cursor.cntl != cntl)) {
 		/* On these chipsets we can only modify the base/size/stride
 		 * whilst the cursor is disabled.
 		 */
 		I915_WRITE_FW(CURCNTR(PIPE_A), 0);
 		POSTING_READ_FW(CURCNTR(PIPE_A));
-		intel_crtc->cursor_cntl = 0;
+		plane->cursor.cntl = 0;
 	}
 
-	if (intel_crtc->cursor_base != base) {
+	if (plane->cursor.base != base) {
 		I915_WRITE_FW(CURBASE(PIPE_A), base);
-		intel_crtc->cursor_base = base;
+		plane->cursor.base = base;
 	}
 
-	if (intel_crtc->cursor_size != size) {
+	if (plane->cursor.size != size) {
 		I915_WRITE_FW(CURSIZE, size);
-		intel_crtc->cursor_size = size;
+		plane->cursor.size = size;
 	}
 
-	if (intel_crtc->cursor_cntl != cntl) {
+	if (plane->cursor.cntl != cntl) {
 		I915_WRITE_FW(CURCNTR(PIPE_A), cntl);
 		POSTING_READ_FW(CURCNTR(PIPE_A));
-		intel_crtc->cursor_cntl = cntl;
+		plane->cursor.cntl = cntl;
 	}
 }
 
@@ -9257,39 +9252,36 @@ static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
 	return cntl;
 }
 
-static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
+
+static void i9xx_update_cursor(struct intel_plane *plane, u32 base,
 			       const struct intel_plane_state *plane_state)
 {
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int pipe = intel_crtc->pipe;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum pipe pipe = plane->pipe;
 	uint32_t cntl = 0;
 
 	if (plane_state && plane_state->base.visible)
 		cntl = plane_state->ctl;
 
-	if (intel_crtc->cursor_cntl != cntl) {
+	if (plane->cursor.cntl != cntl) {
 		I915_WRITE_FW(CURCNTR(pipe), cntl);
 		POSTING_READ_FW(CURCNTR(pipe));
-		intel_crtc->cursor_cntl = cntl;
+		plane->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;
+	plane->cursor.base = 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,
+static void intel_crtc_update_cursor(struct intel_plane *plane,
 				     const struct intel_plane_state *plane_state)
 {
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int pipe = intel_crtc->pipe;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum pipe pipe = plane->pipe;
 	u32 pos = 0, base = 0;
 	unsigned long irqflags;
 
@@ -9309,9 +9301,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 		}
 		pos |= y << CURSOR_Y_SHIFT;
 
-		base = intel_cursor_base(intel_crtc, plane_state);
-	} else {
-		intel_crtc->cursor_addr = 0;
+		base = intel_cursor_base(plane_state);
 	}
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
@@ -9319,9 +9309,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(plane, base, plane_state);
 	else
-		i9xx_update_cursor(crtc, base, plane_state);
+		i9xx_update_cursor(plane, base, plane_state);
 
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
@@ -11882,7 +11872,7 @@ static void verify_wm_state(struct drm_crtc *crtc,
 	 * allocation. In that case since the ddb allocation will be updated
 	 * once the plane becomes visible, we can skip this check
 	 */
-	if (intel_crtc->cursor_addr) {
+	if (1) {
 		hw_plane_wm = &hw_wm.planes[PLANE_CURSOR];
 		sw_plane_wm = &sw_wm->planes[PLANE_CURSOR];
 
@@ -13756,7 +13746,7 @@ static void
 intel_disable_cursor_plane(struct intel_plane *plane,
 			   struct intel_crtc *crtc)
 {
-	intel_crtc_update_cursor(&crtc->base, NULL);
+	intel_crtc_update_cursor(plane, NULL);
 }
 
 static void
@@ -13764,9 +13754,7 @@ intel_update_cursor_plane(struct intel_plane *plane,
 			  const struct intel_crtc_state *crtc_state,
 			  const struct intel_plane_state *state)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-
-	intel_crtc_update_cursor(&crtc->base, state);
+	intel_crtc_update_cursor(plane, state);
 }
 
 static struct intel_plane *
@@ -13800,6 +13788,10 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 	cursor->update_plane = intel_update_cursor_plane;
 	cursor->disable_plane = intel_disable_cursor_plane;
 
+	cursor->cursor.base = ~0;
+	cursor->cursor.cntl = ~0;
+	cursor->cursor.size = ~0;
+
 	ret = drm_universal_plane_init(&dev_priv->drm, &cursor->base,
 				       0, &intel_cursor_plane_funcs,
 				       intel_cursor_formats,
@@ -13907,10 +13899,6 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 	intel_crtc->pipe = pipe;
 	intel_crtc->plane = primary->plane;
 
-	intel_crtc->cursor_base = ~0;
-	intel_crtc->cursor_cntl = ~0;
-	intel_crtc->cursor_size = ~0;
-
 	/* initialize shared scalers */
 	intel_crtc_init_scalers(intel_crtc, crtc_state);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a3de17f3058b..b3235de8263b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -760,11 +760,6 @@ struct intel_crtc {
 	int adjusted_x;
 	int adjusted_y;
 
-	uint32_t cursor_addr;
-	uint32_t cursor_cntl;
-	uint32_t cursor_size;
-	uint32_t cursor_base;
-
 	struct intel_crtc_state *config;
 
 	/* global reset count when the last flip was submitted */
@@ -805,6 +800,10 @@ struct intel_plane {
 	int max_downscale;
 	uint32_t frontbuffer_bit;
 
+	struct {
+		u32 base, cntl, size;
+	} cursor;
+
 	/*
 	 * NOTE: Do not place new plane state fields here (e.g., when adding
 	 * new plane properties).  New runtime state should now be placed in
-- 
2.10.2

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

[PATCH v2 05/15] drm/i915: Refactor CURPOS calculation

[ville.syrjala]

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

Move the CURPOS calculations to seprate function. This will allow
sharing the code between the 845/865 vs. others codepaths when we
otherwise split them apart.

v2: Don't pass intel_plane as it's not needed

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1d55fac397ad..ac568756a835 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9148,6 +9148,27 @@ static u32 intel_cursor_base(const struct intel_plane_state *plane_state)
 	return base;
 }
 
+static u32 intel_cursor_position(const struct intel_plane_state *plane_state)
+{
+	int x = plane_state->base.crtc_x;
+	int y = plane_state->base.crtc_y;
+	u32 pos = 0;
+
+	if (x < 0) {
+		pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
+		x = -x;
+	}
+	pos |= x << CURSOR_X_SHIFT;
+
+	if (y < 0) {
+		pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
+		y = -y;
+	}
+	pos |= y << CURSOR_Y_SHIFT;
+
+	return pos;
+}
+
 static u32 i845_cursor_ctl(const struct intel_crtc_state *crtc_state,
 			   const struct intel_plane_state *plane_state)
 {
@@ -9286,22 +9307,8 @@ static void intel_crtc_update_cursor(struct intel_plane *plane,
 	unsigned long irqflags;
 
 	if (plane_state) {
-		int x = plane_state->base.crtc_x;
-		int y = plane_state->base.crtc_y;
-
-		if (x < 0) {
-			pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
-			x = -x;
-		}
-		pos |= x << CURSOR_X_SHIFT;
-
-		if (y < 0) {
-			pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
-			y = -y;
-		}
-		pos |= y << CURSOR_Y_SHIFT;
-
 		base = intel_cursor_base(plane_state);
+		pos = intel_cursor_position(plane_state);
 	}
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-- 
2.10.2

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

[PATCH v2 06/15] drm/i915: Move cursor position and base handling into the platform specific functions

[ville.syrjala]

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

Supposedly on some platforms we can get extra atomicity guarantees for
CURPOS if we write it between the CURCNTR and CURBASE. Let's move the
CURPOS handling into the platform specific hooks to make the possible
without having to pass the calculated CURPOS around. And while at it,
do the same for the CURBASE to avoid passing that either.

v2: Use I915_WRITE_FW() and grab uncore.lock

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ac568756a835..bc0e71fda90c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9194,11 +9194,13 @@ static u32 i845_cursor_ctl(const struct intel_crtc_state *crtc_state,
 		CURSOR_STRIDE(stride);
 }
 
-static void i845_update_cursor(struct intel_plane *plane, u32 base,
+static void i845_update_cursor(struct intel_plane *plane,
+			       const struct intel_crtc_state *crtc_state,
 			       const struct intel_plane_state *plane_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
-	uint32_t cntl = 0, size = 0;
+	u32 cntl = 0, base = 0, pos = 0, size = 0;
+	unsigned long irqflags;
 
 	if (plane_state && plane_state->base.visible) {
 		unsigned int width = plane_state->base.crtc_w;
@@ -9206,8 +9208,13 @@ static void i845_update_cursor(struct intel_plane *plane, u32 base,
 
 		cntl = plane_state->ctl;
 		size = (height << 12) | width;
+
+		base = intel_cursor_base(plane_state);
+		pos = intel_cursor_position(plane_state);
 	}
 
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
 	if (plane->cursor.cntl != 0 &&
 	    (plane->cursor.base != base ||
 	     plane->cursor.size != size ||
@@ -9230,11 +9237,22 @@ static void i845_update_cursor(struct intel_plane *plane, u32 base,
 		plane->cursor.size = size;
 	}
 
+	if (cntl)
+		I915_WRITE_FW(CURPOS(PIPE_A), pos);
+
 	if (plane->cursor.cntl != cntl) {
 		I915_WRITE_FW(CURCNTR(PIPE_A), cntl);
 		POSTING_READ_FW(CURCNTR(PIPE_A));
 		plane->cursor.cntl = cntl;
 	}
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+}
+
+static void i845_disable_cursor(struct intel_plane *plane,
+				struct intel_crtc *crtc)
+{
+	i845_update_cursor(plane, NULL, NULL);
 }
 
 static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
@@ -9273,54 +9291,46 @@ static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
 	return cntl;
 }
 
-
-static void i9xx_update_cursor(struct intel_plane *plane, u32 base,
+static void i9xx_update_cursor(struct intel_plane *plane,
+			       const struct intel_crtc_state *crtc_state,
 			       const struct intel_plane_state *plane_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	enum pipe pipe = plane->pipe;
-	uint32_t cntl = 0;
+	u32 cntl = 0, base = 0, pos = 0;
+	unsigned long irqflags;
 
-	if (plane_state && plane_state->base.visible)
+	if (plane_state && plane_state->base.visible) {
 		cntl = plane_state->ctl;
 
+		base = intel_cursor_base(plane_state);
+		pos = intel_cursor_position(plane_state);
+	}
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
 	if (plane->cursor.cntl != cntl) {
 		I915_WRITE_FW(CURCNTR(pipe), cntl);
 		POSTING_READ_FW(CURCNTR(pipe));
 		plane->cursor.cntl = cntl;
 	}
 
+	if (cntl)
+		I915_WRITE_FW(CURPOS(pipe), pos);
+
 	/* and commit changes on next vblank */
 	I915_WRITE_FW(CURBASE(pipe), base);
 	POSTING_READ_FW(CURBASE(pipe));
 
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+
 	plane->cursor.base = base;
 }
 
-/* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
-static void intel_crtc_update_cursor(struct intel_plane *plane,
-				     const struct intel_plane_state *plane_state)
+static void i9xx_disable_cursor(struct intel_plane *plane,
+				struct intel_crtc *crtc)
 {
-	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
-	enum pipe pipe = plane->pipe;
-	u32 pos = 0, base = 0;
-	unsigned long irqflags;
-
-	if (plane_state) {
-		base = intel_cursor_base(plane_state);
-		pos = intel_cursor_position(plane_state);
-	}
-
-	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(plane, base, plane_state);
-	else
-		i9xx_update_cursor(plane, base, plane_state);
-
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+	i9xx_update_cursor(plane, NULL, NULL);
 }
 
 static bool cursor_size_ok(struct drm_i915_private *dev_priv,
@@ -13749,23 +13759,9 @@ intel_check_cursor_plane(struct intel_plane *plane,
 	return 0;
 }
 
-static void
-intel_disable_cursor_plane(struct intel_plane *plane,
-			   struct intel_crtc *crtc)
-{
-	intel_crtc_update_cursor(plane, NULL);
-}
-
-static void
-intel_update_cursor_plane(struct intel_plane *plane,
-			  const struct intel_crtc_state *crtc_state,
-			  const struct intel_plane_state *state)
-{
-	intel_crtc_update_cursor(plane, state);
-}
-
 static struct intel_plane *
-intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
+intel_cursor_plane_create(struct drm_i915_private *dev_priv,
+			  enum pipe pipe)
 {
 	struct intel_plane *cursor = NULL;
 	struct intel_plane_state *state = NULL;
@@ -13792,8 +13788,14 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 	cursor->id = PLANE_CURSOR;
 	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
 	cursor->check_plane = intel_check_cursor_plane;
-	cursor->update_plane = intel_update_cursor_plane;
-	cursor->disable_plane = intel_disable_cursor_plane;
+
+	if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
+		cursor->update_plane = i845_update_cursor;
+		cursor->disable_plane = i845_disable_cursor;
+	} else {
+		cursor->update_plane = i9xx_update_cursor;
+		cursor->disable_plane = i9xx_disable_cursor;
+	}
 
 	cursor->cursor.base = ~0;
 	cursor->cursor.cntl = ~0;
-- 
2.10.2

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

[PATCH v2 07/15] drm/i915: Drop useless posting reads from cursor commit

[ville.syrjala]

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

There should be no need to do posting reads between all the cursor
register accessess. Let's just drop them.

v2: Rebase due to I915_WRITE_FW() and uncore.lock

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bc0e71fda90c..bba763b03d28 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9223,30 +9223,28 @@ static void i845_update_cursor(struct intel_plane *plane,
 		 * whilst the cursor is disabled.
 		 */
 		I915_WRITE_FW(CURCNTR(PIPE_A), 0);
-		POSTING_READ_FW(CURCNTR(PIPE_A));
 		plane->cursor.cntl = 0;
 	}
 
-	if (plane->cursor.base != base) {
+	if (plane->cursor.base != base)
 		I915_WRITE_FW(CURBASE(PIPE_A), base);
-		plane->cursor.base = base;
-	}
 
-	if (plane->cursor.size != size) {
+	if (plane->cursor.size != size)
 		I915_WRITE_FW(CURSIZE, size);
-		plane->cursor.size = size;
-	}
 
 	if (cntl)
 		I915_WRITE_FW(CURPOS(PIPE_A), pos);
 
-	if (plane->cursor.cntl != cntl) {
+	if (plane->cursor.cntl != cntl)
 		I915_WRITE_FW(CURCNTR(PIPE_A), cntl);
-		POSTING_READ_FW(CURCNTR(PIPE_A));
-		plane->cursor.cntl = cntl;
-	}
+
+	POSTING_READ_FW(CURCNTR(PIPE_A));
 
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+
+	plane->cursor.cntl = cntl;
+	plane->cursor.base = base;
+	plane->cursor.size = size;
 }
 
 static void i845_disable_cursor(struct intel_plane *plane,
@@ -9309,21 +9307,21 @@ static void i9xx_update_cursor(struct intel_plane *plane,
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
-	if (plane->cursor.cntl != cntl) {
+	if (plane->cursor.cntl != cntl)
 		I915_WRITE_FW(CURCNTR(pipe), cntl);
-		POSTING_READ_FW(CURCNTR(pipe));
-		plane->cursor.cntl = cntl;
-	}
 
 	if (cntl)
 		I915_WRITE_FW(CURPOS(pipe), pos);
 
-	/* and commit changes on next vblank */
-	I915_WRITE_FW(CURBASE(pipe), base);
+	if (plane->cursor.cntl != cntl ||
+	    plane->cursor.base != base)
+		I915_WRITE_FW(CURBASE(pipe), base);
+
 	POSTING_READ_FW(CURBASE(pipe));
 
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 
+	plane->cursor.cntl = cntl;
 	plane->cursor.base = base;
 }
 
-- 
2.10.2

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

[PATCH v2 08/15] drm/i915: Split cursor check_plane into i845 and i9xx variants

[ville.syrjala]

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

The 845/865 and 830/855/9xx+ style cursor don't have that
much in common with each other, so let's just split the
.check_plane() hook into two variants as well.

v2: Keep the common stuff in one place (Chris)

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bba763b03d28..774f9668076f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9169,6 +9169,31 @@ static u32 intel_cursor_position(const struct intel_plane_state *plane_state)
 	return pos;
 }
 
+static int intel_check_cursor(struct intel_crtc_state *crtc_state,
+			      struct intel_plane_state *plane_state)
+{
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	int ret;
+
+	ret = drm_plane_helper_check_state(&plane_state->base,
+					   &plane_state->clip,
+					   DRM_PLANE_HELPER_NO_SCALING,
+					   DRM_PLANE_HELPER_NO_SCALING,
+					   true, true);
+	if (ret)
+		return ret;
+
+	if (!fb)
+		return 0;
+
+	if (fb->modifier != DRM_FORMAT_MOD_NONE) {
+		DRM_DEBUG_KMS("cursor cannot be tiled\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static u32 i845_cursor_ctl(const struct intel_crtc_state *crtc_state,
 			   const struct intel_plane_state *plane_state)
 {
@@ -9194,6 +9219,68 @@ static u32 i845_cursor_ctl(const struct intel_crtc_state *crtc_state,
 		CURSOR_STRIDE(stride);
 }
 
+static bool i845_cursor_size_ok(const struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
+	int width = plane_state->base.crtc_w;
+	int height = plane_state->base.crtc_h;
+
+	if (width == 0 || height == 0)
+		return false;
+
+	/*
+	 * 845g/865g are only limited by the width of their cursors,
+	 * the height is arbitrary up to the precision of the register.
+	 */
+	if (!IS_ALIGNED(width, 64))
+		return false;
+
+	if (width > (IS_I845G(dev_priv) ? 64 : 512))
+		return false;
+
+	if (height > 1023)
+		return false;
+
+	return true;
+}
+
+static int i845_check_cursor(struct intel_plane *plane,
+			     struct intel_crtc_state *crtc_state,
+			     struct intel_plane_state *plane_state)
+{
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	const struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+	unsigned int stride;
+	int ret;
+
+	ret = intel_check_cursor(crtc_state, plane_state);
+	if (ret)
+		return ret;
+
+	/* if we want to turn off the cursor ignore width and height */
+	if (!obj)
+		return 0;
+
+	/* Check for which cursor types we support */
+	if (!i845_cursor_size_ok(plane_state)) {
+		DRM_DEBUG("Cursor dimension %dx%d not supported\n",
+			  plane_state->base.crtc_w,
+			  plane_state->base.crtc_h);
+		return -EINVAL;
+	}
+
+	stride = roundup_pow_of_two(plane_state->base.crtc_w) * 4;
+	if (obj->base.size < stride * plane_state->base.crtc_h) {
+		DRM_DEBUG_KMS("buffer is too small\n");
+		return -ENOMEM;
+	}
+
+	plane_state->ctl = i845_cursor_ctl(crtc_state, plane_state);
+
+	return 0;
+}
+
 static void i845_update_cursor(struct intel_plane *plane,
 			       const struct intel_crtc_state *crtc_state,
 			       const struct intel_plane_state *plane_state)
@@ -9289,6 +9376,88 @@ static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
 	return cntl;
 }
 
+static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
+	int width = plane_state->base.crtc_w;
+	int height = plane_state->base.crtc_h;
+
+	if (width == 0 || height == 0)
+		return false;
+
+	/*
+	 * Cursors are limited to a few power-of-two
+	 * sizes, and they must be square.
+	 */
+	switch (width | height) {
+	case 256:
+	case 128:
+		if (IS_GEN2(dev_priv))
+			return false;
+	case 64:
+		break;
+	default:
+		return false;
+	}
+
+	return true;
+}
+
+static int i9xx_check_cursor(struct intel_plane *plane,
+			     struct intel_crtc_state *crtc_state,
+			     struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	const struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+	enum pipe pipe = plane->pipe;
+	unsigned int stride;
+	int ret;
+
+	ret = intel_check_cursor(crtc_state, plane_state);
+	if (ret)
+		return ret;
+
+	/* if we want to turn off the cursor ignore width and height */
+	if (!obj)
+		return 0;
+
+	/* Check for which cursor types we support */
+	if (!i9xx_cursor_size_ok(plane_state)) {
+		DRM_DEBUG("Cursor dimension %dx%d not supported\n",
+			  plane_state->base.crtc_w,
+			  plane_state->base.crtc_h);
+		return -EINVAL;
+	}
+
+	stride = roundup_pow_of_two(plane_state->base.crtc_w) * 4;
+	if (obj->base.size < stride * plane_state->base.crtc_h) {
+		DRM_DEBUG_KMS("buffer is too small\n");
+		return -ENOMEM;
+	}
+
+	/*
+	 * There's something wrong with the cursor on CHV pipe C.
+	 * If it straddles the left edge of the screen then
+	 * moving it away from the edge or disabling it often
+	 * results in a pipe underrun, and often that can lead to
+	 * dead pipe (constant underrun reported, and it scans
+	 * out just a solid color). To recover from that, the
+	 * display power well must be turned off and on again.
+	 * Refuse the put the cursor into that compromised position.
+	 */
+	if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_C &&
+	    plane_state->base.visible && plane_state->base.crtc_x < 0) {
+		DRM_DEBUG_KMS("CHV cursor C not allowed to straddle the left screen edge\n");
+		return -EINVAL;
+	}
+
+	plane_state->ctl = i9xx_cursor_ctl(crtc_state, plane_state);
+
+	return 0;
+}
+
 static void i9xx_update_cursor(struct intel_plane *plane,
 			       const struct intel_crtc_state *crtc_state,
 			       const struct intel_plane_state *plane_state)
@@ -9331,42 +9500,6 @@ static void i9xx_disable_cursor(struct intel_plane *plane,
 	i9xx_update_cursor(plane, NULL, NULL);
 }
 
-static bool cursor_size_ok(struct drm_i915_private *dev_priv,
-			   uint32_t width, uint32_t height)
-{
-	if (width == 0 || height == 0)
-		return false;
-
-	/*
-	 * 845g/865g are special in that they are only limited by
-	 * the width of their cursors, the height is arbitrary up to
-	 * the precision of the register. Everything else requires
-	 * square cursors, limited to a few power-of-two sizes.
-	 */
-	if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
-		if ((width & 63) != 0)
-			return false;
-
-		if (width > (IS_I845G(dev_priv) ? 64 : 512))
-			return false;
-
-		if (height > 1023)
-			return false;
-	} else {
-		switch (width | height) {
-		case 256:
-		case 128:
-			if (IS_GEN2(dev_priv))
-				return false;
-		case 64:
-			break;
-		default:
-			return false;
-		}
-	}
-
-	return true;
-}
 
 /* VESA 640x480x72Hz mode to set on the pipe */
 static struct drm_display_mode load_detect_mode = {
@@ -13690,73 +13823,6 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 	return ERR_PTR(ret);
 }
 
-static int
-intel_check_cursor_plane(struct intel_plane *plane,
-			 struct intel_crtc_state *crtc_state,
-			 struct intel_plane_state *state)
-{
-	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
-	const struct drm_framebuffer *fb = state->base.fb;
-	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-	enum pipe pipe = plane->pipe;
-	unsigned stride;
-	int ret;
-
-	ret = drm_plane_helper_check_state(&state->base,
-					   &state->clip,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   DRM_PLANE_HELPER_NO_SCALING,
-					   true, true);
-	if (ret)
-		return ret;
-
-	/* if we want to turn off the cursor ignore width and height */
-	if (!obj)
-		return 0;
-
-	/* Check for which cursor types we support */
-	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);
-		return -EINVAL;
-	}
-
-	stride = roundup_pow_of_two(state->base.crtc_w) * 4;
-	if (obj->base.size < stride * state->base.crtc_h) {
-		DRM_DEBUG_KMS("buffer is too small\n");
-		return -ENOMEM;
-	}
-
-	if (fb->modifier != DRM_FORMAT_MOD_NONE) {
-		DRM_DEBUG_KMS("cursor cannot be tiled\n");
-		return -EINVAL;
-	}
-
-	/*
-	 * There's something wrong with the cursor on CHV pipe C.
-	 * If it straddles the left edge of the screen then
-	 * moving it away from the edge or disabling it often
-	 * results in a pipe underrun, and often that can lead to
-	 * dead pipe (constant underrun reported, and it scans
-	 * out just a solid color). To recover from that, the
-	 * display power well must be turned off and on again.
-	 * Refuse the put the cursor into that compromised position.
-	 */
-	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;
-}
-
 static struct intel_plane *
 intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 			  enum pipe pipe)
@@ -13785,14 +13851,15 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 	cursor->plane = pipe;
 	cursor->id = PLANE_CURSOR;
 	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
-	cursor->check_plane = intel_check_cursor_plane;
 
 	if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
 		cursor->update_plane = i845_update_cursor;
 		cursor->disable_plane = i845_disable_cursor;
+		cursor->check_plane = i845_check_cursor;
 	} else {
 		cursor->update_plane = i9xx_update_cursor;
 		cursor->disable_plane = i9xx_disable_cursor;
+		cursor->check_plane = i9xx_check_cursor;
 	}
 
 	cursor->cursor.base = ~0;
-- 
2.10.2

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

[PATCH 09/15] drm/i915: Generalize cursor size checks a bit

[ville.syrjala]

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

We have the maximum cursor dimensions stored in the mode_config, so
let's just consult that information instead of hardcoding the same
information in multiple places.

We still need to keep some per-platform checks as the limitations are
quite diverse.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 774f9668076f..4f7a3e3f03e7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9169,6 +9169,17 @@ static u32 intel_cursor_position(const struct intel_plane_state *plane_state)
 	return pos;
 }
 
+static bool intel_cursor_size_ok(const struct intel_plane_state *plane_state)
+{
+	const struct drm_mode_config *config =
+		&plane_state->base.plane->dev->mode_config;
+	int width = plane_state->base.crtc_w;
+	int height = plane_state->base.crtc_h;
+
+	return width > 0 && width <= config->cursor_width &&
+		height > 0 && height <= config->cursor_height;
+}
+
 static int intel_check_cursor(struct intel_crtc_state *crtc_state,
 			      struct intel_plane_state *plane_state)
 {
@@ -9221,28 +9232,13 @@ static u32 i845_cursor_ctl(const struct intel_crtc_state *crtc_state,
 
 static bool i845_cursor_size_ok(const struct intel_plane_state *plane_state)
 {
-	struct drm_i915_private *dev_priv =
-		to_i915(plane_state->base.plane->dev);
 	int width = plane_state->base.crtc_w;
-	int height = plane_state->base.crtc_h;
-
-	if (width == 0 || height == 0)
-		return false;
 
 	/*
 	 * 845g/865g are only limited by the width of their cursors,
 	 * the height is arbitrary up to the precision of the register.
 	 */
-	if (!IS_ALIGNED(width, 64))
-		return false;
-
-	if (width > (IS_I845G(dev_priv) ? 64 : 512))
-		return false;
-
-	if (height > 1023)
-		return false;
-
-	return true;
+	return intel_cursor_size_ok(plane_state) && IS_ALIGNED(width, 64);
 }
 
 static int i845_check_cursor(struct intel_plane *plane,
@@ -9378,12 +9374,10 @@ static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
 
 static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
 {
-	struct drm_i915_private *dev_priv =
-		to_i915(plane_state->base.plane->dev);
 	int width = plane_state->base.crtc_w;
 	int height = plane_state->base.crtc_h;
 
-	if (width == 0 || height == 0)
+	if (!intel_cursor_size_ok(plane_state))
 		return false;
 
 	/*
@@ -9393,8 +9387,6 @@ static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
 	switch (width | height) {
 	case 256:
 	case 128:
-		if (IS_GEN2(dev_priv))
-			return false;
 	case 64:
 		break;
 	default:
-- 
2.10.2

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

[PATCH v2 10/15] drm/i915: Use fb->pitches[0] in cursor code

[ville.syrjala]

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

The cursor code currently ignores fb->pitches[0] (except when creating
the fb itself), and just uses the cursor_width*4 as the stride. Let's
make sure fb->pitches[0] actually matches what we expect it to be.

We can also relax the stride vs. cursor width relationship on 845/865
since the stride is programmed separately. The only constraint is that
width*cpp doesn't exceed the stride, and that's already been checked
by the core since it makes sure the entire plane fits within the fb.

We can also drop the bo size check as that's already checked when
we create the fb. That is the fb is guaranteed to fit within the bo.

v2: Rebase due to i845_cursor_ctl() and i9xx_cursor_ctl()

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4f7a3e3f03e7..c6e026e617e5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9208,26 +9208,12 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
 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;
-	}
+	const struct drm_framebuffer *fb = plane_state->base.fb;
 
 	return CURSOR_ENABLE |
 		CURSOR_GAMMA_ENABLE |
 		CURSOR_FORMAT_ARGB |
-		CURSOR_STRIDE(stride);
+		CURSOR_STRIDE(fb->pitches[0]);
 }
 
 static bool i845_cursor_size_ok(const struct intel_plane_state *plane_state)
@@ -9246,8 +9232,6 @@ static int i845_check_cursor(struct intel_plane *plane,
 			     struct intel_plane_state *plane_state)
 {
 	const struct drm_framebuffer *fb = plane_state->base.fb;
-	const struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-	unsigned int stride;
 	int ret;
 
 	ret = intel_check_cursor(crtc_state, plane_state);
@@ -9255,7 +9239,7 @@ static int i845_check_cursor(struct intel_plane *plane,
 		return ret;
 
 	/* if we want to turn off the cursor ignore width and height */
-	if (!obj)
+	if (!fb)
 		return 0;
 
 	/* Check for which cursor types we support */
@@ -9266,10 +9250,16 @@ static int i845_check_cursor(struct intel_plane *plane,
 		return -EINVAL;
 	}
 
-	stride = roundup_pow_of_two(plane_state->base.crtc_w) * 4;
-	if (obj->base.size < stride * plane_state->base.crtc_h) {
-		DRM_DEBUG_KMS("buffer is too small\n");
-		return -ENOMEM;
+	switch (fb->pitches[0]) {
+	case 256:
+	case 512:
+	case 1024:
+	case 2048:
+		break;
+	default:
+		DRM_DEBUG_KMS("Invalid cursor stride (%u)\n",
+			      fb->pitches[0]);
+		return -EINVAL;
 	}
 
 	plane_state->ctl = i845_cursor_ctl(crtc_state, plane_state);
@@ -9402,9 +9392,7 @@ static int i9xx_check_cursor(struct intel_plane *plane,
 {
 	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	const struct drm_framebuffer *fb = plane_state->base.fb;
-	const struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	enum pipe pipe = plane->pipe;
-	unsigned int stride;
 	int ret;
 
 	ret = intel_check_cursor(crtc_state, plane_state);
@@ -9412,7 +9400,7 @@ static int i9xx_check_cursor(struct intel_plane *plane,
 		return ret;
 
 	/* if we want to turn off the cursor ignore width and height */
-	if (!obj)
+	if (!fb)
 		return 0;
 
 	/* Check for which cursor types we support */
@@ -9423,10 +9411,10 @@ static int i9xx_check_cursor(struct intel_plane *plane,
 		return -EINVAL;
 	}
 
-	stride = roundup_pow_of_two(plane_state->base.crtc_w) * 4;
-	if (obj->base.size < stride * plane_state->base.crtc_h) {
-		DRM_DEBUG_KMS("buffer is too small\n");
-		return -ENOMEM;
+	if (fb->pitches[0] != plane_state->base.crtc_w * fb->format->cpp[0]) {
+		DRM_DEBUG_KMS("Invalid cursor stride (%u) (cursor width %d)\n",
+			      fb->pitches[0], plane_state->base.crtc_w);
+		return -EINVAL;
 	}
 
 	/*
-- 
2.10.2

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

[PATCH v7 11/15] drm/i915: Support variable cursor height on ivb+

[ville.syrjala]

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

IVB introduced the CUR_FBC_CTL register which allows reducing the cursor
height down to 8 lines from the otherwise square cursor dimensions.
Implement support for it. CUR_FBC_CTL can't be used when the cursor
is rotated.

Commandeer the otherwise unused cursor->cursor.size to track the
current value of CUR_FBC_CTL to optimize away redundant CUR_FBC_CTL
writes, and to notice when we need to arm the update via CURBASE if
just CUR_FBC_CTL changes.

v2: Reverse the gen check to make it sane
v3: Only enable CUR_FBC_CTL when cursor is enabled, adapt to
    earlier code changes which means we now actually turn off
    the cursor when we're supposed to unlike v2
v4: Add a comment about rotation vs. CUR_FBC_CTL,
    rebase due to 'dirty' (Chris)
v5: Rebase to the atomic world
    Handle 180 degree rotation
    Add HAS_CUR_FBC()
v6: Rebase
v7: Rebase due to I915_WRITE_FW/uncore.lock
    s/size/fbc_ctl/

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

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 86f097db8ef6..9524ce0442ad 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2927,6 +2927,7 @@ intel_info(const struct drm_i915_private *dev_priv)
 #define HAS_FW_BLC(dev_priv) 	(INTEL_GEN(dev_priv) > 2)
 #define HAS_PIPE_CXSR(dev_priv) ((dev_priv)->info.has_pipe_cxsr)
 #define HAS_FBC(dev_priv)	((dev_priv)->info.has_fbc)
+#define HAS_CUR_FBC(dev_priv)	(!HAS_GMCH_DISPLAY(dev_priv) && INTEL_INFO(dev_priv)->gen >= 7)
 
 #define HAS_IPS(dev_priv)	(IS_HSW_ULT(dev_priv) || IS_BROADWELL(dev_priv))
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 4d97d9fb2ad0..9027debb1cf9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5447,7 +5447,9 @@ enum {
 #define   CURSOR_POS_SIGN       0x8000
 #define   CURSOR_X_SHIFT        0
 #define   CURSOR_Y_SHIFT        16
-#define CURSIZE			_MMIO(0x700a0)
+#define CURSIZE			_MMIO(0x700a0) /* 845/865 */
+#define _CUR_FBC_CTL_A		0x700a0 /* ivb+ */
+#define   CUR_FBC_CTL_EN	(1 << 31)
 #define _CURBCNTR		0x700c0
 #define _CURBBASE		0x700c4
 #define _CURBPOS		0x700c8
@@ -5463,6 +5465,7 @@ enum {
 #define CURCNTR(pipe) _CURSOR2(pipe, _CURACNTR)
 #define CURBASE(pipe) _CURSOR2(pipe, _CURABASE)
 #define CURPOS(pipe) _CURSOR2(pipe, _CURAPOS)
+#define CUR_FBC_CTL(pipe) _CURSOR2(pipe, _CUR_FBC_CTL_A)
 
 #define CURSOR_A_OFFSET 0x70080
 #define CURSOR_B_OFFSET 0x700c0
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c6e026e617e5..53ec9d30437e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9364,17 +9364,16 @@ static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
 
 static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
 {
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
 	int width = plane_state->base.crtc_w;
 	int height = plane_state->base.crtc_h;
 
 	if (!intel_cursor_size_ok(plane_state))
 		return false;
 
-	/*
-	 * Cursors are limited to a few power-of-two
-	 * sizes, and they must be square.
-	 */
-	switch (width | height) {
+	/* Cursor width is limited to a few power-of-two sizes */
+	switch (width) {
 	case 256:
 	case 128:
 	case 64:
@@ -9383,6 +9382,21 @@ static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
 		return false;
 	}
 
+	/*
+	 * IVB+ have CUR_FBC_CTL which allows an arbitrary cursor
+	 * height from 8 lines up to the cursor width, when the
+	 * cursor is not rotated. Everything else requires square
+	 * cursors.
+	 */
+	if (HAS_CUR_FBC(dev_priv) &&
+	    plane_state->base.rotation & DRM_ROTATE_0) {
+		if (height < 8 || height > width)
+			return false;
+	} else {
+		if (height != width)
+			return false;
+	}
+
 	return true;
 }
 
@@ -9444,12 +9458,15 @@ static void i9xx_update_cursor(struct intel_plane *plane,
 {
 	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	enum pipe pipe = plane->pipe;
-	u32 cntl = 0, base = 0, pos = 0;
+	u32 cntl = 0, base = 0, pos = 0, fbc_ctl = 0;
 	unsigned long irqflags;
 
 	if (plane_state && plane_state->base.visible) {
 		cntl = plane_state->ctl;
 
+		if (plane_state->base.crtc_h != plane_state->base.crtc_w)
+			fbc_ctl = CUR_FBC_CTL_EN | (plane_state->base.crtc_h - 1);
+
 		base = intel_cursor_base(plane_state);
 		pos = intel_cursor_position(plane_state);
 	}
@@ -9459,10 +9476,14 @@ static void i9xx_update_cursor(struct intel_plane *plane,
 	if (plane->cursor.cntl != cntl)
 		I915_WRITE_FW(CURCNTR(pipe), cntl);
 
+	if (plane->cursor.size != fbc_ctl)
+		I915_WRITE_FW(CUR_FBC_CTL(pipe), fbc_ctl);
+
 	if (cntl)
 		I915_WRITE_FW(CURPOS(pipe), pos);
 
 	if (plane->cursor.cntl != cntl ||
+	    plane->cursor.size != fbc_ctl ||
 	    plane->cursor.base != base)
 		I915_WRITE_FW(CURBASE(pipe), base);
 
@@ -9472,6 +9493,7 @@ static void i9xx_update_cursor(struct intel_plane *plane,
 
 	plane->cursor.cntl = cntl;
 	plane->cursor.base = base;
+	plane->cursor.size = fbc_ctl;
 }
 
 static void i9xx_disable_cursor(struct intel_plane *plane,
@@ -13844,7 +13866,9 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 
 	cursor->cursor.base = ~0;
 	cursor->cursor.cntl = ~0;
-	cursor->cursor.size = ~0;
+
+	if (IS_I845G(dev_priv) || IS_I865G(dev_priv) || HAS_CUR_FBC(dev_priv))
+		cursor->cursor.size = ~0;
 
 	ret = drm_universal_plane_init(&dev_priv->drm, &cursor->base,
 				       0, &intel_cursor_plane_funcs,
-- 
2.10.2

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

[PATCH 12/15] drm/i915: Fix gen3 physical cursor alignment requirements

[ville.syrjala]

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

Bspec tells us that gen3 platforms need 4KiB alignment for CURBASE
rather than the 256 byte alignment required by i85x. Let's fix that
and pull the code to determine the correct alignment to a helper
function.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 53ec9d30437e..3a1d7d6530ec 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2084,6 +2084,16 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,
 	}
 }
 
+static unsigned int intel_cursor_alignment(const struct drm_i915_private *dev_priv)
+{
+	if (IS_I830(dev_priv))
+		return 16 * 1024;
+	else if (IS_I85X(dev_priv))
+		return 256;
+	else
+		return 4 * 1024;
+}
+
 static unsigned int intel_linear_alignment(const struct drm_i915_private *dev_priv)
 {
 	if (INTEL_INFO(dev_priv)->gen >= 9)
@@ -13329,7 +13339,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 	if (obj) {
 		if (plane->type == DRM_PLANE_TYPE_CURSOR &&
 		    INTEL_INFO(dev_priv)->cursor_needs_physical) {
-			const int align = IS_I830(dev_priv) ? 16 * 1024 : 256;
+			const int align = intel_cursor_alignment(dev_priv);
 
 			ret = i915_gem_object_attach_phys(obj, align);
 			if (ret) {
@@ -13641,7 +13651,7 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 		goto out_free;
 
 	if (INTEL_INFO(dev_priv)->cursor_needs_physical) {
-		int align = IS_I830(dev_priv) ? 16 * 1024 : 256;
+		int align = intel_cursor_alignment(dev_priv);
 
 		ret = i915_gem_object_attach_phys(intel_fb_obj(fb), align);
 		if (ret) {
-- 
2.10.2

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

[PATCH 13/15] drm/i915: Handle fb offset and src coordinates for cursors

[ville.syrjala]

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

The cursor plane doesn't have any kind of source offset register, so
the only form of panning possible is via a the base address register.
The alignment required by CURBASE ranges from 32B to 16KiB depending
on the platform. Let's make sure the user didn't ask for something
we can't do.

Obviously this is impossible to hit via the legacy cursor ioctl since
the src offsets are always 0, but via the plane/atomic ioctls the user
can ask for pretty much anything so we have to deal with this.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3a1d7d6530ec..420d306e31c9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2396,11 +2396,17 @@ u32 intel_compute_tile_offset(int *x, int *y,
 			      const struct intel_plane_state *state,
 			      int plane)
 {
-	const struct drm_i915_private *dev_priv = to_i915(state->base.plane->dev);
+	struct intel_plane *intel_plane = to_intel_plane(state->base.plane);
+	struct drm_i915_private *dev_priv = to_i915(intel_plane->base.dev);
 	const struct drm_framebuffer *fb = state->base.fb;
 	unsigned int rotation = state->base.rotation;
 	int pitch = intel_fb_pitch(fb, plane, rotation);
-	u32 alignment = intel_surf_alignment(fb, plane);
+	u32 alignment;
+
+	if (intel_plane->id == PLANE_CURSOR)
+		alignment = intel_cursor_alignment(dev_priv);
+	else
+		alignment = intel_surf_alignment(fb, plane);
 
 	return _intel_compute_tile_offset(dev_priv, x, y, fb, plane, pitch,
 					  rotation, alignment);
@@ -9149,6 +9155,8 @@ static u32 intel_cursor_base(const struct intel_plane_state *plane_state)
 	else
 		base = intel_plane_ggtt_offset(plane_state);
 
+	base += plane_state->main.offset;
+
 	/* ILK+ do this automagically */
 	if (HAS_GMCH_DISPLAY(dev_priv) &&
 	    plane_state->base.rotation & DRM_ROTATE_180)
@@ -9194,6 +9202,8 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
 			      struct intel_plane_state *plane_state)
 {
 	const struct drm_framebuffer *fb = plane_state->base.fb;
+	int src_x, src_y;
+	u32 offset;
 	int ret;
 
 	ret = drm_plane_helper_check_state(&plane_state->base,
@@ -9212,6 +9222,19 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
 		return -EINVAL;
 	}
 
+	src_x = plane_state->base.src_x >> 16;
+	src_y = plane_state->base.src_y >> 16;
+
+	intel_add_fb_offsets(&src_x, &src_y, plane_state, 0);
+	offset = intel_compute_tile_offset(&src_x, &src_y, plane_state, 0);
+
+	if (src_x != 0 || src_y != 0) {
+		DRM_DEBUG_KMS("Arbitrary cursor panning not supported\n");
+		return -EINVAL;
+	}
+
+	plane_state->main.offset = offset;
+
 	return 0;
 }
 
-- 
2.10.2

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

[PATCH 14/15] drm/i915: Relax 845/865 CURBASE alignemnt requirement to 32 bytes

[ville.syrjala]

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

Supposedly 845/865 require only 32 byte alignment for CURBASE. Let's
relax the checks to allow that instead of demanding 4KiB alignment.
This will allow cursor panning in 8 pixel units.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 420d306e31c9..d78ab0d35274 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2090,6 +2090,8 @@ static unsigned int intel_cursor_alignment(const struct drm_i915_private *dev_pr
 		return 16 * 1024;
 	else if (IS_I85X(dev_priv))
 		return 256;
+	else if (IS_I845G(dev_priv) || IS_I865G(dev_priv))
+		return 32;
 	else
 		return 4 * 1024;
 }
-- 
2.10.2

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

[PATCH 15/15] drm/i915: Simplify cursor register write sequence

[ville.syrjala]

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

It looks like simply writing all the cursor register every single
time might be slightly faster than checking to see of each of
them need to be written. So if any other register apart from
CURPOS needs to be written let's just write all the registers.

CURPOS is left as a special case mainly for 845/865 where we have to
disable the cursor to change many of the cursor parameters. This
introduces a slight chance of the cursor flickering when things get
updated (since we're not currently doing the vblank evade for cursor
updates). If we write CURPOS alone then that obviously can't happen.
And let's follow the same pattern in the i9xx code just for symmetry.
I wasn't able to see a singificant performance difference between
this and just writing all the registers unconditionally.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d78ab0d35274..40ac0f938a4e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9323,36 +9323,28 @@ static void i845_update_cursor(struct intel_plane *plane,
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
-	if (plane->cursor.cntl != 0 &&
-	    (plane->cursor.base != base ||
-	     plane->cursor.size != size ||
-	     plane->cursor.cntl != cntl)) {
-		/* On these chipsets we can only modify the base/size/stride
-		 * whilst the cursor is disabled.
-		 */
+	/* On these chipsets we can only modify the base/size/stride
+	 * whilst the cursor is disabled.
+	 */
+	if (plane->cursor.base != base ||
+	    plane->cursor.size != size ||
+	    plane->cursor.cntl != cntl) {
 		I915_WRITE_FW(CURCNTR(PIPE_A), 0);
-		plane->cursor.cntl = 0;
-	}
-
-	if (plane->cursor.base != base)
 		I915_WRITE_FW(CURBASE(PIPE_A), base);
-
-	if (plane->cursor.size != size)
 		I915_WRITE_FW(CURSIZE, size);
-
-	if (cntl)
 		I915_WRITE_FW(CURPOS(PIPE_A), pos);
-
-	if (plane->cursor.cntl != cntl)
 		I915_WRITE_FW(CURCNTR(PIPE_A), cntl);
 
+		plane->cursor.base = base;
+		plane->cursor.size = size;
+		plane->cursor.cntl = cntl;
+	} else {
+		I915_WRITE_FW(CURPOS(PIPE_A), pos);
+	}
+
 	POSTING_READ_FW(CURCNTR(PIPE_A));
 
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
-
-	plane->cursor.cntl = cntl;
-	plane->cursor.base = base;
-	plane->cursor.size = size;
 }
 
 static void i845_disable_cursor(struct intel_plane *plane,
@@ -9508,27 +9500,34 @@ static void i9xx_update_cursor(struct intel_plane *plane,
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
-	if (plane->cursor.cntl != cntl)
+	/*
+	 * On some platforms writing CURCNTR first will also
+	 * cause CURPOS to be armed by the CURBASE write.
+	 * Without the CURCNTR write the CURPOS write would
+	 * arm itself.
+	 *
+	 * CURCNTR and CUR_FBC_CTL are always
+	 * armed by the CURBASE write only.
+	 */
+	if (plane->cursor.base != base ||
+	    plane->cursor.size != fbc_ctl ||
+	    plane->cursor.cntl != cntl) {
 		I915_WRITE_FW(CURCNTR(pipe), cntl);
-
-	if (plane->cursor.size != fbc_ctl)
-		I915_WRITE_FW(CUR_FBC_CTL(pipe), fbc_ctl);
-
-	if (cntl)
+		if (HAS_CUR_FBC(dev_priv))
+			I915_WRITE_FW(CUR_FBC_CTL(pipe), fbc_ctl);
 		I915_WRITE_FW(CURPOS(pipe), pos);
-
-	if (plane->cursor.cntl != cntl ||
-	    plane->cursor.size != fbc_ctl ||
-	    plane->cursor.base != base)
 		I915_WRITE_FW(CURBASE(pipe), base);
 
+		plane->cursor.base = base;
+		plane->cursor.size = fbc_ctl;
+		plane->cursor.cntl = cntl;
+	} else {
+		I915_WRITE_FW(CURPOS(pipe), pos);
+	}
+
 	POSTING_READ_FW(CURBASE(pipe));
 
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
-
-	plane->cursor.cntl = cntl;
-	plane->cursor.base = base;
-	plane->cursor.size = fbc_ctl;
 }
 
 static void i9xx_disable_cursor(struct intel_plane *plane,
-- 
2.10.2

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

✓ Fi.CI.BAT: success for drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ (rev2)
URL   : https://patchwork.freedesktop.org/series/20835/
State : success

== Summary ==

Series 20835v2 drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+
https://patchwork.freedesktop.org/api/1.0/series/20835/revisions/2/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007
Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-bxt-t5700) fdo#100125

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

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 464s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 460s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 586s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 544s
fi-bxt-t5700     total:278  pass:257  dwarn:1   dfail:0   fail:0   skip:20  time: 574s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 507s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 509s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 439s
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: 437s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 524s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 498s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 484s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 600s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 488s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 603s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 484s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 518s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 467s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 553s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 426s

2876a3da0b960ea1b9df0306b5afb7f7ed565dc7 drm-tip: 2017y-03m-27d-15h-51m-41s UTC integration manifest
1e39f3c drm/i915: Simplify cursor register write sequence
5460609 drm/i915: Relax 845/865 CURBASE alignemnt requirement to 32 bytes
ee0986d drm/i915: Handle fb offset and src coordinates for cursors
7e4dbdd drm/i915: Fix gen3 physical cursor alignment requirements
e86ee83 drm/i915: Support variable cursor height on ivb+
929e3cc drm/i915: Use fb->pitches[0] in cursor code
8095ccc drm/i915: Generalize cursor size checks a bit
cea0a81 drm/i915: Split cursor check_plane into i845 and i9xx variants
7b61c25 drm/i915: Drop useless posting reads from cursor commit
2617d37 drm/i915: Move cursor position and base handling into the platform specific functions
748ba43 drm/i915: Refactor CURPOS calculation
a40946f drm/i915: Clean up cursor junk from intel_crtc
e235ac9 drm/i915: Refactor CURBASE calculation
d0c1deb drm/i915: Pass intel_plane and intel_crtc to plane hooks
3ac25ce drm/i915: Parametrize cursor/primary pipe select bits

== Logs ==

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

Re: [PATCH 04/15] drm/i915: Clean up cursor junk from intel_crtc

[Imre Deak]

On Mon, Mar 27, 2017 at 09:55:35PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Move cursor_base, cursor_cntl, and cursor_size from intel_crtc
> into intel_plane so that we don't need the crtc for cursor stuff
> so much.
> 
> Also entirely nuke cursor_addr which IMO doesn't provide any benefit
> since it's not actually used by the cursor code itself. I'm not 100%
> sure what the SKL+ DDB is code is after by looking at cursor_addr so
> I just make it do its checks unconditionally. If that's not correct
> then we should likely replace it with somehting like
> plane_state->visible.

Yes, AFAICS in case it's not visible the cursor DDB and WM will be still
computed (to fixed minimum and 0 accordingly) and programmed. Maybe
historical left-over? The code comment about this is also stale then.

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

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  | 48 +++++-----------------
>  drivers/gpu/drm/i915/intel_display.c | 80 +++++++++++++++---------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  9 ++--
>  3 files changed, 48 insertions(+), 89 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 316bc47a8eea..b9410cb845f3 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3040,36 +3040,6 @@ static void intel_connector_info(struct seq_file *m,
>  		intel_seq_print_mode(m, 2, mode);
>  }
>  
> -static bool cursor_active(struct drm_i915_private *dev_priv, int pipe)
> -{
> -	u32 state;
> -
> -	if (IS_I845G(dev_priv) || IS_I865G(dev_priv))
> -		state = I915_READ(CURCNTR(PIPE_A)) & CURSOR_ENABLE;
> -	else
> -		state = I915_READ(CURCNTR(pipe)) & CURSOR_MODE;
> -
> -	return state;
> -}
> -
> -static bool cursor_position(struct drm_i915_private *dev_priv,
> -			    int pipe, int *x, int *y)
> -{
> -	u32 pos;
> -
> -	pos = I915_READ(CURPOS(pipe));
> -
> -	*x = (pos >> CURSOR_X_SHIFT) & CURSOR_POS_MASK;
> -	if (pos & (CURSOR_POS_SIGN << CURSOR_X_SHIFT))
> -		*x = -*x;
> -
> -	*y = (pos >> CURSOR_Y_SHIFT) & CURSOR_POS_MASK;
> -	if (pos & (CURSOR_POS_SIGN << CURSOR_Y_SHIFT))
> -		*y = -*y;
> -
> -	return cursor_active(dev_priv, pipe);
> -}
> -
>  static const char *plane_type(enum drm_plane_type type)
>  {
>  	switch (type) {
> @@ -3191,9 +3161,7 @@ static int i915_display_info(struct seq_file *m, void *unused)
>  	seq_printf(m, "CRTC info\n");
>  	seq_printf(m, "---------\n");
>  	for_each_intel_crtc(dev, crtc) {
> -		bool active;
>  		struct intel_crtc_state *pipe_config;
> -		int x, y;
>  
>  		drm_modeset_lock(&crtc->base.mutex, NULL);
>  		pipe_config = to_intel_crtc_state(crtc->base.state);
> @@ -3205,14 +3173,18 @@ static int i915_display_info(struct seq_file *m, void *unused)
>  			   yesno(pipe_config->dither), pipe_config->pipe_bpp);
>  
>  		if (pipe_config->base.active) {
> +			struct intel_plane *cursor =
> +				to_intel_plane(crtc->base.cursor);
> +
>  			intel_crtc_info(m, crtc);
>  
> -			active = cursor_position(dev_priv, crtc->pipe, &x, &y);
> -			seq_printf(m, "\tcursor visible? %s, position (%d, %d), size %dx%d, addr 0x%08x, active? %s\n",
> -				   yesno(crtc->cursor_base),
> -				   x, y, crtc->base.cursor->state->crtc_w,
> -				   crtc->base.cursor->state->crtc_h,
> -				   crtc->cursor_addr, yesno(active));
> +			seq_printf(m, "\tcursor visible? %s, position (%d, %d), size %dx%d, addr 0x%08x\n",
> +				   yesno(cursor->base.state->visible),
> +				   cursor->base.state->crtc_x,
> +				   cursor->base.state->crtc_y,
> +				   cursor->base.state->crtc_w,
> +				   cursor->base.state->crtc_h,
> +				   cursor->cursor.base);
>  			intel_scaler_info(m, crtc);
>  			intel_plane_info(m, crtc);
>  		}
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5e04f64a0f76..1d55fac397ad 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9126,8 +9126,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	return active;
>  }
>  
> -static u32 intel_cursor_base(struct intel_crtc *crtc,
> -			     const struct intel_plane_state *plane_state)
> +static u32 intel_cursor_base(const struct intel_plane_state *plane_state)
>  {
>  	struct drm_i915_private *dev_priv =
>  		to_i915(plane_state->base.plane->dev);
> @@ -9140,8 +9139,6 @@ static u32 intel_cursor_base(struct intel_crtc *crtc,
>  	else
>  		base = intel_plane_ggtt_offset(plane_state);
>  
> -	crtc->cursor_addr = base;
> -
>  	/* ILK+ do this automagically */
>  	if (HAS_GMCH_DISPLAY(dev_priv) &&
>  	    plane_state->base.rotation & DRM_ROTATE_180)
> @@ -9176,12 +9173,10 @@ static u32 i845_cursor_ctl(const struct intel_crtc_state *crtc_state,
>  		CURSOR_STRIDE(stride);
>  }
>  
> -static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
> +static void i845_update_cursor(struct intel_plane *plane, u32 base,
>  			       const struct intel_plane_state *plane_state)
>  {
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>  	uint32_t cntl = 0, size = 0;
>  
>  	if (plane_state && plane_state->base.visible) {
> @@ -9192,32 +9187,32 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
>  		size = (height << 12) | width;
>  	}
>  
> -	if (intel_crtc->cursor_cntl != 0 &&
> -	    (intel_crtc->cursor_base != base ||
> -	     intel_crtc->cursor_size != size ||
> -	     intel_crtc->cursor_cntl != cntl)) {
> +	if (plane->cursor.cntl != 0 &&
> +	    (plane->cursor.base != base ||
> +	     plane->cursor.size != size ||
> +	     plane->cursor.cntl != cntl)) {
>  		/* On these chipsets we can only modify the base/size/stride
>  		 * whilst the cursor is disabled.
>  		 */
>  		I915_WRITE_FW(CURCNTR(PIPE_A), 0);
>  		POSTING_READ_FW(CURCNTR(PIPE_A));
> -		intel_crtc->cursor_cntl = 0;
> +		plane->cursor.cntl = 0;
>  	}
>  
> -	if (intel_crtc->cursor_base != base) {
> +	if (plane->cursor.base != base) {
>  		I915_WRITE_FW(CURBASE(PIPE_A), base);
> -		intel_crtc->cursor_base = base;
> +		plane->cursor.base = base;
>  	}
>  
> -	if (intel_crtc->cursor_size != size) {
> +	if (plane->cursor.size != size) {
>  		I915_WRITE_FW(CURSIZE, size);
> -		intel_crtc->cursor_size = size;
> +		plane->cursor.size = size;
>  	}
>  
> -	if (intel_crtc->cursor_cntl != cntl) {
> +	if (plane->cursor.cntl != cntl) {
>  		I915_WRITE_FW(CURCNTR(PIPE_A), cntl);
>  		POSTING_READ_FW(CURCNTR(PIPE_A));
> -		intel_crtc->cursor_cntl = cntl;
> +		plane->cursor.cntl = cntl;
>  	}
>  }
>  
> @@ -9257,39 +9252,36 @@ static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
>  	return cntl;
>  }
>  
> -static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
> +
> +static void i9xx_update_cursor(struct intel_plane *plane, u32 base,
>  			       const struct intel_plane_state *plane_state)
>  {
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	int pipe = intel_crtc->pipe;
> +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> +	enum pipe pipe = plane->pipe;
>  	uint32_t cntl = 0;
>  
>  	if (plane_state && plane_state->base.visible)
>  		cntl = plane_state->ctl;
>  
> -	if (intel_crtc->cursor_cntl != cntl) {
> +	if (plane->cursor.cntl != cntl) {
>  		I915_WRITE_FW(CURCNTR(pipe), cntl);
>  		POSTING_READ_FW(CURCNTR(pipe));
> -		intel_crtc->cursor_cntl = cntl;
> +		plane->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;
> +	plane->cursor.base = 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,
> +static void intel_crtc_update_cursor(struct intel_plane *plane,
>  				     const struct intel_plane_state *plane_state)
>  {
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	int pipe = intel_crtc->pipe;
> +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> +	enum pipe pipe = plane->pipe;
>  	u32 pos = 0, base = 0;
>  	unsigned long irqflags;
>  
> @@ -9309,9 +9301,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
>  		}
>  		pos |= y << CURSOR_Y_SHIFT;
>  
> -		base = intel_cursor_base(intel_crtc, plane_state);
> -	} else {
> -		intel_crtc->cursor_addr = 0;
> +		base = intel_cursor_base(plane_state);
>  	}
>  
>  	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> @@ -9319,9 +9309,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(plane, base, plane_state);
>  	else
> -		i9xx_update_cursor(crtc, base, plane_state);
> +		i9xx_update_cursor(plane, base, plane_state);
>  
>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>  }
> @@ -11882,7 +11872,7 @@ static void verify_wm_state(struct drm_crtc *crtc,
>  	 * allocation. In that case since the ddb allocation will be updated
>  	 * once the plane becomes visible, we can skip this check
>  	 */
> -	if (intel_crtc->cursor_addr) {
> +	if (1) {
>  		hw_plane_wm = &hw_wm.planes[PLANE_CURSOR];
>  		sw_plane_wm = &sw_wm->planes[PLANE_CURSOR];
>  
> @@ -13756,7 +13746,7 @@ static void
>  intel_disable_cursor_plane(struct intel_plane *plane,
>  			   struct intel_crtc *crtc)
>  {
> -	intel_crtc_update_cursor(&crtc->base, NULL);
> +	intel_crtc_update_cursor(plane, NULL);
>  }
>  
>  static void
> @@ -13764,9 +13754,7 @@ intel_update_cursor_plane(struct intel_plane *plane,
>  			  const struct intel_crtc_state *crtc_state,
>  			  const struct intel_plane_state *state)
>  {
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> -
> -	intel_crtc_update_cursor(&crtc->base, state);
> +	intel_crtc_update_cursor(plane, state);
>  }
>  
>  static struct intel_plane *
> @@ -13800,6 +13788,10 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>  	cursor->update_plane = intel_update_cursor_plane;
>  	cursor->disable_plane = intel_disable_cursor_plane;
>  
> +	cursor->cursor.base = ~0;
> +	cursor->cursor.cntl = ~0;
> +	cursor->cursor.size = ~0;
> +
>  	ret = drm_universal_plane_init(&dev_priv->drm, &cursor->base,
>  				       0, &intel_cursor_plane_funcs,
>  				       intel_cursor_formats,
> @@ -13907,10 +13899,6 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>  	intel_crtc->pipe = pipe;
>  	intel_crtc->plane = primary->plane;
>  
> -	intel_crtc->cursor_base = ~0;
> -	intel_crtc->cursor_cntl = ~0;
> -	intel_crtc->cursor_size = ~0;
> -
>  	/* initialize shared scalers */
>  	intel_crtc_init_scalers(intel_crtc, crtc_state);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a3de17f3058b..b3235de8263b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -760,11 +760,6 @@ struct intel_crtc {
>  	int adjusted_x;
>  	int adjusted_y;
>  
> -	uint32_t cursor_addr;
> -	uint32_t cursor_cntl;
> -	uint32_t cursor_size;
> -	uint32_t cursor_base;
> -
>  	struct intel_crtc_state *config;
>  
>  	/* global reset count when the last flip was submitted */
> @@ -805,6 +800,10 @@ struct intel_plane {
>  	int max_downscale;
>  	uint32_t frontbuffer_bit;
>  
> +	struct {
> +		u32 base, cntl, size;
> +	} cursor;
> +
>  	/*
>  	 * NOTE: Do not place new plane state fields here (e.g., when adding
>  	 * new plane properties).  New runtime state should now be placed in
> -- 
> 2.10.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/15] drm/i915: Generalize cursor size checks a bit

[Imre Deak]

On Mon, Mar 27, 2017 at 09:55:40PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We have the maximum cursor dimensions stored in the mode_config, so
> let's just consult that information instead of hardcoding the same
> information in multiple places.
> 
> We still need to keep some per-platform checks as the limitations are
> quite diverse.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 34 +++++++++++++---------------------
>  1 file changed, 13 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 774f9668076f..4f7a3e3f03e7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9169,6 +9169,17 @@ static u32 intel_cursor_position(const struct intel_plane_state *plane_state)
>  	return pos;
>  }
>  
> +static bool intel_cursor_size_ok(const struct intel_plane_state *plane_state)
> +{
> +	const struct drm_mode_config *config =
> +		&plane_state->base.plane->dev->mode_config;
> +	int width = plane_state->base.crtc_w;
> +	int height = plane_state->base.crtc_h;
> +
> +	return width > 0 && width <= config->cursor_width &&
> +		height > 0 && height <= config->cursor_height;
> +}
> +
>  static int intel_check_cursor(struct intel_crtc_state *crtc_state,
>  			      struct intel_plane_state *plane_state)
>  {
> @@ -9221,28 +9232,13 @@ static u32 i845_cursor_ctl(const struct intel_crtc_state *crtc_state,
>  
>  static bool i845_cursor_size_ok(const struct intel_plane_state *plane_state)
>  {
> -	struct drm_i915_private *dev_priv =
> -		to_i915(plane_state->base.plane->dev);
>  	int width = plane_state->base.crtc_w;
> -	int height = plane_state->base.crtc_h;
> -
> -	if (width == 0 || height == 0)
> -		return false;
>  
>  	/*
>  	 * 845g/865g are only limited by the width of their cursors,
>  	 * the height is arbitrary up to the precision of the register.
>  	 */
> -	if (!IS_ALIGNED(width, 64))
> -		return false;
> -
> -	if (width > (IS_I845G(dev_priv) ? 64 : 512))
> -		return false;
> -
> -	if (height > 1023)
> -		return false;
> -
> -	return true;
> +	return intel_cursor_size_ok(plane_state) && IS_ALIGNED(width, 64);
>  }
>  
>  static int i845_check_cursor(struct intel_plane *plane,
> @@ -9378,12 +9374,10 @@ static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
>  
>  static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
>  {
> -	struct drm_i915_private *dev_priv =
> -		to_i915(plane_state->base.plane->dev);
>  	int width = plane_state->base.crtc_w;
>  	int height = plane_state->base.crtc_h;
>  
> -	if (width == 0 || height == 0)
> +	if (!intel_cursor_size_ok(plane_state))
>  		return false;
>  
>  	/*
> @@ -9393,8 +9387,6 @@ static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
>  	switch (width | height) {
>  	case 256:
>  	case 128:
> -		if (IS_GEN2(dev_priv))
> -			return false;
>  	case 64:
>  		break;
>  	default:
> -- 
> 2.10.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v7 11/15] drm/i915: Support variable cursor height on ivb+

[Imre Deak]

On Mon, Mar 27, 2017 at 09:55:42PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> IVB introduced the CUR_FBC_CTL register which allows reducing the cursor
> height down to 8 lines from the otherwise square cursor dimensions.
> Implement support for it. CUR_FBC_CTL can't be used when the cursor
> is rotated.
> 
> Commandeer the otherwise unused cursor->cursor.size to track the
> current value of CUR_FBC_CTL to optimize away redundant CUR_FBC_CTL
> writes, and to notice when we need to arm the update via CURBASE if
> just CUR_FBC_CTL changes.
> 
> v2: Reverse the gen check to make it sane
> v3: Only enable CUR_FBC_CTL when cursor is enabled, adapt to
>     earlier code changes which means we now actually turn off
>     the cursor when we're supposed to unlike v2
> v4: Add a comment about rotation vs. CUR_FBC_CTL,
>     rebase due to 'dirty' (Chris)
> v5: Rebase to the atomic world
>     Handle 180 degree rotation
>     Add HAS_CUR_FBC()
> v6: Rebase
> v7: Rebase due to I915_WRITE_FW/uncore.lock
>     s/size/fbc_ctl/
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Nitpick: cursor.size could've been changed to be a union of size,fb_ctl
too. Either way looks ok:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/i915_reg.h      |  5 ++++-
>  drivers/gpu/drm/i915/intel_display.c | 38 +++++++++++++++++++++++++++++-------
>  3 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 86f097db8ef6..9524ce0442ad 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2927,6 +2927,7 @@ intel_info(const struct drm_i915_private *dev_priv)
>  #define HAS_FW_BLC(dev_priv) 	(INTEL_GEN(dev_priv) > 2)
>  #define HAS_PIPE_CXSR(dev_priv) ((dev_priv)->info.has_pipe_cxsr)
>  #define HAS_FBC(dev_priv)	((dev_priv)->info.has_fbc)
> +#define HAS_CUR_FBC(dev_priv)	(!HAS_GMCH_DISPLAY(dev_priv) && INTEL_INFO(dev_priv)->gen >= 7)
>  
>  #define HAS_IPS(dev_priv)	(IS_HSW_ULT(dev_priv) || IS_BROADWELL(dev_priv))
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4d97d9fb2ad0..9027debb1cf9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5447,7 +5447,9 @@ enum {
>  #define   CURSOR_POS_SIGN       0x8000
>  #define   CURSOR_X_SHIFT        0
>  #define   CURSOR_Y_SHIFT        16
> -#define CURSIZE			_MMIO(0x700a0)
> +#define CURSIZE			_MMIO(0x700a0) /* 845/865 */
> +#define _CUR_FBC_CTL_A		0x700a0 /* ivb+ */
> +#define   CUR_FBC_CTL_EN	(1 << 31)
>  #define _CURBCNTR		0x700c0
>  #define _CURBBASE		0x700c4
>  #define _CURBPOS		0x700c8
> @@ -5463,6 +5465,7 @@ enum {
>  #define CURCNTR(pipe) _CURSOR2(pipe, _CURACNTR)
>  #define CURBASE(pipe) _CURSOR2(pipe, _CURABASE)
>  #define CURPOS(pipe) _CURSOR2(pipe, _CURAPOS)
> +#define CUR_FBC_CTL(pipe) _CURSOR2(pipe, _CUR_FBC_CTL_A)
>  
>  #define CURSOR_A_OFFSET 0x70080
>  #define CURSOR_B_OFFSET 0x700c0
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c6e026e617e5..53ec9d30437e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9364,17 +9364,16 @@ static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
>  
>  static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
>  {
> +	struct drm_i915_private *dev_priv =
> +		to_i915(plane_state->base.plane->dev);
>  	int width = plane_state->base.crtc_w;
>  	int height = plane_state->base.crtc_h;
>  
>  	if (!intel_cursor_size_ok(plane_state))
>  		return false;
>  
> -	/*
> -	 * Cursors are limited to a few power-of-two
> -	 * sizes, and they must be square.
> -	 */
> -	switch (width | height) {
> +	/* Cursor width is limited to a few power-of-two sizes */
> +	switch (width) {
>  	case 256:
>  	case 128:
>  	case 64:
> @@ -9383,6 +9382,21 @@ static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
>  		return false;
>  	}
>  
> +	/*
> +	 * IVB+ have CUR_FBC_CTL which allows an arbitrary cursor
> +	 * height from 8 lines up to the cursor width, when the
> +	 * cursor is not rotated. Everything else requires square
> +	 * cursors.
> +	 */
> +	if (HAS_CUR_FBC(dev_priv) &&
> +	    plane_state->base.rotation & DRM_ROTATE_0) {
> +		if (height < 8 || height > width)
> +			return false;
> +	} else {
> +		if (height != width)
> +			return false;
> +	}
> +
>  	return true;
>  }
>  
> @@ -9444,12 +9458,15 @@ static void i9xx_update_cursor(struct intel_plane *plane,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>  	enum pipe pipe = plane->pipe;
> -	u32 cntl = 0, base = 0, pos = 0;
> +	u32 cntl = 0, base = 0, pos = 0, fbc_ctl = 0;
>  	unsigned long irqflags;
>  
>  	if (plane_state && plane_state->base.visible) {
>  		cntl = plane_state->ctl;
>  
> +		if (plane_state->base.crtc_h != plane_state->base.crtc_w)
> +			fbc_ctl = CUR_FBC_CTL_EN | (plane_state->base.crtc_h - 1);
> +
>  		base = intel_cursor_base(plane_state);
>  		pos = intel_cursor_position(plane_state);
>  	}
> @@ -9459,10 +9476,14 @@ static void i9xx_update_cursor(struct intel_plane *plane,
>  	if (plane->cursor.cntl != cntl)
>  		I915_WRITE_FW(CURCNTR(pipe), cntl);
>  
> +	if (plane->cursor.size != fbc_ctl)
> +		I915_WRITE_FW(CUR_FBC_CTL(pipe), fbc_ctl);
> +
>  	if (cntl)
>  		I915_WRITE_FW(CURPOS(pipe), pos);
>  
>  	if (plane->cursor.cntl != cntl ||
> +	    plane->cursor.size != fbc_ctl ||
>  	    plane->cursor.base != base)
>  		I915_WRITE_FW(CURBASE(pipe), base);
>  
> @@ -9472,6 +9493,7 @@ static void i9xx_update_cursor(struct intel_plane *plane,
>  
>  	plane->cursor.cntl = cntl;
>  	plane->cursor.base = base;
> +	plane->cursor.size = fbc_ctl;
>  }
>  
>  static void i9xx_disable_cursor(struct intel_plane *plane,
> @@ -13844,7 +13866,9 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
>  
>  	cursor->cursor.base = ~0;
>  	cursor->cursor.cntl = ~0;
> -	cursor->cursor.size = ~0;
> +
> +	if (IS_I845G(dev_priv) || IS_I865G(dev_priv) || HAS_CUR_FBC(dev_priv))
> +		cursor->cursor.size = ~0;
>  
>  	ret = drm_universal_plane_init(&dev_priv->drm, &cursor->base,
>  				       0, &intel_cursor_plane_funcs,
> -- 
> 2.10.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 12/15] drm/i915: Fix gen3 physical cursor alignment requirements

[Imre Deak]

On Mon, Mar 27, 2017 at 09:55:43PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Bspec tells us that gen3 platforms need 4KiB alignment for CURBASE
> rather than the 256 byte alignment required by i85x. Let's fix that
> and pull the code to determine the correct alignment to a helper
> function.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 53ec9d30437e..3a1d7d6530ec 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2084,6 +2084,16 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,
>  	}
>  }
>  
> +static unsigned int intel_cursor_alignment(const struct drm_i915_private *dev_priv)
> +{
> +	if (IS_I830(dev_priv))
> +		return 16 * 1024;
> +	else if (IS_I85X(dev_priv))
> +		return 256;
> +	else
> +		return 4 * 1024;
> +}
> +
>  static unsigned int intel_linear_alignment(const struct drm_i915_private *dev_priv)
>  {
>  	if (INTEL_INFO(dev_priv)->gen >= 9)
> @@ -13329,7 +13339,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>  	if (obj) {
>  		if (plane->type == DRM_PLANE_TYPE_CURSOR &&
>  		    INTEL_INFO(dev_priv)->cursor_needs_physical) {
> -			const int align = IS_I830(dev_priv) ? 16 * 1024 : 256;
> +			const int align = intel_cursor_alignment(dev_priv);
>  
>  			ret = i915_gem_object_attach_phys(obj, align);
>  			if (ret) {
> @@ -13641,7 +13651,7 @@ intel_legacy_cursor_update(struct drm_plane *plane,
>  		goto out_free;
>  
>  	if (INTEL_INFO(dev_priv)->cursor_needs_physical) {
> -		int align = IS_I830(dev_priv) ? 16 * 1024 : 256;
> +		int align = intel_cursor_alignment(dev_priv);
>  
>  		ret = i915_gem_object_attach_phys(intel_fb_obj(fb), align);
>  		if (ret) {
> -- 
> 2.10.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 13/15] drm/i915: Handle fb offset and src coordinates for cursors

[Imre Deak]

On Mon, Mar 27, 2017 at 09:55:44PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The cursor plane doesn't have any kind of source offset register, so
> the only form of panning possible is via a the base address register.
> The alignment required by CURBASE ranges from 32B to 16KiB depending
> on the platform. Let's make sure the user didn't ask for something
> we can't do.
> 
> Obviously this is impossible to hit via the legacy cursor ioctl since
> the src offsets are always 0, but via the plane/atomic ioctls the user
> can ask for pretty much anything so we have to deal with this.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3a1d7d6530ec..420d306e31c9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2396,11 +2396,17 @@ u32 intel_compute_tile_offset(int *x, int *y,
>  			      const struct intel_plane_state *state,
>  			      int plane)
>  {
> -	const struct drm_i915_private *dev_priv = to_i915(state->base.plane->dev);
> +	struct intel_plane *intel_plane = to_intel_plane(state->base.plane);
> +	struct drm_i915_private *dev_priv = to_i915(intel_plane->base.dev);
>  	const struct drm_framebuffer *fb = state->base.fb;
>  	unsigned int rotation = state->base.rotation;
>  	int pitch = intel_fb_pitch(fb, plane, rotation);
> -	u32 alignment = intel_surf_alignment(fb, plane);
> +	u32 alignment;
> +
> +	if (intel_plane->id == PLANE_CURSOR)
> +		alignment = intel_cursor_alignment(dev_priv);
> +	else
> +		alignment = intel_surf_alignment(fb, plane);
>  
>  	return _intel_compute_tile_offset(dev_priv, x, y, fb, plane, pitch,
>  					  rotation, alignment);
> @@ -9149,6 +9155,8 @@ static u32 intel_cursor_base(const struct intel_plane_state *plane_state)
>  	else
>  		base = intel_plane_ggtt_offset(plane_state);
>  
> +	base += plane_state->main.offset;
> +
>  	/* ILK+ do this automagically */
>  	if (HAS_GMCH_DISPLAY(dev_priv) &&
>  	    plane_state->base.rotation & DRM_ROTATE_180)
> @@ -9194,6 +9202,8 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
>  			      struct intel_plane_state *plane_state)
>  {
>  	const struct drm_framebuffer *fb = plane_state->base.fb;
> +	int src_x, src_y;
> +	u32 offset;
>  	int ret;
>  
>  	ret = drm_plane_helper_check_state(&plane_state->base,
> @@ -9212,6 +9222,19 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
>  		return -EINVAL;
>  	}
>  
> +	src_x = plane_state->base.src_x >> 16;
> +	src_y = plane_state->base.src_y >> 16;
> +
> +	intel_add_fb_offsets(&src_x, &src_y, plane_state, 0);
> +	offset = intel_compute_tile_offset(&src_x, &src_y, plane_state, 0);
> +
> +	if (src_x != 0 || src_y != 0) {
> +		DRM_DEBUG_KMS("Arbitrary cursor panning not supported\n");
> +		return -EINVAL;
> +	}
> +
> +	plane_state->main.offset = offset;
> +
>  	return 0;
>  }
>  
> -- 
> 2.10.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 14/15] drm/i915: Relax 845/865 CURBASE alignemnt requirement to 32 bytes

[Imre Deak]

On Mon, Mar 27, 2017 at 09:55:45PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Supposedly 845/865 require only 32 byte alignment for CURBASE. Let's
> relax the checks to allow that instead of demanding 4KiB alignment.
> This will allow cursor panning in 8 pixel units.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 420d306e31c9..d78ab0d35274 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2090,6 +2090,8 @@ static unsigned int intel_cursor_alignment(const struct drm_i915_private *dev_pr
>  		return 16 * 1024;
>  	else if (IS_I85X(dev_priv))
>  		return 256;
> +	else if (IS_I845G(dev_priv) || IS_I865G(dev_priv))
> +		return 32;
>  	else
>  		return 4 * 1024;
>  }
> -- 
> 2.10.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 15/15] drm/i915: Simplify cursor register write sequence

[Imre Deak]

On Mon, Mar 27, 2017 at 09:55:46PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> It looks like simply writing all the cursor register every single
> time might be slightly faster than checking to see of each of
> them need to be written. So if any other register apart from
> CURPOS needs to be written let's just write all the registers.
> 
> CURPOS is left as a special case mainly for 845/865 where we have to
> disable the cursor to change many of the cursor parameters. This
> introduces a slight chance of the cursor flickering when things get
> updated (since we're not currently doing the vblank evade for cursor
> updates). If we write CURPOS alone then that obviously can't happen.
> And let's follow the same pattern in the i9xx code just for symmetry.
> I wasn't able to see a singificant performance difference between
> this and just writing all the registers unconditionally.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 69 ++++++++++++++++++------------------
>  1 file changed, 34 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d78ab0d35274..40ac0f938a4e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9323,36 +9323,28 @@ static void i845_update_cursor(struct intel_plane *plane,
>  
>  	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>  
> -	if (plane->cursor.cntl != 0 &&
> -	    (plane->cursor.base != base ||
> -	     plane->cursor.size != size ||
> -	     plane->cursor.cntl != cntl)) {
> -		/* On these chipsets we can only modify the base/size/stride
> -		 * whilst the cursor is disabled.
> -		 */
> +	/* On these chipsets we can only modify the base/size/stride
> +	 * whilst the cursor is disabled.
> +	 */
> +	if (plane->cursor.base != base ||
> +	    plane->cursor.size != size ||
> +	    plane->cursor.cntl != cntl) {
>  		I915_WRITE_FW(CURCNTR(PIPE_A), 0);
> -		plane->cursor.cntl = 0;
> -	}
> -
> -	if (plane->cursor.base != base)
>  		I915_WRITE_FW(CURBASE(PIPE_A), base);
> -
> -	if (plane->cursor.size != size)
>  		I915_WRITE_FW(CURSIZE, size);
> -
> -	if (cntl)
>  		I915_WRITE_FW(CURPOS(PIPE_A), pos);
> -
> -	if (plane->cursor.cntl != cntl)
>  		I915_WRITE_FW(CURCNTR(PIPE_A), cntl);
>  
> +		plane->cursor.base = base;
> +		plane->cursor.size = size;
> +		plane->cursor.cntl = cntl;
> +	} else {
> +		I915_WRITE_FW(CURPOS(PIPE_A), pos);
> +	}
> +
>  	POSTING_READ_FW(CURCNTR(PIPE_A));
>  
>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> -
> -	plane->cursor.cntl = cntl;
> -	plane->cursor.base = base;
> -	plane->cursor.size = size;
>  }
>  
>  static void i845_disable_cursor(struct intel_plane *plane,
> @@ -9508,27 +9500,34 @@ static void i9xx_update_cursor(struct intel_plane *plane,
>  
>  	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>  
> -	if (plane->cursor.cntl != cntl)
> +	/*
> +	 * On some platforms writing CURCNTR first will also
> +	 * cause CURPOS to be armed by the CURBASE write.
> +	 * Without the CURCNTR write the CURPOS write would
> +	 * arm itself.
> +	 *
> +	 * CURCNTR and CUR_FBC_CTL are always
> +	 * armed by the CURBASE write only.
> +	 */
> +	if (plane->cursor.base != base ||
> +	    plane->cursor.size != fbc_ctl ||
> +	    plane->cursor.cntl != cntl) {
>  		I915_WRITE_FW(CURCNTR(pipe), cntl);
> -
> -	if (plane->cursor.size != fbc_ctl)
> -		I915_WRITE_FW(CUR_FBC_CTL(pipe), fbc_ctl);
> -
> -	if (cntl)
> +		if (HAS_CUR_FBC(dev_priv))
> +			I915_WRITE_FW(CUR_FBC_CTL(pipe), fbc_ctl);
>  		I915_WRITE_FW(CURPOS(pipe), pos);
> -
> -	if (plane->cursor.cntl != cntl ||
> -	    plane->cursor.size != fbc_ctl ||
> -	    plane->cursor.base != base)
>  		I915_WRITE_FW(CURBASE(pipe), base);
>  
> +		plane->cursor.base = base;
> +		plane->cursor.size = fbc_ctl;
> +		plane->cursor.cntl = cntl;
> +	} else {
> +		I915_WRITE_FW(CURPOS(pipe), pos);
> +	}
> +
>  	POSTING_READ_FW(CURBASE(pipe));
>  
>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> -
> -	plane->cursor.cntl = cntl;
> -	plane->cursor.base = base;
> -	plane->cursor.size = fbc_ctl;
>  }
>  
>  static void i9xx_disable_cursor(struct intel_plane *plane,
> -- 
> 2.10.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 15/15] drm/i915: Simplify cursor register write sequence

[Ville Syrjälä]

On Sat, May 06, 2017 at 12:31:50AM +0300, Imre Deak wrote:
> On Mon, Mar 27, 2017 at 09:55:46PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > It looks like simply writing all the cursor register every single
> > time might be slightly faster than checking to see of each of
> > them need to be written. So if any other register apart from
> > CURPOS needs to be written let's just write all the registers.
> > 
> > CURPOS is left as a special case mainly for 845/865 where we have to
> > disable the cursor to change many of the cursor parameters. This
> > introduces a slight chance of the cursor flickering when things get
> > updated (since we're not currently doing the vblank evade for cursor
> > updates). If we write CURPOS alone then that obviously can't happen.
> > And let's follow the same pattern in the i9xx code just for symmetry.
> > I wasn't able to see a singificant performance difference between
> > this and just writing all the registers unconditionally.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Thanks for the reviews. Entire series pushed to dinq.

> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 69 ++++++++++++++++++------------------
> >  1 file changed, 34 insertions(+), 35 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index d78ab0d35274..40ac0f938a4e 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9323,36 +9323,28 @@ static void i845_update_cursor(struct intel_plane *plane,
> >  
> >  	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> >  
> > -	if (plane->cursor.cntl != 0 &&
> > -	    (plane->cursor.base != base ||
> > -	     plane->cursor.size != size ||
> > -	     plane->cursor.cntl != cntl)) {
> > -		/* On these chipsets we can only modify the base/size/stride
> > -		 * whilst the cursor is disabled.
> > -		 */
> > +	/* On these chipsets we can only modify the base/size/stride
> > +	 * whilst the cursor is disabled.
> > +	 */
> > +	if (plane->cursor.base != base ||
> > +	    plane->cursor.size != size ||
> > +	    plane->cursor.cntl != cntl) {
> >  		I915_WRITE_FW(CURCNTR(PIPE_A), 0);
> > -		plane->cursor.cntl = 0;
> > -	}
> > -
> > -	if (plane->cursor.base != base)
> >  		I915_WRITE_FW(CURBASE(PIPE_A), base);
> > -
> > -	if (plane->cursor.size != size)
> >  		I915_WRITE_FW(CURSIZE, size);
> > -
> > -	if (cntl)
> >  		I915_WRITE_FW(CURPOS(PIPE_A), pos);
> > -
> > -	if (plane->cursor.cntl != cntl)
> >  		I915_WRITE_FW(CURCNTR(PIPE_A), cntl);
> >  
> > +		plane->cursor.base = base;
> > +		plane->cursor.size = size;
> > +		plane->cursor.cntl = cntl;
> > +	} else {
> > +		I915_WRITE_FW(CURPOS(PIPE_A), pos);
> > +	}
> > +
> >  	POSTING_READ_FW(CURCNTR(PIPE_A));
> >  
> >  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> > -
> > -	plane->cursor.cntl = cntl;
> > -	plane->cursor.base = base;
> > -	plane->cursor.size = size;
> >  }
> >  
> >  static void i845_disable_cursor(struct intel_plane *plane,
> > @@ -9508,27 +9500,34 @@ static void i9xx_update_cursor(struct intel_plane *plane,
> >  
> >  	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> >  
> > -	if (plane->cursor.cntl != cntl)
> > +	/*
> > +	 * On some platforms writing CURCNTR first will also
> > +	 * cause CURPOS to be armed by the CURBASE write.
> > +	 * Without the CURCNTR write the CURPOS write would
> > +	 * arm itself.
> > +	 *
> > +	 * CURCNTR and CUR_FBC_CTL are always
> > +	 * armed by the CURBASE write only.
> > +	 */
> > +	if (plane->cursor.base != base ||
> > +	    plane->cursor.size != fbc_ctl ||
> > +	    plane->cursor.cntl != cntl) {
> >  		I915_WRITE_FW(CURCNTR(pipe), cntl);
> > -
> > -	if (plane->cursor.size != fbc_ctl)
> > -		I915_WRITE_FW(CUR_FBC_CTL(pipe), fbc_ctl);
> > -
> > -	if (cntl)
> > +		if (HAS_CUR_FBC(dev_priv))
> > +			I915_WRITE_FW(CUR_FBC_CTL(pipe), fbc_ctl);
> >  		I915_WRITE_FW(CURPOS(pipe), pos);
> > -
> > -	if (plane->cursor.cntl != cntl ||
> > -	    plane->cursor.size != fbc_ctl ||
> > -	    plane->cursor.base != base)
> >  		I915_WRITE_FW(CURBASE(pipe), base);
> >  
> > +		plane->cursor.base = base;
> > +		plane->cursor.size = fbc_ctl;
> > +		plane->cursor.cntl = cntl;
> > +	} else {
> > +		I915_WRITE_FW(CURPOS(pipe), pos);
> > +	}
> > +
> >  	POSTING_READ_FW(CURBASE(pipe));
> >  
> >  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> > -
> > -	plane->cursor.cntl = cntl;
> > -	plane->cursor.base = base;
> > -	plane->cursor.size = fbc_ctl;
> >  }
> >  
> >  static void i9xx_disable_cursor(struct intel_plane *plane,
> > -- 
> > 2.10.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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