From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v2 1/1] drm/i915: Fix VGA handling using stop_machine() or mmio Date: Mon, 30 Sep 2013 20:23:59 +0300 Message-ID: <20130930172359.GF9395@intel.com> References: <1380550111-31590-1-git-send-email-ville.syrjala@linux.intel.com> <1380550111-31590-2-git-send-email-ville.syrjala@linux.intel.com> <1380552231.2674.79.camel@ul30vt.home> <20130930150948.GC9395@intel.com> <1380559503.2674.166.camel@ul30vt.home> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 7C670E6143 for ; Mon, 30 Sep 2013 10:24:04 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1380559503.2674.166.camel@ul30vt.home> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Alex Williamson Cc: Dave Airlie , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Sep 30, 2013 at 10:45:03AM -0600, Alex Williamson wrote: > On Mon, 2013-09-30 at 18:09 +0300, Ville Syrj=E4l=E4 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 wrot= e: > > > > From: Ville Syrj=E4l=E4 > > > > = > > > > 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 le= ave > > > > 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 spa= ce > > > > completely via the PCI command register. > > > > = > > > > But we still need VGA register access during resume (and possibly d= uring > > > > 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 ke= ep > > > > 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=E4l=E4 Intel OTC