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

[Daniel Vetter]

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.

> 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