[17/17] drm/i915: Store active_pipes bitmask in cdclk state

Message ID	20200120174728.21095-19-ville.syrjala@linux.intel.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <SRS0=KxWV=3J=lists.freedesktop.org=intel-gfx-bounces@kernel.org> DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 353A22253D From: Ville Syrjala <ville.syrjala@linux.intel.com> To: intel-gfx@lists.freedesktop.org Date: Mon, 20 Jan 2020 19:47:28 +0200 Message-Id: <20200120174728.21095-19-ville.syrjala@linux.intel.com> In-Reply-To: <20200120174728.21095-1-ville.syrjala@linux.intel.com> References: <20200120174728.21095-1-ville.syrjala@linux.intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 17/17] drm/i915: Store active_pipes bitmask in cdclk state Precedence: list Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>
Series	drm/i915: Global state rework \| expand [00/17] drm/i915: Global state rework [01/17] drm/i915: Polish WM_LINETIME register stuff [02/17] drm/i915: Move linetime wms into the crtc state [03/17] drm/i915: Nuke skl wm.dirty_pipes bitmask [04/17] drm/i915: Move more cdclk state handling into the cdclk code [05/17] drm/i915: Collect more cdclk state under the same roof [06/17] drm/i915: s/need_cd2x_updare/can_cd2x_update/ [07/17] drm/i915: s/cdclk_state/cdclk_config/ [08/17] drm/i915: Simplify intel_set_cdclk_{pre, post}_plane_update() calling convention [09/17] drm/i915: Extract intel_cdclk_state [10/17] drm/i915: swap() the entire cdclk state [11/17] drm/i915: s/init_cdclk/init_cdclk_hw/ [12/17] drm/i915: Move intel_atomic_state_free() into intel_atomic.c [13/17] drm/i915: Intrduce better global state handling [14/17] drm/i915: Convert bandwidth state to global state [15/17] drm/i915: Introduce intel_calc_active_pipes() [16/17] drm/i915: Convert cdclk to global state [17/17] drm/i915: Store active_pipes bitmask in cdclk state

Message ID

20200120174728.21095-19-ville.syrjala@linux.intel.com (mailing list archive)

State

New, archived

Headers

DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 353A22253D
From: Ville Syrjala <ville.syrjala@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Date: Mon, 20 Jan 2020 19:47:28 +0200
Message-Id: <20200120174728.21095-19-ville.syrjala@linux.intel.com>
In-Reply-To: <20200120174728.21095-1-ville.syrjala@linux.intel.com>
References: <20200120174728.21095-1-ville.syrjala@linux.intel.com>
MIME-Version: 1.0
Subject: [Intel-gfx] [PATCH 17/17] drm/i915: Store active_pipes bitmask in
 cdclk state
Precedence: list
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>

Series

drm/i915: Global state rework | expand

Commit Message

Ville Syrjala Jan. 20, 2020, 5:47 p.m. UTC

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

Let's add a copy of the active_pipes bitmask into the cdclk_state.
While this is duplicating a bit of information we may already
have elsewhere, I think it's worth it to decopule the cdclk stuff
from whatever else wants to use that bitmask. Also we want to get
rid of all the old ad-hoc global state which is what the current
bitmask is, so this removes one obstacle.

The one extra thing we have to remember is write locking the cdclk
state whenever the bitmask changes.

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

Comments

Imre Deak Jan. 27, 2020, 5:11 p.m. UTC | #1

