[PATCH 1/2] drm/i915/gen6+: Clear upper data byte during PCODE write

[PATCH 1/2] drm/i915/gen6+: Clear upper data byte during PCODE write

[Imre Deak]

The spec calls for the upper data byte to be cleared before most of the
PCODE write commands, for others like IPS control it doesn't say
anything about this byte. Let's clear it in case it's clobbered somehow,
especially that there are places where we only do a PCODE write without
a preceeding PCODE read.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 29b6653..66c62f3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7838,6 +7838,7 @@ int sandybridge_pcode_write(struct drm_i915_private *dev_priv,
 	}
 
 	I915_WRITE_FW(GEN6_PCODE_DATA, val);
+	I915_WRITE_FW(GEN6_PCODE_DATA1, 0);
 	I915_WRITE_FW(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
 
 	if (intel_wait_for_register_fw(dev_priv,
-- 
2.5.0

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

[PATCH 2/2] drm/i915/skl: Increase PCODE CDCLK change notify timeout

[Imre Deak]

commit 848496e5902833600f7992f4faa82dc1546051ba
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date:   Wed Jul 13 16:32:03 2016 +0300

    drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL

increased the timeout to match the spec, but we still see a timeout on
at least one SKL. A CDCLK change request following the failed one will
succeed nevertheless, so let's try to increase the timeout to 10ms.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=97929
Signed-off-by: Imre Deak <imre.deak@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 5d11002..3b711f4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6261,7 +6261,7 @@ static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
 
 static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
 {
-	return _wait_for(skl_cdclk_pcu_ready(dev_priv), 3000, 10) == 0;
+	return _wait_for(skl_cdclk_pcu_ready(dev_priv), 10000, 10) == 0;
 }
 
 static void skl_set_cdclk(struct drm_i915_private *dev_priv, int cdclk, int vco)
-- 
2.5.0

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

Re: [PATCH 1/2] drm/i915/gen6+: Clear upper data byte during PCODE write

[Chris Wilson]

On Thu, Nov 24, 2016 at 05:32:58PM +0200, Imre Deak wrote:
> The spec calls for the upper data byte to be cleared before most of the
> PCODE write commands, for others like IPS control it doesn't say
> anything about this byte. Let's clear it in case it's clobbered somehow,
> especially that there are places where we only do a PCODE write without
> a preceeding PCODE read.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Seems sensible to me.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/gen6+: Clear upper data byte during PCODE write

[Ville Syrjälä]

On Thu, Nov 24, 2016 at 05:32:58PM +0200, Imre Deak wrote:
> The spec calls for the upper data byte to be cleared before most of the
> PCODE write commands, for others like IPS control it doesn't say
> anything about this byte. Let's clear it in case it's clobbered somehow,
> especially that there are places where we only do a PCODE write without
> a preceeding PCODE read.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 29b6653..66c62f3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -7838,6 +7838,7 @@ int sandybridge_pcode_write(struct drm_i915_private *dev_priv,
>  	}
>  
>  	I915_WRITE_FW(GEN6_PCODE_DATA, val);
> +	I915_WRITE_FW(GEN6_PCODE_DATA1, 0);
>  	I915_WRITE_FW(GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);

Hmm. Somehow we missed it in dddab346d828 ("drm/i915: Clear PCODE_DATA1 on SNB+")
even though my ramblings (which were quoted in the commit msg) stated that we
should do it in both the read and write functions.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  	if (intel_wait_for_register_fw(dev_priv,
> -- 
> 2.5.0

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
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/gen6+: Clear upper data byte during PCODE write

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/gen6+: Clear upper data byte during PCODE write
URL   : https://patchwork.freedesktop.org/series/15911/
State : success

== Summary ==

Series 15911v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/15911/revisions/1/mbox/


fi-bdw-5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:244  pass:204  dwarn:0   dfail:0   fail:0   skip:40 
fi-bxt-t5700     total:244  pass:216  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-j1900     total:244  pass:216  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-n2820     total:244  pass:212  dwarn:0   dfail:0   fail:0   skip:32 
fi-hsw-4770      total:244  pass:224  dwarn:0   dfail:0   fail:0   skip:20 
fi-hsw-4770r     total:244  pass:224  dwarn:0   dfail:0   fail:0   skip:20 
fi-ilk-650       total:244  pass:191  dwarn:0   dfail:0   fail:0   skip:53 
fi-ivb-3520m     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ivb-3770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-kbl-7200u     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-kbl-7500u     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6260u     total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:244  pass:223  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6700k     total:244  pass:222  dwarn:1   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:244  pass:212  dwarn:0   dfail:0   fail:0   skip:32 
fi-snb-2600      total:244  pass:211  dwarn:0   dfail:0   fail:0   skip:33 

262f64d637b5d976bd042a05fc00f2b8a16faee5 drm-tip: 2016y-11m-24d-15h-21m-13s UTC integration manifest
d647418 drm/i915/skl: Increase PCODE CDCLK change notify timeout
fe8604a drm/i915/gen6+: Clear upper data byte during PCODE write

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3110/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/skl: Increase PCODE CDCLK change notify timeout

[Chris Wilson]

On Thu, Nov 24, 2016 at 05:32:59PM +0200, Imre Deak wrote:
> commit 848496e5902833600f7992f4faa82dc1546051ba
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date:   Wed Jul 13 16:32:03 2016 +0300
> 
>     drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
> 
> increased the timeout to match the spec, but we still see a timeout on
> at least one SKL. A CDCLK change request following the failed one will
> succeed nevertheless, so let's try to increase the timeout to 10ms.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=97929
> Signed-off-by: Imre Deak <imre.deak@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 5d11002..3b711f4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6261,7 +6261,7 @@ static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
>  
>  static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
>  {
> -	return _wait_for(skl_cdclk_pcu_ready(dev_priv), 3000, 10) == 0;
> +	return _wait_for(skl_cdclk_pcu_ready(dev_priv), 10000, 10) == 0;
>  }

I can't think of a better approach to kick the pcu. But that is a wakeup
every 10us for 10ms.

{
	int ret;

	ret = wait_for_us(skl_cdclk_pcu_ready(dev_priv), 10);
	if (ret)
		ret = wait_for(skl_cdclk_pcu_ready(dev_priv), 10);

	return ret;
}

would spin for the first 10us then do a more gentle probe for the next
10ms.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/skl: Increase PCODE CDCLK change notify timeout

[Chris Wilson]

On Fri, Nov 25, 2016 at 09:23:01AM +0000, Chris Wilson wrote:
> On Thu, Nov 24, 2016 at 05:32:59PM +0200, Imre Deak wrote:
> > commit 848496e5902833600f7992f4faa82dc1546051ba
> > Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Date:   Wed Jul 13 16:32:03 2016 +0300
> > 
> >     drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
> > 
> > increased the timeout to match the spec, but we still see a timeout on
> > at least one SKL. A CDCLK change request following the failed one will
> > succeed nevertheless, so let's try to increase the timeout to 10ms.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Reference: https://bugs.freedesktop.org/show_bug.cgi?id=97929
> > Signed-off-by: Imre Deak <imre.deak@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 5d11002..3b711f4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6261,7 +6261,7 @@ static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
> >  
> >  static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
> >  {
> > -	return _wait_for(skl_cdclk_pcu_ready(dev_priv), 3000, 10) == 0;
> > +	return _wait_for(skl_cdclk_pcu_ready(dev_priv), 10000, 10) == 0;
> >  }
> 
> I can't think of a better approach to kick the pcu. But that is a wakeup
> every 10us for 10ms.
> 
> {
> 	int ret;
> 
> 	ret = wait_for_us(skl_cdclk_pcu_ready(dev_priv), 10);
> 	if (ret)
> 		ret = wait_for(skl_cdclk_pcu_ready(dev_priv), 10);
> 
> 	return ret;
> }
> 
> would spin for the first 10us then do a more gentle probe for the next
> 10ms.

Otoh, talking to the pcu also includes such spinners within itself, so
probably overkill to spin on top as well.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/2] drm/i915/skl: Increase PCODE CDCLK change notify timeout

[Imre Deak]

commit 848496e5902833600f7992f4faa82dc1546051ba
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date:   Wed Jul 13 16:32:03 2016 +0300

    drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL

increased the timeout to match the spec, but we still see a timeout on
at least one SKL. A CDCLK change request following the failed one will
succeed nevertheless, so let's try to increase the timeout to 10ms.

v2:
- Use 1ms poll period instead of 10us. (Chris)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=97929
Signed-off-by: Imre Deak <imre.deak@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 5d11002..f07b611 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6261,7 +6261,7 @@ static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
 
 static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
 {
-	return _wait_for(skl_cdclk_pcu_ready(dev_priv), 3000, 10) == 0;
+	return wait_for(skl_cdclk_pcu_ready(dev_priv), 10) == 0;
 }
 
 static void skl_set_cdclk(struct drm_i915_private *dev_priv, int cdclk, int vco)
-- 
2.5.0

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

Re: [PATCH v2 2/2] drm/i915/skl: Increase PCODE CDCLK change notify timeout

[Chris Wilson]

On Fri, Nov 25, 2016 at 12:57:01PM +0200, Imre Deak wrote:
> commit 848496e5902833600f7992f4faa82dc1546051ba
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date:   Wed Jul 13 16:32:03 2016 +0300
> 
>     drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
> 
> increased the timeout to match the spec, but we still see a timeout on
> at least one SKL. A CDCLK change request following the failed one will
> succeed nevertheless, so let's try to increase the timeout to 10ms.
> 
> v2:
> - Use 1ms poll period instead of 10us. (Chris)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=97929
> Signed-off-by: Imre Deak <imre.deak@intel.com>

If you are happy with it, so am I!
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Possibly time to volunteer someone to look at the poll interval again.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
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/gen6+: Clear upper data byte during PCODE write (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/gen6+: Clear upper data byte during PCODE write (rev2)
URL   : https://patchwork.freedesktop.org/series/15911/
State : success

== Summary ==

Series 15911v2 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/15911/revisions/2/mbox/


fi-bdw-5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:244  pass:204  dwarn:0   dfail:0   fail:0   skip:40 
fi-byt-j1900     total:244  pass:216  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-n2820     total:244  pass:212  dwarn:0   dfail:0   fail:0   skip:32 
fi-hsw-4770      total:244  pass:224  dwarn:0   dfail:0   fail:0   skip:20 
fi-hsw-4770r     total:244  pass:224  dwarn:0   dfail:0   fail:0   skip:20 
fi-ilk-650       total:244  pass:191  dwarn:0   dfail:0   fail:0   skip:53 
fi-ivb-3520m     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ivb-3770      total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-kbl-7500u     total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6260u     total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:244  pass:223  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6700k     total:244  pass:222  dwarn:1   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:244  pass:212  dwarn:0   dfail:0   fail:0   skip:32 
fi-snb-2600      total:244  pass:211  dwarn:0   dfail:0   fail:0   skip:33 

90e312d771441d1ec7bf3cd72620a3fe9a4aef82 drm-tip: 2016y-11m-25d-07h-11m-59s UTC integration manifest
492a9da drm/i915/skl: Increase PCODE CDCLK change notify timeout
4427f60 drm/i915/gen6+: Clear upper data byte during PCODE write

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3114/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/2] drm/i915/skl: Increase PCODE CDCLK change notify timeout

[Ville Syrjälä]

On Fri, Nov 25, 2016 at 12:57:01PM +0200, Imre Deak wrote:
> commit 848496e5902833600f7992f4faa82dc1546051ba
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date:   Wed Jul 13 16:32:03 2016 +0300
> 
>     drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
> 
> increased the timeout to match the spec, but we still see a timeout on
> at least one SKL. A CDCLK change request following the failed one will
> succeed nevertheless, so let's try to increase the timeout to 10ms.
> 
> v2:
> - Use 1ms poll period instead of 10us. (Chris)

I'm not sure that's what we want. IIRC the spec says no delay between
attempts, so I'm not sure this isn't just going to make it more likely
to fail.

> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=97929
> Signed-off-by: Imre Deak <imre.deak@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 5d11002..f07b611 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6261,7 +6261,7 @@ static bool skl_cdclk_pcu_ready(struct drm_i915_private *dev_priv)
>  
>  static bool skl_cdclk_wait_for_pcu_ready(struct drm_i915_private *dev_priv)
>  {
> -	return _wait_for(skl_cdclk_pcu_ready(dev_priv), 3000, 10) == 0;
> +	return wait_for(skl_cdclk_pcu_ready(dev_priv), 10) == 0;
>  }
>  
>  static void skl_set_cdclk(struct drm_i915_private *dev_priv, int cdclk, int vco)
> -- 
> 2.5.0

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

Re: [PATCH v2 2/2] drm/i915/skl: Increase PCODE CDCLK change notify timeout

[Chris Wilson]

On Fri, Nov 25, 2016 at 01:30:38PM +0200, Ville Syrjälä wrote:
> On Fri, Nov 25, 2016 at 12:57:01PM +0200, Imre Deak wrote:
> > commit 848496e5902833600f7992f4faa82dc1546051ba
> > Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Date:   Wed Jul 13 16:32:03 2016 +0300
> > 
> >     drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
> > 
> > increased the timeout to match the spec, but we still see a timeout on
> > at least one SKL. A CDCLK change request following the failed one will
> > succeed nevertheless, so let's try to increase the timeout to 10ms.
> > 
> > v2:
> > - Use 1ms poll period instead of 10us. (Chris)
> 
> I'm not sure that's what we want. IIRC the spec says no delay between
> attempts, so I'm not sure this isn't just going to make it more likely
> to fail.

Hmm. We're going to be scheduled away eventually, unless we steal the
cpu for 10ms. To be strict to the spec we would need

	preempt_disable();
	ret = wait_for_atomic(pcu_ready(), 10);
	preempt_enable();

	return ret;

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/2] drm/i915/skl: Increase PCODE CDCLK change notify timeout

[Imre Deak]

On pe, 2016-11-25 at 11:44 +0000, Chris Wilson wrote:
> On Fri, Nov 25, 2016 at 01:30:38PM +0200, Ville Syrjälä wrote:
> > On Fri, Nov 25, 2016 at 12:57:01PM +0200, Imre Deak wrote:
> > > commit 848496e5902833600f7992f4faa82dc1546051ba
> > > Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Date:   Wed Jul 13 16:32:03 2016 +0300
> > > 
> > >     drm/i915: Wait up to 3ms for the pcu to ack the cdclk change
> > > request on SKL
> > > 
> > > increased the timeout to match the spec, but we still see a
> > > timeout on
> > > at least one SKL. A CDCLK change request following the failed one
> > > will
> > > succeed nevertheless, so let's try to increase the timeout to
> > > 10ms.
> > > 
> > > v2:
> > > - Use 1ms poll period instead of 10us. (Chris)
> > 
> > I'm not sure that's what we want. IIRC the spec says no delay
> > between
> > attempts, so I'm not sure this isn't just going to make it more
> > likely
> > to fail.
> 
> Hmm. We're going to be scheduled away eventually, unless we steal the
> cpu for 10ms. To be strict to the spec we would need
> 
> 	preempt_disable();
> 	ret = wait_for_atomic(pcu_ready(), 10);
> 	preempt_enable();
> 
> 	return ret;

Yes, after some testing it looks like when things fail we call
pcu_ready() only once after scheduling away for 3ms and so if that one
call fails we time out. So I think we need preempt_disable(), I updated
the patch adding that.

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