[PATCH v2 1/2] drm/panel: Remove drm_panel_detach() calls from all panel drives

Daniel Vetter daniel at ffwll.ch
Tue Mar 6 10:03:43 UTC 2018


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 at ti.com>
> >> ---
> >>  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


More information about the dri-devel mailing list