RFC: drm-misc for small drivers?

RFC: drm-misc for small drivers?

[Daniel Vetter]

Hi all,

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).

- 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.

- 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.

- Who's elligible? I think we could start small with a few volunteers
and their drivers, and then anyone who's willing.

- 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.

- 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.

- Other stuff I've missed?

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Liviu Dudau]

On Thu, Jan 26, 2017 at 06:08:42PM +0100, Daniel Vetter wrote:
> Hi all,

Hi Daniel,

> 
> 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.

> 
> - 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).

> 
> - 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.

> 
> - 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.

> 
> - 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.

> 
> - 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?

Best regards,
Liviu

> 
> Cheers, Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Sean Paul]

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:
> > Hi all,
> 
> Hi Daniel,
> 
> > 
> > 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.

I'm not certain number of people is a good metric, TBH. There are cases
where a lot of people are working on a driver, but the patches are not being
merged to the maintainer tree. In these cases, it makes sense to migrate the
driver to -misc and have the community merge the patches.

I think most candidates should be fairly obvious without coming up with rigid
criteria.

Sean

<snip>

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Liviu Dudau]

On Thu, Jan 26, 2017 at 02:12:16PM -0500, Sean Paul wrote:
> 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:
> > > Hi all,
> > 
> > Hi Daniel,
> > 
> > > 
> > > 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.
> 
> I'm not certain number of people is a good metric, TBH. There are cases
> where a lot of people are working on a driver, but the patches are not being
> merged to the maintainer tree. In these cases, it makes sense to migrate the
> driver to -misc and have the community merge the patches.

Sorry, I should've been more explicit: I was referring to the number of
contributors with public commit rights, not all the engineers involved. In
Mali DP's case we do most of the development in the open, so all the engineers
are free to merge the patches into the current public git tree after internal
review that tries to catch the evident issues.

> 
> I think most candidates should be fairly obvious without coming up with rigid
> criteria.

Agree and I'm not asking for rigidity here, just clarifications (or guidance)
on when a team should consider appying for joining to / splitting from drm-misc.

Best regards,
Liviu

> 
> Sean
> 
> <snip>
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Daniel Vetter]

On Thu, Jan 26, 2017 at 9:48 PM, Liviu Dudau <liviu@dudau.co.uk> wrote:
>> I'm not certain number of people is a good metric, TBH. There are cases
>> where a lot of people are working on a driver, but the patches are not being
>> merged to the maintainer tree. In these cases, it makes sense to migrate the
>> driver to -misc and have the community merge the patches.
>
> Sorry, I should've been more explicit: I was referring to the number of
> contributors with public commit rights, not all the engineers involved. In
> Mali DP's case we do most of the development in the open, so all the engineers
> are free to merge the patches into the current public git tree after internal
> review that tries to catch the evident issues.

Not sure, but this sounds like you do internal review and then just
push to the public tree. That's not good, since it makes cross-driver
collab harder, and it makes it much harder for external people to
contribute to your driver in general. Upstream is about cross
vendor/user/osv/whomever collaboration in the open, if you really do
review internall I strongly urge you to do all patch submission&review
on dri-devel. That's how all the other drivers work (well
amd/i915/nouveau have their own separate mailing lists because we'd
drown dri-devel), and it's the expectation. It would definitely be the
expectation if arm display is managed in drm-misc :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Liviu Dudau]

On Fri, Jan 27, 2017 at 07:55:22AM +0100, Daniel Vetter wrote:
> On Thu, Jan 26, 2017 at 9:48 PM, Liviu Dudau <liviu@dudau.co.uk> wrote:
> >> I'm not certain number of people is a good metric, TBH. There are cases
> >> where a lot of people are working on a driver, but the patches are not being
> >> merged to the maintainer tree. In these cases, it makes sense to migrate the
> >> driver to -misc and have the community merge the patches.
> >
> > Sorry, I should've been more explicit: I was referring to the number of
> > contributors with public commit rights, not all the engineers involved. In
> > Mali DP's case we do most of the development in the open, so all the engineers
> > are free to merge the patches into the current public git tree after internal
> > review that tries to catch the evident issues.
> 
> Not sure, but this sounds like you do internal review and then just
> push to the public tree. That's not good, since it makes cross-driver
> collab harder, and it makes it much harder for external people to
> contribute to your driver in general. Upstream is about cross
> vendor/user/osv/whomever collaboration in the open, if you really do
> review internall I strongly urge you to do all patch submission&review
> on dri-devel. That's how all the other drivers work (well
> amd/i915/nouveau have their own separate mailing lists because we'd
> drown dri-devel), and it's the expectation. It would definitely be the
> expectation if arm display is managed in drm-misc :-)

Yes, we do some lite internal review for some very simple reasons: people are still
getting used to submitting patches to mainline so they make easily fixable mistakes
that we can weed out with the internal review; the second reason (and that applies
only when we add new functionality to the driver) is to scan for possible patent
violations. I'm pretty sure that something similar exists in most companies.

Note that this does not exclude the public mailing lists. We have received contributions
via dri-devel ML (look at the last pull request, it features one external contributor)
and there is no replacement internally for dri-devel. The malidp@foss.arm.com mailing
list exists only to allow for a quick response time from any of the contributors in
case some of us are travelling or on holiday which you don't get with direct email
addresses.

I've made that clear in private conversations and I wil re-iterate it here: Mali DP team
is mainline first and cross-driver focused. However, we are not operating as a separate
entity from the rest of ARM, so we have to obey some internal processes.

Best regards,
Liviu

> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Daniel Vetter]

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).

> > - 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?

> > - 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

> > - 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.

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

Re: RFC: drm-misc for small drivers?

[Liviu Dudau]

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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Gerd Hoffmann]

  Hi,

> https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html
> https://01.org/linuxgraphics/gfx-docs/maintainer-tools/dim.html

apache throws 403.

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Daniel Vetter]

On Mon, Jan 30, 2017 at 10:30:43AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html
> > https://01.org/linuxgraphics/gfx-docs/maintainer-tools/dim.html
> 
> apache throws 403.

We're looking into it. Adding Tomi, who's herding the autobuilder for
these docs.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Tomi Sarvela]

On Monday, 30 January 2017 10:49:10 EET Daniel Vetter wrote:
> On Mon, Jan 30, 2017 at 10:30:43AM +0100, Gerd Hoffmann wrote:
> >   Hi,
> >   
> > > https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.
> > > html
> > > https://01.org/linuxgraphics/gfx-docs/maintainer-tools/dim.html> 
> > apache throws 403.
> 
> We're looking into it. Adding Tomi, who's herding the autobuilder
> for these docs.

https://01.org/ is 403. The problem is not limited to documentation 
only. The issue has been pushed forward, but I don't have better 
information yet.

Tomi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Sean Paul]

On Thu, Jan 26, 2017 at 06:08:42PM +0100, Daniel Vetter wrote:
> Hi all,
> 
> 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).
> 
> - 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. 

Big +1. In addition to spreading out the workload, driver maintainers should
still exercise ownership/stewardship.

> 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.

Sounds reasonable.

> 
> - 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.

At the risk of being on the hook for more driver reviews, I think we should
strive to review and fallback if it can't be sustained.

> 
> - Who's elligible? I think we could start small with a few volunteers
> and their drivers, and then anyone who's willing.

I think we could safely volunteer some drivers we haven't seen pull requests
from in a while.

> 
> - 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.
> 
> - 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.

No issues with dim on my side, seems like a natural choice.

> 
> - Other stuff I've missed?
> 
> Cheers, Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Daniel Vetter]

On Thu, Jan 26, 2017 at 8:11 PM, Sean Paul <seanpaul@chromium.org> wrote:
> On Thu, Jan 26, 2017 at 06:08:42PM +0100, Daniel Vetter wrote:
>> - 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.
>
> Big +1. In addition to spreading out the workload, driver maintainers should
> still exercise ownership/stewardship.

Big +1 on separate driver tree, or that we'll probably can't get it
because no volunteers? For spreading out the workload, I don't expect
(or want) that this will cause more work for the existing drm-misc
group. Well, a few minutes more for the pull request summaries maybe,
but definitely no expectation that suddenly all of drm-misc helps out
with reviewing driver patches. That won't work.

>> - Who's elligible? I think we could start small with a few volunteers
>> and their drivers, and then anyone who's willing.
>
> I think we could safely volunteer some drivers we haven't seen pull requests
> from in a while.

Hm, I didn't think about drivers which aren't well-maintained. But if
there's a volunteer group we can do that ofc, but maybe not start out
with forcing it ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Sean Paul]

On Fri, Jan 27, 2017 at 1:32 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Thu, Jan 26, 2017 at 8:11 PM, Sean Paul <seanpaul@chromium.org> wrote:
>> On Thu, Jan 26, 2017 at 06:08:42PM +0100, Daniel Vetter wrote:
>>> - 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.
>>
>> Big +1. In addition to spreading out the workload, driver maintainers should
>> still exercise ownership/stewardship.
>
> Big +1 on separate driver tree, or that we'll probably can't get it
> because no volunteers?

Neither :-) I was agreeing that we need a volunteer group (or "review
economy") to make this work, as opposed to a single maintainer.

Sean

> For spreading out the workload, I don't expect
> (or want) that this will cause more work for the existing drm-misc
> group. Well, a few minutes more for the pull request summaries maybe,
> but definitely no expectation that suddenly all of drm-misc helps out
> with reviewing driver patches. That won't work.
>
>>> - Who's elligible? I think we could start small with a few volunteers
>>> and their drivers, and then anyone who's willing.
>>
>> I think we could safely volunteer some drivers we haven't seen pull requests
>> from in a while.
>
> Hm, I didn't think about drivers which aren't well-maintained. But if
> there's a volunteer group we can do that ofc, but maybe not start out
> with forcing it ...
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 5069 bytes --]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

> Hi all,
>
> 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).

FWIW, I don't anticipate vc4 needing its own tree again if we can get
into a shared-maintainership one.  I think needing our own tree would
mean that there's something going wrong with the merge process, which we
should work hard to fix.

> - 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.

I'd rather not have an soc tree -- I don't think being soc drivers makes
us special.

> - 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.

This is the only part I'm concerned about.  Would anyone review vc4
patches?  Voluntarily?  Actually thinking about the
functionality/structure of the code, not just style?

It sucks today that as part of the kernel process I send out patches
"for review", knowing that I won't ever get review, and I just have to
wait a while until I think people won't complain at me for sending a PR
too quickly.  But if the change is to just start blocking my patches on
review, I'm concerned I won't be able to get them in at all.

I think there's a middle ground, where you graduate to waiting for
review once you have multiple involved in that area of the code.  This
is what we do in Mesa -- vc4 and freedreno push directly, but on the
code we share (tgsi_to_nir, for example), we do review.

(This is also the point at which I'll offer: Any other ARM drivers that
want to do review exchange with me, let me know and I'll start paying
attention to your stuff)

> - Who's elligible? I think we could start small with a few volunteers
> and their drivers, and then anyone who's willing.

I volunteer as tribute.

(assuming a resolution for the above question)

> - 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.

I'd rather wait on changing policy for new drivers until we have tested
this change with existing drivers for a bit.  And for initial
submission, getting the ack from Dave will always seem mandatory to me.

> - 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.

Keeping the defconfigs compiling is about as much CI as I have
currently, and I suspect that dim will actually improve how I do with
this (right now I'm relying on 0day a lot).  Supposedly kernel-ci.org
also boot-tests things, but I've never seen a breakage report so I'm
pretty skeptical of it.

> - 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.

I haven't tested the scripts out yet, but browsing the docs
(https://cgit.freedesktop.org/drm-intel/tree/dim.rst?h=maintainer-tools
and
https://cgit.freedesktop.org/drm-intel/tree/drm-misc.rst?h=maintainer-tools)
they seem fine for me.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Daniel Vetter]

On Thu, Jan 26, 2017 at 8:48 PM, Eric Anholt <eric@anholt.net> wrote:
>> - 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.
>
> This is the only part I'm concerned about.  Would anyone review vc4
> patches?  Voluntarily?  Actually thinking about the
> functionality/structure of the code, not just style?
>
> It sucks today that as part of the kernel process I send out patches
> "for review", knowing that I won't ever get review, and I just have to
> wait a while until I think people won't complain at me for sending a PR
> too quickly.  But if the change is to just start blocking my patches on
> review, I'm concerned I won't be able to get them in at all.
>
> I think there's a middle ground, where you graduate to waiting for
> review once you have multiple involved in that area of the code.  This
> is what we do in Mesa -- vc4 and freedreno push directly, but on the
> code we share (tgsi_to_nir, for example), we do review.
>
> (This is also the point at which I'll offer: Any other ARM drivers that
> want to do review exchange with me, let me know and I'll start paying
> attention to your stuff)

So part of the evil goal here is really to kickstart a tit-for-tat
review economy. That'd would mean we'd need at least 2-3 drivers to
volunteer for starters, and in many cases a full review is not going
to happen (or would just unduly delay things for no gain at all). What
I think would be great though is something much less formal along the
lines of "read your patch, looks all reasonable, seems to use core drm
code&concepts how it's supposed, ack". Maybe a few recommendations how
code can be simplified/clarified, but definitely no multi-round
bikeshedding tour. So not review to ensure code correctness, but just
as an information and best practice sharing tool. Also this should be
a good way to catch good opportunities for subsystem wide refactorings
when someone copypastes the same few lines for the umpteenth time
(that's how we e.g. got rid of all the dummy callback
implementations). And it shouldn't be much work, when I review a new
driver submission it's about 15 minutes to scroll through patches and
drop a few comments/suggestions on it.

So much less review than for drm core, and I think somehow getting
this off the ground would be really good for the community overall.
But if it doesn't work out, then I'm ok with dropping it, but I think
we really should try first. I think drm core had similar troubles with
review, and with drm-misc collaborations seems to improve already.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Alex Deucher]

