Re: [PATCH v3 1/1] drm/i915: handle failure from vga_get_uninterruptible()

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

On Sat, Mar 07, 2026 at 12:13:16AM +0900, Simon Richter wrote:
> The vga_get_uninterruptible() function can return an error if it fails to
> set up VGA decoding for the requested device.
> 
> If VGA decoding is unavailable, we don't need to be careful to leave the
> VGA emulation in a usable state, as vgacon will also be unable to get
> access later on, so just skip over the VGA accesses and the vga_put() call
> matching the failed vga_get_uninterruptible().
> 
> Signed-off-by: Simon Richter <Simon.Richter@hogyros.de>
> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1824

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

Do you need to take this in via some other tree, or should I just push
it to drm-intel-next?

> ---
>  drivers/gpu/drm/i915/display/intel_vga.c | 33 +++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
> index 6fc3e3702cb8..9832a4ade318 100644
> --- a/drivers/gpu/drm/i915/display/intel_vga.c
> +++ b/drivers/gpu/drm/i915/display/intel_vga.c
> @@ -112,12 +112,16 @@ static bool intel_pci_bridge_set_vga(struct pci_dev *pdev, bool enable)
>  	return old & PCI_BRIDGE_CTL_VGA;
>  }
>  
> -static bool intel_vga_get(struct intel_display *display, bool mmio)
> +static int intel_vga_get(struct intel_display *display, bool mmio,
> +			 bool *old_io_decode)
>  {
>  	struct pci_dev *pdev = to_pci_dev(display->drm->dev);
> +	int err;
>  
> -	if (mmio)
> -		return false;
> +	if (mmio) {
> +		*old_io_decode = false;
> +		return 0;
> +	}
>  
>  	/*
>  	 * Bypass the VGA arbiter on the iGPU and just enable
> @@ -130,10 +134,15 @@ static bool intel_vga_get(struct intel_display *display, bool mmio)
>  	 * grab any VGA IO access when IO decode is enabled, regardless
>  	 * of how any other VGA routing bits are configured.
>  	 */
> -	if (display->platform.dgfx)
> -		vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
> +	if (display->platform.dgfx) {
> +		err = vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
> +		if (err)
> +			return err;
> +	}
> +
> +	*old_io_decode = intel_pci_set_io_decode(pdev, true);
>  
> -	return intel_pci_set_io_decode(pdev, true);
> +	return 0;
>  }
>  
>  static void intel_vga_put(struct intel_display *display, bool io_decode, bool mmio)
> @@ -175,6 +184,7 @@ void intel_vga_disable(struct intel_display *display)
>  	bool io_decode;
>  	u8 msr, sr1;
>  	u32 tmp;
> +	int err;
>  
>  	if (!intel_vga_decode_is_enabled(display)) {
>  		drm_dbg_kms(display->drm, "VGA decode is disabled\n");
> @@ -216,7 +226,16 @@ void intel_vga_disable(struct intel_display *display)
>  			goto reset_vgacntr;
>  	}
>  
> -	io_decode = intel_vga_get(display, mmio);
> +	/*
> +	 * This should not fail, because the vga_get() family of functions
> +	 * will only report errors for dGPUs that are unreachable via the
> +	 * bridge, and cannot be made reachable either. We shouldn't even
> +	 * get here for this case, but if we do, we assume that the bridge
> +	 * will also refuse future requests to forward VGA accesses.
> +	 */
> +	err = intel_vga_get(display, mmio, &io_decode);
> +	if (err)
> +		goto reset_vgacntr;
>  
>  	drm_WARN_ON(display->drm, !mmio && !intel_pci_has_vga_io_decode(pdev));
>  
> -- 
> 2.47.3

-- 
Ville Syrjälä
Intel