[PATCH] drm/i915: revert vga arbiter changes

[Alex Williamson]

On Tue, 2013-10-08 at 17:29 +0100, Chris Wilson wrote:
> On Tue, Oct 08, 2013 at 10:18:54AM -0600, Alex Williamson wrote:
> > On Tue, 2013-10-08 at 17:08 +0100, Chris Wilson wrote:
> > > On Tue, Oct 08, 2013 at 05:27:28PM +0200, Daniel Vetter wrote:
> > > > Adding drm/i915 into the vga arbiter chain means that X (in a piece of
> > > > well-meant paranoia) will do a get/put on the vga decoding around
> > > > _every_ accel call down into the ddx. Which results in some nice
> > > > performance disasters [1].
> > > 
> > > And disables DRI for the system, interferring with the use of OpenGL
> > > compositors, playing games, PRIME, etc. This effect is not limited to
> > > users of i915, but any system with a dGPU and an igfx enabled Core CPU.
> > > 
> > > I still think we are downplaying the complete and utter disaster caused
> > > by X and the patch, and why the revert is required.
> > 
> > I'm failing to understand the disaster myself, can you explain?  i915 is
> > already attempting to make use of the VGA arbiter, but the code hadn't
> > been updated so what it was doing was ineffective.  The external
> > difference I see is that before i915 incorrectly reported that it was
> > not involved in VGA arbitration (even though it still claims VGA
> > transactions).  With the fixes, it opts-out of MMIO arbitration, but
> > requires I/O port arbitration.  I'm OK with reverting these and going
> > back to the drawing board for v3.13, but what i915 is doing is still
> > incorrect and precludes making use of a guest assigned VGA device (or
> > separate display server requiring VGA), because we can't use the VGA
> > arbiter for what it's designed to do, re-route VGA accesses.  Thanks,
> 
> The problem stems from the system reporting to X that there are two
> legacy vga IO capable devices, and then X disabling DRI and wrapping
> every operation with vga_get/vga_put. X will do this even if it is only
> using one of the devices and the other device is not being used by the
> system.
> 
> We are not arguing that your fix isn't correct, it is just the
> rammifications it has to the existing install base outweighs improving
> a currently broken feature.

Sure, my use case is not a priority.  Do you have any thoughts on how we
can redesign the interface?  For my use for VM access, I'm pretty much
resolved and OK with the fact that VGA access is slow.  Without
para-virtualizing VGA, I can't predict or batch accesses, so I take the
arbiter lock for the appropriate resource around each individual access.
If the lock is contended, VM boot can be painful, but eventually
accelerated drivers take over and VGA access becomes rare.

I know Ville was concerned about stopped processes holding the lock when
i915 needs it.  In my case, that can't happen because the VGA arbiter
lock is handled within the VFIO interface.  A read/write to the device
file descriptor will grab and release the lock internally.

It's hard to get around the benefits of knowing your access pattern and
batching accesses within a lock, but I wonder if VGA arbiter's lazy
switching would be sufficient to provide a general purpose legacy VGA
file descriptor that X could open and use for all VGA accesses.  For
example, pci-sysfs will provide legacy I/O interfaces for some
architectures, and I think X already makes use of these.  What if they
were provided on x86 and allowed read/write to VGA ranges wrapped in VGA
arbiter locks?  Thanks,

Alex