On Fri, Jan 27, 2017 at 1:52 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Thu, Jan 26, 2017 at 8:48 PM, Eric Anholt <eric@anholt.net> wrote:
>>> - 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.
>>
>> This is the only part I'm concerned about.  Would anyone review vc4
>> patches?  Voluntarily?  Actually thinking about the
>> functionality/structure of the code, not just style?
>>
>> It sucks today that as part of the kernel process I send out patches
>> "for review", knowing that I won't ever get review, and I just have to
>> wait a while until I think people won't complain at me for sending a PR
>> too quickly.  But if the change is to just start blocking my patches on
>> review, I'm concerned I won't be able to get them in at all.
>>
>> I think there's a middle ground, where you graduate to waiting for
>> review once you have multiple involved in that area of the code.  This
>> is what we do in Mesa -- vc4 and freedreno push directly, but on the
>> code we share (tgsi_to_nir, for example), we do review.
>>
>> (This is also the point at which I'll offer: Any other ARM drivers that
>> want to do review exchange with me, let me know and I'll start paying
>> attention to your stuff)
>
> So part of the evil goal here is really to kickstart a tit-for-tat
> review economy. That'd would mean we'd need at least 2-3 drivers to
> volunteer for starters, and in many cases a full review is not going
> to happen (or would just unduly delay things for no gain at all). What
> I think would be great though is something much less formal along the
> lines of "read your patch, looks all reasonable, seems to use core drm
> code&concepts how it's supposed, ack". Maybe a few recommendations how
> code can be simplified/clarified, but definitely no multi-round
> bikeshedding tour. So not review to ensure code correctness, but just
> as an information and best practice sharing tool. Also this should be
> a good way to catch good opportunities for subsystem wide refactorings
> when someone copypastes the same few lines for the umpteenth time
> (that's how we e.g. got rid of all the dummy callback
> implementations). And it shouldn't be much work, when I review a new
> driver submission it's about 15 minutes to scroll through patches and
> drop a few comments/suggestions on it.
>
> So much less review than for drm core, and I think somehow getting
> this off the ground would be really good for the community overall.
> But if it doesn't work out, then I'm ok with dropping it, but I think
> we really should try first. I think drm core had similar troubles with
> review, and with drm-misc collaborations seems to improve already.

I like the idea, but I'm concerned it would just be work for the sake
of work.  I'm not sure how much value it provides.  I'm worried it'll
just turn into acking for the sake of acking.  For core driver changes
that are independent of anything shared, I'm not really sure what the
ack provides other than a way to slap an ack label on your patch
unless the reviewer really digs into the driver.  I guess an extra set
of eyes never hurts.

Alex
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Daniel Vetter]

On Fri, Jan 27, 2017 at 11:50:42AM -0500, Alex Deucher wrote:
> On Fri, Jan 27, 2017 at 1:52 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > On Thu, Jan 26, 2017 at 8:48 PM, Eric Anholt <eric@anholt.net> wrote:
> >>> - 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.
> >>
> >> This is the only part I'm concerned about.  Would anyone review vc4
> >> patches?  Voluntarily?  Actually thinking about the
> >> functionality/structure of the code, not just style?
> >>
> >> It sucks today that as part of the kernel process I send out patches
> >> "for review", knowing that I won't ever get review, and I just have to
> >> wait a while until I think people won't complain at me for sending a PR
> >> too quickly.  But if the change is to just start blocking my patches on
> >> review, I'm concerned I won't be able to get them in at all.
> >>
> >> I think there's a middle ground, where you graduate to waiting for
> >> review once you have multiple involved in that area of the code.  This
> >> is what we do in Mesa -- vc4 and freedreno push directly, but on the
> >> code we share (tgsi_to_nir, for example), we do review.
> >>
> >> (This is also the point at which I'll offer: Any other ARM drivers that
> >> want to do review exchange with me, let me know and I'll start paying
> >> attention to your stuff)
> >
> > So part of the evil goal here is really to kickstart a tit-for-tat
> > review economy. That'd would mean we'd need at least 2-3 drivers to
> > volunteer for starters, and in many cases a full review is not going
> > to happen (or would just unduly delay things for no gain at all). What
> > I think would be great though is something much less formal along the
> > lines of "read your patch, looks all reasonable, seems to use core drm
> > code&concepts how it's supposed, ack". Maybe a few recommendations how
> > code can be simplified/clarified, but definitely no multi-round
> > bikeshedding tour. So not review to ensure code correctness, but just
> > as an information and best practice sharing tool. Also this should be
> > a good way to catch good opportunities for subsystem wide refactorings
> > when someone copypastes the same few lines for the umpteenth time
> > (that's how we e.g. got rid of all the dummy callback
> > implementations). And it shouldn't be much work, when I review a new
> > driver submission it's about 15 minutes to scroll through patches and
> > drop a few comments/suggestions on it.
> >
> > So much less review than for drm core, and I think somehow getting
> > this off the ground would be really good for the community overall.
> > But if it doesn't work out, then I'm ok with dropping it, but I think
> > we really should try first. I think drm core had similar troubles with
> > review, and with drm-misc collaborations seems to improve already.
> 
> I like the idea, but I'm concerned it would just be work for the sake
> of work.  I'm not sure how much value it provides.  I'm worried it'll
> just turn into acking for the sake of acking.  For core driver changes
> that are independent of anything shared, I'm not really sure what the
> ack provides other than a way to slap an ack label on your patch
> unless the reviewer really digs into the driver.  I guess an extra set
> of eyes never hurts.

Yeah, it would be a bit of a checkbox step, but then there's not that many
drivers which really only have 1 contributor. I think as soon as you have
2-3, group maintainership with real peer review within the group should be
the expectation, whether that's within drm-misc or in a separate driver
repo doesn't matter.

And for single-contributor drivers I still think it's useful, both to
share best practices (display hw all deals with the same problems in the
end, so even when the register don't work the same, the overall logic
often does), and to make sure that when there's more contributors there's
no process change in switching to a real group maintainership model.

And I'm happy to help out kickstarting a small-driver review market by
reviewing patches in exchange for review on my cleanup/core/doc stuff :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Boris Brezillon]

Hi Daniel,

On Thu, 26 Jan 2017 18:08:42 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> Hi all,
> 
> 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).
> 
> - 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.
> 
> - 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.
> 
> - Who's elligible? I think we could start small with a few volunteers
> and their drivers, and then anyone who's willing.

I'd be happy to have the atmel-hlcdc driver maintained in this drm-misc
tree. I just had to send a PR containing a single patch for 4.11, and it
really feels like these simple fixes/improvements patches do not deserve
a dedicated PR (not to mention that sometime I forget to send the
PR and miss a release :-)).

Now, regarding the peer-review thing, I'm not against reviewing a few
simple patches from time to time, but I don't think I'll have time to
review entire new drivers, and I guess that's the kind of thing your
looking for :-/.

> 
> - 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.
> 
> - 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.
> 
> - Other stuff I've missed?
> 
> Cheers, Daniel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: RFC: drm-misc for small drivers?

[Daniel Vetter]

On Mon, Jan 30, 2017 at 11:15:34AM +0100, Boris Brezillon wrote:
> Hi Daniel,
> 
> On Thu, 26 Jan 2017 18:08:42 +0100
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> 
> > Hi all,
> > 
> > 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).
> > 
> > - 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.
> > 
> > - 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.
> > 
> > - Who's elligible? I think we could start small with a few volunteers
> > and their drivers, and then anyone who's willing.
> 
> I'd be happy to have the atmel-hlcdc driver maintained in this drm-misc
> tree. I just had to send a PR containing a single patch for 4.11, and it
> really feels like these simple fixes/improvements patches do not deserve
> a dedicated PR (not to mention that sometime I forget to send the
> PR and miss a release :-)).
> 
> Now, regarding the peer-review thing, I'm not against reviewing a few
> simple patches from time to time, but I don't think I'll have time to
> review entire new drivers, and I guess that's the kind of thing your
> looking for :-/.

It' should be equal for equal really imo, so if you have a few small
patches, then you'd need to review a few small patches to not drain the
review pool. I haven't figured out a good way to offload the
new-driver-review yet :-/

Anyway it sounds like we have enough interested folks for an attempt, I'll
try and type up some rough guidelines for the drm-misc docs and then we'll
see what happens.
-Daniel

> 
> > 
> > - 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.
> > 
> > - 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.
> > 
> > - Other stuff I've missed?
> > 
> > Cheers, Daniel
> 

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