* [PATCH] [drm][i915] Increase LSPCON timeout
@ 2018-08-13 17:51 Fredrik Schön
2018-08-14 13:09 ` ✓ Fi.CI.BAT: success for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Fredrik Schön @ 2018-08-13 17:51 UTC (permalink / raw)
To: intel-gfx; +Cc: 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
^ permalink raw reply related [flat|nested] 16+ messages in thread* ✓ Fi.CI.BAT: success for Increase LSPCON timeout 2018-08-13 17:51 [PATCH] [drm][i915] Increase LSPCON timeout Fredrik Schön @ 2018-08-14 13:09 ` Patchwork 2018-08-14 16:08 ` ✓ Fi.CI.IGT: " Patchwork 2018-08-15 22:39 ` [PATCH] [drm][i915] " Rodrigo Vivi 2 siblings, 0 replies; 16+ messages in thread From: Patchwork @ 2018-08-14 13:09 UTC (permalink / raw) To: Fredrik Schön; +Cc: intel-gfx == 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* ✓ Fi.CI.IGT: success for Increase LSPCON timeout 2018-08-13 17:51 [PATCH] [drm][i915] Increase LSPCON timeout Fredrik Schön 2018-08-14 13:09 ` ✓ Fi.CI.BAT: success for " Patchwork @ 2018-08-14 16:08 ` Patchwork 2018-08-15 22:39 ` [PATCH] [drm][i915] " Rodrigo Vivi 2 siblings, 0 replies; 16+ messages in thread From: Patchwork @ 2018-08-14 16:08 UTC (permalink / raw) To: Fredrik Schön; +Cc: intel-gfx == 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [drm][i915] Increase LSPCON timeout 2018-08-13 17:51 [PATCH] [drm][i915] Increase LSPCON timeout Fredrik Schön 2018-08-14 13:09 ` ✓ Fi.CI.BAT: success for " Patchwork 2018-08-14 16:08 ` ✓ Fi.CI.IGT: " Patchwork @ 2018-08-15 22:39 ` Rodrigo Vivi 2018-08-15 22:45 ` Rodrigo Vivi 2 siblings, 1 reply; 16+ messages in thread From: Rodrigo Vivi @ 2018-08-15 22:39 UTC (permalink / raw) To: Fredrik Schön; +Cc: intel-gfx, Fredrik Schön 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [drm][i915] Increase LSPCON timeout 2018-08-15 22:39 ` [PATCH] [drm][i915] " Rodrigo Vivi @ 2018-08-15 22:45 ` Rodrigo Vivi 2018-08-16 7:17 ` Jani Nikula 0 siblings, 1 reply; 16+ messages in thread From: Rodrigo Vivi @ 2018-08-15 22:45 UTC (permalink / raw) To: Fredrik Schön; +Cc: Jani Nikula, intel-gfx, Fredrik Schön 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [drm][i915] Increase LSPCON timeout 2018-08-15 22:45 ` Rodrigo Vivi @ 2018-08-16 7:17 ` Jani Nikula 2018-08-16 7:33 ` Sharma, Shashank 2018-08-16 9:53 ` Saarinen, Jani 0 siblings, 2 replies; 16+ messages in thread From: Jani Nikula @ 2018-08-16 7:17 UTC (permalink / raw) To: Rodrigo Vivi, Fredrik Schön; +Cc: intel-gfx, Fredrik Schön 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [drm][i915] Increase LSPCON timeout 2018-08-16 7:17 ` Jani Nikula @ 2018-08-16 7:33 ` Sharma, Shashank 2018-08-16 7:43 ` Chris Wilson 2018-08-16 9:53 ` Saarinen, Jani 1 sibling, 1 reply; 16+ messages in thread From: Sharma, Shashank @ 2018-08-16 7:33 UTC (permalink / raw) To: Jani Nikula, Rodrigo Vivi, Fredrik Schön Cc: intel-gfx, Fredrik Schön 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [drm][i915] Increase LSPCON timeout 2018-08-16 7:33 ` Sharma, Shashank @ 2018-08-16 7:43 ` Chris Wilson 2018-08-16 8:15 ` Sharma, Shashank 0 siblings, 1 reply; 16+ messages in thread From: Chris Wilson @ 2018-08-16 7:43 UTC (permalink / raw) To: Sharma, Shashank, Fredrik Schön, Jani Nikula, Rodrigo Vivi Cc: intel-gfx, Fredrik Schön 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [drm][i915] Increase LSPCON timeout 2018-08-16 7:43 ` Chris Wilson @ 2018-08-16 8:15 ` Sharma, Shashank 2018-08-16 8:36 ` Fredrik Schön 0 siblings, 1 reply; 16+ messages in thread From: Sharma, Shashank @ 2018-08-16 8:15 UTC (permalink / raw) To: Chris Wilson, Fredrik Schön, Jani Nikula, Rodrigo Vivi Cc: intel-gfx, Fredrik Schön 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [drm][i915] Increase LSPCON timeout 2018-08-16 8:15 ` Sharma, Shashank @ 2018-08-16 8:36 ` Fredrik Schön 2018-08-16 18:23 ` Rodrigo Vivi 0 siblings, 1 reply; 16+ messages in thread From: Fredrik Schön @ 2018-08-16 8:36 UTC (permalink / raw) To: shashank.sharma; +Cc: jani.nikula, intel-gfx, rodrigo.vivi 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [drm][i915] Increase LSPCON timeout 2018-08-16 8:36 ` Fredrik Schön @ 2018-08-16 18:23 ` Rodrigo Vivi 2018-08-16 21:35 ` fredrikschon 0 siblings, 1 reply; 16+ messages in thread From: Rodrigo Vivi @ 2018-08-16 18:23 UTC (permalink / raw) To: Fredrik Schön; +Cc: jani.nikula, intel-gfx 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [drm][i915] Increase LSPCON timeout 2018-08-16 18:23 ` Rodrigo Vivi @ 2018-08-16 21:35 ` fredrikschon 2018-08-16 21:43 ` Rodrigo Vivi 0 siblings, 1 reply; 16+ messages in thread From: fredrikschon @ 2018-08-16 21:35 UTC (permalink / raw) To: Rodrigo Vivi; +Cc: jani.nikula, intel-gfx 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [drm][i915] Increase LSPCON timeout 2018-08-16 21:35 ` fredrikschon @ 2018-08-16 21:43 ` Rodrigo Vivi 2018-08-16 21:51 ` Fredrik Schön 0 siblings, 1 reply; 16+ messages in thread From: Rodrigo Vivi @ 2018-08-16 21:43 UTC (permalink / raw) To: fredrikschon; +Cc: jani.nikula, intel-gfx 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [drm][i915] Increase LSPCON timeout 2018-08-16 21:43 ` Rodrigo Vivi @ 2018-08-16 21:51 ` Fredrik Schön 2018-08-16 21:58 ` Rodrigo Vivi 0 siblings, 1 reply; 16+ messages in thread From: Fredrik Schön @ 2018-08-16 21:51 UTC (permalink / raw) To: Rodrigo Vivi; +Cc: jani.nikula, intel-gfx 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [drm][i915] Increase LSPCON timeout 2018-08-16 21:51 ` Fredrik Schön @ 2018-08-16 21:58 ` Rodrigo Vivi 0 siblings, 0 replies; 16+ messages in thread From: Rodrigo Vivi @ 2018-08-16 21:58 UTC (permalink / raw) To: Fredrik Schön; +Cc: jani.nikula, intel-gfx 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [drm][i915] Increase LSPCON timeout 2018-08-16 7:17 ` Jani Nikula 2018-08-16 7:33 ` Sharma, Shashank @ 2018-08-16 9:53 ` Saarinen, Jani 1 sibling, 0 replies; 16+ messages in thread From: Saarinen, Jani @ 2018-08-16 9:53 UTC (permalink / raw) To: Nikula, Jani, Vivi, Rodrigo, Fredrik Schön, Sharma, Shashank Cc: intel-gfx@lists.freedesktop.org, Fredrik Schön 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-08-16 21:58 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-08-13 17:51 [PATCH] [drm][i915] Increase LSPCON timeout Fredrik Schön 2018-08-14 13:09 ` ✓ Fi.CI.BAT: success for " Patchwork 2018-08-14 16:08 ` ✓ Fi.CI.IGT: " Patchwork 2018-08-15 22:39 ` [PATCH] [drm][i915] " Rodrigo Vivi 2018-08-15 22:45 ` Rodrigo Vivi 2018-08-16 7:17 ` Jani Nikula 2018-08-16 7:33 ` Sharma, Shashank 2018-08-16 7:43 ` Chris Wilson 2018-08-16 8:15 ` Sharma, Shashank 2018-08-16 8:36 ` Fredrik Schön 2018-08-16 18:23 ` Rodrigo Vivi 2018-08-16 21:35 ` fredrikschon 2018-08-16 21:43 ` Rodrigo Vivi 2018-08-16 21:51 ` Fredrik Schön 2018-08-16 21:58 ` Rodrigo Vivi 2018-08-16 9:53 ` Saarinen, Jani
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).