> -----Original Message----- > From: Daniel Vetter <daniel@xxxxxxxx> > Sent: 03 March 2020 03:15 > To: Emil Velikov <emil.l.velikov@xxxxxxxxx> > Cc: Laxminarayan Bharadiya, Pankaj > <pankaj.laxminarayan.bharadiya@xxxxxxxxx>; Jani Nikula > <jani.nikula@xxxxxxxxxxxxxxx>; Daniel Vetter <daniel@xxxxxxxx>; Intel > Graphics Development <intel-gfx@xxxxxxxxxxxxxxxxxxxxx>; ML dri-devel <dri- > devel@xxxxxxxxxxxxxxxxxxxxx> > Subject: Re: [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@xxxxxxxxx> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
