[PATCH] i915: Update VGA arbiter support for newer devices

[PATCH] i915: Update VGA arbiter support for newer devices

[Alex Williamson]

This is intended to add VGA arbiter support for Intel HD graphics on
Core processors.  The old GMCH registers no longer exist, so even
though it appears that i915 participates in VGA arbitration, it doesn't
work.  On Intel HD graphics we already attempt to disable VGA regions
of the device.  This makes registering as a VGA client unnecessary since
we don't intend to operate differently depending on how many VGA devices
are present.  We can disable VGA memory regions by clearing a memory
enable bit in the VGA MSR.  That only leaves VGA IO, which we update
the VGA arbiter to know that we don't participate in VGA memory
arbitration.  We also add a hook on unload to re-enable memory and
reinstate VGA memory arbitration.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

This patch depends on series "vgaarb: Fixes for partial VGA opt-out".

Is HAS_PCH_SPLIT() the right test to identify newer graphics devices?

I've tried not to disturb the old code since I can't test it, but
welcome feedback whether we should just remove it.  I also don't know
what most of i915_disable_vga() is doing and whether it's still
relevant.

 drivers/gpu/drm/i915/i915_dma.c      |    9 ++++++---
 drivers/gpu/drm/i915/intel_display.c |   26 ++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index f466980..d9cf216 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1287,9 +1287,12 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	 * then we do not take part in VGA arbitration and the
 	 * vga_client_register() fails with -ENODEV.
 	 */
-	ret = vga_client_register(dev->pdev, dev, NULL, i915_vga_set_decode);
-	if (ret && ret != -ENODEV)
-		goto out;
+	if (!HAS_PCH_SPLIT(dev)) {
+		ret = vga_client_register(dev->pdev, dev, NULL,
+					  i915_vga_set_decode);
+		if (ret && ret != -ENODEV)
+			goto out;
+	}
 
 	intel_register_dsm_handler();
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5fb3058..759ad7e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9519,6 +9519,16 @@ static void i915_disable_vga(struct drm_device *dev)
 	outb(SR01, VGA_SR_INDEX);
 	sr1 = inb(VGA_SR_DATA);
 	outb(sr1 | 1<<5, VGA_SR_DATA);
+
+	/* Disable VGA memory on Intel HD */
+	if (HAS_PCH_SPLIT(dev)) {
+		I915_WRITE8(VGA_MSR_WRITE,
+			    I915_READ8(VGA_MSR_READ) & ~VGA_MSR_MEM_EN);
+		vga_set_legacy_decoding(dev->pdev, VGA_RSRC_LEGACY_IO |
+						   VGA_RSRC_NORMAL_IO |
+						   VGA_RSRC_NORMAL_MEM);
+	}
+
 	vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
 	udelay(300);
 
@@ -9526,6 +9536,20 @@ static void i915_disable_vga(struct drm_device *dev)
 	POSTING_READ(vga_reg);
 }
 
+static void i915_enable_vga(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	/* Disable VGA memory on Intel HD */
+	if (HAS_PCH_SPLIT(dev)) {
+		I915_WRITE8(VGA_MSR_WRITE,
+			    I915_READ8(VGA_MSR_READ) | VGA_MSR_MEM_EN);
+		vga_set_legacy_decoding(dev->pdev, VGA_RSRC_LEGACY_MASK |
+						   VGA_RSRC_NORMAL_IO |
+						   VGA_RSRC_NORMAL_MEM);
+	}
+}
+
 void intel_modeset_init_hw(struct drm_device *dev)
 {
 	intel_init_power_well(dev);
@@ -9983,6 +10007,8 @@ void intel_modeset_cleanup(struct drm_device *dev)
 
 	intel_disable_fbc(dev);
 
+	i915_enable_vga(dev);
+
 	intel_disable_gt_powersave(dev);
 
 	ironlake_teardown_rc6(dev);

Re: [PATCH] i915: Update VGA arbiter support for newer devices

[Dave Airlie]

On Fri, Aug 16, 2013 at 8:43 AM, Alex Williamson
<alex.williamson@redhat.com> wrote:
> This is intended to add VGA arbiter support for Intel HD graphics on
> Core processors.  The old GMCH registers no longer exist, so even
> though it appears that i915 participates in VGA arbitration, it doesn't
> work.  On Intel HD graphics we already attempt to disable VGA regions
> of the device.  This makes registering as a VGA client unnecessary since
> we don't intend to operate differently depending on how many VGA devices
> are present.  We can disable VGA memory regions by clearing a memory
> enable bit in the VGA MSR.  That only leaves VGA IO, which we update
> the VGA arbiter to know that we don't participate in VGA memory
> arbitration.  We also add a hook on unload to re-enable memory and
> reinstate VGA memory arbitration.

I would think there is still a VGA disable bit on the Intel device
somewhere, we'd just need
Intel to look in the docs and find it. A bit that can nuke both i/o
and cmd regs.

Dave.

Re: [PATCH] i915: Update VGA arbiter support for newer devices

[Alex Williamson]

On Fri, 2013-08-16 at 08:49 +1000, Dave Airlie wrote:
> On Fri, Aug 16, 2013 at 8:43 AM, Alex Williamson
> <alex.williamson@redhat.com> wrote:
> > This is intended to add VGA arbiter support for Intel HD graphics on
> > Core processors.  The old GMCH registers no longer exist, so even
> > though it appears that i915 participates in VGA arbitration, it doesn't
> > work.  On Intel HD graphics we already attempt to disable VGA regions
> > of the device.  This makes registering as a VGA client unnecessary since
> > we don't intend to operate differently depending on how many VGA devices
> > are present.  We can disable VGA memory regions by clearing a memory
> > enable bit in the VGA MSR.  That only leaves VGA IO, which we update
> > the VGA arbiter to know that we don't participate in VGA memory
> > arbitration.  We also add a hook on unload to re-enable memory and
> > reinstate VGA memory arbitration.
> 
> I would think there is still a VGA disable bit on the Intel device
> somewhere, we'd just need
> Intel to look in the docs and find it. A bit that can nuke both i/o
> and cmd regs.

The only bit available is in the GGC and is a keyed/locked register that
not only disables VGA memory and I/O, but also modifies the class code
of the device.  Early Core processors didn't lock this, but it's
untouchable in newer ones AFAICT.  Thanks,

Alex

Re: [PATCH] i915: Update VGA arbiter support for newer devices

[Ville Syrjälä]

On Thu, Aug 15, 2013 at 04:54:15PM -0600, Alex Williamson wrote:
> On Fri, 2013-08-16 at 08:49 +1000, Dave Airlie wrote:
> > On Fri, Aug 16, 2013 at 8:43 AM, Alex Williamson
> > <alex.williamson@redhat.com> wrote:
> > > This is intended to add VGA arbiter support for Intel HD graphics on
> > > Core processors.  The old GMCH registers no longer exist, so even
> > > though it appears that i915 participates in VGA arbitration, it doesn't
> > > work.  On Intel HD graphics we already attempt to disable VGA regions
> > > of the device.  This makes registering as a VGA client unnecessary since
> > > we don't intend to operate differently depending on how many VGA devices
> > > are present.  We can disable VGA memory regions by clearing a memory
> > > enable bit in the VGA MSR.  That only leaves VGA IO, which we update
> > > the VGA arbiter to know that we don't participate in VGA memory
> > > arbitration.  We also add a hook on unload to re-enable memory and
> > > reinstate VGA memory arbitration.
> > 
> > I would think there is still a VGA disable bit on the Intel device
> > somewhere, we'd just need
> > Intel to look in the docs and find it. A bit that can nuke both i/o
> > and cmd regs.
> 
> The only bit available is in the GGC and is a keyed/locked register that
> not only disables VGA memory and I/O, but also modifies the class code
> of the device.  Early Core processors didn't lock this, but it's
> untouchable in newer ones AFAICT.  Thanks,

I've not found anything else in the docs. And also we _need_ VGA I/O
access to make i915_disable_vga() work. It's not 100% clear whether
we really need to poke at the sequencer register in modern hardware,
but the docs do still list it as a mandatory step. So even if we were
to have a global "disable VGA I/O and mem bit" we'd need to make sure
we already disabled VGA eg. after resume when the BIOS had a chance to
turn the VGA display back on. I think there were also some BIOSen that
turned VGA display back on when closing/opening the laptop lid. Not
sure what would even happen with those if totally disabled VGA I/O
access. I'm not sure they actually frob with the VGA regs though.
Could be they just turn on the VGA display bit in the VGA_CONTROL
register.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] i915: Update VGA arbiter support for newer devices

[Ville Syrjälä]

On Thu, Aug 15, 2013 at 04:54:15PM -0600, Alex Williamson wrote:
> On Fri, 2013-08-16 at 08:49 +1000, Dave Airlie wrote:
> > On Fri, Aug 16, 2013 at 8:43 AM, Alex Williamson
> > <alex.williamson@redhat.com> wrote:
> > > This is intended to add VGA arbiter support for Intel HD graphics on
> > > Core processors.  The old GMCH registers no longer exist, so even
> > > though it appears that i915 participates in VGA arbitration, it doesn't
> > > work.  On Intel HD graphics we already attempt to disable VGA regions
> > > of the device.  This makes registering as a VGA client unnecessary since
> > > we don't intend to operate differently depending on how many VGA devices
> > > are present.  We can disable VGA memory regions by clearing a memory
> > > enable bit in the VGA MSR.  That only leaves VGA IO, which we update
> > > the VGA arbiter to know that we don't participate in VGA memory
> > > arbitration.  We also add a hook on unload to re-enable memory and
> > > reinstate VGA memory arbitration.
> > 
> > I would think there is still a VGA disable bit on the Intel device
> > somewhere, we'd just need
> > Intel to look in the docs and find it. A bit that can nuke both i/o
> > and cmd regs.
> 
> The only bit available is in the GGC and is a keyed/locked register that
> not only disables VGA memory and I/O, but also modifies the class code
> of the device.  Early Core processors didn't lock this, but it's
> untouchable in newer ones AFAICT.  Thanks,

I've not found anything else in the docs. And also we _need_ VGA I/O
access to make i915_disable_vga() work. It's not 100% clear whether
we really need to poke at the sequencer register in modern hardware,
but the docs do still list it as a mandatory step. So even if we were
to have a global "disable VGA I/O and mem bit" we'd need to make sure
we already disabled VGA eg. after resume when the BIOS had a chance to
turn the VGA display back on. I think there were also some BIOSen that
turned VGA display back on when closing/opening the laptop lid. Not
sure what would even happen with those if totally disabled VGA I/O
access. I'm not sure they actually frob with the VGA regs though.
Could be they just turn on the VGA display bit in the VGA_CONTROL
register.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] i915: Update VGA arbiter support for newer devices

[Alex Williamson]

On Fri, 2013-08-16 at 13:20 +0300, Ville Syrjälä wrote:
> On Thu, Aug 15, 2013 at 04:54:15PM -0600, Alex Williamson wrote:
> > On Fri, 2013-08-16 at 08:49 +1000, Dave Airlie wrote:
> > > On Fri, Aug 16, 2013 at 8:43 AM, Alex Williamson
> > > <alex.williamson@redhat.com> wrote:
> > > > This is intended to add VGA arbiter support for Intel HD graphics on
> > > > Core processors.  The old GMCH registers no longer exist, so even
> > > > though it appears that i915 participates in VGA arbitration, it doesn't
> > > > work.  On Intel HD graphics we already attempt to disable VGA regions
> > > > of the device.  This makes registering as a VGA client unnecessary since
> > > > we don't intend to operate differently depending on how many VGA devices
> > > > are present.  We can disable VGA memory regions by clearing a memory
> > > > enable bit in the VGA MSR.  That only leaves VGA IO, which we update
> > > > the VGA arbiter to know that we don't participate in VGA memory
> > > > arbitration.  We also add a hook on unload to re-enable memory and
> > > > reinstate VGA memory arbitration.
> > > 
> > > I would think there is still a VGA disable bit on the Intel device
> > > somewhere, we'd just need
> > > Intel to look in the docs and find it. A bit that can nuke both i/o
> > > and cmd regs.
> > 
> > The only bit available is in the GGC and is a keyed/locked register that
> > not only disables VGA memory and I/O, but also modifies the class code
> > of the device.  Early Core processors didn't lock this, but it's
> > untouchable in newer ones AFAICT.  Thanks,
> 
> I've not found anything else in the docs. And also we _need_ VGA I/O
> access to make i915_disable_vga() work. It's not 100% clear whether
> we really need to poke at the sequencer register in modern hardware,
> but the docs do still list it as a mandatory step. So even if we were
> to have a global "disable VGA I/O and mem bit" we'd need to make sure
> we already disabled VGA eg. after resume when the BIOS had a chance to
> turn the VGA display back on. I think there were also some BIOSen that
> turned VGA display back on when closing/opening the laptop lid. Not
> sure what would even happen with those if totally disabled VGA I/O
> access. I'm not sure they actually frob with the VGA regs though.
> Could be they just turn on the VGA display bit in the VGA_CONTROL
> register.

Hmm, it appears the MSR write isn't fully disabling VGA memory space.
When the VBIOS for the PEG graphics is run in the guest, I get some
corruption of the IGD frame buffer.  If I manually disable PCI memory in
the command register, this doesn't happen.  I also get some strange
artifacts on the PEG display that don't happen when PCI memory is
disabled.  Should that MSR bit give us the whole a_0000-b_ffff range?
Thanks,

Alex

Re: [PATCH] i915: Update VGA arbiter support for newer devices

[Alex Williamson]

On Fri, 2013-08-16 at 13:20 +0300, Ville Syrjälä wrote:
> On Thu, Aug 15, 2013 at 04:54:15PM -0600, Alex Williamson wrote:
> > On Fri, 2013-08-16 at 08:49 +1000, Dave Airlie wrote:
> > > On Fri, Aug 16, 2013 at 8:43 AM, Alex Williamson
> > > <alex.williamson@redhat.com> wrote:
> > > > This is intended to add VGA arbiter support for Intel HD graphics on
> > > > Core processors.  The old GMCH registers no longer exist, so even
> > > > though it appears that i915 participates in VGA arbitration, it doesn't
> > > > work.  On Intel HD graphics we already attempt to disable VGA regions
> > > > of the device.  This makes registering as a VGA client unnecessary since
> > > > we don't intend to operate differently depending on how many VGA devices
> > > > are present.  We can disable VGA memory regions by clearing a memory
> > > > enable bit in the VGA MSR.  That only leaves VGA IO, which we update
> > > > the VGA arbiter to know that we don't participate in VGA memory
> > > > arbitration.  We also add a hook on unload to re-enable memory and
> > > > reinstate VGA memory arbitration.
> > > 
> > > I would think there is still a VGA disable bit on the Intel device
> > > somewhere, we'd just need
> > > Intel to look in the docs and find it. A bit that can nuke both i/o
> > > and cmd regs.
> > 
> > The only bit available is in the GGC and is a keyed/locked register that
> > not only disables VGA memory and I/O, but also modifies the class code
> > of the device.  Early Core processors didn't lock this, but it's
> > untouchable in newer ones AFAICT.  Thanks,
> 
> I've not found anything else in the docs. And also we _need_ VGA I/O
> access to make i915_disable_vga() work. It's not 100% clear whether
> we really need to poke at the sequencer register in modern hardware,
> but the docs do still list it as a mandatory step. So even if we were
> to have a global "disable VGA I/O and mem bit" we'd need to make sure
> we already disabled VGA eg. after resume when the BIOS had a chance to
> turn the VGA display back on. I think there were also some BIOSen that
> turned VGA display back on when closing/opening the laptop lid. Not
> sure what would even happen with those if totally disabled VGA I/O
> access. I'm not sure they actually frob with the VGA regs though.
> Could be they just turn on the VGA display bit in the VGA_CONTROL
> register.

Hmm, it appears the MSR write isn't fully disabling VGA memory space.
When the VBIOS for the PEG graphics is run in the guest, I get some
corruption of the IGD frame buffer.  If I manually disable PCI memory in
the command register, this doesn't happen.  I also get some strange
artifacts on the PEG display that don't happen when PCI memory is
disabled.  Should that MSR bit give us the whole a_0000-b_ffff range?
Thanks,

Alex

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

Re: [PATCH] i915: Update VGA arbiter support for newer devices

[Ville Syrjälä]

On Fri, Aug 16, 2013 at 12:22:14PM -0600, Alex Williamson wrote:
> On Fri, 2013-08-16 at 13:20 +0300, Ville Syrjälä wrote:
> > On Thu, Aug 15, 2013 at 04:54:15PM -0600, Alex Williamson wrote:
> > > On Fri, 2013-08-16 at 08:49 +1000, Dave Airlie wrote:
> > > > On Fri, Aug 16, 2013 at 8:43 AM, Alex Williamson
> > > > <alex.williamson@redhat.com> wrote:
> > > > > This is intended to add VGA arbiter support for Intel HD graphics on
> > > > > Core processors.  The old GMCH registers no longer exist, so even
> > > > > though it appears that i915 participates in VGA arbitration, it doesn't
> > > > > work.  On Intel HD graphics we already attempt to disable VGA regions
> > > > > of the device.  This makes registering as a VGA client unnecessary since
> > > > > we don't intend to operate differently depending on how many VGA devices
> > > > > are present.  We can disable VGA memory regions by clearing a memory
> > > > > enable bit in the VGA MSR.  That only leaves VGA IO, which we update
> > > > > the VGA arbiter to know that we don't participate in VGA memory
> > > > > arbitration.  We also add a hook on unload to re-enable memory and
> > > > > reinstate VGA memory arbitration.
> > > > 
> > > > I would think there is still a VGA disable bit on the Intel device
> > > > somewhere, we'd just need
> > > > Intel to look in the docs and find it. A bit that can nuke both i/o
> > > > and cmd regs.
> > > 
> > > The only bit available is in the GGC and is a keyed/locked register that
> > > not only disables VGA memory and I/O, but also modifies the class code
> > > of the device.  Early Core processors didn't lock this, but it's
> > > untouchable in newer ones AFAICT.  Thanks,
> > 
> > I've not found anything else in the docs. And also we _need_ VGA I/O
> > access to make i915_disable_vga() work. It's not 100% clear whether
> > we really need to poke at the sequencer register in modern hardware,
> > but the docs do still list it as a mandatory step. So even if we were
> > to have a global "disable VGA I/O and mem bit" we'd need to make sure
> > we already disabled VGA eg. after resume when the BIOS had a chance to
> > turn the VGA display back on. I think there were also some BIOSen that
> > turned VGA display back on when closing/opening the laptop lid. Not
> > sure what would even happen with those if totally disabled VGA I/O
> > access. I'm not sure they actually frob with the VGA regs though.
> > Could be they just turn on the VGA display bit in the VGA_CONTROL
> > register.
> 
> Hmm, it appears the MSR write isn't fully disabling VGA memory space.
> When the VBIOS for the PEG graphics is run in the guest, I get some
> corruption of the IGD frame buffer.  If I manually disable PCI memory in
> the command register, this doesn't happen.  I also get some strange
> artifacts on the PEG display that don't happen when PCI memory is
> disabled.  Should that MSR bit give us the whole a_0000-b_ffff range?

Perhaps. It does that on some old graphics cards I've played with, but
frankly I have no idea what it does on our hardware.

I'm trying to find out though. If and when I get an answer I'll let you
know.

-- 
Ville Syrjälä
Intel OTC

Re: [Intel-gfx] [PATCH] i915: Update VGA arbiter support for newer devices

[Ville Syrjälä]

On Tue, Aug 20, 2013 at 10:46:45PM +0300, Ville Syrjälä wrote:
> On Fri, Aug 16, 2013 at 12:22:14PM -0600, Alex Williamson wrote:
> > On Fri, 2013-08-16 at 13:20 +0300, Ville Syrjälä wrote:
> > > On Thu, Aug 15, 2013 at 04:54:15PM -0600, Alex Williamson wrote:
> > > > On Fri, 2013-08-16 at 08:49 +1000, Dave Airlie wrote:
> > > > > On Fri, Aug 16, 2013 at 8:43 AM, Alex Williamson
> > > > > <alex.williamson@redhat.com> wrote:
> > > > > > This is intended to add VGA arbiter support for Intel HD graphics on
> > > > > > Core processors.  The old GMCH registers no longer exist, so even
> > > > > > though it appears that i915 participates in VGA arbitration, it doesn't
> > > > > > work.  On Intel HD graphics we already attempt to disable VGA regions
> > > > > > of the device.  This makes registering as a VGA client unnecessary since
> > > > > > we don't intend to operate differently depending on how many VGA devices
> > > > > > are present.  We can disable VGA memory regions by clearing a memory
> > > > > > enable bit in the VGA MSR.  That only leaves VGA IO, which we update
> > > > > > the VGA arbiter to know that we don't participate in VGA memory
> > > > > > arbitration.  We also add a hook on unload to re-enable memory and
> > > > > > reinstate VGA memory arbitration.
> > > > > 
> > > > > I would think there is still a VGA disable bit on the Intel device
> > > > > somewhere, we'd just need
> > > > > Intel to look in the docs and find it. A bit that can nuke both i/o
> > > > > and cmd regs.
> > > > 
> > > > The only bit available is in the GGC and is a keyed/locked register that
> > > > not only disables VGA memory and I/O, but also modifies the class code
> > > > of the device.  Early Core processors didn't lock this, but it's
> > > > untouchable in newer ones AFAICT.  Thanks,
> > > 
> > > I've not found anything else in the docs. And also we _need_ VGA I/O
> > > access to make i915_disable_vga() work. It's not 100% clear whether
> > > we really need to poke at the sequencer register in modern hardware,
> > > but the docs do still list it as a mandatory step. So even if we were
> > > to have a global "disable VGA I/O and mem bit" we'd need to make sure
> > > we already disabled VGA eg. after resume when the BIOS had a chance to
> > > turn the VGA display back on. I think there were also some BIOSen that
> > > turned VGA display back on when closing/opening the laptop lid. Not
> > > sure what would even happen with those if totally disabled VGA I/O
> > > access. I'm not sure they actually frob with the VGA regs though.
> > > Could be they just turn on the VGA display bit in the VGA_CONTROL
> > > register.
> > 
> > Hmm, it appears the MSR write isn't fully disabling VGA memory space.
> > When the VBIOS for the PEG graphics is run in the guest, I get some
> > corruption of the IGD frame buffer.  If I manually disable PCI memory in
> > the command register, this doesn't happen.  I also get some strange
> > artifacts on the PEG display that don't happen when PCI memory is
> > disabled.  Should that MSR bit give us the whole a_0000-b_ffff range?
> 
> Perhaps. It does that on some old graphics cards I've played with, but
> frankly I have no idea what it does on our hardware.
> 
> I'm trying to find out though. If and when I get an answer I'll let you
> know.

So the answer I basically got is that MSR is the only option here when
the GMCH register can't be used. Supposedly it should work too, but
I felt that I didn't get a 100% definite answer on that subject.

I tried it a bit on an IVB machine and PCI and PCIe Matrox G550 cards,
and for me it does seem to work. In the G550 PCIe case there's an extra
PCIe-PCI bridge on the card, and and in the G550 PCI case there's a
PCI-PCI bridge on the card and a PCIe-PCI bridge on the motherboard.
I don't have any native PCIe graphics cards on me to test the no
extra bridges case.

Based on a bit of manual register/memory banging it looks like the IGD
will decode the access when MSR[1]=1, and won't when MSR[1]=0. Same was
true for PCI_COMMAND[0] in case of VGA I/O. If those bits are disabled
for IGD, the accesses get to the external card. If neither claims it,
I just get 0xff back and writes are ignored.

Curiously I didn't see any problems when I configured both graphics
devices and bridges to decode/forward VGA resources. The IGD was
always the one to answer and the data didn't seem corrupted. Not sure
why that is. Maybe I just got lucky or something.

My tests weren't very thorough however, so I may have missed something.

-- 
Ville Syrjälä
Intel OTC

Re: [Intel-gfx] [PATCH] i915: Update VGA arbiter support for newer devices

[Alex Williamson]

On Fri, 2013-08-23 at 21:21 +0300, Ville Syrjälä wrote:
> On Tue, Aug 20, 2013 at 10:46:45PM +0300, Ville Syrjälä wrote:
> > On Fri, Aug 16, 2013 at 12:22:14PM -0600, Alex Williamson wrote:
> > > On Fri, 2013-08-16 at 13:20 +0300, Ville Syrjälä wrote:
> > > > On Thu, Aug 15, 2013 at 04:54:15PM -0600, Alex Williamson wrote:
> > > > > On Fri, 2013-08-16 at 08:49 +1000, Dave Airlie wrote:
> > > > > > On Fri, Aug 16, 2013 at 8:43 AM, Alex Williamson
> > > > > > <alex.williamson@redhat.com> wrote:
> > > > > > > This is intended to add VGA arbiter support for Intel HD graphics on
> > > > > > > Core processors.  The old GMCH registers no longer exist, so even
> > > > > > > though it appears that i915 participates in VGA arbitration, it doesn't
> > > > > > > work.  On Intel HD graphics we already attempt to disable VGA regions
> > > > > > > of the device.  This makes registering as a VGA client unnecessary since
> > > > > > > we don't intend to operate differently depending on how many VGA devices
> > > > > > > are present.  We can disable VGA memory regions by clearing a memory
> > > > > > > enable bit in the VGA MSR.  That only leaves VGA IO, which we update
> > > > > > > the VGA arbiter to know that we don't participate in VGA memory
> > > > > > > arbitration.  We also add a hook on unload to re-enable memory and
> > > > > > > reinstate VGA memory arbitration.
> > > > > > 
> > > > > > I would think there is still a VGA disable bit on the Intel device
> > > > > > somewhere, we'd just need
> > > > > > Intel to look in the docs and find it. A bit that can nuke both i/o
> > > > > > and cmd regs.
> > > > > 
> > > > > The only bit available is in the GGC and is a keyed/locked register that
> > > > > not only disables VGA memory and I/O, but also modifies the class code
> > > > > of the device.  Early Core processors didn't lock this, but it's
> > > > > untouchable in newer ones AFAICT.  Thanks,
> > > > 
> > > > I've not found anything else in the docs. And also we _need_ VGA I/O
> > > > access to make i915_disable_vga() work. It's not 100% clear whether
> > > > we really need to poke at the sequencer register in modern hardware,
> > > > but the docs do still list it as a mandatory step. So even if we were
> > > > to have a global "disable VGA I/O and mem bit" we'd need to make sure
> > > > we already disabled VGA eg. after resume when the BIOS had a chance to
> > > > turn the VGA display back on. I think there were also some BIOSen that
> > > > turned VGA display back on when closing/opening the laptop lid. Not
> > > > sure what would even happen with those if totally disabled VGA I/O
> > > > access. I'm not sure they actually frob with the VGA regs though.
> > > > Could be they just turn on the VGA display bit in the VGA_CONTROL
> > > > register.
> > > 
> > > Hmm, it appears the MSR write isn't fully disabling VGA memory space.
> > > When the VBIOS for the PEG graphics is run in the guest, I get some
> > > corruption of the IGD frame buffer.  If I manually disable PCI memory in
> > > the command register, this doesn't happen.  I also get some strange
> > > artifacts on the PEG display that don't happen when PCI memory is
> > > disabled.  Should that MSR bit give us the whole a_0000-b_ffff range?
> > 
> > Perhaps. It does that on some old graphics cards I've played with, but
> > frankly I have no idea what it does on our hardware.
> > 
> > I'm trying to find out though. If and when I get an answer I'll let you
> > know.
> 
> So the answer I basically got is that MSR is the only option here when
> the GMCH register can't be used. Supposedly it should work too, but
> I felt that I didn't get a 100% definite answer on that subject.

I can imagine that the GMCH could be modified if we knew where the bit
was that's locking it.  I can't find that in the spec though and I
assume that's intentional.

> I tried it a bit on an IVB machine and PCI and PCIe Matrox G550 cards,
> and for me it does seem to work. In the G550 PCIe case there's an extra
> PCIe-PCI bridge on the card, and and in the G550 PCI case there's a
> PCI-PCI bridge on the card and a PCIe-PCI bridge on the motherboard.
> I don't have any native PCIe graphics cards on me to test the no
> extra bridges case.
> 
> Based on a bit of manual register/memory banging it looks like the IGD
> will decode the access when MSR[1]=1, and won't when MSR[1]=0. Same was
> true for PCI_COMMAND[0] in case of VGA I/O. If those bits are disabled
> for IGD, the accesses get to the external card. If neither claims it,
> I just get 0xff back and writes are ignored.
> 
> Curiously I didn't see any problems when I configured both graphics
> devices and bridges to decode/forward VGA resources. The IGD was
> always the one to answer and the data didn't seem corrupted. Not sure
> why that is. Maybe I just got lucky or something.
> 
> My tests weren't very thorough however, so I may have missed something.

Thanks for checking Ville.  I wrote a test program myself to blast VGA
space through /dev/mem.  I agree, it sure seems like the MSR bit is
doing it's job.  That makes me suspect that I'm not actually getting the
bit cleared or that it's being re-enabled somewhere else.  I'll do some
more digging to make sure the MSR bit is actually cleared on IGD before
I start the VM.  Thanks!

Alex

Re: [Intel-gfx] [PATCH] i915: Update VGA arbiter support for newer devices

[Alex Williamson]

On Fri, 2013-08-23 at 15:18 -0600, Alex Williamson wrote:
> On Fri, 2013-08-23 at 21:21 +0300, Ville Syrjälä wrote:
> > On Tue, Aug 20, 2013 at 10:46:45PM +0300, Ville Syrjälä wrote:
> > > On Fri, Aug 16, 2013 at 12:22:14PM -0600, Alex Williamson wrote:
> > > > On Fri, 2013-08-16 at 13:20 +0300, Ville Syrjälä wrote:
> > > > > On Thu, Aug 15, 2013 at 04:54:15PM -0600, Alex Williamson wrote:
> > > > > > On Fri, 2013-08-16 at 08:49 +1000, Dave Airlie wrote:
> > > > > > > On Fri, Aug 16, 2013 at 8:43 AM, Alex Williamson
> > > > > > > <alex.williamson@redhat.com> wrote:
> > > > > > > > This is intended to add VGA arbiter support for Intel HD graphics on
> > > > > > > > Core processors.  The old GMCH registers no longer exist, so even
> > > > > > > > though it appears that i915 participates in VGA arbitration, it doesn't
> > > > > > > > work.  On Intel HD graphics we already attempt to disable VGA regions
> > > > > > > > of the device.  This makes registering as a VGA client unnecessary since
> > > > > > > > we don't intend to operate differently depending on how many VGA devices
> > > > > > > > are present.  We can disable VGA memory regions by clearing a memory
> > > > > > > > enable bit in the VGA MSR.  That only leaves VGA IO, which we update
> > > > > > > > the VGA arbiter to know that we don't participate in VGA memory
> > > > > > > > arbitration.  We also add a hook on unload to re-enable memory and
> > > > > > > > reinstate VGA memory arbitration.
> > > > > > > 
> > > > > > > I would think there is still a VGA disable bit on the Intel device
> > > > > > > somewhere, we'd just need
> > > > > > > Intel to look in the docs and find it. A bit that can nuke both i/o
> > > > > > > and cmd regs.
> > > > > > 
> > > > > > The only bit available is in the GGC and is a keyed/locked register that
> > > > > > not only disables VGA memory and I/O, but also modifies the class code
> > > > > > of the device.  Early Core processors didn't lock this, but it's
> > > > > > untouchable in newer ones AFAICT.  Thanks,
> > > > > 
> > > > > I've not found anything else in the docs. And also we _need_ VGA I/O
> > > > > access to make i915_disable_vga() work. It's not 100% clear whether
> > > > > we really need to poke at the sequencer register in modern hardware,
> > > > > but the docs do still list it as a mandatory step. So even if we were
> > > > > to have a global "disable VGA I/O and mem bit" we'd need to make sure
> > > > > we already disabled VGA eg. after resume when the BIOS had a chance to
> > > > > turn the VGA display back on. I think there were also some BIOSen that
> > > > > turned VGA display back on when closing/opening the laptop lid. Not
> > > > > sure what would even happen with those if totally disabled VGA I/O
> > > > > access. I'm not sure they actually frob with the VGA regs though.
> > > > > Could be they just turn on the VGA display bit in the VGA_CONTROL
> > > > > register.
> > > > 
> > > > Hmm, it appears the MSR write isn't fully disabling VGA memory space.
> > > > When the VBIOS for the PEG graphics is run in the guest, I get some
> > > > corruption of the IGD frame buffer.  If I manually disable PCI memory in
> > > > the command register, this doesn't happen.  I also get some strange
> > > > artifacts on the PEG display that don't happen when PCI memory is
> > > > disabled.  Should that MSR bit give us the whole a_0000-b_ffff range?
> > > 
> > > Perhaps. It does that on some old graphics cards I've played with, but
> > > frankly I have no idea what it does on our hardware.
> > > 
> > > I'm trying to find out though. If and when I get an answer I'll let you
> > > know.
> > 
> > So the answer I basically got is that MSR is the only option here when
> > the GMCH register can't be used. Supposedly it should work too, but
> > I felt that I didn't get a 100% definite answer on that subject.
> 
> I can imagine that the GMCH could be modified if we knew where the bit
> was that's locking it.  I can't find that in the spec though and I
> assume that's intentional.
> 
> > I tried it a bit on an IVB machine and PCI and PCIe Matrox G550 cards,
> > and for me it does seem to work. In the G550 PCIe case there's an extra
> > PCIe-PCI bridge on the card, and and in the G550 PCI case there's a
> > PCI-PCI bridge on the card and a PCIe-PCI bridge on the motherboard.
> > I don't have any native PCIe graphics cards on me to test the no
> > extra bridges case.
> > 
> > Based on a bit of manual register/memory banging it looks like the IGD
> > will decode the access when MSR[1]=1, and won't when MSR[1]=0. Same was
> > true for PCI_COMMAND[0] in case of VGA I/O. If those bits are disabled
> > for IGD, the accesses get to the external card. If neither claims it,
> > I just get 0xff back and writes are ignored.
> > 
> > Curiously I didn't see any problems when I configured both graphics
> > devices and bridges to decode/forward VGA resources. The IGD was
> > always the one to answer and the data didn't seem corrupted. Not sure
> > why that is. Maybe I just got lucky or something.
> > 
> > My tests weren't very thorough however, so I may have missed something.
> 
> Thanks for checking Ville.  I wrote a test program myself to blast VGA
> space through /dev/mem.  I agree, it sure seems like the MSR bit is
> doing it's job.  That makes me suspect that I'm not actually getting the
> bit cleared or that it's being re-enabled somewhere else.  I'll do some
> more digging to make sure the MSR bit is actually cleared on IGD before
> I start the VM.

Yep, something is broken there.  At the point where I try to read and
set the MSR, I'm reading 0x0.  The device has I/O port enabled in the
command register at that point, so I don't know what's wrong just yet.
By the time I'm booted to an fb login prompt it reads 0x67, so I'm
obviously being unsuccessful in clearing that bit.  Thanks,

Alex