[PATCH 0/6] Remove more staged config usage

[PATCH 0/6] Remove more staged config usage

[Ander Conselvan de Oliveira]

These patches remove usage of the staged config from the modeset path
that I overlooked in my previous patch series.

Ander Conselvan de Oliveira (6):
  drm/i915: Don't use staged config for VLV cdclk calculations
  drm/i915: Don't use intel_crtc->new_config in pll calculation code
  drm/i915: Remove intel_crtc->new_config
  drm/i915: Don't use staged config in check_digital_port_conflicts()
  drm/i915: Don't use staged config in check_encoder_cloning()
  drm/i915: Don't use staged config in intel_mst_pre_enable_dp()

 drivers/gpu/drm/i915/intel_display.c | 158 ++++++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_dp_mst.c  |   8 +-
 drivers/gpu/drm/i915/intel_drv.h     |   1 -
 3 files changed, 95 insertions(+), 72 deletions(-)

-- 
2.1.0

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

[PATCH 1/6] drm/i915: Don't use staged config for VLV cdclk calculations

[Ander Conselvan de Oliveira]

Now that we use a drm atomic state for the legacy modeset, it is
possible to get rid of the usage of intel_crtc->new_config in the
function intel_mode_max_pixclk().

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 55 +++++++++++++++++++++++++++---------
 1 file changed, 41 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 84f5b41..a6cd8c7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5160,36 +5160,48 @@ static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
 }
 
 /* compute the max pixel clock for new configuration */
-static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv)
+static int intel_mode_max_pixclk(struct drm_atomic_state *state)
 {
-	struct drm_device *dev = dev_priv->dev;
+	struct drm_device *dev = state->dev;
 	struct intel_crtc *intel_crtc;
+	struct intel_crtc_state *crtc_state;
 	int max_pixclk = 0;
 
 	for_each_intel_crtc(dev, intel_crtc) {
-		if (intel_crtc->new_enabled)
-			max_pixclk = max(max_pixclk,
-					 intel_crtc->new_config->base.adjusted_mode.crtc_clock);
+		crtc_state = intel_atomic_get_crtc_state(state, intel_crtc);
+		if (IS_ERR(crtc_state))
+			return PTR_ERR(crtc_state);
+
+		if (!crtc_state->base.enable)
+			continue;
+
+		max_pixclk = max(max_pixclk,
+				 crtc_state->base.adjusted_mode.crtc_clock);
 	}
 
 	return max_pixclk;
 }
 
-static void valleyview_modeset_global_pipes(struct drm_device *dev,
+static int valleyview_modeset_global_pipes(struct drm_atomic_state *state,
 					    unsigned *prepare_pipes)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = to_i915(state->dev);
 	struct intel_crtc *intel_crtc;
-	int max_pixclk = intel_mode_max_pixclk(dev_priv);
+	int max_pixclk = intel_mode_max_pixclk(state);
+
+	if (max_pixclk < 0)
+		return max_pixclk;
 
 	if (valleyview_calc_cdclk(dev_priv, max_pixclk) ==
 	    dev_priv->vlv_cdclk_freq)
-		return;
+		return 0;
 
 	/* disable/enable all currently active pipes while we change cdclk */
-	for_each_intel_crtc(dev, intel_crtc)
+	for_each_intel_crtc(state->dev, intel_crtc)
 		if (intel_crtc->base.state->enable)
 			*prepare_pipes |= (1 << intel_crtc->pipe);
+
+	return 0;
 }
 
 static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
@@ -5232,8 +5244,18 @@ static void valleyview_modeset_global_resources(struct drm_atomic_state *state)
 {
 	struct drm_device *dev = state->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int max_pixclk = intel_mode_max_pixclk(dev_priv);
-	int req_cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
+	int max_pixclk = intel_mode_max_pixclk(state);
+	int req_cdclk;
+
+	/* The only reason this can fail is if we fail to add the crtc_state
+	 * to the atomic state. But that can't happen since the call to
+	 * intel_mode_max_pixclk() in valleyview_modeset_global_pipes() (which
+	 * can't have failed otherwise the mode set would be aborted) added all
+	 * the states already. */
+	if (WARN_ON(max_pixclk < 0))
+		return;
+
+	req_cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
 
 	if (req_cdclk != dev_priv->vlv_cdclk_freq) {
 		/*
@@ -11550,6 +11572,8 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 		if (IS_ERR(pipe_config))
 			return pipe_config;
 
+		pipe_config->base.enable = true;
+
 		intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
 				       "[modeset]");
 	}
@@ -11598,6 +11622,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_display_mode *saved_mode;
+	struct drm_atomic_state *state = pipe_config->base.state;
 	struct intel_crtc_state *crtc_state_copy = NULL;
 	struct intel_crtc *intel_crtc;
 	int ret = 0;
@@ -11625,7 +11650,9 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 	 * adjusted_mode bits in the crtc directly.
 	 */
 	if (IS_VALLEYVIEW(dev)) {
-		valleyview_modeset_global_pipes(dev, &prepare_pipes);
+		ret = valleyview_modeset_global_pipes(state, &prepare_pipes);
+		if (ret)
+			goto done;
 
 		/* may have added more to prepare_pipes than we should */
 		prepare_pipes &= ~disable_pipes;
@@ -11669,7 +11696,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 	 * update the the output configuration. */
 	intel_modeset_update_state(dev, prepare_pipes);
 
-	modeset_update_crtc_power_domains(pipe_config->base.state);
+	modeset_update_crtc_power_domains(state);
 
 	/* Set up the DPLL and any encoders state that needs to adjust or depend
 	 * on the DPLL.
-- 
2.1.0

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

[PATCH 2/6] drm/i915: Don't use intel_crtc->new_config in pll calculation code

[Ander Conselvan de Oliveira]

Move towards atomic by using the atomic state instead.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a6cd8c7..0b7ddee 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11581,10 +11581,11 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 	return intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));;
 }
 
-static int __intel_set_mode_setup_plls(struct drm_device *dev,
+static int __intel_set_mode_setup_plls(struct drm_atomic_state *state,
 				       unsigned modeset_pipes,
 				       unsigned disable_pipes)
 {
+	struct drm_device *dev = state->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	unsigned clear_pipes = modeset_pipes | disable_pipes;
 	struct intel_crtc *intel_crtc;
@@ -11598,9 +11599,15 @@ static int __intel_set_mode_setup_plls(struct drm_device *dev,
 		goto done;
 
 	for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) {
-		struct intel_crtc_state *state = intel_crtc->new_config;
+		struct intel_crtc_state *crtc_state =
+			intel_atomic_get_crtc_state(state, intel_crtc);
+
+		/* Modeset pipes should have a new state by now */
+		if (WARN_ON(IS_ERR(crtc_state)))
+			continue;
+
 		ret = dev_priv->display.crtc_compute_clock(intel_crtc,
-							   state);
+							   crtc_state);
 		if (ret) {
 			intel_shared_dpll_abort_config(dev_priv);
 			goto done;
@@ -11658,7 +11665,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 		prepare_pipes &= ~disable_pipes;
 	}
 
-	ret = __intel_set_mode_setup_plls(dev, modeset_pipes, disable_pipes);
+	ret = __intel_set_mode_setup_plls(state, modeset_pipes, disable_pipes);
 	if (ret)
 		goto done;
 
-- 
2.1.0

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

[PATCH 3/6] drm/i915: Remove intel_crtc->new_config

[Ander Conselvan de Oliveira]

It's not needed anymore, now that all the users were converted to using
an atomic state.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 31 -------------------------------
 drivers/gpu/drm/i915/intel_drv.h     |  1 -
 2 files changed, 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0b7ddee..207c713 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9171,7 +9171,6 @@ retry:
 
 	intel_crtc = to_intel_crtc(crtc);
 	intel_crtc->new_enabled = true;
-	intel_crtc->new_config = intel_crtc->config;
 	old->dpms_mode = connector->dpms;
 	old->load_detect_temp = true;
 	old->release_fb = NULL;
@@ -9227,10 +9226,6 @@ retry:
 
  fail:
 	intel_crtc->new_enabled = crtc->state->enable;
-	if (intel_crtc->new_enabled)
-		intel_crtc->new_config = intel_crtc->config;
-	else
-		intel_crtc->new_config = NULL;
 fail_unlock:
 	if (state) {
 		drm_atomic_state_free(state);
@@ -9276,7 +9271,6 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 		to_intel_connector(connector)->new_encoder = NULL;
 		intel_encoder->new_crtc = NULL;
 		intel_crtc->new_enabled = false;
-		intel_crtc->new_config = NULL;
 
 		connector_state->best_encoder = NULL;
 		connector_state->crtc = NULL;
@@ -10416,11 +10410,6 @@ static void intel_modeset_update_staged_output_state(struct drm_device *dev)
 
 	for_each_intel_crtc(dev, crtc) {
 		crtc->new_enabled = crtc->base.state->enable;
-
-		if (crtc->new_enabled)
-			crtc->new_config = crtc->config;
-		else
-			crtc->new_config = NULL;
 	}
 }
 
@@ -10981,9 +10970,6 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
 	/* Double check state. */
 	for_each_intel_crtc(dev, intel_crtc) {
 		WARN_ON(intel_crtc->base.state->enable != intel_crtc_in_use(&intel_crtc->base));
-		WARN_ON(intel_crtc->new_config &&
-			intel_crtc->new_config != intel_crtc->config);
-		WARN_ON(intel_crtc->base.state->enable != !!intel_crtc->new_config);
 	}
 
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
@@ -11646,9 +11632,6 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 
 	*saved_mode = crtc->mode;
 
-	if (modeset_pipes)
-		to_intel_crtc(crtc)->new_config = pipe_config;
-
 	/*
 	 * See if the config requires any additional preparation, e.g.
 	 * to adjust global state with pipes off.  We need to do this
@@ -11741,9 +11724,6 @@ done:
 		       sizeof *crtc_state_copy);
 		intel_crtc->config = crtc_state_copy;
 		intel_crtc->base.state = &crtc_state_copy->base;
-
-		if (modeset_pipes)
-			intel_crtc->new_config = intel_crtc->config;
 	} else {
 		kfree(crtc_state_copy);
 	}
@@ -11922,11 +11902,6 @@ static void intel_set_config_restore_state(struct drm_device *dev,
 	count = 0;
 	for_each_intel_crtc(dev, crtc) {
 		crtc->new_enabled = config->save_crtc_enabled[count++];
-
-		if (crtc->new_enabled)
-			crtc->new_config = crtc->config;
-		else
-			crtc->new_config = NULL;
 	}
 
 	count = 0;
@@ -12153,11 +12128,6 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 				      crtc->new_enabled ? "en" : "dis");
 			config->mode_changed = true;
 		}
-
-		if (crtc->new_enabled)
-			crtc->new_config = crtc->config;
-		else
-			crtc->new_config = NULL;
 	}
 
 	return 0;
@@ -12184,7 +12154,6 @@ static void disable_crtc_nofb(struct intel_crtc *crtc)
 	}
 
 	crtc->new_enabled = false;
-	crtc->new_config = NULL;
 }
 
 static int intel_crtc_set_config(struct drm_mode_set *set)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4799b11..686014b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -471,7 +471,6 @@ struct intel_crtc {
 
 	struct intel_initial_plane_config plane_config;
 	struct intel_crtc_state *config;
-	struct intel_crtc_state *new_config;
 	bool new_enabled;
 
 	/* reset counter value when the last flip was submitted */
-- 
2.1.0

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

[PATCH 4/6] drm/i915: Don't use staged config in check_digital_port_conflicts()

[Ander Conselvan de Oliveira]

Reduce dependency on the staged config by using the atomic state
instead.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 207c713..b1fbe9d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10658,23 +10658,30 @@ static bool check_encoder_cloning(struct intel_crtc *crtc)
 	return true;
 }
 
-static bool check_digital_port_conflicts(struct drm_device *dev)
+static bool check_digital_port_conflicts(struct drm_atomic_state *state)
 {
-	struct intel_connector *connector;
+	struct drm_device *dev = state->dev;
+	struct intel_encoder *encoder;
+	struct drm_connector_state *connector_state;
 	unsigned int used_ports = 0;
+	int i;
 
 	/*
 	 * Walk the connector list instead of the encoder
 	 * list to detect the problem on ddi platforms
 	 * where there's just one encoder per digital port.
 	 */
-	for_each_intel_connector(dev, connector) {
-		struct intel_encoder *encoder = connector->new_encoder;
+	for (i = 0; i < state->num_connector; i++) {
+		if (!state->connectors[i])
+			continue;
 
-		if (!encoder)
+		connector_state = state->connector_states[i];
+		if (!connector_state->best_encoder)
 			continue;
 
-		WARN_ON(!encoder->new_crtc);
+		encoder = to_intel_encoder(connector_state->best_encoder);
+
+		WARN_ON(!connector_state->crtc);
 
 		switch (encoder->type) {
 			unsigned int port_mask;
@@ -10716,7 +10723,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 			  struct drm_display_mode *mode,
 			  struct drm_atomic_state *state)
 {
-	struct drm_device *dev = crtc->dev;
 	struct intel_encoder *encoder;
 	struct intel_connector *connector;
 	struct drm_connector_state *connector_state;
@@ -10730,7 +10736,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 		return ERR_PTR(-EINVAL);
 	}
 
-	if (!check_digital_port_conflicts(dev)) {
+	if (!check_digital_port_conflicts(state)) {
 		DRM_DEBUG_KMS("rejecting conflicting digital port configuration\n");
 		return ERR_PTR(-EINVAL);
 	}
-- 
2.1.0

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

[PATCH 5/6] drm/i915: Don't use staged config in check_encoder_cloning()

[Ander Conselvan de Oliveira]

Reduce dependency on the staged config by using the atomic state
instead.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b1fbe9d..bc8e221 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10625,16 +10625,24 @@ static bool encoders_cloneable(const struct intel_encoder *a,
 			  b->cloneable & (1 << a->type));
 }
 
-static bool check_single_encoder_cloning(struct intel_crtc *crtc,
+static bool check_single_encoder_cloning(struct drm_atomic_state *state,
+					 struct intel_crtc *crtc,
 					 struct intel_encoder *encoder)
 {
-	struct drm_device *dev = crtc->base.dev;
 	struct intel_encoder *source_encoder;
+	struct drm_connector_state *connector_state;
+	int i;
 
-	for_each_intel_encoder(dev, source_encoder) {
-		if (source_encoder->new_crtc != crtc)
+	for (i = 0; i < state->num_connector; i++) {
+		if (!state->connectors[i])
 			continue;
 
+		connector_state = state->connector_states[i];
+		if (connector_state->crtc != &crtc->base)
+			continue;
+
+		source_encoder =
+			to_intel_encoder(connector_state->best_encoder);
 		if (!encoders_cloneable(encoder, source_encoder))
 			return false;
 	}
@@ -10642,16 +10650,23 @@ static bool check_single_encoder_cloning(struct intel_crtc *crtc,
 	return true;
 }
 
-static bool check_encoder_cloning(struct intel_crtc *crtc)
+static bool check_encoder_cloning(struct drm_atomic_state *state,
+				  struct intel_crtc *crtc)
 {
-	struct drm_device *dev = crtc->base.dev;
 	struct intel_encoder *encoder;
+	struct drm_connector_state *connector_state;
+	int i;
 
-	for_each_intel_encoder(dev, encoder) {
-		if (encoder->new_crtc != crtc)
+	for (i = 0; i < state->num_connector; i++) {
+		if (!state->connectors[i])
 			continue;
 
-		if (!check_single_encoder_cloning(crtc, encoder))
+		connector_state = state->connector_states[i];
+		if (connector_state->crtc != &crtc->base)
+			continue;
+
+		encoder = to_intel_encoder(connector_state->best_encoder);
+		if (!check_single_encoder_cloning(state, crtc, encoder))
 			return false;
 	}
 
@@ -10731,7 +10746,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 	int i;
 	bool retry = true;
 
-	if (!check_encoder_cloning(to_intel_crtc(crtc))) {
+	if (!check_encoder_cloning(state, to_intel_crtc(crtc))) {
 		DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
 		return ERR_PTR(-EINVAL);
 	}
-- 
2.1.0

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

[PATCH 6/6] drm/i915: Don't use staged config in intel_mst_pre_enable_dp()

[Ander Conselvan de Oliveira]

For the conversion to atomic. The pre_enable() hooks are called as part
of the crtc enable sequence, at which point the staged config was
already made effective. Furthermore, the function actually changes
hardware state, so it should anyway deal with current and not staged
config.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 5329c85..adcc5e6 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -150,14 +150,14 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder)
 	enum port port = intel_dig_port->port;
 	int ret;
 	uint32_t temp;
-	struct intel_connector *found = NULL, *intel_connector;
+	struct intel_connector *found = NULL, *connector;
 	int slots;
 	struct drm_crtc *crtc = encoder->base.crtc;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	for_each_intel_connector(dev, intel_connector) {
-		if (intel_connector->new_encoder == encoder) {
-			found = intel_connector;
+	for_each_intel_connector(dev, connector) {
+		if (connector->base.state->best_encoder == &encoder->base) {
+			found = connector;
 			break;
 		}
 	}
-- 
2.1.0

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

Re: [PATCH 1/6] drm/i915: Don't use staged config for VLV cdclk calculations

[Daniel Vetter]

On Thu, Apr 02, 2015 at 02:47:56PM +0300, Ander Conselvan de Oliveira wrote:
> -static void valleyview_modeset_global_pipes(struct drm_device *dev,
> +static int valleyview_modeset_global_pipes(struct drm_atomic_state *state,
>  					    unsigned *prepare_pipes)
>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
>  	struct intel_crtc *intel_crtc;
> -	int max_pixclk = intel_mode_max_pixclk(dev_priv);
> +	int max_pixclk = intel_mode_max_pixclk(state);
> +
> +	if (max_pixclk < 0)
> +		return max_pixclk;
>  
>  	if (valleyview_calc_cdclk(dev_priv, max_pixclk) ==
>  	    dev_priv->vlv_cdclk_freq)

We need to move the current cdclk_freq into the global state like the
shared pll state. But that's for another patch series, can you please make
a note about it to make sure we don't forget?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/6] drm/i915: Don't use intel_crtc->new_config in pll calculation code

[Daniel Vetter]

On Thu, Apr 02, 2015 at 02:47:57PM +0300, Ander Conselvan de Oliveira wrote:
> Move towards atomic by using the atomic state instead.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a6cd8c7..0b7ddee 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11581,10 +11581,11 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
>  	return intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));;
>  }
>  
> -static int __intel_set_mode_setup_plls(struct drm_device *dev,
> +static int __intel_set_mode_setup_plls(struct drm_atomic_state *state,
>  				       unsigned modeset_pipes,
>  				       unsigned disable_pipes)

Bikeshed: The name of this function is a bit confusion - it's part of the
compute_config phase of modesets still, but not really named like that.
Same with the call to valleyview_modeset_global_pipes right above it.

The problem is also that this is separated from the other compute_config
code. Hence I think we can only apply some polish here once that's all
unified again and merged into the overall atomic_check function.
-Daniel

>  {
> +	struct drm_device *dev = state->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	unsigned clear_pipes = modeset_pipes | disable_pipes;
>  	struct intel_crtc *intel_crtc;
> @@ -11598,9 +11599,15 @@ static int __intel_set_mode_setup_plls(struct drm_device *dev,
>  		goto done;
>  
>  	for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) {
> -		struct intel_crtc_state *state = intel_crtc->new_config;
> +		struct intel_crtc_state *crtc_state =
> +			intel_atomic_get_crtc_state(state, intel_crtc);
> +
> +		/* Modeset pipes should have a new state by now */
> +		if (WARN_ON(IS_ERR(crtc_state)))
> +			continue;
> +
>  		ret = dev_priv->display.crtc_compute_clock(intel_crtc,
> -							   state);
> +							   crtc_state);
>  		if (ret) {
>  			intel_shared_dpll_abort_config(dev_priv);
>  			goto done;
> @@ -11658,7 +11665,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
>  		prepare_pipes &= ~disable_pipes;
>  	}
>  
> -	ret = __intel_set_mode_setup_plls(dev, modeset_pipes, disable_pipes);
> +	ret = __intel_set_mode_setup_plls(state, modeset_pipes, disable_pipes);
>  	if (ret)
>  		goto done;
>  
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: Don't use staged config in check_digital_port_conflicts()

[Daniel Vetter]

On Thu, Apr 02, 2015 at 02:47:59PM +0300, Ander Conselvan de Oliveira wrote:
> Reduce dependency on the staged config by using the atomic state
> instead.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 207c713..b1fbe9d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10658,23 +10658,30 @@ static bool check_encoder_cloning(struct intel_crtc *crtc)
>  	return true;
>  }
>  
> -static bool check_digital_port_conflicts(struct drm_device *dev)
> +static bool check_digital_port_conflicts(struct drm_atomic_state *state)
>  {
> -	struct intel_connector *connector;
> +	struct drm_device *dev = state->dev;
> +	struct intel_encoder *encoder;
> +	struct drm_connector_state *connector_state;
>  	unsigned int used_ports = 0;
> +	int i;
>  
>  	/*
>  	 * Walk the connector list instead of the encoder
>  	 * list to detect the problem on ddi platforms
>  	 * where there's just one encoder per digital port.
>  	 */
> -	for_each_intel_connector(dev, connector) {
> -		struct intel_encoder *encoder = connector->new_encoder;
> +	for (i = 0; i < state->num_connector; i++) {
> +		if (!state->connectors[i])
> +			continue;

More users for for_each_connector_in_state!
-Daniel

>  
> -		if (!encoder)
> +		connector_state = state->connector_states[i];
> +		if (!connector_state->best_encoder)
>  			continue;
>  
> -		WARN_ON(!encoder->new_crtc);
> +		encoder = to_intel_encoder(connector_state->best_encoder);
> +
> +		WARN_ON(!connector_state->crtc);
>  
>  		switch (encoder->type) {
>  			unsigned int port_mask;
> @@ -10716,7 +10723,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  			  struct drm_display_mode *mode,
>  			  struct drm_atomic_state *state)
>  {
> -	struct drm_device *dev = crtc->dev;
>  	struct intel_encoder *encoder;
>  	struct intel_connector *connector;
>  	struct drm_connector_state *connector_state;
> @@ -10730,7 +10736,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> -	if (!check_digital_port_conflicts(dev)) {
> +	if (!check_digital_port_conflicts(state)) {
>  		DRM_DEBUG_KMS("rejecting conflicting digital port configuration\n");
>  		return ERR_PTR(-EINVAL);
>  	}
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/6] Remove more staged config usage

[Daniel Vetter]

On Thu, Apr 02, 2015 at 02:47:55PM +0300, Ander Conselvan de Oliveira wrote:
> These patches remove usage of the staged config from the modeset path
> that I overlooked in my previous patch series.
> 
> Ander Conselvan de Oliveira (6):
>   drm/i915: Don't use staged config for VLV cdclk calculations
>   drm/i915: Don't use intel_crtc->new_config in pll calculation code
>   drm/i915: Remove intel_crtc->new_config
>   drm/i915: Don't use staged config in check_digital_port_conflicts()
>   drm/i915: Don't use staged config in check_encoder_cloning()
>   drm/i915: Don't use staged config in intel_mst_pre_enable_dp()

Entire series merged, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx