Re: [PATCH 05/29] drm/omap: Move common display enable/disable code to encoder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Wed, Dec 05, 2018 at 04:59:58PM +0200, Laurent Pinchart wrote:
> All .enable() and .disable() handlers for panels and connectors share
> common code that validates and updates the device's state. Move it to
> common locations in the omap_encoder_enable() and omap_encoder_disable()
> handlers.
> 
> The enabled check in the .disable() handler is left untouched, it will
> be addressed separately.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>

-- Sebastian

>  .../omapdrm/displays/connector-analog-tv.c    | 23 +-------------
>  .../gpu/drm/omapdrm/displays/connector-dvi.c  | 17 +----------
>  .../gpu/drm/omapdrm/displays/connector-hdmi.c | 23 +-------------
>  drivers/gpu/drm/omapdrm/displays/panel-dpi.c  | 10 -------
>  .../gpu/drm/omapdrm/displays/panel-dsi-cm.c   | 18 -----------
>  .../displays/panel-lgphilips-lb035q02.c       | 10 -------
>  .../omapdrm/displays/panel-nec-nl8048hl11.c   | 10 -------
>  .../displays/panel-sharp-ls037v7dw01.c        | 10 -------
>  .../omapdrm/displays/panel-sony-acx565akm.c   | 18 +----------
>  .../omapdrm/displays/panel-tpo-td028ttec1.c   | 14 +--------
>  .../omapdrm/displays/panel-tpo-td043mtea1.c   | 10 -------
>  drivers/gpu/drm/omapdrm/omap_encoder.c        | 30 +++++++++++++++++--
>  12 files changed, 32 insertions(+), 161 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/connector-analog-tv.c b/drivers/gpu/drm/omapdrm/displays/connector-analog-tv.c
> index 28a3ce8f88d2..910a5b9c036a 100644
> --- a/drivers/gpu/drm/omapdrm/displays/connector-analog-tv.c
> +++ b/drivers/gpu/drm/omapdrm/displays/connector-analog-tv.c
> @@ -37,40 +37,19 @@ static void tvc_disconnect(struct omap_dss_device *src,
>  
>  static int tvc_enable(struct omap_dss_device *dssdev)
>  {
> -	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  	struct omap_dss_device *src = dssdev->src;
> -	int r;
>  
> -	dev_dbg(ddata->dev, "enable\n");
> -
> -	if (!omapdss_device_is_connected(dssdev))
> -		return -ENODEV;
> -
> -	if (omapdss_device_is_enabled(dssdev))
> -		return 0;
> -
> -	r = src->ops->enable(src);
> -	if (r)
> -		return r;
> -
> -	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> -
> -	return r;
> +	return src->ops->enable(src);
>  }
>  
>  static void tvc_disable(struct omap_dss_device *dssdev)
>  {
> -	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  	struct omap_dss_device *src = dssdev->src;
>  
> -	dev_dbg(ddata->dev, "disable\n");
> -
>  	if (!omapdss_device_is_enabled(dssdev))
>  		return;
>  
>  	src->ops->disable(src);
> -
> -	dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
>  }
>  
>  static const struct omap_dss_device_ops tvc_ops = {
> diff --git a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> index 24b14f44248e..1e0925791c3d 100644
> --- a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> @@ -49,21 +49,8 @@ static void dvic_disconnect(struct omap_dss_device *src,
>  static int dvic_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;
>  
> -	r = src->ops->enable(src);
> -	if (r)
> -		return r;
> -
> -	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> -
> -	return 0;
> +	return src->ops->enable(src);
>  }
>  
>  static void dvic_disable(struct omap_dss_device *dssdev)
> @@ -74,8 +61,6 @@ static void dvic_disable(struct omap_dss_device *dssdev)
>  		return;
>  
>  	src->ops->disable(src);
> -
> -	dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
>  }
>  
>  static int dvic_ddc_read(struct i2c_adapter *adapter,
> diff --git a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
> index e602fa4a50a4..649364e04edd 100644
> --- a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
> @@ -43,40 +43,19 @@ static void hdmic_disconnect(struct omap_dss_device *src,
>  
>  static int hdmic_enable(struct omap_dss_device *dssdev)
>  {
> -	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  	struct omap_dss_device *src = dssdev->src;
> -	int r;
> -
> -	dev_dbg(ddata->dev, "enable\n");
> -
> -	if (!omapdss_device_is_connected(dssdev))
> -		return -ENODEV;
> -
> -	if (omapdss_device_is_enabled(dssdev))
> -		return 0;
>  
> -	r = src->ops->enable(src);
> -	if (r)
> -		return r;
> -
> -	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> -
> -	return r;
> +	return src->ops->enable(src);
>  }
>  
>  static void hdmic_disable(struct omap_dss_device *dssdev)
>  {
> -	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  	struct omap_dss_device *src = dssdev->src;
>  
> -	dev_dbg(ddata->dev, "disable\n");
> -
>  	if (!omapdss_device_is_enabled(dssdev))
>  		return;
>  
>  	src->ops->disable(src);
> -
> -	dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
>  }
>  
>  static bool hdmic_detect(struct omap_dss_device *dssdev)
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> index 1f8161b041be..9439054de8b9 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> @@ -51,12 +51,6 @@ static int panel_dpi_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;
> -
>  	r = src->ops->enable(src);
>  	if (r)
>  		return r;
> @@ -70,8 +64,6 @@ static int panel_dpi_enable(struct omap_dss_device *dssdev)
>  	gpiod_set_value_cansleep(ddata->enable_gpio, 1);
>  	backlight_enable(ddata->backlight);
>  
> -	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> -
>  	return 0;
>  }
>  
> @@ -89,8 +81,6 @@ static void panel_dpi_disable(struct omap_dss_device *dssdev)
>  	regulator_disable(ddata->vcc_supply);
>  
>  	src->ops->disable(src);
> -
> -	dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
>  }
>  
>  static void panel_dpi_get_timings(struct omap_dss_device *dssdev,
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> index 29692a5217c5..e346451647c4 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> @@ -793,20 +793,8 @@ static int dsicm_enable(struct omap_dss_device *dssdev)
>  	struct omap_dss_device *src = dssdev->src;
>  	int r;
>  
> -	dev_dbg(&ddata->pdev->dev, "enable\n");
> -
>  	mutex_lock(&ddata->lock);
>  
> -	if (!omapdss_device_is_connected(dssdev)) {
> -		r = -ENODEV;
> -		goto err;
> -	}
> -
> -	if (omapdss_device_is_enabled(dssdev)) {
> -		r = 0;
> -		goto err;
> -	}
> -
>  	src->ops->dsi.bus_lock(src);
>  
>  	r = dsicm_power_on(ddata);
> @@ -816,8 +804,6 @@ static int dsicm_enable(struct omap_dss_device *dssdev)
>  	if (r)
>  		goto err;
>  
> -	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> -
>  	mutex_unlock(&ddata->lock);
>  
>  	dsicm_bl_power(ddata, true);
> @@ -835,8 +821,6 @@ static void dsicm_disable(struct omap_dss_device *dssdev)
>  	struct omap_dss_device *src = dssdev->src;
>  	int r;
>  
> -	dev_dbg(&ddata->pdev->dev, "disable\n");
> -
>  	dsicm_bl_power(ddata, false);
>  
>  	mutex_lock(&ddata->lock);
> @@ -853,8 +837,6 @@ static void dsicm_disable(struct omap_dss_device *dssdev)
>  
>  	src->ops->dsi.bus_unlock(src);
>  
> -	dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
> -
>  	mutex_unlock(&ddata->lock);
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
> index f6ef8ff964dd..19c0c3e85aa5 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c
> @@ -129,12 +129,6 @@ static int lb035q02_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;
> -
>  	r = src->ops->enable(src);
>  	if (r)
>  		return r;
> @@ -142,8 +136,6 @@ static int lb035q02_enable(struct omap_dss_device *dssdev)
>  	if (ddata->enable_gpio)
>  		gpiod_set_value_cansleep(ddata->enable_gpio, 1);
>  
> -	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> -
>  	return 0;
>  }
>  
> @@ -159,8 +151,6 @@ static void lb035q02_disable(struct omap_dss_device *dssdev)
>  		gpiod_set_value_cansleep(ddata->enable_gpio, 0);
>  
>  	src->ops->disable(src);
> -
> -	dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
>  }
>  
>  static void lb035q02_get_timings(struct omap_dss_device *dssdev,
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c b/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c
> index f445de6369f7..9cef1d16d7d3 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-nec-nl8048hl11.c
> @@ -124,20 +124,12 @@ static int nec_8048_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;
> -
>  	r = src->ops->enable(src);
>  	if (r)
>  		return r;
>  
>  	gpiod_set_value_cansleep(ddata->res_gpio, 1);
>  
> -	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> -
>  	return 0;
>  }
>  
> @@ -152,8 +144,6 @@ static void nec_8048_disable(struct omap_dss_device *dssdev)
>  	gpiod_set_value_cansleep(ddata->res_gpio, 0);
>  
>  	src->ops->disable(src);
> -
> -	dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
>  }
>  
>  static void nec_8048_get_timings(struct omap_dss_device *dssdev,
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> index 64b1369cb274..5a06fbb7984a 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-sharp-ls037v7dw01.c
> @@ -68,12 +68,6 @@ static int sharp_ls_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;
> -
>  	if (ddata->vcc) {
>  		r = regulator_enable(ddata->vcc);
>  		if (r != 0)
> @@ -95,8 +89,6 @@ static int sharp_ls_enable(struct omap_dss_device *dssdev)
>  	if (ddata->ini_gpio)
>  		gpiod_set_value_cansleep(ddata->ini_gpio, 1);
>  
> -	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> -
>  	return 0;
>  }
>  
> @@ -122,8 +114,6 @@ static void sharp_ls_disable(struct omap_dss_device *dssdev)
>  
>  	if (ddata->vcc)
>  		regulator_disable(ddata->vcc);
> -
> -	dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
>  }
>  
>  static void sharp_ls_get_timings(struct omap_dss_device *dssdev,
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c b/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c
> index e04663856b31..209a87c70c99 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-sony-acx565akm.c
> @@ -594,39 +594,23 @@ static int acx565akm_enable(struct omap_dss_device *dssdev)
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  	int r;
>  
> -	dev_dbg(dssdev->dev, "%s\n", __func__);
> -
> -	if (!omapdss_device_is_connected(dssdev))
> -		return -ENODEV;
> -
> -	if (omapdss_device_is_enabled(dssdev))
> -		return 0;
> -
>  	mutex_lock(&ddata->mutex);
>  	r = acx565akm_panel_power_on(dssdev);
>  	mutex_unlock(&ddata->mutex);
> -	if (r)
> -		return r;
>  
> -	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> -
> -	return 0;
> +	return r;
>  }
>  
>  static void acx565akm_disable(struct omap_dss_device *dssdev)
>  {
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  
> -	dev_dbg(dssdev->dev, "%s\n", __func__);
> -
>  	if (!omapdss_device_is_enabled(dssdev))
>  		return;
>  
>  	mutex_lock(&ddata->mutex);
>  	acx565akm_panel_power_off(dssdev);
>  	mutex_unlock(&ddata->mutex);
> -
> -	dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
>  }
>  
>  static void acx565akm_get_timings(struct omap_dss_device *dssdev,
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> index 7ddc8c574a61..b09fea97a441 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c
> @@ -175,12 +175,6 @@ static int td028ttec1_panel_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;
> -
>  	r = src->ops->enable(src);
>  	if (r)
>  		return r;
> @@ -198,7 +192,7 @@ static int td028ttec1_panel_enable(struct omap_dss_device *dssdev)
>  
>  	if (r) {
>  		dev_warn(dssdev->dev, "transfer error\n");
> -		goto transfer_err;
> +		return -EIO;
>  	}
>  
>  	/* deep standby out */
> @@ -268,10 +262,6 @@ static int td028ttec1_panel_enable(struct omap_dss_device *dssdev)
>  
>  	r |= jbt_ret_write_0(ddata, JBT_REG_DISPLAY_ON);
>  
> -	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> -
> -transfer_err:
> -
>  	return r ? -EIO : 0;
>  }
>  
> @@ -291,8 +281,6 @@ static void td028ttec1_panel_disable(struct omap_dss_device *dssdev)
>  	jbt_reg_write_1(ddata, JBT_REG_POWER_ON_OFF, 0x00);
>  
>  	src->ops->disable(src);
> -
> -	dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
>  }
>  
>  static void td028ttec1_panel_get_timings(struct omap_dss_device *dssdev,
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c
> index 8440fcb744d9..998f21f7701a 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-tpo-td043mtea1.c
> @@ -326,12 +326,6 @@ static int tpo_td043_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;
> -
>  	r = src->ops->enable(src);
>  	if (r)
>  		return r;
> @@ -348,8 +342,6 @@ static int tpo_td043_enable(struct omap_dss_device *dssdev)
>  		}
>  	}
>  
> -	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> -
>  	return 0;
>  }
>  
> @@ -365,8 +357,6 @@ static void tpo_td043_disable(struct omap_dss_device *dssdev)
>  
>  	if (!ddata->spi_suspended)
>  		tpo_td043_power_off(ddata);
> -
> -	dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
>  }
>  
>  static void tpo_td043_get_timings(struct omap_dss_device *dssdev,
> diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
> index 452e625f6ce3..e3290f26abab 100644
> --- a/drivers/gpu/drm/omapdrm/omap_encoder.c
> +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
> @@ -139,21 +139,45 @@ static void omap_encoder_disable(struct drm_encoder *encoder)
>  {
>  	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
>  	struct omap_dss_device *dssdev = omap_encoder->display;
> +	struct drm_device *dev = encoder->dev;
> +
> +	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;
>  }
>  
>  static void omap_encoder_enable(struct drm_encoder *encoder)
>  {
>  	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
>  	struct omap_dss_device *dssdev = omap_encoder->display;
> +	struct drm_device *dev = encoder->dev;
>  	int r;
>  
> +	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)
> -		dev_err(encoder->dev->dev,
> -			"Failed to enable display '%s': %d\n",
> -			dssdev->name, r);
> +		goto error;
> +
> +	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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Attachment: signature.asc
Description: PGP signature

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

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux