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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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