Hi Laurent, On 16/4/20 19:22, Laurent Pinchart wrote: > Hi Enric, > > Thank you for the patch. > > On Thu, Apr 16, 2020 at 05:57:13PM +0200, Enric Balletbo i Serra wrote: >> The PS8640 DSI-to-eDP bridge can retrieve the EDID, so implement the >> .get_edid callback and set the flag to indicate the core to use it. >> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@xxxxxxxxxxxxx> >> --- >> >> Changes in v2: None >> >> drivers/gpu/drm/bridge/parade-ps8640.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c >> index d3a53442d449..956b76e0a44d 100644 >> --- a/drivers/gpu/drm/bridge/parade-ps8640.c >> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c >> @@ -242,8 +242,18 @@ static int ps8640_bridge_attach(struct drm_bridge *bridge, >> return ret; >> } >> >> +static struct edid *ps8640_bridge_get_edid(struct drm_bridge *bridge, >> + struct drm_connector *connector) >> +{ >> + struct ps8640 *ps_bridge = bridge_to_ps8640(bridge); >> + >> + return drm_get_edid(connector, >> + ps_bridge->page[PAGE0_DP_CNTL]->adapter); > > This will only work if the DDC signals are connected to the PS8640 > (quite obviously). Is that guaranteed, or could some systems connect > them directory to an SoC I2C controller ? It is possible but IMHO opinion this is hardware tricky, I mean, ps8640 outputs eDP interface and the panel is an eDP interface, so I'd expect hardware engineers do a pin to pin design, not routing the DDC signals to the SoC directly ignoring the eDP interface. > In the latter case we would > have to report this in the DT bindings of the PS8640. That's not > blocking for this patch, I am just wondering, as I would have expected > the driver to already expose EDID one way or another if this was > available and used. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > >> +} >> + >> static const struct drm_bridge_funcs ps8640_bridge_funcs = { >> .attach = ps8640_bridge_attach, >> + .get_edid = ps8640_bridge_get_edid, >> .post_disable = ps8640_post_disable, >> .pre_enable = ps8640_pre_enable, >> }; >> @@ -296,6 +306,8 @@ static int ps8640_probe(struct i2c_client *client) >> >> ps_bridge->bridge.funcs = &ps8640_bridge_funcs; >> ps_bridge->bridge.of_node = dev->of_node; >> + ps_bridge->bridge.ops = DRM_BRIDGE_OP_EDID; >> + ps_bridge->bridge.type = DRM_MODE_CONNECTOR_eDP; >> >> ps_bridge->page[PAGE0_DP_CNTL] = client; >> > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel