diff mbox

[09/10] drm/i915: Use existing power well IDs where possible

Message ID 20180720141504.22832-10-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Imre Deak July 20, 2018, 2:15 p.m. UTC 
There is no need for separate IDs for power wells on a new platform with
the same functionality as an other power well on a previous platform, we
can just reuse the ID from the previous platform. This is only possible
after the previous patches where we removed dependence on the actual
enum values.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         |  3 ---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++------
 2 files changed, 6 insertions(+), 9 deletions(-)

Comments

Zanoni, Paulo R Aug. 2, 2018, 9:39 p.m. UTC | #1 
Em Sex, 2018-07-20 às 17:15 +0300, Imre Deak escreveu:
> There is no need for separate IDs for power wells on a new platform
> with
> the same functionality as an other power well on a previous platform,
> we
> can just reuse the ID from the previous platform. This is only
> possible
> after the previous patches where we removed dependence on the actual
> enum values.
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         |  3 ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++------
>  2 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index b6076f712db5..19b4eac1cc8a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1045,9 +1045,6 @@ enum i915_power_well_id {
>  	SKL_DISP_PW_MISC_IO,
>  	SKL_DISP_PW_1,
>  	SKL_DISP_PW_2,
> -	BXT_DPIO_CMN_BC,
> -	ICL_DISP_PW_1,
> -	ICL_DISP_PW_2,

I mentioned on patch 7 about killing ICL_DISP_PW_2.

Here instead of reusing another ID for it (as the commit title implies)
you just kill it :). Please do it on patch 7 for better organization.

With that:

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>  };
>  
>  #define PUNIT_REG_PWRGT_CTRL			0x60
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 56161d0dc3ca..b7acf54d8a72 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -772,7 +772,7 @@ static void bxt_verify_ddi_phy_power_wells(struct
> drm_i915_private *dev_priv)
>  	if (power_well->count > 0)
>  		bxt_ddi_phy_verify_state(dev_priv, power_well->desc-
> >bxt.phy);
>  
> -	power_well = lookup_power_well(dev_priv, BXT_DPIO_CMN_BC);
> +	power_well = lookup_power_well(dev_priv,
> VLV_DISP_PW_DPIO_CMN_BC);
>  	if (power_well->count > 0)
>  		bxt_ddi_phy_verify_state(dev_priv, power_well->desc-
> >bxt.phy);
>  
> @@ -2456,7 +2456,7 @@ static const struct i915_power_well_desc
> bxt_power_wells[] = {
>  		.name = "dpio-common-bc",
>  		.domains = BXT_DPIO_CMN_BC_POWER_DOMAINS,
>  		.ops = &bxt_dpio_cmn_power_well_ops,
> -		.id = BXT_DPIO_CMN_BC,
> +		.id = VLV_DISP_PW_DPIO_CMN_BC,
>  		{
>  			.bxt.phy = DPIO_PHY0,
>  		},
> @@ -2515,7 +2515,7 @@ static const struct i915_power_well_desc
> glk_power_wells[] = {
>  		.name = "dpio-common-b",
>  		.domains = GLK_DPIO_CMN_B_POWER_DOMAINS,
>  		.ops = &bxt_dpio_cmn_power_well_ops,
> -		.id = BXT_DPIO_CMN_BC,
> +		.id = VLV_DISP_PW_DPIO_CMN_BC,
>  		{
>  			.bxt.phy = DPIO_PHY0,
>  		},
> @@ -2764,7 +2764,7 @@ static const struct i915_power_well_desc
> icl_power_wells[] = {
>  		/* Handled by the DMC firmware */
>  		.domains = 0,
>  		.ops = &hsw_power_well_ops,
> -		.id = ICL_DISP_PW_1,
> +		.id = SKL_DISP_PW_1,
>  		{
>  			.hsw.regs = &hsw_power_well_regs,
>  			.hsw.idx = ICL_PW_CTL_IDX_PW_1,
> @@ -3584,7 +3584,7 @@ static void icl_display_core_init(struct
> drm_i915_private *dev_priv,
>  	 *    The AUX IO power wells will be enabled on demand.
>  	 */
>  	mutex_lock(&power_domains->lock);
> -	well = lookup_power_well(dev_priv, ICL_DISP_PW_1);
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
>  	intel_power_well_enable(dev_priv, well);
>  	mutex_unlock(&power_domains->lock);
>  
> @@ -3625,7 +3625,7 @@ static void icl_display_core_uninit(struct
> drm_i915_private *dev_priv)
>  	 *    disabled at this point.
>  	 */
>  	mutex_lock(&power_domains->lock);
> -	well = lookup_power_well(dev_priv, ICL_DISP_PW_1);
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
>  	intel_power_well_disable(dev_priv, well);
>  	mutex_unlock(&power_domains->lock);
>
Imre Deak Aug. 3, 2018, 9:34 a.m. UTC | #2 
On Thu, Aug 02, 2018 at 02:39:06PM -0700, Paulo Zanoni wrote:
> Em Sex, 2018-07-20 às 17:15 +0300, Imre Deak escreveu:
> > There is no need for separate IDs for power wells on a new platform
> > with
> > the same functionality as an other power well on a previous platform,
> > we
> > can just reuse the ID from the previous platform. This is only
> > possible
> > after the previous patches where we removed dependence on the actual
> > enum values.
> > 
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h         |  3 ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++------
> >  2 files changed, 6 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index b6076f712db5..19b4eac1cc8a 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1045,9 +1045,6 @@ enum i915_power_well_id {
> >  	SKL_DISP_PW_MISC_IO,
> >  	SKL_DISP_PW_1,
> >  	SKL_DISP_PW_2,
> > -	BXT_DPIO_CMN_BC,
> > -	ICL_DISP_PW_1,
> > -	ICL_DISP_PW_2,
> 
> I mentioned on patch 7 about killing ICL_DISP_PW_2.
> 
> Here instead of reusing another ID for it (as the commit title implies)
> you just kill it :). Please do it on patch 7 for better organization.

I'll switch to using SKL_DISP_PW_2 instead as explained earlier.

> 
> With that:
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> >  };
> >  
> >  #define PUNIT_REG_PWRGT_CTRL			0x60
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 56161d0dc3ca..b7acf54d8a72 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -772,7 +772,7 @@ static void bxt_verify_ddi_phy_power_wells(struct
> > drm_i915_private *dev_priv)
> >  	if (power_well->count > 0)
> >  		bxt_ddi_phy_verify_state(dev_priv, power_well->desc-
> > >bxt.phy);
> >  
> > -	power_well = lookup_power_well(dev_priv, BXT_DPIO_CMN_BC);
> > +	power_well = lookup_power_well(dev_priv,
> > VLV_DISP_PW_DPIO_CMN_BC);
> >  	if (power_well->count > 0)
> >  		bxt_ddi_phy_verify_state(dev_priv, power_well->desc-
> > >bxt.phy);
> >  
> > @@ -2456,7 +2456,7 @@ static const struct i915_power_well_desc
> > bxt_power_wells[] = {
> >  		.name = "dpio-common-bc",
> >  		.domains = BXT_DPIO_CMN_BC_POWER_DOMAINS,
> >  		.ops = &bxt_dpio_cmn_power_well_ops,
> > -		.id = BXT_DPIO_CMN_BC,
> > +		.id = VLV_DISP_PW_DPIO_CMN_BC,
> >  		{
> >  			.bxt.phy = DPIO_PHY0,
> >  		},
> > @@ -2515,7 +2515,7 @@ static const struct i915_power_well_desc
> > glk_power_wells[] = {
> >  		.name = "dpio-common-b",
> >  		.domains = GLK_DPIO_CMN_B_POWER_DOMAINS,
> >  		.ops = &bxt_dpio_cmn_power_well_ops,
> > -		.id = BXT_DPIO_CMN_BC,
> > +		.id = VLV_DISP_PW_DPIO_CMN_BC,
> >  		{
> >  			.bxt.phy = DPIO_PHY0,
> >  		},
> > @@ -2764,7 +2764,7 @@ static const struct i915_power_well_desc
> > icl_power_wells[] = {
> >  		/* Handled by the DMC firmware */
> >  		.domains = 0,
> >  		.ops = &hsw_power_well_ops,
> > -		.id = ICL_DISP_PW_1,
> > +		.id = SKL_DISP_PW_1,
> >  		{
> >  			.hsw.regs = &hsw_power_well_regs,
> >  			.hsw.idx = ICL_PW_CTL_IDX_PW_1,
> > @@ -3584,7 +3584,7 @@ static void icl_display_core_init(struct
> > drm_i915_private *dev_priv,
> >  	 *    The AUX IO power wells will be enabled on demand.
> >  	 */
> >  	mutex_lock(&power_domains->lock);
> > -	well = lookup_power_well(dev_priv, ICL_DISP_PW_1);
> > +	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
> >  	intel_power_well_enable(dev_priv, well);
> >  	mutex_unlock(&power_domains->lock);
> >  
> > @@ -3625,7 +3625,7 @@ static void icl_display_core_uninit(struct
> > drm_i915_private *dev_priv)
> >  	 *    disabled at this point.
> >  	 */
> >  	mutex_lock(&power_domains->lock);
> > -	well = lookup_power_well(dev_priv, ICL_DISP_PW_1);
> > +	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
> >  	intel_power_well_disable(dev_priv, well);
> >  	mutex_unlock(&power_domains->lock);
> >
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b6076f712db5..19b4eac1cc8a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1045,9 +1045,6 @@  enum i915_power_well_id {
 	SKL_DISP_PW_MISC_IO,
 	SKL_DISP_PW_1,
 	SKL_DISP_PW_2,
-	BXT_DPIO_CMN_BC,
-	ICL_DISP_PW_1,
-	ICL_DISP_PW_2,
 };
 
 #define PUNIT_REG_PWRGT_CTRL			0x60
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 56161d0dc3ca..b7acf54d8a72 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -772,7 +772,7 @@  static void bxt_verify_ddi_phy_power_wells(struct drm_i915_private *dev_priv)
 	if (power_well->count > 0)
 		bxt_ddi_phy_verify_state(dev_priv, power_well->desc->bxt.phy);
 
-	power_well = lookup_power_well(dev_priv, BXT_DPIO_CMN_BC);
+	power_well = lookup_power_well(dev_priv, VLV_DISP_PW_DPIO_CMN_BC);
 	if (power_well->count > 0)
 		bxt_ddi_phy_verify_state(dev_priv, power_well->desc->bxt.phy);
 
@@ -2456,7 +2456,7 @@  static const struct i915_power_well_desc bxt_power_wells[] = {
 		.name = "dpio-common-bc",
 		.domains = BXT_DPIO_CMN_BC_POWER_DOMAINS,
 		.ops = &bxt_dpio_cmn_power_well_ops,
-		.id = BXT_DPIO_CMN_BC,
+		.id = VLV_DISP_PW_DPIO_CMN_BC,
 		{
 			.bxt.phy = DPIO_PHY0,
 		},
@@ -2515,7 +2515,7 @@  static const struct i915_power_well_desc glk_power_wells[] = {
 		.name = "dpio-common-b",
 		.domains = GLK_DPIO_CMN_B_POWER_DOMAINS,
 		.ops = &bxt_dpio_cmn_power_well_ops,
-		.id = BXT_DPIO_CMN_BC,
+		.id = VLV_DISP_PW_DPIO_CMN_BC,
 		{
 			.bxt.phy = DPIO_PHY0,
 		},
@@ -2764,7 +2764,7 @@  static const struct i915_power_well_desc icl_power_wells[] = {
 		/* Handled by the DMC firmware */
 		.domains = 0,
 		.ops = &hsw_power_well_ops,
-		.id = ICL_DISP_PW_1,
+		.id = SKL_DISP_PW_1,
 		{
 			.hsw.regs = &hsw_power_well_regs,
 			.hsw.idx = ICL_PW_CTL_IDX_PW_1,
@@ -3584,7 +3584,7 @@  static void icl_display_core_init(struct drm_i915_private *dev_priv,
 	 *    The AUX IO power wells will be enabled on demand.
 	 */
 	mutex_lock(&power_domains->lock);
-	well = lookup_power_well(dev_priv, ICL_DISP_PW_1);
+	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
 	intel_power_well_enable(dev_priv, well);
 	mutex_unlock(&power_domains->lock);
 
@@ -3625,7 +3625,7 @@  static void icl_display_core_uninit(struct drm_i915_private *dev_priv)
 	 *    disabled at this point.
 	 */
 	mutex_lock(&power_domains->lock);
-	well = lookup_power_well(dev_priv, ICL_DISP_PW_1);
+	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
 	intel_power_well_disable(dev_priv, well);
 	mutex_unlock(&power_domains->lock);