Re: [PATCH v5 08/10] drm/arm: malidp: Use crtc->mode_valid() callback

[Liviu Dudau]

On Wed, May 31, 2017 at 12:56:59PM +0200, Daniel Vetter wrote:
> On Wed, May 31, 2017 at 12:48 PM, Liviu Dudau <liviu.dudau@xxxxxxx> wrote:
> > On Wed, May 31, 2017 at 10:20:04AM +0200, Daniel Vetter wrote:
> >> On Tue, May 30, 2017 at 10:37:29AM +0100, Liviu Dudau wrote:
> >> > On Tue, May 30, 2017 at 09:29:44AM +0200, Neil Armstrong wrote:
> >> > > On 05/25/2017 04:19 PM, Jose Abreu wrote:
> >> > > > Now that we have a callback to check if crtc supports a given mode
> >> > > > we can use it in malidp so that we restrict the number of probbed
> >> > > > modes to the ones we can actually display.
> >> > > >
> >> > > > Also, remove the mode_fixup() callback as this is no longer needed
> >> > > > because mode_valid() will be called before.
> >> > > >
> >> > > > NOTE: Not even compiled tested
> >> >
> >> > I did compile it, even done some testing, but by no means have I managed
> >> > to cover all the cases. Looks OK to me.
> >> >
> >> > > >
> >> > > > Signed-off-by: Jose Abreu <joabreu@xxxxxxxxxxxx>
> >> > > > Cc: Carlos Palminha <palminha@xxxxxxxxxxxx>
> >> > > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> >> > > > Cc: Liviu Dudau <liviu.dudau@xxxxxxx>
> >> >
> >> > Acked-by: Liviu Dudau <liviu.dudau@xxxxxxx>
> >>
> >> What does this mean? Do you expect me to merge this through drm-misc? Or
> >> do you plan to merge it through your arm tree (all the required patches
> >> are in drm-misc-next and will be in Dave's tree soonish)?
> >>
> >> /me confused.
> >
> > /me too. :) I've only got Cc-ed on one patch, so I'm guessing the whole series is
> > going to be picked up through drm-misc. For patches that are part of a larger
> > series (to me) it makes sense to push them through a single channel. But I'm not
> > the author of the series so I don't know what Jose prefers. If Jose wants this
> > patch to go through mali-dp tree then I'm happy to pull it, otherwise I can sort out
> > the conflict(s) before sending a pull request to Dave.
> >
> > On the larger topic, I'm guessing this is not the first time a series touches multiple
> > drivers that are not together in a single tree. How was this sorted in the past? Is
> > there a better way?
> 
> I change my preferred merge strategy depending upon how invasive the
> patch is. Since this one here is more complex than a simple refactor,
> I prefer it goes in through the right trees. And the required patches
> are already in drm-misc-next now, so this should be doable.
> 
> For simpler stuff it's often easier to just get it landed through
> drm-misc, especially if it's just a dumb patch to e.g. add a new
> argument to a function and fill out the default one everywhere. For
> those I think it's not even required to get an ack from driver
> maintainers, just solid review of the idea&implementation in general.
> 
> A bit a grey thing in-between is refactorings that are simple, but
> require and audit on each driver, and then a final patch at the end to
> remove the old helper functions. My drm_vblank_cleanup removal is such
> a case. There I prefer driver maintainers to pick things up
> themselves, and 1 kernel release afterwards I'll put the leftover
> driver patches + the final cleanup into drm-misc.
> 
> Anyway, long story short: Your choice here. I just need to know
> whether you'll pick it up or want me to merge it through
> drm-misc-next. I think in general it'd be good if maintainers don't
> just ack patches, but also state what they expect to happen, e.g. when
> I ack something I try to make it clear that I expect this to go in
> through a different tree than one I maintain. Otherwise I just pick it
> up and merge (and say so).

OK, if Jose doesn't like a different approach then I'll pick up this patch.
Then I guess I'll keep an eye on when airlied's git tree and see when drm-misc-next
gets merged before sending my pull request.

And sorry for not stating my follow up action with the Ack, like I've said, I
thought the whole series will be picked up by you based on this reply:

https://lists.freedesktop.org/archives/dri-devel/2017-May/142377.html

Best regards,
Liviu

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

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    Â\_(ã)_/Â