On Tue, Jan 29, 2019 at 11:01:31PM +0530, Jagan Teki wrote: > On Tue, Jan 29, 2019 at 8:43 PM Maxime Ripard <maxime.ripard@xxxxxxxxxxx> wrote: > > > > On Mon, Jan 28, 2019 at 03:06:10PM +0530, Jagan Teki wrote: > > > On Sat, Jan 26, 2019 at 2:54 AM Maxime Ripard <maxime.ripard@xxxxxxxxxxx> wrote: > > > > > > > > On Fri, Jan 25, 2019 at 01:28:49AM +0530, Jagan Teki wrote: > > > > > Minimum PLL used for MIPI is 500MHz, as per manual, but > > > > > lowering the min rate by 300MHz can result proper working > > > > > nkms divider with the help of desired dclock rate from > > > > > panel driver. > > > > > > > > > > Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx> > > > > > Acked-by: Stephen Boyd <sboyd@xxxxxxxxxx> > > > > > > > > Going 200MHz below the minimum doesn't seem really reasonable. What > > > > is the issue that you are trying to fix here? > > > > > > > > It looks like it's picking bad dividers, but if that's the case, this > > > > isn't the proper fix. > > > > > > As I stated in earlier patches, the whole idea is pick the desired > > > dclk divider based dclk rate. So the dotclock, sun4i_dclk_round_rate > > > is unable to get the proper dclk divider at the end, so it eventually > > > picking up wrong divider value and fired vblank timeout. > > > > > > So, we come-up with optimal and working min_rate 300MHz in pll-mipi to > > > get the desired clock something like below. > > > [ 2.415773] [drm] No driver support for vblank timestamp query. > > > [ 2.424116] sun4i_dclk_round_rate: min_div = 4 max_div = 127, rate = 55000000 > > > [ 2.424172] ideal = 220000000, rounded = 0 > > > [ 2.424176] ideal = 275000000, rounded = 0 > > > [ 2.424194] ccu_nkm_round_rate: rate = 330000000 > > > [ 2.424197] ideal = 330000000, rounded = 330000000 > > > [ 2.424201] sun4i_dclk_round_rate: div = 6 rate = 55000000 > > > [ 2.424205] sun4i_dclk_round_rate: min_div = 4 max_div = 127, rate = 55000000 > > > [ 2.424209] ideal = 220000000, rounded = 0 > > > [ 2.424213] ideal = 275000000, rounded = 0 > > > [ 2.424230] ccu_nkm_round_rate: rate = 330000000 > > > [ 2.424233] ideal = 330000000, rounded = 330000000 > > > [ 2.424236] sun4i_dclk_round_rate: div = 6 rate = 55000000 > > > [ 2.424253] ccu_nkm_round_rate: rate = 330000000 > > > [ 2.424270] ccu_nkm_round_rate: rate = 330000000 > > > [ 2.424278] sun4i_dclk_recalc_rate: val = 1, rate = 330000000 > > > [ 2.424281] sun4i_dclk_recalc_rate: val = 1, rate = 330000000 > > > [ 2.424306] ccu_nkm_set_rate: rate = 330000000, parent_rate = 297000000 > > > [ 2.424309] ccu_nkm_set_rate: _nkm.n = 5 > > > [ 2.424311] ccu_nkm_set_rate: _nkm.k = 2 > > > [ 2.424313] ccu_nkm_set_rate: _nkm.m = 9 > > > [ 2.424661] sun4i_dclk_set_rate div 6 > > > [ 2.424668] sun4i_dclk_recalc_rate: val = 6, rate = 55000000 > > > > > > But look like this wouldn't valid for all other dclock rates, say BPI > > > panel has 30MHz clock that would failed with this logic. > > > > > > On the other side Allwinner BSP calculating dclk divider based on the > > > SoC's. for A33 [1] it is fixed dclk divider of 4 and for A64 is is > > > calculated based on the bpp/lanes. > > > > It looks like the A64 has the same divider of 4: > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c#L12 > > > > I think you're confusing it with the ratio between the pixel clock and > > the dotclock, called dsi_div: > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/disp_al.c#L198 > > Ahh.. I thought this initially but as far as DSI clock computation is > concern, the L12 tcon_div is local variable which is used for edge0 > computation in burst mode and not for the dsi clock computation. Since > the BSP is unable to get the tcon_div during edge0 computation, they > defined it locally I think. > > You can see the lcd_clk_config() code [2], where we can see DSI clock > computation using dsi_div value. > > Here is dump after the in Line 792 which is after computation[3] > [ 10.800737] lcd_clk_config: dsi_div = 6, tcon_div = 4, lcd_div = 1 > [ 10.800743] lcd_clk_config: lcd_dclk_freq = 55, dclk_rate = 55000000 > [ 10.800749] lcd_clk_config: lcd_rate = 330000000, pll_rate = 330000000 > > The above dump the lcd_rate 330MHz is computed with panel clock, 55MHz > into dsi_div 6. So this can be our actual divider values dclk_min_div, > dclk_max_div in sun4i_dclk_round_rate (from > drivers/gpu/drm/sun4i/sun4i_dotclock.c) I wish it was in your commit log in the first place, instead of having to exchange multiple mails over this. However, I don't think that's quite true, and it might be a bug in Allwinner's implementation (or rather something quite confusing). You're right that the lcd_rate and pll_rate seem to be generated from the pixel clock, and it indeed looks like the ratio between the pixel clock and the TCON dotclock is defined through the number of bits per lanes. However, in this case, dsi_rate is actually the same than lcd_rate, since pll_rate is going to be divided by dsi_div: https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L791 Since lcd_div is 1, it also means that in this case, dsi_rate == dclk_rate. The DSI module clock however, is always set to 148.5 MHz. Indeed, if we look at: https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L804 We can see that the rate in clk_info is used if it's different than 0. This is filled by disp_al_lcd_get_clk_info, which, in the case of a DSI panel, will hardcode it to 148.5 MHz: https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/disp_al.c#L164 So, the DSI clock is set to this here: https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L805 The TCON *module* clock (the one in the clock controller) has been set to lcd_rate (so the pixel clock times the number of bits per lane) here: https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L800 And the PLL has been set to the same rate here: https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L794 Let's take a step back now: that function we were looking at, lcd_clk_config, is called by lcd_clk_enable, which is in turn called by disp_lcd_enable here: https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L1328 The next function being called is disp_al_lcd_cfg, and that function will hardcode the TCON dotclock divider to 4, here: https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/disp_al.c#L240 So, in the end, the dotclock divider is always 4, the DSI module clock is set to 148.5 MHz, and the TCON module clock is set to 330MHz. Since the TCON module clock doesn't have a divider, the PLL is set at that same value but this is redundant. I'll experiment with this and try to see how it works on the A33. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Attachment:
signature.asc
Description: PGP signature
