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

[Inki Dae <inki.dae@samsung.com>]

On 2014년 08월 18일 20:38, Andrzej Hajda wrote:
> 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.

Thanks for comments and CCing Thierry.

I think it'd better to handle this solution flexibly. There was a
concern that my solution makes all commands to be transmitted in low
power or high speed by default. So if we have other solution which
passes a flag per a message, then we could transmit each command in low
power or high speed, which could also be used selectively with my
solution - if msg->flags has MIPI_DSI_MODE_LPM, then each flag of the
new API is ignored.

Thierry, which one do you prefer? Otherwise, do you have other opinions?

Thanks,
Inki Dae

> 
> 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,
> 
> 

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