On Wed, 2013-08-14 at 21:30 +0300, Ville Syrjälä wrote:
> On Wed, Aug 14, 2013 at 11:14:48AM -0600, Alex Williamson wrote:
> > On Wed, 2013-08-14 at 17:47 +0300, Ville Syrjälä wrote:
> > > On Wed, Aug 14, 2013 at 07:23:57AM -0600, Alex Williamson wrote:
> > > > Hi,
> > > >
> > > > I'm trying to add support for device assignment of PCI VGA devices with
> > > > VFIO and QEMU. For normal, discrete discrete graphics the Linux VGA
> > > > arbiter works fairly well, disabling VGA on one bridge and adding it to
> > > > another (though I wish all the kernel VGA drivers made use of it). The
> > > > i915 driver only seems to support disabling VGA on really old GMCH
> > > > devices (see intel_modeset_vga_set_state). This means that if I boot
> > > > with IGD as the primary graphics and attempt to assign a discrete
> > > > graphics device, all the VGA range accesses are still routed to IGD, I
> > > > end up getting some error messages from the IGD interrupt handler, and
> > > > the discrete card never initializes.
> > > >
> > > > I spent some time looking through the Sand Bridge, Ivy Bridge, and
> > > > Haswell datasheets, and I'm a bit concerned whether the hardware even
> > > > provides a reasonable way to disable VGA anymore. Quoting 2.17 from the
> > > > Haswell docs:
> > > >
> > > > Accesses to the VGA memory range are directed to IGD depend on
> > > > the configuration. The configuration is specified by:
> > > > * Internal graphics controller in Device 2 is enabled
> > > > (DEVEN.D2EN bit 4)
> > > > * Internal graphics VGA in Device 0 Function 0 is enabled
> > > > through register GGC bit 1.
> > > > * IGD's memory accesses (PCICMD2 04 – 05h, MAE bit 1) in
> > > > Device 2 configuration space are enabled.
> > > > * VGA compatibility memory accesses (VGA Miscellaneous
> > > > Output register – MSR Register, bit 1) are enabled.
> > > > * Software sets the proper value for VGA Memory Map Mode
> > > > register (VGA GR06 Register, bits 3-2). See the
> > > > following table for translations.
> > > >
> > > > (There's a similar list for VGA I/O range) I've found that if I disable
> > > > memory and I/O in the PCI command register for IGD then I do get VGA
> > > > routing to the PEG device and the discrete VBIOS works. This obviously
> > > > isn't a good option for the VGA arbiter since it entirely disables IGD.
> > > >
> > > > The GGC registers aren't meant for runtime switching and are actually
> > > > locked. Disabling IGD via the device 2 enable bit doesn't seem like and
> > > > option. I don't quite understand the VGA miscellaneous output register
> > > > and VGA memory map mode, but the table provided for the latter makes me
> > > > think they just augment the VGA ranges and don't disable them.
> > >
> > > Bit 1 of MSR (0x3c2/0x3cc) should allow you to turn off VGA mem
> > > access while leaving other memory space access working.
> > >
> > > As for VGA I/O decode, IIRC there's no standard bit for that in VGA
> > > or PCI config registers, and I can't see any other bit for it in the
> > > docs. But I guess you could just turn off I/O space completely
> > > via the PCI_COMMAND register. We shouldn't need it for anything beyond
> > > i915_disable_vga() and that has the necessary vgaarb calls already.
> >
> > Thanks Ville. The MSR seems to work for VGA memory. Disabling I/O via
> > PCI_COMMAND does works, but something is re-enabling it after
> > intel_modeset_vga_set_state(). If I manually disable I/O with setpci
> > then I do have VGA routing to PEG and can still interact with the KMS
> > console on IGD. It's unfortunate that the MSR bit for I/O only disables
> > pieces of the range. If we have no other options, I'll try to hunt down
> > where I/O is being re-enabled and see how feasible it is to avoid.
> > Thanks,
>
> Hmm. Now that I look at vgaarb more it seems I misunderstood the way it
> works. Based on the code it looks like it will permanently remove the
> device from the arbiration if set_vga_decode indicates that it doesn't
> decode legacy resources. And it calls set_vga_decode w/ decode=false
> if there are more than two VGA cards in the system. That means
s/more than two/two or more/
> i915_disable_vga() is actually broken whenever there is another
> VGA card in the system.
I didn't follow why i915_disable_vga is broken. It seems like the
intention is to disable VGA regardless of how many VGA devices are
present.
> To make it work I think what we'd need to do is always return
> VGA_RSRC_LEGACY_IO from i915_vga_set_decode(), which will leave the
> PCI_COMMAND I/O bit in the hands of vgaarb, and then poke at the
> MSR register to disable the VGA mem decode permanently. But to touch
> MSR we actually need VGA I/O, so I guess we'd have to do that right
> after registering w/ vgaarb. Doing it from i915_vga_set_decode()
> doesn't look possible since there's no guarantee that VGA I/O would
> end up at the right device at the time that is called.
>
> So maybe the following patch might work (although maybe vgaarb itself
> should be extended a bit to properly support this use case).
Well, I added the VGA memory bit MSR twiddling to i915_disable_vga since
it's already acquiring VGA I/O for other similar tasks, otherwise tested
the patch as sent. It doesn't work. The arbiter disables both memory
and I/O on IGD. Adding Dave Airlie in case there's an obvious solution
for this. Thanks,
Alex
> From 90070e547f1582f8e73d9221d6a31502dea8246d Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@xxxxxxxxxxxxxxx>
> Date: Wed, 14 Aug 2013 21:20:57 +0300
> Subject: [PATCH] vga arb stuf
>
> ---
> drivers/gpu/drm/i915/i915_dma.c | 13 +++++--------
> drivers/gpu/drm/i915/intel_display.c | 9 ---------
> 2 files changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 0adfe40..9c8f9725 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1225,14 +1225,7 @@ intel_teardown_mchbar(struct drm_device *dev)
> /* true = enable decode, false = disable decoder */
> static unsigned int i915_vga_set_decode(void *cookie, bool state)
> {
> - struct drm_device *dev = cookie;
> -
> - intel_modeset_vga_set_state(dev, state);
> - if (state)
> - return VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM |
> - VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
> - else
> - return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
> + return VGA_RSRC_LEGACY_IO | VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
> }
>
> static void i915_switcheroo_set_state(struct pci_dev *pdev, enum vga_switcheroo_state state)
> @@ -1291,6 +1284,10 @@ static int i915_load_modeset_init(struct drm_device *dev)
> if (ret && ret != -ENODEV)
> goto out;
>
> + vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO);
> + outb(inb(VGA_MSR_READ) & ~VGA_MSR_MEM_EN, VGA_MSR_WRITE);
> + vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
> +
> intel_register_dsm_handler();
>
> ret = vga_switcheroo_register_client(dev->pdev, &i915_switcheroo_ops);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1c8441b..c402308 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10359,15 +10359,6 @@ void intel_connector_attach_encoder(struct intel_connector *connector,
> */
> int intel_modeset_vga_set_state(struct drm_device *dev, bool state)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - u16 gmch_ctrl;
> -
> - pci_read_config_word(dev_priv->bridge_dev, INTEL_GMCH_CTRL, &gmch_ctrl);
> - if (state)
> - gmch_ctrl &= ~INTEL_GMCH_VGA_DISABLE;
> - else
> - gmch_ctrl |= INTEL_GMCH_VGA_DISABLE;
> - pci_write_config_word(dev_priv->bridge_dev, INTEL_GMCH_CTRL, gmch_ctrl);
> return 0;
> }
>
> --
> 1.8.1.5
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
