Re: [PATCH 1/3] mga: Use request_firmware() to load microcode
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
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
Re: [PATCH 1/3] mga: Use request_firmware() to load microcode
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
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
