[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-08-28 Thread Michael Nazzareno Trimarchi
Hi Maxime

On Wed, Aug 28, 2019 at 3:03 PM Maxime Ripard  wrote:
>
> Hi,
>
> On Thu, Aug 15, 2019 at 02:25:57PM +0200, Michael Nazzareno Trimarchi wrote:
> > On Tue, Aug 13, 2019 at 8:05 AM Maxime Ripard  
> > wrote:
> > > On Mon, Jul 29, 2019 at 08:59:04AM +0200, Michael Nazzareno Trimarchi 
> > > wrote:
> > > > Hi
> > > >
> > > > On Wed, Jul 24, 2019 at 11:05 AM Maxime Ripard
> > > >  wrote:
> > > > >
> > > > > On Mon, Jul 22, 2019 at 03:51:04PM +0530, Jagan Teki wrote:
> > > > > > Hi Maxime,
> > > > > >
> > > > > > On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard 
> > > > > >  wrote:
> > > > > > >
> > > > > > > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > > > > > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno 
> > > > > > > > > Trimarchi wrote:
> > > > > > > > > > > > tcon-pixel clock is the rate that you want to achive on 
> > > > > > > > > > > > display side
> > > > > > > > > > > > and if you have 4 lanes 32bit or lanes and different 
> > > > > > > > > > > > bit number that
> > > > > > > > > > > > you need to have a clock that is able to put outside 
> > > > > > > > > > > > bits and speed
> > > > > > > > > > > > equal to pixel-clock * bits / lanes. so If you want a 
> > > > > > > > > > > > pixel-clock of
> > > > > > > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have 
> > > > > > > > > > > > a clock of
> > > > > > > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done 
> > > > > > > > > > > > but most of
> > > > > > > > > > > > the display.
> > > > > > > > > > >
> > > > > > > > > > > So this is what the issue is then?
> > > > > > > > > > >
> > > > > > > > > > > This one does make sense, and you should just change the 
> > > > > > > > > > > rate in the
> > > > > > > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > > > > > > >
> > > > > > > > > > > I'm still wondering why that hasn't been brought up in 
> > > > > > > > > > > either the
> > > > > > > > > > > discussion or the commit log before though.
> > > > > > > > > > >
> > > > > > > > > > Something like this?
> > > > > > > > > >
> > > > > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 
> > > > > > > > > > +++-
> > > > > > > > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> > > > > > > > > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > > > > > > > > >
> > > > > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > > > @@ -263,10 +263,11 @@ static int 
> > > > > > > > > > sun4i_tcon_get_clk_delay(const struct
> > > > > > > > > > drm_display_mode *mode,
> > > > > > > > > >  }
> > > > > > > > > >
> > > > > > > > > >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon 
> > > > > > > > > > *tcon,
> > > > > > > > > > -   const struct 
> > > > > > > > > > drm_display_mode *mode)
> > > > > > > > > > +   const struct 
> > > > > > > > > > drm_display_mode *mode,
> > > > > > > > > > +   u32 tcon_mul)
> > > > > > > > > >  {
> > > > > > > > > > /* Configure the dot clock */
> > > > > > > > > > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > > > > > > +   clk_set_rate(tcon->dclk, mode->crtc_clock * 
> > > > > > > > > > tcon_mul * 1000);
> > > > > > > > > >
> > > > > > > > > > /* Set the resolution */
> > > > > > > > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > > > > > > @@ -335,12 +336,13 @@ static void 
> > > > > > > > > > sun4i_tcon0_mode_set_cpu(struct
> > > > > > > > > > sun4i_tcon *tcon,
> > > > > > > > > > u8 bpp = 
> > > > > > > > > > mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > > > > > > u8 lanes = device->lanes;
> > > > > > > > > > u32 block_space, start_delay;
> > > > > > > > > > -   u32 tcon_div;
> > > > > > > > > > +   u32 tcon_div, tcon_mul;
> > > > > > > > > >
> > > > > > > > > > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > > > > > > > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > > > > > > > +   tcon->dclk_min_div = 4;
> > > > > > > > > > +   tcon->dclk_max_div = 127;
> > > > > > > > > >
> > > > > > > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > > > > +   tcon_mul = bpp / lanes;
> > > > > > > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > > > > > > > >
> > > > > > > > > > /* Set dithering if needed */
> > > > > > > > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > > > > > > > sun4i_tcon_get_connector(encoder));
> > > > > > > > > > @@ -366,7 +368,7 @@ 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-08-28 Thread Maxime Ripard
Hi,

On Thu, Aug 15, 2019 at 02:25:57PM +0200, Michael Nazzareno Trimarchi wrote:
> On Tue, Aug 13, 2019 at 8:05 AM Maxime Ripard  
> wrote:
> > On Mon, Jul 29, 2019 at 08:59:04AM +0200, Michael Nazzareno Trimarchi wrote:
> > > Hi
> > >
> > > On Wed, Jul 24, 2019 at 11:05 AM Maxime Ripard
> > >  wrote:
> > > >
> > > > On Mon, Jul 22, 2019 at 03:51:04PM +0530, Jagan Teki wrote:
> > > > > Hi Maxime,
> > > > >
> > > > > On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard 
> > > > >  wrote:
> > > > > >
> > > > > > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > > > > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno 
> > > > > > > > Trimarchi wrote:
> > > > > > > > > > > tcon-pixel clock is the rate that you want to achive on 
> > > > > > > > > > > display side
> > > > > > > > > > > and if you have 4 lanes 32bit or lanes and different bit 
> > > > > > > > > > > number that
> > > > > > > > > > > you need to have a clock that is able to put outside bits 
> > > > > > > > > > > and speed
> > > > > > > > > > > equal to pixel-clock * bits / lanes. so If you want a 
> > > > > > > > > > > pixel-clock of
> > > > > > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a 
> > > > > > > > > > > clock of
> > > > > > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done 
> > > > > > > > > > > but most of
> > > > > > > > > > > the display.
> > > > > > > > > >
> > > > > > > > > > So this is what the issue is then?
> > > > > > > > > >
> > > > > > > > > > This one does make sense, and you should just change the 
> > > > > > > > > > rate in the
> > > > > > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > > > > > >
> > > > > > > > > > I'm still wondering why that hasn't been brought up in 
> > > > > > > > > > either the
> > > > > > > > > > discussion or the commit log before though.
> > > > > > > > > >
> > > > > > > > > Something like this?
> > > > > > > > >
> > > > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 
> > > > > > > > > +++-
> > > > > > > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> > > > > > > > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > > @@ -263,10 +263,11 @@ static int 
> > > > > > > > > sun4i_tcon_get_clk_delay(const struct
> > > > > > > > > drm_display_mode *mode,
> > > > > > > > >  }
> > > > > > > > >
> > > > > > > > >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon 
> > > > > > > > > *tcon,
> > > > > > > > > -   const struct 
> > > > > > > > > drm_display_mode *mode)
> > > > > > > > > +   const struct 
> > > > > > > > > drm_display_mode *mode,
> > > > > > > > > +   u32 tcon_mul)
> > > > > > > > >  {
> > > > > > > > > /* Configure the dot clock */
> > > > > > > > > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > > > > > +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul 
> > > > > > > > > * 1000);
> > > > > > > > >
> > > > > > > > > /* Set the resolution */
> > > > > > > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > > > > > @@ -335,12 +336,13 @@ static void 
> > > > > > > > > sun4i_tcon0_mode_set_cpu(struct
> > > > > > > > > sun4i_tcon *tcon,
> > > > > > > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > > > > > u8 lanes = device->lanes;
> > > > > > > > > u32 block_space, start_delay;
> > > > > > > > > -   u32 tcon_div;
> > > > > > > > > +   u32 tcon_div, tcon_mul;
> > > > > > > > >
> > > > > > > > > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > > > > > > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > > > > > > +   tcon->dclk_min_div = 4;
> > > > > > > > > +   tcon->dclk_max_div = 127;
> > > > > > > > >
> > > > > > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > > > +   tcon_mul = bpp / lanes;
> > > > > > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > > > > > > >
> > > > > > > > > /* Set dithering if needed */
> > > > > > > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > > > > > > sun4i_tcon_get_connector(encoder));
> > > > > > > > > @@ -366,7 +368,7 @@ static void 
> > > > > > > > > sun4i_tcon0_mode_set_cpu(struct
> > > > > > > > > sun4i_tcon *tcon,
> > > > > > > > >  */
> > > > > > > > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, 
> > > > > > > > > _div);
> > > > > > > > > tcon_div &= GENMASK(6, 0);
> > > > > > > > > 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-08-15 Thread Michael Nazzareno Trimarchi
Hi Maxime

