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

2009-02-23 Thread Ville Syrjälä
On Sun, Feb 22, 2009 at 11:45:21PM +, 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 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. Or perhaps you should even unroll the loop and use the
WARP_UCODE_INSTALL() approach to keep the intention crystal clear.

-- 
Ville Syrjälä
syrj...@sci.fi
http://www.sci.fi/~syrjala/

--
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-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 +, 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 Stephane Marchesin
Hi,

This mga patch replaces a firmware that was split in pieces by
functionality and that had comments with a single blob. So IMO it's
actually decreasing the quality of the code.

Stephane

--
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