Patchwork ARM: imx: propagate DI clock changes up the clock tree to PLL5

login
register
mail settings
Submitter Russell King
Date April 9, 2014, 3:39 p.m.
Message ID <E1WXuag-0007Gs-Ku@rmk-PC.arm.linux.org.uk>
Download mbox | patch
Permalink /patch/337869/
State New
Headers show

Comments

Russell King - April 9, 2014, 3:39 p.m.
Allows clk_set_rate() on the DI clocks to propagate up to PLL5.

This allows imx-drm to produce the exact clock rate required with no
modulation due to the fractional divider.  Modulation of the clock rate
can and does prevent TVs from locking to the HDMI signal - they will
report no signal rather than trying to lock to such a signal.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
I still have this patch which is required so that imx-drm is capable of
producing HDMI clocks which are close enough to the HDMI defined
frequencies such that TVs and similar can recognise the signal.  Without
this, I don't get any kind of output on my TV desite imx-drm being set
to the correct mode.  I have proven that this is because the HDMI clock
is *very* wrong without this patch.

 arch/arm/mach-imx/clk-imx6q.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
Tim Harvey - April 11, 2014, 4:31 a.m.
On Wed, Apr 9, 2014 at 8:39 AM, Russell King
<rmk+kernel@arm.linux.org.uk> wrote:
> Allows clk_set_rate() on the DI clocks to propagate up to PLL5.
>
> This allows imx-drm to produce the exact clock rate required with no
> modulation due to the fractional divider.  Modulation of the clock rate
> can and does prevent TVs from locking to the HDMI signal - they will
> report no signal rather than trying to lock to such a signal.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
> I still have this patch which is required so that imx-drm is capable of
> producing HDMI clocks which are close enough to the HDMI defined
> frequencies such that TVs and similar can recognise the signal.  Without
> this, I don't get any kind of output on my TV desite imx-drm being set
> to the correct mode.  I have proven that this is because the HDMI clock
> is *very* wrong without this patch.
>
>  arch/arm/mach-imx/clk-imx6q.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>

Hi Russell,

This fixed HDMI on the boards I'm testing it on (Gateworks Ventana).
Without it I encounter the same issue you describe above.

Tested-by: Tim Harvey <tharvey@gateworks.com>

Tim
Tim Harvey - April 11, 2014, 4:48 a.m.
On Thu, Apr 10, 2014 at 9:31 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> On Wed, Apr 9, 2014 at 8:39 AM, Russell King
> <rmk+kernel@arm.linux.org.uk> wrote:
>> Allows clk_set_rate() on the DI clocks to propagate up to PLL5.
>>
>> This allows imx-drm to produce the exact clock rate required with no
>> modulation due to the fractional divider.  Modulation of the clock rate
>> can and does prevent TVs from locking to the HDMI signal - they will
>> report no signal rather than trying to lock to such a signal.
>>
>> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
>> ---
>> I still have this patch which is required so that imx-drm is capable of
>> producing HDMI clocks which are close enough to the HDMI defined
>> frequencies such that TVs and similar can recognise the signal.  Without
>> this, I don't get any kind of output on my TV desite imx-drm being set
>> to the correct mode.  I have proven that this is because the HDMI clock
>> is *very* wrong without this patch.
>>
>>  arch/arm/mach-imx/clk-imx6q.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>
> Hi Russell,
>
> This fixed HDMI on the boards I'm testing it on (Gateworks Ventana).
> Without it I encounter the same issue you describe above.
>
> Tested-by: Tim Harvey <tharvey@gateworks.com>
>
> Tim

I neglected to mention that even after this patch I do see an issue
where the monitor does not sync if plugged in on board powerup.  The
following is what I see with DEBUG enabled in imx-hdmi:

on boot with monitor plugged in:
[    4.612634] imx-hdmi 120000.hdmi: Detected HDMI controller
0x13:0x1a:0xa0:0xc1
[    4.620213] hdmi_compute_cts: freq: 48000 pixel_clk: 74250000 ratio: 100
[    4.627039] imx-hdmi 120000.hdmi: hdmi_set_clk_regenerator:
samplerate=48000  ratio=100  pixelclk=74250000  N=6144 cts=74250
[    4.638744] imx-drm display-subsystem.10: bound 120000.hdmi (ops hdmi_ops)
[    4.638774] imx-hdmi 120000.hdmi: EVENT=plugin
[    4.638786] imx-hdmi 120000.hdmi: Non-CEA mode used in HDMI
[    4.638796] imx-hdmi 120000.hdmi: final pixclk = 0
[    4.656902] imx-hdmi 120000.hdmi: imx_hdmi_setup DVI mode
[    4.722049] imx-hdmi 120000.hdmi: got edid: width[70] x height[39]

