[git pull] drm for v4.17-rc1

[Daniel Vetter]

On Tue, Apr 3, 2018 at 1:13 PM, Lucas Stach <dev at lynxeye.de> wrote:
> Hi Daniel,
>
> Am Dienstag, den 03.04.2018, 12:01 +0200 schrieb Daniel Vetter:
>> On Tue, Apr 3, 2018 at 11:58 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
>> > On Thu, Mar 29, 2018 at 11:15:50AM +1000, Dave Airlie wrote:
>> > > Hi Linus,
>> > >
>> > > This is the main drm pull request for 4.17-rc1.
>> > >
>> > > I'm sending it early because Easter is coming and I'm going to be on
>> > > holidays/have relatives staying for most of the next three weeks.
>> > > I'll be near email for any emergency but otherwise not too engaged.
>> > > I'll likely have two days back before the end of the merge window
>> > > to vaccum up any fixes. Cannonlake and Vega12 support are probably the
>> > > two major things. This pull lacks nouveau, Ben had some unforseen
>> > > leave and a few other blockers so we'll see how things look or maybe
>> > > leave it for this merge window.
>> > >
>> > > I'm off to eat my weight in chocolate.
>> >
>> > Droppping down to dri-devel.
>> >
>> > I've had some great fun with scripting maintainer statistics recently. One
>> > thing I've done is looking at patches committed by the author themselves
>> > (= stuff pushed by maintainers/committers), and how much formal
>> > reviews/acks there are.
>> >
>> > Overall we're doing a fairly decent job, with 80+% of these patches
>> > reviewed. Big drivers (i915 and amdgpu) do a pretty much perfect job, as
>> > does everyone who's part of the drm-misc group. But the in-between drivers
>> > less so. And given that everyone else has to go through mandatory reviews
>> > (less than 50% of all patches are merged by maintainers/committers, even
>> > in drm) I don't see why maintainers should be special and can skip review.
>> >
>> > Also, most of the drivers where review doesn't consistently happen are
>> > developed by groups, so not hard to find a suitable review. Anyway, below
>> > the stats of unreviewed maintainer patches for this pull here.
>> >
>> > I think some drivers we could perhaps stuff into drm-misc, others should
>> > probably move to grou maintainership of some form.
>>
>> Aside, here's the list of top non-maintainer commits. Short summary is
>> that AMD really should switch to a group maintainer model, but e.g.
>> Laurent should probably become co-maintainer in some areas too ...
>
> To be honest I don't understand why you are trying to enforce your
> model on everyone. Maybe the drm-misc thing has solved some problems
> for you, but I just don't see the point why others who seem to have
> something that works for them should switch to something different.
>
> Especially the AMD driver seems to work quite well the way it is
> handled by those guys.
>
> I could also do a better job in drumming up reviews for Etnaviv, but it
> simply doesn't buy me anything. "Forced" review just to get the tags
> attached is almost worthless, as people tend to do it in a hurry, so it
>  doesn't really catch the subtle issues. I would rather be honest about
> something not having seen much review than have worthless review tags
> attached to my patches.
>
> My _feeling_ is that the review economy in drm-misc, which gets DRM the
> bragging rights of 80% reviewed patches, has already lowered the weight
> associated with those reviews, as most of them are really shallow. This
> might be okay with you and I'm certainly not trying to change the way
> drm-misc is handled, but I doubt that this is the universal gold
> standard which should be applied to everything.

There's kinda two aspects here:

- I don't get how no review is somehow better than the review we're
doing in drm-misc. I do think there's some pretty huge benefits here
with being able to share code better and spotting at least some of the
common pitfalls. But somehow because it is "forced" it's less useful
than doing no review at all.

- What gripes me here is that for non-maintainers/committers, review
isn't optional, because they can't bypass maintainers. But for
maintainers the review is entirely optional. That kind of hierarchy is
just not good to grow a real community. So either do group
maintainership (and have no one's patches getting reviewed), or
consistently review everything. Same rules for everyone.

And finally I do think that for kernel code some minimal oversight and
basic sanity checking, plus real in-depth review for anything close to
uapi, is a good idea. Mesa just crashes if it goes wrong, in the
kernel that's always an exploit.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch