Re: [Intel-gfx] [PATCH 1/2] drm/i915: Fix pre-skl DP AUX precharge length

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Mar 18, 2021 at 08:10:38PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> DP v1.1+ says:
> "The DisplayPort transmitter, which is the driving end for a request
>  transaction, pre-charges the AUX-CH+ and AUX-CH- to a common mode
>  voltage by transmitting 10 to 16 consecutive 0’s in Manchester II code.
>  After the active pre-charge, the transmitter sends an AUX Sync pattern.
>  The AUX Sync pattern must be as follows:
>  Start with 16 consecutive 0s in Manchester-II code, which results in
>  a transition from low to high in the middle of each bit period.
>  Including active pre-charge pulses, there shall be 26 to 32
>  consecutive 0s before the end of the AUX_SYNC pattern."
> 
> BDW bspec says:
> "Used to determine the precharge time for the Aux Channel. During this
>  time the Aux Channel will drive the SYNC pattern. Every microsecond
>  gives one additional SYNC pulse beyond the hard coded 26 SYNC pulses.
>  The value is the number of microseconds times 2. Default is 3 decimal
>  which gives 6us of precharge which is 6 extra SYN pulses for a total
>  of 32."
> 
> CPT bspec says the same thing apart from:
> "... Default is 5 decimal which gives 10us of precharge which is 10
>  extra SYNC pulses for a total of 36."
> 
> So it looks like to match the max of 32 of the DP spec we should just
> always program this extra precharge time to 3.
> 
> Unfortunately g4x/ibx bspec doesn't have this clarification, but
> since the cpt default was still the same 5 as for g4x/ibx let's
> assume the behaviour was always the same.

I did a bit more archaeology and found the following:

commit e3421a189447 ("drm/i915: enable DP/eDP for Sandybridge/Cougarpoint")
added the precharge==3 for snb.

commit 092945e11c5b ("drm/i915/dp: Use auxch precharge value of 5 everywhere")
tried to change it to be 5 for snb.

commit 6b4e0a93ff6e ("Revert "drm/i915/dp: Use auxch precharge value of 5 everywhere"")
went back to 3 for snb due to a regression.

So I think the value of 5 was just always wrong, but I guess very
few display actually get upset if we do too many SYNCs. Also DP 1.0
did not specify any max value for this, whereas DP 1.1+ added the
max==32 wording.

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_aux.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> index eaebf123310a..d5443d6d6123 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> @@ -126,12 +126,7 @@ static u32 g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_i915_private *dev_priv =
>  			to_i915(dig_port->base.base.dev);
> -	u32 precharge, timeout;
> -
> -	if (IS_GEN(dev_priv, 6))
> -		precharge = 3;
> -	else
> -		precharge = 5;
> +	u32 timeout;
>  
>  	if (IS_BROADWELL(dev_priv))
>  		timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
> @@ -145,7 +140,7 @@ static u32 g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
>  	       timeout |
>  	       DP_AUX_CH_CTL_RECEIVE_ERROR |
>  	       (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> -	       (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
> +	       (3 << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
>  	       (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT);
>  }
>  
> -- 
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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