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

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, May 31, 2017 at 12:48 PM, Liviu Dudau <liviu.dudau@arm.com> 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@synopsys.com>
>> > > > Cc: Carlos Palminha <palminha@synopsys.com>
>> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> > > > Cc: Liviu Dudau <liviu.dudau@arm.com>
>> >
>> > Acked-by: Liviu Dudau <liviu.dudau@arm.com>
>>
>> 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).

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