[PATCH 1/2] drm/i915: Try to suppress more spurious PCH underruns on ILK-IVB

[PATCH 1/2] drm/i915: Try to suppress more spurious PCH underruns on ILK-IVB

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

My ILK seems to generate a spurious PCH underrun with most interlaced
HDMI modes. Add a second vblank wait to avoid it.

We have seen some spurious PCH underruns still in CI as well, some
of which seem to be progressive DP. The logs also point towards some
spurious underrins with progressive HDMI on SNB. While I don't have
a solid explanation for those let's try to kill all the birds with one
stone and always do the double wait.

Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106387
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7604fbda0607..b5fa4943372a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5528,9 +5528,16 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 	if (HAS_PCH_CPT(dev_priv))
 		cpt_verify_modeset(dev, intel_crtc->pipe);
 
-	/* Must wait for vblank to avoid spurious PCH FIFO underruns */
-	if (intel_crtc->config->has_pch_encoder)
+	/*
+	 * Must wait for vblank to avoid spurious PCH FIFO underruns.
+	 * And a second vblank wait is needed at least on ILK with
+	 * some interlaced HDMI modes. Let's do the double wait always
+	 * in case there are more corner cases we don't know about.
+	 */
+	if (intel_crtc->config->has_pch_encoder) {
+		intel_wait_for_vblank(dev_priv, pipe);
 		intel_wait_for_vblank(dev_priv, pipe);
+	}
 	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
 	intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
 }
-- 
2.16.1

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

[PATCH 2/2] drm/i915: Simplify ilk-ivb underrun suppression

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Let's suppress the underruns around every modeset sequence instead
of trying to avoid it. Planes are disabled at this point anyway so
we don't really gain anything from keeping the underrun reporting
enabled. Also for PCH ports we already suppress all underruns here
anyway so trying avoid it for the CPU eDP doesn't seem all that
important.

Maybe this gets rid of some lingering spurious underruns?

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b5fa4943372a..dc7204d27a24 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5470,10 +5470,8 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 	 *
 	 * Spurious PCH underruns also occur during PCH enabling.
 	 */
-	if (intel_crtc->config->has_pch_encoder || IS_GEN5(dev_priv))
-		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
-	if (intel_crtc->config->has_pch_encoder)
-		intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
+	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
+	intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
 
 	if (intel_crtc->config->has_pch_encoder)
 		intel_prepare_shared_dpll(intel_crtc);
@@ -5717,10 +5715,8 @@ static void ironlake_crtc_disable(struct intel_crtc_state *old_crtc_state,
 	 * pipe is already disabled, but FDI RX/TX is still enabled.
 	 * Happens at least with VGA+HDMI cloning. Suppress them.
 	 */
-	if (intel_crtc->config->has_pch_encoder) {
-		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
-		intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
-	}
+	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
+	intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
 
 	intel_encoders_disable(crtc, old_crtc_state, old_state);
 
-- 
2.16.1

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

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Try to suppress more spurious PCH underruns on ILK-IVB

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Try to suppress more spurious PCH underruns on ILK-IVB
URL   : https://patchwork.freedesktop.org/series/43721/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4234 -> Patchwork_9108 =

== Summary - WARNING ==

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

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-cnl-psr:         DMESG-WARN (fdo#104951) -> PASS

    
  fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951


== Participating hosts (44 -> 38) ==

  Missing    (6): fi-ilk-m540 fi-cnl-y3 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4234 -> Patchwork_9108

  CI_DRM_4234: 399e4206d849c5667d8553911e0035c53cd9c24e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4498: f9ecb79ad8b02278cfdb5b82495df47061c04f8f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9108: 2296f07c0461544811deb6f930299d72b1589bec @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2296f07c0461 drm/i915: Simplify ilk-ivb underrun suppression
0e090bdd1211 drm/i915: Try to suppress more spurious PCH underruns on ILK-IVB

== Logs ==

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

Re: [PATCH 1/2] drm/i915: Try to suppress more spurious PCH underruns on ILK-IVB

[Chris Wilson]

Quoting Ville Syrjala (2018-05-24 20:04:05)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> My ILK seems to generate a spurious PCH underrun with most interlaced
> HDMI modes. Add a second vblank wait to avoid it.
> 
> We have seen some spurious PCH underruns still in CI as well, some
> of which seem to be progressive DP. The logs also point towards some
> spurious underrins with progressive HDMI on SNB. While I don't have
> a solid explanation for those let's try to kill all the birds with one
> stone and always do the double wait.
> 
> Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106387
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7604fbda0607..b5fa4943372a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5528,9 +5528,16 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>         if (HAS_PCH_CPT(dev_priv))
>                 cpt_verify_modeset(dev, intel_crtc->pipe);
>  
> -       /* Must wait for vblank to avoid spurious PCH FIFO underruns */
> -       if (intel_crtc->config->has_pch_encoder)
> +       /*
> +        * Must wait for vblank to avoid spurious PCH FIFO underruns.
> +        * And a second vblank wait is needed at least on ILK with
> +        * some interlaced HDMI modes. Let's do the double wait always
> +        * in case there are more corner cases we don't know about.
> +        */
> +       if (intel_crtc->config->has_pch_encoder) {
> +               intel_wait_for_vblank(dev_priv, pipe);
>                 intel_wait_for_vblank(dev_priv, pipe);

The only purpose for the double wait here is for delaying the switching
on of underrun reporting, right? It doesn't accidentally fix anything
else?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Try to suppress more spurious PCH underruns on ILK-IVB

[Chris Wilson]

Quoting Chris Wilson (2018-05-24 21:14:23)
> Quoting Ville Syrjala (2018-05-24 20:04:05)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > My ILK seems to generate a spurious PCH underrun with most interlaced
> > HDMI modes. Add a second vblank wait to avoid it.
> > 
> > We have seen some spurious PCH underruns still in CI as well, some
> > of which seem to be progressive DP. The logs also point towards some
> > spurious underrins with progressive HDMI on SNB. While I don't have
> > a solid explanation for those let's try to kill all the birds with one
> > stone and always do the double wait.
> > 
> > Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106387
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 7604fbda0607..b5fa4943372a 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5528,9 +5528,16 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
> >         if (HAS_PCH_CPT(dev_priv))
> >                 cpt_verify_modeset(dev, intel_crtc->pipe);
> >  
> > -       /* Must wait for vblank to avoid spurious PCH FIFO underruns */
> > -       if (intel_crtc->config->has_pch_encoder)
> > +       /*
> > +        * Must wait for vblank to avoid spurious PCH FIFO underruns.
> > +        * And a second vblank wait is needed at least on ILK with
> > +        * some interlaced HDMI modes. Let's do the double wait always
> > +        * in case there are more corner cases we don't know about.
> > +        */
> > +       if (intel_crtc->config->has_pch_encoder) {
> > +               intel_wait_for_vblank(dev_priv, pipe);
> >                 intel_wait_for_vblank(dev_priv, pipe);
> 
> The only purpose for the double wait here is for delaying the switching
> on of underrun reporting, right? It doesn't accidentally fix anything
> else?

E.g. in a multi-crtc setup, that's a big delay between switching on a
pair of pipes.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Try to suppress more spurious PCH underruns on ILK-IVB

[Ville Syrjälä]

On Thu, May 24, 2018 at 09:15:37PM +0100, Chris Wilson wrote:
> Quoting Chris Wilson (2018-05-24 21:14:23)
> > Quoting Ville Syrjala (2018-05-24 20:04:05)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > My ILK seems to generate a spurious PCH underrun with most interlaced
> > > HDMI modes. Add a second vblank wait to avoid it.
> > > 
> > > We have seen some spurious PCH underruns still in CI as well, some
> > > of which seem to be progressive DP. The logs also point towards some
> > > spurious underrins with progressive HDMI on SNB. While I don't have
> > > a solid explanation for those let's try to kill all the birds with one
> > > stone and always do the double wait.
> > > 
> > > Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106387
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 11 +++++++++--
> > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 7604fbda0607..b5fa4943372a 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -5528,9 +5528,16 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
> > >         if (HAS_PCH_CPT(dev_priv))
> > >                 cpt_verify_modeset(dev, intel_crtc->pipe);
> > >  
> > > -       /* Must wait for vblank to avoid spurious PCH FIFO underruns */
> > > -       if (intel_crtc->config->has_pch_encoder)
> > > +       /*
> > > +        * Must wait for vblank to avoid spurious PCH FIFO underruns.
> > > +        * And a second vblank wait is needed at least on ILK with
> > > +        * some interlaced HDMI modes. Let's do the double wait always
> > > +        * in case there are more corner cases we don't know about.
> > > +        */
> > > +       if (intel_crtc->config->has_pch_encoder) {
> > > +               intel_wait_for_vblank(dev_priv, pipe);
> > >                 intel_wait_for_vblank(dev_priv, pipe);
> > 
> > The only purpose for the double wait here is for delaying the switching
> > on of underrun reporting, right? It doesn't accidentally fix anything
> > else?
> 
> E.g. in a multi-crtc setup, that's a big delay between switching on a
> pair of pipes.

I was testing with one pipe only. So at least it's not meant to do
anything else.

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

Re: [PATCH 1/2] drm/i915: Try to suppress more spurious PCH underruns on ILK-IVB

[Chris Wilson]

Quoting Ville Syrjala (2018-05-24 20:04:05)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> My ILK seems to generate a spurious PCH underrun with most interlaced
> HDMI modes. Add a second vblank wait to avoid it.

Fwiw, a second vblank because of interlacing is very believable.
 
> We have seen some spurious PCH underruns still in CI as well, some
> of which seem to be progressive DP. The logs also point towards some
> spurious underrins with progressive HDMI on SNB. While I don't have
> a solid explanation for those let's try to kill all the birds with one
> stone and always do the double wait.
> 
> Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106387
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>

No point waiting for a vblank worker? ;)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Try to suppress more spurious PCH underruns on ILK-IVB

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Try to suppress more spurious PCH underruns on ILK-IVB
URL   : https://patchwork.freedesktop.org/series/43721/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4234_full -> Patchwork_9108_full =

== Summary - WARNING ==

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

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-blt:
      shard-kbl:          PASS -> SKIP

    igt@gem_mocs_settings@mocs-rc6-bsd2:
      shard-kbl:          SKIP -> PASS

    igt@kms_cursor_legacy@basic-flip-before-cursor-legacy:
      shard-snb:          SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_gtt:
      shard-glk:          PASS -> INCOMPLETE (k.org#198133, fdo#103359)

    igt@drv_selftest@live_hangcheck:
      shard-kbl:          NOTRUN -> DMESG-FAIL (fdo#106560)

    igt@kms_flip@2x-plain-flip-fb-recreate:
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105707)

    igt@kms_flip_tiling@flip-to-y-tiled:
      shard-glk:          PASS -> FAIL (fdo#104724)

    igt@pm_rps@waitboost:
      shard-apl:          PASS -> FAIL (fdo#102250)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_gtt:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-kbl:          INCOMPLETE (fdo#103665, fdo#106023) -> PASS

    igt@kms_flip_tiling@flip-to-x-tiled:
      shard-glk:          FAIL (fdo#103822, fdo#104724) -> PASS

    igt@perf@blocking:
      shard-hsw:          FAIL (fdo#102252) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102250 https://bugs.freedesktop.org/show_bug.cgi?id=102250
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105707 https://bugs.freedesktop.org/show_bug.cgi?id=105707
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  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_4234 -> Patchwork_9108

  CI_DRM_4234: 399e4206d849c5667d8553911e0035c53cd9c24e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4498: f9ecb79ad8b02278cfdb5b82495df47061c04f8f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9108: 2296f07c0461544811deb6f930299d72b1589bec @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

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

Re: [PATCH 1/2] drm/i915: Try to suppress more spurious PCH underruns on ILK-IVB

[Ville Syrjälä]

On Thu, May 24, 2018 at 10:19:02PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-05-24 20:04:05)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > My ILK seems to generate a spurious PCH underrun with most interlaced
> > HDMI modes. Add a second vblank wait to avoid it.
> 
> Fwiw, a second vblank because of interlacing is very believable.
>  
> > We have seen some spurious PCH underruns still in CI as well, some
> > of which seem to be progressive DP. The logs also point towards some
> > spurious underrins with progressive HDMI on SNB. While I don't have
> > a solid explanation for those let's try to kill all the birds with one
> > stone and always do the double wait.
> > 
> > Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106387
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>a

Thanks. Pushed to dinq.

> 
> No point waiting for a vblank worker? ;)

That might take a while. Also I'm not sure we'd want to use it here
because we'd probably want underrun reporting to be active by the
time we enable the planes. So we'd either have to enable planes from
the worker as well, or we'd just sample the vblank counter at the
end of crtc_enable and wait for n+2 just before we start to enable the
planes. Not sure if that latter approach would gain us any practical
parallelism though.

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

Re: [PATCH 1/2] drm/i915: Try to suppress more spurious PCH underruns on ILK-IVB

[Jani Nikula]

On Fri, 25 May 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, May 24, 2018 at 10:19:02PM +0100, Chris Wilson wrote:
>> Quoting Ville Syrjala (2018-05-24 20:04:05)
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > 
>> > My ILK seems to generate a spurious PCH underrun with most interlaced
>> > HDMI modes. Add a second vblank wait to avoid it.
>> 
>> Fwiw, a second vblank because of interlacing is very believable.

/me .oO( could we wait for the 2nd vblank based on interlace? )

>>  
>> > We have seen some spurious PCH underruns still in CI as well, some
>> > of which seem to be progressive DP. The logs also point towards some
>> > spurious underrins with progressive HDMI on SNB. While I don't have
>> > a solid explanation for those let's try to kill all the birds with one
>> > stone and always do the double wait.
>> > 
>> > Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106387
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>a
>
> Thanks. Pushed to dinq.
>
>> 
>> No point waiting for a vblank worker? ;)
>
> That might take a while. Also I'm not sure we'd want to use it here
> because we'd probably want underrun reporting to be active by the
> time we enable the planes. So we'd either have to enable planes from
> the worker as well, or we'd just sample the vblank counter at the
> end of crtc_enable and wait for n+2 just before we start to enable the
> planes. Not sure if that latter approach would gain us any practical
> parallelism though.

-- 
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 2/2] drm/i915: Simplify ilk-ivb underrun suppression

[Chris Wilson]

Quoting Ville Syrjala (2018-05-24 20:04:06)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Let's suppress the underruns around every modeset sequence instead
> of trying to avoid it. Planes are disabled at this point anyway so
> we don't really gain anything from keeping the underrun reporting
> enabled. Also for PCH ports we already suppress all underruns here
> anyway so trying avoid it for the CPU eDP doesn't seem all that
> important.
> 
> Maybe this gets rid of some lingering spurious underruns?

I'll bite. Isn't the reason for enabling underrung report for the
modeset itself to detect errors in our sequence?

How certain are we that these are hw limitations vs sw bugs?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Simplify ilk-ivb underrun suppression

[Ville Syrjälä]

On Fri, May 25, 2018 at 04:20:07PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-05-24 20:04:06)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Let's suppress the underruns around every modeset sequence instead
> > of trying to avoid it. Planes are disabled at this point anyway so
> > we don't really gain anything from keeping the underrun reporting
> > enabled. Also for PCH ports we already suppress all underruns here
> > anyway so trying avoid it for the CPU eDP doesn't seem all that
> > important.
> > 
> > Maybe this gets rid of some lingering spurious underruns?
> 
> I'll bite. Isn't the reason for enabling underrung report for the
> modeset itself to detect errors in our sequence?

In theory CPU FIFO underruns shouldn't happen until we have some planes
enabled. Otherwise we have no data going through the FIFOs and thus
reporting an underrun isn't particularly sane. That doesn't stop gen2
from doing it though, but gen3+ seem to follow the more reasonable
interpretation of what a FIFO underrun means.

I suppose PCH FIFO underruns are a bit different as there is data flowing
as soon as the CPU pipe starts running, whether or not any planes have
been enabled. So those could certainly indicate some kind of programming
sequence error. Or it could just be totally expected that we start the
PCH side of the pipe first and there's a small bit of time when the CPU
pipe isn't yet pushing out pixels, and that's when the PCH side reports
the underrun.

> 
> How certain are we that these are hw limitations vs sw bugs?

To the best of my knowledge we are reasonably close to the sequence
listed in bspec. And while it's at least theoretically possible that
there's some change we could make to eliminate the underruns I don't
suppose anyone has the time or energy to try out all possible
variations.

And as long as the underrun (even if it's real) has vanished by the
time we enable the planes I think we are reasonably safe wrt. getting
the correct looking output to the user's display.

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

Re: [PATCH 1/2] drm/i915: Try to suppress more spurious PCH underruns on ILK-IVB

[Ville Syrjälä]

On Fri, May 25, 2018 at 06:19:17PM +0300, Jani Nikula wrote:
> On Fri, 25 May 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Thu, May 24, 2018 at 10:19:02PM +0100, Chris Wilson wrote:
> >> Quoting Ville Syrjala (2018-05-24 20:04:05)
> >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > 
> >> > My ILK seems to generate a spurious PCH underrun with most interlaced
> >> > HDMI modes. Add a second vblank wait to avoid it.
> >> 
> >> Fwiw, a second vblank because of interlacing is very believable.
> 
> /me .oO( could we wait for the 2nd vblank based on interlace? )

We certainly could. However as explained below we have seen some
similar looking underruns in CI with progressive DP as well, and so
I chose to try the shotgun approach in the hopes of hitting a few
more barn doors.

> 
> >>  
> >> > We have seen some spurious PCH underruns still in CI as well, some
> >> > of which seem to be progressive DP. The logs also point towards some
> >> > spurious underrins with progressive HDMI on SNB. While I don't have
> >> > a solid explanation for those let's try to kill all the birds with one
> >> > stone and always do the double wait.
> >> > 
> >> > Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106387
> >> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>a
> >
> > Thanks. Pushed to dinq.
> >
> >> 
> >> No point waiting for a vblank worker? ;)
> >
> > That might take a while. Also I'm not sure we'd want to use it here
> > because we'd probably want underrun reporting to be active by the
> > time we enable the planes. So we'd either have to enable planes from
> > the worker as well, or we'd just sample the vblank counter at the
> > end of crtc_enable and wait for n+2 just before we start to enable the
> > planes. Not sure if that latter approach would gain us any practical
> > parallelism though.
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

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

Re: [PATCH 2/2] drm/i915: Simplify ilk-ivb underrun suppression

[Chris Wilson]

Quoting Ville Syrjälä (2018-05-25 16:43:42)
> On Fri, May 25, 2018 at 04:20:07PM +0100, Chris Wilson wrote:
> > Quoting Ville Syrjala (2018-05-24 20:04:06)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Let's suppress the underruns around every modeset sequence instead
> > > of trying to avoid it. Planes are disabled at this point anyway so
> > > we don't really gain anything from keeping the underrun reporting
> > > enabled. Also for PCH ports we already suppress all underruns here
> > > anyway so trying avoid it for the CPU eDP doesn't seem all that
> > > important.
> > > 
> > > Maybe this gets rid of some lingering spurious underruns?
> > 
> > I'll bite. Isn't the reason for enabling underrung report for the
> > modeset itself to detect errors in our sequence?
> 
> In theory CPU FIFO underruns shouldn't happen until we have some planes
> enabled. Otherwise we have no data going through the FIFOs and thus
> reporting an underrun isn't particularly sane. That doesn't stop gen2
> from doing it though, but gen3+ seem to follow the more reasonable
> interpretation of what a FIFO underrun means.

Makes sense.
 
> I suppose PCH FIFO underruns are a bit different as there is data flowing
> as soon as the CPU pipe starts running, whether or not any planes have
> been enabled. So those could certainly indicate some kind of programming
> sequence error. Or it could just be totally expected that we start the
> PCH side of the pipe first and there's a small bit of time when the CPU
> pipe isn't yet pushing out pixels, and that's when the PCH side reports
> the underrun.
> 
> > 
> > How certain are we that these are hw limitations vs sw bugs?
> 
> To the best of my knowledge we are reasonably close to the sequence
> listed in bspec. And while it's at least theoretically possible that
> there's some change we could make to eliminate the underruns I don't
> suppose anyone has the time or energy to try out all possible
> variations.
> 
> And as long as the underrun (even if it's real) has vanished by the
> time we enable the planes I think we are reasonably safe wrt. getting
> the correct looking output to the user's display.

Also makes sense. And if glitches during modesetting itself, we hope
nobody complains ;)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Simplify ilk-ivb underrun suppression

[Ville Syrjälä]

On Fri, May 25, 2018 at 04:55:36PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjälä (2018-05-25 16:43:42)
> > On Fri, May 25, 2018 at 04:20:07PM +0100, Chris Wilson wrote:
> > > Quoting Ville Syrjala (2018-05-24 20:04:06)
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Let's suppress the underruns around every modeset sequence instead
> > > > of trying to avoid it. Planes are disabled at this point anyway so
> > > > we don't really gain anything from keeping the underrun reporting
> > > > enabled. Also for PCH ports we already suppress all underruns here
> > > > anyway so trying avoid it for the CPU eDP doesn't seem all that
> > > > important.
> > > > 
> > > > Maybe this gets rid of some lingering spurious underruns?
> > > 
> > > I'll bite. Isn't the reason for enabling underrung report for the
> > > modeset itself to detect errors in our sequence?
> > 
> > In theory CPU FIFO underruns shouldn't happen until we have some planes
> > enabled. Otherwise we have no data going through the FIFOs and thus
> > reporting an underrun isn't particularly sane. That doesn't stop gen2
> > from doing it though, but gen3+ seem to follow the more reasonable
> > interpretation of what a FIFO underrun means.
> 
> Makes sense.
>  
> > I suppose PCH FIFO underruns are a bit different as there is data flowing
> > as soon as the CPU pipe starts running, whether or not any planes have
> > been enabled. So those could certainly indicate some kind of programming
> > sequence error. Or it could just be totally expected that we start the
> > PCH side of the pipe first and there's a small bit of time when the CPU
> > pipe isn't yet pushing out pixels, and that's when the PCH side reports
> > the underrun.
> > 
> > > 
> > > How certain are we that these are hw limitations vs sw bugs?
> > 
> > To the best of my knowledge we are reasonably close to the sequence
> > listed in bspec. And while it's at least theoretically possible that
> > there's some change we could make to eliminate the underruns I don't
> > suppose anyone has the time or energy to try out all possible
> > variations.
> > 
> > And as long as the underrun (even if it's real) has vanished by the
> > time we enable the planes I think we are reasonably safe wrt. getting
> > the correct looking output to the user's display.
> 
> Also makes sense. And if glitches during modesetting itself, we hope
> nobody complains ;)
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks. Pushed.

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