All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5)
@ 2016-05-27 17:59 ville.syrjala
  2016-05-27 17:59 ` [PATCH v2 1/8] drm/i915: Use crtc->name in debug messages ville.syrjala
                   ` (9 more replies)
  0 siblings, 10 replies; 13+ messages in thread
From: ville.syrjala @ 2016-05-27 17:59 UTC (permalink / raw)
  To: intel-gfx

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

This is the remainder (the i915 specific parts) of my earlier
{crtc,plane}->name series [1]. I think at least one i915 specific patch
was already merged, and I think I added one or two new ones at some
point.

I'm not so sure about the last patch. It was somewhat useful in narrowing
down some underrun locations and whatnot for me, but normally it might
be just noise. So I'm happy to keep it in my private stash for a rainy day
if people are fed up with dmesg spam.

Here's a branch as usual:
git://github.com/vsyrjala/linux.git crtc_plane_name_7

[1] https://lists.freedesktop.org/archives/intel-gfx/2015-December/082359.html

Ville Syrjälä (8):
  drm/i915: Use crtc->name in debug messages
  drm/i915: Use plane->name in debug prints
  drm/i915: Set crtc->name to "pipe A", "pipe B", etc.
  drm/i915: Don't leak primary/cursor planes on crtc init failure
  drm/i915: Give meaningful names to all the planes
  drm/i915: Give encoders useful names
  drm/i915: kill STANDARD/CURSOR plane screams
  drm/i915: Add debug prints for encoder modeset hooks

 drivers/gpu/drm/i915/intel_crt.c      |   2 +-
 drivers/gpu/drm/i915/intel_ddi.c      |   2 +-
 drivers/gpu/drm/i915/intel_display.c  | 189 ++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_dp.c       |   2 +-
 drivers/gpu/drm/i915/intel_dp_mst.c   |   2 +-
 drivers/gpu/drm/i915/intel_dpll_mgr.c |  16 +--
 drivers/gpu/drm/i915/intel_dsi.c      |   2 +-
 drivers/gpu/drm/i915/intel_dvo.c      |  18 +++-
 drivers/gpu/drm/i915/intel_fbdev.c    |   4 +-
 drivers/gpu/drm/i915/intel_hdmi.c     |   2 +-
 drivers/gpu/drm/i915/intel_lvds.c     |   2 +-
 drivers/gpu/drm/i915/intel_sdvo.c     |   2 +-
 drivers/gpu/drm/i915/intel_sprite.c   |  16 ++-
 drivers/gpu/drm/i915/intel_tv.c       |   2 +-
 14 files changed, 150 insertions(+), 111 deletions(-)

-- 
2.7.4

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

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

* [PATCH v2 1/8] drm/i915: Use crtc->name in debug messages
  2016-05-27 17:59 [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5) ville.syrjala
@ 2016-05-27 17:59 ` ville.syrjala
  2016-05-27 17:59 ` [PATCH v2 2/8] drm/i915: Use plane->name in debug prints ville.syrjala
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: ville.syrjala @ 2016-05-27 17:59 UTC (permalink / raw)
  To: intel-gfx

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

We have crtc->name, so let's use that in debug messages instead
of just printing the more or less useless object ID.

v2: Rebased due to intel_dpll_mgr.c, slap on a commit message

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c  | 32 ++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 16 ++++++++--------
 drivers/gpu/drm/i915/intel_fbdev.c    |  4 ++--
 3 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e775d08b121b..c59ab52f49b2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4278,8 +4278,9 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state)
 	struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc);
 	const struct drm_display_mode *adjusted_mode = &state->base.adjusted_mode;
 
-	DRM_DEBUG_KMS("Updating scaler for [CRTC:%i] scaler_user index %u.%u\n",
-		      intel_crtc->base.base.id, intel_crtc->pipe, SKL_CRTC_INDEX);
+	DRM_DEBUG_KMS("Updating scaler for [CRTC:%d:%s] scaler_user index %u.%u\n",
+		      intel_crtc->base.base.id, intel_crtc->base.name,
+		      intel_crtc->pipe, SKL_CRTC_INDEX);
 
 	return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX,
 		&state->scaler_state.scaler_id, BIT(DRM_ROTATE_0),
@@ -6323,8 +6324,8 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
 
 	dev_priv->display.crtc_disable(crtc);
 
-	DRM_DEBUG_KMS("[CRTC:%d] hw state adjusted, was enabled, now disabled\n",
-		      crtc->base.id);
+	DRM_DEBUG_KMS("[CRTC:%d:%s] hw state adjusted, was enabled, now disabled\n",
+		      crtc->base.id, crtc->name);
 
 	WARN_ON(drm_atomic_set_mode_for_crtc(crtc->state, NULL) < 0);
 	crtc->state->active = false;
@@ -11932,12 +11933,12 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_plane_state *old_plane_state =
 		to_intel_plane_state(plane->state);
-	int idx = intel_crtc->base.base.id, ret;
 	bool mode_changed = needs_modeset(crtc_state);
 	bool was_crtc_enabled = crtc->state->active;
 	bool is_crtc_enabled = crtc_state->active;
 	bool turn_off, turn_on, visible, was_visible;
 	struct drm_framebuffer *fb = plane_state->fb;
+	int ret;
 
 	if (crtc_state && INTEL_INFO(dev)->gen >= 9 &&
 	    plane->type != DRM_PLANE_TYPE_CURSOR) {
@@ -11976,7 +11977,9 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	turn_off = was_visible && (!visible || mode_changed);
 	turn_on = visible && (!was_visible || mode_changed);
 
-	DRM_DEBUG_ATOMIC("[CRTC:%i] has [PLANE:%i] with fb %i\n", idx,
+	DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%i] with fb %i\n",
+			 intel_crtc->base.base.id,
+			 intel_crtc->base.name,
 			 plane->base.id, fb ? fb->base.id : -1);
 
 	DRM_DEBUG_ATOMIC("[PLANE:%i] visible %i -> %i, off %i, on %i, ms %i\n",
@@ -12271,7 +12274,8 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 	struct intel_plane_state *state;
 	struct drm_framebuffer *fb;
 
-	DRM_DEBUG_KMS("[CRTC:%d]%s config %p for pipe %c\n", crtc->base.base.id,
+	DRM_DEBUG_KMS("[CRTC:%d:%s]%s config %p for pipe %c\n",
+		      crtc->base.base.id, crtc->base.name,
 		      context, pipe_config, pipe_name(crtc->pipe));
 
 	DRM_DEBUG_KMS("cpu_transcoder: %s\n", transcoder_name(pipe_config->cpu_transcoder));
@@ -13061,7 +13065,7 @@ verify_crtc_state(struct drm_crtc *crtc,
 	pipe_config->base.crtc = crtc;
 	pipe_config->base.state = old_state;
 
-	DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
+	DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name);
 
 	active = dev_priv->display.get_pipe_config(intel_crtc, pipe_config);
 
@@ -13878,8 +13882,8 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 
 	state = drm_atomic_state_alloc(dev);
 	if (!state) {
-		DRM_DEBUG_KMS("[CRTC:%d] crtc restore failed, out of memory",
-			      crtc->base.id);
+		DRM_DEBUG_KMS("[CRTC:%d:%s] crtc restore failed, out of memory",
+			      crtc->base.id, crtc->name);
 		return;
 	}
 
@@ -15742,8 +15746,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 	if (INTEL_INFO(dev)->gen < 4 && !intel_check_plane_mapping(crtc)) {
 		bool plane;
 
-		DRM_DEBUG_KMS("[CRTC:%d] wrong plane connection detected!\n",
-			      crtc->base.base.id);
+		DRM_DEBUG_KMS("[CRTC:%d:%s] wrong plane connection detected!\n",
+			      crtc->base.base.id, crtc->base.name);
 
 		/* Pipe has the wrong plane attached and the plane is active.
 		 * Temporarily change the plane mapping and disable everything
@@ -15927,8 +15931,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 
 		readout_plane_state(crtc);
 
-		DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
-			      crtc->base.base.id,
+		DRM_DEBUG_KMS("[CRTC:%d:%s] hw state readout: %s\n",
+			      crtc->base.base.id, crtc->base.name,
 			      crtc->active ? "enabled" : "disabled");
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 02a7962a67f1..c0eff1571731 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -208,8 +208,8 @@ intel_find_shared_dpll(struct intel_crtc *crtc,
 		if (memcmp(&crtc_state->dpll_hw_state,
 			   &shared_dpll[i].hw_state,
 			   sizeof(crtc_state->dpll_hw_state)) == 0) {
-			DRM_DEBUG_KMS("CRTC:%d sharing existing %s (crtc mask 0x%08x, active %x)\n",
-				      crtc->base.base.id, pll->name,
+			DRM_DEBUG_KMS("[CRTC:%d:%s] sharing existing %s (crtc mask 0x%08x, active %x)\n",
+				      crtc->base.base.id, crtc->base.name, pll->name,
 				      shared_dpll[i].crtc_mask,
 				      pll->active_mask);
 			return pll;
@@ -220,8 +220,8 @@ intel_find_shared_dpll(struct intel_crtc *crtc,
 	for (i = range_min; i <= range_max; i++) {
 		pll = &dev_priv->shared_dplls[i];
 		if (shared_dpll[i].crtc_mask == 0) {
-			DRM_DEBUG_KMS("CRTC:%d allocated %s\n",
-				      crtc->base.base.id, pll->name);
+			DRM_DEBUG_KMS("[CRTC:%d:%s] allocated %s\n",
+				      crtc->base.base.id, crtc->base.name, pll->name);
 			return pll;
 		}
 	}
@@ -358,8 +358,8 @@ ibx_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 		i = (enum intel_dpll_id) crtc->pipe;
 		pll = &dev_priv->shared_dplls[i];
 
-		DRM_DEBUG_KMS("CRTC:%d using pre-allocated %s\n",
-			      crtc->base.base.id, pll->name);
+		DRM_DEBUG_KMS("[CRTC:%d:%s] using pre-allocated %s\n",
+			      crtc->base.base.id, crtc->base.name, pll->name);
 	} else {
 		pll = intel_find_shared_dpll(crtc, crtc_state,
 					     DPLL_ID_PCH_PLL_A,
@@ -1613,8 +1613,8 @@ bxt_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 	i = (enum intel_dpll_id) intel_dig_port->port;
 	pll = intel_get_shared_dpll_by_id(dev_priv, i);
 
-	DRM_DEBUG_KMS("CRTC:%d using pre-allocated %s\n",
-		crtc->base.base.id, pll->name);
+	DRM_DEBUG_KMS("[CRTC:%d:%s] using pre-allocated %s\n",
+		      crtc->base.base.id, crtc->base.name, pll->name);
 
 	intel_reference_shared_dpll(pll, crtc_state);
 
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 99e27530e264..ef8e67690f3d 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -490,10 +490,10 @@ retry:
 		}
 		crtcs[i] = new_crtc;
 
-		DRM_DEBUG_KMS("connector %s on pipe %c [CRTC:%d]: %dx%d%s\n",
+		DRM_DEBUG_KMS("connector %s on [CRTC:%d:%s]: %dx%d%s\n",
 			      connector->name,
-			      pipe_name(to_intel_crtc(connector->state->crtc)->pipe),
 			      connector->state->crtc->base.id,
+			      connector->state->crtc->name,
 			      modes[i]->hdisplay, modes[i]->vdisplay,
 			      modes[i]->flags & DRM_MODE_FLAG_INTERLACE ? "i" :"");
 
-- 
2.7.4

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

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

* [PATCH v2 2/8] drm/i915: Use plane->name in debug prints
  2016-05-27 17:59 [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5) ville.syrjala
  2016-05-27 17:59 ` [PATCH v2 1/8] drm/i915: Use crtc->name in debug messages ville.syrjala
@ 2016-05-27 17:59 ` ville.syrjala
  2016-05-27 17:59 ` [PATCH v5 3/8] drm/i915: Set crtc->name to "pipe A", "pipe B", etc ville.syrjala
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: ville.syrjala @ 2016-05-27 17:59 UTC (permalink / raw)
  To: intel-gfx

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

We have plane->name, so let's use that in debug messages instead
of just printing the more or less useless object ID.

v2: slap on a commit message

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c59ab52f49b2..e18dcd9a9df1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4310,9 +4310,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 
 	bool force_detach = !fb || !plane_state->visible;
 
-	DRM_DEBUG_KMS("Updating scaler for [PLANE:%d] scaler_user index %u.%u\n",
-		      intel_plane->base.base.id, intel_crtc->pipe,
-		      drm_plane_index(&intel_plane->base));
+	DRM_DEBUG_KMS("Updating scaler for [PLANE:%d:%s] scaler_user index %u.%u\n",
+		      intel_plane->base.base.id, intel_plane->base.name,
+		      intel_crtc->pipe, drm_plane_index(&intel_plane->base));
 
 	ret = skl_update_scaler(crtc_state, force_detach,
 				drm_plane_index(&intel_plane->base),
@@ -4328,8 +4328,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 
 	/* check colorkey */
 	if (plane_state->ckey.flags != I915_SET_COLORKEY_NONE) {
-		DRM_DEBUG_KMS("[PLANE:%d] scaling with color key not allowed",
-			      intel_plane->base.base.id);
+		DRM_DEBUG_KMS("[PLANE:%d:%s] scaling with color key not allowed",
+			      intel_plane->base.base.id,
+			      intel_plane->base.name);
 		return -EINVAL;
 	}
 
@@ -4348,8 +4349,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 	case DRM_FORMAT_VYUY:
 		break;
 	default:
-		DRM_DEBUG_KMS("[PLANE:%d] FB:%d unsupported scaling format 0x%x\n",
-			intel_plane->base.base.id, fb->base.id, fb->pixel_format);
+		DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format 0x%x\n",
+			      intel_plane->base.base.id, intel_plane->base.name,
+			      fb->base.id, fb->pixel_format);
 		return -EINVAL;
 	}
 
@@ -11977,13 +11979,15 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	turn_off = was_visible && (!visible || mode_changed);
 	turn_on = visible && (!was_visible || mode_changed);
 
-	DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%i] with fb %i\n",
+	DRM_DEBUG_ATOMIC("[CRTC:%d:%s] has [PLANE:%d:%s] with fb %i\n",
 			 intel_crtc->base.base.id,
 			 intel_crtc->base.name,
-			 plane->base.id, fb ? fb->base.id : -1);
+			 plane->base.id, plane->name,
+			 fb ? fb->base.id : -1);
 
-	DRM_DEBUG_ATOMIC("[PLANE:%i] visible %i -> %i, off %i, on %i, ms %i\n",
-			 plane->base.id, was_visible, visible,
+	DRM_DEBUG_ATOMIC("[PLANE:%d:%s] visible %i -> %i, off %i, on %i, ms %i\n",
+			 plane->base.id, plane->name,
+			 was_visible, visible,
 			 turn_off, turn_on, mode_changed);
 
 	if (turn_on) {
@@ -12376,18 +12380,20 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 		state = to_intel_plane_state(plane->state);
 		fb = state->base.fb;
 		if (!fb) {
-			DRM_DEBUG_KMS("%s PLANE:%d plane: %u.%u idx: %d "
-				"disabled, scaler_id = %d\n",
+			DRM_DEBUG_KMS("%s [PLANE:%d:%s] plane: %u.%u idx: %d "
+				      "disabled, scaler_id = %d\n",
 				plane->type == DRM_PLANE_TYPE_CURSOR ? "CURSOR" : "STANDARD",
-				plane->base.id, intel_plane->pipe,
+				plane->base.id, plane->name,
+				intel_plane->pipe,
 				(crtc->base.primary == plane) ? 0 : intel_plane->plane + 1,
 				drm_plane_index(plane), state->scaler_id);
 			continue;
 		}
 
-		DRM_DEBUG_KMS("%s PLANE:%d plane: %u.%u idx: %d enabled",
+		DRM_DEBUG_KMS("%s [PLANE:%d:%s] plane: %u.%u idx: %d enabled",
 			plane->type == DRM_PLANE_TYPE_CURSOR ? "CURSOR" : "STANDARD",
-			plane->base.id, intel_plane->pipe,
+			plane->base.id, plane->name,
+			intel_plane->pipe,
 			crtc->base.primary == plane ? 0 : intel_plane->plane + 1,
 			drm_plane_index(plane));
 		DRM_DEBUG_KMS("\tFB:%d, fb = %ux%u format = 0x%x",
-- 
2.7.4

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

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

* [PATCH v5 3/8] drm/i915: Set crtc->name to "pipe A", "pipe B", etc.
  2016-05-27 17:59 [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5) ville.syrjala
  2016-05-27 17:59 ` [PATCH v2 1/8] drm/i915: Use crtc->name in debug messages ville.syrjala
  2016-05-27 17:59 ` [PATCH v2 2/8] drm/i915: Use plane->name in debug prints ville.syrjala
@ 2016-05-27 17:59 ` ville.syrjala
  2016-05-27 17:59 ` [PATCH 4/8] drm/i915: Don't leak primary/cursor planes on crtc init failure ville.syrjala
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: ville.syrjala @ 2016-05-27 17:59 UTC (permalink / raw)
  To: intel-gfx

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

v2: Fix intel_crtc leak on failure to allocate the name
    Use a local 'name' variable to make things easier
v3: Pass the name as a function arguemnt to drm_crtc_init_with_planes() (Jani)
v4: Pass the printf style format string to drm_crtc_init_with_planes()
v5: Drop spurious code changes

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e18dcd9a9df1..7744812f0831 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14477,7 +14477,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 		goto fail;
 
 	ret = drm_crtc_init_with_planes(dev, &intel_crtc->base, primary,
-					cursor, &intel_crtc_funcs, NULL);
+					cursor, &intel_crtc_funcs,
+					"pipe %c", pipe_name(pipe));
 	if (ret)
 		goto fail;
 
-- 
2.7.4

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

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

* [PATCH 4/8] drm/i915: Don't leak primary/cursor planes on crtc init failure
  2016-05-27 17:59 [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5) ville.syrjala
                   ` (2 preceding siblings ...)
  2016-05-27 17:59 ` [PATCH v5 3/8] drm/i915: Set crtc->name to "pipe A", "pipe B", etc ville.syrjala
@ 2016-05-27 17:59 ` ville.syrjala
  2016-05-27 17:59 ` [PATCH v3 5/8] drm/i915: Give meaningful names to all the planes ville.syrjala
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: ville.syrjala @ 2016-05-27 17:59 UTC (permalink / raw)
  To: intel-gfx

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

Call intel_plane_destroy() instead of drm_plane_cleanup() so that we
also free the plane struct itself when bailing out of the crtc init.

And make intel_plane_destroy() NULL tolerant to avoid having to check
for it in the caller.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7744812f0831..f783415dd82e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14157,9 +14157,11 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc,
  */
 void intel_plane_destroy(struct drm_plane *plane)
 {
-	struct intel_plane *intel_plane = to_intel_plane(plane);
+	if (!plane)
+		return;
+
 	drm_plane_cleanup(plane);
-	kfree(intel_plane);
+	kfree(to_intel_plane(plane));
 }
 
 const struct drm_plane_funcs intel_plane_funcs = {
@@ -14512,10 +14514,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	return;
 
 fail:
-	if (primary)
-		drm_plane_cleanup(primary);
-	if (cursor)
-		drm_plane_cleanup(cursor);
+	intel_plane_destroy(primary);
+	intel_plane_destroy(cursor);
 	kfree(crtc_state);
 	kfree(intel_crtc);
 }
-- 
2.7.4

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

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

* [PATCH v3 5/8] drm/i915: Give meaningful names to all the planes
  2016-05-27 17:59 [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5) ville.syrjala
                   ` (3 preceding siblings ...)
  2016-05-27 17:59 ` [PATCH 4/8] drm/i915: Don't leak primary/cursor planes on crtc init failure ville.syrjala
@ 2016-05-27 17:59 ` ville.syrjala
  2016-05-27 17:59 ` [PATCH v3 6/8] drm/i915: Give encoders useful names ville.syrjala
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: ville.syrjala @ 2016-05-27 17:59 UTC (permalink / raw)
  To: intel-gfx

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

Let's name our planes in a way that makes sense wrt. the spec:
- skl+ -> "plane 1A", "plane 2A", "plane 1C", "cursor A" etc.
- g4x+ -> "primary A", "primary B", "sprite A", "cursor C" etc.
- pre-g4x -> "plane A", "cursor B" etc.

v2: Rebase on top of the fixed/cleaned error paths
    Use a local 'name' variable to make things easier
v3: Pass the name as a function argument to drm_universal_plane_init() (Jani)
v3: Pass the printf style string to drm_universal_plane_init()

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f783415dd82e..6563eb8bf060 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14233,10 +14233,24 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 		primary->disable_plane = i9xx_disable_primary_plane;
 	}
 
-	ret = drm_universal_plane_init(dev, &primary->base, 0,
-				       &intel_plane_funcs,
-				       intel_primary_formats, num_formats,
-				       DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (INTEL_INFO(dev)->gen >= 9)
+		ret = drm_universal_plane_init(dev, &primary->base, 0,
+					       &intel_plane_funcs,
+					       intel_primary_formats, num_formats,
+					       DRM_PLANE_TYPE_PRIMARY,
+					       "plane 1%c", pipe_name(pipe));
+	else if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
+		ret = drm_universal_plane_init(dev, &primary->base, 0,
+					       &intel_plane_funcs,
+					       intel_primary_formats, num_formats,
+					       DRM_PLANE_TYPE_PRIMARY,
+					       "primary %c", pipe_name(pipe));
+	else
+		ret = drm_universal_plane_init(dev, &primary->base, 0,
+					       &intel_plane_funcs,
+					       intel_primary_formats, num_formats,
+					       DRM_PLANE_TYPE_PRIMARY,
+					       "plane %c", plane_name(primary->plane));
 	if (ret)
 		goto fail;
 
@@ -14394,7 +14408,8 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
 				       &intel_plane_funcs,
 				       intel_cursor_formats,
 				       ARRAY_SIZE(intel_cursor_formats),
-				       DRM_PLANE_TYPE_CURSOR, NULL);
+				       DRM_PLANE_TYPE_CURSOR,
+				       "cursor %c", pipe_name(pipe));
 	if (ret)
 		goto fail;
 
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 97b1a54eb09f..324ccb06397d 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1114,10 +1114,18 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 
 	possible_crtcs = (1 << pipe);
 
-	ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
-				       &intel_plane_funcs,
-				       plane_formats, num_plane_formats,
-				       DRM_PLANE_TYPE_OVERLAY, NULL);
+	if (INTEL_INFO(dev)->gen >= 9)
+		ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
+					       &intel_plane_funcs,
+					       plane_formats, num_plane_formats,
+					       DRM_PLANE_TYPE_OVERLAY,
+					       "plane %d%c", plane + 2, pipe_name(pipe));
+	else
+		ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
+					       &intel_plane_funcs,
+					       plane_formats, num_plane_formats,
+					       DRM_PLANE_TYPE_OVERLAY,
+					       "sprite %c", sprite_name(pipe, plane));
 	if (ret)
 		goto fail;
 
-- 
2.7.4

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

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

* [PATCH v3 6/8] drm/i915: Give encoders useful names
  2016-05-27 17:59 [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5) ville.syrjala
                   ` (4 preceding siblings ...)
  2016-05-27 17:59 ` [PATCH v3 5/8] drm/i915: Give meaningful names to all the planes ville.syrjala
@ 2016-05-27 17:59 ` ville.syrjala
  2016-05-27 17:59 ` [PATCH 7/8] drm/i915: kill STANDARD/CURSOR plane screams ville.syrjala
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: ville.syrjala @ 2016-05-27 17:59 UTC (permalink / raw)
  To: intel-gfx

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

Rather than let the core generate usless encoder names, let's pass in
something that actually identifies the piece of hardware we're dealing
with.

v2: Use 'DSI %c' instead of 'MIPI %c' for DSI encoders (Jani)
v3: Use port_name() in DSI code since we have it

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c    |  2 +-
 drivers/gpu/drm/i915/intel_ddi.c    |  2 +-
 drivers/gpu/drm/i915/intel_dp.c     |  2 +-
 drivers/gpu/drm/i915/intel_dp_mst.c |  2 +-
 drivers/gpu/drm/i915/intel_dsi.c    |  2 +-
 drivers/gpu/drm/i915/intel_dvo.c    | 18 ++++++++++++++++--
 drivers/gpu/drm/i915/intel_hdmi.c   |  2 +-
 drivers/gpu/drm/i915/intel_lvds.c   |  2 +-
 drivers/gpu/drm/i915/intel_sdvo.c   |  2 +-
 drivers/gpu/drm/i915/intel_tv.c     |  2 +-
 10 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 3fbb6fc66451..622968161ac7 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -839,7 +839,7 @@ void intel_crt_init(struct drm_device *dev)
 			   &intel_crt_connector_funcs, DRM_MODE_CONNECTOR_VGA);
 
 	drm_encoder_init(dev, &crt->base.base, &intel_crt_enc_funcs,
-			 DRM_MODE_ENCODER_DAC, NULL);
+			 DRM_MODE_ENCODER_DAC, "CRT");
 
 	intel_connector_attach_encoder(intel_connector, &crt->base);
 
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c454744dda0b..022b41d422dc 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2347,7 +2347,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
 	encoder = &intel_encoder->base;
 
 	drm_encoder_init(dev, encoder, &intel_ddi_funcs,
-			 DRM_MODE_ENCODER_TMDS, NULL);
+			 DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
 
 	intel_encoder->compute_config = intel_ddi_compute_config;
 	intel_encoder->enable = intel_enable_ddi;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index aa9c59ee17f6..096acbf04003 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5590,7 +5590,7 @@ intel_dp_init(struct drm_device *dev,
 	encoder = &intel_encoder->base;
 
 	if (drm_encoder_init(dev, &intel_encoder->base, &intel_dp_enc_funcs,
-			     DRM_MODE_ENCODER_TMDS, NULL))
+			     DRM_MODE_ENCODER_TMDS, "DP %c", port_name(port)))
 		goto err_encoder_init;
 
 	intel_encoder->compute_config = intel_dp_compute_config;
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 7a34090cef34..f62ca9a126b3 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -534,7 +534,7 @@ intel_dp_create_fake_mst_encoder(struct intel_digital_port *intel_dig_port, enum
 	intel_mst->primary = intel_dig_port;
 
 	drm_encoder_init(dev, &intel_encoder->base, &intel_dp_mst_enc_funcs,
-			 DRM_MODE_ENCODER_DPMST, NULL);
+			 DRM_MODE_ENCODER_DPMST, "DP-MST %c", pipe_name(pipe));
 
 	intel_encoder->type = INTEL_OUTPUT_DP_MST;
 	intel_encoder->crtc_mask = 0x7;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 4009618a5b34..cbe2537f26f4 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1450,7 +1450,7 @@ void intel_dsi_init(struct drm_device *dev)
 	connector = &intel_connector->base;
 
 	drm_encoder_init(dev, encoder, &intel_dsi_funcs, DRM_MODE_ENCODER_DSI,
-			 NULL);
+			 "DSI %c", port_name(port));
 
 	intel_encoder->compute_config = intel_dsi_compute_config;
 	intel_encoder->pre_enable = intel_dsi_pre_enable;
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 286baec979c8..a456f2eb68b6 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -406,6 +406,18 @@ intel_dvo_get_current_mode(struct drm_connector *connector)
 	return mode;
 }
 
+static char intel_dvo_port_name(i915_reg_t dvo_reg)
+{
+	if (i915_mmio_reg_equal(dvo_reg, DVOA))
+		return 'A';
+	else if (i915_mmio_reg_equal(dvo_reg, DVOB))
+		return 'B';
+	else if (i915_mmio_reg_equal(dvo_reg, DVOC))
+		return 'C';
+	else
+		return '?';
+}
+
 void intel_dvo_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -428,8 +440,6 @@ void intel_dvo_init(struct drm_device *dev)
 	intel_dvo->attached_connector = intel_connector;
 
 	intel_encoder = &intel_dvo->base;
-	drm_encoder_init(dev, &intel_encoder->base,
-			 &intel_dvo_enc_funcs, encoder_type, NULL);
 
 	intel_encoder->disable = intel_disable_dvo;
 	intel_encoder->enable = intel_enable_dvo;
@@ -496,6 +506,10 @@ void intel_dvo_init(struct drm_device *dev)
 		if (!dvoinit)
 			continue;
 
+		drm_encoder_init(dev, &intel_encoder->base,
+				 &intel_dvo_enc_funcs, encoder_type,
+				 "DVO %c", intel_dvo_port_name(dvo->dvo_reg));
+
 		intel_encoder->type = INTEL_OUTPUT_DVO;
 		intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
 		switch (dvo->type) {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 6b52c6accf6a..eb455ea6ea92 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1945,7 +1945,7 @@ void intel_hdmi_init(struct drm_device *dev,
 	intel_encoder = &intel_dig_port->base;
 
 	drm_encoder_init(dev, &intel_encoder->base, &intel_hdmi_enc_funcs,
-			 DRM_MODE_ENCODER_TMDS, NULL);
+			 DRM_MODE_ENCODER_TMDS, "HDMI %c", port_name(port));
 
 	intel_encoder->compute_config = intel_hdmi_compute_config;
 	if (HAS_PCH_SPLIT(dev)) {
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index d65fd945607a..56eb3bdcdb5c 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -978,7 +978,7 @@ void intel_lvds_init(struct drm_device *dev)
 			   DRM_MODE_CONNECTOR_LVDS);
 
 	drm_encoder_init(dev, &intel_encoder->base, &intel_lvds_enc_funcs,
-			 DRM_MODE_ENCODER_LVDS, NULL);
+			 DRM_MODE_ENCODER_LVDS, "LVDS");
 
 	intel_encoder->enable = intel_enable_lvds;
 	intel_encoder->pre_enable = intel_pre_enable_lvds;
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 2128fae5687d..1a71456bd12a 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2981,7 +2981,7 @@ bool intel_sdvo_init(struct drm_device *dev,
 	intel_encoder = &intel_sdvo->base;
 	intel_encoder->type = INTEL_OUTPUT_SDVO;
 	drm_encoder_init(dev, &intel_encoder->base, &intel_sdvo_enc_funcs, 0,
-			 NULL);
+			 "SDVO %c", port_name(port));
 
 	/* Read the regs to test if we can talk to the device */
 	for (i = 0; i < 0x40; i++) {
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 223129d3c765..1f3a0e1e1a1f 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1591,7 +1591,7 @@ intel_tv_init(struct drm_device *dev)
 			   DRM_MODE_CONNECTOR_SVIDEO);
 
 	drm_encoder_init(dev, &intel_encoder->base, &intel_tv_enc_funcs,
-			 DRM_MODE_ENCODER_TVDAC, NULL);
+			 DRM_MODE_ENCODER_TVDAC, "TV");
 
 	intel_encoder->compute_config = intel_tv_compute_config;
 	intel_encoder->get_config = intel_tv_get_config;
-- 
2.7.4

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

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

* [PATCH 7/8] drm/i915: kill STANDARD/CURSOR plane screams
  2016-05-27 17:59 [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5) ville.syrjala
                   ` (5 preceding siblings ...)
  2016-05-27 17:59 ` [PATCH v3 6/8] drm/i915: Give encoders useful names ville.syrjala
@ 2016-05-27 17:59 ` ville.syrjala
  2016-05-27 17:59 ` [PATCH v2 8/8] drm/i915: Add debug prints for encoder modeset hooks ville.syrjala
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: ville.syrjala @ 2016-05-27 17:59 UTC (permalink / raw)
  To: intel-gfx

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

Stop yelling the plane type. "STANDARD" doesn't mean anything anyway.
Let's just use the plane name here.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6563eb8bf060..e1e61e00880b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12380,31 +12380,24 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 		state = to_intel_plane_state(plane->state);
 		fb = state->base.fb;
 		if (!fb) {
-			DRM_DEBUG_KMS("%s [PLANE:%d:%s] plane: %u.%u idx: %d "
-				      "disabled, scaler_id = %d\n",
-				plane->type == DRM_PLANE_TYPE_CURSOR ? "CURSOR" : "STANDARD",
-				plane->base.id, plane->name,
-				intel_plane->pipe,
-				(crtc->base.primary == plane) ? 0 : intel_plane->plane + 1,
-				drm_plane_index(plane), state->scaler_id);
+			DRM_DEBUG_KMS("[PLANE:%d:%s] disabled, scaler_id = %d\n",
+				      plane->base.id, plane->name, state->scaler_id);
 			continue;
 		}
 
-		DRM_DEBUG_KMS("%s [PLANE:%d:%s] plane: %u.%u idx: %d enabled",
-			plane->type == DRM_PLANE_TYPE_CURSOR ? "CURSOR" : "STANDARD",
-			plane->base.id, plane->name,
-			intel_plane->pipe,
-			crtc->base.primary == plane ? 0 : intel_plane->plane + 1,
-			drm_plane_index(plane));
-		DRM_DEBUG_KMS("\tFB:%d, fb = %ux%u format = 0x%x",
-			fb->base.id, fb->width, fb->height, fb->pixel_format);
-		DRM_DEBUG_KMS("\tscaler:%d src (%u, %u) %ux%u dst (%u, %u) %ux%u\n",
-			state->scaler_id,
-			state->src.x1 >> 16, state->src.y1 >> 16,
-			drm_rect_width(&state->src) >> 16,
-			drm_rect_height(&state->src) >> 16,
-			state->dst.x1, state->dst.y1,
-			drm_rect_width(&state->dst), drm_rect_height(&state->dst));
+		DRM_DEBUG_KMS("[PLANE:%d:%s] enabled",
+			      plane->base.id, plane->name);
+		DRM_DEBUG_KMS("\tFB:%d, fb = %ux%u format = %s",
+			      fb->base.id, fb->width, fb->height,
+			      drm_get_format_name(fb->pixel_format));
+		DRM_DEBUG_KMS("\tscaler:%d src %dx%d+%d+%d dst %dx%d+%d+%d\n",
+			      state->scaler_id,
+			      state->src.x1 >> 16, state->src.y1 >> 16,
+			      drm_rect_width(&state->src) >> 16,
+			      drm_rect_height(&state->src) >> 16,
+			      state->dst.x1, state->dst.y1,
+			      drm_rect_width(&state->dst),
+			      drm_rect_height(&state->dst));
 	}
 }
 
-- 
2.7.4

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

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

* [PATCH v2 8/8] drm/i915: Add debug prints for encoder modeset hooks
  2016-05-27 17:59 [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5) ville.syrjala
                   ` (6 preceding siblings ...)
  2016-05-27 17:59 ` [PATCH 7/8] drm/i915: kill STANDARD/CURSOR plane screams ville.syrjala
@ 2016-05-27 17:59 ` ville.syrjala
  2016-05-27 18:38   ` Chris Wilson
  2016-05-27 20:21 ` [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5) Chris Wilson
  2016-05-28  5:34 ` ✓ Ro.CI.BAT: success for " Patchwork
  9 siblings, 1 reply; 13+ messages in thread
From: ville.syrjala @ 2016-05-27 17:59 UTC (permalink / raw)
  To: intel-gfx

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

To get a better idea where exactly some error occurred during modeset,
put in some debug prints to tell us when the variuous encoder hooks are
getting called.

v2: i9xx ->enable hook slipped through, bring it to the fold

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e1e61e00880b..35a5623dfcde 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4726,6 +4726,15 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc, unsigned plane_mask
 	intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_ALL_MASK(pipe));
 }
 
+#define intel_call_encoder_func(encoder, func) \
+do { \
+	if (!(encoder)->func) \
+		break; \
+	DRM_DEBUG_KMS("%s " #func " start\n", (encoder)->base.name); \
+	(encoder)->func(encoder); \
+	DRM_DEBUG_KMS("%s " #func " end\n", (encoder)->base.name); \
+} while (0)
+
 static void ironlake_crtc_enable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -4773,8 +4782,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	intel_crtc->active = true;
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
-		if (encoder->pre_enable)
-			encoder->pre_enable(encoder);
+		intel_call_encoder_func(encoder, pre_enable);
 
 	if (intel_crtc->config->has_pch_encoder) {
 		/* Note: FDI PLL enabling _must_ be done before we enable the
@@ -4805,7 +4813,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	drm_crtc_vblank_on(crtc);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
-		encoder->enable(encoder);
+		intel_call_encoder_func(encoder, enable);
 
 	if (HAS_PCH_CPT(dev))
 		cpt_verify_modeset(dev, intel_crtc->pipe);
@@ -4877,10 +4885,8 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	else
 		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
 
-	for_each_encoder_on_crtc(dev, crtc, encoder) {
-		if (encoder->pre_enable)
-			encoder->pre_enable(encoder);
-	}
+	for_each_encoder_on_crtc(dev, crtc, encoder)
+		intel_call_encoder_func(encoder, pre_enable);
 
 	if (intel_crtc->config->has_pch_encoder)
 		dev_priv->display.fdi_link_train(crtc);
@@ -4922,7 +4928,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	drm_crtc_vblank_on(crtc);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder) {
-		encoder->enable(encoder);
+		intel_call_encoder_func(encoder, enable);
 		intel_opregion_notify_encoder(encoder, true);
 	}
 
@@ -4977,7 +4983,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 	}
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
-		encoder->disable(encoder);
+		intel_call_encoder_func(encoder, disable);
 
 	drm_crtc_vblank_off(crtc);
 	assert_vblank_disabled(crtc);
@@ -4990,8 +4996,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 		ironlake_fdi_disable(crtc);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
-		if (encoder->post_disable)
-			encoder->post_disable(encoder);
+		intel_call_encoder_func(encoder, post_disable);
 
 	if (intel_crtc->config->has_pch_encoder) {
 		ironlake_disable_pch_transcoder(dev_priv, pipe);
@@ -5035,7 +5040,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 
 	for_each_encoder_on_crtc(dev, crtc, encoder) {
 		intel_opregion_notify_encoder(encoder, false);
-		encoder->disable(encoder);
+		intel_call_encoder_func(encoder, disable);
 	}
 
 	drm_crtc_vblank_off(crtc);
@@ -5060,8 +5065,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 		intel_ddi_disable_pipe_clock(intel_crtc);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
-		if (encoder->post_disable)
-			encoder->post_disable(encoder);
+		intel_call_encoder_func(encoder, post_disable);
 
 	if (intel_crtc->config->has_pch_encoder) {
 		lpt_disable_pch_transcoder(dev_priv);
@@ -6157,8 +6161,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
-		if (encoder->pre_pll_enable)
-			encoder->pre_pll_enable(encoder);
+		intel_call_encoder_func(encoder, pre_pll_enable);
 
 	if (IS_CHERRYVIEW(dev)) {
 		chv_prepare_pll(intel_crtc, intel_crtc->config);
@@ -6169,8 +6172,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	}
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
-		if (encoder->pre_enable)
-			encoder->pre_enable(encoder);
+		intel_call_encoder_func(encoder, pre_enable);
 
 	i9xx_pfit_enable(intel_crtc);
 
@@ -6183,7 +6185,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	drm_crtc_vblank_on(crtc);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
-		encoder->enable(encoder);
+		intel_call_encoder_func(encoder, enable);
 }
 
 static void i9xx_set_pll_dividers(struct intel_crtc *crtc)
@@ -6224,8 +6226,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
-		if (encoder->pre_enable)
-			encoder->pre_enable(encoder);
+		intel_call_encoder_func(encoder, pre_enable);
 
 	i9xx_enable_pll(intel_crtc);
 
@@ -6240,7 +6241,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 	drm_crtc_vblank_on(crtc);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
-		encoder->enable(encoder);
+		intel_call_encoder_func(encoder, enable);
 }
 
 static void i9xx_pfit_disable(struct intel_crtc *crtc)
@@ -6274,7 +6275,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 		intel_wait_for_vblank(dev, pipe);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
-		encoder->disable(encoder);
+		intel_call_encoder_func(encoder, disable);
 
 	drm_crtc_vblank_off(crtc);
 	assert_vblank_disabled(crtc);
@@ -6284,8 +6285,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	i9xx_pfit_disable(intel_crtc);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
-		if (encoder->post_disable)
-			encoder->post_disable(encoder);
+		intel_call_encoder_func(encoder, post_disable);
 
 	if (!intel_crtc->config->has_dsi_encoder) {
 		if (IS_CHERRYVIEW(dev))
@@ -6297,8 +6297,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	}
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
-		if (encoder->post_pll_disable)
-			encoder->post_pll_disable(encoder);
+		intel_call_encoder_func(encoder, post_pll_disable);
 
 	if (!IS_GEN2(dev))
 		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
@@ -15830,9 +15829,8 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder)
 			DRM_DEBUG_KMS("[ENCODER:%d:%s] manually disabled\n",
 				      encoder->base.base.id,
 				      encoder->base.name);
-			encoder->disable(encoder);
-			if (encoder->post_disable)
-				encoder->post_disable(encoder);
+			intel_call_encoder_func(encoder, disable);
+			intel_call_encoder_func(encoder, post_disable);
 		}
 		encoder->base.crtc = NULL;
 
-- 
2.7.4

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

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

* Re: [PATCH v2 8/8] drm/i915: Add debug prints for encoder modeset hooks
  2016-05-27 17:59 ` [PATCH v2 8/8] drm/i915: Add debug prints for encoder modeset hooks ville.syrjala
@ 2016-05-27 18:38   ` Chris Wilson
  0 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2016-05-27 18:38 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Fri, May 27, 2016 at 08:59:26PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> To get a better idea where exactly some error occurred during modeset,
> put in some debug prints to tell us when the variuous encoder hooks are
> getting called.
> 
> v2: i9xx ->enable hook slipped through, bring it to the fold

ftrace=function_graph ? (At least compare and contrast)

The macro has the advantage / disadvantage of always being compiled in.

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 58 +++++++++++++++++-------------------
>  1 file changed, 28 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e1e61e00880b..35a5623dfcde 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4726,6 +4726,15 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc, unsigned plane_mask
>  	intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_ALL_MASK(pipe));
>  }
>  
> +#define intel_call_encoder_func(encoder, func) \
> +do { \
> +	if (!(encoder)->func) \
> +		break; \
> +	DRM_DEBUG_KMS("%s " #func " start\n", (encoder)->base.name); \

%pF would lookup the (encoder)->func name, then all the format strings
would be the same at least.

Is it too soon to ask for error codes from modesetting?...
-Chris

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

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

* Re: [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5)
  2016-05-27 17:59 [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5) ville.syrjala
                   ` (7 preceding siblings ...)
  2016-05-27 17:59 ` [PATCH v2 8/8] drm/i915: Add debug prints for encoder modeset hooks ville.syrjala
@ 2016-05-27 20:21 ` Chris Wilson
  2016-05-30 14:47   ` Ville Syrjälä
  2016-05-28  5:34 ` ✓ Ro.CI.BAT: success for " Patchwork
  9 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2016-05-27 20:21 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Fri, May 27, 2016 at 08:59:18PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> This is the remainder (the i915 specific parts) of my earlier
> {crtc,plane}->name series [1]. I think at least one i915 specific patch
> was already merged, and I think I added one or two new ones at some
> point.

Making the message more informative is always appreciated, and I
couldn't spot anything blatantly wrong,

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
 
> I'm not so sure about the last patch. It was somewhat useful in narrowing
> down some underrun locations and whatnot for me, but normally it might
> be just noise. So I'm happy to keep it in my private stash for a rainy day
> if people are fed up with dmesg spam.

If you have a compelling usecase, it's not the worst thing we've ever
done. If there's no rush, we should try to leverage ftrace, or something
similar, rather than roll our own partial solution.
-Chris

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

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

* ✓ Ro.CI.BAT: success for drm/i915: Give crtcs and planes actual names (v5)
  2016-05-27 17:59 [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5) ville.syrjala
                   ` (8 preceding siblings ...)
  2016-05-27 20:21 ` [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5) Chris Wilson
@ 2016-05-28  5:34 ` Patchwork
  9 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2016-05-28  5:34 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Give crtcs and planes actual names (v5)
URL   : https://patchwork.freedesktop.org/series/7905/
State : success

== Summary ==

Series 7905v1 drm/i915: Give crtcs and planes actual names (v5)
http://patchwork.freedesktop.org/api/1.0/series/7905/revisions/1/mbox

Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                dmesg-warn -> PASS       (ro-bdw-i7-5600u)
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (ro-bdw-i7-5600u)

fi-bdw-i7-5557u  total:209  pass:197  dwarn:0   dfail:0   fail:0   skip:12 
fi-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
fi-skl-i7-6700k  total:209  pass:184  dwarn:0   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i7-5600u  total:209  pass:181  dwarn:0   dfail:0   fail:0   skip:28 
ro-bsw-n3050     total:209  pass:168  dwarn:0   dfail:0   fail:2   skip:39 
ro-byt-n2820     total:209  pass:170  dwarn:0   dfail:0   fail:2   skip:37 
ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-ilk-i7-620lm  total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:209  pass:177  dwarn:0   dfail:0   fail:0   skip:32 
ro-ivb2-i7-3770  total:209  pass:181  dwarn:0   dfail:0   fail:0   skip:28 
ro-snb-i7-2620M  total:209  pass:170  dwarn:0   dfail:0   fail:1   skip:38 
fi-bsw-n3050 failed to connect after reboot
fi-byt-n2820 failed to connect after reboot
fi-hsw-i7-4770k failed to connect after reboot
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1043/

5c0712f drm-intel-nightly: 2016y-05m-27d-12h-32m-45s UTC integration manifest
28d32fd drm/i915: Add debug prints for encoder modeset hooks
c80a5fc drm/i915: kill STANDARD/CURSOR plane screams
1f7f0cd drm/i915: Give encoders useful names
f0ded37 drm/i915: Give meaningful names to all the planes
6f5269f drm/i915: Don't leak primary/cursor planes on crtc init failure
0e491ad drm/i915: Set crtc->name to "pipe A", "pipe B", etc.
4b481fd drm/i915: Use plane->name in debug prints
51819f9 drm/i915: Use crtc->name in debug messages

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

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

* Re: [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5)
  2016-05-27 20:21 ` [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5) Chris Wilson
@ 2016-05-30 14:47   ` Ville Syrjälä
  0 siblings, 0 replies; 13+ messages in thread
From: Ville Syrjälä @ 2016-05-30 14:47 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On Fri, May 27, 2016 at 09:21:45PM +0100, Chris Wilson wrote:
> On Fri, May 27, 2016 at 08:59:18PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > This is the remainder (the i915 specific parts) of my earlier
> > {crtc,plane}->name series [1]. I think at least one i915 specific patch
> > was already merged, and I think I added one or two new ones at some
> > point.
> 
> Making the message more informative is always appreciated, and I
> couldn't spot anything blatantly wrong,
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>  
> > I'm not so sure about the last patch. It was somewhat useful in narrowing
> > down some underrun locations and whatnot for me, but normally it might
> > be just noise. So I'm happy to keep it in my private stash for a rainy day
> > if people are fed up with dmesg spam.
> 
> If you have a compelling usecase, it's not the worst thing we've ever
> done. If there's no rush, we should try to leverage ftrace, or something
> similar, rather than roll our own partial solution.

I've pushed everything but the last patch to dinq. Thanks for the
review.

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

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

end of thread, other threads:[~2016-05-30 14:47 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-27 17:59 [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5) ville.syrjala
2016-05-27 17:59 ` [PATCH v2 1/8] drm/i915: Use crtc->name in debug messages ville.syrjala
2016-05-27 17:59 ` [PATCH v2 2/8] drm/i915: Use plane->name in debug prints ville.syrjala
2016-05-27 17:59 ` [PATCH v5 3/8] drm/i915: Set crtc->name to "pipe A", "pipe B", etc ville.syrjala
2016-05-27 17:59 ` [PATCH 4/8] drm/i915: Don't leak primary/cursor planes on crtc init failure ville.syrjala
2016-05-27 17:59 ` [PATCH v3 5/8] drm/i915: Give meaningful names to all the planes ville.syrjala
2016-05-27 17:59 ` [PATCH v3 6/8] drm/i915: Give encoders useful names ville.syrjala
2016-05-27 17:59 ` [PATCH 7/8] drm/i915: kill STANDARD/CURSOR plane screams ville.syrjala
2016-05-27 17:59 ` [PATCH v2 8/8] drm/i915: Add debug prints for encoder modeset hooks ville.syrjala
2016-05-27 18:38   ` Chris Wilson
2016-05-27 20:21 ` [PATCH 0/8] drm/i915: Give crtcs and planes actual names (v5) Chris Wilson
2016-05-30 14:47   ` Ville Syrjälä
2016-05-28  5:34 ` ✓ Ro.CI.BAT: success for " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.