[Intel-gfx] [PATCH 0/6] drm/i915: Proper dbuf global state

[Intel-gfx] [PATCH 0/6] drm/i915: Proper dbuf global state

[Ville Syrjala]

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

Introduce a global state object for dbuf and polish up
some surrounding stuff.

Only lightly smoke tested on kbl, but hopefully the icl+
will just work (tm) as well.

Immediate TODO:
- Rebase on top of current dbuf fixes once they land

Future TODO:
- Relocate the whole thing to some new file
- Try to finally elimninate distrucst_bios_wm
- Maybe other stuff I'm missing?

Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

Ville Syrjälä (6):
  drm/i915: Introduce proper dbuf state
  drm/i915: Polish some dbuf debugs
  drm/i915: Unify the low level dbuf code
  drm/i915: Nuke skl_ddb_get_hw_state()
  drm/i915: Move the dbuf pre/post plane update
  drm/i915: Clean up dbuf debugs during .atomic_check()

 drivers/gpu/drm/i915/display/intel_display.c  |  56 ++--
 .../drm/i915/display/intel_display_power.c    |  82 +++---
 .../drm/i915/display/intel_display_power.h    |   6 +-
 .../drm/i915/display/intel_display_types.h    |  12 +-
 drivers/gpu/drm/i915/i915_drv.h               |  11 +-
 drivers/gpu/drm/i915/intel_pm.c               | 247 +++++++++++++-----
 drivers/gpu/drm/i915/intel_pm.h               |  25 +-
 7 files changed, 273 insertions(+), 166 deletions(-)

-- 
2.24.1

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

[Intel-gfx] [PATCH 1/6] drm/i915: Introduce proper dbuf state

[Ville Syrjala]

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

Add a global state to track the dbuf slices. Gets rid of all the nasty
coupling between state->modeset and dbuf recompulation. Also we can now
totally nuke state->active_pipe_changes.

dev_priv->wm.distrust_bios_wm still remains, but should probably also
get nuked from orbit later. Just didn't spend any significant time
pondering how to go about. The obvious thing would be to just recompute
the thing every time.

Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |  69 ++++---
 .../drm/i915/display/intel_display_power.c    |   4 +-
 .../drm/i915/display/intel_display_types.h    |  13 --
 drivers/gpu/drm/i915/i915_drv.h               |  11 +-
 drivers/gpu/drm/i915/intel_pm.c               | 185 ++++++++++++------
 drivers/gpu/drm/i915/intel_pm.h               |  22 +++
 6 files changed, 205 insertions(+), 99 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e09d3c93c52b..e331ab900336 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7558,6 +7558,8 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
 		to_intel_bw_state(dev_priv->bw_obj.state);
 	struct intel_cdclk_state *cdclk_state =
 		to_intel_cdclk_state(dev_priv->cdclk.obj.state);
+	struct intel_dbuf_state *dbuf_state =
+		to_intel_dbuf_state(dev_priv->dbuf.obj.state);
 	struct intel_crtc_state *crtc_state =
 		to_intel_crtc_state(crtc->base.state);
 	enum intel_display_power_domain domain;
@@ -7630,6 +7632,8 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
 	cdclk_state->min_voltage_level[pipe] = 0;
 	cdclk_state->active_pipes &= ~BIT(pipe);
 
+	dbuf_state->active_pipes &= ~BIT(pipe);
+
 	bw_state->data_rate[pipe] = 0;
 	bw_state->num_active_planes[pipe] = 0;
 }
@@ -14089,10 +14093,10 @@ static void verify_wm_state(struct intel_crtc *crtc,
 	hw_enabled_slices = intel_enabled_dbuf_slices_mask(dev_priv);
 
 	if (INTEL_GEN(dev_priv) >= 11 &&
-	    hw_enabled_slices != dev_priv->enabled_dbuf_slices_mask)
+	    hw_enabled_slices != dev_priv->dbuf.enabled_slices)
 		drm_err(&dev_priv->drm,
 			"mismatch in DBUF Slices (expected 0x%x, got 0x%x)\n",
-			dev_priv->enabled_dbuf_slices_mask,
+			dev_priv->dbuf.enabled_slices,
 			hw_enabled_slices);
 
 	/* planes */
@@ -14627,9 +14631,7 @@ static int intel_modeset_checks(struct intel_atomic_state *state)
 	state->modeset = true;
 	state->active_pipes = intel_calc_active_pipes(state, dev_priv->active_pipes);
 
-	state->active_pipe_changes = state->active_pipes ^ dev_priv->active_pipes;
-
-	if (state->active_pipe_changes) {
+	if (state->active_pipes != dev_priv->active_pipes) {
 		ret = _intel_atomic_lock_global_state(state);
 		if (ret)
 			return ret;
@@ -15450,24 +15452,38 @@ static void intel_update_trans_port_sync_crtcs(struct intel_crtc *crtc,
 static void icl_dbuf_slice_pre_update(struct intel_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-	u8 hw_enabled_slices = dev_priv->enabled_dbuf_slices_mask;
-	u8 required_slices = state->enabled_dbuf_slices_mask;
-	u8 slices_union = hw_enabled_slices | required_slices;
+	const struct intel_dbuf_state *new_dbuf_state =
+		intel_atomic_get_new_dbuf_state(state);
+	const struct intel_dbuf_state *old_dbuf_state =
+		intel_atomic_get_old_dbuf_state(state);
+
+	if (!new_dbuf_state ||
+	    new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices)
+		return;
 
-	/* If 2nd DBuf slice required, enable it here */
-	if (INTEL_GEN(dev_priv) >= 11 && slices_union != hw_enabled_slices)
-		icl_dbuf_slices_update(dev_priv, slices_union);
+	WARN_ON(!new_dbuf_state->base.changed);
+
+	icl_dbuf_slices_update(dev_priv,
+			       old_dbuf_state->enabled_slices |
+			       new_dbuf_state->enabled_slices);
 }
 
 static void icl_dbuf_slice_post_update(struct intel_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-	u8 hw_enabled_slices = dev_priv->enabled_dbuf_slices_mask;
-	u8 required_slices = state->enabled_dbuf_slices_mask;
+	const struct intel_dbuf_state *new_dbuf_state =
+		intel_atomic_get_new_dbuf_state(state);
+	const struct intel_dbuf_state *old_dbuf_state =
+		intel_atomic_get_old_dbuf_state(state);
+
+	if (!new_dbuf_state ||
+	    new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices)
+		return;
 
-	/* If 2nd DBuf slice is no more required disable it */
-	if (INTEL_GEN(dev_priv) >= 11 && required_slices != hw_enabled_slices)
-		icl_dbuf_slices_update(dev_priv, required_slices);
+	WARN_ON(!new_dbuf_state->base.changed);
+
+	icl_dbuf_slices_update(dev_priv,
+			       new_dbuf_state->enabled_slices);
 }
 
 static void skl_commit_modeset_enables(struct intel_atomic_state *state)
@@ -15722,9 +15738,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 	if (state->modeset)
 		intel_encoders_update_prepare(state);
 
-	/* Enable all new slices, we might need */
-	if (state->modeset)
-		icl_dbuf_slice_pre_update(state);
+	icl_dbuf_slice_pre_update(state);
 
 	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
 	dev_priv->display.commit_modeset_enables(state);
@@ -15779,9 +15793,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 			dev_priv->display.optimize_watermarks(state, crtc);
 	}
 
-	/* Disable all slices, we don't need */
-	if (state->modeset)
-		icl_dbuf_slice_post_update(state);
+	icl_dbuf_slice_post_update(state);
 
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		intel_post_plane_update(state, crtc);
@@ -17667,10 +17679,14 @@ void intel_modeset_init_hw(struct drm_i915_private *i915)
 {
 	struct intel_cdclk_state *cdclk_state =
 		to_intel_cdclk_state(i915->cdclk.obj.state);
+	struct intel_dbuf_state *dbuf_state =
+		to_intel_dbuf_state(i915->dbuf.obj.state);
 
 	intel_update_cdclk(i915);
 	intel_dump_cdclk_config(&i915->cdclk.hw, "Current CDCLK");
 	cdclk_state->logical = cdclk_state->actual = i915->cdclk.hw;
+
+	dbuf_state->enabled_slices = i915->dbuf.enabled_slices;
 }
 
 static int sanitize_watermarks_add_affected(struct drm_atomic_state *state)
@@ -17941,6 +17957,10 @@ int intel_modeset_init(struct drm_i915_private *i915)
 	if (ret)
 		return ret;
 
+	ret = intel_dbuf_init(i915);
+	if (ret)
+		return ret;
+
 	ret = intel_bw_init(i915);
 	if (ret)
 		return ret;
@@ -18435,6 +18455,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_cdclk_state *cdclk_state =
 		to_intel_cdclk_state(dev_priv->cdclk.obj.state);
+	struct intel_dbuf_state *dbuf_state =
+		to_intel_dbuf_state(dev_priv->dbuf.obj.state);
 	enum pipe pipe;
 	struct intel_crtc *crtc;
 	struct intel_encoder *encoder;
@@ -18466,7 +18488,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			    enableddisabled(crtc_state->hw.active));
 	}
 
-	dev_priv->active_pipes = cdclk_state->active_pipes = active_pipes;
+	dev_priv->active_pipes = cdclk_state->active_pipes =
+		dbuf_state->active_pipes = active_pipes;
 
 	readout_plane_state(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 53056def5414..c24e1de78b9d 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -1043,7 +1043,7 @@ static bool gen9_dc_off_power_well_enabled(struct drm_i915_private *dev_priv,
 static void gen9_assert_dbuf_enabled(struct drm_i915_private *dev_priv)
 {
 	u8 hw_enabled_dbuf_slices = intel_enabled_dbuf_slices_mask(dev_priv);
-	u8 enabled_dbuf_slices = dev_priv->enabled_dbuf_slices_mask;
+	u8 enabled_dbuf_slices = dev_priv->dbuf.enabled_slices;
 
 	WARN(hw_enabled_dbuf_slices != enabled_dbuf_slices,
 	     "Unexpected DBuf power power state (0x%08x, expected 0x%08x)\n",
@@ -4463,7 +4463,7 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
 				     (req_slices & BIT(i)) != 0);
 	}
 
-	dev_priv->enabled_dbuf_slices_mask = req_slices;
+	dev_priv->dbuf.enabled_slices = req_slices;
 
 	mutex_unlock(&power_domains->lock);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 283c622f8ba1..12ac47c33171 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -468,16 +468,6 @@ struct intel_atomic_state {
 
 	bool dpll_set, modeset;
 
-	/*
-	 * Does this transaction change the pipes that are active?  This mask
-	 * tracks which CRTC's have changed their active state at the end of
-	 * the transaction (not counting the temporary disable during modesets).
-	 * This mask should only be non-zero when intel_state->modeset is true,
-	 * but the converse is not necessarily true; simply changing a mode may
-	 * not flip the final active status of any CRTC's
-	 */
-	u8 active_pipe_changes;
-
 	u8 active_pipes;
 
 	struct intel_shared_dpll_state shared_dpll[I915_NUM_PLLS];
@@ -495,9 +485,6 @@ struct intel_atomic_state {
 	 */
 	bool global_state_changed;
 
-	/* Number of enabled DBuf slices */
-	u8 enabled_dbuf_slices_mask;
-
 	struct i915_sw_fence commit_ready;
 
 	struct llist_node freed;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index da509d9b8895..2638f3fcd1aa 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1007,6 +1007,13 @@ struct drm_i915_private {
 		struct intel_global_obj obj;
 	} cdclk;
 
+	struct {
+		/* The current hardware dbuf configuration */
+		u8 enabled_slices;
+
+		struct intel_global_obj obj;
+	} dbuf;
+
 	/**
 	 * wq - Driver workqueue for GEM.
 	 *
@@ -1182,12 +1189,12 @@ struct drm_i915_private {
 		 * Set during HW readout of watermarks/DDB.  Some platforms
 		 * need to know when we're still using BIOS-provided values
 		 * (which we don't fully trust).
+		 *
+		 * FIXME get rid of this.
 		 */
 		bool distrust_bios_wm;
 	} wm;
 
-	u8 enabled_dbuf_slices_mask; /* GEN11 has configurable 2 slices */
-
 	struct dram_info {
 		bool valid;
 		bool is_16gb_dimm;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ffac0b862ca5..31179e30bfe3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3845,7 +3845,7 @@ static u16 intel_get_ddb_size(struct drm_i915_private *dev_priv)
 static u8 skl_compute_dbuf_slices(const struct intel_crtc_state *crtc_state,
 				  u32 active_pipes);
 
-static void
+static int
 skl_ddb_get_pipe_allocation_limits(struct drm_i915_private *dev_priv,
 				   const struct intel_crtc_state *crtc_state,
 				   const u64 total_data_rate,
@@ -3858,30 +3858,29 @@ skl_ddb_get_pipe_allocation_limits(struct drm_i915_private *dev_priv,
 	const struct intel_crtc *crtc;
 	u32 pipe_width = 0, total_width_in_range = 0, width_before_pipe_in_range = 0;
 	enum pipe for_pipe = to_intel_crtc(for_crtc)->pipe;
+	struct intel_dbuf_state *new_dbuf_state =
+		intel_atomic_get_new_dbuf_state(intel_state);
+	const struct intel_dbuf_state *old_dbuf_state =
+		intel_atomic_get_old_dbuf_state(intel_state);
+	u8 active_pipes = new_dbuf_state->active_pipes;
 	u16 ddb_size;
 	u32 ddb_range_size;
 	u32 i;
 	u32 dbuf_slice_mask;
-	u32 active_pipes;
 	u32 offset;
 	u32 slice_size;
 	u32 total_slice_mask;
 	u32 start, end;
+	int ret;
+
+	*num_active = hweight8(active_pipes);
 
-	if (drm_WARN_ON(&dev_priv->drm, !state) || !crtc_state->hw.active) {
+	if (!crtc_state->hw.active) {
 		alloc->start = 0;
 		alloc->end = 0;
-		*num_active = hweight8(dev_priv->active_pipes);
-		return;
+		return 0;
 	}
 
-	if (intel_state->active_pipe_changes)
-		active_pipes = intel_state->active_pipes;
-	else
-		active_pipes = dev_priv->active_pipes;
-
-	*num_active = hweight8(active_pipes);
-
 	ddb_size = intel_get_ddb_size(dev_priv);
 
 	slice_size = ddb_size / INTEL_INFO(dev_priv)->num_supported_dbuf_slices;
@@ -3894,13 +3893,16 @@ skl_ddb_get_pipe_allocation_limits(struct drm_i915_private *dev_priv,
 	 * that changes the active CRTC list or do modeset would need to
 	 * grab _all_ crtc locks, including the one we currently hold.
 	 */
-	if (!intel_state->active_pipe_changes && !intel_state->modeset) {
+	if (old_dbuf_state->active_pipes == new_dbuf_state->active_pipes &&
+	    !dev_priv->wm.distrust_bios_wm) {
 		/*
 		 * alloc may be cleared by clear_intel_crtc_state,
 		 * copy from old state to be sure
+		 *
+		 * FIXME get rid of this mess
 		 */
 		*alloc = to_intel_crtc_state(for_crtc->state)->wm.skl.ddb;
-		return;
+		return 0;
 	}
 
 	/*
@@ -3979,7 +3981,13 @@ skl_ddb_get_pipe_allocation_limits(struct drm_i915_private *dev_priv,
 	 * FIXME: For now we always enable slice S1 as per
 	 * the Bspec display initialization sequence.
 	 */
-	intel_state->enabled_dbuf_slices_mask = total_slice_mask | BIT(DBUF_S1);
+	new_dbuf_state->enabled_slices = total_slice_mask | BIT(DBUF_S1);
+
+	if (old_dbuf_state->enabled_slices != new_dbuf_state->enabled_slices) {
+		ret = intel_atomic_serialize_global_state(&new_dbuf_state->base);
+		if (ret)
+			return ret;
+	}
 
 	start = ddb_range_size * width_before_pipe_in_range / total_width_in_range;
 	end = ddb_range_size *
@@ -3990,9 +3998,8 @@ skl_ddb_get_pipe_allocation_limits(struct drm_i915_private *dev_priv,
 
 	DRM_DEBUG_KMS("Pipe %d ddb %d-%d\n", for_pipe,
 		      alloc->start, alloc->end);
-	DRM_DEBUG_KMS("Enabled ddb slices mask %x num supported %d\n",
-		      intel_state->enabled_dbuf_slices_mask,
-		      INTEL_INFO(dev_priv)->num_supported_dbuf_slices);
+
+	return 0;
 }
 
 static int skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
@@ -4112,8 +4119,8 @@ void skl_pipe_ddb_get_hw_state(struct intel_crtc *crtc,
 
 void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv)
 {
-	dev_priv->enabled_dbuf_slices_mask =
-				intel_enabled_dbuf_slices_mask(dev_priv);
+	dev_priv->dbuf.enabled_slices =
+		intel_enabled_dbuf_slices_mask(dev_priv);
 }
 
 /*
@@ -4563,6 +4570,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *crtc_state)
 	u64 uv_plane_data_rate[I915_MAX_PLANES] = {};
 	u32 blocks;
 	int level;
+	int ret;
 
 	/* Clear the partitioning for disabled planes. */
 	memset(crtc_state->wm.skl.plane_ddb_y, 0, sizeof(crtc_state->wm.skl.plane_ddb_y));
@@ -4587,8 +4595,12 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *crtc_state)
 							 uv_plane_data_rate);
 
 
-	skl_ddb_get_pipe_allocation_limits(dev_priv, crtc_state, total_data_rate,
-					   alloc, &num_active);
+	ret = skl_ddb_get_pipe_allocation_limits(dev_priv, crtc_state,
+						 total_data_rate,
+						 alloc, &num_active);
+	if (ret)
+		return ret;
+
 	alloc_size = skl_ddb_entry_size(alloc);
 	if (alloc_size == 0)
 		return 0;
@@ -5471,14 +5483,11 @@ skl_ddb_add_affected_planes(const struct intel_crtc_state *old_crtc_state,
 static int
 skl_compute_ddb(struct intel_atomic_state *state)
 {
-	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
 	struct intel_crtc_state *old_crtc_state;
 	struct intel_crtc_state *new_crtc_state;
 	struct intel_crtc *crtc;
 	int ret, i;
 
-	state->enabled_dbuf_slices_mask = dev_priv->enabled_dbuf_slices_mask;
-
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
 		ret = skl_allocate_pipe_ddb(new_crtc_state);
@@ -5618,7 +5627,8 @@ skl_print_wm_changes(struct intel_atomic_state *state)
 	}
 }
 
-static int intel_add_all_pipes(struct intel_atomic_state *state)
+static int intel_add_affected_pipes(struct intel_atomic_state *state,
+				    u8 pipe_mask)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
 	struct intel_crtc *crtc;
@@ -5626,6 +5636,9 @@ static int intel_add_all_pipes(struct intel_atomic_state *state)
 	for_each_intel_crtc(&dev_priv->drm, crtc) {
 		struct intel_crtc_state *crtc_state;
 
+		if ((pipe_mask & BIT(crtc->pipe)) == 0)
+			continue;
+
 		crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
 		if (IS_ERR(crtc_state))
 			return PTR_ERR(crtc_state);
@@ -5638,49 +5651,54 @@ static int
 skl_ddb_add_affected_pipes(struct intel_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-	int ret;
+	struct intel_crtc_state *crtc_state;
+	struct intel_crtc *crtc;
+	int i, ret;
 
-	/*
-	 * If this is our first atomic update following hardware readout,
-	 * we can't trust the DDB that the BIOS programmed for us.  Let's
-	 * pretend that all pipes switched active status so that we'll
-	 * ensure a full DDB recompute.
-	 */
 	if (dev_priv->wm.distrust_bios_wm) {
-		ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
-				       state->base.acquire_ctx);
+		/*
+		 * skl_ddb_get_pipe_allocation_limits() currently requires
+		 * all active pipes to be included in the state so that
+		 * it can redistribute the dbuf among them, and it really
+		 * wants to recompute things when distrust_bios_wm is set
+		 * so we add all the pipes to the state.
+		 */
+		ret = intel_add_affected_pipes(state, ~0);
 		if (ret)
 			return ret;
+	}
 
-		state->active_pipe_changes = INTEL_INFO(dev_priv)->pipe_mask;
+	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
+		struct intel_dbuf_state *new_dbuf_state;
+		const struct intel_dbuf_state *old_dbuf_state;
+
+		new_dbuf_state = intel_atomic_get_dbuf_state(state);
+		if (IS_ERR(new_dbuf_state))
+			return ret;
+
+		old_dbuf_state = intel_atomic_get_old_dbuf_state(state);
+
+		new_dbuf_state->active_pipes =
+			intel_calc_active_pipes(state, old_dbuf_state->active_pipes);
+
+		if (old_dbuf_state->active_pipes == new_dbuf_state->active_pipes)
+			break;
+
+		ret = intel_atomic_lock_global_state(&new_dbuf_state->base);
+		if (ret)
+			return ret;
 
 		/*
-		 * We usually only initialize state->active_pipes if we
-		 * we're doing a modeset; make sure this field is always
-		 * initialized during the sanitization process that happens
-		 * on the first commit too.
+		 * skl_ddb_get_pipe_allocation_limits() currently requires
+		 * all active pipes to be included in the state so that
+		 * it can redistribute the dbuf among them.
 		 */
-		if (!state->modeset)
-			state->active_pipes = dev_priv->active_pipes;
-	}
-
-	/*
-	 * If the modeset changes which CRTC's are active, we need to
-	 * recompute the DDB allocation for *all* active pipes, even
-	 * those that weren't otherwise being modified in any way by this
-	 * atomic commit.  Due to the shrinking of the per-pipe allocations
-	 * when new active CRTC's are added, it's possible for a pipe that
-	 * we were already using and aren't changing at all here to suddenly
-	 * become invalid if its DDB needs exceeds its new allocation.
-	 *
-	 * Note that if we wind up doing a full DDB recompute, we can't let
-	 * any other display updates race with this transaction, so we need
-	 * to grab the lock on *all* CRTC's.
-	 */
-	if (state->active_pipe_changes || state->modeset) {
-		ret = intel_add_all_pipes(state);
+		ret = intel_add_affected_pipes(state,
+					       new_dbuf_state->active_pipes);
 		if (ret)
 			return ret;
+
+		break;
 	}
 
 	return 0;
@@ -7513,3 +7531,52 @@ void intel_pm_setup(struct drm_i915_private *dev_priv)
 	dev_priv->runtime_pm.suspended = false;
 	atomic_set(&dev_priv->runtime_pm.wakeref_count, 0);
 }
+
+static struct intel_global_state *intel_dbuf_duplicate_state(struct intel_global_obj *obj)
+{
+	struct intel_dbuf_state *dbuf_state;
+
+	dbuf_state = kmemdup(obj->state, sizeof(*dbuf_state), GFP_KERNEL);
+	if (!dbuf_state)
+		return NULL;
+
+	return &dbuf_state->base;
+}
+
+static void intel_dbuf_destroy_state(struct intel_global_obj *obj,
+				     struct intel_global_state *state)
+{
+	kfree(state);
+}
+
+static const struct intel_global_state_funcs intel_dbuf_funcs = {
+	.atomic_duplicate_state = intel_dbuf_duplicate_state,
+	.atomic_destroy_state = intel_dbuf_destroy_state,
+};
+
+struct intel_dbuf_state *
+intel_atomic_get_dbuf_state(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_global_state *dbuf_state;
+
+	dbuf_state = intel_atomic_get_global_obj_state(state, &dev_priv->dbuf.obj);
+	if (IS_ERR(dbuf_state))
+		return ERR_CAST(dbuf_state);
+
+	return to_intel_dbuf_state(dbuf_state);
+}
+
+int intel_dbuf_init(struct drm_i915_private *dev_priv)
+{
+	struct intel_dbuf_state *dbuf_state;
+
+	dbuf_state = kzalloc(sizeof(*dbuf_state), GFP_KERNEL);
+	if (!dbuf_state)
+		return -ENOMEM;
+
+	intel_atomic_global_obj_init(dev_priv, &dev_priv->dbuf.obj,
+				     &dbuf_state->base, &intel_dbuf_funcs);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
index d60a85421c5a..fadf7cbc44c4 100644
--- a/drivers/gpu/drm/i915/intel_pm.h
+++ b/drivers/gpu/drm/i915/intel_pm.h
@@ -8,6 +8,8 @@
 
 #include <linux/types.h>
 
+#include "display/intel_global_state.h"
+
 #include "i915_reg.h"
 
 struct drm_device;
@@ -59,4 +61,24 @@ void intel_enable_ipc(struct drm_i915_private *dev_priv);
 
 bool intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable);
 
+struct intel_dbuf_state {
+	struct intel_global_state base;
+
+	u8 enabled_slices;
+	u8 active_pipes;
+};
+
+int intel_dbuf_init(struct drm_i915_private *dev_priv);
+
+struct intel_dbuf_state *
+intel_atomic_get_dbuf_state(struct intel_atomic_state *state);
+
+#define to_intel_dbuf_state(x) container_of((x), struct intel_dbuf_state, base)
+#define intel_atomic_get_old_dbuf_state(state) \
+	to_intel_dbuf_state(intel_atomic_get_old_global_obj_state(state, &to_i915(state->base.dev)->dbuf.obj))
+#define intel_atomic_get_new_dbuf_state(state) \
+	to_intel_dbuf_state(intel_atomic_get_new_global_obj_state(state, &to_i915(state->base.dev)->dbuf.obj))
+
+int intel_dbuf_init(struct drm_i915_private *dev_priv);
+
 #endif /* __INTEL_PM_H__ */
-- 
2.24.1

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

[Intel-gfx] [PATCH 2/6] drm/i915: Polish some dbuf debugs

[Ville Syrjala]

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

Polish some of the dbuf code to give more meaningful debug
messages and whatnot. Also we can switch over to the per-device
debugs/warns at the same time.

Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../drm/i915/display/intel_display_power.c    | 40 +++++++++----------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index c24e1de78b9d..f24f42c5c446 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -4405,11 +4405,12 @@ static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv)
 	mutex_unlock(&power_domains->lock);
 }
 
-static inline
-bool intel_dbuf_slice_set(struct drm_i915_private *dev_priv,
-			  i915_reg_t reg, bool enable)
+static void intel_dbuf_slice_set(struct drm_i915_private *dev_priv,
+				 enum dbuf_slice slice, bool enable)
 {
-	u32 val, status;
+	i915_reg_t reg = DBUF_CTL_S(slice);
+	bool state;
+	u32 val;
 
 	val = intel_de_read(dev_priv, reg);
 	val = enable ? (val | DBUF_POWER_REQUEST) : (val & ~DBUF_POWER_REQUEST);
@@ -4417,13 +4418,10 @@ bool intel_dbuf_slice_set(struct drm_i915_private *dev_priv,
 	intel_de_posting_read(dev_priv, reg);
 	udelay(10);
 
-	status = intel_de_read(dev_priv, reg) & DBUF_POWER_STATE;
-	if ((enable && !status) || (!enable && status)) {
-		drm_err(&dev_priv->drm, "DBus power %s timeout!\n",
-			enable ? "enable" : "disable");
-		return false;
-	}
-	return true;
+	state = intel_de_read(dev_priv, reg) & DBUF_POWER_STATE;
+	drm_WARN(&dev_priv->drm, enable != state,
+		 "DBuf slice %d power %s timeout!\n",
+		 slice, enable ? "enable" : "disable");
 }
 
 static void gen9_dbuf_enable(struct drm_i915_private *dev_priv)
@@ -4439,14 +4437,16 @@ static void gen9_dbuf_disable(struct drm_i915_private *dev_priv)
 void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
 			    u8 req_slices)
 {
-	int i;
-	int max_slices = INTEL_INFO(dev_priv)->num_supported_dbuf_slices;
+	int num_slices = INTEL_INFO(dev_priv)->num_supported_dbuf_slices;
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
+	enum dbuf_slice slice;
 
-	WARN(hweight8(req_slices) > max_slices,
-	     "Invalid number of dbuf slices requested\n");
+	drm_WARN(&dev_priv->drm, req_slices & ~(BIT(num_slices) - 1),
+		 "Invalid set of dbuf slices (0x%x) requested (num dbuf slices %d)\n",
+		 req_slices, num_slices);
 
-	DRM_DEBUG_KMS("Updating dbuf slices to 0x%x\n", req_slices);
+	drm_dbg_kms(&dev_priv->drm,
+		    "Updating dbuf slices to 0x%x\n", req_slices);
 
 	/*
 	 * Might be running this in parallel to gen9_dc_off_power_well_enable
@@ -4457,11 +4457,9 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
 	 */
 	mutex_lock(&power_domains->lock);
 
-	for (i = 0; i < max_slices; i++) {
-		intel_dbuf_slice_set(dev_priv,
-				     DBUF_CTL_S(i),
-				     (req_slices & BIT(i)) != 0);
-	}
+	for (slice = DBUF_S1; slice < num_slices; slice++)
+		intel_dbuf_slice_set(dev_priv, slice,
+				     req_slices & BIT(slice));
 
 	dev_priv->dbuf.enabled_slices = req_slices;
 
-- 
2.24.1

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

[Intel-gfx] [PATCH 3/6] drm/i915: Unify the low level dbuf code

[Ville Syrjala]

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

The low level dbuf slice code is rather inconsitent with its
functiona naming and organization. Make it more consistent.

Also share the enable/disable functions between all platforms
since the same code works just fine for all of them.

Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  | 10 ++--
 .../drm/i915/display/intel_display_power.c    | 46 ++++++++-----------
 .../drm/i915/display/intel_display_power.h    |  6 +--
 3 files changed, 27 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e331ab900336..7fb25c7655d1 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15463,9 +15463,9 @@ static void icl_dbuf_slice_pre_update(struct intel_atomic_state *state)
 
 	WARN_ON(!new_dbuf_state->base.changed);
 
-	icl_dbuf_slices_update(dev_priv,
-			       old_dbuf_state->enabled_slices |
-			       new_dbuf_state->enabled_slices);
+	gen9_dbuf_slices_update(dev_priv,
+				old_dbuf_state->enabled_slices |
+				new_dbuf_state->enabled_slices);
 }
 
 static void icl_dbuf_slice_post_update(struct intel_atomic_state *state)
@@ -15482,8 +15482,8 @@ static void icl_dbuf_slice_post_update(struct intel_atomic_state *state)
 
 	WARN_ON(!new_dbuf_state->base.changed);
 
-	icl_dbuf_slices_update(dev_priv,
-			       new_dbuf_state->enabled_slices);
+	gen9_dbuf_slices_update(dev_priv,
+				new_dbuf_state->enabled_slices);
 }
 
 static void skl_commit_modeset_enables(struct intel_atomic_state *state)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index f24f42c5c446..54715da7dc32 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -4405,15 +4405,18 @@ static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv)
 	mutex_unlock(&power_domains->lock);
 }
 
-static void intel_dbuf_slice_set(struct drm_i915_private *dev_priv,
-				 enum dbuf_slice slice, bool enable)
+static void gen9_dbuf_slice_set(struct drm_i915_private *dev_priv,
+				enum dbuf_slice slice, bool enable)
 {
 	i915_reg_t reg = DBUF_CTL_S(slice);
 	bool state;
 	u32 val;
 
 	val = intel_de_read(dev_priv, reg);
-	val = enable ? (val | DBUF_POWER_REQUEST) : (val & ~DBUF_POWER_REQUEST);
+	if (enable)
+		val |= DBUF_POWER_REQUEST;
+	else
+		val &= ~DBUF_POWER_REQUEST;
 	intel_de_write(dev_priv, reg, val);
 	intel_de_posting_read(dev_priv, reg);
 	udelay(10);
@@ -4424,18 +4427,8 @@ static void intel_dbuf_slice_set(struct drm_i915_private *dev_priv,
 		 slice, enable ? "enable" : "disable");
 }
 
-static void gen9_dbuf_enable(struct drm_i915_private *dev_priv)
-{
-	icl_dbuf_slices_update(dev_priv, BIT(DBUF_S1));
-}
-
-static void gen9_dbuf_disable(struct drm_i915_private *dev_priv)
-{
-	icl_dbuf_slices_update(dev_priv, 0);
-}
-
-void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
-			    u8 req_slices)
+void gen9_dbuf_slices_update(struct drm_i915_private *dev_priv,
+			     u8 req_slices)
 {
 	int num_slices = INTEL_INFO(dev_priv)->num_supported_dbuf_slices;
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
@@ -4458,26 +4451,25 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
 	mutex_lock(&power_domains->lock);
 
 	for (slice = DBUF_S1; slice < num_slices; slice++)
-		intel_dbuf_slice_set(dev_priv, slice,
-				     req_slices & BIT(slice));
+		gen9_dbuf_slice_set(dev_priv, slice, req_slices & BIT(slice));
 
 	dev_priv->dbuf.enabled_slices = req_slices;
 
 	mutex_unlock(&power_domains->lock);
 }
 
-static void icl_dbuf_enable(struct drm_i915_private *dev_priv)
+static void gen9_dbuf_enable(struct drm_i915_private *dev_priv)
 {
-	/*
-	 * Just power up 1 slice, we will
-	 * figure out later which slices we have and what we need.
-	 */
-	icl_dbuf_slices_update(dev_priv, BIT(DBUF_S1));
+	/* TOOD: Rebase on Stan's patch adding the readout here */
+	dev_priv->dbuf.enabled_slices = intel_enabled_dbuf_slices_mask(dev_priv);
+
+	gen9_dbuf_slices_update(dev_priv, BIT(DBUF_S1) |
+				dev_priv->dbuf.enabled_slices);
 }
 
-static void icl_dbuf_disable(struct drm_i915_private *dev_priv)
+static void gen9_dbuf_disable(struct drm_i915_private *dev_priv)
 {
-	icl_dbuf_slices_update(dev_priv, 0);
+	gen9_dbuf_slices_update(dev_priv, 0);
 }
 
 static void icl_mbus_init(struct drm_i915_private *dev_priv)
@@ -5021,7 +5013,7 @@ static void icl_display_core_init(struct drm_i915_private *dev_priv,
 	intel_cdclk_init_hw(dev_priv);
 
 	/* 5. Enable DBUF. */
-	icl_dbuf_enable(dev_priv);
+	gen9_dbuf_enable(dev_priv);
 
 	/* 6. Setup MBUS. */
 	icl_mbus_init(dev_priv);
@@ -5044,7 +5036,7 @@ static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
 	/* 1. Disable all display engine functions -> aready done */
 
 	/* 2. Disable DBUF */
-	icl_dbuf_disable(dev_priv);
+	gen9_dbuf_disable(dev_priv);
 
 	/* 3. Disable CD clock */
 	intel_cdclk_uninit_hw(dev_priv);
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
index 601e000ffd0d..1a275611241e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.h
+++ b/drivers/gpu/drm/i915/display/intel_display_power.h
@@ -312,13 +312,13 @@ enum dbuf_slice {
 	DBUF_S2,
 };
 
+void gen9_dbuf_slices_update(struct drm_i915_private *dev_priv,
+			     u8 req_slices);
+
 #define with_intel_display_power(i915, domain, wf) \
 	for ((wf) = intel_display_power_get((i915), (domain)); (wf); \
 	     intel_display_power_put_async((i915), (domain), (wf)), (wf) = 0)
 
-void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
-			    u8 req_slices);
-
 void chv_phy_powergate_lanes(struct intel_encoder *encoder,
 			     bool override, unsigned int mask);
 bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy phy,
-- 
2.24.1

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

[Intel-gfx] [PATCH 4/6] drm/i915: Nuke skl_ddb_get_hw_state()

[Ville Syrjala]

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

skl_ddb_get_hw_state() is redundant and kinda called in thw wrong
spot anyway. Just kill it.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 31179e30bfe3..0032458f0a5d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4117,12 +4117,6 @@ void skl_pipe_ddb_get_hw_state(struct intel_crtc *crtc,
 	intel_display_power_put(dev_priv, power_domain, wakeref);
 }
 
-void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv)
-{
-	dev_priv->dbuf.enabled_slices =
-		intel_enabled_dbuf_slices_mask(dev_priv);
-}
-
 /*
  * Determines the downscale amount of a plane for the purposes of watermark calculations.
  * The bspec defines downscale amount as:
@@ -5930,7 +5924,6 @@ void skl_wm_get_hw_state(struct drm_i915_private *dev_priv)
 	struct intel_crtc *crtc;
 	struct intel_crtc_state *crtc_state;
 
-	skl_ddb_get_hw_state(dev_priv);
 	for_each_intel_crtc(&dev_priv->drm, crtc) {
 		crtc_state = to_intel_crtc_state(crtc->base.state);
 
diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
index fadf7cbc44c4..1054a0ab1e40 100644
--- a/drivers/gpu/drm/i915/intel_pm.h
+++ b/drivers/gpu/drm/i915/intel_pm.h
@@ -38,7 +38,6 @@ u8 intel_enabled_dbuf_slices_mask(struct drm_i915_private *dev_priv);
 void skl_pipe_ddb_get_hw_state(struct intel_crtc *crtc,
 			       struct skl_ddb_entry *ddb_y,
 			       struct skl_ddb_entry *ddb_uv);
-void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv);
 void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
 			      struct skl_pipe_wm *out);
 void g4x_wm_sanitize(struct drm_i915_private *dev_priv);
-- 
2.24.1

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

[Intel-gfx] [PATCH 5/6] drm/i915: Move the dbuf pre/post plane update

[Ville Syrjala]

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

Encapsulate the dbuf state more by moving the pre/post
plane functions out from intel_display.c. For now we stick
them into intel_pm.c since that's where the rest of the code
lives for now.

Eventually we should add a new file for this stuff at which
point we also need to decide if it makes sense to even split
the wm code from the ddb code, or to keep them together.

Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 41 +-------------------
 drivers/gpu/drm/i915/intel_pm.c              | 37 ++++++++++++++++++
 drivers/gpu/drm/i915/intel_pm.h              |  2 +
 3 files changed, 41 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 7fb25c7655d1..796e7224f1dc 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15449,43 +15449,6 @@ static void intel_update_trans_port_sync_crtcs(struct intel_crtc *crtc,
 				       state);
 }
 
-static void icl_dbuf_slice_pre_update(struct intel_atomic_state *state)
-{
-	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-	const struct intel_dbuf_state *new_dbuf_state =
-		intel_atomic_get_new_dbuf_state(state);
-	const struct intel_dbuf_state *old_dbuf_state =
-		intel_atomic_get_old_dbuf_state(state);
-
-	if (!new_dbuf_state ||
-	    new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices)
-		return;
-
-	WARN_ON(!new_dbuf_state->base.changed);
-
-	gen9_dbuf_slices_update(dev_priv,
-				old_dbuf_state->enabled_slices |
-				new_dbuf_state->enabled_slices);
-}
-
-static void icl_dbuf_slice_post_update(struct intel_atomic_state *state)
-{
-	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-	const struct intel_dbuf_state *new_dbuf_state =
-		intel_atomic_get_new_dbuf_state(state);
-	const struct intel_dbuf_state *old_dbuf_state =
-		intel_atomic_get_old_dbuf_state(state);
-
-	if (!new_dbuf_state ||
-	    new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices)
-		return;
-
-	WARN_ON(!new_dbuf_state->base.changed);
-
-	gen9_dbuf_slices_update(dev_priv,
-				new_dbuf_state->enabled_slices);
-}
-
 static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
@@ -15738,7 +15701,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 	if (state->modeset)
 		intel_encoders_update_prepare(state);
 
-	icl_dbuf_slice_pre_update(state);
+	intel_dbuf_pre_plane_update(state);
 
 	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
 	dev_priv->display.commit_modeset_enables(state);
@@ -15793,7 +15756,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 			dev_priv->display.optimize_watermarks(state, crtc);
 	}
 
-	icl_dbuf_slice_post_update(state);
+	intel_dbuf_post_plane_update(state);
 
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		intel_post_plane_update(state, crtc);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0032458f0a5d..39349d305533 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7573,3 +7573,40 @@ int intel_dbuf_init(struct drm_i915_private *dev_priv)
 
 	return 0;
 }
+
+void intel_dbuf_pre_plane_update(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	const struct intel_dbuf_state *new_dbuf_state =
+		intel_atomic_get_new_dbuf_state(state);
+	const struct intel_dbuf_state *old_dbuf_state =
+		intel_atomic_get_old_dbuf_state(state);
+
+	if (!new_dbuf_state ||
+	    new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices)
+		return;
+
+	WARN_ON(!new_dbuf_state->base.changed);
+
+	gen9_dbuf_slices_update(dev_priv,
+				old_dbuf_state->enabled_slices |
+				new_dbuf_state->enabled_slices);
+}
+
+void intel_dbuf_post_plane_update(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	const struct intel_dbuf_state *new_dbuf_state =
+		intel_atomic_get_new_dbuf_state(state);
+	const struct intel_dbuf_state *old_dbuf_state =
+		intel_atomic_get_old_dbuf_state(state);
+
+	if (!new_dbuf_state ||
+	    new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices)
+		return;
+
+	WARN_ON(!new_dbuf_state->base.changed);
+
+	gen9_dbuf_slices_update(dev_priv,
+				new_dbuf_state->enabled_slices);
+}
diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
index 1054a0ab1e40..8204d6a5526c 100644
--- a/drivers/gpu/drm/i915/intel_pm.h
+++ b/drivers/gpu/drm/i915/intel_pm.h
@@ -79,5 +79,7 @@ intel_atomic_get_dbuf_state(struct intel_atomic_state *state);
 	to_intel_dbuf_state(intel_atomic_get_new_global_obj_state(state, &to_i915(state->base.dev)->dbuf.obj))
 
 int intel_dbuf_init(struct drm_i915_private *dev_priv);
+void intel_dbuf_pre_plane_update(struct intel_atomic_state *state);
+void intel_dbuf_post_plane_update(struct intel_atomic_state *state);
 
 #endif /* __INTEL_PM_H__ */
-- 
2.24.1

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

[Intel-gfx] [PATCH 6/6] drm/i915: Clean up dbuf debugs during .atomic_check()

[Ville Syrjala]

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

Combine the two per-pipe dbuf debugs into one, and use the canonical
[CRTC:%d:%s] style to identify the crtc. Also use the same style as
the plane code uses for the ddb start/end, and prefix bitmask properly
with 0x to make it clear they are in fact bitmasks.

The "how many total slices we are going to use" debug we move to
outside the crtc loop so it gets printed only once at the end.

Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 39349d305533..33f64e4cea51 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3910,10 +3910,6 @@ skl_ddb_get_pipe_allocation_limits(struct drm_i915_private *dev_priv,
 	 */
 	dbuf_slice_mask = skl_compute_dbuf_slices(crtc_state, active_pipes);
 
-	DRM_DEBUG_KMS("DBuf slice mask %x pipe %c active pipes %x\n",
-		      dbuf_slice_mask,
-		      pipe_name(for_pipe), active_pipes);
-
 	/*
 	 * Figure out at which DBuf slice we start, i.e if we start at Dbuf S2
 	 * and slice size is 1024, the offset would be 1024
@@ -3996,8 +3992,10 @@ skl_ddb_get_pipe_allocation_limits(struct drm_i915_private *dev_priv,
 	alloc->start = offset + start;
 	alloc->end = offset + end;
 
-	DRM_DEBUG_KMS("Pipe %d ddb %d-%d\n", for_pipe,
-		      alloc->start, alloc->end);
+	drm_dbg_kms(&dev_priv->drm,
+		    "[CRTC:%d:%s] dbuf slices 0x%x, ddb (%d - %d), active pipes 0x%x\n",
+		    for_crtc->base.id, for_crtc->name,
+		    dbuf_slice_mask, alloc->start, alloc->end, active_pipes);
 
 	return 0;
 }
@@ -5477,7 +5475,10 @@ skl_ddb_add_affected_planes(const struct intel_crtc_state *old_crtc_state,
 static int
 skl_compute_ddb(struct intel_atomic_state *state)
 {
-	struct intel_crtc_state *old_crtc_state;
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	const struct intel_dbuf_state *old_dbuf_state;
+	const struct intel_dbuf_state *new_dbuf_state;
+	const struct intel_crtc_state *old_crtc_state;
 	struct intel_crtc_state *new_crtc_state;
 	struct intel_crtc *crtc;
 	int ret, i;
@@ -5494,6 +5495,17 @@ skl_compute_ddb(struct intel_atomic_state *state)
 			return ret;
 	}
 
+	old_dbuf_state = intel_atomic_get_old_dbuf_state(state);
+	new_dbuf_state = intel_atomic_get_new_dbuf_state(state);
+
+	if (new_dbuf_state &&
+	    new_dbuf_state->enabled_slices != old_dbuf_state->enabled_slices)
+		drm_dbg_kms(&dev_priv->drm,
+			    "Enabled dbuf slices 0x%x -> 0x%x (out of %d dbuf slices)\n",
+			    old_dbuf_state->enabled_slices,
+			    new_dbuf_state->enabled_slices,
+			    INTEL_INFO(dev_priv)->num_supported_dbuf_slices);
+
 	return 0;
 }
 
-- 
2.24.1

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

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Proper dbuf global state

[Patchwork]

== Series Details ==

Series: drm/i915: Proper dbuf global state
URL   : https://patchwork.freedesktop.org/series/73421/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
105574d3d6d9 drm/i915: Introduce proper dbuf state
-:179: CHECK:MULTIPLE_ASSIGNMENTS: multiple assignments should be avoided
#179: FILE: drivers/gpu/drm/i915/display/intel_display.c:18491:
+	dev_priv->active_pipes = cdclk_state->active_pipes =

-:610: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'state' - possible side-effects?
#610: FILE: drivers/gpu/drm/i915/intel_pm.h:77:
+#define intel_atomic_get_old_dbuf_state(state) \
+	to_intel_dbuf_state(intel_atomic_get_old_global_obj_state(state, &to_i915(state->base.dev)->dbuf.obj))

-:611: WARNING:LONG_LINE: line over 100 characters
#611: FILE: drivers/gpu/drm/i915/intel_pm.h:78:
+	to_intel_dbuf_state(intel_atomic_get_old_global_obj_state(state, &to_i915(state->base.dev)->dbuf.obj))

-:612: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'state' - possible side-effects?
#612: FILE: drivers/gpu/drm/i915/intel_pm.h:79:
+#define intel_atomic_get_new_dbuf_state(state) \
+	to_intel_dbuf_state(intel_atomic_get_new_global_obj_state(state, &to_i915(state->base.dev)->dbuf.obj))

-:613: WARNING:LONG_LINE: line over 100 characters
#613: FILE: drivers/gpu/drm/i915/intel_pm.h:80:
+	to_intel_dbuf_state(intel_atomic_get_new_global_obj_state(state, &to_i915(state->base.dev)->dbuf.obj))

total: 0 errors, 2 warnings, 3 checks, 541 lines checked
8c6729a09771 drm/i915: Polish some dbuf debugs
f51308e472a5 drm/i915: Unify the low level dbuf code
23efcff9a67e drm/i915: Nuke skl_ddb_get_hw_state()
246e420dd654 drm/i915: Move the dbuf pre/post plane update
bc797241a9a7 drm/i915: Clean up dbuf debugs during .atomic_check()

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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Proper dbuf global state

[Patchwork]

== Series Details ==

Series: drm/i915: Proper dbuf global state
URL   : https://patchwork.freedesktop.org/series/73421/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7934 -> Patchwork_16562
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/index.html

Known issues
------------

  Here are the changes found in Patchwork_16562 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_close_race@basic-threads:
    - fi-byt-j1900:       [PASS][1] -> [INCOMPLETE][2] ([i915#45])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/fi-byt-j1900/igt@gem_close_race@basic-threads.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/fi-byt-j1900/igt@gem_close_race@basic-threads.html

  * igt@i915_selftest@live_execlists:
    - fi-icl-y:           [PASS][3] -> [DMESG-FAIL][4] ([fdo#108569])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/fi-icl-y/igt@i915_selftest@live_execlists.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/fi-icl-y/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_gtt:
    - fi-bxt-dsi:         [PASS][5] -> [TIMEOUT][6] ([fdo#112271])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/fi-bxt-dsi/igt@i915_selftest@live_gtt.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/fi-bxt-dsi/igt@i915_selftest@live_gtt.html
    - fi-glk-dsi:         [PASS][7] -> [TIMEOUT][8] ([fdo#112271] / [i915#690])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/fi-glk-dsi/igt@i915_selftest@live_gtt.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/fi-glk-dsi/igt@i915_selftest@live_gtt.html

  
#### Possible fixes ####

  * igt@gem_close_race@basic-threads:
    - fi-byt-n2820:       [INCOMPLETE][9] ([i915#45]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/fi-byt-n2820/igt@gem_close_race@basic-threads.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/fi-byt-n2820/igt@gem_close_race@basic-threads.html

  * igt@i915_selftest@live_gtt:
    - fi-skl-6600u:       [TIMEOUT][11] ([fdo#111732] / [fdo#112271]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/fi-skl-6600u/igt@i915_selftest@live_gtt.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/fi-skl-6600u/igt@i915_selftest@live_gtt.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#111732]: https://bugs.freedesktop.org/show_bug.cgi?id=111732
  [fdo#112271]: https://bugs.freedesktop.org/show_bug.cgi?id=112271
  [i915#45]: https://gitlab.freedesktop.org/drm/intel/issues/45
  [i915#690]: https://gitlab.freedesktop.org/drm/intel/issues/690
  [i915#937]: https://gitlab.freedesktop.org/drm/intel/issues/937


Participating hosts (43 -> 46)
------------------------------

  Additional (8): fi-bsw-n3050 fi-hsw-peppy fi-skl-6770hq fi-bwr-2160 fi-gdg-551 fi-cfl-8109u fi-bsw-kefka fi-skl-lmem 
  Missing    (5): fi-hsw-4200u fi-byt-squawks fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7934 -> Patchwork_16562

  CI-20190529: 20190529
  CI_DRM_7934: 16668f8cd3512f56f626acaed0dd9245692ea3dc @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5440: 860924b6ccbed75b66ab4b65897bb9abc91763ea @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16562: bc797241a9a76df14e51a117b2988c8a9d07cc94 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

bc797241a9a7 drm/i915: Clean up dbuf debugs during .atomic_check()
246e420dd654 drm/i915: Move the dbuf pre/post plane update
23efcff9a67e drm/i915: Nuke skl_ddb_get_hw_state()
f51308e472a5 drm/i915: Unify the low level dbuf code
8c6729a09771 drm/i915: Polish some dbuf debugs
105574d3d6d9 drm/i915: Introduce proper dbuf state

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/6] drm/i915: Introduce proper dbuf state

[Lisovskiy, Stanislav]

On Thu, 2020-02-13 at 20:47 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add a global state to track the dbuf slices. Gets rid of all the
> nasty
> coupling between state->modeset and dbuf recompulation. Also we can
> now
> totally nuke state->active_pipe_changes.
> 
> dev_priv->wm.distrust_bios_wm still remains, but should probably also
> get nuked from orbit later. Just didn't spend any significant time
> pondering how to go about. The obvious thing would be to just
> recompute
> the thing every time.
> 
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |  69 ++++---
>  .../drm/i915/display/intel_display_power.c    |   4 +-
>  .../drm/i915/display/intel_display_types.h    |  13 --
>  drivers/gpu/drm/i915/i915_drv.h               |  11 +-
>  drivers/gpu/drm/i915/intel_pm.c               | 185 ++++++++++++--
> ----
>  drivers/gpu/drm/i915/intel_pm.h               |  22 +++
>  6 files changed, 205 insertions(+), 99 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index e09d3c93c52b..e331ab900336 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7558,6 +7558,8 @@ static void intel_crtc_disable_noatomic(struct
> intel_crtc *crtc,
>  		to_intel_bw_state(dev_priv->bw_obj.state);
>  	struct intel_cdclk_state *cdclk_state =
>  		to_intel_cdclk_state(dev_priv->cdclk.obj.state);
> +	struct intel_dbuf_state *dbuf_state =
> +		to_intel_dbuf_state(dev_priv->dbuf.obj.state);
>  	struct intel_crtc_state *crtc_state =
>  		to_intel_crtc_state(crtc->base.state);
>  	enum intel_display_power_domain domain;
> @@ -7630,6 +7632,8 @@ static void intel_crtc_disable_noatomic(struct
> intel_crtc *crtc,
>  	cdclk_state->min_voltage_level[pipe] = 0;
>  	cdclk_state->active_pipes &= ~BIT(pipe);
>  
> +	dbuf_state->active_pipes &= ~BIT(pipe);
> +

May be I'm wrong(so being not offensive here :) ), but feels kind of
redundant to have active_pipes in cdclk_state and at the same time in
dbuf_state. Why can't it be still 
in some more general state, which is inherited/used/aggregated by those
dbuf and cdclk states? Otherwise you will have to do this duplicate
code initializations which I see here, for example if there would be
even more states you have then three or more similar initializations
here,
doing the same thing. This pretty much increases probability that
somewhere in the code, you will eventually forget to do all
initializations(well I guess you understand).
Or if you will have to update the behavior, based on active_pipes here
somehow, you will also have to change the duplicate code all over the
place.


>  	bw_state->data_rate[pipe] = 0;
>  	bw_state->num_active_planes[pipe] = 0;
>  }
> @@ -14089,10 +14093,10 @@ static void verify_wm_state(struct
> intel_crtc *crtc,
>  	hw_enabled_slices = intel_enabled_dbuf_slices_mask(dev_priv);
>  
>  	if (INTEL_GEN(dev_priv) >= 11 &&
> -	    hw_enabled_slices != dev_priv->enabled_dbuf_slices_mask)
> +	    hw_enabled_slices != dev_priv->dbuf.enabled_slices)
>  		drm_err(&dev_priv->drm,
>  			"mismatch in DBUF Slices (expected 0x%x, got
> 0x%x)\n",
> -			dev_priv->enabled_dbuf_slices_mask,
> +			dev_priv->dbuf.enabled_slices,
>  			hw_enabled_slices);
>  
>  	/* planes */
> @@ -14627,9 +14631,7 @@ static int intel_modeset_checks(struct
> intel_atomic_state *state)
>  	state->modeset = true;
>  	state->active_pipes = intel_calc_active_pipes(state, dev_priv-
> >active_pipes);
>  
> -	state->active_pipe_changes = state->active_pipes ^ dev_priv-
> >active_pipes;
> -
> -	if (state->active_pipe_changes) {
> +	if (state->active_pipes != dev_priv->active_pipes) {
>  		ret = _intel_atomic_lock_global_state(state);
>  		if (ret)
>  			return ret;
> @@ -15450,24 +15452,38 @@ static void
> intel_update_trans_port_sync_crtcs(struct intel_crtc *crtc,
>  static void icl_dbuf_slice_pre_update(struct intel_atomic_state
> *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	u8 hw_enabled_slices = dev_priv->enabled_dbuf_slices_mask;
> -	u8 required_slices = state->enabled_dbuf_slices_mask;
> -	u8 slices_union = hw_enabled_slices | required_slices;
> +	const struct intel_dbuf_state *new_dbuf_state =
> +		intel_atomic_get_new_dbuf_state(state);
> +	const struct intel_dbuf_state *old_dbuf_state =
> +		intel_atomic_get_old_dbuf_state(state);
> +
> +	if (!new_dbuf_state ||
> +	    new_dbuf_state->enabled_slices == old_dbuf_state-
> >enabled_slices)
> +		return;
>  
> -	/* If 2nd DBuf slice required, enable it here */
> -	if (INTEL_GEN(dev_priv) >= 11 && slices_union !=
> hw_enabled_slices)
> -		icl_dbuf_slices_update(dev_priv, slices_union);
> +	WARN_ON(!new_dbuf_state->base.changed);
> +
> +	icl_dbuf_slices_update(dev_priv,
> +			       old_dbuf_state->enabled_slices |
> +			       new_dbuf_state->enabled_slices);
>  }
>  
>  static void icl_dbuf_slice_post_update(struct intel_atomic_state
> *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	u8 hw_enabled_slices = dev_priv->enabled_dbuf_slices_mask;
> -	u8 required_slices = state->enabled_dbuf_slices_mask;
> +	const struct intel_dbuf_state *new_dbuf_state =
> +		intel_atomic_get_new_dbuf_state(state);
> +	const struct intel_dbuf_state *old_dbuf_state =
> +		intel_atomic_get_old_dbuf_state(state);
> +
> +	if (!new_dbuf_state ||
> +	    new_dbuf_state->enabled_slices == old_dbuf_state-
> >enabled_slices)
> +		return;
>  
> -	/* If 2nd DBuf slice is no more required disable it */
> -	if (INTEL_GEN(dev_priv) >= 11 && required_slices !=
> hw_enabled_slices)
> -		icl_dbuf_slices_update(dev_priv, required_slices);
> +	WARN_ON(!new_dbuf_state->base.changed);
> +
> +	icl_dbuf_slices_update(dev_priv,
> +			       new_dbuf_state->enabled_slices);
>  }
>  
>  static void skl_commit_modeset_enables(struct intel_atomic_state
> *state)
> @@ -15722,9 +15738,7 @@ static void intel_atomic_commit_tail(struct
> intel_atomic_state *state)
>  	if (state->modeset)
>  		intel_encoders_update_prepare(state);
>  
> -	/* Enable all new slices, we might need */
> -	if (state->modeset)
> -		icl_dbuf_slice_pre_update(state);
> +	icl_dbuf_slice_pre_update(state);
>  
>  	/* Now enable the clocks, plane, pipe, and connectors that we
> set up. */
>  	dev_priv->display.commit_modeset_enables(state);
> @@ -15779,9 +15793,7 @@ static void intel_atomic_commit_tail(struct
> intel_atomic_state *state)
>  			dev_priv->display.optimize_watermarks(state,
> crtc);
>  	}
>  
> -	/* Disable all slices, we don't need */
> -	if (state->modeset)
> -		icl_dbuf_slice_post_update(state);
> +	icl_dbuf_slice_post_update(state);
>  
>  	for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state, new_crtc_state, i) {
>  		intel_post_plane_update(state, crtc);
> @@ -17667,10 +17679,14 @@ void intel_modeset_init_hw(struct
> drm_i915_private *i915)
>  {
>  	struct intel_cdclk_state *cdclk_state =
>  		to_intel_cdclk_state(i915->cdclk.obj.state);
> +	struct intel_dbuf_state *dbuf_state =
> +		to_intel_dbuf_state(i915->dbuf.obj.state);
>  
>  	intel_update_cdclk(i915);
>  	intel_dump_cdclk_config(&i915->cdclk.hw, "Current CDCLK");
>  	cdclk_state->logical = cdclk_state->actual = i915->cdclk.hw;
> +
> +	dbuf_state->enabled_slices = i915->dbuf.enabled_slices;
>  }
>  
>  static int sanitize_watermarks_add_affected(struct drm_atomic_state
> *state)
> @@ -17941,6 +17957,10 @@ int intel_modeset_init(struct
> drm_i915_private *i915)
>  	if (ret)
>  		return ret;
>  
> +	ret = intel_dbuf_init(i915);
> +	if (ret)
> +		return ret;
> +
>  	ret = intel_bw_init(i915);
>  	if (ret)
>  		return ret;
> @@ -18435,6 +18455,8 @@ static void
> intel_modeset_readout_hw_state(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_cdclk_state *cdclk_state =
>  		to_intel_cdclk_state(dev_priv->cdclk.obj.state);
> +	struct intel_dbuf_state *dbuf_state =
> +		to_intel_dbuf_state(dev_priv->dbuf.obj.state);
>  	enum pipe pipe;
>  	struct intel_crtc *crtc;
>  	struct intel_encoder *encoder;
> @@ -18466,7 +18488,8 @@ static void
> intel_modeset_readout_hw_state(struct drm_device *dev)
>  			    enableddisabled(crtc_state->hw.active));
>  	}
>  
> -	dev_priv->active_pipes = cdclk_state->active_pipes =
> active_pipes;
> +	dev_priv->active_pipes = cdclk_state->active_pipes =
> +		dbuf_state->active_pipes = active_pipes;

Same thing about duplication, so would consider having active_pipes,
somewhere in a single place. Previsouly I think it was more logical
to have it in intel_atomic_state as this is kind of general
information, not related to bw, dbuf or cdclk. Or may be it should
be in some separate modeset specific state, just as bw, dbuf, cdclk.

All those states would then contain only their specific info, without
need in duplication and information anyway is easy to obtain.
Also it would be easier then to manipulate and build some dependency
hierarchy for those.

>  
>  	readout_plane_state(dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 53056def5414..c24e1de78b9d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -1043,7 +1043,7 @@ static bool
> gen9_dc_off_power_well_enabled(struct drm_i915_private *dev_priv,
>  static void gen9_assert_dbuf_enabled(struct drm_i915_private
> *dev_priv)
>  {
>  	u8 hw_enabled_dbuf_slices =
> intel_enabled_dbuf_slices_mask(dev_priv);
> -	u8 enabled_dbuf_slices = dev_priv->enabled_dbuf_slices_mask;
> +	u8 enabled_dbuf_slices = dev_priv->dbuf.enabled_slices;
>  
>  	WARN(hw_enabled_dbuf_slices != enabled_dbuf_slices,
>  	     "Unexpected DBuf power power state (0x%08x, expected
> 0x%08x)\n",
> @@ -4463,7 +4463,7 @@ void icl_dbuf_slices_update(struct
> drm_i915_private *dev_priv,
>  				     (req_slices & BIT(i)) != 0);
>  	}
>  
> -	dev_priv->enabled_dbuf_slices_mask = req_slices;
> +	dev_priv->dbuf.enabled_slices = req_slices;
>  
>  	mutex_unlock(&power_domains->lock);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 283c622f8ba1..12ac47c33171 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -468,16 +468,6 @@ struct intel_atomic_state {
>  
>  	bool dpll_set, modeset;
>  
> -	/*
> -	 * Does this transaction change the pipes that are
> active?  This mask
> -	 * tracks which CRTC's have changed their active state at the
> end of
> -	 * the transaction (not counting the temporary disable during
> modesets).
> -	 * This mask should only be non-zero when intel_state->modeset
> is true,
> -	 * but the converse is not necessarily true; simply changing a
> mode may
> -	 * not flip the final active status of any CRTC's
> -	 */
> -	u8 active_pipe_changes;
> -
>  	u8 active_pipes;
>  
>  	struct intel_shared_dpll_state shared_dpll[I915_NUM_PLLS];
> @@ -495,9 +485,6 @@ struct intel_atomic_state {
>  	 */
>  	bool global_state_changed;
>  
> -	/* Number of enabled DBuf slices */
> -	u8 enabled_dbuf_slices_mask;
> -
>  	struct i915_sw_fence commit_ready;
>  
>  	struct llist_node freed;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index da509d9b8895..2638f3fcd1aa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1007,6 +1007,13 @@ struct drm_i915_private {
>  		struct intel_global_obj obj;
>  	} cdclk;
>  
> +	struct {
> +		/* The current hardware dbuf configuration */
> +		u8 enabled_slices;
> +
> +		struct intel_global_obj obj;
> +	} dbuf;
> +
>  	/**
>  	 * wq - Driver workqueue for GEM.
>  	 *
> @@ -1182,12 +1189,12 @@ struct drm_i915_private {
>  		 * Set during HW readout of watermarks/DDB.  Some
> platforms
>  		 * need to know when we're still using BIOS-provided
> values
>  		 * (which we don't fully trust).
> +		 *
> +		 * FIXME get rid of this.
>  		 */
>  		bool distrust_bios_wm;
>  	} wm;
>  
> -	u8 enabled_dbuf_slices_mask; /* GEN11 has configurable 2 slices
> */
> -
>  	struct dram_info {
>  		bool valid;
>  		bool is_16gb_dimm;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index ffac0b862ca5..31179e30bfe3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3845,7 +3845,7 @@ static u16 intel_get_ddb_size(struct
> drm_i915_private *dev_priv)
>  static u8 skl_compute_dbuf_slices(const struct intel_crtc_state
> *crtc_state,
>  				  u32 active_pipes);
>  
> -static void
> +static int
>  skl_ddb_get_pipe_allocation_limits(struct drm_i915_private
> *dev_priv,
>  				   const struct intel_crtc_state
> *crtc_state,
>  				   const u64 total_data_rate,
> @@ -3858,30 +3858,29 @@ skl_ddb_get_pipe_allocation_limits(struct
> drm_i915_private *dev_priv,
>  	const struct intel_crtc *crtc;
>  	u32 pipe_width = 0, total_width_in_range = 0,
> width_before_pipe_in_range = 0;
>  	enum pipe for_pipe = to_intel_crtc(for_crtc)->pipe;
> +	struct intel_dbuf_state *new_dbuf_state =
> +		intel_atomic_get_new_dbuf_state(intel_state);
> +	const struct intel_dbuf_state *old_dbuf_state =
> +		intel_atomic_get_old_dbuf_state(intel_state);
> +	u8 active_pipes = new_dbuf_state->active_pipes;
>  	u16 ddb_size;
>  	u32 ddb_range_size;
>  	u32 i;
>  	u32 dbuf_slice_mask;
> -	u32 active_pipes;
>  	u32 offset;
>  	u32 slice_size;
>  	u32 total_slice_mask;
>  	u32 start, end;
> +	int ret;
> +
> +	*num_active = hweight8(active_pipes);
>  
> -	if (drm_WARN_ON(&dev_priv->drm, !state) || !crtc_state-
> >hw.active) {
> +	if (!crtc_state->hw.active) {
>  		alloc->start = 0;
>  		alloc->end = 0;
> -		*num_active = hweight8(dev_priv->active_pipes);
> -		return;
> +		return 0;
>  	}
>  
> -	if (intel_state->active_pipe_changes)
> -		active_pipes = intel_state->active_pipes;
> -	else
> -		active_pipes = dev_priv->active_pipes;
> -
> -	*num_active = hweight8(active_pipes);
> -
>  	ddb_size = intel_get_ddb_size(dev_priv);
>  
>  	slice_size = ddb_size / INTEL_INFO(dev_priv)-
> >num_supported_dbuf_slices;
> @@ -3894,13 +3893,16 @@ skl_ddb_get_pipe_allocation_limits(struct
> drm_i915_private *dev_priv,
>  	 * that changes the active CRTC list or do modeset would need
> to
>  	 * grab _all_ crtc locks, including the one we currently hold.
>  	 */
> -	if (!intel_state->active_pipe_changes && !intel_state->modeset) 
> {
> +	if (old_dbuf_state->active_pipes == new_dbuf_state-
> >active_pipes &&
> +	    !dev_priv->wm.distrust_bios_wm) {
>  		/*
>  		 * alloc may be cleared by clear_intel_crtc_state,
>  		 * copy from old state to be sure
> +		 *
> +		 * FIXME get rid of this mess
>  		 */
>  		*alloc = to_intel_crtc_state(for_crtc->state)-
> >wm.skl.ddb;
> -		return;
> +		return 0;
>  	}

So we got rid of active_pipe_changes, which was wrongly used to figure
out if there were some DBuf changes, but now we are using active_pipes 
for that. This kind of explains, why you added active_pipes to DBuf
state, however wouldn't it be more logical still to have active_pipes
in more general state or at least in a state, which is related to a
modesetting, while here you would be using some DBuf_state specific 
field, like dbuf_changed or even active_dbuf_changes?

Then you wouldn't need to duplicate this field(benefit) and also
you would have a clear way to determine if DBuf changes were made in
a specific to DBuf state(benefit).

May be you have some other idea, why it should be like this, at least
this is how I see it now.

Stan
 

>  
>  	/*
> @@ -3979,7 +3981,13 @@ skl_ddb_get_pipe_allocation_limits(struct
> drm_i915_private *dev_priv,
>  	 * FIXME: For now we always enable slice S1 as per
>  	 * the Bspec display initialization sequence.
>  	 */
> -	intel_state->enabled_dbuf_slices_mask = total_slice_mask |
> BIT(DBUF_S1);
> +	new_dbuf_state->enabled_slices = total_slice_mask |
> BIT(DBUF_S1);
> +
> +	if (old_dbuf_state->enabled_slices != new_dbuf_state-
> >enabled_slices) {
> +		ret =
> intel_atomic_serialize_global_state(&new_dbuf_state->base);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	start = ddb_range_size * width_before_pipe_in_range /
> total_width_in_range;
>  	end = ddb_range_size *
> @@ -3990,9 +3998,8 @@ skl_ddb_get_pipe_allocation_limits(struct
> drm_i915_private *dev_priv,
>  
>  	DRM_DEBUG_KMS("Pipe %d ddb %d-%d\n", for_pipe,
>  		      alloc->start, alloc->end);
> -	DRM_DEBUG_KMS("Enabled ddb slices mask %x num supported %d\n",
> -		      intel_state->enabled_dbuf_slices_mask,
> -		      INTEL_INFO(dev_priv)->num_supported_dbuf_slices);
> +
> +	return 0;
>  }
>  
>  static int skl_compute_wm_params(const struct intel_crtc_state
> *crtc_state,
> @@ -4112,8 +4119,8 @@ void skl_pipe_ddb_get_hw_state(struct
> intel_crtc *crtc,
>  
>  void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv)
>  {
> -	dev_priv->enabled_dbuf_slices_mask =
> -				intel_enabled_dbuf_slices_mask(dev_priv
> );
> +	dev_priv->dbuf.enabled_slices =
> +		intel_enabled_dbuf_slices_mask(dev_priv);
>  }
>  
>  /*
> @@ -4563,6 +4570,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state
> *crtc_state)
>  	u64 uv_plane_data_rate[I915_MAX_PLANES] = {};
>  	u32 blocks;
>  	int level;
> +	int ret;
>  
>  	/* Clear the partitioning for disabled planes. */
>  	memset(crtc_state->wm.skl.plane_ddb_y, 0, sizeof(crtc_state-
> >wm.skl.plane_ddb_y));
> @@ -4587,8 +4595,12 @@ skl_allocate_pipe_ddb(struct intel_crtc_state
> *crtc_state)
>  							 uv_plane_data_
> rate);
>  
>  
> -	skl_ddb_get_pipe_allocation_limits(dev_priv, crtc_state,
> total_data_rate,
> -					   alloc, &num_active);
> +	ret = skl_ddb_get_pipe_allocation_limits(dev_priv, crtc_state,
> +						 total_data_rate,
> +						 alloc, &num_active);
> +	if (ret)
> +		return ret;
> +
>  	alloc_size = skl_ddb_entry_size(alloc);
>  	if (alloc_size == 0)
>  		return 0;
> @@ -5471,14 +5483,11 @@ skl_ddb_add_affected_planes(const struct
> intel_crtc_state *old_crtc_state,
>  static int
>  skl_compute_ddb(struct intel_atomic_state *state)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>  	struct intel_crtc_state *old_crtc_state;
>  	struct intel_crtc_state *new_crtc_state;
>  	struct intel_crtc *crtc;
>  	int ret, i;
>  
> -	state->enabled_dbuf_slices_mask = dev_priv-
> >enabled_dbuf_slices_mask;
> -
>  	for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state,
>  					    new_crtc_state, i) {
>  		ret = skl_allocate_pipe_ddb(new_crtc_state);
> @@ -5618,7 +5627,8 @@ skl_print_wm_changes(struct intel_atomic_state
> *state)
>  	}
>  }
>  
> -static int intel_add_all_pipes(struct intel_atomic_state *state)
> +static int intel_add_affected_pipes(struct intel_atomic_state
> *state,
> +				    u8 pipe_mask)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>  	struct intel_crtc *crtc;
> @@ -5626,6 +5636,9 @@ static int intel_add_all_pipes(struct
> intel_atomic_state *state)
>  	for_each_intel_crtc(&dev_priv->drm, crtc) {
>  		struct intel_crtc_state *crtc_state;
>  
> +		if ((pipe_mask & BIT(crtc->pipe)) == 0)
> +			continue;
> +
>  		crtc_state = intel_atomic_get_crtc_state(&state->base,
> crtc);
>  		if (IS_ERR(crtc_state))
>  			return PTR_ERR(crtc_state);
> @@ -5638,49 +5651,54 @@ static int
>  skl_ddb_add_affected_pipes(struct intel_atomic_state *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	int ret;
> +	struct intel_crtc_state *crtc_state;
> +	struct intel_crtc *crtc;
> +	int i, ret;
>  
> -	/*
> -	 * If this is our first atomic update following hardware
> readout,
> -	 * we can't trust the DDB that the BIOS programmed for
> us.  Let's
> -	 * pretend that all pipes switched active status so that we'll
> -	 * ensure a full DDB recompute.
> -	 */
>  	if (dev_priv->wm.distrust_bios_wm) {
> -		ret = drm_modeset_lock(&dev_priv-
> >drm.mode_config.connection_mutex,
> -				       state->base.acquire_ctx);
> +		/*
> +		 * skl_ddb_get_pipe_allocation_limits() currently
> requires
> +		 * all active pipes to be included in the state so that
> +		 * it can redistribute the dbuf among them, and it
> really
> +		 * wants to recompute things when distrust_bios_wm is
> set
> +		 * so we add all the pipes to the state.
> +		 */
> +		ret = intel_add_affected_pipes(state, ~0);
>  		if (ret)
>  			return ret;
> +	}
>  
> -		state->active_pipe_changes = INTEL_INFO(dev_priv)-
> >pipe_mask;
> +	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
> +		struct intel_dbuf_state *new_dbuf_state;
> +		const struct intel_dbuf_state *old_dbuf_state;
> +
> +		new_dbuf_state = intel_atomic_get_dbuf_state(state);
> +		if (IS_ERR(new_dbuf_state))
> +			return ret;
> +
> +		old_dbuf_state =
> intel_atomic_get_old_dbuf_state(state);
> +
> +		new_dbuf_state->active_pipes =
> +			intel_calc_active_pipes(state, old_dbuf_state-
> >active_pipes);
> +
> +		if (old_dbuf_state->active_pipes == new_dbuf_state-
> >active_pipes)
> +			break;
> +
> +		ret = intel_atomic_lock_global_state(&new_dbuf_state-
> >base);
> +		if (ret)
> +			return ret;
>  
>  		/*
> -		 * We usually only initialize state->active_pipes if we
> -		 * we're doing a modeset; make sure this field is
> always
> -		 * initialized during the sanitization process that
> happens
> -		 * on the first commit too.
> +		 * skl_ddb_get_pipe_allocation_limits() currently
> requires
> +		 * all active pipes to be included in the state so that
> +		 * it can redistribute the dbuf among them.
>  		 */
> -		if (!state->modeset)
> -			state->active_pipes = dev_priv->active_pipes;
> -	}
> -
> -	/*
> -	 * If the modeset changes which CRTC's are active, we need to
> -	 * recompute the DDB allocation for *all* active pipes, even
> -	 * those that weren't otherwise being modified in any way by
> this
> -	 * atomic commit.  Due to the shrinking of the per-pipe
> allocations
> -	 * when new active CRTC's are added, it's possible for a pipe
> that
> -	 * we were already using and aren't changing at all here to
> suddenly
> -	 * become invalid if its DDB needs exceeds its new allocation.
> -	 *
> -	 * Note that if we wind up doing a full DDB recompute, we can't
> let
> -	 * any other display updates race with this transaction, so we
> need
> -	 * to grab the lock on *all* CRTC's.
> -	 */
> -	if (state->active_pipe_changes || state->modeset) {
> -		ret = intel_add_all_pipes(state);
> +		ret = intel_add_affected_pipes(state,
> +					       new_dbuf_state-
> >active_pipes);
>  		if (ret)
>  			return ret;
> +
> +		break;
>  	}
>  
>  	return 0;
> @@ -7513,3 +7531,52 @@ void intel_pm_setup(struct drm_i915_private
> *dev_priv)
>  	dev_priv->runtime_pm.suspended = false;
>  	atomic_set(&dev_priv->runtime_pm.wakeref_count, 0);
>  }
> +
> +static struct intel_global_state *intel_dbuf_duplicate_state(struct
> intel_global_obj *obj)
> +{
> +	struct intel_dbuf_state *dbuf_state;
> +
> +	dbuf_state = kmemdup(obj->state, sizeof(*dbuf_state),
> GFP_KERNEL);
> +	if (!dbuf_state)
> +		return NULL;
> +
> +	return &dbuf_state->base;
> +}
> +
> +static void intel_dbuf_destroy_state(struct intel_global_obj *obj,
> +				     struct intel_global_state *state)
> +{
> +	kfree(state);
> +}
> +
> +static const struct intel_global_state_funcs intel_dbuf_funcs = {
> +	.atomic_duplicate_state = intel_dbuf_duplicate_state,
> +	.atomic_destroy_state = intel_dbuf_destroy_state,
> +};
> +
> +struct intel_dbuf_state *
> +intel_atomic_get_dbuf_state(struct intel_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	struct intel_global_state *dbuf_state;
> +
> +	dbuf_state = intel_atomic_get_global_obj_state(state,
> &dev_priv->dbuf.obj);
> +	if (IS_ERR(dbuf_state))
> +		return ERR_CAST(dbuf_state);
> +
> +	return to_intel_dbuf_state(dbuf_state);
> +}
> +
> +int intel_dbuf_init(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_dbuf_state *dbuf_state;
> +
> +	dbuf_state = kzalloc(sizeof(*dbuf_state), GFP_KERNEL);
> +	if (!dbuf_state)
> +		return -ENOMEM;
> +
> +	intel_atomic_global_obj_init(dev_priv, &dev_priv->dbuf.obj,
> +				     &dbuf_state->base,
> &intel_dbuf_funcs);
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_pm.h
> b/drivers/gpu/drm/i915/intel_pm.h
> index d60a85421c5a..fadf7cbc44c4 100644
> --- a/drivers/gpu/drm/i915/intel_pm.h
> +++ b/drivers/gpu/drm/i915/intel_pm.h
> @@ -8,6 +8,8 @@
>  
>  #include <linux/types.h>
>  
> +#include "display/intel_global_state.h"
> +
>  #include "i915_reg.h"
>  
>  struct drm_device;
> @@ -59,4 +61,24 @@ void intel_enable_ipc(struct drm_i915_private
> *dev_priv);
>  
>  bool intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool
> enable);
>  
> +struct intel_dbuf_state {
> +	struct intel_global_state base;
> +
> +	u8 enabled_slices;
> +	u8 active_pipes;
> +};
> +
> +int intel_dbuf_init(struct drm_i915_private *dev_priv);
> +
> +struct intel_dbuf_state *
> +intel_atomic_get_dbuf_state(struct intel_atomic_state *state);
> +
> +#define to_intel_dbuf_state(x) container_of((x), struct
> intel_dbuf_state, base)
> +#define intel_atomic_get_old_dbuf_state(state) \
> +	to_intel_dbuf_state(intel_atomic_get_old_global_obj_state(state
> , &to_i915(state->base.dev)->dbuf.obj))
> +#define intel_atomic_get_new_dbuf_state(state) \
> +	to_intel_dbuf_state(intel_atomic_get_new_global_obj_state(state
> , &to_i915(state->base.dev)->dbuf.obj))
> +
> +int intel_dbuf_init(struct drm_i915_private *dev_priv);
> +
>  #endif /* __INTEL_PM_H__ */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: Proper dbuf global state

[Patchwork]

== Series Details ==

Series: drm/i915: Proper dbuf global state
URL   : https://patchwork.freedesktop.org/series/73421/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7934_full -> Patchwork_16562_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_16562_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_16562_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_16562_full:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_cursor_edge_walk@pipe-d-64x64-right-edge:
    - shard-tglb:         [PASS][1] -> [FAIL][2] +24 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb2/igt@kms_cursor_edge_walk@pipe-d-64x64-right-edge.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb2/igt@kms_cursor_edge_walk@pipe-d-64x64-right-edge.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@gem_ctx_persistence@close-replace-race}:
    - shard-iclb:         [PASS][3] -> [FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-iclb8/igt@gem_ctx_persistence@close-replace-race.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-iclb3/igt@gem_ctx_persistence@close-replace-race.html

  
Known issues
------------

  Here are the changes found in Patchwork_16562_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_async@concurrent-writes-bsd:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#112146]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-iclb7/igt@gem_exec_async@concurrent-writes-bsd.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-iclb2/igt@gem_exec_async@concurrent-writes-bsd.html

  * igt@gem_exec_parallel@vcs1-fds:
    - shard-iclb:         [PASS][7] -> [SKIP][8] ([fdo#112080]) +9 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-iclb2/igt@gem_exec_parallel@vcs1-fds.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-iclb3/igt@gem_exec_parallel@vcs1-fds.html

  * igt@gem_exec_schedule@out-order-bsd2:
    - shard-iclb:         [PASS][9] -> [SKIP][10] ([fdo#109276]) +17 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-iclb2/igt@gem_exec_schedule@out-order-bsd2.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-iclb6/igt@gem_exec_schedule@out-order-bsd2.html

  * igt@gem_workarounds@suspend-resume:
    - shard-kbl:          [PASS][11] -> [INCOMPLETE][12] ([fdo#103665])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-kbl7/igt@gem_workarounds@suspend-resume.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-kbl4/igt@gem_workarounds@suspend-resume.html

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          [PASS][13] -> [DMESG-WARN][14] ([i915#180]) +5 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-apl3/igt@gem_workarounds@suspend-resume-context.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-apl1/igt@gem_workarounds@suspend-resume-context.html

  * igt@kms_big_fb@y-tiled-16bpp-rotate-90:
    - shard-tglb:         [PASS][15] -> [FAIL][16] ([i915#1172]) +5 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb8/igt@kms_big_fb@y-tiled-16bpp-rotate-90.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb1/igt@kms_big_fb@y-tiled-16bpp-rotate-90.html

  * igt@kms_ccs@pipe-b-crc-primary-rotation-180:
    - shard-tglb:         [PASS][17] -> [FAIL][18] ([i915#979]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb7/igt@kms_ccs@pipe-b-crc-primary-rotation-180.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb7/igt@kms_ccs@pipe-b-crc-primary-rotation-180.html

  * igt@kms_color@pipe-a-ctm-0-75:
    - shard-skl:          [PASS][19] -> [FAIL][20] ([i915#182])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-skl10/igt@kms_color@pipe-a-ctm-0-75.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-skl1/igt@kms_color@pipe-a-ctm-0-75.html

  * igt@kms_color@pipe-d-ctm-0-5:
    - shard-tglb:         [PASS][21] -> [FAIL][22] ([i915#1149] / [i915#315])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb1/igt@kms_color@pipe-d-ctm-0-5.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb8/igt@kms_color@pipe-d-ctm-0-5.html

  * igt@kms_color@pipe-d-ctm-blue-to-red:
    - shard-tglb:         [PASS][23] -> [FAIL][24] ([i915#1149]) +7 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb2/igt@kms_color@pipe-d-ctm-blue-to-red.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb1/igt@kms_color@pipe-d-ctm-blue-to-red.html

  * igt@kms_cursor_crc@pipe-b-cursor-128x42-onscreen:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([i915#54]) +1 similar issue
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-skl10/igt@kms_cursor_crc@pipe-b-cursor-128x42-onscreen.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-skl1/igt@kms_cursor_crc@pipe-b-cursor-128x42-onscreen.html

  * igt@kms_cursor_crc@pipe-c-cursor-256x256-onscreen:
    - shard-apl:          [PASS][27] -> [FAIL][28] ([i915#54])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-apl3/igt@kms_cursor_crc@pipe-c-cursor-256x256-onscreen.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-apl2/igt@kms_cursor_crc@pipe-c-cursor-256x256-onscreen.html

  * igt@kms_cursor_crc@pipe-c-cursor-dpms:
    - shard-tglb:         [PASS][29] -> [FAIL][30] ([fdo#111703]) +41 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb1/igt@kms_cursor_crc@pipe-c-cursor-dpms.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb1/igt@kms_cursor_crc@pipe-c-cursor-dpms.html

  * igt@kms_cursor_edge_walk@pipe-a-256x256-right-edge:
    - shard-tglb:         [PASS][31] -> [FAIL][32] ([i915#70]) +14 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb8/igt@kms_cursor_edge_walk@pipe-a-256x256-right-edge.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb2/igt@kms_cursor_edge_walk@pipe-a-256x256-right-edge.html

  * igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-atomic:
    - shard-tglb:         [PASS][33] -> [FAIL][34] ([i915#1173])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb7/igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-atomic.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb7/igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-atomic.html

  * igt@kms_cursor_legacy@flip-vs-cursor-legacy:
    - shard-skl:          [PASS][35] -> [FAIL][36] ([IGT#5])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-skl6/igt@kms_cursor_legacy@flip-vs-cursor-legacy.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-skl5/igt@kms_cursor_legacy@flip-vs-cursor-legacy.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-cpu-xtiled:
    - shard-tglb:         [PASS][37] -> [FAIL][38] ([i915#559]) +4 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb3/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-cpu-xtiled.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb3/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-cpu-xtiled.html

  * igt@kms_draw_crc@draw-method-xrgb8888-render-xtiled:
    - shard-tglb:         [PASS][39] -> [DMESG-FAIL][40] ([i915#402]) +21 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb8/igt@kms_draw_crc@draw-method-xrgb8888-render-xtiled.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb2/igt@kms_draw_crc@draw-method-xrgb8888-render-xtiled.html

  * igt@kms_flip_tiling@flip-changes-tiling:
    - shard-tglb:         [PASS][41] -> [FAIL][42] ([i915#699]) +1 similar issue
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb1/igt@kms_flip_tiling@flip-changes-tiling.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb2/igt@kms_flip_tiling@flip-changes-tiling.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][43] -> [DMESG-WARN][44] ([i915#180]) +1 similar issue
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-kbl4/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt:
    - shard-tglb:         [PASS][45] -> [DMESG-FAIL][46] ([i915#402] / [i915#49]) +44 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-pwrite:
    - shard-tglb:         [PASS][47] -> [FAIL][48] ([i915#49]) +25 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb2/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-pwrite.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb8/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-pwrite.html

  * igt@kms_mmap_write_crc@main:
    - shard-tglb:         [PASS][49] -> [FAIL][50] ([i915#1180])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb2/igt@kms_mmap_write_crc@main.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb1/igt@kms_mmap_write_crc@main.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - shard-tglb:         [PASS][51] -> [FAIL][52] ([i915#1183]) +10 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb1/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb5/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-kbl:          [PASS][53] -> [DMESG-WARN][54] ([i915#56])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-kbl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-kbl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-skl:          [PASS][55] -> [INCOMPLETE][56] ([i915#69])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-skl8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-skl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html

  * igt@kms_plane@plane-panning-top-left-pipe-a-planes:
    - shard-tglb:         [PASS][57] -> [FAIL][58] ([i915#1171]) +1 similar issue
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb2/igt@kms_plane@plane-panning-top-left-pipe-a-planes.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb8/igt@kms_plane@plane-panning-top-left-pipe-a-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-mid:
    - shard-tglb:         [PASS][59] -> [FAIL][60] ([i915#1184]) +14 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb7/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-mid.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb7/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-mid.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [PASS][61] -> [FAIL][62] ([fdo#108145]) +1 similar issue
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_cursor@pipe-a-viewport-size-64:
    - shard-tglb:         [PASS][63] -> [FAIL][64] ([i915#1139]) +1 similar issue
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb8/igt@kms_plane_cursor@pipe-a-viewport-size-64.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb2/igt@kms_plane_cursor@pipe-a-viewport-size-64.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-glk:          [PASS][65] -> [FAIL][66] ([i915#899])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-glk4/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-glk9/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_plane_lowres@pipe-c-tiling-x:
    - shard-tglb:         [PASS][67] -> [FAIL][68] ([i915#899]) +3 similar issues
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb5/igt@kms_plane_lowres@pipe-c-tiling-x.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb3/igt@kms_plane_lowres@pipe-c-tiling-x.html

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
    - shard-tglb:         [PASS][69] -> [FAIL][70] ([i915#778]) +3 similar issues
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb8/igt@kms_plane_multiple@atomic-pipe-c-tiling-x.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb7/igt@kms_plane_multiple@atomic-pipe-c-tiling-x.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-tglb:         [PASS][71] -> [FAIL][72] ([fdo#111842] / [i915#608])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb8/igt@kms_psr2_su@frontbuffer.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb7/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@primary_blt:
    - shard-tglb:         [PASS][73] -> [DMESG-WARN][74] ([i915#402]) +17 similar issues
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb8/igt@kms_psr@primary_blt.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb1/igt@kms_psr@primary_blt.html

  * igt@kms_psr@psr2_sprite_render:
    - shard-iclb:         [PASS][75] -> [SKIP][76] ([fdo#109441]) +1 similar issue
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-iclb2/igt@kms_psr@psr2_sprite_render.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-iclb7/igt@kms_psr@psr2_sprite_render.html

  * igt@kms_rotation_crc@primary-rotation-90:
    - shard-tglb:         [PASS][77] -> [FAIL][78] ([i915#65]) +3 similar issues
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb7/igt@kms_rotation_crc@primary-rotation-90.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb7/igt@kms_rotation_crc@primary-rotation-90.html

  * igt@kms_rotation_crc@primary-y-tiled-reflect-x-180:
    - shard-tglb:         [PASS][79] -> [DMESG-FAIL][80] ([i915#402] / [i915#65])
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb8/igt@kms_rotation_crc@primary-y-tiled-reflect-x-180.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb7/igt@kms_rotation_crc@primary-y-tiled-reflect-x-180.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@bcs0-s3:
    - shard-skl:          [INCOMPLETE][81] ([i915#146] / [i915#69]) -> [PASS][82]
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-skl10/igt@gem_ctx_isolation@bcs0-s3.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-skl1/igt@gem_ctx_isolation@bcs0-s3.html

  * igt@gem_ctx_shared@exec-shared-gtt-default:
    - shard-tglb:         [FAIL][83] ([i915#616]) -> [PASS][84] +1 similar issue
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb1/igt@gem_ctx_shared@exec-shared-gtt-default.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb8/igt@gem_ctx_shared@exec-shared-gtt-default.html

  * igt@gem_ctx_shared@exec-shared-gtt-render:
    - shard-tglb:         [FAIL][85] ([i915#607] / [i915#616]) -> [PASS][86]
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb6/igt@gem_ctx_shared@exec-shared-gtt-render.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb8/igt@gem_ctx_shared@exec-shared-gtt-render.html

  * igt@gem_exec_schedule@in-order-bsd:
    - shard-iclb:         [SKIP][87] ([fdo#112146]) -> [PASS][88] +11 similar issues
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-iclb2/igt@gem_exec_schedule@in-order-bsd.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-iclb7/igt@gem_exec_schedule@in-order-bsd.html

  * igt@gem_exec_schedule@pi-userfault-bsd:
    - shard-iclb:         [SKIP][89] ([i915#677]) -> [PASS][90]
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-iclb4/igt@gem_exec_schedule@pi-userfault-bsd.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-iclb3/igt@gem_exec_schedule@pi-userfault-bsd.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-glk:          [FAIL][91] ([i915#644]) -> [PASS][92]
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-glk3/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-glk1/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@i915_pm_rps@reset:
    - shard-iclb:         [FAIL][93] ([i915#413]) -> [PASS][94]
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-iclb7/igt@i915_pm_rps@reset.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-iclb2/igt@i915_pm_rps@reset.html

  * igt@kms_big_fb@y-tiled-16bpp-rotate-270:
    - shard-tglb:         [FAIL][95] ([i915#1172]) -> [PASS][96]
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb7/igt@kms_big_fb@y-tiled-16bpp-rotate-270.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb7/igt@kms_big_fb@y-tiled-16bpp-rotate-270.html

  * igt@kms_cursor_crc@pipe-a-cursor-256x256-sliding:
    - shard-tglb:         [FAIL][97] ([fdo#111703]) -> [PASS][98] +1 similar issue
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb5/igt@kms_cursor_crc@pipe-a-cursor-256x256-sliding.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb8/igt@kms_cursor_crc@pipe-a-cursor-256x256-sliding.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][99] ([i915#180]) -> [PASS][100] +3 similar issues
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          [FAIL][101] ([i915#72]) -> [PASS][102]
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-glk6/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-glk2/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-ytiled:
    - shard-tglb:         [FAIL][103] ([i915#559]) -> [PASS][104]
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb7/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-ytiled.html
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb5/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-ytiled.html

  * {igt@kms_hdr@bpc-switch-dpms}:
    - shard-skl:          [FAIL][105] ([i915#1188]) -> [PASS][106] +1 similar issue
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-skl9/igt@kms_hdr@bpc-switch-dpms.html
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-skl4/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [DMESG-WARN][107] ([i915#180]) -> [PASS][108] +4 similar issues
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-apl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-apl8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [FAIL][109] ([fdo#108145] / [i915#265]) -> [PASS][110]
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-skl7/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [SKIP][111] ([fdo#109441]) -> [PASS][112] +3 similar issues
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-iclb7/igt@kms_psr@psr2_cursor_render.html
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-iclb2/igt@kms_psr@psr2_cursor_render.html

  * igt@kms_rotation_crc@sprite-rotation-90-pos-100-0:
    - shard-tglb:         [FAIL][113] ([i915#65]) -> [PASS][114]
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb2/igt@kms_rotation_crc@sprite-rotation-90-pos-100-0.html
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb2/igt@kms_rotation_crc@sprite-rotation-90-pos-100-0.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][115] ([i915#31]) -> [PASS][116]
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-kbl1/igt@kms_setmode@basic.html
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-kbl7/igt@kms_setmode@basic.html

  * igt@perf_pmu@busy-vcs1:
    - shard-iclb:         [SKIP][117] ([fdo#112080]) -> [PASS][118] +8 similar issues
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-iclb7/igt@perf_pmu@busy-vcs1.html
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-iclb2/igt@perf_pmu@busy-vcs1.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [SKIP][119] ([fdo#109276]) -> [PASS][120] +17 similar issues
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-iclb7/igt@prime_busy@hang-bsd2.html
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-iclb2/igt@prime_busy@hang-bsd2.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc6-dpms:
    - shard-tglb:         [SKIP][121] ([i915#468]) -> [FAIL][122] ([i915#454])
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb2/igt@i915_pm_dc@dc6-dpms.html
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb5/igt@i915_pm_dc@dc6-dpms.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-snb:          [INCOMPLETE][123] ([i915#82]) -> [SKIP][124] ([fdo#109271])
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-snb6/igt@i915_pm_dc@dc6-psr.html
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-snb6/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-onoff:
    - shard-tglb:         [SKIP][125] ([i915#668]) -> [DMESG-FAIL][126] ([i915#402] / [i915#49]) +1 similar issue
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-onoff.html
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-onoff.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-pwrite:
    - shard-tglb:         [SKIP][127] ([i915#668]) -> [FAIL][128] ([i915#49])
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7934/shard-tglb2/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-pwrite.html
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/shard-tglb1/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-pwrite.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [IGT#5]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/5
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#111703]: https://bugs.freedesktop.org/show_bug.cgi?id=111703
  [fdo#111842]: https://bugs.freedesktop.org/show_bug.cgi?id=111842
  [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
  [fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
  [i915#1139]: https://gitlab.freedesktop.org/drm/intel/issues/1139
  [i915#1149]: https://gitlab.freedesktop.org/drm/intel/issues/1149
  [i915#1171]: https://gitlab.freedesktop.org/drm/intel/issues/1171
  [i915#1172]: https://gitlab.freedesktop.org/drm/intel/issues/1172
  [i915#1173]: https://gitlab.freedesktop.org/drm/intel/issues/1173
  [i915#1180]: https://gitlab.freedesktop.org/drm/intel/issues/1180
  [i915#1183]: https://gitlab.freedesktop.org/drm/intel/issues/1183
  [i915#1184]: https://gitlab.freedesktop.org/drm/intel/issues/1184
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#146]: https://gitlab.freedesktop.org/drm/intel/issues/146
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#182]: https://gitlab.freedesktop.org/drm/intel/issues/182
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#315]: https://gitlab.freedesktop.org/drm/intel/issues/315
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#413]: https://gitlab.freedesktop.org/drm/intel/issues/413
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#559]: https://gitlab.freedesktop.org/drm/intel/issues/559
  [i915#56]: https://gitlab.freedesktop.org/drm/intel/issues/56
  [i915#607]: https://gitlab.freedesktop.org/drm/intel/issues/607
  [i915#608]: https://gitlab.freedesktop.org/drm/intel/issues/608
  [i915#616]: https://gitlab.freedesktop.org/drm/intel/issues/616
  [i915#644]: https://gitlab.freedesktop.org/drm/intel/issues/644
  [i915#65]: https://gitlab.freedesktop.org/drm/intel/issues/65
  [i915#668]: https://gitlab.freedesktop.org/drm/intel/issues/668
  [i915#677]: https://gitlab.freedesktop.org/drm/intel/issues/677
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#699]: https://gitlab.freedesktop.org/drm/intel/issues/699
  [i915#70]: https://gitlab.freedesktop.org/drm/intel/issues/70
  [i915#72]: https://gitlab.freedesktop.org/drm/intel/issues/72
  [i915#778]: https://gitlab.freedesktop.org/drm/intel/issues/778
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#899]: https://gitlab.freedesktop.org/drm/intel/issues/899
  [i915#979]: https://gitlab.freedesktop.org/drm/intel/issues/979


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7934 -> Patchwork_16562

  CI-20190529: 20190529
  CI_DRM_7934: 16668f8cd3512f56f626acaed0dd9245692ea3dc @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5440: 860924b6ccbed75b66ab4b65897bb9abc91763ea @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16562: bc797241a9a76df14e51a117b2988c8a9d07cc94 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16562/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/6] drm/i915: Introduce proper dbuf state

[Ville Syrjälä]

On Mon, Feb 17, 2020 at 08:46:40AM +0000, Lisovskiy, Stanislav wrote:
> On Thu, 2020-02-13 at 20:47 +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Add a global state to track the dbuf slices. Gets rid of all the
> > nasty
> > coupling between state->modeset and dbuf recompulation. Also we can
> > now
> > totally nuke state->active_pipe_changes.
> > 
> > dev_priv->wm.distrust_bios_wm still remains, but should probably also
> > get nuked from orbit later. Just didn't spend any significant time
> > pondering how to go about. The obvious thing would be to just
> > recompute
> > the thing every time.
> > 
> > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c  |  69 ++++---
> >  .../drm/i915/display/intel_display_power.c    |   4 +-
> >  .../drm/i915/display/intel_display_types.h    |  13 --
> >  drivers/gpu/drm/i915/i915_drv.h               |  11 +-
> >  drivers/gpu/drm/i915/intel_pm.c               | 185 ++++++++++++--
> > ----
> >  drivers/gpu/drm/i915/intel_pm.h               |  22 +++
> >  6 files changed, 205 insertions(+), 99 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index e09d3c93c52b..e331ab900336 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -7558,6 +7558,8 @@ static void intel_crtc_disable_noatomic(struct
> > intel_crtc *crtc,
> >  		to_intel_bw_state(dev_priv->bw_obj.state);
> >  	struct intel_cdclk_state *cdclk_state =
> >  		to_intel_cdclk_state(dev_priv->cdclk.obj.state);
> > +	struct intel_dbuf_state *dbuf_state =
> > +		to_intel_dbuf_state(dev_priv->dbuf.obj.state);
> >  	struct intel_crtc_state *crtc_state =
> >  		to_intel_crtc_state(crtc->base.state);
> >  	enum intel_display_power_domain domain;
> > @@ -7630,6 +7632,8 @@ static void intel_crtc_disable_noatomic(struct
> > intel_crtc *crtc,
> >  	cdclk_state->min_voltage_level[pipe] = 0;
> >  	cdclk_state->active_pipes &= ~BIT(pipe);
> >  
> > +	dbuf_state->active_pipes &= ~BIT(pipe);
> > +
> 
> May be I'm wrong(so being not offensive here :) ), but feels kind of
> redundant to have active_pipes in cdclk_state and at the same time in
> dbuf_state. Why can't it be still 
> in some more general state, which is inherited/used/aggregated by those
> dbuf and cdclk states? Otherwise you will have to do this duplicate
> code initializations which I see here, for example if there would be
> even more states you have then three or more similar initializations
> here,
> doing the same thing. This pretty much increases probability that
> somewhere in the code, you will eventually forget to do all
> initializations(well I guess you understand).
> Or if you will have to update the behavior, based on active_pipes here
> somehow, you will also have to change the duplicate code all over the
> place.

The problem with putting such things in some central place is that then
we get everything coupled together with said state. You get annoying
ordering requirements on which order you compute those things etc.
IMO better to just encapsulate each thing as much as possible. This way
you don't have to think at all what those other states are doing with
their lives.

The readout is a bit ugly yes, but we could provide a small helper
for that. It would still probably have somewhat annoying ordering
constraints though since we perhaps don't want to do actual readout
of active_pipes multiple times.

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

Re: [Intel-gfx] [PATCH 1/6] drm/i915: Introduce proper dbuf state

[Lisovskiy, Stanislav]

On Mon, 2020-02-17 at 16:45 +0200, Ville Syrjälä wrote:
> On Mon, Feb 17, 2020 at 08:46:40AM +0000, Lisovskiy, Stanislav wrote:
> > On Thu, 2020-02-13 at 20:47 +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Add a global state to track the dbuf slices. Gets rid of all the
> > > nasty
> > > coupling between state->modeset and dbuf recompulation. Also we
> > > can
> > > now
> > > totally nuke state->active_pipe_changes.
> > > 
> > > dev_priv->wm.distrust_bios_wm still remains, but should probably
> > > also
> > > get nuked from orbit later. Just didn't spend any significant
> > > time
> > > pondering how to go about. The obvious thing would be to just
> > > recompute
> > > the thing every time.
> > > 
> > > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c  |  69 ++++---
> > >  .../drm/i915/display/intel_display_power.c    |   4 +-
> > >  .../drm/i915/display/intel_display_types.h    |  13 --
> > >  drivers/gpu/drm/i915/i915_drv.h               |  11 +-
> > >  drivers/gpu/drm/i915/intel_pm.c               | 185
> > > ++++++++++++--
> > > ----
> > >  drivers/gpu/drm/i915/intel_pm.h               |  22 +++
> > >  6 files changed, 205 insertions(+), 99 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index e09d3c93c52b..e331ab900336 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -7558,6 +7558,8 @@ static void
> > > intel_crtc_disable_noatomic(struct
> > > intel_crtc *crtc,
> > >  		to_intel_bw_state(dev_priv->bw_obj.state);
> > >  	struct intel_cdclk_state *cdclk_state =
> > >  		to_intel_cdclk_state(dev_priv->cdclk.obj.state);
> > > +	struct intel_dbuf_state *dbuf_state =
> > > +		to_intel_dbuf_state(dev_priv->dbuf.obj.state);
> > >  	struct intel_crtc_state *crtc_state =
> > >  		to_intel_crtc_state(crtc->base.state);
> > >  	enum intel_display_power_domain domain;
> > > @@ -7630,6 +7632,8 @@ static void
> > > intel_crtc_disable_noatomic(struct
> > > intel_crtc *crtc,
> > >  	cdclk_state->min_voltage_level[pipe] = 0;
> > >  	cdclk_state->active_pipes &= ~BIT(pipe);
> > >  
> > > +	dbuf_state->active_pipes &= ~BIT(pipe);
> > > +
> > 
> > May be I'm wrong(so being not offensive here :) ), but feels kind
> > of
> > redundant to have active_pipes in cdclk_state and at the same time
> > in
> > dbuf_state. Why can't it be still 
> > in some more general state, which is inherited/used/aggregated by
> > those
> > dbuf and cdclk states? Otherwise you will have to do this duplicate
> > code initializations which I see here, for example if there would
> > be
> > even more states you have then three or more similar
> > initializations
> > here,
> > doing the same thing. This pretty much increases probability that
> > somewhere in the code, you will eventually forget to do all
> > initializations(well I guess you understand).
> > Or if you will have to update the behavior, based on active_pipes
> > here
> > somehow, you will also have to change the duplicate code all over
> > the
> > place.
> 
> The problem with putting such things in some central place is that
> then
> we get everything coupled together with said state. You get annoying
> ordering requirements on which order you compute those things etc.
> IMO better to just encapsulate each thing as much as possible. This
> way
> you don't have to think at all what those other states are doing with
> their lives.
> 
> The readout is a bit ugly yes, but we could provide a small helper
> for that. It would still probably have somewhat annoying ordering
> constraints though since we perhaps don't want to do actual readout
> of active_pipes multiple times.

I agree, regarding central place - coupling everything together would
be a step back to what we had before. Another option which we could use
is to not have any "central"  state at all(what you are already
actually doing) but many dedicated "orthogonal" states which contain
only the stuff which is required and related to their semantics.

Like you have bw state and all the stuff _only_ related to that,
dbuf state, sagv state, modeset state, which would have active_pipes 
and so on. Each of those are global state object which you swap 
on commit state. And no central state. Of course you are going to have
a dependencies - but we anyway have them. At least when you have those
as explicit objects, the dependencies can be then explicitly
identified. 
For instance dbuf_state and sagv_state would depend on modeset_state
might require some info from modeset_state however if there hasn't been
any changes we don't have to update modeset_state, but only dbuf_state 
if there were some plane changes. sagv state then would depend on both
dbuf state changes(obviously) and also modeset. In practice it mostly
will mean that we just read necessary info from those, however if those
which we depend on had changed then we certainly need to update
dependent objects as well. 

I.e would be call to have some explicit hiearchy like:

	   (modeset state object changed)
               |                    |
               |                    |
(dbuf/wm state state changed) (bw state changed)
               |                   |
               |                   |
(sagv state changed)         (sagv state changed)

So we would have well identified relashionships regarding
which state needs which information from some states and
also changes in which states require recalculation in other
states.

We anyway can't get rid of those dependencies, they exist even
now - however the problem is that currently they are implicit
and not sometimes clearly visible, which brings problems. 

Stan


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

Re: [Intel-gfx] [PATCH 5/6] drm/i915: Move the dbuf pre/post plane update

[Lisovskiy, Stanislav]

On Thu, 2020-02-13 at 20:47 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Encapsulate the dbuf state more by moving the pre/post
> plane functions out from intel_display.c. For now we stick
> them into intel_pm.c since that's where the rest of the code
> lives for now.
> 
> Eventually we should add a new file for this stuff at which
> point we also need to decide if it makes sense to even split
> the wm code from the ddb code, or to keep them together.

Yes, that definitely makes sense. May be we should one day,
add a separate file for wm/ddb/dbuf management, because intel_pm.c
seems to me a bit _overloaded_ with functionality right now.

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> 
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 41 +-----------------
> --
>  drivers/gpu/drm/i915/intel_pm.c              | 37 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_pm.h              |  2 +
>  3 files changed, 41 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 7fb25c7655d1..796e7224f1dc 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -15449,43 +15449,6 @@ static void
> intel_update_trans_port_sync_crtcs(struct intel_crtc *crtc,
>  				       state);
>  }
>  
> -static void icl_dbuf_slice_pre_update(struct intel_atomic_state
> *state)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	const struct intel_dbuf_state *new_dbuf_state =
> -		intel_atomic_get_new_dbuf_state(state);
> -	const struct intel_dbuf_state *old_dbuf_state =
> -		intel_atomic_get_old_dbuf_state(state);
> -
> -	if (!new_dbuf_state ||
> -	    new_dbuf_state->enabled_slices == old_dbuf_state-
> >enabled_slices)
> -		return;
> -
> -	WARN_ON(!new_dbuf_state->base.changed);
> -
> -	gen9_dbuf_slices_update(dev_priv,
> -				old_dbuf_state->enabled_slices |
> -				new_dbuf_state->enabled_slices);
> -}
> -
> -static void icl_dbuf_slice_post_update(struct intel_atomic_state
> *state)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	const struct intel_dbuf_state *new_dbuf_state =
> -		intel_atomic_get_new_dbuf_state(state);
> -	const struct intel_dbuf_state *old_dbuf_state =
> -		intel_atomic_get_old_dbuf_state(state);
> -
> -	if (!new_dbuf_state ||
> -	    new_dbuf_state->enabled_slices == old_dbuf_state-
> >enabled_slices)
> -		return;
> -
> -	WARN_ON(!new_dbuf_state->base.changed);
> -
> -	gen9_dbuf_slices_update(dev_priv,
> -				new_dbuf_state->enabled_slices);
> -}
> -
>  static void skl_commit_modeset_enables(struct intel_atomic_state
> *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> @@ -15738,7 +15701,7 @@ static void intel_atomic_commit_tail(struct
> intel_atomic_state *state)
>  	if (state->modeset)
>  		intel_encoders_update_prepare(state);
>  
> -	icl_dbuf_slice_pre_update(state);
> +	intel_dbuf_pre_plane_update(state);
>  
>  	/* Now enable the clocks, plane, pipe, and connectors that we
> set up. */
>  	dev_priv->display.commit_modeset_enables(state);
> @@ -15793,7 +15756,7 @@ static void intel_atomic_commit_tail(struct
> intel_atomic_state *state)
>  			dev_priv->display.optimize_watermarks(state,
> crtc);
>  	}
>  
> -	icl_dbuf_slice_post_update(state);
> +	intel_dbuf_post_plane_update(state);
>  
>  	for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state, new_crtc_state, i) {
>  		intel_post_plane_update(state, crtc);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 0032458f0a5d..39349d305533 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -7573,3 +7573,40 @@ int intel_dbuf_init(struct drm_i915_private
> *dev_priv)
>  
>  	return 0;
>  }
> +
> +void intel_dbuf_pre_plane_update(struct intel_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	const struct intel_dbuf_state *new_dbuf_state =
> +		intel_atomic_get_new_dbuf_state(state);
> +	const struct intel_dbuf_state *old_dbuf_state =
> +		intel_atomic_get_old_dbuf_state(state);
> +
> +	if (!new_dbuf_state ||
> +	    new_dbuf_state->enabled_slices == old_dbuf_state-
> >enabled_slices)
> +		return;
> +
> +	WARN_ON(!new_dbuf_state->base.changed);
> +
> +	gen9_dbuf_slices_update(dev_priv,
> +				old_dbuf_state->enabled_slices |
> +				new_dbuf_state->enabled_slices);
> +}
> +
> +void intel_dbuf_post_plane_update(struct intel_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	const struct intel_dbuf_state *new_dbuf_state =
> +		intel_atomic_get_new_dbuf_state(state);
> +	const struct intel_dbuf_state *old_dbuf_state =
> +		intel_atomic_get_old_dbuf_state(state);
> +
> +	if (!new_dbuf_state ||
> +	    new_dbuf_state->enabled_slices == old_dbuf_state-
> >enabled_slices)
> +		return;
> +
> +	WARN_ON(!new_dbuf_state->base.changed);
> +
> +	gen9_dbuf_slices_update(dev_priv,
> +				new_dbuf_state->enabled_slices);
> +}
> diff --git a/drivers/gpu/drm/i915/intel_pm.h
> b/drivers/gpu/drm/i915/intel_pm.h
> index 1054a0ab1e40..8204d6a5526c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.h
> +++ b/drivers/gpu/drm/i915/intel_pm.h
> @@ -79,5 +79,7 @@ intel_atomic_get_dbuf_state(struct
> intel_atomic_state *state);
>  	to_intel_dbuf_state(intel_atomic_get_new_global_obj_state(state
> , &to_i915(state->base.dev)->dbuf.obj))
>  
>  int intel_dbuf_init(struct drm_i915_private *dev_priv);
> +void intel_dbuf_pre_plane_update(struct intel_atomic_state *state);
> +void intel_dbuf_post_plane_update(struct intel_atomic_state *state);
>  
>  #endif /* __INTEL_PM_H__ */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx