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

[Imre Deak <imre.deak@intel.com>]

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