| Message ID | b135c5d6-f06d-030d-333b-5223ef1aca9a@ti.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
* Tero Kristo <t-kristo@ti.com> [170406 09:51]: > On 03/04/17 18:36, Tony Lindgren wrote: > > * Tero Kristo <t-kristo@ti.com> [170403 07:54]: > > > On 30/03/17 19:54, Tony Lindgren wrote: > > > > * Tero Kristo <t-kristo@ti.com> [170330 00:20]: > > > > > On 23/03/17 19:02, Tony Lindgren wrote: > > > > > > * Tony Lindgren <tony@atomide.com> [170322 18:03]: > > > > > > > * Tero Kristo <t-kristo@ti.com> [170317 14:39]: > > > > > > > > On 17/03/17 17:25, Tony Lindgren wrote: > > > > > > > > > * Tero Kristo <t-kristo@ti.com> [170317 02:12]: > > > > > > > > > > Any additional testing on omap4 welcome as this series basically > > > > > > > > > > tweaks every possible peripheral clock on the SoC. > > > > > > > > > > > > > > > > > > Without the last patch in this series, booting fails for me: > > > > > > > > > > > > > > > > > > [ 5.074890] l4_per_cm:clk:0120:0: failed to disable > > > > > > > > > [ 5.085113] l4_per_cm:clk:0128:0: failed to disable > > > > > > > > > > > > > > > > > > Care to check that booting keeps working for each patch in the > > > > > > > > > series to avoid breaking git bisect for booting? > > > > > > > > > > > > > > > > Hmm, I think patch 8+9 need to be squashed then. I can double check this > > > > > > > > next week though. > > > > > > > > > > > > > > Also looks like with this set merged HDMI stops working on > > > > > > > omap4 with: > > > > > > > > > > > > > > HDMIWP: omapdss HDMIWP error: Failed to set PHY power mode to 1 > > > > > > > > > > > > Forgot to mention that's with omapdrm with encoder-tpd12s015 and > > > > > > encoder-tfp410 modules loaded to get HDMI working. Here's more verbose > > > > > > dmesg output in case that provides more clues: > > > > > > > > > > > > [ 91.042877] omapdss HDMICORE error: operation stopped when reading edid > > > > > > [ 91.078308] [drm] Enabling DMM ywrap scrolling > > > > > > [ 91.099243] omapdss HDMIWP error: Failed to set PHY power mode to 1 > > > > > > [ 91.107879] omapdss HDMI error: failed to power on device > > > > > > [ 91.107879] omapdrm omapdrm.0: Failed to enable display 'hdmi': -5 > > > > > > [ 91.359619] omapdrm omapdrm.0: atomic complete timeout (pipe 0)! > > > > > > [ 91.619964] omapdrm omapdrm.0: atomic complete timeout (pipe 0)! > > > > > > [ 91.620300] Console: switching to colour frame buffer device 128x48 > > > > > > [ 91.682434] omapdrm omapdrm.0: fb0: omapdrm frame buffer device > > > > > > [ 91.770812] [drm] Initialized omapdrm 1.0.0 20110917 for omapdrm.0 on minor 0 > > > > > > [ 92.818054] omapdss HDMICORE error: operation stopped when reading edid > > > > > > [ 93.090087] omapdrm omapdrm.0: atomic complete timeout (pipe 0)! > > > > > > [ 93.349853] omapdrm omapdrm.0: atomic complete timeout (pipe 0)! > > > > > > > > > > > > Regards, > > > > > > > > > > > > Tony > > > > > > > > > > > > > > > > Can you try with this additional hwmod data tweak in place? Apply this on > > > > > top of the existing series. > > > > > > > > Does not seem to help, still get the same errors. But maybe I'm doing > > > > something wrong as the patch did not apply and I applied it manually. > > > > > > > > Regards, > > > > > > > > Tony > > > > > > > > > > Hmm ok, can you provide some brief instructions how to test what you are > > > doing with the HDMI? Just connect it to some external monitor? My monitor > > > has a spare HDMI connector so I could try it out. > > > > Well build a kernel using omap2plus_defconfig, then with HDMI cable > > connected load the following modules: > > > > encoder-tpd12s015 encoder-tfp410 connector-hdmi omapfb > > > > And a console should appear on the HDMI monitor. If using omapdrm, then > > load omapdss and omapdrm instead. > > > > And if using NFSroot, you need to have ehci and smsc drivers built-in > > or use an initramfs. > > Ok, I have a solution to the issue. > > Try the slightly modified patch below. It just required a couple of hwmod > flags applied in addition to the patch I sent before. > > I also pushed a branch named "4.11-rc1-clkctrl-wip" as a reference, where > also the hdmi works fine on omap4. OK hdmi works now on panda for both omapfb and omapdrm. Any ideas why that change is now needed though? Regards, Tony > =================================== > > diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c > b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c > index dad871a..43163b5 100644 > --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c > +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c > @@ -775,6 +775,7 @@ > > static struct omap_hwmod_opt_clk dss_hdmi_opt_clks[] = { > { .role = "sys_clk", .clk = "dss_sys_clk" }, > + { .role = "hdmi_clk", .clk = "dss_48mhz_clk" }, > }; > > static struct omap_hwmod omap44xx_dss_hdmi_hwmod = { > @@ -785,7 +786,7 @@ > * HDMI audio requires to use no-idle mode. Hence, > * set idle mode by software. > */ > - .flags = HWMOD_SWSUP_SIDLE, > + .flags = HWMOD_SWSUP_SIDLE | HWMOD_OPT_CLKS_NEEDED, > .mpu_irqs = omap44xx_dss_hdmi_irqs, > .xlate_irq = omap4_xlate_irq, > .sdma_reqs = omap44xx_dss_hdmi_sdma_reqs, > @@ -858,11 +859,16 @@ > }; > > /* dss_venc */ > +static struct omap_hwmod_opt_clk dss_venc_opt_clks[] = { > + { .role = "tv_clk", .clk = "dss_tv_clk" }, > +}; > + > static struct omap_hwmod omap44xx_dss_venc_hwmod = { > .name = "dss_venc", > .class = &omap44xx_venc_hwmod_class, > .clkdm_name = "l3_dss_clkdm", > .main_clk = "dss_tv_clk", > + .flags = HWMOD_OPT_CLKS_NEEDED, > .prcm = { > .omap4 = { > .clkctrl_offs = OMAP4_CM_DSS_DSS_CLKCTRL_OFFSET, > @@ -870,6 +876,8 @@ > }, > }, > .parent_hwmod = &omap44xx_dss_hwmod, > + .opt_clks = dss_venc_opt_clks, > + .opt_clks_cnt = ARRAY_SIZE(dss_venc_opt_clks), > }; > > /* > > > -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/04/17 19:47, Tony Lindgren wrote: > * Tero Kristo <t-kristo@ti.com> [170406 09:51]: >> On 03/04/17 18:36, Tony Lindgren wrote: >>> * Tero Kristo <t-kristo@ti.com> [170403 07:54]: >>>> On 30/03/17 19:54, Tony Lindgren wrote: >>>>> * Tero Kristo <t-kristo@ti.com> [170330 00:20]: >>>>>> On 23/03/17 19:02, Tony Lindgren wrote: >>>>>>> * Tony Lindgren <tony@atomide.com> [170322 18:03]: >>>>>>>> * Tero Kristo <t-kristo@ti.com> [170317 14:39]: >>>>>>>>> On 17/03/17 17:25, Tony Lindgren wrote: >>>>>>>>>> * Tero Kristo <t-kristo@ti.com> [170317 02:12]: >>>>>>>>>>> Any additional testing on omap4 welcome as this series basically >>>>>>>>>>> tweaks every possible peripheral clock on the SoC. >>>>>>>>>> >>>>>>>>>> Without the last patch in this series, booting fails for me: >>>>>>>>>> >>>>>>>>>> [ 5.074890] l4_per_cm:clk:0120:0: failed to disable >>>>>>>>>> [ 5.085113] l4_per_cm:clk:0128:0: failed to disable >>>>>>>>>> >>>>>>>>>> Care to check that booting keeps working for each patch in the >>>>>>>>>> series to avoid breaking git bisect for booting? >>>>>>>>> >>>>>>>>> Hmm, I think patch 8+9 need to be squashed then. I can double check this >>>>>>>>> next week though. >>>>>>>> >>>>>>>> Also looks like with this set merged HDMI stops working on >>>>>>>> omap4 with: >>>>>>>> >>>>>>>> HDMIWP: omapdss HDMIWP error: Failed to set PHY power mode to 1 >>>>>>> >>>>>>> Forgot to mention that's with omapdrm with encoder-tpd12s015 and >>>>>>> encoder-tfp410 modules loaded to get HDMI working. Here's more verbose >>>>>>> dmesg output in case that provides more clues: >>>>>>> >>>>>>> [ 91.042877] omapdss HDMICORE error: operation stopped when reading edid >>>>>>> [ 91.078308] [drm] Enabling DMM ywrap scrolling >>>>>>> [ 91.099243] omapdss HDMIWP error: Failed to set PHY power mode to 1 >>>>>>> [ 91.107879] omapdss HDMI error: failed to power on device >>>>>>> [ 91.107879] omapdrm omapdrm.0: Failed to enable display 'hdmi': -5 >>>>>>> [ 91.359619] omapdrm omapdrm.0: atomic complete timeout (pipe 0)! >>>>>>> [ 91.619964] omapdrm omapdrm.0: atomic complete timeout (pipe 0)! >>>>>>> [ 91.620300] Console: switching to colour frame buffer device 128x48 >>>>>>> [ 91.682434] omapdrm omapdrm.0: fb0: omapdrm frame buffer device >>>>>>> [ 91.770812] [drm] Initialized omapdrm 1.0.0 20110917 for omapdrm.0 on minor 0 >>>>>>> [ 92.818054] omapdss HDMICORE error: operation stopped when reading edid >>>>>>> [ 93.090087] omapdrm omapdrm.0: atomic complete timeout (pipe 0)! >>>>>>> [ 93.349853] omapdrm omapdrm.0: atomic complete timeout (pipe 0)! >>>>>>> >>>>>>> Regards, >>>>>>> >>>>>>> Tony >>>>>>> >>>>>> >>>>>> Can you try with this additional hwmod data tweak in place? Apply this on >>>>>> top of the existing series. >>>>> >>>>> Does not seem to help, still get the same errors. But maybe I'm doing >>>>> something wrong as the patch did not apply and I applied it manually. >>>>> >>>>> Regards, >>>>> >>>>> Tony >>>>> >>>> >>>> Hmm ok, can you provide some brief instructions how to test what you are >>>> doing with the HDMI? Just connect it to some external monitor? My monitor >>>> has a spare HDMI connector so I could try it out. >>> >>> Well build a kernel using omap2plus_defconfig, then with HDMI cable >>> connected load the following modules: >>> >>> encoder-tpd12s015 encoder-tfp410 connector-hdmi omapfb >>> >>> And a console should appear on the HDMI monitor. If using omapdrm, then >>> load omapdss and omapdrm instead. >>> >>> And if using NFSroot, you need to have ehci and smsc drivers built-in >>> or use an initramfs. >> >> Ok, I have a solution to the issue. >> >> Try the slightly modified patch below. It just required a couple of hwmod >> flags applied in addition to the patch I sent before. >> >> I also pushed a branch named "4.11-rc1-clkctrl-wip" as a reference, where >> also the hdmi works fine on omap4. > > OK hdmi works now on panda for both omapfb and omapdrm. Any ideas why > that change is now needed though? Yes, it is pretty clear actually. Previously, the hwmod main clock was controlling the hdmi clock, but this is now rerouted to the clkctrl module clock, which is shared between all DSS submodules. This leaves the hdmi clock disabled, causing the failure. The fix just puts the hdmi clock into the optional clocks list, and forces all the optional clocks on when pm_runtime is enabled for the module. -Tero > > Regards, > > Tony > >> =================================== >> >> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c >> b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c >> index dad871a..43163b5 100644 >> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c >> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c >> @@ -775,6 +775,7 @@ >> >> static struct omap_hwmod_opt_clk dss_hdmi_opt_clks[] = { >> { .role = "sys_clk", .clk = "dss_sys_clk" }, >> + { .role = "hdmi_clk", .clk = "dss_48mhz_clk" }, >> }; >> >> static struct omap_hwmod omap44xx_dss_hdmi_hwmod = { >> @@ -785,7 +786,7 @@ >> * HDMI audio requires to use no-idle mode. Hence, >> * set idle mode by software. >> */ >> - .flags = HWMOD_SWSUP_SIDLE, >> + .flags = HWMOD_SWSUP_SIDLE | HWMOD_OPT_CLKS_NEEDED, >> .mpu_irqs = omap44xx_dss_hdmi_irqs, >> .xlate_irq = omap4_xlate_irq, >> .sdma_reqs = omap44xx_dss_hdmi_sdma_reqs, >> @@ -858,11 +859,16 @@ >> }; >> >> /* dss_venc */ >> +static struct omap_hwmod_opt_clk dss_venc_opt_clks[] = { >> + { .role = "tv_clk", .clk = "dss_tv_clk" }, >> +}; >> + >> static struct omap_hwmod omap44xx_dss_venc_hwmod = { >> .name = "dss_venc", >> .class = &omap44xx_venc_hwmod_class, >> .clkdm_name = "l3_dss_clkdm", >> .main_clk = "dss_tv_clk", >> + .flags = HWMOD_OPT_CLKS_NEEDED, >> .prcm = { >> .omap4 = { >> .clkctrl_offs = OMAP4_CM_DSS_DSS_CLKCTRL_OFFSET, >> @@ -870,6 +876,8 @@ >> }, >> }, >> .parent_hwmod = &omap44xx_dss_hwmod, >> + .opt_clks = dss_venc_opt_clks, >> + .opt_clks_cnt = ARRAY_SIZE(dss_venc_opt_clks), >> }; >> >> /* >> >> >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-clk" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Tero Kristo <t-kristo@ti.com> [170410 00:34]: > On 07/04/17 19:47, Tony Lindgren wrote: > > OK hdmi works now on panda for both omapfb and omapdrm. Any ideas why > > that change is now needed though? > > Yes, it is pretty clear actually. Previously, the hwmod main clock was > controlling the hdmi clock, but this is now rerouted to the clkctrl module > clock, which is shared between all DSS submodules. This leaves the hdmi > clock disabled, causing the failure. > > The fix just puts the hdmi clock into the optional clocks list, and forces > all the optional clocks on when pm_runtime is enabled for the module. OK so no need to fix it in the current kernels then. Thanks, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/04/17 19:18, Tony Lindgren wrote: > * Tero Kristo <t-kristo@ti.com> [170410 00:34]: >> On 07/04/17 19:47, Tony Lindgren wrote: >>> OK hdmi works now on panda for both omapfb and omapdrm. Any ideas why >>> that change is now needed though? >> >> Yes, it is pretty clear actually. Previously, the hwmod main clock was >> controlling the hdmi clock, but this is now rerouted to the clkctrl module >> clock, which is shared between all DSS submodules. This leaves the hdmi >> clock disabled, causing the failure. >> >> The fix just puts the hdmi clock into the optional clocks list, and forces >> all the optional clocks on when pm_runtime is enabled for the module. > > OK so no need to fix it in the current kernels then. Yeah, just need to figure out a clean order of applying these patches. The hwmod data change for DSS modules can most likely be applied before the dts change for clkctrl is going in, as it is just causing double clk_enable / disable for the main clocks in that case. -Tero -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Tero Kristo <t-kristo@ti.com> [170410 11:36]: > On 10/04/17 19:18, Tony Lindgren wrote: > > * Tero Kristo <t-kristo@ti.com> [170410 00:34]: > > > On 07/04/17 19:47, Tony Lindgren wrote: > > > > OK hdmi works now on panda for both omapfb and omapdrm. Any ideas why > > > > that change is now needed though? > > > > > > Yes, it is pretty clear actually. Previously, the hwmod main clock was > > > controlling the hdmi clock, but this is now rerouted to the clkctrl module > > > clock, which is shared between all DSS submodules. This leaves the hdmi > > > clock disabled, causing the failure. > > > > > > The fix just puts the hdmi clock into the optional clocks list, and forces > > > all the optional clocks on when pm_runtime is enabled for the module. > > > > OK so no need to fix it in the current kernels then. > > Yeah, just need to figure out a clean order of applying these patches. The > hwmod data change for DSS modules can most likely be applied before the dts > change for clkctrl is going in, as it is just causing double clk_enable / > disable for the main clocks in that case. Yeah no idea about the hwmod change needed for DSS.. Maybe patch it in only in the no "ti,hwmods" path? For the rest, how about the following for getting things patched with a minimal patching needed to revert in case of regressions: 1. Let's add the clkctrl driver and it's dts clocks. Let's tag the new clkctrl clocks with status = "disabled" and not add the consumers into any existing devices 2. Let's modify the dts files to add the hwmod related items for the wrapper IP module but let's keep things probing the old way using compatible = "simple-bus" for now 3. Let's add the wrapper IP driver and make sure it does nothing except probe the children just like "simple-bus" currently does if the child module has the "ti,hwmods" property set 4. Then we can finally start flipping things over one module at a time just by removing "ti,hwmods" for the child device and status = "disabled" for the related module clock Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
=================================== diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c index dad871a..43163b5 100644 --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c @@ -775,6 +775,7 @@ static struct omap_hwmod_opt_clk dss_hdmi_opt_clks[] = { { .role = "sys_clk", .clk = "dss_sys_clk" }, + { .role = "hdmi_clk", .clk = "dss_48mhz_clk" }, }; static struct omap_hwmod omap44xx_dss_hdmi_hwmod = { @@ -785,7 +786,7 @@ * HDMI audio requires to use no-idle mode. Hence, * set idle mode by software. */ - .flags = HWMOD_SWSUP_SIDLE, + .flags = HWMOD_SWSUP_SIDLE | HWMOD_OPT_CLKS_NEEDED, .mpu_irqs = omap44xx_dss_hdmi_irqs, .xlate_irq = omap4_xlate_irq, .sdma_reqs = omap44xx_dss_hdmi_sdma_reqs, @@ -858,11 +859,16 @@ }; /* dss_venc */ +static struct omap_hwmod_opt_clk dss_venc_opt_clks[] = { + { .role = "tv_clk", .clk = "dss_tv_clk" }, +}; + static struct omap_hwmod omap44xx_dss_venc_hwmod = { .name = "dss_venc", .class = &omap44xx_venc_hwmod_class, .clkdm_name = "l3_dss_clkdm", .main_clk = "dss_tv_clk", + .flags = HWMOD_OPT_CLKS_NEEDED, .prcm = { .omap4 = { .clkctrl_offs = OMAP4_CM_DSS_DSS_CLKCTRL_OFFSET, @@ -870,6 +876,8 @@ }, }, .parent_hwmod = &omap44xx_dss_hwmod, + .opt_clks = dss_venc_opt_clks, + .opt_clks_cnt = ARRAY_SIZE(dss_venc_opt_clks), }; /*