diff mbox series

[v8,02/15] drm/sun4i: tcon: Compute DCLK dividers based on format, lanes

Message ID 20190311133637.18334-3-jagan@amarulasolutions.com (mailing list archive)
State New, archived
Headers show
Series drm/sun4i: Allwinner A64 MIPI-DSI support | expand

Commit Message

Jagan Teki March 11, 2019, 1:36 p.m. UTC 
pll-video => pll-mipi => tcon0 => tcon0-pixel-clock is the typical
MIPI clock topology in Allwinner DSI controller.

TCON dotclock driver is computing the desired DCLK divider based on
panel pixel clock along with input DCLK min, max divider values from
tcon driver and that would eventually set the pll-mipi clock rate.

The current code allows the TCON clock divider to have a default 4
for min, max ranges that would fail to compute the desired pll-mipi
rate while supporting new panels.

So, add the computation logic 'format/lanes' to dclk min and max dividers
and instead of default 4. This computation logic align with Allwinner A64
BSP, hoping that would work even for A33.

Tested this on 4 different DSI panels.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Merlijn Wajer <merlijn@wizzup.org>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Maxime Ripard March 11, 2019, 3:38 p.m. UTC | #1 
On Mon, Mar 11, 2019 at 07:06:24PM +0530, Jagan Teki wrote:
> pll-video => pll-mipi => tcon0 => tcon0-pixel-clock is the typical
> MIPI clock topology in Allwinner DSI controller.
> 
> TCON dotclock driver is computing the desired DCLK divider based on
> panel pixel clock along with input DCLK min, max divider values from
> tcon driver and that would eventually set the pll-mipi clock rate.
> 
> The current code allows the TCON clock divider to have a default 4
> for min, max ranges that would fail to compute the desired pll-mipi
> rate while supporting new panels.
> 
> So, add the computation logic 'format/lanes' to dclk min and max dividers
> and instead of default 4. This computation logic align with Allwinner A64
> BSP, hoping that would work even for A33.

Last time we discussed this, we found out that this wasn't the case,
even in the BSP.

What compelling evidence have you found that makes you say otherwise?

Maxime
Jagan Teki March 11, 2019, 4:06 p.m. UTC | #2 
On Mon, Mar 11, 2019 at 9:08 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Mon, Mar 11, 2019 at 07:06:24PM +0530, Jagan Teki wrote:
> > pll-video => pll-mipi => tcon0 => tcon0-pixel-clock is the typical
> > MIPI clock topology in Allwinner DSI controller.
> >
> > TCON dotclock driver is computing the desired DCLK divider based on
> > panel pixel clock along with input DCLK min, max divider values from
> > tcon driver and that would eventually set the pll-mipi clock rate.
> >
> > The current code allows the TCON clock divider to have a default 4
> > for min, max ranges that would fail to compute the desired pll-mipi
> > rate while supporting new panels.
> >
> > So, add the computation logic 'format/lanes' to dclk min and max dividers
> > and instead of default 4. This computation logic align with Allwinner A64
> > BSP, hoping that would work even for A33.
>
> Last time we discussed this, we found out that this wasn't the case,
> even in the BSP.

This was the case for BSP to compute pll-mipi not for TCON_DSI clock
register, SUN4I_TCON0_DCLK_REG, which marked the divider 4 by default.

>
> What compelling evidence have you found that makes you say otherwise?

divider 4 isn't worked, this I would mentioned before as well.

Tested this on 4 different panels, and below are the desired divider values
and pll-mipi clock rate with respect to pixel clock frequency.

- 55MHz pixel clock with 4-lane panel, and the desired DSI clock divider
  is 6 with the output parent clock rate of 330MHz.
- 30MHz pixel clock with 4-lane panel, and the desired DSI clock divider
  is 6 with parent clock rate of 180MHz.
- 27.5Mhz pixel clock with 2-lane pane, and the desired DSI clock divider
  is 12 with the output parent clock rate of 330MHz.
- 147MHz pixel clock with 4-lane panel, and the desired DSI clock divider
  is 6 with the output parent clock rate of 882MHz.

BSP trying to use this format/lane to compute dsi divider that in-turn
using pll-mipi set_rate but TCON0_DCLK_REG keep constant 4.
Jagan Teki March 18, 2019, 6:24 p.m. UTC | #3 
On Mon, Mar 11, 2019 at 9:36 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Mon, Mar 11, 2019 at 9:08 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Mon, Mar 11, 2019 at 07:06:24PM +0530, Jagan Teki wrote:
> > > pll-video => pll-mipi => tcon0 => tcon0-pixel-clock is the typical
> > > MIPI clock topology in Allwinner DSI controller.
> > >
> > > TCON dotclock driver is computing the desired DCLK divider based on
> > > panel pixel clock along with input DCLK min, max divider values from
> > > tcon driver and that would eventually set the pll-mipi clock rate.
> > >
> > > The current code allows the TCON clock divider to have a default 4
> > > for min, max ranges that would fail to compute the desired pll-mipi
> > > rate while supporting new panels.
> > >
> > > So, add the computation logic 'format/lanes' to dclk min and max dividers
> > > and instead of default 4. This computation logic align with Allwinner A64
> > > BSP, hoping that would work even for A33.
> >
> > Last time we discussed this, we found out that this wasn't the case,
> > even in the BSP.
>
> This was the case for BSP to compute pll-mipi not for TCON_DSI clock
> register, SUN4I_TCON0_DCLK_REG, which marked the divider 4 by default.
>
> >
> > What compelling evidence have you found that makes you say otherwise?
>
> divider 4 isn't worked, this I would mentioned before as well.
>
> Tested this on 4 different panels, and below are the desired divider values
> and pll-mipi clock rate with respect to pixel clock frequency.
>
> - 55MHz pixel clock with 4-lane panel, and the desired DSI clock divider
>   is 6 with the output parent clock rate of 330MHz.
> - 30MHz pixel clock with 4-lane panel, and the desired DSI clock divider
>   is 6 with parent clock rate of 180MHz.
> - 27.5Mhz pixel clock with 2-lane pane, and the desired DSI clock divider
>   is 12 with the output parent clock rate of 330MHz.
> - 147MHz pixel clock with 4-lane panel, and the desired DSI clock divider
>   is 6 with the output parent clock rate of 882MHz.
>
> BSP trying to use this format/lane to compute dsi divider that in-turn
> using pll-mipi set_rate but TCON0_DCLK_REG keep constant 4.

Any comments?

For more information, I have even tying to get the dump here to show
the pll_rate set in BSP code [1], please have a look at this gist[2]
where the bpp/lanes value of 6 is computed for pll_rate and let me
know what do you think?

DSI rate is 148.5Mhz
Panel Pixel clock is 148Mhz
PLL set rate is 888Mhz (which is bpp/lanes = 6 * 148 Mhz)

[1] https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L793
[2] https://gist.github.com/openedev/9bae2d87d2fcc06b999fe48c998b7043
     https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
Maxime Ripard March 19, 2019, 10:53 a.m. UTC | #4 
On Mon, Mar 11, 2019 at 09:36:27PM +0530, Jagan Teki wrote:
> On Mon, Mar 11, 2019 at 9:08 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Mon, Mar 11, 2019 at 07:06:24PM +0530, Jagan Teki wrote:
> > > pll-video => pll-mipi => tcon0 => tcon0-pixel-clock is the typical
> > > MIPI clock topology in Allwinner DSI controller.
> > >
> > > TCON dotclock driver is computing the desired DCLK divider based on
> > > panel pixel clock along with input DCLK min, max divider values from
> > > tcon driver and that would eventually set the pll-mipi clock rate.
> > >
> > > The current code allows the TCON clock divider to have a default 4
> > > for min, max ranges that would fail to compute the desired pll-mipi
> > > rate while supporting new panels.
> > >
> > > So, add the computation logic 'format/lanes' to dclk min and max dividers
> > > and instead of default 4. This computation logic align with Allwinner A64
> > > BSP, hoping that would work even for A33.
> >
> > Last time we discussed this, we found out that this wasn't the case,
> > even in the BSP.
>
> This was the case for BSP to compute pll-mipi not for TCON_DSI clock
> register, SUN4I_TCON0_DCLK_REG, which marked the divider 4 by default.
>
> > What compelling evidence have you found that makes you say otherwise?
>
> divider 4 isn't worked, this I would mentioned before as well.

Maybe you mentionned it before, but it's nowhere to be found in your
commit log.

> Tested this on 4 different panels, and below are the desired divider values
> and pll-mipi clock rate with respect to pixel clock frequency.
>
> - 55MHz pixel clock with 4-lane panel, and the desired DSI clock divider
>   is 6 with the output parent clock rate of 330MHz.
> - 30MHz pixel clock with 4-lane panel, and the desired DSI clock divider
>   is 6 with parent clock rate of 180MHz.
> - 27.5Mhz pixel clock with 2-lane pane, and the desired DSI clock divider
>   is 12 with the output parent clock rate of 330MHz.
> - 147MHz pixel clock with 4-lane panel, and the desired DSI clock divider
>   is 6 with the output parent clock rate of 882MHz.
>
> BSP trying to use this format/lane to compute dsi divider that in-turn
> using pll-mipi set_rate but TCON0_DCLK_REG keep constant 4.

Feel free to reply to
http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/629596.html

And correct whatever is said there that isn't what is happening.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Sergey Suloev March 19, 2019, 12:17 p.m. UTC | #5 
Hi, guys,

On 3/19/19 1:53 PM, Maxime Ripard wrote:
> On Mon, Mar 11, 2019 at 09:36:27PM +0530, Jagan Teki wrote:
>> On Mon, Mar 11, 2019 at 9:08 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>>> On Mon, Mar 11, 2019 at 07:06:24PM +0530, Jagan Teki wrote:
>>>> pll-video => pll-mipi => tcon0 => tcon0-pixel-clock is the typical
>>>> MIPI clock topology in Allwinner DSI controller.
>>>>
>>>> TCON dotclock driver is computing the desired DCLK divider based on
>>>> panel pixel clock along with input DCLK min, max divider values from
>>>> tcon driver and that would eventually set the pll-mipi clock rate.
>>>>
>>>> The current code allows the TCON clock divider to have a default 4
>>>> for min, max ranges that would fail to compute the desired pll-mipi
>>>> rate while supporting new panels.
>>>>
>>>> So, add the computation logic 'format/lanes' to dclk min and max dividers
>>>> and instead of default 4. This computation logic align with Allwinner A64
>>>> BSP, hoping that would work even for A33.
>>> Last time we discussed this, we found out that this wasn't the case,
>>> even in the BSP.
>> This was the case for BSP to compute pll-mipi not for TCON_DSI clock
>> register, SUN4I_TCON0_DCLK_REG, which marked the divider 4 by default.
>>
>>> What compelling evidence have you found that makes you say otherwise?
>> divider 4 isn't worked, this I would mentioned before as well.
> Maybe you mentionned it before, but it's nowhere to be found in your
> commit log.
>
>> Tested this on 4 different panels, and below are the desired divider values
>> and pll-mipi clock rate with respect to pixel clock frequency.
>>
>> - 55MHz pixel clock with 4-lane panel, and the desired DSI clock divider
>>    is 6 with the output parent clock rate of 330MHz.
>> - 30MHz pixel clock with 4-lane panel, and the desired DSI clock divider
>>    is 6 with parent clock rate of 180MHz.
>> - 27.5Mhz pixel clock with 2-lane pane, and the desired DSI clock divider
>>    is 12 with the output parent clock rate of 330MHz.
>> - 147MHz pixel clock with 4-lane panel, and the desired DSI clock divider
>>    is 6 with the output parent clock rate of 882MHz.
>>
>> BSP trying to use this format/lane to compute dsi divider that in-turn
>> using pll-mipi set_rate but TCON0_DCLK_REG keep constant 4.
> Feel free to reply to
> http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/629596.html
>
> And correct whatever is said there that isn't what is happening.


excuse me if my message is out of topic.

I just want let you know  that the code

tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;

where SUN6I_DSI_TCON_DIV = 4  isn't working with my 2-lane panel: I am 
always getting the error:

  [CRTC:38:crtc-0] vblank wait timed out

As soon as I set

tcon->dclk_min_div = tcon->dclk_max_div = bpp/lanes, i.e. 12

the error disappears.


> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html;
      charset=windows-1252">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p>Hi, guys,<br>
    </p>
    <div class="moz-cite-prefix">On 3/19/19 1:53 PM, Maxime Ripard
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:20190319105307.omowzyhqu33e3pzy@flea">
      <pre class="moz-quote-pre" wrap="">On Mon, Mar 11, 2019 at 09:36:27PM +0530, Jagan Teki wrote:
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">On Mon, Mar 11, 2019 at 9:08 PM Maxime Ripard <a class="moz-txt-link-rfc2396E" href="mailto:maxime.ripard@bootlin.com">&lt;maxime.ripard@bootlin.com&gt;</a> wrote:
</pre>
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">
On Mon, Mar 11, 2019 at 07:06:24PM +0530, Jagan Teki wrote:
</pre>
          <blockquote type="cite">
            <pre class="moz-quote-pre" wrap="">pll-video =&gt; pll-mipi =&gt; tcon0 =&gt; tcon0-pixel-clock is the typical
MIPI clock topology in Allwinner DSI controller.

TCON dotclock driver is computing the desired DCLK divider based on
panel pixel clock along with input DCLK min, max divider values from
tcon driver and that would eventually set the pll-mipi clock rate.

The current code allows the TCON clock divider to have a default 4
for min, max ranges that would fail to compute the desired pll-mipi
rate while supporting new panels.

So, add the computation logic 'format/lanes' to dclk min and max dividers
and instead of default 4. This computation logic align with Allwinner A64
BSP, hoping that would work even for A33.
</pre>
          </blockquote>
          <pre class="moz-quote-pre" wrap="">
Last time we discussed this, we found out that this wasn't the case,
even in the BSP.
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
This was the case for BSP to compute pll-mipi not for TCON_DSI clock
register, SUN4I_TCON0_DCLK_REG, which marked the divider 4 by default.

</pre>
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">What compelling evidence have you found that makes you say otherwise?
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
divider 4 isn't worked, this I would mentioned before as well.
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
Maybe you mentionned it before, but it's nowhere to be found in your
commit log.

</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">Tested this on 4 different panels, and below are the desired divider values
and pll-mipi clock rate with respect to pixel clock frequency.

- 55MHz pixel clock with 4-lane panel, and the desired DSI clock divider
  is 6 with the output parent clock rate of 330MHz.
- 30MHz pixel clock with 4-lane panel, and the desired DSI clock divider
  is 6 with parent clock rate of 180MHz.
- 27.5Mhz pixel clock with 2-lane pane, and the desired DSI clock divider
  is 12 with the output parent clock rate of 330MHz.
- 147MHz pixel clock with 4-lane panel, and the desired DSI clock divider
  is 6 with the output parent clock rate of 882MHz.

BSP trying to use this format/lane to compute dsi divider that in-turn
using pll-mipi set_rate but TCON0_DCLK_REG keep constant 4.
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
Feel free to reply to
<a class="moz-txt-link-freetext" href="http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/629596.html">http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/629596.html</a>

And correct whatever is said there that isn't what is happening.
</pre>
    </blockquote>
    <p><br>
    </p>
    <p>excuse me if my message is out of topic.</p>
    <p>I just want let you know  that the code <br>
    </p>
    <p>tcon-&gt;dclk_min_div = SUN6I_DSI_TCON_DIV;<br>
      tcon-&gt;dclk_max_div = SUN6I_DSI_TCON_DIV;</p>
    <p>where SUN6I_DSI_TCON_DIV = 4  isn't working with my 2-lane panel:
      I am always getting the error:</p>
    <p> [CRTC:38:crtc-0] vblank wait timed out<br>
    </p>
    <p>As soon as I set <br>
    </p>
    <p>tcon-&gt;dclk_min_div = tcon-&gt;dclk_max_div = bpp/lanes, i.e.
      12<br>
    </p>
    <p>the error disappears.<br>
    </p>
    <p><br>
    </p>
    <blockquote type="cite"
      cite="mid:20190319105307.omowzyhqu33e3pzy@flea">
      <pre class="moz-quote-pre" wrap="">
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
<a class="moz-txt-link-freetext" href="https://bootlin.com">https://bootlin.com</a>
</pre>
      <br>
      <fieldset class="mimeAttachmentHeader"></fieldset>
      <pre class="moz-quote-pre" wrap="">_______________________________________________
linux-arm-kernel mailing list
<a class="moz-txt-link-abbreviated" href="mailto:linux-arm-kernel@lists.infradead.org">linux-arm-kernel@lists.infradead.org</a>
<a class="moz-txt-link-freetext" href="http://lists.infradead.org/mailman/listinfo/linux-arm-kernel">http://lists.infradead.org/mailman/listinfo/linux-arm-kernel</a>
</pre>
    </blockquote>
  </body>
</html>
Jagan Teki March 21, 2019, 2:10 p.m. UTC | #6 
On Tue, Mar 19, 2019 at 4:23 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Mon, Mar 11, 2019 at 09:36:27PM +0530, Jagan Teki wrote:
> > On Mon, Mar 11, 2019 at 9:08 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Mon, Mar 11, 2019 at 07:06:24PM +0530, Jagan Teki wrote:
> > > > pll-video => pll-mipi => tcon0 => tcon0-pixel-clock is the typical
> > > > MIPI clock topology in Allwinner DSI controller.
> > > >
> > > > TCON dotclock driver is computing the desired DCLK divider based on
> > > > panel pixel clock along with input DCLK min, max divider values from
> > > > tcon driver and that would eventually set the pll-mipi clock rate.
> > > >
> > > > The current code allows the TCON clock divider to have a default 4
> > > > for min, max ranges that would fail to compute the desired pll-mipi
> > > > rate while supporting new panels.
> > > >
> > > > So, add the computation logic 'format/lanes' to dclk min and max dividers
> > > > and instead of default 4. This computation logic align with Allwinner A64
> > > > BSP, hoping that would work even for A33.
> > >
> > > Last time we discussed this, we found out that this wasn't the case,
> > > even in the BSP.
> >
> > This was the case for BSP to compute pll-mipi not for TCON_DSI clock
> > register, SUN4I_TCON0_DCLK_REG, which marked the divider 4 by default.
> >
> > > What compelling evidence have you found that makes you say otherwise?
> >
> > divider 4 isn't worked, this I would mentioned before as well.
>
> Maybe you mentionned it before, but it's nowhere to be found in your
> commit log.

I have added it in 3rd paragraph in commit message, may be you missed
it or you may look for different text.

"The current code allows the TCON clock divider to have a default 4
for min, max ranges that would fail to compute the desired pll-mipi
rate while supporting new panels."

>
> > Tested this on 4 different panels, and below are the desired divider values
> > and pll-mipi clock rate with respect to pixel clock frequency.
> >
> > - 55MHz pixel clock with 4-lane panel, and the desired DSI clock divider
> >   is 6 with the output parent clock rate of 330MHz.
> > - 30MHz pixel clock with 4-lane panel, and the desired DSI clock divider
> >   is 6 with parent clock rate of 180MHz.
> > - 27.5Mhz pixel clock with 2-lane pane, and the desired DSI clock divider
> >   is 12 with the output parent clock rate of 330MHz.
> > - 147MHz pixel clock with 4-lane panel, and the desired DSI clock divider
> >   is 6 with the output parent clock rate of 882MHz.
> >
> > BSP trying to use this format/lane to compute dsi divider that in-turn
> > using pll-mipi set_rate but TCON0_DCLK_REG keep constant 4.
>
> Feel free to reply to
> http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/629596.html

Yes, I have replied at that time itself, please check.

http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/629673.html

Here I have taken the pixel clock to 30Mhz for example in Bananapi
panel, and the above gist the pixel is 148Mhz from another panel.
Jagan Teki March 21, 2019, 2:11 p.m. UTC | #7 
Hi Sergey,

On Tue, Mar 19, 2019 at 5:47 PM Sergey Suloev <ssuloev@orpaltech.com> wrote:
>
> Hi, guys,
>
> On 3/19/19 1:53 PM, Maxime Ripard wrote:
>
> On Mon, Mar 11, 2019 at 09:36:27PM +0530, Jagan Teki wrote:
>
> On Mon, Mar 11, 2019 at 9:08 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Mon, Mar 11, 2019 at 07:06:24PM +0530, Jagan Teki wrote:
>
> pll-video => pll-mipi => tcon0 => tcon0-pixel-clock is the typical
> MIPI clock topology in Allwinner DSI controller.
>
> TCON dotclock driver is computing the desired DCLK divider based on
> panel pixel clock along with input DCLK min, max divider values from
> tcon driver and that would eventually set the pll-mipi clock rate.
>
> The current code allows the TCON clock divider to have a default 4
> for min, max ranges that would fail to compute the desired pll-mipi
> rate while supporting new panels.
>
> So, add the computation logic 'format/lanes' to dclk min and max dividers
> and instead of default 4. This computation logic align with Allwinner A64
> BSP, hoping that would work even for A33.
>
> Last time we discussed this, we found out that this wasn't the case,
> even in the BSP.
>
> This was the case for BSP to compute pll-mipi not for TCON_DSI clock
> register, SUN4I_TCON0_DCLK_REG, which marked the divider 4 by default.
>
> What compelling evidence have you found that makes you say otherwise?
>
> divider 4 isn't worked, this I would mentioned before as well.
>
> Maybe you mentionned it before, but it's nowhere to be found in your
> commit log.
>
> Tested this on 4 different panels, and below are the desired divider values
> and pll-mipi clock rate with respect to pixel clock frequency.
>
> - 55MHz pixel clock with 4-lane panel, and the desired DSI clock divider
>   is 6 with the output parent clock rate of 330MHz.
> - 30MHz pixel clock with 4-lane panel, and the desired DSI clock divider
>   is 6 with parent clock rate of 180MHz.
> - 27.5Mhz pixel clock with 2-lane pane, and the desired DSI clock divider
>   is 12 with the output parent clock rate of 330MHz.
> - 147MHz pixel clock with 4-lane panel, and the desired DSI clock divider
>   is 6 with the output parent clock rate of 882MHz.
>
> BSP trying to use this format/lane to compute dsi divider that in-turn
> using pll-mipi set_rate but TCON0_DCLK_REG keep constant 4.
>
> Feel free to reply to
> http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/629596.html
>
> And correct whatever is said there that isn't what is happening.
>
>
> excuse me if my message is out of topic.
>
> I just want let you know  that the code
>
> tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
>
> where SUN6I_DSI_TCON_DIV = 4  isn't working with my 2-lane panel: I am always getting the error:
>
>  [CRTC:38:crtc-0] vblank wait timed out
>
> As soon as I set
>
> tcon->dclk_min_div = tcon->dclk_max_div = bpp/lanes, i.e. 12
>
> the error disappears.

Yes, I did test the same in 2-lane panel. it worked with the logic.
thanks for testing.
Jagan Teki April 2, 2019, 1:33 p.m. UTC | #8 
Hi Maxime,

On Thu, Mar 21, 2019 at 7:40 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Tue, Mar 19, 2019 at 4:23 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Mon, Mar 11, 2019 at 09:36:27PM +0530, Jagan Teki wrote:
> > > On Mon, Mar 11, 2019 at 9:08 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Mon, Mar 11, 2019 at 07:06:24PM +0530, Jagan Teki wrote:
> > > > > pll-video => pll-mipi => tcon0 => tcon0-pixel-clock is the typical
> > > > > MIPI clock topology in Allwinner DSI controller.
> > > > >
> > > > > TCON dotclock driver is computing the desired DCLK divider based on
> > > > > panel pixel clock along with input DCLK min, max divider values from
> > > > > tcon driver and that would eventually set the pll-mipi clock rate.
> > > > >
> > > > > The current code allows the TCON clock divider to have a default 4
> > > > > for min, max ranges that would fail to compute the desired pll-mipi
> > > > > rate while supporting new panels.
> > > > >
> > > > > So, add the computation logic 'format/lanes' to dclk min and max dividers
> > > > > and instead of default 4. This computation logic align with Allwinner A64
> > > > > BSP, hoping that would work even for A33.
> > > >
> > > > Last time we discussed this, we found out that this wasn't the case,
> > > > even in the BSP.
> > >
> > > This was the case for BSP to compute pll-mipi not for TCON_DSI clock
> > > register, SUN4I_TCON0_DCLK_REG, which marked the divider 4 by default.
> > >
> > > > What compelling evidence have you found that makes you say otherwise?
> > >
> > > divider 4 isn't worked, this I would mentioned before as well.
> >
> > Maybe you mentionned it before, but it's nowhere to be found in your
> > commit log.
>
> I have added it in 3rd paragraph in commit message, may be you missed
> it or you may look for different text.
>
> "The current code allows the TCON clock divider to have a default 4
> for min, max ranges that would fail to compute the desired pll-mipi
> rate while supporting new panels."
>
> >
> > > Tested this on 4 different panels, and below are the desired divider values
> > > and pll-mipi clock rate with respect to pixel clock frequency.
> > >
> > > - 55MHz pixel clock with 4-lane panel, and the desired DSI clock divider
> > >   is 6 with the output parent clock rate of 330MHz.
> > > - 30MHz pixel clock with 4-lane panel, and the desired DSI clock divider
> > >   is 6 with parent clock rate of 180MHz.
> > > - 27.5Mhz pixel clock with 2-lane pane, and the desired DSI clock divider
> > >   is 12 with the output parent clock rate of 330MHz.
> > > - 147MHz pixel clock with 4-lane panel, and the desired DSI clock divider
> > >   is 6 with the output parent clock rate of 882MHz.
> > >
> > > BSP trying to use this format/lane to compute dsi divider that in-turn
> > > using pll-mipi set_rate but TCON0_DCLK_REG keep constant 4.
> >
> > Feel free to reply to
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/629596.html
>
> Yes, I have replied at that time itself, please check.
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/629673.html
>
> Here I have taken the pixel clock to 30Mhz for example in Bananapi
> panel, and the above gist the pixel is 148Mhz from another panel.

Any further comments?
Maxime Ripard April 2, 2019, 2:39 p.m. UTC | #9 
On Thu, Mar 21, 2019 at 07:40:32PM +0530, Jagan Teki wrote:
> On Tue, Mar 19, 2019 at 4:23 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Mon, Mar 11, 2019 at 09:36:27PM +0530, Jagan Teki wrote:
> > > On Mon, Mar 11, 2019 at 9:08 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Mon, Mar 11, 2019 at 07:06:24PM +0530, Jagan Teki wrote:
> > > > > pll-video => pll-mipi => tcon0 => tcon0-pixel-clock is the typical
> > > > > MIPI clock topology in Allwinner DSI controller.
> > > > >
> > > > > TCON dotclock driver is computing the desired DCLK divider based on
> > > > > panel pixel clock along with input DCLK min, max divider values from
> > > > > tcon driver and that would eventually set the pll-mipi clock rate.
> > > > >
> > > > > The current code allows the TCON clock divider to have a default 4
> > > > > for min, max ranges that would fail to compute the desired pll-mipi
> > > > > rate while supporting new panels.
> > > > >
> > > > > So, add the computation logic 'format/lanes' to dclk min and max dividers
> > > > > and instead of default 4. This computation logic align with Allwinner A64
> > > > > BSP, hoping that would work even for A33.
> > > >
> > > > Last time we discussed this, we found out that this wasn't the case,
> > > > even in the BSP.
> > >
> > > This was the case for BSP to compute pll-mipi not for TCON_DSI clock
> > > register, SUN4I_TCON0_DCLK_REG, which marked the divider 4 by default.
> > >
> > > > What compelling evidence have you found that makes you say otherwise?
> > >
> > > divider 4 isn't worked, this I would mentioned before as well.
> >
> > Maybe you mentionned it before, but it's nowhere to be found in your
> > commit log.
>
> I have added it in 3rd paragraph in commit message, may be you missed
> it or you may look for different text.
>
> "The current code allows the TCON clock divider to have a default 4
> for min, max ranges that would fail to compute the desired pll-mipi
> rate while supporting new panels."

And you're still not explaining *why* that is an issue, and what makes
you think that your solution is the right one.

Seriously, I feel like this discussion is going in circles. I've been
asking for that since your very first version.

> > > Tested this on 4 different panels, and below are the desired divider values
> > > and pll-mipi clock rate with respect to pixel clock frequency.
> > >
> > > - 55MHz pixel clock with 4-lane panel, and the desired DSI clock divider
> > >   is 6 with the output parent clock rate of 330MHz.
> > > - 30MHz pixel clock with 4-lane panel, and the desired DSI clock divider
> > >   is 6 with parent clock rate of 180MHz.
> > > - 27.5Mhz pixel clock with 2-lane pane, and the desired DSI clock divider
> > >   is 12 with the output parent clock rate of 330MHz.
> > > - 147MHz pixel clock with 4-lane panel, and the desired DSI clock divider
> > >   is 6 with the output parent clock rate of 882MHz.
> > >
> > > BSP trying to use this format/lane to compute dsi divider that in-turn
> > > using pll-mipi set_rate but TCON0_DCLK_REG keep constant 4.
> >
> > Feel free to reply to
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/629596.html
>
> Yes, I have replied at that time itself, please check.
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/629673.html
>
> Here I have taken the pixel clock to 30Mhz for example in Bananapi
> panel, and the above gist the pixel is 148Mhz from another panel.

Again, without providing any reference, just making vague statements.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
diff mbox series

Patch

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index e75f77ff8e0f..339f9b1f5745 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -341,8 +341,8 @@  static void sun4i_tcon0_mode_set_cpu(struct sun4i_tcon *tcon,
 	u32 block_space, start_delay;
 	u32 tcon_div;
 
-	tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
-	tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
+	tcon->dclk_min_div = bpp/lanes;
+	tcon->dclk_max_div = bpp/lanes;
 
 	sun4i_tcon0_mode_set_common(tcon, mode);