Hi Rob.
Patch looks good, one small improvement proposal.
On Sat, Dec 07, 2019 at 12:35:51PM -0800, Rob Clark wrote:
From: Rob Clark robdclark@chromium.org
To handle the case where there are multiple panel endpoints, only one of which is enabled/installed, add support for a wildcard endpoint value to request finding whichever endpoint is enabled.
Signed-off-by: Rob Clark robdclark@chromium.org
drivers/gpu/drm/drm_of.c | 41 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c index 0ca58803ba46..2baf44e401b8 100644 --- a/drivers/gpu/drm/drm_of.c +++ b/drivers/gpu/drm/drm_of.c @@ -219,11 +219,44 @@ int drm_of_encoder_active_endpoint(struct device_node *node, } EXPORT_SYMBOL_GPL(drm_of_encoder_active_endpoint); +static int find_enabled_endpoint(const struct device_node *node, u32 port) +{
- struct device_node *endpoint_node, *remote;
- u32 endpoint = 0;
- for (endpoint = 0; ; endpoint++) {
endpoint_node = of_graph_get_endpoint_by_regs(node, port, endpoint);
if (!endpoint_node) {
pr_debug("No more endpoints!\n");
return -ENODEV;
}
remote = of_graph_get_remote_port_parent(endpoint_node);
of_node_put(endpoint_node);
if (!remote) {
pr_debug("no valid remote node\n");
continue;
}
if (!of_device_is_available(remote)) {
pr_debug("not available for remote node\n");
of_node_put(remote);
continue;
}
pr_debug("found enabled endpoint %d for %s\n", endpoint, remote->name);
of_node_put(remote);
return endpoint;
- }
- return -ENODEV;
+}
This function seems pretty generic. Should this be part of drivers/of/property.c - as others may have the same need?
/**
- drm_of_find_panel_or_bridge - return connected panel or bridge device
- @np: device tree node containing encoder output ports
- @port: port in the device tree node
- @endpoint: endpoint in the device tree node
- @endpoint: endpoint in the device tree node, or -1 to find an enabled endpoint
- @panel: pointer to hold returned drm_panel
- @bridge: pointer to hold returned drm_bridge
Introducing a define would make it easier to use this function in the right way. For example: #define OF_ENDPOINT_FIRST -1
Then we would see this code in the user side: + ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, OF_ENDPOINT_FIRST, &pdata->panel, NULL);
Or something like this.
Sam
@@ -246,6 +279,12 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, if (panel) *panel = NULL;
- if (endpoint == -1) {
endpoint = find_enabled_endpoint(np, port);
if (endpoint < 0)
return endpoint;
- }
- remote = of_graph_get_remote_node(np, port, endpoint); if (!remote) return -ENODEV;
-- 2.23.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel