Re: [PATCH v2] drm/amd/amdgpu: add firmware file fallback for APU VBIOS discovery

[Mario Limonciello <mario.limonciello@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 5984 bytes --]



On 7/5/26 14:10, Oz Tiram wrote:
> Hi Mario,
> 
>    To make sure I understand correctly: are you suggesting that the bus
>    number in the VFCT was legitimate at BIOS POST time, and that
>    pci=realloc,assign-busses is what changes it at runtime, causing the
>    mismatch?

That's what it sounds like right now.  You can easily drop all the 
superfluous kernel command line optiosn and see.

> 
>    I'm not familiar enough with the PCI subsystem to know the right way to
>    implement that — could you point me in the right direction?

Well there's a variety of ways to do it.  But how about we start here - 
if we make that specific busnr match optional and instead make a VID/DID 
match.

See if the attached patch helps.

> 
>    Oz
> 
> On 7/5/26 20:37, Mario Limonciello wrote:
>>
>>
>> On 7/5/26 05:04, Oz Tiram wrote:
>>> APUs (e.g. AMD Radeon 780M / HawkPoint, PCI 1002:1900) have no
>>> dedicated VBIOS ROM chip.  amdgpu_get_bios_apu() attempts four paths
>>> before giving up:
>>>
>>>    1. ACPI VFCT table
>>>    2. VRAM BAR read
>>>    3. ROM BAR read
>>>    4. platform BIOS
>>>
>>> On some systems all four fail.  The specific case motivating this patch
>>> is a hybrid graphics machine (dGPU + APU) where:
>>>
>>>    - The VFCT table contains the iGPU entry but with a stale PCIBus 
>>> value
>>>      from BIOS POST time (0x6A).  When the kernel boots with
>>>      pci=realloc,assign-busses, PCI bus numbers are reassigned 
>>> dynamically
>>>      and the iGPU lands on bus 0x0B at runtime. amdgpu_acpi_vfct_bios()
>>>      matches entries by bus number, so the entry is never found.
>>>    - The VRAM BAR is unmapped at probe time.
>>>    - The ROM BAR is zero (PCI firmware did not assign it).
>>>    - No platform BIOS mapping exists.
>>>
>>> The UEFI GOP driver initialises the iGPU successfully for early display,
>>> confirming the hardware is functional.  The VBIOS image data embedded in
>>> the VFCT is also valid; only the PCIBus metadata is wrong.
>>
>> So the BIOS on this machine is actually totally fine; it's just when 
>> the kernel is booted to reassign busses there is a problem?
>>
>> In that case; why not detect the kernel was booted this way and keep 
>> track of the original bus number when reassigned to avoid the issue?
>>
>>> The firmware
>>> file can be extracted directly from the VFCT using dd:
>>>
>>>    dd if=/sys/firmware/acpi/tables/VFCT bs=1 skip=$((0x68)) 
>>> count=16896 \
>>>       of=/lib/firmware/amdgpu/1002_1900.bin
>>>
>>> (0x68 is the byte offset of the VBIOS image after the ACPI table header
>>> and VFCT_IMAGE_HEADER; the image length 16896 comes from the ImageLength
>>> field in VFCT_IMAGE_HEADER.)
>>>
>>> The driver then prints "Unable to locate a BIOS ROM" and refuses to
>>> bind, leaving the APU completely unusable under Linux.
>>>
>>> Add a fifth fallback: request a firmware file named
>>> "amdgpu/<vendor>_<device>.bin" (e.g. "amdgpu/1002_1900.bin") via
>>> request_firmware().  This allows a VBIOS image extracted as above to be
>>> placed in /lib/firmware/ and makes the binding succeed without patching
>>> ACPI tables or BIOS.
>>>
>>> The fallback is only reached if all existing paths have already failed,
>>> so there is no regression risk for boards where VFCT or ROM BAR work.
>>
>> What happens if the VBIOS changes in another way one boot to another? 
>> You might have some other stateful information that isn't updated.
>>
>> The whole thing to me feels like a hack for a behavior we can control 
>> in the kernel when doing reassignments.
>>>
>>> Signed-off-by: Oz Tiram <oz@shift-computing.de>
>>> ---
>>> v2: Fix commit message: clarify that VFCT contains the iGPU entry but
>>>      with a stale PCIBus from BIOS POST that mismatches the runtime bus
>>>      number assigned by pci=realloc,assign-busses.  Explain that the 
>>> VBIOS
>>>      image data is valid and document the dd extraction command and byte
>>>      offsets.  Note that the UEFI GOP driver initialises the iGPU
>>>      successfully, confirming the hardware is functional.
>>>
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 23 +++++++++++++++++++++++
>>>   1 file changed, 23 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/ 
>>> drm/amd/amdgpu/amdgpu_bios.c
>>> index aa039e148a5e..86064c753b09 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
>>> @@ -26,6 +26,7 @@
>>>    *          Jerome Glisse
>>>    */
>>>   +#include <linux/firmware.h>
>>>   #include "amdgpu.h"
>>>   #include "atom.h"
>>>   @@ -457,6 +458,28 @@ static bool amdgpu_get_bios_apu(struct 
>>> amdgpu_device *adev)
>>>           goto success;
>>>       }
>>>   +    {
>>> +        const struct firmware *fw;
>>> +        char fw_name[32];
>>> +        size_t fw_size;
>>> +
>>> +        snprintf(fw_name, sizeof(fw_name), "amdgpu/%04x_%04x.bin",
>>> +             adev->pdev->vendor, adev->pdev->device);
>>> +        if (request_firmware(&fw, fw_name, adev->dev) == 0) {
>>> +            adev->bios = kmemdup(fw->data, fw->size, GFP_KERNEL);
>>> +            fw_size = fw->size;
>>> +            release_firmware(fw);
>>> +            if (!adev->bios || !check_atom_bios(adev, fw_size)) {
>>> +                amdgpu_bios_release(adev);
>>> +            } else {
>>> +                adev->bios_size = fw_size;
>>> +                dev_info(adev->dev, "Fetched VBIOS from firmware 
>>> file %s\n",
>>> +                     fw_name);
>>> +                goto success;
>>> +            }
>>> +        }
>>> +    }
>>> +
>>>       dev_err(adev->dev, "Unable to locate a BIOS ROM\n");
>>>       return false;
>>

