[git pull] drm for v4.17-rc1

[Daniel Vetter]

On Tue, Apr 3, 2018 at 1:52 PM, Daniel Vetter <daniel at ffwll.ch> wrote:
> 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.

Not sure why you bring up AMD in support of doing things differently,
because AMD folks is one of those trees that consistently get
everything reviewed, and they're also thinking about switching to a
group maintainership model. Simply didn't get around to implement it
yet. The 2 patches by Alex are imo perfectly fine exceptions that
support the rule (quick revert + misplaced patch it seems). Same for
the 1 patch by Dave to fix compile fail. And the 5 unreviewed
drm-intel.git patches are generated by a script.

So if you want to run the show like AMD, get your stuff reviewed
before pushing :-)

Cheers, Daniel

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



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch