Message ID | 20190311133637.18334-3-jagan@amarulasolutions.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers | show |
Series | drm/sun4i: Allwinner A64 MIPI-DSI support | expand |
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
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.
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
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
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.
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.
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?
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 --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);