[PATCH 1/2] drm/radeon: implement ACPI VFCT vbios fetch

[PATCH 1/2] drm/radeon: implement ACPI VFCT vbios fetch

[alexdeucher]

From: David L <equinox-freedesktopbugs@diac24.net>

This is required for pure UEFI systems.  The vbios is stored
in ACPI rather than at the legacy vga location.

Fixes:
https://bugs.freedesktop.org/show_bug.cgi?id=26891

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_bios.c |   59 ++++++++++++++++++++++++++++++++++
 1 files changed, 59 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
index 501f488..f06494a7 100644
--- a/drivers/gpu/drm/radeon/radeon_bios.c
+++ b/drivers/gpu/drm/radeon/radeon_bios.c
@@ -32,6 +32,9 @@
 
 #include <linux/vga_switcheroo.h>
 #include <linux/slab.h>
+#ifdef CONFIG_ACPI
+#include <linux/acpi.h>
+#endif
 /*
  * BIOS.
  */
@@ -476,6 +479,58 @@ static bool radeon_read_disabled_bios(struct radeon_device *rdev)
 		return legacy_read_disabled_bios(rdev);
 }
 
+#ifdef CONFIG_ACPI
+static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
+{
+	bool ret = false;
+	struct acpi_table_header *hdr;
+	/* acpi_get_table_with_size is not exported :( */
+	acpi_size tbl_size = 0x7fffffff;
+	UEFI_ACPI_VFCT *vfct;
+	GOP_VBIOS_CONTENT *vbios;
+	VFCT_IMAGE_HEADER *vhdr;
+
+	if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
+		return false;
+	if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
+		DRM_ERROR("ACPI VFCT table present but broken (too short #1)\n");
+		goto out_unmap;
+	}
+
+	vfct = (UEFI_ACPI_VFCT *)hdr;
+	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) > tbl_size) {
+		DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");
+		goto out_unmap;
+	}
+
+	vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + vfct->VBIOSImageOffset);
+	vhdr = &vbios->VbiosHeader;
+	DRM_INFO("ACPI VFCT contains a BIOS for %02x:%02x.%d %04x:%04x, size %d\n",
+			vhdr->PCIBus, vhdr->PCIDevice, vhdr->PCIFunction,
+			vhdr->VendorID, vhdr->DeviceID, vhdr->ImageLength);
+
+	if (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) {
+		DRM_INFO("ACPI VFCT table is not for this card\n");
+		goto out_unmap;
+	};
+
+	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) + vhdr->ImageLength > tbl_size) {
+		DRM_ERROR("ACPI VFCT image truncated\n");
+		goto out_unmap;
+	}
+
+	rdev->bios = kmemdup(&vbios->VbiosContent, vhdr->ImageLength, GFP_KERNEL);
+	ret = !!rdev->bios;
+
+out_unmap:
+	/* uh, no idea what to do here... */
+	return ret;
+}
+#endif
 
 bool radeon_get_bios(struct radeon_device *rdev)
 {
@@ -487,6 +542,10 @@ bool radeon_get_bios(struct radeon_device *rdev)
 		r = igp_read_bios_from_vram(rdev);
 	if (r == false)
 		r = radeon_read_bios(rdev);
+#ifdef CONFIG_ACPI
+	if (r == false)
+		r = radeon_acpi_vfct_bios(rdev);
+#endif
 	if (r == false) {
 		r = radeon_read_disabled_bios(rdev);
 	}
-- 
1.7.7.5

[PATCH 2/2] drm/radeon: split ATRM support out from the ATPX handler

[alexdeucher]

From: Alex Deucher <alexander.deucher@amd.com>

There are systems that use ATRM, but not ATPX.

Fixes:
https://bugs.freedesktop.org/show_bug.cgi?id=41265

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon.h              |   15 -----
 drivers/gpu/drm/radeon/radeon_atpx_handler.c |   56 +------------------
 drivers/gpu/drm/radeon/radeon_bios.c         |   79 ++++++++++++++++++++++++--
 3 files changed, 75 insertions(+), 75 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index b70ec30..5dbd591 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -142,21 +142,6 @@ struct radeon_device;
 /*
  * BIOS.
  */
-#define ATRM_BIOS_PAGE 4096
-
-#if defined(CONFIG_VGA_SWITCHEROO)
-bool radeon_atrm_supported(struct pci_dev *pdev);
-int radeon_atrm_get_bios_chunk(uint8_t *bios, int offset, int len);
-#else
-static inline bool radeon_atrm_supported(struct pci_dev *pdev)
-{
-	return false;
-}
-
-static inline int radeon_atrm_get_bios_chunk(uint8_t *bios, int offset, int len){
-	return -EINVAL;
-}
-#endif
 bool radeon_get_bios(struct radeon_device *rdev);
 
 /*
diff --git a/drivers/gpu/drm/radeon/radeon_atpx_handler.c b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
index 98724fc..2a2cf0b 100644
--- a/drivers/gpu/drm/radeon/radeon_atpx_handler.c
+++ b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
@@ -30,57 +30,8 @@ static struct radeon_atpx_priv {
 	/* handle for device - and atpx */
 	acpi_handle dhandle;
 	acpi_handle atpx_handle;
-	acpi_handle atrm_handle;
 } radeon_atpx_priv;
 
-/* retrieve the ROM in 4k blocks */
-static int radeon_atrm_call(acpi_handle atrm_handle, uint8_t *bios,
-			    int offset, int len)
-{
-	acpi_status status;
-	union acpi_object atrm_arg_elements[2], *obj;
-	struct acpi_object_list atrm_arg;
-	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL};
-
-	atrm_arg.count = 2;
-	atrm_arg.pointer = &atrm_arg_elements[0];
-
-	atrm_arg_elements[0].type = ACPI_TYPE_INTEGER;
-	atrm_arg_elements[0].integer.value = offset;
-
-	atrm_arg_elements[1].type = ACPI_TYPE_INTEGER;
-	atrm_arg_elements[1].integer.value = len;
-
-	status = acpi_evaluate_object(atrm_handle, NULL, &atrm_arg, &buffer);
-	if (ACPI_FAILURE(status)) {
-		printk("failed to evaluate ATRM got %s\n", acpi_format_exception(status));
-		return -ENODEV;
-	}
-
-	obj = (union acpi_object *)buffer.pointer;
-	memcpy(bios+offset, obj->buffer.pointer, obj->buffer.length);
-	len = obj->buffer.length;
-	kfree(buffer.pointer);
-	return len;
-}
-
-bool radeon_atrm_supported(struct pci_dev *pdev)
-{
-	/* get the discrete ROM only via ATRM */
-	if (!radeon_atpx_priv.atpx_detected)
-		return false;
-
-	if (radeon_atpx_priv.dhandle == DEVICE_ACPI_HANDLE(&pdev->dev))
-		return false;
-	return true;
-}
-
-
-int radeon_atrm_get_bios_chunk(uint8_t *bios, int offset, int len)
-{
-	return radeon_atrm_call(radeon_atpx_priv.atrm_handle, bios, offset, len);
-}
-
 static int radeon_atpx_get_version(acpi_handle handle)
 {
 	acpi_status status;
@@ -198,7 +149,7 @@ static int radeon_atpx_power_state(enum vga_switcheroo_client_id id,
 
 static bool radeon_atpx_pci_probe_handle(struct pci_dev *pdev)
 {
-	acpi_handle dhandle, atpx_handle, atrm_handle;
+	acpi_handle dhandle, atpx_handle;
 	acpi_status status;
 
 	dhandle = DEVICE_ACPI_HANDLE(&pdev->dev);
@@ -209,13 +160,8 @@ static bool radeon_atpx_pci_probe_handle(struct pci_dev *pdev)
 	if (ACPI_FAILURE(status))
 		return false;
 
-	status = acpi_get_handle(dhandle, "ATRM", &atrm_handle);
-	if (ACPI_FAILURE(status))
-		return false;
-
 	radeon_atpx_priv.dhandle = dhandle;
 	radeon_atpx_priv.atpx_handle = atpx_handle;
-	radeon_atpx_priv.atrm_handle = atrm_handle;
 	return true;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
index f06494a7..c36659a 100644
--- a/drivers/gpu/drm/radeon/radeon_bios.c
+++ b/drivers/gpu/drm/radeon/radeon_bios.c
@@ -101,16 +101,81 @@ static bool radeon_read_bios(struct radeon_device *rdev)
 	return true;
 }
 
+#ifdef CONFIG_ACPI
 /* ATRM is used to get the BIOS on the discrete cards in
  * dual-gpu systems.
  */
+/* retrieve the ROM in 4k blocks */
+#define ATRM_BIOS_PAGE 4096
+/**
+ * radeon_atrm_call - fetch a chunk of the vbios
+ *
+ * @atrm_handle: acpi ATRM handle
+ * @bios: vbios image pointer
+ * @offset: offset of vbios image data to fetch
+ * @len: length of vbios image data to fetch
+ *
+ * Executes ATRM to fetch a chunk of the discrete
+ * vbios image on PX systems (all asics).
+ * Returns the length of the buffer fetched.
+ */
+static int radeon_atrm_call(acpi_handle atrm_handle, uint8_t *bios,
+			    int offset, int len)
+{
+	acpi_status status;
+	union acpi_object atrm_arg_elements[2], *obj;
+	struct acpi_object_list atrm_arg;
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL};
+
+	atrm_arg.count = 2;
+	atrm_arg.pointer = &atrm_arg_elements[0];
+
+	atrm_arg_elements[0].type = ACPI_TYPE_INTEGER;
+	atrm_arg_elements[0].integer.value = offset;
+
+	atrm_arg_elements[1].type = ACPI_TYPE_INTEGER;
+	atrm_arg_elements[1].integer.value = len;
+
+	status = acpi_evaluate_object(atrm_handle, NULL, &atrm_arg, &buffer);
+	if (ACPI_FAILURE(status)) {
+		printk("failed to evaluate ATRM got %s\n", acpi_format_exception(status));
+		return -ENODEV;
+	}
+
+	obj = (union acpi_object *)buffer.pointer;
+	memcpy(bios+offset, obj->buffer.pointer, obj->buffer.length);
+	len = obj->buffer.length;
+	kfree(buffer.pointer);
+	return len;
+}
+
 static bool radeon_atrm_get_bios(struct radeon_device *rdev)
 {
 	int ret;
 	int size = 256 * 1024;
 	int i;
+	struct pci_dev *pdev = NULL;
+	acpi_handle dhandle, atrm_handle;
+	acpi_status status;
+	bool found = false;
+
+	/* ATRM is for the discrete card only */
+	if (rdev->flags & RADEON_IS_IGP)
+		return false;
+
+	while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev)) != NULL) {
+		dhandle = DEVICE_ACPI_HANDLE(&pdev->dev);
+		if (!dhandle)
+			continue;
+
+		status = acpi_get_handle(dhandle, "ATRM", &atrm_handle);
+		if (!ACPI_FAILURE(status)) {
+			found = true;
+			break;
+		}
+	}
 
-	if (!radeon_atrm_supported(rdev->pdev))
+	if (!found)
 		return false;
 
 	rdev->bios = kmalloc(size, GFP_KERNEL);
@@ -120,9 +185,10 @@ static bool radeon_atrm_get_bios(struct radeon_device *rdev)
 	}
 
 	for (i = 0; i < size / ATRM_BIOS_PAGE; i++) {
-		ret = radeon_atrm_get_bios_chunk(rdev->bios,
-						 (i * ATRM_BIOS_PAGE),
-						 ATRM_BIOS_PAGE);
+		ret = radeon_atrm_call(atrm_handle,
+				       rdev->bios,
+				       (i * ATRM_BIOS_PAGE),
+				       ATRM_BIOS_PAGE);
 		if (ret < ATRM_BIOS_PAGE)
 			break;
 	}
@@ -133,6 +199,7 @@ static bool radeon_atrm_get_bios(struct radeon_device *rdev)
 	}
 	return true;
 }
+#endif
 
 static bool ni_read_disabled_bios(struct radeon_device *rdev)
 {
@@ -534,10 +601,12 @@ out_unmap:
 
 bool radeon_get_bios(struct radeon_device *rdev)
 {
-	bool r;
+	bool r = false;
 	uint16_t tmp;
 
+#ifdef CONFIG_ACPI
 	r = radeon_atrm_get_bios(rdev);
+#endif
 	if (r == false)
 		r = igp_read_bios_from_vram(rdev);
 	if (r == false)
-- 
1.7.7.5

Re: [PATCH 1/2] drm/radeon: implement ACPI VFCT vbios fetch

[Jerome Glisse]

On Thu, Aug 16, 2012 at 1:54 PM,  <alexdeucher@gmail.com> wrote:
> From: David L <equinox-freedesktopbugs@diac24.net>
>
> This is required for pure UEFI systems.  The vbios is stored
> in ACPI rather than at the legacy vga location.
>
> Fixes:
> https://bugs.freedesktop.org/show_bug.cgi?id=26891
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Reviewed-by: Jerome Glisse <jglisse@redhat.com>

> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/radeon/radeon_bios.c |   59 ++++++++++++++++++++++++++++++++++
>  1 files changed, 59 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
> index 501f488..f06494a7 100644
> --- a/drivers/gpu/drm/radeon/radeon_bios.c
> +++ b/drivers/gpu/drm/radeon/radeon_bios.c
> @@ -32,6 +32,9 @@
>
>  #include <linux/vga_switcheroo.h>
>  #include <linux/slab.h>
> +#ifdef CONFIG_ACPI
> +#include <linux/acpi.h>
> +#endif
>  /*
>   * BIOS.
>   */
> @@ -476,6 +479,58 @@ static bool radeon_read_disabled_bios(struct radeon_device *rdev)
>                 return legacy_read_disabled_bios(rdev);
>  }
>
> +#ifdef CONFIG_ACPI
> +static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
> +{
> +       bool ret = false;
> +       struct acpi_table_header *hdr;
> +       /* acpi_get_table_with_size is not exported :( */
> +       acpi_size tbl_size = 0x7fffffff;
> +       UEFI_ACPI_VFCT *vfct;
> +       GOP_VBIOS_CONTENT *vbios;
> +       VFCT_IMAGE_HEADER *vhdr;
> +
> +       if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
> +               return false;
> +       if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
> +               DRM_ERROR("ACPI VFCT table present but broken (too short #1)\n");
> +               goto out_unmap;
> +       }
> +
> +       vfct = (UEFI_ACPI_VFCT *)hdr;
> +       if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) > tbl_size) {
> +               DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");
> +               goto out_unmap;
> +       }
> +
> +       vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + vfct->VBIOSImageOffset);
> +       vhdr = &vbios->VbiosHeader;
> +       DRM_INFO("ACPI VFCT contains a BIOS for %02x:%02x.%d %04x:%04x, size %d\n",
> +                       vhdr->PCIBus, vhdr->PCIDevice, vhdr->PCIFunction,
> +                       vhdr->VendorID, vhdr->DeviceID, vhdr->ImageLength);
> +
> +       if (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) {
> +               DRM_INFO("ACPI VFCT table is not for this card\n");
> +               goto out_unmap;
> +       };
> +
> +       if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) + vhdr->ImageLength > tbl_size) {
> +               DRM_ERROR("ACPI VFCT image truncated\n");
> +               goto out_unmap;
> +       }
> +
> +       rdev->bios = kmemdup(&vbios->VbiosContent, vhdr->ImageLength, GFP_KERNEL);
> +       ret = !!rdev->bios;
> +
> +out_unmap:
> +       /* uh, no idea what to do here... */
> +       return ret;
> +}
> +#endif
>
>  bool radeon_get_bios(struct radeon_device *rdev)
>  {
> @@ -487,6 +542,10 @@ bool radeon_get_bios(struct radeon_device *rdev)
>                 r = igp_read_bios_from_vram(rdev);
>         if (r == false)
>                 r = radeon_read_bios(rdev);
> +#ifdef CONFIG_ACPI
> +       if (r == false)
> +               r = radeon_acpi_vfct_bios(rdev);
> +#endif
>         if (r == false) {
>                 r = radeon_read_disabled_bios(rdev);
>         }
> --
> 1.7.7.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/radeon: implement ACPI VFCT vbios fetch

[Greg KH]

On Thu, Aug 16, 2012 at 01:54:55PM -0400, alexdeucher@gmail.com wrote:
> From: David L <equinox-freedesktopbugs@diac24.net>
> 
> This is required for pure UEFI systems.  The vbios is stored
> in ACPI rather than at the legacy vga location.
> 
> Fixes:
> https://bugs.freedesktop.org/show_bug.cgi?id=26891
> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/radeon/radeon_bios.c |   59 ++++++++++++++++++++++++++++++++++
>  1 files changed, 59 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
> index 501f488..f06494a7 100644
> --- a/drivers/gpu/drm/radeon/radeon_bios.c
> +++ b/drivers/gpu/drm/radeon/radeon_bios.c
> @@ -32,6 +32,9 @@
>  
>  #include <linux/vga_switcheroo.h>
>  #include <linux/slab.h>
> +#ifdef CONFIG_ACPI
> +#include <linux/acpi.h>
> +#endif

This #ifdef should not be needed, right?


> +#ifdef CONFIG_ACPI
> +static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
> +{
> +	bool ret = false;
> +	struct acpi_table_header *hdr;
> +	/* acpi_get_table_with_size is not exported :( */
> +	acpi_size tbl_size = 0x7fffffff;
> +	UEFI_ACPI_VFCT *vfct;
> +	GOP_VBIOS_CONTENT *vbios;
> +	VFCT_IMAGE_HEADER *vhdr;
> +
> +	if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
> +		return false;
> +	if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
> +		DRM_ERROR("ACPI VFCT table present but broken (too short #1)\n");
> +		goto out_unmap;
> +	}
> +
> +	vfct = (UEFI_ACPI_VFCT *)hdr;
> +	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) > tbl_size) {
> +		DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");
> +		goto out_unmap;
> +	}
> +
> +	vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + vfct->VBIOSImageOffset);
> +	vhdr = &vbios->VbiosHeader;
> +	DRM_INFO("ACPI VFCT contains a BIOS for %02x:%02x.%d %04x:%04x, size %d\n",
> +			vhdr->PCIBus, vhdr->PCIDevice, vhdr->PCIFunction,
> +			vhdr->VendorID, vhdr->DeviceID, vhdr->ImageLength);
> +
> +	if (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) {
> +		DRM_INFO("ACPI VFCT table is not for this card\n");
> +		goto out_unmap;
> +	};
> +
> +	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) + vhdr->ImageLength > tbl_size) {
> +		DRM_ERROR("ACPI VFCT image truncated\n");
> +		goto out_unmap;
> +	}
> +
> +	rdev->bios = kmemdup(&vbios->VbiosContent, vhdr->ImageLength, GFP_KERNEL);
> +	ret = !!rdev->bios;
> +
> +out_unmap:
> +	/* uh, no idea what to do here... */
> +	return ret;
> +}
> +#endif

Please provide an "empty" function here if the #ifdef is not true, which
then makes the following #ifdef not needed:

> @@ -487,6 +542,10 @@ bool radeon_get_bios(struct radeon_device *rdev)
>  		r = igp_read_bios_from_vram(rdev);
>  	if (r == false)
>  		r = radeon_read_bios(rdev);
> +#ifdef CONFIG_ACPI
> +	if (r == false)
> +		r = radeon_acpi_vfct_bios(rdev);
> +#endif

See, then that #ifdef isn't needed.

That cleans up the code more, remember, we don't want #ifdefs in .c
files unless we _really_ have to have them.

thanks,

greg k-h

Re: [PATCH 2/2] drm/radeon: split ATRM support out from the ATPX handler

[Greg KH]

On Thu, Aug 16, 2012 at 01:54:56PM -0400, alexdeucher@gmail.com wrote:
> From: Alex Deucher <alexander.deucher@amd.com>
> 
> There are systems that use ATRM, but not ATPX.
> 
> Fixes:
> https://bugs.freedesktop.org/show_bug.cgi?id=41265

Same #ifdef comments here as for patch 1/2.

thanks,

greg k-h

[PATCH 1/2] drm/radeon: implement ACPI VFCT vbios fetch (v2)

[alexdeucher]

From: David L <equinox-freedesktopbugs@diac24.net>

This is required for pure UEFI systems.  The vbios is stored
in ACPI rather than at the legacy vga location.

Fixes:
https://bugs.freedesktop.org/show_bug.cgi?id=26891

V2: fix #ifdefs as per Greg's comments

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_bios.c |   62 ++++++++++++++++++++++++++++++++++
 1 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
index 501f488..bf21f65 100644
--- a/drivers/gpu/drm/radeon/radeon_bios.c
+++ b/drivers/gpu/drm/radeon/radeon_bios.c
@@ -32,6 +32,9 @@
 
 #include <linux/vga_switcheroo.h>
 #include <linux/slab.h>
+#ifdef CONFIG_ACPI
+#include <linux/acpi.h>
+#endif
 /*
  * BIOS.
  */
@@ -476,6 +479,63 @@ static bool radeon_read_disabled_bios(struct radeon_device *rdev)
 		return legacy_read_disabled_bios(rdev);
 }
 
+#ifdef CONFIG_ACPI
+static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
+{
+	bool ret = false;
+	struct acpi_table_header *hdr;
+	/* acpi_get_table_with_size is not exported :( */
+	acpi_size tbl_size = 0x7fffffff;
+	UEFI_ACPI_VFCT *vfct;
+	GOP_VBIOS_CONTENT *vbios;
+	VFCT_IMAGE_HEADER *vhdr;
+
+	if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
+		return false;
+	if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
+		DRM_ERROR("ACPI VFCT table present but broken (too short #1)\n");
+		goto out_unmap;
+	}
+
+	vfct = (UEFI_ACPI_VFCT *)hdr;
+	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) > tbl_size) {
+		DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");
+		goto out_unmap;
+	}
+
+	vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + vfct->VBIOSImageOffset);
+	vhdr = &vbios->VbiosHeader;
+	DRM_INFO("ACPI VFCT contains a BIOS for %02x:%02x.%d %04x:%04x, size %d\n",
+			vhdr->PCIBus, vhdr->PCIDevice, vhdr->PCIFunction,
+			vhdr->VendorID, vhdr->DeviceID, vhdr->ImageLength);
+
+	if (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) {
+		DRM_INFO("ACPI VFCT table is not for this card\n");
+		goto out_unmap;
+	};
+
+	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) + vhdr->ImageLength > tbl_size) {
+		DRM_ERROR("ACPI VFCT image truncated\n");
+		goto out_unmap;
+	}
+
+	rdev->bios = kmemdup(&vbios->VbiosContent, vhdr->ImageLength, GFP_KERNEL);
+	ret = !!rdev->bios;
+
+out_unmap:
+	/* uh, no idea what to do here... */
+	return ret;
+}
+#else
+static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
+{
+	return false;
+}
+#endif
 
 bool radeon_get_bios(struct radeon_device *rdev)
 {
@@ -484,6 +544,8 @@ bool radeon_get_bios(struct radeon_device *rdev)
 
 	r = radeon_atrm_get_bios(rdev);
 	if (r == false)
+		r = radeon_acpi_vfct_bios(rdev);
+	if (r == false)
 		r = igp_read_bios_from_vram(rdev);
 	if (r == false)
 		r = radeon_read_bios(rdev);
-- 
1.7.7.5

[PATCH 2/2] drm/radeon: split ATRM support out from the ATPX handler (v2)

[alexdeucher]

From: Alex Deucher <alexander.deucher@amd.com>

There are systems that use ATRM, but not ATPX.

Fixes:
https://bugs.freedesktop.org/show_bug.cgi?id=41265

V2: fix #ifdefs as per Greg's comments

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon.h              |   15 -----
 drivers/gpu/drm/radeon/radeon_atpx_handler.c |   56 +-----------------
 drivers/gpu/drm/radeon/radeon_bios.c         |   82 ++++++++++++++++++++++++--
 3 files changed, 77 insertions(+), 76 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 9930419..59a1531 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -142,21 +142,6 @@ struct radeon_device;
 /*
  * BIOS.
  */
-#define ATRM_BIOS_PAGE 4096
-
-#if defined(CONFIG_VGA_SWITCHEROO)
-bool radeon_atrm_supported(struct pci_dev *pdev);
-int radeon_atrm_get_bios_chunk(uint8_t *bios, int offset, int len);
-#else
-static inline bool radeon_atrm_supported(struct pci_dev *pdev)
-{
-	return false;
-}
-
-static inline int radeon_atrm_get_bios_chunk(uint8_t *bios, int offset, int len){
-	return -EINVAL;
-}
-#endif
 bool radeon_get_bios(struct radeon_device *rdev);
 
 /*
diff --git a/drivers/gpu/drm/radeon/radeon_atpx_handler.c b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
index 98724fc..2a2cf0b 100644
--- a/drivers/gpu/drm/radeon/radeon_atpx_handler.c
+++ b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
@@ -30,57 +30,8 @@ static struct radeon_atpx_priv {
 	/* handle for device - and atpx */
 	acpi_handle dhandle;
 	acpi_handle atpx_handle;
-	acpi_handle atrm_handle;
 } radeon_atpx_priv;
 
-/* retrieve the ROM in 4k blocks */
-static int radeon_atrm_call(acpi_handle atrm_handle, uint8_t *bios,
-			    int offset, int len)
-{
-	acpi_status status;
-	union acpi_object atrm_arg_elements[2], *obj;
-	struct acpi_object_list atrm_arg;
-	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL};
-
-	atrm_arg.count = 2;
-	atrm_arg.pointer = &atrm_arg_elements[0];
-
-	atrm_arg_elements[0].type = ACPI_TYPE_INTEGER;
-	atrm_arg_elements[0].integer.value = offset;
-
-	atrm_arg_elements[1].type = ACPI_TYPE_INTEGER;
-	atrm_arg_elements[1].integer.value = len;
-
-	status = acpi_evaluate_object(atrm_handle, NULL, &atrm_arg, &buffer);
-	if (ACPI_FAILURE(status)) {
-		printk("failed to evaluate ATRM got %s\n", acpi_format_exception(status));
-		return -ENODEV;
-	}
-
-	obj = (union acpi_object *)buffer.pointer;
-	memcpy(bios+offset, obj->buffer.pointer, obj->buffer.length);
-	len = obj->buffer.length;
-	kfree(buffer.pointer);
-	return len;
-}
-
-bool radeon_atrm_supported(struct pci_dev *pdev)
-{
-	/* get the discrete ROM only via ATRM */
-	if (!radeon_atpx_priv.atpx_detected)
-		return false;
-
-	if (radeon_atpx_priv.dhandle == DEVICE_ACPI_HANDLE(&pdev->dev))
-		return false;
-	return true;
-}
-
-
-int radeon_atrm_get_bios_chunk(uint8_t *bios, int offset, int len)
-{
-	return radeon_atrm_call(radeon_atpx_priv.atrm_handle, bios, offset, len);
-}
-
 static int radeon_atpx_get_version(acpi_handle handle)
 {
 	acpi_status status;
@@ -198,7 +149,7 @@ static int radeon_atpx_power_state(enum vga_switcheroo_client_id id,
 
 static bool radeon_atpx_pci_probe_handle(struct pci_dev *pdev)
 {
-	acpi_handle dhandle, atpx_handle, atrm_handle;
+	acpi_handle dhandle, atpx_handle;
 	acpi_status status;
 
 	dhandle = DEVICE_ACPI_HANDLE(&pdev->dev);
@@ -209,13 +160,8 @@ static bool radeon_atpx_pci_probe_handle(struct pci_dev *pdev)
 	if (ACPI_FAILURE(status))
 		return false;
 
-	status = acpi_get_handle(dhandle, "ATRM", &atrm_handle);
-	if (ACPI_FAILURE(status))
-		return false;
-
 	radeon_atpx_priv.dhandle = dhandle;
 	radeon_atpx_priv.atpx_handle = atpx_handle;
-	radeon_atpx_priv.atrm_handle = atrm_handle;
 	return true;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
index bf21f65..e97779d 100644
--- a/drivers/gpu/drm/radeon/radeon_bios.c
+++ b/drivers/gpu/drm/radeon/radeon_bios.c
@@ -32,9 +32,7 @@
 
 #include <linux/vga_switcheroo.h>
 #include <linux/slab.h>
-#ifdef CONFIG_ACPI
 #include <linux/acpi.h>
-#endif
 /*
  * BIOS.
  */
@@ -101,16 +99,81 @@ static bool radeon_read_bios(struct radeon_device *rdev)
 	return true;
 }
 
+#ifdef CONFIG_ACPI
 /* ATRM is used to get the BIOS on the discrete cards in
  * dual-gpu systems.
  */
+/* retrieve the ROM in 4k blocks */
+#define ATRM_BIOS_PAGE 4096
+/**
+ * radeon_atrm_call - fetch a chunk of the vbios
+ *
+ * @atrm_handle: acpi ATRM handle
+ * @bios: vbios image pointer
+ * @offset: offset of vbios image data to fetch
+ * @len: length of vbios image data to fetch
+ *
+ * Executes ATRM to fetch a chunk of the discrete
+ * vbios image on PX systems (all asics).
+ * Returns the length of the buffer fetched.
+ */
+static int radeon_atrm_call(acpi_handle atrm_handle, uint8_t *bios,
+			    int offset, int len)
+{
+	acpi_status status;
+	union acpi_object atrm_arg_elements[2], *obj;
+	struct acpi_object_list atrm_arg;
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL};
+
+	atrm_arg.count = 2;
+	atrm_arg.pointer = &atrm_arg_elements[0];
+
+	atrm_arg_elements[0].type = ACPI_TYPE_INTEGER;
+	atrm_arg_elements[0].integer.value = offset;
+
+	atrm_arg_elements[1].type = ACPI_TYPE_INTEGER;
+	atrm_arg_elements[1].integer.value = len;
+
+	status = acpi_evaluate_object(atrm_handle, NULL, &atrm_arg, &buffer);
+	if (ACPI_FAILURE(status)) {
+		printk("failed to evaluate ATRM got %s\n", acpi_format_exception(status));
+		return -ENODEV;
+	}
+
+	obj = (union acpi_object *)buffer.pointer;
+	memcpy(bios+offset, obj->buffer.pointer, obj->buffer.length);
+	len = obj->buffer.length;
+	kfree(buffer.pointer);
+	return len;
+}
+
 static bool radeon_atrm_get_bios(struct radeon_device *rdev)
 {
 	int ret;
 	int size = 256 * 1024;
 	int i;
+	struct pci_dev *pdev = NULL;
+	acpi_handle dhandle, atrm_handle;
+	acpi_status status;
+	bool found = false;
+
+	/* ATRM is for the discrete card only */
+	if (rdev->flags & RADEON_IS_IGP)
+		return false;
+
+	while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev)) != NULL) {
+		dhandle = DEVICE_ACPI_HANDLE(&pdev->dev);
+		if (!dhandle)
+			continue;
+
+		status = acpi_get_handle(dhandle, "ATRM", &atrm_handle);
+		if (!ACPI_FAILURE(status)) {
+			found = true;
+			break;
+		}
+	}
 
-	if (!radeon_atrm_supported(rdev->pdev))
+	if (!found)
 		return false;
 
 	rdev->bios = kmalloc(size, GFP_KERNEL);
@@ -120,9 +183,10 @@ static bool radeon_atrm_get_bios(struct radeon_device *rdev)
 	}
 
 	for (i = 0; i < size / ATRM_BIOS_PAGE; i++) {
-		ret = radeon_atrm_get_bios_chunk(rdev->bios,
-						 (i * ATRM_BIOS_PAGE),
-						 ATRM_BIOS_PAGE);
+		ret = radeon_atrm_call(atrm_handle,
+				       rdev->bios,
+				       (i * ATRM_BIOS_PAGE),
+				       ATRM_BIOS_PAGE);
 		if (ret < ATRM_BIOS_PAGE)
 			break;
 	}
@@ -133,6 +197,12 @@ static bool radeon_atrm_get_bios(struct radeon_device *rdev)
 	}
 	return true;
 }
+#else
+static bool radeon_atrm_get_bios(struct radeon_device *rdev)
+{
+	return false;
+}
+#endif
 
 static bool ni_read_disabled_bios(struct radeon_device *rdev)
 {
-- 
1.7.7.5

Re: [PATCH 1/2] drm/radeon: implement ACPI VFCT vbios fetch (v2)

[Greg KH]

On Thu, Aug 16, 2012 at 03:13:46PM -0400, alexdeucher@gmail.com wrote:
> From: David L <equinox-freedesktopbugs@diac24.net>
> 
> This is required for pure UEFI systems.  The vbios is stored
> in ACPI rather than at the legacy vga location.
> 
> Fixes:
> https://bugs.freedesktop.org/show_bug.cgi?id=26891
> 
> V2: fix #ifdefs as per Greg's comments
> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/radeon/radeon_bios.c |   62 ++++++++++++++++++++++++++++++++++
>  1 files changed, 62 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
> index 501f488..bf21f65 100644
> --- a/drivers/gpu/drm/radeon/radeon_bios.c
> +++ b/drivers/gpu/drm/radeon/radeon_bios.c
> @@ -32,6 +32,9 @@
>  
>  #include <linux/vga_switcheroo.h>
>  #include <linux/slab.h>
> +#ifdef CONFIG_ACPI
> +#include <linux/acpi.h>
> +#endif

You forgot to remove this one :(

>  /*
>   * BIOS.
>   */
> @@ -476,6 +479,63 @@ static bool radeon_read_disabled_bios(struct radeon_device *rdev)
>  		return legacy_read_disabled_bios(rdev);
>  }
>  
> +#ifdef CONFIG_ACPI
> +static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
> +{
> +	bool ret = false;
> +	struct acpi_table_header *hdr;
> +	/* acpi_get_table_with_size is not exported :( */
> +	acpi_size tbl_size = 0x7fffffff;
> +	UEFI_ACPI_VFCT *vfct;
> +	GOP_VBIOS_CONTENT *vbios;
> +	VFCT_IMAGE_HEADER *vhdr;
> +
> +	if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
> +		return false;
> +	if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
> +		DRM_ERROR("ACPI VFCT table present but broken (too short #1)\n");
> +		goto out_unmap;
> +	}
> +
> +	vfct = (UEFI_ACPI_VFCT *)hdr;
> +	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) > tbl_size) {
> +		DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");
> +		goto out_unmap;
> +	}
> +
> +	vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + vfct->VBIOSImageOffset);
> +	vhdr = &vbios->VbiosHeader;
> +	DRM_INFO("ACPI VFCT contains a BIOS for %02x:%02x.%d %04x:%04x, size %d\n",
> +			vhdr->PCIBus, vhdr->PCIDevice, vhdr->PCIFunction,
> +			vhdr->VendorID, vhdr->DeviceID, vhdr->ImageLength);
> +
> +	if (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) {
> +		DRM_INFO("ACPI VFCT table is not for this card\n");
> +		goto out_unmap;
> +	};
> +
> +	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) + vhdr->ImageLength > tbl_size) {
> +		DRM_ERROR("ACPI VFCT image truncated\n");
> +		goto out_unmap;
> +	}
> +
> +	rdev->bios = kmemdup(&vbios->VbiosContent, vhdr->ImageLength, GFP_KERNEL);
> +	ret = !!rdev->bios;
> +
> +out_unmap:
> +	/* uh, no idea what to do here... */
> +	return ret;
> +}
> +#else
> +static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)

Make it inline, and then the compiler is smart enough to just delete the
whole if () check you make when you call it.

Third time's a charm?

greg k-h

Re: [PATCH 1/2] drm/radeon: implement ACPI VFCT vbios fetch (v2)

[Alex Deucher]

On Thu, Aug 16, 2012 at 3:25 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Aug 16, 2012 at 03:13:46PM -0400, alexdeucher@gmail.com wrote:
>> From: David L <equinox-freedesktopbugs@diac24.net>
>>
>> This is required for pure UEFI systems.  The vbios is stored
>> in ACPI rather than at the legacy vga location.
>>
>> Fixes:
>> https://bugs.freedesktop.org/show_bug.cgi?id=26891
>>
>> V2: fix #ifdefs as per Greg's comments
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> Cc: stable@vger.kernel.org
>> ---
>>  drivers/gpu/drm/radeon/radeon_bios.c |   62 ++++++++++++++++++++++++++++++++++
>>  1 files changed, 62 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
>> index 501f488..bf21f65 100644
>> --- a/drivers/gpu/drm/radeon/radeon_bios.c
>> +++ b/drivers/gpu/drm/radeon/radeon_bios.c
>> @@ -32,6 +32,9 @@
>>
>>  #include <linux/vga_switcheroo.h>
>>  #include <linux/slab.h>
>> +#ifdef CONFIG_ACPI
>> +#include <linux/acpi.h>
>> +#endif
>
> You forgot to remove this one :(

Argh!  I squashed that into the wrong patch when I rebased my fixes.

>
>>  /*
>>   * BIOS.
>>   */
>> @@ -476,6 +479,63 @@ static bool radeon_read_disabled_bios(struct radeon_device *rdev)
>>               return legacy_read_disabled_bios(rdev);
>>  }
>>
>> +#ifdef CONFIG_ACPI
>> +static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
>> +{
>> +     bool ret = false;
>> +     struct acpi_table_header *hdr;
>> +     /* acpi_get_table_with_size is not exported :( */
>> +     acpi_size tbl_size = 0x7fffffff;
>> +     UEFI_ACPI_VFCT *vfct;
>> +     GOP_VBIOS_CONTENT *vbios;
>> +     VFCT_IMAGE_HEADER *vhdr;
>> +
>> +     if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
>> +             return false;
>> +     if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
>> +             DRM_ERROR("ACPI VFCT table present but broken (too short #1)\n");
>> +             goto out_unmap;
>> +     }
>> +
>> +     vfct = (UEFI_ACPI_VFCT *)hdr;
>> +     if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) > tbl_size) {
>> +             DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");
>> +             goto out_unmap;
>> +     }
>> +
>> +     vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + vfct->VBIOSImageOffset);
>> +     vhdr = &vbios->VbiosHeader;
>> +     DRM_INFO("ACPI VFCT contains a BIOS for %02x:%02x.%d %04x:%04x, size %d\n",
>> +                     vhdr->PCIBus, vhdr->PCIDevice, vhdr->PCIFunction,
>> +                     vhdr->VendorID, vhdr->DeviceID, vhdr->ImageLength);
>> +
>> +     if (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) {
>> +             DRM_INFO("ACPI VFCT table is not for this card\n");
>> +             goto out_unmap;
>> +     };
>> +
>> +     if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) + vhdr->ImageLength > tbl_size) {
>> +             DRM_ERROR("ACPI VFCT image truncated\n");
>> +             goto out_unmap;
>> +     }
>> +
>> +     rdev->bios = kmemdup(&vbios->VbiosContent, vhdr->ImageLength, GFP_KERNEL);
>> +     ret = !!rdev->bios;
>> +
>> +out_unmap:
>> +     /* uh, no idea what to do here... */
>> +     return ret;
>> +}
>> +#else
>> +static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
>
> Make it inline, and then the compiler is smart enough to just delete the
> whole if () check you make when you call it.
>
> Third time's a charm?

On the way.

Alex

Re: [PATCH 1/2] drm/radeon: implement ACPI VFCT vbios fetch (v2)

[David Lamparter]

[-- Attachment #1.1: Type: text/plain, Size: 3318 bytes --]

On Thu, Aug 16, 2012 at 03:13:46PM -0400, alexdeucher@gmail.com wrote:
> From: David L <equinox-freedesktopbugs@diac24.net>
From: David Lamparter <equinox@diac24.net>

There are still two rough edges left in here, I didn't get around to
clean it up, other stuff came up -- sorry...

> This is required for pure UEFI systems.  The vbios is stored
> in ACPI rather than at the legacy vga location.
> 
> Fixes:
> https://bugs.freedesktop.org/show_bug.cgi?id=26891
> 
> V2: fix #ifdefs as per Greg's comments
> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: stable@vger.kernel.org
> ---
[...]
> +	struct acpi_table_header *hdr;
> +	/* acpi_get_table_with_size is not exported :( */
> +	acpi_size tbl_size = 0x7fffffff;

I was using acpi_get_table_with_size, but that needs an export, with
0x7fffffff all the tests below are kinda useless because they always
succeed.  I think it'd be useful to keep the length checks in case some
vendor breaks their ACPI tables, so this needs an EXPORT_SYMBOL.

> +	UEFI_ACPI_VFCT *vfct;
> +	GOP_VBIOS_CONTENT *vbios;
> +	VFCT_IMAGE_HEADER *vhdr;
> +
> +	if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
> +		return false;
> +	if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
> +		DRM_ERROR("ACPI VFCT table present but broken (too short #1)\n");
> +		goto out_unmap;
> +	}
> +
> +	vfct = (UEFI_ACPI_VFCT *)hdr;
> +	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) > tbl_size) {
> +		DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");
> +		goto out_unmap;
> +	}
> +
> +	vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + vfct->VBIOSImageOffset);
> +	vhdr = &vbios->VbiosHeader;
> +	DRM_INFO("ACPI VFCT contains a BIOS for %02x:%02x.%d %04x:%04x, size %d\n",
> +			vhdr->PCIBus, vhdr->PCIDevice, vhdr->PCIFunction,
> +			vhdr->VendorID, vhdr->DeviceID, vhdr->ImageLength);
> +
> +	if (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) {
> +		DRM_INFO("ACPI VFCT table is not for this card\n");
> +		goto out_unmap;
> +	};
> +
> +	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) + vhdr->ImageLength > tbl_size) {
> +		DRM_ERROR("ACPI VFCT image truncated\n");
> +		goto out_unmap;
> +	}
> +
> +	rdev->bios = kmemdup(&vbios->VbiosContent, vhdr->ImageLength, GFP_KERNEL);
> +	ret = !!rdev->bios;
> +
> +out_unmap:
> +	/* uh, no idea what to do here... */

So, er, I had no clue how to clean up the return value of acpi_get_table
- does this actually need to be cleaned up?  Or do you just get a
pointer straight to the "real" ACPI table?

> +	return ret;
> +}
> +#else
> +static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
> +{
> +	return false;
> +}
> +#endif
>  
>  bool radeon_get_bios(struct radeon_device *rdev)
>  {
> @@ -484,6 +544,8 @@ bool radeon_get_bios(struct radeon_device *rdev)
>  
>  	r = radeon_atrm_get_bios(rdev);
>  	if (r == false)
> +		r = radeon_acpi_vfct_bios(rdev);
> +	if (r == false)
>  		r = igp_read_bios_from_vram(rdev);
>  	if (r == false)
>  		r = radeon_read_bios(rdev);
> -- 
> 1.7.7.5
> 

Cheers,

-David

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 230 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] drm/radeon: implement ACPI VFCT vbios fetch (v3)

[alexdeucher]

From: David Lamparter <equinox@diac24.net>

This is required for pure UEFI systems.  The vbios is stored
in ACPI rather than at the legacy vga location.

Fixes:
https://bugs.freedesktop.org/show_bug.cgi?id=26891

V2: fix #ifdefs as per Greg's comments
V3: fix it harder

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_bios.c |   60 ++++++++++++++++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
index 501f488..a32232f 100644
--- a/drivers/gpu/drm/radeon/radeon_bios.c
+++ b/drivers/gpu/drm/radeon/radeon_bios.c
@@ -32,6 +32,7 @@
 
 #include <linux/vga_switcheroo.h>
 #include <linux/slab.h>
+#include <linux/acpi.h>
 /*
  * BIOS.
  */
@@ -476,6 +477,63 @@ static bool radeon_read_disabled_bios(struct radeon_device *rdev)
 		return legacy_read_disabled_bios(rdev);
 }
 
+#ifdef CONFIG_ACPI
+static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
+{
+	bool ret = false;
+	struct acpi_table_header *hdr;
+	/* acpi_get_table_with_size is not exported :( */
+	acpi_size tbl_size = 0x7fffffff;
+	UEFI_ACPI_VFCT *vfct;
+	GOP_VBIOS_CONTENT *vbios;
+	VFCT_IMAGE_HEADER *vhdr;
+
+	if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
+		return false;
+	if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
+		DRM_ERROR("ACPI VFCT table present but broken (too short #1)\n");
+		goto out_unmap;
+	}
+
+	vfct = (UEFI_ACPI_VFCT *)hdr;
+	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) > tbl_size) {
+		DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");
+		goto out_unmap;
+	}
+
+	vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + vfct->VBIOSImageOffset);
+	vhdr = &vbios->VbiosHeader;
+	DRM_INFO("ACPI VFCT contains a BIOS for %02x:%02x.%d %04x:%04x, size %d\n",
+			vhdr->PCIBus, vhdr->PCIDevice, vhdr->PCIFunction,
+			vhdr->VendorID, vhdr->DeviceID, vhdr->ImageLength);
+
+	if (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) {
+		DRM_INFO("ACPI VFCT table is not for this card\n");
+		goto out_unmap;
+	};
+
+	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) + vhdr->ImageLength > tbl_size) {
+		DRM_ERROR("ACPI VFCT image truncated\n");
+		goto out_unmap;
+	}
+
+	rdev->bios = kmemdup(&vbios->VbiosContent, vhdr->ImageLength, GFP_KERNEL);
+	ret = !!rdev->bios;
+
+out_unmap:
+	/* uh, no idea what to do here... */
+	return ret;
+}
+#else
+static inline bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
+{
+	return false;
+}
+#endif
 
 bool radeon_get_bios(struct radeon_device *rdev)
 {
@@ -484,6 +542,8 @@ bool radeon_get_bios(struct radeon_device *rdev)
 
 	r = radeon_atrm_get_bios(rdev);
 	if (r == false)
+		r = radeon_acpi_vfct_bios(rdev);
+	if (r == false)
 		r = igp_read_bios_from_vram(rdev);
 	if (r == false)
 		r = radeon_read_bios(rdev);
-- 
1.7.7.5

[PATCH 2/2] drm/radeon: split ATRM support out from the ATPX handler (v3)

[alexdeucher]

From: Alex Deucher <alexander.deucher@amd.com>

There are systems that use ATRM, but not ATPX.

Fixes:
https://bugs.freedesktop.org/show_bug.cgi?id=41265

V2: fix #ifdefs as per Greg's comments
V3: fix it harder

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon.h              |   15 -----
 drivers/gpu/drm/radeon/radeon_atpx_handler.c |   56 +------------------
 drivers/gpu/drm/radeon/radeon_bios.c         |   80 ++++++++++++++++++++++++-
 3 files changed, 77 insertions(+), 74 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 9930419..59a1531 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -142,21 +142,6 @@ struct radeon_device;
 /*
  * BIOS.
  */
-#define ATRM_BIOS_PAGE 4096
-
-#if defined(CONFIG_VGA_SWITCHEROO)
-bool radeon_atrm_supported(struct pci_dev *pdev);
-int radeon_atrm_get_bios_chunk(uint8_t *bios, int offset, int len);
-#else
-static inline bool radeon_atrm_supported(struct pci_dev *pdev)
-{
-	return false;
-}
-
-static inline int radeon_atrm_get_bios_chunk(uint8_t *bios, int offset, int len){
-	return -EINVAL;
-}
-#endif
 bool radeon_get_bios(struct radeon_device *rdev);
 
 /*
diff --git a/drivers/gpu/drm/radeon/radeon_atpx_handler.c b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
index 98724fc..2a2cf0b 100644
--- a/drivers/gpu/drm/radeon/radeon_atpx_handler.c
+++ b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
@@ -30,57 +30,8 @@ static struct radeon_atpx_priv {
 	/* handle for device - and atpx */
 	acpi_handle dhandle;
 	acpi_handle atpx_handle;
-	acpi_handle atrm_handle;
 } radeon_atpx_priv;
 
-/* retrieve the ROM in 4k blocks */
-static int radeon_atrm_call(acpi_handle atrm_handle, uint8_t *bios,
-			    int offset, int len)
-{
-	acpi_status status;
-	union acpi_object atrm_arg_elements[2], *obj;
-	struct acpi_object_list atrm_arg;
-	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL};
-
-	atrm_arg.count = 2;
-	atrm_arg.pointer = &atrm_arg_elements[0];
-
-	atrm_arg_elements[0].type = ACPI_TYPE_INTEGER;
-	atrm_arg_elements[0].integer.value = offset;
-
-	atrm_arg_elements[1].type = ACPI_TYPE_INTEGER;
-	atrm_arg_elements[1].integer.value = len;
-
-	status = acpi_evaluate_object(atrm_handle, NULL, &atrm_arg, &buffer);
-	if (ACPI_FAILURE(status)) {
-		printk("failed to evaluate ATRM got %s\n", acpi_format_exception(status));
-		return -ENODEV;
-	}
-
-	obj = (union acpi_object *)buffer.pointer;
-	memcpy(bios+offset, obj->buffer.pointer, obj->buffer.length);
-	len = obj->buffer.length;
-	kfree(buffer.pointer);
-	return len;
-}
-
-bool radeon_atrm_supported(struct pci_dev *pdev)
-{
-	/* get the discrete ROM only via ATRM */
-	if (!radeon_atpx_priv.atpx_detected)
-		return false;
-
-	if (radeon_atpx_priv.dhandle == DEVICE_ACPI_HANDLE(&pdev->dev))
-		return false;
-	return true;
-}
-
-
-int radeon_atrm_get_bios_chunk(uint8_t *bios, int offset, int len)
-{
-	return radeon_atrm_call(radeon_atpx_priv.atrm_handle, bios, offset, len);
-}
-
 static int radeon_atpx_get_version(acpi_handle handle)
 {
 	acpi_status status;
@@ -198,7 +149,7 @@ static int radeon_atpx_power_state(enum vga_switcheroo_client_id id,
 
 static bool radeon_atpx_pci_probe_handle(struct pci_dev *pdev)
 {
-	acpi_handle dhandle, atpx_handle, atrm_handle;
+	acpi_handle dhandle, atpx_handle;
 	acpi_status status;
 
 	dhandle = DEVICE_ACPI_HANDLE(&pdev->dev);
@@ -209,13 +160,8 @@ static bool radeon_atpx_pci_probe_handle(struct pci_dev *pdev)
 	if (ACPI_FAILURE(status))
 		return false;
 
-	status = acpi_get_handle(dhandle, "ATRM", &atrm_handle);
-	if (ACPI_FAILURE(status))
-		return false;
-
 	radeon_atpx_priv.dhandle = dhandle;
 	radeon_atpx_priv.atpx_handle = atpx_handle;
-	radeon_atpx_priv.atrm_handle = atrm_handle;
 	return true;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
index a32232f..602898d 100644
--- a/drivers/gpu/drm/radeon/radeon_bios.c
+++ b/drivers/gpu/drm/radeon/radeon_bios.c
@@ -99,16 +99,81 @@ static bool radeon_read_bios(struct radeon_device *rdev)
 	return true;
 }
 
+#ifdef CONFIG_ACPI
 /* ATRM is used to get the BIOS on the discrete cards in
  * dual-gpu systems.
  */
+/* retrieve the ROM in 4k blocks */
+#define ATRM_BIOS_PAGE 4096
+/**
+ * radeon_atrm_call - fetch a chunk of the vbios
+ *
+ * @atrm_handle: acpi ATRM handle
+ * @bios: vbios image pointer
+ * @offset: offset of vbios image data to fetch
+ * @len: length of vbios image data to fetch
+ *
+ * Executes ATRM to fetch a chunk of the discrete
+ * vbios image on PX systems (all asics).
+ * Returns the length of the buffer fetched.
+ */
+static int radeon_atrm_call(acpi_handle atrm_handle, uint8_t *bios,
+			    int offset, int len)
+{
+	acpi_status status;
+	union acpi_object atrm_arg_elements[2], *obj;
+	struct acpi_object_list atrm_arg;
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL};
+
+	atrm_arg.count = 2;
+	atrm_arg.pointer = &atrm_arg_elements[0];
+
+	atrm_arg_elements[0].type = ACPI_TYPE_INTEGER;
+	atrm_arg_elements[0].integer.value = offset;
+
+	atrm_arg_elements[1].type = ACPI_TYPE_INTEGER;
+	atrm_arg_elements[1].integer.value = len;
+
+	status = acpi_evaluate_object(atrm_handle, NULL, &atrm_arg, &buffer);
+	if (ACPI_FAILURE(status)) {
+		printk("failed to evaluate ATRM got %s\n", acpi_format_exception(status));
+		return -ENODEV;
+	}
+
+	obj = (union acpi_object *)buffer.pointer;
+	memcpy(bios+offset, obj->buffer.pointer, obj->buffer.length);
+	len = obj->buffer.length;
+	kfree(buffer.pointer);
+	return len;
+}
+
 static bool radeon_atrm_get_bios(struct radeon_device *rdev)
 {
 	int ret;
 	int size = 256 * 1024;
 	int i;
+	struct pci_dev *pdev = NULL;
+	acpi_handle dhandle, atrm_handle;
+	acpi_status status;
+	bool found = false;
 
-	if (!radeon_atrm_supported(rdev->pdev))
+	/* ATRM is for the discrete card only */
+	if (rdev->flags & RADEON_IS_IGP)
+		return false;
+
+	while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev)) != NULL) {
+		dhandle = DEVICE_ACPI_HANDLE(&pdev->dev);
+		if (!dhandle)
+			continue;
+
+		status = acpi_get_handle(dhandle, "ATRM", &atrm_handle);
+		if (!ACPI_FAILURE(status)) {
+			found = true;
+			break;
+		}
+	}
+
+	if (!found)
 		return false;
 
 	rdev->bios = kmalloc(size, GFP_KERNEL);
@@ -118,9 +183,10 @@ static bool radeon_atrm_get_bios(struct radeon_device *rdev)
 	}
 
 	for (i = 0; i < size / ATRM_BIOS_PAGE; i++) {
-		ret = radeon_atrm_get_bios_chunk(rdev->bios,
-						 (i * ATRM_BIOS_PAGE),
-						 ATRM_BIOS_PAGE);
+		ret = radeon_atrm_call(atrm_handle,
+				       rdev->bios,
+				       (i * ATRM_BIOS_PAGE),
+				       ATRM_BIOS_PAGE);
 		if (ret < ATRM_BIOS_PAGE)
 			break;
 	}
@@ -131,6 +197,12 @@ static bool radeon_atrm_get_bios(struct radeon_device *rdev)
 	}
 	return true;
 }
+#else
+static inline bool radeon_atrm_get_bios(struct radeon_device *rdev)
+{
+	return false;
+}
+#endif
 
 static bool ni_read_disabled_bios(struct radeon_device *rdev)
 {
-- 
1.7.7.5

Re: [PATCH 1/2] drm/radeon: implement ACPI VFCT vbios fetch (v2)

[Alex Deucher]

On Thu, Aug 16, 2012 at 3:40 PM, David Lamparter <equinox@diac24.net> wrote:
> On Thu, Aug 16, 2012 at 03:13:46PM -0400, alexdeucher@gmail.com wrote:
>> From: David L <equinox-freedesktopbugs@diac24.net>
> From: David Lamparter <equinox@diac24.net>
>
> There are still two rough edges left in here, I didn't get around to
> clean it up, other stuff came up -- sorry...
>
>> This is required for pure UEFI systems.  The vbios is stored
>> in ACPI rather than at the legacy vga location.
>>
>> Fixes:
>> https://bugs.freedesktop.org/show_bug.cgi?id=26891
>>
>> V2: fix #ifdefs as per Greg's comments
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> Cc: stable@vger.kernel.org
>> ---
> [...]
>> +     struct acpi_table_header *hdr;
>> +     /* acpi_get_table_with_size is not exported :( */
>> +     acpi_size tbl_size = 0x7fffffff;
>
> I was using acpi_get_table_with_size, but that needs an export, with
> 0x7fffffff all the tests below are kinda useless because they always
> succeed.  I think it'd be useful to keep the length checks in case some
> vendor breaks their ACPI tables, so this needs an EXPORT_SYMBOL.

I guess we could leave it as is for now for -fixes and then switch it
use use the new exported symbol for -next?  Is it ok to export a new
symbol for -fixes?

>
>> +     UEFI_ACPI_VFCT *vfct;
>> +     GOP_VBIOS_CONTENT *vbios;
>> +     VFCT_IMAGE_HEADER *vhdr;
>> +
>> +     if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
>> +             return false;
>> +     if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
>> +             DRM_ERROR("ACPI VFCT table present but broken (too short #1)\n");
>> +             goto out_unmap;
>> +     }
>> +
>> +     vfct = (UEFI_ACPI_VFCT *)hdr;
>> +     if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) > tbl_size) {
>> +             DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");
>> +             goto out_unmap;
>> +     }
>> +
>> +     vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + vfct->VBIOSImageOffset);
>> +     vhdr = &vbios->VbiosHeader;
>> +     DRM_INFO("ACPI VFCT contains a BIOS for %02x:%02x.%d %04x:%04x, size %d\n",
>> +                     vhdr->PCIBus, vhdr->PCIDevice, vhdr->PCIFunction,
>> +                     vhdr->VendorID, vhdr->DeviceID, vhdr->ImageLength);
>> +
>> +     if (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) {
>> +             DRM_INFO("ACPI VFCT table is not for this card\n");
>> +             goto out_unmap;
>> +     };
>> +
>> +     if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) + vhdr->ImageLength > tbl_size) {
>> +             DRM_ERROR("ACPI VFCT image truncated\n");
>> +             goto out_unmap;
>> +     }
>> +
>> +     rdev->bios = kmemdup(&vbios->VbiosContent, vhdr->ImageLength, GFP_KERNEL);
>> +     ret = !!rdev->bios;
>> +
>> +out_unmap:
>> +     /* uh, no idea what to do here... */
>
> So, er, I had no clue how to clean up the return value of acpi_get_table
> - does this actually need to be cleaned up?  Or do you just get a
> pointer straight to the "real" ACPI table?

Not sure on that.  Anyone know more about the acpi code?

Alex

Re: [PATCH 1/2] drm/radeon: implement ACPI VFCT vbios fetch (v2)

[Alex Deucher]

Adding Matthew in case he as any ideas.

On Thu, Aug 16, 2012 at 3:51 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Thu, Aug 16, 2012 at 3:40 PM, David Lamparter <equinox@diac24.net> wrote:
>> On Thu, Aug 16, 2012 at 03:13:46PM -0400, alexdeucher@gmail.com wrote:
>>> From: David L <equinox-freedesktopbugs@diac24.net>
>> From: David Lamparter <equinox@diac24.net>
>>
>> There are still two rough edges left in here, I didn't get around to
>> clean it up, other stuff came up -- sorry...
>>
>>> This is required for pure UEFI systems.  The vbios is stored
>>> in ACPI rather than at the legacy vga location.
>>>
>>> Fixes:
>>> https://bugs.freedesktop.org/show_bug.cgi?id=26891
>>>
>>> V2: fix #ifdefs as per Greg's comments
>>>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> Cc: stable@vger.kernel.org
>>> ---
>> [...]
>>> +     struct acpi_table_header *hdr;
>>> +     /* acpi_get_table_with_size is not exported :( */
>>> +     acpi_size tbl_size = 0x7fffffff;
>>
>> I was using acpi_get_table_with_size, but that needs an export, with
>> 0x7fffffff all the tests below are kinda useless because they always
>> succeed.  I think it'd be useful to keep the length checks in case some
>> vendor breaks their ACPI tables, so this needs an EXPORT_SYMBOL.
>
> I guess we could leave it as is for now for -fixes and then switch it
> use use the new exported symbol for -next?  Is it ok to export a new
> symbol for -fixes?
>
>>
>>> +     UEFI_ACPI_VFCT *vfct;
>>> +     GOP_VBIOS_CONTENT *vbios;
>>> +     VFCT_IMAGE_HEADER *vhdr;
>>> +
>>> +     if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
>>> +             return false;
>>> +     if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
>>> +             DRM_ERROR("ACPI VFCT table present but broken (too short #1)\n");
>>> +             goto out_unmap;
>>> +     }
>>> +
>>> +     vfct = (UEFI_ACPI_VFCT *)hdr;
>>> +     if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) > tbl_size) {
>>> +             DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");
>>> +             goto out_unmap;
>>> +     }
>>> +
>>> +     vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + vfct->VBIOSImageOffset);
>>> +     vhdr = &vbios->VbiosHeader;
>>> +     DRM_INFO("ACPI VFCT contains a BIOS for %02x:%02x.%d %04x:%04x, size %d\n",
>>> +                     vhdr->PCIBus, vhdr->PCIDevice, vhdr->PCIFunction,
>>> +                     vhdr->VendorID, vhdr->DeviceID, vhdr->ImageLength);
>>> +
>>> +     if (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) {
>>> +             DRM_INFO("ACPI VFCT table is not for this card\n");
>>> +             goto out_unmap;
>>> +     };
>>> +
>>> +     if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) + vhdr->ImageLength > tbl_size) {
>>> +             DRM_ERROR("ACPI VFCT image truncated\n");
>>> +             goto out_unmap;
>>> +     }
>>> +
>>> +     rdev->bios = kmemdup(&vbios->VbiosContent, vhdr->ImageLength, GFP_KERNEL);
>>> +     ret = !!rdev->bios;
>>> +
>>> +out_unmap:
>>> +     /* uh, no idea what to do here... */
>>
>> So, er, I had no clue how to clean up the return value of acpi_get_table
>> - does this actually need to be cleaned up?  Or do you just get a
>> pointer straight to the "real" ACPI table?
>
> Not sure on that.  Anyone know more about the acpi code?
>
> Alex

Re: [PATCH 1/2] drm/radeon: implement ACPI VFCT vbios fetch (v2)

[Matthew Garrett]

On Fri, Aug 17, 2012 at 12:56:31PM -0400, Alex Deucher wrote:
> > I guess we could leave it as is for now for -fixes and then switch it
> > use use the new exported symbol for -next?  Is it ok to export a new
> > symbol for -fixes?

I don't see why not, providing the ACPI people are happy with it.

> >> So, er, I had no clue how to clean up the return value of acpi_get_table
> >> - does this actually need to be cleaned up?  Or do you just get a
> >> pointer straight to the "real" ACPI table?
> >
> > Not sure on that.  Anyone know more about the acpi code?

Not sure what you're asking here - acpi_get_table returns acpi_status, 
not a pointer.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[PATCH 1/4] drm/radeon: implement ACPI VFCT vbios fetch (v3)

[alexdeucher]

From: David Lamparter <equinox@diac24.net>

This is required for pure UEFI systems.  The vbios is stored
in ACPI rather than at the legacy vga location.

Fixes:
https://bugs.freedesktop.org/show_bug.cgi?id=26891

V2: fix #ifdefs as per Greg's comments
V3: fix it harder

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_bios.c |   60 ++++++++++++++++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
index 501f488..a32232f 100644
--- a/drivers/gpu/drm/radeon/radeon_bios.c
+++ b/drivers/gpu/drm/radeon/radeon_bios.c
@@ -32,6 +32,7 @@
 
 #include <linux/vga_switcheroo.h>
 #include <linux/slab.h>
+#include <linux/acpi.h>
 /*
  * BIOS.
  */
@@ -476,6 +477,63 @@ static bool radeon_read_disabled_bios(struct radeon_device *rdev)
 		return legacy_read_disabled_bios(rdev);
 }
 
+#ifdef CONFIG_ACPI
+static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
+{
+	bool ret = false;
+	struct acpi_table_header *hdr;
+	/* acpi_get_table_with_size is not exported :( */
+	acpi_size tbl_size = 0x7fffffff;
+	UEFI_ACPI_VFCT *vfct;
+	GOP_VBIOS_CONTENT *vbios;
+	VFCT_IMAGE_HEADER *vhdr;
+
+	if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
+		return false;
+	if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
+		DRM_ERROR("ACPI VFCT table present but broken (too short #1)\n");
+		goto out_unmap;
+	}
+
+	vfct = (UEFI_ACPI_VFCT *)hdr;
+	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) > tbl_size) {
+		DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");
+		goto out_unmap;
+	}
+
+	vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + vfct->VBIOSImageOffset);
+	vhdr = &vbios->VbiosHeader;
+	DRM_INFO("ACPI VFCT contains a BIOS for %02x:%02x.%d %04x:%04x, size %d\n",
+			vhdr->PCIBus, vhdr->PCIDevice, vhdr->PCIFunction,
+			vhdr->VendorID, vhdr->DeviceID, vhdr->ImageLength);
+
+	if (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) {
+		DRM_INFO("ACPI VFCT table is not for this card\n");
+		goto out_unmap;
+	};
+
+	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) + vhdr->ImageLength > tbl_size) {
+		DRM_ERROR("ACPI VFCT image truncated\n");
+		goto out_unmap;
+	}
+
+	rdev->bios = kmemdup(&vbios->VbiosContent, vhdr->ImageLength, GFP_KERNEL);
+	ret = !!rdev->bios;
+
+out_unmap:
+	/* uh, no idea what to do here... */
+	return ret;
+}
+#else
+static inline bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
+{
+	return false;
+}
+#endif
 
 bool radeon_get_bios(struct radeon_device *rdev)
 {
@@ -484,6 +542,8 @@ bool radeon_get_bios(struct radeon_device *rdev)
 
 	r = radeon_atrm_get_bios(rdev);
 	if (r == false)
+		r = radeon_acpi_vfct_bios(rdev);
+	if (r == false)
 		r = igp_read_bios_from_vram(rdev);
 	if (r == false)
 		r = radeon_read_bios(rdev);
-- 
1.7.7.5

[PATCH 2/4] ACPI: export symbol acpi_get_table_with_size

[alexdeucher]

From: Alex Deucher <alexander.deucher@amd.com>

We need it in the radeon drm module to fetch
and verify the vbios image on UEFI systems.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
---
 drivers/acpi/acpica/tbxface.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
index ea4c6d5..29e51bc 100644
--- a/drivers/acpi/acpica/tbxface.c
+++ b/drivers/acpi/acpica/tbxface.c
@@ -387,6 +387,7 @@ acpi_get_table_with_size(char *signature,
 
 	return (AE_NOT_FOUND);
 }
+ACPI_EXPORT_SYMBOL(acpi_get_table_with_size)
 
 acpi_status
 acpi_get_table(char *signature,
-- 
1.7.7.5

[PATCH 3/4] drm/radeon: convert radeon vfct code to use acpi_get_table_with_size

[alexdeucher]

From: Alex Deucher <alexander.deucher@amd.com>

Allows us to verify the table size.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_bios.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
index a32232f..ab0b2f7 100644
--- a/drivers/gpu/drm/radeon/radeon_bios.c
+++ b/drivers/gpu/drm/radeon/radeon_bios.c
@@ -482,13 +482,12 @@ static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
 {
 	bool ret = false;
 	struct acpi_table_header *hdr;
-	/* acpi_get_table_with_size is not exported :( */
-	acpi_size tbl_size = 0x7fffffff;
+	acpi_size tbl_size;
 	UEFI_ACPI_VFCT *vfct;
 	GOP_VBIOS_CONTENT *vbios;
 	VFCT_IMAGE_HEADER *vhdr;
 
-	if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
+	if (!ACPI_SUCCESS(acpi_get_table_with_size("VFCT", 1, &hdr, &tbl_size)))
 		return false;
 	if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
 		DRM_ERROR("ACPI VFCT table present but broken (too short #1)\n");
@@ -525,7 +524,6 @@ static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
 	ret = !!rdev->bios;
 
 out_unmap:
-	/* uh, no idea what to do here... */
 	return ret;
 }
 #else
-- 
1.7.7.5

[PATCH 4/4] drm/radeon: split ATRM support out from the ATPX handler (v3)

[alexdeucher]

From: Alex Deucher <alexander.deucher@amd.com>

There are systems that use ATRM, but not ATPX.

Fixes:
https://bugs.freedesktop.org/show_bug.cgi?id=41265

V2: fix #ifdefs as per Greg's comments
V3: fix it harder

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon.h              |   15 -----
 drivers/gpu/drm/radeon/radeon_atpx_handler.c |   56 +------------------
 drivers/gpu/drm/radeon/radeon_bios.c         |   80 ++++++++++++++++++++++++-
 3 files changed, 77 insertions(+), 74 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 9930419..59a1531 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -142,21 +142,6 @@ struct radeon_device;
 /*
  * BIOS.
  */
-#define ATRM_BIOS_PAGE 4096
-
-#if defined(CONFIG_VGA_SWITCHEROO)
-bool radeon_atrm_supported(struct pci_dev *pdev);
-int radeon_atrm_get_bios_chunk(uint8_t *bios, int offset, int len);
-#else
-static inline bool radeon_atrm_supported(struct pci_dev *pdev)
-{
-	return false;
-}
-
-static inline int radeon_atrm_get_bios_chunk(uint8_t *bios, int offset, int len){
-	return -EINVAL;
-}
-#endif
 bool radeon_get_bios(struct radeon_device *rdev);
 
 /*
diff --git a/drivers/gpu/drm/radeon/radeon_atpx_handler.c b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
index 98724fc..2a2cf0b 100644
--- a/drivers/gpu/drm/radeon/radeon_atpx_handler.c
+++ b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
@@ -30,57 +30,8 @@ static struct radeon_atpx_priv {
 	/* handle for device - and atpx */
 	acpi_handle dhandle;
 	acpi_handle atpx_handle;
-	acpi_handle atrm_handle;
 } radeon_atpx_priv;
 
-/* retrieve the ROM in 4k blocks */
-static int radeon_atrm_call(acpi_handle atrm_handle, uint8_t *bios,
-			    int offset, int len)
-{
-	acpi_status status;
-	union acpi_object atrm_arg_elements[2], *obj;
-	struct acpi_object_list atrm_arg;
-	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL};
-
-	atrm_arg.count = 2;
-	atrm_arg.pointer = &atrm_arg_elements[0];
-
-	atrm_arg_elements[0].type = ACPI_TYPE_INTEGER;
-	atrm_arg_elements[0].integer.value = offset;
-
-	atrm_arg_elements[1].type = ACPI_TYPE_INTEGER;
-	atrm_arg_elements[1].integer.value = len;
-
-	status = acpi_evaluate_object(atrm_handle, NULL, &atrm_arg, &buffer);
-	if (ACPI_FAILURE(status)) {
-		printk("failed to evaluate ATRM got %s\n", acpi_format_exception(status));
-		return -ENODEV;
-	}
-
-	obj = (union acpi_object *)buffer.pointer;
-	memcpy(bios+offset, obj->buffer.pointer, obj->buffer.length);
-	len = obj->buffer.length;
-	kfree(buffer.pointer);
-	return len;
-}
-
-bool radeon_atrm_supported(struct pci_dev *pdev)
-{
-	/* get the discrete ROM only via ATRM */
-	if (!radeon_atpx_priv.atpx_detected)
-		return false;
-
-	if (radeon_atpx_priv.dhandle == DEVICE_ACPI_HANDLE(&pdev->dev))
-		return false;
-	return true;
-}
-
-
-int radeon_atrm_get_bios_chunk(uint8_t *bios, int offset, int len)
-{
-	return radeon_atrm_call(radeon_atpx_priv.atrm_handle, bios, offset, len);
-}
-
 static int radeon_atpx_get_version(acpi_handle handle)
 {
 	acpi_status status;
@@ -198,7 +149,7 @@ static int radeon_atpx_power_state(enum vga_switcheroo_client_id id,
 
 static bool radeon_atpx_pci_probe_handle(struct pci_dev *pdev)
 {
-	acpi_handle dhandle, atpx_handle, atrm_handle;
+	acpi_handle dhandle, atpx_handle;
 	acpi_status status;
 
 	dhandle = DEVICE_ACPI_HANDLE(&pdev->dev);
@@ -209,13 +160,8 @@ static bool radeon_atpx_pci_probe_handle(struct pci_dev *pdev)
 	if (ACPI_FAILURE(status))
 		return false;
 
-	status = acpi_get_handle(dhandle, "ATRM", &atrm_handle);
-	if (ACPI_FAILURE(status))
-		return false;
-
 	radeon_atpx_priv.dhandle = dhandle;
 	radeon_atpx_priv.atpx_handle = atpx_handle;
-	radeon_atpx_priv.atrm_handle = atrm_handle;
 	return true;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
index ab0b2f7..d306cc8 100644
--- a/drivers/gpu/drm/radeon/radeon_bios.c
+++ b/drivers/gpu/drm/radeon/radeon_bios.c
@@ -99,16 +99,81 @@ static bool radeon_read_bios(struct radeon_device *rdev)
 	return true;
 }
 
+#ifdef CONFIG_ACPI
 /* ATRM is used to get the BIOS on the discrete cards in
  * dual-gpu systems.
  */
+/* retrieve the ROM in 4k blocks */
+#define ATRM_BIOS_PAGE 4096
+/**
+ * radeon_atrm_call - fetch a chunk of the vbios
+ *
+ * @atrm_handle: acpi ATRM handle
+ * @bios: vbios image pointer
+ * @offset: offset of vbios image data to fetch
+ * @len: length of vbios image data to fetch
+ *
+ * Executes ATRM to fetch a chunk of the discrete
+ * vbios image on PX systems (all asics).
+ * Returns the length of the buffer fetched.
+ */
+static int radeon_atrm_call(acpi_handle atrm_handle, uint8_t *bios,
+			    int offset, int len)
+{
+	acpi_status status;
+	union acpi_object atrm_arg_elements[2], *obj;
+	struct acpi_object_list atrm_arg;
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL};
+
+	atrm_arg.count = 2;
+	atrm_arg.pointer = &atrm_arg_elements[0];
+
+	atrm_arg_elements[0].type = ACPI_TYPE_INTEGER;
+	atrm_arg_elements[0].integer.value = offset;
+
+	atrm_arg_elements[1].type = ACPI_TYPE_INTEGER;
+	atrm_arg_elements[1].integer.value = len;
+
+	status = acpi_evaluate_object(atrm_handle, NULL, &atrm_arg, &buffer);
+	if (ACPI_FAILURE(status)) {
+		printk("failed to evaluate ATRM got %s\n", acpi_format_exception(status));
+		return -ENODEV;
+	}
+
+	obj = (union acpi_object *)buffer.pointer;
+	memcpy(bios+offset, obj->buffer.pointer, obj->buffer.length);
+	len = obj->buffer.length;
+	kfree(buffer.pointer);
+	return len;
+}
+
 static bool radeon_atrm_get_bios(struct radeon_device *rdev)
 {
 	int ret;
 	int size = 256 * 1024;
 	int i;
+	struct pci_dev *pdev = NULL;
+	acpi_handle dhandle, atrm_handle;
+	acpi_status status;
+	bool found = false;
 
-	if (!radeon_atrm_supported(rdev->pdev))
+	/* ATRM is for the discrete card only */
+	if (rdev->flags & RADEON_IS_IGP)
+		return false;
+
+	while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev)) != NULL) {
+		dhandle = DEVICE_ACPI_HANDLE(&pdev->dev);
+		if (!dhandle)
+			continue;
+
+		status = acpi_get_handle(dhandle, "ATRM", &atrm_handle);
+		if (!ACPI_FAILURE(status)) {
+			found = true;
+			break;
+		}
+	}
+
+	if (!found)
 		return false;
 
 	rdev->bios = kmalloc(size, GFP_KERNEL);
@@ -118,9 +183,10 @@ static bool radeon_atrm_get_bios(struct radeon_device *rdev)
 	}
 
 	for (i = 0; i < size / ATRM_BIOS_PAGE; i++) {
-		ret = radeon_atrm_get_bios_chunk(rdev->bios,
-						 (i * ATRM_BIOS_PAGE),
-						 ATRM_BIOS_PAGE);
+		ret = radeon_atrm_call(atrm_handle,
+				       rdev->bios,
+				       (i * ATRM_BIOS_PAGE),
+				       ATRM_BIOS_PAGE);
 		if (ret < ATRM_BIOS_PAGE)
 			break;
 	}
@@ -131,6 +197,12 @@ static bool radeon_atrm_get_bios(struct radeon_device *rdev)
 	}
 	return true;
 }
+#else
+static inline bool radeon_atrm_get_bios(struct radeon_device *rdev)
+{
+	return false;
+}
+#endif
 
 static bool ni_read_disabled_bios(struct radeon_device *rdev)
 {
-- 
1.7.7.5

Re: [PATCH 2/4] ACPI: export symbol acpi_get_table_with_size

[Alex Deucher]

Any objections from the ACPI folks to this patch going into 3.6 and stable?

Alex

On Mon, Aug 20, 2012 at 11:19 AM,  <alexdeucher@gmail.com> wrote:
> From: Alex Deucher <alexander.deucher@amd.com>
>
> We need it in the radeon drm module to fetch
> and verify the vbios image on UEFI systems.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/acpi/acpica/tbxface.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
> index ea4c6d5..29e51bc 100644
> --- a/drivers/acpi/acpica/tbxface.c
> +++ b/drivers/acpi/acpica/tbxface.c
> @@ -387,6 +387,7 @@ acpi_get_table_with_size(char *signature,
>
>         return (AE_NOT_FOUND);
>  }
> +ACPI_EXPORT_SYMBOL(acpi_get_table_with_size)
>
>  acpi_status
>  acpi_get_table(char *signature,
> --
> 1.7.7.5
>

Re: [PATCH 2/4] ACPI: export symbol acpi_get_table_with_size

[Matthew Garrett]

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/4] ACPI: export symbol acpi_get_table_with_size

[Matthew Garrett]

On Tue, Aug 21, 2012 at 10:50:35AM -0400, Alex Deucher wrote:
> Any objections from the ACPI folks to this patch going into 3.6 and stable?

Looks good to me.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/4] ACPI: export symbol acpi_get_table_with_size

[Lan Tianyu]

2012/8/21 Alex Deucher <alexdeucher@gmail.com>:
> Any objections from the ACPI folks to this patch going into 3.6 and stable?
>
hi Alex:
                I saw the patch from Feng Tang.
http://marc.info/?l=linux-acpi&m=134380363332502&w=2
which is trying to replace acpi_get_table_with_size() with
acpi_get_table(). Since the size can be get
via struct acpi_table_header->length, acpi_get_table_with_size() is
redundant and it will be removed.
> Alex
>
> On Mon, Aug 20, 2012 at 11:19 AM,  <alexdeucher@gmail.com> wrote:
>> From: Alex Deucher <alexander.deucher@amd.com>
>>
>> We need it in the radeon drm module to fetch
>> and verify the vbios image on UEFI systems.
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> Cc: stable@vger.kernel.org
>> ---
>>  drivers/acpi/acpica/tbxface.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
>> index ea4c6d5..29e51bc 100644
>> --- a/drivers/acpi/acpica/tbxface.c
>> +++ b/drivers/acpi/acpica/tbxface.c
>> @@ -387,6 +387,7 @@ acpi_get_table_with_size(char *signature,
>>
>>         return (AE_NOT_FOUND);
>>  }
>> +ACPI_EXPORT_SYMBOL(acpi_get_table_with_size)
>>
>>  acpi_status
>>  acpi_get_table(char *signature,
>> --
>> 1.7.7.5
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best regards
Tianyu Lan