drm/i915: Fix pre-skl DP AUX precharge length
authorVille Syrjälä <ville.syrjala@linux.intel.com>
Thu, 18 Mar 2021 18:10:38 +0000 (20:10 +0200)
committerVille Syrjälä <ville.syrjala@linux.intel.com>
Wed, 28 Apr 2021 12:52:23 +0000 (15:52 +0300)
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 also 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.

Additionally I hooked up a scope to a few machines with the following
findings:
- ibx and cpt both give us the expected 32 total sync pulses with
  precharge==3
- ctg is a bit different, it has the 10 hardcoded precharge sync
  pulses same as later platforms (so we get at least 26 sync
  pulses in total). However the additional precharge length (which
  is what we're changing here) is not done with sync pulses.
  Instead ctg does this part of the precharge with a steady DC
  voltage. If we wanted to 100% match DP 1.1+ here we should perhaps
  set prechange length to 0, but less precharge might make AUX less
  reliable, and so far we're not aware of any problems due to the DC
  precharge. Hence I think precharge==3 is probably the best choice
  here too to make the total length of precharge consistent with
  the later platforms.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210318181039.17260-1-ville.syrjala@linux.intel.com
Reviewed-by: Imre Deak <imre.deak@intel.com>
drivers/gpu/drm/i915/display/intel_dp_aux.c

index 7e83bc2..805f695 100644 (file)
@@ -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_SANDYBRIDGE(dev_priv))
-               precharge = 3;
-       else
-               precharge = 5;
+       u32 timeout;
 
        /* Max timeout value on G4x-BDW: 1.6ms */
        if (IS_BROADWELL(dev_priv))
@@ -146,7 +141,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);
 }