| Message ID | 20200703192417.372164-20-sam@ravnborg.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: support chained bridges + panel updates | expand |
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:15PM +0200, Sam Ravnborg wrote: > Add the get_modes() bridge operation to prepare for > use as a chained bridge. > Add helper function that is also used by the connector. > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Andrzej Hajda <a.hajda@samsung.com> > Cc: Neil Armstrong <narmstrong@baylibre.com> > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> > Cc: Jonas Karlman <jonas@kwiboo.se> > Cc: Jernej Skrabec <jernej.skrabec@siol.net> > --- > drivers/gpu/drm/bridge/nxp-ptn3460.c | 52 ++++++++++++++++++---------- > 1 file changed, 33 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c b/drivers/gpu/drm/bridge/nxp-ptn3460.c > index 0bd9f0e451b3..e253c185f94c 100644 > --- a/drivers/gpu/drm/bridge/nxp-ptn3460.c > +++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c > @@ -154,17 +154,13 @@ static void ptn3460_disable(struct drm_bridge *bridge) > gpiod_set_value(ptn_bridge->gpio_pd_n, 0); > } > > -static int ptn3460_get_modes(struct drm_connector *connector) > +static struct edid *ptn3460_get_edid(struct drm_bridge *bridge, > + struct drm_connector *connector) > { > - struct ptn3460_bridge *ptn_bridge; > - u8 *edid; > - int ret, num_modes = 0; > + struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge); > bool power_off; > - > - ptn_bridge = connector_to_ptn3460(connector); > - > - if (ptn_bridge->edid) > - return drm_add_edid_modes(connector, ptn_bridge->edid); > + u8 *edid; > + int ret; > > power_off = !ptn_bridge->enabled; > ptn3460_pre_enable(&ptn_bridge->bridge); > @@ -172,30 +168,46 @@ static int ptn3460_get_modes(struct drm_connector *connector) > edid = kmalloc(EDID_LENGTH, GFP_KERNEL); > if (!edid) { > DRM_ERROR("Failed to allocate EDID\n"); > - return 0; > + return NULL; Don't you need to power off in the error path ? > } > > ret = ptn3460_read_bytes(ptn_bridge, PTN3460_EDID_ADDR, edid, > - EDID_LENGTH); > + EDID_LENGTH); > if (ret) { > kfree(edid); > - goto out; > + return NULL; And here too ? > } > > + if (power_off) > + ptn3460_disable(&ptn_bridge->bridge); > + > + kfree(ptn_bridge->edid); > ptn_bridge->edid = (struct edid *)edid; > - drm_connector_update_edid_property(connector, ptn_bridge->edid); > > - num_modes = drm_add_edid_modes(connector, ptn_bridge->edid); > + return ptn_bridge->edid; Same comment as earlier in this series about storing the edid. > +} > > -out: > - if (power_off) > - ptn3460_disable(&ptn_bridge->bridge); > +static int ptn3460_connector_get_modes(struct drm_connector *connector) > +{ > + struct ptn3460_bridge *ptn_bridge; > + struct edid *edid; > + > + ptn_bridge = connector_to_ptn3460(connector); Maybe struct ptn3460_bridge *ptn_bridge = connector_to_ptn3460(connector); ? > + > + if (ptn_bridge->edid) > + return drm_add_edid_modes(connector, ptn_bridge->edid); > + > + edid = ptn3460_get_edid(&ptn_bridge->bridge, connector); > + if (!edid) > + return 0; > + > + drm_connector_update_edid_property(connector, edid); > > - return num_modes; > + return drm_add_edid_modes(connector, edid); > } > > static const struct drm_connector_helper_funcs ptn3460_connector_helper_funcs = { > - .get_modes = ptn3460_get_modes, > + .get_modes = ptn3460_connector_get_modes, > }; > > static const struct drm_connector_funcs ptn3460_connector_funcs = { > @@ -249,6 +261,7 @@ static const struct drm_bridge_funcs ptn3460_bridge_funcs = { > .pre_enable = ptn3460_pre_enable, > .disable = ptn3460_disable, > .attach = ptn3460_bridge_attach, > + .get_edid = ptn3460_get_edid, > }; > > static int ptn3460_probe(struct i2c_client *client, > @@ -304,6 +317,7 @@ static int ptn3460_probe(struct i2c_client *client, > } > > ptn_bridge->bridge.funcs = &ptn3460_bridge_funcs; > + ptn_bridge->bridge.ops = DRM_BRIDGE_OP_EDID; > ptn_bridge->bridge.of_node = dev->of_node; > drm_bridge_add(&ptn_bridge->bridge); >
diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c b/drivers/gpu/drm/bridge/nxp-ptn3460.c index 0bd9f0e451b3..e253c185f94c 100644 --- a/drivers/gpu/drm/bridge/nxp-ptn3460.c +++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c @@ -154,17 +154,13 @@ static void ptn3460_disable(struct drm_bridge *bridge) gpiod_set_value(ptn_bridge->gpio_pd_n, 0); } -static int ptn3460_get_modes(struct drm_connector *connector) +static struct edid *ptn3460_get_edid(struct drm_bridge *bridge, + struct drm_connector *connector) { - struct ptn3460_bridge *ptn_bridge; - u8 *edid; - int ret, num_modes = 0; + struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge); bool power_off; - - ptn_bridge = connector_to_ptn3460(connector); - - if (ptn_bridge->edid) - return drm_add_edid_modes(connector, ptn_bridge->edid); + u8 *edid; + int ret; power_off = !ptn_bridge->enabled; ptn3460_pre_enable(&ptn_bridge->bridge); @@ -172,30 +168,46 @@ static int ptn3460_get_modes(struct drm_connector *connector) edid = kmalloc(EDID_LENGTH, GFP_KERNEL); if (!edid) { DRM_ERROR("Failed to allocate EDID\n"); - return 0; + return NULL; } ret = ptn3460_read_bytes(ptn_bridge, PTN3460_EDID_ADDR, edid, - EDID_LENGTH); + EDID_LENGTH); if (ret) { kfree(edid); - goto out; + return NULL; } + if (power_off) + ptn3460_disable(&ptn_bridge->bridge); + + kfree(ptn_bridge->edid); ptn_bridge->edid = (struct edid *)edid; - drm_connector_update_edid_property(connector, ptn_bridge->edid); - num_modes = drm_add_edid_modes(connector, ptn_bridge->edid); + return ptn_bridge->edid; +} -out: - if (power_off) - ptn3460_disable(&ptn_bridge->bridge); +static int ptn3460_connector_get_modes(struct drm_connector *connector) +{ + struct ptn3460_bridge *ptn_bridge; + struct edid *edid; + + ptn_bridge = connector_to_ptn3460(connector); + + if (ptn_bridge->edid) + return drm_add_edid_modes(connector, ptn_bridge->edid); + + edid = ptn3460_get_edid(&ptn_bridge->bridge, connector); + if (!edid) + return 0; + + drm_connector_update_edid_property(connector, edid); - return num_modes; + return drm_add_edid_modes(connector, edid); } static const struct drm_connector_helper_funcs ptn3460_connector_helper_funcs = { - .get_modes = ptn3460_get_modes, + .get_modes = ptn3460_connector_get_modes, }; static const struct drm_connector_funcs ptn3460_connector_funcs = { @@ -249,6 +261,7 @@ static const struct drm_bridge_funcs ptn3460_bridge_funcs = { .pre_enable = ptn3460_pre_enable, .disable = ptn3460_disable, .attach = ptn3460_bridge_attach, + .get_edid = ptn3460_get_edid, }; static int ptn3460_probe(struct i2c_client *client, @@ -304,6 +317,7 @@ static int ptn3460_probe(struct i2c_client *client, } ptn_bridge->bridge.funcs = &ptn3460_bridge_funcs; + ptn_bridge->bridge.ops = DRM_BRIDGE_OP_EDID; ptn_bridge->bridge.of_node = dev->of_node; drm_bridge_add(&ptn_bridge->bridge);
Add the get_modes() bridge operation to prepare for use as a chained bridge. Add helper function that is also used by the connector. Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Jernej Skrabec <jernej.skrabec@siol.net> --- drivers/gpu/drm/bridge/nxp-ptn3460.c | 52 ++++++++++++++++++---------- 1 file changed, 33 insertions(+), 19 deletions(-)