public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	Dave Airlie <airlied@redhat.com>
Subject: Re: [PATCH] i915: Add module option to support VGA arbiter on HD devices
Date: Mon, 12 May 2014 21:38:25 +0200	[thread overview]
Message-ID: <20140512193825.GI25056@phenom.ffwll.local> (raw)
In-Reply-To: <1399923039.6734.97.camel@bling.home>

On Mon, May 12, 2014 at 01:30:39PM -0600, Alex Williamson wrote:
> On Mon, 2014-05-12 at 21:08 +0200, Daniel Vetter wrote:
> > On Fri, May 09, 2014 at 02:20:41PM -0600, Alex Williamson wrote:
> > > Commit 81b5c7bc found that the current VGA arbiter support in i915
> > > only works for ancient GMCH-based IGD devices and attempted to update
> > > support for newer HD devices.  Unfortunately newer devices cannot
> > > completely opt-out of VGA arbitration like the old devices could.
> > > The VGA I/O space cannot be disabled internally.  The only way to
> > > route VGA I/O elsewhere is by disabling I/O at the device PCI command
> > > register.  This means that with commit 81b5c7bc and multiple VGA
> > > adapters, the VGA arbiter will report that multiple VGA devices are
> > > participating in arbitration, Xorg will notice this and disable DRI.
> > > Therefore, 81b5c7bc was reverted because DRI is more important than
> > > being correct.
> > > 
> > > There is however an actual need for i915 to correctly participate in
> > > VGA arbitration; VGA device assignment.  If we want to use VFIO to
> > > assign a VGA device to a virtual machine, we need to be able to
> > > access the VGA resources of that device.  By adding an i915 module
> > > option we can allow i915 to continue with its charade by default, but
> > > also allow an easy path for users who require working VGA arbitration.
> > > Hopefully Xorg can someday be taught to behave better with multiple
> > > VGA devices.
> > > 
> > > This also rolls in reverted commit 6e1b4fda, which corrected an
> > > ordering issue with 81b5c7bc by delaying the disabling of VGA memory
> > > until after vgacon->fbcon handoff.
> > > 
> > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Cc: Dave Airlie <airlied@redhat.com>
> > > ---
> > > 
> > > This should be a nop with the default module setting, so if there's
> > > any opportunity to get this into v3.15, it would be appreciated.
> > 
> > I really don't like module options to work around bugs elsewhere. It means
> > the 5 users who know about a given bug are now happen, and the 3
> > bazillion without enough clue/savy/time still suffer from problems.
> > 
> > My goal is to actually add a bit of support to the core kernel's module
> > option parsing code so that most of the options we have can spit warnings
> > into dmesg and taint the kernel.
> > 
> > Can't we fix this in any other way?
> 
> Do you have any suggestions?  I proposed creating a new VGA arbiter
> interface that would allow Xorg to mmap the legacy VGA MMIO space
> regardless of the number of VGA arbitration participants, but didn't get
> any nibbles on supporting that through the rest of the stack.  Dave
> suggested maybe he could rip out the VGA arbiter support from Xorg and
> nobody would notice.  In either case, at what point do we get to flip
> i915 to behave correctly with arbitration?  It seems like anything we do
> has a compatibility period where we must leave i915 in it's current
> broken state, which means that anyone wanting VGA arbitration needs to
> patch their kernel.  In the best case, that compatibility window could
> extend for years.  Since we don't seem to be making progress on any of
> the other fronts, it seemed time to propose a module switch.  It's not a
> good solution, but it's better than leaving it broken.  Thanks,

Ripping out the vga arbiter stuff from X (or at least disabling it if
there's only kms drivers) seems like a good start - we want that in any
case I think.

Then we could also look into disabling the userspace interface on the
kernel side if we have modeset drivers for everything, and X is the
requesting process. I.e. lie to Xorg. Ugly, but might work.

Then once we've fixed userspace to no longer be stupid and have some way
to work around old stupid userspace, we can fix i915.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

  reply	other threads:[~2014-05-12 19:38 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-09 20:20 [PATCH] i915: Add module option to support VGA arbiter on HD devices Alex Williamson
2014-05-12 19:08 ` Daniel Vetter
2014-05-12 19:30   ` Alex Williamson
2014-05-12 19:38     ` Daniel Vetter [this message]
2014-05-15 21:43       ` Alex Williamson
2014-05-15 22:50         ` Daniel Vetter
2014-05-16  4:46           ` Alex Williamson
2014-05-16  9:06             ` Daniel Vetter
2014-05-16 14:38               ` Alex Williamson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140512193825.GI25056@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=airlied@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox