Re: [PATCH] radeon_cp: use request_firmware

From: Ben Hutchings
Date: Tue Dec 23 2008 - 10:22:25 EST

On Mon, 2008-12-22 at 11:39 +0100, Julien Cristau wrote:
> On Sun, Dec 21, 2008 at 20:58:28 +0000, Dave Airlie wrote:
>
> >
> > >
> > > Is available at
> > > http://git.infradead.org/users/jaswinder/firm-jsr-2.6.git?a=commit;h=3a911a216742e4ab998f3281409d46a62f252716
> > >
> > >
> > > Please let me know, should I need to resend this patch for :
> > > 1. git kernel.org:/pub/scm/linux/kernel/git/airlied/drm-2.6.git
> > > OR
> > > 2. git kernel.org:/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> >
> > How much testing has this seen on what range of cards and on what
> > architectures? I'd like some success stories from people using this patch.
> > Is Debian shipping it?
> >
> Not at this time. AFAIK Ben Hutchings has tested that patch, although I
> don't know on what hardware, and if he got other testers. Ben?

I had to revise Jaswinder's patch as it allowed a double-free. I'm
attaching the version I've tested (which is against v2.6.26 so the paths
are different). That was tested on a Radeon Mobility 7500 (R100 if I
remember correctly).

Ben.

--
Ben Hutchings
Unix is many things to many people,
but it's never been everything to anybody.
commit 61114ee8e98e86b3160d547b6dbe7ec7bac445f5
Author: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
Date: Wed Oct 15 01:29:35 2008 +0100

radeon: Use request_firmware() to load CP microcode

Tested on Radeon 7500 (RV200) with and without firmware installed.

diff --git a/drivers/char/drm/Kconfig b/drivers/char/drm/Kconfig
index 3b0dd6a..17edd8a 100644
--- a/drivers/char/drm/Kconfig
+++ b/drivers/char/drm/Kconfig
@@ -35,7 +35,7 @@ config DRM_R128
config DRM_RADEON
tristate "ATI Radeon"
depends on DRM && PCI
- depends on BROKEN
+ select FW_LOADER
help
Choose this option if you have an ATI Radeon graphics card. There
are both PCI and AGP versions. You don't need to choose this to
diff --git a/drivers/char/drm/radeon_cp.c b/drivers/char/drm/radeon_cp.c
index e53158f..2670dd3 100644
--- a/drivers/char/drm/radeon_cp.c
+++ b/drivers/char/drm/radeon_cp.c
@@ -35,10 +35,23 @@
#include "radeon_drv.h"
#include "r300_reg.h"

-#include "radeon_microcode.h"
-
#define RADEON_FIFO_DEBUG 0

+/* Firmware Names */
+#define FIRMWARE_R100 "radeon/R100_cp.bin"
+#define FIRMWARE_R200 "radeon/R200_cp.bin"
+#define FIRMWARE_R300 "radeon/R300_cp.bin"
+#define FIRMWARE_R420 "radeon/R420_cp.bin"
+#define FIRMWARE_RS690 "radeon/RS690_cp.bin"
+#define FIRMWARE_R520 "radeon/R520_cp.bin"
+
+MODULE_FIRMWARE(FIRMWARE_R100);
+MODULE_FIRMWARE(FIRMWARE_R200);
+MODULE_FIRMWARE(FIRMWARE_R300);
+MODULE_FIRMWARE(FIRMWARE_R420);
+MODULE_FIRMWARE(FIRMWARE_RS690);
+MODULE_FIRMWARE(FIRMWARE_R520);
+
static int radeon_do_cleanup_cp(struct drm_device * dev);

static u32 R500_READ_MCIND(drm_radeon_private_t *dev_priv, int addr)
@@ -320,66 +333,48 @@ static void radeon_init_pipes(drm_radeon_private_t *dev_priv)
*/

/* Load the microcode for the CP */
-static void radeon_cp_load_microcode(drm_radeon_private_t * dev_priv)
+static int radeon_cp_init_microcode(drm_radeon_private_t *dev_priv)
{
- int i;
+ struct platform_device *pdev;
+ const char *fw_name = NULL;
+ int err;
+
DRM_DEBUG("\n");

- radeon_do_wait_for_idle(dev_priv);
+ pdev = platform_device_register_simple("radeon_cp", 0, NULL, 0);
+ err = IS_ERR(pdev);
+ if (err) {
+ printk(KERN_ERR "radeon_cp: Failed to register firmware\n");
+ return -EINVAL;
+ }

- RADEON_WRITE(RADEON_CP_ME_RAM_ADDR, 0);
if (((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_R100) ||
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RV100) ||
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RV200) ||
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RS100) ||
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RS200)) {
DRM_INFO("Loading R100 Microcode\n");
- for (i = 0; i < 256; i++) {
- RADEON_WRITE(RADEON_CP_ME_RAM_DATAH,
- R100_cp_microcode[i][1]);
- RADEON_WRITE(RADEON_CP_ME_RAM_DATAL,
- R100_cp_microcode[i][0]);
- }
+ fw_name = FIRMWARE_R100;
} else if (((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_R200) ||
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RV250) ||
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RV280) ||
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RS300)) {
DRM_INFO("Loading R200 Microcode\n");
- for (i = 0; i < 256; i++) {
- RADEON_WRITE(RADEON_CP_ME_RAM_DATAH,
- R200_cp_microcode[i][1]);
- RADEON_WRITE(RADEON_CP_ME_RAM_DATAL,
- R200_cp_microcode[i][0]);
- }
+ fw_name = FIRMWARE_R200;
} else if (((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_R300) ||
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_R350) ||
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RV350) ||
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RV380) ||
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RS480)) {
DRM_INFO("Loading R300 Microcode\n");
- for (i = 0; i < 256; i++) {
- RADEON_WRITE(RADEON_CP_ME_RAM_DATAH,
- R300_cp_microcode[i][1]);
- RADEON_WRITE(RADEON_CP_ME_RAM_DATAL,
- R300_cp_microcode[i][0]);
- }
+ fw_name = FIRMWARE_R300;
} else if (((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_R420) ||
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RV410)) {
DRM_INFO("Loading R400 Microcode\n");
- for (i = 0; i < 256; i++) {
- RADEON_WRITE(RADEON_CP_ME_RAM_DATAH,
- R420_cp_microcode[i][1]);
- RADEON_WRITE(RADEON_CP_ME_RAM_DATAL,
- R420_cp_microcode[i][0]);
- }
+ fw_name = FIRMWARE_R420;
} else if ((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RS690) {
DRM_INFO("Loading RS690 Microcode\n");
- for (i = 0; i < 256; i++) {
- RADEON_WRITE(RADEON_CP_ME_RAM_DATAH,
- RS690_cp_microcode[i][1]);
- RADEON_WRITE(RADEON_CP_ME_RAM_DATAL,
- RS690_cp_microcode[i][0]);
- }
+ fw_name = FIRMWARE_RS690;
} else if (((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RV515) ||
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_R520) ||
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RV530) ||
@@ -387,11 +382,41 @@ static void radeon_cp_load_microcode(drm_radeon_private_t * dev_priv)
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RV560) ||
((dev_priv->flags & RADEON_FAMILY_MASK) == CHIP_RV570)) {
DRM_INFO("Loading R500 Microcode\n");
- for (i = 0; i < 256; i++) {
+ fw_name = FIRMWARE_R520;
+ }
+
+ err = request_firmware(&dev_priv->fw, fw_name, &pdev->dev);
+ platform_device_unregister(pdev);
+ if (err) {
+ printk(KERN_ERR "radeon_cp: Failed to load firmware \"%s\"\n",
+ fw_name);
+ } else if (dev_priv->fw->size % 8) {
+ printk(KERN_ERR
+ "radeon_cp: Bogus length %zu in firmware \"%s\"\n",
+ dev_priv->fw->size, fw_name);
+ err = -EINVAL;
+ release_firmware(dev_priv->fw);
+ dev_priv->fw = NULL;
+ }
+ return err;
+}
+
+static void radeon_cp_load_microcode(drm_radeon_private_t *dev_priv)
+{
+ const __be32 *fw_data;
+ int i, size;
+
+ radeon_do_wait_for_idle(dev_priv);
+
+ if (dev_priv->fw) {
+ size = dev_priv->fw->size / 4;
+ fw_data = (const __be32 *)&dev_priv->fw->data[0];
+ RADEON_WRITE(RADEON_CP_ME_RAM_ADDR, 0);
+ for (i = 0; i < size; i += 2) {
RADEON_WRITE(RADEON_CP_ME_RAM_DATAH,
- R520_cp_microcode[i][1]);
+ be32_to_cpup(&fw_data[i]));
RADEON_WRITE(RADEON_CP_ME_RAM_DATAL,
- R520_cp_microcode[i][0]);
+ be32_to_cpup(&fw_data[i + 1]));
}
}
}
@@ -1195,6 +1220,14 @@ static int radeon_do_init_cp(struct drm_device * dev, drm_radeon_init_t * init)
radeon_set_pcigart(dev_priv, 1);
}

+ if (!dev_priv->fw) {
+ int err = radeon_cp_init_microcode(dev_priv);
+ if (err) {
+ DRM_ERROR("Failed to load firmware!\n");
+ radeon_do_cleanup_cp(dev);
+ return err;
+ }
+ }
radeon_cp_load_microcode(dev_priv);
radeon_cp_init_ring_buffer(dev, dev_priv);

@@ -1421,6 +1454,10 @@ void radeon_do_release(struct drm_device * dev)

/* deallocate kernel resources */
radeon_do_cleanup_cp(dev);
+ if (dev_priv->fw) {
+ release_firmware(dev_priv->fw);
+ dev_priv->fw = NULL;
+ }
}
}

diff --git a/drivers/char/drm/radeon_drv.h b/drivers/char/drm/radeon_drv.h
index 3f0eca9..5cc808e 100644
--- a/drivers/char/drm/radeon_drv.h
+++ b/drivers/char/drm/radeon_drv.h
@@ -31,6 +31,9 @@
#ifndef __RADEON_DRV_H__
#define __RADEON_DRV_H__

+#include <linux/firmware.h>
+#include <linux/platform_device.h>
+
/* General customization:
*/

@@ -311,6 +314,9 @@ typedef struct drm_radeon_private {
unsigned long fb_aper_offset;

int num_gb_pipes;
+
+ /* firmware */
+ const struct firmware *fw;
} drm_radeon_private_t;

typedef struct drm_radeon_buf_priv {

Attachment: signature.asc
Description: This is a digitally signed message part