diff mbox series

[09/10] drm/i915: Avoid taking the PPS lock for non-eDP/VLV/CHV

Message ID 20190502232648.4450-10-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Add support for asynchronous display power disabling | expand

Commit Message

Imre Deak May 2, 2019, 11:26 p.m. UTC
On ICL we have to make sure that we enable the AUX power domain in a
controlled way (corresponding to the port's actual TypeC mode). Since
the PPS lock - which takes an AUX power ref - is only needed on
eDP/VLV/CHV avoid taking it in other cases.

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

Comments

Ville Syrjälä May 6, 2019, 12:35 p.m. UTC | #1
On Fri, May 03, 2019 at 02:26:47AM +0300, Imre Deak wrote:
> On ICL we have to make sure that we enable the AUX power domain in a
> controlled way (corresponding to the port's actual TypeC mode). Since
> the PPS lock - which takes an AUX power ref - is only needed on
> eDP/VLV/CHV avoid taking it in other cases.
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f56cbda59fb3..1ee9b7ebd801 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -6263,6 +6263,10 @@ void intel_dp_encoder_reset(struct drm_encoder *encoder)
>  
>  	intel_dp->reset_link_params = true;
>  
> +	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv) &&
> +	    !intel_dp_is_edp(intel_dp))
> +		return;

vlv/chv need this for all DP ports.

> +
>  	with_pps_lock(intel_dp, wakeref) {
>  		if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>  			intel_dp->active_pipe = vlv_active_pipe(intel_dp);
> -- 
> 2.17.1
Ville Syrjälä May 6, 2019, 1:12 p.m. UTC | #2
On Mon, May 06, 2019 at 03:35:52PM +0300, Ville Syrjälä wrote:
> On Fri, May 03, 2019 at 02:26:47AM +0300, Imre Deak wrote:
> > On ICL we have to make sure that we enable the AUX power domain in a
> > controlled way (corresponding to the port's actual TypeC mode). Since
> > the PPS lock - which takes an AUX power ref - is only needed on
> > eDP/VLV/CHV avoid taking it in other cases.
> > 
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index f56cbda59fb3..1ee9b7ebd801 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -6263,6 +6263,10 @@ void intel_dp_encoder_reset(struct drm_encoder *encoder)
> >  
> >  	intel_dp->reset_link_params = true;
> >  
> > +	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv) &&
> > +	    !intel_dp_is_edp(intel_dp))
> > +		return;
> 
> vlv/chv need this for all DP ports.

Which is what this does. The wording in the commit message confused me.

> 
> > +
> >  	with_pps_lock(intel_dp, wakeref) {
> >  		if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> >  			intel_dp->active_pipe = vlv_active_pipe(intel_dp);
> > -- 
> > 2.17.1
> 
> -- 
> Ville Syrjälä
> Intel
Imre Deak May 6, 2019, 1:15 p.m. UTC | #3
On Mon, May 06, 2019 at 04:12:50PM +0300, Ville Syrjälä wrote:
> On Mon, May 06, 2019 at 03:35:52PM +0300, Ville Syrjälä wrote:
> > On Fri, May 03, 2019 at 02:26:47AM +0300, Imre Deak wrote:
> > > On ICL we have to make sure that we enable the AUX power domain in a
> > > controlled way (corresponding to the port's actual TypeC mode). Since
> > > the PPS lock - which takes an AUX power ref - is only needed on
> > > eDP/VLV/CHV avoid taking it in other cases.
> > > 
> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index f56cbda59fb3..1ee9b7ebd801 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -6263,6 +6263,10 @@ void intel_dp_encoder_reset(struct drm_encoder *encoder)
> > >  
> > >  	intel_dp->reset_link_params = true;
> > >  
> > > +	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv) &&
> > > +	    !intel_dp_is_edp(intel_dp))
> > > +		return;
> > 
> > vlv/chv need this for all DP ports.
> 
> Which is what this does. The wording in the commit message confused me.

Yep, can make it clearer like:

... only needed on eDP on all platforms and eDP/DP on VLV/CHV, ...

> > 
> > > +
> > >  	with_pps_lock(intel_dp, wakeref) {
> > >  		if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > >  			intel_dp->active_pipe = vlv_active_pipe(intel_dp);
> > > -- 
> > > 2.17.1
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> 
> -- 
> Ville Syrjälä
> Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f56cbda59fb3..1ee9b7ebd801 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6263,6 +6263,10 @@  void intel_dp_encoder_reset(struct drm_encoder *encoder)
 
 	intel_dp->reset_link_params = true;
 
+	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv) &&
+	    !intel_dp_is_edp(intel_dp))
+		return;
+
 	with_pps_lock(intel_dp, wakeref) {
 		if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 			intel_dp->active_pipe = vlv_active_pipe(intel_dp);