On Wednesday, August 9, 2023 9:54 PM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: > > On Mon, 7 Aug 2023 at 08:06, Liu Ying <victor.liu@xxxxxxx> wrote: > > > > Add the device link when panel bridge is attached and delete the link > > when panel bridge is detached. The drm device is the consumer while > > the panel device is the supplier. This makes sure that the drm device > > suspends eariler and resumes later than the panel device, hence resolves > > problems where the order is reversed, like the problematic case mentioned > > in the below link. > > > > Link: > https://lore.k/ > ernel.org%2Flkml%2FCAPDyKFr0XjrU_udKoUKQ_q8RWaUkyqL%2B8fV- > 7s1CTMqi7u3- > Rg%40mail.gmail.com%2FT%2F&data=05%7C01%7Cvictor.liu%40nxp.com%7 > Cb498937c20c94ab9148908db98e02662%7C686ea1d3bc2b4c6fa92cd99c5c30 > 1635%7C0%7C0%7C638271860697989733%7CUnknown%7CTWFpbGZsb3d8e > yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D > %7C3000%7C%7C%7C&sdata=iGMdYWbOeyVxzy9T9THCNh%2Ff%2BbKFLP0tI > m%2BowL7h5Og%3D&reserved=0 > > Suggested-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx> > > Signed-off-by: Liu Ying <victor.liu@xxxxxxx> > > Looks good to me! Just a minor question though, don't we need to > manage runtime PM too - or this is solely for system wide > suspend/resume? I think this is solely for system wide suspend/resume. AFAICS, there is no any particular need to manage runtime PM. > > Reviewed-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx> Thank you for your review. Regards, Liu Ying > > Kind regards > Uffe > > > --- > > v2->v3: > > * Improve commit message s/swapped/reversed/. > > > > v1->v2: > > * Fix bailout for panel_bridge_attach() in case device_link_add() fails. > > > > drivers/gpu/drm/bridge/panel.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/gpu/drm/bridge/panel.c > b/drivers/gpu/drm/bridge/panel.c > > index 9316384b4474..a6587d233505 100644 > > --- a/drivers/gpu/drm/bridge/panel.c > > +++ b/drivers/gpu/drm/bridge/panel.c > > @@ -4,6 +4,8 @@ > > * Copyright (C) 2017 Broadcom > > */ > > > > +#include <linux/device.h> > > + > > #include <drm/drm_atomic_helper.h> > > #include <drm/drm_bridge.h> > > #include <drm/drm_connector.h> > > @@ -19,6 +21,7 @@ struct panel_bridge { > > struct drm_bridge bridge; > > struct drm_connector connector; > > struct drm_panel *panel; > > + struct device_link *link; > > u32 connector_type; > > }; > > > > @@ -60,6 +63,8 @@ static int panel_bridge_attach(struct drm_bridge > *bridge, > > { > > struct panel_bridge *panel_bridge = > drm_bridge_to_panel_bridge(bridge); > > struct drm_connector *connector = &panel_bridge->connector; > > + struct drm_panel *panel = panel_bridge->panel; > > + struct drm_device *drm_dev = bridge->dev; > > int ret; > > > > if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) > > @@ -70,6 +75,14 @@ static int panel_bridge_attach(struct drm_bridge > *bridge, > > return -ENODEV; > > } > > > > + panel_bridge->link = device_link_add(drm_dev->dev, panel->dev, > > + DL_FLAG_STATELESS); > > + if (!panel_bridge->link) { > > + DRM_ERROR("Failed to add device link between %s and %s\n", > > + dev_name(drm_dev->dev), dev_name(panel->dev)); > > + return -EINVAL; > > + } > > + > > drm_connector_helper_add(connector, > > &panel_bridge_connector_helper_funcs); > > > > @@ -78,6 +91,7 @@ static int panel_bridge_attach(struct drm_bridge > *bridge, > > panel_bridge->connector_type); > > if (ret) { > > DRM_ERROR("Failed to initialize connector\n"); > > + device_link_del(panel_bridge->link); > > return ret; > > } > > > > @@ -100,6 +114,8 @@ static void panel_bridge_detach(struct drm_bridge > *bridge) > > struct panel_bridge *panel_bridge = > drm_bridge_to_panel_bridge(bridge); > > struct drm_connector *connector = &panel_bridge->connector; > > > > + device_link_del(panel_bridge->link); > > + > > /* > > * Cleanup the connector if we know it was initialized. > > * > > -- > > 2.37.1 > >