RFC: drm-misc for small drivers?

Thu Jan 26 20:54:38 UTC 2017

On Thu, Jan 26, 2017 at 08:57:25PM +0100, Daniel Vetter wrote:
> Hi Liviu 
> 
> On Thu, Jan 26, 2017 at 05:42:12PM +0000, Liviu Dudau wrote:
> > On Thu, Jan 26, 2017 at 06:08:42PM +0100, Daniel Vetter wrote:
> > > We've discussed this a bit at LCA (with Dave and Eric), and it's
> > > probably best if I just summarize all the questions and opens and
> > > throw them out here for discussions:
> > > 
> > > - When's a driver small enough for a shared tree, and when is a
> > > separate tree a good idea? i915 and amdgpu are definitely big, and
> > > there's definitely drivers who are really small and in-between it's
> > > unclear. Personally I think this is easy to do with a sliding scale,
> > > with using topic branches (we can do them in drm-misc easily) for
> > > bigger stuff, and if that's a common thing, split out the driver
> > > (thanks to the drm-tip integration tree there's not much of a
> > > difference in handling conflicts due to that anyway).
> > 
> > IMO the criteria should be based on the number of people involved in
> > each driver that have commit rights. The size of the driver should not
> > matter. There can be only one person working on a large and established
> > code base and be easy to handle with a topic branch in drm-misc, while
> > having a small (because it is new) driver that 3-5 people can all commit
> > into leads to a different dynamic. I'm interested on what are your (and
> > others) thoughts on the process for this.
> 
> Hm, good point. Otoh we have 15 committers (and a lot more people) working
> on i915 in one tree, so a pile of people in one place isn't all that much
> of a reason to split it out I think. And drm-misc is still rather small,
> so 3-5 folks here and there won't matter.
> 
> For a new driver the topic branch would just be the initial pull request I
> think, assuming that afterwards the patch rate goes down a lot. If it keeps
> high (10+ patches per week or so), then better to have a separate tree
> from the start imo. I think patch rate is probably the best measure for
> whether a driver is small or big, not code size and maybe also not team
> size (since even if there's a bunch of people there's a big difference
> between dedicated to upstream or also absorbed with product and customer
> support).

Yes, I think patch rate is a better metric. I was using number of people as
a proxy for that for no good reason.

> 
> > > - Should it be an entire separate tree for soc drivers? Problem here
> > > is that we lack a volunteer group (and imo it really should be a group
> > > to avoid the single-maintainer troubles) to run that. I think it's
> > > easier to proof the process first, and if we want a separate tree,
> > > split that out later on. This is the same thing we've done with
> > > drm-misc, first with a topic branch in drm-intel.git, then separate. I
> > > think it worked really well.
> > 
> > Hmm, are we mixing here GPU drivers with display ones, like we do on
> > the #dri-devel IRC channel? Because different people are going to have
> > different interests (obviously).
> 
> Yup, this is for the drm subsystem overall. For process issues like these,
> why would display be different from rendering?

Fair enough.

> 
> > > - Should we require review or at least acks for patches committed by
> > > the author? We have a bunch of drivers with effectively just 1 person
> > > working on it, where getting real review is hard. But otoh a few of
> > > those 1-person drivers will become popular, and then it's good to
> > > start with establishing peer-review early on. I also think that
> > > requiring peer-review is good to share best practices and knowledge
> > > between different people in our community, not just to make sure the
> > > code is correct. For all these reasons I'm leaning towards not making
> > > an exception for drivers, and requiring the same amount of review for
> > > them if they go in through drm-misc as for any other patch.
> > 
> > I think it depends on the responsiveness of that driver maintainer. IF
> > we are happy with people making mistakes and fixing them quickly, then
> > we can be more lax on the review. I would personally like (or expect)
> > that at least the people sending pull request for drm-misc that includes
> > that driver do a quick review of the code because they know it is only
> > one person working on it.
> 
> The point here is to train maintainers on peer-reviewing their own stuff,
> instead of getting way to used with just being able to do whatever they
> want. So process, not code quality. In short, it's all about the topics I
> raised in my lca talk and establishing group maintainership models as the
> norm. Link: http://blog.ffwll.ch/2017/01/maintainers-dont-scale.html

Sorry, I still need to go and read it, got a glimpse on LWN about it. Will do it
tonight.

> 
> > > - Who's elligible? I think we could start small with a few volunteers
> > > and their drivers, and then anyone who's willing.
> > 
> > I am not volunteering not excusing me from this, I would like more details
> > on the process before making a decision.
> 
> Current drm-misc process documentation:
> 
> https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html
> 
> > > - Should we force new submissions to be managed in that shared treee?
> > > I think for initial submission a separate pull request for
> > > approval-by-Dave is good (but we could do that with topic branches
> > > too). And it's also way too early to tell, probably better to first
> > > figure out how well this goes.
> > > 
> > > - CI, needed? It would be great, but we're not there yet :( Atm
> > > drm-misc just has a bunch of defconfigs that need to always compile,
> > > and that's it. Long term I definitely want more, but we're just not
> > > there yet. And it's a problem in general for drm-misc.
> > 
> > +1
> > 
> > > 
> > > - dim scripts. Since we don't have a github flow where we can
> > > reasonably automate stuff on the server side we need something to
> > > automate on the client side. Thus far almost everyone seemed ok with
> > > the scripting that's used to drive drm-misc/intel/tip, but we can
> > > always improve things. And long term we can rework the approach
> > > however we want to really.
> > 
> > Can you be more specific on who "everyone" is? I haven't used any of the
> > scripts from drm-misc/intel/tip.
> 
> The tooling also has docs:
> 
> https://01.org/linuxgraphics/gfx-docs/maintainer-tools/dim.html
> 
> > > - Other stuff I've missed?
> > 
> > Suggestions on how we address the ordering of pulls? If we depend in a
> > drivers in drm-misc on a change making it into Dave's tree, how will that
> > work?
> 
> This really is about drivers where having a separate tree/branch doesn't
> make much sense because they're really small, and separate trees/branches
> cause more coordination overhead. Note that "small" is rather relative,
> currently we're pushing through 50 patches per week in drm-intel-next and
> it all works well, and drm-misc is a lot smaller than that still so could
> accomodate a _lot_ of the smaller drivers we have easily. Question is
> whether it makes sense.
> 
> Otherwise it's the same as with any other cross-tree issue: Either topic
> branch, or backmerge once drm-misc has flown to Dave's tree.

I'm clear on the mechanics, I was looking for more information on how to
trigger the backmerges and the pull requests. If one's driver is in drm-misc
and you need things to step in sync, is it just #dri-devel pings or something
more formal / automated?

Best regards,
Liviu

> 
> Cheers, Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel