[PATCH] drm/i915/display: Increase timeout for DP Aux channel ctl signal

[PATCH] drm/i915/display: Increase timeout for DP Aux channel ctl signal

[Vanshidhar Konda]

Extend the timeout for the hardware to signal SEND_BUSY on the DP
Aux Channel Controller register.

This is needed to address FDO #109982
https://bugzilla.freedesktop.org/show_bug.cgi?id=109982

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 47857f96c3b1..fd6de33c5664 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1053,6 +1053,8 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
 	}
 }
 
+#define DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS 15
+
 static u32
 intel_dp_aux_wait_done(struct intel_dp *intel_dp)
 {
@@ -1063,7 +1065,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
 
 #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
 	done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
-				  msecs_to_jiffies_timeout(10));
+				  msecs_to_jiffies_timeout(DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS));
 
 	/* just trace the final value */
 	trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
-- 
2.20.1

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

✓ Fi.CI.BAT: success for drm/i915/display: Increase timeout for DP Aux channel ctl signal

[Patchwork]

== Series Details ==

Series: drm/i915/display: Increase timeout for DP Aux channel ctl signal
URL   : https://patchwork.freedesktop.org/series/58077/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5753 -> Patchwork_12485
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58077/revisions/1/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12485 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-compute:
    - fi-kbl-8809g:       NOTRUN -> FAIL [fdo#108094]

  * igt@amdgpu/amd_cs_nop@sync-gfx0:
    - fi-kbl-7567u:       NOTRUN -> SKIP [fdo#109271] +33

  * igt@gem_exec_basic@gtt-bsd:
    - fi-bwr-2160:        NOTRUN -> SKIP [fdo#109271] +103

  * igt@gem_exec_basic@readonly-bsd2:
    - fi-pnv-d510:        NOTRUN -> SKIP [fdo#109271] +76

  * igt@i915_selftest@live_evict:
    - fi-bsw-kefka:       PASS -> DMESG-WARN [fdo#107709]

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         PASS -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@kms_busy@basic-flip-c:
    - fi-bwr-2160:        NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
    - fi-blb-e6850:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
    - fi-pnv-d510:        NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-kbl-8809g:       NOTRUN -> SKIP [fdo#109271] +65

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-kbl-7560u:       NOTRUN -> SKIP [fdo#109271] +33

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-c:
    - fi-blb-e6850:       NOTRUN -> SKIP [fdo#109271] +48

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@runner@aborted:
    - fi-bsw-kefka:       NOTRUN -> FAIL [fdo#107709]
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720]

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-kbl-7560u:       INCOMPLETE [fdo#103665] -> PASS

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@kms_busy@basic-flip-a:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS +1

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u3:          FAIL [fdo#103167] -> PASS
    - fi-byt-clapper:     FAIL [fdo#103167] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS +1

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108094]: https://bugs.freedesktop.org/show_bug.cgi?id=108094
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720


Participating hosts (44 -> 43)
------------------------------

  Additional (4): fi-kbl-7567u fi-kbl-8809g fi-bwr-2160 fi-pnv-d510 
  Missing    (5): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 


Build changes
-------------

    * Linux: CI_DRM_5753 -> Patchwork_12485

  CI_DRM_5753: 0eb0838c0c26378949de6816166117c8b2d73caa @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4887: 5a7c7575b5bb9542f722ed6ba095b9d62609cd56 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12485: 3dc430594d09c21b1fe8b5fb0f5c941afe2707a8 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

3dc430594d09 drm/i915/display: Increase timeout for DP Aux channel ctl signal

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12485/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/display: Increase timeout for DP Aux channel ctl signal

[Rodrigo Vivi]

On Fri, Mar 15, 2019 at 11:39:54AM -0700, Vanshidhar Konda wrote:
> Extend the timeout for the hardware to signal SEND_BUSY on the DP
> Aux Channel Controller register.
> 
> This is needed to address FDO #109982
> https://bugzilla.freedesktop.org/show_bug.cgi?id=109982

instead of mentioning like this, please use:
Bugzilla: https://bugzilla.freedesktop.org/show_bug.cgi?id=109982

Also instead of the "needed to address" it would be better
to add some reasoning explaining that
"empirically we got some bugs workarounded by increasing the timeout
 from 10ms to 15ms although spec was only requiring 4ms"
or something like that...

> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 47857f96c3b1..fd6de33c5664 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1053,6 +1053,8 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> 	}
> }
> 
> +#define DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS 15

This define is spurious since it's in use in a single place.

Also, giving the timeout a name, like this, makes it appear it came from
the spec. Well, if it came from Spec it should be defined in the proper .h
files.

Since I don't believe this came from spec I believe we can just remove it
and go for the timeout directly on the function below.

> +
> static u32
> intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> {
> @@ -1063,7 +1065,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> 
> #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> 	done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> -				  msecs_to_jiffies_timeout(10));
> +				  msecs_to_jiffies_timeout(DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS));

Is this just a guess that you are trying to check?
I'm asking because I didn't see any indication that the increase
really fixed the issue.

So, if you are trying to just validate your approach maybe the try-bot
could be used?

Thanks,
Rodrigo.

> 
> 	/* just trace the final value */
> 	trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/display: Increase timeout for DP Aux channel ctl signal

[Vanshidhar Konda]

On Fri, Mar 15, 2019 at 12:38:41PM -0700, Rodrigo Vivi wrote:
>On Fri, Mar 15, 2019 at 11:39:54AM -0700, Vanshidhar Konda wrote:
>> Extend the timeout for the hardware to signal SEND_BUSY on the DP
>> Aux Channel Controller register.
>>
>> This is needed to address FDO #109982
>> https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
>
>instead of mentioning like this, please use:
>Bugzilla: https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
>
>Also instead of the "needed to address" it would be better
>to add some reasoning explaining that
>"empirically we got some bugs workarounded by increasing the timeout
> from 10ms to 15ms although spec was only requiring 4ms"
>or something like that...
>
Thanks! I'll keep these in mind for next patches.
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_dp.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 47857f96c3b1..fd6de33c5664 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1053,6 +1053,8 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
>> 	}
>> }
>>
>> +#define DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS 15
>
>This define is spurious since it's in use in a single place.
>
>Also, giving the timeout a name, like this, makes it appear it came from
>the spec. Well, if it came from Spec it should be defined in the proper .h
>files.
>
>Since I don't believe this came from spec I believe we can just remove it
>and go for the timeout directly on the function below.
>
I'll make this change in the next patch.
>> +
>> static u32
>> intel_dp_aux_wait_done(struct intel_dp *intel_dp)
>> {
>> @@ -1063,7 +1065,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
>>
>> #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
>> 	done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
>> -				  msecs_to_jiffies_timeout(10));
>> +				  msecs_to_jiffies_timeout(DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS));
>
>Is this just a guess that you are trying to check?
>I'm asking because I didn't see any indication that the increase
>really fixed the issue.
>
>So, if you are trying to just validate your approach maybe the try-bot
>could be used?
>
I also would have preferred to use the trybot. The error in Bugzilla 109982 is only reproduced on only shard-iclb2 machine once
in approximately 2 days. So the trybot is not of much help for this.

Is there another way of testing experimental patches for bugs that are
not reproduced locally or on other machines? Or, if the issue is only
happening on one machine, should it be lowered in priority/whitelisted
on the specific CI machine?

>Thanks,
>Rodrigo.
>
>>
>> 	/* just trace the final value */
>> 	trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
>> --
>> 2.20.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/display: Increase timeout for DP Aux channel ctl signal

[Rodrigo Vivi]

On Fri, Mar 15, 2019 at 01:27:22PM -0700, Vanshidhar Konda wrote:
> On Fri, Mar 15, 2019 at 12:38:41PM -0700, Rodrigo Vivi wrote:
> > On Fri, Mar 15, 2019 at 11:39:54AM -0700, Vanshidhar Konda wrote:
> > > Extend the timeout for the hardware to signal SEND_BUSY on the DP
> > > Aux Channel Controller register.
> > > 
> > > This is needed to address FDO #109982
> > > https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
> > 
> > instead of mentioning like this, please use:
> > Bugzilla: https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
> > 
> > Also instead of the "needed to address" it would be better
> > to add some reasoning explaining that
> > "empirically we got some bugs workarounded by increasing the timeout
> > from 10ms to 15ms although spec was only requiring 4ms"
> > or something like that...
> > 
> Thanks! I'll keep these in mind for next patches.
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 47857f96c3b1..fd6de33c5664 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -1053,6 +1053,8 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> > > 	}
> > > }
> > > 
> > > +#define DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS 15
> > 
> > This define is spurious since it's in use in a single place.
> > 
> > Also, giving the timeout a name, like this, makes it appear it came from
> > the spec. Well, if it came from Spec it should be defined in the proper .h
> > files.
> > 
> > Since I don't believe this came from spec I believe we can just remove it
> > and go for the timeout directly on the function below.
> > 
> I'll make this change in the next patch.
> > > +
> > > static u32
> > > intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > {
> > > @@ -1063,7 +1065,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > 
> > > #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> > > 	done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > > -				  msecs_to_jiffies_timeout(10));
> > > +				  msecs_to_jiffies_timeout(DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS));
> > 
> > Is this just a guess that you are trying to check?
> > I'm asking because I didn't see any indication that the increase
> > really fixed the issue.
> > 
> > So, if you are trying to just validate your approach maybe the try-bot
> > could be used?
> > 
> I also would have preferred to use the trybot. The error in Bugzilla 109982 is only reproduced on only shard-iclb2 machine once
> in approximately 2 days. So the trybot is not of much help for this.
> 
> Is there another way of testing experimental patches for bugs that are
> not reproduced locally or on other machines? Or, if the issue is only
> happening on one machine, should it be lowered in priority/whitelisted
> on the specific CI machine?

For now I believe the best alternative is to --subject-prefix="[CI]"

But also you need to be sure that the fail rate don't fool us...

I believe there will be better alternatives for using CI for cases like
this soon, so +Martin

> 
> > Thanks,
> > Rodrigo.
> > 
> > > 
> > > 	/* just trace the final value */
> > > 	trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
> > > --
> > > 2.20.1
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/display: Increase timeout for DP Aux channel ctl signal

[Rodrigo Vivi]

On Fri, Mar 15, 2019 at 02:31:40PM -0700, Rodrigo Vivi wrote:
> On Fri, Mar 15, 2019 at 01:27:22PM -0700, Vanshidhar Konda wrote:
> > On Fri, Mar 15, 2019 at 12:38:41PM -0700, Rodrigo Vivi wrote:
> > > On Fri, Mar 15, 2019 at 11:39:54AM -0700, Vanshidhar Konda wrote:
> > > > Extend the timeout for the hardware to signal SEND_BUSY on the DP
> > > > Aux Channel Controller register.
> > > > 
> > > > This is needed to address FDO #109982
> > > > https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
> > > 
> > > instead of mentioning like this, please use:
> > > Bugzilla: https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
> > > 
> > > Also instead of the "needed to address" it would be better
> > > to add some reasoning explaining that
> > > "empirically we got some bugs workarounded by increasing the timeout
> > > from 10ms to 15ms although spec was only requiring 4ms"
> > > or something like that...
> > > 
> > Thanks! I'll keep these in mind for next patches.
> > > > 
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 47857f96c3b1..fd6de33c5664 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -1053,6 +1053,8 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> > > > 	}
> > > > }
> > > > 
> > > > +#define DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS 15
> > > 
> > > This define is spurious since it's in use in a single place.
> > > 
> > > Also, giving the timeout a name, like this, makes it appear it came from
> > > the spec. Well, if it came from Spec it should be defined in the proper .h
> > > files.
> > > 
> > > Since I don't believe this came from spec I believe we can just remove it
> > > and go for the timeout directly on the function below.
> > > 
> > I'll make this change in the next patch.
> > > > +
> > > > static u32
> > > > intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > > {
> > > > @@ -1063,7 +1065,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > > 
> > > > #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> > > > 	done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > > > -				  msecs_to_jiffies_timeout(10));
> > > > +				  msecs_to_jiffies_timeout(DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS));
> > > 
> > > Is this just a guess that you are trying to check?
> > > I'm asking because I didn't see any indication that the increase
> > > really fixed the issue.
> > > 
> > > So, if you are trying to just validate your approach maybe the try-bot
> > > could be used?
> > > 
> > I also would have preferred to use the trybot. The error in Bugzilla 109982 is only reproduced on only shard-iclb2 machine once
> > in approximately 2 days. So the trybot is not of much help for this.
> > 
> > Is there another way of testing experimental patches for bugs that are
> > not reproduced locally or on other machines? Or, if the issue is only
> > happening on one machine, should it be lowered in priority/whitelisted
> > on the specific CI machine?
> 
> For now I believe the best alternative is to --subject-prefix="[CI]"
> 
> But also you need to be sure that the fail rate don't fool us...

hmm... this testcase seems to have a passrate of 77% of the runs.
So probably using the CI infrastructure here isn't the best alternative
to validate this approach in general.

> 
> I believe there will be better alternatives for using CI for cases like
> this soon, so +Martin
> 
> > 
> > > Thanks,
> > > Rodrigo.
> > > 
> > > > 
> > > > 	/* just trace the final value */
> > > > 	trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
> > > > --
> > > > 2.20.1
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/display: Increase timeout for DP Aux channel ctl signal

[Rodrigo Vivi]

On Fri, Mar 15, 2019 at 02:39:25PM -0700, Rodrigo Vivi wrote:
> On Fri, Mar 15, 2019 at 02:31:40PM -0700, Rodrigo Vivi wrote:
> > On Fri, Mar 15, 2019 at 01:27:22PM -0700, Vanshidhar Konda wrote:
> > > On Fri, Mar 15, 2019 at 12:38:41PM -0700, Rodrigo Vivi wrote:
> > > > On Fri, Mar 15, 2019 at 11:39:54AM -0700, Vanshidhar Konda wrote:
> > > > > Extend the timeout for the hardware to signal SEND_BUSY on the DP
> > > > > Aux Channel Controller register.
> > > > > 
> > > > > This is needed to address FDO #109982
> > > > > https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
> > > > 
> > > > instead of mentioning like this, please use:
> > > > Bugzilla: https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
> > > > 
> > > > Also instead of the "needed to address" it would be better
> > > > to add some reasoning explaining that
> > > > "empirically we got some bugs workarounded by increasing the timeout
> > > > from 10ms to 15ms although spec was only requiring 4ms"
> > > > or something like that...
> > > > 
> > > Thanks! I'll keep these in mind for next patches.
> > > > > 
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> > > > > ---
> > > > > drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> > > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index 47857f96c3b1..fd6de33c5664 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -1053,6 +1053,8 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> > > > > 	}
> > > > > }
> > > > > 
> > > > > +#define DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS 15
> > > > 
> > > > This define is spurious since it's in use in a single place.
> > > > 
> > > > Also, giving the timeout a name, like this, makes it appear it came from
> > > > the spec. Well, if it came from Spec it should be defined in the proper .h
> > > > files.
> > > > 
> > > > Since I don't believe this came from spec I believe we can just remove it
> > > > and go for the timeout directly on the function below.
> > > > 
> > > I'll make this change in the next patch.
> > > > > +
> > > > > static u32
> > > > > intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > > > {
> > > > > @@ -1063,7 +1065,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > > > 
> > > > > #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> > > > > 	done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > > > > -				  msecs_to_jiffies_timeout(10));
> > > > > +				  msecs_to_jiffies_timeout(DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS));
> > > > 
> > > > Is this just a guess that you are trying to check?
> > > > I'm asking because I didn't see any indication that the increase
> > > > really fixed the issue.
> > > > 
> > > > So, if you are trying to just validate your approach maybe the try-bot
> > > > could be used?
> > > > 
> > > I also would have preferred to use the trybot. The error in Bugzilla 109982 is only reproduced on only shard-iclb2 machine once
> > > in approximately 2 days. So the trybot is not of much help for this.
> > > 
> > > Is there another way of testing experimental patches for bugs that are
> > > not reproduced locally or on other machines? Or, if the issue is only
> > > happening on one machine, should it be lowered in priority/whitelisted
> > > on the specific CI machine?
> > 
> > For now I believe the best alternative is to --subject-prefix="[CI]"
> > 
> > But also you need to be sure that the fail rate don't fool us...
> 
> hmm... this testcase seems to have a passrate of 77% of the runs.
> So probably using the CI infrastructure here isn't the best alternative
> to validate this approach in general.

