| Message ID | 20220307175955.363057-2-kieran.bingham+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: ti-sn65dsi86: Support non-eDP DisplayPort connectors | expand |
Hi, On Mon, Mar 7, 2022 at 10:00 AM Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> wrote: > > From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > Implement the bridge connector-related .get_edid() operation, and report > the related bridge capabilities and type. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > Reviewed-by: Stephen Boyd <swboyd@chromium.org> > Reviewed-by: Douglas Anderson <dianders@chromium.org> > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > --- > 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@ideasonboard.com/ > > 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. -Doug
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@ideasonboard.com> wrote: > > > > From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > > Implement the bridge connector-related .get_edid() operation, and report > > the related bridge capabilities and type. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > Reviewed-by: Stephen Boyd <swboyd@chromium.org> > > Reviewed-by: Douglas Anderson <dianders@chromium.org> > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > --- > > 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@ideasonboard.com/ > > > > 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
Hi, On Thu, Mar 10, 2022 at 3:43 AM Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> wrote: > > 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@ideasonboard.com> wrote: > > > > > > From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > > > > Implement the bridge connector-related .get_edid() operation, and report > > > the related bridge capabilities and type. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > Reviewed-by: Stephen Boyd <swboyd@chromium.org> > > > Reviewed-by: Douglas Anderson <dianders@chromium.org> > > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > > --- > > > 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@ideasonboard.com/ > > > > > > 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? Yes. -Doug
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; drm_bridge_add(&pdata->bridge);