[-- Attachment #2: 0001-drm-amdgpu-radeon-Fix-VFCT-bus-number-matching-with-.patch --]
[-- Type: text/x-patch, Size: 8107 bytes --]

From 42ac2fe3085cc3593523ac3e514bc36616078b4f Mon Sep 17 00:00:00 2001
From: Mario Limonciello <mario.limonciello@amd.com>
Date: Sun, 5 Jul 2026 15:48:05 -0500
Subject: [PATCH] drm/amdgpu/radeon: Fix VFCT bus number matching with soft
 filter

On systems where PCI bus renumbering occurs (e.g. pci=realloc,
resource conflicts), the runtime bus number may differ from the
BIOS POST bus number recorded in the VFCT table. This causes
amdgpu_acpi_vfct_bios() to fail finding the VBIOS even though
the correct device entry exists.

Introduce amdgpu_acpi_vfct_match() which treats the bus number
as a soft filter: vendor/device/function identity is the hard
requirement, while exact bus match is the preferred path. When
bus numbers disagree but device identity matches, accept the
VFCT entry and log a dev_notice for diagnostics.

Apply the same fix to radeon_acpi_vfct_bios().

While updating radeon, also modernize the error handling:
- Replace DRM_ERROR with dev_info/dev_notice
- Remove legacy goto out + acpi_put_table pattern (acpi_put_table
  was removed from the kernel)
- Use early return instead of goto for cleaner control flow

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 48 +++++++++++++--
 drivers/gpu/drm/radeon/radeon_bios.c     | 78 +++++++++++++++++-------
 2 files changed, 99 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
index 35d04e69aec0..fa730af5e979 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
@@ -370,6 +370,46 @@ static bool amdgpu_read_disabled_bios(struct amdgpu_device *adev)
 		false : amdgpu_asic_read_disabled_bios(adev);
 }
 
+/**
+ * amdgpu_acpi_vfct_match() - Check if a VFCT entry matches the device
+ * @adev: AMDGPU device
+ * @vhdr: VFCT image header to check
+ *
+ * VFCT entries contain the PCI bus number as recorded during BIOS POST.
+ * On systems where the kernel renumbers PCI buses (e.g. pci=realloc or
+ * resource conflicts), the runtime bus number may differ from the POST
+ * value.  Match by device identity (vendor + device + function) and use
+ * the bus number as a preference: exact bus match is preferred, but when
+ * the bus numbers disagree we accept the entry if the device identity
+ * matches.
+ *
+ * Returns: 0 on match, -ENODEV on no match
+ */
+static int amdgpu_acpi_vfct_match(struct amdgpu_device *adev,
+				  VFCT_IMAGE_HEADER *vhdr)
+{
+	/* Vendor and device IDs must always match */
+	if (vhdr->VendorID != adev->pdev->vendor ||
+	    vhdr->DeviceID != adev->pdev->device)
+		return -ENODEV;
+
+	if (vhdr->PCIDevice != PCI_SLOT(adev->pdev->devfn) ||
+	    vhdr->PCIFunction != PCI_FUNC(adev->pdev->devfn))
+		return -ENODEV;
+
+	/* Exact bus number match - preferred */
+	if (vhdr->PCIBus == adev->pdev->bus->number)
+		return 0;
+
+	/* Bus mismatch but device identity matches (PCI renumbering case) */
+	dev_notice(adev->dev,
+		   "VFCT bus number mismatch: table %u != runtime %u, "
+		   "matching by device identity (vendor 0x%04x device 0x%04x)\\n",
+		   vhdr->PCIBus, adev->pdev->bus->number,
+		   adev->pdev->vendor, adev->pdev->device);
+	return 0;
+}
+
 #ifdef CONFIG_ACPI
 static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
 {
@@ -406,11 +446,7 @@ static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
 		}
 
 		if (vhdr->ImageLength &&
-		    vhdr->PCIBus == adev->pdev->bus->number &&
-		    vhdr->PCIDevice == PCI_SLOT(adev->pdev->devfn) &&
-		    vhdr->PCIFunction == PCI_FUNC(adev->pdev->devfn) &&
-		    vhdr->VendorID == adev->pdev->vendor &&
-		    vhdr->DeviceID == adev->pdev->device) {
+		    !amdgpu_acpi_vfct_match(adev, vhdr)) {
 			adev->bios = kmemdup(&vbios->VbiosContent,
 					     vhdr->ImageLength,
 					     GFP_KERNEL);
@@ -424,7 +460,7 @@ static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
 		}
 	}
 
-	dev_info(adev->dev, "ACPI VFCT table present but broken (too short #2),skipping\n");
+	dev_info(adev->dev, "ACPI VFCT table present but broken (too short #2),skipping\\n");
 	return false;
 }
 #else
diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
index 3a8c5199a0fe..5938a8ed6e73 100644
--- a/drivers/gpu/drm/radeon/radeon_bios.c
+++ b/drivers/gpu/drm/radeon/radeon_bios.c
@@ -596,21 +596,60 @@ static bool radeon_read_disabled_bios(struct radeon_device *rdev)
 		return legacy_read_disabled_bios(rdev);
 }
 
+/**
+ * radeon_acpi_vfct_match() - Check if a VFCT entry matches the device
+ * @rdev: Radeon device
+ * @vhdr: VFCT image header to check
+ *
+ * VFCT entries contain the PCI bus number as recorded during BIOS POST.
+ * On systems where the kernel renumbers PCI buses (e.g. pci=realloc or
+ * resource conflicts), the runtime bus number may differ from the POST
+ * value.  Match by device identity (vendor + device + function) and use
+ * the bus number as a preference: exact bus match is preferred, but when
+ * the bus numbers disagree we accept the entry if the device identity
+ * matches.
+ *
+ * Returns: 0 on match, -ENODEV on no match
+ */
+static int radeon_acpi_vfct_match(struct radeon_device *rdev,
+				  VFCT_IMAGE_HEADER *vhdr)
+{
+	/* Vendor and device IDs must always match */
+	if (vhdr->VendorID != rdev->pdev->vendor ||
+	    vhdr->DeviceID != rdev->pdev->device)
+		return -ENODEV;
+
+	if (vhdr->PCIDevice != PCI_SLOT(rdev->pdev->devfn) ||
+	    vhdr->PCIFunction != PCI_FUNC(rdev->pdev->devfn))
+		return -ENODEV;
+
+	/* Exact bus number match - preferred */
+	if (vhdr->PCIBus == rdev->pdev->bus->number)
+		return 0;
+
+	/* Bus mismatch but device identity matches (PCI renumbering case) */
+	dev_notice(&rdev->pdev->dev,
+		   "VFCT bus number mismatch: table %u != runtime %u, "
+		   "matching by device identity (vendor 0x%04x device 0x%04x)\n",
+		   vhdr->PCIBus, rdev->pdev->bus->number,
+		   rdev->pdev->vendor, rdev->pdev->device);
+	return 0;
+}
+
 #ifdef CONFIG_ACPI
 static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
 {
 	struct acpi_table_header *hdr;
 	acpi_size tbl_size;
 	UEFI_ACPI_VFCT *vfct;
-	unsigned offset;
-	bool r = false;
+	unsigned int offset;
 
 	if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
 		return false;
 	tbl_size = hdr->length;
 	if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
-		DRM_ERROR("ACPI VFCT table present but broken (too short #1)\n");
-		goto out;
+		dev_info(&rdev->pdev->dev, "ACPI VFCT table present but broken (too short #1),skipping\n");
+		return false;
 	}
 
 	vfct = (UEFI_ACPI_VFCT *)hdr;
@@ -622,37 +661,34 @@ static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
 
 		offset += sizeof(VFCT_IMAGE_HEADER);
 		if (offset > tbl_size) {
-			DRM_ERROR("ACPI VFCT image header truncated\n");
-			goto out;
+			dev_info(&rdev->pdev->dev, "ACPI VFCT image header truncated,skipping\n");
+			return false;
 		}
 
 		offset += vhdr->ImageLength;
 		if (offset > tbl_size) {
-			DRM_ERROR("ACPI VFCT image truncated\n");
-			goto out;
+			dev_info(&rdev->pdev->dev, "ACPI VFCT image truncated,skipping\n");
+			return false;
 		}
 
 		if (vhdr->ImageLength &&
-		    vhdr->PCIBus == rdev->pdev->bus->number &&
-		    vhdr->PCIDevice == PCI_SLOT(rdev->pdev->devfn) &&
-		    vhdr->PCIFunction == PCI_FUNC(rdev->pdev->devfn) &&
-		    vhdr->VendorID == rdev->pdev->vendor &&
-		    vhdr->DeviceID == rdev->pdev->device) {
+		    !radeon_acpi_vfct_match(rdev, vhdr)) {
 			rdev->bios = kmemdup(&vbios->VbiosContent,
 					     vhdr->ImageLength,
 					     GFP_KERNEL);
-			if (rdev->bios)
-				r = true;
 
-			goto out;
+			if (!rdev->bios ||
+			    rdev->bios[0] != 0x55 || rdev->bios[1] != 0xaa) {
+				kfree(rdev->bios);
+				rdev->bios = NULL;
+				return false;
+			}
+			return true;
 		}
 	}
 
-	DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");
-
-out:
-	acpi_put_table(hdr);
-	return r;
+	dev_info(&rdev->pdev->dev, "ACPI VFCT table present but broken (too short #2),skipping\n");
+	return false;
 }
 #else
 static inline bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
-- 
2.53.0