public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
* mxsfb on i.MX28 uses bypassed ref_xtal 24MHz clock for LCD
       [not found] <5196375A.1070308@digi.com>
@ 2013-09-20 13:50 ` Hector Palacios
  2013-09-22  1:05   ` Shawn Guo
  2013-09-22 16:16   ` Fabio Estevam
  0 siblings, 2 replies; 3+ messages in thread
From: Hector Palacios @ 2013-09-20 13:50 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/17/2013 03:57 PM, Hector Palacios wrote:
> Hello,
>
> I was testing the framebuffer on an i.MX28 based platform and found out that the lcdif
> clock is actually bypassed to use the ref_xtal of 24MHz, which makes the
> clk_set_rate() call in the driver useless (as well as the pixclock settings in
> fb_videomode variables in mach-mxs.c):
>
> static void mxsfb_enable_controller(struct fb_info *fb_info)
> {
>      struct mxsfb_info *host = to_imxfb_host(fb_info);
>      u32 reg;
>
>      dev_dbg(&host->pdev->dev, "%s\n", __func__);
>
>      clk_prepare_enable(host->clk);
>      clk_set_rate(host->clk, PICOS2KHZ(fb_info->var.pixclock) * 1000U);
>
> This can easily be checked by probing the pixel clock signal.
> Is this known or did it go unnoticed because the supported displays work ok with the
> bypassed 24MHz frequency?
> If using a pixel clock below 24MHz, the divisor will round to 2 thus generating a
> 12MHz signal.

Going back to this, after a while. The lcdif clock hierarchy is the following:

ref_clk (24MHz)-----\
                      +--+ lcdif_sel (MUX type)
frac1_clk (480Mhz)--/   |
                         \--+ lcdif_div (DIV type)
                            |
                            \-- lcdif (GATE type)

The mxsfb driver retrieves the 'lcdif_div' clock from the DT, but the 'lcdif_sel' is 
by default sourced from 'ref_clk'. Since the driver doesn't know about the different 
clocks that can source its clk parent from, would it be acceptable to select the 
parent during the general clocks initialization?

diff --git a/drivers/clk/mxs/clk-imx28.c b/drivers/clk/mxs/clk-imx28.c
index fd17995..211b4d9 100644
--- a/drivers/clk/mxs/clk-imx28.c
+++ b/drivers/clk/mxs/clk-imx28.c
@@ -246,6 +246,12 @@ int __init mx28_clocks_init(void)
	clk_data.clk_num = ARRAY_SIZE(clks);
	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);

+	/*
+	 * Select PLL as the parent source of lcdif_sel clk to have a finer
+	 * granularity when calculating the LCD pixelclock
+	 */
+	clk_set_parent(clks[lcdif_sel], clks[ref_pix]);
+
	clk_register_clkdev(clks[enet_out], NULL, "enet_out");

	for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)


Without this, the pixelclock can only be a power of two divisor of 24MHz, and I need a 
20MHz signal, otherwise I see bad pixels in true color images on my LCD.

Best regards,
--
Hector Palacios

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* mxsfb on i.MX28 uses bypassed ref_xtal 24MHz clock for LCD
  2013-09-20 13:50 ` mxsfb on i.MX28 uses bypassed ref_xtal 24MHz clock for LCD Hector Palacios
@ 2013-09-22  1:05   ` Shawn Guo
  2013-09-22 16:16   ` Fabio Estevam
  1 sibling, 0 replies; 3+ messages in thread
