[PATCH 07/29] drm/omap: Remove connection checks from display .enable() and .remove()

Sebastian Reichel sebastian.reichel at collabora.com
Sun Dec 9 21:54:50 UTC 2018


Hi,

On Wed, Dec 05, 2018 at 05:00:00PM +0200, Laurent Pinchart wrote:
> The displays (connectors, panels and encoders) return an error from
> their .enable() handler when the dss device is not connected. They also
> disconnect the dss device explicitly from their .remove() handler if it
> is still connected.
> 
> Those safety checks are not needed:
> 
> - The .enable() handler is called from code paths that access the dss
>   devices chain from the display device, which is set to NULL when the
>   device isn't connected.
> 
> - The .remove() handler can only be called when unloading the module as
>   the driver has the suppress_bind_attrs attribute set, and a reference
>   to the module is taken when constructing the dss devices chain, so the
>   module can only be unloaded when the dss device is disconnected.
> 
> Remove the safety checks.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel at collabora.com>

-- Sebastian

>  .../gpu/drm/omapdrm/displays/encoder-opa362.c   |  7 -------
>  .../gpu/drm/omapdrm/displays/encoder-tfp410.c   |  7 -------
>  .../drm/omapdrm/displays/encoder-tpd12s015.c    |  4 ----
>  drivers/gpu/drm/omapdrm/dss/base.c              |  5 +++++
>  drivers/gpu/drm/omapdrm/dss/omapdss.h           |  5 -----
>  drivers/gpu/drm/omapdrm/omap_encoder.c          | 17 +++++------------
>  6 files changed, 10 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
> index 4fefd80f53bb..0b1032625e42 100644
> --- a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
> @@ -49,9 +49,6 @@ static int opa362_enable(struct omap_dss_device *dssdev)
>  
>  	dev_dbg(dssdev->dev, "enable\n");
>  
> -	if (!omapdss_device_is_connected(dssdev))
> -		return -ENODEV;
> -
>  	if (omapdss_device_is_enabled(dssdev))
>  		return 0;
>  
> @@ -145,10 +142,6 @@ static int __exit opa362_remove(struct platform_device *pdev)
>  	if (omapdss_device_is_enabled(dssdev))
>  		opa362_disable(dssdev);
>  
> -	WARN_ON(omapdss_device_is_connected(dssdev));
> -	if (omapdss_device_is_connected(dssdev))
> -		omapdss_device_disconnect(NULL, dssdev);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c b/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c
> index f1a748353279..fcc2dc5188a2 100644
> --- a/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c
> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c
> @@ -42,9 +42,6 @@ static int tfp410_enable(struct omap_dss_device *dssdev)
>  	struct omap_dss_device *src = dssdev->src;
>  	int r;
>  
> -	if (!omapdss_device_is_connected(dssdev))
> -		return -ENODEV;
> -
>  	if (omapdss_device_is_enabled(dssdev))
>  		return 0;
>  
> @@ -139,10 +136,6 @@ static int __exit tfp410_remove(struct platform_device *pdev)
>  	if (omapdss_device_is_enabled(dssdev))
>  		tfp410_disable(dssdev);
>  
> -	WARN_ON(omapdss_device_is_connected(dssdev));
> -	if (omapdss_device_is_connected(dssdev))
> -		omapdss_device_disconnect(NULL, dssdev);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> index 94de55fd8884..1a2bc59bf104 100644
> --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> @@ -229,10 +229,6 @@ static int __exit tpd_remove(struct platform_device *pdev)
>  	if (omapdss_device_is_enabled(dssdev))
>  		tpd_disable(dssdev);
>  
> -	WARN_ON(omapdss_device_is_connected(dssdev));
> -	if (omapdss_device_is_connected(dssdev))
> -		omapdss_device_disconnect(NULL, dssdev);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/dss/base.c b/drivers/gpu/drm/omapdrm/dss/base.c
> index 472f56e3de70..787157b00694 100644
> --- a/drivers/gpu/drm/omapdrm/dss/base.c
> +++ b/drivers/gpu/drm/omapdrm/dss/base.c
> @@ -185,6 +185,11 @@ struct omap_dss_device *omapdss_device_get_next(struct omap_dss_device *from,
>  }
>  EXPORT_SYMBOL(omapdss_device_get_next);
>  
> +static bool omapdss_device_is_connected(struct omap_dss_device *dssdev)
> +{
> +	return dssdev->src;
> +}
> +
>  int omapdss_device_connect(struct dss_device *dss,
>  			   struct omap_dss_device *src,
>  			   struct omap_dss_device *dst)
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> index 4e1e58e85429..bd21387ba494 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> @@ -505,11 +505,6 @@ int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
>  int omapdss_compat_init(void);
>  void omapdss_compat_uninit(void);
>  
> -static inline bool omapdss_device_is_connected(struct omap_dss_device *dssdev)
> -{
> -	return dssdev->src;
> -}
> -
>  static inline bool omapdss_device_is_enabled(struct omap_dss_device *dssdev)
>  {
>  	return dssdev->state == OMAP_DSS_DISPLAY_ACTIVE;
> diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
> index e3290f26abab..936756b18545 100644
> --- a/drivers/gpu/drm/omapdrm/omap_encoder.c
> +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
> @@ -160,24 +160,17 @@ static void omap_encoder_enable(struct drm_encoder *encoder)
>  
>  	dev_dbg(dev->dev, "enable(%s)\n", dssdev->name);
>  
> -	if (!omapdss_device_is_connected(dssdev)) {
> -		r = -ENODEV;
> -		goto error;
> -	}
> -
>  	if (omapdss_device_is_enabled(dssdev))
>  		return;
>  
>  	r = dssdev->ops->enable(dssdev);
> -	if (r)
> -		goto error;
> +	if (r) {
> +		dev_err(dev->dev, "Failed to enable display '%s': %d\n",
> +			dssdev->name, r);
> +		return;
> +	}
>  
>  	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> -	return;
> -
> -error:
> -	dev_err(dev->dev, "Failed to enable display '%s': %d\n",
> -		dssdev->name, r);
>  }
>  
>  static int omap_encoder_atomic_check(struct drm_encoder *encoder,
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20181209/312d7c8d/attachment-0001.sig>


More information about the dri-devel mailing list