diff mbox series

[v3,29/56] drm/omap: dsi: do ULPS in host driver

Message ID 20201105120333.947408-30-tomi.valkeinen@ti.com
State Superseded
Headers show
Series Convert DSI code to use drm_mipi_dsi and drm_panel | expand

Commit Message

Tomi Valkeinen Nov. 5, 2020, 12:03 p.m. UTC
From: Sebastian Reichel <sebastian.reichel@collabora.com>

Move ULPS handling into the DSI host controller, so that we
no longer need a custom API for the DSI client.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 .../gpu/drm/omapdrm/displays/panel-dsi-cm.c   | 273 +-----------------
 drivers/gpu/drm/omapdrm/dss/dsi.c             |  63 +++-
 drivers/gpu/drm/omapdrm/dss/omapdss.h         |   2 -
 3 files changed, 62 insertions(+), 276 deletions(-)

Comments

Laurent Pinchart Nov. 9, 2020, 10:03 a.m. UTC | #1
Hi Tomi and Sebastian,

Thank you for the patch.

On Thu, Nov 05, 2020 at 02:03:06PM +0200, Tomi Valkeinen wrote:
> From: Sebastian Reichel <sebastian.reichel@collabora.com>

> 

> Move ULPS handling into the DSI host controller, so that we

> no longer need a custom API for the DSI client.

> 

> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

> ---

>  .../gpu/drm/omapdrm/displays/panel-dsi-cm.c   | 273 +-----------------

>  drivers/gpu/drm/omapdrm/dss/dsi.c             |  63 +++-

>  drivers/gpu/drm/omapdrm/dss/omapdss.h         |   2 -

>  3 files changed, 62 insertions(+), 276 deletions(-)

> 

> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c

> index 78247dcb1848..030a8fa140db 100644

> --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c

> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c

> @@ -16,7 +16,6 @@

>  #include <linux/module.h>

>  #include <linux/sched/signal.h>

>  #include <linux/slab.h>

> -#include <linux/workqueue.h>

>  #include <linux/of_device.h>

>  #include <linux/regulator/consumer.h>

>  

> @@ -69,21 +68,13 @@ struct panel_drv_data {

>  

>  	bool intro_printed;

>  

> -	struct workqueue_struct *workqueue;

> -

>  	bool ulps_enabled;

> -	unsigned int ulps_timeout;

> -	struct delayed_work ulps_work;

>  };

>  

>  #define to_panel_data(p) container_of(p, struct panel_drv_data, dssdev)

>  

>  static int _dsicm_enable_te(struct panel_drv_data *ddata, bool enable);

>  

> -static int dsicm_panel_reset(struct panel_drv_data *ddata);

> -

> -static void dsicm_ulps_work(struct work_struct *work);

> -

>  static void dsicm_bl_power(struct panel_drv_data *ddata, bool enable)

>  {

>  	struct backlight_device *backlight;

> @@ -207,94 +198,6 @@ static int dsicm_set_update_window(struct panel_drv_data *ddata,

>  	return 0;

>  }

>  

> -static void dsicm_queue_ulps_work(struct panel_drv_data *ddata)

> -{

> -	if (ddata->ulps_timeout > 0)

> -		queue_delayed_work(ddata->workqueue, &ddata->ulps_work,

> -				msecs_to_jiffies(ddata->ulps_timeout));

> -}

> -

> -static void dsicm_cancel_ulps_work(struct panel_drv_data *ddata)

> -{

> -	cancel_delayed_work(&ddata->ulps_work);

> -}

> -

> -static int dsicm_enter_ulps(struct panel_drv_data *ddata)

> -{

> -	struct omap_dss_device *src = ddata->src;

> -	int r;

> -

> -	if (ddata->ulps_enabled)

> -		return 0;

> -

> -	dsicm_cancel_ulps_work(ddata);

> -

> -	r = _dsicm_enable_te(ddata, false);

> -	if (r)

> -		goto err;

> -

> -	src->ops->dsi.ulps(src, true);

> -

> -	ddata->ulps_enabled = true;

> -

> -	return 0;

> -

> -err:

> -	dev_err(&ddata->dsi->dev, "enter ULPS failed");

> -	dsicm_panel_reset(ddata);

> -

> -	ddata->ulps_enabled = false;

> -

> -	dsicm_queue_ulps_work(ddata);

> -

> -	return r;

> -}

> -

> -static int dsicm_exit_ulps(struct panel_drv_data *ddata)

> -{

> -	struct omap_dss_device *src = ddata->src;

> -	int r;

> -

> -	if (!ddata->ulps_enabled)

> -		return 0;

> -

> -	src->ops->dsi.ulps(src, false);

> -	ddata->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;

> -

> -	r = _dsicm_enable_te(ddata, ddata->te_enabled);

> -	if (r) {

> -		dev_err(&ddata->dsi->dev, "failed to re-enable TE");

> -		goto err2;

> -	}

> -

> -	dsicm_queue_ulps_work(ddata);

> -

> -	ddata->ulps_enabled = false;

> -

> -	return 0;

> -

> -err2:

> -	dev_err(&ddata->dsi->dev, "failed to exit ULPS");

> -

> -	r = dsicm_panel_reset(ddata);

> -	if (!r)

> -		ddata->ulps_enabled = false;

> -

> -	dsicm_queue_ulps_work(ddata);

> -

> -	return r;

> -}

> -

> -static int dsicm_wake_up(struct panel_drv_data *ddata)

> -{

> -	if (ddata->ulps_enabled)

> -		return dsicm_exit_ulps(ddata);

> -

> -	dsicm_cancel_ulps_work(ddata);

> -	dsicm_queue_ulps_work(ddata);

> -	return 0;

> -}

> -

>  static int dsicm_bl_update_status(struct backlight_device *dev)

>  {

>  	struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev);

> @@ -312,11 +215,8 @@ static int dsicm_bl_update_status(struct backlight_device *dev)

>  	mutex_lock(&ddata->lock);

>  

>  	if (ddata->enabled) {

> -		r = dsicm_wake_up(ddata);

> -		if (!r) {

> -			r = dsicm_dcs_write_1(ddata,

> -				MIPI_DCS_SET_DISPLAY_BRIGHTNESS, level);

> -		}

> +		r = dsicm_dcs_write_1(ddata, MIPI_DCS_SET_DISPLAY_BRIGHTNESS,

> +				      level);

>  	}

>  

>  	mutex_unlock(&ddata->lock);

> @@ -343,18 +243,12 @@ static ssize_t dsicm_num_errors_show(struct device *dev,

>  {

>  	struct panel_drv_data *ddata = dev_get_drvdata(dev);

>  	u8 errors = 0;

> -	int r;

> +	int r = -ENODEV;

>  

>  	mutex_lock(&ddata->lock);

>  

> -	if (ddata->enabled) {

> -		r = dsicm_wake_up(ddata);

> -		if (!r)

> -			r = dsicm_dcs_read_1(ddata, DCS_READ_NUM_ERRORS,

> -					&errors);

> -	} else {

> -		r = -ENODEV;

> -	}

> +	if (ddata->enabled)

> +		r = dsicm_dcs_read_1(ddata, DCS_READ_NUM_ERRORS, &errors);

>  

>  	mutex_unlock(&ddata->lock);

>  

> @@ -369,17 +263,12 @@ static ssize_t dsicm_hw_revision_show(struct device *dev,

>  {

>  	struct panel_drv_data *ddata = dev_get_drvdata(dev);

>  	u8 id1, id2, id3;

> -	int r;

> +	int r = -ENODEV;

>  

>  	mutex_lock(&ddata->lock);

>  

> -	if (ddata->enabled) {

> -		r = dsicm_wake_up(ddata);

> -		if (!r)

> -			r = dsicm_get_id(ddata, &id1, &id2, &id3);

> -	} else {

> -		r = -ENODEV;

> -	}

> +	if (ddata->enabled)

> +		r = dsicm_get_id(ddata, &id1, &id2, &id3);

>  

>  	mutex_unlock(&ddata->lock);

>  

> @@ -389,103 +278,12 @@ static ssize_t dsicm_hw_revision_show(struct device *dev,

>  	return snprintf(buf, PAGE_SIZE, "%02x.%02x.%02x\n", id1, id2, id3);

>  }

>  

> -static ssize_t dsicm_store_ulps(struct device *dev,

> -		struct device_attribute *attr,

> -		const char *buf, size_t count)

> -{

> -	struct panel_drv_data *ddata = dev_get_drvdata(dev);

> -	unsigned long t;

> -	int r;

> -

> -	r = kstrtoul(buf, 0, &t);

> -	if (r)

> -		return r;

> -

> -	mutex_lock(&ddata->lock);

> -

> -	if (ddata->enabled) {

> -		if (t)

> -			r = dsicm_enter_ulps(ddata);

> -		else

> -			r = dsicm_wake_up(ddata);

> -	}

> -

> -	mutex_unlock(&ddata->lock);

> -

> -	if (r)

> -		return r;

> -

> -	return count;

> -}

> -

> -static ssize_t dsicm_show_ulps(struct device *dev,

> -		struct device_attribute *attr,

> -		char *buf)

> -{

> -	struct panel_drv_data *ddata = dev_get_drvdata(dev);

> -	unsigned int t;

> -

> -	mutex_lock(&ddata->lock);

> -	t = ddata->ulps_enabled;

> -	mutex_unlock(&ddata->lock);

> -

> -	return snprintf(buf, PAGE_SIZE, "%u\n", t);

> -}

> -

> -static ssize_t dsicm_store_ulps_timeout(struct device *dev,

> -		struct device_attribute *attr,

> -		const char *buf, size_t count)

> -{

> -	struct panel_drv_data *ddata = dev_get_drvdata(dev);

> -	unsigned long t;

> -	int r;

> -

> -	r = kstrtoul(buf, 0, &t);

> -	if (r)

> -		return r;

> -

> -	mutex_lock(&ddata->lock);

> -	ddata->ulps_timeout = t;

> -

> -	if (ddata->enabled) {

> -		/* dsicm_wake_up will restart the timer */

> -		r = dsicm_wake_up(ddata);

> -	}

> -

> -	mutex_unlock(&ddata->lock);

> -

> -	if (r)

> -		return r;

> -

> -	return count;

> -}

> -

> -static ssize_t dsicm_show_ulps_timeout(struct device *dev,

> -		struct device_attribute *attr,

> -		char *buf)

> -{

> -	struct panel_drv_data *ddata = dev_get_drvdata(dev);

> -	unsigned int t;

> -

> -	mutex_lock(&ddata->lock);

> -	t = ddata->ulps_timeout;

> -	mutex_unlock(&ddata->lock);

> -

> -	return snprintf(buf, PAGE_SIZE, "%u\n", t);

> -}

> -

>  static DEVICE_ATTR(num_dsi_errors, S_IRUGO, dsicm_num_errors_show, NULL);

>  static DEVICE_ATTR(hw_revision, S_IRUGO, dsicm_hw_revision_show, NULL);

> -static DEVICE_ATTR(ulps, S_IRUGO | S_IWUSR,

> -		dsicm_show_ulps, dsicm_store_ulps);

> -static DEVICE_ATTR(ulps_timeout, S_IRUGO | S_IWUSR,

> -		dsicm_show_ulps_timeout, dsicm_store_ulps_timeout);

>  

>  static struct attribute *dsicm_attrs[] = {

>  	&dev_attr_num_dsi_errors.attr,

>  	&dev_attr_hw_revision.attr,

> -	&dev_attr_ulps.attr,

> -	&dev_attr_ulps_timeout.attr,

>  	NULL,

>  };

>  

> @@ -621,15 +419,6 @@ static void dsicm_power_off(struct panel_drv_data *ddata)

>  	ddata->enabled = false;

>  }

>  

> -static int dsicm_panel_reset(struct panel_drv_data *ddata)

> -{

> -	dev_err(&ddata->dsi->dev, "performing LCD reset\n");

> -

> -	dsicm_power_off(ddata);

> -	dsicm_hw_reset(ddata);

> -	return dsicm_power_on(ddata);

> -}

> -

>  static int dsicm_connect(struct omap_dss_device *src,

>  			 struct omap_dss_device *dst)

>  {

> @@ -671,17 +460,12 @@ static void dsicm_enable(struct omap_dss_device *dssdev)

>  static void dsicm_disable(struct omap_dss_device *dssdev)

>  {

>  	struct panel_drv_data *ddata = to_panel_data(dssdev);

> -	int r;

>  

>  	dsicm_bl_power(ddata, false);

>  

>  	mutex_lock(&ddata->lock);

>  

> -	dsicm_cancel_ulps_work(ddata);

> -

> -	r = dsicm_wake_up(ddata);

> -	if (!r)

> -		dsicm_power_off(ddata);

> +	dsicm_power_off(ddata);

>  

>  	mutex_unlock(&ddata->lock);

>  }

> @@ -705,10 +489,6 @@ static int dsicm_update(struct omap_dss_device *dssdev,

>  

>  	mutex_lock(&ddata->lock);

>  

> -	r = dsicm_wake_up(ddata);

> -	if (r)

> -		goto err;

> -

>  	if (!ddata->enabled) {

>  		r = 0;

>  		goto err;

> @@ -748,24 +528,6 @@ static int _dsicm_enable_te(struct panel_drv_data *ddata, bool enable)

>  	return r;

>  }

>  

> -static void dsicm_ulps_work(struct work_struct *work)

> -{

> -	struct panel_drv_data *ddata = container_of(work, struct panel_drv_data,

> -			ulps_work.work);

> -	struct omap_dss_device *dssdev = &ddata->dssdev;

> -

> -	mutex_lock(&ddata->lock);

> -

> -	if (dssdev->state != OMAP_DSS_DISPLAY_ACTIVE || !ddata->enabled) {

> -		mutex_unlock(&ddata->lock);

> -		return;

> -	}

> -

> -	dsicm_enter_ulps(ddata);

> -

> -	mutex_unlock(&ddata->lock);

> -}

> -

>  static int dsicm_get_modes(struct omap_dss_device *dssdev,

>  			   struct drm_connector *connector)

>  {

> @@ -863,7 +625,7 @@ static int dsicm_probe_of(struct mipi_dsi_device *dsi)

>  	else

>  		ddata->use_dsi_backlight = true;

>  

> -	/* TODO: ulps */

> +	/* TODO: ulps_enabled */

>  

>  	return 0;

>  }

> @@ -911,13 +673,6 @@ static int dsicm_probe(struct mipi_dsi_device *dsi)

>  

>  	mutex_init(&ddata->lock);

>  

> -	ddata->workqueue = create_singlethread_workqueue("dsicm_wq");

> -	if (!ddata->workqueue) {

> -		r = -ENOMEM;

> -		goto err_reg;

> -	}

> -	INIT_DELAYED_WORK(&ddata->ulps_work, dsicm_ulps_work);

> -

>  	dsicm_hw_reset(ddata);

>  

>  	if (ddata->use_dsi_backlight) {

> @@ -948,6 +703,9 @@ static int dsicm_probe(struct mipi_dsi_device *dsi)

>  	dsi->hs_rate = 300000000;

>  	dsi->lp_rate = 10000000;

>  

> +	if (ddata->ulps_enabled)

> +		dsi->mode_flags |= MIPI_DSI_MODE_ULPS_IDLE;

> +

>  	r = mipi_dsi_attach(dsi);

>  	if (r < 0)

>  		goto err_dsi_attach;

> @@ -957,8 +715,6 @@ static int dsicm_probe(struct mipi_dsi_device *dsi)

>  err_dsi_attach:

>  	sysfs_remove_group(&dsi->dev.kobj, &dsicm_attr_group);

>  err_bl:

> -	destroy_workqueue(ddata->workqueue);

> -err_reg:

>  	if (ddata->extbldev)

>  		put_device(&ddata->extbldev->dev);

>  

> @@ -985,9 +741,6 @@ static int __exit dsicm_remove(struct mipi_dsi_device *dsi)

>  	if (ddata->extbldev)

>  		put_device(&ddata->extbldev->dev);

>  

> -	dsicm_cancel_ulps_work(ddata);

> -	destroy_workqueue(ddata->workqueue);

> -

>  	/* reset, to be sure that the panel is in a valid state */

>  	dsicm_hw_reset(ddata);

>  

> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c

> index 937362ade4b4..0f264654792d 100644

> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c

> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c

> @@ -207,6 +207,8 @@ struct dsi_reg { u16 module; u16 idx; };

>  typedef void (*omap_dsi_isr_t) (void *arg, u32 mask);

>  struct dsi_data;

>  

> +static void dsi_set_ulps_auto(struct dsi_data *dsi, bool enable);

> +


Would it be possible to avoid forward declarations by reordering
functions ?

This patch removes the ability for userspace to configure the ULPS
timeout (and thus, if I understand correctly, to disable the auto-ULPS
feature). I understand the simplification is needed, but I can't tell if
the feature removal could have an adverse effect.

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


>  static int dsi_display_init_dispc(struct dsi_data *dsi);

>  static void dsi_display_uninit_dispc(struct dsi_data *dsi);

>  

> @@ -377,6 +379,9 @@ struct dsi_data {

>  

>  	bool te_enabled;

>  	bool ulps_enabled;

> +	bool ulps_auto_idle;

> +

> +	struct delayed_work ulps_work;

>  

>  	void (*framedone_callback)(int, void *);

>  	void *framedone_data;

> @@ -3794,6 +3799,7 @@ static void dsi_handle_framedone(struct dsi_data *dsi, int error)

>  		REG_FLD_MOD(dsi, DSI_TIMING2, 1, 15, 15); /* LP_RX_TO */

>  	}

>  

> +	dsi_set_ulps_auto(dsi, true);

>  	dsi_bus_unlock(dsi);

>  

>  	dsi->framedone_callback(error, dsi->framedone_data);

> @@ -3856,6 +3862,7 @@ static int dsi_update(struct omap_dss_device *dssdev, int channel,

>  	struct dsi_data *dsi = to_dsi_data(dssdev);

>  

>  	dsi_bus_lock(dsi);

> +	dsi_set_ulps_auto(dsi, false);

>  

>  	dsi->update_channel = channel;

>  	dsi->framedone_callback = callback;

> @@ -4119,18 +4126,6 @@ static void dsi_display_disable(struct omap_dss_device *dssdev)

>  	dsi_display_ulps_disable(dsi, true, false);

>  }

>  

> -static void dsi_ulps(struct omap_dss_device *dssdev, bool enable)

> -{

> -	struct dsi_data *dsi = to_dsi_data(dssdev);

> -

> -	DSSDBG("dsi_ulps\n");

> -

> -	if (enable)

> -		dsi_display_ulps_disable(dsi, false, true);

> -	else

> -		dsi_display_ulps_enable(dsi);

> -}

> -

>  static int dsi_enable_te(struct dsi_data *dsi, bool enable)

>  {

>  	dsi->te_enabled = enable;

> @@ -4145,6 +4140,40 @@ static int dsi_enable_te(struct dsi_data *dsi, bool enable)

>  	return 0;

>  }

>  

> +static void omap_dsi_ulps_work_callback(struct work_struct *work)

> +{

> +	struct dsi_data *dsi = container_of(work, struct dsi_data,

> +					    ulps_work.work);

> +

> +	dsi_bus_lock(dsi);

> +

> +	dsi_enable_te(dsi, false);

> +

> +	dsi_display_ulps_disable(dsi, false, true);

> +

> +	dsi_bus_unlock(dsi);

> +}

> +

> +static void dsi_set_ulps_auto(struct dsi_data *dsi, bool enable)

> +{

> +	WARN_ON(!dsi_bus_is_locked(dsi));

> +

> +	if (!dsi->ulps_auto_idle)

> +		return;

> +

> +	if (enable) {

> +		schedule_delayed_work(&dsi->ulps_work, msecs_to_jiffies(250));

> +	} else {

> +		cancel_delayed_work_sync(&dsi->ulps_work);

> +

> +		if (!dsi->ulps_enabled)

> +			return;

> +

> +		dsi_display_ulps_enable(dsi);

> +		dsi_enable_te(dsi, true);

> +	}

> +}

> +

>  #ifdef PRINT_VERBOSE_VM_TIMINGS

>  static void print_dsi_vm(const char *str,

>  		const struct omap_dss_dsi_videomode_timings *t)

> @@ -4796,7 +4825,9 @@ static ssize_t omap_dsi_host_transfer(struct mipi_dsi_host *host,

>  	int r;

>  

>  	dsi_bus_lock(dsi);

> +	dsi_set_ulps_auto(dsi, false);

>  	r = _omap_dsi_host_transfer(dsi, msg);

> +	dsi_set_ulps_auto(dsi, true);

>  	dsi_bus_unlock(dsi);

>  

>  	return r;

> @@ -4836,8 +4867,6 @@ static const struct omap_dss_device_ops dsi_ops = {

>  	.disable = dsi_display_disable,

>  

>  	.dsi = {

> -		.ulps = dsi_ulps,

> -

>  		.set_config = dsi_set_config,

>  

>  		.enable_video_output = dsi_enable_video_output,

> @@ -4959,6 +4988,12 @@ static int omap_dsi_host_attach(struct mipi_dsi_host *host,

>  	dsi->vc[channel].dest = client;

>  	dsi->pix_fmt = client->format;

>  

> +	INIT_DEFERRABLE_WORK(&dsi->ulps_work,

> +			     omap_dsi_ulps_work_callback);

> +

> +	dsi->ulps_auto_idle = !!(client->mode_flags & MIPI_DSI_MODE_ULPS_IDLE);

> +	dsi_set_ulps_auto(dsi, true);

> +

>  	return 0;

>  }

>  

> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h

> index 0d82ba34ca89..524cb87cd9ea 100644

> --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h

> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h

> @@ -282,8 +282,6 @@ struct omap_dss_writeback_info {

>  

>  struct omapdss_dsi_ops {

>  	/* bus configuration */

> -	void (*ulps)(struct omap_dss_device *dssdev, bool enable);

> -

>  	int (*set_config)(struct omap_dss_device *dssdev,

>  			const struct omap_dss_dsi_config *cfg);

>  


-- 
Regards,

Laurent Pinchart
Tomi Valkeinen Nov. 11, 2020, 3:29 p.m. UTC | #2
On 09/11/2020 12:03, Laurent Pinchart wrote:
> Hi Tomi and Sebastian,

> 

> Thank you for the patch.

> 

> On Thu, Nov 05, 2020 at 02:03:06PM +0200, Tomi Valkeinen wrote:

>> From: Sebastian Reichel <sebastian.reichel@collabora.com>

>>

>> Move ULPS handling into the DSI host controller, so that we

>> no longer need a custom API for the DSI client.

>>

>> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

>> ---

>>  .../gpu/drm/omapdrm/displays/panel-dsi-cm.c   | 273 +-----------------

>>  drivers/gpu/drm/omapdrm/dss/dsi.c             |  63 +++-

>>  drivers/gpu/drm/omapdrm/dss/omapdss.h         |   2 -

>>  3 files changed, 62 insertions(+), 276 deletions(-)

>>

>> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c

>> index 78247dcb1848..030a8fa140db 100644

>> --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c

>> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c

>> @@ -16,7 +16,6 @@

>>  #include <linux/module.h>

>>  #include <linux/sched/signal.h>

>>  #include <linux/slab.h>

>> -#include <linux/workqueue.h>

>>  #include <linux/of_device.h>

>>  #include <linux/regulator/consumer.h>

>>  

>> @@ -69,21 +68,13 @@ struct panel_drv_data {

>>  

>>  	bool intro_printed;

>>  

>> -	struct workqueue_struct *workqueue;

>> -

>>  	bool ulps_enabled;

>> -	unsigned int ulps_timeout;

>> -	struct delayed_work ulps_work;

>>  };

>>  

>>  #define to_panel_data(p) container_of(p, struct panel_drv_data, dssdev)

>>  

>>  static int _dsicm_enable_te(struct panel_drv_data *ddata, bool enable);

>>  

>> -static int dsicm_panel_reset(struct panel_drv_data *ddata);

>> -

>> -static void dsicm_ulps_work(struct work_struct *work);

>> -

>>  static void dsicm_bl_power(struct panel_drv_data *ddata, bool enable)

>>  {

>>  	struct backlight_device *backlight;

>> @@ -207,94 +198,6 @@ static int dsicm_set_update_window(struct panel_drv_data *ddata,

>>  	return 0;

>>  }

>>  

>> -static void dsicm_queue_ulps_work(struct panel_drv_data *ddata)

>> -{

>> -	if (ddata->ulps_timeout > 0)

>> -		queue_delayed_work(ddata->workqueue, &ddata->ulps_work,

>> -				msecs_to_jiffies(ddata->ulps_timeout));

>> -}

>> -

>> -static void dsicm_cancel_ulps_work(struct panel_drv_data *ddata)

>> -{

>> -	cancel_delayed_work(&ddata->ulps_work);

>> -}

>> -

>> -static int dsicm_enter_ulps(struct panel_drv_data *ddata)

>> -{

>> -	struct omap_dss_device *src = ddata->src;

>> -	int r;

>> -

>> -	if (ddata->ulps_enabled)

>> -		return 0;

>> -

>> -	dsicm_cancel_ulps_work(ddata);

>> -

>> -	r = _dsicm_enable_te(ddata, false);

>> -	if (r)

>> -		goto err;

>> -

>> -	src->ops->dsi.ulps(src, true);

>> -

>> -	ddata->ulps_enabled = true;

>> -

>> -	return 0;

>> -

>> -err:

>> -	dev_err(&ddata->dsi->dev, "enter ULPS failed");

>> -	dsicm_panel_reset(ddata);

>> -

>> -	ddata->ulps_enabled = false;

>> -

>> -	dsicm_queue_ulps_work(ddata);

>> -

>> -	return r;

>> -}

>> -

>> -static int dsicm_exit_ulps(struct panel_drv_data *ddata)

>> -{

>> -	struct omap_dss_device *src = ddata->src;

>> -	int r;

>> -

>> -	if (!ddata->ulps_enabled)

>> -		return 0;

>> -

>> -	src->ops->dsi.ulps(src, false);

>> -	ddata->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;

>> -

>> -	r = _dsicm_enable_te(ddata, ddata->te_enabled);

>> -	if (r) {

>> -		dev_err(&ddata->dsi->dev, "failed to re-enable TE");

>> -		goto err2;

>> -	}

>> -

>> -	dsicm_queue_ulps_work(ddata);

>> -

>> -	ddata->ulps_enabled = false;

>> -

>> -	return 0;

>> -

>> -err2:

>> -	dev_err(&ddata->dsi->dev, "failed to exit ULPS");

>> -

>> -	r = dsicm_panel_reset(ddata);

>> -	if (!r)

>> -		ddata->ulps_enabled = false;

>> -

>> -	dsicm_queue_ulps_work(ddata);

>> -

>> -	return r;

>> -}

>> -

>> -static int dsicm_wake_up(struct panel_drv_data *ddata)

>> -{

>> -	if (ddata->ulps_enabled)

>> -		return dsicm_exit_ulps(ddata);

>> -

>> -	dsicm_cancel_ulps_work(ddata);

>> -	dsicm_queue_ulps_work(ddata);

>> -	return 0;

>> -}

>> -

>>  static int dsicm_bl_update_status(struct backlight_device *dev)

>>  {

>>  	struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev);

>> @@ -312,11 +215,8 @@ static int dsicm_bl_update_status(struct backlight_device *dev)

>>  	mutex_lock(&ddata->lock);

>>  

>>  	if (ddata->enabled) {

>> -		r = dsicm_wake_up(ddata);

>> -		if (!r) {

>> -			r = dsicm_dcs_write_1(ddata,

>> -				MIPI_DCS_SET_DISPLAY_BRIGHTNESS, level);

>> -		}

>> +		r = dsicm_dcs_write_1(ddata, MIPI_DCS_SET_DISPLAY_BRIGHTNESS,

>> +				      level);

>>  	}

>>  

>>  	mutex_unlock(&ddata->lock);

>> @@ -343,18 +243,12 @@ static ssize_t dsicm_num_errors_show(struct device *dev,

>>  {

>>  	struct panel_drv_data *ddata = dev_get_drvdata(dev);

>>  	u8 errors = 0;

>> -	int r;

>> +	int r = -ENODEV;

>>  

>>  	mutex_lock(&ddata->lock);

>>  

>> -	if (ddata->enabled) {

>> -		r = dsicm_wake_up(ddata);

>> -		if (!r)

>> -			r = dsicm_dcs_read_1(ddata, DCS_READ_NUM_ERRORS,

>> -					&errors);

>> -	} else {

>> -		r = -ENODEV;

>> -	}

>> +	if (ddata->enabled)

>> +		r = dsicm_dcs_read_1(ddata, DCS_READ_NUM_ERRORS, &errors);

>>  

>>  	mutex_unlock(&ddata->lock);

>>  

>> @@ -369,17 +263,12 @@ static ssize_t dsicm_hw_revision_show(struct device *dev,

>>  {

>>  	struct panel_drv_data *ddata = dev_get_drvdata(dev);

>>  	u8 id1, id2, id3;

>> -	int r;

>> +	int r = -ENODEV;

>>  

>>  	mutex_lock(&ddata->lock);

>>  

>> -	if (ddata->enabled) {

>> -		r = dsicm_wake_up(ddata);

>> -		if (!r)

>> -			r = dsicm_get_id(ddata, &id1, &id2, &id3);

>> -	} else {

>> -		r = -ENODEV;

>> -	}

>> +	if (ddata->enabled)

>> +		r = dsicm_get_id(ddata, &id1, &id2, &id3);

>>  

>>  	mutex_unlock(&ddata->lock);

>>  

>> @@ -389,103 +278,12 @@ static ssize_t dsicm_hw_revision_show(struct device *dev,

>>  	return snprintf(buf, PAGE_SIZE, "%02x.%02x.%02x\n", id1, id2, id3);

>>  }

>>  

>> -static ssize_t dsicm_store_ulps(struct device *dev,

>> -		struct device_attribute *attr,

>> -		const char *buf, size_t count)

>> -{

>> -	struct panel_drv_data *ddata = dev_get_drvdata(dev);

>> -	unsigned long t;

>> -	int r;

>> -

>> -	r = kstrtoul(buf, 0, &t);

>> -	if (r)

>> -		return r;

>> -

>> -	mutex_lock(&ddata->lock);

>> -

>> -	if (ddata->enabled) {

>> -		if (t)

>> -			r = dsicm_enter_ulps(ddata);

>> -		else

>> -			r = dsicm_wake_up(ddata);

>> -	}

>> -

>> -	mutex_unlock(&ddata->lock);

>> -

>> -	if (r)

>> -		return r;

>> -

>> -	return count;

>> -}

>> -

>> -static ssize_t dsicm_show_ulps(struct device *dev,

>> -		struct device_attribute *attr,

>> -		char *buf)

>> -{

>> -	struct panel_drv_data *ddata = dev_get_drvdata(dev);

>> -	unsigned int t;

>> -

>> -	mutex_lock(&ddata->lock);

>> -	t = ddata->ulps_enabled;

>> -	mutex_unlock(&ddata->lock);

>> -

>> -	return snprintf(buf, PAGE_SIZE, "%u\n", t);

>> -}

>> -

>> -static ssize_t dsicm_store_ulps_timeout(struct device *dev,

>> -		struct device_attribute *attr,

>> -		const char *buf, size_t count)

>> -{

>> -	struct panel_drv_data *ddata = dev_get_drvdata(dev);

>> -	unsigned long t;

>> -	int r;

>> -

>> -	r = kstrtoul(buf, 0, &t);

>> -	if (r)

>> -		return r;

>> -

>> -	mutex_lock(&ddata->lock);

>> -	ddata->ulps_timeout = t;

>> -

>> -	if (ddata->enabled) {

>> -		/* dsicm_wake_up will restart the timer */

>> -		r = dsicm_wake_up(ddata);

>> -	}

>> -

>> -	mutex_unlock(&ddata->lock);

>> -

>> -	if (r)

>> -		return r;

>> -

>> -	return count;

>> -}

>> -

>> -static ssize_t dsicm_show_ulps_timeout(struct device *dev,

>> -		struct device_attribute *attr,

>> -		char *buf)

>> -{

>> -	struct panel_drv_data *ddata = dev_get_drvdata(dev);

>> -	unsigned int t;

>> -

>> -	mutex_lock(&ddata->lock);

>> -	t = ddata->ulps_timeout;

>> -	mutex_unlock(&ddata->lock);

>> -

>> -	return snprintf(buf, PAGE_SIZE, "%u\n", t);

>> -}

>> -

>>  static DEVICE_ATTR(num_dsi_errors, S_IRUGO, dsicm_num_errors_show, NULL);

>>  static DEVICE_ATTR(hw_revision, S_IRUGO, dsicm_hw_revision_show, NULL);

>> -static DEVICE_ATTR(ulps, S_IRUGO | S_IWUSR,

>> -		dsicm_show_ulps, dsicm_store_ulps);

>> -static DEVICE_ATTR(ulps_timeout, S_IRUGO | S_IWUSR,

>> -		dsicm_show_ulps_timeout, dsicm_store_ulps_timeout);

>>  

>>  static struct attribute *dsicm_attrs[] = {

>>  	&dev_attr_num_dsi_errors.attr,

>>  	&dev_attr_hw_revision.attr,

>> -	&dev_attr_ulps.attr,

>> -	&dev_attr_ulps_timeout.attr,

>>  	NULL,

>>  };

>>  

>> @@ -621,15 +419,6 @@ static void dsicm_power_off(struct panel_drv_data *ddata)

>>  	ddata->enabled = false;

>>  }

>>  

>> -static int dsicm_panel_reset(struct panel_drv_data *ddata)

>> -{

>> -	dev_err(&ddata->dsi->dev, "performing LCD reset\n");

>> -

>> -	dsicm_power_off(ddata);

>> -	dsicm_hw_reset(ddata);

>> -	return dsicm_power_on(ddata);

>> -}

>> -

>>  static int dsicm_connect(struct omap_dss_device *src,

>>  			 struct omap_dss_device *dst)

>>  {

>> @@ -671,17 +460,12 @@ static void dsicm_enable(struct omap_dss_device *dssdev)

>>  static void dsicm_disable(struct omap_dss_device *dssdev)

>>  {

>>  	struct panel_drv_data *ddata = to_panel_data(dssdev);

>> -	int r;

>>  

>>  	dsicm_bl_power(ddata, false);

>>  

>>  	mutex_lock(&ddata->lock);

>>  

>> -	dsicm_cancel_ulps_work(ddata);

>> -

>> -	r = dsicm_wake_up(ddata);

>> -	if (!r)

>> -		dsicm_power_off(ddata);

>> +	dsicm_power_off(ddata);

>>  

>>  	mutex_unlock(&ddata->lock);

>>  }

>> @@ -705,10 +489,6 @@ static int dsicm_update(struct omap_dss_device *dssdev,

>>  

>>  	mutex_lock(&ddata->lock);

>>  

>> -	r = dsicm_wake_up(ddata);

>> -	if (r)

>> -		goto err;

>> -

>>  	if (!ddata->enabled) {

>>  		r = 0;

>>  		goto err;

>> @@ -748,24 +528,6 @@ static int _dsicm_enable_te(struct panel_drv_data *ddata, bool enable)

>>  	return r;

>>  }

>>  

>> -static void dsicm_ulps_work(struct work_struct *work)

>> -{

>> -	struct panel_drv_data *ddata = container_of(work, struct panel_drv_data,

>> -			ulps_work.work);

>> -	struct omap_dss_device *dssdev = &ddata->dssdev;

>> -

>> -	mutex_lock(&ddata->lock);

>> -

>> -	if (dssdev->state != OMAP_DSS_DISPLAY_ACTIVE || !ddata->enabled) {

>> -		mutex_unlock(&ddata->lock);

>> -		return;

>> -	}

>> -

>> -	dsicm_enter_ulps(ddata);

>> -

>> -	mutex_unlock(&ddata->lock);

>> -}

>> -

>>  static int dsicm_get_modes(struct omap_dss_device *dssdev,

>>  			   struct drm_connector *connector)

>>  {

>> @@ -863,7 +625,7 @@ static int dsicm_probe_of(struct mipi_dsi_device *dsi)

>>  	else

>>  		ddata->use_dsi_backlight = true;

>>  

>> -	/* TODO: ulps */

>> +	/* TODO: ulps_enabled */

>>  

>>  	return 0;

>>  }

>> @@ -911,13 +673,6 @@ static int dsicm_probe(struct mipi_dsi_device *dsi)

>>  

>>  	mutex_init(&ddata->lock);

>>  

>> -	ddata->workqueue = create_singlethread_workqueue("dsicm_wq");

>> -	if (!ddata->workqueue) {

>> -		r = -ENOMEM;

>> -		goto err_reg;

>> -	}

>> -	INIT_DELAYED_WORK(&ddata->ulps_work, dsicm_ulps_work);

>> -

>>  	dsicm_hw_reset(ddata);

>>  

>>  	if (ddata->use_dsi_backlight) {

>> @@ -948,6 +703,9 @@ static int dsicm_probe(struct mipi_dsi_device *dsi)

>>  	dsi->hs_rate = 300000000;

>>  	dsi->lp_rate = 10000000;

>>  

>> +	if (ddata->ulps_enabled)

>> +		dsi->mode_flags |= MIPI_DSI_MODE_ULPS_IDLE;

>> +

>>  	r = mipi_dsi_attach(dsi);

>>  	if (r < 0)

>>  		goto err_dsi_attach;

>> @@ -957,8 +715,6 @@ static int dsicm_probe(struct mipi_dsi_device *dsi)

>>  err_dsi_attach:

>>  	sysfs_remove_group(&dsi->dev.kobj, &dsicm_attr_group);

>>  err_bl:

>> -	destroy_workqueue(ddata->workqueue);

>> -err_reg:

>>  	if (ddata->extbldev)

>>  		put_device(&ddata->extbldev->dev);

>>  

>> @@ -985,9 +741,6 @@ static int __exit dsicm_remove(struct mipi_dsi_device *dsi)

>>  	if (ddata->extbldev)

>>  		put_device(&ddata->extbldev->dev);

>>  

>> -	dsicm_cancel_ulps_work(ddata);

>> -	destroy_workqueue(ddata->workqueue);

>> -

>>  	/* reset, to be sure that the panel is in a valid state */

>>  	dsicm_hw_reset(ddata);

>>  

>> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c

>> index 937362ade4b4..0f264654792d 100644

>> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c

>> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c

>> @@ -207,6 +207,8 @@ struct dsi_reg { u16 module; u16 idx; };

>>  typedef void (*omap_dsi_isr_t) (void *arg, u32 mask);

>>  struct dsi_data;

>>  

>> +static void dsi_set_ulps_auto(struct dsi_data *dsi, bool enable);

>> +

> 

> Would it be possible to avoid forward declarations by reordering

> functions ?


Maybe, but doing it here leads to lots of conflicts...

> This patch removes the ability for userspace to configure the ULPS

> timeout (and thus, if I understand correctly, to disable the auto-ULPS

> feature). I understand the simplification is needed, but I can't tell if

> the feature removal could have an adverse effect.


Using ULPS may reduce power consumption in some cases, but to be honest, I'm not sure if it's worth
it or not, and I find it unlikely that anyone has used or dependend on it. I think it's something
that people can finetune afterwards if they really need it.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
diff mbox series

Patch

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
index 78247dcb1848..030a8fa140db 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
@@ -16,7 +16,6 @@ 
 #include <linux/module.h>
 #include <linux/sched/signal.h>
 #include <linux/slab.h>
-#include <linux/workqueue.h>
 #include <linux/of_device.h>
 #include <linux/regulator/consumer.h>
 
@@ -69,21 +68,13 @@  struct panel_drv_data {
 
 	bool intro_printed;
 
-	struct workqueue_struct *workqueue;
-
 	bool ulps_enabled;
-	unsigned int ulps_timeout;
-	struct delayed_work ulps_work;
 };
 
 #define to_panel_data(p) container_of(p, struct panel_drv_data, dssdev)
 
 static int _dsicm_enable_te(struct panel_drv_data *ddata, bool enable);
 
-static int dsicm_panel_reset(struct panel_drv_data *ddata);
-
-static void dsicm_ulps_work(struct work_struct *work);
-
 static void dsicm_bl_power(struct panel_drv_data *ddata, bool enable)
 {
 	struct backlight_device *backlight;
@@ -207,94 +198,6 @@  static int dsicm_set_update_window(struct panel_drv_data *ddata,
 	return 0;
 }
 
-static void dsicm_queue_ulps_work(struct panel_drv_data *ddata)
-{
-	if (ddata->ulps_timeout > 0)
-		queue_delayed_work(ddata->workqueue, &ddata->ulps_work,
-				msecs_to_jiffies(ddata->ulps_timeout));
-}
-
-static void dsicm_cancel_ulps_work(struct panel_drv_data *ddata)
-{
-	cancel_delayed_work(&ddata->ulps_work);
-}
-
-static int dsicm_enter_ulps(struct panel_drv_data *ddata)
-{
-	struct omap_dss_device *src = ddata->src;
-	int r;
-
-	if (ddata->ulps_enabled)
-		return 0;
-
-	dsicm_cancel_ulps_work(ddata);
-
-	r = _dsicm_enable_te(ddata, false);
-	if (r)
-		goto err;
-
-	src->ops->dsi.ulps(src, true);
-
-	ddata->ulps_enabled = true;
-
-	return 0;
-
-err:
-	dev_err(&ddata->dsi->dev, "enter ULPS failed");
-	dsicm_panel_reset(ddata);
-
-	ddata->ulps_enabled = false;
-
-	dsicm_queue_ulps_work(ddata);
-
-	return r;
-}
-
-static int dsicm_exit_ulps(struct panel_drv_data *ddata)
-{
-	struct omap_dss_device *src = ddata->src;
-	int r;
-
-	if (!ddata->ulps_enabled)
-		return 0;
-
-	src->ops->dsi.ulps(src, false);
-	ddata->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
-
-	r = _dsicm_enable_te(ddata, ddata->te_enabled);
-	if (r) {
-		dev_err(&ddata->dsi->dev, "failed to re-enable TE");
-		goto err2;
-	}
-
-	dsicm_queue_ulps_work(ddata);
-
-	ddata->ulps_enabled = false;
-
-	return 0;
-
-err2:
-	dev_err(&ddata->dsi->dev, "failed to exit ULPS");
-
-	r = dsicm_panel_reset(ddata);
-	if (!r)
-		ddata->ulps_enabled = false;
-
-	dsicm_queue_ulps_work(ddata);
-
-	return r;
-}
-
-static int dsicm_wake_up(struct panel_drv_data *ddata)
-{
-	if (ddata->ulps_enabled)
-		return dsicm_exit_ulps(ddata);
-
-	dsicm_cancel_ulps_work(ddata);
-	dsicm_queue_ulps_work(ddata);
-	return 0;
-}
-
 static int dsicm_bl_update_status(struct backlight_device *dev)
 {
 	struct panel_drv_data *ddata = dev_get_drvdata(&dev->dev);
@@ -312,11 +215,8 @@  static int dsicm_bl_update_status(struct backlight_device *dev)
 	mutex_lock(&ddata->lock);
 
 	if (ddata->enabled) {
-		r = dsicm_wake_up(ddata);
-		if (!r) {
-			r = dsicm_dcs_write_1(ddata,
-				MIPI_DCS_SET_DISPLAY_BRIGHTNESS, level);
-		}
+		r = dsicm_dcs_write_1(ddata, MIPI_DCS_SET_DISPLAY_BRIGHTNESS,
+				      level);
 	}
 
 	mutex_unlock(&ddata->lock);
@@ -343,18 +243,12 @@  static ssize_t dsicm_num_errors_show(struct device *dev,
 {
 	struct panel_drv_data *ddata = dev_get_drvdata(dev);
 	u8 errors = 0;
-	int r;
+	int r = -ENODEV;
 
 	mutex_lock(&ddata->lock);
 
-	if (ddata->enabled) {
-		r = dsicm_wake_up(ddata);
-		if (!r)
-			r = dsicm_dcs_read_1(ddata, DCS_READ_NUM_ERRORS,
-					&errors);
-	} else {
-		r = -ENODEV;
-	}
+	if (ddata->enabled)
+		r = dsicm_dcs_read_1(ddata, DCS_READ_NUM_ERRORS, &errors);
 
 	mutex_unlock(&ddata->lock);
 
@@ -369,17 +263,12 @@  static ssize_t dsicm_hw_revision_show(struct device *dev,
 {
 	struct panel_drv_data *ddata = dev_get_drvdata(dev);
 	u8 id1, id2, id3;
-	int r;
+	int r = -ENODEV;
 
 	mutex_lock(&ddata->lock);
 
-	if (ddata->enabled) {
-		r = dsicm_wake_up(ddata);
-		if (!r)
-			r = dsicm_get_id(ddata, &id1, &id2, &id3);
-	} else {
-		r = -ENODEV;
-	}
+	if (ddata->enabled)
+		r = dsicm_get_id(ddata, &id1, &id2, &id3);
 
 	mutex_unlock(&ddata->lock);
 
@@ -389,103 +278,12 @@  static ssize_t dsicm_hw_revision_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%02x.%02x.%02x\n", id1, id2, id3);
 }
 
-static ssize_t dsicm_store_ulps(struct device *dev,
-		struct device_attribute *attr,
-		const char *buf, size_t count)
-{
-	struct panel_drv_data *ddata = dev_get_drvdata(dev);
-	unsigned long t;
-	int r;
-
-	r = kstrtoul(buf, 0, &t);
-	if (r)
-		return r;
-
-	mutex_lock(&ddata->lock);
-
-	if (ddata->enabled) {
-		if (t)
-			r = dsicm_enter_ulps(ddata);
-		else
-			r = dsicm_wake_up(ddata);
-	}
-
-	mutex_unlock(&ddata->lock);
-
-	if (r)
-		return r;
-
-	return count;
-}
-
-static ssize_t dsicm_show_ulps(struct device *dev,
-		struct device_attribute *attr,
-		char *buf)
-{
-	struct panel_drv_data *ddata = dev_get_drvdata(dev);
-	unsigned int t;
-
-	mutex_lock(&ddata->lock);
-	t = ddata->ulps_enabled;
-	mutex_unlock(&ddata->lock);
-
-	return snprintf(buf, PAGE_SIZE, "%u\n", t);
-}
-
-static ssize_t dsicm_store_ulps_timeout(struct device *dev,
-		struct device_attribute *attr,
-		const char *buf, size_t count)
-{
-	struct panel_drv_data *ddata = dev_get_drvdata(dev);
-	unsigned long t;
-	int r;
-
-	r = kstrtoul(buf, 0, &t);
-	if (r)
-		return r;
-
-	mutex_lock(&ddata->lock);
-	ddata->ulps_timeout = t;
-
-	if (ddata->enabled) {
-		/* dsicm_wake_up will restart the timer */
-		r = dsicm_wake_up(ddata);
-	}
-
-	mutex_unlock(&ddata->lock);
-
-	if (r)
-		return r;
-
-	return count;
-}
-
-static ssize_t dsicm_show_ulps_timeout(struct device *dev,
-		struct device_attribute *attr,
-		char *buf)
-{
-	struct panel_drv_data *ddata = dev_get_drvdata(dev);
-	unsigned int t;
-
-	mutex_lock(&ddata->lock);
-	t = ddata->ulps_timeout;
-	mutex_unlock(&ddata->lock);
-
-	return snprintf(buf, PAGE_SIZE, "%u\n", t);
-}
-
 static DEVICE_ATTR(num_dsi_errors, S_IRUGO, dsicm_num_errors_show, NULL);
 static DEVICE_ATTR(hw_revision, S_IRUGO, dsicm_hw_revision_show, NULL);
-static DEVICE_ATTR(ulps, S_IRUGO | S_IWUSR,
-		dsicm_show_ulps, dsicm_store_ulps);
-static DEVICE_ATTR(ulps_timeout, S_IRUGO | S_IWUSR,
-		dsicm_show_ulps_timeout, dsicm_store_ulps_timeout);
 
 static struct attribute *dsicm_attrs[] = {
 	&dev_attr_num_dsi_errors.attr,
 	&dev_attr_hw_revision.attr,
-	&dev_attr_ulps.attr,
-	&dev_attr_ulps_timeout.attr,
 	NULL,
 };
 
@@ -621,15 +419,6 @@  static void dsicm_power_off(struct panel_drv_data *ddata)
 	ddata->enabled = false;
 }
 
-static int dsicm_panel_reset(struct panel_drv_data *ddata)
-{
-	dev_err(&ddata->dsi->dev, "performing LCD reset\n");
-
-	dsicm_power_off(ddata);
-	dsicm_hw_reset(ddata);
-	return dsicm_power_on(ddata);
-}
-
 static int dsicm_connect(struct omap_dss_device *src,
 			 struct omap_dss_device *dst)
 {
@@ -671,17 +460,12 @@  static void dsicm_enable(struct omap_dss_device *dssdev)
 static void dsicm_disable(struct omap_dss_device *dssdev)
 {
 	struct panel_drv_data *ddata = to_panel_data(dssdev);
-	int r;
 
 	dsicm_bl_power(ddata, false);
 
 	mutex_lock(&ddata->lock);
 
-	dsicm_cancel_ulps_work(ddata);
-
-	r = dsicm_wake_up(ddata);
-	if (!r)
-		dsicm_power_off(ddata);
+	dsicm_power_off(ddata);
 
 	mutex_unlock(&ddata->lock);
 }
@@ -705,10 +489,6 @@  static int dsicm_update(struct omap_dss_device *dssdev,
 
 	mutex_lock(&ddata->lock);
 
-	r = dsicm_wake_up(ddata);
-	if (r)
-		goto err;
-
 	if (!ddata->enabled) {
 		r = 0;
 		goto err;
@@ -748,24 +528,6 @@  static int _dsicm_enable_te(struct panel_drv_data *ddata, bool enable)
 	return r;
 }
 
-static void dsicm_ulps_work(struct work_struct *work)
-{
-	struct panel_drv_data *ddata = container_of(work, struct panel_drv_data,
-			ulps_work.work);
-	struct omap_dss_device *dssdev = &ddata->dssdev;
-
-	mutex_lock(&ddata->lock);
-
-	if (dssdev->state != OMAP_DSS_DISPLAY_ACTIVE || !ddata->enabled) {
-		mutex_unlock(&ddata->lock);
-		return;
-	}
-
-	dsicm_enter_ulps(ddata);
-
-	mutex_unlock(&ddata->lock);
-}
-
 static int dsicm_get_modes(struct omap_dss_device *dssdev,
 			   struct drm_connector *connector)
 {
@@ -863,7 +625,7 @@  static int dsicm_probe_of(struct mipi_dsi_device *dsi)
 	else
 		ddata->use_dsi_backlight = true;
 
-	/* TODO: ulps */
+	/* TODO: ulps_enabled */
 
 	return 0;
 }
@@ -911,13 +673,6 @@  static int dsicm_probe(struct mipi_dsi_device *dsi)
 
 	mutex_init(&ddata->lock);
 
-	ddata->workqueue = create_singlethread_workqueue("dsicm_wq");
-	if (!ddata->workqueue) {
-		r = -ENOMEM;
-		goto err_reg;
-	}
-	INIT_DELAYED_WORK(&ddata->ulps_work, dsicm_ulps_work);
-
 	dsicm_hw_reset(ddata);
 
 	if (ddata->use_dsi_backlight) {
@@ -948,6 +703,9 @@  static int dsicm_probe(struct mipi_dsi_device *dsi)
 	dsi->hs_rate = 300000000;
 	dsi->lp_rate = 10000000;
 
+	if (ddata->ulps_enabled)
+		dsi->mode_flags |= MIPI_DSI_MODE_ULPS_IDLE;
+
 	r = mipi_dsi_attach(dsi);
 	if (r < 0)
 		goto err_dsi_attach;
@@ -957,8 +715,6 @@  static int dsicm_probe(struct mipi_dsi_device *dsi)
 err_dsi_attach:
 	sysfs_remove_group(&dsi->dev.kobj, &dsicm_attr_group);
 err_bl:
-	destroy_workqueue(ddata->workqueue);
-err_reg:
 	if (ddata->extbldev)
 		put_device(&ddata->extbldev->dev);
 
@@ -985,9 +741,6 @@  static int __exit dsicm_remove(struct mipi_dsi_device *dsi)
 	if (ddata->extbldev)
 		put_device(&ddata->extbldev->dev);
 
-	dsicm_cancel_ulps_work(ddata);
-	destroy_workqueue(ddata->workqueue);
-
 	/* reset, to be sure that the panel is in a valid state */
 	dsicm_hw_reset(ddata);
 
diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
index 937362ade4b4..0f264654792d 100644
--- a/drivers/gpu/drm/omapdrm/dss/dsi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
@@ -207,6 +207,8 @@  struct dsi_reg { u16 module; u16 idx; };
 typedef void (*omap_dsi_isr_t) (void *arg, u32 mask);
 struct dsi_data;
 
+static void dsi_set_ulps_auto(struct dsi_data *dsi, bool enable);
+
 static int dsi_display_init_dispc(struct dsi_data *dsi);
 static void dsi_display_uninit_dispc(struct dsi_data *dsi);
 
@@ -377,6 +379,9 @@  struct dsi_data {
 
 	bool te_enabled;
 	bool ulps_enabled;
+	bool ulps_auto_idle;
+
+	struct delayed_work ulps_work;
 
 	void (*framedone_callback)(int, void *);
 	void *framedone_data;
@@ -3794,6 +3799,7 @@  static void dsi_handle_framedone(struct dsi_data *dsi, int error)
 		REG_FLD_MOD(dsi, DSI_TIMING2, 1, 15, 15); /* LP_RX_TO */
 	}
 
+	dsi_set_ulps_auto(dsi, true);
 	dsi_bus_unlock(dsi);
 
 	dsi->framedone_callback(error, dsi->framedone_data);
@@ -3856,6 +3862,7 @@  static int dsi_update(struct omap_dss_device *dssdev, int channel,
 	struct dsi_data *dsi = to_dsi_data(dssdev);
 
 	dsi_bus_lock(dsi);
+	dsi_set_ulps_auto(dsi, false);
 
 	dsi->update_channel = channel;
 	dsi->framedone_callback = callback;
@@ -4119,18 +4126,6 @@  static void dsi_display_disable(struct omap_dss_device *dssdev)
 	dsi_display_ulps_disable(dsi, true, false);
 }
 
-static void dsi_ulps(struct omap_dss_device *dssdev, bool enable)
-{
-	struct dsi_data *dsi = to_dsi_data(dssdev);
-
-	DSSDBG("dsi_ulps\n");
-
-	if (enable)
-		dsi_display_ulps_disable(dsi, false, true);
-	else
-		dsi_display_ulps_enable(dsi);
-}
-
 static int dsi_enable_te(struct dsi_data *dsi, bool enable)
 {
 	dsi->te_enabled = enable;
@@ -4145,6 +4140,40 @@  static int dsi_enable_te(struct dsi_data *dsi, bool enable)
 	return 0;
 }
 
+static void omap_dsi_ulps_work_callback(struct work_struct *work)
+{
+	struct dsi_data *dsi = container_of(work, struct dsi_data,
+					    ulps_work.work);
+
+	dsi_bus_lock(dsi);
+
+	dsi_enable_te(dsi, false);
+
+	dsi_display_ulps_disable(dsi, false, true);
+
+	dsi_bus_unlock(dsi);
+}
+
+static void dsi_set_ulps_auto(struct dsi_data *dsi, bool enable)
+{
+	WARN_ON(!dsi_bus_is_locked(dsi));
+
+	if (!dsi->ulps_auto_idle)
+		return;
+
+	if (enable) {
+		schedule_delayed_work(&dsi->ulps_work, msecs_to_jiffies(250));
+	} else {
+		cancel_delayed_work_sync(&dsi->ulps_work);
+
+		if (!dsi->ulps_enabled)
+			return;
+
+		dsi_display_ulps_enable(dsi);
+		dsi_enable_te(dsi, true);
+	}
+}
+
 #ifdef PRINT_VERBOSE_VM_TIMINGS
 static void print_dsi_vm(const char *str,
 		const struct omap_dss_dsi_videomode_timings *t)
@@ -4796,7 +4825,9 @@  static ssize_t omap_dsi_host_transfer(struct mipi_dsi_host *host,
 	int r;
 
 	dsi_bus_lock(dsi);
+	dsi_set_ulps_auto(dsi, false);
 	r = _omap_dsi_host_transfer(dsi, msg);
+	dsi_set_ulps_auto(dsi, true);
 	dsi_bus_unlock(dsi);
 
 	return r;
@@ -4836,8 +4867,6 @@  static const struct omap_dss_device_ops dsi_ops = {
 	.disable = dsi_display_disable,
 
 	.dsi = {
-		.ulps = dsi_ulps,
-
 		.set_config = dsi_set_config,
 
 		.enable_video_output = dsi_enable_video_output,
@@ -4959,6 +4988,12 @@  static int omap_dsi_host_attach(struct mipi_dsi_host *host,
 	dsi->vc[channel].dest = client;
 	dsi->pix_fmt = client->format;
 
+	INIT_DEFERRABLE_WORK(&dsi->ulps_work,
+			     omap_dsi_ulps_work_callback);
+
+	dsi->ulps_auto_idle = !!(client->mode_flags & MIPI_DSI_MODE_ULPS_IDLE);
+	dsi_set_ulps_auto(dsi, true);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index 0d82ba34ca89..524cb87cd9ea 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -282,8 +282,6 @@  struct omap_dss_writeback_info {
 
 struct omapdss_dsi_ops {
 	/* bus configuration */
-	void (*ulps)(struct omap_dss_device *dssdev, bool enable);
-
 	int (*set_config)(struct omap_dss_device *dssdev,
 			const struct omap_dss_dsi_config *cfg);