Re: [Intel-gfx] [PATCH] drm/i915: Increase poll time for BDW FCLK_DONE

[Lofstedt, Marta]

> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Friday, September 8, 2017 3:10 PM
> To: Lofstedt, Marta <marta.lofst...@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.lofst...@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.vet...@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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Increase poll time for BDW FCLK_DONE

[Daniel Vetter]

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 

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 

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Increase poll time for BDW FCLK_DONE

[Marta Lofstedt]

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 
---
 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