All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Dave Airlie <airlied@redhat.com>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2 1/1] drm/i915: Fix VGA handling using stop_machine() or mmio
Date: Mon, 30 Sep 2013 10:45:03 -0600	[thread overview]
Message-ID: <1380559503.2674.166.camel@ul30vt.home> (raw)
In-Reply-To: <20130930150948.GC9395@intel.com>

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,

Alex

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2013-09-30 16:45 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-30 14:08 [PATCH 0/1] drm/i915: Another version of the vga stop_machine monstrosity ville.syrjala
2013-09-30 14:08 ` [PATCH v2 1/1] drm/i915: Fix VGA handling using stop_machine() or mmio ville.syrjala
2013-09-30 14:24   ` Chris Wilson
2013-09-30 14:41     ` Ville Syrjälä
2013-09-30 14:53       ` Chris Wilson
2013-09-30 15:10         ` Ville Syrjälä
2013-09-30 16:45           ` Chris Wilson
2013-09-30 18:37     ` Alex Williamson
2013-10-07  0:23       ` Dave Airlie
2013-10-07  7:34         ` Daniel Vetter
2013-10-02 13:42     ` [PATCH v3] " ville.syrjala
2013-10-07  9:15       ` Chris Wilson
2013-10-07  9:25         ` Ville Syrjälä
2013-10-07  9:44           ` Chris Wilson
2013-09-30 14:43   ` [PATCH v2 1/1] " Alex Williamson
2013-09-30 15:09     ` Ville Syrjälä
2013-09-30 16:45       ` Alex Williamson [this message]
2013-09-30 17:23         ` Ville Syrjälä

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=1380559503.2674.166.camel@ul30vt.home \
    --to=alex.williamson@redhat.com \
    --cc=airlied@redhat.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=ville.syrjala@linux.intel.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.