On Mon, Jan 20, 2020 at 07:47:28PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Let's add a copy of the active_pipes bitmask into the cdclk_state.
> While this is duplicating a bit of information we may already
> have elsewhere, I think it's worth it to decopule the cdclk stuff
> from whatever else wants to use that bitmask. Also we want to get
> rid of all the old ad-hoc global state which is what the current
> bitmask is, so this removes one obstacle.
> 
> The one extra thing we have to remember is write locking the cdclk
> state whenever the bitmask changes.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c   | 20 +++++++++++---------
>  drivers/gpu/drm/i915/display/intel_cdclk.h   |  3 +++
>  drivers/gpu/drm/i915/display/intel_display.c |  8 +++++---
>  3 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index e14bda2bec71..f8e4510f4bd9 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -2125,7 +2125,7 @@ static int vlv_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
>  	cdclk_state->logical.voltage_level =
>  		vlv_calc_voltage_level(dev_priv, cdclk);
>  
> -	if (!state->active_pipes) {
> +	if (!cdclk_state->active_pipes) {
>  		cdclk = vlv_calc_cdclk(dev_priv, cdclk_state->force_min_cdclk);
>  
>  		cdclk_state->actual.cdclk = cdclk;
> @@ -2140,7 +2140,6 @@ static int vlv_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
>  
>  static int bdw_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
>  {
> -	struct intel_atomic_state *state = cdclk_state->base.state;
>  	int min_cdclk, cdclk;
>  
>  	min_cdclk = intel_compute_min_cdclk(cdclk_state);
> @@ -2157,7 +2156,7 @@ static int bdw_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
>  	cdclk_state->logical.voltage_level =
>  		bdw_calc_voltage_level(cdclk);
>  
> -	if (!state->active_pipes) {
> +	if (!cdclk_state->active_pipes) {
>  		cdclk = bdw_calc_cdclk(cdclk_state->force_min_cdclk);
>  
>  		cdclk_state->actual.cdclk = cdclk;
> @@ -2209,7 +2208,6 @@ static int skl_dpll0_vco(struct intel_cdclk_state *cdclk_state)
>  
>  static int skl_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
>  {
> -	struct intel_atomic_state *state = cdclk_state->base.state;
>  	int min_cdclk, cdclk, vco;
>  
>  	min_cdclk = intel_compute_min_cdclk(cdclk_state);
> @@ -2229,7 +2227,7 @@ static int skl_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
>  	cdclk_state->logical.voltage_level =
>  		skl_calc_voltage_level(cdclk);
>  
> -	if (!state->active_pipes) {
> +	if (!cdclk_state->active_pipes) {
>  		cdclk = skl_calc_cdclk(cdclk_state->force_min_cdclk, vco);
>  
>  		cdclk_state->actual.vco = vco;
> @@ -2266,7 +2264,7 @@ static int bxt_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
>  		max_t(int, min_voltage_level,
>  		      dev_priv->display.calc_voltage_level(cdclk));
>  
> -	if (!state->active_pipes) {
> +	if (!cdclk_state->active_pipes) {
>  		cdclk = bxt_calc_cdclk(dev_priv, cdclk_state->force_min_cdclk);
>  		vco = bxt_calc_cdclk_pll_vco(dev_priv, cdclk);
>  
> @@ -2402,6 +2400,9 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
>  
>  	old_cdclk_state = intel_atomic_get_old_cdclk_state(state);
>  
> +	new_cdclk_state->active_pipes =
> +		intel_calc_active_pipes(state, old_cdclk_state->active_pipes);
> +
>  	ret = dev_priv->display.modeset_calc_cdclk(new_cdclk_state);
>  	if (ret)
>  		return ret;
> @@ -2415,7 +2416,8 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
>  		ret = intel_atomic_serialize_global_state(&new_cdclk_state->base);
>  		if (ret)
>  			return ret;
> -	} else if (intel_cdclk_changed(&old_cdclk_state->logical,
> +	} else if (old_cdclk_state->active_pipes != new_cdclk_state->active_pipes ||
> +		   intel_cdclk_changed(&old_cdclk_state->logical,
>  				       &new_cdclk_state->logical)) {
>  		ret = intel_atomic_lock_global_state(&new_cdclk_state->base);
>  		if (ret)
> @@ -2424,14 +2426,14 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
>  		return 0;
>  	}
>  
> -	if (is_power_of_2(state->active_pipes) &&
> +	if (is_power_of_2(new_cdclk_state->active_pipes) &&
>  	    intel_cdclk_can_cd2x_update(dev_priv,
>  					&old_cdclk_state->actual,
>  					&new_cdclk_state->actual)) {
>  		struct intel_crtc *crtc;
>  		struct intel_crtc_state *crtc_state;
>  
> -		pipe = ilog2(state->active_pipes);
> +		pipe = ilog2(new_cdclk_state->active_pipes);
>  		crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
>  
>  		crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
> index 195fca70bfcb..df21dbdcc575 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> @@ -50,6 +50,9 @@ struct intel_cdclk_state {
>  	/* forced minimum cdclk for glk+ audio w/a */
>  	int force_min_cdclk;
>  	bool force_min_cdclk_changed;
> +
> +	/* bitmask of active pipes */
> +	u8 active_pipes;
>  };
>  
>  int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index dca100546be8..27878c484307 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7551,6 +7551,7 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
>  	dev_priv->active_pipes &= ~BIT(pipe);
>  	cdclk_state->min_cdclk[pipe] = 0;
>  	cdclk_state->min_voltage_level[pipe] = 0;
> +	cdclk_state->active_pipes &= ~BIT(pipe);
>  
>  	bw_state->data_rate[pipe] = 0;
>  	bw_state->num_active_planes[pipe] = 0;
> @@ -18120,10 +18121,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  	struct intel_encoder *encoder;
>  	struct intel_connector *connector;
>  	struct drm_connector_list_iter conn_iter;
> +	u8 active_pipes = 0;
>  	int i;
>  
> -	dev_priv->active_pipes = 0;
> -
>  	for_each_intel_crtc(dev, crtc) {
>  		struct intel_crtc_state *crtc_state =
>  			to_intel_crtc_state(crtc->base.state);
> @@ -18139,13 +18139,15 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  		crtc->active = crtc_state->hw.active;
>  
>  		if (crtc_state->hw.active)
> -			dev_priv->active_pipes |= BIT(crtc->pipe);
> +			active_pipes |= BIT(crtc->pipe);
>  
>  		DRM_DEBUG_KMS("[CRTC:%d:%s] hw state readout: %s\n",
>  			      crtc->base.base.id, crtc->base.name,
>  			      enableddisabled(crtc_state->hw.active));
>  	}
>  
> +	dev_priv->active_pipes = cdclk_state->active_pipes = active_pipes;
> +
>  	readout_plane_state(dev_priv);
>  
>  	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> -- 
> 2.24.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index e14bda2bec71..f8e4510f4bd9 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -2125,7 +2125,7 @@  static int vlv_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
 	cdclk_state->logical.voltage_level =
 		vlv_calc_voltage_level(dev_priv, cdclk);
 
-	if (!state->active_pipes) {
+	if (!cdclk_state->active_pipes) {
 		cdclk = vlv_calc_cdclk(dev_priv, cdclk_state->force_min_cdclk);
 
 		cdclk_state->actual.cdclk = cdclk;
@@ -2140,7 +2140,6 @@  static int vlv_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
 
 static int bdw_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
 {
-	struct intel_atomic_state *state = cdclk_state->base.state;
 	int min_cdclk, cdclk;
 
 	min_cdclk = intel_compute_min_cdclk(cdclk_state);
@@ -2157,7 +2156,7 @@  static int bdw_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
 	cdclk_state->logical.voltage_level =
 		bdw_calc_voltage_level(cdclk);
 
-	if (!state->active_pipes) {
+	if (!cdclk_state->active_pipes) {
 		cdclk = bdw_calc_cdclk(cdclk_state->force_min_cdclk);
 
 		cdclk_state->actual.cdclk = cdclk;
@@ -2209,7 +2208,6 @@  static int skl_dpll0_vco(struct intel_cdclk_state *cdclk_state)
 
 static int skl_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
 {
-	struct intel_atomic_state *state = cdclk_state->base.state;
 	int min_cdclk, cdclk, vco;
 
 	min_cdclk = intel_compute_min_cdclk(cdclk_state);
@@ -2229,7 +2227,7 @@  static int skl_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
 	cdclk_state->logical.voltage_level =
 		skl_calc_voltage_level(cdclk);
 
-	if (!state->active_pipes) {
+	if (!cdclk_state->active_pipes) {
 		cdclk = skl_calc_cdclk(cdclk_state->force_min_cdclk, vco);
 
 		cdclk_state->actual.vco = vco;
@@ -2266,7 +2264,7 @@  static int bxt_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
 		max_t(int, min_voltage_level,
 		      dev_priv->display.calc_voltage_level(cdclk));
 
-	if (!state->active_pipes) {
+	if (!cdclk_state->active_pipes) {
 		cdclk = bxt_calc_cdclk(dev_priv, cdclk_state->force_min_cdclk);
 		vco = bxt_calc_cdclk_pll_vco(dev_priv, cdclk);
 
@@ -2402,6 +2400,9 @@  int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
 
 	old_cdclk_state = intel_atomic_get_old_cdclk_state(state);
 
+	new_cdclk_state->active_pipes =
+		intel_calc_active_pipes(state, old_cdclk_state->active_pipes);
+
 	ret = dev_priv->display.modeset_calc_cdclk(new_cdclk_state);
 	if (ret)
 		return ret;
@@ -2415,7 +2416,8 @@  int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
 		ret = intel_atomic_serialize_global_state(&new_cdclk_state->base);
 		if (ret)
 			return ret;
-	} else if (intel_cdclk_changed(&old_cdclk_state->logical,
+	} else if (old_cdclk_state->active_pipes != new_cdclk_state->active_pipes ||
+		   intel_cdclk_changed(&old_cdclk_state->logical,
 				       &new_cdclk_state->logical)) {
 		ret = intel_atomic_lock_global_state(&new_cdclk_state->base);
 		if (ret)
@@ -2424,14 +2426,14 @@  int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
 		return 0;
 	}
 
-	if (is_power_of_2(state->active_pipes) &&
+	if (is_power_of_2(new_cdclk_state->active_pipes) &&
 	    intel_cdclk_can_cd2x_update(dev_priv,
 					&old_cdclk_state->actual,
 					&new_cdclk_state->actual)) {
 		struct intel_crtc *crtc;
 		struct intel_crtc_state *crtc_state;
 
-		pipe = ilog2(state->active_pipes);
+		pipe = ilog2(new_cdclk_state->active_pipes);
 		crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
 
 		crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index 195fca70bfcb..df21dbdcc575 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -50,6 +50,9 @@  struct intel_cdclk_state {
 	/* forced minimum cdclk for glk+ audio w/a */
 	int force_min_cdclk;
 	bool force_min_cdclk_changed;
+
+	/* bitmask of active pipes */
+	u8 active_pipes;
 };
 
 int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index dca100546be8..27878c484307 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7551,6 +7551,7 @@  static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
 	dev_priv->active_pipes &= ~BIT(pipe);
 	cdclk_state->min_cdclk[pipe] = 0;
 	cdclk_state->min_voltage_level[pipe] = 0;
+	cdclk_state->active_pipes &= ~BIT(pipe);
 
 	bw_state->data_rate[pipe] = 0;
 	bw_state->num_active_planes[pipe] = 0;
@@ -18120,10 +18121,9 @@  static void intel_modeset_readout_hw_state(struct drm_device *dev)
 	struct intel_encoder *encoder;
 	struct intel_connector *connector;
 	struct drm_connector_list_iter conn_iter;
+	u8 active_pipes = 0;
 	int i;
 
-	dev_priv->active_pipes = 0;
-
 	for_each_intel_crtc(dev, crtc) {
 		struct intel_crtc_state *crtc_state =
 			to_intel_crtc_state(crtc->base.state);
@@ -18139,13 +18139,15 @@  static void intel_modeset_readout_hw_state(struct drm_device *dev)
 		crtc->active = crtc_state->hw.active;
 
 		if (crtc_state->hw.active)
-			dev_priv->active_pipes |= BIT(crtc->pipe);
+			active_pipes |= BIT(crtc->pipe);
 
 		DRM_DEBUG_KMS("[CRTC:%d:%s] hw state readout: %s\n",
 			      crtc->base.base.id, crtc->base.name,
 			      enableddisabled(crtc_state->hw.active));
 	}
 
+	dev_priv->active_pipes = cdclk_state->active_pipes = active_pipes;
+
 	readout_plane_state(dev_priv);
 
 	for (i = 0; i < dev_priv->num_shared_dpll; i++) {

[17/17] drm/i915: Store active_pipes bitmask in cdclk state

Commit Message

Comments

Patch