Re: DRM code reorganization

From: Ian Romanick (idr_at_us.ibm.com)
Date: 08/02/04

  • Next message: Chris Wedgwood: "Re: [Patch for review] BSD accounting IO stats"
    Date:	Mon, 02 Aug 2004 13:11:26 -0700
    To: Dave Jones <davej@redhat.com>
    
    

    Dave Jones wrote:

    > On Mon, Aug 02, 2004 at 11:02:43AM -0700, Ian Romanick wrote:
    >
    > > This would be *very* non-trivial to do. Doing the DRM like this has
    > > come up probably a dozen times (or more) over the last 3 years.
    >
    > Which should ring alarm bells that something might not be quite right.

    And that it hasn't been done all those times should be a sign of
    *something*. ;)

    > > The problem is that each driver has different needs based on hardware
    > > functionality.
    >
    > How does this differ from any other subsystem that supports
    > cards with features that may not be present in another model ?
    > Other subsystems have dealt with this problem without the need
    > to introduce horrors like the abstractions in DRM.

    There are a couple of other things to consider.

    1. There is a lot more variability among graphics cards that there is
    among, say, network cards. Look at the output of 'grep __HAVE_ | grep
    define' on any two <card>.h files to see what I mean. The output for
    tdfx.h and radeon.h, or mga.h and savage.h is *very* different. That,
    by itself, makes a huge difference on what code is needed.

    2. We have an extra dimension to our matrix. Most other drivers don't
    need to worry about working on BSD.

    3. The ever classic....It seemed like a good idea at the time.

    > > We've managed to classify these needs into a few groups,
    > > and drivers can select which functionality they need via a set of
    > > defines. These per-driver defines determine what code gets compiled
    > > into the different routines (as well as which routines even get built).
    >
    > And among other horrors, crap like typedef's that magically change their
    > type completely depending on which file they are #include'd into.
    > Overuse of typedefs is one thing, but what goes on with stuff like
    > DRIVER_BUF_PRIV_T is bordering on obscene.
    >
    > If this kind of abuse wasn't so widespread, abstracting this code
    > out into shared sections and driver specific parts would be a lot
    > simpler. Sadly, this is the tip of the iceberg.

    I think it comes down to the fact that the original DRM developers
    wanted templates. C doesn't have them, so they did the "next best" thing.

    > > Trying to make this into a library would just be a mess.
    >
    > Depends on which direction you're looking at things.
    >>From the view of many kernel developers, anything would be
    > better than the trainwreck of wrappers/macros/preprocessor abuse
    > that's there right now. I'd put money on a lot more people being
    > prepared to hack on DRM's kernel code if it were more readable.
    >
    > To give an example of just how bad some folks view on this code is:
    > An actual conversation at OLS last week..
    > "I found it easier to look at the C preprocessor output than the
    > actual source code to find the types of the variables I was looking at".

    That's not surprising.

    > > If this is something that we really want to pursue, someone needs to dig
    > > in and figure out:
    > >
    > > 1. How much / which code can be "trivially" shared?
    > > 2. How much / which code can be shared with very little work?
    > > 3. How much / which code would we rather get a root-canal than try to
    > > make shared?
    > >
    > > The concern has been that, by making a "generic" library layer, we'd
    > > actually make the DRM more difficult to maintain.
    >
    > I don't maintain upstream DRM, but I have a fair amount of responsibility
    > regarding the Fedora kernel, and I'll state publically that looking at
    > bugs in drivers/char/drm is right up there on my list of
    > "things I'd rather not do after lunch". Maintainability goes much
    > further than 'the guy that wrote the code can grok it'.
    >
    > People trying to pick up 3d driver programming on Linux have a huge
    > hurdle in their place, as that code resembles *NO OTHER* driver code
    > in the tree.

    In all fairness, what's in the kernel does not constitute the guts of
    the 3D driver. Few people that do 3D driver development ever need touch
    anything in the kernel. That's by design. I've been working on the
    open-source 3D drivers for 3 years, and I've made *maybe* 4 changes to
    the kernel code.

    In this case that's a two edged sword. The kernel parts have gotten to
    be like a container pushed too far to the back of the refridgerator for
    too long...

    > > Nobody has really had
    > > the time to do the research required to either substantiate or refute
    > > those claims. Based on the little experience I have in the DRM, I tend
    > > to believe them.
    >
    > I spent 2-3 days a few months back attempting to clean up some of the
    > goo, by peeling away the layers of abstraction. After the 3rd day,
    > I'd estimate I wasn't even a fraction of the way through, and I lost
    > the will to keep fighting.

    That's the core question. Everyone *knows* that the current DRM code is
    an ugly mess, but is it worth the effort at this point to fix it? You
    seem to have come to the same conclusion that everyone else that's
    looked at the problem over the last 2 years has.

    Just to reiterate. I am *NOT* saying that it's not a problem. Nor am I
    saying that nothing should be done. What I am saying is that it's a
    really big problem, and fixing it may not be worth the effort required.

    I think the only realistic approach is to attack the problem bit by bit,
    rather than en masse. Gradual elimination of the '#if __HAVE_FOO' and
    gradual datatype refactoring is really the only way to attack it. The
    problem is just too big and there are just too few interested developers.

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/


  • Next message: Chris Wedgwood: "Re: [Patch for review] BSD accounting IO stats"

    Relevant Pages

    • Re: Linux drivers management
      ... developers when some idiot company's binary driver modules is buggy ... and corrupts the kernel in hard-to-debug ways; ... The assumption by many developers is that if we concetrate on making ... work with the Linux kernel development community. ...
      (Linux-Kernel)
    • Re: [opensuse] Why are there not more using Linux?
      ... It takes 100 hours to make a driver, plus 1 hour per month to ... Ah, this is a simple one, turn the code over to the 300+ developers at ... It is *really* daunting to update a kernel and find, ... of the Linux kernel, and I'm not going to do ANYTHING to help them out. ...
      (SuSE)
    • Re: your mail
      ... Graphics drivers in the kernel are broken. ... Since fbdev has taken the path to pretend that DRM ... Part of being a first class driver means that DRM has ...
      (Linux-Kernel)
    • New Linux Development Model
      ... I'm a linux use since many years. ... For example take the ipw2200 driver. ... I had also several problems with some other not in kernel drivers. ... Don't you think that this makes 3'rd party driver developers ...
      (Linux-Kernel)
    • Re: radeon-pre-2
      ... DRM code. ... Most of the changes are in kernel and the rest are in the ... normal Linux driver and stealth where it uses the resources without ... Stealth/primary mode is a transition tool until ...
      (Linux-Kernel)