| Message ID | 1406512857-7213-3-git-send-email-inki.dae@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. 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. 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. The former is already triggered by MIPI_DSI_MSG_USE_LPM flag. Why do not 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;
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 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. > 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 >
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)". 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 >> >
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? 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 >
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 >> >
On 2014? 07? 29? 22:16, Andrzej Hajda wrote: > 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 We know what the spec says. As I mentioned already before and in other threads, the important thing is how we should handle HS clock in mipi-dsi framework and host driver. To make sure that, 1. In case of supporting non-continuous clock mode, who should control HS clock of host controller properly every time D-PHY detected LP-11? by D-PHY hw regardless of host driver? So in this case, doesn't the host driver need to check MSG_LPM flag and to control HS clock before host controller transmits command data in LPM? One more thing, in this case, is the operation mode HS mode or LP mode while transmitting command data - there would be LP-11 signal between command packets and operation mode would be entered to LP mode only when D-PHY detected LP-11? Therefore, If HS mode, MIPI_DSI_CLOCK_NON_CONTINUOUS would have different operation from MSG_LPM because command data will be transmitted to panel in HS mode, not in LP mode. 2. In case of not supporting non-continuous clock mode but requesting MSG_LPM, should the host driver check the flag and control HS clock properly before host controller transmits command data in LPM? Thanks, Inki Dae > 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 >>> >> > >
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); +} + 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;