From: Shawn Guo @ 2013-09-22  1:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Sep 20, 2013 at 03:50:23PM +0200, Hector Palacios wrote:
> On 05/17/2013 03:57 PM, Hector Palacios wrote:
> >Hello,
> >
> >I was testing the framebuffer on an i.MX28 based platform and found out that the lcdif
> >clock is actually bypassed to use the ref_xtal of 24MHz, which makes the
> >clk_set_rate() call in the driver useless (as well as the pixclock settings in
> >fb_videomode variables in mach-mxs.c):
> >
> >static void mxsfb_enable_controller(struct fb_info *fb_info)
> >{
> >     struct mxsfb_info *host = to_imxfb_host(fb_info);
> >     u32 reg;
> >
> >     dev_dbg(&host->pdev->dev, "%s\n", __func__);
> >
> >     clk_prepare_enable(host->clk);
> >     clk_set_rate(host->clk, PICOS2KHZ(fb_info->var.pixclock) * 1000U);
> >
> >This can easily be checked by probing the pixel clock signal.
> >Is this known or did it go unnoticed because the supported displays work ok with the
> >bypassed 24MHz frequency?
> >If using a pixel clock below 24MHz, the divisor will round to 2 thus generating a
> >12MHz signal.
> 
> Going back to this, after a while. The lcdif clock hierarchy is the following:
> 
> ref_clk (24MHz)-----\
>                      +--+ lcdif_sel (MUX type)
> frac1_clk (480Mhz)--/   |
>                         \--+ lcdif_div (DIV type)
>                            |
>                            \-- lcdif (GATE type)
> 
> The mxsfb driver retrieves the 'lcdif_div' clock from the DT, but
> the 'lcdif_sel' is by default sourced from 'ref_clk'. Since the
> driver doesn't know about the different clocks that can source its
> clk parent from, would it be acceptable to select the parent during
> the general clocks initialization?

I'm fine with it.

Shawn

> diff --git a/drivers/clk/mxs/clk-imx28.c b/drivers/clk/mxs/clk-imx28.c
> index fd17995..211b4d9 100644
> --- a/drivers/clk/mxs/clk-imx28.c
> +++ b/drivers/clk/mxs/clk-imx28.c
> @@ -246,6 +246,12 @@ int __init mx28_clocks_init(void)
> 	clk_data.clk_num = ARRAY_SIZE(clks);
> 	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
> 
> +	/*
> +	 * Select PLL as the parent source of lcdif_sel clk to have a finer
> +	 * granularity when calculating the LCD pixelclock
> +	 */
> +	clk_set_parent(clks[lcdif_sel], clks[ref_pix]);
> +
> 	clk_register_clkdev(clks[enet_out], NULL, "enet_out");
> 
> 	for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
> 
> 
> Without this, the pixelclock can only be a power of two divisor of
> 24MHz, and I need a 20MHz signal, otherwise I see bad pixels in true
> color images on my LCD.
> 
> Best regards,
> --
> Hector Palacios

^ permalink raw reply	[flat|nested] 3+ messages in thread

* mxsfb on i.MX28 uses bypassed ref_xtal 24MHz clock for LCD
  2013-09-20 13:50 ` mxsfb on i.MX28 uses bypassed ref_xtal 24MHz clock for LCD Hector Palacios
  2013-09-22  1:05   ` Shawn Guo
@ 2013-09-22 16:16   ` Fabio Estevam
  1 sibling, 0 replies; 3+ messages in thread
From: Fabio Estevam @ 2013-09-22 16:16 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Hector,

On Fri, Sep 20, 2013 at 10:50 AM, Hector Palacios
<hector.palacios@digi•com> wrote:

> --- a/drivers/clk/mxs/clk-imx28.c
> +++ b/drivers/clk/mxs/clk-imx28.c
> @@ -246,6 +246,12 @@ int __init mx28_clocks_init(void)
>         clk_data.clk_num = ARRAY_SIZE(clks);
>         of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
>
> +       /*
> +        * Select PLL as the parent source of lcdif_sel clk to have a finer
> +        * granularity when calculating the LCD pixelclock
> +        */
> +       clk_set_parent(clks[lcdif_sel], clks[ref_pix]);
> +
>         clk_register_clkdev(clks[enet_out], NULL, "enet_out");
>
>         for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
>
>
> Without this, the pixelclock can only be a power of two divisor of 24MHz,
> and I need a 20MHz signal, otherwise I see bad pixels in true color images
> on my LCD.

When you submit this patch, you can add:

Tested-by: Fabio Estevam <fabio.estevam@freescale•com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-09-22 16:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <5196375A.1070308@digi.com>
2013-09-20 13:50 ` mxsfb on i.MX28 uses bypassed ref_xtal 24MHz clock for LCD Hector Palacios
2013-09-22  1:05   ` Shawn Guo
2013-09-22 16:16   ` Fabio Estevam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox