Hi,
Here is an attempt at getting the HDMI controller running.
This HDMI controller is found on a number of old Allwinner SoCs (A10, A10s, A20, A31).
This driver only supports for now the A10s because it was an easy target, being very close to the A13 that is already supported by our DRM driver.
There's nothing out of the extraordinary there, except maybe the clock setup. All the internal clocks (TMDS, DDC) have been modeled using the common clock framework, the TMDS clock being the parent of the DDC one.
While this might sound overkill, other SoC have a different, external source for the DDC clock, which will be easier to support through the clock framework.
It's still a bit rough around the edges, as it doesn't work for all the modes. This will need to be fixed before being merged obviously.
The IP also supports audio (through an already supported i2s controller, and some missing configuration in the HDMI controller) and CEC. Both will come eventually.
Let me know what you think! Maxime
Maxime Ripard (15): clk: divider: Make divider_round_rate take the parent clock clk: sunxi-ng: Pass the parent and a pointer to the clocks round rate clk: sunxi-ng: div: Switch to divider_round_rate clk: sunxi-ng: mux: Don't just rely on the parent for CLK_SET_RATE_PARENT clk: sunxi-ng: sun5i: Export video PLLs dt-bindings: display: sun4i: Add HDMI display bindings dt-bindings: display: sun4i: Add allwinner,tcon-channel property drm/sun4i: tcon: Add channel debug drm/sun4i: tcon: Pass the encoder to the mode set functions drm/sun4i: tcon: Switch mux on only for composite drm/sun4i: tcon: Fix tcon channel 1 backporch calculation drm/sun4i: tcon: multiply the vtotal when not in interlace drm/sun4i: Add HDMI support ARM: sun5i: a10s: Add the HDMI controller node ARM: sun5i: a10s-olinuxino: Enable HDMI
Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 32 +- arch/arm/boot/dts/sun5i-a10s-olinuxino-micro.dts | 12 +- arch/arm/boot/dts/sun5i-a10s.dtsi | 34 +- arch/arm/boot/dts/sun5i.dtsi | 1 +- drivers/clk/clk-divider.c | 18 +- drivers/clk/hisilicon/clkdivider-hi6220.c | 5 +- drivers/clk/meson/clk-cpu.c | 5 +- drivers/clk/nxp/clk-lpc32xx.c | 5 +- drivers/clk/qcom/clk-alpha-pll.c | 5 +- drivers/clk/qcom/clk-regmap-divider.c | 3 +- drivers/clk/sunxi-ng/ccu-sun5i.h | 6 +- drivers/clk/sunxi-ng/ccu_div.c | 28 +- drivers/clk/sunxi-ng/ccu_mp.c | 7 +- drivers/clk/sunxi-ng/ccu_mult.c | 11 +- drivers/clk/sunxi-ng/ccu_mux.c | 22 +- drivers/clk/sunxi-ng/ccu_mux.h | 3 +- drivers/clk/sunxi-ng/ccu_nkm.c | 7 +- drivers/gpu/drm/sun4i/Makefile | 5 +- drivers/gpu/drm/sun4i/sun4i_hdmi.h | 124 ++- drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c | 128 ++- drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 449 +++++++- drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 236 ++++- drivers/gpu/drm/sun4i/sun4i_rgb.c | 2 +- drivers/gpu/drm/sun4i/sun4i_tcon.c | 25 +- drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 +- drivers/gpu/drm/sun4i/sun4i_tv.c | 2 +- drivers/rtc/rtc-ac100.c | 6 +- include/dt-bindings/clock/sun5i-ccu.h | 3 +- include/linux/clk-provider.h | 5 +- 29 files changed, 1103 insertions(+), 90 deletions(-) create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi.h create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c
base-commit: d1bee31b9da7222c6be3248d1f3b087e8cc9004c
On Thu, Mar 09, 2017 at 07:31:27PM +0800, Chen-Yu Tsai wrote:
Additionally, the mux registers are only valid in the first TCON, meaning it must available be active in 2 pipeline chips. It's also why we'd pass "struct drm_device *" instead of "struct sun4i_tcon *".
Hmmmm. That's going to be tricky to support. Has this been confirmed somehow? Is the register used for something else on TCON1?
At this point, the only reference is Allwinner's kernel, and the old 3.4 kernel for A10/A20. I could try getting HDMI working on the A31 to get some real results.
FWIW, the registers do not seem to be aliased across the two TCONs.
Then maybe we don't need to care, and we can just always write to the mux?
Maxime
So far, divider_round_rate only considers the parent clock returned by clk_hw_get_parent.
This works fine on clocks that have a single parents, this doesn't work on muxes, since we will only consider the first parent, while other parents may totally be able to provide a better combination.
Clocks in that case cannot use divider_round_rate, so would have to come up with a very similar logic to work around it. Instead of having to do something like this, and duplicate that logic everywhere, give an additional parameter for the parent clock to consider.
Current users have been converted using the following coccinelle script
@@ identifier hw, rate, prate, table, width, flags; @@
-long divider_round_rate(struct clk_hw *hw, +long divider_round_rate(struct clk_hw *hw, struct clk_hw *parent, unsigned long rate, unsigned long *prate, const struct clk_div_table *table, u8 width, unsigned long flags) { ... }
@@ identifier fn, hw; expression E2, E3, E4, E5, E6; @@ fn (struct clk_hw *hw, ...) { <... -divider_round_rate(hw, E2, E3, E4, E5, E6) +divider_round_rate(hw, clk_hw_get_parent(hw), E2, E3, E4, E5, E6) ...> }
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com Cc: Carlo Caione carlo@caione.org Cc: Kevin Hilman khilman@baylibre.com Cc: Vladimir Zapolskiy vz@mleia.com Cc: Sylvain Lemieux slemieux.tyco@gmail.com Cc: Andy Gross andy.gross@linaro.org Cc: David Brown david.brown@linaro.org Cc: Alessandro Zummo a.zummo@towertech.it Cc: Alexandre Belloni alexandre.belloni@free-electrons.com Cc: linux-amlogic@lists.infradead.org Cc: linux-arm-msm@vger.kernel.org Cc: linux-soc@vger.kernel.org Cc: rtc-linux@googlegroups.com --- drivers/clk/clk-divider.c | 18 ++++++++++-------- drivers/clk/hisilicon/clkdivider-hi6220.c | 5 +++-- drivers/clk/meson/clk-cpu.c | 5 +++-- drivers/clk/nxp/clk-lpc32xx.c | 5 +++-- drivers/clk/qcom/clk-alpha-pll.c | 5 +++-- drivers/clk/qcom/clk-regmap-divider.c | 3 ++- drivers/clk/sunxi-ng/ccu_div.c | 6 +++--- drivers/rtc/rtc-ac100.c | 6 ++++-- include/linux/clk-provider.h | 5 +++-- 9 files changed, 34 insertions(+), 24 deletions(-)
diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c index 96386ffc8483..d8d7dc84956a 100644 --- a/drivers/clk/clk-divider.c +++ b/drivers/clk/clk-divider.c @@ -275,7 +275,8 @@ static int _next_div(const struct clk_div_table *table, int div, return div; }
-static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate, +static int clk_divider_bestdiv(struct clk_hw *hw, struct clk_hw *parent, + unsigned long rate, unsigned long *best_parent_rate, const struct clk_div_table *table, u8 width, unsigned long flags) @@ -314,8 +315,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate, *best_parent_rate = parent_rate_saved; return i; } - parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), - rate * i); + parent_rate = clk_hw_round_rate(parent, rate * i); now = DIV_ROUND_UP_ULL((u64)parent_rate, i); if (_is_best_div(rate, now, best, flags)) { bestdiv = i; @@ -326,19 +326,20 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
if (!bestdiv) { bestdiv = _get_maxdiv(table, width, flags); - *best_parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), 1); + *best_parent_rate = clk_hw_round_rate(parent, 1); }
return bestdiv; }
-long divider_round_rate(struct clk_hw *hw, unsigned long rate, +long divider_round_rate(struct clk_hw *hw, struct clk_hw *parent, + unsigned long rate, unsigned long *prate, const struct clk_div_table *table, u8 width, unsigned long flags) { int div;
- div = clk_divider_bestdiv(hw, rate, prate, table, width, flags); + div = clk_divider_bestdiv(hw, parent, rate, prate, table, width, flags);
return DIV_ROUND_UP_ULL((u64)*prate, div); } @@ -359,8 +360,9 @@ static long clk_divider_round_rate(struct clk_hw *hw, unsigned long rate, return DIV_ROUND_UP_ULL((u64)*prate, bestdiv); }
- return divider_round_rate(hw, rate, prate, divider->table, - divider->width, divider->flags); + return divider_round_rate(hw, clk_hw_get_parent(hw), rate, prate, + divider->table, divider->width, + divider->flags); }
int divider_get_val(unsigned long rate, unsigned long parent_rate, diff --git a/drivers/clk/hisilicon/clkdivider-hi6220.c b/drivers/clk/hisilicon/clkdivider-hi6220.c index a1c1f684ad58..deaa72902555 100644 --- a/drivers/clk/hisilicon/clkdivider-hi6220.c +++ b/drivers/clk/hisilicon/clkdivider-hi6220.c @@ -64,8 +64,9 @@ static long hi6220_clkdiv_round_rate(struct clk_hw *hw, unsigned long rate, { struct hi6220_clk_divider *dclk = to_hi6220_clk_divider(hw);
- return divider_round_rate(hw, rate, prate, dclk->table, - dclk->width, CLK_DIVIDER_ROUND_CLOSEST); + return divider_round_rate(hw, clk_hw_get_parent(hw), rate, prate, + dclk->table, dclk->width, + CLK_DIVIDER_ROUND_CLOSEST); }
static int hi6220_clkdiv_set_rate(struct clk_hw *hw, unsigned long rate, diff --git a/drivers/clk/meson/clk-cpu.c b/drivers/clk/meson/clk-cpu.c index f8b2b7efd016..63c44d8c7ea4 100644 --- a/drivers/clk/meson/clk-cpu.c +++ b/drivers/clk/meson/clk-cpu.c @@ -59,8 +59,9 @@ static long meson_clk_cpu_round_rate(struct clk_hw *hw, unsigned long rate, { struct meson_clk_cpu *clk_cpu = to_meson_clk_cpu_hw(hw);
- return divider_round_rate(hw, rate, prate, clk_cpu->div_table, - MESON_N_WIDTH, CLK_DIVIDER_ROUND_CLOSEST); + return divider_round_rate(hw, clk_hw_get_parent(hw), rate, prate, + clk_cpu->div_table, MESON_N_WIDTH, + CLK_DIVIDER_ROUND_CLOSEST); }
static int meson_clk_cpu_set_rate(struct clk_hw *hw, unsigned long rate, diff --git a/drivers/clk/nxp/clk-lpc32xx.c b/drivers/clk/nxp/clk-lpc32xx.c index 5b98ff9076f3..73e83a7da7d6 100644 --- a/drivers/clk/nxp/clk-lpc32xx.c +++ b/drivers/clk/nxp/clk-lpc32xx.c @@ -975,8 +975,9 @@ static long clk_divider_round_rate(struct clk_hw *hw, unsigned long rate, return DIV_ROUND_UP(*prate, bestdiv); }
- return divider_round_rate(hw, rate, prate, divider->table, - divider->width, divider->flags); + return divider_round_rate(hw, clk_hw_get_parent(hw), rate, prate, + divider->table, divider->width, + divider->flags); }
static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate, diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c index 47a1da3739ce..7c08afecc811 100644 --- a/drivers/clk/qcom/clk-alpha-pll.c +++ b/drivers/clk/qcom/clk-alpha-pll.c @@ -478,8 +478,9 @@ clk_alpha_pll_postdiv_round_rate(struct clk_hw *hw, unsigned long rate, { struct clk_alpha_pll_postdiv *pll = to_clk_alpha_pll_postdiv(hw);
- return divider_round_rate(hw, rate, prate, clk_alpha_div_table, - pll->width, CLK_DIVIDER_POWER_OF_TWO); + return divider_round_rate(hw, clk_hw_get_parent(hw), rate, prate, + clk_alpha_div_table, pll->width, + CLK_DIVIDER_POWER_OF_TWO); }
static int clk_alpha_pll_postdiv_set_rate(struct clk_hw *hw, unsigned long rate, diff --git a/drivers/clk/qcom/clk-regmap-divider.c b/drivers/clk/qcom/clk-regmap-divider.c index 53484912301e..63489f0f2a02 100644 --- a/drivers/clk/qcom/clk-regmap-divider.c +++ b/drivers/clk/qcom/clk-regmap-divider.c @@ -28,7 +28,8 @@ static long div_round_rate(struct clk_hw *hw, unsigned long rate, { struct clk_regmap_div *divider = to_clk_regmap_div(hw);
- return divider_round_rate(hw, rate, prate, NULL, divider->width, + return divider_round_rate(hw, clk_hw_get_parent(hw), rate, prate, + NULL, divider->width, CLK_DIVIDER_ROUND_CLOSEST); }
diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c index 4057e6021aa9..92855c2b30bb 100644 --- a/drivers/clk/sunxi-ng/ccu_div.c +++ b/drivers/clk/sunxi-ng/ccu_div.c @@ -78,10 +78,10 @@ static int ccu_div_determine_rate(struct clk_hw *hw, struct ccu_div *cd = hw_to_ccu_div(hw);
if (clk_hw_get_num_parents(hw) == 1) { - req->rate = divider_round_rate(hw, req->rate, + req->rate = divider_round_rate(hw, clk_hw_get_parent(hw), + req->rate, &req->best_parent_rate, - cd->div.table, - cd->div.width, + cd->div.table, cd->div.width, cd->div.flags);
req->best_parent_hw = clk_hw_get_parent(hw); diff --git a/drivers/rtc/rtc-ac100.c b/drivers/rtc/rtc-ac100.c index 9e336184491c..0cad1bc41aa7 100644 --- a/drivers/rtc/rtc-ac100.c +++ b/drivers/rtc/rtc-ac100.c @@ -153,13 +153,15 @@ static long ac100_clkout_round_rate(struct clk_hw *hw, unsigned long rate, int i;
if (prate == AC100_RTC_32K_RATE) - return divider_round_rate(hw, rate, &prate, NULL, + return divider_round_rate(hw, clk_hw_get_parent(hw), rate, + &prate, NULL, AC100_CLKOUT_DIV_WIDTH, CLK_DIVIDER_POWER_OF_TWO);
for (i = 0; ac100_clkout_prediv[i].div; i++) { tmp_prate = DIV_ROUND_UP(prate, ac100_clkout_prediv[i].val); - tmp_rate = divider_round_rate(hw, rate, &tmp_prate, NULL, + tmp_rate = divider_round_rate(hw, clk_hw_get_parent(hw), rate, + &tmp_prate, NULL, AC100_CLKOUT_DIV_WIDTH, CLK_DIVIDER_POWER_OF_TWO);
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index a428aec36ace..e17d4cc7660c 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -412,8 +412,9 @@ extern const struct clk_ops clk_divider_ro_ops; unsigned long divider_recalc_rate(struct clk_hw *hw, unsigned long parent_rate, unsigned int val, const struct clk_div_table *table, unsigned long flags); -long divider_round_rate(struct clk_hw *hw, unsigned long rate, - unsigned long *prate, const struct clk_div_table *table, +long divider_round_rate(struct clk_hw *hw, struct clk_hw *parent, + unsigned long rate, unsigned long *prate, + const struct clk_div_table *table, u8 width, unsigned long flags); int divider_get_val(unsigned long rate, unsigned long parent_rate, const struct clk_div_table *table, u8 width,
On 03/07, Maxime Ripard wrote:
So far, divider_round_rate only considers the parent clock returned by clk_hw_get_parent.
This works fine on clocks that have a single parents, this doesn't work on muxes, since we will only consider the first parent, while other parents may totally be able to provide a better combination.
Clocks in that case cannot use divider_round_rate, so would have to come up with a very similar logic to work around it. Instead of having to do something like this, and duplicate that logic everywhere, give an additional parameter for the parent clock to consider.
Current users have been converted using the following coccinelle script
@@ identifier hw, rate, prate, table, width, flags; @@
-long divider_round_rate(struct clk_hw *hw, +long divider_round_rate(struct clk_hw *hw, struct clk_hw *parent, unsigned long rate, unsigned long *prate, const struct clk_div_table *table, u8 width, unsigned long flags) { ... }
@@ identifier fn, hw; expression E2, E3, E4, E5, E6; @@ fn (struct clk_hw *hw, ...) { <... -divider_round_rate(hw, E2, E3, E4, E5, E6) +divider_round_rate(hw, clk_hw_get_parent(hw), E2, E3, E4, E5, E6) ...> }
Why not introduce another function like
divider_round_rate_parent() divider_round_rate_mux()
that takes the extra parent argument? Technically, a divider is considered to only have one parent, and if it has more than one parent, then it is a mux and a divider.
Hi Stephen,
On Tue, Mar 07, 2017 at 06:11:57AM -0800, Stephen Boyd wrote:
On 03/07, Maxime Ripard wrote:
So far, divider_round_rate only considers the parent clock returned by clk_hw_get_parent.
This works fine on clocks that have a single parents, this doesn't work on muxes, since we will only consider the first parent, while other parents may totally be able to provide a better combination.
Clocks in that case cannot use divider_round_rate, so would have to come up with a very similar logic to work around it. Instead of having to do something like this, and duplicate that logic everywhere, give an additional parameter for the parent clock to consider.
Current users have been converted using the following coccinelle script
@@ identifier hw, rate, prate, table, width, flags; @@
-long divider_round_rate(struct clk_hw *hw, +long divider_round_rate(struct clk_hw *hw, struct clk_hw *parent, unsigned long rate, unsigned long *prate, const struct clk_div_table *table, u8 width, unsigned long flags) { ... }
@@ identifier fn, hw; expression E2, E3, E4, E5, E6; @@ fn (struct clk_hw *hw, ...) { <... -divider_round_rate(hw, E2, E3, E4, E5, E6) +divider_round_rate(hw, clk_hw_get_parent(hw), E2, E3, E4, E5, E6) ...> }
Why not introduce another function like
divider_round_rate_parent() divider_round_rate_mux()
that takes the extra parent argument? Technically, a divider is considered to only have one parent, and if it has more than one parent, then it is a mux and a divider.
Yes, that would work too, without needing the cross tree change. I'll do that in the next version.
Thanks! Maxime
The clocks might need to modify their parent clocks. In order to make that possible, give them access to the parent clock being evaluated, and to a pointer to the parent rate so that they can modify it if needed.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- drivers/clk/sunxi-ng/ccu_div.c | 7 ++++--- drivers/clk/sunxi-ng/ccu_mp.c | 7 ++++--- drivers/clk/sunxi-ng/ccu_mult.c | 11 ++++++----- drivers/clk/sunxi-ng/ccu_mux.c | 8 +++++--- drivers/clk/sunxi-ng/ccu_mux.h | 3 ++- drivers/clk/sunxi-ng/ccu_nkm.c | 7 ++++--- 6 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c index 92855c2b30bb..7f8b06e38636 100644 --- a/drivers/clk/sunxi-ng/ccu_div.c +++ b/drivers/clk/sunxi-ng/ccu_div.c @@ -14,7 +14,8 @@ #include "ccu_div.h"
static unsigned long ccu_div_round_rate(struct ccu_mux_internal *mux, - unsigned long parent_rate, + struct clk_hw *parent, + unsigned long *parent_rate, unsigned long rate, void *data) { @@ -26,10 +27,10 @@ static unsigned long ccu_div_round_rate(struct ccu_mux_internal *mux, * several parents, while we might be called to evaluate * several different parents. */ - val = divider_get_val(rate, parent_rate, cd->div.table, cd->div.width, + val = divider_get_val(rate, *parent_rate, cd->div.table, cd->div.width, cd->div.flags);
- return divider_recalc_rate(&cd->common.hw, parent_rate, val, + return divider_recalc_rate(&cd->common.hw, *parent_rate, val, cd->div.table, cd->div.flags); }
diff --git a/drivers/clk/sunxi-ng/ccu_mp.c b/drivers/clk/sunxi-ng/ccu_mp.c index b583f186a804..de02e6c386d8 100644 --- a/drivers/clk/sunxi-ng/ccu_mp.c +++ b/drivers/clk/sunxi-ng/ccu_mp.c @@ -41,7 +41,8 @@ static void ccu_mp_find_best(unsigned long parent, unsigned long rate, }
static unsigned long ccu_mp_round_rate(struct ccu_mux_internal *mux, - unsigned long parent_rate, + struct clk_hw *hw, + unsigned long *parent_rate, unsigned long rate, void *data) { @@ -52,9 +53,9 @@ static unsigned long ccu_mp_round_rate(struct ccu_mux_internal *mux, max_m = cmp->m.max ?: 1 << cmp->m.width; max_p = cmp->p.max ?: 1 << ((1 << cmp->p.width) - 1);
- ccu_mp_find_best(parent_rate, rate, max_m, max_p, &m, &p); + ccu_mp_find_best(*parent_rate, rate, max_m, max_p, &m, &p);
- return parent_rate / p / m; + return *parent_rate / p / m; }
static void ccu_mp_disable(struct clk_hw *hw) diff --git a/drivers/clk/sunxi-ng/ccu_mult.c b/drivers/clk/sunxi-ng/ccu_mult.c index 8724c01171b1..76d17162366f 100644 --- a/drivers/clk/sunxi-ng/ccu_mult.c +++ b/drivers/clk/sunxi-ng/ccu_mult.c @@ -33,9 +33,10 @@ static void ccu_mult_find_best(unsigned long parent, unsigned long rate, }
static unsigned long ccu_mult_round_rate(struct ccu_mux_internal *mux, - unsigned long parent_rate, - unsigned long rate, - void *data) + struct clk_hw *parent, + unsigned long *parent_rate, + unsigned long rate, + void *data) { struct ccu_mult *cm = data; struct _ccu_mult _cm; @@ -47,9 +48,9 @@ static unsigned long ccu_mult_round_rate(struct ccu_mux_internal *mux, else _cm.max = (1 << cm->mult.width) + cm->mult.offset - 1;
- ccu_mult_find_best(parent_rate, rate, &_cm); + ccu_mult_find_best(*parent_rate, rate, &_cm);
- return parent_rate * _cm.mult; + return *parent_rate * _cm.mult; }
static void ccu_mult_disable(struct clk_hw *hw) diff --git a/drivers/clk/sunxi-ng/ccu_mux.c b/drivers/clk/sunxi-ng/ccu_mux.c index c6bb1f523232..bae735e252b6 100644 --- a/drivers/clk/sunxi-ng/ccu_mux.c +++ b/drivers/clk/sunxi-ng/ccu_mux.c @@ -61,7 +61,8 @@ int ccu_mux_helper_determine_rate(struct ccu_common *common, struct ccu_mux_internal *cm, struct clk_rate_request *req, unsigned long (*round)(struct ccu_mux_internal *, - unsigned long, + struct clk_hw *, + unsigned long *, unsigned long, void *), void *data) @@ -80,7 +81,8 @@ int ccu_mux_helper_determine_rate(struct ccu_common *common, ccu_mux_helper_adjust_parent_for_prediv(common, cm, -1, &adj_parent_rate);
- best_rate = round(cm, adj_parent_rate, req->rate, data); + best_rate = round(cm, best_parent, &adj_parent_rate, + req->rate, data);
goto out; } @@ -109,7 +111,7 @@ int ccu_mux_helper_determine_rate(struct ccu_common *common, ccu_mux_helper_adjust_parent_for_prediv(common, cm, i, &adj_parent_rate);
- tmp_rate = round(cm, adj_parent_rate, req->rate, data); + tmp_rate = round(cm, parent, &adj_parent_rate, req->rate, data); if (tmp_rate == req->rate) { best_parent = parent; best_parent_rate = parent_rate; diff --git a/drivers/clk/sunxi-ng/ccu_mux.h b/drivers/clk/sunxi-ng/ccu_mux.h index 47aba3a48245..4be56eee2bfd 100644 --- a/drivers/clk/sunxi-ng/ccu_mux.h +++ b/drivers/clk/sunxi-ng/ccu_mux.h @@ -86,7 +86,8 @@ int ccu_mux_helper_determine_rate(struct ccu_common *common, struct ccu_mux_internal *cm, struct clk_rate_request *req, unsigned long (*round)(struct ccu_mux_internal *, - unsigned long, + struct clk_hw *, + unsigned long *, unsigned long, void *), void *data); diff --git a/drivers/clk/sunxi-ng/ccu_nkm.c b/drivers/clk/sunxi-ng/ccu_nkm.c index 71f81e95a061..c5e010eb5991 100644 --- a/drivers/clk/sunxi-ng/ccu_nkm.c +++ b/drivers/clk/sunxi-ng/ccu_nkm.c @@ -102,7 +102,8 @@ static unsigned long ccu_nkm_recalc_rate(struct clk_hw *hw, }
static unsigned long ccu_nkm_round_rate(struct ccu_mux_internal *mux, - unsigned long parent_rate, + struct clk_hw *hw, + unsigned long *parent_rate, unsigned long rate, void *data) { @@ -116,9 +117,9 @@ static unsigned long ccu_nkm_round_rate(struct ccu_mux_internal *mux, _nkm.min_m = 1; _nkm.max_m = nkm->m.max ?: 1 << nkm->m.width;
- ccu_nkm_find_best(parent_rate, rate, &_nkm); + ccu_nkm_find_best(*parent_rate, rate, &_nkm);
- return parent_rate * _nkm.n * _nkm.k / _nkm.m; + return *parent_rate * _nkm.n * _nkm.k / _nkm.m; }
static int ccu_nkm_determine_rate(struct clk_hw *hw,
divider_round_rate already evaluates changing the parent rate if CLK_SET_RATE_PARENT is set. Now that we can do that on muxes too, let's just use it.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- drivers/clk/sunxi-ng/ccu_div.c | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c index 7f8b06e38636..0ccdd3dcc20c 100644 --- a/drivers/clk/sunxi-ng/ccu_div.c +++ b/drivers/clk/sunxi-ng/ccu_div.c @@ -20,18 +20,9 @@ static unsigned long ccu_div_round_rate(struct ccu_mux_internal *mux, void *data) { struct ccu_div *cd = data; - unsigned long val; - - /* - * We can't use divider_round_rate that assumes that there's - * several parents, while we might be called to evaluate - * several different parents. - */ - val = divider_get_val(rate, *parent_rate, cd->div.table, cd->div.width, - cd->div.flags);
- return divider_recalc_rate(&cd->common.hw, *parent_rate, val, - cd->div.table, cd->div.flags); + return divider_round_rate(&cd->common.hw, parent, rate, parent_rate, + cd->div.table, cd->div.width, cd->div.flags); }
static void ccu_div_disable(struct clk_hw *hw) @@ -78,18 +69,6 @@ static int ccu_div_determine_rate(struct clk_hw *hw, { struct ccu_div *cd = hw_to_ccu_div(hw);
- if (clk_hw_get_num_parents(hw) == 1) { - req->rate = divider_round_rate(hw, clk_hw_get_parent(hw), - req->rate, - &req->best_parent_rate, - cd->div.table, cd->div.width, - cd->div.flags); - - req->best_parent_hw = clk_hw_get_parent(hw); - - return 0; - } - return ccu_mux_helper_determine_rate(&cd->common, &cd->mux, req, ccu_div_round_rate, cd); }
The current code only rely on the parent to change its rate in the case where CLK_SET_RATE_PARENT is set.
However, some clock rates might be obtained only through a modification of the parent and the clock divider. Just rely on the round rate of the clocks to give us the best computation that might be achieved for a given rate.
round_rate functions now need to honor CLK_SET_RATE_PARENT, but either the functions already do that if they modify the parent, or don't modify the praents at all.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- drivers/clk/sunxi-ng/ccu_mux.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu_mux.c b/drivers/clk/sunxi-ng/ccu_mux.c index bae735e252b6..58b6e349a0ed 100644 --- a/drivers/clk/sunxi-ng/ccu_mux.c +++ b/drivers/clk/sunxi-ng/ccu_mux.c @@ -95,19 +95,7 @@ int ccu_mux_helper_determine_rate(struct ccu_common *common, if (!parent) continue;
- if (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) { - struct clk_rate_request parent_req = *req; - int ret = __clk_determine_rate(parent, &parent_req); - - if (ret) - continue; - - parent_rate = parent_req.rate; - } else { - parent_rate = clk_hw_get_rate(parent); - } - - adj_parent_rate = parent_rate; + adj_parent_rate = parent_rate = clk_hw_get_rate(parent); ccu_mux_helper_adjust_parent_for_prediv(common, cm, i, &adj_parent_rate);
The video PLLs are used directly by the HDMI controller. Export them so that we can use them in our DT node.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- drivers/clk/sunxi-ng/ccu-sun5i.h | 6 ++++-- include/dt-bindings/clock/sun5i-ccu.h | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun5i.h b/drivers/clk/sunxi-ng/ccu-sun5i.h index 8144487eb7ca..16f7aa92957e 100644 --- a/drivers/clk/sunxi-ng/ccu-sun5i.h +++ b/drivers/clk/sunxi-ng/ccu-sun5i.h @@ -28,15 +28,17 @@ #define CLK_PLL_AUDIO_4X 6 #define CLK_PLL_AUDIO_8X 7 #define CLK_PLL_VIDEO0 8 -#define CLK_PLL_VIDEO0_2X 9 + +/* The PLL_VIDEO0_2X is exported for HDMI */ + #define CLK_PLL_VE 10 #define CLK_PLL_DDR_BASE 11 #define CLK_PLL_DDR 12 #define CLK_PLL_DDR_OTHER 13 #define CLK_PLL_PERIPH 14 #define CLK_PLL_VIDEO1 15 -#define CLK_PLL_VIDEO1_2X 16
+/* The PLL_VIDEO0_2X is exported for HDMI */ /* The CPU clock is exported */
#define CLK_AXI 18 diff --git a/include/dt-bindings/clock/sun5i-ccu.h b/include/dt-bindings/clock/sun5i-ccu.h index aeb2e2f781fb..81f34d477aeb 100644 --- a/include/dt-bindings/clock/sun5i-ccu.h +++ b/include/dt-bindings/clock/sun5i-ccu.h @@ -19,6 +19,9 @@
#define CLK_HOSC 1
+#define CLK_PLL_VIDEO0_2X 9 + +#define CLK_PLL_VIDEO1_2X 16 #define CLK_CPU 17
#define CLK_AHB_OTG 23
Hi Maxime,
On Tue, Mar 7, 2017 at 7:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The video PLLs are used directly by the HDMI controller. Export them so that we can use them in our DT node.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
drivers/clk/sunxi-ng/ccu-sun5i.h | 6 ++++-- include/dt-bindings/clock/sun5i-ccu.h | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun5i.h b/drivers/clk/sunxi-ng/ccu-sun5i.h index 8144487eb7ca..16f7aa92957e 100644 --- a/drivers/clk/sunxi-ng/ccu-sun5i.h +++ b/drivers/clk/sunxi-ng/ccu-sun5i.h @@ -28,15 +28,17 @@ #define CLK_PLL_AUDIO_4X 6 #define CLK_PLL_AUDIO_8X 7 #define CLK_PLL_VIDEO0 8 -#define CLK_PLL_VIDEO0_2X 9
+/* The PLL_VIDEO0_2X is exported for HDMI */
#define CLK_PLL_VE 10 #define CLK_PLL_DDR_BASE 11 #define CLK_PLL_DDR 12 #define CLK_PLL_DDR_OTHER 13 #define CLK_PLL_PERIPH 14 #define CLK_PLL_VIDEO1 15 -#define CLK_PLL_VIDEO1_2X 16
+/* The PLL_VIDEO0_2X is exported for HDMI */
PLL_VIDEO*1*_2X, right?
/* The CPU clock is exported */
#define CLK_AXI 18
Thanks,
Hi Julian,
On Tue, Mar 07, 2017 at 09:21:19PM +1100, Julian Calaby wrote:
Hi Maxime,
On Tue, Mar 7, 2017 at 7:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The video PLLs are used directly by the HDMI controller. Export them so that we can use them in our DT node.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
drivers/clk/sunxi-ng/ccu-sun5i.h | 6 ++++-- include/dt-bindings/clock/sun5i-ccu.h | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun5i.h b/drivers/clk/sunxi-ng/ccu-sun5i.h index 8144487eb7ca..16f7aa92957e 100644 --- a/drivers/clk/sunxi-ng/ccu-sun5i.h +++ b/drivers/clk/sunxi-ng/ccu-sun5i.h @@ -28,15 +28,17 @@ #define CLK_PLL_AUDIO_4X 6 #define CLK_PLL_AUDIO_8X 7 #define CLK_PLL_VIDEO0 8 -#define CLK_PLL_VIDEO0_2X 9
+/* The PLL_VIDEO0_2X is exported for HDMI */
#define CLK_PLL_VE 10 #define CLK_PLL_DDR_BASE 11 #define CLK_PLL_DDR 12 #define CLK_PLL_DDR_OTHER 13 #define CLK_PLL_PERIPH 14 #define CLK_PLL_VIDEO1 15 -#define CLK_PLL_VIDEO1_2X 16
+/* The PLL_VIDEO0_2X is exported for HDMI */
PLL_VIDEO*1*_2X, right?
Good catch, I'll fix it. Thanks! Maxime
One of the possible output of the display pipeline, on the SoCs that have it, is the HDMI controller.
Add a binding for it.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 21 +++++++- 1 file changed, 21 insertions(+), 0 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index b82c00449468..4b280672658e 100644 --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt @@ -4,6 +4,27 @@ Allwinner A10 Display Pipeline The Allwinner A10 Display pipeline is composed of several components that are going to be documented below:
+HDMI Encoder +------------ + +The HDMI Encoder supports the HDMI video and audio outputs, and does +CEC. It is one end of the pipeline. + +Required properties: + - compatible: value must be one of: + * allwinner,sun5i-a10s-hdmi + - reg: base address and size of memory-mapped region + - clocks: phandles to the clocks feeding the HDMI encoder + * ahb: the HDMI interface clock + * mod: the HDMI module clock + * pll-0: the first video PLL + * pll-1: the second video PLL + - clock-names: the clock names mentioned above + + - ports: A ports node with endpoint definitions as defined in + Documentation/devicetree/bindings/media/video-interfaces.txt. The + first port should be the input endpoint. + TV Encoder ----------
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
One of the possible output of the display pipeline, on the SoCs that have it, is the HDMI controller.
Add a binding for it.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
Acked-by: Chen-Yu Tsai wens@csie.org
TODO: A31 will also need a DDC clock.
On Tue, Mar 07, 2017 at 09:56:25AM +0100, Maxime Ripard wrote:
One of the possible output of the display pipeline, on the SoCs that have it, is the HDMI controller.
Add a binding for it.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 21 +++++++- 1 file changed, 21 insertions(+), 0 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index b82c00449468..4b280672658e 100644 --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt @@ -4,6 +4,27 @@ Allwinner A10 Display Pipeline The Allwinner A10 Display pipeline is composed of several components that are going to be documented below:
+HDMI Encoder +------------
+The HDMI Encoder supports the HDMI video and audio outputs, and does +CEC. It is one end of the pipeline.
+Required properties:
- compatible: value must be one of:
- allwinner,sun5i-a10s-hdmi
- reg: base address and size of memory-mapped region
- clocks: phandles to the clocks feeding the HDMI encoder
- ahb: the HDMI interface clock
- mod: the HDMI module clock
- pll-0: the first video PLL
- pll-1: the second video PLL
- clock-names: the clock names mentioned above
- ports: A ports node with endpoint definitions as defined in
- Documentation/devicetree/bindings/media/video-interfaces.txt. The
- first port should be the input endpoint.
You need an output port to an HDMI connector node and an audio port.
TV Encoder
-- git-series 0.8.11
Hi Rob,
On Wed, Mar 15, 2017 at 12:26:22PM -0500, Rob Herring wrote:
+HDMI Encoder +------------
+The HDMI Encoder supports the HDMI video and audio outputs, and does +CEC. It is one end of the pipeline.
+Required properties:
- compatible: value must be one of:
- allwinner,sun5i-a10s-hdmi
- reg: base address and size of memory-mapped region
- clocks: phandles to the clocks feeding the HDMI encoder
- ahb: the HDMI interface clock
- mod: the HDMI module clock
- pll-0: the first video PLL
- pll-1: the second video PLL
- clock-names: the clock names mentioned above
- ports: A ports node with endpoint definitions as defined in
- Documentation/devicetree/bindings/media/video-interfaces.txt. The
- first port should be the input endpoint.
You need an output port to an HDMI connector node and an audio port.
I started to look at the audio, and I can't find a use for an audio port in the OF graph. As far as I understand, we will be using the hdmi-codec, that still requires an ASoC card to create the link between our i2s controller and the HDMI controller.
This work perfectly for us, but as far as I know, the simple-card stuff only requires a phandle, and not an OF graph endpoint, right?
Thanks! Maxime
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
One of the possible output of the display pipeline, on the SoCs that have it, is the HDMI controller.
Add a binding for it.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 21 +++++++- 1 file changed, 21 insertions(+), 0 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index b82c00449468..4b280672658e 100644 --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt @@ -4,6 +4,27 @@ Allwinner A10 Display Pipeline The Allwinner A10 Display pipeline is composed of several components that are going to be documented below:
+HDMI Encoder +------------
+The HDMI Encoder supports the HDMI video and audio outputs, and does +CEC. It is one end of the pipeline.
+Required properties:
- compatible: value must be one of:
- allwinner,sun5i-a10s-hdmi
- reg: base address and size of memory-mapped region
- clocks: phandles to the clocks feeding the HDMI encoder
- ahb: the HDMI interface clock
- mod: the HDMI module clock
- pll-0: the first video PLL
- pll-1: the second video PLL
- clock-names: the clock names mentioned above
The audio part needs a DMA handle. May we add this from day one?
Thanks ChenYu
- ports: A ports node with endpoint definitions as defined in
- Documentation/devicetree/bindings/media/video-interfaces.txt. The
- first port should be the input endpoint.
TV Encoder
-- git-series 0.8.11
The Allwinner Timings Controller has two, mutually exclusive, channels. When the binding has been introduced, it was assumed that there would be only a single user per channel in the system.
While this is likely for the channel 0 which only connects to LCD displays, it turns out that the channel 1 can be connected to multiple controllers in the SoC (HDMI and TV encoders for example). And while the simultaneous use of HDMI and TV outputs cannot be achieved, switching from one to the other at runtime definitely sounds plausible.
Add an extra property, allwinner,tcon-channel, to specify for a given endpoint which TCON channel it is connected to, while falling back to the previous mechanism if that property is missing.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 11 ++++--- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index 4b280672658e..18d445723c3e 100644 --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt @@ -68,10 +68,13 @@ Required properties: Documentation/devicetree/bindings/media/video-interfaces.txt. The first port should be the input endpoint, the second one the output
- The output should have two endpoints. The first is the block - connected to the TCON channel 0 (usually a panel or a bridge), the - second the block connected to the TCON channel 1 (usually the TV - encoder) + The output may have multiple endpoints. The TCON has two channels, + usually with the first channel being used for the panels interfaces + (RGB, LVDS, etc.), and the second being used for the outputs that + require another controller (TV Encoder, HDMI, etc.). The endpoints + will take an extra property, allwinner,tcon-channel, to specify the + channel the endpoint is associated to. If that property is not + present, the endpoint number will be used as the channel number.
On SoCs other than the A33, there is one more clock required: - 'tcon-ch1': The clock driving the TCON channel 1
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The Allwinner Timings Controller has two, mutually exclusive, channels. When the binding has been introduced, it was assumed that there would be only a single user per channel in the system.
While this is likely for the channel 0 which only connects to LCD displays, it turns out that the channel 1 can be connected to multiple controllers in the SoC (HDMI and TV encoders for example). And while the simultaneous use of HDMI and TV outputs cannot be achieved, switching from one to the other at runtime definitely sounds plausible.
Add an extra property, allwinner,tcon-channel, to specify for a given endpoint which TCON channel it is connected to, while falling back to the previous mechanism if that property is missing.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
Acked-by: Chen-Yu Tsai wens@csie.org
On Tue, Mar 07, 2017 at 09:56:26AM +0100, Maxime Ripard wrote:
The Allwinner Timings Controller has two, mutually exclusive, channels. When the binding has been introduced, it was assumed that there would be only a single user per channel in the system.
While this is likely for the channel 0 which only connects to LCD displays, it turns out that the channel 1 can be connected to multiple controllers in the SoC (HDMI and TV encoders for example). And while the simultaneous use of HDMI and TV outputs cannot be achieved, switching from one to the other at runtime definitely sounds plausible.
Add an extra property, allwinner,tcon-channel, to specify for a given endpoint which TCON channel it is connected to, while falling back to the previous mechanism if that property is missing.
I think perhaps TCON channels should have been ports rather than endpoints. The fact that the channels are mutually exclusive can be handled in the driver and doesn't really matter in the binding. How painful would it be to rework things to move TCON channel 1 from port 0, endpoint 1 to port 1?
Rob
On Thu, Mar 16, 2017 at 1:37 AM, Rob Herring robh@kernel.org wrote:
On Tue, Mar 07, 2017 at 09:56:26AM +0100, Maxime Ripard wrote:
The Allwinner Timings Controller has two, mutually exclusive, channels. When the binding has been introduced, it was assumed that there would be only a single user per channel in the system.
While this is likely for the channel 0 which only connects to LCD displays, it turns out that the channel 1 can be connected to multiple controllers in the SoC (HDMI and TV encoders for example). And while the simultaneous use of HDMI and TV outputs cannot be achieved, switching from one to the other at runtime definitely sounds plausible.
Add an extra property, allwinner,tcon-channel, to specify for a given endpoint which TCON channel it is connected to, while falling back to the previous mechanism if that property is missing.
I think perhaps TCON channels should have been ports rather than endpoints. The fact that the channels are mutually exclusive can be handled in the driver and doesn't really matter in the binding. How painful would it be to rework things to move TCON channel 1 from port 0, endpoint 1 to port 1?
Having a separate output port for channel 1 was one option we discussed. However it wouldn't work well with the kernel's of_graph based crtc detection (drm_of_find_possible_crtcs / drm_of_find_possible_crtcs), which has the crtcs bound via the output port. As the logic is used by multiple drivers, I'm not sure it's easy to rework or test.
Also, we still have to support old device trees using channel 1 from output port 0 endpoint 1. This is the TV encoder on sun5i (A10s/A13/R8).
Regards ChenYu
On Fri, Mar 17, 2017 at 11:34:45AM +0800, Chen-Yu Tsai wrote:
On Thu, Mar 16, 2017 at 1:37 AM, Rob Herring robh@kernel.org wrote:
On Tue, Mar 07, 2017 at 09:56:26AM +0100, Maxime Ripard wrote:
The Allwinner Timings Controller has two, mutually exclusive, channels. When the binding has been introduced, it was assumed that there would be only a single user per channel in the system.
While this is likely for the channel 0 which only connects to LCD displays, it turns out that the channel 1 can be connected to multiple controllers in the SoC (HDMI and TV encoders for example). And while the simultaneous use of HDMI and TV outputs cannot be achieved, switching from one to the other at runtime definitely sounds plausible.
Add an extra property, allwinner,tcon-channel, to specify for a given endpoint which TCON channel it is connected to, while falling back to the previous mechanism if that property is missing.
I think perhaps TCON channels should have been ports rather than endpoints. The fact that the channels are mutually exclusive can be handled in the driver and doesn't really matter in the binding. How painful would it be to rework things to move TCON channel 1 from port 0, endpoint 1 to port 1?
Having a separate output port for channel 1 was one option we discussed. However it wouldn't work well with the kernel's of_graph based crtc detection (drm_of_find_possible_crtcs / drm_of_find_possible_crtcs), which has the crtcs bound via the output port. As the logic is used by multiple drivers, I'm not sure it's easy to rework or test.
Can't we use a different logic than drm_of_find_possible_crtcs to fill the same role?
Also, we still have to support old device trees using channel 1 from output port 0 endpoint 1. This is the TV encoder on sun5i (A10s/A13/R8).
We could probably work something out if we go that way to deal with old DTs though.
Maxime
While all functions have debug logs, the channel enable and disable are not logged. Make sure this is the case.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 4 ++++ 1 file changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 505520baa585..7461ae107e54 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -55,6 +55,8 @@ EXPORT_SYMBOL(sun4i_tcon_enable);
void sun4i_tcon_channel_disable(struct sun4i_tcon *tcon, int channel) { + DRM_DEBUG_DRIVER("Disabling TCON channel %d\n", channel); + /* Disable the TCON's channel */ if (channel == 0) { regmap_update_bits(tcon->regs, SUN4I_TCON0_CTL_REG, @@ -72,6 +74,8 @@ EXPORT_SYMBOL(sun4i_tcon_channel_disable);
void sun4i_tcon_channel_enable(struct sun4i_tcon *tcon, int channel) { + DRM_DEBUG_DRIVER("Enabling TCON channel %d\n", channel); + /* Enable the TCON's channel */ if (channel == 0) { regmap_update_bits(tcon->regs, SUN4I_TCON0_CTL_REG,
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
While all functions have debug logs, the channel enable and disable are not logged. Make sure this is the case.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
Acked-by: Chen-Yu Tsai wens@csie.org
The mode set function need some changes based on which encoder is being used. Make sure we can differentiate between our encoders by passing the encoder structure asking for the mode set.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- drivers/gpu/drm/sun4i/sun4i_rgb.c | 2 +- drivers/gpu/drm/sun4i/sun4i_tcon.c | 4 ++-- drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 ++-- drivers/gpu/drm/sun4i/sun4i_tv.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c index 1147451eb993..1d4a59a44d04 100644 --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c @@ -173,7 +173,7 @@ static void sun4i_rgb_encoder_mode_set(struct drm_encoder *encoder, struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder); struct sun4i_tcon *tcon = rgb->tcon;
- sun4i_tcon0_mode_set(tcon, mode); + sun4i_tcon0_mode_set(tcon, encoder, mode);
clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 7461ae107e54..d2335f109601 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -127,7 +127,7 @@ static int sun4i_tcon_get_clk_delay(struct drm_display_mode *mode, return delay; }
-void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon, +void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon, struct drm_encoder *encoder, struct drm_display_mode *mode) { unsigned int bp, hsync, vsync; @@ -200,7 +200,7 @@ void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon, } EXPORT_SYMBOL(sun4i_tcon0_mode_set);
-void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon, +void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon, struct drm_encoder *encoder, struct drm_display_mode *mode) { unsigned int bp, hsync, vsync; diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h index f636343a935d..95b7e76eb1f8 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h @@ -190,9 +190,9 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable); /* Mode Related Controls */ void sun4i_tcon_switch_interlace(struct sun4i_tcon *tcon, bool enable); -void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon, +void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon, struct drm_encoder *encoder, struct drm_display_mode *mode); -void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon, +void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon, struct drm_encoder *encoder, struct drm_display_mode *mode);
#endif /* __SUN4I_TCON_H__ */ diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c b/drivers/gpu/drm/sun4i/sun4i_tv.c index 32ed5fdf0c4d..2d36df092a6a 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tv.c +++ b/drivers/gpu/drm/sun4i/sun4i_tv.c @@ -389,7 +389,7 @@ static void sun4i_tv_mode_set(struct drm_encoder *encoder, struct sun4i_tcon *tcon = drv->tcon; const struct tv_mode *tv_mode = sun4i_tv_find_tv_by_mode(mode);
- sun4i_tcon1_mode_set(tcon, mode); + sun4i_tcon1_mode_set(tcon, encoder, mode);
/* Enable and map the DAC to the output */ regmap_update_bits(tv->regs, SUN4I_TVE_EN_REG,
Even though that mux is undocumented, it seems like it needs to be set to 1 when using composite, and 0 when using HDMI.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index d2335f109601..93249c5ab1e4 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -268,11 +268,16 @@ void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon, struct drm_encoder *encoder, SUN4I_TCON_GCTL_IOMAP_MASK, SUN4I_TCON_GCTL_IOMAP_TCON1);
+ if (encoder->encoder_type == DRM_MODE_ENCODER_TVDAC) + val = 1; + else + val = 0; + /* * FIXME: Undocumented bits */ if (tcon->quirks->has_unknown_mux) - regmap_write(tcon->regs, SUN4I_TCON_MUX_CTRL_REG, 1); + regmap_write(tcon->regs, SUN4I_TCON_MUX_CTRL_REG, val); } EXPORT_SYMBOL(sun4i_tcon1_mode_set);
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
Even though that mux is undocumented, it seems like it needs to be set to 1 when using composite, and 0 when using HDMI.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
drivers/gpu/drm/sun4i/sun4i_tcon.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index d2335f109601..93249c5ab1e4 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -268,11 +268,16 @@ void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon, struct drm_encoder *encoder, SUN4I_TCON_GCTL_IOMAP_MASK, SUN4I_TCON_GCTL_IOMAP_TCON1);
if (encoder->encoder_type == DRM_MODE_ENCODER_TVDAC)val = 1;elseval = 0;/* * FIXME: Undocumented bits */ if (tcon->quirks->has_unknown_mux)
regmap_write(tcon->regs, SUN4I_TCON_MUX_CTRL_REG, 1);
regmap_write(tcon->regs, SUN4I_TCON_MUX_CTRL_REG, val);
We might want to do this the other way around, i.e. exporting
int sun4i_tcon_mux_set(struct drm_device *drm, int encoder_type, int pipeline)
and have downstream encoders call it. For the A31, the mux is not exclusively used for channel 1; there is a mux setting for MIPI DSI as well, but AFAIK DSI is connected to channel 0.
Additionally, the mux registers are only valid in the first TCON, meaning it must available be active in 2 pipeline chips. It's also why we'd pass "struct drm_device *" instead of "struct sun4i_tcon *".
Regards ChenYu
} EXPORT_SYMBOL(sun4i_tcon1_mode_set);
-- git-series 0.8.11
On Wed, Mar 08, 2017 at 11:51:39AM +0800, Chen-Yu Tsai wrote:
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
Even though that mux is undocumented, it seems like it needs to be set to 1 when using composite, and 0 when using HDMI.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
drivers/gpu/drm/sun4i/sun4i_tcon.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index d2335f109601..93249c5ab1e4 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -268,11 +268,16 @@ void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon, struct drm_encoder *encoder, SUN4I_TCON_GCTL_IOMAP_MASK, SUN4I_TCON_GCTL_IOMAP_TCON1);
if (encoder->encoder_type == DRM_MODE_ENCODER_TVDAC)val = 1;elseval = 0;/* * FIXME: Undocumented bits */ if (tcon->quirks->has_unknown_mux)
regmap_write(tcon->regs, SUN4I_TCON_MUX_CTRL_REG, 1);
regmap_write(tcon->regs, SUN4I_TCON_MUX_CTRL_REG, val);We might want to do this the other way around, i.e. exporting
int sun4i_tcon_mux_set(struct drm_device *drm, int encoder_type, int pipeline)and have downstream encoders call it. For the A31, the mux is not exclusively used for channel 1; there is a mux setting for MIPI DSI as well, but AFAIK DSI is connected to channel 0.
We could make it part of sun4i_tcon_channel_enable too, though. What do you think?
Additionally, the mux registers are only valid in the first TCON, meaning it must available be active in 2 pipeline chips. It's also why we'd pass "struct drm_device *" instead of "struct sun4i_tcon *".
Hmmmm. That's going to be tricky to support. Has this been confirmed somehow? Is the register used for something else on TCON1?
Thanks, Maxime
On Thu, Mar 9, 2017 at 6:58 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
On Wed, Mar 08, 2017 at 11:51:39AM +0800, Chen-Yu Tsai wrote:
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
Even though that mux is undocumented, it seems like it needs to be set to 1 when using composite, and 0 when using HDMI.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
drivers/gpu/drm/sun4i/sun4i_tcon.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index d2335f109601..93249c5ab1e4 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -268,11 +268,16 @@ void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon, struct drm_encoder *encoder, SUN4I_TCON_GCTL_IOMAP_MASK, SUN4I_TCON_GCTL_IOMAP_TCON1);
if (encoder->encoder_type == DRM_MODE_ENCODER_TVDAC)val = 1;elseval = 0;/* * FIXME: Undocumented bits */ if (tcon->quirks->has_unknown_mux)
regmap_write(tcon->regs, SUN4I_TCON_MUX_CTRL_REG, 1);
regmap_write(tcon->regs, SUN4I_TCON_MUX_CTRL_REG, val);We might want to do this the other way around, i.e. exporting
int sun4i_tcon_mux_set(struct drm_device *drm, int encoder_type, int pipeline)and have downstream encoders call it. For the A31, the mux is not exclusively used for channel 1; there is a mux setting for MIPI DSI as well, but AFAIK DSI is connected to channel 0.
We could make it part of sun4i_tcon_channel_enable too, though. What do you think?
We still need some way of figuring out what mux value to set for those cases. Let's keep your solution for now. We can work on it later when we have an actual use case to deal with.
Additionally, the mux registers are only valid in the first TCON, meaning it must available be active in 2 pipeline chips. It's also why we'd pass "struct drm_device *" instead of "struct sun4i_tcon *".
Hmmmm. That's going to be tricky to support. Has this been confirmed somehow? Is the register used for something else on TCON1?
At this point, the only reference is Allwinner's kernel, and the old 3.4 kernel for A10/A20. I could try getting HDMI working on the A31 to get some real results.
FWIW, the registers do not seem to be aliased across the two TCONs.
Regards ChenYu
It seems like what's called a backporch in the datasheet is actually the backporch plus the sync period. Fix that in our driver.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 93249c5ab1e4..e44217fb4f6f 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -240,15 +240,14 @@ void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon, struct drm_encoder *encoder, SUN4I_TCON1_BASIC2_Y(mode->crtc_vdisplay));
/* Set horizontal display timings */ - bp = mode->crtc_htotal - mode->crtc_hsync_end; + bp = mode->crtc_htotal - mode->crtc_hsync_start; DRM_DEBUG_DRIVER("Setting horizontal total %d, backporch %d\n", mode->htotal, bp); regmap_write(tcon->regs, SUN4I_TCON1_BASIC3_REG, SUN4I_TCON1_BASIC3_H_TOTAL(mode->crtc_htotal) | SUN4I_TCON1_BASIC3_H_BACKPORCH(bp));
- /* Set vertical display timings */ - bp = mode->crtc_vtotal - mode->crtc_vsync_end; + bp = mode->crtc_vtotal - mode->crtc_vsync_start; DRM_DEBUG_DRIVER("Setting vertical total %d, backporch %d\n", mode->vtotal, bp); regmap_write(tcon->regs, SUN4I_TCON1_BASIC4_REG,
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
It seems like what's called a backporch in the datasheet is actually the backporch plus the sync period. Fix that in our driver.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 93249c5ab1e4..e44217fb4f6f 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -240,15 +240,14 @@ void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon, struct drm_encoder *encoder, SUN4I_TCON1_BASIC2_Y(mode->crtc_vdisplay));
/* Set horizontal display timings */
bp = mode->crtc_htotal - mode->crtc_hsync_end;
bp = mode->crtc_htotal - mode->crtc_hsync_start; DRM_DEBUG_DRIVER("Setting horizontal total %d, backporch %d\n", mode->htotal, bp); regmap_write(tcon->regs, SUN4I_TCON1_BASIC3_REG, SUN4I_TCON1_BASIC3_H_TOTAL(mode->crtc_htotal) | SUN4I_TCON1_BASIC3_H_BACKPORCH(bp));
/* Set vertical display timings */
Why remove the comment?
Otherwise,
Acked-by: Chen-Yu Tsai wens@csie.org
bp = mode->crtc_vtotal - mode->crtc_vsync_end;
bp = mode->crtc_vtotal - mode->crtc_vsync_start; DRM_DEBUG_DRIVER("Setting vertical total %d, backporch %d\n", mode->vtotal, bp); regmap_write(tcon->regs, SUN4I_TCON1_BASIC4_REG,-- git-series 0.8.11
On Wed, Mar 08, 2017 at 12:25:59PM +0800, Chen-Yu Tsai wrote:
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
It seems like what's called a backporch in the datasheet is actually the backporch plus the sync period. Fix that in our driver.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 93249c5ab1e4..e44217fb4f6f 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -240,15 +240,14 @@ void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon, struct drm_encoder *encoder, SUN4I_TCON1_BASIC2_Y(mode->crtc_vdisplay));
/* Set horizontal display timings */
bp = mode->crtc_htotal - mode->crtc_hsync_end;
bp = mode->crtc_htotal - mode->crtc_hsync_start; DRM_DEBUG_DRIVER("Setting horizontal total %d, backporch %d\n", mode->htotal, bp); regmap_write(tcon->regs, SUN4I_TCON1_BASIC3_REG, SUN4I_TCON1_BASIC3_H_TOTAL(mode->crtc_htotal) | SUN4I_TCON1_BASIC3_H_BACKPORCH(bp));
/* Set vertical display timings */Why remove the comment?
I have no idea :)
This will be fixed. Thanks! Maxime
It appears that the total vertical resolution needs to be doubled when we're not in interlaced. Make sure that is the case.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index e44217fb4f6f..515fa56c1e6a 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -248,10 +248,13 @@ void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon, struct drm_encoder *encoder, SUN4I_TCON1_BASIC3_H_BACKPORCH(bp));
bp = mode->crtc_vtotal - mode->crtc_vsync_start; + val = mode->crtc_vtotal; + if (!(mode->flags & DRM_MODE_FLAG_INTERLACE)) + val = val * 2; DRM_DEBUG_DRIVER("Setting vertical total %d, backporch %d\n", mode->vtotal, bp); regmap_write(tcon->regs, SUN4I_TCON1_BASIC4_REG, - SUN4I_TCON1_BASIC4_V_TOTAL(mode->vtotal) | + SUN4I_TCON1_BASIC4_V_TOTAL(val) | SUN4I_TCON1_BASIC4_V_BACKPORCH(bp));
/* Set Hsync and Vsync length */
Hi,
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
It appears that the total vertical resolution needs to be doubled when we're not in interlaced. Make sure that is the case.
This is true for both channels, though we handle them differently.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index e44217fb4f6f..515fa56c1e6a 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -248,10 +248,13 @@ void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon, struct drm_encoder *encoder, SUN4I_TCON1_BASIC3_H_BACKPORCH(bp));
bp = mode->crtc_vtotal - mode->crtc_vsync_start;
val = mode->crtc_vtotal;if (!(mode->flags & DRM_MODE_FLAG_INTERLACE))val = val * 2; DRM_DEBUG_DRIVER("Setting vertical total %d, backporch %d\n", mode->vtotal, bp); regmap_write(tcon->regs, SUN4I_TCON1_BASIC4_REG,
SUN4I_TCON1_BASIC4_V_TOTAL(mode->vtotal) |
SUN4I_TCON1_BASIC4_V_TOTAL(val) |
For channel 0, the SUN4I_TCON0_BASIC2_V_TOTAL macro multiplies the passed in value by 2. I think we should do the same for channel 1, and instead halve the value passed in if we are outputting interlaced data. I think this makes more sense because:
1) The register description for both channels are the same. Handling them consistently will result in less confusion, such as this one.
2) The definition of interlaced modes is a frame is separated into odd and even fields, with each field contains half the number of lines of the full frame. One field is displayed during each VSYNC cycle. The TCON does not know whether it's interlaced video or not. It only knows the display timings. In this case, the number of horizontal lines per cycle is key.
Regards ChenYu
SUN4I_TCON1_BASIC4_V_BACKPORCH(bp)); /* Set Hsync and Vsync length */-- git-series 0.8.11
On Tue, Mar 07, 2017 at 06:05:17PM +0800, Chen-Yu Tsai wrote:
Hi,
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
It appears that the total vertical resolution needs to be doubled when we're not in interlaced. Make sure that is the case.
This is true for both channels, though we handle them differently.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index e44217fb4f6f..515fa56c1e6a 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -248,10 +248,13 @@ void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon, struct drm_encoder *encoder, SUN4I_TCON1_BASIC3_H_BACKPORCH(bp));
bp = mode->crtc_vtotal - mode->crtc_vsync_start;
val = mode->crtc_vtotal;if (!(mode->flags & DRM_MODE_FLAG_INTERLACE))val = val * 2; DRM_DEBUG_DRIVER("Setting vertical total %d, backporch %d\n", mode->vtotal, bp); regmap_write(tcon->regs, SUN4I_TCON1_BASIC4_REG,
SUN4I_TCON1_BASIC4_V_TOTAL(mode->vtotal) |
SUN4I_TCON1_BASIC4_V_TOTAL(val) |For channel 0, the SUN4I_TCON0_BASIC2_V_TOTAL macro multiplies the passed in value by 2. I think we should do the same for channel 1, and instead halve the value passed in if we are outputting interlaced data. I think this makes more sense because:
The register description for both channels are the same. Handling them consistently will result in less confusion, such as this one.
The definition of interlaced modes is a frame is separated into odd and even fields, with each field contains half the number of lines of the full frame. One field is displayed during each VSYNC cycle. The TCON does not know whether it's interlaced video or not. It only knows the display timings. In this case, the number of horizontal lines per cycle is key.
Hmm, yes, you're right. I'll fix it in the next release.
Thanks! Maxime
The earlier Allwinner SoCs (A10, A10s, A20, A31) have an embedded HDMI controller.
That HDMI controller is able to do audio and CEC, but those have been left out for now.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- drivers/gpu/drm/sun4i/Makefile | 5 +- drivers/gpu/drm/sun4i/sun4i_hdmi.h | 124 ++++++- drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c | 128 ++++++- drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 449 +++++++++++++++++++++- drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 236 +++++++++++- 5 files changed, 942 insertions(+), 0 deletions(-) create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi.h create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c
diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile index 59b757350a1f..68a0f6244a59 100644 --- a/drivers/gpu/drm/sun4i/Makefile +++ b/drivers/gpu/drm/sun4i/Makefile @@ -7,7 +7,12 @@ sun4i-tcon-y += sun4i_dotclock.o sun4i-tcon-y += sun4i_crtc.o sun4i-tcon-y += sun4i_layer.o
+sun4i-drm-hdmi-y += sun4i_hdmi_enc.o +sun4i-drm-hdmi-y += sun4i_hdmi_ddc_clk.o +sun4i-drm-hdmi-y += sun4i_hdmi_tmds_clk.o + obj-$(CONFIG_DRM_SUN4I) += sun4i-drm.o sun4i-tcon.o obj-$(CONFIG_DRM_SUN4I) += sun4i_backend.o obj-$(CONFIG_DRM_SUN4I) += sun6i_drc.o +obj-$(CONFIG_DRM_SUN4I) += sun4i-drm-hdmi.o obj-$(CONFIG_DRM_SUN4I) += sun4i_tv.o diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi.h b/drivers/gpu/drm/sun4i/sun4i_hdmi.h new file mode 100644 index 000000000000..2ad25b8fd3cd --- /dev/null +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi.h @@ -0,0 +1,124 @@ +/* + * Copyright (C) 2016 Maxime Ripard + * + * Maxime Ripard maxime.ripard@free-electrons.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + */ + +#ifndef _SUN4I_HDMI_H_ +#define _SUN4I_HDMI_H_ + +#include <drm/drm_connector.h> +#include <drm/drm_encoder.h> + +#define SUN4I_HDMI_CTRL_REG 0x004 +#define SUN4I_HDMI_CTRL_ENABLE BIT(31) + +#define SUN4I_HDMI_IRQ_REG 0x008 +#define SUN4I_HDMI_IRQ_STA_MASK 0x73 +#define SUN4I_HDMI_IRQ_STA_FIFO_OF BIT(1) +#define SUN4I_HDMI_IRQ_STA_FIFO_UF BIT(0) + +#define SUN4I_HDMI_HPD_REG 0x00c +#define SUN4I_HDMI_HPD_HIGH BIT(0) + +#define SUN4I_HDMI_VID_CTRL_REG 0x010 +#define SUN4I_HDMI_VID_CTRL_ENABLE BIT(31) +#define SUN4I_HDMI_VID_CTRL_HDMI_MODE BIT(30) + +#define SUN4I_HDMI_VID_TIMING_ACT_REG 0x014 +#define SUN4I_HDMI_VID_TIMING_BP_REG 0x018 +#define SUN4I_HDMI_VID_TIMING_FP_REG 0x01c +#define SUN4I_HDMI_VID_TIMING_SPW_REG 0x020 + +#define SUN4I_HDMI_VID_TIMING_X(x) ((((x) - 1) & GENMASK(11, 0))) +#define SUN4I_HDMI_VID_TIMING_Y(y) ((((y) - 1) & GENMASK(11, 0)) << 16) + +#define SUN4I_HDMI_VID_TIMING_POL_REG 0x024 +#define SUN4I_HDMI_VID_TIMING_POL_TX_CLK (0x3e0 << 16) +#define SUN4I_HDMI_VID_TIMING_POL_VSYNC BIT(1) +#define SUN4I_HDMI_VID_TIMING_POL_HSYNC BIT(0) + +#define SUN4I_HDMI_AVI_INFOFRAME_REG(n) (0x080 + (n)) + +#define SUN4I_HDMI_PAD_CTRL0_REG 0x200 + +#define SUN4I_HDMI_PAD_CTRL1_REG 0x204 +#define SUN4I_HDMI_PAD_CTRL1_HALVE_CLK BIT(6) + +#define SUN4I_HDMI_PLL_CTRL_REG 0x208 +#define SUN4I_HDMI_PLL_CTRL_DIV(n) ((n) << 4) +#define SUN4I_HDMI_PLL_CTRL_DIV_MASK GENMASK(7, 4) + +#define SUN4I_HDMI_PLL_DBG0_REG 0x20c +#define SUN4I_HDMI_PLL_DBG0_TMDS_PARENT(n) (((n) & 1) << 21) +#define SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_MASK BIT(21) +#define SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_SHIFT 21 + +#define SUN4I_HDMI_PKT_CTRL_REG(n) (0x2f0 + (4 * (n))) +#define SUN4I_HDMI_PKT_CTRL_TYPE(n, t) ((t) << (((n) % 4) * 4)) + +#define SUN4I_HDMI_UNKNOWN_REG 0x300 +#define SUN4I_HDMI_UNKNOWN_INPUT_SYNC BIT(27) + +#define SUN4I_HDMI_DDC_CTRL_REG 0x500 +#define SUN4I_HDMI_DDC_CTRL_ENABLE BIT(31) +#define SUN4I_HDMI_DDC_CTRL_START_CMD BIT(30) +#define SUN4I_HDMI_DDC_CTRL_RESET BIT(0) + +#define SUN4I_HDMI_DDC_ADDR_REG 0x504 +#define SUN4I_HDMI_DDC_ADDR_SEGMENT(seg) (((seg) & 0xff) << 24) +#define SUN4I_HDMI_DDC_ADDR_EDDC(addr) (((addr) & 0xff) << 16) +#define SUN4I_HDMI_DDC_ADDR_OFFSET(off) (((off) & 0xff) << 8) +#define SUN4I_HDMI_DDC_ADDR_SLAVE(addr) ((addr) & 0xff) + +#define SUN4I_HDMI_DDC_FIFO_CTRL_REG 0x510 +#define SUN4I_HDMI_DDC_FIFO_CTRL_CLEAR BIT(31) + +#define SUN4I_HDMI_DDC_FIFO_DATA_REG 0x518 +#define SUN4I_HDMI_DDC_BYTE_COUNT_REG 0x51c + +#define SUN4I_HDMI_DDC_CMD_REG 0x520 +#define SUN4I_HDMI_DDC_CMD_EXPLICIT_EDDC_READ 6 + +#define SUN4I_HDMI_DDC_CLK_REG 0x528 +#define SUN4I_HDMI_DDC_CLK_M(m) (((m) & 0x7) << 3) +#define SUN4I_HDMI_DDC_CLK_N(n) ((n) & 0x7) + +#define SUN4I_HDMI_DDC_LINE_CTRL_REG 0x540 +#define SUN4I_HDMI_DDC_LINE_CTRL_SDA_ENABLE BIT(9) +#define SUN4I_HDMI_DDC_LINE_CTRL_SCL_ENABLE BIT(8) + +#define SUN4I_HDMI_DDC_FIFO_SIZE 16 + +enum sun4i_hdmi_pkt_type { + SUN4I_HDMI_PKT_AVI = 2, + SUN4I_HDMI_PKT_END = 15, +}; + +struct sun4i_hdmi { + struct drm_connector connector; + struct drm_encoder encoder; + struct device *dev; + + void __iomem *base; + struct clk *bus_clk; + struct clk *ddc_clk; + struct clk *mod_clk; + struct clk *pll0_clk; + struct clk *pll1_clk; + struct clk *tmds_clk; + + struct sun4i_drv *drv; + + bool hdmi_monitor; +}; + +int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *clk); +int sun4i_tmds_create(struct sun4i_hdmi *hdmi); + +#endif /* _SUN4I_HDMI_H_ */ diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c new file mode 100644 index 000000000000..5125b14ea7a5 --- /dev/null +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c @@ -0,0 +1,128 @@ +/* + * Copyright (C) 2016 Free Electrons + * Copyright (C) 2016 NextThing Co + * + * Maxime Ripard maxime.ripard@free-electrons.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + */ + +#include <linux/clk-provider.h> + +#include "sun4i_tcon.h" +#include "sun4i_hdmi.h" + +struct sun4i_ddc { + struct clk_hw hw; + struct sun4i_hdmi *hdmi; +}; + +static inline struct sun4i_ddc *hw_to_ddc(struct clk_hw *hw) +{ + return container_of(hw, struct sun4i_ddc, hw); +} + +static unsigned long sun4i_ddc_calc_divider(unsigned long rate, + unsigned long parent_rate, + u8 *m, u8 *n) +{ + unsigned long best_rate = 0; + u8 best_m = 0, best_n = 0, _m, _n; + + for (_m = 0; _m < 8; _m++) { + for (_n = 0; _n < 8; _n++) { + unsigned long tmp_rate; + + tmp_rate = (((parent_rate / 2) / 10) >> _n) / (_m + 1); + + if (tmp_rate > rate) + continue; + + if (abs(rate - tmp_rate) < abs(rate - best_rate)) { + best_rate = tmp_rate; + best_m = _m; + best_n = _n; + } + } + } + + if (m && n) { + *m = best_m; + *n = best_n; + } + + return best_rate; +} + +static long sun4i_ddc_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *prate) +{ + return sun4i_ddc_calc_divider(rate, *prate, NULL, NULL); +} + +static unsigned long sun4i_ddc_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct sun4i_ddc *ddc = hw_to_ddc(hw); + u32 reg; + u8 m, n; + + reg = readl(ddc->hdmi->base + SUN4I_HDMI_DDC_CLK_REG); + m = (reg >> 3) & 0x7; + n = reg & 0x7; + + return (((parent_rate / 2) / 10) >> n) / (m + 1); +} + +static int sun4i_ddc_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) +{ + struct sun4i_ddc *ddc = hw_to_ddc(hw); + u8 div_m, div_n; + + sun4i_ddc_calc_divider(rate, parent_rate, &div_m, &div_n); + + writel(SUN4I_HDMI_DDC_CLK_M(div_m) | SUN4I_HDMI_DDC_CLK_N(div_n), + ddc->hdmi->base + SUN4I_HDMI_DDC_CLK_REG); + + return 0; +} + +static const struct clk_ops sun4i_ddc_ops = { + .recalc_rate = sun4i_ddc_recalc_rate, + .round_rate = sun4i_ddc_round_rate, + .set_rate = sun4i_ddc_set_rate, +}; + +int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *parent) +{ + struct clk_init_data init; + struct sun4i_ddc *ddc; + const char *parent_name; + + parent_name = __clk_get_name(parent); + if (!parent_name) + return -ENODEV; + + ddc = devm_kzalloc(hdmi->dev, sizeof(*ddc), GFP_KERNEL); + if (!ddc) + return -ENOMEM; + + init.name = "hdmi-ddc"; + init.ops = &sun4i_ddc_ops; + init.parent_names = &parent_name; + init.num_parents = 1; + init.flags = CLK_SET_RATE_PARENT; + + ddc->hdmi = hdmi; + ddc->hw.init = &init; + + hdmi->ddc_clk = devm_clk_register(hdmi->dev, &ddc->hw); + if (IS_ERR(hdmi->ddc_clk)) + return PTR_ERR(hdmi->ddc_clk); + + return 0; +} diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c new file mode 100644 index 000000000000..33175308c2ed --- /dev/null +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c @@ -0,0 +1,449 @@ +/* + * Copyright (C) 2016 Maxime Ripard + * + * Maxime Ripard maxime.ripard@free-electrons.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + */ + +#include <drm/drmP.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_edid.h> +#include <drm/drm_encoder.h> +#include <drm/drm_panel.h> + +#include <linux/clk.h> +#include <linux/component.h> +#include <linux/iopoll.h> +#include <linux/of_address.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> + +#include "sun4i_backend.h" +#include "sun4i_drv.h" +#include "sun4i_hdmi.h" +#include "sun4i_tcon.h" + +static inline struct sun4i_hdmi * +drm_encoder_to_sun4i_hdmi(struct drm_encoder *encoder) +{ + return container_of(encoder, struct sun4i_hdmi, + encoder); +} + +static inline struct sun4i_hdmi * +drm_connector_to_sun4i_hdmi(struct drm_connector *connector) +{ + return container_of(connector, struct sun4i_hdmi, + connector); +} + +static int sun4i_hdmi_setup_avi_infoframes(struct sun4i_hdmi *hdmi, + struct drm_display_mode *mode) +{ + struct hdmi_avi_infoframe frame; + u8 buffer[17]; + int i, ret; + + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); + if (ret < 0) { + DRM_ERROR("Failed to get infoframes from mode\n"); + return ret; + } + + ret = hdmi_avi_infoframe_pack(&frame, buffer, sizeof(buffer)); + if (ret < 0) { + DRM_ERROR("Failed to pack infoframes\n"); + return ret; + } + + for (i = 0; i < sizeof(buffer); i++) + writeb(buffer[i], hdmi->base + SUN4I_HDMI_AVI_INFOFRAME_REG(i)); + + return 0; +} + +static void sun4i_hdmi_disable(struct drm_encoder *encoder) +{ + struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder); + struct sun4i_drv *drv = hdmi->drv; + struct sun4i_tcon *tcon = drv->tcon; + u32 val; + + DRM_DEBUG_DRIVER("Disabling the HDMI Output\n"); + + val = readl(hdmi->base + SUN4I_HDMI_VID_CTRL_REG); + val &= ~SUN4I_HDMI_VID_CTRL_ENABLE; + writel(val, hdmi->base + SUN4I_HDMI_VID_CTRL_REG); + + sun4i_tcon_channel_disable(tcon, 1); +} + +static void sun4i_hdmi_enable(struct drm_encoder *encoder) +{ + struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; + struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder); + struct sun4i_drv *drv = hdmi->drv; + struct sun4i_tcon *tcon = drv->tcon; + u32 val = 0; + + DRM_DEBUG_DRIVER("Enabling the HDMI Output\n"); + + sun4i_tcon_channel_enable(tcon, 1); + + sun4i_hdmi_setup_avi_infoframes(hdmi, mode); + val |= SUN4I_HDMI_PKT_CTRL_TYPE(0, SUN4I_HDMI_PKT_AVI); + val |= SUN4I_HDMI_PKT_CTRL_TYPE(1, SUN4I_HDMI_PKT_END); + writel(val, hdmi->base + SUN4I_HDMI_PKT_CTRL_REG(0)); + + val = SUN4I_HDMI_VID_CTRL_ENABLE; + if (hdmi->hdmi_monitor) + val |= SUN4I_HDMI_VID_CTRL_HDMI_MODE; + + writel(val, hdmi->base + SUN4I_HDMI_VID_CTRL_REG); +} + +static void sun4i_hdmi_mode_set(struct drm_encoder *encoder, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ + struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder); + struct sun4i_drv *drv = hdmi->drv; + struct sun4i_tcon *tcon = drv->tcon; + unsigned int x, y; + u32 val; + + sun4i_tcon1_mode_set(tcon, encoder, mode); + clk_set_rate(tcon->sclk1, mode->crtc_clock * 1000); + clk_set_rate(hdmi->tmds_clk, mode->crtc_clock * 1000); + + /* Set input sync enable */ + writel(SUN4I_HDMI_UNKNOWN_INPUT_SYNC, + hdmi->base + SUN4I_HDMI_UNKNOWN_REG); + + /* Setup timing registers */ + writel(SUN4I_HDMI_VID_TIMING_X(mode->hdisplay) | + SUN4I_HDMI_VID_TIMING_Y(mode->vdisplay), + hdmi->base + SUN4I_HDMI_VID_TIMING_ACT_REG); + + x = mode->htotal - mode->hsync_start; + y = mode->vtotal - mode->vsync_start; + writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y), + hdmi->base + SUN4I_HDMI_VID_TIMING_BP_REG); + + x = mode->hsync_start - mode->hdisplay; + y = mode->vsync_start - mode->vdisplay; + writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y), + hdmi->base + SUN4I_HDMI_VID_TIMING_FP_REG); + + x = mode->hsync_end - mode->hsync_start; + y = mode->vsync_end - mode->vsync_start; + writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y), + hdmi->base + SUN4I_HDMI_VID_TIMING_SPW_REG); + + val = SUN4I_HDMI_VID_TIMING_POL_TX_CLK; + if (mode->flags & DRM_MODE_FLAG_PHSYNC) + val |= SUN4I_HDMI_VID_TIMING_POL_HSYNC; + + if (mode->flags & DRM_MODE_FLAG_PVSYNC) + val |= SUN4I_HDMI_VID_TIMING_POL_VSYNC; + + writel(val, hdmi->base + SUN4I_HDMI_VID_TIMING_POL_REG); +} + +static struct drm_encoder_helper_funcs sun4i_hdmi_helper_funcs = { + .disable = sun4i_hdmi_disable, + .enable = sun4i_hdmi_enable, + .mode_set = sun4i_hdmi_mode_set, +}; + +static struct drm_encoder_funcs sun4i_hdmi_funcs = { + .destroy = drm_encoder_cleanup, +}; + +static int sun4i_hdmi_read_sub_block(struct sun4i_hdmi *hdmi, + unsigned int blk, unsigned int offset, + u8 *buf, unsigned int count) +{ + unsigned long reg; + int i; + + reg = readl(hdmi->base + SUN4I_HDMI_DDC_FIFO_CTRL_REG); + writel(reg | SUN4I_HDMI_DDC_FIFO_CTRL_CLEAR, + hdmi->base + SUN4I_HDMI_DDC_FIFO_CTRL_REG); + writel(SUN4I_HDMI_DDC_ADDR_SEGMENT(offset >> 8) | + SUN4I_HDMI_DDC_ADDR_EDDC(0x60) | + SUN4I_HDMI_DDC_ADDR_OFFSET(offset) | + SUN4I_HDMI_DDC_ADDR_SLAVE(0x50), + hdmi->base + SUN4I_HDMI_DDC_ADDR_REG); + + writel(count, hdmi->base + SUN4I_HDMI_DDC_BYTE_COUNT_REG); + writel(SUN4I_HDMI_DDC_CMD_EXPLICIT_EDDC_READ, + hdmi->base + SUN4I_HDMI_DDC_CMD_REG); + + reg = readl(hdmi->base + SUN4I_HDMI_DDC_CTRL_REG); + writel(reg | SUN4I_HDMI_DDC_CTRL_START_CMD, + hdmi->base + SUN4I_HDMI_DDC_CTRL_REG); + + if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_DDC_CTRL_REG, reg, + !(reg & SUN4I_HDMI_DDC_CTRL_START_CMD), + 100, 2000)) + return -EIO; + + for (i = 0; i < count; i++) + buf[i] = readb(hdmi->base + SUN4I_HDMI_DDC_FIFO_DATA_REG); + + return 0; +} + +static int sun4i_hdmi_read_edid_block(void *data, u8 *buf, unsigned int blk, + size_t length) +{ + struct sun4i_hdmi *hdmi = data; + int retry = 2, i; + + do { + for (i = 0; i < length; i += SUN4I_HDMI_DDC_FIFO_SIZE) { + unsigned char offset = blk * EDID_LENGTH + i; + unsigned int count = min((unsigned int)SUN4I_HDMI_DDC_FIFO_SIZE, + length - i); + int ret; + + ret = sun4i_hdmi_read_sub_block(hdmi, blk, offset, + buf + i, count); + if (ret) + return ret; + } + } while (!drm_edid_block_valid(buf, blk, true, NULL) && (retry--)); + + return 0; +} + +static int sun4i_hdmi_get_modes(struct drm_connector *connector) +{ + struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector); + unsigned long reg; + struct edid *edid; + int ret; + + /* Reset i2c controller */ + writel(SUN4I_HDMI_DDC_CTRL_ENABLE | SUN4I_HDMI_DDC_CTRL_RESET, + hdmi->base + SUN4I_HDMI_DDC_CTRL_REG); + if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_DDC_CTRL_REG, reg, + !(reg & SUN4I_HDMI_DDC_CTRL_RESET), + 100, 2000)) + return -EIO; + + writel(SUN4I_HDMI_DDC_LINE_CTRL_SDA_ENABLE | + SUN4I_HDMI_DDC_LINE_CTRL_SCL_ENABLE, + hdmi->base + SUN4I_HDMI_DDC_LINE_CTRL_REG); + + clk_set_rate(hdmi->ddc_clk, 100000); + + edid = drm_do_get_edid(connector, sun4i_hdmi_read_edid_block, hdmi); + if (!edid) + return 0; + + hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid); + DRM_DEBUG_DRIVER("Monitor is %s monitor\n", + hdmi->hdmi_monitor ? "an HDMI" : "a DVI"); + + drm_mode_connector_update_edid_property(connector, edid); + ret = drm_add_edid_modes(connector, edid); + kfree(edid); + + return ret; +} + +static struct drm_connector_helper_funcs sun4i_hdmi_connector_helper_funcs = { + .get_modes = sun4i_hdmi_get_modes, +}; + +static enum drm_connector_status +sun4i_hdmi_connector_detect(struct drm_connector *connector, bool force) +{ + struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector); + unsigned long reg; + + if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_HPD_REG, reg, + reg & SUN4I_HDMI_HPD_HIGH, + 0, 500000)) + return connector_status_disconnected; + + return connector_status_connected; +} + +static struct drm_connector_funcs sun4i_hdmi_connector_funcs = { + .dpms = drm_atomic_helper_connector_dpms, + .detect = sun4i_hdmi_connector_detect, + .fill_modes = drm_helper_probe_single_connector_modes, + .destroy = drm_connector_cleanup, + .reset = drm_atomic_helper_connector_reset, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + +static int sun4i_hdmi_bind(struct device *dev, struct device *master, + void *data) +{ + struct drm_device *drm = data; + struct sun4i_drv *drv = drm->dev_private; + struct sun4i_hdmi *hdmi = dev_get_drvdata(dev); + int ret; + + hdmi->drv = drv; + drm_encoder_helper_add(&hdmi->encoder, + &sun4i_hdmi_helper_funcs); + ret = drm_encoder_init(drm, + &hdmi->encoder, + &sun4i_hdmi_funcs, + DRM_MODE_ENCODER_TMDS, + NULL); + if (ret) { + dev_err(dev, "Couldn't initialise the HDMI encoder\n"); + return ret; + } + + hdmi->encoder.possible_crtcs = BIT(0); + + drm_connector_helper_add(&hdmi->connector, + &sun4i_hdmi_connector_helper_funcs); + ret = drm_connector_init(drm, &hdmi->connector, + &sun4i_hdmi_connector_funcs, + DRM_MODE_CONNECTOR_HDMIA); + if (ret) { + dev_err(dev, + "Couldn't initialise the Composite connector\n"); + goto err_cleanup_connector; + } + hdmi->connector.interlace_allowed = true; + + drm_mode_connector_attach_encoder(&hdmi->connector, &hdmi->encoder); + + return 0; + +err_cleanup_connector: + drm_encoder_cleanup(&hdmi->encoder); + return ret; +} + +static void sun4i_hdmi_unbind(struct device *dev, struct device *master, + void *data) +{ + struct sun4i_hdmi *hdmi = dev_get_drvdata(dev); + + drm_connector_cleanup(&hdmi->connector); + drm_encoder_cleanup(&hdmi->encoder); +} + +static struct component_ops sun4i_hdmi_ops = { + .bind = sun4i_hdmi_bind, + .unbind = sun4i_hdmi_unbind, +}; + +static int sun4i_hdmi_probe(struct platform_device *pdev) +{ + struct sun4i_hdmi *hdmi; + struct resource *res; + int ret; + + hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL); + if (!hdmi) + return -ENOMEM; + dev_set_drvdata(&pdev->dev, hdmi); + hdmi->dev = &pdev->dev; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + hdmi->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(hdmi->base)) { + dev_err(&pdev->dev, "Couldn't map the HDMI encoder registers\n"); + return PTR_ERR(hdmi->base); + } + + hdmi->bus_clk = devm_clk_get(&pdev->dev, "ahb"); + if (IS_ERR(hdmi->bus_clk)) { + dev_err(&pdev->dev, "Couldn't get the HDMI bus clock\n"); + return PTR_ERR(hdmi->bus_clk); + } + clk_prepare_enable(hdmi->bus_clk); + + hdmi->mod_clk = devm_clk_get(&pdev->dev, "mod"); + if (IS_ERR(hdmi->mod_clk)) { + dev_err(&pdev->dev, "Couldn't get the HDMI mod clock\n"); + return PTR_ERR(hdmi->mod_clk); + } + clk_prepare_enable(hdmi->mod_clk); + + hdmi->pll0_clk = devm_clk_get(&pdev->dev, "pll-0"); + if (IS_ERR(hdmi->pll0_clk)) { + dev_err(&pdev->dev, "Couldn't get the HDMI PLL 0 clock\n"); + return PTR_ERR(hdmi->pll0_clk); + } + + hdmi->pll1_clk = devm_clk_get(&pdev->dev, "pll-1"); + if (IS_ERR(hdmi->pll1_clk)) { + dev_err(&pdev->dev, "Couldn't get the HDMI PLL 1 clock\n"); + return PTR_ERR(hdmi->pll1_clk); + } + + ret = sun4i_tmds_create(hdmi); + if (ret) { + dev_err(&pdev->dev, "Couldn't create the TMDS clock\n"); + return ret; + } + + writel(SUN4I_HDMI_CTRL_ENABLE, hdmi->base + SUN4I_HDMI_CTRL_REG); + +#define SUN4I_HDMI_PAD_CTRL0 0xfe800000 + + writel(SUN4I_HDMI_PAD_CTRL0, hdmi->base + SUN4I_HDMI_PAD_CTRL0_REG); + + /* TODO: defines */ + writel((6 << 3) | (2 << 10) | BIT(14) | BIT(15) | + BIT(19) | BIT(20) | BIT(22) | BIT(23), + hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG); + + /* TODO: defines */ + writel((8 << 0) | (7 << 8) | (239 << 12) | (7 << 17) | (4 << 20) | + BIT(25) | BIT(27) | BIT(28) | BIT(29) | BIT(30) | BIT(31), + hdmi->base + SUN4I_HDMI_PLL_CTRL_REG); + + ret = sun4i_ddc_create(hdmi, hdmi->tmds_clk); + if (ret) { + dev_err(&pdev->dev, "Couldn't create the DDC clock\n"); + return ret; + } + + return component_add(&pdev->dev, &sun4i_hdmi_ops); +} + +static int sun4i_hdmi_remove(struct platform_device *pdev) +{ + component_del(&pdev->dev, &sun4i_hdmi_ops); + + return 0; +} + +static const struct of_device_id sun4i_hdmi_of_table[] = { + { .compatible = "allwinner,sun5i-a10s-hdmi" }, + { } +}; +MODULE_DEVICE_TABLE(of, sun4i_hdmi_of_table); + +static struct platform_driver sun4i_hdmi_driver = { + .probe = sun4i_hdmi_probe, + .remove = sun4i_hdmi_remove, + .driver = { + .name = "sun4i-hdmi", + .of_match_table = sun4i_hdmi_of_table, + }, +}; +module_platform_driver(sun4i_hdmi_driver); + +MODULE_AUTHOR("Maxime Ripard maxime.ripard@free-electrons.com"); +MODULE_DESCRIPTION("Allwinner A10 HDMI Driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c new file mode 100644 index 000000000000..40f48f1d4685 --- /dev/null +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c @@ -0,0 +1,236 @@ +/* + * Copyright (C) 2016 Free Electrons + * Copyright (C) 2016 NextThing Co + * + * Maxime Ripard maxime.ripard@free-electrons.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + */ + +#include <linux/clk-provider.h> + +#include "sun4i_tcon.h" +#include "sun4i_hdmi.h" + +struct sun4i_tmds { + struct clk_hw hw; + struct sun4i_hdmi *hdmi; +}; + +static inline struct sun4i_tmds *hw_to_tmds(struct clk_hw *hw) +{ + return container_of(hw, struct sun4i_tmds, hw); +} + + +static unsigned long sun4i_tmds_calc_divider(unsigned long rate, + unsigned long parent_rate, + u8 *div, + bool *half) +{ + unsigned long best_rate = 0; + u8 best_m = 0, m; + bool is_double; + + for (m = 1; m < 16; m++) { + u8 d; + + for (d = 1; d < 3; d++) { + unsigned long tmp_rate; + + tmp_rate = parent_rate / m / d; + + if (tmp_rate > rate) + continue; + + if (!best_rate || + (rate - tmp_rate) < (rate - best_rate)) { + best_rate = tmp_rate; + best_m = m; + is_double = d; + } + } + } + + if (div && half) { + *div = best_m; + *half = is_double; + } + + return best_rate; +} + + +static int sun4i_tmds_determine_rate(struct clk_hw *hw, + struct clk_rate_request *req) +{ + struct clk_hw *parent; + unsigned long best_parent = 0; + unsigned long rate = req->rate; + int best_div = 1, best_half = 1; + int i, j; + + printk("%s %d rate %lu\n", __func__, __LINE__, rate); + + /* + * We only consider PLL3, since the TCON is very likely to be + * clocked from it, and to have the same rate than our HDMI + * clock, so we should not need to do anything. + */ + + parent = clk_hw_get_parent_by_index(hw, 0); + if (!parent) + return -EINVAL; + + for (i = 1; i < 3; i++) { + for (j = 1; j < 16; j++) { + unsigned long ideal = rate * i * j; + unsigned long rounded; + + rounded = clk_hw_round_rate(parent, ideal); + + if (rounded == ideal) { + best_parent = rounded; + best_half = i; + best_div = j; + goto out; + } + + if (abs(rate - rounded / i) < + abs(rate - best_parent / best_div)) { + best_parent = rounded; + best_div = i; + } + } + } + +out: + req->rate = best_parent / best_half / best_div; + req->best_parent_rate = best_parent; + req->best_parent_hw = parent; + + printk("%s %d rate %lu parent rate %lu (%s) div %d half %d\n", + __func__, __LINE__, req->rate, req->best_parent_rate, + clk_hw_get_name(req->best_parent_hw), + best_div, best_half); + + return 0; +} + +static unsigned long sun4i_tmds_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct sun4i_tmds *tmds = hw_to_tmds(hw); + u32 reg; + + reg = readl(tmds->hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG); + if (reg & SUN4I_HDMI_PAD_CTRL1_HALVE_CLK) + parent_rate /= 2; + + reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_CTRL_REG); + reg = (reg >> 4) & 0xf; + if (!reg) + reg = 1; + + return parent_rate / reg; +} + +static int sun4i_tmds_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) +{ + struct sun4i_tmds *tmds = hw_to_tmds(hw); + bool half; + u32 reg; + u8 div; + + sun4i_tmds_calc_divider(rate, parent_rate, &div, &half); + + printk("%s %d rate %lu parent rate %lu div %d half %s\n", + __func__, __LINE__, rate, parent_rate, div, + half ? "yes" : "no"); + + reg = readl(tmds->hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG); + reg &= ~SUN4I_HDMI_PAD_CTRL1_HALVE_CLK; + if (half) + reg |= SUN4I_HDMI_PAD_CTRL1_HALVE_CLK; + writel(reg, tmds->hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG); + + reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_CTRL_REG); + reg &= ~SUN4I_HDMI_PLL_CTRL_DIV_MASK; + writel(reg | SUN4I_HDMI_PLL_CTRL_DIV(div), + tmds->hdmi->base + SUN4I_HDMI_PLL_CTRL_REG); + + return 0; +} + +static u8 sun4i_tmds_get_parent(struct clk_hw *hw) +{ + struct sun4i_tmds *tmds = hw_to_tmds(hw); + u32 reg; + + reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_DBG0_REG); + return ((reg & SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_MASK) >> + SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_SHIFT); +} + +static int sun4i_tmds_set_parent(struct clk_hw *hw, u8 index) +{ + struct sun4i_tmds *tmds = hw_to_tmds(hw); + u32 reg; + + if (index > 1) + return -EINVAL; + + reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_DBG0_REG); + reg &= ~SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_MASK; + writel(reg | SUN4I_HDMI_PLL_DBG0_TMDS_PARENT(index), + tmds->hdmi->base + SUN4I_HDMI_PLL_DBG0_REG); + + return 0; +} + +static const struct clk_ops sun4i_tmds_ops = { + .determine_rate = sun4i_tmds_determine_rate, + .recalc_rate = sun4i_tmds_recalc_rate, + .set_rate = sun4i_tmds_set_rate, + + .get_parent = sun4i_tmds_get_parent, + .set_parent = sun4i_tmds_set_parent, +}; + +int sun4i_tmds_create(struct sun4i_hdmi *hdmi) +{ + struct clk_init_data init; + struct sun4i_tmds *tmds; + const char *parents[2]; + + parents[0] = __clk_get_name(hdmi->pll0_clk); + if (!parents[0]) + return -ENODEV; + + parents[1] = __clk_get_name(hdmi->pll1_clk); + if (!parents[1]) + return -ENODEV; + + tmds = devm_kzalloc(hdmi->dev, sizeof(*tmds), GFP_KERNEL); + if (!tmds) + return -ENOMEM; + + init.name = "hdmi-tmds"; + init.ops = &sun4i_tmds_ops; + init.parent_names = parents; + init.num_parents = 2; + init.flags = CLK_SET_RATE_PARENT; + + tmds->hdmi = hdmi; + tmds->hw.init = &init; + + hdmi->tmds_clk = devm_clk_register(hdmi->dev, &tmds->hw); + if (IS_ERR(hdmi->tmds_clk)) + return PTR_ERR(hdmi->tmds_clk); + + return 0; +}
Hi,
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The earlier Allwinner SoCs (A10, A10s, A20, A31) have an embedded HDMI controller.
That HDMI controller is able to do audio and CEC, but those have been left out for now.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
drivers/gpu/drm/sun4i/Makefile | 5 +- drivers/gpu/drm/sun4i/sun4i_hdmi.h | 124 ++++++- drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c | 128 ++++++- drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 449 +++++++++++++++++++++- drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 236 +++++++++++- 5 files changed, 942 insertions(+), 0 deletions(-) create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi.h create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c
Applying patch #9608371 using 'git am' Description: [13/15] drm/sun4i: Add HDMI support Applying: drm/sun4i: Add HDMI support .git/rebase-apply/patch:116: trailing whitespace.
.git/rebase-apply/patch:531: trailing whitespace.
.git/rebase-apply/patch:701: trailing whitespace.
warning: 3 lines add whitespace errors.
diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile index 59b757350a1f..68a0f6244a59 100644 --- a/drivers/gpu/drm/sun4i/Makefile +++ b/drivers/gpu/drm/sun4i/Makefile @@ -7,7 +7,12 @@ sun4i-tcon-y += sun4i_dotclock.o sun4i-tcon-y += sun4i_crtc.o sun4i-tcon-y += sun4i_layer.o
+sun4i-drm-hdmi-y += sun4i_hdmi_enc.o +sun4i-drm-hdmi-y += sun4i_hdmi_ddc_clk.o +sun4i-drm-hdmi-y += sun4i_hdmi_tmds_clk.o
obj-$(CONFIG_DRM_SUN4I) += sun4i-drm.o sun4i-tcon.o obj-$(CONFIG_DRM_SUN4I) += sun4i_backend.o obj-$(CONFIG_DRM_SUN4I) += sun6i_drc.o +obj-$(CONFIG_DRM_SUN4I) += sun4i-drm-hdmi.o obj-$(CONFIG_DRM_SUN4I) += sun4i_tv.o diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi.h b/drivers/gpu/drm/sun4i/sun4i_hdmi.h new file mode 100644 index 000000000000..2ad25b8fd3cd --- /dev/null +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi.h @@ -0,0 +1,124 @@ +/*
- Copyright (C) 2016 Maxime Ripard
- Maxime Ripard maxime.ripard@free-electrons.com
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- */
+#ifndef _SUN4I_HDMI_H_ +#define _SUN4I_HDMI_H_
+#include <drm/drm_connector.h> +#include <drm/drm_encoder.h>
+#define SUN4I_HDMI_CTRL_REG 0x004 +#define SUN4I_HDMI_CTRL_ENABLE BIT(31)
+#define SUN4I_HDMI_IRQ_REG 0x008 +#define SUN4I_HDMI_IRQ_STA_MASK 0x73 +#define SUN4I_HDMI_IRQ_STA_FIFO_OF BIT(1) +#define SUN4I_HDMI_IRQ_STA_FIFO_UF BIT(0)
+#define SUN4I_HDMI_HPD_REG 0x00c +#define SUN4I_HDMI_HPD_HIGH BIT(0)
+#define SUN4I_HDMI_VID_CTRL_REG 0x010 +#define SUN4I_HDMI_VID_CTRL_ENABLE BIT(31) +#define SUN4I_HDMI_VID_CTRL_HDMI_MODE BIT(30)
+#define SUN4I_HDMI_VID_TIMING_ACT_REG 0x014 +#define SUN4I_HDMI_VID_TIMING_BP_REG 0x018 +#define SUN4I_HDMI_VID_TIMING_FP_REG 0x01c +#define SUN4I_HDMI_VID_TIMING_SPW_REG 0x020
+#define SUN4I_HDMI_VID_TIMING_X(x) ((((x) - 1) & GENMASK(11, 0))) +#define SUN4I_HDMI_VID_TIMING_Y(y) ((((y) - 1) & GENMASK(11, 0)) << 16)
+#define SUN4I_HDMI_VID_TIMING_POL_REG 0x024 +#define SUN4I_HDMI_VID_TIMING_POL_TX_CLK (0x3e0 << 16) +#define SUN4I_HDMI_VID_TIMING_POL_VSYNC BIT(1) +#define SUN4I_HDMI_VID_TIMING_POL_HSYNC BIT(0)
+#define SUN4I_HDMI_AVI_INFOFRAME_REG(n) (0x080 + (n))
+#define SUN4I_HDMI_PAD_CTRL0_REG 0x200
+#define SUN4I_HDMI_PAD_CTRL1_REG 0x204 +#define SUN4I_HDMI_PAD_CTRL1_HALVE_CLK BIT(6)
+#define SUN4I_HDMI_PLL_CTRL_REG 0x208 +#define SUN4I_HDMI_PLL_CTRL_DIV(n) ((n) << 4) +#define SUN4I_HDMI_PLL_CTRL_DIV_MASK GENMASK(7, 4)
+#define SUN4I_HDMI_PLL_DBG0_REG 0x20c +#define SUN4I_HDMI_PLL_DBG0_TMDS_PARENT(n) (((n) & 1) << 21) +#define SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_MASK BIT(21) +#define SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_SHIFT 21
+#define SUN4I_HDMI_PKT_CTRL_REG(n) (0x2f0 + (4 * (n))) +#define SUN4I_HDMI_PKT_CTRL_TYPE(n, t) ((t) << (((n) % 4) * 4))
+#define SUN4I_HDMI_UNKNOWN_REG 0x300 +#define SUN4I_HDMI_UNKNOWN_INPUT_SYNC BIT(27)
+#define SUN4I_HDMI_DDC_CTRL_REG 0x500 +#define SUN4I_HDMI_DDC_CTRL_ENABLE BIT(31) +#define SUN4I_HDMI_DDC_CTRL_START_CMD BIT(30) +#define SUN4I_HDMI_DDC_CTRL_RESET BIT(0)
+#define SUN4I_HDMI_DDC_ADDR_REG 0x504 +#define SUN4I_HDMI_DDC_ADDR_SEGMENT(seg) (((seg) & 0xff) << 24) +#define SUN4I_HDMI_DDC_ADDR_EDDC(addr) (((addr) & 0xff) << 16) +#define SUN4I_HDMI_DDC_ADDR_OFFSET(off) (((off) & 0xff) << 8) +#define SUN4I_HDMI_DDC_ADDR_SLAVE(addr) ((addr) & 0xff)
+#define SUN4I_HDMI_DDC_FIFO_CTRL_REG 0x510 +#define SUN4I_HDMI_DDC_FIFO_CTRL_CLEAR BIT(31)
+#define SUN4I_HDMI_DDC_FIFO_DATA_REG 0x518 +#define SUN4I_HDMI_DDC_BYTE_COUNT_REG 0x51c
+#define SUN4I_HDMI_DDC_CMD_REG 0x520 +#define SUN4I_HDMI_DDC_CMD_EXPLICIT_EDDC_READ 6
+#define SUN4I_HDMI_DDC_CLK_REG 0x528 +#define SUN4I_HDMI_DDC_CLK_M(m) (((m) & 0x7) << 3) +#define SUN4I_HDMI_DDC_CLK_N(n) ((n) & 0x7)
+#define SUN4I_HDMI_DDC_LINE_CTRL_REG 0x540 +#define SUN4I_HDMI_DDC_LINE_CTRL_SDA_ENABLE BIT(9) +#define SUN4I_HDMI_DDC_LINE_CTRL_SCL_ENABLE BIT(8)
+#define SUN4I_HDMI_DDC_FIFO_SIZE 16
+enum sun4i_hdmi_pkt_type {
SUN4I_HDMI_PKT_AVI = 2,SUN4I_HDMI_PKT_END = 15,+};
+struct sun4i_hdmi {
struct drm_connector connector;struct drm_encoder encoder;struct device *dev;void __iomem *base;struct clk *bus_clk;struct clk *ddc_clk;struct clk *mod_clk;struct clk *pll0_clk;struct clk *pll1_clk;struct clk *tmds_clk;struct sun4i_drv *drv;bool hdmi_monitor;+};
+int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *clk); +int sun4i_tmds_create(struct sun4i_hdmi *hdmi);
+#endif /* _SUN4I_HDMI_H_ */ diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c new file mode 100644 index 000000000000..5125b14ea7a5 --- /dev/null +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c @@ -0,0 +1,128 @@ +/*
- Copyright (C) 2016 Free Electrons
- Copyright (C) 2016 NextThing Co
- Maxime Ripard maxime.ripard@free-electrons.com
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- */
+#include <linux/clk-provider.h>
+#include "sun4i_tcon.h" +#include "sun4i_hdmi.h"
+struct sun4i_ddc {
struct clk_hw hw;struct sun4i_hdmi *hdmi;+};
+static inline struct sun4i_ddc *hw_to_ddc(struct clk_hw *hw) +{
return container_of(hw, struct sun4i_ddc, hw);+}
+static unsigned long sun4i_ddc_calc_divider(unsigned long rate,
unsigned long parent_rate,u8 *m, u8 *n)+{
unsigned long best_rate = 0;u8 best_m = 0, best_n = 0, _m, _n;for (_m = 0; _m < 8; _m++) {for (_n = 0; _n < 8; _n++) {unsigned long tmp_rate;tmp_rate = (((parent_rate / 2) / 10) >> _n) / (_m + 1);if (tmp_rate > rate)continue;if (abs(rate - tmp_rate) < abs(rate - best_rate)) {best_rate = tmp_rate;best_m = _m;best_n = _n;}}}if (m && n) {*m = best_m;*n = best_n;}return best_rate;+}
+static long sun4i_ddc_round_rate(struct clk_hw *hw, unsigned long rate,
unsigned long *prate)+{
return sun4i_ddc_calc_divider(rate, *prate, NULL, NULL);+}
+static unsigned long sun4i_ddc_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)+{
struct sun4i_ddc *ddc = hw_to_ddc(hw);u32 reg;u8 m, n;reg = readl(ddc->hdmi->base + SUN4I_HDMI_DDC_CLK_REG);m = (reg >> 3) & 0x7;n = reg & 0x7;return (((parent_rate / 2) / 10) >> n) / (m + 1);+}
+static int sun4i_ddc_set_rate(struct clk_hw *hw, unsigned long rate,
unsigned long parent_rate)+{
struct sun4i_ddc *ddc = hw_to_ddc(hw);u8 div_m, div_n;sun4i_ddc_calc_divider(rate, parent_rate, &div_m, &div_n);writel(SUN4I_HDMI_DDC_CLK_M(div_m) | SUN4I_HDMI_DDC_CLK_N(div_n),ddc->hdmi->base + SUN4I_HDMI_DDC_CLK_REG);return 0;+}
+static const struct clk_ops sun4i_ddc_ops = {
.recalc_rate = sun4i_ddc_recalc_rate,.round_rate = sun4i_ddc_round_rate,.set_rate = sun4i_ddc_set_rate,+};
+int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *parent) +{
struct clk_init_data init;struct sun4i_ddc *ddc;const char *parent_name;parent_name = __clk_get_name(parent);if (!parent_name)return -ENODEV;ddc = devm_kzalloc(hdmi->dev, sizeof(*ddc), GFP_KERNEL);if (!ddc)return -ENOMEM;init.name = "hdmi-ddc";init.ops = &sun4i_ddc_ops;init.parent_names = &parent_name;init.num_parents = 1;init.flags = CLK_SET_RATE_PARENT;
I don't think this is really needed. It probably doesn't hurt though, since DDC is used when HDMI is not used for displaying, but it might affect any upstream PLLs, which theoretically may affect other users of said PLLs. The DDC clock is slow enough that we should be able to generate a usable clock rate anyway.
ddc->hdmi = hdmi;ddc->hw.init = &init;hdmi->ddc_clk = devm_clk_register(hdmi->dev, &ddc->hw);if (IS_ERR(hdmi->ddc_clk))return PTR_ERR(hdmi->ddc_clk);return 0;+} diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c new file mode 100644 index 000000000000..33175308c2ed --- /dev/null +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c @@ -0,0 +1,449 @@ +/*
- Copyright (C) 2016 Maxime Ripard
- Maxime Ripard maxime.ripard@free-electrons.com
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- */
+#include <drm/drmP.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_edid.h> +#include <drm/drm_encoder.h> +#include <drm/drm_panel.h>
+#include <linux/clk.h> +#include <linux/component.h> +#include <linux/iopoll.h> +#include <linux/of_address.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h>
+#include "sun4i_backend.h" +#include "sun4i_drv.h" +#include "sun4i_hdmi.h" +#include "sun4i_tcon.h"
+static inline struct sun4i_hdmi * +drm_encoder_to_sun4i_hdmi(struct drm_encoder *encoder) +{
return container_of(encoder, struct sun4i_hdmi,encoder);+}
+static inline struct sun4i_hdmi * +drm_connector_to_sun4i_hdmi(struct drm_connector *connector) +{
return container_of(connector, struct sun4i_hdmi,connector);+}
+static int sun4i_hdmi_setup_avi_infoframes(struct sun4i_hdmi *hdmi,
struct drm_display_mode *mode)+{
struct hdmi_avi_infoframe frame;u8 buffer[17];int i, ret;ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);if (ret < 0) {DRM_ERROR("Failed to get infoframes from mode\n");return ret;}ret = hdmi_avi_infoframe_pack(&frame, buffer, sizeof(buffer));if (ret < 0) {DRM_ERROR("Failed to pack infoframes\n");return ret;}for (i = 0; i < sizeof(buffer); i++)writeb(buffer[i], hdmi->base + SUN4I_HDMI_AVI_INFOFRAME_REG(i));return 0;+}
+static void sun4i_hdmi_disable(struct drm_encoder *encoder) +{
struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder);struct sun4i_drv *drv = hdmi->drv;struct sun4i_tcon *tcon = drv->tcon;u32 val;DRM_DEBUG_DRIVER("Disabling the HDMI Output\n");val = readl(hdmi->base + SUN4I_HDMI_VID_CTRL_REG);val &= ~SUN4I_HDMI_VID_CTRL_ENABLE;writel(val, hdmi->base + SUN4I_HDMI_VID_CTRL_REG);sun4i_tcon_channel_disable(tcon, 1);+}
+static void sun4i_hdmi_enable(struct drm_encoder *encoder) +{
struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder);struct sun4i_drv *drv = hdmi->drv;struct sun4i_tcon *tcon = drv->tcon;u32 val = 0;DRM_DEBUG_DRIVER("Enabling the HDMI Output\n");sun4i_tcon_channel_enable(tcon, 1);sun4i_hdmi_setup_avi_infoframes(hdmi, mode);val |= SUN4I_HDMI_PKT_CTRL_TYPE(0, SUN4I_HDMI_PKT_AVI);val |= SUN4I_HDMI_PKT_CTRL_TYPE(1, SUN4I_HDMI_PKT_END);writel(val, hdmi->base + SUN4I_HDMI_PKT_CTRL_REG(0));val = SUN4I_HDMI_VID_CTRL_ENABLE;if (hdmi->hdmi_monitor)val |= SUN4I_HDMI_VID_CTRL_HDMI_MODE;writel(val, hdmi->base + SUN4I_HDMI_VID_CTRL_REG);+}
+static void sun4i_hdmi_mode_set(struct drm_encoder *encoder,
struct drm_display_mode *mode,struct drm_display_mode *adjusted_mode)+{
struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder);struct sun4i_drv *drv = hdmi->drv;struct sun4i_tcon *tcon = drv->tcon;unsigned int x, y;u32 val;sun4i_tcon1_mode_set(tcon, encoder, mode);clk_set_rate(tcon->sclk1, mode->crtc_clock * 1000);clk_set_rate(hdmi->tmds_clk, mode->crtc_clock * 1000);/* Set input sync enable */writel(SUN4I_HDMI_UNKNOWN_INPUT_SYNC,hdmi->base + SUN4I_HDMI_UNKNOWN_REG);/* Setup timing registers */writel(SUN4I_HDMI_VID_TIMING_X(mode->hdisplay) |SUN4I_HDMI_VID_TIMING_Y(mode->vdisplay),hdmi->base + SUN4I_HDMI_VID_TIMING_ACT_REG);x = mode->htotal - mode->hsync_start;y = mode->vtotal - mode->vsync_start;
I'm a bit skeptical about this one. All the other parameters are not inclusive of other, why would this one be different? Shouldn't it be "Xtotal - Xsync_end" instead?
writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),hdmi->base + SUN4I_HDMI_VID_TIMING_BP_REG);x = mode->hsync_start - mode->hdisplay;y = mode->vsync_start - mode->vdisplay;writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),hdmi->base + SUN4I_HDMI_VID_TIMING_FP_REG);x = mode->hsync_end - mode->hsync_start;y = mode->vsync_end - mode->vsync_start;writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),hdmi->base + SUN4I_HDMI_VID_TIMING_SPW_REG);val = SUN4I_HDMI_VID_TIMING_POL_TX_CLK;if (mode->flags & DRM_MODE_FLAG_PHSYNC)val |= SUN4I_HDMI_VID_TIMING_POL_HSYNC;if (mode->flags & DRM_MODE_FLAG_PVSYNC)val |= SUN4I_HDMI_VID_TIMING_POL_VSYNC;writel(val, hdmi->base + SUN4I_HDMI_VID_TIMING_POL_REG);
You don't handle the interlaced video here, even though you set
hdmi->connector.interlace_allowed = true
later.
The double clock and double scan flags aren't handled either, though I don't understand which one is supposed to represent the need for the HDMI pixel repeater. AFAIK this is required for resolutions with pixel clocks lower than 25 MHz, the lower limit of HDMI's TMDS link.
+}
+static struct drm_encoder_helper_funcs sun4i_hdmi_helper_funcs = {
.disable = sun4i_hdmi_disable,.enable = sun4i_hdmi_enable,.mode_set = sun4i_hdmi_mode_set,+};
+static struct drm_encoder_funcs sun4i_hdmi_funcs = {
.destroy = drm_encoder_cleanup,+};
+static int sun4i_hdmi_read_sub_block(struct sun4i_hdmi *hdmi,
unsigned int blk, unsigned int offset,u8 *buf, unsigned int count)+{
unsigned long reg;int i;reg = readl(hdmi->base + SUN4I_HDMI_DDC_FIFO_CTRL_REG);writel(reg | SUN4I_HDMI_DDC_FIFO_CTRL_CLEAR,hdmi->base + SUN4I_HDMI_DDC_FIFO_CTRL_REG);writel(SUN4I_HDMI_DDC_ADDR_SEGMENT(offset >> 8) |SUN4I_HDMI_DDC_ADDR_EDDC(0x60) |SUN4I_HDMI_DDC_ADDR_OFFSET(offset) |SUN4I_HDMI_DDC_ADDR_SLAVE(0x50),
You can use DDC_ADDR from drm_edid.h.
hdmi->base + SUN4I_HDMI_DDC_ADDR_REG);writel(count, hdmi->base + SUN4I_HDMI_DDC_BYTE_COUNT_REG);writel(SUN4I_HDMI_DDC_CMD_EXPLICIT_EDDC_READ,hdmi->base + SUN4I_HDMI_DDC_CMD_REG);reg = readl(hdmi->base + SUN4I_HDMI_DDC_CTRL_REG);writel(reg | SUN4I_HDMI_DDC_CTRL_START_CMD,hdmi->base + SUN4I_HDMI_DDC_CTRL_REG);if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_DDC_CTRL_REG, reg,!(reg & SUN4I_HDMI_DDC_CTRL_START_CMD),100, 2000))return -EIO;for (i = 0; i < count; i++)buf[i] = readb(hdmi->base + SUN4I_HDMI_DDC_FIFO_DATA_REG);return 0;+}
+static int sun4i_hdmi_read_edid_block(void *data, u8 *buf, unsigned int blk,
size_t length)+{
struct sun4i_hdmi *hdmi = data;int retry = 2, i;do {for (i = 0; i < length; i += SUN4I_HDMI_DDC_FIFO_SIZE) {unsigned char offset = blk * EDID_LENGTH + i;unsigned int count = min((unsigned int)SUN4I_HDMI_DDC_FIFO_SIZE,length - i);int ret;ret = sun4i_hdmi_read_sub_block(hdmi, blk, offset,buf + i, count);if (ret)return ret;}} while (!drm_edid_block_valid(buf, blk, true, NULL) && (retry--));return 0;+}
+static int sun4i_hdmi_get_modes(struct drm_connector *connector) +{
struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);unsigned long reg;struct edid *edid;int ret;/* Reset i2c controller */writel(SUN4I_HDMI_DDC_CTRL_ENABLE | SUN4I_HDMI_DDC_CTRL_RESET,hdmi->base + SUN4I_HDMI_DDC_CTRL_REG);if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_DDC_CTRL_REG, reg,!(reg & SUN4I_HDMI_DDC_CTRL_RESET),100, 2000))return -EIO;writel(SUN4I_HDMI_DDC_LINE_CTRL_SDA_ENABLE |SUN4I_HDMI_DDC_LINE_CTRL_SCL_ENABLE,hdmi->base + SUN4I_HDMI_DDC_LINE_CTRL_REG);clk_set_rate(hdmi->ddc_clk, 100000);edid = drm_do_get_edid(connector, sun4i_hdmi_read_edid_block, hdmi);if (!edid)return 0;hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid);DRM_DEBUG_DRIVER("Monitor is %s monitor\n",hdmi->hdmi_monitor ? "an HDMI" : "a DVI");drm_mode_connector_update_edid_property(connector, edid);ret = drm_add_edid_modes(connector, edid);kfree(edid);return ret;+}
+static struct drm_connector_helper_funcs sun4i_hdmi_connector_helper_funcs = {
.get_modes = sun4i_hdmi_get_modes,+};
+static enum drm_connector_status +sun4i_hdmi_connector_detect(struct drm_connector *connector, bool force) +{
struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);unsigned long reg;if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_HPD_REG, reg,reg & SUN4I_HDMI_HPD_HIGH,0, 500000))
We shouldn't need to do polling here. It should just return the status at the instance it's called. Instead we should have a worker that does polling to check if something is plugged or unplugged. I don't see any interrupt bits for this though. :(
return connector_status_disconnected;return connector_status_connected;+}
+static struct drm_connector_funcs sun4i_hdmi_connector_funcs = {
.dpms = drm_atomic_helper_connector_dpms,.detect = sun4i_hdmi_connector_detect,.fill_modes = drm_helper_probe_single_connector_modes,.destroy = drm_connector_cleanup,.reset = drm_atomic_helper_connector_reset,.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,+};
+static int sun4i_hdmi_bind(struct device *dev, struct device *master,
void *data)+{
struct drm_device *drm = data;struct sun4i_drv *drv = drm->dev_private;struct sun4i_hdmi *hdmi = dev_get_drvdata(dev);int ret;hdmi->drv = drv;drm_encoder_helper_add(&hdmi->encoder,&sun4i_hdmi_helper_funcs);ret = drm_encoder_init(drm,&hdmi->encoder,&sun4i_hdmi_funcs,DRM_MODE_ENCODER_TMDS,NULL);if (ret) {dev_err(dev, "Couldn't initialise the HDMI encoder\n");return ret;}hdmi->encoder.possible_crtcs = BIT(0);
You can use drm_of_find_possible_crtcs() now. See the TV encoder driver.
drm_connector_helper_add(&hdmi->connector,&sun4i_hdmi_connector_helper_funcs);ret = drm_connector_init(drm, &hdmi->connector,&sun4i_hdmi_connector_funcs,DRM_MODE_CONNECTOR_HDMIA);if (ret) {dev_err(dev,"Couldn't initialise the Composite connector\n");
Wrong connector.
goto err_cleanup_connector;}hdmi->connector.interlace_allowed = true;drm_mode_connector_attach_encoder(&hdmi->connector, &hdmi->encoder);return 0;+err_cleanup_connector:
drm_encoder_cleanup(&hdmi->encoder);return ret;+}
+static void sun4i_hdmi_unbind(struct device *dev, struct device *master,
void *data)+{
struct sun4i_hdmi *hdmi = dev_get_drvdata(dev);drm_connector_cleanup(&hdmi->connector);drm_encoder_cleanup(&hdmi->encoder);+}
+static struct component_ops sun4i_hdmi_ops = {
.bind = sun4i_hdmi_bind,.unbind = sun4i_hdmi_unbind,+};
+static int sun4i_hdmi_probe(struct platform_device *pdev) +{
struct sun4i_hdmi *hdmi;struct resource *res;int ret;hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);if (!hdmi)return -ENOMEM;dev_set_drvdata(&pdev->dev, hdmi);hdmi->dev = &pdev->dev;res = platform_get_resource(pdev, IORESOURCE_MEM, 0);hdmi->base = devm_ioremap_resource(&pdev->dev, res);if (IS_ERR(hdmi->base)) {dev_err(&pdev->dev, "Couldn't map the HDMI encoder registers\n");return PTR_ERR(hdmi->base);}hdmi->bus_clk = devm_clk_get(&pdev->dev, "ahb");if (IS_ERR(hdmi->bus_clk)) {dev_err(&pdev->dev, "Couldn't get the HDMI bus clock\n");return PTR_ERR(hdmi->bus_clk);}clk_prepare_enable(hdmi->bus_clk);hdmi->mod_clk = devm_clk_get(&pdev->dev, "mod");if (IS_ERR(hdmi->mod_clk)) {dev_err(&pdev->dev, "Couldn't get the HDMI mod clock\n");return PTR_ERR(hdmi->mod_clk);}clk_prepare_enable(hdmi->mod_clk);hdmi->pll0_clk = devm_clk_get(&pdev->dev, "pll-0");if (IS_ERR(hdmi->pll0_clk)) {dev_err(&pdev->dev, "Couldn't get the HDMI PLL 0 clock\n");return PTR_ERR(hdmi->pll0_clk);}hdmi->pll1_clk = devm_clk_get(&pdev->dev, "pll-1");if (IS_ERR(hdmi->pll1_clk)) {dev_err(&pdev->dev, "Couldn't get the HDMI PLL 1 clock\n");return PTR_ERR(hdmi->pll1_clk);}ret = sun4i_tmds_create(hdmi);if (ret) {dev_err(&pdev->dev, "Couldn't create the TMDS clock\n");return ret;}writel(SUN4I_HDMI_CTRL_ENABLE, hdmi->base + SUN4I_HDMI_CTRL_REG);+#define SUN4I_HDMI_PAD_CTRL0 0xfe800000
writel(SUN4I_HDMI_PAD_CTRL0, hdmi->base + SUN4I_HDMI_PAD_CTRL0_REG);/* TODO: defines */writel((6 << 3) | (2 << 10) | BIT(14) | BIT(15) |BIT(19) | BIT(20) | BIT(22) | BIT(23),hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);/* TODO: defines */writel((8 << 0) | (7 << 8) | (239 << 12) | (7 << 17) | (4 << 20) |BIT(25) | BIT(27) | BIT(28) | BIT(29) | BIT(30) | BIT(31),hdmi->base + SUN4I_HDMI_PLL_CTRL_REG);
FYI some bits in this register look a lot like the MIPI PLL on the A33. Bit 31 looks like the enable bit.
ret = sun4i_ddc_create(hdmi, hdmi->tmds_clk);if (ret) {dev_err(&pdev->dev, "Couldn't create the DDC clock\n");return ret;}
We do all this in the bind function for all the other components. Any particular reason to do it differently here?
return component_add(&pdev->dev, &sun4i_hdmi_ops);+}
+static int sun4i_hdmi_remove(struct platform_device *pdev) +{
component_del(&pdev->dev, &sun4i_hdmi_ops);return 0;+}
+static const struct of_device_id sun4i_hdmi_of_table[] = {
{ .compatible = "allwinner,sun5i-a10s-hdmi" },{ }+}; +MODULE_DEVICE_TABLE(of, sun4i_hdmi_of_table);
+static struct platform_driver sun4i_hdmi_driver = {
.probe = sun4i_hdmi_probe,.remove = sun4i_hdmi_remove,.driver = {.name = "sun4i-hdmi",.of_match_table = sun4i_hdmi_of_table,},+}; +module_platform_driver(sun4i_hdmi_driver);
+MODULE_AUTHOR("Maxime Ripard maxime.ripard@free-electrons.com"); +MODULE_DESCRIPTION("Allwinner A10 HDMI Driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c new file mode 100644 index 000000000000..40f48f1d4685 --- /dev/null +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c @@ -0,0 +1,236 @@ +/*
- Copyright (C) 2016 Free Electrons
- Copyright (C) 2016 NextThing Co
- Maxime Ripard maxime.ripard@free-electrons.com
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- */
+#include <linux/clk-provider.h>
+#include "sun4i_tcon.h" +#include "sun4i_hdmi.h"
+struct sun4i_tmds {
struct clk_hw hw;struct sun4i_hdmi *hdmi;+};
+static inline struct sun4i_tmds *hw_to_tmds(struct clk_hw *hw) +{
return container_of(hw, struct sun4i_tmds, hw);+}
+static unsigned long sun4i_tmds_calc_divider(unsigned long rate,
unsigned long parent_rate,u8 *div,bool *half)+{
unsigned long best_rate = 0;u8 best_m = 0, m;bool is_double;for (m = 1; m < 16; m++) {u8 d;for (d = 1; d < 3; d++) {unsigned long tmp_rate;tmp_rate = parent_rate / m / d;if (tmp_rate > rate)continue;if (!best_rate ||(rate - tmp_rate) < (rate - best_rate)) {best_rate = tmp_rate;best_m = m;is_double = d;}}}if (div && half) {*div = best_m;*half = is_double;}return best_rate;+}
+static int sun4i_tmds_determine_rate(struct clk_hw *hw,
struct clk_rate_request *req)+{
struct clk_hw *parent;unsigned long best_parent = 0;unsigned long rate = req->rate;int best_div = 1, best_half = 1;int i, j;printk("%s %d rate %lu\n", __func__, __LINE__, rate);
Stray printk?
/** We only consider PLL3, since the TCON is very likely to be* clocked from it, and to have the same rate than our HDMI* clock, so we should not need to do anything.*/parent = clk_hw_get_parent_by_index(hw, 0);if (!parent)return -EINVAL;for (i = 1; i < 3; i++) {for (j = 1; j < 16; j++) {unsigned long ideal = rate * i * j;unsigned long rounded;rounded = clk_hw_round_rate(parent, ideal);if (rounded == ideal) {best_parent = rounded;best_half = i;best_div = j;goto out;}if (abs(rate - rounded / i) <abs(rate - best_parent / best_div)) {best_parent = rounded;best_div = i;}}}+out:
req->rate = best_parent / best_half / best_div;req->best_parent_rate = best_parent;req->best_parent_hw = parent;printk("%s %d rate %lu parent rate %lu (%s) div %d half %d\n",__func__, __LINE__, req->rate, req->best_parent_rate,clk_hw_get_name(req->best_parent_hw),best_div, best_half);
Stray printk?
return 0;+}
+static unsigned long sun4i_tmds_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)+{
struct sun4i_tmds *tmds = hw_to_tmds(hw);u32 reg;reg = readl(tmds->hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);if (reg & SUN4I_HDMI_PAD_CTRL1_HALVE_CLK)parent_rate /= 2;reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_CTRL_REG);reg = (reg >> 4) & 0xf;if (!reg)reg = 1;return parent_rate / reg;+}
+static int sun4i_tmds_set_rate(struct clk_hw *hw, unsigned long rate,
unsigned long parent_rate)+{
struct sun4i_tmds *tmds = hw_to_tmds(hw);bool half;u32 reg;u8 div;sun4i_tmds_calc_divider(rate, parent_rate, &div, &half);printk("%s %d rate %lu parent rate %lu div %d half %s\n",__func__, __LINE__, rate, parent_rate, div,half ? "yes" : "no");
Stray printk?
reg = readl(tmds->hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);reg &= ~SUN4I_HDMI_PAD_CTRL1_HALVE_CLK;if (half)reg |= SUN4I_HDMI_PAD_CTRL1_HALVE_CLK;writel(reg, tmds->hdmi->base + SUN4I_HDMI_PAD_CTRL1_REG);reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_CTRL_REG);reg &= ~SUN4I_HDMI_PLL_CTRL_DIV_MASK;writel(reg | SUN4I_HDMI_PLL_CTRL_DIV(div),tmds->hdmi->base + SUN4I_HDMI_PLL_CTRL_REG);return 0;+}
+static u8 sun4i_tmds_get_parent(struct clk_hw *hw) +{
struct sun4i_tmds *tmds = hw_to_tmds(hw);u32 reg;reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_DBG0_REG);return ((reg & SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_MASK) >>SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_SHIFT);+}
+static int sun4i_tmds_set_parent(struct clk_hw *hw, u8 index) +{
struct sun4i_tmds *tmds = hw_to_tmds(hw);u32 reg;if (index > 1)return -EINVAL;reg = readl(tmds->hdmi->base + SUN4I_HDMI_PLL_DBG0_REG);reg &= ~SUN4I_HDMI_PLL_DBG0_TMDS_PARENT_MASK;writel(reg | SUN4I_HDMI_PLL_DBG0_TMDS_PARENT(index),tmds->hdmi->base + SUN4I_HDMI_PLL_DBG0_REG);return 0;+}
+static const struct clk_ops sun4i_tmds_ops = {
.determine_rate = sun4i_tmds_determine_rate,.recalc_rate = sun4i_tmds_recalc_rate,.set_rate = sun4i_tmds_set_rate,.get_parent = sun4i_tmds_get_parent,.set_parent = sun4i_tmds_set_parent,+};
+int sun4i_tmds_create(struct sun4i_hdmi *hdmi) +{
struct clk_init_data init;struct sun4i_tmds *tmds;const char *parents[2];parents[0] = __clk_get_name(hdmi->pll0_clk);if (!parents[0])return -ENODEV;parents[1] = __clk_get_name(hdmi->pll1_clk);if (!parents[1])return -ENODEV;tmds = devm_kzalloc(hdmi->dev, sizeof(*tmds), GFP_KERNEL);if (!tmds)return -ENOMEM;init.name = "hdmi-tmds";init.ops = &sun4i_tmds_ops;init.parent_names = parents;init.num_parents = 2;init.flags = CLK_SET_RATE_PARENT;tmds->hdmi = hdmi;tmds->hw.init = &init;hdmi->tmds_clk = devm_clk_register(hdmi->dev, &tmds->hw);if (IS_ERR(hdmi->tmds_clk))return PTR_ERR(hdmi->tmds_clk);return 0;+}
I also compared the manuals of A20 and A31, and the existing U-boot driver.
So far it looks like the DDC bits are quite different. We could probably use regfields to work around it, but the DDC clock formula is completely different. The TMDS clock pre-divider is also different, your usual sun4i vs sun6i factor offset. Last, the initial values for the 3 PLL related registers are different.
I'm currently working on an A31 variant for this, basically just copying the DDC and TMDS bits.
Regards ChenYu
git-series 0.8.11
-- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Hi Chen-Yu,
On Fri, Apr 21, 2017 at 11:17:17PM +0800, Chen-Yu Tsai wrote:
Hi,
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The earlier Allwinner SoCs (A10, A10s, A20, A31) have an embedded HDMI controller.
That HDMI controller is able to do audio and CEC, but those have been left out for now.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
drivers/gpu/drm/sun4i/Makefile | 5 +- drivers/gpu/drm/sun4i/sun4i_hdmi.h | 124 ++++++- drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c | 128 ++++++- drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 449 +++++++++++++++++++++- drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 236 +++++++++++- 5 files changed, 942 insertions(+), 0 deletions(-) create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi.h create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c
Applying patch #9608371 using 'git am' Description: [13/15] drm/sun4i: Add HDMI support Applying: drm/sun4i: Add HDMI support .git/rebase-apply/patch:116: trailing whitespace.
.git/rebase-apply/patch:531: trailing whitespace.
.git/rebase-apply/patch:701: trailing whitespace.
warning: 3 lines add whitespace errors.
Fixed.
+int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *parent) +{
struct clk_init_data init;struct sun4i_ddc *ddc;const char *parent_name;parent_name = __clk_get_name(parent);if (!parent_name)return -ENODEV;ddc = devm_kzalloc(hdmi->dev, sizeof(*ddc), GFP_KERNEL);if (!ddc)return -ENOMEM;init.name = "hdmi-ddc";init.ops = &sun4i_ddc_ops;init.parent_names = &parent_name;init.num_parents = 1;init.flags = CLK_SET_RATE_PARENT;I don't think this is really needed. It probably doesn't hurt though, since DDC is used when HDMI is not used for displaying, but it might affect any upstream PLLs, which theoretically may affect other users of said PLLs. The DDC clock is slow enough that we should be able to generate a usable clock rate anyway.
Good point, I removed it.
writel(SUN4I_HDMI_VID_TIMING_X(mode->hdisplay) |SUN4I_HDMI_VID_TIMING_Y(mode->vdisplay),hdmi->base + SUN4I_HDMI_VID_TIMING_ACT_REG);x = mode->htotal - mode->hsync_start;y = mode->vtotal - mode->vsync_start;I'm a bit skeptical about this one. All the other parameters are not inclusive of other, why would this one be different? Shouldn't it be "Xtotal - Xsync_end" instead?
By the usual meaning of backporch, you're right. However, Allwinner's seems to have it's own, which is actually the backporch + sync length.
We also have that on all the other connectors (and TCON), and this was confirmed at the time using a scope on an RGB signal.
writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),hdmi->base + SUN4I_HDMI_VID_TIMING_BP_REG);x = mode->hsync_start - mode->hdisplay;y = mode->vsync_start - mode->vdisplay;writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),hdmi->base + SUN4I_HDMI_VID_TIMING_FP_REG);x = mode->hsync_end - mode->hsync_start;y = mode->vsync_end - mode->vsync_start;writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),hdmi->base + SUN4I_HDMI_VID_TIMING_SPW_REG);val = SUN4I_HDMI_VID_TIMING_POL_TX_CLK;if (mode->flags & DRM_MODE_FLAG_PHSYNC)val |= SUN4I_HDMI_VID_TIMING_POL_HSYNC;if (mode->flags & DRM_MODE_FLAG_PVSYNC)val |= SUN4I_HDMI_VID_TIMING_POL_VSYNC;writel(val, hdmi->base + SUN4I_HDMI_VID_TIMING_POL_REG);You don't handle the interlaced video here, even though you set
hdmi->connector.interlace_allowed = truelater.
I'll fix that.
The double clock and double scan flags aren't handled either, though I don't understand which one is supposed to represent the need for the HDMI pixel repeater. AFAIK this is required for resolutions with pixel clocks lower than 25 MHz, the lower limit of HDMI's TMDS link.
I'm not sure about this one though. I'd like to keep things quite simple for now and build up on that once the basis is working. Is it common in the wild?
hdmi->base + SUN4I_HDMI_DDC_FIFO_CTRL_REG);writel(SUN4I_HDMI_DDC_ADDR_SEGMENT(offset >> 8) |SUN4I_HDMI_DDC_ADDR_EDDC(0x60) |SUN4I_HDMI_DDC_ADDR_OFFSET(offset) |SUN4I_HDMI_DDC_ADDR_SLAVE(0x50),You can use DDC_ADDR from drm_edid.h.
Done.
+static enum drm_connector_status +sun4i_hdmi_connector_detect(struct drm_connector *connector, bool force) +{
struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);unsigned long reg;if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_HPD_REG, reg,reg & SUN4I_HDMI_HPD_HIGH,0, 500000))We shouldn't need to do polling here. It should just return the status at the instance it's called. Instead we should have a worker that does polling to check if something is plugged or unplugged. I don't see any interrupt bits for this though. :(
As far as I know, polling in detect is okay. Why would you want to remove it?
ret = drm_encoder_init(drm,&hdmi->encoder,&sun4i_hdmi_funcs,DRM_MODE_ENCODER_TMDS,NULL);if (ret) {dev_err(dev, "Couldn't initialise the HDMI encoder\n");return ret;}hdmi->encoder.possible_crtcs = BIT(0);You can use drm_of_find_possible_crtcs() now. See the TV encoder driver.
Ack.
drm_connector_helper_add(&hdmi->connector,&sun4i_hdmi_connector_helper_funcs);ret = drm_connector_init(drm, &hdmi->connector,&sun4i_hdmi_connector_funcs,DRM_MODE_CONNECTOR_HDMIA);if (ret) {dev_err(dev,"Couldn't initialise the Composite connector\n");Wrong connector.
Fixed.
ret = sun4i_ddc_create(hdmi, hdmi->tmds_clk);if (ret) {dev_err(&pdev->dev, "Couldn't create the DDC clock\n");return ret;}We do all this in the bind function for all the other components. Any particular reason to do it differently here?
Not really, I'll change it.
Thanks! Maxime
On Wed, Apr 26, 2017 at 2:50 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
Hi Chen-Yu,
On Fri, Apr 21, 2017 at 11:17:17PM +0800, Chen-Yu Tsai wrote:
Hi,
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The earlier Allwinner SoCs (A10, A10s, A20, A31) have an embedded HDMI controller.
That HDMI controller is able to do audio and CEC, but those have been left out for now.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
drivers/gpu/drm/sun4i/Makefile | 5 +- drivers/gpu/drm/sun4i/sun4i_hdmi.h | 124 ++++++- drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c | 128 ++++++- drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 449 +++++++++++++++++++++- drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 236 +++++++++++- 5 files changed, 942 insertions(+), 0 deletions(-) create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi.h create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c
Applying patch #9608371 using 'git am' Description: [13/15] drm/sun4i: Add HDMI support Applying: drm/sun4i: Add HDMI support .git/rebase-apply/patch:116: trailing whitespace.
.git/rebase-apply/patch:531: trailing whitespace.
.git/rebase-apply/patch:701: trailing whitespace.
warning: 3 lines add whitespace errors.
Fixed.
+int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *parent) +{
struct clk_init_data init;struct sun4i_ddc *ddc;const char *parent_name;parent_name = __clk_get_name(parent);if (!parent_name)return -ENODEV;ddc = devm_kzalloc(hdmi->dev, sizeof(*ddc), GFP_KERNEL);if (!ddc)return -ENOMEM;init.name = "hdmi-ddc";init.ops = &sun4i_ddc_ops;init.parent_names = &parent_name;init.num_parents = 1;init.flags = CLK_SET_RATE_PARENT;I don't think this is really needed. It probably doesn't hurt though, since DDC is used when HDMI is not used for displaying, but it might affect any upstream PLLs, which theoretically may affect other users of said PLLs. The DDC clock is slow enough that we should be able to generate a usable clock rate anyway.
Good point, I removed it.
writel(SUN4I_HDMI_VID_TIMING_X(mode->hdisplay) |SUN4I_HDMI_VID_TIMING_Y(mode->vdisplay),hdmi->base + SUN4I_HDMI_VID_TIMING_ACT_REG);x = mode->htotal - mode->hsync_start;y = mode->vtotal - mode->vsync_start;I'm a bit skeptical about this one. All the other parameters are not inclusive of other, why would this one be different? Shouldn't it be "Xtotal - Xsync_end" instead?
By the usual meaning of backporch, you're right. However, Allwinner's seems to have it's own, which is actually the backporch + sync length.
We also have that on all the other connectors (and TCON), and this was confirmed at the time using a scope on an RGB signal.
Yes. On the later SoCs such as the A31, the user manual actually has timing diagrams showing this.
Unlike the TCON, the HDMI controller's timings lists the front porch separately, instead of an all inclusive Xtotal. This is what made me look twice. This should be easy to confirm though. Since the HDMI modes are well known and can be exactly reproduced on our hardware, we can just check for any distortions or refresh rate errors.
writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),hdmi->base + SUN4I_HDMI_VID_TIMING_BP_REG);x = mode->hsync_start - mode->hdisplay;y = mode->vsync_start - mode->vdisplay;writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),hdmi->base + SUN4I_HDMI_VID_TIMING_FP_REG);x = mode->hsync_end - mode->hsync_start;y = mode->vsync_end - mode->vsync_start;writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),hdmi->base + SUN4I_HDMI_VID_TIMING_SPW_REG);val = SUN4I_HDMI_VID_TIMING_POL_TX_CLK;if (mode->flags & DRM_MODE_FLAG_PHSYNC)val |= SUN4I_HDMI_VID_TIMING_POL_HSYNC;if (mode->flags & DRM_MODE_FLAG_PVSYNC)val |= SUN4I_HDMI_VID_TIMING_POL_VSYNC;writel(val, hdmi->base + SUN4I_HDMI_VID_TIMING_POL_REG);You don't handle the interlaced video here, even though you set
hdmi->connector.interlace_allowed = truelater.
I'll fix that.
The double clock and double scan flags aren't handled either, though I don't understand which one is supposed to represent the need for the HDMI pixel repeater. AFAIK this is required for resolutions with pixel clocks lower than 25 MHz, the lower limit of HDMI's TMDS link.
I'm not sure about this one though. I'd like to keep things quite simple for now and build up on that once the basis is working. Is it common in the wild?
If you drive the display at SDTV resolutions, then yes. Mode lines from my HDMI monitor:
720x576i 50 720 732 795 864 576 580 586 625 flags: nhsync, nvsync, interlace, dblclk; type: driver 720x480i 60 720 739 801 858 480 488 494 525 flags: nhsync, nvsync, interlace, dblclk; type: driver 720x480i 60 720 739 801 858 480 488 494 525 flags: nhsync, nvsync, interlace, dblclk; type: driver
AFAIK these are standard modes that all devices should support. Whether they are used daily is another thing. Maybe block modes with dblclk in .mode_fixup for now?
hdmi->base + SUN4I_HDMI_DDC_FIFO_CTRL_REG);writel(SUN4I_HDMI_DDC_ADDR_SEGMENT(offset >> 8) |SUN4I_HDMI_DDC_ADDR_EDDC(0x60) |SUN4I_HDMI_DDC_ADDR_OFFSET(offset) |SUN4I_HDMI_DDC_ADDR_SLAVE(0x50),You can use DDC_ADDR from drm_edid.h.
Done.
There's also DDC_SEGMENT_ADDR (which is 0x30) you can use to replace 0x60. The 1 bit shift is probably something related to I2C.
+static enum drm_connector_status +sun4i_hdmi_connector_detect(struct drm_connector *connector, bool force) +{
struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);unsigned long reg;if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_HPD_REG, reg,reg & SUN4I_HDMI_HPD_HIGH,0, 500000))We shouldn't need to do polling here. It should just return the status at the instance it's called. Instead we should have a worker that does polling to check if something is plugged or unplugged. I don't see any interrupt bits for this though. :(
As far as I know, polling in detect is okay. Why would you want to remove it?
Hmm, I guess it only serves to debounce the detection, i.e. extend the time period of validity from the instance the function is run to the instance plus 500 ms.
To be clear I'm not against it. However this only really works when the DRM subsystem is brought up. We still need something else for hotplugging, which is what I was arguing for.
Regards ChenYu
ret = drm_encoder_init(drm,&hdmi->encoder,&sun4i_hdmi_funcs,DRM_MODE_ENCODER_TMDS,NULL);if (ret) {dev_err(dev, "Couldn't initialise the HDMI encoder\n");return ret;}hdmi->encoder.possible_crtcs = BIT(0);You can use drm_of_find_possible_crtcs() now. See the TV encoder driver.
Ack.
drm_connector_helper_add(&hdmi->connector,&sun4i_hdmi_connector_helper_funcs);ret = drm_connector_init(drm, &hdmi->connector,&sun4i_hdmi_connector_funcs,DRM_MODE_CONNECTOR_HDMIA);if (ret) {dev_err(dev,"Couldn't initialise the Composite connector\n");Wrong connector.
Fixed.
ret = sun4i_ddc_create(hdmi, hdmi->tmds_clk);if (ret) {dev_err(&pdev->dev, "Couldn't create the DDC clock\n");return ret;}We do all this in the bind function for all the other components. Any particular reason to do it differently here?
Not really, I'll change it.
Thanks! Maxime
-- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
On Wed, Apr 26, 2017 at 03:59:28PM +0800, Chen-Yu Tsai wrote:
writel(SUN4I_HDMI_VID_TIMING_X(mode->hdisplay) |SUN4I_HDMI_VID_TIMING_Y(mode->vdisplay),hdmi->base + SUN4I_HDMI_VID_TIMING_ACT_REG);x = mode->htotal - mode->hsync_start;y = mode->vtotal - mode->vsync_start;I'm a bit skeptical about this one. All the other parameters are not inclusive of other, why would this one be different? Shouldn't it be "Xtotal - Xsync_end" instead?
By the usual meaning of backporch, you're right. However, Allwinner's seems to have it's own, which is actually the backporch + sync length.
We also have that on all the other connectors (and TCON), and this was confirmed at the time using a scope on an RGB signal.
Yes. On the later SoCs such as the A31, the user manual actually has timing diagrams showing this.
Unlike the TCON, the HDMI controller's timings lists the front porch separately, instead of an all inclusive Xtotal. This is what made me look twice. This should be easy to confirm though. Since the HDMI modes are well known and can be exactly reproduced on our hardware, we can just check for any distortions or refresh rate errors.
This isn't as trivial as that. Screens usually have some tolerancies on the timings, which will probably make it go unnoticed, even though they are wrong.
writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),hdmi->base + SUN4I_HDMI_VID_TIMING_BP_REG);x = mode->hsync_start - mode->hdisplay;y = mode->vsync_start - mode->vdisplay;writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),hdmi->base + SUN4I_HDMI_VID_TIMING_FP_REG);x = mode->hsync_end - mode->hsync_start;y = mode->vsync_end - mode->vsync_start;writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),hdmi->base + SUN4I_HDMI_VID_TIMING_SPW_REG);val = SUN4I_HDMI_VID_TIMING_POL_TX_CLK;if (mode->flags & DRM_MODE_FLAG_PHSYNC)val |= SUN4I_HDMI_VID_TIMING_POL_HSYNC;if (mode->flags & DRM_MODE_FLAG_PVSYNC)val |= SUN4I_HDMI_VID_TIMING_POL_VSYNC;writel(val, hdmi->base + SUN4I_HDMI_VID_TIMING_POL_REG);You don't handle the interlaced video here, even though you set
hdmi->connector.interlace_allowed = truelater.
I'll fix that.
The double clock and double scan flags aren't handled either, though I don't understand which one is supposed to represent the need for the HDMI pixel repeater. AFAIK this is required for resolutions with pixel clocks lower than 25 MHz, the lower limit of HDMI's TMDS link.
I'm not sure about this one though. I'd like to keep things quite simple for now and build up on that once the basis is working. Is it common in the wild?
If you drive the display at SDTV resolutions, then yes. Mode lines from my HDMI monitor:
720x576i 50 720 732 795 864 576 580 586 625 flags: nhsync, nvsync, interlace, dblclk; type: driver 720x480i 60 720 739 801 858 480 488 494 525 flags: nhsync, nvsync, interlace, dblclk; type: driver 720x480i 60 720 739 801 858 480 488 494 525 flags: nhsync, nvsync, interlace, dblclk; type: driver
AFAIK these are standard modes that all devices should support. Whether they are used daily is another thing. Maybe block modes with dblclk in .mode_fixup for now?
That would rather be atomic_check and / or mode_valid, but yeah, I can do that.
Thanks! Maxime
The A10s has an HDMI controller connected to the second TCON channel. Add it to our DT.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- arch/arm/boot/dts/sun5i-a10s.dtsi | 34 ++++++++++++++++++++++++++++++++- arch/arm/boot/dts/sun5i.dtsi | 1 +- 2 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi index 074485782a4a..3482c9d2b120 100644 --- a/arch/arm/boot/dts/sun5i-a10s.dtsi +++ b/arch/arm/boot/dts/sun5i-a10s.dtsi @@ -72,7 +72,33 @@ }; };
+ display-engine { + compatible = "allwinner,sun5i-a10s-display-engine", + "allwinner,sun5i-a13-display-engine"; + allwinner,pipelines = <&fe0>; + }; + soc@01c00000 { + hdmi0: hdmi@01c16000 { + compatible = "allwinner,sun5i-a10s-hdmi"; + reg = <0x01c16000 0x1000>; + clocks = <&ccu CLK_AHB_HDMI>, <&ccu CLK_HDMI>, + <&ccu CLK_PLL_VIDEO0_2X>, + <&ccu CLK_PLL_VIDEO1_2X>; + clock-names = "ahb", "mod", "pll-0", "pll-1"; + status = "disabled"; + + port { + #address-cells = <1>; + #size-cells = <0>; + + hdmi0_in_tcon0: endpoint@0 { + reg = <0>; + remote-endpoint = <&tcon0_out_hdmi0>; + }; + }; + }; + pwm: pwm@01c20e00 { compatible = "allwinner,sun5i-a10s-pwm"; reg = <0x01c20e00 0xc>; @@ -129,3 +155,11 @@
&sram_a { }; + +&tcon0_out { + tcon0_out_hdmi0: endpoint@2 { + reg = <2>; + remote-endpoint = <&hdmi0_in_tcon0>; + allwinner,tcon-channel = <1>; + }; +}; diff --git a/arch/arm/boot/dts/sun5i.dtsi b/arch/arm/boot/dts/sun5i.dtsi index f3b6e19244f9..3d009b2aa42a 100644 --- a/arch/arm/boot/dts/sun5i.dtsi +++ b/arch/arm/boot/dts/sun5i.dtsi @@ -273,6 +273,7 @@ tcon0_out_tve0: endpoint@1 { reg = <1>; remote-endpoint = <&tve0_in_tcon0>; + allwinner,tcon-channel = <1>; }; }; };
Hi,
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The A10s has an HDMI controller connected to the second TCON channel. Add it to our DT.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
arch/arm/boot/dts/sun5i-a10s.dtsi | 34 ++++++++++++++++++++++++++++++++- arch/arm/boot/dts/sun5i.dtsi | 1 +- 2 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi index 074485782a4a..3482c9d2b120 100644 --- a/arch/arm/boot/dts/sun5i-a10s.dtsi +++ b/arch/arm/boot/dts/sun5i-a10s.dtsi @@ -72,7 +72,33 @@ }; };
display-engine {compatible = "allwinner,sun5i-a10s-display-engine","allwinner,sun5i-a13-display-engine";allwinner,pipelines = <&fe0>;};soc@01c00000 {hdmi0: hdmi@01c16000 {
Nit: is the 0 suffix needed? I don't see any indication that there is a second controller.
compatible = "allwinner,sun5i-a10s-hdmi";reg = <0x01c16000 0x1000>;clocks = <&ccu CLK_AHB_HDMI>, <&ccu CLK_HDMI>,<&ccu CLK_PLL_VIDEO0_2X>,<&ccu CLK_PLL_VIDEO1_2X>;clock-names = "ahb", "mod", "pll-0", "pll-1";status = "disabled";port {#address-cells = <1>;#size-cells = <0>;hdmi0_in_tcon0: endpoint@0 {reg = <0>;remote-endpoint = <&tcon0_out_hdmi0>;};};};pwm: pwm@01c20e00 { compatible = "allwinner,sun5i-a10s-pwm"; reg = <0x01c20e00 0xc>;@@ -129,3 +155,11 @@
&sram_a { };
+&tcon0_out {
tcon0_out_hdmi0: endpoint@2 {reg = <2>;remote-endpoint = <&hdmi0_in_tcon0>;allwinner,tcon-channel = <1>;};+}; diff --git a/arch/arm/boot/dts/sun5i.dtsi b/arch/arm/boot/dts/sun5i.dtsi index f3b6e19244f9..3d009b2aa42a 100644 --- a/arch/arm/boot/dts/sun5i.dtsi +++ b/arch/arm/boot/dts/sun5i.dtsi @@ -273,6 +273,7 @@ tcon0_out_tve0: endpoint@1 { reg = <1>; remote-endpoint = <&tve0_in_tcon0>;
allwinner,tcon-channel = <1>;
This looks like a separate patch, probably following the binding change?
Regards ChenYu
}; }; };-- git-series 0.8.11
-- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
1;4601;0c On Wed, Mar 08, 2017 at 11:35:39AM +0800, Chen-Yu Tsai wrote:
Hi,
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The A10s has an HDMI controller connected to the second TCON channel. Add it to our DT.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
arch/arm/boot/dts/sun5i-a10s.dtsi | 34 ++++++++++++++++++++++++++++++++- arch/arm/boot/dts/sun5i.dtsi | 1 +- 2 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi index 074485782a4a..3482c9d2b120 100644 --- a/arch/arm/boot/dts/sun5i-a10s.dtsi +++ b/arch/arm/boot/dts/sun5i-a10s.dtsi @@ -72,7 +72,33 @@ }; };
display-engine {compatible = "allwinner,sun5i-a10s-display-engine","allwinner,sun5i-a13-display-engine";allwinner,pipelines = <&fe0>;};soc@01c00000 {hdmi0: hdmi@01c16000 {Nit: is the 0 suffix needed? I don't see any indication that there is a second controller.
compatible = "allwinner,sun5i-a10s-hdmi";reg = <0x01c16000 0x1000>;clocks = <&ccu CLK_AHB_HDMI>, <&ccu CLK_HDMI>,<&ccu CLK_PLL_VIDEO0_2X>,<&ccu CLK_PLL_VIDEO1_2X>;clock-names = "ahb", "mod", "pll-0", "pll-1";status = "disabled";port {#address-cells = <1>;#size-cells = <0>;hdmi0_in_tcon0: endpoint@0 {reg = <0>;remote-endpoint = <&tcon0_out_hdmi0>;};};};pwm: pwm@01c20e00 { compatible = "allwinner,sun5i-a10s-pwm"; reg = <0x01c20e00 0xc>;@@ -129,3 +155,11 @@
&sram_a { };
+&tcon0_out {
tcon0_out_hdmi0: endpoint@2 {reg = <2>;remote-endpoint = <&hdmi0_in_tcon0>;allwinner,tcon-channel = <1>;};+}; diff --git a/arch/arm/boot/dts/sun5i.dtsi b/arch/arm/boot/dts/sun5i.dtsi index f3b6e19244f9..3d009b2aa42a 100644 --- a/arch/arm/boot/dts/sun5i.dtsi +++ b/arch/arm/boot/dts/sun5i.dtsi @@ -273,6 +273,7 @@ tcon0_out_tve0: endpoint@1 { reg = <1>; remote-endpoint = <&tve0_in_tcon0>;
allwinner,tcon-channel = <1>;This looks like a separate patch, probably following the binding change?
I don't know, the binding says that without anything specified, reg would be used. I was assuming that we only needed it once we had the new endpoint to make it consistent, therefore it didn't need an extra patch.
But I can definitely create one if you want. Maxime
On Thu, Mar 9, 2017 at 6:59 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
1;4601;0c On Wed, Mar 08, 2017 at 11:35:39AM +0800, Chen-Yu Tsai wrote:
Hi,
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The A10s has an HDMI controller connected to the second TCON channel. Add it to our DT.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
arch/arm/boot/dts/sun5i-a10s.dtsi | 34 ++++++++++++++++++++++++++++++++- arch/arm/boot/dts/sun5i.dtsi | 1 +- 2 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi index 074485782a4a..3482c9d2b120 100644 --- a/arch/arm/boot/dts/sun5i-a10s.dtsi +++ b/arch/arm/boot/dts/sun5i-a10s.dtsi @@ -72,7 +72,33 @@ }; };
display-engine {compatible = "allwinner,sun5i-a10s-display-engine","allwinner,sun5i-a13-display-engine";allwinner,pipelines = <&fe0>;};soc@01c00000 {hdmi0: hdmi@01c16000 {Nit: is the 0 suffix needed? I don't see any indication that there is a second controller.
compatible = "allwinner,sun5i-a10s-hdmi";reg = <0x01c16000 0x1000>;clocks = <&ccu CLK_AHB_HDMI>, <&ccu CLK_HDMI>,<&ccu CLK_PLL_VIDEO0_2X>,<&ccu CLK_PLL_VIDEO1_2X>;clock-names = "ahb", "mod", "pll-0", "pll-1";status = "disabled";port {#address-cells = <1>;#size-cells = <0>;hdmi0_in_tcon0: endpoint@0 {reg = <0>;remote-endpoint = <&tcon0_out_hdmi0>;};};};pwm: pwm@01c20e00 { compatible = "allwinner,sun5i-a10s-pwm"; reg = <0x01c20e00 0xc>;@@ -129,3 +155,11 @@
&sram_a { };
+&tcon0_out {
tcon0_out_hdmi0: endpoint@2 {reg = <2>;remote-endpoint = <&hdmi0_in_tcon0>;allwinner,tcon-channel = <1>;};+}; diff --git a/arch/arm/boot/dts/sun5i.dtsi b/arch/arm/boot/dts/sun5i.dtsi index f3b6e19244f9..3d009b2aa42a 100644 --- a/arch/arm/boot/dts/sun5i.dtsi +++ b/arch/arm/boot/dts/sun5i.dtsi @@ -273,6 +273,7 @@ tcon0_out_tve0: endpoint@1 { reg = <1>; remote-endpoint = <&tve0_in_tcon0>;
allwinner,tcon-channel = <1>;This looks like a separate patch, probably following the binding change?
I don't know, the binding says that without anything specified, reg would be used. I was assuming that we only needed it once we had the new endpoint to make it consistent, therefore it didn't need an extra patch.
But I can definitely create one if you want.
It just seems a bit out of place, that's all. Mentioning it in the commit message would be good enough for me.
ChenYu
Maxime
-- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
The A10s Olinuxino has an HDMI connector. Make sure we can use it.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- arch/arm/boot/dts/sun5i-a10s-olinuxino-micro.dts | 12 ++++++++++++ 1 file changed, 12 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/sun5i-a10s-olinuxino-micro.dts b/arch/arm/boot/dts/sun5i-a10s-olinuxino-micro.dts index baee64d61f6d..3102c27b04df 100644 --- a/arch/arm/boot/dts/sun5i-a10s-olinuxino-micro.dts +++ b/arch/arm/boot/dts/sun5i-a10s-olinuxino-micro.dts @@ -77,6 +77,10 @@ }; };
+&be0 { + status = "okay"; +}; + &ehci0 { status = "okay"; }; @@ -92,6 +96,10 @@ status = "okay"; };
+&hdmi0 { + status = "okay"; +}; + &i2c0 { pinctrl-names = "default"; pinctrl-0 = <&i2c0_pins_a>; @@ -249,6 +257,10 @@ status = "okay"; };
+&tcon0 { + status = "okay"; +}; + &uart0 { pinctrl-names = "default"; pinctrl-0 = <&uart0_pins_a>;
On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The A10s Olinuxino has an HDMI connector. Make sure we can use it.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
Acked-by: Chen-Yu Tsai wens@csie.org
dri-devel@lists.freedesktop.org
-
Chen-Yu Tsai -
Julian Calaby -
Maxime Ripard -
Rob Herring -
Stefan Monnier -
Stephen Boyd