Re: [PATCH] agpgart: Reprobe VGA devices when a new GART device is added

2010-03-01 Thread Ben Hutchings 
On Mon, Mar 01, 2010 at 08:19:02AM -0800, Eric Anholt wrote:
> On Mon, 1 Mar 2010 18:33:14 +1000, Dave Airlie  wrote:
> > On Sun, Feb 28, 2010 at 1:30 PM, Ben Hutchings  wrote:
> > > This addresses <http://bugzilla.kernel.org/show_bug.cgi?id=15021>.
> > >
> > > DRM drivers may fail to probe their devices when the associated AGP
> > > GART does not yet have a driver, so we reprobe unbound VGA devices
> > > when a GART device is added.
> > >
> > > Signed-off-by: Ben Hutchings 
> > > ---
> > > This is intended to address the dependency problem highlighted in the
> > > above bug report.  That specific bug can be fixed by adding a module
> > > dependency from i915 to intel_agp, but in general there is no fixed
> > > relationship betweem DRM and GART drivers.
> > >
> > > There is a narrow race between the check for a current driver binding
> > > and the call to device_reprobe().  This could be closed by adding a
> > > variant on device_attach() and device_reprobe() that is a no-op for
> > > bound devices.
> > >
> > 
> > This isn't useful, generally there is no AGP binding, and most drivers
> > if they can't find an AGP backend will still run fine without it. i.e. 
> > radeon,
> > mga etc.

I see, only the Intel GPU drivers set DRIVER_REQUIRE_AGP.

> > Intel is a special case and I think we've already merged an explicit
> > depend.
> > 
> > Just build agp drivers into the kernel, I'm tempted to make them all
> > non-modular.
> 
> That seems easier.

Easier, yes, but it's a fair amount of bloat for i386 kernels (less so for
x86-64) since there are many different GART drivers.  I was hoping to avoid
that.

Ben.

-- 
Ben Hutchings
Life is like a sewer:
what you get out of it depends on what you put into it.

--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: [PATCH] agpgart: Reprobe VGA devices when a new GART device is added

2010-03-01 Thread Ben Hutchings 
On Sun, 2010-02-28 at 03:30 +, Ben Hutchings wrote:
> +static void agp_probe_video(struct work_struct *work)
> +{
> + struct pci_dev *pdev = NULL;
> +
> + while ((pdev = pci_get_class(0x03, pdev)) != NULL) {

Only without the opening brace here...

Ben.

> + if (!pdev->dev.driver && device_reprobe(&pdev->dev))
> + pr_err(PFX "failed to reprobe %s\n", pci_name(pdev));
> +}

-- 
Ben Hutchings
Horngren's Observation:
   Among economists, the real world is often a special case.


signature.asc
Description: This is a digitally signed message part
--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

[PATCH] agpgart: Reprobe VGA devices when a new GART device is added

2010-03-01 Thread Ben Hutchings 
This addresses <http://bugzilla.kernel.org/show_bug.cgi?id=15021>.

DRM drivers may fail to probe their devices when the associated AGP
GART does not yet have a driver, so we reprobe unbound VGA devices
when a GART device is added.

Signed-off-by: Ben Hutchings 
---
This is intended to address the dependency problem highlighted in the
above bug report.  That specific bug can be fixed by adding a module
dependency from i915 to intel_agp, but in general there is no fixed
relationship betweem DRM and GART drivers.

There is a narrow race between the check for a current driver binding
and the call to device_reprobe().  This could be closed by adding a
variant on device_attach() and device_reprobe() that is a no-op for
bound devices.

Ben.

 drivers/char/agp/backend.c |   20 
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/drivers/char/agp/backend.c b/drivers/char/agp/backend.c
index c3ab46d..f9680bf 100644
--- a/drivers/char/agp/backend.c
+++ b/drivers/char/agp/backend.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include "agp.h"
 
@@ -281,6 +282,24 @@ void agp_put_bridge(struct agp_bridge_data *bridge)
 EXPORT_SYMBOL(agp_put_bridge);
 

+/*
+ * DRM drivers may fail to probe their devices when the associated AGP
+ * GART does not yet have a driver, so we reprobe unbound VGA devices
+ * when a GART device is added.  This problem applies not only to true
+ * AGP devices which would be children of the affected bridge, but
+ * also to PCI Express devices that may be siblings of the GART
+ * device.  Therefore iterate over all PCI VGA devices.
+ */
+static void agp_probe_video(struct work_struct *work)
+{
+   struct pci_dev *pdev = NULL;
+
+   while ((pdev = pci_get_class(0x03, pdev)) != NULL) {
+   if (!pdev->dev.driver && device_reprobe(&pdev->dev))
+   pr_err(PFX "failed to reprobe %s\n", pci_name(pdev));
+}
+static DECLARE_WORK(agp_probe_video_work, agp_probe_video);
+
 int agp_add_bridge(struct agp_bridge_data *bridge)
 {
int error;
@@ -324,6 +343,7 @@ int agp_add_bridge(struct agp_bridge_data *bridge)
}
 
list_add(&bridge->list, &agp_bridges);
+   schedule_work(&agp_probe_video_work);
return 0;
 
 frontend_err:
-- 
1.6.6.2



--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

[PATCH 2/3] r128: Use request_firmware() to load CCE microcode

2009-08-23 Thread Ben Hutchings 
Firmware blob looks like this:
__be32 datah
__be32 datal

Signed-off-by: Ben Hutchings 
---
This has been tested in Debian as a change to 2.6.30.  Note that
initialisation can fail as a result of missing firmware, and my previous
patch adding initialisation tests to more ioctl implementations should
be applied first.

Ben.

 drivers/gpu/drm/Kconfig |1 +
 drivers/gpu/drm/r128/r128_cce.c |   98 ++---
 firmware/Makefile   |1 +
 firmware/WHENCE |   31 +
 firmware/r128/r128_cce.bin.ihex |  129 +++
 5 files changed, 210 insertions(+), 50 deletions(-)
 create mode 100644 firmware/r128/r128_cce.bin.ihex

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 10edc9b..a07abb8 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -36,6 +36,7 @@ config DRM_TDFX
 config DRM_R128
tristate "ATI Rage 128"
depends on DRM && PCI
+   select FW_LOADER
help
  Choose this option if you have an ATI Rage 128 graphics card.  If M
  is selected, the module will be called r128.  AGP support for
diff --git a/drivers/gpu/drm/r128/r128_cce.c b/drivers/gpu/drm/r128/r128_cce.c
index c75fd35..15252f6 100644
--- a/drivers/gpu/drm/r128/r128_cce.c
+++ b/drivers/gpu/drm/r128/r128_cce.c
@@ -29,6 +29,9 @@
  *Gareth Hughes 
  */
 
+#include 
+#include 
+
 #include "drmP.h"
 #include "drm.h"
 #include "r128_drm.h"
@@ -36,50 +39,9 @@
 
 #define R128_FIFO_DEBUG0
 
-/* CCE microcode (from ATI) */
-static u32 r128_cce_microcode[] = {
-   0, 276838400, 0, 268449792, 2, 142, 2, 145, 0, 1076765731, 0,
-   1617039951, 0, 774592877, 0, 1987540286, 0, 2307490946U, 0,
-   599558925, 0, 589505315, 0, 596487092, 0, 589505315, 1,
-   11544576, 1, 206848, 1, 311296, 1, 198656, 2, 912273422, 11,
-   262144, 0, 0, 1, 33559837, 1, 7438, 1, 14809, 1, 6615, 12, 28,
-   1, 6614, 12, 28, 2, 23, 11, 18874368, 0, 16790922, 1, 409600, 9,
-   30, 1, 147854772, 16, 420483072, 3, 8192, 0, 10240, 1, 198656,
-   1, 15630, 1, 51200, 10, 34858, 9, 42, 1, 33559823, 2, 10276, 1,
-   15717, 1, 15718, 2, 43, 1, 15936948, 1, 570480831, 1, 14715071,
-   12, 322123831, 1, 33953125, 12, 55, 1, 33559908, 1, 15718, 2,
-   46, 4, 2099258, 1, 526336, 1, 442623, 4, 4194365, 1, 509952, 1,
-   459007, 3, 0, 12, 92, 2, 46, 12, 176, 1, 15734, 1, 206848, 1,
-   18432, 1, 133120, 1, 100670734, 1, 149504, 1, 165888, 1,
-   15975928, 1, 1048576, 6, 3145806, 1, 15715, 16, 2150645232U, 2,
-   268449859, 2, 10307, 12, 176, 1, 15734, 1, 15735, 1, 15630, 1,
-   15631, 1, 5253120, 6, 3145810, 16, 2150645232U, 1, 15864, 2, 82,
-   1, 343310, 1, 1064207, 2, 3145813, 1, 15728, 1, 7817, 1, 15729,
-   3, 15730, 12, 92, 2, 98, 1, 16168, 1, 16167, 1, 16002, 1, 16008,
-   1, 15974, 1, 15975, 1, 15990, 1, 15976, 1, 15977, 1, 15980, 0,
-   15981, 1, 10240, 1, 5253120, 1, 15720, 1, 198656, 6, 110, 1,
-   180224, 1, 103824738, 2, 112, 2, 3145839, 0, 536885440, 1,
-   114880, 14, 125, 12, 206975, 1, 33559995, 12, 198784, 0,
-   33570236, 1, 15803, 0, 15804, 3, 294912, 1, 294912, 3, 442370,
-   1, 11544576, 0, 811612160, 1, 12593152, 1, 11536384, 1,
-   14024704, 7, 310382726, 0, 10240, 1, 14796, 1, 14797, 1, 14793,
-   1, 14794, 0, 14795, 1, 268679168, 1, 9437184, 1, 268449792, 1,
-   198656, 1, 9452827, 1, 1075854602, 1, 1075854603, 1, 557056, 1,
-   114880, 14, 159, 12, 198784, 1, 1109409213, 12, 198783, 1,
-   1107312059, 12, 198784, 1, 1109409212, 2, 162, 1, 1075854781, 1,
-   1073757627, 1, 1075854780, 1, 540672, 1, 10485760, 6, 3145894,
-   16, 274741248, 9, 168, 3, 4194304, 3, 4209949, 0, 0, 0, 256, 14,
-   174, 1, 114857, 1, 33560007, 12, 176, 0, 10240, 1, 114858, 1,
-   33560018, 1, 114857, 3, 33560007, 1, 16008, 1, 114874, 1,
-   33560360, 1, 114875, 1, 33560154, 0, 15963, 0, 256, 0, 4096, 1,
-   409611, 9, 188, 0, 10240, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
-};
+#define FIRMWARE_NAME  "r128/r128_cce.bin"
+
+MODULE_FIRMWARE(FIRMWARE_NAME);
 
 static int R128_READ_PLL(struct drm_device * dev, int addr)
 {
@@ -176,20 +138,50 @@ static int r128_do_wait_for_idle(drm_r128_private_t * 
dev_priv)
  */
 
 /* Load the microcode for the CCE */
-static void r128_cce_load_microcode(drm_r128_private_t * dev_priv)
+static int r128_cce_load_microcode(drm_r128_private_t *dev_priv)
 {
-   int i;
+   struc

[PATCH] r128: Add test for initialisation to all ioctls that require it

2009-08-23 Thread Ben Hutchings 
Almost all r128's private ioctls require that the CCE state has
already been initialised.  However, most do not test that this has
been done, and will proceed to dereference a null pointer.  This may
result in a security vulnerability, since some ioctls are
unprivileged.

This adds a macro for the common initialisation test and changes all
ioctl implementations that require prior initialisation to use that
macro.

Also, r128_do_init_cce() does not test that the CCE state has not
been initialised already.  Repeated initialisation may lead to a crash
or resource leak.  This adds that test.

Signed-off-by: Ben Hutchings 
---
 drivers/gpu/drm/r128/r128_cce.c   |   18 ++
 drivers/gpu/drm/r128/r128_drv.h   |8 
 drivers/gpu/drm/r128/r128_state.c |   36 +++-
 3 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/r128/r128_cce.c b/drivers/gpu/drm/r128/r128_cce.c
index c75fd35..ebf9f63 100644
--- a/drivers/gpu/drm/r128/r128_cce.c
+++ b/drivers/gpu/drm/r128/r128_cce.c
@@ -353,6 +353,11 @@ static int r128_do_init_cce(struct drm_device * dev, 
drm_r128_init_t * init)
 
DRM_DEBUG("\n");
 
+   if (dev->dev_private) {
+   DRM_DEBUG("called when already initialized\n");
+   return -EINVAL;
+   }
+
dev_priv = kzalloc(sizeof(drm_r128_private_t), GFP_KERNEL);
if (dev_priv == NULL)
return -ENOMEM;
@@ -649,6 +654,8 @@ int r128_cce_start(struct drm_device *dev, void *data, 
struct drm_file *file_pri
 
LOCK_TEST_WITH_RETURN(dev, file_priv);
 
+   DEV_INIT_TEST_WITH_RETURN(dev_priv);
+
if (dev_priv->cce_running || dev_priv->cce_mode == R128_PM4_NONPM4) {
DRM_DEBUG("while CCE running\n");
return 0;
@@ -671,6 +678,8 @@ int r128_cce_stop(struct drm_device *dev, void *data, 
struct drm_file *file_priv
 
LOCK_TEST_WITH_RETURN(dev, file_priv);
 
+   DEV_INIT_TEST_WITH_RETURN(dev_priv);
+
/* Flush any pending CCE commands.  This ensures any outstanding
 * commands are exectuted by the engine before we turn it off.
 */
@@ -708,10 +717,7 @@ int r128_cce_reset(struct drm_device *dev, void *data, 
struct drm_file *file_pri
 
LOCK_TEST_WITH_RETURN(dev, file_priv);
 
-   if (!dev_priv) {
-   DRM_DEBUG("called before init done\n");
-   return -EINVAL;
-   }
+   DEV_INIT_TEST_WITH_RETURN(dev_priv);
 
r128_do_cce_reset(dev_priv);
 
@@ -728,6 +734,8 @@ int r128_cce_idle(struct drm_device *dev, void *data, 
struct drm_file *file_priv
 
LOCK_TEST_WITH_RETURN(dev, file_priv);
 
+   DEV_INIT_TEST_WITH_RETURN(dev_priv);
+
if (dev_priv->cce_running) {
r128_do_cce_flush(dev_priv);
}
@@ -741,6 +749,8 @@ int r128_engine_reset(struct drm_device *dev, void *data, 
struct drm_file *file_
 
LOCK_TEST_WITH_RETURN(dev, file_priv);
 
+   DEV_INIT_TEST_WITH_RETURN(dev->dev_private);
+
return r128_do_engine_reset(dev);
 }
 
diff --git a/drivers/gpu/drm/r128/r128_drv.h b/drivers/gpu/drm/r128/r128_drv.h
index 797a26c..3c60829 100644
--- a/drivers/gpu/drm/r128/r128_drv.h
+++ b/drivers/gpu/drm/r128/r128_drv.h
@@ -422,6 +422,14 @@ static __inline__ void 
r128_update_ring_snapshot(drm_r128_private_t * dev_priv)
  * Misc helper macros
  */
 
+#define DEV_INIT_TEST_WITH_RETURN(_dev_priv)   \
+do {   \
+   if (!_dev_priv) {   \
+   DRM_ERROR("called with no initialization\n");   \
+   return -EINVAL; \
+   }   \
+} while (0)
+
 #define RING_SPACE_TEST_WITH_RETURN( dev_priv )
\
 do {   \
drm_r128_ring_buffer_t *ring = &dev_priv->ring; int i;  \
diff --git a/drivers/gpu/drm/r128/r128_state.c 
b/drivers/gpu/drm/r128/r128_state.c
index 026a48c..af2665c 100644
--- a/drivers/gpu/drm/r128/r128_state.c
+++ b/drivers/gpu/drm/r128/r128_state.c
@@ -1244,14 +1244,18 @@ static void r128_cce_dispatch_stipple(struct drm_device 
* dev, u32 * stipple)
 static int r128_cce_clear(struct drm_device *dev, void *data, struct drm_file 
*file_priv)
 {
drm_r128_private_t *dev_priv = dev->dev_private;
-   drm_r128_sarea_t *sarea_priv = dev_priv->sarea_priv;
+   drm_r128_sarea_t *sarea_priv;
drm_r128_clear_t *clear = data;
DRM_DEBUG("\n");
 
LOCK_TEST_WITH_RETURN(dev, file_priv);
 
+   DEV_INIT_TEST_WITH_RETURN(dev_priv);
+
RING_SPACE_TEST_WITH_RETURN(dev_priv);
 
+   sarea_priv = dev_priv->sarea_

Re: [PATCH] radeon_cp: use request_firmware

2009-04-14 Thread Ben Hutchings 
On Tue, Apr 14, 2009 at 10:24:41AM +0530, Jaswinder Singh Rajput wrote:
> On Mon, 2009-04-13 at 19:26 +0100, Ben Hutchings wrote:
> > Please use  for all mail related to my work on
> > firmware separation, which has nothing to do with my employment.
> > 
> > On Mon, 2009-04-13 at 21:57 +0530, Jaswinder Singh Rajput wrote:
> > > Moved datah before datal because datah is required before datal.
> > > 
> > > Firmware blob looks like this...
> > >   __be32 datah
> > >   __be32 datal
> > > 
> > > Thanks to Ben for help and testing.
> > [...]
> > 
> > This is out of date as there are now an r600_microcode.h and r600_cp.c.
> > My latest patch (not posted anywhere yet) is at
> > <http://womble.decadent.org.uk/tmp/fw-patch/0003-radeon-Use-request_firmware.patch>
> >  (patches 0001-0003 should be applicable in order to drm-2.6/drm-linus).  
> > But I don't have any R600/R700 hardware to test it on.
> > 
> 
> I am very surprise you take my patches and post them without CC to me ?

As I said, I haven't posted those yet.

> Please use my signed-off in my patches in first place:
> 
> Signed-off-by: Jaswinder Singh Rajput 
> 
> Or prepare incremental patch to my patch.
> 
> Here are other stolen patches without CC and my Signed-off:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=06e1f9ffa023c030bc87491e75f625f5da4e7d97

So far as I can remember (I haven't checked) your patch was defective and
I considered that to be a complete rewrite.

> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=b775a750c3afacbfac884537d466d34d50b1023b

That was credited and cc'd to  which maybe isn't the
best address to use.  Well, now that I look again, it appears that I forgot to
cc you on the final version which was accepted.  I'm sorry about that.

> Why you do this ?
 
I don't copy Signed-off-by if I make significant changes as the original
author should not be held responsible for any errors.  I'll be happy to
ask for your S-o-b before submitting a patch based on your work in future.

Ben.

-- 
Ben Hutchings
Experience is directly proportional to the value of equipment destroyed.
 - Carolyn Scheppner

--
This SF.net email is sponsored by:
High Quality Requirements in a Collaborative Environment.
Download a free trial of Rational Requirements Composer Now!
http://p.sf.net/sfu/www-ibm-com
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: [PATCH] radeon_cp: use request_firmware

2009-04-13 Thread Ben Hutchings 
Please use  for all mail related to my work on
firmware separation, which has nothing to do with my employment.

On Mon, 2009-04-13 at 21:57 +0530, Jaswinder Singh Rajput wrote:
> Moved datah before datal because datah is required before datal.
> 
> Firmware blob looks like this...
>   __be32 datah
>   __be32 datal
> 
> Thanks to Ben for help and testing.
[...]

This is out of date as there are now an r600_microcode.h and r600_cp.c.
My latest patch (not posted anywhere yet) is at

 (patches 0001-0003 should be applicable in order to drm-2.6/drm-linus).  But I 
don't have any R600/R700 hardware to test it on.

Ben.



signature.asc
Description: This is a digitally signed message part
--
This SF.net email is sponsored by:
High Quality Requirements in a Collaborative Environment.
Download a free trial of Rational Requirements Composer Now!
http://p.sf.net/sfu/www-ibm-com--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: [PATCH 1/3] mga: Use request_firmware() to load microcode

2009-02-23 Thread Ben Hutchings 
On Mon, 2009-02-23 at 13:14 +0200, Ville Syrjälä wrote:
> On Sun, Feb 22, 2009 at 11:45:21PM +0000, Ben Hutchings wrote:
> > On Mon, 2009-02-23 at 00:06 +0100, Stephane Marchesin wrote:
> > > Hi,
> > > 
> > > This mga patch replaces a firmware that was split in pieces by
> > > functionality and that had comments with a single blob.
> > 
> > Each pipe's code was converted to a seperate line of the ihex file.
> > 
> > > So IMO it's actually decreasing the quality of the code.
> > 
> > You can read the microcode?!
> 
> Each blob matches a specific set of pipe flags. The pipe flags are part
> of the userspace API. So the correct order of the blobs is very important.

I assumed the pipe flags were determined by the hardware.  Regardless, I
have no intention of trying to reorder them!

> > I believe it's possible to include comments in ihex files, so the pipe
> > names could be added as comments.  I don't really see the point though -
> > who's going to be editing them?
> 
> AFAICS your code doesn't the convey the MGA_WARP_FOO <-> specific ucode
> blob mapping in any way. The old code made that part very clear. I
> suppose sufficient comments whould be enough to fix the problem. You
> should also do 'where = MGA_WARP_TGZ' instead of 'where = 0' when copying
> the ucode.

It's iterating over the pipes in order so it clearly needs to start with
pipe 0.  The fact that pipe 0 is TGZ is definitely not significant in
the iteration.

> Or perhaps you should even unroll the loop and use the
> WARP_UCODE_INSTALL() approach to keep the intention crystal clear.

Now that's just silly.

Ben.



signature.asc
Description: This is a digitally signed message part
--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: [PATCH 1/3] mga: Use request_firmware() to load microcode

2009-02-22 Thread Ben Hutchings 
On Mon, 2009-02-23 at 00:06 +0100, Stephane Marchesin wrote:
> Hi,
> 
> This mga patch replaces a firmware that was split in pieces by
> functionality and that had comments with a single blob.

Each pipe's code was converted to a seperate line of the ihex file.

> So IMO it's actually decreasing the quality of the code.

You can read the microcode?!

I believe it's possible to include comments in ihex files, so the pipe
names could be added as comments.  I don't really see the point though -
who's going to be editing them?

Ben.



signature.asc
Description: This is a digitally signed message part
--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

[PATCH 2/3] r128: Use request_firmware() to load CCE microcode

2009-02-22 Thread Ben Hutchings 
Firmware blob looks like this:
__be32 datah
__be32 datal

Compile-tested only.

Signed-off-by: Ben Hutchings 
---
 drivers/gpu/drm/Kconfig |1 +
 drivers/gpu/drm/r128/r128_cce.c |   98 ++---
 firmware/Makefile   |1 +
 firmware/WHENCE |   31 +
 firmware/r128/r128_cce.bin.ihex |  129 +++
 5 files changed, 210 insertions(+), 50 deletions(-)
 create mode 100644 firmware/r128/r128_cce.bin.ihex

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 6c82d2a..9f46d92 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -28,6 +28,7 @@ config DRM_TDFX
 config DRM_R128
tristate "ATI Rage 128"
depends on DRM && PCI
+   select FW_LOADER
help
  Choose this option if you have an ATI Rage 128 graphics card.  If M
  is selected, the module will be called r128.  AGP support for
diff --git a/drivers/gpu/drm/r128/r128_cce.c b/drivers/gpu/drm/r128/r128_cce.c
index 32de4ce..1156b43 100644
--- a/drivers/gpu/drm/r128/r128_cce.c
+++ b/drivers/gpu/drm/r128/r128_cce.c
@@ -29,6 +29,9 @@
  *Gareth Hughes 
  */
 
+#include 
+#include 
+
 #include "drmP.h"
 #include "drm.h"
 #include "r128_drm.h"
@@ -36,50 +39,9 @@
 
 #define R128_FIFO_DEBUG0
 
-/* CCE microcode (from ATI) */
-static u32 r128_cce_microcode[] = {
-   0, 276838400, 0, 268449792, 2, 142, 2, 145, 0, 1076765731, 0,
-   1617039951, 0, 774592877, 0, 1987540286, 0, 2307490946U, 0,
-   599558925, 0, 589505315, 0, 596487092, 0, 589505315, 1,
-   11544576, 1, 206848, 1, 311296, 1, 198656, 2, 912273422, 11,
-   262144, 0, 0, 1, 33559837, 1, 7438, 1, 14809, 1, 6615, 12, 28,
-   1, 6614, 12, 28, 2, 23, 11, 18874368, 0, 16790922, 1, 409600, 9,
-   30, 1, 147854772, 16, 420483072, 3, 8192, 0, 10240, 1, 198656,
-   1, 15630, 1, 51200, 10, 34858, 9, 42, 1, 33559823, 2, 10276, 1,
-   15717, 1, 15718, 2, 43, 1, 15936948, 1, 570480831, 1, 14715071,
-   12, 322123831, 1, 33953125, 12, 55, 1, 33559908, 1, 15718, 2,
-   46, 4, 2099258, 1, 526336, 1, 442623, 4, 4194365, 1, 509952, 1,
-   459007, 3, 0, 12, 92, 2, 46, 12, 176, 1, 15734, 1, 206848, 1,
-   18432, 1, 133120, 1, 100670734, 1, 149504, 1, 165888, 1,
-   15975928, 1, 1048576, 6, 3145806, 1, 15715, 16, 2150645232U, 2,
-   268449859, 2, 10307, 12, 176, 1, 15734, 1, 15735, 1, 15630, 1,
-   15631, 1, 5253120, 6, 3145810, 16, 2150645232U, 1, 15864, 2, 82,
-   1, 343310, 1, 1064207, 2, 3145813, 1, 15728, 1, 7817, 1, 15729,
-   3, 15730, 12, 92, 2, 98, 1, 16168, 1, 16167, 1, 16002, 1, 16008,
-   1, 15974, 1, 15975, 1, 15990, 1, 15976, 1, 15977, 1, 15980, 0,
-   15981, 1, 10240, 1, 5253120, 1, 15720, 1, 198656, 6, 110, 1,
-   180224, 1, 103824738, 2, 112, 2, 3145839, 0, 536885440, 1,
-   114880, 14, 125, 12, 206975, 1, 33559995, 12, 198784, 0,
-   33570236, 1, 15803, 0, 15804, 3, 294912, 1, 294912, 3, 442370,
-   1, 11544576, 0, 811612160, 1, 12593152, 1, 11536384, 1,
-   14024704, 7, 310382726, 0, 10240, 1, 14796, 1, 14797, 1, 14793,
-   1, 14794, 0, 14795, 1, 268679168, 1, 9437184, 1, 268449792, 1,
-   198656, 1, 9452827, 1, 1075854602, 1, 1075854603, 1, 557056, 1,
-   114880, 14, 159, 12, 198784, 1, 1109409213, 12, 198783, 1,
-   1107312059, 12, 198784, 1, 1109409212, 2, 162, 1, 1075854781, 1,
-   1073757627, 1, 1075854780, 1, 540672, 1, 10485760, 6, 3145894,
-   16, 274741248, 9, 168, 3, 4194304, 3, 4209949, 0, 0, 0, 256, 14,
-   174, 1, 114857, 1, 33560007, 12, 176, 0, 10240, 1, 114858, 1,
-   33560018, 1, 114857, 3, 33560007, 1, 16008, 1, 114874, 1,
-   33560360, 1, 114875, 1, 33560154, 0, 15963, 0, 256, 0, 4096, 1,
-   409611, 9, 188, 0, 10240, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
-};
+#define FIRMWARE_NAME  "r128/r128_cce.bin"
+
+MODULE_FIRMWARE(FIRMWARE_NAME);
 
 static int R128_READ_PLL(struct drm_device * dev, int addr)
 {
@@ -176,20 +138,50 @@ static int r128_do_wait_for_idle(drm_r128_private_t * 
dev_priv)
  */
 
 /* Load the microcode for the CCE */
-static void r128_cce_load_microcode(drm_r128_private_t * dev_priv)
+static int r128_cce_load_microcode(drm_r128_private_t *dev_priv)
 {
-   int i;
+   struct platform_device *pdev;
+   const struct firmware *fw;
+   const __be32 *fw_data;
+   int rc, i;
 
DRM_DEBUG("\n");
 
+   pdev = platform_device_register_simple("r128_cce&q

Re: [PATCH] radeon_cp: use request_firmware

2008-12-23 Thread Ben Hutchings 
On Mon, 2008-12-22 at 11:39 +0100, Julien Cristau wrote:
> On Sun, Dec 21, 2008 at 20:58:28 +, 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 
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 & RADEO

[PATCH] r128: Use request_firmware() to load CCE microcode

2008-10-19 Thread Ben Hutchings 
Firmware blob looks like this:
__be32 datah
__be32 datal

Compile-tested only.

Signed-off-by: Ben Hutchings <[EMAIL PROTECTED]>
---
This is pretty similar to Jaswinder Singh's patch for radeon.

The licence information is based on conversation with David Airlie.

Ben.

 drivers/gpu/drm/Kconfig |1 +
 drivers/gpu/drm/r128/r128_cce.c |   98 ++---
 firmware/Makefile   |1 +
 firmware/WHENCE |   31 +
 firmware/r128/r128_cce.bin.ihex |  129 +++
 5 files changed, 210 insertions(+), 50 deletions(-)
 create mode 100644 firmware/r128/r128_cce.bin.ihex

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 8de9e0b..dc6cf87 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -26,6 +26,7 @@ config DRM_TDFX
 config DRM_R128
tristate "ATI Rage 128"
depends on DRM && PCI
+   select FW_LOADER
help
  Choose this option if you have an ATI Rage 128 graphics card.  If M
  is selected, the module will be called r128.  AGP support for
diff --git a/drivers/gpu/drm/r128/r128_cce.c b/drivers/gpu/drm/r128/r128_cce.c
index c31afbd..63bed21 100644
--- a/drivers/gpu/drm/r128/r128_cce.c
+++ b/drivers/gpu/drm/r128/r128_cce.c
@@ -29,6 +29,9 @@
  *Gareth Hughes <[EMAIL PROTECTED]>
  */
 
+#include 
+#include 
+
 #include "drmP.h"
 #include "drm.h"
 #include "r128_drm.h"
@@ -36,50 +39,9 @@
 
 #define R128_FIFO_DEBUG0
 
-/* CCE microcode (from ATI) */
-static u32 r128_cce_microcode[] = {
-   0, 276838400, 0, 268449792, 2, 142, 2, 145, 0, 1076765731, 0,
-   1617039951, 0, 774592877, 0, 1987540286, 0, 2307490946U, 0,
-   599558925, 0, 589505315, 0, 596487092, 0, 589505315, 1,
-   11544576, 1, 206848, 1, 311296, 1, 198656, 2, 912273422, 11,
-   262144, 0, 0, 1, 33559837, 1, 7438, 1, 14809, 1, 6615, 12, 28,
-   1, 6614, 12, 28, 2, 23, 11, 18874368, 0, 16790922, 1, 409600, 9,
-   30, 1, 147854772, 16, 420483072, 3, 8192, 0, 10240, 1, 198656,
-   1, 15630, 1, 51200, 10, 34858, 9, 42, 1, 33559823, 2, 10276, 1,
-   15717, 1, 15718, 2, 43, 1, 15936948, 1, 570480831, 1, 14715071,
-   12, 322123831, 1, 33953125, 12, 55, 1, 33559908, 1, 15718, 2,
-   46, 4, 2099258, 1, 526336, 1, 442623, 4, 4194365, 1, 509952, 1,
-   459007, 3, 0, 12, 92, 2, 46, 12, 176, 1, 15734, 1, 206848, 1,
-   18432, 1, 133120, 1, 100670734, 1, 149504, 1, 165888, 1,
-   15975928, 1, 1048576, 6, 3145806, 1, 15715, 16, 2150645232U, 2,
-   268449859, 2, 10307, 12, 176, 1, 15734, 1, 15735, 1, 15630, 1,
-   15631, 1, 5253120, 6, 3145810, 16, 2150645232U, 1, 15864, 2, 82,
-   1, 343310, 1, 1064207, 2, 3145813, 1, 15728, 1, 7817, 1, 15729,
-   3, 15730, 12, 92, 2, 98, 1, 16168, 1, 16167, 1, 16002, 1, 16008,
-   1, 15974, 1, 15975, 1, 15990, 1, 15976, 1, 15977, 1, 15980, 0,
-   15981, 1, 10240, 1, 5253120, 1, 15720, 1, 198656, 6, 110, 1,
-   180224, 1, 103824738, 2, 112, 2, 3145839, 0, 536885440, 1,
-   114880, 14, 125, 12, 206975, 1, 33559995, 12, 198784, 0,
-   33570236, 1, 15803, 0, 15804, 3, 294912, 1, 294912, 3, 442370,
-   1, 11544576, 0, 811612160, 1, 12593152, 1, 11536384, 1,
-   14024704, 7, 310382726, 0, 10240, 1, 14796, 1, 14797, 1, 14793,
-   1, 14794, 0, 14795, 1, 268679168, 1, 9437184, 1, 268449792, 1,
-   198656, 1, 9452827, 1, 1075854602, 1, 1075854603, 1, 557056, 1,
-   114880, 14, 159, 12, 198784, 1, 1109409213, 12, 198783, 1,
-   1107312059, 12, 198784, 1, 1109409212, 2, 162, 1, 1075854781, 1,
-   1073757627, 1, 1075854780, 1, 540672, 1, 10485760, 6, 3145894,
-   16, 274741248, 9, 168, 3, 4194304, 3, 4209949, 0, 0, 0, 256, 14,
-   174, 1, 114857, 1, 33560007, 12, 176, 0, 10240, 1, 114858, 1,
-   33560018, 1, 114857, 3, 33560007, 1, 16008, 1, 114874, 1,
-   33560360, 1, 114875, 1, 33560154, 0, 15963, 0, 256, 0, 4096, 1,
-   409611, 9, 188, 0, 10240, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
-};
+#define FIRMWARE_NAME  "r128/r128_cce.bin"
+
+MODULE_FIRMWARE(FIRMWARE_NAME);
 
 static int R128_READ_PLL(struct drm_device * dev, int addr)
 {
@@ -176,20 +138,50 @@ static int r128_do_wait_for_idle(drm_r128_private_t * 
dev_priv)
  */
 
 /* Load the microcode for the CCE */
-static void r128_cce_load_microcode(drm_r128_private_t * dev_priv)
+static int r128_cce_load_microcode(drm_r128_private_t *dev_priv)
 {
-   int i;
+   struct platform_dev