On Tue, Aug 13, 2019 at 8:05 AM Maxime Ripard  wrote:
>
> On Mon, Jul 29, 2019 at 08:59:04AM +0200, Michael Nazzareno Trimarchi wrote:
> > Hi
> >
> > On Wed, Jul 24, 2019 at 11:05 AM Maxime Ripard
> >  wrote:
> > >
> > > On Mon, Jul 22, 2019 at 03:51:04PM +0530, Jagan Teki wrote:
> > > > Hi Maxime,
> > > >
> > > > On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard 
> > > >  wrote:
> > > > >
> > > > > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > > > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > > > > >  wrote:
> > > > > > >
> > > > > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno 
> > > > > > > Trimarchi wrote:
> > > > > > > > > > tcon-pixel clock is the rate that you want to achive on 
> > > > > > > > > > display side
> > > > > > > > > > and if you have 4 lanes 32bit or lanes and different bit 
> > > > > > > > > > number that
> > > > > > > > > > you need to have a clock that is able to put outside bits 
> > > > > > > > > > and speed
> > > > > > > > > > equal to pixel-clock * bits / lanes. so If you want a 
> > > > > > > > > > pixel-clock of
> > > > > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a 
> > > > > > > > > > clock of
> > > > > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but 
> > > > > > > > > > most of
> > > > > > > > > > the display.
> > > > > > > > >
> > > > > > > > > So this is what the issue is then?
> > > > > > > > >
> > > > > > > > > This one does make sense, and you should just change the rate 
> > > > > > > > > in the
> > > > > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > > > > >
> > > > > > > > > I'm still wondering why that hasn't been brought up in either 
> > > > > > > > > the
> > > > > > > > > discussion or the commit log before though.
> > > > > > > > >
> > > > > > > > Something like this?
> > > > > > > >
> > > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > > > > > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> > > > > > > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const 
> > > > > > > > struct
> > > > > > > > drm_display_mode *mode,
> > > > > > > >  }
> > > > > > > >
> > > > > > > >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon 
> > > > > > > > *tcon,
> > > > > > > > -   const struct 
> > > > > > > > drm_display_mode *mode)
> > > > > > > > +   const struct 
> > > > > > > > drm_display_mode *mode,
> > > > > > > > +   u32 tcon_mul)
> > > > > > > >  {
> > > > > > > > /* Configure the dot clock */
> > > > > > > > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > > > > +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 
> > > > > > > > 1000);
> > > > > > > >
> > > > > > > > /* Set the resolution */
> > > > > > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > > > > @@ -335,12 +336,13 @@ static void 
> > > > > > > > sun4i_tcon0_mode_set_cpu(struct
> > > > > > > > sun4i_tcon *tcon,
> > > > > > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > > > > u8 lanes = device->lanes;
> > > > > > > > u32 block_space, start_delay;
> > > > > > > > -   u32 tcon_div;
> > > > > > > > +   u32 tcon_div, tcon_mul;
> > > > > > > >
> > > > > > > > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > > > > > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > > > > > +   tcon->dclk_min_div = 4;
> > > > > > > > +   tcon->dclk_max_div = 127;
> > > > > > > >
> > > > > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > > +   tcon_mul = bpp / lanes;
> > > > > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > > > > > >
> > > > > > > > /* Set dithering if needed */
> > > > > > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > > > > > sun4i_tcon_get_connector(encoder));
> > > > > > > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > > > sun4i_tcon *tcon,
> > > > > > > >  */
> > > > > > > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, 
> > > > > > > > _div);
> > > > > > > > tcon_div &= GENMASK(6, 0);
> > > > > > > > -   block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > > > > > > +   block_space = mode->htotal * tcon_div * tcon_mul;
> > > > > > > > block_space -= mode->hdisplay + 40;
> > > > > > > >
> > > > > > > > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > > > > > > @@ -408,7 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-08-13 Thread Maxime Ripard
On Mon, Jul 29, 2019 at 08:59:04AM +0200, Michael Nazzareno Trimarchi wrote:
> Hi
>
> On Wed, Jul 24, 2019 at 11:05 AM Maxime Ripard
>  wrote:
> >
> > On Mon, Jul 22, 2019 at 03:51:04PM +0530, Jagan Teki wrote:
> > > Hi Maxime,
> > >
> > > On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard  
> > > wrote:
> > > >
> > > > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > > > >  wrote:
> > > > > >
> > > > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno 
> > > > > > Trimarchi wrote:
> > > > > > > > > tcon-pixel clock is the rate that you want to achive on 
> > > > > > > > > display side
> > > > > > > > > and if you have 4 lanes 32bit or lanes and different bit 
> > > > > > > > > number that
> > > > > > > > > you need to have a clock that is able to put outside bits and 
> > > > > > > > > speed
> > > > > > > > > equal to pixel-clock * bits / lanes. so If you want a 
> > > > > > > > > pixel-clock of
> > > > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a 
> > > > > > > > > clock of
> > > > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but 
> > > > > > > > > most of
> > > > > > > > > the display.
> > > > > > > >
> > > > > > > > So this is what the issue is then?
> > > > > > > >
> > > > > > > > This one does make sense, and you should just change the rate 
> > > > > > > > in the
> > > > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > > > >
> > > > > > > > I'm still wondering why that hasn't been brought up in either 
> > > > > > > > the
> > > > > > > > discussion or the commit log before though.
> > > > > > > >
> > > > > > > Something like this?
> > > > > > >
> > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > > > > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> > > > > > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const 
> > > > > > > struct
> > > > > > > drm_display_mode *mode,
> > > > > > >  }
> > > > > > >
> > > > > > >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > > > > -   const struct 
> > > > > > > drm_display_mode *mode)
> > > > > > > +   const struct 
> > > > > > > drm_display_mode *mode,
> > > > > > > +   u32 tcon_mul)
> > > > > > >  {
> > > > > > > /* Configure the dot clock */
> > > > > > > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > > > +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 
> > > > > > > 1000);
> > > > > > >
> > > > > > > /* Set the resolution */
> > > > > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > > > u8 lanes = device->lanes;
> > > > > > > u32 block_space, start_delay;
> > > > > > > -   u32 tcon_div;
> > > > > > > +   u32 tcon_div, tcon_mul;
> > > > > > >
> > > > > > > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > > > > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > > > > +   tcon->dclk_min_div = 4;
> > > > > > > +   tcon->dclk_max_div = 127;
> > > > > > >
> > > > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > +   tcon_mul = bpp / lanes;
> > > > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > > > > >
> > > > > > > /* Set dithering if needed */
> > > > > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > > > > sun4i_tcon_get_connector(encoder));
> > > > > > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > >  */
> > > > > > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, _div);
> > > > > > > tcon_div &= GENMASK(6, 0);
> > > > > > > -   block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > > > > > +   block_space = mode->htotal * tcon_div * tcon_mul;
> > > > > > > block_space -= mode->hdisplay + 40;
> > > > > > >
> > > > > > > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > > > > > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > >
> > > > > > > tcon->dclk_min_div = 7;
> > > > > > > tcon->dclk_max_div = 7;
> > > > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-08-02 Thread Michael Nazzareno Trimarchi
Hi Maxime

On Mon, Jul 29, 2019 at 8:59 AM Michael Nazzareno Trimarchi
 wrote:
>
> Hi
>
> On Wed, Jul 24, 2019 at 11:05 AM Maxime Ripard
>  wrote:
> >
> > On Mon, Jul 22, 2019 at 03:51:04PM +0530, Jagan Teki wrote:
> > > Hi Maxime,
> > >
> > > On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard  
> > > wrote:
> > > >
> > > > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > > > >  wrote:
> > > > > >
> > > > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno 
> > > > > > Trimarchi wrote:
> > > > > > > > > tcon-pixel clock is the rate that you want to achive on 
> > > > > > > > > display side
> > > > > > > > > and if you have 4 lanes 32bit or lanes and different bit 
> > > > > > > > > number that
> > > > > > > > > you need to have a clock that is able to put outside bits and 
> > > > > > > > > speed
> > > > > > > > > equal to pixel-clock * bits / lanes. so If you want a 
> > > > > > > > > pixel-clock of
> > > > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a 
> > > > > > > > > clock of
> > > > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but 
> > > > > > > > > most of
> > > > > > > > > the display.
> > > > > > > >
> > > > > > > > So this is what the issue is then?
> > > > > > > >
> > > > > > > > This one does make sense, and you should just change the rate 
> > > > > > > > in the
> > > > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > > > >
> > > > > > > > I'm still wondering why that hasn't been brought up in either 
> > > > > > > > the
> > > > > > > > discussion or the commit log before though.
> > > > > > > >
> > > > > > > Something like this?
> > > > > > >
> > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > > > > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> > > > > > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const 
> > > > > > > struct
> > > > > > > drm_display_mode *mode,
> > > > > > >  }
> > > > > > >
> > > > > > >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > > > > -   const struct 
> > > > > > > drm_display_mode *mode)
> > > > > > > +   const struct 
> > > > > > > drm_display_mode *mode,
> > > > > > > +   u32 tcon_mul)
> > > > > > >  {
> > > > > > > /* Configure the dot clock */
> > > > > > > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > > > +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 
> > > > > > > 1000);
> > > > > > >
> > > > > > > /* Set the resolution */
> > > > > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > > > u8 lanes = device->lanes;
> > > > > > > u32 block_space, start_delay;
> > > > > > > -   u32 tcon_div;
> > > > > > > +   u32 tcon_div, tcon_mul;
> > > > > > >
> > > > > > > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > > > > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > > > > +   tcon->dclk_min_div = 4;
> > > > > > > +   tcon->dclk_max_div = 127;
> > > > > > >
> > > > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > +   tcon_mul = bpp / lanes;
> > > > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > > > > >
> > > > > > > /* Set dithering if needed */
> > > > > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > > > > sun4i_tcon_get_connector(encoder));
> > > > > > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > >  */
> > > > > > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, _div);
> > > > > > > tcon_div &= GENMASK(6, 0);
> > > > > > > -   block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > > > > > +   block_space = mode->htotal * tcon_div * tcon_mul;
> > > > > > > block_space -= mode->hdisplay + 40;
> > > > > > >
> > > > > > > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > > > > > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > >
> > > > > > > tcon->dclk_min_div = 7;
> > > > > > > tcon->dclk_max_div = 7;
> > > > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-07-29 Thread Michael Nazzareno Trimarchi
Hi

On Wed, Jul 24, 2019 at 11:05 AM Maxime Ripard
 wrote:
>
> On Mon, Jul 22, 2019 at 03:51:04PM +0530, Jagan Teki wrote:
> > Hi Maxime,
> >
> > On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard  
> > wrote:
> > >
> > > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > > >  wrote:
> > > > >
> > > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi 
> > > > > wrote:
> > > > > > > > tcon-pixel clock is the rate that you want to achive on display 
> > > > > > > > side
> > > > > > > > and if you have 4 lanes 32bit or lanes and different bit number 
> > > > > > > > that
> > > > > > > > you need to have a clock that is able to put outside bits and 
> > > > > > > > speed
> > > > > > > > equal to pixel-clock * bits / lanes. so If you want a 
> > > > > > > > pixel-clock of
> > > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock 
> > > > > > > > of
> > > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but 
> > > > > > > > most of
> > > > > > > > the display.
> > > > > > >
> > > > > > > So this is what the issue is then?
> > > > > > >
> > > > > > > This one does make sense, and you should just change the rate in 
> > > > > > > the
> > > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > > >
> > > > > > > I'm still wondering why that hasn't been brought up in either the
> > > > > > > discussion or the commit log before though.
> > > > > > >
> > > > > > Something like this?
> > > > > >
> > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > > > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> > > > > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const 
> > > > > > struct
> > > > > > drm_display_mode *mode,
> > > > > >  }
> > > > > >
> > > > > >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > > > -   const struct 
> > > > > > drm_display_mode *mode)
> > > > > > +   const struct 
> > > > > > drm_display_mode *mode,
> > > > > > +   u32 tcon_mul)
> > > > > >  {
> > > > > > /* Configure the dot clock */
> > > > > > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > > +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 
> > > > > > 1000);
> > > > > >
> > > > > > /* Set the resolution */
> > > > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > sun4i_tcon *tcon,
> > > > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > > u8 lanes = device->lanes;
> > > > > > u32 block_space, start_delay;
> > > > > > -   u32 tcon_div;
> > > > > > +   u32 tcon_div, tcon_mul;
> > > > > >
> > > > > > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > > > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > > > +   tcon->dclk_min_div = 4;
> > > > > > +   tcon->dclk_max_div = 127;
> > > > > >
> > > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > +   tcon_mul = bpp / lanes;
> > > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > > > >
> > > > > > /* Set dithering if needed */
> > > > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > > > sun4i_tcon_get_connector(encoder));
> > > > > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > sun4i_tcon *tcon,
> > > > > >  */
> > > > > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, _div);
> > > > > > tcon_div &= GENMASK(6, 0);
> > > > > > -   block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > > > > +   block_space = mode->htotal * tcon_div * tcon_mul;
> > > > > > block_space -= mode->hdisplay + 40;
> > > > > >
> > > > > > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > > > > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > > > > sun4i_tcon *tcon,
> > > > > >
> > > > > > tcon->dclk_min_div = 7;
> > > > > > tcon->dclk_max_div = 7;
> > > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > > > >
> > > > > > /* Set dithering if needed */
> > > > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > > > sun4i_tcon_get_connector(encoder));
> > > > > > @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> > > > > > sun4i_tcon *tcon,
> > > > > >
> > > > > > 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-07-24 Thread Maxime Ripard
On Mon, Jul 22, 2019 at 03:51:04PM +0530, Jagan Teki wrote:
> Hi Maxime,
>
> On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard  
> wrote:
> >
> > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > >  wrote:
> > > >
> > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi 
> > > > wrote:
> > > > > > > tcon-pixel clock is the rate that you want to achive on display 
> > > > > > > side
> > > > > > > and if you have 4 lanes 32bit or lanes and different bit number 
> > > > > > > that
> > > > > > > you need to have a clock that is able to put outside bits and 
> > > > > > > speed
> > > > > > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock 
> > > > > > > of
> > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but most 
> > > > > > > of
> > > > > > > the display.
> > > > > >
> > > > > > So this is what the issue is then?
> > > > > >
> > > > > > This one does make sense, and you should just change the rate in the
> > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > >
> > > > > > I'm still wondering why that hasn't been brought up in either the
> > > > > > discussion or the commit log before though.
> > > > > >
> > > > > Something like this?
> > > > >
> > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> > > > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> > > > > drm_display_mode *mode,
> > > > >  }
> > > > >
> > > > >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > > -   const struct drm_display_mode 
> > > > > *mode)
> > > > > +   const struct drm_display_mode 
> > > > > *mode,
> > > > > +   u32 tcon_mul)
> > > > >  {
> > > > > /* Configure the dot clock */
> > > > > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
> > > > >
> > > > > /* Set the resolution */
> > > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > sun4i_tcon *tcon,
> > > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > u8 lanes = device->lanes;
> > > > > u32 block_space, start_delay;
> > > > > -   u32 tcon_div;
> > > > > +   u32 tcon_div, tcon_mul;
> > > > >
> > > > > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > > +   tcon->dclk_min_div = 4;
> > > > > +   tcon->dclk_max_div = 127;
> > > > >
> > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > +   tcon_mul = bpp / lanes;
> > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > > >
> > > > > /* Set dithering if needed */
> > > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > > sun4i_tcon_get_connector(encoder));
> > > > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > sun4i_tcon *tcon,
> > > > >  */
> > > > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, _div);
> > > > > tcon_div &= GENMASK(6, 0);
> > > > > -   block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > > > +   block_space = mode->htotal * tcon_div * tcon_mul;
> > > > > block_space -= mode->hdisplay + 40;
> > > > >
> > > > > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > > > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > > > sun4i_tcon *tcon,
> > > > >
> > > > > tcon->dclk_min_div = 7;
> > > > > tcon->dclk_max_div = 7;
> > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > > >
> > > > > /* Set dithering if needed */
> > > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > > sun4i_tcon_get_connector(encoder));
> > > > > @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> > > > > sun4i_tcon *tcon,
> > > > >
> > > > > tcon->dclk_min_div = 6;
> > > > > tcon->dclk_max_div = 127;
> > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > > >
> > > > > /* Set dithering if needed */
> > > > > sun4i_tcon0_mode_set_dithering(tcon, connector);
> > > > > diff 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-07-22 Thread Jagan Teki
On Mon, Jul 22, 2019 at 3:55 PM Michael Nazzareno Trimarchi
 wrote:
