Re: [PATCH v3 3/3] drm/vc4: dpi: inline vc4_dpi_init_bridge() into vc4_dpi_bind()

["Maíra Canal" <mcanal@igalia.com>]

Hi Luca,

If you go ahead with this patch, could you merge it with [1/3]
Personally, I prefer to keep it as a separate function. vc4_dpi_bind()
is already a big function, with several initializations. I have the
impression that vc4_dpi_init_bridge() makes the error handling more
clear.

But that's bikeshed territory...

Best regards,
- Maíra

On 13/10/26 12:23, Luca Ceresoli wrote:
> There isn't much benefit in this small static function. Inline it in the
> caller.
> 
> Link: https://lore.kernel.org/lkml/accaa9f4ad05ef73071e5cfb5368fcbdd394d3ab.camel@bootlin.com/
> Suggested-by: Paul Cercueil <paul.cercueil@bootlin.com>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> ---
> 
> New patch in v3
> ---
>   drivers/gpu/drm/vc4/vc4_dpi.c | 37 ++++++++++++++-----------------------
>   1 file changed, 14 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
> index feefefcf3ad7..5439e73da0d4 100644
> --- a/drivers/gpu/drm/vc4/vc4_dpi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dpi.c
> @@ -278,26 +278,6 @@ static const struct of_device_id vc4_dpi_dt_match[] = {
>   	{}
>   };
>   
> -/* Sets up the next link in the display chain */
> -static int vc4_dpi_init_bridge(struct vc4_dpi *dpi)
> -{
> -	struct device *dev = &dpi->pdev->dev;
> -
> -	struct drm_bridge *bridge __free(drm_bridge_put) =
> -		of_drm_get_bridge_by_endpoint(dev->of_node, 0, 0);
> -	if (IS_ERR(bridge)) {
> -		/* If nothing was connected in the DT, that's not an
> -		 * error.
> -		 */
> -		if (PTR_ERR(bridge) == -ENODEV)
> -			return 0;
> -		else
> -			return PTR_ERR(bridge);
> -	}
> -
> -	return drm_bridge_attach(&dpi->encoder.base, bridge, NULL, 0);
> -}
> -
>   static void vc4_dpi_disable_clock(void *ptr)
>   {
>   	struct vc4_dpi *dpi = ptr;
> @@ -366,9 +346,20 @@ static int vc4_dpi_bind(struct device *dev, struct device *master, void *data)
>   
>   	drm_encoder_helper_add(&dpi->encoder.base, &vc4_dpi_encoder_helper_funcs);
>   
> -	ret = vc4_dpi_init_bridge(dpi);
> -	if (ret)
> -		return ret;
> +	/* Set up the next link in the display chain */
> +	struct drm_bridge *bridge __free(drm_bridge_put) =
> +		of_drm_get_bridge_by_endpoint(dev->of_node, 0, 0);
> +	if (IS_ERR(bridge)) {
> +		/* If nothing was connected in the DT (-ENODEV), that's not
> +		 * an error.
> +		 */
> +		if (PTR_ERR(bridge) != -ENODEV)
> +			return PTR_ERR(bridge);
> +	} else {
> +		ret = drm_bridge_attach(&dpi->encoder.base, bridge, NULL, 0);
> +		if (ret)
> +			return ret;
> +	}
>   
>   	dev_set_drvdata(dev, dpi);
>   
>