Re: [PATCH 08/24] drm/omap: Separate the dssdevs array setup from the connect function

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

Thank you for the patch.

On Monday, 12 February 2018 11:44:38 EET Tomi Valkeinen wrote:
> From: Peter Ujfalusi <peter.ujfalusi@ti.com>
> 
> In order to ease up on the logic,

I have some doubts about this :-) I find the logic both larger (40 insertions, 
14 deletions) and more complex.

> break the current code to gather the dssdevs:
> 
> first get all available dssdevs, then call connect on each dssdev. As the
> last step remove the dssdevs which failed to connect from the available
> dssdev list.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 54 ++++++++++++++++++++++++++--------
>  1 file changed, 40 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index 869a8ab6aa4e..b5061fc7241a
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -165,34 +165,60 @@ static void omap_disconnect_dssdevs(struct drm_device
> *ddev) priv->num_dssdevs = 0;
>  }
> 
> -static int omap_connect_dssdevs(struct drm_device *ddev)
> +static void omap_collect_dssdevs(struct drm_device *ddev)
>  {
>  	struct omap_drm_private *priv = ddev->dev_private;
>  	struct omap_dss_device *dssdev = NULL;
> -	int r;
> +
> +	for_each_dss_dev(dssdev) {
> +		omap_dss_get_device(dssdev);
> +		priv->dssdevs[priv->num_dssdevs++] = dssdev;
> +		if (priv->num_dssdevs == ARRAY_SIZE(priv->dssdevs)) {
> +			/* To balance the 'for_each_dss_dev' loop */
> +			omap_dss_put_device(dssdev);
> +			break;
> +		}
> +	}
> +}
> +
> +static int omap_connect_dssdevs(struct drm_device *ddev)
> +{
> +	struct omap_drm_private *priv = ddev->dev_private;
> +	u32 working = 0;
> +	int r, i, j;
> 
>  	if (!omapdss_stack_is_ready())
>  		return -EPROBE_DEFER;
> 
> -	for_each_dss_dev(dssdev) {
> +	omap_collect_dssdevs(ddev);
> +
> +	for (i = 0; i < priv->num_dssdevs; i++) {
> +		struct omap_dss_device *dssdev = priv->dssdevs[i];
> +
>  		r = dssdev->driver->connect(dssdev);
> -		if (r == -EPROBE_DEFER) {
> -			omap_dss_put_device(dssdev);
> +		if (r == -EPROBE_DEFER)
>  			goto cleanup;
> -		} else if (r) {
> +		else if (r)
>  			dev_warn(dssdev->dev, "could not connect display: %s\n",
> -				dssdev->name);
> +				 dssdev->name);
> +		else
> +			working |= BIT(i);
> +	}
> +
> +	/* Remove the dssdevs if their connect failed */
> +	j = 0;
> +	for (i = 0; i < priv->num_dssdevs; i++) {
> +		if (working & BIT(i)) {
> +			if (j != i)
> +				priv->dssdevs[j] = priv->dssdevs[i];
> +			j++;
>  		} else {
> -			omap_dss_get_device(dssdev);
> -			priv->dssdevs[priv->num_dssdevs++] = dssdev;
> -			if (priv->num_dssdevs == ARRAY_SIZE(priv->dssdevs)) {
> -				/* To balance the 'for_each_dss_dev' loop */
> -				omap_dss_put_device(dssdev);
> -				break;
> -			}
> +			omap_dss_put_device(priv->dssdevs[i]);
>  		}
>  	}
> 
> +	priv->num_dssdevs = j;
> +
>  	return 0;
> 
>  cleanup:

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel