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.pinch...@ideasonboard.com> > ---
Reviewed-by: Sebastian Reichel <sebastian.reic...@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
