On Tue, Feb 07, 2017 at 12:44:19PM +0100, Thierry Reding wrote: > On Tue, Feb 07, 2017 at 12:21:28PM +0100, Daniel Vetter wrote: > > On Tue, Feb 07, 2017 at 12:11:28PM +0100, Thierry Reding wrote: > > > On Tue, Feb 07, 2017 at 08:28:16AM +1000, Dave Airlie wrote: > > > > > > > > > > I definitely don't want that we don't attempt this. But brought from > > > > > years > > > > > of experience, I recommend to merge first (with pre-refactoring > > > > > already > > > > > applied, but helpers only extracted, not yet at the right spot), and > > > > > then > > > > > follow up with. Because on average, there's way too many trees with > > > > > overloaded maintainers who maybe look at your patch once per kernel > > > > > release cycle. > > > > > > > > > > If you know that backlight and spi isn't one of these areas (anything > > > > > that > > > > > goes through takashi/sound is a similar good experience for us on the > > > > > i915 > > > > > side), then I guess we can try. But then Noralf has already written a > > > > > few > > > > > months worth of really great refactoring, and I'm seriously starting > > > > > to > > > > > feel guilty for volunteering him for all of this. Even though he > > > > > seems to > > > > > be really good at it, and seems to not mind, it's getting a bit silly. > > > > > Given that I'd say up to Noralf. > > > > > > > > > > In short, there's always a balance. > > > > > > > > I don't think we can make a rule for this, it will always depend on the > > > > code. There is always going to be stuff we put in drm that should go > > > > elsewhere, and stuff that is elsewhere that drm should use. > > > > > > > > I think however if we do add stuff like this, someone should keep track > > > > of them and try to make them get further into the kernel. > > > > > > Yes, I think having some sort of TODO in drivers/gpu/drm could help > > > track things that we know should eventually be moved out. It could serve > > > as a list of janitorial tasks for newcomers that want to get their hands > > > dirty and tackle relatively trivial tasks. > > > > We have this list already, it's at: http://www.x.org/wiki/DRMJanitors/ > > > > I guess I should highlight it more, maybe even add it to the docs? Eric > > just asked about it last week too. > > Yeah, I'm aware of that list. I think it's a little problematic that > it's in a wiki and far removed from where the actual work is happening. > I think we should just take that list and add it as a TODO in > drivers/gpu/drm, or alternatively keep it as part of the GPU > documentation. That way we can more easily mark things as done or add > new stuff as work gets done. > > For cases like this I think we could just add new items as they are > pointed out during review. For things that are already merged we can > add items separately. Once the refactoring is done, the patch series > can contain a final patch that simply removes the items again. I think > that has much less potential to become out-dated than a separate wiki > page. > > FWIW, I'll volunteer to move the list to git if we decide to go ahead > with that.
One upside of a wiki is that it's quicker to edit, if someone spots a drive-by refactoring they might not bother with the formal requirements of a full patch. Otoh, having it in-source-tree definitely has benefits, too. If you do the conversion I'd vote for Documentation/gpu/TODO.rst, and linking it into our documentation (maybe even cross-link from introduction.rst under a "Getting Started" heading, as reasonable ramp-up tasks after some checkpatch patches). I think that would help highlight it a bit. And of course the wiki page needs to be removed and replaced with a link to the new canonical thing (probably best to point at the source-file in drm-tip.git, that should be the most up-to-date). Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch