[PATCH] [drm][i915] Increase LSPCON timeout

[PATCH] [drm][i915] Increase LSPCON timeout

[Fredrik Schön]

100 ms is not enough time for the LSPCON adapter on Intel NUC devices to
settle. This causes dropped display modes at driver initialisation.

Increase timeout to 1000 ms.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
Signed-off-by: Fredrik Schön <fredrik.schon@gmail.com>
---
 drivers/gpu/drm/i915/intel_lspcon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 8ae8f42f430a..be1b08f589a4 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -74,7 +74,7 @@ static enum drm_lspcon_mode lspcon_wait_mode(struct intel_lspcon *lspcon,
 	DRM_DEBUG_KMS("Waiting for LSPCON mode %s to settle\n",
 		      lspcon_mode_name(mode));
 
-	wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 100);
+	wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 1000);
 	if (current_mode != mode)
 		DRM_ERROR("LSPCON mode hasn't settled\n");
 
-- 
2.17.1

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

✓ Fi.CI.BAT: success for Increase LSPCON timeout

[Patchwork]

== Series Details ==

Series: Increase LSPCON timeout
URL   : https://patchwork.freedesktop.org/series/48183/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4665 -> Patchwork_9937 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@debugfs_test@read_all_entries:
      {fi-icl-u}:         PASS -> DMESG-FAIL (fdo#107411)

    igt@drv_selftest@live_hangcheck:
      fi-kbl-guc:         PASS -> INCOMPLETE (fdo#106693, fdo#107207)

    igt@gem_exec_reloc@basic-gtt-read-noreloc:
      {fi-icl-u}:         PASS -> DMESG-WARN (fdo#107411) +77

    igt@kms_frontbuffer_tracking@basic:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#107362)

    
    ==== Possible fixes ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    igt@drv_selftest@live_hangcheck:
      fi-skl-guc:         DMESG-FAIL (fdo#107174) -> PASS

    igt@drv_selftest@live_workarounds:
      fi-cnl-psr:         DMESG-FAIL (fdo#107292) -> PASS
      fi-skl-6700k2:      DMESG-FAIL (fdo#107292) -> PASS

    igt@kms_chamelium@dp-crc-fast:
      fi-kbl-7500u:       DMESG-FAIL (fdo#103841) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

    {igt@kms_psr@primary_mmap_gtt}:
      fi-cnl-psr:         DMESG-WARN (fdo#107372) -> 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#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#106693 https://bugs.freedesktop.org/show_bug.cgi?id=106693
  fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
  fdo#107207 https://bugs.freedesktop.org/show_bug.cgi?id=107207
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372
  fdo#107411 https://bugs.freedesktop.org/show_bug.cgi?id=107411


== Participating hosts (54 -> 49) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4665 -> Patchwork_9937

  CI_DRM_4665: 3a43bf5efa33be9f88d70427dbfba4db4377eaf6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4594: b0263e5d0563a81a42cf66e7d3b84662d3222862 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9937: 977c85e40247ce7d9dc570748f6bbea3473bad75 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

977c85e40247 Increase LSPCON timeout

== Logs ==

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

✓ Fi.CI.IGT: success for Increase LSPCON timeout

[Patchwork]

== Series Details ==

Series: Increase LSPCON timeout
URL   : https://patchwork.freedesktop.org/series/48183/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4665_full -> Patchwork_9937_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_workarounds@suspend-resume:
      shard-glk:          NOTRUN -> FAIL (fdo#103375)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@drv_suspend@shrink:
      shard-glk:          INCOMPLETE (fdo#106886, k.org#198133, fdo#103359) -> PASS

    igt@gem_workarounds@suspend-resume:
      shard-apl:          FAIL (fdo#103375) -> PASS

    
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4665 -> Patchwork_9937

  CI_DRM_4665: 3a43bf5efa33be9f88d70427dbfba4db4377eaf6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4594: b0263e5d0563a81a42cf66e7d3b84662d3222862 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9937: 977c85e40247ce7d9dc570748f6bbea3473bad75 @ 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_9937/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] [drm][i915] Increase LSPCON timeout

[Rodrigo Vivi]

On Mon, Aug 13, 2018 at 07:51:33PM +0200, Fredrik Schön wrote:

First of all we need to fix the commit subject:

drm/i915: Increase LSPCON timeout

(this can be done when merging, no need to resend)

> 100 ms is not enough time for the LSPCON adapter on Intel NUC devices to
> settle. This causes dropped display modes at driver initialisation.
> 
> Increase timeout to 1000 ms.
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1570392

Missusage of "Fixes:" tag, please read

https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html#fixes

But also no need for resending... could be fixed when merging

The right one would be:

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
Fixes: 357c0ae9198a ("drm/i915/lspcon: Wait for expected LSPCON mode to settle")
Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: <stable@vger.kernel.org> # v4.11+

Since initial 100 seemed to be empirical and this increase seems to
help other cases I'm in favor of this move so

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

However I will wait a bit before merging it
so Imre, Shashank, and/or Jani can take a look here...

> Signed-off-by: Fredrik Schön <fredrik.schon@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_lspcon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> index 8ae8f42f430a..be1b08f589a4 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -74,7 +74,7 @@ static enum drm_lspcon_mode lspcon_wait_mode(struct intel_lspcon *lspcon,
>  	DRM_DEBUG_KMS("Waiting for LSPCON mode %s to settle\n",
>  		      lspcon_mode_name(mode));
>  
> -	wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 100);
> +	wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 1000);
>  	if (current_mode != mode)
>  		DRM_ERROR("LSPCON mode hasn't settled\n");
>  
> -- 
> 2.17.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] Increase LSPCON timeout

[Rodrigo Vivi]

On Wed, Aug 15, 2018 at 03:39:40PM -0700, Rodrigo Vivi wrote:
> On Mon, Aug 13, 2018 at 07:51:33PM +0200, Fredrik Schön wrote:
> 
> First of all we need to fix the commit subject:
> 
> drm/i915: Increase LSPCON timeout
> 
> (this can be done when merging, no need to resend)
> 
> > 100 ms is not enough time for the LSPCON adapter on Intel NUC devices to
> > settle. This causes dropped display modes at driver initialisation.
> > 
> > Increase timeout to 1000 ms.
> > 
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> 
> Missusage of "Fixes:" tag, please read
> 
> https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html#fixes
> 
> But also no need for resending... could be fixed when merging
> 
> The right one would be:
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> Fixes: 357c0ae9198a ("drm/i915/lspcon: Wait for expected LSPCON mode to settle")
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: <stable@vger.kernel.org> # v4.11+
> 
> Since initial 100 seemed to be empirical and this increase seems to
> help other cases I'm in favor of this move so
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> However I will wait a bit before merging it
> so Imre, Shashank, and/or Jani can take a look here...

now, really cc'ing them...

> 
> > Signed-off-by: Fredrik Schön <fredrik.schon@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/intel_lspcon.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> > index 8ae8f42f430a..be1b08f589a4 100644
> > --- a/drivers/gpu/drm/i915/intel_lspcon.c
> > +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> > @@ -74,7 +74,7 @@ static enum drm_lspcon_mode lspcon_wait_mode(struct intel_lspcon *lspcon,
> >  	DRM_DEBUG_KMS("Waiting for LSPCON mode %s to settle\n",
> >  		      lspcon_mode_name(mode));
> >  
> > -	wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 100);
> > +	wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 1000);
> >  	if (current_mode != mode)
> >  		DRM_ERROR("LSPCON mode hasn't settled\n");
> >  
> > -- 
> > 2.17.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] Increase LSPCON timeout

[Jani Nikula]

On Wed, 15 Aug 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Wed, Aug 15, 2018 at 03:39:40PM -0700, Rodrigo Vivi wrote:
>> On Mon, Aug 13, 2018 at 07:51:33PM +0200, Fredrik Schön wrote:
>> 
>> First of all we need to fix the commit subject:
>> 
>> drm/i915: Increase LSPCON timeout
>> 
>> (this can be done when merging, no need to resend)
>> 
>> > 100 ms is not enough time for the LSPCON adapter on Intel NUC devices to
>> > settle. This causes dropped display modes at driver initialisation.
>> > 
>> > Increase timeout to 1000 ms.
>> > 
>> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
>> 
>> Missusage of "Fixes:" tag, please read
>> 
>> https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html#fixes
>> 
>> But also no need for resending... could be fixed when merging
>> 
>> The right one would be:
>> 
>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
>> Fixes: 357c0ae9198a ("drm/i915/lspcon: Wait for expected LSPCON mode to settle")
>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: <stable@vger.kernel.org> # v4.11+
>> 
>> Since initial 100 seemed to be empirical and this increase seems to
>> help other cases I'm in favor of this move so
>> 
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> 
>> However I will wait a bit before merging it
>> so Imre, Shashank, and/or Jani can take a look here...
>
> now, really cc'ing them...

Shashank? Does this slow down non-LSPCON paths?

BR,
Jani.


>
>> 
>> > Signed-off-by: Fredrik Schön <fredrik.schon@gmail.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_lspcon.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
>> > index 8ae8f42f430a..be1b08f589a4 100644
>> > --- a/drivers/gpu/drm/i915/intel_lspcon.c
>> > +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>> > @@ -74,7 +74,7 @@ static enum drm_lspcon_mode lspcon_wait_mode(struct intel_lspcon *lspcon,
>> >  	DRM_DEBUG_KMS("Waiting for LSPCON mode %s to settle\n",
>> >  		      lspcon_mode_name(mode));
>> >  
>> > -	wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 100);
>> > +	wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 1000);
>> >  	if (current_mode != mode)
>> >  		DRM_ERROR("LSPCON mode hasn't settled\n");
>> >  
>> > -- 
>> > 2.17.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

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] [drm][i915] Increase LSPCON timeout

[Sharma, Shashank]

Regards

Shashank


On 8/16/2018 12:47 PM, Jani Nikula wrote:
> On Wed, 15 Aug 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>> On Wed, Aug 15, 2018 at 03:39:40PM -0700, Rodrigo Vivi wrote:
>>> On Mon, Aug 13, 2018 at 07:51:33PM +0200, Fredrik Schön wrote:
>>>
>>> First of all we need to fix the commit subject:
>>>
>>> drm/i915: Increase LSPCON timeout
>>>
>>> (this can be done when merging, no need to resend)
>>>
>>>> 100 ms is not enough time for the LSPCON adapter on Intel NUC devices to
>>>> settle. This causes dropped display modes at driver initialisation.
>>>>
>>>> Increase timeout to 1000 ms.
>>>>
>>>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
>>> Missusage of "Fixes:" tag, please read
>>>
>>> https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html#fixes
>>>
>>> But also no need for resending... could be fixed when merging
>>>
>>> The right one would be:
>>>
>>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
>>> Fixes: 357c0ae9198a ("drm/i915/lspcon: Wait for expected LSPCON mode to settle")
>>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>>> Cc: Imre Deak <imre.deak@intel.com>
>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>> Cc: <stable@vger.kernel.org> # v4.11+
>>>
>>> Since initial 100 seemed to be empirical and this increase seems to
>>> help other cases I'm in favor of this move so
>>>
>>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>
>>> However I will wait a bit before merging it
>>> so Imre, Shashank, and/or Jani can take a look here...
>> now, really cc'ing them...
> Shashank? Does this slow down non-LSPCON paths?
This will slow down the lspcon probing and resume part, but both of them 
happen only when LSPCON device is found. So to answer your question, 
this will not slow down the non-lspcon path, but will slow down the 
LSPCON connector resume and probe time. but I would recommend, instead 
of increasing it to 1000 ms in a single shot, we might want to gradually 
pick this up, on a wake-and-check way.

- Shashank
>
> BR,
> Jani.
>
>
>>>> Signed-off-by: Fredrik Schön <fredrik.schon@gmail.com>
>>>> ---
>>>>   drivers/gpu/drm/i915/intel_lspcon.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
>>>> index 8ae8f42f430a..be1b08f589a4 100644
>>>> --- a/drivers/gpu/drm/i915/intel_lspcon.c
>>>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>>>> @@ -74,7 +74,7 @@ static enum drm_lspcon_mode lspcon_wait_mode(struct intel_lspcon *lspcon,
>>>>   	DRM_DEBUG_KMS("Waiting for LSPCON mode %s to settle\n",
>>>>   		      lspcon_mode_name(mode));
>>>>   
>>>> -	wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 100);
>>>> +	wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 1000);
>>>>   	if (current_mode != mode)
>>>>   		DRM_ERROR("LSPCON mode hasn't settled\n");
>>>>   
>>>> -- 
>>>> 2.17.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] Increase LSPCON timeout

[Chris Wilson]

Quoting Sharma, Shashank (2018-08-16 08:33:36)
> Regards
> 
> Shashank
> 
> 
> On 8/16/2018 12:47 PM, Jani Nikula wrote:
> > On Wed, 15 Aug 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> >> On Wed, Aug 15, 2018 at 03:39:40PM -0700, Rodrigo Vivi wrote:
> >>> On Mon, Aug 13, 2018 at 07:51:33PM +0200, Fredrik Schön wrote:
> >>>
> >>> First of all we need to fix the commit subject:
> >>>
> >>> drm/i915: Increase LSPCON timeout
> >>>
> >>> (this can be done when merging, no need to resend)
> >>>
> >>>> 100 ms is not enough time for the LSPCON adapter on Intel NUC devices to
> >>>> settle. This causes dropped display modes at driver initialisation.
> >>>>
> >>>> Increase timeout to 1000 ms.
> >>>>
> >>>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> >>> Missusage of "Fixes:" tag, please read
> >>>
> >>> https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html#fixes
> >>>
> >>> But also no need for resending... could be fixed when merging
> >>>
> >>> The right one would be:
> >>>
> >>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> >>> Fixes: 357c0ae9198a ("drm/i915/lspcon: Wait for expected LSPCON mode to settle")
> >>> Cc: Shashank Sharma <shashank.sharma@intel.com>
> >>> Cc: Imre Deak <imre.deak@intel.com>
> >>> Cc: Jani Nikula <jani.nikula@intel.com>
> >>> Cc: <stable@vger.kernel.org> # v4.11+
> >>>
> >>> Since initial 100 seemed to be empirical and this increase seems to
> >>> help other cases I'm in favor of this move so
> >>>
> >>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >>>
> >>> However I will wait a bit before merging it
> >>> so Imre, Shashank, and/or Jani can take a look here...
> >> now, really cc'ing them...
> > Shashank? Does this slow down non-LSPCON paths?
> This will slow down the lspcon probing and resume part, but both of them 
> happen only when LSPCON device is found. So to answer your question, 
> this will not slow down the non-lspcon path, but will slow down the 
> LSPCON connector resume and probe time. but I would recommend, instead 
> of increasing it to 1000 ms in a single shot, we might want to gradually 
> pick this up, on a wake-and-check way.

wait_for() checks every [10us, 1ms] until the condition is met, or it
times out. So, so long as we don't enter this path for !LPSCON where we
know that it will timeout, the wait_for() will only take as long as is
required for the connector to settle.

Can we do other connectors at the same time, or does probing LSPCON
block the system?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] [drm][i915] Increase LSPCON timeout

[Sharma, Shashank]

Hey Chris,


On 8/16/2018 1:13 PM, Chris Wilson wrote:
> Quoting Sharma, Shashank (2018-08-16 08:33:36)
>> Regards
>>
>> Shashank
>>
>>
>> On 8/16/2018 12:47 PM, Jani Nikula wrote:
>>> On Wed, 15 Aug 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>>>> On Wed, Aug 15, 2018 at 03:39:40PM -0700, Rodrigo Vivi wrote:
>>>>> On Mon, Aug 13, 2018 at 07:51:33PM +0200, Fredrik Schön wrote:
>>>>>
>>>>> First of all we need to fix the commit subject:
>>>>>
>>>>> drm/i915: Increase LSPCON timeout
>>>>>
>>>>> (this can be done when merging, no need to resend)
>>>>>
>>>>>> 100 ms is not enough time for the LSPCON adapter on Intel NUC devices to
>>>>>> settle. This causes dropped display modes at driver initialisation.
>>>>>>
>>>>>> Increase timeout to 1000 ms.
>>>>>>
>>>>>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
>>>>> Missusage of "Fixes:" tag, please read
>>>>>
>>>>> https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html#fixes
>>>>>
>>>>> But also no need for resending... could be fixed when merging
>>>>>
>>>>> The right one would be:
>>>>>
>>>>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
>>>>> Fixes: 357c0ae9198a ("drm/i915/lspcon: Wait for expected LSPCON mode to settle")
>>>>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>>>>> Cc: Imre Deak <imre.deak@intel.com>
>>>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>>>> Cc: <stable@vger.kernel.org> # v4.11+
>>>>>
>>>>> Since initial 100 seemed to be empirical and this increase seems to
>>>>> help other cases I'm in favor of this move so
>>>>>
>>>>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>>>
>>>>> However I will wait a bit before merging it
>>>>> so Imre, Shashank, and/or Jani can take a look here...
>>>> now, really cc'ing them...
>>> Shashank? Does this slow down non-LSPCON paths?
>> This will slow down the lspcon probing and resume part, but both of them
>> happen only when LSPCON device is found. So to answer your question,
>> this will not slow down the non-lspcon path, but will slow down the
>> LSPCON connector resume and probe time. but I would recommend, instead
>> of increasing it to 1000 ms in a single shot, we might want to gradually
>> pick this up, on a wake-and-check way.
> wait_for() checks every [10us, 1ms] until the condition is met, or it
> times out. So, so long as we don't enter this path for !LPSCON where we
> know that it will timeout, the wait_for() will only take as long as is
> required for the connector to settle.
We wont hit !LSPCON timeout case here, as we have already read the 
dongle signature successfully by now.  But I was thinking that, if the 
spec recommends max wait time as 100ms (which is of course doesn't seem 
enough), if we can't detect i2c-over-aux after first 500ms, I guess we 
wont be able to do that in next 500ms too. So is it really ok to wait 
this long in the resume sequence ?

I guess Fredrik can provide some inputs here on if there are some 
experiments behind this number of 1000ms, or this is just a safe bet ?
>
> Can we do other connectors at the same time, or does probing LSPCON
> block the system?
We can do other connectors at the same time in DRM layer at-least, 
LSPCON blocks only this connector. I was curious if are we doing this 
during the resume scenario or is this in the sequential get_connector() 
type of call  ?
- Shashank
> -Chris

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

Re: [PATCH] [drm][i915] Increase LSPCON timeout

[Fredrik Schön]

Shashank,

Den tors 16 aug. 2018 kl 10:15 skrev Sharma, Shashank
<shashank.sharma@intel.com>:
>
> Hey Chris,
>
>
> On 8/16/2018 1:13 PM, Chris Wilson wrote:
> > Quoting Sharma, Shashank (2018-08-16 08:33:36)
> >> Regards
> >>
> >> Shashank
> >>
> >>
> >> On 8/16/2018 12:47 PM, Jani Nikula wrote:
> >>> On Wed, 15 Aug 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> >>>> On Wed, Aug 15, 2018 at 03:39:40PM -0700, Rodrigo Vivi wrote:
> >>>>> On Mon, Aug 13, 2018 at 07:51:33PM +0200, Fredrik Schön wrote:
> >>>>>
> >>>>> First of all we need to fix the commit subject:
> >>>>>
> >>>>> drm/i915: Increase LSPCON timeout
> >>>>>
> >>>>> (this can be done when merging, no need to resend)
> >>>>>
> >>>>>> 100 ms is not enough time for the LSPCON adapter on Intel NUC devices to
> >>>>>> settle. This causes dropped display modes at driver initialisation.
> >>>>>>
> >>>>>> Increase timeout to 1000 ms.
> >>>>>>
> >>>>>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> >>>>> Missusage of "Fixes:" tag, please read
> >>>>>
> >>>>> https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html#fixes
> >>>>>
> >>>>> But also no need for resending... could be fixed when merging
> >>>>>
> >>>>> The right one would be:
> >>>>>
> >>>>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> >>>>> Fixes: 357c0ae9198a ("drm/i915/lspcon: Wait for expected LSPCON mode to settle")
> >>>>> Cc: Shashank Sharma <shashank.sharma@intel.com>
> >>>>> Cc: Imre Deak <imre.deak@intel.com>
> >>>>> Cc: Jani Nikula <jani.nikula@intel.com>
> >>>>> Cc: <stable@vger.kernel.org> # v4.11+
> >>>>>
> >>>>> Since initial 100 seemed to be empirical and this increase seems to
> >>>>> help other cases I'm in favor of this move so
> >>>>>
> >>>>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >>>>>
> >>>>> However I will wait a bit before merging it
> >>>>> so Imre, Shashank, and/or Jani can take a look here...
> >>>> now, really cc'ing them...
> >>> Shashank? Does this slow down non-LSPCON paths?
> >> This will slow down the lspcon probing and resume part, but both of them
> >> happen only when LSPCON device is found. So to answer your question,
> >> this will not slow down the non-lspcon path, but will slow down the
> >> LSPCON connector resume and probe time. but I would recommend, instead
> >> of increasing it to 1000 ms in a single shot, we might want to gradually
> >> pick this up, on a wake-and-check way.
> > wait_for() checks every [10us, 1ms] until the condition is met, or it
> > times out. So, so long as we don't enter this path for !LPSCON where we
> > know that it will timeout, the wait_for() will only take as long as is
> > required for the connector to settle.
> We wont hit !LSPCON timeout case here, as we have already read the
> dongle signature successfully by now.  But I was thinking that, if the
> spec recommends max wait time as 100ms (which is of course doesn't seem
> enough), if we can't detect i2c-over-aux after first 500ms, I guess we
> wont be able to do that in next 500ms too. So is it really ok to wait
> this long in the resume sequence ?
>
> I guess Fredrik can provide some inputs here on if there are some
> experiments behind this number of 1000ms, or this is just a safe bet ?
> >

My first patch attempt - which is attached to the Redhat and FDO Bugzilla
bugs - added a retry loop around the original 100 ms timeout. The retry loop
did trigger, but never more than once in a row in my testing.

So possibly 200 ms would be a sufficient timeout, but as the wait_for() loop
terminates early on success I suggested a conservative value of 1000 ms.

> > Can we do other connectors at the same time, or does probing LSPCON
> > block the system?
> We can do other connectors at the same time in DRM layer at-least,
> LSPCON blocks only this connector. I was curious if are we doing this
> during the resume scenario or is this in the sequential get_connector()
> type of call  ?
> - Shashank
> > -Chris
/F
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] [drm][i915] Increase LSPCON timeout

