[PATCH v2 00/13] drm/i915: Plane cdclk requirements and fp16 for gen4+

[PATCH v2 00/13] drm/i915: Plane cdclk requirements and fp16 for gen4+

[Ville Syrjala]

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

Several patches have been pushed, new patches have appeared at the end.
I wonder if I've invented a perpetual motion patch series...

The new stuff is mostly just cleaning up the cdclk state (mis)management
we have going on. I also modifier the global state locking stuff
a bit to make sure we correctly serialize the hardware stuff when
cdclk actually changes (and we need it for the QGV stuff as well).

I also tweaked the debug messages to trigger only when a plane
actually needs to bump the cdclk. Previously it would print that
stuff all the time.

Ville Syrjälä (13):
  drm/i915: Add debugs to distingiush a cd2x update from a full cdclk
    pll update
  drm/i915: Rework global state locking
  drm/i915: Move check_digital_port_conflicts() earier
  drm/i915: Allow planes to declare their minimum acceptable cdclk
  drm/i915: Eliminate skl_check_pipe_max_pixel_rate()
  drm/i915: Simplify skl_max_scale()
  drm/i915: Add support for half float framebuffers for skl+
  drm/i915: Add support for half float framebuffers for gen4+ primary
    planes
  drm/i915: Add support for half float framebuffers for ivb+ sprites
  drm/i915: Add support for half float framebuffers on snb sprites
  drm/i915: Move more cdclk state handling into
    intel_modeset_calc_cdclk()
  drm/i915: Consolidate more cdclk state handling
  drm/i915: Collect more cdclk state under the same roof

 drivers/gpu/drm/i915/display/intel_atomic.c   |  43 ++
 drivers/gpu/drm/i915/display/intel_atomic.h   |   5 +
 .../gpu/drm/i915/display/intel_atomic_plane.c |  39 ++
 .../gpu/drm/i915/display/intel_atomic_plane.h |   2 +
 drivers/gpu/drm/i915/display/intel_audio.c    |  10 +-
 drivers/gpu/drm/i915/display/intel_cdclk.c    | 167 ++++---
 drivers/gpu/drm/i915/display/intel_cdclk.h    |   1 +
 drivers/gpu/drm/i915/display/intel_display.c  | 339 ++++++++++----
 drivers/gpu/drm/i915/display/intel_display.h  |   2 -
 .../drm/i915/display/intel_display_types.h    |  22 +-
 drivers/gpu/drm/i915/display/intel_sprite.c   | 428 +++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_sprite.h   |   7 +
 drivers/gpu/drm/i915/i915_drv.h               |  20 +-
 drivers/gpu/drm/i915/intel_pm.c               |  87 ----
 drivers/gpu/drm/i915/intel_pm.h               |   2 -
 15 files changed, 914 insertions(+), 260 deletions(-)

-- 
2.21.0

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

[PATCH v2 01/13] drm/i915: Add debugs to distingiush a cd2x update from a full cdclk pll update

[Ville Syrjala]

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

To make the logs a bit less confusing let's toss in some
debug prints to indicate whether the cdclk reprogramming
is going to happen with a single pipe active or whether we
need to turn all pipes off for the duration.

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

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 3d867963a6d1..6eaebc38ee01 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -2343,6 +2343,9 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
 			return ret;
 
 		state->cdclk.pipe = pipe;
+
+		DRM_DEBUG_KMS("Can change cdclk with pipe %c active\n",
+			      pipe_name(pipe));
 	} else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
 					     &state->cdclk.actual)) {
 		ret = intel_modeset_all_pipes(state);
@@ -2350,6 +2353,8 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
 			return ret;
 
 		state->cdclk.pipe = INVALID_PIPE;
+
+		DRM_DEBUG_KMS("Modeset required for cdclk change\n");
 	}
 
 	DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz, actual %u kHz\n",
-- 
2.21.0

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

[PATCH v2 02/13] drm/i915: Rework global state locking

[Ville Syrjala]

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

So far we've sort of protected the global state under dev_priv with
the connection_mutex. I wan to change that so that we can change the
cdclk even for pure plane updates. To that end let's formalize the
protection of the global state to follow what I started with the cdclk
code already (though not entirely properly) such that any crtc mutex
will suffice as a read lock, and all crtcs mutexes act as the write
lock.

We'll also pimp intel_atomic_state_clear() to clear the entire global
state, so that we don't accidentally leak stale information between
the locking retries.

As a slight optimization we'll only lock the crtc mutexes to protect
the global state, however if and when we actually have to poke the
hw (eg. if the actual cdclk changes) we must serialize commits
across all crtcs so that a parallel nonblocking commit can't get
ahead of the cdclk reprogamming. We do that by adding all crtcs to
the state.

TODO: the old global state examined during commit may still
be a problem since it always looks at the _latest_ swapped state
in dev_priv. Need to add proper old/new state for that too I think.

v2: Remeber to serialize the commits if necessary

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_atomic.c   |  44 ++++++++
 drivers/gpu/drm/i915/display/intel_atomic.h   |   5 +
 drivers/gpu/drm/i915/display/intel_audio.c    |  10 +-
 drivers/gpu/drm/i915/display/intel_cdclk.c    | 102 ++++++++++--------
 drivers/gpu/drm/i915/display/intel_display.c  |  29 ++++-
 .../drm/i915/display/intel_display_types.h    |   8 ++
 drivers/gpu/drm/i915/i915_drv.h               |  11 +-
 7 files changed, 153 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
index c5a552a69752..9cd6d2348a1e 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -429,6 +429,13 @@ void intel_atomic_state_clear(struct drm_atomic_state *s)
 	struct intel_atomic_state *state = to_intel_atomic_state(s);
 	drm_atomic_state_default_clear(&state->base);
 	state->dpll_set = state->modeset = false;
+	state->global_state_changed = false;
+	state->active_pipes = 0;
+	memset(&state->min_cdclk, 0, sizeof(state->min_cdclk));
+	memset(&state->min_voltage_level, 0, sizeof(state->min_voltage_level));
+	memset(&state->cdclk.logical, 0, sizeof(state->cdclk.logical));
+	memset(&state->cdclk.actual, 0, sizeof(state->cdclk.actual));
+	state->cdclk.pipe = INVALID_PIPE;
 }
 
 struct intel_crtc_state *
@@ -442,3 +449,40 @@ intel_atomic_get_crtc_state(struct drm_atomic_state *state,
 
 	return to_intel_crtc_state(crtc_state);
 }
+
+int intel_atomic_lock_global_state(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_crtc *crtc;
+
+	state->global_state_changed = true;
+
+	for_each_intel_crtc(&dev_priv->drm, crtc) {
+		int ret;
+
+		ret = drm_modeset_lock(&crtc->base.mutex,
+				       state->base.acquire_ctx);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+int intel_atomic_serialize_global_state(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_crtc *crtc;
+
+	state->global_state_changed = true;
+
+	for_each_intel_crtc(&dev_priv->drm, crtc) {
+		struct intel_crtc_state *crtc_state;
+
+		crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
+		if (IS_ERR(crtc_state))
+			return PTR_ERR(crtc_state);
+	}
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.h b/drivers/gpu/drm/i915/display/intel_atomic.h
index 58065d3161a3..49d5cb1b9e0a 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic.h
@@ -16,6 +16,7 @@ struct drm_crtc_state;
 struct drm_device;
 struct drm_i915_private;
 struct drm_property;
+struct intel_atomic_state;
 struct intel_crtc;
 struct intel_crtc_state;
 
@@ -46,4 +47,8 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
 			       struct intel_crtc *intel_crtc,
 			       struct intel_crtc_state *crtc_state);
 
+int intel_atomic_lock_global_state(struct intel_atomic_state *state);
+
+int intel_atomic_serialize_global_state(struct intel_atomic_state *state);
+
 #endif /* __INTEL_ATOMIC_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index ed18511befa3..85e6b2bbb34f 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -28,6 +28,7 @@
 #include <drm/i915_component.h>
 
 #include "i915_drv.h"
+#include "intel_atomic.h"
 #include "intel_audio.h"
 #include "intel_display_types.h"
 #include "intel_lpe_audio.h"
@@ -818,13 +819,8 @@ static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
 	to_intel_atomic_state(state)->cdclk.force_min_cdclk =
 		enable ? 2 * 96000 : 0;
 
-	/*
-	 * Protects dev_priv->cdclk.force_min_cdclk
-	 * Need to lock this here in case we have no active pipes
-	 * and thus wouldn't lock it during the commit otherwise.
-	 */
-	ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
-			       &ctx);
+	/* Protects dev_priv->cdclk.force_min_cdclk */
+	ret = intel_atomic_lock_global_state(to_intel_atomic_state(state));
 	if (!ret)
 		ret = drm_atomic_commit(state);
 
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 6eaebc38ee01..6c17a3bbf866 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -2007,11 +2007,20 @@ static int intel_compute_min_cdclk(struct intel_atomic_state *state)
 	       sizeof(state->min_cdclk));
 
 	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
+		int ret;
+
 		min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
 		if (min_cdclk < 0)
 			return min_cdclk;
 
+		if (state->min_cdclk[i] == min_cdclk)
+			continue;
+
 		state->min_cdclk[i] = min_cdclk;
+
+		ret = intel_atomic_lock_global_state(state);
+		if (ret)
+			return ret;
 	}
 
 	min_cdclk = state->cdclk.force_min_cdclk;
@@ -2034,7 +2043,7 @@ static int intel_compute_min_cdclk(struct intel_atomic_state *state)
  * future platforms this code will need to be
  * adjusted.
  */
-static u8 bxt_compute_min_voltage_level(struct intel_atomic_state *state)
+static int bxt_compute_min_voltage_level(struct intel_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
 	struct intel_crtc *crtc;
@@ -2047,11 +2056,21 @@ static u8 bxt_compute_min_voltage_level(struct intel_atomic_state *state)
 	       sizeof(state->min_voltage_level));
 
 	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
+		int ret;
+
 		if (crtc_state->base.enable)
-			state->min_voltage_level[i] =
-				crtc_state->min_voltage_level;
+			min_voltage_level = crtc_state->min_voltage_level;
 		else
-			state->min_voltage_level[i] = 0;
+			min_voltage_level = 0;
+
+		if (state->min_voltage_level[i] == min_voltage_level)
+			continue;
+
+		state->min_voltage_level[i] = min_voltage_level;
+
+		ret = intel_atomic_lock_global_state(state);
+		if (ret)
+			return ret;
 	}
 
 	min_voltage_level = 0;
@@ -2195,20 +2214,24 @@ static int skl_modeset_calc_cdclk(struct intel_atomic_state *state)
 static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-	int min_cdclk, cdclk, vco;
+	int min_cdclk, min_voltage_level, cdclk, vco;
 
 	min_cdclk = intel_compute_min_cdclk(state);
 	if (min_cdclk < 0)
 		return min_cdclk;
 
+	min_voltage_level = bxt_compute_min_voltage_level(state);
+	if (min_voltage_level < 0)
+		return min_voltage_level;
+
 	cdclk = bxt_calc_cdclk(dev_priv, min_cdclk);
 	vco = bxt_calc_cdclk_pll_vco(dev_priv, cdclk);
 
 	state->cdclk.logical.vco = vco;
 	state->cdclk.logical.cdclk = cdclk;
 	state->cdclk.logical.voltage_level =
-		max(dev_priv->display.calc_voltage_level(cdclk),
-		    bxt_compute_min_voltage_level(state));
+		max_t(int, min_voltage_level,
+		      dev_priv->display.calc_voltage_level(cdclk));
 
 	if (!state->active_pipes) {
 		cdclk = bxt_calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
@@ -2225,23 +2248,6 @@ static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
 	return 0;
 }
 
