Re: Patch "drm/i915/gen9: Fix PCODE polling during CDCLK change notification" has been added to the 4.9-stable tree

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Jan 16, 2017 at 06:59:46PM +0200, Imre Deak wrote:
> On Sat, Jan 14, 2017 at 09:45:18PM +0200, Thomas Backlund wrote:
> > Den 13.01.2017 kl. 12:57, skrev gregkh@linuxfoundation.org:
> > >
> > >This is a note to let you know that I've just added the patch titled
> > >
> > >    drm/i915/gen9: Fix PCODE polling during CDCLK change notification
> > >
> > >to the 4.9-stable tree which can be found at:
> > >    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > >
> > >The filename of the patch is:
> > >     drm-i915-gen9-fix-pcode-polling-during-cdclk-change-notification.patch
> > >and it can be found in the queue-4.9 subdirectory.
> > >
> > >If you, or anyone else, feels it should not be added to the stable tree,
> > >please let <stable@vger.kernel.org> know about it.
> > >
> > > [...]
> > >+#define COND skl_pcode_try_request(dev_priv, mbox, request, reply_mask, reply, \
> > >+				   &status)
> > >+
> > >+	/*
> > >+	 * Prime the PCODE by doing a request first. Normally it guarantees
> > >+	 * that a subsequent request, at most @timeout_base_ms later, succeeds.
> > >+	 * _wait_for() doesn't guarantee when its passed condition is evaluated
> > >+	 * first, so send the first request explicitly.
> > >+	 */
> > >+	if (COND) {
> > >+		ret = 0;
> > >+		goto out;
> > >+	}
> > >+	ret = _wait_for(COND, timeout_base_ms * 1000, 10);
> > >+	if (!ret)
> > >+		goto out;
> > >+
> > >+	/*
> > >+	 * The above can time out if the number of requests was low (2 in the
> > >+	 * worst case) _and_ PCODE was busy for some reason even after a
> > >+	 * (queued) request and @timeout_base_ms delay. As a workaround retry
> > >+	 * the poll with preemption disabled to maximize the number of
> > >+	 * requests. Increase the timeout from @timeout_base_ms to 50ms to
> > >+	 * account for interrupts that could reduce the number of these
> > >+	 * requests.
> > >+	 */
> > >+	DRM_DEBUG_KMS("PCODE timeout, retrying with preemption disabled\n");
> > >+	WARN_ON_ONCE(timeout_base_ms > 3);
> > >+	preempt_disable();
> > >+	ret = wait_for_atomic(COND, 50);
> > 
> > 
> > Hm...
> > 
> > This does not match the upstream commit.
> > 
> > the upstream commit has:
> > 
> > ret = wait_for_atomic(COND, 10);
> > 
> > 
> > 
> > so 50 in this backport vs 10 in upstream patch
> > 
> > 
> > Is that intentional ?
> 
> Err, no, I screwed this up, thanks for catching it. The 10->50ms change
> is something considered for later, but it's not yet upstream and so
> shouldn't be in stable trees.
> 
> Greg, the following fixes this up on top of the patch that you already
> queued for 4.9 stable, so that the change will match what is upstream.
> This fixed up version is what we'd also need for stable trees starting
> from 4.2.
> 
> Thanks and sorry for being sloppy.

Thanks for this, now queued up.

greg k-h