Hi Kieran, Thank you for the patch. On Thu, Mar 10, 2022 at 03:22:26PM +0000, Kieran Bingham 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> > 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 > - Fix on top of Sam Ravnborg's DRM_BRIDGE_STATE_OPS > > Changes since v2: [Kieran] > - Only support EDID on DRM_MODE_CONNECTOR_DisplayPort modes. > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > index 93b54fcba8ba..d581c820e5d8 100644 > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > @@ -1135,10 +1135,24 @@ static void ti_sn_bridge_atomic_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, > .mode_valid = ti_sn_bridge_mode_valid, > + .get_edid = ti_sn_bridge_get_edid, > .atomic_pre_enable = ti_sn_bridge_atomic_pre_enable, > .atomic_enable = ti_sn_bridge_atomic_enable, > .atomic_disable = ti_sn_bridge_atomic_disable, > @@ -1233,6 +1247,9 @@ static int ti_sn_bridge_probe(struct auxiliary_device *adev, > pdata->bridge.type = pdata->next_bridge->type == DRM_MODE_CONNECTOR_DisplayPort > ? DRM_MODE_CONNECTOR_DisplayPort : DRM_MODE_CONNECTOR_eDP; > > + if (pdata->bridge.type == DRM_MODE_CONNECTOR_DisplayPort) > + pdata->bridge.ops = DRM_BRIDGE_OP_EDID; You could write this as if (pdata->bridge.type == DRM_MODE_CONNECTOR_DisplayPort) pdata->bridge.ops |= DRM_BRIDGE_OP_EDID; to be ready to support other ops, but that doesn't help DRM_BRIDGE_OP_DETECT in 3/3, and I don't foresee the need for other ops. In any case, Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > + > drm_bridge_add(&pdata->bridge); > > ret = ti_sn_attach_host(pdata); -- Regards, Laurent Pinchart