| Message ID | 20210322140152.101709-3-jagan@amarulasolutions.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: sun4i: dsi: Convert drm bridge | expand |
Hi Jagan, Thank you for the patch. On Mon, Mar 22, 2021 at 07:31:50PM +0530, Jagan Teki wrote: > Some display panels would come up with a non-DSI output which Did you mean input instead of output ? > can have an option to connect DSI interface by means of bridge > converter. > > This DSI to non-DSI bridge converter would require a bridge > driver that would communicate the DSI controller for bridge > functionalities. > > So, add support for bridge functionalities in Allwinner DSI > controller. > > Cc: Samuel Holland <samuel@sholland.org> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > Note: > Samuel Holland, The existing kms hotplug dropped in order to > attach the bridge properly. > > However, I did try several ways to support hotplug with the > bridge but it's resulting in a deadlock where bind never attach > bridge until bridge pointer found and bridge pointer cannot > found until bind finishes. Any inputs on this would be appreciated. > > Changes for v4: > - none > Changes for v3: > - updated with new API's > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 34 +++++++++++++++++--------- > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 +- > 2 files changed, 23 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > index 2e9e7b2d4145..39321299dc27 100644 > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > @@ -773,6 +773,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder) > if (dsi->panel) > drm_panel_prepare(dsi->panel); > > + if (dsi->panel_bridge) > + dsi->panel_bridge->funcs->pre_enable(dsi->panel_bridge); > + > /* > * FIXME: This should be moved after the switch to HS mode. > * > @@ -788,6 +791,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder) > if (dsi->panel) > drm_panel_enable(dsi->panel); > > + if (dsi->panel_bridge) > + dsi->panel_bridge->funcs->enable(dsi->panel_bridge); > + > sun6i_dsi_start(dsi, DSI_START_HSC); > > udelay(1000); > @@ -804,6 +810,9 @@ static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder) > if (dsi->panel) { > drm_panel_disable(dsi->panel); > drm_panel_unprepare(dsi->panel); > + } else if (dsi->panel_bridge) { > + dsi->panel_bridge->funcs->disable(dsi->panel_bridge); > + dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge); > } Instead of having code paths that depend on whether you have a panel or a bridge, it would be better to wrap the panel a bridge (using drivers/gpu/drm/bridge/panel.c). The dsi->panel_bridge pointer should be renamed to next_bridge, and all the code (except in probe) can the use next_bridge without caring if it's a direct connection to a panel or another bridge. Furthermore, the encoder should call bridge functions explicitly, this should be handled by the DRM core. > > phy_power_off(dsi->dphy); > @@ -964,23 +973,17 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host, > struct mipi_dsi_device *device) > { > struct sun6i_dsi *dsi = host_to_sun6i_dsi(host); > - struct drm_panel *panel; > int ret; > > ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0, > - &panel, NULL); > + &dsi->panel, &dsi->panel_bridge); > if (ret) > return ret; > > - if (!dsi->drm || !dsi->drm->registered) > - return -EPROBE_DEFER; > - > - dsi->panel = panel; > dsi->device = device; > > - drm_kms_helper_hotplug_event(dsi->drm); > - > - dev_info(host->dev, "Attached device %s\n", device->name); > + dev_info(host->dev, "Attached %s %s\n", > + device->name, dsi->panel ? "panel" : "bridge"); > > return 0; > } > @@ -991,9 +994,10 @@ static int sun6i_dsi_detach(struct mipi_dsi_host *host, > struct sun6i_dsi *dsi = host_to_sun6i_dsi(host); > > dsi->panel = NULL; > + dsi->panel_bridge = NULL; > dsi->device = NULL; > > - drm_kms_helper_hotplug_event(dsi->drm); > + drm_of_panel_bridge_remove(dsi->dev->of_node, 0, 0); > > return 0; > } > @@ -1082,7 +1086,13 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master, > > drm_connector_attach_encoder(&dsi->connector, &dsi->encoder); > > - dsi->drm = drm; > + if (dsi->panel_bridge) { > + ret = drm_bridge_attach(&dsi->encoder, dsi->panel_bridge, NULL, 0); > + if (ret) { > + dev_err(dsi->dev, "Couldn't attach drm bridge\n"); > + goto err_cleanup_connector; > + } > + } > > return 0; > > @@ -1096,7 +1106,7 @@ static void sun6i_dsi_unbind(struct device *dev, struct device *master, > { > struct sun6i_dsi *dsi = dev_get_drvdata(dev); > > - dsi->drm = NULL; > + drm_encoder_cleanup(&dsi->encoder); > } > > static const struct component_ops sun6i_dsi_ops = { > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h > index c863900ae3b4..370ecb356a63 100644 > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h > @@ -29,8 +29,8 @@ struct sun6i_dsi { > > struct device *dev; > struct mipi_dsi_device *device; > - struct drm_device *drm; > struct drm_panel *panel; > + struct drm_bridge *panel_bridge; > }; > > static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host)
On 3/22/21 9:01 AM, Jagan Teki wrote: > Some display panels would come up with a non-DSI output which > can have an option to connect DSI interface by means of bridge > converter. > > This DSI to non-DSI bridge converter would require a bridge > driver that would communicate the DSI controller for bridge > functionalities. > > So, add support for bridge functionalities in Allwinner DSI > controller. > > Cc: Samuel Holland <samuel@sholland.org> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > Note: > Samuel Holland, The existing kms hotplug dropped in order to > attach the bridge properly. > > However, I did try several ways to support hotplug with the > bridge but it's resulting in a deadlock where bind never attach > bridge until bridge pointer found and bridge pointer cannot > found until bind finishes. Any inputs on this would be appreciated. The intended behavior is that sun6i_dsi_bind() is independent of any DSI device. And sun6i_dsi_attach() must only be called after bind completes and the DRM device is registered. This design allows the rest of the display engine (such as the HDMI output) to work even if no panel is listed in the device tree, or if a panel driver is missing. > Changes for v4: > - none > Changes for v3: > - updated with new API's > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 34 +++++++++++++++++--------- > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 +- > 2 files changed, 23 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > index 2e9e7b2d4145..39321299dc27 100644 > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c > @@ -773,6 +773,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder) > if (dsi->panel) > drm_panel_prepare(dsi->panel); > > + if (dsi->panel_bridge) > + dsi->panel_bridge->funcs->pre_enable(dsi->panel_bridge); > + > /* > * FIXME: This should be moved after the switch to HS mode. > * > @@ -788,6 +791,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder) > if (dsi->panel) > drm_panel_enable(dsi->panel); > > + if (dsi->panel_bridge) > + dsi->panel_bridge->funcs->enable(dsi->panel_bridge); > + > sun6i_dsi_start(dsi, DSI_START_HSC); > > udelay(1000); > @@ -804,6 +810,9 @@ static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder) > if (dsi->panel) { > drm_panel_disable(dsi->panel); > drm_panel_unprepare(dsi->panel); > + } else if (dsi->panel_bridge) { > + dsi->panel_bridge->funcs->disable(dsi->panel_bridge); > + dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge); > } > > phy_power_off(dsi->dphy); > @@ -964,23 +973,17 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host, > struct mipi_dsi_device *device) > { > struct sun6i_dsi *dsi = host_to_sun6i_dsi(host); > - struct drm_panel *panel; > int ret; > > ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0, > - &panel, NULL); > + &dsi->panel, &dsi->panel_bridge); > if (ret) > return ret; > > - if (!dsi->drm || !dsi->drm->registered) > - return -EPROBE_DEFER; > - > - dsi->panel = panel; > dsi->device = device; > > - drm_kms_helper_hotplug_event(dsi->drm); > - > - dev_info(host->dev, "Attached device %s\n", device->name); > + dev_info(host->dev, "Attached %s %s\n", > + device->name, dsi->panel ? "panel" : "bridge"); > > return 0; > } > @@ -991,9 +994,10 @@ static int sun6i_dsi_detach(struct mipi_dsi_host *host, > struct sun6i_dsi *dsi = host_to_sun6i_dsi(host); > > dsi->panel = NULL; > + dsi->panel_bridge = NULL; > dsi->device = NULL; > > - drm_kms_helper_hotplug_event(dsi->drm); > + drm_of_panel_bridge_remove(dsi->dev->of_node, 0, 0); > > return 0; > } > @@ -1082,7 +1086,13 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master, > > drm_connector_attach_encoder(&dsi->connector, &dsi->encoder); > > - dsi->drm = drm; > + if (dsi->panel_bridge) { > + ret = drm_bridge_attach(&dsi->encoder, dsi->panel_bridge, NULL, 0); > + if (ret) { > + dev_err(dsi->dev, "Couldn't attach drm bridge\n"); > + goto err_cleanup_connector; > + } > + } You initialize dsi->panel_bridge in sun6i_dsi_attach() above, but that function may run long after sun6i_dsi_bind() has completed -- for example if this driver is built in, but the panel/bridge driver is built as a module. So you cannot reference dsi->panel_bridge from this function. Regards, Samuel > > return 0; > > @@ -1096,7 +1106,7 @@ static void sun6i_dsi_unbind(struct device *dev, struct device *master, > { > struct sun6i_dsi *dsi = dev_get_drvdata(dev); > > - dsi->drm = NULL; > + drm_encoder_cleanup(&dsi->encoder); > } > > static const struct component_ops sun6i_dsi_ops = { > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h > index c863900ae3b4..370ecb356a63 100644 > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h > @@ -29,8 +29,8 @@ struct sun6i_dsi { > > struct device *dev; > struct mipi_dsi_device *device; > - struct drm_device *drm; > struct drm_panel *panel; > + struct drm_bridge *panel_bridge; > }; > > static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host) >
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index 2e9e7b2d4145..39321299dc27 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c @@ -773,6 +773,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder) if (dsi->panel) drm_panel_prepare(dsi->panel); + if (dsi->panel_bridge) + dsi->panel_bridge->funcs->pre_enable(dsi->panel_bridge); + /* * FIXME: This should be moved after the switch to HS mode. * @@ -788,6 +791,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder) if (dsi->panel) drm_panel_enable(dsi->panel); + if (dsi->panel_bridge) + dsi->panel_bridge->funcs->enable(dsi->panel_bridge); + sun6i_dsi_start(dsi, DSI_START_HSC); udelay(1000); @@ -804,6 +810,9 @@ static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder) if (dsi->panel) { drm_panel_disable(dsi->panel); drm_panel_unprepare(dsi->panel); + } else if (dsi->panel_bridge) { + dsi->panel_bridge->funcs->disable(dsi->panel_bridge); + dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge); } phy_power_off(dsi->dphy); @@ -964,23 +973,17 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host, struct mipi_dsi_device *device) { struct sun6i_dsi *dsi = host_to_sun6i_dsi(host); - struct drm_panel *panel; int ret; ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0, - &panel, NULL); + &dsi->panel, &dsi->panel_bridge); if (ret) return ret; - if (!dsi->drm || !dsi->drm->registered) - return -EPROBE_DEFER; - - dsi->panel = panel; dsi->device = device; - drm_kms_helper_hotplug_event(dsi->drm); - - dev_info(host->dev, "Attached device %s\n", device->name); + dev_info(host->dev, "Attached %s %s\n", + device->name, dsi->panel ? "panel" : "bridge"); return 0; } @@ -991,9 +994,10 @@ static int sun6i_dsi_detach(struct mipi_dsi_host *host, struct sun6i_dsi *dsi = host_to_sun6i_dsi(host); dsi->panel = NULL; + dsi->panel_bridge = NULL; dsi->device = NULL; - drm_kms_helper_hotplug_event(dsi->drm); + drm_of_panel_bridge_remove(dsi->dev->of_node, 0, 0); return 0; } @@ -1082,7 +1086,13 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master, drm_connector_attach_encoder(&dsi->connector, &dsi->encoder); - dsi->drm = drm; + if (dsi->panel_bridge) { + ret = drm_bridge_attach(&dsi->encoder, dsi->panel_bridge, NULL, 0); + if (ret) { + dev_err(dsi->dev, "Couldn't attach drm bridge\n"); + goto err_cleanup_connector; + } + } return 0; @@ -1096,7 +1106,7 @@ static void sun6i_dsi_unbind(struct device *dev, struct device *master, { struct sun6i_dsi *dsi = dev_get_drvdata(dev); - dsi->drm = NULL; + drm_encoder_cleanup(&dsi->encoder); } static const struct component_ops sun6i_dsi_ops = { diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h index c863900ae3b4..370ecb356a63 100644 --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h @@ -29,8 +29,8 @@ struct sun6i_dsi { struct device *dev; struct mipi_dsi_device *device; - struct drm_device *drm; struct drm_panel *panel; + struct drm_bridge *panel_bridge; }; static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host)
Some display panels would come up with a non-DSI output which can have an option to connect DSI interface by means of bridge converter. This DSI to non-DSI bridge converter would require a bridge driver that would communicate the DSI controller for bridge functionalities. So, add support for bridge functionalities in Allwinner DSI controller. Cc: Samuel Holland <samuel@sholland.org> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- Note: Samuel Holland, The existing kms hotplug dropped in order to attach the bridge properly. However, I did try several ways to support hotplug with the bridge but it's resulting in a deadlock where bind never attach bridge until bridge pointer found and bridge pointer cannot found until bind finishes. Any inputs on this would be appreciated. Changes for v4: - none Changes for v3: - updated with new API's drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 34 +++++++++++++++++--------- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 +- 2 files changed, 23 insertions(+), 13 deletions(-)