Re: [PATCH 1/2] drm/mipi-dsi: consider low power transmission

[Andrzej Hajda <a.hajda@samsung.com>]

On 08/18/2014 10:11 AM, Inki Dae wrote:
> This patch adds a new flag, MIPI_DSI-MODE_LPM, to transmit data
> in low power. With this flag, msg.flags has MIPI_DSI_MSG_USE_LPM
> so that host driver of each SoC can clear or set relevant register
> bit for low power transmission.
>
> All host drivers shall support continuous clock behavior on the
> Clock Lane, and optionally may support non-continuous clock behavior.
> Both of them can transmit data in high speed of low power.
>
> With each clock behavior, non-continuous or continuous clock mode,
> host controller will transmit data in high speed by default so if
> peripheral wants to receive data in low power, the peripheral driver
> should set MIPI_DSI_MODE_LPM flag.

I think it would be better to remove last two paragraphs as irrelevant,
LPM and clock behavior are orthogonal.

>
> Signed-off-by: Inki Dae <inki.dae@samsung.com>
> ---
>  drivers/gpu/drm/drm_mipi_dsi.c |    6 ++++++
>  include/drm/drm_mipi_dsi.h     |    2 ++
>  2 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 6aa6a9e..eb6dfe5 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -231,6 +231,9 @@ ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, const void *data,
>  		break;
>  	}
>  
> +	if (dsi->mode_flags & MIPI_DSI_MODE_LPM)
> +		msg.flags = MIPI_DSI_MSG_USE_LPM;
> +
>  	return ops->transfer(dsi->host, &msg);
>  }
>  EXPORT_SYMBOL(mipi_dsi_dcs_write);
> @@ -260,6 +263,9 @@ ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, u8 cmd, void *data,
>  	if (!ops || !ops->transfer)
>  		return -ENOSYS;
>  
> +	if (dsi->mode_flags & MIPI_DSI_MODE_LPM)
> +		msg.flags = MIPI_DSI_MSG_USE_LPM;
> +

I see three other ways of adding LPM to DCS:

1. Add flags argument to DCS command, eg:
   mipi_dsi_dcs_write(dsi, data, len, flags)

2. Pass struct mipi_dsi_device to transfer callback:
    ssize_t (*transfer)(struct mipi_dsi_device *dsi,
                struct mipi_dsi_msg *msg);
   or
    ssize_t (*transfer)(struct mipi_dsi_host *host,
        struct mipi_dsi_device *dsi,
        struct mipi_dsi_msg *msg);

    This way DSI host will have access to mipi_dsi_device
    and to its flags.

3. Create new API function, lets call it dsi_transfer, which
    should by called by DCS helpers instead of transfer op.
    This function shall translate device flags to message flags
    and call the original op.

I think the 3rd solution is the best one, but I have no strong feelings
against the other ones, including your.
As I remember Thierry have also some ideas about changes in DSI API
so I have added him to CC.

Regards
Andrzej


>  	return ops->transfer(dsi->host, &msg);
>  }
>  EXPORT_SYMBOL(mipi_dsi_dcs_read);
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 2bb55b8..8569dc5 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -96,6 +96,8 @@ void mipi_dsi_host_unregister(struct mipi_dsi_host *host);
>  #define MIPI_DSI_MODE_EOT_PACKET	BIT(9)
>  /* device supports non-continuous clock behavior (DSI spec 5.6.1) */
>  #define MIPI_DSI_CLOCK_NON_CONTINUOUS	BIT(10)
> +/* transmit data in low power */
> +#define MIPI_DSI_MODE_LPM		BIT(11)
>  
>  enum mipi_dsi_pixel_format {
>  	MIPI_DSI_FMT_RGB888,