ARM: AM335x: LCDC platform support
v2 cover ARM: AM335x: LCDC platform support
Hi,
This series make am335x lcdc capable of providing display. Certain changes were required in generic OMAP clock handling to attain it. Clock nodes in LCDC path is marked such that rate can get propogated to upstream clocks till display PLL.
Based on 3.8-rc3.
Tested on AM335x EVM.
To test on AM335x based boards, tree
@ git://gitorious.org/x0148406-public/linux-kernel.git tags/da8xx-fb-dt-v4
Regards
Afzal
v2: As DEFINE_CLK_DIVIDER args has no change, make it's usage as reqd.
Afzal Mohammed (4):
ARM: OMAP2+: dpll: round rate to closest value
ARM: OMAP2+: dpll: am335x - avoid freqsel
ARM: OMAP2+: clock: DEFINE_STRUCT_CLK_FLAGS helper
ARM: AM33XX: clock: SET_RATE_PARENT in lcd path
arch/arm/mach-omap2/cclock33xx_data.c | 10 ++++++---- arch/arm/mach-omap2/clkt_dpll.c | 12 +++++++----- arch/arm/mach-omap2/clock.h | 11 +++++++++++ arch/arm/mach-omap2/dpll3xxx.c | 5 +++-- 4 files changed, 27 insertions(+), 11 deletions(-)
Signature · 1 line
1.7.12
PATCH v2 1/4 ARM: OMAP2+: dpll: round rate to closest value
Currently round rate function would return proper rate iff requested rate exactly matches the PLL lockable rate. This causes set_rate to fail if exact rate could not be set. Instead round rate may return closest rate possible (less than the requested). And if any user is badly in need of exact rate, then return value of round rate could be used to decide whether to invoke set rate or not.
Modify round rate so that it return closest possible rate.
This was required to get display working on am335x. Without this display rate could not be set (taking help of SET_RATE_PARENT). Couple of the downstream clocks of display PLL are basic clock dividers and they do MULT_ROUND_UP before requesting rate on PLL causing values that mostly could not be locked by PLL. And even otherwise, if requested rate for a particular pixel clock could not be satisfied by PLL, display would not work. This change will resolve the issue.
--- arch/arm/mach-omap2/clkt_dpll.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
arch/arm/mach-omap2/clkt_dpll.c +7 -5
@@ -345,20 +345,22 @@ long omap2_dpll_round_rate(struct clk_hw *hw, unsigned long target_rate,pr_debug ("clock: %s: m = %d: n = %d: new_rate = %ld\n" , clk_name, m, n, new_rate); -if (target_rate == new_rate) { +if ((new_rate <= target_rate) && + (new_rate > dd->last_rounded_rate)) { dd->last_rounded_m = m; dd->last_rounded_n = n; - dd->last_rounded_rate = target_rate; -break ; + dd->last_rounded_rate = new_rate; +if (new_rate == target_rate) +break ; } } -if (target_rate != new_rate) { +if (!dd->last_rounded_rate) {pr_debug ("clock: %s: cannot round to rate %ld\n" , clk_name, target_rate);return ~0 ; } -return target_rate; +return dd->last_rounded_rate; }
Signature · 1 line
1.7.12
PATCH v2 2/4 ARM: OMAP2+: dpll: am335x - avoid freqsel
In reply to Afzal Mohammedam335x does not have freqsel, avoid it.
--- arch/arm/mach-omap2/dpll3xxx.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/dpll3xxx.c +3 -2
@@ -500,8 +500,9 @@ int omap3_noncore_dpll_set_rate(struct clk_hw *hw, unsigned long rate,if (dd->last_rounded_rate ==0 )return -EINVAL ; -/* No freqsel on OMAP4 and OMAP3630 */ -if (!cpu_is_omap44xx () && !cpu_is_omap3630 ()) { +/* No freqsel on AM335x, OMAP4 and OMAP3630 */ +if (!soc_is_am33xx () && !cpu_is_omap44xx () && + !cpu_is_omap3630 ()) { freqsel =_omap3_dpll_compute_freqsel (clk, dd->last_rounded_n);WARN_ON (!freqsel);
Signature · 1 line
1.7.12
PATCH v2 3/4 ARM: OMAP2+: clock: DEFINE_STRUCT_CLK_FLAGS helper
In reply to Afzal MohammedDEFINE_STRUCT_CLK does not have the capability to set flags, define DEFINE_STRUCT_CLK_FLAGS to handle flags. This is needed to add SET_RATE_PARENT flag in statically defined lcd clock in am335x.
--- arch/arm/mach-omap2/clock.h | 11 +++++++++++ 1 file changed, 11 insertions(+)
arch/arm/mach-omap2/clock.h +11 -0
@@ -65,6 +65,17 @@ struct clockdomain; .ops = &_clkops_name, \ }; +#define DEFINE_STRUCT_CLK_FLAGS (_name, _parent_array_name, \ + _clkops_name, _flags) \ + static struct clk _name = { \ + .name = #_name, \ + .hw = &_name##_hw.hw, \ + .parent_names = _parent_array_name, \ + .num_parents = ARRAY_SIZE(_parent_array_name), \ + .ops = &_clkops_name, \ + .flags = _flags, \ + }; +#define DEFINE_STRUCT_CLK_HW_OMAP (_name, _clkdm_name) \ static struct clk_hw_omap _name##_hw = { \ .hw = { \
Signature · 1 line
1.7.12
PATCH v2 4/4 ARM: AM33XX: clock: SET_RATE_PARENT in lcd path
In reply to Afzal MohammedLCDC clock node is a one that does not have set rate capability. It just passes on the rate that is sent downstream by it's parent. While lcdc clock parent and it's grand parent - dpll_disp_m2_ck and dpll_disp_ck has the capability to configure rate.
And the default rates provided by LCDC clock's ancestors are not sufficient to obtain pixel clock for current LCDC use cases, hence currently display would not work on AM335x SoC's (with driver modifications in platfrom independent way).
Hence inform clock framework to propogate set rate for LCDC clock as well as it's parent - dpll_disp_m2_ck. With this change, set rate on LCDC clock would get propogated till dpll_disp_ck via dpll_disp_m2_ck, hence allowing the driver (same driver is used in DaVinci too) to set rates using LCDC clock without worrying about platform dependent clock details.
---
v2: As DEFINE_CLK_DIVIDER args has no change, make it's usage as reqd.
arch/arm/mach-omap2/cclock33xx_data.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/cclock33xx_data.c +6 -4
@@ -284,9 +284,10 @@ DEFINE_STRUCT_CLK(dpll_disp_ck, dpll_core_ck_parents, dpll_ddr_ck_ops);* TODO: Add clksel here (sys_clkin, CORE_CLKOUTM6, PER_CLKOUTM2 * and ALT_CLK1/2) */ -DEFINE_CLK_DIVIDER (dpll_disp_m2_ck,"dpll_disp_ck" , &dpll_disp_ck,0x0 , -AM33XX_CM_DIV_M2_DPLL_DISP ,AM33XX_DPLL_CLKOUT_DIV_SHIFT , -AM33XX_DPLL_CLKOUT_DIV_WIDTH ,CLK_DIVIDER_ONE_BASED ,NULL ); +DEFINE_CLK_DIVIDER (dpll_disp_m2_ck,"dpll_disp_ck" , &dpll_disp_ck, +CLK_SET_RATE_PARENT ,AM33XX_CM_DIV_M2_DPLL_DISP , +AM33XX_DPLL_CLKOUT_DIV_SHIFT ,AM33XX_DPLL_CLKOUT_DIV_WIDTH , +CLK_DIVIDER_ONE_BASED ,NULL );/* DPLL_PER */ static struct dpll_data dpll_per_dd = {
@@ -723,7 +724,8 @@ static struct clk_hw_omap lcd_gclk_hw = { .clksel_mask = AM33XX_CLKSEL_0_1_MASK, }; -DEFINE_STRUCT_CLK (lcd_gclk, lcd_ck_parents, gpio_fck_ops); +DEFINE_STRUCT_CLK_FLAGS (lcd_gclk, lcd_ck_parents, + gpio_fck_ops,CLK_SET_RATE_PARENT );DEFINE_CLK_FIXED_FACTOR (mmc_clk,"dpll_per_m2_ck" , &dpll_per_m2_ck,0x0 ,1 ,2 );
Signature · 1 line
1.7.12
Paul Walmsley
maintainerRe: [PATCH v2 1/4] ARM: OMAP2+: dpll: round rate to closest value
In reply to Afzal MohammedHi
Quoted 8 lines · Afzal Mohammed
Currently round rate function would return proper rate iff requested rate exactly matches the PLL lockable rate. This causes
set_rateto fail if exact rate could not be set. Instead round rate may return closest rate possible (less than the requested). And if any user is badly in need of exact rate, then return value of round rate could be used to decide whether to invoke set rate or not.Modify round rate so that it return closest possible rate.
This doesn't look like the right approach to me. For some PLLs, an exact rate is desired.
We removed the rate tolerance code in commit 241d3a8dca239610d3d991bf58d4fe38c2d86fd5, but that was probably premature. We've encountered several situations now where we could really use it, like MPU CPUFreq. I'd suggest reverting 241d3a8dca239610d3d991bf58d4fe38c2d86fd5 or using a similar approach.
- Paul
RE: [PATCH v2 1/4] ARM: OMAP2+: dpll: round rate to closest value
Hi Paul,
Quoted 9 lines · Paul Walmsley
Quoted 7 lines · Afzal Mohammed
Currently round rate function would return proper rate iff requested rate exactly matches the PLL lockable rate. This causes
set_rateto fail if exact rate could not be set. Instead round rate may return closest rate possible (less than the requested). And if any user is badly in need of exact rate, then return value of round rate could be used to decide whether to invoke set rate or not.
Quoted 4 lines
Quoted 1 line
Modify round rate so that it return closest possible rate.
This doesn't look like the right approach to me. For some PLLs, an exact rate is desired.
If exact rate is required, there is a way to achieve it as mentioned in the commit message, i.e. by first invoking round rate over reqd. rate and if it doesn't match, bail out w/o invoking set_rate.
And it seems requirement of CCF w.r.t to round rate is to return closest possible rate.
Quoted 5 lines
We removed the rate tolerance code in commit
241d3a8dca239610d3d991bf58d4fe38c2d86fd5, but that was probably premature. We've encountered several situations now where we could really use it, like MPU CPUFreq. I'd suggest reverting241d3a8dca239610d3d991bf58d4fe38c2d86fd5or using a similar approach.
As you prefer reverting the above commit, I will proceed so, hmm.. got not so simple merge conflict, wish there was a command,
git revert logical ..
Regards
Afzal
Re: RE: [PATCH v2 1/4] ARM: OMAP2+: dpll: round rate to closest value
Quoting Mohammed, Afzal (2013-01-25 04:18:22)
Quoted 17 lines
Hi Paul,
Quoted 9 lines · Paul Walmsley
Quoted 7 lines · Afzal Mohammed
Currently round rate function would return proper rate iff requested rate exactly matches the PLL lockable rate. This causes
set_rateto fail if exact rate could not be set. Instead round rate may return closest rate possible (less than the requested). And if any user is badly in need of exact rate, then return value of round rate could be used to decide whether to invoke set rate or not.Quoted 4 lines
Quoted 1 line
Modify round rate so that it return closest possible rate.
This doesn't look like the right approach to me. For some PLLs, an exact rate is desired.
Quoted 6 lines
If exact rate is required, there is a way to achieve it as mentioned in the commit message, i.e. by first invoking round rate over reqd. rate and if it doesn't match, bail out w/o invoking
set_rate.And it seems requirement of CCF w.r.t to round rate is to return closest possible rate.
Is MULT_ROUND_UP doing the right thing for you in the clk_divider code? What is the clock rate requested of the parent PLL? I just want to make sure that we're doing the right thing in the basic divider code.
Thanks,
Mike
Paul Walmsley
maintainerRE: [PATCH v2 1/4] ARM: OMAP2+: dpll: round rate to closest value
In reply to Mohammed, AfzalHi
Quoted 15 lines · Afzal
Quoted 9 lines · Paul Walmsley
Quoted 7 lines · Afzal Mohammed
Currently round rate function would return proper rate iff requested rate exactly matches the PLL lockable rate. This causes
set_rateto fail if exact rate could not be set. Instead round rate may return closest rate possible (less than the requested). And if any user is badly in need of exact rate, then return value of round rate could be used to decide whether to invoke set rate or not.Quoted 4 lines
Quoted 1 line
Modify round rate so that it return closest possible rate.
This doesn't look like the right approach to me. For some PLLs, an exact rate is desired.
Quoted 6 lines
If exact rate is required, there is a way to achieve it as mentioned in the commit message, i.e. by first invoking round rate over reqd. rate and if it doesn't match, bail out w/o invoking
set_rate.And it seems requirement of CCF w.r.t to round rate is to return closest possible rate.
Hmm. Maybe I need to take a closer look. I'm a little worried that, since __clk_round_rate() can be called from omap3_noncore_dpll_set_rate(), we might wind up with inconsistent behavior. Effectively we'd need to mandate that clk_round_rate() would have to be called first for any DPLL where we'd expect to set an exact rate.
- Paul
RE: RE: [PATCH v2 1/4] ARM: OMAP2+: dpll: round rate to closest value
In reply to Mike TurquetteHi Mike,
Quoted 3 lines · Mike Turquette
Is
MULT_ROUND_UPdoing the right thing for you in theclk_dividercode? What is the clock rate requested of the parent PLL? I just want to make sure that we're doing the right thing in the basic divider code.
Actually MULT_ROUND_UP made my life difficult earlier, and finally came up with this solution instead of removing it.
It was something like 60000000 requested of PLL, for i = 1, but for other values, it was something like 60000001, 60000002 etc.
Even if round rate rounds, I thought removing MULT_ROUND_UP would be ok, couldn't spend time to understand fully rational behind it, and as it was in generic code, kept away from doing it.
Regards
Afzal
RE: [PATCH v2 1/4] ARM: OMAP2+: dpll: round rate to closest value
In reply to Mohammed, AfzalHi Paul,
Quoted 6 lines · Afzal
Quoted 2 lines · Paul Walmsley
like MPU CPUFreq. I'd suggest reverting
241d3a8dca239610d3d991bf58d4fe38c2d86fd5or using a similar approach.As you prefer reverting the above commit, I will proceed so, hmm.. got
This patch or reverting the above mentioned commit is not required for lcdc to be usable on am335x, instead please consider for inclusion patches 3 & 4 of this series,
"ARM: OMAP2+: clock: DEFINE_STRUCT_CLK_FLAGS helper" and "ARM: AM33XX: clock: SET_RATE_PARENT in lcd path"
Patches 3 & 4 are required to have a functional frame buffer driver on am335x, also it may help drm driver for the lcdc ip in am335x to be a platform independent one.
Regards
Afzal