[PATCH 00/14] cleanup radeon_asic.h
Hi all, This patch pile moves the static struct radeon_asic asic definitions form radeon_asic.h into the asic-specific files, where I think they belong. This way radeon_asic.h becomes a real header file that can be #included. And indeed, with all the copypasting of function declarations, one has gotten out of sync. The next step would be to collect asic specific declarations in radeon_asic.h - atm they are somewhat scattered. But this can easily be done on the go and has way too much potential for conflicts with other patches. So I didn't do this. Tested on my rv570. Comments higly welcome. Yours, Daniel Daniel Vetter (14): drm/radoen: move r100 asic struct to r100.c drm/radoen: move r200 asic struct to r200.c drm/radeon: move r300 asic structs to r300.c drm/radeon: move r420 asic struct to r420.c drm/radoen: move rs400 asic struct to rs400.c drm/radoen: move rs600 asic struct to rs600.c drm/radoen: move rs690 asic struct to rs690.c drm/radoen: move rv515 asic struct to rv515.c drm/radoen: move r520 asic struct to r520.c drm/radoen: move r600 asic struct to r600.c drm/radoen: move rv770 asic struct to rv770.c drm/radoen: move evergreen asic struct to evergreen.c drm/radoen: unconfuse return value of radeon_asic-clear_surface_reg drm/radeon: include radeon_asic.h in asic.c drivers/gpu/drm/radeon/evergreen.c | 39 +++- drivers/gpu/drm/radeon/r100.c| 39 +++ drivers/gpu/drm/radeon/r200.c| 38 +++ drivers/gpu/drm/radeon/r300.c| 76 ++ drivers/gpu/drm/radeon/r420.c| 39 +++ drivers/gpu/drm/radeon/r520.c| 39 +++ drivers/gpu/drm/radeon/r600.c| 43 +++- drivers/gpu/drm/radeon/radeon.h |3 +- drivers/gpu/drm/radeon/radeon_asic.h | 494 ++ drivers/gpu/drm/radeon/rs400.c | 39 +++ drivers/gpu/drm/radeon/rs600.c | 43 +++- drivers/gpu/drm/radeon/rs690.c | 39 +++ drivers/gpu/drm/radeon/rv515.c | 41 +++- drivers/gpu/drm/radeon/rv770.c | 42 +++- 14 files changed, 518 insertions(+), 496 deletions(-) -- Download Intel#174; 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 00/14] cleanup radeon_asic.h
On Thu, Mar 11, 2010 at 02:06:02PM +0100, Daniel Vetter wrote: Hi all, This patch pile moves the static struct radeon_asic asic definitions form radeon_asic.h into the asic-specific files, where I think they belong. This way radeon_asic.h becomes a real header file that can be #included. And indeed, with all the copypasting of function declarations, one has gotten out of sync. The next step would be to collect asic specific declarations in radeon_asic.h - atm they are somewhat scattered. But this can easily be done on the go and has way too much potential for conflicts with other patches. So I didn't do this. Tested on my rv570. Comments higly welcome. Yours, Daniel It all looks good from a quick read through of the patches. For gathering asic function prototype i kind of started adding them to radeon.h at the bottom (there is already a bunch of them). Thus i think radeon_asic.h can be kill and extern declaration directly put into radeon_asic.c Cheers, Jerome Daniel Vetter (14): drm/radoen: move r100 asic struct to r100.c drm/radoen: move r200 asic struct to r200.c drm/radeon: move r300 asic structs to r300.c drm/radeon: move r420 asic struct to r420.c drm/radoen: move rs400 asic struct to rs400.c drm/radoen: move rs600 asic struct to rs600.c drm/radoen: move rs690 asic struct to rs690.c drm/radoen: move rv515 asic struct to rv515.c drm/radoen: move r520 asic struct to r520.c drm/radoen: move r600 asic struct to r600.c drm/radoen: move rv770 asic struct to rv770.c drm/radoen: move evergreen asic struct to evergreen.c drm/radoen: unconfuse return value of radeon_asic-clear_surface_reg drm/radeon: include radeon_asic.h in asic.c drivers/gpu/drm/radeon/evergreen.c | 39 +++- drivers/gpu/drm/radeon/r100.c| 39 +++ drivers/gpu/drm/radeon/r200.c| 38 +++ drivers/gpu/drm/radeon/r300.c| 76 ++ drivers/gpu/drm/radeon/r420.c| 39 +++ drivers/gpu/drm/radeon/r520.c| 39 +++ drivers/gpu/drm/radeon/r600.c| 43 +++- drivers/gpu/drm/radeon/radeon.h |3 +- drivers/gpu/drm/radeon/radeon_asic.h | 494 ++ drivers/gpu/drm/radeon/rs400.c | 39 +++ drivers/gpu/drm/radeon/rs600.c | 43 +++- drivers/gpu/drm/radeon/rs690.c | 39 +++ drivers/gpu/drm/radeon/rv515.c | 41 +++- drivers/gpu/drm/radeon/rv770.c | 42 +++- 14 files changed, 518 insertions(+), 496 deletions(-) -- Download Intel#174; 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 -- Download Intel#174; 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 00/14] cleanup radeon_asic.h
On Thu, Mar 11, 2010 at 8:06 AM, Daniel Vetter daniel.vet...@ffwll.ch wrote: Hi all, This patch pile moves the static struct radeon_asic asic definitions form radeon_asic.h into the asic-specific files, where I think they belong. This way radeon_asic.h becomes a real header file that can be #included. And indeed, with all the copypasting of function declarations, one has gotten out of sync. The next step would be to collect asic specific declarations in radeon_asic.h - atm they are somewhat scattered. But this can easily be done on the go and has way too much potential for conflicts with other patches. So I didn't do this. Tested on my rv570. Comments higly welcome. I like keeping all the asic definitions in one file as you tend to need to update them all at one time and having them spread across all the asic files increases the likelihood of one or more of them getting missed. But I can live with it if other folks think it's a good idea. Alex Yours, Daniel Daniel Vetter (14): drm/radoen: move r100 asic struct to r100.c drm/radoen: move r200 asic struct to r200.c drm/radeon: move r300 asic structs to r300.c drm/radeon: move r420 asic struct to r420.c drm/radoen: move rs400 asic struct to rs400.c drm/radoen: move rs600 asic struct to rs600.c drm/radoen: move rs690 asic struct to rs690.c drm/radoen: move rv515 asic struct to rv515.c drm/radoen: move r520 asic struct to r520.c drm/radoen: move r600 asic struct to r600.c drm/radoen: move rv770 asic struct to rv770.c drm/radoen: move evergreen asic struct to evergreen.c drm/radoen: unconfuse return value of radeon_asic-clear_surface_reg This one should be applied. drm/radeon: include radeon_asic.h in asic.c drivers/gpu/drm/radeon/evergreen.c | 39 +++- drivers/gpu/drm/radeon/r100.c | 39 +++ drivers/gpu/drm/radeon/r200.c | 38 +++ drivers/gpu/drm/radeon/r300.c | 76 ++ drivers/gpu/drm/radeon/r420.c | 39 +++ drivers/gpu/drm/radeon/r520.c | 39 +++ drivers/gpu/drm/radeon/r600.c | 43 +++- drivers/gpu/drm/radeon/radeon.h | 3 +- drivers/gpu/drm/radeon/radeon_asic.h | 494 ++ drivers/gpu/drm/radeon/rs400.c | 39 +++ drivers/gpu/drm/radeon/rs600.c | 43 +++- drivers/gpu/drm/radeon/rs690.c | 39 +++ drivers/gpu/drm/radeon/rv515.c | 41 +++- drivers/gpu/drm/radeon/rv770.c | 42 +++- 14 files changed, 518 insertions(+), 496 deletions(-) -- Download Intel#174; 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 -- Download Intel#174; 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 00/14] cleanup radeon_asic.h
On Thu, Mar 11, 2010 at 10:54:09AM -0500, Alex Deucher wrote: I like keeping all the asic definitions in one file as you tend to need to update them all at one time and having them spread across all the asic files increases the likelihood of one or more of them getting missed. But I can live with it if other folks think it's a good idea. Alex I've also thought about putting all asic structs into one file but decided against it for two reasons: - putting the asics into the asic specific files allows us to mark asic-private functions as static. This makes code-reading easier. Of course, as someone who has just started to look at the radeon drm, I'm biased ;) - there was no .c file around where they'd fit. Creating a new radeon_asic.c file would be another option of course. If you think that's much better, I could respin the series. Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 -- Download Intel#174; 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 00/14] cleanup radeon_asic.h
On Thu, Mar 11, 2010 at 04:46:01PM +0100, Jerome Glisse wrote: On Thu, Mar 11, 2010 at 02:06:02PM +0100, Daniel Vetter wrote: Hi all, This patch pile moves the static struct radeon_asic asic definitions form radeon_asic.h into the asic-specific files, where I think they belong. This way radeon_asic.h becomes a real header file that can be #included. And indeed, with all the copypasting of function declarations, one has gotten out of sync. The next step would be to collect asic specific declarations in radeon_asic.h - atm they are somewhat scattered. But this can easily be done on the go and has way too much potential for conflicts with other patches. So I didn't do this. Tested on my rv570. Comments higly welcome. Yours, Daniel It all looks good from a quick read through of the patches. For gathering asic function prototype i kind of started adding them to radeon.h at the bottom (there is already a bunch of them). Thus i think radeon_asic.h can be kill and extern declaration directly put into radeon_asic.c Yes, I've noticed that radeon.h has started to accumulate some declarations (because radeon_asic.h could not be included, I think). IMHO radeon.h is already growing out of bounds, so my plan was to move these declarations to radeon_asic.h. Most of the functions are only used by the asic specific support code (eg. r600 the blit stuff) so would only pollute the general namespace in radeon.h Does that sound reasonable? btw, radeon_asic.c doesn't exist here in my tree. Cheers, Jerome Cheers, Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 -- Download Intel#174; 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 00/14] cleanup radeon_asic.h
2010/3/11 Alex Deucher alexdeuc...@gmail.com: I like keeping all the asic definitions in one file as you tend to need to update them all at one time and having them spread across all the asic files increases the likelihood of one or more of them getting missed. But I can live with it if other folks think it's a good idea. Same here. One file means easier editing. Maybe we could use some other of proposed tricks? -- Rafał -- Download Intel#174; 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 00/14] cleanup radeon_asic.h
2010/3/11 Jerome Glisse gli...@freedesktop.org: On Thu, Mar 11, 2010 at 05:24:22PM +0100, Rafał Miłecki wrote: 2010/3/11 Alex Deucher alexdeuc...@gmail.com: I like keeping all the asic definitions in one file as you tend to need to update them all at one time and having them spread across all the asic files increases the likelihood of one or more of them getting missed. But I can live with it if other folks think it's a good idea. Same here. One file means easier editing. Maybe we could use some other of proposed tricks? -- Rafał I don't have strong feeling but Alex has a point, right now we often update them, maybe we should add radeon_asic.c and move asic init (function now in radeon_device.c) along structure there. That seems like a good plan. Alex -- Download Intel#174; 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 00/14] cleanup radeon_asic.h
On Thu, Mar 11, 2010 at 05:34:28PM +0100, Jerome Glisse wrote: On Thu, Mar 11, 2010 at 05:24:22PM +0100, Rafał Miłecki wrote: 2010/3/11 Alex Deucher alexdeuc...@gmail.com: I like keeping all the asic definitions in one file as you tend to need to update them all at one time and having them spread across all the asic files increases the likelihood of one or more of them getting missed. But I can live with it if other folks think it's a good idea. Same here. One file means easier editing. Maybe we could use some other of proposed tricks? -- Rafał I don't have strong feeling but Alex has a point, right now we often update them, maybe we should add radeon_asic.c and move asic init (function now in radeon_device.c) along structure there. Ok, convinced. I'll respin the patch series along your idea (creating radeon_asic.c) and resend. Yours, Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 -- Download Intel#174; 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 00/14] cleanup radeon_asic.h
On Thu, Mar 11, 2010 at 05:24:22PM +0100, Rafał Miłecki wrote: 2010/3/11 Alex Deucher alexdeuc...@gmail.com: I like keeping all the asic definitions in one file as you tend to need to update them all at one time and having them spread across all the asic files increases the likelihood of one or more of them getting missed. But I can live with it if other folks think it's a good idea. Same here. One file means easier editing. Maybe we could use some other of proposed tricks? -- Rafał I don't have strong feeling but Alex has a point, right now we often update them, maybe we should add radeon_asic.c and move asic init (function now in radeon_device.c) along structure there. Cheers, Jerome -- Download Intel#174; 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 00/14] cleanup radeon_asic.h
2010/3/12 Jerome Glisse gli...@freedesktop.org: On Thu, Mar 11, 2010 at 05:24:22PM +0100, Rafał Miłecki wrote: 2010/3/11 Alex Deucher alexdeuc...@gmail.com: I like keeping all the asic definitions in one file as you tend to need to update them all at one time and having them spread across all the asic files increases the likelihood of one or more of them getting missed. But I can live with it if other folks think it's a good idea. Same here. One file means easier editing. Maybe we could use some other of proposed tricks? -- Rafał I don't have strong feeling but Alex has a point, right now we often update them, maybe we should add radeon_asic.c and move asic init (function now in radeon_device.c) along structure there. I've seen it sugggested earlier, Just don't use declarations in the C file, that isn't acceptable coding. If we add radeon_asic.h make sure to include that in places that define the functions as well. Last thing we want is declarations to diverge by accident. Dave. -- Download Intel#174; 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 00/14] cleanup radeon_asic.h
On Fri, Mar 12, 2010 at 06:52:43AM +1000, Dave Airlie wrote: I don't have strong feeling but Alex has a point, right now we often update them, maybe we should add radeon_asic.c and move asic init (function now in radeon_device.c) along structure there. I've seen it sugggested earlier, Just don't use declarations in the C file, that isn't acceptable coding. If we add radeon_asic.h make sure to include that in places that define the functions as well. Last thing we want is declarations to diverge by accident. Well, this is exactly what I'm trying to fix here. atm radeon_asic.h contains static struct definitions (i.e. should be a C file) and is therefore included only exactly _once_. And contains tons of declarations for the functions it uses. Which are actually in one case not coherent with the actual definitions! -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 -- Download Intel#174; 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
