| Message ID | 1478826234-8002-1-git-send-email-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 11 Nov 2016, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > According to Bspec we need to > "Poll for PORT_REF_DW3_A grc_done == 1b" > only on ports B and C initialization sequence when > copying rcomp from port A. > > So let's follow the spec and only poll for that case > and not on every port A initialization. > > Cc: Imre Deak <imre.deak@intel.com> > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Fixes: ? Cc: stable? > --- > drivers/gpu/drm/i915/intel_dpio_phy.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c > index 7a8e82d..277b1aa 100644 > --- a/drivers/gpu/drm/i915/intel_dpio_phy.c > +++ b/drivers/gpu/drm/i915/intel_dpio_phy.c > @@ -367,6 +367,9 @@ static void _bxt_ddi_phy_init(struct drm_i915_private *dev_priv, > > if (phy_info->rcomp_phy != -1) { > uint32_t grc_code; > + > + bxt_phy_wait_grc_done(dev_priv, phy_info->rcomp_phy); > + > /* > * PHY0 isn't connected to an RCOMP resistor so copy over > * the corresponding calibrated value from PHY1, and disable > @@ -387,10 +390,6 @@ static void _bxt_ddi_phy_init(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); > - > - if (phy_info->rcomp_phy == -1) > - bxt_phy_wait_grc_done(dev_priv, phy); > - > } > > void bxt_ddi_phy_uninit(struct drm_i915_private *dev_priv, enum dpio_phy phy)
On to, 2016-11-10 at 17:03 -0800, Rodrigo Vivi wrote: > According to Bspec we need to > "Poll for PORT_REF_DW3_A grc_done == 1b" > only on ports B and C initialization sequence when > copying rcomp from port A. > > So let's follow the spec and only poll for that case > and not on every port A initialization. > > Cc: Imre Deak <imre.deak@intel.com> > Cc: Ander Conselvan de Oliveira > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> The current code isn't against the spec, we just wait for the calibration to complete earlier. This way we also wait in case only port A is enabled which is imo safer to do before a subsequent modeset on port A. But yes, the spec suggests the HW will handle the wait for this - only port A - case internally, so we can move the wait later to reduce somewhat the init time: Reviewed-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/intel_dpio_phy.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c > index 7a8e82d..277b1aa 100644 > --- a/drivers/gpu/drm/i915/intel_dpio_phy.c > +++ b/drivers/gpu/drm/i915/intel_dpio_phy.c > @@ -367,6 +367,9 @@ static void _bxt_ddi_phy_init(struct drm_i915_private *dev_priv, > > if (phy_info->rcomp_phy != -1) { > uint32_t grc_code; > + > + bxt_phy_wait_grc_done(dev_priv, phy_info->rcomp_phy); > + > /* > * PHY0 isn't connected to an RCOMP resistor so copy over > * the corresponding calibrated value from PHY1, and disable > @@ -387,10 +390,6 @@ static void _bxt_ddi_phy_init(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); > - > - if (phy_info->rcomp_phy == -1) > - bxt_phy_wait_grc_done(dev_priv, phy); > - > } > > void bxt_ddi_phy_uninit(struct drm_i915_private *dev_priv, enum dpio_phy phy)
On Fri, 2016-11-11 at 15:09 +0200, Imre Deak wrote: > On to, 2016-11-10 at 17:03 -0800, Rodrigo Vivi wrote: > > According to Bspec we need to > > "Poll for PORT_REF_DW3_A grc_done == 1b" > > only on ports B and C initialization sequence when > > copying rcomp from port A. > > > > So let's follow the spec and only poll for that case > > and not on every port A initialization. > > > > Cc: Imre Deak <imre.deak@intel.com> > > Cc: Ander Conselvan de Oliveira > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > The current code isn't against the spec, we just wait for the > calibration to complete earlier. This way we also wait in case only > port A is enabled which is imo safer to do before a subsequent modeset > on port A. But yes, the spec suggests the HW will handle the wait for > this - only port A - case internally, so we can move the wait later to > reduce somewhat the init time: > > Reviewed-by: Imre Deak <imre.deak@intel.com> Ops, actually I noticed later that I need to remove the warning block with return false from is_phy_enabled if grc isn't done. otherwise it will force a reprograming without this line there. So, do you believe it is worth to do a v2 removing that block or better to just ignore this patch? > > > --- > > drivers/gpu/drm/i915/intel_dpio_phy.c | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c > > index 7a8e82d..277b1aa 100644 > > --- a/drivers/gpu/drm/i915/intel_dpio_phy.c > > +++ b/drivers/gpu/drm/i915/intel_dpio_phy.c > > @@ -367,6 +367,9 @@ static void _bxt_ddi_phy_init(struct drm_i915_private *dev_priv, > > > > if (phy_info->rcomp_phy != -1) { > > uint32_t grc_code; > > + > > + bxt_phy_wait_grc_done(dev_priv, phy_info->rcomp_phy); > > + > > /* > > * PHY0 isn't connected to an RCOMP resistor so copy over > > * the corresponding calibrated value from PHY1, and disable > > @@ -387,10 +390,6 @@ static void _bxt_ddi_phy_init(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); > > - > > - if (phy_info->rcomp_phy == -1) > > - bxt_phy_wait_grc_done(dev_priv, phy); > > - > > } > > > > void bxt_ddi_phy_uninit(struct drm_i915_private *dev_priv, enum dpio_phy phy)
On ma, 2016-11-14 at 21:51 +0200, Vivi, Rodrigo wrote: > On Fri, 2016-11-11 at 15:09 +0200, Imre Deak wrote: > > On to, 2016-11-10 at 17:03 -0800, Rodrigo Vivi wrote: > > > According to Bspec we need to > > > "Poll for PORT_REF_DW3_A grc_done == 1b" > > > only on ports B and C initialization sequence when > > > copying rcomp from port A. > > > > > > So let's follow the spec and only poll for that case > > > and not on every port A initialization. > > > > > > Cc: Imre Deak <imre.deak@intel.com> > > > Cc: Ander Conselvan de Oliveira > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > The current code isn't against the spec, we just wait for the > > calibration to complete earlier. This way we also wait in case only > > port A is enabled which is imo safer to do before a subsequent > > modeset > > on port A. But yes, the spec suggests the HW will handle the wait > > for > > this - only port A - case internally, so we can move the wait later > > to > > reduce somewhat the init time: > > > > Reviewed-by: Imre Deak <imre.deak@intel.com> > > Ops, actually I noticed later that I need to remove the warning block > with return false from is_phy_enabled if grc isn't done. otherwise it > will force a reprograming without this line there. Yea, missed that too. > > So, do you believe it is worth to do a v2 removing that block or > better > to just ignore this patch? The calibration takes ~2ms on my APL, so I think it's worth reducing that delay. > > > > > > --- > > > drivers/gpu/drm/i915/intel_dpio_phy.c | 7 +++---- > > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c > > > b/drivers/gpu/drm/i915/intel_dpio_phy.c > > > index 7a8e82d..277b1aa 100644 > > > --- a/drivers/gpu/drm/i915/intel_dpio_phy.c > > > +++ b/drivers/gpu/drm/i915/intel_dpio_phy.c > > > @@ -367,6 +367,9 @@ static void _bxt_ddi_phy_init(struct > > > drm_i915_private *dev_priv, > > > > > > if (phy_info->rcomp_phy != -1) { > > > uint32_t grc_code; > > > + > > > + bxt_phy_wait_grc_done(dev_priv, phy_info- > > > >rcomp_phy); > > > + > > > /* > > > * PHY0 isn't connected to an RCOMP resistor so > > > copy over > > > * the corresponding calibrated value from PHY1, > > > and disable > > > @@ -387,10 +390,6 @@ static void _bxt_ddi_phy_init(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); > > > - > > > - if (phy_info->rcomp_phy == -1) > > > - bxt_phy_wait_grc_done(dev_priv, phy); > > > - > > > } > > > > > > void bxt_ddi_phy_uninit(struct drm_i915_private *dev_priv, enum > > > dpio_phy phy) >
diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c index 7a8e82d..277b1aa 100644 --- a/drivers/gpu/drm/i915/intel_dpio_phy.c +++ b/drivers/gpu/drm/i915/intel_dpio_phy.c @@ -367,6 +367,9 @@ static void _bxt_ddi_phy_init(struct drm_i915_private *dev_priv, if (phy_info->rcomp_phy != -1) { uint32_t grc_code; + + bxt_phy_wait_grc_done(dev_priv, phy_info->rcomp_phy); + /* * PHY0 isn't connected to an RCOMP resistor so copy over * the corresponding calibrated value from PHY1, and disable @@ -387,10 +390,6 @@ static void _bxt_ddi_phy_init(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); - - if (phy_info->rcomp_phy == -1) - bxt_phy_wait_grc_done(dev_priv, phy); - } void bxt_ddi_phy_uninit(struct drm_i915_private *dev_priv, enum dpio_phy phy)
According to Bspec we need to "Poll for PORT_REF_DW3_A grc_done == 1b" only on ports B and C initialization sequence when copying rcomp from port A. So let's follow the spec and only poll for that case and not on every port A initialization. Cc: Imre Deak <imre.deak@intel.com> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/intel_dpio_phy.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)