Re: [PATCH 05/19] drm/i915/vga: Don't touch VGA registers if VGA decode is fully disabled

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 08 Dec 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> On some systems the BIOS will disable the VGA decode logic in the
> iGPU (via GMCH_CTRL) when an external GPU is used as the primary
> VGA device. In that case the iGPU will never claim any VGA register
> accesses, and any access we do will in fact end up on the external
> GPU. Don't go poking around in the other GPUs registers.
>
> Note that (at least on the g4x board where I tested this) the BIOS
> forgets to set the VGACNTR VGA_DISP_DISABLE bit, and the reset
> value for said bit is 0. That apparently prevents the pipes from
> running, so we must still remember to set the bit, despite the VGA
> plane was never actually enabled. On more modern platforms (hsw+
> maybe?) the reset value for VGACNTR was changed to have
> VGA_DISP_DISABLE already set.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I'll take your word for it.

Acked-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_vga.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
> index 84fd5475d336..744812260ae3 100644
> --- a/drivers/gpu/drm/i915/display/intel_vga.c
> +++ b/drivers/gpu/drm/i915/display/intel_vga.c
> @@ -23,6 +23,18 @@ static unsigned int intel_gmch_ctrl_reg(struct intel_display *display)
>  	return DISPLAY_VER(display) >= 6 ? SNB_GMCH_CTRL : INTEL_GMCH_CTRL;
>  }
>  
> +static bool intel_vga_decode_is_enabled(struct intel_display *display)
> +{
> +	struct pci_dev *pdev = to_pci_dev(display->drm->dev);
> +	u16 gmch_ctrl = 0;
> +
> +	if (pci_bus_read_config_word(pdev->bus, PCI_DEVFN(0, 0),
> +				     intel_gmch_ctrl_reg(display), &gmch_ctrl))
> +		return false;
> +
> +	return !(gmch_ctrl & INTEL_GMCH_VGA_DISABLE);
> +}
> +
>  static i915_reg_t intel_vga_cntrl_reg(struct intel_display *display)
>  {
>  	if (display->platform.valleyview || display->platform.cherryview)
> @@ -55,6 +67,17 @@ void intel_vga_disable(struct intel_display *display)
>  	u8 msr, sr1;
>  	u32 tmp;
>  
> +	if (!intel_vga_decode_is_enabled(display)) {
> +		drm_dbg_kms(display->drm, "VGA decode is disabled\n");
> +
> +		/*
> +		 * On older hardware VGA_DISP_DISABLE defaults to 0, but
> +		 * it *must* be set or else the pipe will be completely
> +		 * stuck (at least on g4x).
> +		 */
> +		goto reset_vgacntr;
> +	}
> +
>  	tmp = intel_de_read(display, vga_reg);
>  	if (tmp & VGA_DISP_DISABLE)
>  		return;
> @@ -96,6 +119,7 @@ void intel_vga_disable(struct intel_display *display)
>  
>  	udelay(300);
>  
> +reset_vgacntr:
>  	intel_de_write(display, vga_reg, VGA_DISP_DISABLE);
>  	intel_de_posting_read(display, vga_reg);
>  }

-- 
Jani Nikula, Intel