<div dir="auto"><div>Hi</div><div dir="auto"><br><br><div class="gmail_quote" dir="auto"><div dir="ltr" class="gmail_attr">On Sat., 20 Jul. 2019, 8:58 am Maxime Ripard, <<a href="mailto:maxime.ripard@bootlin.com">maxime.ripard@bootlin.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi wrote:<br>
> > > tcon-pixel clock is the rate that you want to achive on display side<br>
> > > and if you have 4 lanes 32bit or lanes and different bit number that<br>
> > > you need to have a clock that is able to put outside bits and speed<br>
> > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock of<br>
> > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of<br>
> > > 40 * 32 / 4 in no-burst mode. I think that this is done but most of<br>
> > > the display.<br>
> ><br>
> > So this is what the issue is then?<br>
> ><br>
> > This one does make sense, and you should just change the rate in the<br>
> > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.<br>
> ><br>
> > I'm still wondering why that hasn't been brought up in either the<br>
> > discussion or the commit log before though.<br>
> ><br>
> Something like this?<br>
><br>
> drivers/gpu/drm/sun4i/sun4i_tcon.c     | 20 +++++++++++---------<br>
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --<br>
>  2 files changed, 11 insertions(+), 11 deletions(-)<br>
><br>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c<br>
> b/drivers/gpu/drm/sun4i/sun4i_tcon.c<br>
> index 64c43ee6bd92..42560d5c327c 100644<br>
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c<br>
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c<br>
> @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct<br>
> drm_display_mode *mode,<br>
>  }<br>
><br>
>  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,<br>
> -                                       const struct drm_display_mode *mode)<br>
> +                                       const struct drm_display_mode *mode,<br>
> +                                       u32 tcon_mul)<br>
>  {<br>
>         /* Configure the dot clock */<br>
> -       clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);<br>
> +       clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);<br>
><br>
>         /* Set the resolution */<br>
>         regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,<br>
> @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct<br>
> sun4i_tcon *tcon,<br>
>         u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);<br>
>         u8 lanes = device->lanes;<br>
>         u32 block_space, start_delay;<br>
> -       u32 tcon_div;<br>
> +       u32 tcon_div, tcon_mul;<br>
><br>
> -       tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;<br>
> -       tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;<br>
> +       tcon->dclk_min_div = 4;<br>
> +       tcon->dclk_max_div = 127;<br>
><br>
> -       sun4i_tcon0_mode_set_common(tcon, mode);<br>
> +       tcon_mul = bpp / lanes;<br>
> +       sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);<br>
><br>
>         /* Set dithering if needed */<br>
>         sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));<br>
> @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct<br>
> sun4i_tcon *tcon,<br>
>          */<br>
>         regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, &tcon_div);<br>
>         tcon_div &= GENMASK(6, 0);<br>
> -       block_space = mode->htotal * bpp / (tcon_div * lanes);<br>
> +       block_space = mode->htotal * tcon_div * tcon_mul;<br>
>         block_space -= mode->hdisplay + 40;<br>
><br>
>         regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,<br>
> @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct<br>
> sun4i_tcon *tcon,<br>
><br>
>         tcon->dclk_min_div = 7;<br>
>         tcon->dclk_max_div = 7;<br>
> -       sun4i_tcon0_mode_set_common(tcon, mode);<br>
> +       sun4i_tcon0_mode_set_common(tcon, mode, 1);<br>
><br>
>         /* Set dithering if needed */<br>
>         sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));<br>
> @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct<br>
> sun4i_tcon *tcon,<br>
><br>
>         tcon->dclk_min_div = 6;<br>
>         tcon->dclk_max_div = 127;<br>
> -       sun4i_tcon0_mode_set_common(tcon, mode);<br>
> +       sun4i_tcon0_mode_set_common(tcon, mode, 1);<br>
><br>
>         /* Set dithering if needed */<br>
>         sun4i_tcon0_mode_set_dithering(tcon, connector);<br>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h<br>
> b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h<br>
> index 5c3ad5be0690..a07090579f84 100644<br>
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h<br>
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h<br>
> @@ -13,8 +13,6 @@<br>
>  #include <drm/drm_encoder.h><br>
>  #include <drm/drm_mipi_dsi.h><br>
><br>
> -#define SUN6I_DSI_TCON_DIV     4<br>
> -<br>
>  struct sun6i_dsi {<br>
>         struct drm_connector    connector;<br>
>         struct drm_encoder      encoder;<br>
<br>
I had more something like this in mind:<br>
<a href="http://code.bulix.org/nlp5a4-803511" rel="noreferrer noreferrer" target="_blank">http://code.bulix.org/nlp5a4-803511</a></blockquote></div></div><div dir="auto"><br></div><div dir="auto">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</div><div dir="auto"><br></div><div dir="auto">We are on same line</div><div dir="auto"><br></div><div dir="auto">Thank you</div><div dir="auto"><div class="gmail_quote" dir="auto"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br><br><br><br>
You really don't need to change the divider range (or this is another<br>
issue that the one you mentionned).<br>
<br>
Maxime<br>
<br>
--<br>
Maxime Ripard, Bootlin<br>
Embedded Linux and Kernel engineering<br>
<a href="https://bootlin.com" rel="noreferrer noreferrer" target="_blank">https://bootlin.com</a><br>
</blockquote></div></div></div>