Re: [git pull] drm for v4.17-rc1

2018-04-04 Thread Daniel Vetter
On Wed, Apr 04, 2018 at 12:36:20PM +0200, Michel Dänzer wrote:
> On 2018-04-03 02:03 PM, Daniel Vetter wrote:
> > On Tue, Apr 3, 2018 at 1:52 PM, Daniel Vetter  wrote:
> >> On Tue, Apr 3, 2018 at 1:13 PM, Lucas Stach  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  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.
> 
> What exactly do you mean by "group maintainership model"? FWIW, AMD
> developers are already pushing their own reviewed changes to the
> internal amd-staging-drm-next branch, it's just usually Alex which sends
> them to Dave (but Christian has done that before when Alex was away).

Yeah I know, but Alex reapplies them all again, so it doesn't show up as
that. Or maybe he just rebases, and that's why they show up as all
committed by Alex. It is still slightly different from drm-misc/intel,
where committers push to permanent history directly.
-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: [git pull] drm for v4.17-rc1

2018-04-04 Thread Michel Dänzer
On 2018-04-03 02:03 PM, Daniel Vetter wrote:
> On Tue, Apr 3, 2018 at 1:52 PM, Daniel Vetter  wrote:
>> On Tue, Apr 3, 2018 at 1:13 PM, Lucas Stach  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  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.

What exactly do you mean by "group maintainership model"? FWIW, AMD
developers are already pushing their own reviewed changes to the
internal amd-staging-drm-next branch, it's just usually Alex which sends
them to Dave (but Christian has done that before when Alex was away).


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [git pull] drm for v4.17-rc1

2018-04-03 Thread Rodrigo Vivi
On Tue, Apr 03, 2018 at 11:15:13AM +0100, Liviu Dudau wrote:
> Hi Daniel,
> 
> On Tue, Apr 03, 2018 at 11:58:17AM +0200, Daniel Vetter 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.
> > 
> > Cheers, Daniel
> > 
> > Alex Deucher (2):
> >   Revert "drm/radeon/pm: autoswitch power state when in balanced mode"
> >   drm/amdgpu: add documentation for amdgpu_device.c
> > 
> > Dave Airlie (1):
> >   drm/amd/pp: fix missing CONFIG_ACPI.
> > 
> > Frank Rowand (4):
> >   of: change overlay apply input data from unflattened to FDT
> >   of: Documentation: of_overlay_apply() replaced by 
> > of_overlay_fdt_apply()
> >   of: convert unittest overlay devicetree source to sugar syntax
> >   of: improve reporting invalid overlay target path
> > 
> > Joonas Lahtinen (5):
> >   drm/i915: Update DRIVER_DATE to 20180207
> >   drm/i915: Update DRIVER_DATE to 20180214
> >   drm/i915: Update DRIVER_DATE to 20180221
> >   drm/i915: Update DRIVER_DATE to 20180305
> >   drm/i915: Update DRIVER_DATE to 20180308
> > 
> > Liviu Dudau (5):
> >   drm/mali-dp: Rotated planes need a larger pitch size.
> >   drm/mali-dp: Align pitch size to be multiple of bus burst read size.
> >   drm/mali-dp: Don't enable scaling engine for planes that only rotate.
> >   drm/mali-dp: Fix malidp_atomic_commit_hw_done() for event sending.
> >   drm: mali-dp: Turn off CRTC vblank when removing module.
> 
> On the mali-dp driver there was a period of time where it was only me doing
> the work on mainline driver, with the promise that more people are going to
> join. That has now happened, so there are people reviewing the patches
> internally, but we are currently failing because of old habits to record
> their Reviewed-by properly when we transition the patches from internal
> discussions to the public ones. We're going to try harder in the future to
> not let maintainer patches go without proper review tags into the drm-next
> pull request.

This is a dilemma that we suffer on drm-intel as well:
Record or no record internal reviews when moving the code upstream.

The benefits of recording: Keep credits. People did work and spent their time
on reviewing the code already. Not recording it properly could demotivate
people of doing any internal reviews.

The cons of recording it: The upstream code might have moved a lot
that the reviewed-by tag from the old version is not the same anymore.

So, we decided for keeping the internal history, but using a common sense
rule where the person in charge of upstreaming internal patches ping
the old-reviewer, before merging, asking to do a double check to see if the old
review is still valid or if a new review should happen before code gets
really pushed. This is working pretty well so far.

Either way the Review is very important and pays off. I regretted myself
for merging some patches on internal without review at some point where
I believed that only having the final review would be enough.

A bad rubber stamp review shouldn't be an excuse to avoid reviews at all.
The bad rubber stamp review should be addressed and fixed with a process
that allow pr

Re: [git pull] drm for v4.17-rc1

2018-04-03 Thread Daniel Vetter
On Tue, Apr 3, 2018 at 3:23 PM, Jani Nikula  wrote:
> On Tue, 03 Apr 2018, Lucas Stach  wrote:
>> 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.
>
> I think you need to substantiate your claims about rubber stamping
> reviews. I'm not seeing that. And I do pay attention to the reviews that
> happen on i915 and drm display parts, kind of review-of-review. I'm
> personally pretty diligent about review, and I'm honestly *more* ashamed
> of patches I reviewed regressing than patches I wrote. Looking around, I
> don't think I'm alone.

Yeah, a thing to keep in mind is that we've had this "forced review"
in drm-intel since well over 5 years (so much longer than commit
rights). And the same rules apply to any core drm patches that get
merged into drm-misc. Similar for amd drivers, and Dave Airlie as
subsystem maintainer. Small drivers are explictly excempt from strict
review requirements in drm-misc, there we just want an Acked-by: to
signal a 2nd person at least looked at the patch. But even there a lot
of the patches got through full review scrutinies, with a bunch of
revisions until the patch is right. And all this has defacto run on a
"you review mine, I review yours" review economy all this time.

That amounts to 50+ people, some who've done this for 5+ years, you
accused of doing rubber stamp reviews. I agree with Jani that this
deserves some more concrete data than your personal feelings.
-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: [git pull] drm for v4.17-rc1

2018-04-03 Thread Jani Nikula
On Tue, 03 Apr 2018, Lucas Stach  wrote:
> 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 fully agree people need to decide for themselves what kind of
maintainership models suit them best. I'll only urge you to look into
the alternatives. If something works for you, it doesn't mean something
else couldn't work for you better. I'll leave it at that.

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

Again, I think as maintainer you should be free to do what you think
suits you and your contributors best. That said, I sincerely think
you're misguided about the value of review.

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

I think you need to substantiate your claims about rubber stamping
reviews. I'm not seeing that. And I do pay attention to the reviews that
happen on i915 and drm display parts, kind of review-of-review. I'm
personally pretty diligent about review, and I'm honestly *more* ashamed
of patches I reviewed regressing than patches I wrote. Looking around, I
don't think I'm alone.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [git pull] drm for v4.17-rc1

2018-04-03 Thread Daniel Vetter
Hi Liviu,

On Tue, Apr 3, 2018 at 12:15 PM, Liviu Dudau  wrote:
> On Tue, Apr 03, 2018 at 11:58:17AM +0200, Daniel Vetter 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.
>>
>> Cheers, Daniel
>>
>> Alex Deucher (2):
>>   Revert "drm/radeon/pm: autoswitch power state when in balanced mode"
>>   drm/amdgpu: add documentation for amdgpu_device.c
>>
>> Dave Airlie (1):
>>   drm/amd/pp: fix missing CONFIG_ACPI.
>>
>> Frank Rowand (4):
>>   of: change overlay apply input data from unflattened to FDT
>>   of: Documentation: of_overlay_apply() replaced by 
>> of_overlay_fdt_apply()
>>   of: convert unittest overlay devicetree source to sugar syntax
>>   of: improve reporting invalid overlay target path
>>
>> Joonas Lahtinen (5):
>>   drm/i915: Update DRIVER_DATE to 20180207
>>   drm/i915: Update DRIVER_DATE to 20180214
>>   drm/i915: Update DRIVER_DATE to 20180221
>>   drm/i915: Update DRIVER_DATE to 20180305
>>   drm/i915: Update DRIVER_DATE to 20180308
>>
>> Liviu Dudau (5):
>>   drm/mali-dp: Rotated planes need a larger pitch size.
>>   drm/mali-dp: Align pitch size to be multiple of bus burst read size.
>>   drm/mali-dp: Don't enable scaling engine for planes that only rotate.
>>   drm/mali-dp: Fix malidp_atomic_commit_hw_done() for event sending.
>>   drm: mali-dp: Turn off CRTC vblank when removing module.
>
> On the mali-dp driver there was a period of time where it was only me doing
> the work on mainline driver, with the promise that more people are going to
> join. That has now happened, so there are people reviewing the patches
> internally, but we are currently failing because of old habits to record
> their Reviewed-by properly when we transition the patches from internal
> discussions to the public ones. We're going to try harder in the future to
> not let maintainer patches go without proper review tags into the drm-next
> pull request.

Sounds great. Even better if you could move that internal review to
public lists, but that kind of shift tends to take a while. And for
pre-production work it's often impossible unfortunately.

Cheers, Daniel
>
> Best regards,
> Liviu
>
>>
>> Lucas Stach (17):
>>   drm/etnaviv: don't fail to build on arches without PHYS_OFFSET
>>   drm/etnaviv: add missing major features field to debugfs
>>   drm/etnaviv: hook up DRM GPU scheduler
>>   drm/etnaviv: move dependency handling to scheduler
>>   drm/etnaviv: lock BOs after all other submit work is done
>>   drm/etnaviv: replace hangcheck with scheduler timeout
>>   drm/etnaviv: use correct format specifier for size_t
>>   drm/etnaviv: split out and optimize MMU fault dumping
>>   drm/etnaviv: add support for slave interface clock
>>   drm/etnaviv: update hardware headers from rnndb
>>   drm/etnaviv: add more minor features fields
>>   drm/etnaviv: add hardware database
>>   drm/etnaviv: add security handling mode enum
>>   drm/etnaviv: handle security states
>>   drm/etnaviv: add function to load the initial PTA state
>>   drm/etnaviv: add PTA handling to MMUv2
>>   drm/etnaviv: bump HW job limit to 4
>>
>> Oded Gabbay (1):
>>  

Re: [git pull] drm for v4.17-rc1

2018-04-03 Thread Daniel Vetter
On Tue, Apr 3, 2018 at 1:52 PM, Daniel Vetter  wrote:
> On Tue, Apr 3, 2018 at 1:13 PM, Lucas Stach  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  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 fina

Re: [git pull] drm for v4.17-rc1

2018-04-03 Thread Daniel Vetter
On Tue, Apr 3, 2018 at 1:13 PM, Lucas Stach  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  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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [git pull] drm for v4.17-rc1

2018-04-03 Thread Lucas Stach
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  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.

Just my 2 cents,
Lucas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [git pull] drm for v4.17-rc1

2018-04-03 Thread Liviu Dudau
Hi Daniel,

On Tue, Apr 03, 2018 at 11:58:17AM +0200, Daniel Vetter 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.
> 
> Cheers, Daniel
> 
> Alex Deucher (2):
>   Revert "drm/radeon/pm: autoswitch power state when in balanced mode"
>   drm/amdgpu: add documentation for amdgpu_device.c
> 
> Dave Airlie (1):
>   drm/amd/pp: fix missing CONFIG_ACPI.
> 
> Frank Rowand (4):
>   of: change overlay apply input data from unflattened to FDT
>   of: Documentation: of_overlay_apply() replaced by of_overlay_fdt_apply()
>   of: convert unittest overlay devicetree source to sugar syntax
>   of: improve reporting invalid overlay target path
> 
> Joonas Lahtinen (5):
>   drm/i915: Update DRIVER_DATE to 20180207
>   drm/i915: Update DRIVER_DATE to 20180214
>   drm/i915: Update DRIVER_DATE to 20180221
>   drm/i915: Update DRIVER_DATE to 20180305
>   drm/i915: Update DRIVER_DATE to 20180308
> 
> Liviu Dudau (5):
>   drm/mali-dp: Rotated planes need a larger pitch size.
>   drm/mali-dp: Align pitch size to be multiple of bus burst read size.
>   drm/mali-dp: Don't enable scaling engine for planes that only rotate.
>   drm/mali-dp: Fix malidp_atomic_commit_hw_done() for event sending.
>   drm: mali-dp: Turn off CRTC vblank when removing module.

On the mali-dp driver there was a period of time where it was only me doing
the work on mainline driver, with the promise that more people are going to
join. That has now happened, so there are people reviewing the patches
internally, but we are currently failing because of old habits to record
their Reviewed-by properly when we transition the patches from internal
discussions to the public ones. We're going to try harder in the future to
not let maintainer patches go without proper review tags into the drm-next
pull request.

Best regards,
Liviu

> 
> Lucas Stach (17):
>   drm/etnaviv: don't fail to build on arches without PHYS_OFFSET
>   drm/etnaviv: add missing major features field to debugfs
>   drm/etnaviv: hook up DRM GPU scheduler
>   drm/etnaviv: move dependency handling to scheduler
>   drm/etnaviv: lock BOs after all other submit work is done
>   drm/etnaviv: replace hangcheck with scheduler timeout
>   drm/etnaviv: use correct format specifier for size_t
>   drm/etnaviv: split out and optimize MMU fault dumping
>   drm/etnaviv: add support for slave interface clock
>   drm/etnaviv: update hardware headers from rnndb
>   drm/etnaviv: add more minor features fields
>   drm/etnaviv: add hardware database
>   drm/etnaviv: add security handling mode enum
>   drm/etnaviv: handle security states
>   drm/etnaviv: add function to load the initial PTA state
>   drm/etnaviv: add PTA handling to MMUv2
>   drm/etnaviv: bump HW job limit to 4
> 
> Oded Gabbay (1):
>   drm/amdkfd: add missing include of mm.h
> 
> Rob Clark (8):
>   drm/msm: add a5xx specific debugfs
>   drm/msm: add sudo flag to submit ioctl
>   drm/msm: strip out msm_fence_cb
>   drm/msm/dsi: fix direct caller of msm_gem_free_object()
>   drm/msm/mdp5: rework CTL START signal handling
>   drm/msm/mdp5: print a bit more of the atomi

Re: [git pull] drm for v4.17-rc1

2018-04-03 Thread Daniel Vetter
On Tue, Apr 3, 2018 at 11:58 AM, Daniel Vetter  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 ...
Below the sorted list of people who didn't push their own patches, cut
off at 10.
-Daniel

   119  Christian König
   117  Rex Zhu
48  Laurent Pinchart
35  Felix Kuehling
32  Harry Wentland
24  Dhinakaran Pandiyan
23  Leo (Sunpeng) Li
21  Evan Quan
21  Yongqiang Sun
19  Changbin Du
18  Tom St Denis
17  Monk Liu
16  Jernej Skrabec
15  Samuel Li
13  Charlene Liu
12  Archit Taneja
12  Philippe CORNU
12  Ramalingam C
12  Shirish S
11  Hawking Zhang
11  Tony Cheng
10  Jeffy Chen
10  Mahesh Kumar
10  Meghana Madhyastha
-- 
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: [git pull] drm for v4.17-rc1

2018-04-03 Thread Daniel Vetter
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.

Cheers, Daniel

Alex Deucher (2):
  Revert "drm/radeon/pm: autoswitch power state when in balanced mode"
  drm/amdgpu: add documentation for amdgpu_device.c

Dave Airlie (1):
  drm/amd/pp: fix missing CONFIG_ACPI.

Frank Rowand (4):
  of: change overlay apply input data from unflattened to FDT
  of: Documentation: of_overlay_apply() replaced by of_overlay_fdt_apply()
  of: convert unittest overlay devicetree source to sugar syntax
  of: improve reporting invalid overlay target path

Joonas Lahtinen (5):
  drm/i915: Update DRIVER_DATE to 20180207
  drm/i915: Update DRIVER_DATE to 20180214
  drm/i915: Update DRIVER_DATE to 20180221
  drm/i915: Update DRIVER_DATE to 20180305
  drm/i915: Update DRIVER_DATE to 20180308

Liviu Dudau (5):
  drm/mali-dp: Rotated planes need a larger pitch size.
  drm/mali-dp: Align pitch size to be multiple of bus burst read size.
  drm/mali-dp: Don't enable scaling engine for planes that only rotate.
  drm/mali-dp: Fix malidp_atomic_commit_hw_done() for event sending.
  drm: mali-dp: Turn off CRTC vblank when removing module.

Lucas Stach (17):
  drm/etnaviv: don't fail to build on arches without PHYS_OFFSET
  drm/etnaviv: add missing major features field to debugfs
  drm/etnaviv: hook up DRM GPU scheduler
  drm/etnaviv: move dependency handling to scheduler
  drm/etnaviv: lock BOs after all other submit work is done
  drm/etnaviv: replace hangcheck with scheduler timeout
  drm/etnaviv: use correct format specifier for size_t
  drm/etnaviv: split out and optimize MMU fault dumping
  drm/etnaviv: add support for slave interface clock
  drm/etnaviv: update hardware headers from rnndb
  drm/etnaviv: add more minor features fields
  drm/etnaviv: add hardware database
  drm/etnaviv: add security handling mode enum
  drm/etnaviv: handle security states
  drm/etnaviv: add function to load the initial PTA state
  drm/etnaviv: add PTA handling to MMUv2
  drm/etnaviv: bump HW job limit to 4

Oded Gabbay (1):
  drm/amdkfd: add missing include of mm.h

Rob Clark (8):
  drm/msm: add a5xx specific debugfs
  drm/msm: add sudo flag to submit ioctl
  drm/msm: strip out msm_fence_cb
  drm/msm/dsi: fix direct caller of msm_gem_free_object()
  drm/msm/mdp5: rework CTL START signal handling
  drm/msm/mdp5: print a bit more of the atomic state
  drm/msm/mdp5: add missing LM flush bits
  drm/msm/mdp5: don't pre-reserve LM's if no dual-dsi

Thierry Reding (8):
  drm/tegra: gem: Reshuffle declarations
  drm/tegra: gem: Make __tegra_gem_mmap() available more widely
  drm/tegra: fb: Implement ->fb_mmap() callback
  drm/tegra: plane: Support format modifiers
  drm/tegra: fb: Properly support linear modifier
  drm/tegra: hub: Use private object for global state
  drm/tegra: gem: Map pages via the DMA API
  drm/tegra: prime: Implement ->{begin,end}_cpu_access()

Thomas Hellstrom (1):
  drm/vmwgfx: Bump version patchlevel and date

Tomi Valkeinen (11):
  drm/omap: reorganize locking in mgr_fld_write
  drm/omap: acx565akm:  use __be32 when reading status
  drm/omap: fbdev: use 'screen_buffer' fi