| Message ID | 20230227173231.950107-2-jagan@amarulasolutions.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
On Mon, Feb 27, 2023 at 11:02:30PM +0530, Jagan Teki wrote: > drmm_panel_bridge_add_nodrm is an another type of DRM-managed action > helper with nodrm pointer. > > DRM pointer is required to perform DRM-managed action, > - The conventional component-based drm bridges, the DRM pointer can > access in component ops bind API. > - The non-component-based bridges (like host DSI bridges), the DRM > pointer can access only when a specific bridge has been found > via bridge->dev. > > This drmm_panel_bridge_add_nodrm helper exclusively for the > non-component-based bridges. > > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> It's the third or fourth time I'm telling you this, and second time today. You keep respinning that patch, but the issue lies in the samsung driver. Fix it and you won't need that helper. Until then, you'll only get an automatic NAK from me, I'm done reviewing those patches over and over again, making the same comments that you'll just ignore. Maxime
On Mon, Feb 27, 2023 at 11:17 PM Maxime Ripard <maxime@cerno.tech> wrote: > > On Mon, Feb 27, 2023 at 11:02:30PM +0530, Jagan Teki wrote: > > drmm_panel_bridge_add_nodrm is an another type of DRM-managed action > > helper with nodrm pointer. > > > > DRM pointer is required to perform DRM-managed action, > > - The conventional component-based drm bridges, the DRM pointer can > > access in component ops bind API. > > - The non-component-based bridges (like host DSI bridges), the DRM > > pointer can access only when a specific bridge has been found > > via bridge->dev. > > > > This drmm_panel_bridge_add_nodrm helper exclusively for the > > non-component-based bridges. > > > > Cc: Maxime Ripard <mripard@kernel.org> > > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> > > Cc: Linus Walleij <linus.walleij@linaro.org> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > It's the third or fourth time I'm telling you this, and second time > today. Apologies for this. > > You keep respinning that patch, but the issue lies in the samsung > driver. Fix it and you won't need that helper. This is where all get' stuck. You mentioned in a previous e-mail was, "I'm not quite sure what you mean. You shouldn't retrieve the bridge from the attach hook but from the probe / bind ones. If that's not working for you, this is a bug in the documentation we should address." If I understand correctly this would be the third case from the bridge documentation https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#special-care-with-mipi-dsi-bridges "The upstream driver uses the component framework and is a MIPI-DSI host. The bridge device uses the MIPI-DCS commands to be controlled. This is the same situation as above, and can run mipi_dsi_host_register() in either its probe or bind hooks." But the problem here, The samsung-dsim bridge common bridge across Exynos and i.MX8MM. So, the - Upstream Exynos is component-based and - Upstream i.MX8MM is non-component-based With this dual scenario, retrieving the bridge pointer in a bind will work for the Exynos case, and retrieving the bridge pointer in bridge. attach will work for i.MX8MM. This is what I mentioned deadlock scenario in a previous e-mail. Hope you clear the real issue now. Thanks, Jagan.
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c index e8aae3cdc73d..d235a3843fcb 100644 --- a/drivers/gpu/drm/bridge/panel.c +++ b/drivers/gpu/drm/bridge/panel.c @@ -378,6 +378,22 @@ static void drmm_drm_panel_bridge_release(struct drm_device *drm, void *ptr) drm_panel_bridge_remove(bridge); } +static struct drm_bridge * +drmm_panel_bridge_add_action(struct drm_device *drm, struct drm_panel *panel, + struct drm_bridge *bridge) +{ + int ret; + + ret = drmm_add_action_or_reset(drm, drmm_drm_panel_bridge_release, + bridge); + if (ret) + return ERR_PTR(ret); + + bridge->pre_enable_prev_first = panel->prepare_prev_first; + + return bridge; +} + /** * drmm_panel_bridge_add - Creates a DRM-managed &drm_bridge and * &drm_connector that just calls the @@ -394,22 +410,38 @@ struct drm_bridge *drmm_panel_bridge_add(struct drm_device *drm, struct drm_panel *panel) { struct drm_bridge *bridge; - int ret; bridge = drm_panel_bridge_add_typed(panel, panel->connector_type); if (IS_ERR(bridge)) return bridge; - ret = drmm_add_action_or_reset(drm, drmm_drm_panel_bridge_release, - bridge); - if (ret) - return ERR_PTR(ret); + return drmm_panel_bridge_add_action(drm, panel, bridge); +} +EXPORT_SYMBOL(drmm_panel_bridge_add); - bridge->pre_enable_prev_first = panel->prepare_prev_first; +/** + * drmm_panel_bridge_add_nodrm - Creates a DRM-managed &drm_bridge and + * &drm_connector that just calls the + * appropriate functions from &drm_panel + * with no @dev. + * + * @panel: The drm_panel being wrapped. Must be non-NULL. + * + * This is the DRM-managed version of drm_panel_bridge_add() which + * automatically calls drm_panel_bridge_remove() when @dev is cleaned + * up. + */ +struct drm_bridge *drmm_panel_bridge_add_nodrm(struct drm_panel *panel) +{ + struct drm_bridge *bridge; - return bridge; + bridge = drm_panel_bridge_add_typed(panel, panel->connector_type); + if (IS_ERR(bridge)) + return bridge; + + return drmm_panel_bridge_add_action(bridge->dev, panel, bridge); } -EXPORT_SYMBOL(drmm_panel_bridge_add); +EXPORT_SYMBOL(drmm_panel_bridge_add_nodrm); /** * drm_panel_bridge_connector - return the connector for the panel bridge diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 42f86327b40a..acc118bab758 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -912,6 +912,7 @@ struct drm_bridge *devm_drm_panel_bridge_add_typed(struct device *dev, u32 connector_type); struct drm_bridge *drmm_panel_bridge_add(struct drm_device *drm, struct drm_panel *panel); +struct drm_bridge *drmm_panel_bridge_add_nodrm(struct drm_panel *panel); struct drm_connector *drm_panel_bridge_connector(struct drm_bridge *bridge); #else static inline bool drm_bridge_is_panel(const struct drm_bridge *bridge)
drmm_panel_bridge_add_nodrm is an another type of DRM-managed action helper with nodrm pointer. DRM pointer is required to perform DRM-managed action, - The conventional component-based drm bridges, the DRM pointer can access in component ops bind API. - The non-component-based bridges (like host DSI bridges), the DRM pointer can access only when a specific bridge has been found via bridge->dev. This drmm_panel_bridge_add_nodrm helper exclusively for the non-component-based bridges. Cc: Maxime Ripard <mripard@kernel.org> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- Changes for v14: - new patch drivers/gpu/drm/bridge/panel.c | 48 ++++++++++++++++++++++++++++------ include/drm/drm_bridge.h | 1 + 2 files changed, 41 insertions(+), 8 deletions(-)