[Saarinen, Jani]

Hi, 
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Jani Nikula
> Sent: torstai 16. elokuuta 2018 10.18
> To: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Fredrik Schön
> <fredrikschon@gmail.com>
> Cc: intel-gfx@lists.freedesktop.org; Fredrik Schön
> <fredrik.schon@gmail.com>
> Subject: Re: [Intel-gfx] [PATCH] [drm][i915] Increase LSPCON timeout
> 
> On Wed, 15 Aug 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > On Wed, Aug 15, 2018 at 03:39:40PM -0700, Rodrigo Vivi wrote:
> >> On Mon, Aug 13, 2018 at 07:51:33PM +0200, Fredrik Schön wrote:
> >>
> >> First of all we need to fix the commit subject:
> >>
> >> drm/i915: Increase LSPCON timeout
> >>
> >> (this can be done when merging, no need to resend)
> >>
> >> > 100 ms is not enough time for the LSPCON adapter on Intel NUC
> >> > devices to settle. This causes dropped display modes at driver
> initialisation.
> >> >
> >> > Increase timeout to 1000 ms.
> >> >
> >> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> >>
> >> Missusage of "Fixes:" tag, please read
> >>
> >> https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html
> >> #fixes
> >>
> >> But also no need for resending... could be fixed when merging
> >>
> >> The right one would be:
> >>
> >> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> >> Fixes: 357c0ae9198a ("drm/i915/lspcon: Wait for expected LSPCON mode
> >> to settle")
> >> Cc: Shashank Sharma <shashank.sharma@intel.com>
> >> Cc: Imre Deak <imre.deak@intel.com>
> >> Cc: Jani Nikula <jani.nikula@intel.com>
> >> Cc: <stable@vger.kernel.org> # v4.11+
> >>
> >> Since initial 100 seemed to be empirical and this increase seems to
> >> help other cases I'm in favor of this move so
> >>
> >> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >>
> >> However I will wait a bit before merging it so Imre, Shashank, and/or
> >> Jani can take a look here...
> >
> > now, really cc'ing them...
> 
> Shashank? Does this slow down non-LSPCON paths?
+ Shashank for real
> 
> BR,
> Jani.
> 
> 
> >
> >>
> >> > Signed-off-by: Fredrik Schön <fredrik.schon@gmail.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/intel_lspcon.c | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/intel_lspcon.c
> >> > b/drivers/gpu/drm/i915/intel_lspcon.c
> >> > index 8ae8f42f430a..be1b08f589a4 100644
> >> > --- a/drivers/gpu/drm/i915/intel_lspcon.c
> >> > +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> >> > @@ -74,7 +74,7 @@ static enum drm_lspcon_mode
> lspcon_wait_mode(struct intel_lspcon *lspcon,
> >> >  	DRM_DEBUG_KMS("Waiting for LSPCON mode %s to settle\n",
> >> >  		      lspcon_mode_name(mode));
> >> >
> >> > -	wait_for((current_mode = lspcon_get_current_mode(lspcon)) ==
> mode, 100);
> >> > +	wait_for((current_mode = lspcon_get_current_mode(lspcon)) ==
> >> > +mode, 1000);
> >> >  	if (current_mode != mode)
> >> >  		DRM_ERROR("LSPCON mode hasn't settled\n");
> >> >
> >> > --
> >> > 2.17.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
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> 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] Increase LSPCON timeout

[Rodrigo Vivi]

On Thu, Aug 16, 2018 at 10:36:43AM +0200, Fredrik Schön wrote:
> Shashank,
> 
> Den tors 16 aug. 2018 kl 10:15 skrev Sharma, Shashank
> <shashank.sharma@intel.com>:
> >
> > Hey Chris,
> >
> >
> > On 8/16/2018 1:13 PM, Chris Wilson wrote:
> > > Quoting Sharma, Shashank (2018-08-16 08:33:36)
> > >> Regards
> > >>
> > >> Shashank
> > >>
> > >>
> > >> On 8/16/2018 12:47 PM, Jani Nikula wrote:
> > >>> On Wed, 15 Aug 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > >>>> On Wed, Aug 15, 2018 at 03:39:40PM -0700, Rodrigo Vivi wrote:
> > >>>>> On Mon, Aug 13, 2018 at 07:51:33PM +0200, Fredrik Schön wrote:
> > >>>>>
> > >>>>> First of all we need to fix the commit subject:
> > >>>>>
> > >>>>> drm/i915: Increase LSPCON timeout
> > >>>>>
> > >>>>> (this can be done when merging, no need to resend)
> > >>>>>
> > >>>>>> 100 ms is not enough time for the LSPCON adapter on Intel NUC devices to
> > >>>>>> settle. This causes dropped display modes at driver initialisation.
> > >>>>>>
> > >>>>>> Increase timeout to 1000 ms.
> > >>>>>>
> > >>>>>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> > >>>>> Missusage of "Fixes:" tag, please read
> > >>>>>
> > >>>>> https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html#fixes
> > >>>>>
> > >>>>> But also no need for resending... could be fixed when merging
> > >>>>>
> > >>>>> The right one would be:
> > >>>>>
> > >>>>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> > >>>>> Fixes: 357c0ae9198a ("drm/i915/lspcon: Wait for expected LSPCON mode to settle")
> > >>>>> Cc: Shashank Sharma <shashank.sharma@intel.com>
> > >>>>> Cc: Imre Deak <imre.deak@intel.com>
> > >>>>> Cc: Jani Nikula <jani.nikula@intel.com>
> > >>>>> Cc: <stable@vger.kernel.org> # v4.11+
> > >>>>>
> > >>>>> Since initial 100 seemed to be empirical and this increase seems to
> > >>>>> help other cases I'm in favor of this move so
> > >>>>>
> > >>>>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > >>>>>
> > >>>>> However I will wait a bit before merging it
> > >>>>> so Imre, Shashank, and/or Jani can take a look here...
> > >>>> now, really cc'ing them...
> > >>> Shashank? Does this slow down non-LSPCON paths?
> > >> This will slow down the lspcon probing and resume part, but both of them
> > >> happen only when LSPCON device is found. So to answer your question,
> > >> this will not slow down the non-lspcon path, but will slow down the
> > >> LSPCON connector resume and probe time. but I would recommend, instead
> > >> of increasing it to 1000 ms in a single shot, we might want to gradually
> > >> pick this up, on a wake-and-check way.
> > > wait_for() checks every [10us, 1ms] until the condition is met, or it
> > > times out. So, so long as we don't enter this path for !LPSCON where we
> > > know that it will timeout, the wait_for() will only take as long as is
> > > required for the connector to settle.
> > We wont hit !LSPCON timeout case here, as we have already read the
> > dongle signature successfully by now.  But I was thinking that, if the
> > spec recommends max wait time as 100ms (which is of course doesn't seem
> > enough), if we can't detect i2c-over-aux after first 500ms, I guess we
> > wont be able to do that in next 500ms too. So is it really ok to wait
> > this long in the resume sequence ?
> >
> > I guess Fredrik can provide some inputs here on if there are some
> > experiments behind this number of 1000ms, or this is just a safe bet ?
> > >
> 
> My first patch attempt - which is attached to the Redhat and FDO Bugzilla
> bugs - added a retry loop around the original 100 ms timeout. The retry loop
> did trigger, but never more than once in a row in my testing.
> 
> So possibly 200 ms would be a sufficient timeout, but as the wait_for() loop
> terminates early on success I suggested a conservative value of 1000 ms.

Since Shashank mentioned 100us came from some spec, maybe the double is already
a conservative value.

Since there is the concerns of delaying something when LSPCON fails
and we are possibly looping on connectors somewhere/somehow I believe we need
to have a balanced approach here.

could you please try the 200 ms approach on your case there for a while and
see how it goes?

> 
> > > Can we do other connectors at the same time, or does probing LSPCON
> > > block the system?
> > We can do other connectors at the same time in DRM layer at-least,
> > LSPCON blocks only this connector. I was curious if are we doing this
> > during the resume scenario or is this in the sequential get_connector()
> > type of call  ?
> > - Shashank
> > > -Chris
> /F
> _______________________________________________
> 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] Increase LSPCON timeout

[fredrikschon]

tor 2018-08-16 klockan 11:23 -0700 skrev Rodrigo Vivi:
> On Thu, Aug 16, 2018 at 10:36:43AM +0200, Fredrik Schön wrote:
> > Shashank,
> > 
> > Den tors 16 aug. 2018 kl 10:15 skrev Sharma, Shashank
> > <shashank.sharma@intel.com>:
> > > 
> > > Hey Chris,
> > > 
> > > 
> > > On 8/16/2018 1:13 PM, Chris Wilson wrote:
> > > > Quoting Sharma, Shashank (2018-08-16 08:33:36)
> > > > > Regards
> > > > > 
> > > > > Shashank
> > > > > 
> > > > > 
> > > > > On 8/16/2018 12:47 PM, Jani Nikula wrote:
> > > > > > On Wed, 15 Aug 2018, Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > wrote:
> > > > > > > On Wed, Aug 15, 2018 at 03:39:40PM -0700, Rodrigo Vivi
> > > > > > > wrote:
> > > > > > > > On Mon, Aug 13, 2018 at 07:51:33PM +0200, Fredrik Schön
> > > > > > > > wrote:
> > > > > > > > 
> > > > > > > > First of all we need to fix the commit subject:
> > > > > > > > 
> > > > > > > > drm/i915: Increase LSPCON timeout
> > > > > > > > 
> > > > > > > > (this can be done when merging, no need to resend)
> > > > > > > > 
> > > > > > > > > 100 ms is not enough time for the LSPCON adapter on
> > > > > > > > > Intel NUC devices to
> > > > > > > > > settle. This causes dropped display modes at driver
> > > > > > > > > initialisation.
> > > > > > > > > 
> > > > > > > > > Increase timeout to 1000 ms.
> > > > > > > > > 
> > > > > > > > > Fixes: 
> > > > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> > > > > > > > 
> > > > > > > > Missusage of "Fixes:" tag, please read
> > > > > > > > 
> > > > > > > > 
https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html#fixes
> > > > > > > > 
> > > > > > > > But also no need for resending... could be fixed when
> > > > > > > > merging
> > > > > > > > 
> > > > > > > > The right one would be:
> > > > > > > > 
> > > > > > > > Bugzilla: 
> > > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> > > > > > > > Fixes: 357c0ae9198a ("drm/i915/lspcon: Wait for
> > > > > > > > expected LSPCON mode to settle")
> > > > > > > > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > > > > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > > > > > Cc: <stable@vger.kernel.org> # v4.11+
> > > > > > > > 
> > > > > > > > Since initial 100 seemed to be empirical and this
> > > > > > > > increase seems to
> > > > > > > > help other cases I'm in favor of this move so
> > > > > > > > 
> > > > > > > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > > 
> > > > > > > > However I will wait a bit before merging it
> > > > > > > > so Imre, Shashank, and/or Jani can take a look here...
> > > > > > > 
> > > > > > > now, really cc'ing them...
> > > > > > 
> > > > > > Shashank? Does this slow down non-LSPCON paths?
> > > > > 
> > > > > This will slow down the lspcon probing and resume part, but
> > > > > both of them
> > > > > happen only when LSPCON device is found. So to answer your
> > > > > question,
> > > > > this will not slow down the non-lspcon path, but will slow
> > > > > down the
> > > > > LSPCON connector resume and probe time. but I would
> > > > > recommend, instead
> > > > > of increasing it to 1000 ms in a single shot, we might want
> > > > > to gradually
> > > > > pick this up, on a wake-and-check way.
> > > > 
> > > > wait_for() checks every [10us, 1ms] until the condition is met,
> > > > or it
> > > > times out. So, so long as we don't enter this path for !LPSCON
> > > > where we
> > > > know that it will timeout, the wait_for() will only take as
> > > > long as is
> > > > required for the connector to settle.
> > > 
> > > We wont hit !LSPCON timeout case here, as we have already read
> > > the
> > > dongle signature successfully by now.  But I was thinking that,
> > > if the
> > > spec recommends max wait time as 100ms (which is of course
> > > doesn't seem
> > > enough), if we can't detect i2c-over-aux after first 500ms, I
> > > guess we
> > > wont be able to do that in next 500ms too. So is it really ok to
> > > wait
> > > this long in the resume sequence ?
> > > 
> > > I guess Fredrik can provide some inputs here on if there are some
> > > experiments behind this number of 1000ms, or this is just a safe
> > > bet ?
> > > > 
> > 
> > My first patch attempt - which is attached to the Redhat and FDO
> > Bugzilla
> > bugs - added a retry loop around the original 100 ms timeout. The
> > retry loop
> > did trigger, but never more than once in a row in my testing.
> > 
> > So possibly 200 ms would be a sufficient timeout, but as the
> > wait_for() loop
> > terminates early on success I suggested a conservative value of
> > 1000 ms.
> 
> Since Shashank mentioned 100us came from some spec, maybe the double
> is already
> a conservative value.
> 
> Since there is the concerns of delaying something when LSPCON fails
> and we are possibly looping on connectors somewhere/somehow I believe
> we need
> to have a balanced approach here.
> 
> could you please try the 200 ms approach on your case there for a
> while and
> see how it goes?
> 

I ran a few stress tests using Nicholas test case from [1]. I can
quickly reproduce the failure with timeouts 100 ms, 110 ms, 130 ms, 150
ms and 170 ms. I am unable to reproduce any failures with timeouts 190
ms (n=18) and 200 ms (n=20+16).

So while 200 ms appears to work on my hardware with reasonable
confidence, I wouldn't call 200 conservative. But then again, I do not
know the specifications. I'm just being empirical.

[1] https://bugs.freedesktop.org/show_bug.cgi?id=107503#c15

> > 
> > > > Can we do other connectors at the same time, or does probing
> > > > LSPCON
> > > > block the system?
> > > 
> > > We can do other connectors at the same time in DRM layer at-
> > > least,
> > > LSPCON blocks only this connector. I was curious if are we doing
> > > this
> > > during the resume scenario or is this in the sequential
> > > get_connector()
> > > type of call  ?
> > > - Shashank
> > > > -Chris
> > 
> > /F
> > _______________________________________________
> > 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] Increase LSPCON timeout

