Message ID | 20190803104719.gwb6hmreeaqyye6n@flea (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [PULL] drm-misc-next | expand |
On Sat, 3 Aug 2019 at 20:47, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > Hi Daniel, Dave, > > Here is the first (and pretty late) drm-misc-next PR. > > It's pretty big due to the lateness, but there's nothing really major > showing up. It's pretty much the usual bunch of reworks, fixes, and > new helpers being introduced. > dim: 415d2e9e0757 ("Revert "drm/gem: Rename drm_gem_dumb_map_offset() to drm_gem_map_offset()""): mandatory review missing. dim: be855382bacb ("Revert "drm/panfrost: Use drm_gem_map_offset()""): mandatory review missing. dim: e4eee93d2577 ("drm/vgem: drop DRM_AUTH usage from the driver"): mandatory review missing. dim: 88209d2c5035 ("drm/msm: drop DRM_AUTH usage from the driver"): mandatory review missing. dim: ccdae4257569 ("drm/nouveau: remove open-coded drm_invalid_op()"): mandatory review missing. Pretty sure review in drm-misc-next is a rule. I don't even see acks on most of these. Dave.
On Tue, Aug 6, 2019 at 2:34 AM Dave Airlie <airlied@gmail.com> wrote: > > On Sat, 3 Aug 2019 at 20:47, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > Hi Daniel, Dave, > > > > Here is the first (and pretty late) drm-misc-next PR. > > > > It's pretty big due to the lateness, but there's nothing really major > > showing up. It's pretty much the usual bunch of reworks, fixes, and > > new helpers being introduced. > > > > dim: 415d2e9e0757 ("Revert "drm/gem: Rename drm_gem_dumb_map_offset() > to drm_gem_map_offset()""): mandatory review missing. > dim: be855382bacb ("Revert "drm/panfrost: Use drm_gem_map_offset()""): > mandatory review missing. > dim: e4eee93d2577 ("drm/vgem: drop DRM_AUTH usage from the driver"): > mandatory review missing. > dim: 88209d2c5035 ("drm/msm: drop DRM_AUTH usage from the driver"): > mandatory review missing. > dim: ccdae4257569 ("drm/nouveau: remove open-coded drm_invalid_op()"): > mandatory review missing. > > Pretty sure review in drm-misc-next is a rule. I don't even see acks > on most of these. Yes. I guess for reverts it's not cool, but also not the worst. Still better to get someone to ack, heck I can pull that off for emergency reverts with a few pings on irc, and the 2 reverts landed much later. But for normal patches it's definitely not ok at all. Also only possible if people bypass the tooling, or override the tooling with the -f flag to force a push. Rob, Emil, what's up here? Thanks, Daniel
On Tue, 6 Aug 2019 at 08:34, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > On Tue, Aug 6, 2019 at 2:34 AM Dave Airlie <airlied@gmail.com> wrote: > > > > On Sat, 3 Aug 2019 at 20:47, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > Hi Daniel, Dave, > > > > > > Here is the first (and pretty late) drm-misc-next PR. > > > > > > It's pretty big due to the lateness, but there's nothing really major > > > showing up. It's pretty much the usual bunch of reworks, fixes, and > > > new helpers being introduced. > > > > > > > dim: 415d2e9e0757 ("Revert "drm/gem: Rename drm_gem_dumb_map_offset() > > to drm_gem_map_offset()""): mandatory review missing. > > dim: be855382bacb ("Revert "drm/panfrost: Use drm_gem_map_offset()""): > > mandatory review missing. > > dim: e4eee93d2577 ("drm/vgem: drop DRM_AUTH usage from the driver"): > > mandatory review missing. > > dim: 88209d2c5035 ("drm/msm: drop DRM_AUTH usage from the driver"): > > mandatory review missing. > > dim: ccdae4257569 ("drm/nouveau: remove open-coded drm_invalid_op()"): > > mandatory review missing. > > > > Pretty sure review in drm-misc-next is a rule. I don't even see acks > > on most of these. > > Yes. I guess for reverts it's not cool, but also not the worst. Still > better to get someone to ack, heck I can pull that off for emergency > reverts with a few pings on irc, and the 2 reverts landed much later. > But for normal patches it's definitely not ok at all. Also only > possible if people bypass the tooling, or override the tooling with > the -f flag to force a push. > > Rob, Emil, what's up here? > I've got was an "Thanks" [1] from Ben on the nouveau patch - so I merged it. The msm and vgem ones are my bad - must have missed those one inbetween the other patches. Will double-check and follow-up on all of those. -Emil [1] https://lists.freedesktop.org/archives/dri-devel/2019-May/218823.html
On Tue, Aug 6, 2019 at 11:40 AM Emil Velikov <emil.l.velikov@gmail.com> wrote: > On Tue, 6 Aug 2019 at 08:34, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > > > On Tue, Aug 6, 2019 at 2:34 AM Dave Airlie <airlied@gmail.com> wrote: > > > > > > On Sat, 3 Aug 2019 at 20:47, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > > > Hi Daniel, Dave, > > > > > > > > Here is the first (and pretty late) drm-misc-next PR. > > > > > > > > It's pretty big due to the lateness, but there's nothing really major > > > > showing up. It's pretty much the usual bunch of reworks, fixes, and > > > > new helpers being introduced. > > > > > > > > > > dim: 415d2e9e0757 ("Revert "drm/gem: Rename drm_gem_dumb_map_offset() > > > to drm_gem_map_offset()""): mandatory review missing. > > > dim: be855382bacb ("Revert "drm/panfrost: Use drm_gem_map_offset()""): > > > mandatory review missing. > > > dim: e4eee93d2577 ("drm/vgem: drop DRM_AUTH usage from the driver"): > > > mandatory review missing. > > > dim: 88209d2c5035 ("drm/msm: drop DRM_AUTH usage from the driver"): > > > mandatory review missing. > > > dim: ccdae4257569 ("drm/nouveau: remove open-coded drm_invalid_op()"): > > > mandatory review missing. > > > > > > Pretty sure review in drm-misc-next is a rule. I don't even see acks > > > on most of these. > > > > Yes. I guess for reverts it's not cool, but also not the worst. Still > > better to get someone to ack, heck I can pull that off for emergency > > reverts with a few pings on irc, and the 2 reverts landed much later. > > But for normal patches it's definitely not ok at all. Also only > > possible if people bypass the tooling, or override the tooling with > > the -f flag to force a push. > > > > Rob, Emil, what's up here? > > > I've got was an "Thanks" [1] from Ben on the nouveau patch - so I merged it. > The msm and vgem ones are my bad - must have missed those one > inbetween the other patches. The thing is, dim push shouldn't allow you to do that. And the patches have clearly been applied with dim apply (or at least you added the Link), unlike Rob who seems to just have pushed the revert. If you used git push directly, then I guess you just volunteered to implement Daniel Stone's idea to enforce dim tooling. Adding Daniel, since I guess that was just an irc chat. -Daniel > Will double-check and follow-up on all of those. > > -Emil > [1] https://lists.freedesktop.org/archives/dri-devel/2019-May/218823.html -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Tue, 6 Aug 2019 at 10:49, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > On Tue, Aug 6, 2019 at 11:40 AM Emil Velikov <emil.l.velikov@gmail.com> wrote: > > On Tue, 6 Aug 2019 at 08:34, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > > > > > On Tue, Aug 6, 2019 at 2:34 AM Dave Airlie <airlied@gmail.com> wrote: > > > > > > > > On Sat, 3 Aug 2019 at 20:47, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > > > > > Hi Daniel, Dave, > > > > > > > > > > Here is the first (and pretty late) drm-misc-next PR. > > > > > > > > > > It's pretty big due to the lateness, but there's nothing really major > > > > > showing up. It's pretty much the usual bunch of reworks, fixes, and > > > > > new helpers being introduced. > > > > > > > > > > > > > dim: 415d2e9e0757 ("Revert "drm/gem: Rename drm_gem_dumb_map_offset() > > > > to drm_gem_map_offset()""): mandatory review missing. > > > > dim: be855382bacb ("Revert "drm/panfrost: Use drm_gem_map_offset()""): > > > > mandatory review missing. > > > > dim: e4eee93d2577 ("drm/vgem: drop DRM_AUTH usage from the driver"): > > > > mandatory review missing. > > > > dim: 88209d2c5035 ("drm/msm: drop DRM_AUTH usage from the driver"): > > > > mandatory review missing. > > > > dim: ccdae4257569 ("drm/nouveau: remove open-coded drm_invalid_op()"): > > > > mandatory review missing. > > > > > > > > Pretty sure review in drm-misc-next is a rule. I don't even see acks > > > > on most of these. > > > > > > Yes. I guess for reverts it's not cool, but also not the worst. Still > > > better to get someone to ack, heck I can pull that off for emergency > > > reverts with a few pings on irc, and the 2 reverts landed much later. > > > But for normal patches it's definitely not ok at all. Also only > > > possible if people bypass the tooling, or override the tooling with > > > the -f flag to force a push. > > > > > > Rob, Emil, what's up here? > > > > > I've got was an "Thanks" [1] from Ben on the nouveau patch - so I merged it. > > The msm and vgem ones are my bad - must have missed those one > > inbetween the other patches. > > The thing is, dim push shouldn't allow you to do that. And the patches > have clearly been applied with dim apply (or at least you added the > Link), unlike Rob who seems to just have pushed the revert. > Thanks, did not know about dim push. Will make sure I use it. -Emil
On Tue, Aug 6, 2019 at 11:49 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > On Tue, Aug 6, 2019 at 11:40 AM Emil Velikov <emil.l.velikov@gmail.com> wrote: > > On Tue, 6 Aug 2019 at 08:34, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > > > > > On Tue, Aug 6, 2019 at 2:34 AM Dave Airlie <airlied@gmail.com> wrote: > > > > > > > > On Sat, 3 Aug 2019 at 20:47, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > > > > > Hi Daniel, Dave, > > > > > > > > > > Here is the first (and pretty late) drm-misc-next PR. > > > > > > > > > > It's pretty big due to the lateness, but there's nothing really major > > > > > showing up. It's pretty much the usual bunch of reworks, fixes, and > > > > > new helpers being introduced. > > > > > > > > > > > > > dim: 415d2e9e0757 ("Revert "drm/gem: Rename drm_gem_dumb_map_offset() > > > > to drm_gem_map_offset()""): mandatory review missing. > > > > dim: be855382bacb ("Revert "drm/panfrost: Use drm_gem_map_offset()""): > > > > mandatory review missing. > > > > dim: e4eee93d2577 ("drm/vgem: drop DRM_AUTH usage from the driver"): > > > > mandatory review missing. > > > > dim: 88209d2c5035 ("drm/msm: drop DRM_AUTH usage from the driver"): > > > > mandatory review missing. > > > > dim: ccdae4257569 ("drm/nouveau: remove open-coded drm_invalid_op()"): > > > > mandatory review missing. > > > > > > > > Pretty sure review in drm-misc-next is a rule. I don't even see acks > > > > on most of these. > > > > > > Yes. I guess for reverts it's not cool, but also not the worst. Still > > > better to get someone to ack, heck I can pull that off for emergency > > > reverts with a few pings on irc, and the 2 reverts landed much later. > > > But for normal patches it's definitely not ok at all. Also only > > > possible if people bypass the tooling, or override the tooling with > > > the -f flag to force a push. > > > > > > Rob, Emil, what's up here? > > > > > I've got was an "Thanks" [1] from Ben on the nouveau patch - so I merged it. > > The msm and vgem ones are my bad - must have missed those one > > inbetween the other patches. > > The thing is, dim push shouldn't allow you to do that. And the patches > have clearly been applied with dim apply (or at least you added the > Link), unlike Rob who seems to just have pushed the revert. > > If you used git push directly, then I guess you just volunteered to > implement Daniel Stone's idea to enforce dim tooling. Adding Daniel, > since I guess that was just an irc chat. Helps if I actually add Daniel. -Daniel
On Tue, Aug 6, 2019 at 11:55 AM Emil Velikov <emil.l.velikov@gmail.com> wrote: > > On Tue, 6 Aug 2019 at 10:49, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > > > On Tue, Aug 6, 2019 at 11:40 AM Emil Velikov <emil.l.velikov@gmail.com> wrote: > > > On Tue, 6 Aug 2019 at 08:34, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > > > > > > > On Tue, Aug 6, 2019 at 2:34 AM Dave Airlie <airlied@gmail.com> wrote: > > > > > > > > > > On Sat, 3 Aug 2019 at 20:47, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > > > > > > > Hi Daniel, Dave, > > > > > > > > > > > > Here is the first (and pretty late) drm-misc-next PR. > > > > > > > > > > > > It's pretty big due to the lateness, but there's nothing really major > > > > > > showing up. It's pretty much the usual bunch of reworks, fixes, and > > > > > > new helpers being introduced. > > > > > > > > > > > > > > > > dim: 415d2e9e0757 ("Revert "drm/gem: Rename drm_gem_dumb_map_offset() > > > > > to drm_gem_map_offset()""): mandatory review missing. > > > > > dim: be855382bacb ("Revert "drm/panfrost: Use drm_gem_map_offset()""): > > > > > mandatory review missing. > > > > > dim: e4eee93d2577 ("drm/vgem: drop DRM_AUTH usage from the driver"): > > > > > mandatory review missing. > > > > > dim: 88209d2c5035 ("drm/msm: drop DRM_AUTH usage from the driver"): > > > > > mandatory review missing. > > > > > dim: ccdae4257569 ("drm/nouveau: remove open-coded drm_invalid_op()"): > > > > > mandatory review missing. > > > > > > > > > > Pretty sure review in drm-misc-next is a rule. I don't even see acks > > > > > on most of these. > > > > > > > > Yes. I guess for reverts it's not cool, but also not the worst. Still > > > > better to get someone to ack, heck I can pull that off for emergency > > > > reverts with a few pings on irc, and the 2 reverts landed much later. > > > > But for normal patches it's definitely not ok at all. Also only > > > > possible if people bypass the tooling, or override the tooling with > > > > the -f flag to force a push. > > > > > > > > Rob, Emil, what's up here? > > > > > > > I've got was an "Thanks" [1] from Ben on the nouveau patch - so I merged it. > > > The msm and vgem ones are my bad - must have missed those one > > > inbetween the other patches. > > > > The thing is, dim push shouldn't allow you to do that. And the patches > > have clearly been applied with dim apply (or at least you added the > > Link), unlike Rob who seems to just have pushed the revert. > > > Thanks, did not know about dim push. Will make sure I use it. So the intro doc isn't good enough, and we need to enforce it. I think Daniel's idea was to have a pre-merge hook which checks for a git variable using --push-option. Can you pls look into this? I guess we'd need the dim patch, and example premerge hook to be installed server-side. Should have a nice error message too ofc. -Daniel
Hi, On Tue, 6 Aug 2019 at 10:58, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > On Tue, Aug 6, 2019 at 11:55 AM Emil Velikov <emil.l.velikov@gmail.com> wrote: > > On Tue, 6 Aug 2019 at 10:49, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > > The thing is, dim push shouldn't allow you to do that. And the patches > > > have clearly been applied with dim apply (or at least you added the > > > Link), unlike Rob who seems to just have pushed the revert. > > > > > Thanks, did not know about dim push. Will make sure I use it. > > So the intro doc isn't good enough, and we need to enforce it. I think > Daniel's idea was to have a pre-merge hook which checks for a git > variable using --push-option. Can you pls look into this? I guess we'd > need the dim patch, and example premerge hook to be installed > server-side. Should have a nice error message too ofc. Yeah, the docs are already quite clear that you cannot push to the DRM trees with normal git, and that you have to use dim. Not only does it check and enforce all the rules in the documentation, but it also rebuilds drm-tip and keeps other trees in sync, which isn't done with a regular git push. The idea I had a few weeks ago was to have dim use 'git push --push-option fdo.pushedWithDim=this-was-pushed-with-dim-and-not-manually', then have the hooks on the server side check for that option and refuse any direct pushes. (Or at least, if people are pushing directly, they have to _really_ try to be doing it, and can't do it by accident.) If someone types up the dim patch to do that and gets it committed, after a couple of days' grace period for everyone to update I can roll out the server-side hooks which refuse non-dim pushes. Cheers, Daniel
On Tue, 6 Aug 2019 at 11:14, Daniel Stone <daniel@fooishbar.org> wrote: > The idea I had a few weeks ago was to have dim use 'git push > --push-option fdo.pushedWithDim=this-was-pushed-with-dim-and-not-manually', > then have the hooks on the server side check for that option and > refuse any direct pushes. (Or at least, if people are pushing > directly, they have to _really_ try to be doing it, and can't do it by > accident.) > Let me try and write a DIM patch for that. -Emil
On Tue, 06 Aug 2019, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On Tue, 6 Aug 2019 at 11:14, Daniel Stone <daniel@fooishbar.org> wrote: > >> The idea I had a few weeks ago was to have dim use 'git push >> --push-option fdo.pushedWithDim=this-was-pushed-with-dim-and-not-manually', >> then have the hooks on the server side check for that option and >> refuse any direct pushes. (Or at least, if people are pushing >> directly, they have to _really_ try to be doing it, and can't do it by >> accident.) >> > Let me try and write a DIM patch for that. Ooops, I was wondering how this would all work out, and ended up writing the patch [1]. BR, Jani. [1] http://marc.info/?i=20190806104630.14675-1-jani.nikula@intel.com
On Tue, Aug 6, 2019 at 1:34 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > On Tue, Aug 6, 2019 at 2:34 AM Dave Airlie <airlied@gmail.com> wrote: > > > > On Sat, 3 Aug 2019 at 20:47, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > Hi Daniel, Dave, > > > > > > Here is the first (and pretty late) drm-misc-next PR. > > > > > > It's pretty big due to the lateness, but there's nothing really major > > > showing up. It's pretty much the usual bunch of reworks, fixes, and > > > new helpers being introduced. > > > > > > > dim: 415d2e9e0757 ("Revert "drm/gem: Rename drm_gem_dumb_map_offset() > > to drm_gem_map_offset()""): mandatory review missing. > > dim: be855382bacb ("Revert "drm/panfrost: Use drm_gem_map_offset()""): > > mandatory review missing. > > dim: e4eee93d2577 ("drm/vgem: drop DRM_AUTH usage from the driver"): > > mandatory review missing. > > dim: 88209d2c5035 ("drm/msm: drop DRM_AUTH usage from the driver"): > > mandatory review missing. > > dim: ccdae4257569 ("drm/nouveau: remove open-coded drm_invalid_op()"): > > mandatory review missing. > > > > Pretty sure review in drm-misc-next is a rule. I don't even see acks > > on most of these. > > Yes. I guess for reverts it's not cool, but also not the worst. Still > better to get someone to ack, heck I can pull that off for emergency > reverts with a few pings on irc, and the 2 reverts landed much later. > But for normal patches it's definitely not ok at all. Also only > possible if people bypass the tooling, or override the tooling with > the -f flag to force a push. > > Rob, Emil, what's up here? I committed the changes, they turned out to clearly break things and not be fixable in any way. I said I was going to revert them[1] in reply to the original, got no reply, and so I reverted them. Seemed sufficient to me, but next time I'll keep the tool happy. Rob [1] https://lists.freedesktop.org/archives/dri-devel/2019-July/225092.html
On Tue, Aug 6, 2019 at 4:25 PM Rob Herring <robh@kernel.org> wrote: > > On Tue, Aug 6, 2019 at 1:34 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > > > On Tue, Aug 6, 2019 at 2:34 AM Dave Airlie <airlied@gmail.com> wrote: > > > > > > On Sat, 3 Aug 2019 at 20:47, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > > > Hi Daniel, Dave, > > > > > > > > Here is the first (and pretty late) drm-misc-next PR. > > > > > > > > It's pretty big due to the lateness, but there's nothing really major > > > > showing up. It's pretty much the usual bunch of reworks, fixes, and > > > > new helpers being introduced. > > > > > > > > > > dim: 415d2e9e0757 ("Revert "drm/gem: Rename drm_gem_dumb_map_offset() > > > to drm_gem_map_offset()""): mandatory review missing. > > > dim: be855382bacb ("Revert "drm/panfrost: Use drm_gem_map_offset()""): > > > mandatory review missing. > > > dim: e4eee93d2577 ("drm/vgem: drop DRM_AUTH usage from the driver"): > > > mandatory review missing. > > > dim: 88209d2c5035 ("drm/msm: drop DRM_AUTH usage from the driver"): > > > mandatory review missing. > > > dim: ccdae4257569 ("drm/nouveau: remove open-coded drm_invalid_op()"): > > > mandatory review missing. > > > > > > Pretty sure review in drm-misc-next is a rule. I don't even see acks > > > on most of these. > > > > Yes. I guess for reverts it's not cool, but also not the worst. Still > > better to get someone to ack, heck I can pull that off for emergency > > reverts with a few pings on irc, and the 2 reverts landed much later. > > But for normal patches it's definitely not ok at all. Also only > > possible if people bypass the tooling, or override the tooling with > > the -f flag to force a push. > > > > Rob, Emil, what's up here? > > I committed the changes, they turned out to clearly break things and > not be fixable in any way. I said I was going to revert them[1] in > reply to the original, got no reply, and so I reverted them. Seemed > sufficient to me, but next time I'll keep the tool happy. Generally submitting the full revert and then going ack-shopping on irc you can land reverts within a few hours tops. Seeing a patch with the dreaded "Revert" subject gets people going a lot more than a reply in some old thread somewhere, which is easily overlooked. -Daniel > > Rob > > [1] https://lists.freedesktop.org/archives/dri-devel/2019-July/225092.html
On Tue, Aug 06, 2019 at 10:33:53AM +1000, Dave Airlie wrote: > On Sat, 3 Aug 2019 at 20:47, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > Hi Daniel, Dave, > > > > Here is the first (and pretty late) drm-misc-next PR. > > > > It's pretty big due to the lateness, but there's nothing really major > > showing up. It's pretty much the usual bunch of reworks, fixes, and > > new helpers being introduced. > > dim: 415d2e9e0757 ("Revert "drm/gem: Rename drm_gem_dumb_map_offset() > to drm_gem_map_offset()""): mandatory review missing. > dim: be855382bacb ("Revert "drm/panfrost: Use drm_gem_map_offset()""): > mandatory review missing. > dim: e4eee93d2577 ("drm/vgem: drop DRM_AUTH usage from the driver"): > mandatory review missing. > dim: 88209d2c5035 ("drm/msm: drop DRM_AUTH usage from the driver"): > mandatory review missing. > dim: ccdae4257569 ("drm/nouveau: remove open-coded drm_invalid_op()"): > mandatory review missing. > > Pretty sure review in drm-misc-next is a rule. I don't even see acks > on most of these. Ugh, sorry for that. I guess I'm still pretty new to the maintainer-side of dim, which commands did you use to check that? Thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Tue, Aug 06, 2019 at 06:01:46PM +0200, Maxime Ripard wrote: > On Tue, Aug 06, 2019 at 10:33:53AM +1000, Dave Airlie wrote: > > On Sat, 3 Aug 2019 at 20:47, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > Hi Daniel, Dave, > > > > > > Here is the first (and pretty late) drm-misc-next PR. > > > > > > It's pretty big due to the lateness, but there's nothing really major > > > showing up. It's pretty much the usual bunch of reworks, fixes, and > > > new helpers being introduced. > > > > dim: 415d2e9e0757 ("Revert "drm/gem: Rename drm_gem_dumb_map_offset() > > to drm_gem_map_offset()""): mandatory review missing. > > dim: be855382bacb ("Revert "drm/panfrost: Use drm_gem_map_offset()""): > > mandatory review missing. > > dim: e4eee93d2577 ("drm/vgem: drop DRM_AUTH usage from the driver"): > > mandatory review missing. > > dim: 88209d2c5035 ("drm/msm: drop DRM_AUTH usage from the driver"): > > mandatory review missing. > > dim: ccdae4257569 ("drm/nouveau: remove open-coded drm_invalid_op()"): > > mandatory review missing. > > > > Pretty sure review in drm-misc-next is a rule. I don't even see acks > > on most of these. > > Ugh, sorry for that. I guess I'm still pretty new to the > maintainer-side of dim, which commands did you use to check that? dim apply-pull does this. If all committers use the tooling as they should they shouldn't be able to push patches which violate anything here, that's why dim request-pull doesn't reject. We're now working on patches to make sure you really have to use dim for managing drm-misc and applying patches. -Daniel
On Tue, Aug 06, 2019 at 06:11:32PM +0200, Daniel Vetter wrote: > On Tue, Aug 06, 2019 at 06:01:46PM +0200, Maxime Ripard wrote: > > On Tue, Aug 06, 2019 at 10:33:53AM +1000, Dave Airlie wrote: > > > On Sat, 3 Aug 2019 at 20:47, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > > > Hi Daniel, Dave, > > > > > > > > Here is the first (and pretty late) drm-misc-next PR. > > > > > > > > It's pretty big due to the lateness, but there's nothing really major > > > > showing up. It's pretty much the usual bunch of reworks, fixes, and > > > > new helpers being introduced. > > > > > > dim: 415d2e9e0757 ("Revert "drm/gem: Rename drm_gem_dumb_map_offset() > > > to drm_gem_map_offset()""): mandatory review missing. > > > dim: be855382bacb ("Revert "drm/panfrost: Use drm_gem_map_offset()""): > > > mandatory review missing. > > > dim: e4eee93d2577 ("drm/vgem: drop DRM_AUTH usage from the driver"): > > > mandatory review missing. > > > dim: 88209d2c5035 ("drm/msm: drop DRM_AUTH usage from the driver"): > > > mandatory review missing. > > > dim: ccdae4257569 ("drm/nouveau: remove open-coded drm_invalid_op()"): > > > mandatory review missing. > > > > > > Pretty sure review in drm-misc-next is a rule. I don't even see acks > > > on most of these. > > > > Ugh, sorry for that. I guess I'm still pretty new to the > > maintainer-side of dim, which commands did you use to check that? > > dim apply-pull does this. If all committers use the tooling as they should > they shouldn't be able to push patches which violate anything here, that's > why dim request-pull doesn't reject. Yeah, sure, I meant to ask if there was anyway to check this before sending the PR on our end. > We're now working on patqches to make sure you really have to use > dim for managing drm-misc and applying patches. Great, thanks Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Wed, Aug 7, 2019 at 2:02 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > On Tue, Aug 06, 2019 at 06:11:32PM +0200, Daniel Vetter wrote: > > On Tue, Aug 06, 2019 at 06:01:46PM +0200, Maxime Ripard wrote: > > > On Tue, Aug 06, 2019 at 10:33:53AM +1000, Dave Airlie wrote: > > > > On Sat, 3 Aug 2019 at 20:47, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > > > > > Hi Daniel, Dave, > > > > > > > > > > Here is the first (and pretty late) drm-misc-next PR. > > > > > > > > > > It's pretty big due to the lateness, but there's nothing really major > > > > > showing up. It's pretty much the usual bunch of reworks, fixes, and > > > > > new helpers being introduced. > > > > > > > > dim: 415d2e9e0757 ("Revert "drm/gem: Rename drm_gem_dumb_map_offset() > > > > to drm_gem_map_offset()""): mandatory review missing. > > > > dim: be855382bacb ("Revert "drm/panfrost: Use drm_gem_map_offset()""): > > > > mandatory review missing. > > > > dim: e4eee93d2577 ("drm/vgem: drop DRM_AUTH usage from the driver"): > > > > mandatory review missing. > > > > dim: 88209d2c5035 ("drm/msm: drop DRM_AUTH usage from the driver"): > > > > mandatory review missing. > > > > dim: ccdae4257569 ("drm/nouveau: remove open-coded drm_invalid_op()"): > > > > mandatory review missing. > > > > > > > > Pretty sure review in drm-misc-next is a rule. I don't even see acks > > > > on most of these. > > > > > > Ugh, sorry for that. I guess I'm still pretty new to the > > > maintainer-side of dim, which commands did you use to check that? > > > > dim apply-pull does this. If all committers use the tooling as they should > > they shouldn't be able to push patches which violate anything here, that's > > why dim request-pull doesn't reject. > > Yeah, sure, I meant to ask if there was anyway to check this before > sending the PR on our end. I think we'd need to create new command. Or maybe we should integrate it as part of the pull request generation, as an information/warning that there might be problems you need to explain. Want to write a dim patch? -Daniel > > > We're now working on patqches to make sure you really have to use > > dim for managing drm-misc and applying patches. > > Great, thanks > > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
Hi, On Wed, Aug 07, 2019 at 02:30:25PM +0200, Daniel Vetter wrote: > On Wed, Aug 7, 2019 at 2:02 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Tue, Aug 06, 2019 at 06:11:32PM +0200, Daniel Vetter wrote: > > > On Tue, Aug 06, 2019 at 06:01:46PM +0200, Maxime Ripard wrote: > > > > On Tue, Aug 06, 2019 at 10:33:53AM +1000, Dave Airlie wrote: > > > > > On Sat, 3 Aug 2019 at 20:47, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > > > > > > > Hi Daniel, Dave, > > > > > > > > > > > > Here is the first (and pretty late) drm-misc-next PR. > > > > > > > > > > > > It's pretty big due to the lateness, but there's nothing really major > > > > > > showing up. It's pretty much the usual bunch of reworks, fixes, and > > > > > > new helpers being introduced. > > > > > > > > > > dim: 415d2e9e0757 ("Revert "drm/gem: Rename drm_gem_dumb_map_offset() > > > > > to drm_gem_map_offset()""): mandatory review missing. > > > > > dim: be855382bacb ("Revert "drm/panfrost: Use drm_gem_map_offset()""): > > > > > mandatory review missing. > > > > > dim: e4eee93d2577 ("drm/vgem: drop DRM_AUTH usage from the driver"): > > > > > mandatory review missing. > > > > > dim: 88209d2c5035 ("drm/msm: drop DRM_AUTH usage from the driver"): > > > > > mandatory review missing. > > > > > dim: ccdae4257569 ("drm/nouveau: remove open-coded drm_invalid_op()"): > > > > > mandatory review missing. > > > > > > > > > > Pretty sure review in drm-misc-next is a rule. I don't even see acks > > > > > on most of these. > > > > > > > > Ugh, sorry for that. I guess I'm still pretty new to the > > > > maintainer-side of dim, which commands did you use to check that? > > > > > > dim apply-pull does this. If all committers use the tooling as they should > > > they shouldn't be able to push patches which violate anything here, that's > > > why dim request-pull doesn't reject. > > > > Yeah, sure, I meant to ask if there was anyway to check this before > > sending the PR on our end. > > I think we'd need to create new command. Or maybe we should integrate > it as part of the pull request generation, as an information/warning > that there might be problems you need to explain. Want to write a dim > patch? I just sent something that should catch that, but considering my dim-fu, you probably want to review it :) Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com