| Message ID | 20190707181937.6250-9-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/omap: Replace custom display drivers with drm_bridge and drm_panel | expand |
Hi Laurent et all. > +static int panel_bridge_get_modes(struct drm_bridge *bridge, > + struct drm_connector *connector) > +{ > + struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge); > + > + /* > + * FIXME: drm_panel_get_modes() should take the connector as an > + * argument. > + */ > + return drm_panel_get_modes(panel_bridge->panel); > +} I took a look at this - it seems simple: - Update drm_panel.get_modes() to take controller as argument, and fix all callers. All callers already have connector available. - Drop drm_panel_attach(), drm_panel_detach() and update all callers. In reality just drop all code around attach(), detach(). drm_panel_attach(), drm_panel_detach() will be noops when the connector stored in drm_panel is no longer used. The semantic difference is that we supply the connector when we call drm_panel_get_modes() and not at panel creation time with an drm_panel_attach(). So it should be doable without any migration from one world to the other. If someone can say "yes it should be that simple", then I will give it a spin. Sam
Hi Sam, On Tue, Jul 16, 2019 at 01:08:27PM +0200, Sam Ravnborg wrote: > Hi Laurent et all. > > > +static int panel_bridge_get_modes(struct drm_bridge *bridge, > > + struct drm_connector *connector) > > +{ > > + struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge); > > + > > + /* > > + * FIXME: drm_panel_get_modes() should take the connector as an > > + * argument. > > + */ > > + return drm_panel_get_modes(panel_bridge->panel); > > +} > > I took a look at this - it seems simple: > - Update drm_panel.get_modes() to take controller as argument, and fix I assume you meant connector, not controller. > all callers. All callers already have connector available. > - Drop drm_panel_attach(), drm_panel_detach() and update all callers. > In reality just drop all code around attach(), detach(). > drm_panel_attach(), drm_panel_detach() will be noops when the > connector stored in drm_panel is no longer used. > > The semantic difference is that we supply the connector when we call > drm_panel_get_modes() and not at panel creation time with an drm_panel_attach(). > > So it should be doable without any migration from one world to the other. > > If someone can say "yes it should be that simple", then I will > give it a spin. Looking forward to that :-)
Hi Laurent. As I said in another mail, you have managed to keep me busy... > > I took a look at this - it seems simple: > > - Update drm_panel.get_modes() to take drm_connector as argument, and fix > > all callers. All callers already have connector available. > > - Drop drm_panel_attach(), drm_panel_detach() and update all callers. > > In reality just drop all code around attach(), detach(). > > drm_panel_attach(), drm_panel_detach() will be noops when the > > connector stored in drm_panel is no longer used. > > > > The semantic difference is that we supply the connector when we call > > drm_panel_get_modes() and not at panel creation time with an drm_panel_attach(). > > > > So it should be doable without any migration from one world to the other. > > > > If someone can say "yes it should be that simple", then I will > > give it a spin. > > Looking forward to that :-) Almost there.... I have all the preparation patches on dri-devel, with positive feedback on most. And locally I have updated all get_modes() to take drm_connector as argument. A few drivers access drm_panel->connector, still need to look into this. And then for drm_panel_attach(), drm_panel_detach() - so far they are kept but changed to take a drm_device*. Just sharing this so you do not jump at it and duplicate the work. It will take a little time before I can invest time in this again. Will post patches when something is ready for review. Sam
Hi Sam, On Thu, Aug 08, 2019 at 06:52:53PM +0200, Sam Ravnborg wrote: > Hi Laurent. > > As I said in another mail, you have managed to keep me busy... > > > > I took a look at this - it seems simple: > > > - Update drm_panel.get_modes() to take drm_connector as argument, and fix > > > all callers. All callers already have connector available. > > > - Drop drm_panel_attach(), drm_panel_detach() and update all callers. > > > In reality just drop all code around attach(), detach(). > > > drm_panel_attach(), drm_panel_detach() will be noops when the > > > connector stored in drm_panel is no longer used. > > > > > > The semantic difference is that we supply the connector when we call > > > drm_panel_get_modes() and not at panel creation time with an drm_panel_attach(). > > > > > > So it should be doable without any migration from one world to the other. > > > > > > If someone can say "yes it should be that simple", then I will > > > give it a spin. > > > > Looking forward to that :-) > > Almost there.... > I have all the preparation patches on dri-devel, with positive > feedback on most. > > And locally I have updated all get_modes() to take drm_connector as > argument. > > A few drivers access drm_panel->connector, still need to look into this. > > And then for drm_panel_attach(), drm_panel_detach() - so far they are > kept but changed to take a drm_device*. > > Just sharing this so you do not jump at it and duplicate the work. > It will take a little time before I can invest time in this again. > Will post patches when something is ready for review. Thanks for the update. Take your time, this isn't blocking me.
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c index 98ad4abf2409..628e37babb09 100644 --- a/drivers/gpu/drm/bridge/panel.c +++ b/drivers/gpu/drm/bridge/panel.c @@ -59,7 +59,7 @@ static int panel_bridge_attach(struct drm_bridge *bridge, bool create_connector) int ret; if (!create_connector) - return -EINVAL; + return 0; if (!bridge->encoder) { DRM_ERROR("Missing encoder\n"); @@ -122,6 +122,18 @@ static void panel_bridge_post_disable(struct drm_bridge *bridge) drm_panel_unprepare(panel_bridge->panel); } +static int panel_bridge_get_modes(struct drm_bridge *bridge, + struct drm_connector *connector) +{ + struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge); + + /* + * FIXME: drm_panel_get_modes() should take the connector as an + * argument. + */ + return drm_panel_get_modes(panel_bridge->panel); +} + static const struct drm_bridge_funcs panel_bridge_bridge_funcs = { .attach = panel_bridge_attach, .detach = panel_bridge_detach, @@ -129,6 +141,7 @@ static const struct drm_bridge_funcs panel_bridge_bridge_funcs = { .enable = panel_bridge_enable, .disable = panel_bridge_disable, .post_disable = panel_bridge_post_disable, + .get_modes = panel_bridge_get_modes, }; /** @@ -174,6 +187,9 @@ struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel, #ifdef CONFIG_OF panel_bridge->bridge.of_node = panel->dev->of_node; #endif + panel_bridge->bridge.ops = DRM_BRIDGE_OP_MODES; + /* FIXME: The panel should report its type. */ + panel_bridge->bridge.type = DRM_MODE_CONNECTOR_DPI; drm_bridge_add(&panel_bridge->bridge);
Implement the newly added bridge connector operations, allowing the usage of drm_bridge_panel with drm_bridge_connector. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/gpu/drm/bridge/panel.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)