| Message ID | 1439394260-15137-5-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Make LPT:LP checks look neater by wrapping the details in a > new HAS_PCH_LPT_LP() macro. This has the potential to be confusing since HAS_PCH_LPT() is also true for cases where HAS_PCH_LPT_LP() is true. Maybe we could rename the macro in some way that it wouldn't be HAS_PCH_XXX in order to reduce the probability of confusion? But I can't think of any suggestions... Anyway, the patch is technically correct, so I'll let the decision to you and Daniel. Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_display.c | 13 +++++-------- > drivers/gpu/drm/i915/intel_pm.c | 4 ++-- > 3 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 55611d8..4e391dd 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2573,6 +2573,7 @@ struct drm_i915_cmd_table { > #define INTEL_PCH_TYPE(dev) (__I915__(dev)->pch_type) > #define HAS_PCH_SPT(dev) (INTEL_PCH_TYPE(dev) == PCH_SPT) > #define HAS_PCH_LPT(dev) (INTEL_PCH_TYPE(dev) == PCH_LPT) > +#define HAS_PCH_LPT_LP(dev) (__I915__(dev)->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) > #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT) > #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX) > #define HAS_PCH_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP) > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 4b3012b..97c6368 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8381,8 +8381,7 @@ static void lpt_enable_clkout_dp(struct drm_device *dev, bool with_spread, > > if (WARN(with_fdi && !with_spread, "FDI requires downspread\n")) > with_spread = true; > - if (WARN(dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE && > - with_fdi, "LP PCH doesn't have FDI\n")) > + if (WARN(HAS_PCH_LPT_LP(dev) && with_fdi, "LP PCH doesn't have FDI\n")) > with_fdi = false; > > mutex_lock(&dev_priv->sb_lock); > @@ -8405,8 +8404,7 @@ static void lpt_enable_clkout_dp(struct drm_device *dev, bool with_spread, > } > } > > - reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ? > - SBI_GEN0 : SBI_DBUFF0; > + reg = HAS_PCH_LPT_LP(dev) ? SBI_GEN0 : SBI_DBUFF0; > tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK); > tmp |= SBI_GEN0_CFG_BUFFENABLE_DISABLE; > intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK); > @@ -8422,8 +8420,7 @@ static void lpt_disable_clkout_dp(struct drm_device *dev) > > mutex_lock(&dev_priv->sb_lock); > > - reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ? > - SBI_GEN0 : SBI_DBUFF0; > + reg = HAS_PCH_LPT_LP(dev) ? SBI_GEN0 : SBI_DBUFF0; > tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK); > tmp &= ~SBI_GEN0_CFG_BUFFENABLE_DISABLE; > intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK); > @@ -9435,7 +9432,7 @@ void hsw_enable_pc8(struct drm_i915_private *dev_priv) > > DRM_DEBUG_KMS("Enabling package C8+\n"); > > - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) { > + if (HAS_PCH_LPT_LP(dev)) { > val = I915_READ(SOUTH_DSPCLK_GATE_D); > val &= ~PCH_LP_PARTITION_LEVEL_DISABLE; > I915_WRITE(SOUTH_DSPCLK_GATE_D, val); > @@ -9455,7 +9452,7 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv) > hsw_restore_lcpll(dev_priv); > lpt_init_pch_refclk(dev); > > - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) { > + if (HAS_PCH_LPT_LP(dev)) { > val = I915_READ(SOUTH_DSPCLK_GATE_D); > val |= PCH_LP_PARTITION_LEVEL_DISABLE; > I915_WRITE(SOUTH_DSPCLK_GATE_D, val); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index fff0c226..ea49661 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -6588,7 +6588,7 @@ static void lpt_init_clock_gating(struct drm_device *dev) > * TODO: this bit should only be enabled when really needed, then > * disabled when not needed anymore in order to save power. > */ > - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) > + if (HAS_PCH_LPT_LP(dev)) > I915_WRITE(SOUTH_DSPCLK_GATE_D, > I915_READ(SOUTH_DSPCLK_GATE_D) | > PCH_LP_PARTITION_LEVEL_DISABLE); > @@ -6603,7 +6603,7 @@ static void lpt_suspend_hw(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > > - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) { > + if (HAS_PCH_LPT_LP(dev)) { > uint32_t val = I915_READ(SOUTH_DSPCLK_GATE_D); > > val &= ~PCH_LP_PARTITION_LEVEL_DISABLE; > -- > 2.4.6 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Aug 26, 2015 at 03:58:11PM -0300, Paulo Zanoni wrote: > 2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Make LPT:LP checks look neater by wrapping the details in a > > new HAS_PCH_LPT_LP() macro. > > This has the potential to be confusing since HAS_PCH_LPT() is also > true for cases where HAS_PCH_LPT_LP() is true. Maybe we could rename > the macro in some way that it wouldn't be HAS_PCH_XXX in order to > reduce the probability of confusion? But I can't think of any > suggestions... We could rename HAS_PCH_LPT to HAS_PCH_LPT_LP_OR_H but that's just too ugly IMO. I guess it might be clearer if we also had HAS_PCH_LPT_H but since we have not use for both adding one is a bit dubious. So no nice ideas here either. > > Anyway, the patch is technically correct, so I'll let the decision to > you and Daniel. > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > drivers/gpu/drm/i915/intel_display.c | 13 +++++-------- > > drivers/gpu/drm/i915/intel_pm.c | 4 ++-- > > 3 files changed, 8 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 55611d8..4e391dd 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -2573,6 +2573,7 @@ struct drm_i915_cmd_table { > > #define INTEL_PCH_TYPE(dev) (__I915__(dev)->pch_type) > > #define HAS_PCH_SPT(dev) (INTEL_PCH_TYPE(dev) == PCH_SPT) > > #define HAS_PCH_LPT(dev) (INTEL_PCH_TYPE(dev) == PCH_LPT) > > +#define HAS_PCH_LPT_LP(dev) (__I915__(dev)->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) > > #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT) > > #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX) > > #define HAS_PCH_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 4b3012b..97c6368 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -8381,8 +8381,7 @@ static void lpt_enable_clkout_dp(struct drm_device *dev, bool with_spread, > > > > if (WARN(with_fdi && !with_spread, "FDI requires downspread\n")) > > with_spread = true; > > - if (WARN(dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE && > > - with_fdi, "LP PCH doesn't have FDI\n")) > > + if (WARN(HAS_PCH_LPT_LP(dev) && with_fdi, "LP PCH doesn't have FDI\n")) > > with_fdi = false; > > > > mutex_lock(&dev_priv->sb_lock); > > @@ -8405,8 +8404,7 @@ static void lpt_enable_clkout_dp(struct drm_device *dev, bool with_spread, > > } > > } > > > > - reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ? > > - SBI_GEN0 : SBI_DBUFF0; > > + reg = HAS_PCH_LPT_LP(dev) ? SBI_GEN0 : SBI_DBUFF0; > > tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK); > > tmp |= SBI_GEN0_CFG_BUFFENABLE_DISABLE; > > intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK); > > @@ -8422,8 +8420,7 @@ static void lpt_disable_clkout_dp(struct drm_device *dev) > > > > mutex_lock(&dev_priv->sb_lock); > > > > - reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ? > > - SBI_GEN0 : SBI_DBUFF0; > > + reg = HAS_PCH_LPT_LP(dev) ? SBI_GEN0 : SBI_DBUFF0; > > tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK); > > tmp &= ~SBI_GEN0_CFG_BUFFENABLE_DISABLE; > > intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK); > > @@ -9435,7 +9432,7 @@ void hsw_enable_pc8(struct drm_i915_private *dev_priv) > > > > DRM_DEBUG_KMS("Enabling package C8+\n"); > > > > - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) { > > + if (HAS_PCH_LPT_LP(dev)) { > > val = I915_READ(SOUTH_DSPCLK_GATE_D); > > val &= ~PCH_LP_PARTITION_LEVEL_DISABLE; > > I915_WRITE(SOUTH_DSPCLK_GATE_D, val); > > @@ -9455,7 +9452,7 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv) > > hsw_restore_lcpll(dev_priv); > > lpt_init_pch_refclk(dev); > > > > - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) { > > + if (HAS_PCH_LPT_LP(dev)) { > > val = I915_READ(SOUTH_DSPCLK_GATE_D); > > val |= PCH_LP_PARTITION_LEVEL_DISABLE; > > I915_WRITE(SOUTH_DSPCLK_GATE_D, val); > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index fff0c226..ea49661 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -6588,7 +6588,7 @@ static void lpt_init_clock_gating(struct drm_device *dev) > > * TODO: this bit should only be enabled when really needed, then > > * disabled when not needed anymore in order to save power. > > */ > > - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) > > + if (HAS_PCH_LPT_LP(dev)) > > I915_WRITE(SOUTH_DSPCLK_GATE_D, > > I915_READ(SOUTH_DSPCLK_GATE_D) | > > PCH_LP_PARTITION_LEVEL_DISABLE); > > @@ -6603,7 +6603,7 @@ static void lpt_suspend_hw(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > > > - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) { > > + if (HAS_PCH_LPT_LP(dev)) { > > uint32_t val = I915_READ(SOUTH_DSPCLK_GATE_D); > > > > val &= ~PCH_LP_PARTITION_LEVEL_DISABLE; > > -- > > 2.4.6 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 55611d8..4e391dd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2573,6 +2573,7 @@ struct drm_i915_cmd_table { #define INTEL_PCH_TYPE(dev) (__I915__(dev)->pch_type) #define HAS_PCH_SPT(dev) (INTEL_PCH_TYPE(dev) == PCH_SPT) #define HAS_PCH_LPT(dev) (INTEL_PCH_TYPE(dev) == PCH_LPT) +#define HAS_PCH_LPT_LP(dev) (__I915__(dev)->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT) #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX) #define HAS_PCH_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4b3012b..97c6368 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8381,8 +8381,7 @@ static void lpt_enable_clkout_dp(struct drm_device *dev, bool with_spread, if (WARN(with_fdi && !with_spread, "FDI requires downspread\n")) with_spread = true; - if (WARN(dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE && - with_fdi, "LP PCH doesn't have FDI\n")) + if (WARN(HAS_PCH_LPT_LP(dev) && with_fdi, "LP PCH doesn't have FDI\n")) with_fdi = false; mutex_lock(&dev_priv->sb_lock); @@ -8405,8 +8404,7 @@ static void lpt_enable_clkout_dp(struct drm_device *dev, bool with_spread, } } - reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ? - SBI_GEN0 : SBI_DBUFF0; + reg = HAS_PCH_LPT_LP(dev) ? SBI_GEN0 : SBI_DBUFF0; tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK); tmp |= SBI_GEN0_CFG_BUFFENABLE_DISABLE; intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK); @@ -8422,8 +8420,7 @@ static void lpt_disable_clkout_dp(struct drm_device *dev) mutex_lock(&dev_priv->sb_lock); - reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ? - SBI_GEN0 : SBI_DBUFF0; + reg = HAS_PCH_LPT_LP(dev) ? SBI_GEN0 : SBI_DBUFF0; tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK); tmp &= ~SBI_GEN0_CFG_BUFFENABLE_DISABLE; intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK); @@ -9435,7 +9432,7 @@ void hsw_enable_pc8(struct drm_i915_private *dev_priv) DRM_DEBUG_KMS("Enabling package C8+\n"); - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) { + if (HAS_PCH_LPT_LP(dev)) { val = I915_READ(SOUTH_DSPCLK_GATE_D); val &= ~PCH_LP_PARTITION_LEVEL_DISABLE; I915_WRITE(SOUTH_DSPCLK_GATE_D, val); @@ -9455,7 +9452,7 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv) hsw_restore_lcpll(dev_priv); lpt_init_pch_refclk(dev); - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) { + if (HAS_PCH_LPT_LP(dev)) { val = I915_READ(SOUTH_DSPCLK_GATE_D); val |= PCH_LP_PARTITION_LEVEL_DISABLE; I915_WRITE(SOUTH_DSPCLK_GATE_D, val); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index fff0c226..ea49661 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6588,7 +6588,7 @@ static void lpt_init_clock_gating(struct drm_device *dev) * TODO: this bit should only be enabled when really needed, then * disabled when not needed anymore in order to save power. */ - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) + if (HAS_PCH_LPT_LP(dev)) I915_WRITE(SOUTH_DSPCLK_GATE_D, I915_READ(SOUTH_DSPCLK_GATE_D) | PCH_LP_PARTITION_LEVEL_DISABLE); @@ -6603,7 +6603,7 @@ static void lpt_suspend_hw(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) { + if (HAS_PCH_LPT_LP(dev)) { uint32_t val = I915_READ(SOUTH_DSPCLK_GATE_D); val &= ~PCH_LP_PARTITION_LEVEL_DISABLE;