# monitor has no sync - EDID is correct, but looks like it
mis-interprets the mode (above)

root@ventana:~# hexdump -C
/sys/devices/soc0/display-subsystem.10/drm/card0/card0-HDMI-A-1/edid
00000000  00 ff ff ff ff ff ff 00  4a 8b 54 4c 01 00 00 00  |........J.TL....|
00000010  0c 11 01 03 81 46 27 78  8a a5 8e a6 54 4a 9c 26  |.....F'x....TJ.&|
00000020  12 45 46 af cf 00 95 00  95 0f 95 19 01 01 01 01  |.EF.............|
00000030  01 01 01 01 01 01 01 1d  00 72 51 d0 1e 20 6e 28  |.........rQ.. n(|
00000040  55 00 b9 88 21 00 00 1e  8c 0a d0 8a 20 e0 2d 10  |U...!....... .-.|
00000050  10 3e 96 00 b9 88 21 00  00 18 00 00 00 fd 00 32  |.>....!........2|
00000060  4b 18 3c 0b 00 0a 20 20  20 20 20 20 00 00 00 fc  |K.<...      ....|
00000070  00 33 32 56 33 48 2d 48  36 41 0a 20 20 20 01 29  |.32V3H-H6A.   .)|
00000080  02 03 21 71 4e 06 07 02  03 15 96 11 12 13 04 14  |..!qN...........|
00000090  05 1f 90 23 09 07 07 83  01 00 00 65 03 0c 00 10  |...#.......e....|
000000a0  00 8c 0a d0 90 20 40 31  20 0c 40 55 00 b9 88 21  |..... @1 .@U...!|
000000b0  00 00 18 01 1d 80 18 71  1c 16 20 58 2c 25 00 b9  |.......q.. X,%..|
000000c0  88 21 00 00 9e 01 1d 80  d0 72 1c 16 20 10 2c 25  |.!.......r.. .,%|
000000d0  80 b9 88 21 00 00 9e 01  1d 00 bc 52 d0 1e 20 b8  |...!.......R.. .|
000000e0  28 55 40 b9 88 21 00 00  1e 02 3a 80 d0 72 38 2d  |(U@..!....:..r8-|
000000f0  40 10 2c 45 80 b9 88 21  00 00 1e 00 00 00 00 d0  |@.,E...!........|
00000100

root@ventana:~# cat
/sys/devices/soc0/display-subsystem.10/drm/card0/card0-HDMI-A-1/modes
1280x720
1920x1080
1920x1080
1920x1080
1280x1024
1440x900
1440x900
1440x900
1280x720
1280x720
1024x768
1024x768
1024x768
800x600
800x600
800x600
800x600
720x576
848x480
720x480
720x480
640x480
640x480
640x480
640x480
640x480
720x400

and hotplug the monitor:
[   51.946693] imx-hdmi 120000.hdmi: EVENT=plugout
[   54.918144] imx-hdmi 120000.hdmi: EVENT=plugin
[   54.922619] imx-hdmi 120000.hdmi: Non-CEA mode used in HDMI
[   54.928346] imx-hdmi 120000.hdmi: final pixclk = 0
[   54.951254] imx-hdmi 120000.hdmi: imx_hdmi_setup DVI mode
[   55.007786] imx-hdmi 120000.hdmi: got edid: width[70] x height[39]
[   55.016677] imx-hdmi 120000.hdmi: CEA mode used vic=4
[   55.021966] imx-hdmi 120000.hdmi: final pixclk = 74250000
[   55.046148] imx-hdmi 120000.hdmi: imx_hdmi_setup CEA mode
[   55.051744] hdmi_compute_cts: freq: 48000 pixel_clk: 74250000 ratio: 100
[   55.058563] imx-hdmi 120000.hdmi: hdmi_set_clk_regenerator:
samplerate=48000  ratio=100  pixelclk=74250000  N=6144 cts=74250
[   55.071574] imx-hdmi 120000.hdmi: CEA mode used vic=4
[   55.076656] imx-hdmi 120000.hdmi: final pixclk = 74250000
[   55.100279] imx-hdmi 120000.hdmi: imx_hdmi_setup CEA mode
[   55.105707] hdmi_compute_cts: freq: 48000 pixel_clk: 74250000 ratio: 100
[   55.112534] imx-hdmi 120000.hdmi: hdmi_set_clk_regenerator:
samplerate=48000  ratio=100  pixelclk=74250000  N=6144 cts=74250

# monitor is now sync'd with proper output and EDID is same as before

root@ventana:~# cat
/sys/devices/soc0/display-subsystem.10/drm/card0/card0-HDMI-A-1/modes
1280x720
1280x720
1280x720
800x600
800x600
800x600
800x600
720x576
848x480
720x480
720x480
640x480
640x480
640x480
640x480
640x480
720x400

Any ideas?

Regards,

Tim
Lucas Stach - April 11, 2014, 2 p.m.
Hi Tim,

Am Donnerstag, den 10.04.2014, 21:48 -0700 schrieb Tim Harvey:
> On Thu, Apr 10, 2014 at 9:31 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> > On Wed, Apr 9, 2014 at 8:39 AM, Russell King
> > <rmk+kernel@arm.linux.org.uk> wrote:
> >> Allows clk_set_rate() on the DI clocks to propagate up to PLL5.
> >>
> >> This allows imx-drm to produce the exact clock rate required with no
> >> modulation due to the fractional divider.  Modulation of the clock rate
> >> can and does prevent TVs from locking to the HDMI signal - they will
> >> report no signal rather than trying to lock to such a signal.
> >>
> >> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> >> ---
> >> I still have this patch which is required so that imx-drm is capable of
> >> producing HDMI clocks which are close enough to the HDMI defined
> >> frequencies such that TVs and similar can recognise the signal.  Without
> >> this, I don't get any kind of output on my TV desite imx-drm being set
> >> to the correct mode.  I have proven that this is because the HDMI clock
> >> is *very* wrong without this patch.
> >>
> >>  arch/arm/mach-imx/clk-imx6q.c | 12 ++++++------
> >>  1 file changed, 6 insertions(+), 6 deletions(-)
> >>
> >
> > Hi Russell,
> >
> > This fixed HDMI on the boards I'm testing it on (Gateworks Ventana).
> > Without it I encounter the same issue you describe above.
> >
> > Tested-by: Tim Harvey <tharvey@gateworks.com>
> >
> > Tim
> 
> I neglected to mention that even after this patch I do see an issue
> where the monitor does not sync if plugged in on board powerup.  The
> following is what I see with DEBUG enabled in imx-hdmi:
> 
I think this is fallout from how we handle the EDID fetch on startup. We
have patches lying around to fix this. Will post them in a minute.

Regards,
Lucas
Tim Harvey - April 12, 2014, 8:04 a.m.
On Fri, Apr 11, 2014 at 7:00 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Hi Tim,
>
> I think this is fallout from how we handle the EDID fetch on startup. We
> have patches lying around to fix this. Will post them in a minute.
>
> Regards,
> Lucas

Lucas,

I applied your 4 patches, but that didn't resolve my situation where
the display isn't enabled when connected on board powerup. It appears
that imx_hdmi_poweron() isn't called in this case.

It looks like the issue is that when drm_helper_hpd_irq_event is
called from the first imx_hdmi_irq polling is not yet enabled and thus
it doesn't call the other helpers which would end up calling
imx_hdmi_poweron. Polling gets enabled at the very end of
imx_drm_driver_load which is after the irq is requested, fired, and
serviced.

I'm not clear what the right solution is - should the poll be enabled
earlier, or should imx_hdmi_encoder_dpms(...) be called someplace at
registration?

Tim

Patch

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index 4d677f442539..3cf03a132534 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -262,10 +262,10 @@  static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	clk[ipu1_di1_pre_sel] = imx_clk_mux("ipu1_di1_pre_sel", base + 0x34, 15, 3, ipu_di_pre_sels,   ARRAY_SIZE(ipu_di_pre_sels));
 	clk[ipu2_di0_pre_sel] = imx_clk_mux("ipu2_di0_pre_sel", base + 0x38, 6,  3, ipu_di_pre_sels,   ARRAY_SIZE(ipu_di_pre_sels));
 	clk[ipu2_di1_pre_sel] = imx_clk_mux("ipu2_di1_pre_sel", base + 0x38, 15, 3, ipu_di_pre_sels,   ARRAY_SIZE(ipu_di_pre_sels));
-	clk[ipu1_di0_sel]     = imx_clk_mux("ipu1_di0_sel",     base + 0x34, 0,  3, ipu1_di0_sels,     ARRAY_SIZE(ipu1_di0_sels));
-	clk[ipu1_di1_sel]     = imx_clk_mux("ipu1_di1_sel",     base + 0x34, 9,  3, ipu1_di1_sels,     ARRAY_SIZE(ipu1_di1_sels));
-	clk[ipu2_di0_sel]     = imx_clk_mux("ipu2_di0_sel",     base + 0x38, 0,  3, ipu2_di0_sels,     ARRAY_SIZE(ipu2_di0_sels));
-	clk[ipu2_di1_sel]     = imx_clk_mux("ipu2_di1_sel",     base + 0x38, 9,  3, ipu2_di1_sels,     ARRAY_SIZE(ipu2_di1_sels));
+	clk[ipu1_di0_sel]     = imx_clk_mux_flags("ipu1_di0_sel",     base + 0x34, 0,  3, ipu1_di0_sels,     ARRAY_SIZE(ipu1_di0_sels), CLK_SET_RATE_PARENT);
+	clk[ipu1_di1_sel]     = imx_clk_mux_flags("ipu1_di1_sel",     base + 0x34, 9,  3, ipu1_di1_sels,     ARRAY_SIZE(ipu1_di1_sels), CLK_SET_RATE_PARENT);
+	clk[ipu2_di0_sel]     = imx_clk_mux_flags("ipu2_di0_sel",     base + 0x38, 0,  3, ipu2_di0_sels,     ARRAY_SIZE(ipu2_di0_sels), CLK_SET_RATE_PARENT);
+	clk[ipu2_di1_sel]     = imx_clk_mux_flags("ipu2_di1_sel",     base + 0x38, 9,  3, ipu2_di1_sels,     ARRAY_SIZE(ipu2_di1_sels), CLK_SET_RATE_PARENT);
 	clk[hsi_tx_sel]       = imx_clk_mux("hsi_tx_sel",       base + 0x30, 28, 1, hsi_tx_sels,       ARRAY_SIZE(hsi_tx_sels));
 	clk[pcie_axi_sel]     = imx_clk_mux("pcie_axi_sel",     base + 0x18, 10, 1, pcie_axi_sels,     ARRAY_SIZE(pcie_axi_sels));
 	clk[ssi1_sel]         = imx_clk_fixup_mux("ssi1_sel",   base + 0x1c, 10, 2, ssi_sels,          ARRAY_SIZE(ssi_sels),          imx_cscmr1_fixup);
@@ -307,9 +307,9 @@  static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	clk[ipu1_podf]        = imx_clk_divider("ipu1_podf",        "ipu1_sel",          base + 0x3c, 11, 3);
 	clk[ipu2_podf]        = imx_clk_divider("ipu2_podf",        "ipu2_sel",          base + 0x3c, 16, 3);
 	clk[ldb_di0_div_3_5]  = imx_clk_fixed_factor("ldb_di0_div_3_5", "ldb_di0_sel", 2, 7);
-	clk[ldb_di0_podf]     = imx_clk_divider_flags("ldb_di0_podf", "ldb_di0_div_3_5", base + 0x20, 10, 1, 0);
+	clk[ldb_di0_podf]     = imx_clk_divider("ldb_di0_podf", "ldb_di0_div_3_5", base + 0x20, 10, 1);
 	clk[ldb_di1_div_3_5]  = imx_clk_fixed_factor("ldb_di1_div_3_5", "ldb_di1_sel", 2, 7);
-	clk[ldb_di1_podf]     = imx_clk_divider_flags("ldb_di1_podf", "ldb_di1_div_3_5", base + 0x20, 11, 1, 0);
+	clk[ldb_di1_podf]     = imx_clk_divider("ldb_di1_podf", "ldb_di1_div_3_5", base + 0x20, 11, 1);
 	clk[ipu1_di0_pre]     = imx_clk_divider("ipu1_di0_pre",     "ipu1_di0_pre_sel",  base + 0x34, 3,  3);
 	clk[ipu1_di1_pre]     = imx_clk_divider("ipu1_di1_pre",     "ipu1_di1_pre_sel",  base + 0x34, 12, 3);
 	clk[ipu2_di0_pre]     = imx_clk_divider("ipu2_di0_pre",     "ipu2_di0_pre_sel",  base + 0x38, 3,  3);