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);
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);