Re: [PATCH] Revert "drm/panel: Add device_link from panel device to DRM device"

[Sean Paul <sean@poorly.run>]

On Thu, Sep 27, 2018 at 04:25:56PM +0200, Andrzej Hajda wrote:
> On 27.09.2018 15:50, Sean Paul wrote:
> > [actually Cc Eric and Andrzej]
> >
> > On Thu, Sep 27, 2018 at 02:41:30PM +0200, Linus Walleij wrote:
> >> This reverts commit 0c08754b59da5557532d946599854e6df28edc22.
> >>
> >> commit 0c08754b59da
> >> ("drm/panel: Add device_link from panel device to DRM device")
> >> creates a circular dependency under these circumstances:
> >>
> >> 1. The panel depends on dsi-host because it is MIPI-DSI child
> >>    device.
> >> 2. dsi-host depends on the drm parent device (connector->dev->dev)
> >>    this should be allowed.
> >> 3. drm parent dev (connector->dev->dev) depends on the panel
> >>    after this patch.
> >>
> >> This makes the dependency circular and while it appears it
> >> does not affect any in-tree drivers (they do not seem to have
> >> dsi hosts depending on the same parent device) this does not
> >> seem right.
> > Hey Linus,
> > I'm trying to wrap my head around this :-)
> >
> > I just read through the original patch thread. It doesn't seem like this was
> > introduced to fix a bug? Will reverting this cause regressions on in-tree
> > drivers?
> 
> As I wrote in original patch thread, the original patch breaks platforms
> with exynos-dsi encoder.
> Quite detailed explanation in the original thread [1], 1st response to
> the patch.
> Apparently my response was ignored :)

Thanks Andrzej,
I've pushed this to drm-misc-fixes. It'll go out this week or next.

Thanks,

Sean

> 
> [1]: https://patchwork.freedesktop.org/patch/218878/
> 
> Regards
> Andrzej
> 
> >
> > What's different in your in-development driver that's causing you to hit
> > this?
> >
> > Thanks!
> >
> > Sean
> >
> >> As noted in a response from Andrzej Hajda, the intent is
> >> likely to make the panel dependent on the DRM device
> >> (connector->dev) not its parent. But we have no way of
> >> doing that since the DRM device doesn't contain any
> >> struct device on its own (arguably it should).
> >>
> >> Revert this until a proper approach is figured out.
> >>
> >> Cc: Jyri Sarha <jsarha@ti.com>
> >> Cc: Eric Anholt <eric@anholt.net>
> >> Cc: Andrzej Hajda <a.hajda@samsung.com>
> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> >> ---
> >>  drivers/gpu/drm/drm_panel.c | 10 ----------
> >>  include/drm/drm_panel.h     |  1 -
> >>  2 files changed, 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> >> index b902361dee6e..1d9a9d2fe0e0 100644
> >> --- a/drivers/gpu/drm/drm_panel.c
> >> +++ b/drivers/gpu/drm/drm_panel.c
> >> @@ -24,7 +24,6 @@
> >>  #include <linux/err.h>
> >>  #include <linux/module.h>
> >>  
> >> -#include <drm/drm_device.h>
> >>  #include <drm/drm_crtc.h>
> >>  #include <drm/drm_panel.h>
> >>  
> >> @@ -105,13 +104,6 @@ int drm_panel_attach(struct drm_panel *panel, struct drm_connector *connector)
> >>  	if (panel->connector)
> >>  		return -EBUSY;
> >>  
> >> -	panel->link = device_link_add(connector->dev->dev, panel->dev, 0);
> >> -	if (!panel->link) {
> >> -		dev_err(panel->dev, "failed to link panel to %s\n",
> >> -			dev_name(connector->dev->dev));
> >> -		return -EINVAL;
> >> -	}
> >> -
> >>  	panel->connector = connector;
> >>  	panel->drm = connector->dev;
> >>  
> >> @@ -133,8 +125,6 @@ EXPORT_SYMBOL(drm_panel_attach);
> >>   */
> >>  int drm_panel_detach(struct drm_panel *panel)
> >>  {
> >> -	device_link_del(panel->link);
> >> -
> >>  	panel->connector = NULL;
> >>  	panel->drm = NULL;
> >>  
> >> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> >> index 582a0ec0aa70..777814755fa6 100644
> >> --- a/include/drm/drm_panel.h
> >> +++ b/include/drm/drm_panel.h
> >> @@ -89,7 +89,6 @@ struct drm_panel {
> >>  	struct drm_device *drm;
> >>  	struct drm_connector *connector;
> >>  	struct device *dev;
> >> -	struct device_link *link;
> >>  
> >>  	const struct drm_panel_funcs *funcs;
> >>  
> >> -- 
> >> 2.17.1
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel