Re: [PATCH v2 1/1] drm/i915: Fix VGA handling using stop_machine() or mmio

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Sep 30, 2013 at 10:45:03AM -0600, Alex Williamson wrote:
> On Mon, 2013-09-30 at 18:09 +0300, Ville Syrjälä wrote:
> > On Mon, Sep 30, 2013 at 08:43:51AM -0600, Alex Williamson wrote:
> > > On Mon, 2013-09-30 at 17:08 +0300, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > We have several problems with out VGA handling:
> > > > - We try to use the GMCH control VGA disable bit even though it may
> > > >   be locked
> > > > - If we manage to disable VGA throuh GMCH control, we're no longer
> > > >   able to correctly disable the VGA plane
> > > > - Taking part in the VGA arbitration is too expensive for X [1]
> > > > 
> > > > So let's treat the GMCH control VGA disable bit as read-only and leave
> > > > it for the BIOS to set, as it was intended. To disable VGA we will use
> > > > the VGA misc register, and to disable VGA IO we will disable IO space
> > > > completely via the PCI command register.
> > > > 
> > > > But we still need VGA register access during resume (and possibly during
> > > > lid event on insane BIOSen) to disable the VGA plane. Also we need to
> > > > re-disable VGA memory decode via the VGA misc register on resume.
> > > > 
> > > > Luckily up to gen4, VGA registers can be accessed through MMIO.
> > > > Unfortunately from gen5 onwards only the legacy VGA IO port range
> > > > works. So on gen5+ we still need IO space to be enabled during those
> > > > few special moments when we need to access VGA registers.
> > > > 
> > > > We still want to opt out of VGA arbitration on gen5+, so we have keep
> > > > IO space disabled most of the time. And when we do need to poke at VGA
> > > > registers, we enable IO space briefly while no one is looking. To
> > > > guarantee that no one is looking we will use stop_machine().
> > > 
> > > What?!  Why would we not simply wait for the arbiter lock?
> > 
> > Well, there are the X problems which I really don't want to
> > attempt solving.
> > 
> > Also the arbiter looks a lot like deadlock heaven to me.
> > 
> > What if the other guy doesn't release the arbiter lock in a timely
> > fashion? It could be some userspace process that's stopped inside
> > gdb or something.
> > 
> > What if we're doing the restore thing in intel_lid_notify()
> > and we've already locked the modeset locks and are now waiting
> > for the arbiter lock, but the other guy who is holding the arbiter
> > lock is doing a modeset ioctl at the same time and gets stuck
> > waiting for a modeset lock?
> > 
> > I guess we might be able to solve those problems by killing
> > the userspace client after a while. But I'd rather just hide
> > and go code up something more productive ;)
> 
> So in summary, ignore the infrastructure intended to solve this problem
> because it's hard and may have corner cases and instead stop the entire
> machine so we can be sure our access is exclusive.  If this is the
> solution then VGA arbiter has failed us.  Thanks,

Well, if you want to fix the vga arbiter (assuming it's possible) feel
free to send patches. I can even give a vague promise of reviewing them.
But I guess we need to fix this ASAP or face the wrath of the angry mob
who lost their dri capabilities.

The other option is to move the stop_machine infrastructure inside
vgaarb and then we can say we're using vgaarb as *deity* intended ;)

-- 
Ville Syrjälä
Intel OTC