Skip to contentSkip to conversation

drm/i915: vlv pre_enable/enable callback refactoring

PATCH Superseded Jani Nikula 4 participants 13 messages Jul 30, 2013 – Aug 5, 2013

Subsystems (2)
DRM drivers Intel i915
lore
Share & tools
lore

Jani Nikula

author
#0 lore raw

cover drm/i915: vlv pre_enable/enable callback refactoring

Address Daniel's old complaints about ->pre_enable and ->enable called in the same spot in valleyview_crtc_enable [1].

Rebased on top of Chris' yet-to-be-merged locking patch [2].

Cheers,
Jani.

[1] http://mid.gmane.org/CAKMK7uFs9EMvMW8BnS24e5UNm1D7JrfVg3SD5SDFtVEamGfOOg@mail.gmail.com

[2] http://mid.gmane.org/[email protected]

Jani Nikula (3):
drm/i915: rearrange vlv dp enable and pre_enable callbacks drm/i915: rearrange vlv hdmi enable and pre_enable callbacks drm/i915: move encoder->enable callback later in VLV crtc enable

 drivers/gpu/drm/i915/intel_display.c |    7 ++--
 drivers/gpu/drm/i915/intel_dp.c      |   73 ++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_hdmi.c    |   20 +++++-----
 3 files changed, 53 insertions(+), 47 deletions(-)
Signature · 1 line
1.7.10.4
author
#1 lore raw

PATCH 1/3 drm/i915: rearrange vlv dp enable and pre_enable callbacks

Currently ->pre_enable and ->enable are called back to back. Rearrange the DP callbacks to make it possible to move ->enable call later.

Basically do everything in ->pre_enable on VLV, and make ->enable a NOP.

There should be no functional changes.

Signed-off-by Jani Nikula <[email protected]>
---
 drivers/gpu/drm/i915/intel_dp.c |   73 +++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 34 deletions(-)
drivers/gpu/drm/i915/intel_dp.c +39 -34
index 928b42f..c6b38a6 100644
@@ -1691,49 +1691,50 @@ static void intel_enable_dp(struct intel_encoder *encoder)
 	intel_dp_complete_link_train(intel_dp);
 	intel_dp_stop_link_train(intel_dp);
 	ironlake_edp_backlight_on(intel_dp);
+}
 
-	if (IS_VALLEYVIEW(dev)) {
-		struct intel_digital_port *dport =
-			enc_to_dig_port(&encoder->base);
-		int channel = vlv_dport_to_channel(dport);
-
-		vlv_wait_port_ready(dev_priv, channel);
-	}
+static void vlv_enable_dp(struct intel_encoder *encoder)
+{
 }
 
 static void intel_pre_enable_dp(struct intel_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
+
+	if (dport->port == PORT_A)
+		ironlake_edp_pll_on(intel_dp);
+}
+
+static void vlv_pre_enable_dp(struct intel_encoder *encoder)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+	int port = vlv_dport_to_channel(dport);
+	int pipe = intel_crtc->pipe;
+	u32 val;
 
-	if (dport->port == PORT_A && !IS_VALLEYVIEW(dev))
-		ironlake_edp_pll_on(intel_dp);
+	mutex_lock(&dev_priv->dpio_lock);
 
-	if (IS_VALLEYVIEW(dev)) {
-		struct intel_crtc *intel_crtc =
-			to_intel_crtc(encoder->base.crtc);
-		int port = vlv_dport_to_channel(dport);
-		int pipe = intel_crtc->pipe;
-		u32 val;
-
-		mutex_lock(&dev_priv->dpio_lock);
-		val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
-		val = 0;
-		if (pipe)
-			val |= (1<<21);
-		else
-			val &= ~(1<<21);
-		val |= 0x001000c4;
-		vlv_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
+	val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
+	val = 0;
+	if (pipe)
+		val |= (1<<21);
+	else
+		val &= ~(1<<21);
+	val |= 0x001000c4;
+	vlv_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
+	vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port), 0x00760018);
+	vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port), 0x00400888);
 
-		vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port),
-				 0x00760018);
-		vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
-				 0x00400888);
-		mutex_unlock(&dev_priv->dpio_lock);
-	}
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	intel_enable_dp(encoder);
+
+	vlv_wait_port_ready(dev_priv, port);
 }
 
 static void intel_dp_pre_pll_enable(struct intel_encoder *encoder)

@@ -3513,14 +3514,18 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 
 	intel_encoder->compute_config = intel_dp_compute_config;
 	intel_encoder->mode_set = intel_dp_mode_set;
-	intel_encoder->enable = intel_enable_dp;
-	intel_encoder->pre_enable = intel_pre_enable_dp;
 	intel_encoder->disable = intel_disable_dp;
 	intel_encoder->post_disable = intel_post_disable_dp;
 	intel_encoder->get_hw_state = intel_dp_get_hw_state;
 	intel_encoder->get_config = intel_dp_get_config;
-	if (IS_VALLEYVIEW(dev))
+	if (IS_VALLEYVIEW(dev)) {
 		intel_encoder->pre_pll_enable = intel_dp_pre_pll_enable;
+		intel_encoder->pre_enable = vlv_pre_enable_dp;
+		intel_encoder->enable = vlv_enable_dp;
+	} else {
+		intel_encoder->pre_enable = intel_pre_enable_dp;
+		intel_encoder->enable = intel_enable_dp;
+	}
 
 	intel_dig_port->port = port;
 	intel_dig_port->dp.output_reg = output_reg;
Signature · 1 line
1.7.10.4
#2 lore raw

Re: [PATCH 1/3] drm/i915: rearrange vlv dp enable and pre_enable callbacks

Quoted 2 lines · Jani Nikula

Currently ->pre_enable and ->enable are called back to back. Rearrange the DP callbacks to make it possible to move ->enable call later.

A one sentence explanation why we want ->enable later would be perfect. If you can concisely explain why the code was originally written as is, and why we want to do it differently in the near future, that would be ideal.
-Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
author
#3 lore raw

Re: [PATCH 1/3] drm/i915: rearrange vlv dp enable and pre_enable callbacks

Quoted 8 lines · Chris Wilson
Quoted 2 lines · Jani Nikula

Currently ->pre_enable and ->enable are called back to back. Rearrange the DP callbacks to make it possible to move ->enable call later.

A one sentence explanation why we want ->enable later would be perfect. If you can concisely explain why the code was originally written as is, and why we want to do it differently in the near future, that would be ideal.

Good point. How about,

for patches 1&2:

"""
VLV wants encoder enabling before the pipe is up. This is currently achieved through calling the ->enable callback early, right after the ->pre_enable callback, in valleyview_crtc_enable(). This loses both the distinction between ->pre_enable and ->enable on VLV and the possibility to use a hook at the end of the modeset sequence.

Rearrange the DP callbacks to make it possible to move ->enable call later. Basically do everything in ->pre_enable on VLV, and make ->enable a NOP.

There should be no functional changes.
"""

for patch 3:

"""
VLV wants encoder enabling before the pipe is up. With the previously rearranged VLV DP and HDMI ->pre_enable and ->enable callbacks in place, this no longer depends on the early ->enable hook call. Move the ->enable call at the end of the sequence, similar to the crtc enable on other platforms. This will be needed e.g. for moving the eDP backlight enabling to the right place in the sequence, currently done too early on VLV.

There should be no functional changes.
"""

I'll send the revised patches once we agree on the wording.

Thanks,
Jani.

#4 lore raw

Re: [PATCH 1/3] drm/i915: rearrange vlv dp enable and pre_enable callbacks

Quoted 11 lines · Jani Nikula
Quoted 8 lines · Chris Wilson
Quoted 2 lines · Jani Nikula

Currently ->pre_enable and ->enable are called back to back. Rearrange the DP callbacks to make it possible to move ->enable call later.

A one sentence explanation why we want ->enable later would be perfect. If you can concisely explain why the code was originally written as is, and why we want to do it differently in the near future, that would be ideal.

Good point. How about,

Thank you, that was the explanation I was looking for! :) -Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
author
#5 lore raw

PATCH 2/3 drm/i915: rearrange vlv hdmi enable and pre_enable callbacks

In reply to Jani Nikula

Currently ->pre_enable and ->enable are called back to back. Rearrange the HDMI callbacks to make it possible to move ->enable call later.

Basically do everything in ->pre_enable on VLV, and make ->enable a NOP.

There should be no functional changes.

Signed-off-by Jani Nikula <[email protected]>
---
 drivers/gpu/drm/i915/intel_hdmi.c |   20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)
drivers/gpu/drm/i915/intel_hdmi.c +11 -9
index a2be6b2..68a27c2 100644
@@ -718,14 +718,10 @@ static void intel_enable_hdmi(struct intel_encoder *encoder)
 		I915_WRITE(intel_hdmi->hdmi_reg, temp);
 		POSTING_READ(intel_hdmi->hdmi_reg);
 	}
+}
 
-	if (IS_VALLEYVIEW(dev)) {
-		struct intel_digital_port *dport =
-			enc_to_dig_port(&encoder->base);
-		int channel = vlv_dport_to_channel(dport);
-
-		vlv_wait_port_ready(dev_priv, channel);
-	}
+static void vlv_enable_hdmi(struct intel_encoder *encoder)
+{
 }
 
 static void intel_disable_hdmi(struct intel_encoder *encoder)

@@ -1064,6 +1060,10 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
 	vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
 			 0x00400888);
 	mutex_unlock(&dev_priv->dpio_lock);
+
+	intel_enable_hdmi(encoder);
+
+	vlv_wait_port_ready(dev_priv, port);
 }
 
 static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder)

@@ -1244,14 +1244,16 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
 
 	intel_encoder->compute_config = intel_hdmi_compute_config;
 	intel_encoder->mode_set = intel_hdmi_mode_set;
-	intel_encoder->enable = intel_enable_hdmi;
 	intel_encoder->disable = intel_disable_hdmi;
 	intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
 	intel_encoder->get_config = intel_hdmi_get_config;
 	if (IS_VALLEYVIEW(dev)) {
-		intel_encoder->pre_enable = intel_hdmi_pre_enable;
 		intel_encoder->pre_pll_enable = intel_hdmi_pre_pll_enable;
+		intel_encoder->pre_enable = intel_hdmi_pre_enable;
+		intel_encoder->enable = vlv_enable_hdmi;
 		intel_encoder->post_disable = intel_hdmi_post_disable;
+	} else {
+		intel_encoder->enable = intel_enable_hdmi;
 	}
 
 	intel_encoder->type = INTEL_OUTPUT_HDMI;
Signature · 1 line
1.7.10.4
author
#6 lore raw

PATCH 3/3 drm/i915: move encoder->enable callback later in VLV crtc enable

In reply to Jani Nikula

Encoder ->pre_enable and ->enable callbacks were moved back to back in VLV crtc enable sequence, which is not very useful. Move ->enable call at the end of the sequence.

With the previously rearranged VLV DP and HDMI ->pre_enable and ->enable callbacks in place, this should not cause any functional changes.

Signed-off-by Jani Nikula <[email protected]>
---
 drivers/gpu/drm/i915/intel_display.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/gpu/drm/i915/intel_display.c +3 -4
index 7d63d8d..c3c0bf1 100644
@@ -3663,10 +3663,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 		if (encoder->pre_enable)
 			encoder->pre_enable(encoder);
 
-	/* VLV wants encoder enabling _before_ the pipe is up. */
-	for_each_encoder_on_crtc(dev, crtc, encoder)
-		encoder->enable(encoder);
-
 	i9xx_pfit_enable(intel_crtc);
 
 	intel_crtc_load_lut(crtc);

@@ -3677,6 +3673,9 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	intel_crtc_update_cursor(crtc, true);
 
 	intel_update_fbc(dev);
+
+	for_each_encoder_on_crtc(dev, crtc, encoder)
+		encoder->enable(encoder);
 }
 
 static void i9xx_crtc_enable(struct drm_crtc *crtc)
Signature · 1 line
1.7.10.4
author
#7 lore raw

PATCH v3 1/3 drm/i915: rearrange vlv dp enable and pre_enable callbacks

In reply to Jani Nikula

VLV wants encoder enabling before the pipe is up. This is currently achieved through calling the ->enable callback early, right after the ->pre_enable callback, in valleyview_crtc_enable(). This loses both the distinction between ->pre_enable and ->enable on VLV and the possibility to use a hook at the end of the modeset sequence.

Rearrange the DP callbacks to make it possible to move ->enable call later. Basically do everything in ->pre_enable on VLV, and make ->enable a NOP.

There should be no functional changes.

v2: Rebase.

v3: Explain why this is needed in the commit message (Chris).

Signed-off-by Jani Nikula <[email protected]>
---
 drivers/gpu/drm/i915/intel_dp.c |   73 +++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 34 deletions(-)
drivers/gpu/drm/i915/intel_dp.c +39 -34
index 928b42f..c6b38a6 100644
@@ -1691,49 +1691,50 @@ static void intel_enable_dp(struct intel_encoder *encoder)
 	intel_dp_complete_link_train(intel_dp);
 	intel_dp_stop_link_train(intel_dp);
 	ironlake_edp_backlight_on(intel_dp);
+}
 
-	if (IS_VALLEYVIEW(dev)) {
-		struct intel_digital_port *dport =
-			enc_to_dig_port(&encoder->base);
-		int channel = vlv_dport_to_channel(dport);
-
-		vlv_wait_port_ready(dev_priv, channel);
-	}
+static void vlv_enable_dp(struct intel_encoder *encoder)
+{
 }
 
 static void intel_pre_enable_dp(struct intel_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
+
+	if (dport->port == PORT_A)
+		ironlake_edp_pll_on(intel_dp);
+}
+
+static void vlv_pre_enable_dp(struct intel_encoder *encoder)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+	int port = vlv_dport_to_channel(dport);
+	int pipe = intel_crtc->pipe;
+	u32 val;
 
-	if (dport->port == PORT_A && !IS_VALLEYVIEW(dev))
-		ironlake_edp_pll_on(intel_dp);
+	mutex_lock(&dev_priv->dpio_lock);
 
-	if (IS_VALLEYVIEW(dev)) {
-		struct intel_crtc *intel_crtc =
-			to_intel_crtc(encoder->base.crtc);
-		int port = vlv_dport_to_channel(dport);
-		int pipe = intel_crtc->pipe;
-		u32 val;
-
-		mutex_lock(&dev_priv->dpio_lock);
-		val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
-		val = 0;
-		if (pipe)
-			val |= (1<<21);
-		else
-			val &= ~(1<<21);
-		val |= 0x001000c4;
-		vlv_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
+	val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
+	val = 0;
+	if (pipe)
+		val |= (1<<21);
+	else
+		val &= ~(1<<21);
+	val |= 0x001000c4;
+	vlv_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
+	vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port), 0x00760018);
+	vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port), 0x00400888);
 
-		vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port),
-				 0x00760018);
-		vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
-				 0x00400888);
-		mutex_unlock(&dev_priv->dpio_lock);
-	}
+	mutex_unlock(&dev_priv->dpio_lock);
+
+	intel_enable_dp(encoder);
+
+	vlv_wait_port_ready(dev_priv, port);
 }
 
 static void intel_dp_pre_pll_enable(struct intel_encoder *encoder)

@@ -3513,14 +3514,18 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 
 	intel_encoder->compute_config = intel_dp_compute_config;
 	intel_encoder->mode_set = intel_dp_mode_set;
-	intel_encoder->enable = intel_enable_dp;
-	intel_encoder->pre_enable = intel_pre_enable_dp;
 	intel_encoder->disable = intel_disable_dp;
 	intel_encoder->post_disable = intel_post_disable_dp;
 	intel_encoder->get_hw_state = intel_dp_get_hw_state;
 	intel_encoder->get_config = intel_dp_get_config;
-	if (IS_VALLEYVIEW(dev))
+	if (IS_VALLEYVIEW(dev)) {
 		intel_encoder->pre_pll_enable = intel_dp_pre_pll_enable;
+		intel_encoder->pre_enable = vlv_pre_enable_dp;
+		intel_encoder->enable = vlv_enable_dp;
+	} else {
+		intel_encoder->pre_enable = intel_pre_enable_dp;
+		intel_encoder->enable = intel_enable_dp;
+	}
 
 	intel_dig_port->port = port;
 	intel_dig_port->dp.output_reg = output_reg;
Signature · 1 line
1.7.10.4
author
#8 lore raw

PATCH v3 2/3 drm/i915: rearrange vlv hdmi enable and pre_enable callbacks

VLV wants encoder enabling before the pipe is up. This is currently achieved through calling the ->enable callback early, right after the ->pre_enable callback, in valleyview_crtc_enable(). This loses both the distinction between ->pre_enable and ->enable on VLV and the possibility to use a hook at the end of the modeset sequence.

Rearrange the HDMI callbacks to make it possible to move ->enable call later. Basically do everything in ->pre_enable on VLV, and make ->enable a NOP.

There should be no functional changes.

v2: Rebase.

v3: Explain why this is needed in the commit message (Chris).

Signed-off-by Jani Nikula <[email protected]>
---
 drivers/gpu/drm/i915/intel_hdmi.c |   20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)
drivers/gpu/drm/i915/intel_hdmi.c +11 -9
index a2be6b2..68a27c2 100644
@@ -718,14 +718,10 @@ static void intel_enable_hdmi(struct intel_encoder *encoder)
 		I915_WRITE(intel_hdmi->hdmi_reg, temp);
 		POSTING_READ(intel_hdmi->hdmi_reg);
 	}
+}
 
-	if (IS_VALLEYVIEW(dev)) {
-		struct intel_digital_port *dport =
-			enc_to_dig_port(&encoder->base);
-		int channel = vlv_dport_to_channel(dport);
-
-		vlv_wait_port_ready(dev_priv, channel);
-	}
+static void vlv_enable_hdmi(struct intel_encoder *encoder)
+{
 }
 
 static void intel_disable_hdmi(struct intel_encoder *encoder)

@@ -1064,6 +1060,10 @@ static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
 	vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
 			 0x00400888);
 	mutex_unlock(&dev_priv->dpio_lock);
+
+	intel_enable_hdmi(encoder);
+
+	vlv_wait_port_ready(dev_priv, port);
 }
 
 static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder)

@@ -1244,14 +1244,16 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
 
 	intel_encoder->compute_config = intel_hdmi_compute_config;
 	intel_encoder->mode_set = intel_hdmi_mode_set;
-	intel_encoder->enable = intel_enable_hdmi;
 	intel_encoder->disable = intel_disable_hdmi;
 	intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
 	intel_encoder->get_config = intel_hdmi_get_config;
 	if (IS_VALLEYVIEW(dev)) {
-		intel_encoder->pre_enable = intel_hdmi_pre_enable;
 		intel_encoder->pre_pll_enable = intel_hdmi_pre_pll_enable;
+		intel_encoder->pre_enable = intel_hdmi_pre_enable;
+		intel_encoder->enable = vlv_enable_hdmi;
 		intel_encoder->post_disable = intel_hdmi_post_disable;
+	} else {
+		intel_encoder->enable = intel_enable_hdmi;
 	}
 
 	intel_encoder->type = INTEL_OUTPUT_HDMI;
Signature · 1 line
1.7.10.4
author
#9 lore raw

PATCH v3 3/3 drm/i915: move encoder->enable callback later in VLV crtc enable

In reply to Jani Nikula

VLV wants encoder enabling before the pipe is up. With the previously rearranged VLV DP and HDMI ->pre_enable and ->enable callbacks in place, this no longer depends on the early ->enable hook call. Move the ->enable call at the end of the sequence, similar to the crtc enable on other platforms. This will be needed e.g. for moving the eDP backlight enabling to the right place in the sequence, currently done too early on VLV.

There should be no functional changes.

v2: Rebase.

v3: Explain why this is needed in the commit message (Chris).

Signed-off-by Jani Nikula <[email protected]>
---
 drivers/gpu/drm/i915/intel_display.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/gpu/drm/i915/intel_display.c +3 -4
index 7d63d8d..c3c0bf1 100644
@@ -3663,10 +3663,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 		if (encoder->pre_enable)
 			encoder->pre_enable(encoder);
 
-	/* VLV wants encoder enabling _before_ the pipe is up. */
-	for_each_encoder_on_crtc(dev, crtc, encoder)
-		encoder->enable(encoder);
-
 	i9xx_pfit_enable(intel_crtc);
 
 	intel_crtc_load_lut(crtc);

@@ -3677,6 +3673,9 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	intel_crtc_update_cursor(crtc, true);
 
 	intel_update_fbc(dev);
+
+	for_each_encoder_on_crtc(dev, crtc, encoder)
+		encoder->enable(encoder);
 }
 
 static void i9xx_crtc_enable(struct drm_crtc *crtc)
Signature · 1 line
1.7.10.4
Reviewed-by on v3 3/3
#10 lore raw

Re: [PATCH v3 3/3] drm/i915: move encoder->enable callback later in VLV crtc enable

Quoted 8 lines · Jani Nikula

VLV wants encoder enabling before the pipe is up. With the previously rearranged VLV DP and HDMI ->pre_enable and ->enable callbacks in place, this no longer depends on the early ->enable hook call. Move the ->enable call at the end of the sequence, similar to the crtc enable on other platforms. This will be needed e.g. for moving the eDP backlight enabling to the right place in the sequence, currently done too early on VLV.

Quoted 7 lines

There should be no functional changes.

v2: Rebase.

v3: Explain why this is needed in the commit message (Chris).

Signed-off-by Jani Nikula <[email protected]>

I couldn't spot any functional changes in the 3 patches, so

Reviewed-by Chris Wilson <[email protected]>

-Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
#11 lore raw

Re: [PATCH v3 3/3] drm/i915: move encoder->enable callback later in VLV crtc enable

Quoted 11 lines · Chris Wilson
Quoted 10 lines · Jani Nikula

VLV wants encoder enabling before the pipe is up. With the previously rearranged VLV DP and HDMI ->pre_enable and ->enable callbacks in place, this no longer depends on the early ->enable hook call. Move the ->enable call at the end of the sequence, similar to the crtc enable on other platforms. This will be needed e.g. for moving the eDP backlight enabling to the right place in the sequence, currently done too early on VLV.

There should be no functional changes.

Quoted 8 lines
Quoted 5 lines

v2: Rebase.

v3: Explain why this is needed in the commit message (Chris).

Signed-off-by Jani Nikula <[email protected]>

I couldn't spot any functional changes in the 3 patches, so

Reviewed-by Chris Wilson <[email protected]>

Series merged to dinq, thanks for the patches&review. -Daniel

Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Reviewed-by on v3 3/3
#12 lore raw

Re: [PATCH v3 3/3] drm/i915: move encoder->enable callback later in VLV crtc enable

In reply to Jani Nikula
Quoted 8 lines · Jani Nikula

VLV wants encoder enabling before the pipe is up. With the previously rearranged VLV DP and HDMI ->pre_enable and ->enable callbacks in place, this no longer depends on the early ->enable hook call. Move the ->enable call at the end of the sequence, similar to the crtc enable on other platforms. This will be needed e.g. for moving the eDP backlight enabling to the right place in the sequence, currently done too early on VLV.

Quoted 7 lines

There should be no functional changes.

v2: Rebase.

v3: Explain why this is needed in the commit message (Chris).

Signed-off-by Jani Nikula <[email protected]>

Looks sane to me as well. For the series:

Reviewed-by Ville Syrjälä <[email protected]>

Note that I just lost my VLV board so I wasn't able to test this.

I'm assuming there will eventually be some followon patch to move the backlight stuff to right place for VLV?

Also should we make encoder->enable optional to avoid the stubs for VLV?

And it might make sense to rename all the VLV specific dp/hdmi functions to vlv_foo instead of the intel_foo that they are called now.

Quoted 36 lines
---
 drivers/gpu/drm/i915/intel_display.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/gpu/drm/i915/intel_display.c +3 -4
index 7d63d8d..c3c0bf1 100644
@@ -3663,10 +3663,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 		if (encoder->pre_enable)
 			encoder->pre_enable(encoder);
 
-	/* VLV wants encoder enabling _before_ the pipe is up. */
-	for_each_encoder_on_crtc(dev, crtc, encoder)
-		encoder->enable(encoder);
-
 	i9xx_pfit_enable(intel_crtc);
 
 	intel_crtc_load_lut(crtc);

@@ -3677,6 +3673,9 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	intel_crtc_update_cursor(crtc, true);
 
 	intel_update_fbc(dev);
+
+	for_each_encoder_on_crtc(dev, crtc, encoder)
+		encoder->enable(encoder);
 }
 
 static void i9xx_crtc_enable(struct drm_crtc *crtc)
Signature · 6 lines
1.7.10.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 2 lines
Ville Syrjälä
Intel OTC