[Intel-gfx] [PATCH v2 0/7] drm: drm_fb_helper cleanup.

[Laxminarayan Bharadiya, Pankaj]

> -----Original Message-----
> From: Daniel Vetter <daniel at ffwll.ch>
> Sent: 03 March 2020 03:15
> To: Emil Velikov <emil.l.velikov at gmail.com>
> Cc: Laxminarayan Bharadiya, Pankaj
> <pankaj.laxminarayan.bharadiya at intel.com>; Jani Nikula
> <jani.nikula at linux.intel.com>; Daniel Vetter <daniel at ffwll.ch>; Intel
> Graphics Development <intel-gfx at lists.freedesktop.org>; ML dri-devel <dri-
> devel at lists.freedesktop.org>
> Subject: Re: [Intel-gfx] [PATCH v2 0/7] drm: drm_fb_helper cleanup.
> 
> On Mon, Mar 02, 2020 at 10:43:19PM +0100, Daniel Vetter wrote:
> > On Mon, Mar 02, 2020 at 06:21:23PM +0000, Emil Velikov wrote:
> > > Hi Pankaj,
> > >
> > > On Mon, 2 Mar 2020 at 16:33, Pankaj Bharadiya
> > > <pankaj.laxminarayan.bharadiya at intel.com> wrote:
> > > >
> > > > This series addresses below drm_fb_helper tasks from
> > > > Documentation/gpu/todo.rst.
> > > >
> > > > - The max connector argument for drm_fb_helper_init() isn't used
> > > >   anymore and can be removed.
> > > >
> > > > - The helper doesn't keep an array of connectors anymore so these can
> > > >   be removed: drm_fb_helper_single_add_all_connectors(),
> > > >   drm_fb_helper_add_one_connector() and
> > > >   drm_fb_helper_remove_one_connector().
> > > >
> > > > Changes since v1:
> > > >    - Squashed warning fixes into the patch that introduced the
> > > >      warnings (into 5/7) (Laurent)
> > > >    - Fixed reflow in in 9/9 (Laurent)
> > > >
> > > For the future, include the changelog in the respective patches.
> > > This makes it easier for reviewers...
> > > Plus you're already changing the commit - might as well mention
> > > what/why :-)
> > >
> > > Also do include the R-B, Acked-by, other tags accumulated up-to that
> > > point, when sending new revision.
> >
> > Yup, can you pls resend that entire pile with all the ack/review tags
> > from the earlier versions included? If you don't do that you waste
> > reviewers time. Another one is that resend right away also kinda
> > wastes peoples time, because there's a much bigger chance that someone
> > will review the old version, instead of your new one. Better wait of
> > at least 1-2 days or so.
> 
> Hm just noticed that people are still reviewing v1. I'd say lets forget about
> this v2 here, wait 1-2 days, and then resend with everything combined.
> Hopefully not too many will re-review v2 here and waste time on stuff that's
> reviewed already. Resending within hours is really not good on mailing lists
> (with merge requests or whatever it doesn't matter, because everyone
> always looks at the latest version).

Noted 😊. Thanks for the feedback. 
Will send the new series with tags accumulated after 1-2 days.

Thanks,
Pankaj
> -Daniel
> 
> >
> > > That said, if you're interested in further cleaning this up, one can
> > > cleanup the drm_dp_mst_topology_cbs hooks.
> > > In particular ::register_connector is identical across the board -
> > > create a helper function using it directly in core, killing the hook.
> > >
> > > While for ::destroy_connector - there's some amdgpu specific code in
> > > there... which I'm not sure if it should stay or not.
> > > To be on the save side - create a helper which will be called for
> > > drivers where the hook is !=NULL (aka everyone but amdgpu).
> >
> > Yeah that stuff looks fishy. Smells a bit like old mst code developed
> > before Lyude fixed the refcounting for real, it seems to manually shut
> > down stuff that should be cleaned up with normal code paths (modeset
> > and/or final kref_put on the connector).
> > -Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch