On Wed, Feb 28, 2018 at 11:31:53PM +0200, Jyri Sarha wrote: > On 28/02/18 20:53, Thierry Reding wrote: > > On Wed, Feb 28, 2018 at 01:09:29PM +0200, Jyri Sarha wrote: > >> Setting the connector and drm to NULL when the drm panel device is > >> going away hardly serves any purpose. Usually the the whole memory > >> stucture is freed right after the remove call. > >> > >> Signed-off-by: Jyri Sarha <jsarha@xxxxxx> > >> --- > >> drivers/gpu/drm/panel/panel-innolux-p079zca.c | 1 - > >> drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 1 - > >> drivers/gpu/drm/panel/panel-lvds.c | 1 - > >> drivers/gpu/drm/panel/panel-panasonic-vvx10f034n00.c | 1 - > >> drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 1 - > >> drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 1 - > >> drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 1 - > >> drivers/gpu/drm/panel/panel-simple.c | 1 - > >> drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 1 - > >> 9 files changed, 9 deletions(-) > > > > I don't understand the purpose of this patch. I'll grant you that the > > current implementation of drm_panel_detach() is not very useful, but > > then you add code to drm_panel_detach() in the next patch and mention > > in the commit message that panel drivers should be calling the > > drm_panel_detach() function to remove the link. > > > > This is confusing. Can you clarify? > > > > When looking at the current implementation it does not make any sense to > me to call drm_panel_detach() from the panel driver itself. However, it > makes perfect sense calling it from drm driver. Setting panel->connector > = NULL marks it free and attachable to other devices, but the panel > driver that the passive element in the picture should not go and mark > itself available on its own. > > But now that I take the steps to make the drm_panel_detach() to be > called only from drm device I should at least document it too. > > Also in general I think it is hard to come up with a detach > implementation that would work from both panel and the drm device. I think we first need a series which changes drm_panel_detach to be called by drm drivers (not panel drivers), and have a drm_panel_remove of similar (like we do with bridges) to remove the panel driver. Then I think this series here makes a lot more sense as a follow-up. Otherwise it's indeed rather confusing. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