>
> Hi Jagan
>
> On Mon, Jul 22, 2019 at 12:21 PM Jagan Teki  
> wrote:
> >
> > Hi Maxime,
> >
> > On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard  
> > wrote:
> > >
> > > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > > >  wrote:
> > > > >
> > > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi 
> > > > > wrote:
> > > > > > > > tcon-pixel clock is the rate that you want to achive on display 
> > > > > > > > side
> > > > > > > > and if you have 4 lanes 32bit or lanes and different bit number 
> > > > > > > > that
> > > > > > > > you need to have a clock that is able to put outside bits and 
> > > > > > > > speed
> > > > > > > > equal to pixel-clock * bits / lanes. so If you want a 
> > > > > > > > pixel-clock of
> > > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock 
> > > > > > > > of
> > > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but 
> > > > > > > > most of
> > > > > > > > the display.
> > > > > > >
> > > > > > > So this is what the issue is then?
> > > > > > >
> > > > > > > This one does make sense, and you should just change the rate in 
> > > > > > > the
> > > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > > >
> > > > > > > I'm still wondering why that hasn't been brought up in either the
> > > > > > > discussion or the commit log before though.
> > > > > > >
> > > > > > Something like this?
> > > > > >
> > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > > > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> > > > > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const 
> > > > > > struct
> > > > > > drm_display_mode *mode,
> > > > > >  }
> > > > > >
> > > > > >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > > > -   const struct 
> > > > > > drm_display_mode *mode)
> > > > > > +   const struct 
> > > > > > drm_display_mode *mode,
> > > > > > +   u32 tcon_mul)
> > > > > >  {
> > > > > > /* Configure the dot clock */
> > > > > > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > > +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 
> > > > > > 1000);
> > > > > >
> > > > > > /* Set the resolution */
> > > > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > sun4i_tcon *tcon,
> > > > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > > u8 lanes = device->lanes;
> > > > > > u32 block_space, start_delay;
> > > > > > -   u32 tcon_div;
> > > > > > +   u32 tcon_div, tcon_mul;
> > > > > >
> > > > > > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > > > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > > > +   tcon->dclk_min_div = 4;
> > > > > > +   tcon->dclk_max_div = 127;
> > > > > >
> > > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > +   tcon_mul = bpp / lanes;
> > > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > > > >
> > > > > > /* Set dithering if needed */
> > > > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > > > sun4i_tcon_get_connector(encoder));
> > > > > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > sun4i_tcon *tcon,
> > > > > >  */
> > > > > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, _div);
> > > > > > tcon_div &= GENMASK(6, 0);
> > > > > > -   block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > > > > +   block_space = mode->htotal * tcon_div * tcon_mul;
> > > > > > block_space -= mode->hdisplay + 40;
> > > > > >
> > > > > > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > > > > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > > > > sun4i_tcon *tcon,
> > > > > >
> > > > > > tcon->dclk_min_div = 7;
> > > > > > tcon->dclk_max_div = 7;
> > > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > > > >
> > > > > > /* Set dithering if needed */
> > > > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > > > sun4i_tcon_get_connector(encoder));
> > > > > > @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> > > > > > sun4i_tcon *tcon,
> > > > > >
> 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-07-22 Thread Michael Nazzareno Trimarchi
Hi Jagan

On Mon, Jul 22, 2019 at 12:21 PM Jagan Teki  wrote:
>
> Hi Maxime,
>
> On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard  
> wrote:
> >
> > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > >  wrote:
> > > >
> > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi 
> > > > wrote:
> > > > > > > tcon-pixel clock is the rate that you want to achive on display 
> > > > > > > side
> > > > > > > and if you have 4 lanes 32bit or lanes and different bit number 
> > > > > > > that
> > > > > > > you need to have a clock that is able to put outside bits and 
> > > > > > > speed
> > > > > > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock 
> > > > > > > of
> > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but most 
> > > > > > > of
> > > > > > > the display.
> > > > > >
> > > > > > So this is what the issue is then?
> > > > > >
> > > > > > This one does make sense, and you should just change the rate in the
> > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > >
> > > > > > I'm still wondering why that hasn't been brought up in either the
> > > > > > discussion or the commit log before though.
> > > > > >
> > > > > Something like this?
> > > > >
> > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> > > > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> > > > > drm_display_mode *mode,
> > > > >  }
> > > > >
> > > > >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > > -   const struct drm_display_mode 
> > > > > *mode)
> > > > > +   const struct drm_display_mode 
> > > > > *mode,
> > > > > +   u32 tcon_mul)
> > > > >  {
> > > > > /* Configure the dot clock */
> > > > > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
> > > > >
> > > > > /* Set the resolution */
> > > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > sun4i_tcon *tcon,
> > > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > u8 lanes = device->lanes;
> > > > > u32 block_space, start_delay;
> > > > > -   u32 tcon_div;
> > > > > +   u32 tcon_div, tcon_mul;
> > > > >
> > > > > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > > +   tcon->dclk_min_div = 4;
> > > > > +   tcon->dclk_max_div = 127;
> > > > >
> > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > +   tcon_mul = bpp / lanes;
> > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > > >
> > > > > /* Set dithering if needed */
> > > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > > sun4i_tcon_get_connector(encoder));
> > > > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > sun4i_tcon *tcon,
> > > > >  */
> > > > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, _div);
> > > > > tcon_div &= GENMASK(6, 0);
> > > > > -   block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > > > +   block_space = mode->htotal * tcon_div * tcon_mul;
> > > > > block_space -= mode->hdisplay + 40;
> > > > >
> > > > > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > > > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > > > sun4i_tcon *tcon,
> > > > >
> > > > > tcon->dclk_min_div = 7;
> > > > > tcon->dclk_max_div = 7;
> > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > > >
> > > > > /* Set dithering if needed */
> > > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > > sun4i_tcon_get_connector(encoder));
> > > > > @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> > > > > sun4i_tcon *tcon,
> > > > >
> > > > > tcon->dclk_min_div = 6;
> > > > > tcon->dclk_max_div = 127;
> > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > > >
> > > > > /* Set dithering if needed */
> > > > > sun4i_tcon0_mode_set_dithering(tcon, connector);
> > > > > diff 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-07-22 Thread Jagan Teki
Hi Maxime,

On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard  wrote:
>
> On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> >  wrote:
> > >
> > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi 
> > > wrote:
> > > > > > tcon-pixel clock is the rate that you want to achive on display side
> > > > > > and if you have 4 lanes 32bit or lanes and different bit number that
> > > > > > you need to have a clock that is able to put outside bits and speed
> > > > > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock of
> > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but most of
> > > > > > the display.
> > > > >
> > > > > So this is what the issue is then?
> > > > >
> > > > > This one does make sense, and you should just change the rate in the
> > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > >
> > > > > I'm still wondering why that hasn't been brought up in either the
> > > > > discussion or the commit log before though.
> > > > >
> > > > Something like this?
> > > >
> > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> > > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> > > > drm_display_mode *mode,
> > > >  }
> > > >
> > > >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > -   const struct drm_display_mode 
> > > > *mode)
> > > > +   const struct drm_display_mode 
> > > > *mode,
> > > > +   u32 tcon_mul)
> > > >  {
> > > > /* Configure the dot clock */
> > > > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
> > > >
> > > > /* Set the resolution */
> > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > sun4i_tcon *tcon,
> > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > u8 lanes = device->lanes;
> > > > u32 block_space, start_delay;
> > > > -   u32 tcon_div;
> > > > +   u32 tcon_div, tcon_mul;
> > > >
> > > > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > +   tcon->dclk_min_div = 4;
> > > > +   tcon->dclk_max_div = 127;
> > > >
> > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > +   tcon_mul = bpp / lanes;
> > > > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > >
> > > > /* Set dithering if needed */
> > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > sun4i_tcon_get_connector(encoder));
> > > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > sun4i_tcon *tcon,
> > > >  */
> > > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, _div);
> > > > tcon_div &= GENMASK(6, 0);
> > > > -   block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > > +   block_space = mode->htotal * tcon_div * tcon_mul;
> > > > block_space -= mode->hdisplay + 40;
> > > >
> > > > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > > sun4i_tcon *tcon,
> > > >
> > > > tcon->dclk_min_div = 7;
> > > > tcon->dclk_max_div = 7;
> > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > >
> > > > /* Set dithering if needed */
> > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > sun4i_tcon_get_connector(encoder));
> > > > @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> > > > sun4i_tcon *tcon,
> > > >
> > > > tcon->dclk_min_div = 6;
> > > > tcon->dclk_max_div = 127;
> > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > >
> > > > /* Set dithering if needed */
> > > > sun4i_tcon0_mode_set_dithering(tcon, connector);
> > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > index 5c3ad5be0690..a07090579f84 100644
> > > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > @@ -13,8 +13,6 @@
> > > >  #include 
> > > >  #include 
> > > >
> > > > -#define 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-07-20 Thread Michael Nazzareno Trimarchi
Hi

On Sat, Jul 20, 2019 at 11:32 AM Maxime Ripard
 wrote:
>
> On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> >  wrote:
> > >
> > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi 
> > > wrote:
> > > > > > tcon-pixel clock is the rate that you want to achive on display side
> > > > > > and if you have 4 lanes 32bit or lanes and different bit number that
> > > > > > you need to have a clock that is able to put outside bits and speed
> > > > > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock of
> > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but most of
> > > > > > the display.
> > > > >
> > > > > So this is what the issue is then?
> > > > >
> > > > > This one does make sense, and you should just change the rate in the
> > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > >
> > > > > I'm still wondering why that hasn't been brought up in either the
> > > > > discussion or the commit log before though.
> > > > >
> > > > Something like this?
> > > >
> > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> > > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> > > > drm_display_mode *mode,
> > > >  }
> > > >
> > > >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > -   const struct drm_display_mode 
> > > > *mode)
> > > > +   const struct drm_display_mode 
> > > > *mode,
> > > > +   u32 tcon_mul)
> > > >  {
> > > > /* Configure the dot clock */
> > > > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
> > > >
> > > > /* Set the resolution */
> > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > sun4i_tcon *tcon,
> > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > u8 lanes = device->lanes;
> > > > u32 block_space, start_delay;
> > > > -   u32 tcon_div;
> > > > +   u32 tcon_div, tcon_mul;
> > > >
> > > > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > +   tcon->dclk_min_div = 4;
> > > > +   tcon->dclk_max_div = 127;
> > > >
> > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > +   tcon_mul = bpp / lanes;
> > > > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > >
> > > > /* Set dithering if needed */
> > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > sun4i_tcon_get_connector(encoder));
> > > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > sun4i_tcon *tcon,
> > > >  */
> > > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, _div);
> > > > tcon_div &= GENMASK(6, 0);
> > > > -   block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > > +   block_space = mode->htotal * tcon_div * tcon_mul;
> > > > block_space -= mode->hdisplay + 40;
> > > >
> > > > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > > sun4i_tcon *tcon,
> > > >
> > > > tcon->dclk_min_div = 7;
> > > > tcon->dclk_max_div = 7;
> > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > >
> > > > /* Set dithering if needed */
> > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > sun4i_tcon_get_connector(encoder));
> > > > @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> > > > sun4i_tcon *tcon,
> > > >
> > > > tcon->dclk_min_div = 6;
> > > > tcon->dclk_max_div = 127;
> > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > >
> > > > /* Set dithering if needed */
> > > > sun4i_tcon0_mode_set_dithering(tcon, connector);
> > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > index 5c3ad5be0690..a07090579f84 100644
> > > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > @@ -13,8 +13,6 @@
> > > >  #include 
> > > >  #include 
> > > >
> > > > -#define SUN6I_DSI_TCON_DIV

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-07-20 Thread Maxime Ripard
On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
>  wrote:
> >
> > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi wrote:
> > > > > tcon-pixel clock is the rate that you want to achive on display side
> > > > > and if you have 4 lanes 32bit or lanes and different bit number that
> > > > > you need to have a clock that is able to put outside bits and speed
> > > > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock of
> > > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but most of
> > > > > the display.
> > > >
> > > > So this is what the issue is then?
> > > >
> > > > This one does make sense, and you should just change the rate in the
> > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > >
> > > > I'm still wondering why that hasn't been brought up in either the
> > > > discussion or the commit log before though.
> > > >
> > > Something like this?
> > >
> > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > index 64c43ee6bd92..42560d5c327c 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> > > drm_display_mode *mode,
> > >  }
> > >
> > >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > -   const struct drm_display_mode 
> > > *mode)
> > > +   const struct drm_display_mode 
> > > *mode,
> > > +   u32 tcon_mul)
> > >  {
> > > /* Configure the dot clock */
> > > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
> > >
> > > /* Set the resolution */
> > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > sun4i_tcon *tcon,
> > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > u8 lanes = device->lanes;
> > > u32 block_space, start_delay;
> > > -   u32 tcon_div;
> > > +   u32 tcon_div, tcon_mul;
> > >
> > > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > +   tcon->dclk_min_div = 4;
> > > +   tcon->dclk_max_div = 127;
> > >
> > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > +   tcon_mul = bpp / lanes;
> > > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > >
> > > /* Set dithering if needed */
> > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > sun4i_tcon_get_connector(encoder));
> > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > sun4i_tcon *tcon,
> > >  */
> > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, _div);
> > > tcon_div &= GENMASK(6, 0);
> > > -   block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > +   block_space = mode->htotal * tcon_div * tcon_mul;
> > > block_space -= mode->hdisplay + 40;
> > >
> > > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > sun4i_tcon *tcon,
> > >
> > > tcon->dclk_min_div = 7;
> > > tcon->dclk_max_div = 7;
> > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > >
> > > /* Set dithering if needed */
> > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > sun4i_tcon_get_connector(encoder));
> > > @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> > > sun4i_tcon *tcon,
> > >
> > > tcon->dclk_min_div = 6;
> > > tcon->dclk_max_div = 127;
> > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > >
> > > /* Set dithering if needed */
> > > sun4i_tcon0_mode_set_dithering(tcon, connector);
> > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > index 5c3ad5be0690..a07090579f84 100644
> > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > @@ -13,8 +13,6 @@
> > >  #include 
> > >  #include 
> > >
> > > -#define SUN6I_DSI_TCON_DIV 4
> > > -
> > >  struct sun6i_dsi {
> > > struct drm_connectorconnector;
> > > struct drm_encoder  encoder;
> >
> > I had more something like this in mind:
> > http://code.bulix.org/nlp5a4-803511
>
> Worth to look at it. was it working on your panel? meanwhile I 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-07-20 Thread Jagan Teki
On Sat, Jul 20, 2019 at 12:46 PM Jagan Teki  wrote:
>
> On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
>  wrote:
> >
> > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi wrote:
> > > > > tcon-pixel clock is the rate that you want to achive on display side
> > > > > and if you have 4 lanes 32bit or lanes and different bit number that
> > > > > you need to have a clock that is able to put outside bits and speed
> > > > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock of
> > > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but most of
> > > > > the display.
> > > >
> > > > So this is what the issue is then?
> > > >
> > > > This one does make sense, and you should just change the rate in the
> > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > >
> > > > I'm still wondering why that hasn't been brought up in either the
> > > > discussion or the commit log before though.
> > > >
> > > Something like this?
> > >
> > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > index 64c43ee6bd92..42560d5c327c 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> > > drm_display_mode *mode,
> > >  }
> > >
> > >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > -   const struct drm_display_mode 
> > > *mode)
> > > +   const struct drm_display_mode 
> > > *mode,
> > > +   u32 tcon_mul)
> > >  {
> > > /* Configure the dot clock */
> > > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
> > >
> > > /* Set the resolution */
> > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > sun4i_tcon *tcon,
> > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > u8 lanes = device->lanes;
> > > u32 block_space, start_delay;
> > > -   u32 tcon_div;
> > > +   u32 tcon_div, tcon_mul;
> > >
> > > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > +   tcon->dclk_min_div = 4;
> > > +   tcon->dclk_max_div = 127;
> > >
> > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > +   tcon_mul = bpp / lanes;
> > > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > >
> > > /* Set dithering if needed */
> > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > sun4i_tcon_get_connector(encoder));
> > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > sun4i_tcon *tcon,
> > >  */
> > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, _div);
> > > tcon_div &= GENMASK(6, 0);
> > > -   block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > +   block_space = mode->htotal * tcon_div * tcon_mul;
> > > block_space -= mode->hdisplay + 40;
> > >
> > > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > sun4i_tcon *tcon,
> > >
> > > tcon->dclk_min_div = 7;
> > > tcon->dclk_max_div = 7;
> > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > >
> > > /* Set dithering if needed */
> > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > sun4i_tcon_get_connector(encoder));
> > > @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> > > sun4i_tcon *tcon,
> > >
> > > tcon->dclk_min_div = 6;
> > > tcon->dclk_max_div = 127;
> > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > >
> > > /* Set dithering if needed */
> > > sun4i_tcon0_mode_set_dithering(tcon, connector);
> > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > index 5c3ad5be0690..a07090579f84 100644
> > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > @@ -13,8 +13,6 @@
> > >  #include 
> > >  #include 
> > >
> > > -#define SUN6I_DSI_TCON_DIV 4
> > > -
> > >  struct sun6i_dsi {
> > > struct drm_connectorconnector;
> > > struct drm_encoder  encoder;
> >
> > I had more something like this in mind:
> > http://code.bulix.org/nlp5a4-803511
>
> Worth to look at it. was it working on your panel? meanwhile I will 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-07-20 Thread Michael Nazzareno Trimarchi
Hi


On Sat., 20 Jul. 2019, 8:58 am Maxime Ripard, 
wrote:

> On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi
> wrote:
> > > > tcon-pixel clock is the rate that you want to achive on display side
> > > > and if you have 4 lanes 32bit or lanes and different bit number that
> > > > you need to have a clock that is able to put outside bits and speed
> > > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock of
> > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > > 40 * 32 / 4 in no-burst mode. I think that this is done but most of
> > > > the display.
> > >
> > > So this is what the issue is then?
> > >
> > > This one does make sense, and you should just change the rate in the
> > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > >
> > > I'm still wondering why that hasn't been brought up in either the
> > > discussion or the commit log before though.
> > >
> > Something like this?
> >
> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> >  2 files changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > index 64c43ee6bd92..42560d5c327c 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> > drm_display_mode *mode,
> >  }
> >
> >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > -   const struct drm_display_mode
> *mode)
> > +   const struct drm_display_mode
> *mode,
> > +   u32 tcon_mul)
> >  {
> > /* Configure the dot clock */
> > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
> >
> > /* Set the resolution */
> > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > sun4i_tcon *tcon,
> > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > u8 lanes = device->lanes;
> > u32 block_space, start_delay;
> > -   u32 tcon_div;
> > +   u32 tcon_div, tcon_mul;
> >
> > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > +   tcon->dclk_min_div = 4;
> > +   tcon->dclk_max_div = 127;
> >
> > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > +   tcon_mul = bpp / lanes;
> > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> >
> > /* Set dithering if needed */
> > sun4i_tcon0_mode_set_dithering(tcon,
> sun4i_tcon_get_connector(encoder));
> > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > sun4i_tcon *tcon,
> >  */
> > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, _div);
> > tcon_div &= GENMASK(6, 0);
> > -   block_space = mode->htotal * bpp / (tcon_div * lanes);
> > +   block_space = mode->htotal * tcon_div * tcon_mul;
> > block_space -= mode->hdisplay + 40;
> >
> > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > sun4i_tcon *tcon,
> >
> > tcon->dclk_min_div = 7;
> > tcon->dclk_max_div = 7;
> > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> >
> > /* Set dithering if needed */
> > sun4i_tcon0_mode_set_dithering(tcon,
> sun4i_tcon_get_connector(encoder));
> > @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> > sun4i_tcon *tcon,
> >
> > tcon->dclk_min_div = 6;
> > tcon->dclk_max_div = 127;
> > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> >
> > /* Set dithering if needed */
> > sun4i_tcon0_mode_set_dithering(tcon, connector);
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > index 5c3ad5be0690..a07090579f84 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > @@ -13,8 +13,6 @@
> >  #include 
> >  #include 
> >
> > -#define SUN6I_DSI_TCON_DIV 4
> > -
> >  struct sun6i_dsi {
> > struct drm_connectorconnector;
> > struct drm_encoder  encoder;
>
> I had more something like this in mind:
> http://code.bulix.org/nlp5a4-803511


I sent another patch to jagan and I'm fully agree that the proper change
need to show the reason and not change constraints as we discuss

We are on same line

Thank you

>
>
>
>
>
> You really don't need to change the divider range (or this is another
> issue that the one you mentionned).
>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-07-20 Thread Jagan Teki
On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
 wrote:
>
> On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi wrote:
> > > > tcon-pixel clock is the rate that you want to achive on display side
> > > > and if you have 4 lanes 32bit or lanes and different bit number that
> > > > you need to have a clock that is able to put outside bits and speed
> > > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock of
> > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > > 40 * 32 / 4 in no-burst mode. I think that this is done but most of
> > > > the display.
> > >
> > > So this is what the issue is then?
> > >
> > > This one does make sense, and you should just change the rate in the
> > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > >
> > > I'm still wondering why that hasn't been brought up in either the
> > > discussion or the commit log before though.
> > >
> > Something like this?
> >
> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> >  2 files changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > index 64c43ee6bd92..42560d5c327c 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> > drm_display_mode *mode,
> >  }
> >
> >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > -   const struct drm_display_mode *mode)
> > +   const struct drm_display_mode *mode,
> > +   u32 tcon_mul)
> >  {
> > /* Configure the dot clock */
> > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
> >
> > /* Set the resolution */
> > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > sun4i_tcon *tcon,
> > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > u8 lanes = device->lanes;
> > u32 block_space, start_delay;
> > -   u32 tcon_div;
> > +   u32 tcon_div, tcon_mul;
> >
> > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > +   tcon->dclk_min_div = 4;
> > +   tcon->dclk_max_div = 127;
> >
> > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > +   tcon_mul = bpp / lanes;
> > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> >
> > /* Set dithering if needed */
> > sun4i_tcon0_mode_set_dithering(tcon, 
> > sun4i_tcon_get_connector(encoder));
> > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > sun4i_tcon *tcon,
> >  */
> > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, _div);
> > tcon_div &= GENMASK(6, 0);
> > -   block_space = mode->htotal * bpp / (tcon_div * lanes);
> > +   block_space = mode->htotal * tcon_div * tcon_mul;
> > block_space -= mode->hdisplay + 40;
> >
> > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > sun4i_tcon *tcon,
> >
> > tcon->dclk_min_div = 7;
> > tcon->dclk_max_div = 7;
> > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> >
> > /* Set dithering if needed */
> > sun4i_tcon0_mode_set_dithering(tcon, 
> > sun4i_tcon_get_connector(encoder));
> > @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> > sun4i_tcon *tcon,
> >
> > tcon->dclk_min_div = 6;
> > tcon->dclk_max_div = 127;
> > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
> >
> > /* Set dithering if needed */
> > sun4i_tcon0_mode_set_dithering(tcon, connector);
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > index 5c3ad5be0690..a07090579f84 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > @@ -13,8 +13,6 @@
> >  #include 
> >  #include 
> >
> > -#define SUN6I_DSI_TCON_DIV 4
> > -
> >  struct sun6i_dsi {
> > struct drm_connectorconnector;
> > struct drm_encoder  encoder;
>
> I had more something like this in mind:
> http://code.bulix.org/nlp5a4-803511

Worth to look at it. was it working on your panel? meanwhile I will check it.

We have updated with below change [1], seems working on but is
actually checking the each divider as before start with 4... till 127.

This new approach, is start looking the best divider from 4.. based on
the idea vs rounded it will ended up best divider like [2]


[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-07-20 Thread Maxime Ripard
On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi wrote:
> > > tcon-pixel clock is the rate that you want to achive on display side
> > > and if you have 4 lanes 32bit or lanes and different bit number that
> > > you need to have a clock that is able to put outside bits and speed
> > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock of
> > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > 40 * 32 / 4 in no-burst mode. I think that this is done but most of
> > > the display.
> >
> > So this is what the issue is then?
> >
> > This one does make sense, and you should just change the rate in the
> > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> >
> > I'm still wondering why that hasn't been brought up in either the
> > discussion or the commit log before though.
> >
> Something like this?
>
> drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
>  2 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 64c43ee6bd92..42560d5c327c 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> drm_display_mode *mode,
>  }
>
>  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> -   const struct drm_display_mode *mode)
> +   const struct drm_display_mode *mode,
> +   u32 tcon_mul)
>  {
> /* Configure the dot clock */
> -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
>
> /* Set the resolution */
> regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> sun4i_tcon *tcon,
> u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> u8 lanes = device->lanes;
> u32 block_space, start_delay;
> -   u32 tcon_div;
> +   u32 tcon_div, tcon_mul;
>
> -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> +   tcon->dclk_min_div = 4;
> +   tcon->dclk_max_div = 127;
>
> -   sun4i_tcon0_mode_set_common(tcon, mode);
> +   tcon_mul = bpp / lanes;
> +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
>
> /* Set dithering if needed */
> sun4i_tcon0_mode_set_dithering(tcon, 
> sun4i_tcon_get_connector(encoder));
> @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> sun4i_tcon *tcon,
>  */
> regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, _div);
> tcon_div &= GENMASK(6, 0);
> -   block_space = mode->htotal * bpp / (tcon_div * lanes);
> +   block_space = mode->htotal * tcon_div * tcon_mul;
> block_space -= mode->hdisplay + 40;
>
> regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> sun4i_tcon *tcon,
>
> tcon->dclk_min_div = 7;
> tcon->dclk_max_div = 7;
> -   sun4i_tcon0_mode_set_common(tcon, mode);
> +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
>
> /* Set dithering if needed */
> sun4i_tcon0_mode_set_dithering(tcon, 
> sun4i_tcon_get_connector(encoder));
> @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> sun4i_tcon *tcon,
>
> tcon->dclk_min_div = 6;
> tcon->dclk_max_div = 127;
> -   sun4i_tcon0_mode_set_common(tcon, mode);
> +   sun4i_tcon0_mode_set_common(tcon, mode, 1);
>
> /* Set dithering if needed */
> sun4i_tcon0_mode_set_dithering(tcon, connector);
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> index 5c3ad5be0690..a07090579f84 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> @@ -13,8 +13,6 @@
>  #include 
>  #include 
>
> -#define SUN6I_DSI_TCON_DIV 4
> -
>  struct sun6i_dsi {
> struct drm_connectorconnector;
> struct drm_encoder  encoder;

I had more something like this in mind:
http://code.bulix.org/nlp5a4-803511

You really don't need to change the divider range (or this is another
issue that the one you mentionned).

Maxime

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/20190720065830.zn3txpyduakywcva%40flea.


signature.asc
Description: PGP signature


[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-07-11 Thread Michael Nazzareno Trimarchi
Hi Maxime

On Thu, Jul 11, 2019 at 2:23 PM Maxime Ripard  wrote:
>
> On Fri, Jul 05, 2019 at 07:52:27PM +0200, Michael Nazzareno Trimarchi wrote:
> > On Wed, Jul 3, 2019 at 1:49 PM Maxime Ripard  
> > wrote:
> > >
> > > On Tue, Jun 25, 2019 at 09:00:36PM +0530, Jagan Teki wrote:
> > > > On Tue, Jun 25, 2019 at 8:19 PM Maxime Ripard 
> > > >  wrote:
> > > > >
> > > > > On Thu, Jun 20, 2019 at 11:57:44PM +0530, Jagan Teki wrote:
> > > > > > On Fri, Jun 14, 2019 at 7:54 PM Maxime Ripard 
> > > > > >  wrote:
> > > > > > >
> > > > > > > On Wed, Jun 05, 2019 at 01:03:16PM +0530, Jagan Teki wrote:
> > > > > > > > On Wed, Jun 5, 2019 at 12:19 PM Maxime Ripard 
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > I've reordered the mail a bit to work on chunks
> > > > > > > > >
> > > > > > > > > On Fri, May 24, 2019 at 03:37:42PM +0530, Jagan Teki wrote:
> > > > > > > > > > > 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
> > > > > > > > > >
> > > > > > > > > > Let me explain, something more.
> > > > > > > > > >
> > > > > > > > > > According to bsp there are clk_info.tcon_div which I will 
> > > > > > > > > > explain below.
> > > > > > > > > > clk_info.dsi_div which is dynamic and it depends on 
> > > > > > > > > > bpp/lanes, so it
> > > > > > > > > > is 6 for 24bpp and 4 lanes devices.
> > > > > > > > > >
> > > > > > > > > > PLL rate here depends on dsi_div (not tcon_div)
> > > > > > > > > >
> > > > > > > > > > Code here
> > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L784
> > > > > > > > > >
> > > > > > > > > > is computing the actual set rate, which depends on dsi_rate.
> > > > > > > > > >
> > > > > > > > > > lcd_rate = dclk_rate * clk_info.dsi_div;
> > > > > > > > > > dsi_rate = pll_rate / clk_info.dsi_div;
> > > > > > > > > >
> > > > > > > > > > Say if the dclk_rate 148MHz then the dsi_rate is 888MHz 
> > > > > > > > > > which set rate
> > > > > > > > > > for above link you mentioned.
> > > > > > > > > >
> > > > > > > > > > Here are the evidence with some prints.
> > > > > > > > > >
> > > > > > > > > > https://gist.github.com/openedev/9bae2d87d2fcc06b999fe48c998b7043
> > > > > > > > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> > > > > > > > >
> > > > > > > > > Ok, so we agree up to this point, and the prints confirm that 
> > > > > > > > > the
> > > > > > > > > analysis above is the right one.
> > > > > > > > >
> > > > > > > > > > > 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
> > > > > > > > >
> > > > > > > > > Your patch doesn't address that, so let's leave that one 
> > > > > > > > > alone.
> > > > > > > >
> > > > > > > > Basically this is final pll set rate when sun4i_dotclock.c 
> > > > > > > > called the
> > > > > 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-07-11 Thread Maxime Ripard
On Fri, Jul 05, 2019 at 07:52:27PM +0200, Michael Nazzareno Trimarchi wrote:
> On Wed, Jul 3, 2019 at 1:49 PM Maxime Ripard  
> wrote:
> >
> > On Tue, Jun 25, 2019 at 09:00:36PM +0530, Jagan Teki wrote:
> > > On Tue, Jun 25, 2019 at 8:19 PM Maxime Ripard  
> > > wrote:
> > > >
> > > > On Thu, Jun 20, 2019 at 11:57:44PM +0530, Jagan Teki wrote:
> > > > > On Fri, Jun 14, 2019 at 7:54 PM Maxime Ripard 
> > > > >  wrote:
> > > > > >
> > > > > > On Wed, Jun 05, 2019 at 01:03:16PM +0530, Jagan Teki wrote:
> > > > > > > On Wed, Jun 5, 2019 at 12:19 PM Maxime Ripard 
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > I've reordered the mail a bit to work on chunks
> > > > > > > >
> > > > > > > > On Fri, May 24, 2019 at 03:37:42PM +0530, Jagan Teki wrote:
> > > > > > > > > > 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
> > > > > > > > >
> > > > > > > > > Let me explain, something more.
> > > > > > > > >
> > > > > > > > > According to bsp there are clk_info.tcon_div which I will 
> > > > > > > > > explain below.
> > > > > > > > > clk_info.dsi_div which is dynamic and it depends on 
> > > > > > > > > bpp/lanes, so it
> > > > > > > > > is 6 for 24bpp and 4 lanes devices.
> > > > > > > > >
> > > > > > > > > PLL rate here depends on dsi_div (not tcon_div)
> > > > > > > > >
> > > > > > > > > Code here
> > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L784
> > > > > > > > >
> > > > > > > > > is computing the actual set rate, which depends on dsi_rate.
> > > > > > > > >
> > > > > > > > > lcd_rate = dclk_rate * clk_info.dsi_div;
> > > > > > > > > dsi_rate = pll_rate / clk_info.dsi_div;
> > > > > > > > >
> > > > > > > > > Say if the dclk_rate 148MHz then the dsi_rate is 888MHz which 
> > > > > > > > > set rate
> > > > > > > > > for above link you mentioned.
> > > > > > > > >
> > > > > > > > > Here are the evidence with some prints.
> > > > > > > > >
> > > > > > > > > https://gist.github.com/openedev/9bae2d87d2fcc06b999fe48c998b7043
> > > > > > > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> > > > > > > >
> > > > > > > > Ok, so we agree up to this point, and the prints confirm that 
> > > > > > > > the
> > > > > > > > analysis above is the right one.
> > > > > > > >
> > > > > > > > > > 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
> > > > > > > >
> > > > > > > > Your patch doesn't address that, so let's leave that one alone.
> > > > > > >
> > > > > > > Basically this is final pll set rate when sun4i_dotclock.c called 
> > > > > > > the
> > > > > > > desired rate with ccu_nkm.c so it ended the final rate with 
> > > > > > > parent as
> > > > > > > Line 8 of
> > > > > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> > > > > >
> > > > > > If that's important to the driver, it should be set explicitly 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-07-05 Thread Michael Nazzareno Trimarchi
Hi Maxime

On Wed, Jul 3, 2019 at 1:49 PM Maxime Ripard  wrote:
>
> On Tue, Jun 25, 2019 at 09:00:36PM +0530, Jagan Teki wrote:
> > On Tue, Jun 25, 2019 at 8:19 PM Maxime Ripard  
> > wrote:
> > >
> > > On Thu, Jun 20, 2019 at 11:57:44PM +0530, Jagan Teki wrote:
> > > > On Fri, Jun 14, 2019 at 7:54 PM Maxime Ripard 
> > > >  wrote:
> > > > >
> > > > > On Wed, Jun 05, 2019 at 01:03:16PM +0530, Jagan Teki wrote:
> > > > > > On Wed, Jun 5, 2019 at 12:19 PM Maxime Ripard 
> > > > > >  wrote:
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > I've reordered the mail a bit to work on chunks
> > > > > > >
> > > > > > > On Fri, May 24, 2019 at 03:37:42PM +0530, Jagan Teki wrote:
> > > > > > > > > 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
> > > > > > > >
> > > > > > > > Let me explain, something more.
> > > > > > > >
> > > > > > > > According to bsp there are clk_info.tcon_div which I will 
> > > > > > > > explain below.
> > > > > > > > clk_info.dsi_div which is dynamic and it depends on bpp/lanes, 
> > > > > > > > so it
> > > > > > > > is 6 for 24bpp and 4 lanes devices.
> > > > > > > >
> > > > > > > > PLL rate here depends on dsi_div (not tcon_div)
> > > > > > > >
> > > > > > > > Code here
> > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L784
> > > > > > > >
> > > > > > > > is computing the actual set rate, which depends on dsi_rate.
> > > > > > > >
> > > > > > > > lcd_rate = dclk_rate * clk_info.dsi_div;
> > > > > > > > dsi_rate = pll_rate / clk_info.dsi_div;
> > > > > > > >
> > > > > > > > Say if the dclk_rate 148MHz then the dsi_rate is 888MHz which 
> > > > > > > > set rate
> > > > > > > > for above link you mentioned.
> > > > > > > >
> > > > > > > > Here are the evidence with some prints.
> > > > > > > >
> > > > > > > > https://gist.github.com/openedev/9bae2d87d2fcc06b999fe48c998b7043
> > > > > > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> > > > > > >
> > > > > > > Ok, so we agree up to this point, and the prints confirm that the
> > > > > > > analysis above is the right one.
> > > > > > >
> > > > > > > > > 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
> > > > > > >
> > > > > > > Your patch doesn't address that, so let's leave that one alone.
> > > > > >
> > > > > > Basically this is final pll set rate when sun4i_dotclock.c called 
> > > > > > the
> > > > > > desired rate with ccu_nkm.c so it ended the final rate with parent 
> > > > > > as
> > > > > > Line 8 of
> > > > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> > > > >
> > > > > If that's important to the driver, it should be set explicitly then,
> > > > > and not work by accident.
> > > > >
> > > > > > > > > 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:
> > > > > > > > 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-07-03 Thread Maxime Ripard
On Tue, Jun 25, 2019 at 09:00:36PM +0530, Jagan Teki wrote:
> On Tue, Jun 25, 2019 at 8:19 PM Maxime Ripard  
> wrote:
> >
> > On Thu, Jun 20, 2019 at 11:57:44PM +0530, Jagan Teki wrote:
> > > On Fri, Jun 14, 2019 at 7:54 PM Maxime Ripard  
> > > wrote:
> > > >
> > > > On Wed, Jun 05, 2019 at 01:03:16PM +0530, Jagan Teki wrote:
> > > > > On Wed, Jun 5, 2019 at 12:19 PM Maxime Ripard 
> > > > >  wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > I've reordered the mail a bit to work on chunks
> > > > > >
> > > > > > On Fri, May 24, 2019 at 03:37:42PM +0530, Jagan Teki wrote:
> > > > > > > > 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
> > > > > > >
> > > > > > > Let me explain, something more.
> > > > > > >
> > > > > > > According to bsp there are clk_info.tcon_div which I will explain 
> > > > > > > below.
> > > > > > > clk_info.dsi_div which is dynamic and it depends on bpp/lanes, so 
> > > > > > > it
> > > > > > > is 6 for 24bpp and 4 lanes devices.
> > > > > > >
> > > > > > > PLL rate here depends on dsi_div (not tcon_div)
> > > > > > >
> > > > > > > Code here
> > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L784
> > > > > > >
> > > > > > > is computing the actual set rate, which depends on dsi_rate.
> > > > > > >
> > > > > > > lcd_rate = dclk_rate * clk_info.dsi_div;
> > > > > > > dsi_rate = pll_rate / clk_info.dsi_div;
> > > > > > >
> > > > > > > Say if the dclk_rate 148MHz then the dsi_rate is 888MHz which set 
> > > > > > > rate
> > > > > > > for above link you mentioned.
> > > > > > >
> > > > > > > Here are the evidence with some prints.
> > > > > > >
> > > > > > > https://gist.github.com/openedev/9bae2d87d2fcc06b999fe48c998b7043
> > > > > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> > > > > >
> > > > > > Ok, so we agree up to this point, and the prints confirm that the
> > > > > > analysis above is the right one.
> > > > > >
> > > > > > > > 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
> > > > > >
> > > > > > Your patch doesn't address that, so let's leave that one alone.
> > > > >
> > > > > Basically this is final pll set rate when sun4i_dotclock.c called the
> > > > > desired rate with ccu_nkm.c so it ended the final rate with parent as
> > > > > Line 8 of
> > > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> > > >
> > > > If that's important to the driver, it should be set explicitly then,
> > > > and not work by accident.
> > > >
> > > > > > > > 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:
> > > > > > > > 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-06-25 Thread Jagan Teki
On Tue, Jun 25, 2019 at 8:19 PM Maxime Ripard  wrote:
>
> On Thu, Jun 20, 2019 at 11:57:44PM +0530, Jagan Teki wrote:
> > On Fri, Jun 14, 2019 at 7:54 PM Maxime Ripard  
> > wrote:
> > >
> > > On Wed, Jun 05, 2019 at 01:03:16PM +0530, Jagan Teki wrote:
> > > > On Wed, Jun 5, 2019 at 12:19 PM Maxime Ripard 
> > > >  wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > I've reordered the mail a bit to work on chunks
> > > > >
> > > > > On Fri, May 24, 2019 at 03:37:42PM +0530, Jagan Teki wrote:
> > > > > > > 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
> > > > > >
> > > > > > Let me explain, something more.
> > > > > >
> > > > > > According to bsp there are clk_info.tcon_div which I will explain 
> > > > > > below.
> > > > > > clk_info.dsi_div which is dynamic and it depends on bpp/lanes, so it
> > > > > > is 6 for 24bpp and 4 lanes devices.
> > > > > >
> > > > > > PLL rate here depends on dsi_div (not tcon_div)
> > > > > >
> > > > > > Code here
> > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L784
> > > > > >
> > > > > > is computing the actual set rate, which depends on dsi_rate.
> > > > > >
> > > > > > lcd_rate = dclk_rate * clk_info.dsi_div;
> > > > > > dsi_rate = pll_rate / clk_info.dsi_div;
> > > > > >
> > > > > > Say if the dclk_rate 148MHz then the dsi_rate is 888MHz which set 
> > > > > > rate
> > > > > > for above link you mentioned.
> > > > > >
> > > > > > Here are the evidence with some prints.
> > > > > >
> > > > > > https://gist.github.com/openedev/9bae2d87d2fcc06b999fe48c998b7043
> > > > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> > > > >
> > > > > Ok, so we agree up to this point, and the prints confirm that the
> > > > > analysis above is the right one.
> > > > >
> > > > > > > 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
> > > > >
> > > > > Your patch doesn't address that, so let's leave that one alone.
> > > >
> > > > Basically this is final pll set rate when sun4i_dotclock.c called the
> > > > desired rate with ccu_nkm.c so it ended the final rate with parent as
> > > > Line 8 of
> > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> > >
> > > If that's important to the driver, it should be set explicitly then,
> > > and not work by accident.
> > >
> > > > > > > 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:
> > > > > > > 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-06-25 Thread Maxime Ripard
On Thu, Jun 20, 2019 at 11:57:44PM +0530, Jagan Teki wrote:
> On Fri, Jun 14, 2019 at 7:54 PM Maxime Ripard  
> wrote:
> >
> > On Wed, Jun 05, 2019 at 01:03:16PM +0530, Jagan Teki wrote:
> > > On Wed, Jun 5, 2019 at 12:19 PM Maxime Ripard  
> > > wrote:
> > > >
> > > > Hi,
> > > >
> > > > I've reordered the mail a bit to work on chunks
> > > >
> > > > On Fri, May 24, 2019 at 03:37:42PM +0530, Jagan Teki wrote:
> > > > > > 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
> > > > >
> > > > > Let me explain, something more.
> > > > >
> > > > > According to bsp there are clk_info.tcon_div which I will explain 
> > > > > below.
> > > > > clk_info.dsi_div which is dynamic and it depends on bpp/lanes, so it
> > > > > is 6 for 24bpp and 4 lanes devices.
> > > > >
> > > > > PLL rate here depends on dsi_div (not tcon_div)
> > > > >
> > > > > Code here
> > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L784
> > > > >
> > > > > is computing the actual set rate, which depends on dsi_rate.
> > > > >
> > > > > lcd_rate = dclk_rate * clk_info.dsi_div;
> > > > > dsi_rate = pll_rate / clk_info.dsi_div;
> > > > >
> > > > > Say if the dclk_rate 148MHz then the dsi_rate is 888MHz which set rate
> > > > > for above link you mentioned.
> > > > >
> > > > > Here are the evidence with some prints.
> > > > >
> > > > > https://gist.github.com/openedev/9bae2d87d2fcc06b999fe48c998b7043
> > > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> > > >
> > > > Ok, so we agree up to this point, and the prints confirm that the
> > > > analysis above is the right one.
> > > >
> > > > > > 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
> > > >
> > > > Your patch doesn't address that, so let's leave that one alone.
> > >
> > > Basically this is final pll set rate when sun4i_dotclock.c called the
> > > desired rate with ccu_nkm.c so it ended the final rate with parent as
> > > Line 8 of
> > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> >
> > If that's important to the driver, it should be set explicitly then,
> > and not work by accident.
> >
> > > > > > 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:
> > > > > > 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-06-20 Thread Jagan Teki
On Fri, Jun 14, 2019 at 7:54 PM Maxime Ripard  wrote:
>
> On Wed, Jun 05, 2019 at 01:03:16PM +0530, Jagan Teki wrote:
> > On Wed, Jun 5, 2019 at 12:19 PM Maxime Ripard  
> > wrote:
> > >
> > > Hi,
> > >
> > > I've reordered the mail a bit to work on chunks
> > >
> > > On Fri, May 24, 2019 at 03:37:42PM +0530, Jagan Teki wrote:
> > > > > 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
> > > >
> > > > Let me explain, something more.
> > > >
> > > > According to bsp there are clk_info.tcon_div which I will explain below.
> > > > clk_info.dsi_div which is dynamic and it depends on bpp/lanes, so it
> > > > is 6 for 24bpp and 4 lanes devices.
> > > >
> > > > PLL rate here depends on dsi_div (not tcon_div)
> > > >
> > > > Code here
> > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L784
> > > >
> > > > is computing the actual set rate, which depends on dsi_rate.
> > > >
> > > > lcd_rate = dclk_rate * clk_info.dsi_div;
> > > > dsi_rate = pll_rate / clk_info.dsi_div;
> > > >
> > > > Say if the dclk_rate 148MHz then the dsi_rate is 888MHz which set rate
> > > > for above link you mentioned.
> > > >
> > > > Here are the evidence with some prints.
> > > >
> > > > https://gist.github.com/openedev/9bae2d87d2fcc06b999fe48c998b7043
> > > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> > >
> > > Ok, so we agree up to this point, and the prints confirm that the
> > > analysis above is the right one.
> > >
> > > > > 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
> > >
> > > Your patch doesn't address that, so let's leave that one alone.
> >
> > Basically this is final pll set rate when sun4i_dotclock.c called the
> > desired rate with ccu_nkm.c so it ended the final rate with parent as
> > Line 8 of
> > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
>
> If that's important to the driver, it should be set explicitly then,
> and not work by accident.
>
> > > > > 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
> > > >
> > > > tcon_div from BSP point-of-view of there are two variants
> > > > 00) clk_info.tcon_div which is 4 and same is set the divider position
> > > > in SUN4I_TCON0_DCLK_REG (like above link refer)
> > > > 01) tcon_div which is 4 and used for edge timings computation
> > > > 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-06-14 Thread Maxime Ripard
On Wed, Jun 05, 2019 at 01:03:16PM +0530, Jagan Teki wrote:
> On Wed, Jun 5, 2019 at 12:19 PM Maxime Ripard  
> wrote:
> >
> > Hi,
> >
> > I've reordered the mail a bit to work on chunks
> >
> > On Fri, May 24, 2019 at 03:37:42PM +0530, Jagan Teki wrote:
> > > > 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
> > >
> > > Let me explain, something more.
> > >
> > > According to bsp there are clk_info.tcon_div which I will explain below.
> > > clk_info.dsi_div which is dynamic and it depends on bpp/lanes, so it
> > > is 6 for 24bpp and 4 lanes devices.
> > >
> > > PLL rate here depends on dsi_div (not tcon_div)
> > >
> > > Code here
> > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L784
> > >
> > > is computing the actual set rate, which depends on dsi_rate.
> > >
> > > lcd_rate = dclk_rate * clk_info.dsi_div;
> > > dsi_rate = pll_rate / clk_info.dsi_div;
> > >
> > > Say if the dclk_rate 148MHz then the dsi_rate is 888MHz which set rate
> > > for above link you mentioned.
> > >
> > > Here are the evidence with some prints.
> > >
> > > https://gist.github.com/openedev/9bae2d87d2fcc06b999fe48c998b7043
> > > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
> >
> > Ok, so we agree up to this point, and the prints confirm that the
> > analysis above is the right one.
> >
> > > > 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
> >
> > Your patch doesn't address that, so let's leave that one alone.
>
> Basically this is final pll set rate when sun4i_dotclock.c called the
> desired rate with ccu_nkm.c so it ended the final rate with parent as
> Line 8 of
> https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a

If that's important to the driver, it should be set explicitly then,
and not work by accident.

> > > > 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
> > >
> > > tcon_div from BSP point-of-view of there are two variants
> > > 00) clk_info.tcon_div which is 4 and same is set the divider position
> > > in SUN4I_TCON0_DCLK_REG (like above link refer)
> > > 01) tcon_div which is 4 and used for edge timings computation
> > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c#L12
> > >
> > > The real reason for 01) is again 4 is they set the divider to 4 in 00)
> > > which is technically wrong because the 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-06-05 Thread Jagan Teki
On Wed, Jun 5, 2019 at 12:19 PM Maxime Ripard  wrote:
>
> Hi,
>
> I've reordered the mail a bit to work on chunks
>
> On Fri, May 24, 2019 at 03:37:42PM +0530, Jagan Teki wrote:
> > > 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
> >
> > Let me explain, something more.
> >
> > According to bsp there are clk_info.tcon_div which I will explain below.
> > clk_info.dsi_div which is dynamic and it depends on bpp/lanes, so it
> > is 6 for 24bpp and 4 lanes devices.
> >
> > PLL rate here depends on dsi_div (not tcon_div)
> >
> > Code here
> > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L784
> >
> > is computing the actual set rate, which depends on dsi_rate.
> >
> > lcd_rate = dclk_rate * clk_info.dsi_div;
> > dsi_rate = pll_rate / clk_info.dsi_div;
> >
> > Say if the dclk_rate 148MHz then the dsi_rate is 888MHz which set rate
> > for above link you mentioned.
> >
> > Here are the evidence with some prints.
> >
> > https://gist.github.com/openedev/9bae2d87d2fcc06b999fe48c998b7043
> > https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a
>
> Ok, so we agree up to this point, and the prints confirm that the
> analysis above is the right one.
>
> > > 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
>
> Your patch doesn't address that, so let's leave that one alone.

Basically this is final pll set rate when sun4i_dotclock.c called the
desired rate with ccu_nkm.c so it ended the final rate with parent as
Line 8 of
https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a

>
> > > 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
> >
> > tcon_div from BSP point-of-view of there are two variants
> > 00) clk_info.tcon_div which is 4 and same is set the divider position
> > in SUN4I_TCON0_DCLK_REG (like above link refer)
> > 01) tcon_div which is 4 and used for edge timings computation
> > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c#L12
> >
> > The real reason for 01) is again 4 is they set the divider to 4 in 00)
> > which is technically wrong because the dividers which used during
> > dotclock in above (dsi_div) should be used here as well. Since there
> > is no dynamic way of doing this BSP hard-coding these values.
> >
> > Patches 5,6,7 on this series doing this
> > https://patchwork.freedesktop.org/series/60847/
> >
> > Hope this explanation helps?
>
> It doesn't.
>
> The clock tree is this one:

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-06-05 Thread Maxime Ripard
Hi,

I've reordered the mail a bit to work on chunks

On Fri, May 24, 2019 at 03:37:42PM +0530, Jagan Teki wrote:
> > 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
>
> Let me explain, something more.
>
> According to bsp there are clk_info.tcon_div which I will explain below.
> clk_info.dsi_div which is dynamic and it depends on bpp/lanes, so it
> is 6 for 24bpp and 4 lanes devices.
>
> PLL rate here depends on dsi_div (not tcon_div)
>
> Code here
> https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L784
>
> is computing the actual set rate, which depends on dsi_rate.
>
> lcd_rate = dclk_rate * clk_info.dsi_div;
> dsi_rate = pll_rate / clk_info.dsi_div;
>
> Say if the dclk_rate 148MHz then the dsi_rate is 888MHz which set rate
> for above link you mentioned.
>
> Here are the evidence with some prints.
>
> https://gist.github.com/openedev/9bae2d87d2fcc06b999fe48c998b7043
> https://gist.github.com/openedev/700de2e3701b2bf3ad1aa0f0fa862c9a

Ok, so we agree up to this point, and the prints confirm that the
analysis above is the right one.

> > 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

Your patch doesn't address that, so let's leave that one alone.

> > 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
>
> tcon_div from BSP point-of-view of there are two variants
> 00) clk_info.tcon_div which is 4 and same is set the divider position
> in SUN4I_TCON0_DCLK_REG (like above link refer)
> 01) tcon_div which is 4 and used for edge timings computation
> https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c#L12
>
> The real reason for 01) is again 4 is they set the divider to 4 in 00)
> which is technically wrong because the dividers which used during
> dotclock in above (dsi_div) should be used here as well. Since there
> is no dynamic way of doing this BSP hard-coding these values.
>
> Patches 5,6,7 on this series doing this
> https://patchwork.freedesktop.org/series/60847/
>
> Hope this explanation helps?

It doesn't.

The clock tree is this one:

PLL(s) -> TCON module clock -> TCON dotclock.

The links I mentioned above show that the clock set to lcd_rate is the
TCON module clocks (and it should be the one taking the bpp and lanes
into account), while the TCON dotclock uses a fixed divider of 4.

In your patches, you're using the bpp / lanes divider on the TCON
dotclock, ie, the wrong clock.

Again, I'm not saying that my analysis of the source code is correct
here. But you haven't said anything to 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-05-24 Thread Jagan Teki
On Fri, Feb 1, 2019 at 8:01 PM Maxime Ripard  wrote:
>
> On Tue, Jan 29, 2019 at 11:01:31PM +0530, Jagan Teki wrote:
> > On Tue, Jan 29, 2019 at 8:43 PM Maxime Ripard  
> > 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 
> > > >  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 
> > > > > > Acked-by: Stephen Boyd 
> > > > >
> > > > > 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 = 
> > > > 5500
> > > > [2.424172] ideal = 22000, rounded = 0
> > > > [2.424176] ideal = 27500, rounded = 0
> > > > [2.424194] ccu_nkm_round_rate: rate = 33000
> > > > [2.424197] ideal = 33000, rounded = 33000
> > > > [2.424201] sun4i_dclk_round_rate: div = 6 rate = 5500
> > > > [2.424205] sun4i_dclk_round_rate: min_div = 4 max_div = 127, rate = 
> > > > 5500
> > > > [2.424209] ideal = 22000, rounded = 0
> > > > [2.424213] ideal = 27500, rounded = 0
> > > > [2.424230] ccu_nkm_round_rate: rate = 33000
> > > > [2.424233] ideal = 33000, rounded = 33000
> > > > [2.424236] sun4i_dclk_round_rate: div = 6 rate = 5500
> > > > [2.424253] ccu_nkm_round_rate: rate = 33000
> > > > [2.424270] ccu_nkm_round_rate: rate = 33000
> > > > [2.424278] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> > > > [2.424281] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> > > > [2.424306] ccu_nkm_set_rate: rate = 33000, parent_rate = 
> > > > 29700
> > > > [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 = 5500
> > > >
> > > > 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 = 5500
> > [   10.800749] lcd_clk_config: lcd_rate = 33000, pll_rate = 33000
> >
> > 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).
>

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-02-12 Thread Jagan Teki
On Tue, Feb 12, 2019 at 3:00 PM Maxime Ripard  wrote:
>
> On Mon, Feb 11, 2019 at 07:37:57PM +0530, Jagan Teki wrote:
> > Hi Maxime,
> >
> > On Fri, Feb 1, 2019 at 8:01 PM Maxime Ripard  
> > wrote:
> > >
> > > On Tue, Jan 29, 2019 at 11:01:31PM +0530, Jagan Teki wrote:
> > > > On Tue, Jan 29, 2019 at 8:43 PM Maxime Ripard 
> > > >  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 
> > > > > >  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 
> > > > > > > > Acked-by: Stephen Boyd 
> > > > > > >
> > > > > > > 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 = 5500
> > > > > > [2.424172] ideal = 22000, rounded = 0
> > > > > > [2.424176] ideal = 27500, rounded = 0
> > > > > > [2.424194] ccu_nkm_round_rate: rate = 33000
> > > > > > [2.424197] ideal = 33000, rounded = 33000
> > > > > > [2.424201] sun4i_dclk_round_rate: div = 6 rate = 5500
> > > > > > [2.424205] sun4i_dclk_round_rate: min_div = 4 max_div = 127, 
> > > > > > rate = 5500
> > > > > > [2.424209] ideal = 22000, rounded = 0
> > > > > > [2.424213] ideal = 27500, rounded = 0
> > > > > > [2.424230] ccu_nkm_round_rate: rate = 33000
> > > > > > [2.424233] ideal = 33000, rounded = 33000
> > > > > > [2.424236] sun4i_dclk_round_rate: div = 6 rate = 5500
> > > > > > [2.424253] ccu_nkm_round_rate: rate = 33000
> > > > > > [2.424270] ccu_nkm_round_rate: rate = 33000
> > > > > > [2.424278] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> > > > > > [2.424281] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> > > > > > [2.424306] ccu_nkm_set_rate: rate = 33000, parent_rate = 
> > > > > > 29700
> > > > > > [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 = 5500
> > > > > >
> > > > > > 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 = 5500
> > > > [   

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-02-12 Thread Maxime Ripard
On Mon, Feb 11, 2019 at 07:37:57PM +0530, Jagan Teki wrote:
> Hi Maxime,
> 
> On Fri, Feb 1, 2019 at 8:01 PM Maxime Ripard  
> wrote:
> >
> > On Tue, Jan 29, 2019 at 11:01:31PM +0530, Jagan Teki wrote:
> > > On Tue, Jan 29, 2019 at 8:43 PM Maxime Ripard  
> > > 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 
> > > > >  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 
> > > > > > > Acked-by: Stephen Boyd 
> > > > > >
> > > > > > 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 
> > > > > = 5500
> > > > > [2.424172] ideal = 22000, rounded = 0
> > > > > [2.424176] ideal = 27500, rounded = 0
> > > > > [2.424194] ccu_nkm_round_rate: rate = 33000
> > > > > [2.424197] ideal = 33000, rounded = 33000
> > > > > [2.424201] sun4i_dclk_round_rate: div = 6 rate = 5500
> > > > > [2.424205] sun4i_dclk_round_rate: min_div = 4 max_div = 127, rate 
> > > > > = 5500
> > > > > [2.424209] ideal = 22000, rounded = 0
> > > > > [2.424213] ideal = 27500, rounded = 0
> > > > > [2.424230] ccu_nkm_round_rate: rate = 33000
> > > > > [2.424233] ideal = 33000, rounded = 33000
> > > > > [2.424236] sun4i_dclk_round_rate: div = 6 rate = 5500
> > > > > [2.424253] ccu_nkm_round_rate: rate = 33000
> > > > > [2.424270] ccu_nkm_round_rate: rate = 33000
> > > > > [2.424278] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> > > > > [2.424281] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> > > > > [2.424306] ccu_nkm_set_rate: rate = 33000, parent_rate = 
> > > > > 29700
> > > > > [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 = 5500
> > > > >
> > > > > 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 = 5500
> > > [   10.800749] lcd_clk_config: lcd_rate = 33000, pll_rate = 33000
> > >
> > > 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
> > > 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-02-11 Thread Jagan Teki
Hi Maxime,

On Fri, Feb 1, 2019 at 8:01 PM Maxime Ripard  wrote:
>
> On Tue, Jan 29, 2019 at 11:01:31PM +0530, Jagan Teki wrote:
> > On Tue, Jan 29, 2019 at 8:43 PM Maxime Ripard  
> > 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 
> > > >  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 
> > > > > > Acked-by: Stephen Boyd 
> > > > >
> > > > > 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 = 
> > > > 5500
> > > > [2.424172] ideal = 22000, rounded = 0
> > > > [2.424176] ideal = 27500, rounded = 0
> > > > [2.424194] ccu_nkm_round_rate: rate = 33000
> > > > [2.424197] ideal = 33000, rounded = 33000
> > > > [2.424201] sun4i_dclk_round_rate: div = 6 rate = 5500
> > > > [2.424205] sun4i_dclk_round_rate: min_div = 4 max_div = 127, rate = 
> > > > 5500
> > > > [2.424209] ideal = 22000, rounded = 0
> > > > [2.424213] ideal = 27500, rounded = 0
> > > > [2.424230] ccu_nkm_round_rate: rate = 33000
> > > > [2.424233] ideal = 33000, rounded = 33000
> > > > [2.424236] sun4i_dclk_round_rate: div = 6 rate = 5500
> > > > [2.424253] ccu_nkm_round_rate: rate = 33000
> > > > [2.424270] ccu_nkm_round_rate: rate = 33000
> > > > [2.424278] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> > > > [2.424281] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> > > > [2.424306] ccu_nkm_set_rate: rate = 33000, parent_rate = 
> > > > 29700
> > > > [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 = 5500
> > > >
> > > > 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 = 5500
> > [   10.800749] lcd_clk_config: lcd_rate = 33000, pll_rate = 33000
> >
> > 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 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-02-01 Thread Jagan Teki
On Fri, Feb 1, 2019 at 8:01 PM Maxime Ripard  wrote:
>
> On Tue, Jan 29, 2019 at 11:01:31PM +0530, Jagan Teki wrote:
> > On Tue, Jan 29, 2019 at 8:43 PM Maxime Ripard  
> > 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 
> > > >  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 
> > > > > > Acked-by: Stephen Boyd 
> > > > >
> > > > > 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 = 
> > > > 5500
> > > > [2.424172] ideal = 22000, rounded = 0
> > > > [2.424176] ideal = 27500, rounded = 0
> > > > [2.424194] ccu_nkm_round_rate: rate = 33000
> > > > [2.424197] ideal = 33000, rounded = 33000
> > > > [2.424201] sun4i_dclk_round_rate: div = 6 rate = 5500
> > > > [2.424205] sun4i_dclk_round_rate: min_div = 4 max_div = 127, rate = 
> > > > 5500
> > > > [2.424209] ideal = 22000, rounded = 0
> > > > [2.424213] ideal = 27500, rounded = 0
> > > > [2.424230] ccu_nkm_round_rate: rate = 33000
> > > > [2.424233] ideal = 33000, rounded = 33000
> > > > [2.424236] sun4i_dclk_round_rate: div = 6 rate = 5500
> > > > [2.424253] ccu_nkm_round_rate: rate = 33000
> > > > [2.424270] ccu_nkm_round_rate: rate = 33000
> > > > [2.424278] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> > > > [2.424281] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> > > > [2.424306] ccu_nkm_set_rate: rate = 33000, parent_rate = 
> > > > 29700
> > > > [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 = 5500
> > > >
> > > > 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 = 5500
> > [   10.800749] lcd_clk_config: lcd_rate = 33000, pll_rate = 33000
> >
> > 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).
>

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-02-01 Thread Maxime Ripard
On Tue, Jan 29, 2019 at 11:01:31PM +0530, Jagan Teki wrote:
> On Tue, Jan 29, 2019 at 8:43 PM Maxime Ripard  
> 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  
> > > 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 
> > > > > Acked-by: Stephen Boyd 
> > > >
> > > > 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 = 
> > > 5500
> > > [2.424172] ideal = 22000, rounded = 0
> > > [2.424176] ideal = 27500, rounded = 0
> > > [2.424194] ccu_nkm_round_rate: rate = 33000
> > > [2.424197] ideal = 33000, rounded = 33000
> > > [2.424201] sun4i_dclk_round_rate: div = 6 rate = 5500
> > > [2.424205] sun4i_dclk_round_rate: min_div = 4 max_div = 127, rate = 
> > > 5500
> > > [2.424209] ideal = 22000, rounded = 0
> > > [2.424213] ideal = 27500, rounded = 0
> > > [2.424230] ccu_nkm_round_rate: rate = 33000
> > > [2.424233] ideal = 33000, rounded = 33000
> > > [2.424236] sun4i_dclk_round_rate: div = 6 rate = 5500
> > > [2.424253] ccu_nkm_round_rate: rate = 33000
> > > [2.424270] ccu_nkm_round_rate: rate = 33000
> > > [2.424278] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> > > [2.424281] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> > > [2.424306] ccu_nkm_set_rate: rate = 33000, parent_rate = 29700
> > > [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 = 5500
> > >
> > > 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 = 5500
> [   10.800749] lcd_clk_config: lcd_rate = 33000, pll_rate = 33000
> 
> 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, 

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-01-29 Thread Jagan Teki
On Tue, Jan 29, 2019 at 8:43 PM Maxime Ripard  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  
> > 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 
> > > > Acked-by: Stephen Boyd 
> > >
> > > 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 = 
> > 5500
> > [2.424172] ideal = 22000, rounded = 0
> > [2.424176] ideal = 27500, rounded = 0
> > [2.424194] ccu_nkm_round_rate: rate = 33000
> > [2.424197] ideal = 33000, rounded = 33000
> > [2.424201] sun4i_dclk_round_rate: div = 6 rate = 5500
> > [2.424205] sun4i_dclk_round_rate: min_div = 4 max_div = 127, rate = 
> > 5500
> > [2.424209] ideal = 22000, rounded = 0
> > [2.424213] ideal = 27500, rounded = 0
> > [2.424230] ccu_nkm_round_rate: rate = 33000
> > [2.424233] ideal = 33000, rounded = 33000
> > [2.424236] sun4i_dclk_round_rate: div = 6 rate = 5500
> > [2.424253] ccu_nkm_round_rate: rate = 33000
> > [2.424270] ccu_nkm_round_rate: rate = 33000
> > [2.424278] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> > [2.424281] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> > [2.424306] ccu_nkm_set_rate: rate = 33000, parent_rate = 29700
> > [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 = 5500
> >
> > 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 = 5500
[   10.800749] lcd_clk_config: lcd_rate = 33000, pll_rate = 33000

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)

We can even confirm this from Mainline code:
[1.866128] sun4i_dclk_round_rate: min_div = 6 max_div = 6, rate = 5500
[1.873112] round_rate, parent = 33000
[1.877351] round_rate, rate = 33000
[1.881338] ideal = 33000, rounded = 33000, div = 6
[1.887232] sun4i_dclk_round_rate: div = 6 rate = 5500
[1.887239] sun4i_dclk_round_rate: min_div = 6 max_div = 6, rate = 5500
[1.887243] round_rate, parent = 33000
[1.887259] round_rate, rate = 33000
[1.887264] ideal = 33000, rounded = 33000, div = 6
[1.887267] sun4i_dclk_round_rate: div = 6 rate = 5500
[1.887270] round_rate, parent = 33000
[1.887286] round_rate, rate = 33000
[   

[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-01-29 Thread Maxime Ripard
On Mon, Jan 28, 2019 at 03:06:10PM +0530, Jagan Teki wrote:
> On Sat, Jan 26, 2019 at 2:54 AM Maxime Ripard  
> 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 
> > > Acked-by: Stephen Boyd 
> >
> > 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 = 
> 5500
> [2.424172] ideal = 22000, rounded = 0
> [2.424176] ideal = 27500, rounded = 0
> [2.424194] ccu_nkm_round_rate: rate = 33000
> [2.424197] ideal = 33000, rounded = 33000
> [2.424201] sun4i_dclk_round_rate: div = 6 rate = 5500
> [2.424205] sun4i_dclk_round_rate: min_div = 4 max_div = 127, rate = 
> 5500
> [2.424209] ideal = 22000, rounded = 0
> [2.424213] ideal = 27500, rounded = 0
> [2.424230] ccu_nkm_round_rate: rate = 33000
> [2.424233] ideal = 33000, rounded = 33000
> [2.424236] sun4i_dclk_round_rate: div = 6 rate = 5500
> [2.424253] ccu_nkm_round_rate: rate = 33000
> [2.424270] ccu_nkm_round_rate: rate = 33000
> [2.424278] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> [2.424281] sun4i_dclk_recalc_rate: val = 1, rate = 33000
> [2.424306] ccu_nkm_set_rate: rate = 33000, parent_rate = 29700
> [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 = 5500
> 
> 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

Maxime

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: PGP signature


[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-01-28 Thread Jagan Teki
On Sat, Jan 26, 2019 at 2:54 AM Maxime Ripard  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 
> > Acked-by: Stephen Boyd 
>
> 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 = 5500
[2.424172] ideal = 22000, rounded = 0
[2.424176] ideal = 27500, rounded = 0
[2.424194] ccu_nkm_round_rate: rate = 33000
[2.424197] ideal = 33000, rounded = 33000
[2.424201] sun4i_dclk_round_rate: div = 6 rate = 5500
[2.424205] sun4i_dclk_round_rate: min_div = 4 max_div = 127, rate = 5500
[2.424209] ideal = 22000, rounded = 0
[2.424213] ideal = 27500, rounded = 0
[2.424230] ccu_nkm_round_rate: rate = 33000
[2.424233] ideal = 33000, rounded = 33000
[2.424236] sun4i_dclk_round_rate: div = 6 rate = 5500
[2.424253] ccu_nkm_round_rate: rate = 33000
[2.424270] ccu_nkm_round_rate: rate = 33000
[2.424278] sun4i_dclk_recalc_rate: val = 1, rate = 33000
[2.424281] sun4i_dclk_recalc_rate: val = 1, rate = 33000
[2.424306] ccu_nkm_set_rate: rate = 33000, parent_rate = 29700
[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 = 5500

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.

Since the above min_rate is not desired for possible panels clock, I
think we can rely on BSP to make a move here. I'm planning to do these
changes in next version.

Let me know if you have any comments here?

[1] https://patchwork.kernel.org/patch/10777519/

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[linux-sunxi] Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

2019-01-25 Thread Maxime Ripard
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 
> Acked-by: Stephen Boyd 

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.

Maxime

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

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.