Hi Sam, On Tue, Aug 20, 2019 at 12:37:06PM +0200, Sam Ravnborg wrote: > On Tue, Aug 20, 2019 at 04:16:44AM +0300, Laurent Pinchart wrote: > > Implement the newly added bridge connector operations, allowing the > > usage of drm_bridge_panel with drm_bridge_connector. > > > > Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > > --- > > drivers/gpu/drm/bridge/panel.c | 18 +++++++++++++++++- > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > > index f5b8e55301ac..1c7f5b648f05 100644 > > --- a/drivers/gpu/drm/bridge/panel.c > > +++ b/drivers/gpu/drm/bridge/panel.c > > @@ -60,7 +60,7 @@ static int panel_bridge_attach(struct drm_bridge *bridge, > > int ret; > > > > if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) > > - return -EINVAL; > > + return 0; > > > > if (!bridge->encoder) { > > DRM_ERROR("Missing encoder\n"); > > @@ -123,6 +123,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. > > + */ > > Noted, I have patches to fix this. Needs a little testing/polishing > before I post them.
Take your time. Thank you for addressing this issue :-) > > + 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, > > @@ -130,6 +142,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, > > }; > > > > /** > > @@ -175,6 +188,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; > > Confused. > We move the connector to the display controller. > So the panel does not know the type. > > In others words - please put a few more words on this FIXME. I mean the panel should report if it's a DPI, LVDS, or other type of panel, so that the display controller will know what to initialise the connector type to. I think the drm_panel structure should get a type field, similar to the bridge type field. Does that make sense to you ? > > > > drm_bridge_add(&panel_bridge->bridge); > > -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel