Hi Tomi and Sebastian,

Thank you for the patch.

On Thu, Nov 05, 2020 at 02:03:05PM +0200, Tomi Valkeinen wrote:
> From: Sebastian Reichel <sebastian.reic...@collabora.com>
> 
> Create a custom function pointer for ULPS and use it instead of
> reusing disable/enable functions for ULPS mode switch. This allows
> us to use the common disable/enable functions pointers for DSI.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reic...@collabora.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkei...@ti.com>
> ---
>  .../gpu/drm/omapdrm/displays/panel-dsi-cm.c   |  8 ++--
>  drivers/gpu/drm/omapdrm/dss/dsi.c             | 42 ++++++++++++++-----
>  drivers/gpu/drm/omapdrm/dss/omapdss.h         |  5 +--
>  3 files changed, 38 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c 
> b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> index 4be0c9dbcc43..78247dcb1848 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> @@ -233,7 +233,7 @@ static int dsicm_enter_ulps(struct panel_drv_data *ddata)
>       if (r)
>               goto err;
>  
> -     src->ops->dsi.disable(src, false, true);
> +     src->ops->dsi.ulps(src, true);
>  
>       ddata->ulps_enabled = true;
>  
> @@ -258,7 +258,7 @@ static int dsicm_exit_ulps(struct panel_drv_data *ddata)
>       if (!ddata->ulps_enabled)
>               return 0;
>  
> -     src->ops->enable(src);
> +     src->ops->dsi.ulps(src, false);
>       ddata->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
>  
>       r = _dsicm_enable_te(ddata, ddata->te_enabled);
> @@ -586,7 +586,7 @@ static int dsicm_power_on(struct panel_drv_data *ddata)
>  
>       dsicm_hw_reset(ddata);
>  
> -     src->ops->dsi.disable(src, true, false);
> +     src->ops->disable(src);
>  err_regulators:
>       r = regulator_bulk_disable(DCS_REGULATOR_SUPPLY_NUM, ddata->supplies);
>       if (r)
> @@ -612,7 +612,7 @@ static void dsicm_power_off(struct panel_drv_data *ddata)
>               dsicm_hw_reset(ddata);
>       }
>  
> -     src->ops->dsi.disable(src, true, false);
> +     src->ops->disable(src);
>  
>       r = regulator_bulk_disable(DCS_REGULATOR_SUPPLY_NUM, ddata->supplies);
>       if (r)
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c 
> b/drivers/gpu/drm/omapdrm/dss/dsi.c
> index d54b743c2b48..937362ade4b4 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -4055,13 +4055,10 @@ static void dsi_display_uninit_dsi(struct dsi_data 
> *dsi, bool disconnect_lanes,
>       }
>  }
>  
> -static void dsi_display_enable(struct omap_dss_device *dssdev)
> +static void dsi_display_ulps_enable(struct dsi_data *dsi)
>  {
> -     struct dsi_data *dsi = to_dsi_data(dssdev);
>       int r;
>  
> -     DSSDBG("dsi_display_enable\n");
> -
>       WARN_ON(!dsi_bus_is_locked(dsi));
>  
>       mutex_lock(&dsi->lock);
> @@ -4084,16 +4081,19 @@ static void dsi_display_enable(struct omap_dss_device 
> *dssdev)
>       dsi_runtime_put(dsi);
>  err_get_dsi:
>       mutex_unlock(&dsi->lock);
> -     DSSDBG("dsi_display_enable FAILED\n");
> +     DSSDBG("dsi_display_ulps_enable FAILED\n");
>  }
>  
> -static void dsi_display_disable(struct omap_dss_device *dssdev,
> -             bool disconnect_lanes, bool enter_ulps)
> +static void dsi_display_enable(struct omap_dss_device *dssdev)
>  {
>       struct dsi_data *dsi = to_dsi_data(dssdev);
> +     DSSDBG("dsi_display_enable\n");
> +     dsi_display_ulps_enable(dsi);
> +}
>  
> -     DSSDBG("dsi_display_disable\n");
> -
> +static void dsi_display_ulps_disable(struct dsi_data *dsi,
> +             bool disconnect_lanes, bool enter_ulps)
> +{
>       WARN_ON(!dsi_bus_is_locked(dsi));
>  
>       mutex_lock(&dsi->lock);
> @@ -4110,6 +4110,27 @@ static void dsi_display_disable(struct omap_dss_device 
> *dssdev,
>       mutex_unlock(&dsi->lock);
>  }
>  
> +static void dsi_display_disable(struct omap_dss_device *dssdev)
> +{
> +     struct dsi_data *dsi = to_dsi_data(dssdev);
> +
> +     DSSDBG("dsi_display_disable\n");
> +
> +     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);

The names are fairly confusing. I would expect
dsi_display_ulps_disable() to disable ULPS mode.

> +}
> +
>  static int dsi_enable_te(struct dsi_data *dsi, bool enable)
>  {
>       dsi->te_enabled = enable;
> @@ -4812,9 +4833,10 @@ static const struct omap_dss_device_ops dsi_ops = {
>       .connect = dsi_connect,
>       .disconnect = dsi_disconnect,
>       .enable = dsi_display_enable,
> +     .disable = dsi_display_disable,
>  
>       .dsi = {
> -             .disable = dsi_display_disable,
> +             .ulps = dsi_ulps,
>  
>               .set_config = dsi_set_config,
>  
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h 
> b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> index 43eba2ea1f96..0d82ba34ca89 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> @@ -281,10 +281,9 @@ struct omap_dss_writeback_info {
>  };
>  
>  struct omapdss_dsi_ops {
> -     void (*disable)(struct omap_dss_device *dssdev, bool disconnect_lanes,
> -                     bool enter_ulps);
> -
>       /* 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to