diff mbox series

[v3,2/8] drm/i915/dp: Ensure panel power remains enabled during connector detection

Message ID 20241016132405.2231744-3-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Write source OUI for non-eDP sinks | expand

Commit Message

Imre Deak Oct. 16, 2024, 1:23 p.m. UTC
The sink's capabilities, like the DSC caps, depend on the source OUI
written to the sink's DPCD registers and so this OUI value should be
valid for the whole duration of the detection. An eDP sink will reset
this OUI value when the panel power is disabled, so prevent the
disabling - happening by default after a 1 sec idle period - for the
whole duration of detection.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c  | 18 +++++++++++++-----
 drivers/gpu/drm/i915/display/intel_pps.c | 11 +++++++++++
 drivers/gpu/drm/i915/display/intel_pps.h |  1 +
 3 files changed, 25 insertions(+), 5 deletions(-)

Comments

Ville Syrjälä Oct. 23, 2024, 12:43 p.m. UTC | #1
On Wed, Oct 16, 2024 at 04:23:59PM +0300, Imre Deak wrote:
> The sink's capabilities, like the DSC caps, depend on the source OUI
> written to the sink's DPCD registers and so this OUI value should be
> valid for the whole duration of the detection. An eDP sink will reset
> this OUI value when the panel power is disabled, so prevent the
> disabling - happening by default after a 1 sec idle period - for the
> whole duration of detection.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_dp.c  | 18 +++++++++++++-----
>  drivers/gpu/drm/i915/display/intel_pps.c | 11 +++++++++++
>  drivers/gpu/drm/i915/display/intel_pps.h |  1 +
>  3 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 977ff2ce18eeb..3da06d25bc4ef 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5616,6 +5616,8 @@ intel_dp_detect(struct drm_connector *connector,
>  
>  	intel_dp_flush_connector_commits(intel_connector);
>  
> +	intel_pps_vdd_on(intel_dp);
> +
>  	/* Can't disconnect eDP */
>  	if (intel_dp_is_edp(intel_dp))
>  		status = edp_detect(intel_dp);
> @@ -5646,12 +5648,15 @@ intel_dp_detect(struct drm_connector *connector,
>  
>  		intel_dp_tunnel_disconnect(intel_dp);
>  
> -		goto out;
> +		goto out_unset_edid;
>  	}
>  
>  	ret = intel_dp_tunnel_detect(intel_dp, ctx);
> -	if (ret == -EDEADLK)
> -		return ret;
> +	if (ret == -EDEADLK) {
> +		status = ret;
> +
> +		goto out_vdd_off;
> +	}
>  
>  	if (ret == 1)
>  		intel_connector->base.epoch_counter++;
> @@ -5679,7 +5684,7 @@ intel_dp_detect(struct drm_connector *connector,
>  		 * with EDID on it
>  		 */
>  		status = connector_status_disconnected;
> -		goto out;
> +		goto out_unset_edid;
>  	}
>  
>  	/*
> @@ -5708,7 +5713,7 @@ intel_dp_detect(struct drm_connector *connector,
>  
>  	intel_dp_check_device_service_irq(intel_dp);
>  
> -out:
> +out_unset_edid:
>  	if (status != connector_status_connected && !intel_dp->is_mst)
>  		intel_dp_unset_edid(intel_dp);
>  
> @@ -5717,6 +5722,9 @@ intel_dp_detect(struct drm_connector *connector,
>  						 status,
>  						 intel_dp->dpcd,
>  						 intel_dp->downstream_ports);
> +out_vdd_off:
> +	intel_pps_vdd_off(intel_dp);
> +
>  	return status;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> index ffeee9daa5689..64f1f7ea94993 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -950,6 +950,17 @@ void intel_pps_vdd_off_unlocked(struct intel_dp *intel_dp, bool sync)
>  		edp_panel_vdd_schedule_off(intel_dp);
>  }
>  
> +void intel_pps_vdd_off(struct intel_dp *intel_dp)
> +{
> +	intel_wakeref_t wakeref;
> +
> +	if (!intel_dp_is_edp(intel_dp))
> +		return;
> +
> +	with_intel_pps_lock(intel_dp, wakeref)
> +		intel_pps_vdd_off_unlocked(intel_dp, false);
> +}
> +
>  void intel_pps_on_unlocked(struct intel_dp *intel_dp)
>  {
>  	struct intel_display *display = to_intel_display(intel_dp);
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.h b/drivers/gpu/drm/i915/display/intel_pps.h
> index bc5046d536264..c83007152f07d 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.h
> +++ b/drivers/gpu/drm/i915/display/intel_pps.h
> @@ -34,6 +34,7 @@ void intel_pps_off_unlocked(struct intel_dp *intel_dp);
>  void intel_pps_check_power_unlocked(struct intel_dp *intel_dp);
>  
>  void intel_pps_vdd_on(struct intel_dp *intel_dp);
> +void intel_pps_vdd_off(struct intel_dp *intel_dp);
>  void intel_pps_on(struct intel_dp *intel_dp);
>  void intel_pps_off(struct intel_dp *intel_dp);
>  void intel_pps_vdd_off_sync(struct intel_dp *intel_dp);
> -- 
> 2.44.2
Jani Nikula Oct. 23, 2024, 3:19 p.m. UTC | #2
On Wed, 16 Oct 2024, Imre Deak <imre.deak@intel.com> wrote:
> The sink's capabilities, like the DSC caps, depend on the source OUI
> written to the sink's DPCD registers and so this OUI value should be
> valid for the whole duration of the detection. An eDP sink will reset
> this OUI value when the panel power is disabled, so prevent the
> disabling - happening by default after a 1 sec idle period - for the
> whole duration of detection.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c  | 18 +++++++++++++-----
>  drivers/gpu/drm/i915/display/intel_pps.c | 11 +++++++++++
>  drivers/gpu/drm/i915/display/intel_pps.h |  1 +
>  3 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 977ff2ce18eeb..3da06d25bc4ef 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5616,6 +5616,8 @@ intel_dp_detect(struct drm_connector *connector,
>  
>  	intel_dp_flush_connector_commits(intel_connector);
>  
> +	intel_pps_vdd_on(intel_dp);
> +

The comment above this one says, "Must be paired with intel_pps_off()."

Needs an update.

BR,
Jani.


>  	/* Can't disconnect eDP */
>  	if (intel_dp_is_edp(intel_dp))
>  		status = edp_detect(intel_dp);
> @@ -5646,12 +5648,15 @@ intel_dp_detect(struct drm_connector *connector,
>  
>  		intel_dp_tunnel_disconnect(intel_dp);
>  
> -		goto out;
> +		goto out_unset_edid;
>  	}
>  
>  	ret = intel_dp_tunnel_detect(intel_dp, ctx);
> -	if (ret == -EDEADLK)
> -		return ret;
> +	if (ret == -EDEADLK) {
> +		status = ret;
> +
> +		goto out_vdd_off;
> +	}
>  
>  	if (ret == 1)
>  		intel_connector->base.epoch_counter++;
> @@ -5679,7 +5684,7 @@ intel_dp_detect(struct drm_connector *connector,
>  		 * with EDID on it
>  		 */
>  		status = connector_status_disconnected;
> -		goto out;
> +		goto out_unset_edid;
>  	}
>  
>  	/*
> @@ -5708,7 +5713,7 @@ intel_dp_detect(struct drm_connector *connector,
>  
>  	intel_dp_check_device_service_irq(intel_dp);
>  
> -out:
> +out_unset_edid:
>  	if (status != connector_status_connected && !intel_dp->is_mst)
>  		intel_dp_unset_edid(intel_dp);
>  
> @@ -5717,6 +5722,9 @@ intel_dp_detect(struct drm_connector *connector,
>  						 status,
>  						 intel_dp->dpcd,
>  						 intel_dp->downstream_ports);
> +out_vdd_off:
> +	intel_pps_vdd_off(intel_dp);
> +
>  	return status;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> index ffeee9daa5689..64f1f7ea94993 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -950,6 +950,17 @@ void intel_pps_vdd_off_unlocked(struct intel_dp *intel_dp, bool sync)
>  		edp_panel_vdd_schedule_off(intel_dp);
>  }
>  
> +void intel_pps_vdd_off(struct intel_dp *intel_dp)
> +{
> +	intel_wakeref_t wakeref;
> +
> +	if (!intel_dp_is_edp(intel_dp))
> +		return;
> +
> +	with_intel_pps_lock(intel_dp, wakeref)
> +		intel_pps_vdd_off_unlocked(intel_dp, false);
> +}
> +
>  void intel_pps_on_unlocked(struct intel_dp *intel_dp)
>  {
>  	struct intel_display *display = to_intel_display(intel_dp);
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.h b/drivers/gpu/drm/i915/display/intel_pps.h
> index bc5046d536264..c83007152f07d 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.h
> +++ b/drivers/gpu/drm/i915/display/intel_pps.h
> @@ -34,6 +34,7 @@ void intel_pps_off_unlocked(struct intel_dp *intel_dp);
>  void intel_pps_check_power_unlocked(struct intel_dp *intel_dp);
>  
>  void intel_pps_vdd_on(struct intel_dp *intel_dp);
> +void intel_pps_vdd_off(struct intel_dp *intel_dp);
>  void intel_pps_on(struct intel_dp *intel_dp);
>  void intel_pps_off(struct intel_dp *intel_dp);
>  void intel_pps_vdd_off_sync(struct intel_dp *intel_dp);
Imre Deak Oct. 23, 2024, 3:48 p.m. UTC | #3
On Wed, Oct 23, 2024 at 06:19:11PM +0300, Jani Nikula wrote:
> On Wed, 16 Oct 2024, Imre Deak <imre.deak@intel.com> wrote:
> > The sink's capabilities, like the DSC caps, depend on the source OUI
> > written to the sink's DPCD registers and so this OUI value should be
> > valid for the whole duration of the detection. An eDP sink will reset
> > this OUI value when the panel power is disabled, so prevent the
> > disabling - happening by default after a 1 sec idle period - for the
> > whole duration of detection.
> >
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c  | 18 +++++++++++++-----
> >  drivers/gpu/drm/i915/display/intel_pps.c | 11 +++++++++++
> >  drivers/gpu/drm/i915/display/intel_pps.h |  1 +
> >  3 files changed, 25 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 977ff2ce18eeb..3da06d25bc4ef 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -5616,6 +5616,8 @@ intel_dp_detect(struct drm_connector *connector,
> >  
> >  	intel_dp_flush_connector_commits(intel_connector);
> >  
> > +	intel_pps_vdd_on(intel_dp);
> > +
> 
> The comment above this one says, "Must be paired with intel_pps_off()."
> 
> Needs an update.

Ok, missed that. Can change it to "Must be paired with
intel_pps_vdd_off() or - to disable both VDD and panel power -
intel_pps_off()". (The locking requirement stays the same.)

> 
> BR,
> Jani.
> 
> 
> >  	/* Can't disconnect eDP */
> >  	if (intel_dp_is_edp(intel_dp))
> >  		status = edp_detect(intel_dp);
> > @@ -5646,12 +5648,15 @@ intel_dp_detect(struct drm_connector *connector,
> >  
> >  		intel_dp_tunnel_disconnect(intel_dp);
> >  
> > -		goto out;
> > +		goto out_unset_edid;
> >  	}
> >  
> >  	ret = intel_dp_tunnel_detect(intel_dp, ctx);
> > -	if (ret == -EDEADLK)
> > -		return ret;
> > +	if (ret == -EDEADLK) {
> > +		status = ret;
> > +
> > +		goto out_vdd_off;
> > +	}
> >  
> >  	if (ret == 1)
> >  		intel_connector->base.epoch_counter++;
> > @@ -5679,7 +5684,7 @@ intel_dp_detect(struct drm_connector *connector,
> >  		 * with EDID on it
> >  		 */
> >  		status = connector_status_disconnected;
> > -		goto out;
> > +		goto out_unset_edid;
> >  	}
> >  
> >  	/*
> > @@ -5708,7 +5713,7 @@ intel_dp_detect(struct drm_connector *connector,
> >  
> >  	intel_dp_check_device_service_irq(intel_dp);
> >  
> > -out:
> > +out_unset_edid:
> >  	if (status != connector_status_connected && !intel_dp->is_mst)
> >  		intel_dp_unset_edid(intel_dp);
> >  
> > @@ -5717,6 +5722,9 @@ intel_dp_detect(struct drm_connector *connector,
> >  						 status,
> >  						 intel_dp->dpcd,
> >  						 intel_dp->downstream_ports);
> > +out_vdd_off:
> > +	intel_pps_vdd_off(intel_dp);
> > +
> >  	return status;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> > index ffeee9daa5689..64f1f7ea94993 100644
> > --- a/drivers/gpu/drm/i915/display/intel_pps.c
> > +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> > @@ -950,6 +950,17 @@ void intel_pps_vdd_off_unlocked(struct intel_dp *intel_dp, bool sync)
> >  		edp_panel_vdd_schedule_off(intel_dp);
> >  }
> >  
> > +void intel_pps_vdd_off(struct intel_dp *intel_dp)
> > +{
> > +	intel_wakeref_t wakeref;
> > +
> > +	if (!intel_dp_is_edp(intel_dp))
> > +		return;
> > +
> > +	with_intel_pps_lock(intel_dp, wakeref)
> > +		intel_pps_vdd_off_unlocked(intel_dp, false);
> > +}
> > +
> >  void intel_pps_on_unlocked(struct intel_dp *intel_dp)
> >  {
> >  	struct intel_display *display = to_intel_display(intel_dp);
> > diff --git a/drivers/gpu/drm/i915/display/intel_pps.h b/drivers/gpu/drm/i915/display/intel_pps.h
> > index bc5046d536264..c83007152f07d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_pps.h
> > +++ b/drivers/gpu/drm/i915/display/intel_pps.h
> > @@ -34,6 +34,7 @@ void intel_pps_off_unlocked(struct intel_dp *intel_dp);
> >  void intel_pps_check_power_unlocked(struct intel_dp *intel_dp);
> >  
> >  void intel_pps_vdd_on(struct intel_dp *intel_dp);
> > +void intel_pps_vdd_off(struct intel_dp *intel_dp);
> >  void intel_pps_on(struct intel_dp *intel_dp);
> >  void intel_pps_off(struct intel_dp *intel_dp);
> >  void intel_pps_vdd_off_sync(struct intel_dp *intel_dp);
> 
> -- 
> Jani Nikula, Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 977ff2ce18eeb..3da06d25bc4ef 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5616,6 +5616,8 @@  intel_dp_detect(struct drm_connector *connector,
 
 	intel_dp_flush_connector_commits(intel_connector);
 
+	intel_pps_vdd_on(intel_dp);
+
 	/* Can't disconnect eDP */
 	if (intel_dp_is_edp(intel_dp))
 		status = edp_detect(intel_dp);
@@ -5646,12 +5648,15 @@  intel_dp_detect(struct drm_connector *connector,
 
 		intel_dp_tunnel_disconnect(intel_dp);
 
-		goto out;
+		goto out_unset_edid;
 	}
 
 	ret = intel_dp_tunnel_detect(intel_dp, ctx);
-	if (ret == -EDEADLK)
-		return ret;
+	if (ret == -EDEADLK) {
+		status = ret;
+
+		goto out_vdd_off;
+	}
 
 	if (ret == 1)
 		intel_connector->base.epoch_counter++;
@@ -5679,7 +5684,7 @@  intel_dp_detect(struct drm_connector *connector,
 		 * with EDID on it
 		 */
 		status = connector_status_disconnected;
-		goto out;
+		goto out_unset_edid;
 	}
 
 	/*
@@ -5708,7 +5713,7 @@  intel_dp_detect(struct drm_connector *connector,
 
 	intel_dp_check_device_service_irq(intel_dp);
 
-out:
+out_unset_edid:
 	if (status != connector_status_connected && !intel_dp->is_mst)
 		intel_dp_unset_edid(intel_dp);
 
@@ -5717,6 +5722,9 @@  intel_dp_detect(struct drm_connector *connector,
 						 status,
 						 intel_dp->dpcd,
 						 intel_dp->downstream_ports);
+out_vdd_off:
+	intel_pps_vdd_off(intel_dp);
+
 	return status;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
index ffeee9daa5689..64f1f7ea94993 100644
--- a/drivers/gpu/drm/i915/display/intel_pps.c
+++ b/drivers/gpu/drm/i915/display/intel_pps.c
@@ -950,6 +950,17 @@  void intel_pps_vdd_off_unlocked(struct intel_dp *intel_dp, bool sync)
 		edp_panel_vdd_schedule_off(intel_dp);
 }
 
+void intel_pps_vdd_off(struct intel_dp *intel_dp)
+{
+	intel_wakeref_t wakeref;
+
+	if (!intel_dp_is_edp(intel_dp))
+		return;
+
+	with_intel_pps_lock(intel_dp, wakeref)
+		intel_pps_vdd_off_unlocked(intel_dp, false);
+}
+
 void intel_pps_on_unlocked(struct intel_dp *intel_dp)
 {
 	struct intel_display *display = to_intel_display(intel_dp);
diff --git a/drivers/gpu/drm/i915/display/intel_pps.h b/drivers/gpu/drm/i915/display/intel_pps.h
index bc5046d536264..c83007152f07d 100644
--- a/drivers/gpu/drm/i915/display/intel_pps.h
+++ b/drivers/gpu/drm/i915/display/intel_pps.h
@@ -34,6 +34,7 @@  void intel_pps_off_unlocked(struct intel_dp *intel_dp);
 void intel_pps_check_power_unlocked(struct intel_dp *intel_dp);
 
 void intel_pps_vdd_on(struct intel_dp *intel_dp);
+void intel_pps_vdd_off(struct intel_dp *intel_dp);
 void intel_pps_on(struct intel_dp *intel_dp);
 void intel_pps_off(struct intel_dp *intel_dp);
 void intel_pps_vdd_off_sync(struct intel_dp *intel_dp);