All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] i915: Update VGA arbiter support for newer devices
@ 2013-08-28 15:39 Alex Williamson
  2013-08-30 12:43 ` Ville Syrjälä
  0 siblings, 1 reply; 3+ messages in thread
From: Alex Williamson @ 2013-08-28 15:39 UTC (permalink / raw)
  To: intel-gfx; +Cc: airlied, linux-kernel, ville.syrjala

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 the 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>
---

v3: Use explicit LEGACY_IO | LEGACY_MEM when restoring rather than
    LEGACY_MASK, per Ville's comments.

v2: I915_READ/WRITE accessors don't work in i915_disable_vga, use inb/outb
    directly.  Also, on the driver unbind VGA enable path, acquire legacy
    IO to re-enable VGA memory.  Correct comment.

As with v1, this depends on "vgaarb: Fixes for partial VGA opt-out".  With
all patches I'm able to assign a discrete PEG VGA device to a guest and
execute the VBIOS w/o interference from IGD or corruption of the IGD
framebuffer.

 drivers/gpu/drm/i915/i915_dma.c      |    9 ++++++---
 drivers/gpu/drm/i915/intel_display.c |   25 +++++++++++++++++++++++++
 2 files changed, 31 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..2807760 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9519,6 +9519,15 @@ 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)) {
+		outb(inb(VGA_MSR_READ) & ~VGA_MSR_MEM_EN, VGA_MSR_WRITE);
+		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 +9535,20 @@ static void i915_disable_vga(struct drm_device *dev)
 	POSTING_READ(vga_reg);
 }
 
+static void i915_enable_vga(struct drm_device *dev)
+{
+	/* Enable VGA memory on Intel HD */
+	if (HAS_PCH_SPLIT(dev)) {
+		vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO);
+		outb(inb(VGA_MSR_READ) | VGA_MSR_MEM_EN, VGA_MSR_WRITE);
+		vga_set_legacy_decoding(dev->pdev, VGA_RSRC_LEGACY_IO |
+						   VGA_RSRC_LEGACY_MEM |
+						   VGA_RSRC_NORMAL_IO |
+						   VGA_RSRC_NORMAL_MEM);
+		vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
+	}
+}
+
 void intel_modeset_init_hw(struct drm_device *dev)
 {
 	intel_init_power_well(dev);
@@ -9983,6 +10006,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);


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH v3] i915: Update VGA arbiter support for newer devices
  2013-08-28 15:39 [PATCH v3] i915: Update VGA arbiter support for newer devices Alex Williamson
@ 2013-08-30 12:43 ` Ville Syrjälä
  2013-09-02  6:21   ` [Intel-gfx] " Daniel Vetter
  0 siblings, 1 reply; 3+ messages in thread
From: Ville Syrjälä @ 2013-08-30 12:43 UTC (permalink / raw)
  To: Alex Williamson; +Cc: intel-gfx, airlied, linux-kernel

On Wed, Aug 28, 2013 at 09:39:08AM -0600, Alex Williamson 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 the 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>

Looking good.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
> 
> v3: Use explicit LEGACY_IO | LEGACY_MEM when restoring rather than
>     LEGACY_MASK, per Ville's comments.
> 
> v2: I915_READ/WRITE accessors don't work in i915_disable_vga, use inb/outb
>     directly.  Also, on the driver unbind VGA enable path, acquire legacy
>     IO to re-enable VGA memory.  Correct comment.
> 
> As with v1, this depends on "vgaarb: Fixes for partial VGA opt-out".  With
> all patches I'm able to assign a discrete PEG VGA device to a guest and
> execute the VBIOS w/o interference from IGD or corruption of the IGD
> framebuffer.
> 
>  drivers/gpu/drm/i915/i915_dma.c      |    9 ++++++---
>  drivers/gpu/drm/i915/intel_display.c |   25 +++++++++++++++++++++++++
>  2 files changed, 31 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..2807760 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9519,6 +9519,15 @@ 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)) {
> +		outb(inb(VGA_MSR_READ) & ~VGA_MSR_MEM_EN, VGA_MSR_WRITE);
> +		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 +9535,20 @@ static void i915_disable_vga(struct drm_device *dev)
>  	POSTING_READ(vga_reg);
>  }
>  
> +static void i915_enable_vga(struct drm_device *dev)
> +{
> +	/* Enable VGA memory on Intel HD */
> +	if (HAS_PCH_SPLIT(dev)) {
> +		vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO);
> +		outb(inb(VGA_MSR_READ) | VGA_MSR_MEM_EN, VGA_MSR_WRITE);
> +		vga_set_legacy_decoding(dev->pdev, VGA_RSRC_LEGACY_IO |
> +						   VGA_RSRC_LEGACY_MEM |
> +						   VGA_RSRC_NORMAL_IO |
> +						   VGA_RSRC_NORMAL_MEM);
> +		vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
> +	}
> +}
> +
>  void intel_modeset_init_hw(struct drm_device *dev)
>  {
>  	intel_init_power_well(dev);
> @@ -9983,6 +10006,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);

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [Intel-gfx] [PATCH v3] i915: Update VGA arbiter support for newer devices
  2013-08-30 12:43 ` Ville Syrjälä
@ 2013-09-02  6:21   ` Daniel Vetter
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2013-09-02  6:21 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Alex Williamson, airlied, intel-gfx, linux-kernel

On Fri, Aug 30, 2013 at 03:43:05PM +0300, Ville Syrjälä wrote:
> On Wed, Aug 28, 2013 at 09:39:08AM -0600, Alex Williamson 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 the 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>
> 
> Looking good.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the patch.
> 
> > ---
> > 
> > v3: Use explicit LEGACY_IO | LEGACY_MEM when restoring rather than
> >     LEGACY_MASK, per Ville's comments.
> > 
> > v2: I915_READ/WRITE accessors don't work in i915_disable_vga, use inb/outb
> >     directly.  Also, on the driver unbind VGA enable path, acquire legacy
> >     IO to re-enable VGA memory.  Correct comment.

I've added the patch changelog here to the commmit message - I kinda
prefer to keep this bit of information around ...
-Daniel

> > 
> > As with v1, this depends on "vgaarb: Fixes for partial VGA opt-out".  With
> > all patches I'm able to assign a discrete PEG VGA device to a guest and
> > execute the VBIOS w/o interference from IGD or corruption of the IGD
> > framebuffer.
> > 
> >  drivers/gpu/drm/i915/i915_dma.c      |    9 ++++++---
> >  drivers/gpu/drm/i915/intel_display.c |   25 +++++++++++++++++++++++++
> >  2 files changed, 31 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..2807760 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9519,6 +9519,15 @@ 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)) {
> > +		outb(inb(VGA_MSR_READ) & ~VGA_MSR_MEM_EN, VGA_MSR_WRITE);
> > +		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 +9535,20 @@ static void i915_disable_vga(struct drm_device *dev)
> >  	POSTING_READ(vga_reg);
> >  }
> >  
> > +static void i915_enable_vga(struct drm_device *dev)
> > +{
> > +	/* Enable VGA memory on Intel HD */
> > +	if (HAS_PCH_SPLIT(dev)) {
> > +		vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO);
> > +		outb(inb(VGA_MSR_READ) | VGA_MSR_MEM_EN, VGA_MSR_WRITE);
> > +		vga_set_legacy_decoding(dev->pdev, VGA_RSRC_LEGACY_IO |
> > +						   VGA_RSRC_LEGACY_MEM |
> > +						   VGA_RSRC_NORMAL_IO |
> > +						   VGA_RSRC_NORMAL_MEM);
> > +		vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
> > +	}
> > +}
> > +
> >  void intel_modeset_init_hw(struct drm_device *dev)
> >  {
> >  	intel_init_power_well(dev);
> > @@ -9983,6 +10006,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);
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-09-02  6:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-28 15:39 [PATCH v3] i915: Update VGA arbiter support for newer devices Alex Williamson
2013-08-30 12:43 ` Ville Syrjälä
2013-09-02  6:21   ` [Intel-gfx] " Daniel Vetter

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.