Re: [PATCH 3/4] drm/i915: enable VT switchless resume v3

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Tue, 20 May 2014 12:53:29 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Fri, 16 May 2014 23:42:23 +0100
> Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> > On Fri, May 16, 2014 at 11:38:07PM +0100, Chris Wilson wrote:
> > > On Fri, May 16, 2014 at 03:20:47PM -0700, Jesse Barnes wrote:
> > > > On Mon, 21 Apr 2014 18:37:31 +0200
> > > > Knut Petersen <Knut_Petersen@t-online.de> wrote:
> > > > > > +	/* This driver doesn't need a VT switch to restore the mode on resume */
> > > > > > +	info->skip_vt_switch = true;
> > > > > > +
> > > > > >   	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
> > > > > >   	drm_fb_helper_fill_var(info, &ifbdev->helper, sizes->fb_width, sizes->fb_height);
> > > > 
> > > > Is it sufficient to just revert this part?  Or are the other bits
> > > > needed too?
> > > > 
> > > > Sorry for the delay on this, I've been traveling a lot the past month
> > > > and buried in other stuff so out of touch with much of my email.
> > > 
> > > The key step here is that X is restarted after resume. The slow down was
> > > due to X not finding any connected outputs and so disabling them all,
> > > setting up a dummy 1024x768 fb which really confused KDE. (KDE queries
> > > the config causing a forced reprobe of all outputs, setups the display
> > > for the native 1280x1024, but screws up KDE's own bookkeeping.)
> > > 
> > > The impact has been fixed by handling the connector->status more
> > > robusting during initial output probing in X. What remains is the
> > > question whether connector->status can be set appropriately upon resume?
> > > It requires a detection cycle to be sure that the outputs are still
> > > there, which is arguably better deferred to userspace. To be consistent
> > > the BIOS take over code should mark connector->status as unknown for the
> > > CRTCs it takes over without doing a detection cycle (where we just
> > > presume that the CRTC/output being enabled means something is on the
> > > other end of the pipe and is still valid).
> > 
> > Hmm. Why didn't fbcon respond to the hotplug event on resume and perform
> > a detection cycle before Knut was able to type startx on the console?
> > That I think is the bug.
> 
> Well, fbcon resume is delayed, but we do perform an fb re-probe via
> intel_resume_hotplug() that should have done this.  Not sure why that's
> sufficient... but I agree userspace should probably re-probe on
> resume.  We're supposed to generate a uevent on resume, but the fb
> layer may suppress that if it detects that no change has occurred.
> 
> Maybe X is racing with our resume_hotplug somehow?  It doesn't look
> like that should happen...

Ah, poll_enable is false until after _thaw finishes, so
our hotplug_resume call of hpd_irq_event does nothing.  So aside from
the encoder hot_plug callbacks (which really just check dp link status,
which ought to be a no-op) our resume_hotplug function doesn't do
anything right now.  May as well kill it and just send an unconditional
uevent?

-- 
Jesse Barnes, Intel Open Source Technology Center