[Intel-gfx] [RFC PATCH] drm/i915: Use universal planes for cursor on skylake.

Maarten Lankhorst maarten at mblankhorst.nl
Thu Aug 27 03:08:30 PDT 2015


This appears to make all the cursor tests really slow because of the many calls to skl_update_wm
when the cursor plane visibility is changed. It performs does 3 vblanks each time it's called, and
it's probably called more than once on each update.

Smarter watermark updates will hopefully fix this..

This patch is tested with Xorg and kms_cursor_crc, and is just a RFC. :)

Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
---
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 05523491ad6f..bf5f814f72ae 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -263,11 +263,11 @@ struct i915_hotplug {
 	for ((__p) = 0; (__p) < INTEL_INFO(__dev_priv)->num_pipes; (__p)++)
 #define for_each_plane(__dev_priv, __pipe, __p)				\
 	for ((__p) = 0;							\
-	     (__p) < INTEL_INFO(__dev_priv)->num_sprites[(__pipe)] + 1;	\
+	     (__p) < INTEL_INFO(__dev_priv)->num_sprites[(__pipe)] + 1 + IS_GEN9(__dev_priv);	\
 	     (__p)++)
 #define for_each_sprite(__dev_priv, __p, __s)				\
 	for ((__s) = 0;							\
-	     (__s) < INTEL_INFO(__dev_priv)->num_sprites[(__p)];	\
+	     (__s) < INTEL_INFO(__dev_priv)->num_sprites[(__p)] + IS_GEN9(__dev_priv);	\
 	     (__s)++)
 
 #define for_each_crtc(dev, crtc) \
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 92e3f074ae30..307d1374daa5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1402,7 +1402,7 @@ static void assert_sprites_disabled(struct drm_i915_private *dev_priv,
 
 	if (INTEL_INFO(dev)->gen >= 9) {
 		for_each_sprite(dev_priv, pipe, sprite) {
-			val = I915_READ(PLANE_CTL(pipe, sprite));
+			val = I915_READ(PLANE_CTL(pipe, sprite + 1));
 			I915_STATE_WARN(val & PLANE_CTL_ENABLE,
 			     "plane %d assertion failure, should be off on pipe %c but is still active\n",
 			     sprite, pipe_name(pipe));
@@ -11563,12 +11563,15 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	bool mode_changed = needs_modeset(crtc_state);
 	bool was_crtc_enabled = crtc->state->active;
 	bool is_crtc_enabled = crtc_state->active;
-
+	enum drm_plane_type plane_type = plane->type;
 	bool turn_off, turn_on, visible, was_visible;
 	struct drm_framebuffer *fb = plane_state->fb;
 
+	if (IS_GEN9(dev) && plane->type == DRM_PLANE_TYPE_CURSOR)
+		plane_type = DRM_PLANE_TYPE_OVERLAY;
+
 	if (crtc_state && INTEL_INFO(dev)->gen >= 9 &&
-	    plane->type != DRM_PLANE_TYPE_CURSOR) {
+	    plane_type != DRM_PLANE_TYPE_CURSOR) {
 		ret = skl_update_scaler_plane(
 			to_intel_crtc_state(crtc_state),
 			to_intel_plane_state(plane_state));
@@ -11601,7 +11604,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	if (turn_on) {
 		intel_crtc->atomic.update_wm_pre = true;
 		/* must disable cxsr around plane enable/disable */
-		if (plane->type != DRM_PLANE_TYPE_CURSOR) {
+		if (plane_type != DRM_PLANE_TYPE_CURSOR) {
 			intel_crtc->atomic.disable_cxsr = true;
 			/* to potentially re-enable cxsr */
 			intel_crtc->atomic.wait_vblank = true;
@@ -11610,7 +11613,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	} else if (turn_off) {
 		intel_crtc->atomic.update_wm_post = true;
 		/* must disable cxsr around plane enable/disable */
-		if (plane->type != DRM_PLANE_TYPE_CURSOR) {
+		if (plane_type != DRM_PLANE_TYPE_CURSOR) {
 			if (is_crtc_enabled)
 				intel_crtc->atomic.wait_vblank = true;
 			intel_crtc->atomic.disable_cxsr = true;
@@ -11623,7 +11626,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 		intel_crtc->atomic.fb_bits |=
 			to_intel_plane(plane)->frontbuffer_bit;
 
-	switch (plane->type) {
+	switch (plane_type) {
 	case DRM_PLANE_TYPE_PRIMARY:
 		intel_crtc->atomic.wait_for_flips = true;
 		intel_crtc->atomic.pre_disable_primary = turn_off;
@@ -13770,7 +13773,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	struct intel_crtc_state *crtc_state = NULL;
 	struct drm_plane *primary = NULL;
 	struct drm_plane *cursor = NULL;
-	int i, ret;
+	int i, ret, sprite;
 
 	intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL);
 	if (intel_crtc == NULL)
@@ -13797,9 +13800,31 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	if (!primary)
 		goto fail;
 
-	cursor = intel_cursor_plane_create(dev, pipe);
-	if (!cursor)
-		goto fail;
+	if (!IS_GEN9(dev)) {
+		cursor = intel_cursor_plane_create(dev, pipe);
+		if (!cursor)
+			goto fail;
+	}
+
+	for_each_sprite(dev_priv, pipe, sprite) {
+		enum drm_plane_type plane_type;
+		struct drm_plane *plane;
+
+		if (sprite < INTEL_INFO(dev_priv)->num_sprites[pipe])
+			plane_type = DRM_PLANE_TYPE_OVERLAY;
+		else
+			plane_type = DRM_PLANE_TYPE_CURSOR;
+
+		plane = intel_plane_init(dev, pipe, sprite, plane_type);
+		if (IS_ERR(plane)) {
+			DRM_DEBUG_KMS("pipe %c sprite %c init failed: %ld\n",
+				      pipe_name(pipe), sprite_name(pipe, sprite), PTR_ERR(plane));
+			goto fail;
+		}
+
+		if (plane_type == DRM_PLANE_TYPE_CURSOR)
+			cursor = plane;
+	}
 
 	ret = drm_crtc_init_with_planes(dev, &intel_crtc->base, primary,
 					cursor, &intel_crtc_funcs);
@@ -14724,7 +14749,6 @@ void intel_modeset_init_hw(struct drm_device *dev)
 void intel_modeset_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int sprite, ret;
 	enum pipe pipe;
 	struct intel_crtc *crtc;
 
@@ -14778,15 +14802,8 @@ void intel_modeset_init(struct drm_device *dev)
 		      INTEL_INFO(dev)->num_pipes,
 		      INTEL_INFO(dev)->num_pipes > 1 ? "s" : "");
 
-	for_each_pipe(dev_priv, pipe) {
+	for_each_pipe(dev_priv, pipe)
 		intel_crtc_init(dev, pipe);
-		for_each_sprite(dev_priv, pipe, sprite) {
-			ret = intel_plane_init(dev, pipe, sprite);
-			if (ret)
-				DRM_DEBUG_KMS("pipe %c sprite %c init failed: %d\n",
-					      pipe_name(pipe), sprite_name(pipe, sprite), ret);
-		}
-	}
 
 	intel_init_dpio(dev);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3908059fc42f..df3bca6e4031 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -890,11 +890,12 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
 
 /*
  * Returns the number of planes for this pipe, ie the number of sprites + 1
- * (primary plane). This doesn't count the cursor plane then.
+ * (primary plane). This doesn't count the cursor plane except on GEN9,
+ * which has a universal plane masked as cursor plane.
  */
 static inline unsigned int intel_num_planes(struct intel_crtc *crtc)
 {
-	return INTEL_INFO(crtc->base.dev)->num_sprites[crtc->pipe] + 1;
+	return INTEL_INFO(crtc->base.dev)->num_sprites[crtc->pipe] + 1 + IS_GEN9(crtc->base.dev);
 }
 
 /* intel_fifo_underrun.c */
@@ -1388,7 +1389,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob);
 
 
 /* intel_sprite.c */
-int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane);
+struct drm_plane *intel_plane_init(struct drm_device *dev, enum pipe pipe,
+				   int plane, enum drm_plane_type plane_type);
 int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
 			      struct drm_file *file_priv);
 void intel_pipe_update_start(struct intel_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fff0c22682ee..b6ea345e2f1b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2970,7 +2970,7 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	enum pipe pipe = intel_crtc->pipe;
 	struct skl_ddb_entry *alloc = &ddb->pipe[pipe];
-	uint16_t alloc_size, start, cursor_blocks;
+	uint16_t alloc_size, start;
 	uint16_t minimum[I915_MAX_PLANES];
 	uint16_t y_minimum[I915_MAX_PLANES];
 	unsigned int total_data_rate;
@@ -2984,12 +2984,16 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
 		return;
 	}
 
-	cursor_blocks = skl_cursor_allocation(config);
-	ddb->cursor[pipe].start = alloc->end - cursor_blocks;
-	ddb->cursor[pipe].end = alloc->end;
+	if (!IS_GEN9(dev)) {
+		uint16_t cursor_blocks;
 
-	alloc_size -= cursor_blocks;
-	alloc->end -= cursor_blocks;
+		cursor_blocks = skl_cursor_allocation(config);
+		ddb->cursor[pipe].start = alloc->end - cursor_blocks;
+		ddb->cursor[pipe].end = alloc->end;
+
+		alloc_size -= cursor_blocks;
+		alloc->end -= cursor_blocks;
+	}
 
 	/* 1. Allocate the mininum required blocks for each active plane */
 	for_each_plane(dev_priv, pipe, plane) {
@@ -3186,7 +3190,7 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
 
 		fb = crtc->cursor->state->fb;
 		p->cursor.y_bytes_per_pixel = 0;
-		if (fb) {
+		if (!IS_GEN9(dev) && fb) {
 			p->cursor.enabled = true;
 			p->cursor.bytes_per_pixel = fb->bits_per_pixel / 8;
 			p->cursor.horiz_pixels = crtc->cursor->state->crtc_w;
@@ -3202,8 +3206,11 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
 	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
 		struct intel_plane *intel_plane = to_intel_plane(plane);
 
-		if (intel_plane->pipe == pipe &&
-			plane->type == DRM_PLANE_TYPE_OVERLAY)
+		if (intel_plane->pipe != pipe)
+			continue;
+
+		if (plane->type == DRM_PLANE_TYPE_OVERLAY ||
+		    (IS_GEN9(dev) && plane->type == DRM_PLANE_TYPE_CURSOR))
 			p->plane[i++] = intel_plane->wm;
 	}
 }
@@ -3390,11 +3397,13 @@ static void skl_compute_wm_results(struct drm_device *dev,
 
 		temp = 0;
 
-		temp |= p_wm->wm[level].cursor_res_l << PLANE_WM_LINES_SHIFT;
-		temp |= p_wm->wm[level].cursor_res_b;
+		if (!IS_GEN9(dev)) {
+			temp |= p_wm->wm[level].cursor_res_l << PLANE_WM_LINES_SHIFT;
+			temp |= p_wm->wm[level].cursor_res_b;
 
-		if (p_wm->wm[level].cursor_en)
-			temp |= PLANE_WM_EN;
+			if (p_wm->wm[level].cursor_en)
+				temp |= PLANE_WM_EN;
+		}
 
 		r->cursor[pipe][level] = temp;
 
@@ -3412,10 +3421,12 @@ static void skl_compute_wm_results(struct drm_device *dev,
 	}
 
 	temp = 0;
-	temp |= p_wm->trans_wm.cursor_res_l << PLANE_WM_LINES_SHIFT;
-	temp |= p_wm->trans_wm.cursor_res_b;
-	if (p_wm->trans_wm.cursor_en)
-		temp |= PLANE_WM_EN;
+	if (!IS_GEN9(dev)) {
+		temp |= p_wm->trans_wm.cursor_res_l << PLANE_WM_LINES_SHIFT;
+		temp |= p_wm->trans_wm.cursor_res_b;
+		if (p_wm->trans_wm.cursor_en)
+			temp |= PLANE_WM_EN;
+	}
 
 	r->cursor_trans[pipe] = temp;
 
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 9d8af2f8a875..596fa4319681 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -961,7 +961,8 @@ int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
 		return -EINVAL;
 
 	plane = drm_plane_find(dev, set->plane_id);
-	if (!plane || plane->type != DRM_PLANE_TYPE_OVERLAY)
+	if (!plane || plane->type == DRM_PLANE_TYPE_PRIMARY ||
+	    (!IS_GEN9(dev) && plane->type == DRM_PLANE_TYPE_CURSOR))
 		return -ENOENT;
 
 	drm_modeset_acquire_init(&ctx, 0);
@@ -1040,8 +1041,8 @@ static uint32_t skl_plane_formats[] = {
 	DRM_FORMAT_VYUY,
 };
 
-int
-intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
+struct drm_plane *
+intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane, enum drm_plane_type plane_type)
 {
 	struct intel_plane *intel_plane;
 	struct intel_plane_state *state;
@@ -1051,16 +1052,16 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 	int ret;
 
 	if (INTEL_INFO(dev)->gen < 5)
-		return -ENODEV;
+		return ERR_PTR(-ENODEV);
 
 	intel_plane = kzalloc(sizeof(*intel_plane), GFP_KERNEL);
 	if (!intel_plane)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	state = intel_create_plane_state(&intel_plane->base);
 	if (!state) {
 		kfree(intel_plane);
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 	}
 	intel_plane->base.state = &state->base;
 
@@ -1116,7 +1117,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 		break;
 	default:
 		kfree(intel_plane);
-		return -ENODEV;
+		return ERR_PTR(-ENODEV);
 	}
 
 	intel_plane->pipe = pipe;
@@ -1128,7 +1129,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 	ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
 				       &intel_plane_funcs,
 				       plane_formats, num_plane_formats,
-				       DRM_PLANE_TYPE_OVERLAY);
+				       plane_type);
 	if (ret) {
 		kfree(intel_plane);
 		goto out;
@@ -1138,6 +1139,8 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 
 	drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
 
+	return &intel_plane->base;
+
 out:
-	return ret;
+	return ERR_PTR(ret);
 }



More information about the Intel-gfx mailing list