[PATCH] drm/i915: Don't sanitize frame start delay if the pipe is off

[PATCH] drm/i915: Don't sanitize frame start delay if the pipe is off

[Ville Syrjala]

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

Avoid touching PIPECONF in intel_sanitize_crtc() unless the pipe is
actually on. Should cure some unclaimed register accesses.

We don't have to sanitize this if the pipe is off since we will
overwrite the frame start delay anyway when turning the pipe on.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 23aa7191024e..e6fcbc5abc75 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14812,7 +14812,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
 	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
 
 	/* Clear any frame start delays used for debugging left by the BIOS */
-	if (!transcoder_is_dsi(cpu_transcoder)) {
+	if (crtc->active && !transcoder_is_dsi(cpu_transcoder)) {
 		i915_reg_t reg = PIPECONF(cpu_transcoder);
 
 		I915_WRITE(reg,
-- 
2.13.6

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

Re: [PATCH] drm/i915: Don't sanitize frame start delay if the pipe is off

[Chris Wilson]

Quoting Ville Syrjala (2017-11-15 20:04:42)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Avoid touching PIPECONF in intel_sanitize_crtc() unless the pipe is
> actually on. Should cure some unclaimed register accesses.

While using crtc->active is consistent with other sanitization, is this
really a fix for unclaimed register access?

We should be holding all the powerwells at this moment in bringing up
the hw, right? So the only unclaimed access would be if the reg didn't
exist. So are we looking at an invalid cpu_transcoder?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Don't sanitize frame start delay if the pipe is off

[Patchwork]

== Series Details ==

Series: drm/i915: Don't sanitize frame start delay if the pipe is off
URL   : https://patchwork.freedesktop.org/series/33901/
State : success

== Summary ==

Series 33901v1 drm/i915: Don't sanitize frame start delay if the pipe is off
https://patchwork.freedesktop.org/api/1.0/series/33901/revisions/1/mbox/

Test gem_ringfill:
        Subgroup basic-default-hang:
                pass       -> DMESG-WARN (fi-pnv-d510) fdo#101600

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

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:443s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:456s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:382s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:545s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:279s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:514s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:509s
fi-byt-j1900     total:289  pass:254  dwarn:0   dfail:0   fail:0   skip:35  time:507s
fi-byt-n2820     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:498s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:426s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:542s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:434s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:442s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:428s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:482s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:465s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:485s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:532s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:478s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:536s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:578s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:458s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:543s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:567s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:520s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:503s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:466s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:560s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:424s
Blacklisted hosts:
fi-cfl-s         total:289  pass:254  dwarn:3   dfail:0   fail:0   skip:32  time:530s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:554s
fi-gdg-551 failed to connect after reboot

bd5e410d0f036c80ae80e075592ee6dd90c660a8 drm-tip: 2017y-11m-15d-17h-22m-19s UTC integration manifest
752f13b518e6 drm/i915: Don't sanitize frame start delay if the pipe is off

== Logs ==

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

Re: [PATCH] drm/i915: Don't sanitize frame start delay if the pipe is off

[Ville Syrjälä]

On Wed, Nov 15, 2017 at 08:17:21PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-11-15 20:04:42)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Avoid touching PIPECONF in intel_sanitize_crtc() unless the pipe is
> > actually on. Should cure some unclaimed register accesses.
> 
> While using crtc->active is consistent with other sanitization, is this
> really a fix for unclaimed register access?
> 
> We should be holding all the powerwells at this moment in bringing up
> the hw, right? So the only unclaimed access would be if the reg didn't
> exist. So are we looking at an invalid cpu_transcoder?

I was thinking we'd have dropped the power references already. But
I guess not. And that should definitely then give unclaimed register
accesses during driver init.

I think these fails are in the "pretend display gets clobbered by GPU
reset" path. And there we don't actually seem to grabbing the init power
reference, which could well explain this. 

Not sure we want to add the init power there either. Most of the readout
code now has the power domain handling in place, so maybe we're close
to being able to nuke the init power thing entirely?

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

Re: [PATCH] drm/i915: Don't sanitize frame start delay if the pipe is off

[Chris Wilson]

Quoting Ville Syrjälä (2017-11-15 20:35:41)
> On Wed, Nov 15, 2017 at 08:17:21PM +0000, Chris Wilson wrote:
> > Quoting Ville Syrjala (2017-11-15 20:04:42)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Avoid touching PIPECONF in intel_sanitize_crtc() unless the pipe is
> > > actually on. Should cure some unclaimed register accesses.
> > 
> > While using crtc->active is consistent with other sanitization, is this
> > really a fix for unclaimed register access?
> > 
> > We should be holding all the powerwells at this moment in bringing up
> > the hw, right? So the only unclaimed access would be if the reg didn't
> > exist. So are we looking at an invalid cpu_transcoder?
> 
> I was thinking we'd have dropped the power references already. But
> I guess not. And that should definitely then give unclaimed register
> accesses during driver init.
> 
> I think these fails are in the "pretend display gets clobbered by GPU
> reset" path. And there we don't actually seem to grabbing the init power
> reference, which could well explain this. 

Hmm, it is in reset's __intel_dsiplay_resume() so that's a likely
candidate.

> Not sure we want to add the init power there either. Most of the readout
> code now has the power domain handling in place, so maybe we're close
> to being able to nuke the init power thing entirely?

But for reset, shouldn't we be taking the display powerwells in
intel_prepare_reset() or is that already there buried away in the
indirection?

Oh, that intel_display_set_init_power() definitely seems out of place
along reset.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Don't sanitize frame start delay if the pipe is off

[Chris Wilson]

Quoting Ville Syrjala (2017-11-15 20:04:42)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Avoid touching PIPECONF in intel_sanitize_crtc() unless the pipe is
> actually on. Should cure some unclaimed register accesses.
+ during reset, as we are rather cavalier in our approach to powerdomain
management.

> We don't have to sanitize this if the pipe is off since we will
> overwrite the frame start delay anyway when turning the pipe on.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102249
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Let's not even get started on the handling of modesets vs display reset.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Don't sanitize frame start delay if the pipe is off

[Patchwork]

== Series Details ==

Series: drm/i915: Don't sanitize frame start delay if the pipe is off
URL   : https://patchwork.freedesktop.org/series/33901/
State : success

== Summary ==

Test kms_busy:
        Subgroup extended-modeset-hang-newfb-with-reset-render-a:
                dmesg-warn -> PASS       (shard-hsw) fdo#102249
        Subgroup extended-modeset-hang-newfb-with-reset-render-b:
                dmesg-warn -> PASS       (shard-hsw) fdo#103038
Test drv_module_reload:
        Subgroup basic-reload-inject:
                dmesg-warn -> PASS       (shard-hsw) fdo#102707

fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249
fdo#103038 https://bugs.freedesktop.org/show_bug.cgi?id=103038
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707

shard-hsw        total:2584 pass:1473 dwarn:2   dfail:1   fail:9   skip:1099 time:9603s
Blacklisted hosts:
shard-apl        total:2565 pass:1602 dwarn:3   dfail:1   fail:22  skip:936 time:13167s
shard-kbl        total:2500 pass:1613 dwarn:48  dfail:4   fail:23  skip:810 time:10513s
shard-snb        total:2584 pass:1259 dwarn:2   dfail:1   fail:11  skip:1311 time:8110s

== Logs ==

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

Re: ✓ Fi.CI.IGT: success for drm/i915: Don't sanitize frame start delay if the pipe is off

[Ville Syrjälä]

On Wed, Nov 15, 2017 at 09:17:01PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Don't sanitize frame start delay if the pipe is off
> URL   : https://patchwork.freedesktop.org/series/33901/
> State : success
> 
> == Summary ==
> 
> Test kms_busy:
>         Subgroup extended-modeset-hang-newfb-with-reset-render-a:
>                 dmesg-warn -> PASS       (shard-hsw) fdo#102249
>         Subgroup extended-modeset-hang-newfb-with-reset-render-b:
>                 dmesg-warn -> PASS       (shard-hsw) fdo#103038
> Test drv_module_reload:
>         Subgroup basic-reload-inject:
>                 dmesg-warn -> PASS       (shard-hsw) fdo#102707
> 
> fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249
> fdo#103038 https://bugs.freedesktop.org/show_bug.cgi?id=103038

Oh we had two bugs for the same thing.

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

This one is the fb leak. Not fixed by the patch, just fails randomly it
seems.

> 
> shard-hsw        total:2584 pass:1473 dwarn:2   dfail:1   fail:9   skip:1099 time:9603s
> Blacklisted hosts:
> shard-apl        total:2565 pass:1602 dwarn:3   dfail:1   fail:22  skip:936 time:13167s
> shard-kbl        total:2500 pass:1613 dwarn:48  dfail:4   fail:23  skip:810 time:10513s
> shard-snb        total:2584 pass:1259 dwarn:2   dfail:1   fail:11  skip:1311 time:8110s
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7151/shards.html

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

Re: [PATCH] drm/i915: Don't sanitize frame start delay if the pipe is off

[Ville Syrjälä]

On Wed, Nov 15, 2017 at 09:13:28PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-11-15 20:04:42)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Avoid touching PIPECONF in intel_sanitize_crtc() unless the pipe is
> > actually on. Should cure some unclaimed register accesses.
> + during reset, as we are rather cavalier in our approach to powerdomain
> management.

Amended, and pushed. Thanks.

> 
> > We don't have to sanitize this if the pipe is off since we will
> > overwrite the frame start delay anyway when turning the pipe on.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102249
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Let's not even get started on the handling of modesets vs display reset.
> -Chris

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