Re: [Intel-gfx] [PATCH] drm/i915: Kill the remaining CHV HBR2 leftovers

[Pandiyan, Dhinakaran]

On Thu, 2018-03-08 at 14:58 +0200, Ville Syrjälä wrote:
> On Wed, Mar 07, 2018 at 09:41:06PM +, Pandiyan, Dhinakaran wrote:
> > 
> > 
> > 
> > On Fri, 2018-03-02 at 11:56 +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä 
> > > 
> > > AFAIK CHV was supposed to have HBR2 originally, but in the end the feature
> > > was dropped. We still have some code leftovers from those early days.
> > > Eliminate them.
> > > 
> > 
> > Not much in the spec about HBR2 other than the support for TP3. Since we
> > don't support HBR2 on CHV, removing the unused TPS3 bits looks correct.
> > 
> > 
> > 
> > > The extra bit for the training pattern seems to be dead in the hardware.
> > > I can set it (in fact I can set almost any reserved bit in the
> > > registers) but it doesn't seem to interfere with the operation of the
> > > hardware. Either that or I'm very lucky that my displays complete link
> > > training with the incorrect pattern being sent out.
> > > 
> > 
> > I don't think I follow this, are you saying there's no need to clear the
> > TPS3 bit? Isn't it better to have the mask include 1 << 14, so that
> > _intel_dp_set_link_train() clears the bit?
> 
> I'm saying the bit doesn't actually seem to exist in hardware. In fact
> most of my specs don't list it either. Looks like only the "K0" specs
> have it.
> 

Thanks for clarifying.

Reviewed-by: Dhinakaran Pandiyan 

> > 
> > 
> > 
> > > Signed-off-by: Ville Syrjälä 
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h |  2 --
> > >  drivers/gpu/drm/i915/intel_dp.c | 20 
> > >  2 files changed, 4 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 95a2e51ecbb0..f3efc242df2d 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -5250,8 +5250,6 @@ enum {
> > >  #define   DP_LINK_TRAIN_OFF  (3 << 28)
> > >  #define   DP_LINK_TRAIN_MASK (3 << 28)
> > >  #define   DP_LINK_TRAIN_SHIFT28
> > > -#define   DP_LINK_TRAIN_PAT_3_CHV(1 << 14)
> > > -#define   DP_LINK_TRAIN_MASK_CHV ((3 << 28)|(1<<14))
> > >  
> > >  /* CPT Link training mode */
> > >  #define   DP_LINK_TRAIN_PAT_1_CPT(0 << 8)
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index aba2f45819d8..df1772044208 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -92,8 +92,6 @@ static const struct dp_link_dpll chv_dpll[] = {
> > >   { .p1 = 4, .p2 = 2, .n = 1, .m1 = 2, .m2 = 0x81a } },
> > >   { 27,   /* m2_int = 27, m2_fraction = 0 */
> > >   { .p1 = 4, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } },
> > > - { 54,   /* m2_int = 27, m2_fraction = 0 */
> > > - { .p1 = 2, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } }
> > >  };
> > >  
> > >  /**
> > > @@ -2908,10 +2906,7 @@ _intel_dp_set_link_train(struct intel_dp *intel_dp,
> > >   }
> > >  
> > >   } else {
> > > - if (IS_CHERRYVIEW(dev_priv))
> > > - *DP &= ~DP_LINK_TRAIN_MASK_CHV;
> > > - else
> > > - *DP &= ~DP_LINK_TRAIN_MASK;
> > > + *DP &= ~DP_LINK_TRAIN_MASK;
> > >  
> > >   switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
> > >   case DP_TRAINING_PATTERN_DISABLE:
> > > @@ -2924,12 +2919,8 @@ _intel_dp_set_link_train(struct intel_dp *intel_dp,
> > >   *DP |= DP_LINK_TRAIN_PAT_2;
> > >   break;
> > >   case DP_TRAINING_PATTERN_3:
> > > - if (IS_CHERRYVIEW(dev_priv)) {
> > > - *DP |= DP_LINK_TRAIN_PAT_3_CHV;
> > > - } else {
> > > - DRM_DEBUG_KMS("TPS3 not supported, using TPS2 
> > > instead\n");
> > > - *DP |= DP_LINK_TRAIN_PAT_2;
> > > - }
> > > + DRM_DEBUG_KMS("TPS3 not supported, using TPS2 
> > > instead\n");
> > > + *DP |= DP_LINK_TRAIN_PAT_2;
> > >   break;
> > >   }
> > >   }
> > > @@ -3668,10 +3659,7 @@ intel_dp_link_down(struct intel_encoder *encoder,
> > >   DP &= ~DP_LINK_TRAIN_MASK_CPT;
> > >   DP |= DP_LINK_TRAIN_PAT_IDLE_CPT;
> > >   } else {
> > > - if (IS_CHERRYVIEW(dev_priv))
> > > - DP &= ~DP_LINK_TRAIN_MASK_CHV;
> > > - else
> > > - DP &= ~DP_LINK_TRAIN_MASK;
> > > + DP &= ~DP_LINK_TRAIN_MASK;
> > >   DP |= DP_LINK_TRAIN_PAT_IDLE;
> > >   }
> > >   I915_WRITE(intel_dp->output_reg, DP);
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Kill the remaining CHV HBR2 leftovers

[Ville Syrjälä]

On Wed, Mar 07, 2018 at 09:41:06PM +, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Fri, 2018-03-02 at 11:56 +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > AFAIK CHV was supposed to have HBR2 originally, but in the end the feature
> > was dropped. We still have some code leftovers from those early days.
> > Eliminate them.
> > 
> 
> Not much in the spec about HBR2 other than the support for TP3. Since we
> don't support HBR2 on CHV, removing the unused TPS3 bits looks correct.
> 
> 
> 
> > The extra bit for the training pattern seems to be dead in the hardware.
> > I can set it (in fact I can set almost any reserved bit in the
> > registers) but it doesn't seem to interfere with the operation of the
> > hardware. Either that or I'm very lucky that my displays complete link
> > training with the incorrect pattern being sent out.
> > 
> 
> I don't think I follow this, are you saying there's no need to clear the
> TPS3 bit? Isn't it better to have the mask include 1 << 14, so that
> _intel_dp_set_link_train() clears the bit?

I'm saying the bit doesn't actually seem to exist in hardware. In fact
most of my specs don't list it either. Looks like only the "K0" specs
have it.

> 
> 
> 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h |  2 --
> >  drivers/gpu/drm/i915/intel_dp.c | 20 
> >  2 files changed, 4 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 95a2e51ecbb0..f3efc242df2d 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5250,8 +5250,6 @@ enum {
> >  #define   DP_LINK_TRAIN_OFF(3 << 28)
> >  #define   DP_LINK_TRAIN_MASK   (3 << 28)
> >  #define   DP_LINK_TRAIN_SHIFT  28
> > -#define   DP_LINK_TRAIN_PAT_3_CHV  (1 << 14)
> > -#define   DP_LINK_TRAIN_MASK_CHV   ((3 << 28)|(1<<14))
> >  
> >  /* CPT Link training mode */
> >  #define   DP_LINK_TRAIN_PAT_1_CPT  (0 << 8)
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index aba2f45819d8..df1772044208 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -92,8 +92,6 @@ static const struct dp_link_dpll chv_dpll[] = {
> > { .p1 = 4, .p2 = 2, .n = 1, .m1 = 2, .m2 = 0x81a } },
> > { 27,   /* m2_int = 27, m2_fraction = 0 */
> > { .p1 = 4, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } },
> > -   { 54,   /* m2_int = 27, m2_fraction = 0 */
> > -   { .p1 = 2, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } }
> >  };
> >  
> >  /**
> > @@ -2908,10 +2906,7 @@ _intel_dp_set_link_train(struct intel_dp *intel_dp,
> > }
> >  
> > } else {
> > -   if (IS_CHERRYVIEW(dev_priv))
> > -   *DP &= ~DP_LINK_TRAIN_MASK_CHV;
> > -   else
> > -   *DP &= ~DP_LINK_TRAIN_MASK;
> > +   *DP &= ~DP_LINK_TRAIN_MASK;
> >  
> > switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
> > case DP_TRAINING_PATTERN_DISABLE:
> > @@ -2924,12 +2919,8 @@ _intel_dp_set_link_train(struct intel_dp *intel_dp,
> > *DP |= DP_LINK_TRAIN_PAT_2;
> > break;
> > case DP_TRAINING_PATTERN_3:
> > -   if (IS_CHERRYVIEW(dev_priv)) {
> > -   *DP |= DP_LINK_TRAIN_PAT_3_CHV;
> > -   } else {
> > -   DRM_DEBUG_KMS("TPS3 not supported, using TPS2 
> > instead\n");
> > -   *DP |= DP_LINK_TRAIN_PAT_2;
> > -   }
> > +   DRM_DEBUG_KMS("TPS3 not supported, using TPS2 
> > instead\n");
> > +   *DP |= DP_LINK_TRAIN_PAT_2;
> > break;
> > }
> > }
> > @@ -3668,10 +3659,7 @@ intel_dp_link_down(struct intel_encoder *encoder,
> > DP &= ~DP_LINK_TRAIN_MASK_CPT;
> > DP |= DP_LINK_TRAIN_PAT_IDLE_CPT;
> > } else {
> > -   if (IS_CHERRYVIEW(dev_priv))
> > -   DP &= ~DP_LINK_TRAIN_MASK_CHV;
> > -   else
> > -   DP &= ~DP_LINK_TRAIN_MASK;
> > +   DP &= ~DP_LINK_TRAIN_MASK;
> > DP |= DP_LINK_TRAIN_PAT_IDLE;
> > }
> > I915_WRITE(intel_dp->output_reg, DP);

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Kill the remaining CHV HBR2 leftovers

[Pandiyan, Dhinakaran]

On Fri, 2018-03-02 at 11:56 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> AFAIK CHV was supposed to have HBR2 originally, but in the end the feature
> was dropped. We still have some code leftovers from those early days.
> Eliminate them.
> 

Not much in the spec about HBR2 other than the support for TP3. Since we
don't support HBR2 on CHV, removing the unused TPS3 bits looks correct.



> The extra bit for the training pattern seems to be dead in the hardware.
> I can set it (in fact I can set almost any reserved bit in the
> registers) but it doesn't seem to interfere with the operation of the
> hardware. Either that or I'm very lucky that my displays complete link
> training with the incorrect pattern being sent out.
> 

I don't think I follow this, are you saying there's no need to clear the
TPS3 bit? Isn't it better to have the mask include 1 << 14, so that
_intel_dp_set_link_train() clears the bit?



> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  2 --
>  drivers/gpu/drm/i915/intel_dp.c | 20 
>  2 files changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 95a2e51ecbb0..f3efc242df2d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5250,8 +5250,6 @@ enum {
>  #define   DP_LINK_TRAIN_OFF  (3 << 28)
>  #define   DP_LINK_TRAIN_MASK (3 << 28)
>  #define   DP_LINK_TRAIN_SHIFT28
> -#define   DP_LINK_TRAIN_PAT_3_CHV(1 << 14)
> -#define   DP_LINK_TRAIN_MASK_CHV ((3 << 28)|(1<<14))
>  
>  /* CPT Link training mode */
>  #define   DP_LINK_TRAIN_PAT_1_CPT(0 << 8)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index aba2f45819d8..df1772044208 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -92,8 +92,6 @@ static const struct dp_link_dpll chv_dpll[] = {
>   { .p1 = 4, .p2 = 2, .n = 1, .m1 = 2, .m2 = 0x81a } },
>   { 27,   /* m2_int = 27, m2_fraction = 0 */
>   { .p1 = 4, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } },
> - { 54,   /* m2_int = 27, m2_fraction = 0 */
> - { .p1 = 2, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } }
>  };
>  
>  /**
> @@ -2908,10 +2906,7 @@ _intel_dp_set_link_train(struct intel_dp *intel_dp,
>   }
>  
>   } else {
> - if (IS_CHERRYVIEW(dev_priv))
> - *DP &= ~DP_LINK_TRAIN_MASK_CHV;
> - else
> - *DP &= ~DP_LINK_TRAIN_MASK;
> + *DP &= ~DP_LINK_TRAIN_MASK;
>  
>   switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
>   case DP_TRAINING_PATTERN_DISABLE:
> @@ -2924,12 +2919,8 @@ _intel_dp_set_link_train(struct intel_dp *intel_dp,
>   *DP |= DP_LINK_TRAIN_PAT_2;
>   break;
>   case DP_TRAINING_PATTERN_3:
> - if (IS_CHERRYVIEW(dev_priv)) {
> - *DP |= DP_LINK_TRAIN_PAT_3_CHV;
> - } else {
> - DRM_DEBUG_KMS("TPS3 not supported, using TPS2 
> instead\n");
> - *DP |= DP_LINK_TRAIN_PAT_2;
> - }
> + DRM_DEBUG_KMS("TPS3 not supported, using TPS2 
> instead\n");
> + *DP |= DP_LINK_TRAIN_PAT_2;
>   break;
>   }
>   }
> @@ -3668,10 +3659,7 @@ intel_dp_link_down(struct intel_encoder *encoder,
>   DP &= ~DP_LINK_TRAIN_MASK_CPT;
>   DP |= DP_LINK_TRAIN_PAT_IDLE_CPT;
>   } else {
> - if (IS_CHERRYVIEW(dev_priv))
> - DP &= ~DP_LINK_TRAIN_MASK_CHV;
> - else
> - DP &= ~DP_LINK_TRAIN_MASK;
> + DP &= ~DP_LINK_TRAIN_MASK;
>   DP |= DP_LINK_TRAIN_PAT_IDLE;
>   }
>   I915_WRITE(intel_dp->output_reg, DP);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Kill the remaining CHV HBR2 leftovers

[Ville Syrjala]

From: Ville Syrjälä 

AFAIK CHV was supposed to have HBR2 originally, but in the end the feature
was dropped. We still have some code leftovers from those early days.
Eliminate them.

The extra bit for the training pattern seems to be dead in the hardware.
I can set it (in fact I can set almost any reserved bit in the
registers) but it doesn't seem to interfere with the operation of the
hardware. Either that or I'm very lucky that my displays complete link
training with the incorrect pattern being sent out.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_reg.h |  2 --
 drivers/gpu/drm/i915/intel_dp.c | 20 
 2 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 95a2e51ecbb0..f3efc242df2d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5250,8 +5250,6 @@ enum {
 #define   DP_LINK_TRAIN_OFF(3 << 28)
 #define   DP_LINK_TRAIN_MASK   (3 << 28)
 #define   DP_LINK_TRAIN_SHIFT  28
-#define   DP_LINK_TRAIN_PAT_3_CHV  (1 << 14)
-#define   DP_LINK_TRAIN_MASK_CHV   ((3 << 28)|(1<<14))
 
 /* CPT Link training mode */
 #define   DP_LINK_TRAIN_PAT_1_CPT  (0 << 8)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index aba2f45819d8..df1772044208 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -92,8 +92,6 @@ static const struct dp_link_dpll chv_dpll[] = {
{ .p1 = 4, .p2 = 2, .n = 1, .m1 = 2, .m2 = 0x81a } },
{ 27,   /* m2_int = 27, m2_fraction = 0 */
{ .p1 = 4, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } },
-   { 54,   /* m2_int = 27, m2_fraction = 0 */
-   { .p1 = 2, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } }
 };
 
 /**
@@ -2908,10 +2906,7 @@ _intel_dp_set_link_train(struct intel_dp *intel_dp,
}
 
} else {
-   if (IS_CHERRYVIEW(dev_priv))
-   *DP &= ~DP_LINK_TRAIN_MASK_CHV;
-   else
-   *DP &= ~DP_LINK_TRAIN_MASK;
+   *DP &= ~DP_LINK_TRAIN_MASK;
 
switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
case DP_TRAINING_PATTERN_DISABLE:
@@ -2924,12 +2919,8 @@ _intel_dp_set_link_train(struct intel_dp *intel_dp,
*DP |= DP_LINK_TRAIN_PAT_2;
break;
case DP_TRAINING_PATTERN_3:
-   if (IS_CHERRYVIEW(dev_priv)) {
-   *DP |= DP_LINK_TRAIN_PAT_3_CHV;
-   } else {
-   DRM_DEBUG_KMS("TPS3 not supported, using TPS2 
instead\n");
-   *DP |= DP_LINK_TRAIN_PAT_2;
-   }
+   DRM_DEBUG_KMS("TPS3 not supported, using TPS2 
instead\n");
+   *DP |= DP_LINK_TRAIN_PAT_2;
break;
}
}
@@ -3668,10 +3659,7 @@ intel_dp_link_down(struct intel_encoder *encoder,
DP &= ~DP_LINK_TRAIN_MASK_CPT;
DP |= DP_LINK_TRAIN_PAT_IDLE_CPT;
} else {
-   if (IS_CHERRYVIEW(dev_priv))
-   DP &= ~DP_LINK_TRAIN_MASK_CHV;
-   else
-   DP &= ~DP_LINK_TRAIN_MASK;
+   DP &= ~DP_LINK_TRAIN_MASK;
DP |= DP_LINK_TRAIN_PAT_IDLE;
}
I915_WRITE(intel_dp->output_reg, DP);
-- 
2.13.6

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx