Message ID | 20210318181039.17260-1-ville.syrjala@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/2] drm/i915: Fix pre-skl DP AUX precharge length | expand |
On Thu, Mar 18, 2021 at 08:09:43PM -0000, Patchwork wrote: > == Series Details == > > Series: series starting with [1/2] drm/i915: Fix pre-skl DP AUX precharge length > URL : https://patchwork.freedesktop.org/series/88132/ > State : failure > > == Summary == > > CI Bug Log - changes from CI_DRM_9870 -> Patchwork_19807 > ==================================================== > > Summary > ------- > > **FAILURE** > > Serious unknown changes coming with Patchwork_19807 absolutely need to be > verified manually. > > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_19807, please notify your bug team to allow them > to document this new failure mode, which will reduce false positives in CI. > > External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19807/index.html > > Possible new issues > ------------------- > > Here are the unknown changes that may have been introduced in Patchwork_19807: > > ### IGT changes ### > > #### Possible regressions #### > > * igt@i915_selftest@live@execlists: > - fi-kbl-soraka: [PASS][1] -> [DMESG-WARN][2] > [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9870/fi-kbl-soraka/igt@i915_selftest@live@execlists.html > [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19807/fi-kbl-soraka/igt@i915_selftest@live@execlists.html <3> [1019.090846] i915 0000:00:02.0: [drm] *ERROR* AUX B/DDI B/PHY B: did not complete or timeout within 10ms (status 0xad4003ff) Unrelated to the patch since it doesn't touch the skl+ AUX code and this machine is a kbl. So seems to be just some random fail. > > > Known issues > ------------ > > Here are the changes found in Patchwork_19807 that come from known issues: > > ### IGT changes ### > > #### Issues hit #### > > * igt@core_hotunplug@unbind-rebind: > - fi-bwr-2160: [PASS][3] -> [FAIL][4] ([i915#3194]) > [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9870/fi-bwr-2160/igt@core_hotunplug@unbind-rebind.html > [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19807/fi-bwr-2160/igt@core_hotunplug@unbind-rebind.html > > > [i915#3194]: https://gitlab.freedesktop.org/drm/intel/issues/3194 > > > Participating hosts (44 -> 40) > ------------------------------ > > Missing (4): fi-ilk-m540 fi-bsw-cyan fi-bdw-samus fi-hsw-4200u > > > Build changes > ------------- > > * Linux: CI_DRM_9870 -> Patchwork_19807 > > CI-20190529: 20190529 > CI_DRM_9870: a9a5ed8d2432e5335e6c26118cefb2cfff28ae37 @ git://anongit.freedesktop.org/gfx-ci/linux > IGT_6036: 5b535494abcdf5ce2b9be99b7bb5df8ab4733083 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools > Patchwork_19807: 89c9ce9bcfbf7286c2ec25ffaa26041909ec84d6 @ git://anongit.freedesktop.org/gfx-ci/linux > > > == Linux commits == > > 89c9ce9bcfbf drm/i915: Remove stray newlines > 6b3f9e7ed7f6 drm/i915: Fix pre-skl DP AUX precharge length > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19807/index.html
Re-reported. -----Original Message----- From: Ville Syrjälä <ville.syrjala@linux.intel.com> Sent: Thursday, March 18, 2021 1:53 PM To: intel-gfx@lists.freedesktop.org Cc: Vudum, Lakshminarayana <lakshminarayana.vudum@intel.com> Subject: Re: ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Fix pre-skl DP AUX precharge length On Thu, Mar 18, 2021 at 08:09:43PM -0000, Patchwork wrote: > == Series Details == > > Series: series starting with [1/2] drm/i915: Fix pre-skl DP AUX precharge length > URL : https://patchwork.freedesktop.org/series/88132/ > State : failure > > == Summary == > > CI Bug Log - changes from CI_DRM_9870 -> Patchwork_19807 > ==================================================== > > Summary > ------- > > **FAILURE** > > Serious unknown changes coming with Patchwork_19807 absolutely need to be > verified manually. > > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_19807, please notify your bug team to allow them > to document this new failure mode, which will reduce false positives in CI. > > External URL: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19807/index.html > > Possible new issues > ------------------- > > Here are the unknown changes that may have been introduced in Patchwork_19807: > > ### IGT changes ### > > #### Possible regressions #### > > * igt@i915_selftest@live@execlists: > - fi-kbl-soraka: [PASS][1] -> [DMESG-WARN][2] > [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9870/fi-kbl-soraka/igt@i915_selftest@live@execlists.html > [2]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19807/fi-kbl-soraka > /igt@i915_selftest@live@execlists.html <3> [1019.090846] i915 0000:00:02.0: [drm] *ERROR* AUX B/DDI B/PHY B: did not complete or timeout within 10ms (status 0xad4003ff) Unrelated to the patch since it doesn't touch the skl+ AUX code and this machine is a kbl. So seems to be just some random fail. > > > Known issues > ------------ > > Here are the changes found in Patchwork_19807 that come from known issues: > > ### IGT changes ### > > #### Issues hit #### > > * igt@core_hotunplug@unbind-rebind: > - fi-bwr-2160: [PASS][3] -> [FAIL][4] ([i915#3194]) > [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9870/fi-bwr-2160/igt@core_hotunplug@unbind-rebind.html > [4]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19807/fi-bwr-2160/i > gt@core_hotunplug@unbind-rebind.html > > > [i915#3194]: https://gitlab.freedesktop.org/drm/intel/issues/3194 > > > Participating hosts (44 -> 40) > ------------------------------ > > Missing (4): fi-ilk-m540 fi-bsw-cyan fi-bdw-samus fi-hsw-4200u > > > Build changes > ------------- > > * Linux: CI_DRM_9870 -> Patchwork_19807 > > CI-20190529: 20190529 > CI_DRM_9870: a9a5ed8d2432e5335e6c26118cefb2cfff28ae37 @ git://anongit.freedesktop.org/gfx-ci/linux > IGT_6036: 5b535494abcdf5ce2b9be99b7bb5df8ab4733083 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools > Patchwork_19807: 89c9ce9bcfbf7286c2ec25ffaa26041909ec84d6 @ > git://anongit.freedesktop.org/gfx-ci/linux > > > == Linux commits == > > 89c9ce9bcfbf drm/i915: Remove stray newlines > 6b3f9e7ed7f6 drm/i915: Fix pre-skl DP AUX precharge length > > == Logs == > > For more details see: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19807/index.html -- Ville Syrjälä Intel
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
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. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Imre Deak <imre.deak@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
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); }