Re: [PATCH] radeon_cp: use request_firmware

[Ben Hutchings <ben@decadent.org.uk>]

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

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.

[-- Attachment #1.2: drivers-char-drm-radeon-request_firmware.patch --]
[-- Type: text/x-patch, Size: 7723 bytes --]

commit 61114ee8e98e86b3160d547b6dbe7ec7bac445f5
Author: Ben Hutchings <ben@decadent.org.uk>
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 #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]