| Message ID | 20210106043438.27754-1-anshuman.gupta@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,v2] drm/i915/pps: Add PPS power domain | expand |
On Wed, Jan 06, 2021 at 10:04:38AM +0530, Anshuman Gupta wrote: > It abstracts getting the AUX power domain in pps_lock under > PPS power domain. This makes sure that the platforms which really > requires AUX power in order to access PPS registers will get the > reference to necessary power wells. > > PPS power domain requires only to track the AUX_A associated > power wells as the platforms need AUX power in order to access PPS > registers supports eDP only on PORT_A. > > v2: > - Fixed missed POWER_DOMAIN_PPS in pps_unlock(). > > Cc: Imre Deak <imre.deak@intel.com> > Cc: Jani Nikula <jani.nikula@intel.com> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display_power.c | 7 +++++++ > drivers/gpu/drm/i915/display/intel_display_power.h | 1 + > drivers/gpu/drm/i915/display/intel_dp.c | 7 ++----- > 3 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index d52374f01316..1dc4ca9e5d1a 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -107,6 +107,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) > return "VGA"; > case POWER_DOMAIN_AUDIO: > return "AUDIO"; > + case POWER_DOMAIN_PPS: > + return "PPS"; > case POWER_DOMAIN_AUX_A: > return "AUX_A"; > case POWER_DOMAIN_AUX_B: > @@ -2651,11 +2653,13 @@ intel_display_power_put_mask_in_set(struct drm_i915_private *i915, > BIT_ULL(POWER_DOMAIN_GT_IRQ) | \ > BIT_ULL(POWER_DOMAIN_MODESET) | \ > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > + BIT_ULL(POWER_DOMAIN_PPS) | \ > BIT_ULL(POWER_DOMAIN_GMBUS) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > #define BXT_DPIO_CMN_A_POWER_DOMAINS ( \ > BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) | \ > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > + BIT_ULL(POWER_DOMAIN_PPS) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > #define BXT_DPIO_CMN_BC_POWER_DOMAINS ( \ > BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) | \ > @@ -2688,6 +2692,7 @@ intel_display_power_put_mask_in_set(struct drm_i915_private *i915, > #define GLK_DPIO_CMN_A_POWER_DOMAINS ( \ > BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) | \ > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > + BIT_ULL(POWER_DOMAIN_PPS) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > #define GLK_DPIO_CMN_B_POWER_DOMAINS ( \ > BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) | \ > @@ -2700,6 +2705,7 @@ intel_display_power_put_mask_in_set(struct drm_i915_private *i915, > #define GLK_DISPLAY_AUX_A_POWER_DOMAINS ( \ > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \ > + BIT_ULL(POWER_DOMAIN_PPS) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > #define GLK_DISPLAY_AUX_B_POWER_DOMAINS ( \ > BIT_ULL(POWER_DOMAIN_AUX_B) | \ > @@ -2712,6 +2718,7 @@ intel_display_power_put_mask_in_set(struct drm_i915_private *i915, > BIT_ULL(POWER_DOMAIN_GT_IRQ) | \ > BIT_ULL(POWER_DOMAIN_MODESET) | \ > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > + BIT_ULL(POWER_DOMAIN_PPS) | \ > BIT_ULL(POWER_DOMAIN_GMBUS) | \ > BIT_ULL(POWER_DOMAIN_INIT)) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h > index bc30c479be53..7642be3c8e2e 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > @@ -55,6 +55,7 @@ enum intel_display_power_domain { > POWER_DOMAIN_PORT_OTHER, > POWER_DOMAIN_VGA, > POWER_DOMAIN_AUDIO, > + POWER_DOMAIN_PPS, > POWER_DOMAIN_AUX_A, > POWER_DOMAIN_AUX_B, > POWER_DOMAIN_AUX_C, > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 8a00e609085f..99b4bec3c926 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -895,8 +895,7 @@ pps_lock(struct intel_dp *intel_dp) > * See intel_power_sequencer_reset() why we need > * a power domain reference here. > */ > - wakeref = intel_display_power_get(dev_priv, > - intel_aux_power_domain(dp_to_dig_port(intel_dp))); > + wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_PPS); The purpose of this reference is to prevent a race when resetting the PPS state (see the comment), so I think we could reuse POWER_DOMAIN_DISPLAY_CORE instead of adding a new domain. > > mutex_lock(&dev_priv->pps_mutex); > > @@ -909,9 +908,7 @@ pps_unlock(struct intel_dp *intel_dp, intel_wakeref_t wakeref) > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > mutex_unlock(&dev_priv->pps_mutex); > - intel_display_power_put(dev_priv, > - intel_aux_power_domain(dp_to_dig_port(intel_dp)), > - wakeref); > + intel_display_power_put(dev_priv, POWER_DOMAIN_PPS, wakeref); > return 0; > } > > -- > 2.26.2 >
> -----Original Message----- > From: Imre Deak <imre.deak@intel.com> > Sent: Wednesday, January 6, 2021 9:33 PM > To: Gupta, Anshuman <anshuman.gupta@intel.com> > Cc: intel-gfx@lists.freedesktop.org; Mason, Michael W > <michael.w.mason@intel.com>; Nikula, Jani <jani.nikula@intel.com>; Ville > Syrjälä <ville.syrjala@linux.intel.com> > Subject: Re: [RFC v2] drm/i915/pps: Add PPS power domain > > On Wed, Jan 06, 2021 at 10:04:38AM +0530, Anshuman Gupta wrote: > > It abstracts getting the AUX power domain in pps_lock under PPS power > > domain. This makes sure that the platforms which really requires AUX > > power in order to access PPS registers will get the reference to > > necessary power wells. > > > > PPS power domain requires only to track the AUX_A associated power > > wells as the platforms need AUX power in order to access PPS registers > > supports eDP only on PORT_A. > > > > v2: > > - Fixed missed POWER_DOMAIN_PPS in pps_unlock(). > > > > Cc: Imre Deak <imre.deak@intel.com> > > Cc: Jani Nikula <jani.nikula@intel.com> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display_power.c | 7 +++++++ > > drivers/gpu/drm/i915/display/intel_display_power.h | 1 + > > drivers/gpu/drm/i915/display/intel_dp.c | 7 ++----- > > 3 files changed, 10 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c > > b/drivers/gpu/drm/i915/display/intel_display_power.c > > index d52374f01316..1dc4ca9e5d1a 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > > @@ -107,6 +107,8 @@ intel_display_power_domain_str(enum > intel_display_power_domain domain) > > return "VGA"; > > case POWER_DOMAIN_AUDIO: > > return "AUDIO"; > > + case POWER_DOMAIN_PPS: > > + return "PPS"; > > case POWER_DOMAIN_AUX_A: > > return "AUX_A"; > > case POWER_DOMAIN_AUX_B: > > @@ -2651,11 +2653,13 @@ > intel_display_power_put_mask_in_set(struct drm_i915_private *i915, > > BIT_ULL(POWER_DOMAIN_GT_IRQ) | \ > > BIT_ULL(POWER_DOMAIN_MODESET) | \ > > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > > + BIT_ULL(POWER_DOMAIN_PPS) | \ > > BIT_ULL(POWER_DOMAIN_GMBUS) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > #define BXT_DPIO_CMN_A_POWER_DOMAINS ( \ > > BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) | \ > > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > > + BIT_ULL(POWER_DOMAIN_PPS) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > #define BXT_DPIO_CMN_BC_POWER_DOMAINS ( \ > > BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) | \ > > @@ -2688,6 +2692,7 @@ intel_display_power_put_mask_in_set(struct > drm_i915_private *i915, > > #define GLK_DPIO_CMN_A_POWER_DOMAINS ( \ > > BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) | \ > > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > > + BIT_ULL(POWER_DOMAIN_PPS) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > #define GLK_DPIO_CMN_B_POWER_DOMAINS ( \ > > BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) | \ > > @@ -2700,6 +2705,7 @@ intel_display_power_put_mask_in_set(struct > drm_i915_private *i915, > > #define GLK_DISPLAY_AUX_A_POWER_DOMAINS ( \ > > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > > BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \ > > + BIT_ULL(POWER_DOMAIN_PPS) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > #define GLK_DISPLAY_AUX_B_POWER_DOMAINS ( \ > > BIT_ULL(POWER_DOMAIN_AUX_B) | \ > > @@ -2712,6 +2718,7 @@ intel_display_power_put_mask_in_set(struct > drm_i915_private *i915, > > BIT_ULL(POWER_DOMAIN_GT_IRQ) | \ > > BIT_ULL(POWER_DOMAIN_MODESET) | \ > > BIT_ULL(POWER_DOMAIN_AUX_A) | \ > > + BIT_ULL(POWER_DOMAIN_PPS) | \ > > BIT_ULL(POWER_DOMAIN_GMBUS) | \ > > BIT_ULL(POWER_DOMAIN_INIT)) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h > > b/drivers/gpu/drm/i915/display/intel_display_power.h > > index bc30c479be53..7642be3c8e2e 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > > @@ -55,6 +55,7 @@ enum intel_display_power_domain { > > POWER_DOMAIN_PORT_OTHER, > > POWER_DOMAIN_VGA, > > POWER_DOMAIN_AUDIO, > > + POWER_DOMAIN_PPS, > > POWER_DOMAIN_AUX_A, > > POWER_DOMAIN_AUX_B, > > POWER_DOMAIN_AUX_C, > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index 8a00e609085f..99b4bec3c926 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -895,8 +895,7 @@ pps_lock(struct intel_dp *intel_dp) > > * See intel_power_sequencer_reset() why we need > > * a power domain reference here. > > */ > > - wakeref = intel_display_power_get(dev_priv, > > - > intel_aux_power_domain(dp_to_dig_port(intel_dp))); > > + wakeref = intel_display_power_get(dev_priv, > POWER_DOMAIN_PPS); > > The purpose of this reference is to prevent a race when resetting the PPS > state (see the comment), so I think we could reuse > POWER_DOMAIN_DISPLAY_CORE instead of adding a new domain. Thanks Imre for review, I will send a new patch with the fix. Thanks, Anshuman Gupta. > > > > > mutex_lock(&dev_priv->pps_mutex); > > > > @@ -909,9 +908,7 @@ pps_unlock(struct intel_dp *intel_dp, > intel_wakeref_t wakeref) > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > > > mutex_unlock(&dev_priv->pps_mutex); > > - intel_display_power_put(dev_priv, > > - > intel_aux_power_domain(dp_to_dig_port(intel_dp)), > > - wakeref); > > + intel_display_power_put(dev_priv, POWER_DOMAIN_PPS, > wakeref); > > return 0; > > } > > > > -- > > 2.26.2 > >
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c index d52374f01316..1dc4ca9e5d1a 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.c +++ b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -107,6 +107,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain) return "VGA"; case POWER_DOMAIN_AUDIO: return "AUDIO"; + case POWER_DOMAIN_PPS: + return "PPS"; case POWER_DOMAIN_AUX_A: return "AUX_A"; case POWER_DOMAIN_AUX_B: @@ -2651,11 +2653,13 @@ intel_display_power_put_mask_in_set(struct drm_i915_private *i915, BIT_ULL(POWER_DOMAIN_GT_IRQ) | \ BIT_ULL(POWER_DOMAIN_MODESET) | \ BIT_ULL(POWER_DOMAIN_AUX_A) | \ + BIT_ULL(POWER_DOMAIN_PPS) | \ BIT_ULL(POWER_DOMAIN_GMBUS) | \ BIT_ULL(POWER_DOMAIN_INIT)) #define BXT_DPIO_CMN_A_POWER_DOMAINS ( \ BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) | \ BIT_ULL(POWER_DOMAIN_AUX_A) | \ + BIT_ULL(POWER_DOMAIN_PPS) | \ BIT_ULL(POWER_DOMAIN_INIT)) #define BXT_DPIO_CMN_BC_POWER_DOMAINS ( \ BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) | \ @@ -2688,6 +2692,7 @@ intel_display_power_put_mask_in_set(struct drm_i915_private *i915, #define GLK_DPIO_CMN_A_POWER_DOMAINS ( \ BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) | \ BIT_ULL(POWER_DOMAIN_AUX_A) | \ + BIT_ULL(POWER_DOMAIN_PPS) | \ BIT_ULL(POWER_DOMAIN_INIT)) #define GLK_DPIO_CMN_B_POWER_DOMAINS ( \ BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) | \ @@ -2700,6 +2705,7 @@ intel_display_power_put_mask_in_set(struct drm_i915_private *i915, #define GLK_DISPLAY_AUX_A_POWER_DOMAINS ( \ BIT_ULL(POWER_DOMAIN_AUX_A) | \ BIT_ULL(POWER_DOMAIN_AUX_IO_A) | \ + BIT_ULL(POWER_DOMAIN_PPS) | \ BIT_ULL(POWER_DOMAIN_INIT)) #define GLK_DISPLAY_AUX_B_POWER_DOMAINS ( \ BIT_ULL(POWER_DOMAIN_AUX_B) | \ @@ -2712,6 +2718,7 @@ intel_display_power_put_mask_in_set(struct drm_i915_private *i915, BIT_ULL(POWER_DOMAIN_GT_IRQ) | \ BIT_ULL(POWER_DOMAIN_MODESET) | \ BIT_ULL(POWER_DOMAIN_AUX_A) | \ + BIT_ULL(POWER_DOMAIN_PPS) | \ BIT_ULL(POWER_DOMAIN_GMBUS) | \ BIT_ULL(POWER_DOMAIN_INIT)) diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h index bc30c479be53..7642be3c8e2e 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.h +++ b/drivers/gpu/drm/i915/display/intel_display_power.h @@ -55,6 +55,7 @@ enum intel_display_power_domain { POWER_DOMAIN_PORT_OTHER, POWER_DOMAIN_VGA, POWER_DOMAIN_AUDIO, + POWER_DOMAIN_PPS, POWER_DOMAIN_AUX_A, POWER_DOMAIN_AUX_B, POWER_DOMAIN_AUX_C, diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 8a00e609085f..99b4bec3c926 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -895,8 +895,7 @@ pps_lock(struct intel_dp *intel_dp) * See intel_power_sequencer_reset() why we need * a power domain reference here. */ - wakeref = intel_display_power_get(dev_priv, - intel_aux_power_domain(dp_to_dig_port(intel_dp))); + wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_PPS); mutex_lock(&dev_priv->pps_mutex); @@ -909,9 +908,7 @@ pps_unlock(struct intel_dp *intel_dp, intel_wakeref_t wakeref) struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); mutex_unlock(&dev_priv->pps_mutex); - intel_display_power_put(dev_priv, - intel_aux_power_domain(dp_to_dig_port(intel_dp)), - wakeref); + intel_display_power_put(dev_priv, POWER_DOMAIN_PPS, wakeref); return 0; }
It abstracts getting the AUX power domain in pps_lock under PPS power domain. This makes sure that the platforms which really requires AUX power in order to access PPS registers will get the reference to necessary power wells. PPS power domain requires only to track the AUX_A associated power wells as the platforms need AUX power in order to access PPS registers supports eDP only on PORT_A. v2: - Fixed missed POWER_DOMAIN_PPS in pps_unlock(). Cc: Imre Deak <imre.deak@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> --- drivers/gpu/drm/i915/display/intel_display_power.c | 7 +++++++ drivers/gpu/drm/i915/display/intel_display_power.h | 1 + drivers/gpu/drm/i915/display/intel_dp.c | 7 ++----- 3 files changed, 10 insertions(+), 5 deletions(-)