Hi Doug, Quoting Doug Anderson (2022-03-07 19:52:08) > Hi, > > On Mon, Mar 7, 2022 at 10:00 AM Kieran Bingham > <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> wrote: > > > > From: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > > > > Implement the bridge connector-related .get_edid() operation, and report > > the related bridge capabilities and type. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > > Reviewed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx> > > Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> > > --- > > Changes since v1: > > > > - The connector .get_modes() operation doesn't rely on EDID anymore, > > __ti_sn_bridge_get_edid() and ti_sn_bridge_get_edid() got merged > > together > > > > Notes from Kieran: > > > > RB Tags collected from: > > https://lore.kernel.org/all/20210322030128.2283-9-laurent.pinchart+renesas@xxxxxxxxxxxxxxxx/ > > > > However this was over a year ago, so let me know if other patches now > > superceed this one or otherwise invalidate this update. > > > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > index c55848588123..ffb6c04f6c46 100644 > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > @@ -1154,6 +1154,19 @@ static void ti_sn_bridge_post_disable(struct drm_bridge *bridge) > > pm_runtime_put_sync(pdata->dev); > > } > > > > +static struct edid *ti_sn_bridge_get_edid(struct drm_bridge *bridge, > > + struct drm_connector *connector) > > +{ > > + struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge); > > + struct edid *edid; > > + > > + pm_runtime_get_sync(pdata->dev); > > + edid = drm_get_edid(connector, &pdata->aux.ddc); > > + pm_runtime_put_autosuspend(pdata->dev); > > + > > + return edid; > > +} > > + > > static const struct drm_bridge_funcs ti_sn_bridge_funcs = { > > .attach = ti_sn_bridge_attach, > > .detach = ti_sn_bridge_detach, > > @@ -1162,6 +1175,7 @@ static const struct drm_bridge_funcs ti_sn_bridge_funcs = { > > .enable = ti_sn_bridge_enable, > > .disable = ti_sn_bridge_disable, > > .post_disable = ti_sn_bridge_post_disable, > > + .get_edid = ti_sn_bridge_get_edid, > > }; > > > > static void ti_sn_bridge_parse_lanes(struct ti_sn65dsi86 *pdata, > > @@ -1248,6 +1262,8 @@ static int ti_sn_bridge_probe(struct auxiliary_device *adev, > > > > pdata->bridge.funcs = &ti_sn_bridge_funcs; > > pdata->bridge.of_node = np; > > + pdata->bridge.ops = DRM_BRIDGE_OP_EDID; > > + pdata->bridge.type = DRM_MODE_CONNECTOR_eDP; > > This doesn't look right to me. In the eDP case the EDID reading is > driven by the panel. Now that I have the optional connector working based on Sam's series I think this is the last issue to solve before reposting the DP/HPD support. Are you saying that the bridge.ops should only set DRM_BRIDGE_OP_EDID when pdata->bridge.type == DRM_MODE_CONNECTOR_DisplayPort? -- Regards Kieran > > -Doug
