| Message ID | 20170908093712.28796-1-marta.lofstedt@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 08, 2017 at 12:37:12PM +0300, Marta Lofstedt wrote: > During IGT testing it has been shown that the specification > defined polling time of 1 us for FCLK_DONE, is sometimes not > enough. This is most probably due to a firmware flaw. As a > workaround, it is better to wait a little bit longer for the > FCLK_DONE to come around, than to leave with an DRM_ERROR and > having FCLK_DONE at a randome time after. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102243 > Signed-off-by: Marta Lofstedt <marta.lofstedt@intel.com> I think it'd be good to add a summary of the alternate approaches you've tested (preempt_disable, pm_qos, ...), just for the record to indicate that there's quite a story hiding behind this patch. With that fixed, plus ofc assuming full igt doesn't spot any issues: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Neat piece of hw r/eing here :-) -Daniel > --- > drivers/gpu/drm/i915/intel_cdclk.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index d32911816fc2..f89232e0f6fa 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -669,8 +669,11 @@ static void bdw_set_cdclk(struct drm_i915_private *dev_priv, > val |= LCPLL_CD_SOURCE_FCLK; > I915_WRITE(LCPLL_CTL, val); > > + /* According to the spec, it should be enough to poll for this 1 us. > + * However, extensive testing shows that this can take longer. > + */ > if (wait_for_us(I915_READ(LCPLL_CTL) & > - LCPLL_CD_SOURCE_FCLK_DONE, 1)) > + LCPLL_CD_SOURCE_FCLK_DONE, 100)) > DRM_ERROR("Switching to FCLK failed\n"); > > val = I915_READ(LCPLL_CTL); > -- > 2.11.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel > Vetter > Sent: Friday, September 8, 2017 3:10 PM > To: Lofstedt, Marta <marta.lofstedt@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Increase poll time for BDW > FCLK_DONE > > On Fri, Sep 08, 2017 at 12:37:12PM +0300, Marta Lofstedt wrote: > > During IGT testing it has been shown that the specification defined > > polling time of 1 us for FCLK_DONE, is sometimes not enough. This is > > most probably due to a firmware flaw. As a workaround, it is better to > > wait a little bit longer for the FCLK_DONE to come around, than to > > leave with an DRM_ERROR and having FCLK_DONE at a randome time > after. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102243 > > Signed-off-by: Marta Lofstedt <marta.lofstedt@intel.com> > > I think it'd be good to add a summary of the alternate approaches you've > tested (preempt_disable, pm_qos, ...), just for the record to indicate that > there's quite a story hiding behind this patch. > > With that fixed, plus ofc assuming full igt doesn't spot any issues: > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Neat piece of hw r/eing here :-) > -Daniel Thanks Daniel, Imre has been a great help while doing this. V2 with more documentation is coming up. /Marta > > > --- > > drivers/gpu/drm/i915/intel_cdclk.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c > > b/drivers/gpu/drm/i915/intel_cdclk.c > > index d32911816fc2..f89232e0f6fa 100644 > > --- a/drivers/gpu/drm/i915/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > > @@ -669,8 +669,11 @@ static void bdw_set_cdclk(struct drm_i915_private > *dev_priv, > > val |= LCPLL_CD_SOURCE_FCLK; > > I915_WRITE(LCPLL_CTL, val); > > > > + /* According to the spec, it should be enough to poll for this 1 > us. > > + * However, extensive testing shows that this can take longer. > > + */ > > if (wait_for_us(I915_READ(LCPLL_CTL) & > > - LCPLL_CD_SOURCE_FCLK_DONE, > 1)) > > + LCPLL_CD_SOURCE_FCLK_DONE, > 100)) > > DRM_ERROR("Switching to FCLK failed\n"); > > > > val = I915_READ(LCPLL_CTL); > > -- > > 2.11.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Mon, Sep 11, 2017 at 12:15:21PM +0000, Patchwork wrote: > == Series Details == > > Series: drm/i915: Increase poll time for BDW FCLK_DONE (rev2) > URL : https://patchwork.freedesktop.org/series/30006/ > State : success Thanks for the patch and review, pushed it to -dinq. For the future please check the preferred multi-line comment format. Fixed this up now while applying. > > == Summary == > > Series 30006v2 drm/i915: Increase poll time for BDW FCLK_DONE > https://patchwork.freedesktop.org/api/1.0/series/30006/revisions/2/mbox/ > > Test kms_cursor_legacy: > Subgroup basic-busy-flip-before-cursor-legacy: > fail -> PASS (fi-snb-2600) fdo#100215 > > fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 > > fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:445s > fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:456s > fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:377s > fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:535s > fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:271s > fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:505s > fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:502s > fi-byt-n2820 total:289 pass:250 dwarn:1 dfail:0 fail:0 skip:38 time:496s > fi-cfl-s total:289 pass:250 dwarn:4 dfail:0 fail:0 skip:35 time:447s > fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:452s > fi-glk-2a total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:590s > fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:427s > fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:405s > fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:439s > fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:496s > fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:467s > fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:488s > fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:587s > fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:589s > fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:554s > fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:459s > fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:521s > 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:457s > fi-skl-x1585l total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:480s > fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:565s > fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:1 skip:39 time:426s > > b8ed44bfbb7cf1dd40a5b5c15da77053fdc0e0d0 drm-tip: 2017y-09m-11d-11h-22m-27s UTC integration manifest > 4df9d5006ad9 drm/i915: Increase poll time for BDW FCLK_DONE > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5639/ > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index d32911816fc2..f89232e0f6fa 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -669,8 +669,11 @@ static void bdw_set_cdclk(struct drm_i915_private *dev_priv, val |= LCPLL_CD_SOURCE_FCLK; I915_WRITE(LCPLL_CTL, val); + /* According to the spec, it should be enough to poll for this 1 us. + * However, extensive testing shows that this can take longer. + */ if (wait_for_us(I915_READ(LCPLL_CTL) & - LCPLL_CD_SOURCE_FCLK_DONE, 1)) + LCPLL_CD_SOURCE_FCLK_DONE, 100)) DRM_ERROR("Switching to FCLK failed\n"); val = I915_READ(LCPLL_CTL);
During IGT testing it has been shown that the specification defined polling time of 1 us for FCLK_DONE, is sometimes not enough. This is most probably due to a firmware flaw. As a workaround, it is better to wait a little bit longer for the FCLK_DONE to come around, than to leave with an DRM_ERROR and having FCLK_DONE at a randome time after. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102243 Signed-off-by: Marta Lofstedt <marta.lofstedt@intel.com> --- drivers/gpu/drm/i915/intel_cdclk.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)