Re: [PATCH v2 1/2] drm/mipi-dsi: add (LPM) Low Power Mode transfer support

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

Thanks for comments.


On 2014년 08월 05일 20:12, Thierry Reding wrote:
> On Mon, Jul 28, 2014 at 06:09:58PM +0200, Andrzej Hajda wrote:
>> On 07/28/2014 04:00 AM, Inki Dae wrote:
>>> This patch adds below two flags for LPM transfer, and it attaches LPM flags
>>> to a msg in accordance with master's mode_flags set by LCD Panel driver.
>>>
>>> MIPI_DSI_MODE_CMD_LPM
>>> - If this flag is set by Panel driver, MIPI-DSI controller will tranfer
>>> command data to Panel device in Low Power Mode.
>>
>> What do you mean by command data? It could be:
>> - all transfer in command mode of operations,
>> - transfer initialized by the driver by writing to DSIM registers.
>>
>>>
>>> MIPI_DSI_MODE_VIDEO_LPM
>>> - If this flag is set by Panel driver, MIPI-DSI controller will tranfer
>>> image data to Panel device in Low Power Mode.
>>
>> What is the meaning of this flag in case of command mode of operation?
>>
>> Maybe it would be better to create flags based on source of data/FIFOs:
>> - commands send by SFR registers,
>> - commands generated from data sent from Display Controller.
> 
> I have no idea what SFR is. But it sounds like you're talking about two
> ways to generate command packets here. We have something similar on
> Tegra, where it's called "host-driven command mode" and "DC-driven
> command mode".
> 
> In host-driven command mode the driver needs to explicitly program some
> of the DSI controller registers to send command packets. This is
> essentially a FIFO register and a control register to trigger
> transmission and poll for completion.
> 
> DC-driven command mode is typically used to update contents of a remote
> framebuffer (what MIPI calls "Type 1 Display Architecture"). This is
> done by programming a different set of registers that cause the DSI
> controller to take data output by the display controller and wrap it
> into DCS commands (e.g. write_memory_start and write_memory_continue).
> 

Exynos also supports above two command mode but we have never used
DC-driven command mode.

> I think that low power mode is more often used for command transmission
> (in host-driven mode). I'm not sure how much sense it really makes to
> transmit video data in low power mode. It also seems like low power mode
> is what all peripherals need to support (if they can do command mode).
> Hence I'd like to propose the attached patch that makes all command
> messages use low power mode.

To use low power mode, I think SoC drivers should add more codes:
checking xxx_MSG_LPM, and maybe disabling HS clock. My patch does
exactly that,
http://www.spinics.net/lists/linux-samsung-soc/msg34866.html

And what I and Andrzej don't make sure is non-continuous clock mode. Do
you know how non-continuous clock mode is related to HS clock?

> 
> The .transfer() function was really designed with initialization
> commands in mind, so it doesn't deal with mixing video data and commands
> anyway and for initialization low-power mode should be fast enough. The
> downside is that it may not be optimal for some peripherals, but it
> gives us a good solution for the general case since it should support
> all devices.
> 
> If we absolutely must have faster initialization, or if we come across a
> device that can only initialize in high speed mode, then I think we
> should introduce a new flag to allow DSI host controllers to optimize in
> those cases.

Originally, mipi-dsi framework enforces implicitly using HS mode for
video and command data as default so I added LPM relevant flags - for
video and also command data
- for host driver can consider Low Power Mode. However, your patch makes
it use
Low Power Mode for command data as default. So your patch would mean
that default transfer mode for command data is Low Power Mode, but HS
mode for video data.

Do you intend to control transfer mode - HS or LPM - only for command
data? If so, we would need only one flag, i.e., MIPI_DSI_MODE_HS.

Thanks,
Inki Dae

> 
> Note that this is based on some of my local patches, so it won't apply
> as-is. But if anybody wants to give this a go it should be easy to apply
> manually as well.
> 
> Thierry
>