-static int intel_lock_all_pipes(struct intel_atomic_state *state)
-{
-	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-	struct intel_crtc *crtc;
-
-	/* Add all pipes to the state */
-	for_each_intel_crtc(&dev_priv->drm, crtc) {
-		struct intel_crtc_state *crtc_state;
-
-		crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
-		if (IS_ERR(crtc_state))
-			return PTR_ERR(crtc_state);
-	}
-
-	return 0;
-}
-
 static int intel_modeset_all_pipes(struct intel_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
@@ -2308,46 +2314,56 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
 		return ret;
 
 	/*
-	 * Writes to dev_priv->cdclk.logical must protected by
-	 * holding all the crtc locks, even if we don't end up
+	 * Writes to dev_priv->cdclk.{actual,logical} must protected
+	 * by holding all the crtc mutexes even if we don't end up
 	 * touching the hardware
 	 */
-	if (intel_cdclk_changed(&dev_priv->cdclk.logical,
-				&state->cdclk.logical)) {
-		ret = intel_lock_all_pipes(state);
-		if (ret < 0)
+	if (intel_cdclk_changed(&dev_priv->cdclk.actual,
+				&state->cdclk.actual)) {
+		/*
+		 * Also serialize commits across all crtcs
+		 * if the actual hw needs to be poked.
+		 */
+		ret = intel_atomic_serialize_global_state(state);
+		if (ret)
+			return ret;
+	} else if (intel_cdclk_changed(&dev_priv->cdclk.logical,
+				       &state->cdclk.logical)) {
+		ret = intel_atomic_lock_global_state(state);
+		if (ret)
 			return ret;
+	} else {
+		return 0;
 	}
 
-	if (is_power_of_2(state->active_pipes)) {
+	if (is_power_of_2(state->active_pipes) &&
+	    intel_cdclk_needs_cd2x_update(dev_priv,
+					  &dev_priv->cdclk.actual,
+					  &state->cdclk.actual)) {
 		struct intel_crtc *crtc;
 		struct intel_crtc_state *crtc_state;
 
 		pipe = ilog2(state->active_pipes);
 		crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
-		crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
-		if (crtc_state &&
-		    drm_atomic_crtc_needs_modeset(&crtc_state->base))
+
+		crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
+		if (IS_ERR(crtc_state))
+			return PTR_ERR(crtc_state);
+
+		if (drm_atomic_crtc_needs_modeset(&crtc_state->base))
 			pipe = INVALID_PIPE;
 	} else {
 		pipe = INVALID_PIPE;
 	}
 
-	/* All pipes must be switched off while we change the cdclk. */
-	if (pipe != INVALID_PIPE &&
-	    intel_cdclk_needs_cd2x_update(dev_priv,
-					  &dev_priv->cdclk.actual,
-					  &state->cdclk.actual)) {
-		ret = intel_lock_all_pipes(state);
-		if (ret)
-			return ret;
-
+	if (pipe != INVALID_PIPE) {
 		state->cdclk.pipe = pipe;
 
 		DRM_DEBUG_KMS("Can change cdclk with pipe %c active\n",
 			      pipe_name(pipe));
 	} else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
 					     &state->cdclk.actual)) {
+		/* All pipes must be switched off while we change the cdclk. */
 		ret = intel_modeset_all_pipes(state);
 		if (ret)
 			return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 7d7d1859775a..a8444d9841c1 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -12191,6 +12191,12 @@ static bool check_digital_port_conflicts(struct intel_atomic_state *state)
 	unsigned int used_mst_ports = 0;
 	bool ret = true;
 
+	/*
+	 * We're going to peek into connector->state,
+	 * hence connection_mutex must be held.
+	 */
+	drm_modeset_lock_assert_held(&dev->mode_config.connection_mutex);
+
 	/*
 	 * Walk the connector list instead of the encoder
 	 * list to detect the problem on ddi platforms
@@ -13463,7 +13469,6 @@ static int intel_modeset_checks(struct intel_atomic_state *state)
 	state->active_pipes = dev_priv->active_pipes;
 	state->cdclk.logical = dev_priv->cdclk.logical;
 	state->cdclk.actual = dev_priv->cdclk.actual;
-	state->cdclk.pipe = INVALID_PIPE;
 
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
@@ -13476,6 +13481,12 @@ static int intel_modeset_checks(struct intel_atomic_state *state)
 			state->active_pipe_changes |= BIT(crtc->pipe);
 	}
 
+	if (state->active_pipe_changes) {
+		ret = intel_atomic_lock_global_state(state);
+		if (ret)
+			return ret;
+	}
+
 	ret = intel_modeset_calc_cdclk(state);
 	if (ret)
 		return ret;
@@ -13577,7 +13588,7 @@ static int intel_atomic_check(struct drm_device *dev,
 	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
 	struct intel_crtc *crtc;
 	int ret, i;
-	bool any_ms = state->cdclk.force_min_cdclk_changed;
+	bool any_ms = false;
 
 	/* Catch I915_MODE_FLAG_INHERITED */
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
@@ -13615,6 +13626,8 @@ static int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
+	any_ms |= state->cdclk.force_min_cdclk_changed;
+
 	if (any_ms) {
 		ret = intel_modeset_checks(state);
 		if (ret)
@@ -14234,6 +14247,14 @@ static void intel_atomic_track_fbs(struct intel_atomic_state *state)
 					plane->frontbuffer_bit);
 }
 
+static void assert_global_state_locked(struct drm_i915_private *dev_priv)
+{
+	struct intel_crtc *crtc;
+
+	for_each_intel_crtc(&dev_priv->drm, crtc)
+		drm_modeset_lock_assert_held(&crtc->base.mutex);
+}
+
 static int intel_atomic_commit(struct drm_device *dev,
 			       struct drm_atomic_state *_state,
 			       bool nonblock)
@@ -14299,7 +14320,9 @@ static int intel_atomic_commit(struct drm_device *dev,
 	intel_shared_dpll_swap_state(state);
 	intel_atomic_track_fbs(state);
 
-	if (state->modeset) {
+	if (state->global_state_changed) {
+		assert_global_state_locked(dev_priv);
+
 		memcpy(dev_priv->min_cdclk, state->min_cdclk,
 		       sizeof(state->min_cdclk));
 		memcpy(dev_priv->min_voltage_level, state->min_voltage_level,
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 40390d855815..01fed8957ade 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -506,6 +506,14 @@ struct intel_atomic_state {
 
 	bool rps_interactive;
 
+	/*
+	 * active_pipes
+	 * min_cdclk[]
+	 * min_voltage_level[]
+	 * cdclk.*
+	 */
+	bool global_state_changed;
+
 	/* Gen9+ only */
 	struct skl_ddb_values wm_results;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c46b339064c0..d3e61152d924 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1105,13 +1105,14 @@ struct drm_i915_private {
 	unsigned int fdi_pll_freq;
 	unsigned int czclk_freq;
 
+	/*
+	 * For reading holding any crtc lock is sufficient,
+	 * for writing must hold all of them.
+	 */
 	struct {
 		/*
 		 * The current logical cdclk state.
 		 * See intel_atomic_state.cdclk.logical
-		 *
-		 * For reading holding any crtc lock is sufficient,
-		 * for writing must hold all of them.
 		 */
 		struct intel_cdclk_state logical;
 		/*
@@ -1181,6 +1182,10 @@ struct drm_i915_private {
 	 */
 	struct mutex dpll_lock;
 
+	/*
+	 * For reading active_pipes, min_cdclk, min_voltage_level holding
+	 * any crtc lock is sufficient, for writing must hold all of them.
+	 */
 	u8 active_pipes;
 	/* minimum acceptable cdclk for each pipe */
 	int min_cdclk[I915_MAX_PIPES];
-- 
2.21.0

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

[PATCH v2 03/13] drm/i915: Move check_digital_port_conflicts() earier

[Ville Syrjala]

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

check_digital_port_conflicts() is done needlessly late. Move it earlier.
This will be needed as later on we want to set any_ms=true a bit later
for non-modesets too and we can't call this guy without the
connection_mutex held.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a8444d9841c1..44bd4d15019b 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -13456,11 +13456,6 @@ static int intel_modeset_checks(struct intel_atomic_state *state)
 	struct intel_crtc *crtc;
 	int ret, i;
 
-	if (!check_digital_port_conflicts(state)) {
-		DRM_DEBUG_KMS("rejecting conflicting digital port configuration\n");
-		return -EINVAL;
-	}
-
 	/* keep the current setting */
 	if (!state->cdclk.force_min_cdclk_changed)
 		state->cdclk.force_min_cdclk = dev_priv->cdclk.force_min_cdclk;
@@ -13622,6 +13617,12 @@ static int intel_atomic_check(struct drm_device *dev,
 			any_ms = true;
 	}
 
+	if (any_ms && !check_digital_port_conflicts(state)) {
+		DRM_DEBUG_KMS("rejecting conflicting digital port configuration\n");
+		ret = EINVAL;
+		goto fail;
+	}
+
 	ret = drm_dp_mst_atomic_check(&state->base);
 	if (ret)
 		goto fail;
-- 
2.21.0

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

[PATCH v2 04/13] drm/i915: Allow planes to declare their minimum acceptable cdclk

[Ville Syrjala]

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

Various pixel formats and plane scaling impose additional constraints
on the cdclk frequency. Provide a new plane->min_cdclk() hook that
will be used to compute the minimum acceptable cdclk frequency for
each plane.

Annoyingly on some platforms the numer of active planes affects
this calculation so we must also toss in more planes into the
state when the number of active planes changes.

The sequence of state computation must also be changed:
1. check_plane() (updates plane's visibility etc.)
2. figure out if more planes now require update min_cdclk
   computaion
3. calculate the new min cdclk for each plane in the state
4. if the minimum of any plane now exceeds the current
   logical cdclk we recompute the cdclk
4. during cdclk computation take the planes' min_cdclk into
   accoutn
5. follow the normal cdclk programming to change the
   cdclk frequency. This may now require a modeset (except
   on bxt/glk in some cases), which either succeeds or
   fails depending on whether userspace has given
   us permission to perform a modeset or not.

v2: Fix plane id check in intel_crtc_add_planes_to_state()
    Only print the debug message when cdclk needs bumping
    Use dev_priv->cdclk... as the old state explicitly

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../gpu/drm/i915/display/intel_atomic_plane.c |  39 ++
 .../gpu/drm/i915/display/intel_atomic_plane.h |   2 +
 drivers/gpu/drm/i915/display/intel_cdclk.c    |  16 +
 drivers/gpu/drm/i915/display/intel_display.c  | 184 ++++++++--
 .../drm/i915/display/intel_display_types.h    |   4 +
 drivers/gpu/drm/i915/display/intel_sprite.c   | 341 ++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_sprite.h   |   7 +
 7 files changed, 571 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index a6cff5a160fb..98f557a9f8ee 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -138,6 +138,44 @@ unsigned int intel_plane_data_rate(const struct intel_crtc_state *crtc_state,
 	return cpp * crtc_state->pixel_rate;
 }
 
+bool intel_plane_calc_min_cdclk(struct intel_atomic_state *state,
+				struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	const struct intel_plane_state *plane_state =
+		intel_atomic_get_new_plane_state(state, plane);
+	struct intel_crtc *crtc = to_intel_crtc(plane_state->base.crtc);
+	struct intel_crtc_state *crtc_state;
+
+	if (!plane_state->base.visible || !plane->min_cdclk)
+		return false;
+
+	crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
+
+	crtc_state->min_cdclk[plane->id] =
+		plane->min_cdclk(crtc_state, plane_state);
+
+	/*
+	 * Does the cdclk need to be bumbed up?
+	 *
+	 * Note: we obviously need to be called before the new
+	 * cdclk frequency is calculated so state->cdclk.logical
+	 * hasn't been populated yet. Hence we look at the old
+	 * cdclk state under dev_priv->cdclk.logical. This is
+	 * safe as long we hold at least one crtc mutex (which
+	 * must be true since we have crtc_state).
+	 */
+	if (crtc_state->min_cdclk[plane->id] > dev_priv->cdclk.logical.cdclk) {
+		DRM_DEBUG_KMS("[PLANE:%d:%s] min_cdclk (%d kHz) > logical cdclk (%d kHz)\n",
+			      plane->base.base.id, plane->base.name,
+			      crtc_state->min_cdclk[plane->id],
+			      dev_priv->cdclk.logical.cdclk);
+		return true;
+	}
+
+	return false;
+}
+
 int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_state,
 					struct intel_crtc_state *new_crtc_state,
 					const struct intel_plane_state *old_plane_state,
@@ -151,6 +189,7 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
 	new_crtc_state->nv12_planes &= ~BIT(plane->id);
 	new_crtc_state->c8_planes &= ~BIT(plane->id);
 	new_crtc_state->data_rate[plane->id] = 0;
+	new_crtc_state->min_cdclk[plane->id] = 0;
 	new_plane_state->base.visible = false;
 
 	if (!new_plane_state->base.crtc && !old_plane_state->base.crtc)
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
index dc85af02e9b7..e61e9a82aadf 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
@@ -47,5 +47,7 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
 				    struct intel_crtc_state *crtc_state,
 				    const struct intel_plane_state *old_plane_state,
 				    struct intel_plane_state *plane_state);
+bool intel_plane_calc_min_cdclk(struct intel_atomic_state *state,
+				struct intel_plane *plane);
 
 #endif /* __INTEL_ATOMIC_PLANE_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 6c17a3bbf866..0caef2592a7e 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1918,6 +1918,19 @@ static int intel_pixel_rate_to_cdclk(const struct intel_crtc_state *crtc_state)
 		return DIV_ROUND_UP(pixel_rate * 100, 90);
 }
 
+static int intel_planes_min_cdclk(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_plane *plane;
+	int min_cdclk = 0;
+
+	for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane)
+		min_cdclk = max(crtc_state->min_cdclk[plane->id], min_cdclk);
+
+	return min_cdclk;
+}
+
 int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv =
@@ -1986,6 +1999,9 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 	    IS_GEMINILAKE(dev_priv))
 		min_cdclk = max(158400, min_cdclk);
 
+	/* Account for additional needs from the planes */
+	min_cdclk = max(intel_planes_min_cdclk(crtc_state), min_cdclk);
+
 	if (min_cdclk > dev_priv->max_cdclk_freq) {
 		DRM_DEBUG_KMS("required cdclk (%d kHz) exceeds max (%d kHz)\n",
 			      min_cdclk, dev_priv->max_cdclk_freq);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 44bd4d15019b..67bb5fa03b2b 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3140,6 +3140,7 @@ static void intel_plane_disable_noatomic(struct intel_crtc *crtc,
 	intel_set_plane_visible(crtc_state, plane_state, false);
 	fixup_active_planes(crtc_state);
 	crtc_state->data_rate[plane->id] = 0;
+	crtc_state->min_cdclk[plane->id] = 0;
 
 	if (plane->id == PLANE_PRIMARY)
 		intel_pre_disable_primary_noatomic(&crtc->base);
@@ -3563,6 +3564,53 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
 	return 0;
 }
 
+static void i9xx_plane_ratio(const struct intel_crtc_state *crtc_state,
+			     const struct intel_plane_state *plane_state,
+			     unsigned int *num, unsigned int *den)
+{
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	unsigned int cpp = fb->format->cpp[0];
+
+	/*
+	 * g4x bspec says 64bpp pixel rate can't exceed 80%
+	 * of cdclk when the sprite plane is enabled on the
+	 * same pipe. ilk/snb bspec says 64bpp pixel rate is
+	 * never allowed to exceed 80% of cdclk. Let's just go
+	 * with the ilk/snb limit always.
+	 */
+	if (cpp == 8) {
+		*num = 10;
+		*den = 8;
+	} else {
+		*num = 1;
+		*den = 1;
+	}
+}
+
+static int i9xx_plane_min_cdclk(const struct intel_crtc_state *crtc_state,
+				const struct intel_plane_state *plane_state)
+{
+	unsigned int pixel_rate;
+	unsigned int num, den;
+
+	/*
+	 * Note that crtc_state->pixel_rate accounts for both
+	 * horizontal and vertical panel fitter downscaling factors.
+	 * Pre-HSW bspec tells us to only consider the horizontal
+	 * downscaling factor here. We ignore that and just consider
+	 * both for simplicity.
+	 */
+	pixel_rate = crtc_state->pixel_rate;
+
+	i9xx_plane_ratio(crtc_state, plane_state, &num, &den);
+
+	/* two pixels per clock with double wide pipe */
+	if (crtc_state->double_wide)
+		den *= 2;
+
+	return DIV_ROUND_UP(pixel_rate * num, den);
+}
+
 unsigned int
 i9xx_plane_max_stride(struct intel_plane *plane,
 		      u32 pixel_format, u64 modifier,
@@ -11557,6 +11605,7 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
 		plane_state->base.visible = visible = false;
 		crtc_state->active_planes &= ~BIT(plane->id);
 		crtc_state->data_rate[plane->id] = 0;
+		crtc_state->min_cdclk[plane->id] = 0;
 	}
 
 	if (!was_visible && !visible)
@@ -11838,9 +11887,6 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state,
 	if (INTEL_GEN(dev_priv) >= 9) {
 		if (mode_changed || crtc_state->update_pipe)
 			ret = skl_update_scaler_crtc(crtc_state);
-
-		if (!ret)
-			ret = icl_check_nv12_planes(crtc_state);
 		if (!ret)
 			ret = skl_check_pipe_max_pixel_rate(crtc, crtc_state);
 		if (!ret)
@@ -13534,12 +13580,49 @@ static void intel_crtc_check_fastset(const struct intel_crtc_state *old_crtc_sta
 	new_crtc_state->has_drrs = old_crtc_state->has_drrs;
 }
 
-static int intel_atomic_check_planes(struct intel_atomic_state *state)
+static int intel_crtc_add_planes_to_state(struct intel_atomic_state *state,
+					  struct intel_crtc *crtc,
+					  u8 plane_ids_mask)
 {
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_plane *plane;
+
+	for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
+		struct intel_plane_state *plane_state;
+
+		if ((plane_ids_mask & BIT(plane->id)) == 0)
+			continue;
+
+		plane_state = intel_atomic_get_plane_state(state, plane);
+		if (IS_ERR(plane_state))
+			return PTR_ERR(plane_state);
+	}
+
+	return 0;
+}
+
+static bool active_planes_affects_min_cdclk(struct drm_i915_private *dev_priv)
+{
+	/* See {hsw,vlv,ivb}_plane_ratio() */
+	return IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv) ||
+		IS_CHERRYVIEW(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
+		IS_IVYBRIDGE(dev_priv);
+}
+
+static int intel_atomic_check_planes(struct intel_atomic_state *state,
+				     bool *need_modeset)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
 	struct intel_plane_state *plane_state;
 	struct intel_plane *plane;
+	struct intel_crtc *crtc;
 	int i, ret;
 
+	ret = icl_add_linked_planes(state);
+	if (ret)
+		return ret;
+
 	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
 		ret = intel_plane_atomic_check(state, plane);
 		if (ret) {
@@ -13549,6 +13632,41 @@ static int intel_atomic_check_planes(struct intel_atomic_state *state)
 		}
 	}
 
+	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+					    new_crtc_state, i) {
+		u8 old_active_planes, new_active_planes;
+
+		ret = icl_check_nv12_planes(new_crtc_state);
+		if (ret)
+			return ret;
+
+		/*
+		 * On some platforms the number of active planes affects
+		 * the planes' minimum cdclk calculation. Add such planes
+		 * to the state before we compute the minimum cdclk.
+		 */
+		if (!active_planes_affects_min_cdclk(dev_priv))
+			continue;
+
+		old_active_planes = old_crtc_state->active_planes & ~BIT(PLANE_CURSOR);
+		new_active_planes = new_crtc_state->active_planes & ~BIT(PLANE_CURSOR);
+
+		if (hweight8(old_active_planes) == hweight8(new_active_planes))
+			continue;
+
+		ret = intel_crtc_add_planes_to_state(state, crtc, new_active_planes);
+		if (ret)
+			return ret;
+	}
+
+	/*
+	 * active_planes bitmask has been updated, and potentially
+	 * affected planes are part of the state. We can now
+	 * compute the minimum cdclk for each plane.
+	 */
+	for_each_new_intel_plane_in_state(state, plane, plane_state, i)
+		*need_modeset |= intel_plane_calc_min_cdclk(state, plane);
+
 	return 0;
 }
 
@@ -13629,22 +13747,16 @@ static int intel_atomic_check(struct drm_device *dev,
 
 	any_ms |= state->cdclk.force_min_cdclk_changed;
 
+	ret = intel_atomic_check_planes(state, &any_ms);
+	if (ret)
+		goto fail;
+
 	if (any_ms) {
 		ret = intel_modeset_checks(state);
 		if (ret)
 			goto fail;
-	} else {
-		state->cdclk.logical = dev_priv->cdclk.logical;
 	}
 
-	ret = icl_add_linked_planes(state);
-	if (ret)
-		goto fail;
-
-	ret = intel_atomic_check_planes(state);
-	if (ret)
-		goto fail;
-
 	ret = intel_atomic_check_crtcs(state);
 	if (ret)
 		goto fail;
@@ -14919,6 +15031,15 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 		plane->get_hw_state = i9xx_plane_get_hw_state;
 		plane->check_plane = i9xx_plane_check;
 
+		if (IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
+			plane->min_cdclk = hsw_plane_min_cdclk;
+		else if (IS_IVYBRIDGE(dev_priv))
+			plane->min_cdclk = ivb_plane_min_cdclk;
+		else if (IS_CHERRYVIEW(dev_priv) || IS_VALLEYVIEW(dev_priv))
+			plane->min_cdclk = vlv_plane_min_cdclk;
+		else
+			plane->min_cdclk = i9xx_plane_min_cdclk;
+
 		plane_funcs = &i965_plane_funcs;
 	} else {
 		formats = i8xx_primary_formats;
@@ -14930,6 +15051,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 		plane->disable_plane = i9xx_disable_plane;
 		plane->get_hw_state = i9xx_plane_get_hw_state;
 		plane->check_plane = i9xx_plane_check;
+		plane->min_cdclk = i9xx_plane_min_cdclk;
 
 		plane_funcs = &i8xx_plane_funcs;
 	}
@@ -16832,19 +16954,11 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 
 			intel_crtc_compute_pixel_rate(crtc_state);
 
-			min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
-			if (WARN_ON(min_cdclk < 0))
-				min_cdclk = 0;
-
 			drm_calc_timestamping_constants(&crtc->base,
 							&crtc_state->base.adjusted_mode);
 			update_scanline_offset(crtc_state);
 		}
 
-		dev_priv->min_cdclk[crtc->pipe] = min_cdclk;
-		dev_priv->min_voltage_level[crtc->pipe] =
-			crtc_state->min_voltage_level;
-
 		for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
 			const struct intel_plane_state *plane_state =
 				to_intel_plane_state(plane->base.state);
@@ -16856,8 +16970,34 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			if (plane_state->base.visible)
 				crtc_state->data_rate[plane->id] =
 					4 * crtc_state->pixel_rate;
+			/*
+			 * FIXME don't have the fb yet, so can't
+			 * use plane->min_cdclk() :(
+			 */
+			if (plane_state->base.visible && plane->min_cdclk) {
+				if (crtc_state->double_wide ||
+				    INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+					crtc_state->min_cdclk[plane->id] =
+						DIV_ROUND_UP(crtc_state->pixel_rate, 2);
+				else
+					crtc_state->min_cdclk[plane->id] =
+						crtc_state->pixel_rate;
+			}
+			DRM_DEBUG_KMS("[PLANE:%d:%s] min_cdclk %d kHz\n",
+				      plane->base.base.id, plane->base.name,
+				      crtc_state->min_cdclk[plane->id]);
+		}
+
+		if (crtc_state->base.active) {
+			min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
+			if (WARN_ON(min_cdclk < 0))
+				min_cdclk = 0;
 		}
 
+		dev_priv->min_cdclk[crtc->pipe] = min_cdclk;
+		dev_priv->min_voltage_level[crtc->pipe] =
+			crtc_state->min_voltage_level;
+
 		intel_bw_crtc_update(bw_state, crtc_state);
 
 		intel_pipe_config_sanity_check(dev_priv, crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 01fed8957ade..6a50c074a5c3 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -940,6 +940,8 @@ struct intel_crtc_state {
 
 	struct intel_crtc_wm_state wm;
 
+	int min_cdclk[I915_MAX_PLANES];
+
 	u32 data_rate[I915_MAX_PLANES];
 
 	/* Gamma mode programmed on the pipe */
@@ -1079,6 +1081,8 @@ struct intel_plane {
 	bool (*get_hw_state)(struct intel_plane *plane, enum pipe *pipe);
 	int (*check_plane)(struct intel_crtc_state *crtc_state,
 			   struct intel_plane_state *plane_state);
+	int (*min_cdclk)(const struct intel_crtc_state *crtc_state,
+			 const struct intel_plane_state *plane_state);
 };
 
 struct intel_watermark_params {
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index 5ae12ab3c5b7..469f79b01114 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -322,6 +322,55 @@ bool icl_is_hdr_plane(struct drm_i915_private *dev_priv, enum plane_id plane_id)
 		icl_hdr_plane_mask() & BIT(plane_id);
 }
 
+static void
+skl_plane_ratio(const struct intel_crtc_state *crtc_state,
+		const struct intel_plane_state *plane_state,
+		unsigned int *num, unsigned int *den)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane_state->base.plane->dev);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+
+	if (fb->format->cpp[0] == 8) {
+		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
+			*num = 10;
+			*den = 8;
+		} else {
+			*num = 9;
+			*den = 8;
+		}
+	} else {
+		*num = 1;
+		*den = 1;
+	}
+}
+
+static int skl_plane_min_cdclk(const struct intel_crtc_state *crtc_state,
+			       const struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane_state->base.plane->dev);
+	unsigned int pixel_rate = crtc_state->pixel_rate;
+	unsigned int src_w, src_h, dst_w, dst_h;
+	unsigned int num, den;
+
+	skl_plane_ratio(crtc_state, plane_state, &num, &den);
+
+	/* two pixels per clock on glk+ */
+	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+		den *= 2;
+
+	src_w = drm_rect_width(&plane_state->base.src) >> 16;
+	src_h = drm_rect_height(&plane_state->base.src) >> 16;
+	dst_w = drm_rect_width(&plane_state->base.dst);
+	dst_h = drm_rect_height(&plane_state->base.dst);
+
+	/* Downscaling limits the maximum pixel rate */
+	dst_w = min(src_w, dst_w);
+	dst_h = min(src_h, dst_h);
+
+	return DIV64_U64_ROUND_UP(mul_u32_u32(pixel_rate * num, src_w * src_h),
+				  mul_u32_u32(den, dst_w * dst_h));
+}
+
 static unsigned int
 skl_plane_max_stride(struct intel_plane *plane,
 		     u32 pixel_format, u64 modifier,
@@ -811,6 +860,85 @@ vlv_update_clrc(const struct intel_plane_state *plane_state)
 		      SP_SH_SIN(sh_sin) | SP_SH_COS(sh_cos));
 }
 
+static void
+vlv_plane_ratio(const struct intel_crtc_state *crtc_state,
+		const struct intel_plane_state *plane_state,
+		unsigned int *num, unsigned int *den)
+{
+	u8 active_planes = crtc_state->active_planes & ~BIT(PLANE_CURSOR);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	unsigned int cpp = fb->format->cpp[0];
+
+	/*
+	 * VLV bspec only considers cases where all three planes are
+	 * enabled, and cases where the primary and one sprite is enabled.
+	 * Let's assume the case with just two sprites enabled also
+	 * maps to the latter case.
+	 */
+	if (hweight8(active_planes) == 3) {
+		switch (cpp) {
+		case 8:
+			*num = 11;
+			*den = 8;
+			break;
+		case 4:
+			*num = 18;
+			*den = 16;
+			break;
+		default:
+			*num = 1;
+			*den = 1;
+			break;
+		}
+	} else if (hweight8(active_planes) == 2) {
+		switch (cpp) {
+		case 8:
+			*num = 10;
+			*den = 8;
+			break;
+		case 4:
+			*num = 17;
+			*den = 16;
+			break;
+		default:
+			*num = 1;
+			*den = 1;
+			break;
+		}
+	} else {
+		switch (cpp) {
+		case 8:
+			*num = 10;
+			*den = 8;
+			break;
+		default:
+			*num = 1;
+			*den = 1;
+			break;
+		}
+	}
+}
+
+int vlv_plane_min_cdclk(const struct intel_crtc_state *crtc_state,
+			const struct intel_plane_state *plane_state)
+{
+	unsigned int pixel_rate;
+	unsigned int num, den;
+
+	/*
+	 * Note that crtc_state->pixel_rate accounts for both
+	 * horizontal and vertical panel fitter downscaling factors.
+	 * Pre-HSW bspec tells us to only consider the horizontal
+	 * downscaling factor here. We ignore that and just consider
+	 * both for simplicity.
+	 */
+	pixel_rate = crtc_state->pixel_rate;
+
+	vlv_plane_ratio(crtc_state, plane_state, &num, &den);
+
+	return DIV_ROUND_UP(pixel_rate * num, den);
+}
+
 static u32 vlv_sprite_ctl_crtc(const struct intel_crtc_state *crtc_state)
 {
 	u32 sprctl = 0;
@@ -1017,6 +1145,164 @@ vlv_plane_get_hw_state(struct intel_plane *plane,
 	return ret;
 }
 
+static void ivb_plane_ratio(const struct intel_crtc_state *crtc_state,
+			    const struct intel_plane_state *plane_state,
+			    unsigned int *num, unsigned int *den)
+{
+	u8 active_planes = crtc_state->active_planes & ~BIT(PLANE_CURSOR);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	unsigned int cpp = fb->format->cpp[0];
+
+	if (hweight8(active_planes) == 2) {
+		switch (cpp) {
+		case 8:
+			*num = 10;
+			*den = 8;
+			break;
+		case 4:
+			*num = 17;
+			*den = 16;
+			break;
+		default:
+			*num = 1;
+			*den = 1;
+			break;
+		}
+	} else {
+		switch (cpp) {
+		case 8:
+			*num = 9;
+			*den = 8;
+			break;
+		default:
+			*num = 1;
+			*den = 1;
+			break;
+		}
+	}
+}
+
+static void ivb_plane_ratio_scaling(const struct intel_crtc_state *crtc_state,
+				    const struct intel_plane_state *plane_state,
+				    unsigned int *num, unsigned int *den)
+{
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	unsigned int cpp = fb->format->cpp[0];
+
+	switch (cpp) {
+	case 8:
+		*num = 12;
+		*den = 8;
+		break;
+	case 4:
+		*num = 19;
+		*den = 16;
+		break;
+	case 2:
+		*num = 33;
+		*den = 32;
+		break;
+	default:
+		*num = 1;
+		*den = 1;
+		break;
+	}
+}
+
+int ivb_plane_min_cdclk(const struct intel_crtc_state *crtc_state,
+			const struct intel_plane_state *plane_state)
+{
+	unsigned int pixel_rate;
+	unsigned int num, den;
+
+	/*
+	 * Note that crtc_state->pixel_rate accounts for both
+	 * horizontal and vertical panel fitter downscaling factors.
+	 * Pre-HSW bspec tells us to only consider the horizontal
+	 * downscaling factor here. We ignore that and just consider
+	 * both for simplicity.
+	 */
+	pixel_rate = crtc_state->pixel_rate;
+
+	ivb_plane_ratio(crtc_state, plane_state, &num, &den);
+
+	return DIV_ROUND_UP(pixel_rate * num, den);
+}
+
+static int ivb_sprite_min_cdclk(const struct intel_crtc_state *crtc_state,
+				const struct intel_plane_state *plane_state)
+{
+	unsigned int src_w, dst_w, pixel_rate;
+	unsigned int num, den;
+
+	/*
+	 * Note that crtc_state->pixel_rate accounts for both
+	 * horizontal and vertical panel fitter downscaling factors.
+	 * Pre-HSW bspec tells us to only consider the horizontal
+	 * downscaling factor here. We ignore that and just consider
+	 * both for simplicity.
+	 */
+	pixel_rate = crtc_state->pixel_rate;
+
+	src_w = drm_rect_width(&plane_state->base.src) >> 16;
+	dst_w = drm_rect_width(&plane_state->base.dst);
+
+	if (src_w != dst_w)
+		ivb_plane_ratio_scaling(crtc_state, plane_state, &num, &den);
+	else
+		ivb_plane_ratio(crtc_state, plane_state, &num, &den);
+
+	/* Horizontal downscaling limits the maximum pixel rate */
+	dst_w = min(src_w, dst_w);
+
+	return DIV_ROUND_UP_ULL(mul_u32_u32(pixel_rate, num * src_w),
+				den * dst_w);
+}
+
+static void hsw_plane_ratio(const struct intel_crtc_state *crtc_state,
+			    const struct intel_plane_state *plane_state,
+			    unsigned int *num, unsigned int *den)
+{
+	u8 active_planes = crtc_state->active_planes & ~BIT(PLANE_CURSOR);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	unsigned int cpp = fb->format->cpp[0];
+
+	if (hweight8(active_planes) == 2) {
+		switch (cpp) {
+		case 8:
+			*num = 10;
+			*den = 8;
+			break;
+		default:
+			*num = 1;
+			*den = 1;
+			break;
+		}
+	} else {
+		switch (cpp) {
+		case 8:
+			*num = 9;
+			*den = 8;
+			break;
+		default:
+			*num = 1;
+			*den = 1;
+			break;
+		}
+	}
+}
+
+int hsw_plane_min_cdclk(const struct intel_crtc_state *crtc_state,
+			const struct intel_plane_state *plane_state)
+{
+	unsigned int pixel_rate = crtc_state->pixel_rate;
+	unsigned int num, den;
+
+	hsw_plane_ratio(crtc_state, plane_state, &num, &den);
+
+	return DIV_ROUND_UP(pixel_rate * num, den);
+}
+
 static u32 ivb_sprite_ctl_crtc(const struct intel_crtc_state *crtc_state)
 {
 	u32 sprctl = 0;
@@ -1243,6 +1529,53 @@ ivb_plane_get_hw_state(struct intel_plane *plane,
 	return ret;
 }
 
+static int g4x_sprite_min_cdclk(const struct intel_crtc_state *crtc_state,
+				const struct intel_plane_state *plane_state)
+{
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	unsigned int hscale, pixel_rate;
+	unsigned int limit, decimate;
+
+	/*
+	 * Note that crtc_state->pixel_rate accounts for both
+	 * horizontal and vertical panel fitter downscaling factors.
+	 * Pre-HSW bspec tells us to only consider the horizontal
+	 * downscaling factor here. We ignore that and just consider
+	 * both for simplicity.
+	 */
+	pixel_rate = crtc_state->pixel_rate;
+
+	/* Horizontal downscaling limits the maximum pixel rate */
+	hscale = drm_rect_calc_hscale(&plane_state->base.src,
+				      &plane_state->base.dst,
+				      0, INT_MAX);
+	if (hscale < 0x10000)
+		return pixel_rate;
+
+	/* Decimation steps at 2x,4x,8x,16x */
+	decimate = ilog2(hscale >> 16);
+	hscale >>= decimate;
+
+	/* Starting limit is 90% of cdclk */
+	limit = 9;
+
+	/* -10% per decimation step */
+	limit -= decimate;
+
+	/* -10% for RGB */
+	if (fb->format->cpp[0] >= 4)
+		limit--; /* -10% for RGB */
+
+	/*
+	 * We should also do -10% if sprite scaling is enabled
+	 * on the other pipe, but we can't really check for that,
+	 * so we ignore it.
+	 */
+
+	return DIV_ROUND_UP_ULL(mul_u32_u32(pixel_rate, 10 * hscale),
+				limit << 16);
+}
+
 static unsigned int
 g4x_sprite_max_stride(struct intel_plane *plane,
 		      u32 pixel_format, u64 modifier,
@@ -2511,6 +2844,7 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
 	plane->disable_plane = skl_disable_plane;
 	plane->get_hw_state = skl_plane_get_hw_state;
 	plane->check_plane = skl_plane_check;
+	plane->min_cdclk = skl_plane_min_cdclk;
 	if (icl_is_nv12_y_plane(plane_id))
 		plane->update_slave = icl_update_slave;
 
@@ -2618,6 +2952,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 		plane->disable_plane = vlv_disable_plane;
 		plane->get_hw_state = vlv_plane_get_hw_state;
 		plane->check_plane = vlv_sprite_check;
+		plane->min_cdclk = vlv_plane_min_cdclk;
 
 		formats = vlv_plane_formats;
 		num_formats = ARRAY_SIZE(vlv_plane_formats);
@@ -2631,6 +2966,11 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 		plane->get_hw_state = ivb_plane_get_hw_state;
 		plane->check_plane = g4x_sprite_check;
 
+		if (IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
+			plane->min_cdclk = hsw_plane_min_cdclk;
+		else
+			plane->min_cdclk = ivb_sprite_min_cdclk;
+
 		formats = snb_plane_formats;
 		num_formats = ARRAY_SIZE(snb_plane_formats);
 		modifiers = i9xx_plane_format_modifiers;
@@ -2642,6 +2982,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 		plane->disable_plane = g4x_disable_plane;
 		plane->get_hw_state = g4x_plane_get_hw_state;
 		plane->check_plane = g4x_sprite_check;
+		plane->min_cdclk = g4x_sprite_min_cdclk;
 
 		modifiers = i9xx_plane_format_modifiers;
 		if (IS_GEN(dev_priv, 6)) {
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.h b/drivers/gpu/drm/i915/display/intel_sprite.h
index 229336214f68..5eeaa92420d1 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.h
+++ b/drivers/gpu/drm/i915/display/intel_sprite.h
@@ -49,4 +49,11 @@ static inline u8 icl_hdr_plane_mask(void)
 
 bool icl_is_hdr_plane(struct drm_i915_private *dev_priv, enum plane_id plane_id);
 
+int ivb_plane_min_cdclk(const struct intel_crtc_state *crtc_state,
+			const struct intel_plane_state *plane_state);
+int hsw_plane_min_cdclk(const struct intel_crtc_state *crtc_state,
+			const struct intel_plane_state *plane_state);
+int vlv_plane_min_cdclk(const struct intel_crtc_state *crtc_state,
+			const struct intel_plane_state *plane_state);
+
 #endif /* __INTEL_SPRITE_H__ */
-- 
2.21.0

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

[PATCH v2 05/13] drm/i915: Eliminate skl_check_pipe_max_pixel_rate()

[Ville Syrjala]

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

The normal cdclk handling now takes care of making sure the
plane's pixel rate doesn't exceed the spec appointed percentage
of the cdclk frequency. Thus we can nuke
skl_check_pipe_max_pixel_rate().

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c |  2 -
 drivers/gpu/drm/i915/intel_pm.c              | 87 --------------------
 drivers/gpu/drm/i915/intel_pm.h              |  2 -
 3 files changed, 91 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 67bb5fa03b2b..cbc807ea08c5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -11887,8 +11887,6 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state,
 	if (INTEL_GEN(dev_priv) >= 9) {
 		if (mode_changed || crtc_state->update_pipe)
 			ret = skl_update_scaler_crtc(crtc_state);
-		if (!ret)
-			ret = skl_check_pipe_max_pixel_rate(crtc, crtc_state);
 		if (!ret)
 			ret = intel_atomic_setup_scalers(dev_priv, crtc,
 							 crtc_state);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b306e2338f5a..643b6b65fba8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4096,93 +4096,6 @@ skl_plane_downscale_amount(const struct intel_crtc_state *crtc_state,
 	return mul_fixed16(downscale_w, downscale_h);
 }
 
-static uint_fixed_16_16_t
-skl_pipe_downscale_amount(const struct intel_crtc_state *crtc_state)
-{
-	uint_fixed_16_16_t pipe_downscale = u32_to_fixed16(1);
-
-	if (!crtc_state->base.enable)
-		return pipe_downscale;
-
-	if (crtc_state->pch_pfit.enabled) {
-		u32 src_w, src_h, dst_w, dst_h;
-		u32 pfit_size = crtc_state->pch_pfit.size;
-		uint_fixed_16_16_t fp_w_ratio, fp_h_ratio;
-		uint_fixed_16_16_t downscale_h, downscale_w;
-
-		src_w = crtc_state->pipe_src_w;
-		src_h = crtc_state->pipe_src_h;
-		dst_w = pfit_size >> 16;
-		dst_h = pfit_size & 0xffff;
-
-		if (!dst_w || !dst_h)
-			return pipe_downscale;
-
-		fp_w_ratio = div_fixed16(src_w, dst_w);
-		fp_h_ratio = div_fixed16(src_h, dst_h);
-		downscale_w = max_fixed16(fp_w_ratio, u32_to_fixed16(1));
-		downscale_h = max_fixed16(fp_h_ratio, u32_to_fixed16(1));
-
-		pipe_downscale = mul_fixed16(downscale_w, downscale_h);
-	}
-
-	return pipe_downscale;
-}
-
-int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
-				  struct intel_crtc_state *crtc_state)
-{
-	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
-	struct drm_atomic_state *state = crtc_state->base.state;
-	const struct intel_plane_state *plane_state;
-	struct intel_plane *plane;
-	int crtc_clock, dotclk;
-	u32 pipe_max_pixel_rate;
-	uint_fixed_16_16_t pipe_downscale;
-	uint_fixed_16_16_t max_downscale = u32_to_fixed16(1);
-
-	if (!crtc_state->base.enable)
-		return 0;
-
-	intel_atomic_crtc_state_for_each_plane_state(plane, plane_state, crtc_state) {
-		uint_fixed_16_16_t plane_downscale;
-		uint_fixed_16_16_t fp_9_div_8 = div_fixed16(9, 8);
-		int bpp;
-
-		if (!intel_wm_plane_visible(crtc_state, plane_state))
-			continue;
-
-		if (WARN_ON(!plane_state->base.fb))
-			return -EINVAL;
-
-		plane_downscale = skl_plane_downscale_amount(crtc_state, plane_state);
-		bpp = plane_state->base.fb->format->cpp[0] * 8;
-		if (bpp == 64)
-			plane_downscale = mul_fixed16(plane_downscale,
-						      fp_9_div_8);
-
-		max_downscale = max_fixed16(plane_downscale, max_downscale);
-	}
-	pipe_downscale = skl_pipe_downscale_amount(crtc_state);
-
-	pipe_downscale = mul_fixed16(pipe_downscale, max_downscale);
-
-	crtc_clock = crtc_state->base.adjusted_mode.crtc_clock;
-	dotclk = to_intel_atomic_state(state)->cdclk.logical.cdclk;
-
-	if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
-		dotclk *= 2;
-
-	pipe_max_pixel_rate = div_round_up_u32_fixed16(dotclk, pipe_downscale);
-
-	if (pipe_max_pixel_rate < crtc_clock) {
-		DRM_DEBUG_KMS("Max supported pixel clock with scaling exceeded\n");
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static u64
 skl_plane_relative_data_rate(const struct intel_crtc_state *crtc_state,
 			     const struct intel_plane_state *plane_state,
diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
index 93d192d0610a..00a5801dfc06 100644
--- a/drivers/gpu/drm/i915/intel_pm.h
+++ b/drivers/gpu/drm/i915/intel_pm.h
@@ -64,8 +64,6 @@ void skl_write_plane_wm(struct intel_plane *plane,
 void skl_write_cursor_wm(struct intel_plane *plane,
 			 const struct intel_crtc_state *crtc_state);
 bool ilk_disable_lp_wm(struct drm_device *dev);
-int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
-				  struct intel_crtc_state *cstate);
 void intel_init_ipc(struct drm_i915_private *dev_priv);
 void intel_enable_ipc(struct drm_i915_private *dev_priv);
 
-- 
2.21.0

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

[PATCH v2 06/13] drm/i915: Simplify skl_max_scale()

[Ville Syrjala]

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

Now that the planes declare their minimum cdclk requirements properly
we don't need to check the cdclk in skl_max_scale() anymore. Just check
against the maximum downscale ratio, and move the code next to it's
only caller.

v2: Add a comment explaining the HQ vs. not thing

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 38 --------------------
 drivers/gpu/drm/i915/display/intel_display.h |  2 --
 drivers/gpu/drm/i915/display/intel_sprite.c  | 18 +++++++++-
 3 files changed, 17 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index cbc807ea08c5..e31a8af808f5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14707,44 +14707,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
 	intel_plane_unpin_fb(old_plane_state);
 }
 
-int
-skl_max_scale(const struct intel_crtc_state *crtc_state,
-	      const struct drm_format_info *format)
-{
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	int max_scale;
-	int crtc_clock, max_dotclk, tmpclk1, tmpclk2;
-
-	if (!crtc_state->base.enable)
-		return DRM_PLANE_HELPER_NO_SCALING;
-
-	crtc_clock = crtc_state->base.adjusted_mode.crtc_clock;
-	max_dotclk = to_intel_atomic_state(crtc_state->base.state)->cdclk.logical.cdclk;
-
-	if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
-		max_dotclk *= 2;
-
-	if (WARN_ON_ONCE(!crtc_clock || max_dotclk < crtc_clock))
-		return DRM_PLANE_HELPER_NO_SCALING;
-
-	/*
-	 * skl max scale is lower of:
-	 *    close to 3 but not 3, -1 is for that purpose
-	 *            or
-	 *    cdclk/crtc_clock
-	 */
-	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) ||
-	    !drm_format_info_is_yuv_semiplanar(format))
-		tmpclk1 = 0x30000 - 1;
-	else
-		tmpclk1 = 0x20000 - 1;
-	tmpclk2 = (1 << 8) * ((max_dotclk << 8) / crtc_clock);
-	max_scale = min(tmpclk1, tmpclk2);
-
-	return max_scale;
-}
-
 /**
  * intel_plane_destroy - destroy a plane
  * @plane: plane to destroy
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index bed203ec1df8..c7ec8984f007 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -558,8 +558,6 @@ void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,
 
 u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_center);
 int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
-int skl_max_scale(const struct intel_crtc_state *crtc_state,
-		  const struct drm_format_info *format);
 u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
 			const struct intel_plane_state *plane_state);
 u32 glk_plane_color_ctl_crtc(const struct intel_crtc_state *crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index 469f79b01114..6b2eaf26700c 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -2120,6 +2120,22 @@ static int skl_plane_check_nv12_rotation(const struct intel_plane_state *plane_s
 	return 0;
 }
 
+static int skl_plane_max_scale(struct drm_i915_private *dev_priv,
+			       const struct drm_framebuffer *fb)
+{
+	/*
+	 * We don't yet know the final source width nor
+	 * whether we can use the HQ scaler mode. Assume
+	 * the best case.
+	 * FIXME need to properly check this later.
+	 */
+	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) ||
+	    !drm_format_info_is_yuv_semiplanar(fb->format))
+		return 0x30000 - 1;
+	else
+		return 0x20000 - 1;
+}
+
 static int skl_plane_check(struct intel_crtc_state *crtc_state,
 			   struct intel_plane_state *plane_state)
 {
@@ -2137,7 +2153,7 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state,
 	/* use scaler when colorkey is not required */
 	if (!plane_state->ckey.flags && intel_fb_scalable(fb)) {
 		min_scale = 1;
-		max_scale = skl_max_scale(crtc_state, fb->format);
+		max_scale = skl_plane_max_scale(dev_priv, fb);
 	}
 
 	ret = drm_atomic_helper_check_plane_state(&plane_state->base,
-- 
2.21.0

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

[PATCH v2 07/13] drm/i915: Add support for half float framebuffers for skl+

[Ville Syrjala]

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

skl+ supports fp16 pixel formats on all universal planes. Add the
necessary bits to expose that capability. The main different to
icl is that we can't scale fp16, so need to add the relevant
checks.

v2: Rebase on top of icl fp16
    Split skl+ bits into a separate patch

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 11 +++++++----
 drivers/gpu/drm/i915/display/intel_sprite.c  | 11 +++++++++++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e31a8af808f5..64e1e5e933e0 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5577,10 +5577,6 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 	case DRM_FORMAT_ARGB8888:
 	case DRM_FORMAT_XRGB2101010:
 	case DRM_FORMAT_XBGR2101010:
-	case DRM_FORMAT_XBGR16161616F:
-	case DRM_FORMAT_ABGR16161616F:
-	case DRM_FORMAT_XRGB16161616F:
-	case DRM_FORMAT_ARGB16161616F:
 	case DRM_FORMAT_YUYV:
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_UYVY:
@@ -5596,6 +5592,13 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 	case DRM_FORMAT_XVYU12_16161616:
 	case DRM_FORMAT_XVYU16161616:
 		break;
+	case DRM_FORMAT_XBGR16161616F:
+	case DRM_FORMAT_ABGR16161616F:
+	case DRM_FORMAT_XRGB16161616F:
+	case DRM_FORMAT_ARGB16161616F:
+		if (INTEL_GEN(dev_priv) >= 11)
+			break;
+		/* fall through */
 	default:
 		DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format 0x%x\n",
 			      intel_plane->base.base.id, intel_plane->base.name,
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index 6b2eaf26700c..e8442299989b 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -1832,6 +1832,11 @@ static bool intel_fb_scalable(const struct drm_framebuffer *fb)
 	switch (fb->format->format) {
 	case DRM_FORMAT_C8:
 		return false;
+	case DRM_FORMAT_XRGB16161616F:
+	case DRM_FORMAT_ARGB16161616F:
+	case DRM_FORMAT_XBGR16161616F:
+	case DRM_FORMAT_ABGR16161616F:
+		return INTEL_GEN(to_i915(fb->dev)) >= 11;
 	default:
 		return true;
 	}
@@ -2359,6 +2364,8 @@ static const u32 skl_plane_formats[] = {
 	DRM_FORMAT_ABGR8888,
 	DRM_FORMAT_XRGB2101010,
 	DRM_FORMAT_XBGR2101010,
+	DRM_FORMAT_XRGB16161616F,
+	DRM_FORMAT_XBGR16161616F,
 	DRM_FORMAT_YUYV,
 	DRM_FORMAT_YVYU,
 	DRM_FORMAT_UYVY,
@@ -2374,6 +2381,8 @@ static const u32 skl_planar_formats[] = {
 	DRM_FORMAT_ABGR8888,
 	DRM_FORMAT_XRGB2101010,
 	DRM_FORMAT_XBGR2101010,
+	DRM_FORMAT_XRGB16161616F,
+	DRM_FORMAT_XBGR16161616F,
 	DRM_FORMAT_YUYV,
 	DRM_FORMAT_YVYU,
 	DRM_FORMAT_UYVY,
@@ -2390,6 +2399,8 @@ static const u32 glk_planar_formats[] = {
 	DRM_FORMAT_ABGR8888,
 	DRM_FORMAT_XRGB2101010,
 	DRM_FORMAT_XBGR2101010,
+	DRM_FORMAT_XRGB16161616F,
+	DRM_FORMAT_XBGR16161616F,
 	DRM_FORMAT_YUYV,
 	DRM_FORMAT_YVYU,
 	DRM_FORMAT_UYVY,
-- 
2.21.0

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

[PATCH v2 08/13] drm/i915: Add support for half float framebuffers for gen4+ primary planes

[Ville Syrjala]

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

gen4+ supports fp16 pixel formats on the primary planes. Add the
relevant code.

On ivb fp16 scanout is slightly busted. The output from the plane will
have 1/4 the expected value. For the primary plane we would have to
use the pipe gamma or pipe csc to correct that which would affect all
the other planes as well, hence we simply choose not to expose fp16
on the ivb primary plane. On hsw the primary plane got fixed.

On gmch platforms I observed that the plane width must be below 2k
pixels with fp16 or else we get a corrupted image. This limitation
does not seem to be documented in bspec. I verified the exact limit
using the chv pipe B primary plane since it has windowing capability.
The stride limits are unaffected by fp16.

v2: Rebase on top of icl fp16
    Split thea gen4+ primary plane bits into a separate patch
    Deal with HAS_GMCH()

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 49 ++++++++++++++++++--
 1 file changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 64e1e5e933e0..2a77d5d5051a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -88,7 +88,17 @@ static const u32 i8xx_primary_formats[] = {
 	DRM_FORMAT_XRGB8888,
 };
 
-/* Primary plane formats for gen >= 4 */
+/* Primary plane formats for ivb (no fp16 due to hw issue) */
+static const u32 ivb_primary_formats[] = {
+	DRM_FORMAT_C8,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_XRGB2101010,
+	DRM_FORMAT_XBGR2101010,
+};
+
+/* Primary plane formats for gen >= 4, except ivb */
 static const u32 i965_primary_formats[] = {
 	DRM_FORMAT_C8,
 	DRM_FORMAT_RGB565,
@@ -96,6 +106,7 @@ static const u32 i965_primary_formats[] = {
 	DRM_FORMAT_XBGR8888,
 	DRM_FORMAT_XRGB2101010,
 	DRM_FORMAT_XBGR2101010,
+	DRM_FORMAT_XBGR16161616F,
 };
 
 static const u64 i9xx_format_modifiers[] = {
@@ -2957,6 +2968,8 @@ static int i9xx_format_to_fourcc(int format)
 		return DRM_FORMAT_XRGB2101010;
 	case DISPPLANE_RGBX101010:
 		return DRM_FORMAT_XBGR2101010;
+	case DISPPLANE_RGBX161616:
+		return DRM_FORMAT_XBGR16161616F;
 	}
 }
 
@@ -3693,6 +3706,9 @@ static u32 i9xx_plane_ctl(const struct intel_crtc_state *crtc_state,
 	case DRM_FORMAT_XBGR2101010:
 		dspcntr |= DISPPLANE_RGBX101010;
 		break;
+	case DRM_FORMAT_XBGR16161616F:
+		dspcntr |= DISPPLANE_RGBX161616;
+		break;
 	default:
 		MISSING_CASE(fb->format->format);
 		return 0;
@@ -3715,7 +3731,8 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(plane_state->base.plane->dev);
-	int src_x, src_y;
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+	int src_x, src_y, src_w;
 	u32 offset;
 	int ret;
 
@@ -3726,9 +3743,14 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
 	if (!plane_state->base.visible)
 		return 0;
 
+	src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	src_x = plane_state->base.src.x1 >> 16;
 	src_y = plane_state->base.src.y1 >> 16;
 
+	/* Undocumented hardware limit on i965/g4x/vlv/chv */
+	if (HAS_GMCH(dev_priv) && fb->format->cpp[0] == 8 && src_w > 2048)
+		return -EINVAL;
+
 	intel_add_fb_offsets(&src_x, &src_y, plane_state, 0);
 
 	if (INTEL_GEN(dev_priv) >= 4)
@@ -14764,6 +14786,7 @@ static bool i965_plane_format_mod_supported(struct drm_plane *_plane,
 	case DRM_FORMAT_XBGR8888:
 	case DRM_FORMAT_XRGB2101010:
 	case DRM_FORMAT_XBGR2101010:
+	case DRM_FORMAT_XBGR16161616F:
 		return modifier == DRM_FORMAT_MOD_LINEAR ||
 			modifier == I915_FORMAT_MOD_X_TILED;
 	default:
@@ -14984,8 +15007,26 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 	}
 
 	if (INTEL_GEN(dev_priv) >= 4) {
-		formats = i965_primary_formats;
-		num_formats = ARRAY_SIZE(i965_primary_formats);
+		/*
+		 * WaFP16GammaEnabling:ivb
+		 * "Workaround : When using the 64-bit format, the plane
+		 *  output on each color channel has one quarter amplitude.
+		 *  It can be brought up to full amplitude by using pipe
+		 *  gamma correction or pipe color space conversion to
+		 *  multiply the plane output by four."
+		 *
+		 * There is no dedicated plane gamma for the primary plane,
+		 * and using the pipe gamma/csc could conflict with other
+		 * planes, so we choose not to expose fp16 on IVB primary
+		 * planes. HSW primary planes no longer have this problem.
+		 */
+		if (IS_IVYBRIDGE(dev_priv)) {
+			formats = ivb_primary_formats;
+			num_formats = ARRAY_SIZE(ivb_primary_formats);
+		} else {
+			formats = i965_primary_formats;
+			num_formats = ARRAY_SIZE(i965_primary_formats);
+		}
 		modifiers = i9xx_format_modifiers;
 
 		plane->max_stride = i9xx_plane_max_stride;
-- 
2.21.0

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

[PATCH v2 09/13] drm/i915: Add support for half float framebuffers for ivb+ sprites

[Ville Syrjala]

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

ivb+ supports fp16 pixel formats on the sprite planes planes. Expose
that capability.

On ivb/hsw fp16 scanout is slightly busted. The output from the plane
will have 1/4 the expected value. For the sprite plane we can fix that
up with the plane gamma unit. This was fixed on bdw.

v2: Rebase on top of icl fp16
    Split the ivb+ sprite birs into a separate patch
v3: Move ivb_need_sprite_gamma() check one level up so that
    we don't waste time programming garbage into he gamma registers

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_sprite.c | 48 ++++++++++++++++++---
 1 file changed, 42 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index e8442299989b..db1e2dce6636 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -1316,6 +1316,16 @@ static u32 ivb_sprite_ctl_crtc(const struct intel_crtc_state *crtc_state)
 	return sprctl;
 }
 
+static bool ivb_need_sprite_gamma(const struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
+	const struct drm_framebuffer *fb = plane_state->base.fb;
+
+	return fb->format->cpp[0] == 8 &&
+		(IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv));
+}
+
 static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
 			  const struct intel_plane_state *plane_state)
 {
@@ -1338,6 +1348,12 @@ static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
 	case DRM_FORMAT_XRGB8888:
 		sprctl |= SPRITE_FORMAT_RGBX888;
 		break;
+	case DRM_FORMAT_XBGR16161616F:
+		sprctl |= SPRITE_FORMAT_RGBX161616 | SPRITE_RGB_ORDER_RGBX;
+		break;
+	case DRM_FORMAT_XRGB16161616F:
+		sprctl |= SPRITE_FORMAT_RGBX161616;
+		break;
 	case DRM_FORMAT_YUYV:
 		sprctl |= SPRITE_FORMAT_YUV422 | SPRITE_YUV_ORDER_YUYV;
 		break;
@@ -1355,7 +1371,8 @@ static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
 		return 0;
 	}
 
-	sprctl |= SPRITE_INT_GAMMA_DISABLE;
+	if (!ivb_need_sprite_gamma(plane_state))
+		sprctl |= SPRITE_INT_GAMMA_DISABLE;
 
 	if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
 		sprctl |= SPRITE_YUV_TO_RGB_CSC_FORMAT_BT709;
@@ -1377,12 +1394,26 @@ static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
 	return sprctl;
 }
 
-static void ivb_sprite_linear_gamma(u16 gamma[18])
+static void ivb_sprite_linear_gamma(const struct intel_plane_state *plane_state,
+				    u16 gamma[18])
 {
-	int i;
+	int scale, i;
 
-	for (i = 0; i < 17; i++)
-		gamma[i] = (i << 10) / 16;
+	/*
+	 * WaFP16GammaEnabling:ivb,hsw
+	 * "Workaround : When using the 64-bit format, the sprite output
+	 *  on each color channel has one quarter amplitude. It can be
+	 *  brought up to full amplitude by using sprite internal gamma
+	 *  correction, pipe gamma correction, or pipe color space
+	 *  conversion to multiply the sprite output by four."
+	 */
+	scale = 4;
+
+	for (i = 0; i < 16; i++)
+		gamma[i] = min((scale * i << 10) / 16, (1 << 10) - 1);
+
+	gamma[i] = min((scale * i << 10) / 16, 1 << 10);
+	i++;
 
 	gamma[i] = 3 << 10;
 	i++;
@@ -1396,7 +1427,10 @@ static void ivb_update_gamma(const struct intel_plane_state *plane_state)
 	u16 gamma[18];
 	int i;
 
-	ivb_sprite_linear_gamma(gamma);
+	if (!ivb_need_sprite_gamma(plane_state))
+		return;
+
+	ivb_sprite_linear_gamma(plane_state, gamma);
 
 	/* FIXME these register are single buffered :( */
 	for (i = 0; i < 16; i++)
@@ -2551,6 +2585,8 @@ static bool snb_sprite_format_mod_supported(struct drm_plane *_plane,
 	switch (format) {
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_XRGB16161616F:
+	case DRM_FORMAT_XBGR16161616F:
 	case DRM_FORMAT_YUYV:
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_UYVY:
-- 
2.21.0

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

[PATCH v2 10/13] drm/i915: Add support for half float framebuffers on snb sprites

[Ville Syrjala]

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

snb supports fp16 pixel formats on the sprite planes. Expose that
capability. Nothing special needs to be done, it just works.

v2: Rebase on top of icl fp16
    Split snb+ sprite bits into a separate patch

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_sprite.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index db1e2dce6636..edc41fc40726 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -1653,6 +1653,12 @@ static u32 g4x_sprite_ctl(const struct intel_crtc_state *crtc_state,
 	case DRM_FORMAT_XRGB8888:
 		dvscntr |= DVS_FORMAT_RGBX888;
 		break;
+	case DRM_FORMAT_XBGR16161616F:
+		dvscntr |= DVS_FORMAT_RGBX161616 | DVS_RGB_ORDER_XBGR;
+		break;
+	case DRM_FORMAT_XRGB16161616F:
+		dvscntr |= DVS_FORMAT_RGBX161616;
+		break;
 	case DRM_FORMAT_YUYV:
 		dvscntr |= DVS_FORMAT_YUV422 | DVS_YUV_ORDER_YUYV;
 		break;
@@ -2367,8 +2373,10 @@ static const u64 i9xx_plane_format_modifiers[] = {
 };
 
 static const u32 snb_plane_formats[] = {
-	DRM_FORMAT_XBGR8888,
 	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_XRGB16161616F,
+	DRM_FORMAT_XBGR16161616F,
 	DRM_FORMAT_YUYV,
 	DRM_FORMAT_YVYU,
 	DRM_FORMAT_UYVY,
-- 
2.21.0

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

[PATCH v2 11/13] drm/i915: Move more cdclk state handling into intel_modeset_calc_cdclk()

[Ville Syrjala]

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

Encapsulate the cdclk state handling a bit better by performing
the copy from dev_priv->cdclk into the current intel_atomic_state
within the cdclk code.

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

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 0caef2592a7e..27addbd35d9c 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -2325,6 +2325,13 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
 	enum pipe pipe;
 	int ret;
 
+	/* keep the current setting */
+	if (!state->cdclk.force_min_cdclk_changed)
+		state->cdclk.force_min_cdclk = dev_priv->cdclk.force_min_cdclk;
+
+	state->cdclk.logical = dev_priv->cdclk.logical;
+	state->cdclk.actual = dev_priv->cdclk.actual;
+
 	ret = dev_priv->display.modeset_calc_cdclk(state);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 2a77d5d5051a..94fec0958f39 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -13525,14 +13525,8 @@ static int intel_modeset_checks(struct intel_atomic_state *state)
 	struct intel_crtc *crtc;
 	int ret, i;
 
-	/* keep the current setting */
-	if (!state->cdclk.force_min_cdclk_changed)
-		state->cdclk.force_min_cdclk = dev_priv->cdclk.force_min_cdclk;
-
 	state->modeset = true;
 	state->active_pipes = dev_priv->active_pipes;
-	state->cdclk.logical = dev_priv->cdclk.logical;
-	state->cdclk.actual = dev_priv->cdclk.actual;
 
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
-- 
2.21.0

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

[PATCH v2 12/13] drm/i915: Consolidate more cdclk state handling

[Ville Syrjala]

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

Move the initial setup of state->min_cdclk[]/min_voltage_level[]
into intel_modeset_calc_cdclk() alongside the rest of the global
cdclk state. And the counterparts we move into
intel_cdclk_swap_state().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c   | 19 +++++++++++++------
 drivers/gpu/drm/i915/display/intel_display.c |  5 -----
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 27addbd35d9c..f6e2048dd2b8 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1817,6 +1817,14 @@ void intel_cdclk_swap_state(struct intel_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
 
+	/* FIXME maybe swap() these too */
+	memcpy(dev_priv->min_cdclk, state->min_cdclk,
+	       sizeof(state->min_cdclk));
+	memcpy(dev_priv->min_voltage_level, state->min_voltage_level,
+	       sizeof(state->min_voltage_level));
+
+	dev_priv->cdclk.force_min_cdclk = state->cdclk.force_min_cdclk;
+
 	swap(state->cdclk.logical, dev_priv->cdclk.logical);
 	swap(state->cdclk.actual, dev_priv->cdclk.actual);
 }
@@ -2019,9 +2027,6 @@ static int intel_compute_min_cdclk(struct intel_atomic_state *state)
 	int min_cdclk, i;
 	enum pipe pipe;
 
-	memcpy(state->min_cdclk, dev_priv->min_cdclk,
-	       sizeof(state->min_cdclk));
-
 	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
 		int ret;
 
@@ -2068,9 +2073,6 @@ static int bxt_compute_min_voltage_level(struct intel_atomic_state *state)
 	int i;
 	enum pipe pipe;
 
-	memcpy(state->min_voltage_level, dev_priv->min_voltage_level,
-	       sizeof(state->min_voltage_level));
-
 	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
 		int ret;
 
@@ -2325,6 +2327,11 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
 	enum pipe pipe;
 	int ret;
 
+	memcpy(state->min_cdclk, dev_priv->min_cdclk,
+	       sizeof(state->min_cdclk));
+	memcpy(state->min_voltage_level, dev_priv->min_voltage_level,
+	       sizeof(state->min_voltage_level));
+
 	/* keep the current setting */
 	if (!state->cdclk.force_min_cdclk_changed)
 		state->cdclk.force_min_cdclk = dev_priv->cdclk.force_min_cdclk;
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 94fec0958f39..8e607dad87d3 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14453,12 +14453,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 	if (state->global_state_changed) {
 		assert_global_state_locked(dev_priv);
 
-		memcpy(dev_priv->min_cdclk, state->min_cdclk,
-		       sizeof(state->min_cdclk));
-		memcpy(dev_priv->min_voltage_level, state->min_voltage_level,
-		       sizeof(state->min_voltage_level));
 		dev_priv->active_pipes = state->active_pipes;
-		dev_priv->cdclk.force_min_cdclk = state->cdclk.force_min_cdclk;
 
 		intel_cdclk_swap_state(state);
 	}
-- 
2.21.0

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

[PATCH v2 13/13] drm/i915: Collect more cdclk state under the same roof

[Ville Syrjala]

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

Move the min_cdclk[] and min_voltage_level[] arrays under the
rest of the cdclk state. And while at it provide a simple
helper (intel_cdclk_clear_state()) to clear the state during
the ww_mutex backoff dance.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_atomic.c   |  9 ++---
 drivers/gpu/drm/i915/display/intel_cdclk.c    | 40 ++++++++++++-------
 drivers/gpu/drm/i915/display/intel_cdclk.h    |  1 +
 drivers/gpu/drm/i915/display/intel_display.c  |  8 ++--
 .../drm/i915/display/intel_display_types.h    | 10 +++--
 drivers/gpu/drm/i915/i915_drv.h               |  9 +++--
 6 files changed, 46 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
index 9cd6d2348a1e..6b2cddb3c867 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -35,6 +35,7 @@
 #include <drm/drm_plane_helper.h>
 
 #include "intel_atomic.h"
+#include "intel_cdclk.h"
 #include "intel_display_types.h"
 #include "intel_hdcp.h"
 #include "intel_sprite.h"
@@ -427,15 +428,13 @@ intel_atomic_state_alloc(struct drm_device *dev)
 void intel_atomic_state_clear(struct drm_atomic_state *s)
 {
 	struct intel_atomic_state *state = to_intel_atomic_state(s);
+
 	drm_atomic_state_default_clear(&state->base);
+
 	state->dpll_set = state->modeset = false;
 	state->global_state_changed = false;
 	state->active_pipes = 0;
-	memset(&state->min_cdclk, 0, sizeof(state->min_cdclk));
-	memset(&state->min_voltage_level, 0, sizeof(state->min_voltage_level));
-	memset(&state->cdclk.logical, 0, sizeof(state->cdclk.logical));
-	memset(&state->cdclk.actual, 0, sizeof(state->cdclk.actual));
-	state->cdclk.pipe = INVALID_PIPE;
+	intel_cdclk_clear_state(state);
 }
 
 struct intel_crtc_state *
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index f6e2048dd2b8..41e2f0298d41 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1801,6 +1801,18 @@ static bool intel_cdclk_changed(const struct intel_cdclk_state *a,
 		a->voltage_level != b->voltage_level;
 }
 
+/**
+ * intel_cdclk_clear_state - clear the cdclk state
+ * @state: atomic state
+ *
+ * Clear the cdclk state for ww_mutex backoff.
+ */
+void intel_cdclk_clear_state(struct intel_atomic_state *state)
+{
+	memset(&state->cdclk, 0, sizeof(state->cdclk));
+	state->cdclk.pipe = INVALID_PIPE;
+}
+
 /**
  * intel_cdclk_swap_state - make atomic CDCLK configuration effective
  * @state: atomic state
@@ -1818,10 +1830,10 @@ void intel_cdclk_swap_state(struct intel_atomic_state *state)
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
 
 	/* FIXME maybe swap() these too */
-	memcpy(dev_priv->min_cdclk, state->min_cdclk,
-	       sizeof(state->min_cdclk));
-	memcpy(dev_priv->min_voltage_level, state->min_voltage_level,
-	       sizeof(state->min_voltage_level));
+	memcpy(dev_priv->cdclk.min_cdclk, state->cdclk.min_cdclk,
+	       sizeof(state->cdclk.min_cdclk));
+	memcpy(dev_priv->cdclk.min_voltage_level, state->cdclk.min_voltage_level,
+	       sizeof(state->cdclk.min_voltage_level));
 
 	dev_priv->cdclk.force_min_cdclk = state->cdclk.force_min_cdclk;
 
@@ -2034,10 +2046,10 @@ static int intel_compute_min_cdclk(struct intel_atomic_state *state)
 		if (min_cdclk < 0)
 			return min_cdclk;
 
-		if (state->min_cdclk[i] == min_cdclk)
+		if (state->cdclk.min_cdclk[i] == min_cdclk)
 			continue;
 
-		state->min_cdclk[i] = min_cdclk;
+		state->cdclk.min_cdclk[i] = min_cdclk;
 
 		ret = intel_atomic_lock_global_state(state);
 		if (ret)
@@ -2046,7 +2058,7 @@ static int intel_compute_min_cdclk(struct intel_atomic_state *state)
 
 	min_cdclk = state->cdclk.force_min_cdclk;
 	for_each_pipe(dev_priv, pipe)
-		min_cdclk = max(state->min_cdclk[pipe], min_cdclk);
+		min_cdclk = max(state->cdclk.min_cdclk[pipe], min_cdclk);
 
 	return min_cdclk;
 }
@@ -2081,10 +2093,10 @@ static int bxt_compute_min_voltage_level(struct intel_atomic_state *state)
 		else
 			min_voltage_level = 0;
 
-		if (state->min_voltage_level[i] == min_voltage_level)
+		if (state->cdclk.min_voltage_level[i] == min_voltage_level)
 			continue;
 
-		state->min_voltage_level[i] = min_voltage_level;
+		state->cdclk.min_voltage_level[i] = min_voltage_level;
 
 		ret = intel_atomic_lock_global_state(state);
 		if (ret)
@@ -2093,7 +2105,7 @@ static int bxt_compute_min_voltage_level(struct intel_atomic_state *state)
 
 	min_voltage_level = 0;
 	for_each_pipe(dev_priv, pipe)
-		min_voltage_level = max(state->min_voltage_level[pipe],
+		min_voltage_level = max(state->cdclk.min_voltage_level[pipe],
 					min_voltage_level);
 
 	return min_voltage_level;
@@ -2327,10 +2339,10 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
 	enum pipe pipe;
 	int ret;
 
-	memcpy(state->min_cdclk, dev_priv->min_cdclk,
-	       sizeof(state->min_cdclk));
-	memcpy(state->min_voltage_level, dev_priv->min_voltage_level,
-	       sizeof(state->min_voltage_level));
+	memcpy(state->cdclk.min_cdclk, dev_priv->cdclk.min_cdclk,
+	       sizeof(state->cdclk.min_cdclk));
+	memcpy(state->cdclk.min_voltage_level, dev_priv->cdclk.min_voltage_level,
+	       sizeof(state->cdclk.min_voltage_level));
 
 	/* keep the current setting */
 	if (!state->cdclk.force_min_cdclk_changed)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index cf71394cc79c..3f3773c582ae 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -31,6 +31,7 @@ void intel_update_cdclk(struct drm_i915_private *dev_priv);
 void intel_update_rawclk(struct drm_i915_private *dev_priv);
 bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
 			       const struct intel_cdclk_state *b);
+void intel_cdclk_clear_state(struct intel_atomic_state *state);
 void intel_cdclk_swap_state(struct intel_atomic_state *state);
 void
 intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 8e607dad87d3..e4caa775b502 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7122,8 +7122,8 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
 	intel_crtc->enabled_power_domains = 0;
 
 	dev_priv->active_pipes &= ~BIT(intel_crtc->pipe);
-	dev_priv->min_cdclk[intel_crtc->pipe] = 0;
-	dev_priv->min_voltage_level[intel_crtc->pipe] = 0;
+	dev_priv->cdclk.min_cdclk[intel_crtc->pipe] = 0;
+	dev_priv->cdclk.min_voltage_level[intel_crtc->pipe] = 0;
 
 	bw_state->data_rate[intel_crtc->pipe] = 0;
 	bw_state->num_active_planes[intel_crtc->pipe] = 0;
@@ -16987,8 +16987,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 				min_cdclk = 0;
 		}
 
-		dev_priv->min_cdclk[crtc->pipe] = min_cdclk;
-		dev_priv->min_voltage_level[crtc->pipe] =
+		dev_priv->cdclk.min_cdclk[crtc->pipe] = min_cdclk;
+		dev_priv->cdclk.min_voltage_level[crtc->pipe] =
 			crtc_state->min_voltage_level;
 
 		intel_bw_crtc_update(bw_state, crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 6a50c074a5c3..70ed1d498a06 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -474,6 +474,12 @@ struct intel_atomic_state {
 
 		int force_min_cdclk;
 		bool force_min_cdclk_changed;
+
+		/* minimum acceptable cdclk for each pipe */
+		int min_cdclk[I915_MAX_PIPES];
+		/* minimum acceptable voltage level for each pipe */
+		u8 min_voltage_level[I915_MAX_PIPES];
+
 		/* pipe to which cd2x update is synchronized */
 		enum pipe pipe;
 	} cdclk;
@@ -491,10 +497,6 @@ struct intel_atomic_state {
 	u8 active_pipe_changes;
 
 	u8 active_pipes;
-	/* minimum acceptable cdclk for each pipe */
-	int min_cdclk[I915_MAX_PIPES];
-	/* minimum acceptable voltage level for each pipe */
-	u8 min_voltage_level[I915_MAX_PIPES];
 
 	struct intel_shared_dpll_state shared_dpll[I915_NUM_PLLS];
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d3e61152d924..cb4bae641ac3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1127,6 +1127,11 @@ struct drm_i915_private {
 		const struct intel_cdclk_vals *table;
 
 		int force_min_cdclk;
+
+		/* minimum acceptable cdclk for each pipe */
+		int min_cdclk[I915_MAX_PIPES];
+		/* minimum acceptable voltage level for each pipe */
+		u8 min_voltage_level[I915_MAX_PIPES];
 	} cdclk;
 
 	/**
@@ -1187,10 +1192,6 @@ struct drm_i915_private {
 	 * any crtc lock is sufficient, for writing must hold all of them.
 	 */
 	u8 active_pipes;
-	/* minimum acceptable cdclk for each pipe */
-	int min_cdclk[I915_MAX_PIPES];
-	/* minimum acceptable voltage level for each pipe */
-	u8 min_voltage_level[I915_MAX_PIPES];
 
 	int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
 
-- 
2.21.0

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

✗ Fi.CI.CHECKPATCH: warning for drm/i915: Plane cdclk requirements and fp16 for gen4+ (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Plane cdclk requirements and fp16 for gen4+ (rev2)
URL   : https://patchwork.freedesktop.org/series/63373/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
d780d2521e9c drm/i915: Add debugs to distingiush a cd2x update from a full cdclk pll update
8f6777bc8259 drm/i915: Rework global state locking
592968317718 drm/i915: Move check_digital_port_conflicts() earier
3ce3e1bdf217 drm/i915: Allow planes to declare their minimum acceptable cdclk
-:316: CHECK:SPACING: spaces preferred around that '*' (ctx:ExV)
#316: FILE: drivers/gpu/drm/i915/display/intel_display.c:13668:
+		*need_modeset |= intel_plane_calc_min_cdclk(state, plane);
 		^

total: 0 errors, 0 warnings, 1 checks, 770 lines checked
b21fdbb98b1d drm/i915: Eliminate skl_check_pipe_max_pixel_rate()
17fd610dc415 drm/i915: Simplify skl_max_scale()
58c4817d7539 drm/i915: Add support for half float framebuffers for skl+
2689307aa01f drm/i915: Add support for half float framebuffers for gen4+ primary planes
10d0783bf6fc drm/i915: Add support for half float framebuffers for ivb+ sprites
7fb22d625662 drm/i915: Add support for half float framebuffers on snb sprites
5ba5da60c0ef drm/i915: Move more cdclk state handling into intel_modeset_calc_cdclk()
45ef641ea5c7 drm/i915: Consolidate more cdclk state handling
667e88eaa390 drm/i915: Collect more cdclk state under the same roof

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

✓ Fi.CI.BAT: success for drm/i915: Plane cdclk requirements and fp16 for gen4+ (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Plane cdclk requirements and fp16 for gen4+ (rev2)
URL   : https://patchwork.freedesktop.org/series/63373/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7099 -> Patchwork_14821
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724]) +3 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/fi-icl-u3/igt@gem_exec_suspend@basic-s4-devices.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/fi-icl-u3/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-blb-e6850:       [PASS][3] -> [INCOMPLETE][4] ([fdo#107718])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  
#### Possible fixes ####

  * igt@debugfs_test@read_all_entries:
    - {fi-tgl-u}:         [INCOMPLETE][5] -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/fi-tgl-u/igt@debugfs_test@read_all_entries.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/fi-tgl-u/igt@debugfs_test@read_all_entries.html

  * igt@gem_basic@bad-close:
    - fi-skl-6770hq:      [DMESG-WARN][7] ([fdo#105541]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/fi-skl-6770hq/igt@gem_basic@bad-close.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/fi-skl-6770hq/igt@gem_basic@bad-close.html

  * igt@gem_ctx_switch@legacy-render:
    - fi-bxt-dsi:         [INCOMPLETE][9] ([fdo#103927] / [fdo#111381]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/fi-bxt-dsi/igt@gem_ctx_switch@legacy-render.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/fi-bxt-dsi/igt@gem_ctx_switch@legacy-render.html

  * igt@gem_ringfill@basic-default-fd:
    - fi-icl-u3:          [DMESG-WARN][11] ([fdo#107724]) -> [PASS][12] +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/fi-icl-u3/igt@gem_ringfill@basic-default-fd.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/fi-icl-u3/igt@gem_ringfill@basic-default-fd.html

  * igt@gem_wait@basic-busy-all:
    - {fi-tgl-u2}:        [INCOMPLETE][13] -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/fi-tgl-u2/igt@gem_wait@basic-busy-all.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/fi-tgl-u2/igt@gem_wait@basic-busy-all.html

  * igt@i915_selftest@live_gem_contexts:
    - {fi-icl-dsi}:       [DMESG-FAIL][15] -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/fi-icl-dsi/igt@i915_selftest@live_gem_contexts.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/fi-icl-dsi/igt@i915_selftest@live_gem_contexts.html

  * igt@kms_busy@basic-flip-a:
    - fi-icl-u2:          [TIMEOUT][17] ([fdo#111800]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/fi-icl-u2/igt@kms_busy@basic-flip-a.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/fi-icl-u2/igt@kms_busy@basic-flip-a.html

  * igt@prime_self_import@basic-llseek-bad:
    - {fi-icl-dsi}:       [DMESG-WARN][19] ([fdo#106107]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/fi-icl-dsi/igt@prime_self_import@basic-llseek-bad.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/fi-icl-dsi/igt@prime_self_import@basic-llseek-bad.html

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

  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105541]: https://bugs.freedesktop.org/show_bug.cgi?id=105541
  [fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111381]: https://bugs.freedesktop.org/show_bug.cgi?id=111381
  [fdo#111800]: https://bugs.freedesktop.org/show_bug.cgi?id=111800


Participating hosts (52 -> 46)
------------------------------

  Additional (1): fi-hsw-peppy 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7099 -> Patchwork_14821

  CI-20190529: 20190529
  CI_DRM_7099: fccd0abc9c05536751c60aabe5710c173fb8ffa6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5231: e293051f8f99c72cb01d21e4b73a5928ea351eb3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14821: 667e88eaa3902f08de041f3198b6c36b31772489 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

667e88eaa390 drm/i915: Collect more cdclk state under the same roof
45ef641ea5c7 drm/i915: Consolidate more cdclk state handling
5ba5da60c0ef drm/i915: Move more cdclk state handling into intel_modeset_calc_cdclk()
7fb22d625662 drm/i915: Add support for half float framebuffers on snb sprites
10d0783bf6fc drm/i915: Add support for half float framebuffers for ivb+ sprites
2689307aa01f drm/i915: Add support for half float framebuffers for gen4+ primary planes
58c4817d7539 drm/i915: Add support for half float framebuffers for skl+
17fd610dc415 drm/i915: Simplify skl_max_scale()
b21fdbb98b1d drm/i915: Eliminate skl_check_pipe_max_pixel_rate()
3ce3e1bdf217 drm/i915: Allow planes to declare their minimum acceptable cdclk
592968317718 drm/i915: Move check_digital_port_conflicts() earier
8f6777bc8259 drm/i915: Rework global state locking
d780d2521e9c drm/i915: Add debugs to distingiush a cd2x update from a full cdclk pll update

== Logs ==

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

✗ Fi.CI.IGT: failure for drm/i915: Plane cdclk requirements and fp16 for gen4+ (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Plane cdclk requirements and fp16 for gen4+ (rev2)
URL   : https://patchwork.freedesktop.org/series/63373/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7099_full -> Patchwork_14821_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_14821_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_14821_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_14821_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_persistent_relocs@forked-thrashing:
    - shard-apl:          NOTRUN -> [DMESG-WARN][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-apl3/igt@gem_persistent_relocs@forked-thrashing.html

  
#### Suppressed ####

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

  * {igt@kms_cursor_crc@pipe-d-cursor-256x256-onscreen}:
    - {shard-tglb}:       NOTRUN -> [FAIL][2] +3 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-tglb1/igt@kms_cursor_crc@pipe-d-cursor-256x256-onscreen.html

  * {igt@kms_cursor_crc@pipe-d-cursor-512x512-sliding}:
    - {shard-tglb}:       NOTRUN -> [SKIP][3] +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-tglb7/igt@kms_cursor_crc@pipe-d-cursor-512x512-sliding.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-apl:          [PASS][4] -> [DMESG-WARN][5] ([fdo#108566]) +3 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-apl6/igt@gem_ctx_isolation@rcs0-s3.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-apl5/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][6] -> [SKIP][7] ([fdo#110841])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-iclb6/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-iclb1/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_eio@in-flight-contexts-immediate:
    - shard-snb:          [PASS][8] -> [FAIL][9] ([fdo#111925])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-snb7/igt@gem_eio@in-flight-contexts-immediate.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-snb5/igt@gem_eio@in-flight-contexts-immediate.html

  * igt@gem_exec_schedule@preempt-other-chain-bsd:
    - shard-iclb:         [PASS][10] -> [SKIP][11] ([fdo#111325]) +6 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-iclb6/igt@gem_exec_schedule@preempt-other-chain-bsd.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-iclb1/igt@gem_exec_schedule@preempt-other-chain-bsd.html

  * igt@kms_cursor_crc@pipe-b-cursor-128x42-sliding:
    - shard-kbl:          [PASS][12] -> [FAIL][13] ([fdo#103232])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-kbl2/igt@kms_cursor_crc@pipe-b-cursor-128x42-sliding.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-kbl2/igt@kms_cursor_crc@pipe-b-cursor-128x42-sliding.html

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
    - shard-iclb:         [PASS][14] -> [INCOMPLETE][15] ([fdo#107713])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-iclb6/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-iclb7/igt@kms_cursor_crc@pipe-b-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-skl:          [PASS][16] -> [INCOMPLETE][17] ([fdo#110741])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-skl9/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-skl7/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [PASS][18] -> [FAIL][19] ([fdo#105363])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-skl7/igt@kms_flip@flip-vs-expired-vblank.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-skl9/igt@kms_flip@flip-vs-expired-vblank.html
    - shard-glk:          [PASS][20] -> [FAIL][21] ([fdo#105363])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-glk7/igt@kms_flip@flip-vs-expired-vblank.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-glk5/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_flip@plain-flip-fb-recreate:
    - shard-skl:          [PASS][22] -> [FAIL][23] ([fdo#100368])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-skl7/igt@kms_flip@plain-flip-fb-recreate.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-skl10/igt@kms_flip@plain-flip-fb-recreate.html

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - shard-iclb:         [PASS][24] -> [FAIL][25] ([fdo#103167]) +4 similar issues
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][26] -> [FAIL][27] ([fdo#103166])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_sprite_blt:
    - shard-iclb:         [PASS][28] -> [SKIP][29] ([fdo#109441])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-iclb2/igt@kms_psr@psr2_sprite_blt.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-iclb7/igt@kms_psr@psr2_sprite_blt.html

  * igt@kms_setmode@basic:
    - shard-skl:          [PASS][30] -> [FAIL][31] ([fdo#99912])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-skl7/igt@kms_setmode@basic.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-skl10/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-a-wait-forked-hang:
    - shard-apl:          [PASS][32] -> [INCOMPLETE][33] ([fdo#103927]) +1 similar issue
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-apl8/igt@kms_vblank@pipe-a-wait-forked-hang.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-apl5/igt@kms_vblank@pipe-a-wait-forked-hang.html

  * igt@prime_busy@after-bsd2:
    - shard-iclb:         [PASS][34] -> [SKIP][35] ([fdo#109276]) +18 similar issues
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-iclb2/igt@prime_busy@after-bsd2.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-iclb7/igt@prime_busy@after-bsd2.html

  
#### Possible fixes ####

  * igt@gem_exec_schedule@wide-bsd:
    - shard-iclb:         [SKIP][36] ([fdo#111325]) -> [PASS][37] +3 similar issues
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-iclb2/igt@gem_exec_schedule@wide-bsd.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-iclb7/igt@gem_exec_schedule@wide-bsd.html

  * igt@gem_linear_blits@interruptible:
    - shard-apl:          [INCOMPLETE][38] ([fdo#103927]) -> [PASS][39] +1 similar issue
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-apl6/igt@gem_linear_blits@interruptible.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-apl3/igt@gem_linear_blits@interruptible.html

  * igt@gem_userptr_blits@dmabuf-sync:
    - shard-snb:          [DMESG-WARN][40] ([fdo#111870]) -> [PASS][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-snb1/igt@gem_userptr_blits@dmabuf-sync.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-snb1/igt@gem_userptr_blits@dmabuf-sync.html

  * igt@gem_userptr_blits@sync-unmap-after-close:
    - shard-glk:          [DMESG-WARN][42] ([fdo#111870]) -> [PASS][43]
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-glk7/igt@gem_userptr_blits@sync-unmap-after-close.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-glk3/igt@gem_userptr_blits@sync-unmap-after-close.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [DMESG-WARN][44] ([fdo#108566]) -> [PASS][45] +5 similar issues
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-apl5/igt@i915_suspend@sysfs-reader.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-apl2/igt@i915_suspend@sysfs-reader.html

  * igt@kms_big_fb@linear-64bpp-rotate-180:
    - shard-apl:          [SKIP][46] ([fdo#109271]) -> [PASS][47] +4 similar issues
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-apl6/igt@kms_big_fb@linear-64bpp-rotate-180.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-apl5/igt@kms_big_fb@linear-64bpp-rotate-180.html
    - shard-glk:          [SKIP][48] ([fdo#109271]) -> [PASS][49] +5 similar issues
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-glk9/igt@kms_big_fb@linear-64bpp-rotate-180.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-glk6/igt@kms_big_fb@linear-64bpp-rotate-180.html

  * igt@kms_big_fb@x-tiled-64bpp-rotate-0:
    - shard-snb:          [SKIP][50] ([fdo#109271]) -> [PASS][51] +3 similar issues
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-snb6/igt@kms_big_fb@x-tiled-64bpp-rotate-0.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-snb6/igt@kms_big_fb@x-tiled-64bpp-rotate-0.html

  * igt@kms_big_fb@x-tiled-64bpp-rotate-180:
    - shard-skl:          [SKIP][52] ([fdo#109271]) -> [PASS][53] +3 similar issues
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-skl5/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-skl8/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html

  * igt@kms_big_fb@y-tiled-64bpp-rotate-0:
    - shard-kbl:          [SKIP][54] ([fdo#109271]) -> [PASS][55] +5 similar issues
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-kbl7/igt@kms_big_fb@y-tiled-64bpp-rotate-0.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-kbl2/igt@kms_big_fb@y-tiled-64bpp-rotate-0.html

  * igt@kms_cursor_crc@pipe-b-cursor-128x42-sliding:
    - shard-apl:          [FAIL][56] ([fdo#103232]) -> [PASS][57]
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-apl3/igt@kms_cursor_crc@pipe-b-cursor-128x42-sliding.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-apl4/igt@kms_cursor_crc@pipe-b-cursor-128x42-sliding.html

  * igt@kms_cursor_legacy@cursora-vs-flipa-legacy:
    - shard-iclb:         [INCOMPLETE][58] ([fdo#107713]) -> [PASS][59]
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-iclb7/igt@kms_cursor_legacy@cursora-vs-flipa-legacy.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-iclb7/igt@kms_cursor_legacy@cursora-vs-flipa-legacy.html

  * igt@kms_cursor_legacy@flip-vs-cursor-toggle:
    - shard-skl:          [FAIL][60] ([fdo#102670] / [fdo#106081]) -> [PASS][61]
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-skl3/igt@kms_cursor_legacy@flip-vs-cursor-toggle.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-skl3/igt@kms_cursor_legacy@flip-vs-cursor-toggle.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [FAIL][62] ([fdo#105363]) -> [PASS][63]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-skl3/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-skl5/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@modeset-vs-vblank-race:
    - shard-glk:          [FAIL][64] ([fdo#111609]) -> [PASS][65]
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-glk1/igt@kms_flip@modeset-vs-vblank-race.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-glk1/igt@kms_flip@modeset-vs-vblank-race.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [FAIL][66] ([fdo#103167]) -> [PASS][67] +3 similar issues
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
    - shard-skl:          [FAIL][68] ([fdo#103191]) -> [PASS][69]
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-skl5/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-skl7/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [FAIL][70] ([fdo#108145]) -> [PASS][71]
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-skl5/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-skl7/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         [SKIP][72] ([fdo#109441]) -> [PASS][73] +1 similar issue
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-iclb1/igt@kms_psr@psr2_no_drrs.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-iclb2/igt@kms_psr@psr2_no_drrs.html

  * igt@kms_vblank@pipe-d-ts-continuation-dpms-suspend:
    - {shard-tglb}:       [INCOMPLETE][74] ([fdo#111850]) -> [PASS][75]
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-tglb7/igt@kms_vblank@pipe-d-ts-continuation-dpms-suspend.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-tglb4/igt@kms_vblank@pipe-d-ts-continuation-dpms-suspend.html

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         [SKIP][76] ([fdo#109276]) -> [PASS][77] +11 similar issues
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-iclb8/igt@prime_vgem@fence-wait-bsd2.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-iclb1/igt@prime_vgem@fence-wait-bsd2.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv:
    - shard-iclb:         [FAIL][78] ([fdo#111329]) -> [SKIP][79] ([fdo#109276])
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-iclb1/igt@gem_ctx_isolation@vcs1-nonpriv.html
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-iclb8/igt@gem_ctx_isolation@vcs1-nonpriv.html

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [FAIL][80] ([fdo#111330]) -> [SKIP][81] ([fdo#109276])
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7099/shard-iclb4/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14821/shard-iclb3/igt@gem_mocs_settings@mocs-reset-bsd2.html

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

  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#102670]: https://bugs.freedesktop.org/show_bug.cgi?id=102670
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#106081]: https://bugs.freedesktop.org/show_bug.cgi?id=106081
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [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#110548]: https://bugs.freedesktop.org/show_bug.cgi?id=110548
  [fdo#110741]: https://bugs.freedesktop.org/show_bug.cgi?id=110741
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111329]: https://bugs.freedesktop.org/show_bug.cgi?id=111329
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
  [fdo#111609]: https://bugs.freedesktop.org/show_bug.cgi?id=111609
  [fdo#111714]: https://bugs.freedesktop.org/show_bug.cgi?id=111714
  [fdo#111795 ]: https://bugs.freedesktop.org/show_bug.cgi?id=111795 
  [fdo#111832]: https://bugs.freedesktop.org/show_bug.cgi?id=111832
  [fdo#111850]: https://bugs.freedesktop.org/show_bug.cgi?id=111850
  [fdo#111870]: https://bugs.freedesktop.org/show_bug.cgi?id=111870
  [fdo#111925]: https://bugs.freedesktop.org/show_bug.cgi?id=111925
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


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

  Additional (1): pig-skl-6260u 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7099 -> Patchwork_14821

  CI-20190529: 20190529
  CI_DRM_7099: fccd0abc9c05536751c60aabe5710c173fb8ffa6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5231: e293051f8f99c72cb01d21e4b73a5928ea351eb3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14821: 667e88eaa3902f08de041f3198b6c36b31772489 @ 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_14821/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 02/13] drm/i915: Rework global state locking

[Lisovskiy, Stanislav]

On Tue, 2019-10-15 at 22:30 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> So far we've sort of protected the global state under dev_priv with
> the connection_mutex. I wan to change that so that we can change the
> cdclk even for pure plane updates. To that end let's formalize the
> protection of the global state to follow what I started with the
> cdclk
> code already (though not entirely properly) such that any crtc mutex
> will suffice as a read lock, and all crtcs mutexes act as the write
> lock.
> 
> We'll also pimp intel_atomic_state_clear() to clear the entire global
> state, so that we don't accidentally leak stale information between
> the locking retries.
> 
> As a slight optimization we'll only lock the crtc mutexes to protect
> the global state, however if and when we actually have to poke the
> hw (eg. if the actual cdclk changes) we must serialize commits
> across all crtcs so that a parallel nonblocking commit can't get
> ahead of the cdclk reprogamming. We do that by adding all crtcs to
> the state.
> 
> TODO: the old global state examined during commit may still
> be a problem since it always looks at the _latest_ swapped state
> in dev_priv. Need to add proper old/new state for that too I think.
> 
> v2: Remeber to serialize the commits if necessary
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/display/intel_atomic.c   |  44 ++++++++
>  drivers/gpu/drm/i915/display/intel_atomic.h   |   5 +
>  drivers/gpu/drm/i915/display/intel_audio.c    |  10 +-
>  drivers/gpu/drm/i915/display/intel_cdclk.c    | 102 ++++++++++----
> ----
>  drivers/gpu/drm/i915/display/intel_display.c  |  29 ++++-
>  .../drm/i915/display/intel_display_types.h    |   8 ++
>  drivers/gpu/drm/i915/i915_drv.h               |  11 +-
>  7 files changed, 153 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index c5a552a69752..9cd6d2348a1e 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -429,6 +429,13 @@ void intel_atomic_state_clear(struct
> drm_atomic_state *s)
>  	struct intel_atomic_state *state = to_intel_atomic_state(s);
>  	drm_atomic_state_default_clear(&state->base);
>  	state->dpll_set = state->modeset = false;
> +	state->global_state_changed = false;
> +	state->active_pipes = 0;
> +	memset(&state->min_cdclk, 0, sizeof(state->min_cdclk));
> +	memset(&state->min_voltage_level, 0, sizeof(state-
> >min_voltage_level));
> +	memset(&state->cdclk.logical, 0, sizeof(state->cdclk.logical));
> +	memset(&state->cdclk.actual, 0, sizeof(state->cdclk.actual));
> +	state->cdclk.pipe = INVALID_PIPE;
>  }
>  
>  struct intel_crtc_state *
> @@ -442,3 +449,40 @@ intel_atomic_get_crtc_state(struct
> drm_atomic_state *state,
>  
>  	return to_intel_crtc_state(crtc_state);
>  }
> +
> +int intel_atomic_lock_global_state(struct intel_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	struct intel_crtc *crtc;
> +
> +	state->global_state_changed = true;
> +
> +	for_each_intel_crtc(&dev_priv->drm, crtc) {
> +		int ret;
> +
> +		ret = drm_modeset_lock(&crtc->base.mutex,
> +				       state->base.acquire_ctx);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +int intel_atomic_serialize_global_state(struct intel_atomic_state
> *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	struct intel_crtc *crtc;
> +
> +	state->global_state_changed = true;
> +
> +	for_each_intel_crtc(&dev_priv->drm, crtc) {
> +		struct intel_crtc_state *crtc_state;
> +
> +		crtc_state = intel_atomic_get_crtc_state(&state->base,
> crtc);
> +		if (IS_ERR(crtc_state))
> +			return PTR_ERR(crtc_state);
> +	}
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.h
> b/drivers/gpu/drm/i915/display/intel_atomic.h
> index 58065d3161a3..49d5cb1b9e0a 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.h
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.h
> @@ -16,6 +16,7 @@ struct drm_crtc_state;
>  struct drm_device;
>  struct drm_i915_private;
>  struct drm_property;
> +struct intel_atomic_state;
>  struct intel_crtc;
>  struct intel_crtc_state;
>  
> @@ -46,4 +47,8 @@ int intel_atomic_setup_scalers(struct
> drm_i915_private *dev_priv,
>  			       struct intel_crtc *intel_crtc,
>  			       struct intel_crtc_state *crtc_state);
>  
> +int intel_atomic_lock_global_state(struct intel_atomic_state
> *state);
> +
> +int intel_atomic_serialize_global_state(struct intel_atomic_state
> *state);
> +
>  #endif /* __INTEL_ATOMIC_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
> b/drivers/gpu/drm/i915/display/intel_audio.c
> index ed18511befa3..85e6b2bbb34f 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -28,6 +28,7 @@
>  #include <drm/i915_component.h>
>  
>  #include "i915_drv.h"
> +#include "intel_atomic.h"
>  #include "intel_audio.h"
>  #include "intel_display_types.h"
>  #include "intel_lpe_audio.h"
> @@ -818,13 +819,8 @@ static void glk_force_audio_cdclk(struct
> drm_i915_private *dev_priv,
>  	to_intel_atomic_state(state)->cdclk.force_min_cdclk =
>  		enable ? 2 * 96000 : 0;
>  
> -	/*
> -	 * Protects dev_priv->cdclk.force_min_cdclk
> -	 * Need to lock this here in case we have no active pipes
> -	 * and thus wouldn't lock it during the commit otherwise.
> -	 */
> -	ret = drm_modeset_lock(&dev_priv-
> >drm.mode_config.connection_mutex,
> -			       &ctx);
> +	/* Protects dev_priv->cdclk.force_min_cdclk */
> +	ret =
> intel_atomic_lock_global_state(to_intel_atomic_state(state));
>  	if (!ret)
>  		ret = drm_atomic_commit(state);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 6eaebc38ee01..6c17a3bbf866 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -2007,11 +2007,20 @@ static int intel_compute_min_cdclk(struct
> intel_atomic_state *state)
>  	       sizeof(state->min_cdclk));
>  
>  	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
> +		int ret;
> +
>  		min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
>  		if (min_cdclk < 0)
>  			return min_cdclk;
>  
> +		if (state->min_cdclk[i] == min_cdclk)
> +			continue;
> +
>  		state->min_cdclk[i] = min_cdclk;
> +
> +		ret = intel_atomic_lock_global_state(state);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	min_cdclk = state->cdclk.force_min_cdclk;
> @@ -2034,7 +2043,7 @@ static int intel_compute_min_cdclk(struct
> intel_atomic_state *state)
>   * future platforms this code will need to be
>   * adjusted.
>   */
> -static u8 bxt_compute_min_voltage_level(struct intel_atomic_state
> *state)
> +static int bxt_compute_min_voltage_level(struct intel_atomic_state
> *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>  	struct intel_crtc *crtc;
> @@ -2047,11 +2056,21 @@ static u8
> bxt_compute_min_voltage_level(struct intel_atomic_state *state)
>  	       sizeof(state->min_voltage_level));
>  
>  	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
> +		int ret;
> +
>  		if (crtc_state->base.enable)
> -			state->min_voltage_level[i] =
> -				crtc_state->min_voltage_level;
> +			min_voltage_level = crtc_state-
> >min_voltage_level;
>  		else
> -			state->min_voltage_level[i] = 0;
> +			min_voltage_level = 0;
> +
> +		if (state->min_voltage_level[i] == min_voltage_level)
> +			continue;
> +
> +		state->min_voltage_level[i] = min_voltage_level;
> +
> +		ret = intel_atomic_lock_global_state(state);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	min_voltage_level = 0;
> @@ -2195,20 +2214,24 @@ static int skl_modeset_calc_cdclk(struct
> intel_atomic_state *state)
>  static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	int min_cdclk, cdclk, vco;
> +	int min_cdclk, min_voltage_level, cdclk, vco;
>  
>  	min_cdclk = intel_compute_min_cdclk(state);
>  	if (min_cdclk < 0)
>  		return min_cdclk;
>  
> +	min_voltage_level = bxt_compute_min_voltage_level(state);
> +	if (min_voltage_level < 0)
> +		return min_voltage_level;
> +
>  	cdclk = bxt_calc_cdclk(dev_priv, min_cdclk);
>  	vco = bxt_calc_cdclk_pll_vco(dev_priv, cdclk);
>  
>  	state->cdclk.logical.vco = vco;
>  	state->cdclk.logical.cdclk = cdclk;
>  	state->cdclk.logical.voltage_level =
> -		max(dev_priv->display.calc_voltage_level(cdclk),
> -		    bxt_compute_min_voltage_level(state));
> +		max_t(int, min_voltage_level,
> +		      dev_priv->display.calc_voltage_level(cdclk));
>  
>  	if (!state->active_pipes) {
>  		cdclk = bxt_calc_cdclk(dev_priv, state-
> >cdclk.force_min_cdclk);
> @@ -2225,23 +2248,6 @@ static int bxt_modeset_calc_cdclk(struct
> intel_atomic_state *state)
>  	return 0;
>  }
>  
> -static int intel_lock_all_pipes(struct intel_atomic_state *state)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	struct intel_crtc *crtc;
> -
> -	/* Add all pipes to the state */
> -	for_each_intel_crtc(&dev_priv->drm, crtc) {
> -		struct intel_crtc_state *crtc_state;
> -
> -		crtc_state = intel_atomic_get_crtc_state(&state->base,
> crtc);
> -		if (IS_ERR(crtc_state))
> -			return PTR_ERR(crtc_state);
> -	}
> -
> -	return 0;
> -}
> -
>  static int intel_modeset_all_pipes(struct intel_atomic_state *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> @@ -2308,46 +2314,56 @@ int intel_modeset_calc_cdclk(struct
> intel_atomic_state *state)
>  		return ret;
>  
>  	/*
> -	 * Writes to dev_priv->cdclk.logical must protected by
> -	 * holding all the crtc locks, even if we don't end up
> +	 * Writes to dev_priv->cdclk.{actual,logical} must protected
> +	 * by holding all the crtc mutexes even if we don't end up
>  	 * touching the hardware
>  	 */
> -	if (intel_cdclk_changed(&dev_priv->cdclk.logical,
> -				&state->cdclk.logical)) {
> -		ret = intel_lock_all_pipes(state);
> -		if (ret < 0)
> +	if (intel_cdclk_changed(&dev_priv->cdclk.actual,
> +				&state->cdclk.actual)) {
> +		/*
> +		 * Also serialize commits across all crtcs
> +		 * if the actual hw needs to be poked.
> +		 */
> +		ret = intel_atomic_serialize_global_state(state);
> +		if (ret)
> +			return ret;
> +	} else if (intel_cdclk_changed(&dev_priv->cdclk.logical,
> +				       &state->cdclk.logical)) {
> +		ret = intel_atomic_lock_global_state(state);
> +		if (ret)
>  			return ret;
> +	} else {
> +		return 0;
>  	}
>  
> -	if (is_power_of_2(state->active_pipes)) {
> +	if (is_power_of_2(state->active_pipes) &&
> +	    intel_cdclk_needs_cd2x_update(dev_priv,
> +					  &dev_priv->cdclk.actual,
> +					  &state->cdclk.actual)) {
>  		struct intel_crtc *crtc;
>  		struct intel_crtc_state *crtc_state;
>  
>  		pipe = ilog2(state->active_pipes);
>  		crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> -		crtc_state = intel_atomic_get_new_crtc_state(state,
> crtc);
> -		if (crtc_state &&
> -		    drm_atomic_crtc_needs_modeset(&crtc_state->base))
> +
> +		crtc_state = intel_atomic_get_crtc_state(&state->base,
> crtc);
> +		if (IS_ERR(crtc_state))
> +			return PTR_ERR(crtc_state);
> +
> +		if (drm_atomic_crtc_needs_modeset(&crtc_state->base))
>  			pipe = INVALID_PIPE;
>  	} else {
>  		pipe = INVALID_PIPE;
>  	}
>  
> -	/* All pipes must be switched off while we change the cdclk. */
> -	if (pipe != INVALID_PIPE &&
> -	    intel_cdclk_needs_cd2x_update(dev_priv,
> -					  &dev_priv->cdclk.actual,
> -					  &state->cdclk.actual)) {
> -		ret = intel_lock_all_pipes(state);
> -		if (ret)
> -			return ret;
> -
> +	if (pipe != INVALID_PIPE) {
>  		state->cdclk.pipe = pipe;
>  
>  		DRM_DEBUG_KMS("Can change cdclk with pipe %c active\n",
>  			      pipe_name(pipe));
>  	} else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
>  					     &state->cdclk.actual)) {
> +		/* All pipes must be switched off while we change the
> cdclk. */
>  		ret = intel_modeset_all_pipes(state);
>  		if (ret)
>  			return ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 7d7d1859775a..a8444d9841c1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -12191,6 +12191,12 @@ static bool
> check_digital_port_conflicts(struct intel_atomic_state *state)
>  	unsigned int used_mst_ports = 0;
>  	bool ret = true;
>  
> +	/*
> +	 * We're going to peek into connector->state,
> +	 * hence connection_mutex must be held.
> +	 */
> +	drm_modeset_lock_assert_held(&dev-
> >mode_config.connection_mutex);
> +
>  	/*
>  	 * Walk the connector list instead of the encoder
>  	 * list to detect the problem on ddi platforms
> @@ -13463,7 +13469,6 @@ static int intel_modeset_checks(struct
> intel_atomic_state *state)
>  	state->active_pipes = dev_priv->active_pipes;
>  	state->cdclk.logical = dev_priv->cdclk.logical;
>  	state->cdclk.actual = dev_priv->cdclk.actual;
> -	state->cdclk.pipe = INVALID_PIPE;
>  
>  	for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state,
>  					    new_crtc_state, i) {
> @@ -13476,6 +13481,12 @@ static int intel_modeset_checks(struct
> intel_atomic_state *state)
>  			state->active_pipe_changes |= BIT(crtc->pipe);
>  	}
>  
> +	if (state->active_pipe_changes) {
> +		ret = intel_atomic_lock_global_state(state);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	ret = intel_modeset_calc_cdclk(state);
>  	if (ret)
>  		return ret;
> @@ -13577,7 +13588,7 @@ static int intel_atomic_check(struct
> drm_device *dev,
>  	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
>  	struct intel_crtc *crtc;
>  	int ret, i;
> -	bool any_ms = state->cdclk.force_min_cdclk_changed;
> +	bool any_ms = false;
>  
>  	/* Catch I915_MODE_FLAG_INHERITED */
>  	for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state,
> @@ -13615,6 +13626,8 @@ static int intel_atomic_check(struct
> drm_device *dev,
>  	if (ret)
>  		goto fail;
>  
> +	any_ms |= state->cdclk.force_min_cdclk_changed;
> +
>  	if (any_ms) {
>  		ret = intel_modeset_checks(state);
>  		if (ret)
> @@ -14234,6 +14247,14 @@ static void intel_atomic_track_fbs(struct
> intel_atomic_state *state)
>  					plane->frontbuffer_bit);
>  }
>  
> +static void assert_global_state_locked(struct drm_i915_private
> *dev_priv)
> +{
> +	struct intel_crtc *crtc;
> +
> +	for_each_intel_crtc(&dev_priv->drm, crtc)
> +		drm_modeset_lock_assert_held(&crtc->base.mutex);
> +}
> +
>  static int intel_atomic_commit(struct drm_device *dev,
>  			       struct drm_atomic_state *_state,
>  			       bool nonblock)
> @@ -14299,7 +14320,9 @@ static int intel_atomic_commit(struct
> drm_device *dev,
>  	intel_shared_dpll_swap_state(state);
>  	intel_atomic_track_fbs(state);
>  
> -	if (state->modeset) {
> +	if (state->global_state_changed) {
> +		assert_global_state_locked(dev_priv);
> +
>  		memcpy(dev_priv->min_cdclk, state->min_cdclk,
>  		       sizeof(state->min_cdclk));
>  		memcpy(dev_priv->min_voltage_level, state-
> >min_voltage_level,
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 40390d855815..01fed8957ade 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -506,6 +506,14 @@ struct intel_atomic_state {
>  
>  	bool rps_interactive;
>  
> +	/*
> +	 * active_pipes
> +	 * min_cdclk[]
> +	 * min_voltage_level[]
> +	 * cdclk.*
> +	 */
> +	bool global_state_changed;
> +
>  	/* Gen9+ only */
>  	struct skl_ddb_values wm_results;
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index c46b339064c0..d3e61152d924 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1105,13 +1105,14 @@ struct drm_i915_private {
>  	unsigned int fdi_pll_freq;
>  	unsigned int czclk_freq;
>  
> +	/*
> +	 * For reading holding any crtc lock is sufficient,
> +	 * for writing must hold all of them.
> +	 */
>  	struct {
>  		/*
>  		 * The current logical cdclk state.
>  		 * See intel_atomic_state.cdclk.logical
> -		 *
> -		 * For reading holding any crtc lock is sufficient,
> -		 * for writing must hold all of them.
>  		 */
>  		struct intel_cdclk_state logical;
>  		/*
> @@ -1181,6 +1182,10 @@ struct drm_i915_private {
>  	 */
>  	struct mutex dpll_lock;
>  
> +	/*
> +	 * For reading active_pipes, min_cdclk, min_voltage_level
> holding
> +	 * any crtc lock is sufficient, for writing must hold all of
> them.
> +	 */
>  	u8 active_pipes;
>  	/* minimum acceptable cdclk for each pipe */
>  	int min_cdclk[I915_MAX_PIPES];
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 02/13] drm/i915: Rework global state locking

[Lisovskiy, Stanislav]

On Tue, 2019-10-15 at 22:30 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> So far we've sort of protected the global state under dev_priv with
> the connection_mutex. I wan to change that so that we can change the
> cdclk even for pure plane updates. To that end let's formalize the
> protection of the global state to follow what I started with the
> cdclk
> code already (though not entirely properly) such that any crtc mutex
> will suffice as a read lock, and all crtcs mutexes act as the write
> lock.
> 
> We'll also pimp intel_atomic_state_clear() to clear the entire global
> state, so that we don't accidentally leak stale information between
> the locking retries.
> 
> As a slight optimization we'll only lock the crtc mutexes to protect
> the global state, however if and when we actually have to poke the
> hw (eg. if the actual cdclk changes) we must serialize commits
> across all crtcs so that a parallel nonblocking commit can't get
> ahead of the cdclk reprogamming. We do that by adding all crtcs to
> the state.
> 
> TODO: the old global state examined during commit may still
> be a problem since it always looks at the _latest_ swapped state
> in dev_priv. Need to add proper old/new state for that too I think.
> 
> v2: Remeber to serialize the commits if necessary
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/display/intel_atomic.c   |  44 ++++++++
>  drivers/gpu/drm/i915/display/intel_atomic.h   |   5 +
>  drivers/gpu/drm/i915/display/intel_audio.c    |  10 +-
>  drivers/gpu/drm/i915/display/intel_cdclk.c    | 102 ++++++++++----
> ----
>  drivers/gpu/drm/i915/display/intel_display.c  |  29 ++++-
>  .../drm/i915/display/intel_display_types.h    |   8 ++
>  drivers/gpu/drm/i915/i915_drv.h               |  11 +-
>  7 files changed, 153 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index c5a552a69752..9cd6d2348a1e 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -429,6 +429,13 @@ void intel_atomic_state_clear(struct
> drm_atomic_state *s)
>  	struct intel_atomic_state *state = to_intel_atomic_state(s);
>  	drm_atomic_state_default_clear(&state->base);
>  	state->dpll_set = state->modeset = false;
> +	state->global_state_changed = false;
> +	state->active_pipes = 0;
> +	memset(&state->min_cdclk, 0, sizeof(state->min_cdclk));
> +	memset(&state->min_voltage_level, 0, sizeof(state-
> >min_voltage_level));
> +	memset(&state->cdclk.logical, 0, sizeof(state->cdclk.logical));
> +	memset(&state->cdclk.actual, 0, sizeof(state->cdclk.actual));
> +	state->cdclk.pipe = INVALID_PIPE;
>  }
>  
>  struct intel_crtc_state *
> @@ -442,3 +449,40 @@ intel_atomic_get_crtc_state(struct
> drm_atomic_state *state,
>  
>  	return to_intel_crtc_state(crtc_state);
>  }
> +
> +int intel_atomic_lock_global_state(struct intel_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	struct intel_crtc *crtc;
> +
> +	state->global_state_changed = true;
> +
> +	for_each_intel_crtc(&dev_priv->drm, crtc) {
> +		int ret;
> +
> +		ret = drm_modeset_lock(&crtc->base.mutex,
> +				       state->base.acquire_ctx);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +int intel_atomic_serialize_global_state(struct intel_atomic_state
> *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	struct intel_crtc *crtc;
> +
> +	state->global_state_changed = true;
> +
> +	for_each_intel_crtc(&dev_priv->drm, crtc) {
> +		struct intel_crtc_state *crtc_state;
> +
> +		crtc_state = intel_atomic_get_crtc_state(&state->base,
> crtc);
> +		if (IS_ERR(crtc_state))
> +			return PTR_ERR(crtc_state);
> +	}
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.h
> b/drivers/gpu/drm/i915/display/intel_atomic.h
> index 58065d3161a3..49d5cb1b9e0a 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.h
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.h
> @@ -16,6 +16,7 @@ struct drm_crtc_state;
>  struct drm_device;
>  struct drm_i915_private;
>  struct drm_property;
> +struct intel_atomic_state;
>  struct intel_crtc;
>  struct intel_crtc_state;
>  
> @@ -46,4 +47,8 @@ int intel_atomic_setup_scalers(struct
> drm_i915_private *dev_priv,
>  			       struct intel_crtc *intel_crtc,
>  			       struct intel_crtc_state *crtc_state);
>  
> +int intel_atomic_lock_global_state(struct intel_atomic_state
> *state);
> +
> +int intel_atomic_serialize_global_state(struct intel_atomic_state
> *state);
> +
>  #endif /* __INTEL_ATOMIC_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
> b/drivers/gpu/drm/i915/display/intel_audio.c
> index ed18511befa3..85e6b2bbb34f 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -28,6 +28,7 @@
>  #include <drm/i915_component.h>
>  
>  #include "i915_drv.h"
> +#include "intel_atomic.h"
>  #include "intel_audio.h"
>  #include "intel_display_types.h"
>  #include "intel_lpe_audio.h"
> @@ -818,13 +819,8 @@ static void glk_force_audio_cdclk(struct
> drm_i915_private *dev_priv,
>  	to_intel_atomic_state(state)->cdclk.force_min_cdclk =
>  		enable ? 2 * 96000 : 0;
>  
> -	/*
> -	 * Protects dev_priv->cdclk.force_min_cdclk
> -	 * Need to lock this here in case we have no active pipes
> -	 * and thus wouldn't lock it during the commit otherwise.
> -	 */
> -	ret = drm_modeset_lock(&dev_priv-
> >drm.mode_config.connection_mutex,
> -			       &ctx);
> +	/* Protects dev_priv->cdclk.force_min_cdclk */
> +	ret =
> intel_atomic_lock_global_state(to_intel_atomic_state(state));
>  	if (!ret)
>  		ret = drm_atomic_commit(state);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 6eaebc38ee01..6c17a3bbf866 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -2007,11 +2007,20 @@ static int intel_compute_min_cdclk(struct
> intel_atomic_state *state)
>  	       sizeof(state->min_cdclk));
>  
>  	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
> +		int ret;
> +
>  		min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
>  		if (min_cdclk < 0)
>  			return min_cdclk;
>  
> +		if (state->min_cdclk[i] == min_cdclk)
> +			continue;
> +
>  		state->min_cdclk[i] = min_cdclk;
> +
> +		ret = intel_atomic_lock_global_state(state);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	min_cdclk = state->cdclk.force_min_cdclk;
> @@ -2034,7 +2043,7 @@ static int intel_compute_min_cdclk(struct
> intel_atomic_state *state)
>   * future platforms this code will need to be
>   * adjusted.
>   */
> -static u8 bxt_compute_min_voltage_level(struct intel_atomic_state
> *state)
> +static int bxt_compute_min_voltage_level(struct intel_atomic_state
> *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>  	struct intel_crtc *crtc;
> @@ -2047,11 +2056,21 @@ static u8
> bxt_compute_min_voltage_level(struct intel_atomic_state *state)
>  	       sizeof(state->min_voltage_level));
>  
>  	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
> +		int ret;
> +
>  		if (crtc_state->base.enable)
> -			state->min_voltage_level[i] =
> -				crtc_state->min_voltage_level;
> +			min_voltage_level = crtc_state-
> >min_voltage_level;
>  		else
> -			state->min_voltage_level[i] = 0;
> +			min_voltage_level = 0;
> +
> +		if (state->min_voltage_level[i] == min_voltage_level)
> +			continue;
> +
> +		state->min_voltage_level[i] = min_voltage_level;
> +
> +		ret = intel_atomic_lock_global_state(state);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	min_voltage_level = 0;
> @@ -2195,20 +2214,24 @@ static int skl_modeset_calc_cdclk(struct
> intel_atomic_state *state)
>  static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	int min_cdclk, cdclk, vco;
> +	int min_cdclk, min_voltage_level, cdclk, vco;
>  
>  	min_cdclk = intel_compute_min_cdclk(state);
>  	if (min_cdclk < 0)
>  		return min_cdclk;
>  
> +	min_voltage_level = bxt_compute_min_voltage_level(state);
> +	if (min_voltage_level < 0)
> +		return min_voltage_level;
> +
>  	cdclk = bxt_calc_cdclk(dev_priv, min_cdclk);
>  	vco = bxt_calc_cdclk_pll_vco(dev_priv, cdclk);
>  
>  	state->cdclk.logical.vco = vco;
>  	state->cdclk.logical.cdclk = cdclk;
>  	state->cdclk.logical.voltage_level =
> -		max(dev_priv->display.calc_voltage_level(cdclk),
> -		    bxt_compute_min_voltage_level(state));
> +		max_t(int, min_voltage_level,
> +		      dev_priv->display.calc_voltage_level(cdclk));
>  
>  	if (!state->active_pipes) {
>  		cdclk = bxt_calc_cdclk(dev_priv, state-
> >cdclk.force_min_cdclk);
> @@ -2225,23 +2248,6 @@ static int bxt_modeset_calc_cdclk(struct
> intel_atomic_state *state)
>  	return 0;
>  }
>  
> -static int intel_lock_all_pipes(struct intel_atomic_state *state)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	struct intel_crtc *crtc;
> -
> -	/* Add all pipes to the state */
> -	for_each_intel_crtc(&dev_priv->drm, crtc) {
> -		struct intel_crtc_state *crtc_state;
> -
> -		crtc_state = intel_atomic_get_crtc_state(&state->base,
> crtc);
> -		if (IS_ERR(crtc_state))
> -			return PTR_ERR(crtc_state);
> -	}
> -
> -	return 0;
> -}
> -
>  static int intel_modeset_all_pipes(struct intel_atomic_state *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> @@ -2308,46 +2314,56 @@ int intel_modeset_calc_cdclk(struct
> intel_atomic_state *state)
>  		return ret;
>  
>  	/*
> -	 * Writes to dev_priv->cdclk.logical must protected by
> -	 * holding all the crtc locks, even if we don't end up
> +	 * Writes to dev_priv->cdclk.{actual,logical} must protected
> +	 * by holding all the crtc mutexes even if we don't end up
>  	 * touching the hardware
>  	 */
> -	if (intel_cdclk_changed(&dev_priv->cdclk.logical,
> -				&state->cdclk.logical)) {
> -		ret = intel_lock_all_pipes(state);
> -		if (ret < 0)
> +	if (intel_cdclk_changed(&dev_priv->cdclk.actual,
> +				&state->cdclk.actual)) {
> +		/*
> +		 * Also serialize commits across all crtcs
> +		 * if the actual hw needs to be poked.
> +		 */
> +		ret = intel_atomic_serialize_global_state(state);
> +		if (ret)
> +			return ret;
> +	} else if (intel_cdclk_changed(&dev_priv->cdclk.logical,
> +				       &state->cdclk.logical)) {
> +		ret = intel_atomic_lock_global_state(state);
> +		if (ret)
>  			return ret;
> +	} else {
> +		return 0;
>  	}
>  
> -	if (is_power_of_2(state->active_pipes)) {
> +	if (is_power_of_2(state->active_pipes) &&
> +	    intel_cdclk_needs_cd2x_update(dev_priv,
> +					  &dev_priv->cdclk.actual,
> +					  &state->cdclk.actual)) {
>  		struct intel_crtc *crtc;
>  		struct intel_crtc_state *crtc_state;
>  
>  		pipe = ilog2(state->active_pipes);
>  		crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> -		crtc_state = intel_atomic_get_new_crtc_state(state,
> crtc);
> -		if (crtc_state &&
> -		    drm_atomic_crtc_needs_modeset(&crtc_state->base))
> +
> +		crtc_state = intel_atomic_get_crtc_state(&state->base,
> crtc);
> +		if (IS_ERR(crtc_state))
> +			return PTR_ERR(crtc_state);
> +
> +		if (drm_atomic_crtc_needs_modeset(&crtc_state->base))
>  			pipe = INVALID_PIPE;
>  	} else {
>  		pipe = INVALID_PIPE;
>  	}
>  
> -	/* All pipes must be switched off while we change the cdclk. */
> -	if (pipe != INVALID_PIPE &&
> -	    intel_cdclk_needs_cd2x_update(dev_priv,
> -					  &dev_priv->cdclk.actual,
> -					  &state->cdclk.actual)) {
> -		ret = intel_lock_all_pipes(state);
> -		if (ret)
> -			return ret;
> -
> +	if (pipe != INVALID_PIPE) {
>  		state->cdclk.pipe = pipe;
>  
>  		DRM_DEBUG_KMS("Can change cdclk with pipe %c active\n",
>  			      pipe_name(pipe));
>  	} else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
>  					     &state->cdclk.actual)) {
> +		/* All pipes must be switched off while we change the
> cdclk. */
>  		ret = intel_modeset_all_pipes(state);
>  		if (ret)
>  			return ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 7d7d1859775a..a8444d9841c1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -12191,6 +12191,12 @@ static bool
> check_digital_port_conflicts(struct intel_atomic_state *state)
>  	unsigned int used_mst_ports = 0;
>  	bool ret = true;
>  
> +	/*
> +	 * We're going to peek into connector->state,
> +	 * hence connection_mutex must be held.
> +	 */
> +	drm_modeset_lock_assert_held(&dev-
> >mode_config.connection_mutex);
> +
>  	/*
>  	 * Walk the connector list instead of the encoder
>  	 * list to detect the problem on ddi platforms
> @@ -13463,7 +13469,6 @@ static int intel_modeset_checks(struct
> intel_atomic_state *state)
>  	state->active_pipes = dev_priv->active_pipes;
>  	state->cdclk.logical = dev_priv->cdclk.logical;
>  	state->cdclk.actual = dev_priv->cdclk.actual;
> -	state->cdclk.pipe = INVALID_PIPE;
>  
>  	for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state,
>  					    new_crtc_state, i) {
> @@ -13476,6 +13481,12 @@ static int intel_modeset_checks(struct
> intel_atomic_state *state)
>  			state->active_pipe_changes |= BIT(crtc->pipe);
>  	}
>  
> +	if (state->active_pipe_changes) {
> +		ret = intel_atomic_lock_global_state(state);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	ret = intel_modeset_calc_cdclk(state);
>  	if (ret)
>  		return ret;
> @@ -13577,7 +13588,7 @@ static int intel_atomic_check(struct
> drm_device *dev,
>  	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
>  	struct intel_crtc *crtc;
>  	int ret, i;
> -	bool any_ms = state->cdclk.force_min_cdclk_changed;
> +	bool any_ms = false;
>  
>  	/* Catch I915_MODE_FLAG_INHERITED */
>  	for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state,
> @@ -13615,6 +13626,8 @@ static int intel_atomic_check(struct
> drm_device *dev,
>  	if (ret)
>  		goto fail;
>  
> +	any_ms |= state->cdclk.force_min_cdclk_changed;
> +
>  	if (any_ms) {
>  		ret = intel_modeset_checks(state);
>  		if (ret)
> @@ -14234,6 +14247,14 @@ static void intel_atomic_track_fbs(struct
> intel_atomic_state *state)
>  					plane->frontbuffer_bit);
>  }
>  
> +static void assert_global_state_locked(struct drm_i915_private
> *dev_priv)
> +{
> +	struct intel_crtc *crtc;
> +
> +	for_each_intel_crtc(&dev_priv->drm, crtc)
> +		drm_modeset_lock_assert_held(&crtc->base.mutex);
> +}
> +
>  static int intel_atomic_commit(struct drm_device *dev,
>  			       struct drm_atomic_state *_state,
>  			       bool nonblock)
> @@ -14299,7 +14320,9 @@ static int intel_atomic_commit(struct
> drm_device *dev,
>  	intel_shared_dpll_swap_state(state);
>  	intel_atomic_track_fbs(state);
>  
> -	if (state->modeset) {
> +	if (state->global_state_changed) {
> +		assert_global_state_locked(dev_priv);
> +
>  		memcpy(dev_priv->min_cdclk, state->min_cdclk,
>  		       sizeof(state->min_cdclk));
>  		memcpy(dev_priv->min_voltage_level, state-
> >min_voltage_level,
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 40390d855815..01fed8957ade 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -506,6 +506,14 @@ struct intel_atomic_state {
>  
>  	bool rps_interactive;
>  
> +	/*
> +	 * active_pipes
> +	 * min_cdclk[]
> +	 * min_voltage_level[]
> +	 * cdclk.*
> +	 */
> +	bool global_state_changed;
> +
>  	/* Gen9+ only */
>  	struct skl_ddb_values wm_results;
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index c46b339064c0..d3e61152d924 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1105,13 +1105,14 @@ struct drm_i915_private {
>  	unsigned int fdi_pll_freq;
>  	unsigned int czclk_freq;
>  
> +	/*
> +	 * For reading holding any crtc lock is sufficient,
> +	 * for writing must hold all of them.
> +	 */
>  	struct {
>  		/*
>  		 * The current logical cdclk state.
>  		 * See intel_atomic_state.cdclk.logical
> -		 *
> -		 * For reading holding any crtc lock is sufficient,
> -		 * for writing must hold all of them.
>  		 */
>  		struct intel_cdclk_state logical;
>  		/*
> @@ -1181,6 +1182,10 @@ struct drm_i915_private {
>  	 */
>  	struct mutex dpll_lock;
>  
> +	/*
> +	 * For reading active_pipes, min_cdclk, min_voltage_level
> holding
> +	 * any crtc lock is sufficient, for writing must hold all of
> them.
> +	 */
>  	u8 active_pipes;
>  	/* minimum acceptable cdclk for each pipe */
>  	int min_cdclk[I915_MAX_PIPES];
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 01/13] drm/i915: Add debugs to distingiush a cd2x update from a full cdclk pll update

[Lisovskiy, Stanislav]

On Tue, 2019-10-15 at 22:30 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> To make the logs a bit less confusing let's toss in some
> debug prints to indicate whether the cdclk reprogramming
> is going to happen with a single pipe active or whether we
> need to turn all pipes off for the duration.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 3d867963a6d1..6eaebc38ee01 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -2343,6 +2343,9 @@ int intel_modeset_calc_cdclk(struct
> intel_atomic_state *state)
>  			return ret;
>  
>  		state->cdclk.pipe = pipe;
> +
> +		DRM_DEBUG_KMS("Can change cdclk with pipe %c active\n",
> +			      pipe_name(pipe));
>  	} else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
>  					     &state->cdclk.actual)) {
>  		ret = intel_modeset_all_pipes(state);
> @@ -2350,6 +2353,8 @@ int intel_modeset_calc_cdclk(struct
> intel_atomic_state *state)
>  			return ret;
>  
>  		state->cdclk.pipe = INVALID_PIPE;
> +
> +		DRM_DEBUG_KMS("Modeset required for cdclk change\n");
>  	}
>  
>  	DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz,
> actual %u kHz\n",
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 01/13] drm/i915: Add debugs to distingiush a cd2x update from a full cdclk pll update

[Lisovskiy, Stanislav]

On Tue, 2019-10-15 at 22:30 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> To make the logs a bit less confusing let's toss in some
> debug prints to indicate whether the cdclk reprogramming
> is going to happen with a single pipe active or whether we
> need to turn all pipes off for the duration.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 3d867963a6d1..6eaebc38ee01 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -2343,6 +2343,9 @@ int intel_modeset_calc_cdclk(struct
> intel_atomic_state *state)
>  			return ret;
>  
>  		state->cdclk.pipe = pipe;
> +
> +		DRM_DEBUG_KMS("Can change cdclk with pipe %c active\n",
> +			      pipe_name(pipe));
>  	} else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
>  					     &state->cdclk.actual)) {
>  		ret = intel_modeset_all_pipes(state);
> @@ -2350,6 +2353,8 @@ int intel_modeset_calc_cdclk(struct
> intel_atomic_state *state)
>  			return ret;
>  
>  		state->cdclk.pipe = INVALID_PIPE;
> +
> +		DRM_DEBUG_KMS("Modeset required for cdclk change\n");
>  	}
>  
>  	DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz,
> actual %u kHz\n",
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 03/13] drm/i915: Move check_digital_port_conflicts() earier

[Lisovskiy, Stanislav]

On Tue, 2019-10-15 at 22:30 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> check_digital_port_conflicts() is done needlessly late. Move it
> earlier.
> This will be needed as later on we want to set any_ms=true a bit
> later
> for non-modesets too and we can't call this guy without the
> connection_mutex held.

Reviewed-by: 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 | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index a8444d9841c1..44bd4d15019b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13456,11 +13456,6 @@ static int intel_modeset_checks(struct
> intel_atomic_state *state)
>  	struct intel_crtc *crtc;
>  	int ret, i;
>  
> -	if (!check_digital_port_conflicts(state)) {
> -		DRM_DEBUG_KMS("rejecting conflicting digital port
> configuration\n");
> -		return -EINVAL;
> -	}
> -
>  	/* keep the current setting */
>  	if (!state->cdclk.force_min_cdclk_changed)
>  		state->cdclk.force_min_cdclk = dev_priv-
> >cdclk.force_min_cdclk;
> @@ -13622,6 +13617,12 @@ static int intel_atomic_check(struct
> drm_device *dev,
>  			any_ms = true;
>  	}
>  
> +	if (any_ms && !check_digital_port_conflicts(state)) {
> +		DRM_DEBUG_KMS("rejecting conflicting digital port
> configuration\n");
> +		ret = EINVAL;
> +		goto fail;
> +	}
> +
>  	ret = drm_dp_mst_atomic_check(&state->base);
>  	if (ret)
>  		goto fail;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 03/13] drm/i915: Move check_digital_port_conflicts() earier

[Lisovskiy, Stanislav]

On Tue, 2019-10-15 at 22:30 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> check_digital_port_conflicts() is done needlessly late. Move it
> earlier.
> This will be needed as later on we want to set any_ms=true a bit
> later
> for non-modesets too and we can't call this guy without the
> connection_mutex held.

Reviewed-by: 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 | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index a8444d9841c1..44bd4d15019b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13456,11 +13456,6 @@ static int intel_modeset_checks(struct
> intel_atomic_state *state)
>  	struct intel_crtc *crtc;
>  	int ret, i;
>  
> -	if (!check_digital_port_conflicts(state)) {
> -		DRM_DEBUG_KMS("rejecting conflicting digital port
> configuration\n");
> -		return -EINVAL;
> -	}
> -
>  	/* keep the current setting */
>  	if (!state->cdclk.force_min_cdclk_changed)
>  		state->cdclk.force_min_cdclk = dev_priv-
> >cdclk.force_min_cdclk;
> @@ -13622,6 +13617,12 @@ static int intel_atomic_check(struct
> drm_device *dev,
>  			any_ms = true;
>  	}
>  
> +	if (any_ms && !check_digital_port_conflicts(state)) {
> +		DRM_DEBUG_KMS("rejecting conflicting digital port
> configuration\n");
> +		ret = EINVAL;
> +		goto fail;
> +	}
> +
>  	ret = drm_dp_mst_atomic_check(&state->base);
>  	if (ret)
>  		goto fail;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx