Eric Anholt wrote:
> On Tue, 2005-08-30 at 14:43 -0700, Ian Romanick wrote:
>
>>I've been looking a bit at various Radeon 3D drivers lately. Frankly,
>>they are in a woeful state. For years, literally, we've talked about
>>unifying the radeon and r200 drivers. Now a new driver, r300, has been
>>added. Each time there is a change to core Mesa, someone has to go in
>>and make nearly identical changes in 3 separate drivers. Ouch.
>>
>>We've procrastinated doing this work for a couple valid reasons. The
>>primary reason being that it's a rather large chunk of work to do. The
>>3 drivers a very similar in places, but are, for the most part, not
>>quite identical. That makes the required changes more intrusive than
>>just renaming a bunch of common functions and fixing the Makefiles.
>>
>>The secondary reason, which follows from the previous, is that there
>>would be a lot of testing. We'd have to test numerous programs across
>>at least 3 different pieces of hardware. There is a lot of opportunity
>>for things to be subtly wrong, and we'd sure like to catch those issues
>>before they hit production.
>>
>>In spite of those large issues, we need to do something. Put another
>>way, some progress towards unification would improve the situation.
>>That is exactly what I propose. We should create a new directory,
>>called radeon_common, and gradually migrate common code there. We'll
>>continue to have 3 separate drivers, but each of those drivers will pull
>>in code from the radeon_common directory. This is similar to the way
>>drivers pull in code from the common directory.
>>
>>Eventually "all" of the common code will end up in radeon_common. Once
>>we get to that point, it will be a much smaller step to unify the
>>drivers. Moreover, the testing burden is reduced because it is spread
>>out, in small pieces, over time.
>>
>>Am I off my rocker, or does this seem like a workable idea? If this
>>sounds reasonable, I'll go ahead and create the radeon_common directory
>>and do some initial refactoring.
>
>
> I do really like this idea, for the most part. I'm afraid of doing this
> as a single step like Roland's previous driver -- I didn't jump on the
> bandwagon back them basically because I didn't ever review it, at all,
> since it was so daunting.
>
> However, I'm leaning away from a radeon_common directory, because I care
> about cvs history so much. That, and I would like to see a single
> driver binary eventually, so the common directory seems like just an
> extra stage in the process. So here's the plan I'd been thinking about:
>
> - Create a new define RADEON_COMMON, and define it in r200
> - Block out most of radeon_context.h for !COMMON, except for pieces of
> radeon_context I want to share
> - make radeon_context the first entry of r200_context.h
> - start making r200 use radeon code and radeon_context entries where
> possible.
>
> This way, radeon can stay mostly unaware of the things merging in around
> it for a while, and testing effort can be focused on each r200 bit. For
> some of this, r300 can get the treatment quickly, too.
>
> I'm attacking this idea for the span code right now, as a proof of
> concept. If it works semi-sanely, what do people think? Since I've got
> copious free time right now, I'm up for doing the brunt of the work.
FWIW, I think the #ifdef approach is more error-prone. When one's
editing a particular section of code, it's easy to overlook the
ramifications of whether a particular preprocessor symbol is set or
not. If the #ifdef covers a large section of code, this is especially
true.
But I'll defer to whoever is doing the actual work.
-Brian
|