[PATCH 1/2] drm/i915: Initialize PPS timetamps on vlv/chv

[PATCH 1/2] drm/i915: Initialize PPS timetamps on vlv/chv

[ville.syrjala]

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

The pps timestamp initialization was accidentally lost on vlv/chv in

 commit a4a5d2f8a96e09844a91469e889f15bd5e927399
 Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
 Date:   Thu Sep 4 14:54:20 2014 +0300

    drm/i915: Track which port is using which pipe's power sequencer

Restore it so that we avoid introducing random delays into the pps operations
during/after driver init time.

Cc: Imre Deak <imre.deak@intel.com>
Reported-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 82e47da..25fe260 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5383,12 +5383,11 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 
 	if (is_edp(intel_dp)) {
 		pps_lock(intel_dp);
-		if (IS_VALLEYVIEW(dev)) {
+		intel_dp_init_panel_power_timestamps(intel_dp);
+		if (IS_VALLEYVIEW(dev))
 			vlv_initial_power_sequencer_setup(intel_dp);
-		} else {
-			intel_dp_init_panel_power_timestamps(intel_dp);
+		else
 			intel_dp_init_panel_power_sequencer(dev, intel_dp);
-		}
 		pps_unlock(intel_dp);
 	}
 
-- 
2.0.4

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

[PATCH 2/2] drm/i915: Read out the power sequencer port assignment on resume on vlv/chv

[ville.syrjala]

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

When we suspend we turn everything off so the pps should be idle, and we
also (or at least should) disable all power wells which will reset the
power sequencer port assignment. So when we resume all power sequencers
should be in their reset state. However it's at least theoretically
possible that the BIOS would touch the power seuqencer(s), so to be safe
we ought to read out the current port assignment like we do at driver
init time.

To do that we can simply call vlv_initial_power_sequencer_setup() from
the encoder ->reset() hook before calling intel_edp_panel_vdd_sanitize().
There's no danger or clobbering the pps delays since we now have those
stored within intel_dp and we don't change them once initialized.

This will make sure that the vdd state gets correctly tracked post-resume
in case the BIOS enabled it.

We need to shuffle things around a bit to get the locking right, and
while at it, make intel_edp_panel_vdd_sanitize() static and move it
around a bit to avoid a forward declaration.

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 80 +++++++++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_drv.h |  1 -
 2 files changed, 47 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 25fe260..8f074cd 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4684,9 +4684,52 @@ static void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
 	pps_unlock(intel_dp);
 }
 
+static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum intel_display_power_domain power_domain;
+
+	lockdep_assert_held(&dev_priv->pps_mutex);
+
+	if (!edp_have_panel_vdd(intel_dp))
+		return;
+
+	/*
+	 * The VDD bit needs a power domain reference, so if the bit is
+	 * already enabled when we boot or resume, grab this reference and
+	 * schedule a vdd off, so we don't hold on to the reference
+	 * indefinitely.
+	 */
+	DRM_DEBUG_KMS("VDD left on by BIOS, adjusting state tracking\n");
+	power_domain = intel_display_port_power_domain(&intel_dig_port->base);
+	intel_display_power_get(dev_priv, power_domain);
+
+	edp_panel_vdd_schedule_off(intel_dp);
+}
+
 static void intel_dp_encoder_reset(struct drm_encoder *encoder)
 {
-	intel_edp_panel_vdd_sanitize(to_intel_encoder(encoder));
+	struct intel_dp *intel_dp;
+
+	if (to_intel_encoder(encoder)->type != INTEL_OUTPUT_EDP)
+		return;
+
+	intel_dp = enc_to_intel_dp(encoder);
+
+	pps_lock(intel_dp);
+
+	/*
+	 * Read out the current power sequencer assignment,
+	 * in case the BIOS did something with it.
+	 */
+	if (IS_VALLEYVIEW(encoder->dev))
+		vlv_initial_power_sequencer_setup(intel_dp);
+
+	intel_edp_panel_vdd_sanitize(intel_dp);
+
+	pps_unlock(intel_dp);
 }
 
 static const struct drm_connector_funcs intel_dp_connector_funcs = {
@@ -5169,37 +5212,6 @@ intel_dp_drrs_init(struct intel_digital_port *intel_dig_port,
 	return downclock_mode;
 }
 
-void intel_edp_panel_vdd_sanitize(struct intel_encoder *intel_encoder)
-{
-	struct drm_device *dev = intel_encoder->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_dp *intel_dp;
-	enum intel_display_power_domain power_domain;
-
-	if (intel_encoder->type != INTEL_OUTPUT_EDP)
-		return;
-
-	intel_dp = enc_to_intel_dp(&intel_encoder->base);
-
-	pps_lock(intel_dp);
-
-	if (!edp_have_panel_vdd(intel_dp))
-		goto out;
-	/*
-	 * The VDD bit needs a power domain reference, so if the bit is
-	 * already enabled when we boot or resume, grab this reference and
-	 * schedule a vdd off, so we don't hold on to the reference
-	 * indefinitely.
-	 */
-	DRM_DEBUG_KMS("VDD left on by BIOS, adjusting state tracking\n");
-	power_domain = intel_display_port_power_domain(intel_encoder);
-	intel_display_power_get(dev_priv, power_domain);
-
-	edp_panel_vdd_schedule_off(intel_dp);
- out:
-	pps_unlock(intel_dp);
-}
-
 static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 				     struct intel_connector *intel_connector)
 {
@@ -5219,7 +5231,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	if (!is_edp(intel_dp))
 		return true;
 
-	intel_edp_panel_vdd_sanitize(intel_encoder);
+	pps_lock(intel_dp);
+	intel_edp_panel_vdd_sanitize(intel_dp);
+	pps_unlock(intel_dp);
 
 	/* Cache DPCD and EDID for edp. */
 	has_dpcd = intel_dp_get_dpcd(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d53ac23..a3415d3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -980,7 +980,6 @@ bool intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port,
 void intel_edp_backlight_on(struct intel_dp *intel_dp);
 void intel_edp_backlight_off(struct intel_dp *intel_dp);
 void intel_edp_panel_vdd_on(struct intel_dp *intel_dp);
-void intel_edp_panel_vdd_sanitize(struct intel_encoder *intel_encoder);
 void intel_edp_panel_on(struct intel_dp *intel_dp);
 void intel_edp_panel_off(struct intel_dp *intel_dp);
 void intel_edp_psr_enable(struct intel_dp *intel_dp);
-- 
2.0.4

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

Re: [PATCH 1/2] drm/i915: Initialize PPS timetamps on vlv/chv

[Imre Deak]

On Tue, 2014-10-28 at 16:15 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The pps timestamp initialization was accidentally lost on vlv/chv in
> 
>  commit a4a5d2f8a96e09844a91469e889f15bd5e927399
>  Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
>  Date:   Thu Sep 4 14:54:20 2014 +0300
> 
>     drm/i915: Track which port is using which pipe's power sequencer
> 
> Restore it so that we avoid introducing random delays into the pps operations
> during/after driver init time.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Reported-by: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Both patches look good to me:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 82e47da..25fe260 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5383,12 +5383,11 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  
>  	if (is_edp(intel_dp)) {
>  		pps_lock(intel_dp);
> -		if (IS_VALLEYVIEW(dev)) {
> +		intel_dp_init_panel_power_timestamps(intel_dp);
> +		if (IS_VALLEYVIEW(dev))
>  			vlv_initial_power_sequencer_setup(intel_dp);
> -		} else {
> -			intel_dp_init_panel_power_timestamps(intel_dp);
> +		else
>  			intel_dp_init_panel_power_sequencer(dev, intel_dp);
> -		}
>  		pps_unlock(intel_dp);
>  	}
>  


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

Re: [PATCH 1/2] drm/i915: Initialize PPS timetamps on vlv/chv

[Daniel Vetter]

On Tue, Oct 28, 2014 at 05:48:34PM +0200, Imre Deak wrote:
> On Tue, 2014-10-28 at 16:15 +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The pps timestamp initialization was accidentally lost on vlv/chv in
> > 
> >  commit a4a5d2f8a96e09844a91469e889f15bd5e927399
> >  Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >  Date:   Thu Sep 4 14:54:20 2014 +0300
> > 
> >     drm/i915: Track which port is using which pipe's power sequencer
> > 
> > Restore it so that we avoid introducing random delays into the pps operations
> > during/after driver init time.
> > 
> > Cc: Imre Deak <imre.deak@intel.com>
> > Reported-by: Imre Deak <imre.deak@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Both patches look good to me:
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Both queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx