From: Laurent Pinchart <[email protected]>
Implement the bridge connector-related .get_edid() operation, and report
the related bridge capabilities and type.
Signed-off-by: Laurent Pinchart <[email protected]>
Signed-off-by: Kieran Bingham <[email protected]>
---
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;
+
drm_bridge_add(&pdata->bridge);
ret = ti_sn_attach_host(pdata);
--
2.32.0
Hi,
On Thu, Mar 10, 2022 at 7:22 AM Kieran Bingham
<[email protected]> wrote:
>
> From: Laurent Pinchart <[email protected]>
>
> Implement the bridge connector-related .get_edid() operation, and report
> the related bridge capabilities and type.
>
> Signed-off-by: Laurent Pinchart <[email protected]>
> Signed-off-by: Kieran Bingham <[email protected]>
> ---
> 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);
I'm 99% sure that the pm_runtime calls here are not needed and can be
removed.. The AUX transfer function handles the pm_runtime_get_sync()
and it also does the "put" with autosuspend so that the whole EDID can
be read without constantly powering the bridge up and down again.
Hi Doug,
Quoting Doug Anderson (2022-03-10 23:10:20)
> Hi,
>
> On Thu, Mar 10, 2022 at 7:22 AM Kieran Bingham
> <[email protected]> wrote:
> >
> > From: Laurent Pinchart <[email protected]>
> >
> > Implement the bridge connector-related .get_edid() operation, and report
> > the related bridge capabilities and type.
> >
> > Signed-off-by: Laurent Pinchart <[email protected]>
> > Signed-off-by: Kieran Bingham <[email protected]>
> > ---
> > 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);
>
> I'm 99% sure that the pm_runtime calls here are not needed and can be
> removed.. The AUX transfer function handles the pm_runtime_get_sync()
> and it also does the "put" with autosuspend so that the whole EDID can
> be read without constantly powering the bridge up and down again.
Yes, digging through I agree - It does look like this may be the case.
ti_sn_aux_transfer() certainly looks like it handles the pm_runtime_
calls, and drm_get_edid() looks like it goes through there from the core
using the standard i2c interface, with nothing else expected to touch
the hw between.
So that more or less simplifies this function to just
return drm_get_edid(connector, &pdata->aux.ddc);
Thanks
--
Kieran