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

[Michael Walle <mwalle@kernel.org>]

> 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 <tony@atomide.com>
> ---
>  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 <mwalle@kernel.org>

-michael