[Intel-gfx] [PATCH 3/3] drm/i915: Don't wait interruptible for possible plane buffer flush

[Eric Anholt]

On Tue, 1 Dec 2009 14:18:14 +0800, Zhenyu Wang <zhenyuw at linux.intel.com> wrote:
> On 2009.12.01 13:33:24 +0800, Zhenyu Wang wrote:
> > On 2009.11.30 12:59:30 -0800, Jesse Barnes wrote:
> > > On Thu, 26 Nov 2009 09:08:52 +0800
> > > Zhenyu Wang <zhenyuw at linux.intel.com> wrote:
> > > 
> > > > On 2009.11.25 12:16:33 -0800, Eric Anholt wrote:
> > > > > On Wed, 25 Nov 2009 13:09:39 +0800, Zhenyu Wang
> > > > > <zhenyuw at linux.intel.com> wrote:
> > > > > > When we setup buffer for display plane, we'll check any pending
> > > > > > required GPU flush and possible make interruptible wait for flush
> > > > > > complete. But that wait would be most possibly to fail in case of
> > > > > > signals received for X process, which will then fail modeset
> > > > > > process and put display engine in unconsistent state. The result
> > > > > > could be blank screen or CPU hang, and DDX driver would always
> > > > > > turn on outputs DPMS after whatever modeset fails or not.
> > > > > > 
> > > > > > So this one creates new helper for setup display plane buffer, and
> > > > > > when needing flush using uninterruptible wait for that.
> > > > > 
> > > > > Can you explain why the modeset process is special and shouldn't be
> > > > > interruptible?  It certainly seems to me like it should be
> > > > > interruptible like other driver commands are.
> > > > 
> > > > If wait is interrupted by something else, it will abort plane setup
> > > > and mode_set function, within that we've already done new DPLL
> > > > setting and other stuff. Failure in mode_set also won't proceed on
> > > > real pipe setup later in commit function, so we're stopped in the
> > > > middle of the timing sensitive modesetting process. Normally no plane
> > > > enabled means blank screen like for bug 24009.
> > > 
> > > It's a nasty bug, thanks for catching it.  We can either make our GTT
> > > transition uninterruptible (what you've done), or we can make the
> > > failure paths of the mode setting core a bit more robust against
> > > signals.  Have you looked at what it would take to do the latter?  It
> > > might be good to do anyway, since even with an uninterruptible flush we
> > > might fail (or timeout, do we do that?) and so would need to unwind any
> > > previous programming...
> > > 
> > 
> > We don't have a timeout version of that yet. I don't have good idea on how
> > to handle it in failure path, undo pipe and output off action? I was trying
> > to put fb domain change earlier, but current crtc->prepare() has void return,
> > so not a good place for this. Or earlier in mode_set, we might still need
> > to handle for signal. So my point is we'd better not to be interrupted in
> > mode setting.
> > 
> > I think another approach might be simpler like below patch, that we always
> > try to wait rendering on fb bo to be finished before modesetting. So later
> > kernel will just see it fine for plane and ready to be used, we may remove
> > the domain change in kernel as well, but that depends on user space version.
> > This patch is totally _untested_.
> > 
> 
> Sorry, this version fixes compiling errors...Any comment?
> 
> From 909300f6b91a0fc367397d6880759cc91fec4806 Mon Sep 17 00:00:00 2001
> From: Zhenyu Wang <zhenyuw at linux.intel.com>
> Date: Tue, 1 Dec 2009 14:16:32 +0800
> Subject: [PATCH] Wait rendering to fb before modesetting
> 
> Safely wait rendering to be complete for front buffer before
> modesetting. Kernel does interruptible wait for this, but signals
> would abort it which fails the whole modesetting process.
> 
> Signed-off-by: Zhenyu Wang <zhenyuw at linux.intel.com>

This is itself racy, so I've gone ahead and applied your kernel change.
Uninterruptible ioctls suck, but maybe those can get fixed later.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20100112/8052d3f0/attachment.sig>