On Tue, 2009-06-30 at 05:15 -0700, Jakob Bornecrantz wrote: > On 30 jun 2009, at 12.42, Keith Whitwell wrote: > > On Tue, 2009-06-30 at 03:27 -0700, Jakob Bornecrantz wrote: > >> Module: Mesa > >> Branch: master > >> Commit: 303cbb45b558a2b94e6922252cf57d115ba60b82 > >> URL: > >> http://cgit.freedesktop.org/mesa/mesa/commit/?id=303cbb45b558a2b94e6922252cf57d115ba60b82 > >> > >> Author: Jakob Bornecrantz <ja...@vmware.com> > >> Date: Tue Jun 30 11:49:43 2009 +0200 > >> > >> drm/st: Return drm_api struct from a function > > [SNIP] > > > Jakob, > > > > What's going on with this change? I'm not really sure that softpipe > > should be growing a dependency on the drm_api struct. Can you please > > explain what's going on here? > > I agree, I was completely wrong in making softpipe and i915simple > depend on the drm_api struct not that it breaks any compilation. As > the functions that grew that dependancy was made specifically for > drm_api in the first place, nor do they include drm_api and if they > did the drm_api header don't have any dependancies so the source can > still be compiled on any platform if it did.
OK, I understand where you're coming from. I think that keeping the drm struct out of the driver adds a bit of work, but it's clearly the right thing to do. > There where two reasons for doing this change. Firstly, doing any kind > of layering with other drivers (like rbug) was a pain since you had to > do compile time selection of which driver to pick, by including a > special header with wrapper code and renaming a struct, or create code > that mutates the exported drm_api struct depending on which you > wanted. If you look at the following change you can see its a lot > easier now. I agree -- getting rid of the global drm_hooks struct and turning it into something wrappable is clearly the right direction. I think the general thrust of what you're trying to do is sensible. > Secondly it is food for thought about the pipe driver interface, also > makes a bit more easier to implement it should we want to. > > > > > > > The (non-existent!) description of your change doesn't make any of > > this > > clear and doesn't give any indication why softpipe or any other > > os-independent code is being modified in this way. > > > > I suspect what you want to do is leave the os-independent code as it > > was, and place wrapper functions in a winsys or somewhere else that > > call > > into the os-independent code after discarding the drm_api parameter. > > Yeah that would be the right way of doing it, I'll fix that. > > > > > Try and imagine a situation where there is a peer to drm_api for some > > other OS, or even an alternate implementation for DRI/DRI2 that used > > some different interface -- the driver shouldn't be coded to one over > > the other, but should remain neutral and be parameterized in the > > winsys. > > > > When doing vaguely controversial things like this, it would be > > preferable to post the patch for review ahead of time, and at least > > put > > some description of your thinking into the change comment... > > I was planning on sending out a email later explaining things, I guess > I should have hold off committing the changes until then. OK, but an email follow-up doesn't show up in gitk six months down the line... A few choice sentences in the commit message go a long way... Thanks for turning this around Jakob. Keith ------------------------------------------------------------------------------ _______________________________________________ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
