| Message ID | 20230227113925.875425-3-jagan@amarulasolutions.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm: Add Samsung MIPI DSIM bridge | expand |
On Mon, Feb 27, 2023 at 05:09:09PM +0530, Jagan Teki wrote: > drmm_panel_bridge_add DRM-managed action helper is useful for the bridge > which automatically removes the bridge when drm pointer is cleaned. > > Supporting the same on non-component bridges like host DSI bridge requires > a drm pointer which is indeed available only when a panel-bridge is found. > > For these use cases, the caller would call the drmm_panel_bridge_add by > passing NULL to drm pointer. > > So, assign the bridge->dev to drm pointer for those cases. > > 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 v13: > - new patch > > Note: use case on > "[PATCH v13 04/18] drm: exynos: dsi: Switch to DSI panel or bridge find helper" > > drivers/gpu/drm/bridge/panel.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > index d4b112911a99..45a0c6671000 100644 > --- a/drivers/gpu/drm/bridge/panel.c > +++ b/drivers/gpu/drm/bridge/panel.c > @@ -402,6 +402,13 @@ struct drm_bridge *drmm_panel_bridge_add(struct drm_device *drm, > if (IS_ERR(bridge)) > return bridge; > > + /* > + * For non-component bridges, like host DSI bridge the DRM pointer > + * can be available only when a panel-bridge is found. > + */ > + if (!drm) > + drm = bridge->dev; > + Why can't the caller use bridge->dev? Also, where did the devm_drm_of_dsi_get_bridge go? I thought you were going to convert it to a drm-managed version? Maxime
On Mon, Feb 27, 2023 at 5:41 PM Maxime Ripard <maxime@cerno.tech> wrote: > > On Mon, Feb 27, 2023 at 05:09:09PM +0530, Jagan Teki wrote: > > drmm_panel_bridge_add DRM-managed action helper is useful for the bridge > > which automatically removes the bridge when drm pointer is cleaned. > > > > Supporting the same on non-component bridges like host DSI bridge requires > > a drm pointer which is indeed available only when a panel-bridge is found. > > > > For these use cases, the caller would call the drmm_panel_bridge_add by > > passing NULL to drm pointer. > > > > So, assign the bridge->dev to drm pointer for those cases. > > > > 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 v13: > > - new patch > > > > Note: use case on > > "[PATCH v13 04/18] drm: exynos: dsi: Switch to DSI panel or bridge find helper" > > > > drivers/gpu/drm/bridge/panel.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > > index d4b112911a99..45a0c6671000 100644 > > --- a/drivers/gpu/drm/bridge/panel.c > > +++ b/drivers/gpu/drm/bridge/panel.c > > @@ -402,6 +402,13 @@ struct drm_bridge *drmm_panel_bridge_add(struct drm_device *drm, > > if (IS_ERR(bridge)) > > return bridge; > > > > + /* > > + * For non-component bridges, like host DSI bridge the DRM pointer > > + * can be available only when a panel-bridge is found. > > + */ > > + if (!drm) > > + drm = bridge->dev; > > + > > Why can't the caller use bridge->dev? The first step of drmm_panel_bridge_add is to find the panel-bridge, so we can only get bridge->dev after this step. The caller doesn't know anything about the panel bridge it just sends a panel pointer to find the panel-bridge and then assigns bridge->dev to drm for DRM action. Please check this patch about the caller, https://patchwork.kernel.org/project/dri-devel/patch/20230227113925.875425-5-jagan@amarulasolutions.com/ > > Also, where did the devm_drm_of_dsi_get_bridge go? I thought you were > going to convert it to a drm-managed version? Look like your suggestion to can't use devm_drm_of_dsi_get_bridge and call the DRM-action from the driver, that is the reason I have removed this and done the same as your previous inputs. https://lore.kernel.org/all/20230203110437.otzl2azscbujigv6@houat/ Jagan.
On Mon, Feb 27, 2023 at 5:41 PM Maxime Ripard <maxime@cerno.tech> wrote: > > On Mon, Feb 27, 2023 at 05:09:09PM +0530, Jagan Teki wrote: > > drmm_panel_bridge_add DRM-managed action helper is useful for the bridge > > which automatically removes the bridge when drm pointer is cleaned. > > > > Supporting the same on non-component bridges like host DSI bridge requires > > a drm pointer which is indeed available only when a panel-bridge is found. > > > > For these use cases, the caller would call the drmm_panel_bridge_add by > > passing NULL to drm pointer. > > > > So, assign the bridge->dev to drm pointer for those cases. > > > > 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 v13: > > - new patch > > > > Note: use case on > > "[PATCH v13 04/18] drm: exynos: dsi: Switch to DSI panel or bridge find helper" > > > > drivers/gpu/drm/bridge/panel.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > > index d4b112911a99..45a0c6671000 100644 > > --- a/drivers/gpu/drm/bridge/panel.c > > +++ b/drivers/gpu/drm/bridge/panel.c > > @@ -402,6 +402,13 @@ struct drm_bridge *drmm_panel_bridge_add(struct drm_device *drm, > > if (IS_ERR(bridge)) > > return bridge; > > > > + /* > > + * For non-component bridges, like host DSI bridge the DRM pointer > > + * can be available only when a panel-bridge is found. > > + */ > > + if (!drm) > > + drm = bridge->dev; > > + > > Why can't the caller use bridge->dev? > > Also, where did the devm_drm_of_dsi_get_bridge go? I thought you were > going to convert it to a drm-managed version? I found another solution that supports DRM-managed action common across dsi and normal bridges, can I send those patches alone by excluding them from this series? Please let me know. Jagan.
On Mon, Feb 27, 2023 at 05:58:03PM +0530, Jagan Teki wrote: > On Mon, Feb 27, 2023 at 5:41 PM Maxime Ripard <maxime@cerno.tech> wrote: > > > > On Mon, Feb 27, 2023 at 05:09:09PM +0530, Jagan Teki wrote: > > > drmm_panel_bridge_add DRM-managed action helper is useful for the bridge > > > which automatically removes the bridge when drm pointer is cleaned. > > > > > > Supporting the same on non-component bridges like host DSI bridge requires > > > a drm pointer which is indeed available only when a panel-bridge is found. > > > > > > For these use cases, the caller would call the drmm_panel_bridge_add by > > > passing NULL to drm pointer. > > > > > > So, assign the bridge->dev to drm pointer for those cases. > > > > > > 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 v13: > > > - new patch > > > > > > Note: use case on > > > "[PATCH v13 04/18] drm: exynos: dsi: Switch to DSI panel or bridge find helper" > > > > > > drivers/gpu/drm/bridge/panel.c | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > > > index d4b112911a99..45a0c6671000 100644 > > > --- a/drivers/gpu/drm/bridge/panel.c > > > +++ b/drivers/gpu/drm/bridge/panel.c > > > @@ -402,6 +402,13 @@ struct drm_bridge *drmm_panel_bridge_add(struct drm_device *drm, > > > if (IS_ERR(bridge)) > > > return bridge; > > > > > > + /* > > > + * For non-component bridges, like host DSI bridge the DRM pointer > > > + * can be available only when a panel-bridge is found. > > > + */ > > > + if (!drm) > > > + drm = bridge->dev; > > > + > > > > Why can't the caller use bridge->dev? > > The first step of drmm_panel_bridge_add is to find the panel-bridge, > so we can only get bridge->dev after this step. The caller doesn't > know anything about the panel bridge it just sends a panel pointer to > find the panel-bridge Ah, yes, indeed. This is still a hack we don't need. > then assigns bridge->dev to drm for DRM action > > Please check this patch about the caller, > https://patchwork.kernel.org/project/dri-devel/patch/20230227113925.875425-5-jagan@amarulasolutions.com/ Because we've already been over this. You can't call that function from DSI's attach. So you should change that to what I already pointed you to, and then you'll have the drm device pointer available. > > Also, where did the devm_drm_of_dsi_get_bridge go? I thought you were > > going to convert it to a drm-managed version? > > Look like your suggestion to can't use devm_drm_of_dsi_get_bridge and > call the DRM-action from the driver, that is the reason I have removed > this and done the same as your previous inputs. > https://lore.kernel.org/all/20230203110437.otzl2azscbujigv6@houat/ You can't use devm. You can and should definitely use drmm. Maxime
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c index d4b112911a99..45a0c6671000 100644 --- a/drivers/gpu/drm/bridge/panel.c +++ b/drivers/gpu/drm/bridge/panel.c @@ -402,6 +402,13 @@ struct drm_bridge *drmm_panel_bridge_add(struct drm_device *drm, if (IS_ERR(bridge)) return bridge; + /* + * For non-component bridges, like host DSI bridge the DRM pointer + * can be available only when a panel-bridge is found. + */ + if (!drm) + drm = bridge->dev; + ret = drmm_add_action_or_reset(drm, drmm_drm_panel_bridge_release, bridge); if (ret)
drmm_panel_bridge_add DRM-managed action helper is useful for the bridge which automatically removes the bridge when drm pointer is cleaned. Supporting the same on non-component bridges like host DSI bridge requires a drm pointer which is indeed available only when a panel-bridge is found. For these use cases, the caller would call the drmm_panel_bridge_add by passing NULL to drm pointer. So, assign the bridge->dev to drm pointer for those cases. 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 v13: - new patch Note: use case on "[PATCH v13 04/18] drm: exynos: dsi: Switch to DSI panel or bridge find helper" drivers/gpu/drm/bridge/panel.c | 7 +++++++ 1 file changed, 7 insertions(+)