ops... I forgot to paste the link from these data:
https://intel-gfx-ci.01.org/tree/drm-tip/shard-iclb.html
(30 runs out of 45 passing)

> 
> > 
> > I believe there will be better alternatives for using CI for cases like
> > this soon, so +Martin
> > 
> > > 
> > > > Thanks,
> > > > Rodrigo.
> > > > 
> > > > > 
> > > > > 	/* just trace the final value */
> > > > > 	trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
> > > > > --
> > > > > 2.20.1
> > > > > 
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/display: Increase timeout for DP Aux channel ctl signal

[Vanshidhar Konda]

On Fri, Mar 15, 2019 at 02:43:46PM -0700, Rodrigo Vivi wrote:
>On Fri, Mar 15, 2019 at 02:39:25PM -0700, Rodrigo Vivi wrote:
>> On Fri, Mar 15, 2019 at 02:31:40PM -0700, Rodrigo Vivi wrote:
>> > On Fri, Mar 15, 2019 at 01:27:22PM -0700, Vanshidhar Konda wrote:
>> > > On Fri, Mar 15, 2019 at 12:38:41PM -0700, Rodrigo Vivi wrote:
>> > > > On Fri, Mar 15, 2019 at 11:39:54AM -0700, Vanshidhar Konda wrote:
>> > > > > Extend the timeout for the hardware to signal SEND_BUSY on the DP
>> > > > > Aux Channel Controller register.
>> > > > >
>> > > > > This is needed to address FDO #109982
>> > > > > https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
>> > > >
>> > > > instead of mentioning like this, please use:
>> > > > Bugzilla: https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
>> > > >
>> > > > Also instead of the "needed to address" it would be better
>> > > > to add some reasoning explaining that
>> > > > "empirically we got some bugs workarounded by increasing the timeout
>> > > > from 10ms to 15ms although spec was only requiring 4ms"
>> > > > or something like that...
>> > > >
>> > > Thanks! I'll keep these in mind for next patches.
>> > > > >
>> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > > > > Cc: Imre Deak <imre.deak@intel.com>
>> > > > > Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
>> > > > > ---
>> > > > > drivers/gpu/drm/i915/intel_dp.c | 4 +++-
>> > > > > 1 file changed, 3 insertions(+), 1 deletion(-)
>> > > > >
>> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> > > > > index 47857f96c3b1..fd6de33c5664 100644
>> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > > > > @@ -1053,6 +1053,8 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
>> > > > > 	}
>> > > > > }
>> > > > >
>> > > > > +#define DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS 15
>> > > >
>> > > > This define is spurious since it's in use in a single place.
>> > > >
>> > > > Also, giving the timeout a name, like this, makes it appear it came from
>> > > > the spec. Well, if it came from Spec it should be defined in the proper .h
>> > > > files.
>> > > >
>> > > > Since I don't believe this came from spec I believe we can just remove it
>> > > > and go for the timeout directly on the function below.
>> > > >
>> > > I'll make this change in the next patch.
>> > > > > +
>> > > > > static u32
>> > > > > intel_dp_aux_wait_done(struct intel_dp *intel_dp)
>> > > > > {
>> > > > > @@ -1063,7 +1065,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
>> > > > >
>> > > > > #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
>> > > > > 	done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
>> > > > > -				  msecs_to_jiffies_timeout(10));
>> > > > > +				  msecs_to_jiffies_timeout(DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS));
>> > > >
>> > > > Is this just a guess that you are trying to check?
>> > > > I'm asking because I didn't see any indication that the increase
>> > > > really fixed the issue.
>> > > >
>> > > > So, if you are trying to just validate your approach maybe the try-bot
>> > > > could be used?
>> > > >
>> > > I also would have preferred to use the trybot. The error in Bugzilla 109982 is only reproduced on only shard-iclb2 machine once
>> > > in approximately 2 days. So the trybot is not of much help for this.
>> > >
>> > > Is there another way of testing experimental patches for bugs that are
>> > > not reproduced locally or on other machines? Or, if the issue is only
>> > > happening on one machine, should it be lowered in priority/whitelisted
>> > > on the specific CI machine?
>> >
>> > For now I believe the best alternative is to --subject-prefix="[CI]"
>> >
>> > But also you need to be sure that the fail rate don't fool us...
>>
>> hmm... this testcase seems to have a passrate of 77% of the runs.
>> So probably using the CI infrastructure here isn't the best alternative
>> to validate this approach in general.
>
>ops... I forgot to paste the link from these data:
>https://intel-gfx-ci.01.org/tree/drm-tip/shard-iclb.html
>(30 runs out of 45 passing)
>
Sorry, I don't think I understood what you are saying here. The CI/IGT
test that is reported in the bug itself doesn't have an issue. It pases
successfully.

The issue reported by Martin is that there is an error printed out to dmesg.
<3> [1128.098368] [drm:intel_dp_aux_xfer [i915]] *ERROR* dp aux hw did
not signal timeout!

I noticed that there is a similar failure on one other KBL system
(fi-kbl-7500u) where this issue occurs regularly. But on that platform
even after 5 try programming the DP Aux Channel Controller fails.

So, this might be a one off hardware platform issue?

>>
>> >
>> > I believe there will be better alternatives for using CI for cases like
>> > this soon, so +Martin
>> >
>> > >
>> > > > Thanks,
>> > > > Rodrigo.
>> > > >
>> > > > >
>> > > > > 	/* just trace the final value */
>> > > > > 	trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
>> > > > > --
>> > > > > 2.20.1
>> > > > >
>> > > > > _______________________________________________
>> > > > > Intel-gfx mailing list
>> > > > > Intel-gfx@lists.freedesktop.org
>> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915/display: Increase timeout for DP Aux channel ctl signal

[Patchwork]

== Series Details ==

Series: drm/i915/display: Increase timeout for DP Aux channel ctl signal
URL   : https://patchwork.freedesktop.org/series/58077/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5753_full -> Patchwork_12485_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_12485_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_12485_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_12485_full:

### IGT changes ###

#### Warnings ####

  * igt@gem_tiled_fence_blits@normal:
    - shard-iclb:         TIMEOUT [fdo#109673] -> INCOMPLETE

  
Known issues
------------

  Here are the changes found in Patchwork_12485_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@in-flight-immediate:
    - shard-hsw:          PASS -> INCOMPLETE [fdo#103540]

  * igt@gem_exec_parallel@bsd1:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +78

  * igt@gem_exec_params@no-blt:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109283]

  * igt@gem_ppgtt@blt-vs-render-ctxn:
    - shard-iclb:         NOTRUN -> INCOMPLETE [fdo#107713] / [fdo#109766] / [fdo#109801]

  * igt@gem_pwrite@huge-gtt-random:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109290]

  * igt@i915_pm_backlight@fade_with_suspend:
    - shard-iclb:         NOTRUN -> FAIL [fdo#107847]

  * igt@i915_pm_rps@reset:
    - shard-iclb:         NOTRUN -> FAIL [fdo#108059] +1

  * igt@kms_atomic_transition@1x-modeset-transitions-fencing:
    - shard-skl:          PASS -> FAIL [fdo#107815] / [fdo#108470]

  * igt@kms_busy@basic-modeset-e:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-hsw:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-hang-oldfb-render-d:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278] +4

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107956]
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956] +1

  * igt@kms_chamelium@vga-edid-read:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109284] +1

  * igt@kms_color@pipe-a-legacy-gamma:
    - shard-skl:          PASS -> FAIL [fdo#104782] / [fdo#108145]

  * igt@kms_color@pipe-b-degamma:
    - shard-skl:          PASS -> FAIL [fdo#104782]

  * igt@kms_cursor_crc@cursor-128x128-onscreen:
    - shard-skl:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-256x85-random:
    - shard-apl:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x64-sliding:
    - shard-snb:          PASS -> SKIP [fdo#109271] +2

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          PASS -> FAIL [fdo#104873]

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions:
    - shard-iclb:         PASS -> FAIL [fdo#103355]

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109349]

  * igt@kms_flip@2x-blocking-absolute-wf_vblank-interruptible:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274] +1

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc:
    - shard-skl:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-apl:          PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@fbc-2p-pri-indfb-multidraw:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +13

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +44
    - shard-glk:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbc-stridechange:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +4

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-pwrite:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +13

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-fullscreen:
    - shard-iclb:         NOTRUN -> FAIL [fdo#109247] +4

  * igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +2

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815]

  * igt@kms_plane_scaling@pipe-c-scaler-with-pixel-format:
    - shard-iclb:         NOTRUN -> FAIL [fdo#109052]

  * igt@kms_properties@connector-properties-atomic:
    - shard-apl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] +3

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +2

  * igt@kms_psr@psr2_sprite_render:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109441]

  * igt@kms_psr@sprite_plane_onoff:
    - shard-iclb:         PASS -> FAIL [fdo#107383] +2

  * igt@kms_rotation_crc@bad-pixel-format:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109289]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          PASS -> FAIL [fdo#109016]

  * igt@kms_universal_plane@cursor-fb-leak-pipe-d:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +10

  * igt@perf@polling:
    - shard-iclb:         PASS -> FAIL [fdo#108587]

  * igt@prime_nv_api@i915_nv_import_twice_check_flink_name:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109291] +2

  * igt@prime_vgem@coherency-gtt:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109292]

  * igt@prime_vgem@fence-wait-bsd1:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +10

  
#### Possible fixes ####

  * igt@gem_exec_schedule@wide-blt:
    - shard-iclb:         DMESG-WARN [fdo#109638] -> PASS

  * igt@gem_exec_suspend@basic-s3:
    - shard-skl:          INCOMPLETE [fdo#104108] -> PASS

  * igt@gem_mmap_gtt@big-copy-xy:
    - shard-iclb:         TIMEOUT [fdo#109673] -> PASS

  * igt@gem_mmap_gtt@hang:
    - shard-iclb:         FAIL [fdo#109677] -> PASS

  * igt@gem_partial_pwrite_pread@writes-after-reads-snoop:
    - shard-iclb:         INCOMPLETE [fdo#107713] -> PASS

  * igt@gem_ppgtt@blt-vs-render-ctx0:
    - shard-iclb:         INCOMPLETE [fdo#109100] / [fdo#109766] / [fdo#109801] -> PASS

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
    - shard-iclb:         FAIL [fdo#107725] -> PASS

  * igt@kms_cursor_legacy@cursor-vs-flip-legacy:
    - shard-iclb:         FAIL [fdo#103355] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          FAIL [fdo#105363] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-mmap-gtt:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +8

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-cpu:
    - shard-apl:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
    - shard-glk:          FAIL [fdo#103167] -> PASS +2

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +14

  * {igt@kms_plane@pixel-format-pipe-b-planes-source-clamping}:
    - shard-apl:          FAIL [fdo#110033] -> PASS

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          FAIL [fdo#108145] -> PASS

  * {igt@kms_plane_multiple@atomic-pipe-a-tiling-x}:
    - shard-apl:          FAIL [fdo#110037] -> PASS +1

  * {igt@kms_plane_multiple@atomic-pipe-b-tiling-none}:
    - shard-glk:          FAIL [fdo#110037] -> PASS

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         SKIP [fdo#109642] -> PASS

  * igt@kms_psr@cursor_mmap_cpu:
    - shard-iclb:         FAIL [fdo#107383] -> PASS

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         SKIP [fdo#109441] -> PASS

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          DMESG-FAIL [fdo#105763] -> PASS

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> PASS
    - shard-glk:          FAIL [fdo#99912] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107847]: https://bugs.freedesktop.org/show_bug.cgi?id=107847
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108059]: https://bugs.freedesktop.org/show_bug.cgi?id=108059
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108470]: https://bugs.freedesktop.org/show_bug.cgi?id=108470
  [fdo#108587]: https://bugs.freedesktop.org/show_bug.cgi?id=108587
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109052]: https://bugs.freedesktop.org/show_bug.cgi?id=109052
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109283]: https://bugs.freedesktop.org/show_bug.cgi?id=109283
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109290]: https://bugs.freedesktop.org/show_bug.cgi?id=109290
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109292]: https://bugs.freedesktop.org/show_bug.cgi?id=109292
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109638]: https://bugs.freedesktop.org/show_bug.cgi?id=109638
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#109677]: https://bugs.freedesktop.org/show_bug.cgi?id=109677
  [fdo#109766]: https://bugs.freedesktop.org/show_bug.cgi?id=109766
  [fdo#109801]: https://bugs.freedesktop.org/show_bug.cgi?id=109801
  [fdo#110033]: https://bugs.freedesktop.org/show_bug.cgi?id=110033
  [fdo#110037]: https://bugs.freedesktop.org/show_bug.cgi?id=110037
  [fdo#110038]: https://bugs.freedesktop.org/show_bug.cgi?id=110038
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

    * Linux: CI_DRM_5753 -> Patchwork_12485

  CI_DRM_5753: 0eb0838c0c26378949de6816166117c8b2d73caa @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4887: 5a7c7575b5bb9542f722ed6ba095b9d62609cd56 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12485: 3dc430594d09c21b1fe8b5fb0f5c941afe2707a8 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12485/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/display: Increase timeout for DP Aux channel ctl signal

[Rodrigo Vivi]

On Fri, Mar 15, 2019 at 04:05:12PM -0700, Vanshidhar Konda wrote:
> On Fri, Mar 15, 2019 at 02:43:46PM -0700, Rodrigo Vivi wrote:
> > On Fri, Mar 15, 2019 at 02:39:25PM -0700, Rodrigo Vivi wrote:
> > > On Fri, Mar 15, 2019 at 02:31:40PM -0700, Rodrigo Vivi wrote:
> > > > On Fri, Mar 15, 2019 at 01:27:22PM -0700, Vanshidhar Konda wrote:
> > > > > On Fri, Mar 15, 2019 at 12:38:41PM -0700, Rodrigo Vivi wrote:
> > > > > > On Fri, Mar 15, 2019 at 11:39:54AM -0700, Vanshidhar Konda wrote:
> > > > > > > Extend the timeout for the hardware to signal SEND_BUSY on the DP
> > > > > > > Aux Channel Controller register.
> > > > > > >
> > > > > > > This is needed to address FDO #109982
> > > > > > > https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
> > > > > >
> > > > > > instead of mentioning like this, please use:
> > > > > > Bugzilla: https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
> > > > > >
> > > > > > Also instead of the "needed to address" it would be better
> > > > > > to add some reasoning explaining that
> > > > > > "empirically we got some bugs workarounded by increasing the timeout
> > > > > > from 10ms to 15ms although spec was only requiring 4ms"
> > > > > > or something like that...
> > > > > >
> > > > > Thanks! I'll keep these in mind for next patches.
> > > > > > >
> > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > > > Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> > > > > > > ---
> > > > > > > drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> > > > > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > > index 47857f96c3b1..fd6de33c5664 100644
> > > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > > @@ -1053,6 +1053,8 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> > > > > > > 	}
> > > > > > > }
> > > > > > >
> > > > > > > +#define DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS 15
> > > > > >
> > > > > > This define is spurious since it's in use in a single place.
> > > > > >
> > > > > > Also, giving the timeout a name, like this, makes it appear it came from
> > > > > > the spec. Well, if it came from Spec it should be defined in the proper .h
> > > > > > files.
> > > > > >
> > > > > > Since I don't believe this came from spec I believe we can just remove it
> > > > > > and go for the timeout directly on the function below.
> > > > > >
> > > > > I'll make this change in the next patch.
> > > > > > > +
> > > > > > > static u32
> > > > > > > intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > > > > > {
> > > > > > > @@ -1063,7 +1065,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > > > > >
> > > > > > > #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
> > > > > > > 	done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > > > > > > -				  msecs_to_jiffies_timeout(10));
> > > > > > > +				  msecs_to_jiffies_timeout(DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS));
> > > > > >
> > > > > > Is this just a guess that you are trying to check?
> > > > > > I'm asking because I didn't see any indication that the increase
> > > > > > really fixed the issue.
> > > > > >
> > > > > > So, if you are trying to just validate your approach maybe the try-bot
> > > > > > could be used?
> > > > > >
> > > > > I also would have preferred to use the trybot. The error in Bugzilla 109982 is only reproduced on only shard-iclb2 machine once
> > > > > in approximately 2 days. So the trybot is not of much help for this.
> > > > >
> > > > > Is there another way of testing experimental patches for bugs that are
> > > > > not reproduced locally or on other machines? Or, if the issue is only
> > > > > happening on one machine, should it be lowered in priority/whitelisted
> > > > > on the specific CI machine?
> > > >
> > > > For now I believe the best alternative is to --subject-prefix="[CI]"
> > > >
> > > > But also you need to be sure that the fail rate don't fool us...
> > > 
> > > hmm... this testcase seems to have a passrate of 77% of the runs.
> > > So probably using the CI infrastructure here isn't the best alternative
> > > to validate this approach in general.
> > 
> > ops... I forgot to paste the link from these data:
> > https://intel-gfx-ci.01.org/tree/drm-tip/shard-iclb.html
> > (30 runs out of 45 passing)
> > 
> Sorry, I don't think I understood what you are saying here. The CI/IGT
> test that is reported in the bug itself doesn't have an issue. It pases
> successfully.
> 
> The issue reported by Martin is that there is an error printed out to dmesg.
> <3> [1128.098368] [drm:intel_dp_aux_xfer [i915]] *ERROR* dp aux hw did
> not signal timeout!

30 runs out of 45 of this test on this ICL comes with full clean result.
This mean this message of *ERROR* didn't occur.

Martin can correct me if my interpretation is wrong here.

But with this "high" pass rate you won't be able to prove that your
timeout increase is the real fix of the problem. Unless you are able
to reproduce it locally and run many times enough to validate this is
reliably and constantly passing.

> 
> I noticed that there is a similar failure on one other KBL system
> (fi-kbl-7500u) where this issue occurs regularly. But on that platform
> even after 5 try programming the DP Aux Channel Controller fails.
> 
> So, this might be a one off hardware platform issue?
> 
> > > 
> > > >
> > > > I believe there will be better alternatives for using CI for cases like
> > > > this soon, so +Martin
> > > >
> > > > >
> > > > > > Thanks,
> > > > > > Rodrigo.
> > > > > >
> > > > > > >
> > > > > > > 	/* just trace the final value */
> > > > > > > 	trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
> > > > > > > --
> > > > > > > 2.20.1
> > > > > > >
> > > > > > > _______________________________________________
> > > > > > > Intel-gfx mailing list
> > > > > > > Intel-gfx@lists.freedesktop.org
> > > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx