Re: [PATCH v3 29/56] drm/omap: dsi: do ULPS in host driver
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
>>
>> 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
>> Signed-off-by: Tomi Valkeinen
>> ---
>> .../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
>> #include
>> #include
>> -#include
>> #include
>> #include
>>
>> @@ -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
Re: [PATCH v3 29/56] drm/omap: dsi: do ULPS in host driver
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
>
> 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
> Signed-off-by: Tomi Valkeinen
> ---
> .../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
> #include
> #include
> -#include
> #include
> #include
>
> @@ -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);
> -
