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

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

On 07/29/2014 02:08 PM, Inki Dae wrote:
> On 2014년 07월 29일 20:39, Andrzej Hajda wrote:
>> On 07/29/2014 05:42 AM, Inki Dae wrote:
>>> On 2014년 07월 29일 00:50, Andrzej Hajda wrote:
>>>> On 07/28/2014 04:00 AM, Inki Dae wrote:
>>>>> This patch adds LPM transfer support for video or command data.
>>>>>
>>>>> With this patch, Exynos MIPI DSI controller can transfer command or
>>>>> video data with HS or LP mode in accordance with mode_flags set
>>>>> by LCD Panel driver.
>>>>>
>>>>> Changelog v2: Enable High Speed clock only in case of stand by request.
>>>>>
>>>>> Signed-off-by: Inki Dae <inki.dae@samsung.com>
>>>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>>> ---
>>>>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c |   22 ++++++++++++++++++++--
>>>>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>>> index 5e78d45..1bed105 100644
>>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>>> @@ -493,8 +493,11 @@ static int exynos_dsi_enable_clock(struct exynos_dsi *dsi)
>>>>>  			| DSIM_ESC_PRESCALER(esc_div)
>>>>>  			| DSIM_LANE_ESC_CLK_EN_CLK
>>>>>  			| DSIM_LANE_ESC_CLK_EN_DATA(BIT(dsi->lanes) - 1)
>>>>> -			| DSIM_BYTE_CLK_SRC(0)
>>>>> -			| DSIM_TX_REQUEST_HSCLK;
>>>>> +			| DSIM_BYTE_CLK_SRC(0);
>>>>> +
>>>>> +	if (!(dsi->mode_flags & MIPI_DSI_MODE_CMD_LPM))
>>>>> +		reg |= DSIM_TX_REQUEST_HSCLK;
>>>>> +
>>>>>  	writel(reg, dsi->reg_base + DSIM_CLKCTRL_REG);
>>>>>  
>>>>>  	return 0;
>>>>> @@ -553,6 +556,18 @@ static void exynos_dsi_set_phy_ctrl(struct exynos_dsi *dsi)
>>>>>  	writel(reg, dsi->reg_base + DSIM_PHYTIMING2_REG);
>>>>>  }
>>>>>  
>>>>> +static void exynos_dsi_enable_hs_clock(struct exynos_dsi *dsi,
>>>>> +					bool enable)
>>>>> +{
>>>>> +	u32 reg = readl(dsi->reg_base + DSIM_CLKCTRL_REG);
>>>>> +
>>>>> +	reg &= ~DSIM_TX_REQUEST_HSCLK;
>>>>> +	if (enable)
>>>>> +		reg |= DSIM_TX_REQUEST_HSCLK;
>>>>> +
>>>>> +	writel(reg, dsi->reg_base + DSIM_CLKCTRL_REG);
>>>>> +}
>>>>> +
>>>> I have tested DSIM_TX_REQUEST_HSCLK bit on trats device(video mode HS) -
>>>> it works with and without the bit set.
>>> If you can test thmorstat board, you will face with that its panel is
>>> not worked.
>> So it means this panel requires proper driving of this bit, but it does
>> not prove it is
>> LPM related.
>>
>>>> So I start to suspect this bit is not just for simply enable/disable HS
>>>> clock as function name suggests, do you know what is its
>>>> exact meaning? The specs are quite succinct on it.
>>> HSCLK only has meaning when it is used with CmdLpdt and TxLpdt bits.
>> This sounds very strange. DSI specs and D-PHY specs says clearly
>> that LPM transmissions are unrelated to HS clock [1][2]. Even timing
>> diagrams
>> in Exynos specs shows no dependency of LPM transmissions on HS clock.
>> And the
>> description of TxRequestHsClk bit says "HS clock request for HS transfer
>> at clock lane (Turn
>> on HS clock)".
> There are three System power states of D-PHY, Low-Power mode, High-Speed
> mode and Ultra-Low Power mode. High-Speed mode needs 80Mbps ~ 1Gbps per
> Lane. Therefore, to use HS mode, HS clock should be enabled. On the
> other hand, LP mode needs only 10MHz (max).
>
> So do you really think LP mode will be worked well with HS clock
> enabled? The purpose of LP mode is to reduce power consumption while
> transmitting data. Can you reduce the power consumption in LP mode with
> HS clock enabled?

As MIPI specs says "All DSI transmitters and receivers shall support
continuous clock
behavior on the Clock Lane, and optionally may support non-continuous
clock behavior"
and "For continuous clock behavior, the Clock Lane remains in high-speed
mode generating
active clock signals between HS data packet transmissions".
This means that HS clock should be on even in LP mode, unless panel
supports non-continuous clock
behavior. For signaling non-continuous clock capability there is
MIPI_DSI_CLOCK_NON_CONTINUOUS flag already.

Regards
Andrzej

>
> Thanks,
> Inki Dae
>
>> Maybe different flag should be used to describe your panel requirements
>> regarding this bit.
>>
>> It would be good to see the real initialization sequence in form of
>> pseudo-code or better in the driver.
>>
>>
>> [1]: MIPI DSI: "Note that in Low Power signaling mode, LP clock is
>> functionally embedded in the data signals. When LP
>> data transmission ends, the clock effectively stops and subsequent LP
>> clocks are not available to the
>> peripheral. The peripheral shall not require additional bits, bytes, or
>> packets from the host processor in
>> order to complete processing or pipeline movement of received data in LP
>> mode transmissions. There are a
>> variety of ways to meet this requirement. For example, the peripheral
>> may generate its own clock or it may
>> require the host processor to keep the HS serial clock running."
>>
>> [2]: MIPI D-PHY: "The data is self-clocked by the applied bit encoding
>> and does not rely on the Clock Lane".
>>
>> Regards
>> Andrzej
>>
>>>> On the other side I have found DSIM_TX_LPDT_LP and DSIM_CMD_LPDT_LP bits
>>>> in DSIM_ESCMODE register
>>>> which seems to be related to flags you have introduced:
>>>> - DSIM_CMD_LPDT_LP - transmit commands in LP mode,
>>>> - DSIM_TX_LPDT_LP - transmit data in LP mode.
>>> As I mentioned already at other email thread, DSIM_TX_LPDT_LP specifies
>>> that host can transmit command and also image data to Panel in Low Power
>>> Mode. So these flags are specific to MIPI-DSI controller of Exynos.
>>>
>>>> The former is already triggered by MIPI_DSI_MSG_USE_LPM flag. Why do not
>>> This flag is set only when command msg transmission is requested by
>>> Panel driver. So we would need separated flags, MIPI_DSI_MODE_CMD_LPM
>>> and MIPI_DSI_MODE_VIDEO_LPM, to notify how host controller should
>>> transmit command and also image.
>>>
>>> Thanks,
>>> Inki Dae
>>>
>>>> you use the latter?
>>>>
>>>> Regards
>>>> Andrzej
>>>>
>>>>>  static void exynos_dsi_disable_clock(struct exynos_dsi *dsi)
>>>>>  {
>>>>>  	u32 reg;
>>>>> @@ -705,6 +720,9 @@ static void exynos_dsi_set_display_enable(struct exynos_dsi *dsi, bool enable)
>>>>>  {
>>>>>  	u32 reg;
>>>>>  
>>>>> +	if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_LPM) && enable)
>>>>> +		exynos_dsi_enable_hs_clock(dsi, true);
>>>>> +
>>>>>  	reg = readl(dsi->reg_base + DSIM_MDRESOL_REG);
>>>>>  	if (enable)
>>>>>  		reg |= DSIM_MAIN_STAND_BY;
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>

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