| Message ID | 1459515767-29228-11-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 01 Apr 2016, Imre Deak <imre.deak@intel.com> wrote: > The power-down step logically belongs to the individual PHY uninit > sequence so move it there. The only functional change is that we will > power down now PHY 1 separately before PHY 0 and preserve the other bits > in the register which are defined as reserved. > > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 29017a4..d16effd 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1849,15 +1849,16 @@ static void broxton_phy_uninit(struct drm_i915_private *dev_priv, > val = I915_READ(BXT_PHY_CTL_FAMILY(phy)); > val &= ~COMMON_RESET_DIS; > I915_WRITE(BXT_PHY_CTL_FAMILY(phy), val); > + > + val = I915_READ(BXT_P_CR_GT_DISP_PWRON); > + val &= ~GT_DISPLAY_POWER_ON(phy); > + I915_WRITE(BXT_P_CR_GT_DISP_PWRON, val); > } > > void broxton_ddi_phy_uninit(struct drm_i915_private *dev_priv) > { > broxton_phy_uninit(dev_priv, DPIO_PHY1); > broxton_phy_uninit(dev_priv, DPIO_PHY0); Unrelated to this patch, but since you're hashing stuff around here... The init order is: void broxton_ddi_phy_init(struct drm_device *dev) { /* Enable PHY1 first since it provides Rcomp for PHY0 */ broxton_phy_init(dev->dev_private, DPIO_PHY1); broxton_phy_init(dev->dev_private, DPIO_PHY0); } Should the uninit order be reversed? BR, Jani. > - > - /* FIXME: do this in broxton_phy_uninit per phy */ > - I915_WRITE(BXT_P_CR_GT_DISP_PWRON, 0); > } > > void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
On pe, 2016-04-01 at 16:29 +0300, Jani Nikula wrote: > On Fri, 01 Apr 2016, Imre Deak <imre.deak@intel.com> wrote: > > The power-down step logically belongs to the individual PHY uninit > > sequence so move it there. The only functional change is that we > > will > > power down now PHY 1 separately before PHY 0 and preserve the other > > bits > > in the register which are defined as reserved. > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index 29017a4..d16effd 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1849,15 +1849,16 @@ static void broxton_phy_uninit(struct > > drm_i915_private *dev_priv, > > val = I915_READ(BXT_PHY_CTL_FAMILY(phy)); > > val &= ~COMMON_RESET_DIS; > > I915_WRITE(BXT_PHY_CTL_FAMILY(phy), val); > > + > > + val = I915_READ(BXT_P_CR_GT_DISP_PWRON); > > + val &= ~GT_DISPLAY_POWER_ON(phy); > > + I915_WRITE(BXT_P_CR_GT_DISP_PWRON, val); > > } > > > > void broxton_ddi_phy_uninit(struct drm_i915_private *dev_priv) > > { > > broxton_phy_uninit(dev_priv, DPIO_PHY1); > > broxton_phy_uninit(dev_priv, DPIO_PHY0); > > Unrelated to this patch, but since you're hashing stuff around > here... > > The init order is: > > void broxton_ddi_phy_init(struct drm_device *dev) > { > /* Enable PHY1 first since it provides Rcomp for PHY0 */ > broxton_phy_init(dev->dev_private, DPIO_PHY1); > broxton_phy_init(dev->dev_private, DPIO_PHY0); > } > > Should the uninit order be reversed? That would be logical, but the bspec specifies this uninit order. The init order is also fixed in the above way but it's understandable why. --Imre
On Fri, Apr 01, 2016 at 04:02:41PM +0300, Imre Deak wrote: > The power-down step logically belongs to the individual PHY uninit > sequence so move it there. The only functional change is that we will > power down now PHY 1 separately before PHY 0 and preserve the other bits > in the register which are defined as reserved. > > Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 29017a4..d16effd 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1849,15 +1849,16 @@ static void broxton_phy_uninit(struct drm_i915_private *dev_priv, > val = I915_READ(BXT_PHY_CTL_FAMILY(phy)); > val &= ~COMMON_RESET_DIS; > I915_WRITE(BXT_PHY_CTL_FAMILY(phy), val); > + > + val = I915_READ(BXT_P_CR_GT_DISP_PWRON); > + val &= ~GT_DISPLAY_POWER_ON(phy); > + I915_WRITE(BXT_P_CR_GT_DISP_PWRON, val); > } > > void broxton_ddi_phy_uninit(struct drm_i915_private *dev_priv) > { > broxton_phy_uninit(dev_priv, DPIO_PHY1); > broxton_phy_uninit(dev_priv, DPIO_PHY0); > - > - /* FIXME: do this in broxton_phy_uninit per phy */ > - I915_WRITE(BXT_P_CR_GT_DISP_PWRON, 0); > } > > void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp) > -- > 2.5.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 29017a4..d16effd 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1849,15 +1849,16 @@ static void broxton_phy_uninit(struct drm_i915_private *dev_priv, val = I915_READ(BXT_PHY_CTL_FAMILY(phy)); val &= ~COMMON_RESET_DIS; I915_WRITE(BXT_PHY_CTL_FAMILY(phy), val); + + val = I915_READ(BXT_P_CR_GT_DISP_PWRON); + val &= ~GT_DISPLAY_POWER_ON(phy); + I915_WRITE(BXT_P_CR_GT_DISP_PWRON, val); } void broxton_ddi_phy_uninit(struct drm_i915_private *dev_priv) { broxton_phy_uninit(dev_priv, DPIO_PHY1); broxton_phy_uninit(dev_priv, DPIO_PHY0); - - /* FIXME: do this in broxton_phy_uninit per phy */ - I915_WRITE(BXT_P_CR_GT_DISP_PWRON, 0); } void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
The power-down step logically belongs to the individual PHY uninit sequence so move it there. The only functional change is that we will power down now PHY 1 separately before PHY 0 and preserve the other bits in the register which are defined as reserved. Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/intel_ddi.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)