[Rodrigo Vivi]

On Thu, Aug 16, 2018 at 11:35:26PM +0200, fredrikschon@gmail.com wrote:
> tor 2018-08-16 klockan 11:23 -0700 skrev Rodrigo Vivi:
> > On Thu, Aug 16, 2018 at 10:36:43AM +0200, Fredrik Schön wrote:
> > > Shashank,
> > > 
> > > Den tors 16 aug. 2018 kl 10:15 skrev Sharma, Shashank
> > > <shashank.sharma@intel.com>:
> > > > 
> > > > Hey Chris,
> > > > 
> > > > 
> > > > On 8/16/2018 1:13 PM, Chris Wilson wrote:
> > > > > Quoting Sharma, Shashank (2018-08-16 08:33:36)
> > > > > > Regards
> > > > > > 
> > > > > > Shashank
> > > > > > 
> > > > > > 
> > > > > > On 8/16/2018 12:47 PM, Jani Nikula wrote:
> > > > > > > On Wed, 15 Aug 2018, Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > wrote:
> > > > > > > > On Wed, Aug 15, 2018 at 03:39:40PM -0700, Rodrigo Vivi
> > > > > > > > wrote:
> > > > > > > > > On Mon, Aug 13, 2018 at 07:51:33PM +0200, Fredrik Schön
> > > > > > > > > wrote:
> > > > > > > > > 
> > > > > > > > > First of all we need to fix the commit subject:
> > > > > > > > > 
> > > > > > > > > drm/i915: Increase LSPCON timeout
> > > > > > > > > 
> > > > > > > > > (this can be done when merging, no need to resend)
> > > > > > > > > 
> > > > > > > > > > 100 ms is not enough time for the LSPCON adapter on
> > > > > > > > > > Intel NUC devices to
> > > > > > > > > > settle. This causes dropped display modes at driver
> > > > > > > > > > initialisation.
> > > > > > > > > > 
> > > > > > > > > > Increase timeout to 1000 ms.
> > > > > > > > > > 
> > > > > > > > > > Fixes: 
> > > > > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> > > > > > > > > 
> > > > > > > > > Missusage of "Fixes:" tag, please read
> > > > > > > > > 
> > > > > > > > > 
> https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html#fixes
> > > > > > > > > 
> > > > > > > > > But also no need for resending... could be fixed when
> > > > > > > > > merging
> > > > > > > > > 
> > > > > > > > > The right one would be:
> > > > > > > > > 
> > > > > > > > > Bugzilla: 
> > > > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> > > > > > > > > Fixes: 357c0ae9198a ("drm/i915/lspcon: Wait for
> > > > > > > > > expected LSPCON mode to settle")
> > > > > > > > > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > > > > > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > > > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > > > > > > Cc: <stable@vger.kernel.org> # v4.11+
> > > > > > > > > 
> > > > > > > > > Since initial 100 seemed to be empirical and this
> > > > > > > > > increase seems to
> > > > > > > > > help other cases I'm in favor of this move so
> > > > > > > > > 
> > > > > > > > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > > > 
> > > > > > > > > However I will wait a bit before merging it
> > > > > > > > > so Imre, Shashank, and/or Jani can take a look here...
> > > > > > > > 
> > > > > > > > now, really cc'ing them...
> > > > > > > 
> > > > > > > Shashank? Does this slow down non-LSPCON paths?
> > > > > > 
> > > > > > This will slow down the lspcon probing and resume part, but
> > > > > > both of them
> > > > > > happen only when LSPCON device is found. So to answer your
> > > > > > question,
> > > > > > this will not slow down the non-lspcon path, but will slow
> > > > > > down the
> > > > > > LSPCON connector resume and probe time. but I would
> > > > > > recommend, instead
> > > > > > of increasing it to 1000 ms in a single shot, we might want
> > > > > > to gradually
> > > > > > pick this up, on a wake-and-check way.
> > > > > 
> > > > > wait_for() checks every [10us, 1ms] until the condition is met,
> > > > > or it
> > > > > times out. So, so long as we don't enter this path for !LPSCON
> > > > > where we
> > > > > know that it will timeout, the wait_for() will only take as
> > > > > long as is
> > > > > required for the connector to settle.
> > > > 
> > > > We wont hit !LSPCON timeout case here, as we have already read
> > > > the
> > > > dongle signature successfully by now.  But I was thinking that,
> > > > if the
> > > > spec recommends max wait time as 100ms (which is of course
> > > > doesn't seem
> > > > enough), if we can't detect i2c-over-aux after first 500ms, I
> > > > guess we
> > > > wont be able to do that in next 500ms too. So is it really ok to
> > > > wait
> > > > this long in the resume sequence ?
> > > > 
> > > > I guess Fredrik can provide some inputs here on if there are some
> > > > experiments behind this number of 1000ms, or this is just a safe
> > > > bet ?
> > > > > 
> > > 
> > > My first patch attempt - which is attached to the Redhat and FDO
> > > Bugzilla
> > > bugs - added a retry loop around the original 100 ms timeout. The
> > > retry loop
> > > did trigger, but never more than once in a row in my testing.
> > > 
> > > So possibly 200 ms would be a sufficient timeout, but as the
> > > wait_for() loop
> > > terminates early on success I suggested a conservative value of
> > > 1000 ms.
> > 
> > Since Shashank mentioned 100us came from some spec, maybe the double
> > is already
> > a conservative value.
> > 
> > Since there is the concerns of delaying something when LSPCON fails
> > and we are possibly looping on connectors somewhere/somehow I believe
> > we need
> > to have a balanced approach here.
> > 
> > could you please try the 200 ms approach on your case there for a
> > while and
> > see how it goes?
> > 
> 
> I ran a few stress tests using Nicholas test case from [1]. I can
> quickly reproduce the failure with timeouts 100 ms, 110 ms, 130 ms, 150
> ms and 170 ms. I am unable to reproduce any failures with timeouts 190
> ms (n=18) and 200 ms (n=20+16).
> 
> So while 200 ms appears to work on my hardware with reasonable
> confidence, I wouldn't call 200 conservative. But then again, I do not
> know the specifications. I'm just being empirical.

I don't know this specification either and if that exists the empirical
shows that it is wrong or we have another bug somewhere else.

So... let's call 400 safe enough for now then?!

> 
> [1] https://bugs.freedesktop.org/show_bug.cgi?id=107503#c15
> 
> > > 
> > > > > Can we do other connectors at the same time, or does probing
> > > > > LSPCON
> > > > > block the system?
> > > > 
> > > > We can do other connectors at the same time in DRM layer at-
> > > > least,
> > > > LSPCON blocks only this connector. I was curious if are we doing
> > > > this
> > > > during the resume scenario or is this in the sequential
> > > > get_connector()
> > > > type of call  ?
> > > > - Shashank
> > > > > -Chris
> > > 
> > > /F
> > > _______________________________________________
> > > 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] Increase LSPCON timeout

[Fredrik Schön]

tor 2018-08-16 klockan 14:43 -0700 skrev Rodrigo Vivi:
> On Thu, Aug 16, 2018 at 11:35:26PM +0200, fredrikschon@gmail.com
> wrote:
> > tor 2018-08-16 klockan 11:23 -0700 skrev Rodrigo Vivi:
> > > On Thu, Aug 16, 2018 at 10:36:43AM +0200, Fredrik Schön wrote:
> > > > Shashank,
> > > > 
> > > > Den tors 16 aug. 2018 kl 10:15 skrev Sharma, Shashank
> > > > <shashank.sharma@intel.com>:
> > > > > 
> > > > > Hey Chris,
> > > > > 
> > > > > 
> > > > > On 8/16/2018 1:13 PM, Chris Wilson wrote:
> > > > > > Quoting Sharma, Shashank (2018-08-16 08:33:36)
> > > > > > > Regards
> > > > > > > 
> > > > > > > Shashank
> > > > > > > 
> > > > > > > 
> > > > > > > On 8/16/2018 12:47 PM, Jani Nikula wrote:
> > > > > > > > On Wed, 15 Aug 2018, Rodrigo Vivi <
> > > > > > > > rodrigo.vivi@intel.com>
> > > > > > > > wrote:
> > > > > > > > > On Wed, Aug 15, 2018 at 03:39:40PM -0700, Rodrigo
> > > > > > > > > Vivi
> > > > > > > > > wrote:
> > > > > > > > > > On Mon, Aug 13, 2018 at 07:51:33PM +0200, Fredrik
> > > > > > > > > > Schön
> > > > > > > > > > wrote:
> > > > > > > > > > 
> > > > > > > > > > First of all we need to fix the commit subject:
> > > > > > > > > > 
> > > > > > > > > > drm/i915: Increase LSPCON timeout
> > > > > > > > > > 
> > > > > > > > > > (this can be done when merging, no need to resend)
> > > > > > > > > > 
> > > > > > > > > > > 100 ms is not enough time for the LSPCON adapter
> > > > > > > > > > > on
> > > > > > > > > > > Intel NUC devices to
> > > > > > > > > > > settle. This causes dropped display modes at
> > > > > > > > > > > driver
> > > > > > > > > > > initialisation.
> > > > > > > > > > > 
> > > > > > > > > > > Increase timeout to 1000 ms.
> > > > > > > > > > > 
> > > > > > > > > > > Fixes: 
> > > > > > > > > > > 
https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> > > > > > > > > > 
> > > > > > > > > > Missusage of "Fixes:" tag, please read
> > > > > > > > > > 
> > > > > > > > > > 
> > 
> > 
https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html#fixes
> > > > > > > > > > 
> > > > > > > > > > But also no need for resending... could be fixed
> > > > > > > > > > when
> > > > > > > > > > merging
> > > > > > > > > > 
> > > > > > > > > > The right one would be:
> > > > > > > > > > 
> > > > > > > > > > Bugzilla: 
> > > > > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> > > > > > > > > > Fixes: 357c0ae9198a ("drm/i915/lspcon: Wait for
> > > > > > > > > > expected LSPCON mode to settle")
> > > > > > > > > > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > > > > > > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > > > > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > > > > > > > Cc: <stable@vger.kernel.org> # v4.11+
> > > > > > > > > > 
> > > > > > > > > > Since initial 100 seemed to be empirical and this
> > > > > > > > > > increase seems to
> > > > > > > > > > help other cases I'm in favor of this move so
> > > > > > > > > > 
> > > > > > > > > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > > > > 
> > > > > > > > > > However I will wait a bit before merging it
> > > > > > > > > > so Imre, Shashank, and/or Jani can take a look
> > > > > > > > > > here...
> > > > > > > > > 
> > > > > > > > > now, really cc'ing them...
> > > > > > > > 
> > > > > > > > Shashank? Does this slow down non-LSPCON paths?
> > > > > > > 
> > > > > > > This will slow down the lspcon probing and resume part,
> > > > > > > but
> > > > > > > both of them
> > > > > > > happen only when LSPCON device is found. So to answer
> > > > > > > your
> > > > > > > question,
> > > > > > > this will not slow down the non-lspcon path, but will
> > > > > > > slow
> > > > > > > down the
> > > > > > > LSPCON connector resume and probe time. but I would
> > > > > > > recommend, instead
> > > > > > > of increasing it to 1000 ms in a single shot, we might
> > > > > > > want
> > > > > > > to gradually
> > > > > > > pick this up, on a wake-and-check way.
> > > > > > 
> > > > > > wait_for() checks every [10us, 1ms] until the condition is
> > > > > > met,
> > > > > > or it
> > > > > > times out. So, so long as we don't enter this path for
> > > > > > !LPSCON
> > > > > > where we
> > > > > > know that it will timeout, the wait_for() will only take as
> > > > > > long as is
> > > > > > required for the connector to settle.
> > > > > 
> > > > > We wont hit !LSPCON timeout case here, as we have already
> > > > > read
> > > > > the
> > > > > dongle signature successfully by now.  But I was thinking
> > > > > that,
> > > > > if the
> > > > > spec recommends max wait time as 100ms (which is of course
> > > > > doesn't seem
> > > > > enough), if we can't detect i2c-over-aux after first 500ms, I
> > > > > guess we
> > > > > wont be able to do that in next 500ms too. So is it really ok
> > > > > to
> > > > > wait
> > > > > this long in the resume sequence ?
> > > > > 
> > > > > I guess Fredrik can provide some inputs here on if there are
> > > > > some
> > > > > experiments behind this number of 1000ms, or this is just a
> > > > > safe
> > > > > bet ?
> > > > > > 
> > > > 
> > > > My first patch attempt - which is attached to the Redhat and
> > > > FDO
> > > > Bugzilla
> > > > bugs - added a retry loop around the original 100 ms timeout.
> > > > The
> > > > retry loop
> > > > did trigger, but never more than once in a row in my testing.
> > > > 
> > > > So possibly 200 ms would be a sufficient timeout, but as the
> > > > wait_for() loop
> > > > terminates early on success I suggested a conservative value of
> > > > 1000 ms.
> > > 
> > > Since Shashank mentioned 100us came from some spec, maybe the
> > > double
> > > is already
> > > a conservative value.
> > > 
> > > Since there is the concerns of delaying something when LSPCON
> > > fails
> > > and we are possibly looping on connectors somewhere/somehow I
> > > believe
> > > we need
> > > to have a balanced approach here.
> > > 
> > > could you please try the 200 ms approach on your case there for a
> > > while and
> > > see how it goes?
> > > 
> > 
> > I ran a few stress tests using Nicholas test case from [1]. I can
> > quickly reproduce the failure with timeouts 100 ms, 110 ms, 130 ms,
> > 150
> > ms and 170 ms. I am unable to reproduce any failures with timeouts
> > 190
> > ms (n=18) and 200 ms (n=20+16).
> > 
> > So while 200 ms appears to work on my hardware with reasonable
> > confidence, I wouldn't call 200 conservative. But then again, I do
> > not
> > know the specifications. I'm just being empirical.
> 
> I don't know this specification either and if that exists the
> empirical
> shows that it is wrong or we have another bug somewhere else.
> 
> So... let's call 400 safe enough for now then?!
> 

Sound reasonable. Do you want me to respin the patch?
> > 
> > [1] https://bugs.freedesktop.org/show_bug.cgi?id=107503#c15
> > 
> > > > 
> > > > > > Can we do other connectors at the same time, or does
> > > > > > probing
> > > > > > LSPCON
> > > > > > block the system?
> > > > > 
> > > > > We can do other connectors at the same time in DRM layer at-
> > > > > least,
> > > > > LSPCON blocks only this connector. I was curious if are we
> > > > > doing
> > > > > this
> > > > > during the resume scenario or is this in the sequential
> > > > > get_connector()
> > > > > type of call  ?
> > > > > - Shashank
> > > > > > -Chris
> > > > 
> > > > /F
> > > > _______________________________________________
> > > > 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] Increase LSPCON timeout

[Rodrigo Vivi]

On Thu, Aug 16, 2018 at 11:51:07PM +0200, Fredrik Schön wrote:
> tor 2018-08-16 klockan 14:43 -0700 skrev Rodrigo Vivi:
> > On Thu, Aug 16, 2018 at 11:35:26PM +0200, fredrikschon@gmail.com
> > wrote:
> > > tor 2018-08-16 klockan 11:23 -0700 skrev Rodrigo Vivi:
> > > > On Thu, Aug 16, 2018 at 10:36:43AM +0200, Fredrik Schön wrote:
> > > > > Shashank,
> > > > > 
> > > > > Den tors 16 aug. 2018 kl 10:15 skrev Sharma, Shashank
> > > > > <shashank.sharma@intel.com>:
> > > > > > 
> > > > > > Hey Chris,
> > > > > > 
> > > > > > 
> > > > > > On 8/16/2018 1:13 PM, Chris Wilson wrote:
> > > > > > > Quoting Sharma, Shashank (2018-08-16 08:33:36)
> > > > > > > > Regards
> > > > > > > > 
> > > > > > > > Shashank
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On 8/16/2018 12:47 PM, Jani Nikula wrote:
> > > > > > > > > On Wed, 15 Aug 2018, Rodrigo Vivi <
> > > > > > > > > rodrigo.vivi@intel.com>
> > > > > > > > > wrote:
> > > > > > > > > > On Wed, Aug 15, 2018 at 03:39:40PM -0700, Rodrigo
> > > > > > > > > > Vivi
> > > > > > > > > > wrote:
> > > > > > > > > > > On Mon, Aug 13, 2018 at 07:51:33PM +0200, Fredrik
> > > > > > > > > > > Schön
> > > > > > > > > > > wrote:
> > > > > > > > > > > 
> > > > > > > > > > > First of all we need to fix the commit subject:
> > > > > > > > > > > 
> > > > > > > > > > > drm/i915: Increase LSPCON timeout
> > > > > > > > > > > 
> > > > > > > > > > > (this can be done when merging, no need to resend)
> > > > > > > > > > > 
> > > > > > > > > > > > 100 ms is not enough time for the LSPCON adapter
> > > > > > > > > > > > on
> > > > > > > > > > > > Intel NUC devices to
> > > > > > > > > > > > settle. This causes dropped display modes at
> > > > > > > > > > > > driver
> > > > > > > > > > > > initialisation.
> > > > > > > > > > > > 
> > > > > > > > > > > > Increase timeout to 1000 ms.
> > > > > > > > > > > > 
> > > > > > > > > > > > Fixes: 
> > > > > > > > > > > > 
> https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> > > > > > > > > > > 
> > > > > > > > > > > Missusage of "Fixes:" tag, please read
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > 
> > > 
> https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html#fixes
> > > > > > > > > > > 
> > > > > > > > > > > But also no need for resending... could be fixed
> > > > > > > > > > > when
> > > > > > > > > > > merging
> > > > > > > > > > > 
> > > > > > > > > > > The right one would be:
> > > > > > > > > > > 
> > > > > > > > > > > Bugzilla: 
> > > > > > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1570392
> > > > > > > > > > > Fixes: 357c0ae9198a ("drm/i915/lspcon: Wait for
> > > > > > > > > > > expected LSPCON mode to settle")
> > > > > > > > > > > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > > > > > > > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > > > > > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > > > > > > > > Cc: <stable@vger.kernel.org> # v4.11+
> > > > > > > > > > > 
> > > > > > > > > > > Since initial 100 seemed to be empirical and this
> > > > > > > > > > > increase seems to
> > > > > > > > > > > help other cases I'm in favor of this move so
> > > > > > > > > > > 
> > > > > > > > > > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > > > > > 
> > > > > > > > > > > However I will wait a bit before merging it
> > > > > > > > > > > so Imre, Shashank, and/or Jani can take a look
> > > > > > > > > > > here...
> > > > > > > > > > 
> > > > > > > > > > now, really cc'ing them...
> > > > > > > > > 
> > > > > > > > > Shashank? Does this slow down non-LSPCON paths?
> > > > > > > > 
> > > > > > > > This will slow down the lspcon probing and resume part,
> > > > > > > > but
> > > > > > > > both of them
> > > > > > > > happen only when LSPCON device is found. So to answer
> > > > > > > > your
> > > > > > > > question,
> > > > > > > > this will not slow down the non-lspcon path, but will
> > > > > > > > slow
> > > > > > > > down the
> > > > > > > > LSPCON connector resume and probe time. but I would
> > > > > > > > recommend, instead
> > > > > > > > of increasing it to 1000 ms in a single shot, we might
> > > > > > > > want
> > > > > > > > to gradually
> > > > > > > > pick this up, on a wake-and-check way.
> > > > > > > 
> > > > > > > wait_for() checks every [10us, 1ms] until the condition is
> > > > > > > met,
> > > > > > > or it
> > > > > > > times out. So, so long as we don't enter this path for
> > > > > > > !LPSCON
> > > > > > > where we
> > > > > > > know that it will timeout, the wait_for() will only take as
> > > > > > > long as is
> > > > > > > required for the connector to settle.
> > > > > > 
> > > > > > We wont hit !LSPCON timeout case here, as we have already
> > > > > > read
> > > > > > the
> > > > > > dongle signature successfully by now.  But I was thinking
> > > > > > that,
> > > > > > if the
> > > > > > spec recommends max wait time as 100ms (which is of course
> > > > > > doesn't seem
> > > > > > enough), if we can't detect i2c-over-aux after first 500ms, I
> > > > > > guess we
> > > > > > wont be able to do that in next 500ms too. So is it really ok
> > > > > > to
> > > > > > wait
> > > > > > this long in the resume sequence ?
> > > > > > 
> > > > > > I guess Fredrik can provide some inputs here on if there are
> > > > > > some
> > > > > > experiments behind this number of 1000ms, or this is just a
> > > > > > safe
> > > > > > bet ?
> > > > > > > 
> > > > > 
> > > > > My first patch attempt - which is attached to the Redhat and
> > > > > FDO
> > > > > Bugzilla
> > > > > bugs - added a retry loop around the original 100 ms timeout.
> > > > > The
> > > > > retry loop
> > > > > did trigger, but never more than once in a row in my testing.
> > > > > 
> > > > > So possibly 200 ms would be a sufficient timeout, but as the
> > > > > wait_for() loop
> > > > > terminates early on success I suggested a conservative value of
> > > > > 1000 ms.
> > > > 
> > > > Since Shashank mentioned 100us came from some spec, maybe the
> > > > double
> > > > is already
> > > > a conservative value.
> > > > 
> > > > Since there is the concerns of delaying something when LSPCON
> > > > fails
> > > > and we are possibly looping on connectors somewhere/somehow I
> > > > believe
> > > > we need
> > > > to have a balanced approach here.
> > > > 
> > > > could you please try the 200 ms approach on your case there for a
> > > > while and
> > > > see how it goes?
> > > > 
> > > 
> > > I ran a few stress tests using Nicholas test case from [1]. I can
> > > quickly reproduce the failure with timeouts 100 ms, 110 ms, 130 ms,
> > > 150
> > > ms and 170 ms. I am unable to reproduce any failures with timeouts
> > > 190
> > > ms (n=18) and 200 ms (n=20+16).
> > > 
> > > So while 200 ms appears to work on my hardware with reasonable
> > > confidence, I wouldn't call 200 conservative. But then again, I do
> > > not
> > > know the specifications. I'm just being empirical.
> > 
> > I don't know this specification either and if that exists the
> > empirical
> > shows that it is wrong or we have another bug somewhere else.
> > 
> > So... let's call 400 safe enough for now then?!
> > 
> 
> Sound reasonable. Do you want me to respin the patch?

we can give a day for the guys on other timezone to have a chance
to ack, nack or give another suggestions and after that yes, please
send a v2.

Thanks,
Rodrigo.

> > > 
> > > [1] https://bugs.freedesktop.org/show_bug.cgi?id=107503#c15
> > > 
> > > > > 
> > > > > > > Can we do other connectors at the same time, or does
> > > > > > > probing
> > > > > > > LSPCON
> > > > > > > block the system?
> > > > > > 
> > > > > > We can do other connectors at the same time in DRM layer at-
> > > > > > least,
> > > > > > LSPCON blocks only this connector. I was curious if are we
> > > > > > doing
> > > > > > this
> > > > > > during the resume scenario or is this in the sequential
> > > > > > get_connector()
> > > > > > type of call  ?
> > > > > > - Shashank
> > > > > > > -Chris
> > > > > 
> > > > > /F
> > > > > _______________________________________________
> > > > > 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