Re: [PATCH 1/3] platform/x86: apple-gmux: add MacBookPro15, 1 dGPU power sequence

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `discrete_pdev` pointer caches the discrete GPU `pci_dev` but is not updated when the client is unregistered and a new device is registered, preventing power switching from functioning if the PCI device is rescanned.
--

--- Patch [1]: [PATCH 1/3] platform/x86: apple-gmux: add MacBookPro15,1 dGPU power sequence ---
commit 35744aea76b96c0341cb3a18f76cff6290e2d999
Author: Atharva Tiwari <[email protected]>

platform/x86: apple-gmux: add MacBookPro15,1 dGPU power sequence

This commit introduces a dGPU power sequence for MacBookPro15,1 devices.
It introduces ACPI method evaluation during gmux transitions and verifies
PCI configuration space accessibility before completing power-on.

> diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
> index fbc30f1f8abdf..1771c6bb756ef 100644
> --- a/drivers/platform/x86/apple-gmux.c
> +++ b/drivers/platform/x86/apple-gmux.c

[ ... ]

> @@ -549,11 +615,14 @@ static enum vga_switcheroo_client_id gmux_get_client_id(struct pci_dev *pdev)
>  	 */
>  	if (pdev->vendor == PCI_VENDOR_ID_INTEL)
>  		return VGA_SWITCHEROO_IGD;
> -	else if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
> -		 pdev->device == 0x0863)
> +	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA && pdev->device == 0x0863)
>  		return VGA_SWITCHEROO_IGD;
> -	else
> -		return VGA_SWITCHEROO_DIS;
> +
> +	if (apple_gmux_data->use_pwg_power_sequence &&
> +	    !apple_gmux_data->discrete_pdev)
> +		apple_gmux_data->discrete_pdev = pci_dev_get(pdev);

[Severity: Medium]
Does this caching mechanism properly handle the PCI device being removed and
rescanned?

If an administrator logically removes the discrete GPU via sysfs (for example,
echo 1 > /sys/bus/pci/devices/.../remove) and rescans the PCI bus, the GPU
driver will re-register with vga_switcheroo.

Because of the !apple_gmux_data->discrete_pdev check, discrete_pdev won't be
updated when the new device is registered. The power switching could break
permanently by returning -ETIMEDOUT, because it will attempt to read the
configuration space on the stale, disconnected pci_dev.

Should there be a way to clear or update the cached discrete_pdev when a
client is unregistered?

> +
> +	return VGA_SWITCHEROO_DIS;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1