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

Sebastian Reichel sebastian.reichel at collabora.com
Sun Dec 9 21:55:09 UTC 2018


Hi,

On Wed, Dec 05, 2018 at 05:00:01PM +0200, Laurent Pinchart wrote:
> The displays (connectors, panels and encoders) bail out from their
> .enable() and .disable() handlers if the dss device is already enabled
> or disabled. Those safety checks are not needed when the functions are
> called through the omapdss_device_ops, as the .enable() and .disable()
> handlers are called from the DRM atomic helpers that already guarantee
> that no double enabling or disabling can occur.
> 
> However, the handlers are also called directly from the .remove()
> handler. While this shouldn't be needed either as the modules can't be
> removed as long as the device is in use, it's still a good practice to
> disable the device explicitly. There is currently a safety check in
> .remove() in some drivers but not all of them.
> 
> Remove the safety checks from the .enable() and .disable() handlers, and
> add missing ones in the .remove() handler.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---

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

-- Sebastian

>  .../gpu/drm/omapdrm/displays/connector-analog-tv.c    |  6 ++----
>  drivers/gpu/drm/omapdrm/displays/connector-dvi.c      |  6 ++----
>  drivers/gpu/drm/omapdrm/displays/connector-hdmi.c     |  6 ++----
>  drivers/gpu/drm/omapdrm/displays/encoder-opa362.c     |  6 ------
>  drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c     |  6 ------
>  drivers/gpu/drm/omapdrm/displays/panel-dpi.c          |  6 ++----
>  drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c       | 11 +++++------
>  .../drm/omapdrm/displays/panel-lgphilips-lb035q02.c   |  6 ++----
>  .../gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c   |  6 ++----
>  .../drm/omapdrm/displays/panel-sharp-ls037v7dw01.c    |  6 ++----
>  .../gpu/drm/omapdrm/displays/panel-sony-acx565akm.c   |  6 ++----
>  .../gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c   |  6 ++----
>  .../gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c   |  6 ++----
>  drivers/gpu/drm/omapdrm/omap_encoder.c                |  6 ------
>  14 files changed, 25 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/connector-analog-tv.c b/drivers/gpu/drm/omapdrm/displays/connector-analog-tv.c
> index 910a5b9c036a..2b5b77627cfb 100644
> --- a/drivers/gpu/drm/omapdrm/displays/connector-analog-tv.c
> +++ b/drivers/gpu/drm/omapdrm/displays/connector-analog-tv.c
> @@ -46,9 +46,6 @@ static void tvc_disable(struct omap_dss_device *dssdev)
>  {
>  	struct omap_dss_device *src = dssdev->src;
>  
> -	if (!omapdss_device_is_enabled(dssdev))
> -		return;
> -
>  	src->ops->disable(src);
>  }
>  
> @@ -92,7 +89,8 @@ static int __exit tvc_remove(struct platform_device *pdev)
>  
>  	omapdss_device_unregister(&ddata->dssdev);
>  
> -	tvc_disable(dssdev);
> +	if (omapdss_device_is_enabled(dssdev))
> +		tvc_disable(dssdev);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> index 1e0925791c3d..a1784e263835 100644
> --- a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> @@ -57,9 +57,6 @@ static void dvic_disable(struct omap_dss_device *dssdev)
>  {
>  	struct omap_dss_device *src = dssdev->src;
>  
> -	if (!omapdss_device_is_enabled(dssdev))
> -		return;
> -
>  	src->ops->disable(src);
>  }
>  
> @@ -282,7 +279,8 @@ static int __exit dvic_remove(struct platform_device *pdev)
>  
>  	omapdss_device_unregister(&ddata->dssdev);
>  
> -	dvic_disable(dssdev);
> +	if (omapdss_device_is_enabled(dssdev))
> +		dvic_disable(dssdev);
>  
>  	i2c_put_adapter(ddata->i2c_adapter);
>  
> diff --git a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
> index 649364e04edd..05cd503c4d29 100644
> --- a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
> @@ -52,9 +52,6 @@ static void hdmic_disable(struct omap_dss_device *dssdev)
>  {
>  	struct omap_dss_device *src = dssdev->src;
>  
> -	if (!omapdss_device_is_enabled(dssdev))
> -		return;
> -
>  	src->ops->disable(src);
>  }
>  
> @@ -179,7 +176,8 @@ static int __exit hdmic_remove(struct platform_device *pdev)
>  
>  	omapdss_device_unregister(&ddata->dssdev);
>  
> -	hdmic_disable(dssdev);
> +	if (omapdss_device_is_enabled(dssdev))
> +		hdmic_disable(dssdev);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c b/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c
> index 0b1032625e42..ce116c28479f 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_enabled(dssdev))
> -		return 0;
> -
>  	r = src->ops->enable(src);
>  	if (r)
>  		return r;
> @@ -71,9 +68,6 @@ static void opa362_disable(struct omap_dss_device *dssdev)
>  
>  	dev_dbg(dssdev->dev, "disable\n");
>  
> -	if (!omapdss_device_is_enabled(dssdev))
> -		return;
> -
>  	if (ddata->enable_gpio)
>  		gpiod_set_value_cansleep(ddata->enable_gpio, 0);
>  
> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c b/drivers/gpu/drm/omapdrm/displays/encoder-tfp410.c
> index fcc2dc5188a2..d51410ed1e13 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_enabled(dssdev))
> -		return 0;
> -
>  	r = src->ops->enable(src);
>  	if (r)
>  		return r;
> @@ -62,9 +59,6 @@ static void tfp410_disable(struct omap_dss_device *dssdev)
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  	struct omap_dss_device *src = dssdev->src;
>  
> -	if (!omapdss_device_is_enabled(dssdev))
> -		return;
> -
>  	if (ddata->pd_gpio)
>  		gpiod_set_value_cansleep(ddata->pd_gpio, 0);
>  
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> index 9439054de8b9..5ca774c712a6 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> @@ -72,9 +72,6 @@ static void panel_dpi_disable(struct omap_dss_device *dssdev)
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  	struct omap_dss_device *src = dssdev->src;
>  
> -	if (!omapdss_device_is_enabled(dssdev))
> -		return;
> -
>  	backlight_disable(ddata->backlight);
>  
>  	gpiod_set_value_cansleep(ddata->enable_gpio, 0);
> @@ -181,7 +178,8 @@ static int __exit panel_dpi_remove(struct platform_device *pdev)
>  
>  	omapdss_device_unregister(dssdev);
>  
> -	panel_dpi_disable(dssdev);
> +	if (omapdss_device_is_enabled(dssdev))
> +		panel_dpi_disable(dssdev);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> index e346451647c4..a7c8688237fb 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> @@ -829,11 +829,9 @@ static void dsicm_disable(struct omap_dss_device *dssdev)
>  
>  	src->ops->dsi.bus_lock(src);
>  
> -	if (omapdss_device_is_enabled(dssdev)) {
> -		r = dsicm_wake_up(ddata);
> -		if (!r)
> -			dsicm_power_off(ddata);
> -	}
> +	r = dsicm_wake_up(ddata);
> +	if (!r)
> +		dsicm_power_off(ddata);
>  
>  	src->ops->dsi.bus_unlock(src);
>  
> @@ -1367,7 +1365,8 @@ static int __exit dsicm_remove(struct platform_device *pdev)
>  
>  	omapdss_device_unregister(dssdev);
>  
> -	dsicm_disable(dssdev);
> +	if (omapdss_device_is_enabled(dssdev))
> +		dsicm_disable(dssdev);
>  	omapdss_device_disconnect(dssdev->src, dssdev);
>  
>  	sysfs_remove_group(&pdev->dev.kobj, &dsicm_attr_group);
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
> index 19c0c3e85aa5..2c3b15ba5a39 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
> @@ -144,9 +144,6 @@ static void lb035q02_disable(struct omap_dss_device *dssdev)
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  	struct omap_dss_device *src = dssdev->src;
>  
> -	if (!omapdss_device_is_enabled(dssdev))
> -		return;
> -
>  	if (ddata->enable_gpio)
>  		gpiod_set_value_cansleep(ddata->enable_gpio, 0);
>  
> @@ -235,7 +232,8 @@ static int lb035q02_panel_spi_remove(struct spi_device *spi)
>  
>  	omapdss_device_unregister(dssdev);
>  
> -	lb035q02_disable(dssdev);
> +	if (omapdss_device_is_enabled(dssdev))
> +		lb035q02_disable(dssdev);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c b/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c
> index 9cef1d16d7d3..ef83459611be 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c
> @@ -138,9 +138,6 @@ static void nec_8048_disable(struct omap_dss_device *dssdev)
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  	struct omap_dss_device *src = dssdev->src;
>  
> -	if (!omapdss_device_is_enabled(dssdev))
> -		return;
> -
>  	gpiod_set_value_cansleep(ddata->res_gpio, 0);
>  
>  	src->ops->disable(src);
> @@ -226,7 +223,8 @@ static int nec_8048_remove(struct spi_device *spi)
>  
>  	omapdss_device_unregister(dssdev);
>  
> -	nec_8048_disable(dssdev);
> +	if (omapdss_device_is_enabled(dssdev))
> +		nec_8048_disable(dssdev);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> index 5a06fbb7984a..0c5b405b4c9e 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> @@ -97,9 +97,6 @@ static void sharp_ls_disable(struct omap_dss_device *dssdev)
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  	struct omap_dss_device *src = dssdev->src;
>  
> -	if (!omapdss_device_is_enabled(dssdev))
> -		return;
> -
>  	if (ddata->ini_gpio)
>  		gpiod_set_value_cansleep(ddata->ini_gpio, 0);
>  
> @@ -233,7 +230,8 @@ static int __exit sharp_ls_remove(struct platform_device *pdev)
>  
>  	omapdss_device_unregister(dssdev);
>  
> -	sharp_ls_disable(dssdev);
> +	if (omapdss_device_is_enabled(dssdev))
> +		sharp_ls_disable(dssdev);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c b/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c
> index 209a87c70c99..99c2c4f27dd5 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c
> @@ -605,9 +605,6 @@ static void acx565akm_disable(struct omap_dss_device *dssdev)
>  {
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  
> -	if (!omapdss_device_is_enabled(dssdev))
> -		return;
> -
>  	mutex_lock(&ddata->mutex);
>  	acx565akm_panel_power_off(dssdev);
>  	mutex_unlock(&ddata->mutex);
> @@ -750,7 +747,8 @@ static int acx565akm_remove(struct spi_device *spi)
>  
>  	omapdss_device_unregister(dssdev);
>  
> -	acx565akm_disable(dssdev);
> +	if (omapdss_device_is_enabled(dssdev))
> +		acx565akm_disable(dssdev);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> index b09fea97a441..8551a1df3ad6 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> @@ -270,9 +270,6 @@ static void td028ttec1_panel_disable(struct omap_dss_device *dssdev)
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  	struct omap_dss_device *src = dssdev->src;
>  
> -	if (!omapdss_device_is_enabled(dssdev))
> -		return;
> -
>  	dev_dbg(dssdev->dev, "td028ttec1_panel_disable()\n");
>  
>  	jbt_ret_write_0(ddata, JBT_REG_DISPLAY_OFF);
> @@ -357,7 +354,8 @@ static int td028ttec1_panel_remove(struct spi_device *spi)
>  
>  	omapdss_device_unregister(dssdev);
>  
> -	td028ttec1_panel_disable(dssdev);
> +	if (omapdss_device_is_enabled(dssdev))
> +		td028ttec1_panel_disable(dssdev);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c
> index 998f21f7701a..527abed69d34 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c
> @@ -350,9 +350,6 @@ static void tpo_td043_disable(struct omap_dss_device *dssdev)
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  	struct omap_dss_device *src = dssdev->src;
>  
> -	if (!omapdss_device_is_enabled(dssdev))
> -		return;
> -
>  	src->ops->disable(src);
>  
>  	if (!ddata->spi_suspended)
> @@ -457,7 +454,8 @@ static int tpo_td043_remove(struct spi_device *spi)
>  
>  	omapdss_device_unregister(dssdev);
>  
> -	tpo_td043_disable(dssdev);
> +	if (omapdss_device_is_enabled(dssdev))
> +		tpo_td043_disable(dssdev);
>  
>  	sysfs_remove_group(&spi->dev.kobj, &tpo_td043_attr_group);
>  
> diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
> index 936756b18545..b0c06103b5cd 100644
> --- a/drivers/gpu/drm/omapdrm/omap_encoder.c
> +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
> @@ -143,9 +143,6 @@ static void omap_encoder_disable(struct drm_encoder *encoder)
>  
>  	dev_dbg(dev->dev, "disable(%s)\n", dssdev->name);
>  
> -	if (!omapdss_device_is_enabled(dssdev))
> -		return;
> -
>  	dssdev->ops->disable(dssdev);
>  
>  	dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
> @@ -160,9 +157,6 @@ static void omap_encoder_enable(struct drm_encoder *encoder)
>  
>  	dev_dbg(dev->dev, "enable(%s)\n", dssdev->name);
>  
> -	if (omapdss_device_is_enabled(dssdev))
> -		return;
> -
>  	r = dssdev->ops->enable(dssdev);
>  	if (r) {
>  		dev_err(dev->dev, "Failed to enable display '%s': %d\n",
> -- 
> 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/3dd15d6b/attachment-0001.sig>


More information about the dri-devel mailing list