Re: [PATCH v2 06/10] drm/bridge: tc358775: Get bridge data lanes instead of the DSI host lanes

[Michael Walle]

> The current code assumes the data-lanes property is configured on the
> DSI host side instead of the bridge side, and assumes DSI host endpoint 1.
> 
> Let's standardize on what the other bridge drivers are doing and parse the
> data-lanes property for the bridge. Only if data-lanes property is not found,
> let's be nice and also check the DSI host for old dtb in use and warn.
> 
> Signed-off-by: Tony Lindgren 
> ---
>  drivers/gpu/drm/bridge/tc358775.c | 25 +++--
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358775.c 
> b/drivers/gpu/drm/bridge/tc358775.c
> --- a/drivers/gpu/drm/bridge/tc358775.c
> +++ b/drivers/gpu/drm/bridge/tc358775.c
> @@ -525,27 +525,24 @@ tc_mode_valid(struct drm_bridge *bridge,
>  static int tc358775_parse_dt(struct device_node *np, struct tc_data *tc)
>  {
>   struct device_node *endpoint;
> - struct device_node *parent;
>   struct device_node *remote;
>   int dsi_lanes = -1;
>  
> - /*
> -  * To get the data-lanes of dsi, we need to access the dsi0_out of port1
> -  *  of dsi0 endpoint from bridge port0 of d2l_in
> -  */
>   endpoint = of_graph_get_endpoint_by_regs(tc->dev->of_node,
>TC358775_DSI_IN, -1);
> - if (endpoint) {
> - /* dsi0_out node */
> - parent = of_graph_get_remote_port_parent(endpoint);
> - of_node_put(endpoint);
> - if (parent) {
> - /* dsi0 port 1 */
> - dsi_lanes = drm_of_get_data_lanes_count_ep(parent, 1, 
> -1, 1, 4);
> - of_node_put(parent);
> - }
> + dsi_lanes = drm_of_get_data_lanes_count(endpoint, 1, 4);
> +
> + /* Quirk old dtb: Use data lanes from the DSI host side instead of 
> bridge */
> + if (dsi_lanes == -EINVAL || dsi_lanes == -ENODEV) {
> + remote = of_graph_get_remote_endpoint(endpoint);
> + dsi_lanes = drm_of_get_data_lanes_count(remote, 1, 4);
> + of_node_put(remote);
> + if (dsi_lanes >= 1)
> + dev_warn(tc->dev, "missing dsi-lanes property for the 
> bridge\n");

It wasn't obvious what this warning should tell me at first. Maybe
add something like ". Falling back to the property of the remote
endpoint". A little verbose, maybe you could come up with a more
dense wording.

In any case,

Reviewed-by: Michael Walle 

-michael

Re: [PATCH v2 06/10] drm/bridge: tc358775: Get bridge data lanes instead of the DSI host lanes

[Dmitry Baryshkov]

On Sat, 2 Dec 2023 at 10:01, Tony Lindgren  wrote:
>
> The current code assumes the data-lanes property is configured on the
> DSI host side instead of the bridge side, and assumes DSI host endpoint 1.
>
> Let's standardize on what the other bridge drivers are doing and parse the
> data-lanes property for the bridge. Only if data-lanes property is not found,
> let's be nice and also check the DSI host for old dtb in use and warn.

It might be worth adding that lanes configuration for the host and for
the bridge might be different (e.g. the lanes might be swapped on the
host side).

Reviewed-by: Dmitry Baryshkov 

> Signed-off-by: Tony Lindgren 
> ---
>  drivers/gpu/drm/bridge/tc358775.c | 25 +++--
>  1 file changed, 11 insertions(+), 14 deletions(-)

-- 
With best wishes
Dmitry

[PATCH v2 06/10] drm/bridge: tc358775: Get bridge data lanes instead of the DSI host lanes

[Tony Lindgren]

The current code assumes the data-lanes property is configured on the
DSI host side instead of the bridge side, and assumes DSI host endpoint 1.

Let's standardize on what the other bridge drivers are doing and parse the
data-lanes property for the bridge. Only if data-lanes property is not found,
let's be nice and also check the DSI host for old dtb in use and warn.

Signed-off-by: Tony Lindgren 
---
 drivers/gpu/drm/bridge/tc358775.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358775.c 
b/drivers/gpu/drm/bridge/tc358775.c
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -525,27 +525,24 @@ tc_mode_valid(struct drm_bridge *bridge,
 static int tc358775_parse_dt(struct device_node *np, struct tc_data *tc)
 {
struct device_node *endpoint;
-   struct device_node *parent;
struct device_node *remote;
int dsi_lanes = -1;
 
-   /*
-* To get the data-lanes of dsi, we need to access the dsi0_out of port1
-*  of dsi0 endpoint from bridge port0 of d2l_in
-*/
endpoint = of_graph_get_endpoint_by_regs(tc->dev->of_node,
 TC358775_DSI_IN, -1);
-   if (endpoint) {
-   /* dsi0_out node */
-   parent = of_graph_get_remote_port_parent(endpoint);
-   of_node_put(endpoint);
-   if (parent) {
-   /* dsi0 port 1 */
-   dsi_lanes = drm_of_get_data_lanes_count_ep(parent, 1, 
-1, 1, 4);
-   of_node_put(parent);
-   }
+   dsi_lanes = drm_of_get_data_lanes_count(endpoint, 1, 4);
+
+   /* Quirk old dtb: Use data lanes from the DSI host side instead of 
bridge */
+   if (dsi_lanes == -EINVAL || dsi_lanes == -ENODEV) {
+   remote = of_graph_get_remote_endpoint(endpoint);
+   dsi_lanes = drm_of_get_data_lanes_count(remote, 1, 4);
+   of_node_put(remote);
+   if (dsi_lanes >= 1)
+   dev_warn(tc->dev, "missing dsi-lanes property for the 
bridge\n");
}
 
+   of_node_put(endpoint);
+
if (dsi_lanes < 0)
return dsi_lanes;
 
-- 
2.43.0