| Message ID | 20180519183127.2718-1-jernej.skrabec@siol.net |
|---|---|
| Headers | show |
| Series | Add support for R40 HDMI pipeline | expand |
Hi Jernej, On Sun, May 20, 2018 at 4:31 AM, Jernej Skrabec <jernej.skrabec@siol.net> wrote: > R40 display pipeline has a lot of possible configurations. HDMI can be > connected to 2 different TCONs (out of 4) and mixers can be connected to > any TCON. All this must be configured in TCON TOP. > > Along with definition of TCON capabilities also add mux callback, which > can configure this complex pipeline. > > For now, only TCON TV is supported. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > --- > drivers/gpu/drm/sun4i/sun4i_tcon.c | 39 ++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c > index e0c562ce1c22..81b9551e4f78 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -1274,6 +1274,31 @@ static int sun6i_tcon_set_mux(struct sun4i_tcon *tcon, > return 0; > } > > +static int sun8i_r40_tcon_tv_set_mux(struct sun4i_tcon *tcon, > + const struct drm_encoder *encoder, > + int index) > +{ > + if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS) > + sun8i_tcon_top_set_hdmi_src(tcon->tcon_top, index); > + > + sun8i_tcon_top_de_config(tcon->tcon_top, tcon->id, > + tcon_type_tv, index); > + > + return 0; > +} > + > +static int sun8i_r40_tcon_tv_set_mux_0(struct sun4i_tcon *tcon, > + const struct drm_encoder *encoder) > +{ > + return sun8i_r40_tcon_tv_set_mux(tcon, encoder, 0); > +} > + > +static int sun8i_r40_tcon_tv_set_mux_1(struct sun4i_tcon *tcon, > + const struct drm_encoder *encoder) > +{ > + return sun8i_r40_tcon_tv_set_mux(tcon, encoder, 1); > +} Are TCON-TOPs going to be a common thing in new SoCs from Allwinner? If so, maybe we should add an index to the TCON quirks and have a common TCON-TOP set_mux function. Thanks,
Hi Jernej, On Sun, May 20, 2018 at 11:57 AM, Julian Calaby <julian.calaby@gmail.com> wrote: > Hi Jernej, > > On Sun, May 20, 2018 at 4:31 AM, Jernej Skrabec <jernej.skrabec@siol.net> wrote: >> R40 display pipeline has a lot of possible configurations. HDMI can be >> connected to 2 different TCONs (out of 4) and mixers can be connected to >> any TCON. All this must be configured in TCON TOP. >> >> Along with definition of TCON capabilities also add mux callback, which >> can configure this complex pipeline. >> >> For now, only TCON TV is supported. >> >> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> >> --- >> drivers/gpu/drm/sun4i/sun4i_tcon.c | 39 ++++++++++++++++++++++++++++++ >> 1 file changed, 39 insertions(+) >> >> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> index e0c562ce1c22..81b9551e4f78 100644 >> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> @@ -1274,6 +1274,31 @@ static int sun6i_tcon_set_mux(struct sun4i_tcon *tcon, >> return 0; >> } >> >> +static int sun8i_r40_tcon_tv_set_mux(struct sun4i_tcon *tcon, >> + const struct drm_encoder *encoder, >> + int index) >> +{ >> + if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS) >> + sun8i_tcon_top_set_hdmi_src(tcon->tcon_top, index); >> + >> + sun8i_tcon_top_de_config(tcon->tcon_top, tcon->id, >> + tcon_type_tv, index); >> + >> + return 0; >> +} >> + >> +static int sun8i_r40_tcon_tv_set_mux_0(struct sun4i_tcon *tcon, >> + const struct drm_encoder *encoder) >> +{ >> + return sun8i_r40_tcon_tv_set_mux(tcon, encoder, 0); >> +} >> + >> +static int sun8i_r40_tcon_tv_set_mux_1(struct sun4i_tcon *tcon, >> + const struct drm_encoder *encoder) >> +{ >> + return sun8i_r40_tcon_tv_set_mux(tcon, encoder, 1); >> +} > > Are TCON-TOPs going to be a common thing in new SoCs from Allwinner? > If so, maybe we should add an index to the TCON quirks and have a > common TCON-TOP set_mux function. Actually, that only moves it up a level. Should it be a devicetree property? Thanks,
Hi, Dne nedelja, 20. maj 2018 ob 04:09:52 CEST je Julian Calaby napisal(a): > Hi Jernej, > > On Sun, May 20, 2018 at 11:57 AM, Julian Calaby <julian.calaby@gmail.com> wrote: > > Hi Jernej, > > > > On Sun, May 20, 2018 at 4:31 AM, Jernej Skrabec <jernej.skrabec@siol.net> wrote: > >> R40 display pipeline has a lot of possible configurations. HDMI can be > >> connected to 2 different TCONs (out of 4) and mixers can be connected to > >> any TCON. All this must be configured in TCON TOP. > >> > >> Along with definition of TCON capabilities also add mux callback, which > >> can configure this complex pipeline. > >> > >> For now, only TCON TV is supported. > >> > >> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > >> --- > >> > >> drivers/gpu/drm/sun4i/sun4i_tcon.c | 39 ++++++++++++++++++++++++++++++ > >> 1 file changed, 39 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> b/drivers/gpu/drm/sun4i/sun4i_tcon.c index e0c562ce1c22..81b9551e4f78 > >> 100644 > >> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> @@ -1274,6 +1274,31 @@ static int sun6i_tcon_set_mux(struct sun4i_tcon > >> *tcon,>> > >> return 0; > >> > >> } > >> > >> +static int sun8i_r40_tcon_tv_set_mux(struct sun4i_tcon *tcon, > >> + const struct drm_encoder *encoder, > >> + int index) > >> +{ > >> + if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS) > >> + sun8i_tcon_top_set_hdmi_src(tcon->tcon_top, index); > >> + > >> + sun8i_tcon_top_de_config(tcon->tcon_top, tcon->id, > >> + tcon_type_tv, index); > >> + > >> + return 0; > >> +} > >> + > >> +static int sun8i_r40_tcon_tv_set_mux_0(struct sun4i_tcon *tcon, > >> + const struct drm_encoder *encoder) > >> +{ > >> + return sun8i_r40_tcon_tv_set_mux(tcon, encoder, 0); > >> +} > >> + > >> +static int sun8i_r40_tcon_tv_set_mux_1(struct sun4i_tcon *tcon, > >> + const struct drm_encoder *encoder) > >> +{ > >> + return sun8i_r40_tcon_tv_set_mux(tcon, encoder, 1); > >> +} > > > > Are TCON-TOPs going to be a common thing in new SoCs from Allwinner? > > If so, maybe we should add an index to the TCON quirks and have a > > common TCON-TOP set_mux function. > > Actually, that only moves it up a level. Should it be a devicetree property? > TCON-TOP is besides R40 part of two newest Allwinner SoCs, H6 and A63. However, they have only one TV TCON and one LCD TCON, so indexes are not needed for them (always 0). This makes R40 somewhat special. I don't think it makes sense to expand everything just for one SoC. Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, May 19, 2018 at 08:31:18PM +0200, Jernej Skrabec wrote: > If SoC has TCON TOP unit, it has to be configured from TCON, since it > has all information needed. Additionally, if it is TCON TV, it must also > enable bus gate inside TCON TOP unit. Why? > Add support for such TCONs. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > --- > drivers/gpu/drm/sun4i/sun4i_tcon.c | 28 ++++++++++++++++++++++++++++ > drivers/gpu/drm/sun4i/sun4i_tcon.h | 8 ++++++++ > 2 files changed, 36 insertions(+) > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c > index 08747fc3ee71..e0c562ce1c22 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device *dev, > dev_err(dev, "Couldn't get the TCON bus clock\n"); > return PTR_ERR(tcon->clk); > } > + > + if (tcon->quirks->needs_tcon_top && tcon->quirks->has_channel_1) { > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); > + if (IS_ERR(tcon->top_clk)) { > + dev_err(dev, "Couldn't get the TCON TOP bus clock\n"); > + return PTR_ERR(tcon->top_clk); > + } > + clk_prepare_enable(tcon->top_clk); > + } > + > clk_prepare_enable(tcon->clk); > > if (tcon->quirks->has_channel_0) { > @@ -712,6 +722,7 @@ static int sun4i_tcon_init_clocks(struct device *dev, > static void sun4i_tcon_free_clocks(struct sun4i_tcon *tcon) > { > clk_disable_unprepare(tcon->clk); > + clk_disable_unprepare(tcon->top_clk); > } > > static int sun4i_tcon_init_irq(struct device *dev, > @@ -980,6 +991,23 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master, > tcon->id = engine->id; > tcon->quirks = of_device_get_match_data(dev); > > + if (tcon->quirks->needs_tcon_top) { > + struct device_node *np; > + > + np = of_parse_phandle(dev->of_node, "allwinner,tcon-top", 0); > + if (np) { > + struct platform_device *pdev; > + > + pdev = of_find_device_by_node(np); > + if (pdev) > + tcon->tcon_top = platform_get_drvdata(pdev); > + of_node_put(np); > + > + if (!tcon->tcon_top) > + return -EPROBE_DEFER; > + } > + } > + I might have missed it, but I've not seen the bindings additions for that property. This shouldn't really be done that way anyway, instead of using a direct phandle, you should be using the of-graph, with the TCON-top sitting where it belongs in the flow of data. Maxime
On Sat, May 19, 2018 at 08:31:24PM +0200, Jernej Skrabec wrote: > Expand HDMI PHY clock driver to support second clock parent. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > --- > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 6 +- > drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 29 ++++++- > drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c | 90 ++++++++++++++++------ > 3 files changed, 98 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > index 801a17222762..aadbe0a10b0c 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > @@ -98,7 +98,8 @@ > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO2_EN BIT(29) > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO1_EN BIT(28) > #define SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33 BIT(27) > -#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL BIT(26) > +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK BIT(26) > +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT 26 > #define SUN8I_HDMI_PHY_PLL_CFG1_PLLEN BIT(25) > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO_VSET(x) ((x) << 22) > #define SUN8I_HDMI_PHY_PLL_CFG1_UNKNOWN(x) ((x) << 20) > @@ -190,6 +191,7 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi *hdmi); > void sun8i_hdmi_phy_init(struct sun8i_hdmi_phy *phy); > const struct dw_hdmi_phy_ops *sun8i_hdmi_phy_get_ops(void); > > -int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev); > +int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev, > + bool second_parent); > > #endif /* _SUN8I_DW_HDMI_H_ */ > diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > index deba47ed69d8..7a911f0a3ae3 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > @@ -183,7 +183,13 @@ static int sun8i_hdmi_phy_config_h3(struct dw_hdmi *hdmi, > regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG, > SUN8I_HDMI_PHY_ANA_CFG1_TXEN_MASK, 0); > > - regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, pll_cfg1_init); > + /* > + * NOTE: We have to be careful not to overwrite PHY parent > + * clock selection bit and clock divider. > + */ > + regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, > + ~SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK, > + pll_cfg1_init); It feels like it belongs in a separate patch. > regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG, > (u32)~SUN8I_HDMI_PHY_PLL_CFG2_PREDIV_MSK, > pll_cfg2_init); > @@ -352,6 +358,10 @@ static void sun8i_hdmi_phy_init_h3(struct sun8i_hdmi_phy *phy) > SUN8I_HDMI_PHY_ANA_CFG3_SCLEN | > SUN8I_HDMI_PHY_ANA_CFG3_SDAEN); > > + /* reset PLL clock configuration */ > + regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, 0); > + regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG, 0); > + Ditto, > + > + /* > + * Even though HDMI PHY clock doesn't have enable/disable > + * handlers, we have to enable it. Otherwise it could happen > + * that parent PLL is not enabled by clock framework in a > + * highly unlikely event when parent PLL is used solely for > + * HDMI PHY clock. > + */ > + clk_prepare_enable(phy->clk_phy); The implementation of the clock doesn't really matter in our API usage. If we're using a clock, we have to call clk_prepare_enable. That's documented everywhere, and mentionning how the clock is implemented is an abstraction leakage and it's irrelevant. I'd simply remove the comment here. And it should be in a separate patch as well. Maxime
Hi, Dne ponedeljek, 21. maj 2018 ob 10:12:53 CEST je Maxime Ripard napisal(a): > On Sat, May 19, 2018 at 08:31:24PM +0200, Jernej Skrabec wrote: > > Expand HDMI PHY clock driver to support second clock parent. > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > --- > > > > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 6 +- > > drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 29 ++++++- > > drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c | 90 ++++++++++++++++------ > > 3 files changed, 98 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > > b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 801a17222762..aadbe0a10b0c > > 100644 > > --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > > +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > > @@ -98,7 +98,8 @@ > > > > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO2_EN BIT(29) > > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO1_EN BIT(28) > > #define SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33 BIT(27) > > > > -#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL BIT(26) > > +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK BIT(26) > > +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT 26 > > > > #define SUN8I_HDMI_PHY_PLL_CFG1_PLLEN BIT(25) > > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO_VSET(x) ((x) << 22) > > #define SUN8I_HDMI_PHY_PLL_CFG1_UNKNOWN(x) ((x) << 20) > > > > @@ -190,6 +191,7 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi > > *hdmi); > > > > void sun8i_hdmi_phy_init(struct sun8i_hdmi_phy *phy); > > const struct dw_hdmi_phy_ops *sun8i_hdmi_phy_get_ops(void); > > > > -int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev); > > +int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev, > > + bool second_parent); > > > > #endif /* _SUN8I_DW_HDMI_H_ */ > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > > b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index deba47ed69d8..7a911f0a3ae3 > > 100644 > > --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > > +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > > @@ -183,7 +183,13 @@ static int sun8i_hdmi_phy_config_h3(struct dw_hdmi > > *hdmi,> > > regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG, > > > > SUN8I_HDMI_PHY_ANA_CFG1_TXEN_MASK, 0); > > > > - regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, pll_cfg1_init); > > + /* > > + * NOTE: We have to be careful not to overwrite PHY parent > > + * clock selection bit and clock divider. > > + */ > > + regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, > > + ~SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK, > > + pll_cfg1_init); > > It feels like it belongs in a separate patch. Why? clk_set_rate() on HDMI PHY clock is called before this function, so SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL bit is already set to correct value. Original code doesn't take this into account and sets it to 0 here in all cases, which obviously is not right. If you insist on splitting this in new patch, it has to be applied before clock changes. It would also need to include "reset PLL clock configuration" chunk (next change in this patch), otherwise other SoCs with only one parent could get 1 there, which is obviously wrong. Note that I didn't really tested if default value of this bit is 0 or 1, but I wouldn't really like to depend on default values. > > > regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG, > > > > (u32)~SUN8I_HDMI_PHY_PLL_CFG2_PREDIV_MSK, > > pll_cfg2_init); > > > > @@ -352,6 +358,10 @@ static void sun8i_hdmi_phy_init_h3(struct > > sun8i_hdmi_phy *phy)> > > SUN8I_HDMI_PHY_ANA_CFG3_SCLEN | > > SUN8I_HDMI_PHY_ANA_CFG3_SDAEN); > > > > + /* reset PLL clock configuration */ > > + regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, 0); > > + regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG, 0); > > + > > Ditto, > > > + > > + /* > > + * Even though HDMI PHY clock doesn't have enable/disable > > + * handlers, we have to enable it. Otherwise it could happen > > + * that parent PLL is not enabled by clock framework in a > > + * highly unlikely event when parent PLL is used solely for > > + * HDMI PHY clock. > > + */ > > + clk_prepare_enable(phy->clk_phy); > > The implementation of the clock doesn't really matter in our API > usage. If we're using a clock, we have to call > clk_prepare_enable. That's documented everywhere, and mentionning how > the clock is implemented is an abstraction leakage and it's > irrelevant. I'd simply remove the comment here. > > And it should be in a separate patch as well. OK. Should I add Fixes tag, since current code obviously is not by the specs? It could still get in 4.17... Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Dne ponedeljek, 21. maj 2018 ob 10:07:59 CEST je Maxime Ripard napisal(a): > On Sat, May 19, 2018 at 08:31:18PM +0200, Jernej Skrabec wrote: > > If SoC has TCON TOP unit, it has to be configured from TCON, since it > > has all information needed. Additionally, if it is TCON TV, it must also > > enable bus gate inside TCON TOP unit. > > Why? I'll explain my design decision below. > > > Add support for such TCONs. > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > --- > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 28 ++++++++++++++++++++++++++++ > > drivers/gpu/drm/sun4i/sun4i_tcon.h | 8 ++++++++ > > 2 files changed, 36 insertions(+) > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..e0c562ce1c22 > > 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device *dev, > > > > dev_err(dev, "Couldn't get the TCON bus clock\n"); > > return PTR_ERR(tcon->clk); > > > > } > > > > + > > + if (tcon->quirks->needs_tcon_top && tcon->quirks->has_channel_1) { > > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); > > + if (IS_ERR(tcon->top_clk)) { > > + dev_err(dev, "Couldn't get the TCON TOP bus clock\n"); > > + return PTR_ERR(tcon->top_clk); > > + } > > + clk_prepare_enable(tcon->top_clk); > > + } > > + > > > > clk_prepare_enable(tcon->clk); > > > > if (tcon->quirks->has_channel_0) { > > > > @@ -712,6 +722,7 @@ static int sun4i_tcon_init_clocks(struct device *dev, > > > > static void sun4i_tcon_free_clocks(struct sun4i_tcon *tcon) > > { > > > > clk_disable_unprepare(tcon->clk); > > > > + clk_disable_unprepare(tcon->top_clk); > > > > } > > > > static int sun4i_tcon_init_irq(struct device *dev, > > > > @@ -980,6 +991,23 @@ static int sun4i_tcon_bind(struct device *dev, struct > > device *master,> > > tcon->id = engine->id; > > tcon->quirks = of_device_get_match_data(dev); > > > > + if (tcon->quirks->needs_tcon_top) { > > + struct device_node *np; > > + > > + np = of_parse_phandle(dev->of_node, "allwinner,tcon-top", 0); > > + if (np) { > > + struct platform_device *pdev; > > + > > + pdev = of_find_device_by_node(np); > > + if (pdev) > > + tcon->tcon_top = platform_get_drvdata(pdev); > > + of_node_put(np); > > + > > + if (!tcon->tcon_top) > > + return -EPROBE_DEFER; > > + } > > + } > > + > > I might have missed it, but I've not seen the bindings additions for > that property. This shouldn't really be done that way anyway, instead > of using a direct phandle, you should be using the of-graph, with the > TCON-top sitting where it belongs in the flow of data. Just to answer to the first question, it did describe it in "[PATCH 07/15] dt- bindings: display: sun4i-drm: Add R40 HDMI pipeline". As why I designed it that way - HW representation could be described that way (ASCII art makes sense when fixed width font is used to view it): / LCD0/LVDS0 / TCON-LCD0 | \ MIPI DSI mixer0 | \ / TCON-LCD1 - LCD1/LVDS1 TCON-TOP / \ TCON-TV0 - TVE0/RGB mixer1 | \ | TCON-TOP - HDMI | / \ TCON-TV1 - TVE1/RGB This is a bit simplified, since there is also TVE-TOP, which is responsible for sharing 4 DACs between both TVE encoders. You can have two TV outs (PAL/ NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It even seems that you can arbitrarly choose which DAC is responsible for which signal, so there is a ton of possible end combinations, but I'm not 100% sure. Even though I wrote TCON-TOP twice, this is same unit in HW. R40 manual suggest more possibilities, although some of them seem wrong, like RGB feeding from LCD TCON. That is confirmed to be wrong when checking BSP code. Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, TVE1 and LCD1 for pin muxing, although I'm not sure why is that needed at all, since according to R40 datasheet, TVE0 and TVE1 pins are dedicated and not on PORT D and PORT H, respectively, as TCON-TOP documentation suggest. However, HSYNC and PSYNC lines might be shared between TVE (when it works in RGB mode) and LCD. But that is just my guess since I'm not really familiar with RGB and LCD interfaces. I'm really not sure what would be the best representation in OF-graph. Can you suggest one? On the other hand, mux callback in TCON driver has all available informations at hand. It knows mixer ID, TCON ID and most importantly, encoder type. Based on all that informations, it's easy to configure TCON TOP. I hope you understand. If you have better idea, I'm all ears, since phandle seems a bit weird to me too, but I think it's the only future proof, when adding LVDS, RGB, TVE or LCD support. Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 21, 2018 at 05:02:23PM +0200, Jernej Škrabec wrote: > Hi, > > Dne ponedeljek, 21. maj 2018 ob 10:12:53 CEST je Maxime Ripard napisal(a): > > On Sat, May 19, 2018 at 08:31:24PM +0200, Jernej Skrabec wrote: > > > Expand HDMI PHY clock driver to support second clock parent. > > > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > > --- > > > > > > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 6 +- > > > drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 29 ++++++- > > > drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c | 90 ++++++++++++++++------ > > > 3 files changed, 98 insertions(+), 27 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > > > b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 801a17222762..aadbe0a10b0c > > > 100644 > > > --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > > > +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > > > @@ -98,7 +98,8 @@ > > > > > > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO2_EN BIT(29) > > > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO1_EN BIT(28) > > > #define SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33 BIT(27) > > > > > > -#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL BIT(26) > > > +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK BIT(26) > > > +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT 26 > > > > > > #define SUN8I_HDMI_PHY_PLL_CFG1_PLLEN BIT(25) > > > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO_VSET(x) ((x) << 22) > > > #define SUN8I_HDMI_PHY_PLL_CFG1_UNKNOWN(x) ((x) << 20) > > > > > > @@ -190,6 +191,7 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi > > > *hdmi); > > > > > > void sun8i_hdmi_phy_init(struct sun8i_hdmi_phy *phy); > > > const struct dw_hdmi_phy_ops *sun8i_hdmi_phy_get_ops(void); > > > > > > -int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev); > > > +int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev, > > > + bool second_parent); > > > > > > #endif /* _SUN8I_DW_HDMI_H_ */ > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > > > b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index deba47ed69d8..7a911f0a3ae3 > > > 100644 > > > --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > > > +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > > > @@ -183,7 +183,13 @@ static int sun8i_hdmi_phy_config_h3(struct dw_hdmi > > > *hdmi,> > > > regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG, > > > > > > SUN8I_HDMI_PHY_ANA_CFG1_TXEN_MASK, 0); > > > > > > - regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, pll_cfg1_init); > > > + /* > > > + * NOTE: We have to be careful not to overwrite PHY parent > > > + * clock selection bit and clock divider. > > > + */ > > > + regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, > > > + ~SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK, > > > + pll_cfg1_init); > > > > It feels like it belongs in a separate patch. > > Why? clk_set_rate() on HDMI PHY clock is called before this function, so > SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL bit is already set to correct value. Original > code doesn't take this into account and sets it to 0 here in all cases, which > obviously is not right. > > If you insist on splitting this in new patch, it has to be applied before > clock changes. It would also need to include "reset PLL clock configuration" > chunk (next change in this patch), otherwise other SoCs with only one parent > could get 1 there, which is obviously wrong. Note that I didn't really tested > if default value of this bit is 0 or 1, but I wouldn't really like to depend > on default values. I don't have a strong feeling about this, but to me, the fact that you don't want to overwrite the muxing bit because the clock might use it is sensible in itself, disregarding the fact that the clock *will* use it. > > > > > > regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG, > > > > > > (u32)~SUN8I_HDMI_PHY_PLL_CFG2_PREDIV_MSK, > > > pll_cfg2_init); > > > > > > @@ -352,6 +358,10 @@ static void sun8i_hdmi_phy_init_h3(struct > > > sun8i_hdmi_phy *phy)> > > > SUN8I_HDMI_PHY_ANA_CFG3_SCLEN | > > > SUN8I_HDMI_PHY_ANA_CFG3_SDAEN); > > > > > > + /* reset PLL clock configuration */ > > > + regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, 0); > > > + regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG, 0); > > > + > > > > Ditto, > > > > > + > > > + /* > > > + * Even though HDMI PHY clock doesn't have enable/disable > > > + * handlers, we have to enable it. Otherwise it could happen > > > + * that parent PLL is not enabled by clock framework in a > > > + * highly unlikely event when parent PLL is used solely for > > > + * HDMI PHY clock. > > > + */ > > > + clk_prepare_enable(phy->clk_phy); > > > > The implementation of the clock doesn't really matter in our API > > usage. If we're using a clock, we have to call > > clk_prepare_enable. That's documented everywhere, and mentionning how > > the clock is implemented is an abstraction leakage and it's > > irrelevant. I'd simply remove the comment here. > > > > And it should be in a separate patch as well. > > OK. Should I add Fixes tag, since current code obviously is not by the specs? > It could still get in 4.17... Yep! Maxime
On Mon, May 21, 2018 at 07:27:46PM +0200, Jernej Škrabec wrote: > Hi, > > Dne ponedeljek, 21. maj 2018 ob 10:07:59 CEST je Maxime Ripard napisal(a): > > On Sat, May 19, 2018 at 08:31:18PM +0200, Jernej Skrabec wrote: > > > If SoC has TCON TOP unit, it has to be configured from TCON, since it > > > has all information needed. Additionally, if it is TCON TV, it must also > > > enable bus gate inside TCON TOP unit. > > > > Why? > > I'll explain my design decision below. > > > > > > Add support for such TCONs. > > > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > > --- > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 28 ++++++++++++++++++++++++++++ > > > drivers/gpu/drm/sun4i/sun4i_tcon.h | 8 ++++++++ > > > 2 files changed, 36 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..e0c562ce1c22 > > > 100644 > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device *dev, > > > > > > dev_err(dev, "Couldn't get the TCON bus clock\n"); > > > return PTR_ERR(tcon->clk); > > > > > > } > > > > > > + > > > + if (tcon->quirks->needs_tcon_top && tcon->quirks->has_channel_1) { > > > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); > > > + if (IS_ERR(tcon->top_clk)) { > > > + dev_err(dev, "Couldn't get the TCON TOP bus clock\n"); > > > + return PTR_ERR(tcon->top_clk); > > > + } > > > + clk_prepare_enable(tcon->top_clk); > > > + } > > > + > > > > > > clk_prepare_enable(tcon->clk); > > > > > > if (tcon->quirks->has_channel_0) { > > > > > > @@ -712,6 +722,7 @@ static int sun4i_tcon_init_clocks(struct device *dev, > > > > > > static void sun4i_tcon_free_clocks(struct sun4i_tcon *tcon) > > > { > > > > > > clk_disable_unprepare(tcon->clk); > > > > > > + clk_disable_unprepare(tcon->top_clk); > > > > > > } > > > > > > static int sun4i_tcon_init_irq(struct device *dev, > > > > > > @@ -980,6 +991,23 @@ static int sun4i_tcon_bind(struct device *dev, struct > > > device *master,> > > > tcon->id = engine->id; > > > tcon->quirks = of_device_get_match_data(dev); > > > > > > + if (tcon->quirks->needs_tcon_top) { > > > + struct device_node *np; > > > + > > > + np = of_parse_phandle(dev->of_node, "allwinner,tcon-top", 0); > > > + if (np) { > > > + struct platform_device *pdev; > > > + > > > + pdev = of_find_device_by_node(np); > > > + if (pdev) > > > + tcon->tcon_top = platform_get_drvdata(pdev); > > > + of_node_put(np); > > > + > > > + if (!tcon->tcon_top) > > > + return -EPROBE_DEFER; > > > + } > > > + } > > > + > > > > I might have missed it, but I've not seen the bindings additions for > > that property. This shouldn't really be done that way anyway, instead > > of using a direct phandle, you should be using the of-graph, with the > > TCON-top sitting where it belongs in the flow of data. > > Just to answer to the first question, it did describe it in "[PATCH 07/15] dt- > bindings: display: sun4i-drm: Add R40 HDMI pipeline". > > As why I designed it that way - HW representation could be described that way > (ASCII art makes sense when fixed width font is used to view it): > > / LCD0/LVDS0 > / TCON-LCD0 > | \ MIPI DSI > mixer0 | > \ / TCON-LCD1 - LCD1/LVDS1 > TCON-TOP > / \ TCON-TV0 - TVE0/RGB > mixer1 | \ > | TCON-TOP - HDMI > | / > \ TCON-TV1 - TVE1/RGB > > This is a bit simplified, since there is also TVE-TOP, which is responsible > for sharing 4 DACs between both TVE encoders. You can have two TV outs (PAL/ > NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It even seems that you > can arbitrarly choose which DAC is responsible for which signal, so there is a > ton of possible end combinations, but I'm not 100% sure. > > Even though I wrote TCON-TOP twice, this is same unit in HW. R40 manual > suggest more possibilities, although some of them seem wrong, like RGB feeding > from LCD TCON. That is confirmed to be wrong when checking BSP code. > > Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, TVE1 and LCD1 for > pin muxing, although I'm not sure why is that needed at all, since according > to R40 datasheet, TVE0 and TVE1 pins are dedicated and not on PORT D and PORT > H, respectively, as TCON-TOP documentation suggest. However, HSYNC and PSYNC > lines might be shared between TVE (when it works in RGB mode) and LCD. But > that is just my guess since I'm not really familiar with RGB and LCD > interfaces. > > I'm really not sure what would be the best representation in OF-graph. Can you > suggest one? Rob might disagree on this one, but I don't see anything wrong with having loops in the graph. If the TCON-TOP can be both the input and output of the TCONs, then so be it, and have it described that way in the graph. The code is already able to filter out nodes that have already been added to the list of devices we need to wait for in the component framework, so that should work as well. And we'd need to describe TVE-TOP as well, even though we don't have a driver for it yet. That will simplify the backward compatibility later on. Maxime
On Thu, May 24, 2018 at 1:50 AM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > On Mon, May 21, 2018 at 07:27:46PM +0200, Jernej Škrabec wrote: >> Hi, >> >> Dne ponedeljek, 21. maj 2018 ob 10:07:59 CEST je Maxime Ripard napisal(a): >> > On Sat, May 19, 2018 at 08:31:18PM +0200, Jernej Skrabec wrote: >> > > If SoC has TCON TOP unit, it has to be configured from TCON, since it >> > > has all information needed. Additionally, if it is TCON TV, it must also >> > > enable bus gate inside TCON TOP unit. >> > >> > Why? >> >> I'll explain my design decision below. >> >> > >> > > Add support for such TCONs. >> > > >> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> >> > > --- >> > > >> > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 28 ++++++++++++++++++++++++++++ >> > > drivers/gpu/drm/sun4i/sun4i_tcon.h | 8 ++++++++ >> > > 2 files changed, 36 insertions(+) >> > > >> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..e0c562ce1c22 >> > > 100644 >> > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> > > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device *dev, >> > > >> > > dev_err(dev, "Couldn't get the TCON bus clock\n"); >> > > return PTR_ERR(tcon->clk); >> > > >> > > } >> > > >> > > + >> > > + if (tcon->quirks->needs_tcon_top && tcon->quirks->has_channel_1) { >> > > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); >> > > + if (IS_ERR(tcon->top_clk)) { >> > > + dev_err(dev, "Couldn't get the TCON TOP bus clock\n"); >> > > + return PTR_ERR(tcon->top_clk); >> > > + } >> > > + clk_prepare_enable(tcon->top_clk); >> > > + } >> > > + >> > > >> > > clk_prepare_enable(tcon->clk); >> > > >> > > if (tcon->quirks->has_channel_0) { >> > > >> > > @@ -712,6 +722,7 @@ static int sun4i_tcon_init_clocks(struct device *dev, >> > > >> > > static void sun4i_tcon_free_clocks(struct sun4i_tcon *tcon) >> > > { >> > > >> > > clk_disable_unprepare(tcon->clk); >> > > >> > > + clk_disable_unprepare(tcon->top_clk); >> > > >> > > } >> > > >> > > static int sun4i_tcon_init_irq(struct device *dev, >> > > >> > > @@ -980,6 +991,23 @@ static int sun4i_tcon_bind(struct device *dev, struct >> > > device *master,> >> > > tcon->id = engine->id; >> > > tcon->quirks = of_device_get_match_data(dev); >> > > >> > > + if (tcon->quirks->needs_tcon_top) { >> > > + struct device_node *np; >> > > + >> > > + np = of_parse_phandle(dev->of_node, "allwinner,tcon-top", 0); >> > > + if (np) { >> > > + struct platform_device *pdev; >> > > + >> > > + pdev = of_find_device_by_node(np); >> > > + if (pdev) >> > > + tcon->tcon_top = platform_get_drvdata(pdev); >> > > + of_node_put(np); >> > > + >> > > + if (!tcon->tcon_top) >> > > + return -EPROBE_DEFER; >> > > + } >> > > + } >> > > + >> > >> > I might have missed it, but I've not seen the bindings additions for >> > that property. This shouldn't really be done that way anyway, instead >> > of using a direct phandle, you should be using the of-graph, with the >> > TCON-top sitting where it belongs in the flow of data. >> >> Just to answer to the first question, it did describe it in "[PATCH 07/15] dt- >> bindings: display: sun4i-drm: Add R40 HDMI pipeline". >> >> As why I designed it that way - HW representation could be described that way >> (ASCII art makes sense when fixed width font is used to view it): >> >> / LCD0/LVDS0 >> / TCON-LCD0 >> | \ MIPI DSI >> mixer0 | >> \ / TCON-LCD1 - LCD1/LVDS1 >> TCON-TOP >> / \ TCON-TV0 - TVE0/RGB >> mixer1 | \ >> | TCON-TOP - HDMI >> | / >> \ TCON-TV1 - TVE1/RGB >> >> This is a bit simplified, since there is also TVE-TOP, which is responsible >> for sharing 4 DACs between both TVE encoders. You can have two TV outs (PAL/ >> NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It even seems that you >> can arbitrarly choose which DAC is responsible for which signal, so there is a >> ton of possible end combinations, but I'm not 100% sure. >> >> Even though I wrote TCON-TOP twice, this is same unit in HW. R40 manual >> suggest more possibilities, although some of them seem wrong, like RGB feeding >> from LCD TCON. That is confirmed to be wrong when checking BSP code. >> >> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, TVE1 and LCD1 for >> pin muxing, although I'm not sure why is that needed at all, since according >> to R40 datasheet, TVE0 and TVE1 pins are dedicated and not on PORT D and PORT >> H, respectively, as TCON-TOP documentation suggest. However, HSYNC and PSYNC >> lines might be shared between TVE (when it works in RGB mode) and LCD. But >> that is just my guess since I'm not really familiar with RGB and LCD >> interfaces. >> >> I'm really not sure what would be the best representation in OF-graph. Can you >> suggest one? > > Rob might disagree on this one, but I don't see anything wrong with > having loops in the graph. If the TCON-TOP can be both the input and > output of the TCONs, then so be it, and have it described that way in > the graph. > > The code is already able to filter out nodes that have already been > added to the list of devices we need to wait for in the component > framework, so that should work as well. > > And we'd need to describe TVE-TOP as well, even though we don't have a > driver for it yet. That will simplify the backward compatibility later > on. I'm getting the feeling that TCON-TOP / TVE-TOP is the glue layer that binds everything together, and provides signal routing, kind of like DE-TOP on A64. So the signal mux controls that were originally found in TCON0 and TVE0 were moved out. The driver needs to know about that, but the graph about doesn't make much sense directly. Without looking at the manual, I understand it to likely be one mux between the mixers and TCONs, and one between the TCON-TVs and HDMI. Would it make more sense to just have the graph connections between the muxed components, and remove TCON-TOP from it, like we had in the past? A phandle could be used to reference the TCON-TOP for mux controls, in addition to the clocks and resets. For TVE, we would need something to represent each of the output pins, so the device tree can actually describe what kind of signal, be it each component of RGB/YUV or composite video, is wanted on each pin, if any. This is also needed on the A20 for the Cubietruck, so we can describe which pins are tied to the VGA connector, and which one does R, G, or B. Regards ChenYu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 24, 2018 at 03:01:09PM -0700, Chen-Yu Tsai wrote: > >> > > + if (tcon->quirks->needs_tcon_top) { > >> > > + struct device_node *np; > >> > > + > >> > > + np = of_parse_phandle(dev->of_node, "allwinner,tcon-top", 0); > >> > > + if (np) { > >> > > + struct platform_device *pdev; > >> > > + > >> > > + pdev = of_find_device_by_node(np); > >> > > + if (pdev) > >> > > + tcon->tcon_top = platform_get_drvdata(pdev); > >> > > + of_node_put(np); > >> > > + > >> > > + if (!tcon->tcon_top) > >> > > + return -EPROBE_DEFER; > >> > > + } > >> > > + } > >> > > + > >> > > >> > I might have missed it, but I've not seen the bindings additions for > >> > that property. This shouldn't really be done that way anyway, instead > >> > of using a direct phandle, you should be using the of-graph, with the > >> > TCON-top sitting where it belongs in the flow of data. > >> > >> Just to answer to the first question, it did describe it in "[PATCH 07/15] dt- > >> bindings: display: sun4i-drm: Add R40 HDMI pipeline". > >> > >> As why I designed it that way - HW representation could be described that way > >> (ASCII art makes sense when fixed width font is used to view it): > >> > >> / LCD0/LVDS0 > >> / TCON-LCD0 > >> | \ MIPI DSI > >> mixer0 | > >> \ / TCON-LCD1 - LCD1/LVDS1 > >> TCON-TOP > >> / \ TCON-TV0 - TVE0/RGB > >> mixer1 | \ > >> | TCON-TOP - HDMI > >> | / > >> \ TCON-TV1 - TVE1/RGB > >> > >> This is a bit simplified, since there is also TVE-TOP, which is responsible > >> for sharing 4 DACs between both TVE encoders. You can have two TV outs (PAL/ > >> NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It even seems that you > >> can arbitrarly choose which DAC is responsible for which signal, so there is a > >> ton of possible end combinations, but I'm not 100% sure. > >> > >> Even though I wrote TCON-TOP twice, this is same unit in HW. R40 manual > >> suggest more possibilities, although some of them seem wrong, like RGB feeding > >> from LCD TCON. That is confirmed to be wrong when checking BSP code. > >> > >> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, TVE1 and LCD1 for > >> pin muxing, although I'm not sure why is that needed at all, since according > >> to R40 datasheet, TVE0 and TVE1 pins are dedicated and not on PORT D and PORT > >> H, respectively, as TCON-TOP documentation suggest. However, HSYNC and PSYNC > >> lines might be shared between TVE (when it works in RGB mode) and LCD. But > >> that is just my guess since I'm not really familiar with RGB and LCD > >> interfaces. > >> > >> I'm really not sure what would be the best representation in OF-graph. Can you > >> suggest one? > > > > Rob might disagree on this one, but I don't see anything wrong with > > having loops in the graph. If the TCON-TOP can be both the input and > > output of the TCONs, then so be it, and have it described that way in > > the graph. > > > > The code is already able to filter out nodes that have already been > > added to the list of devices we need to wait for in the component > > framework, so that should work as well. > > > > And we'd need to describe TVE-TOP as well, even though we don't have a > > driver for it yet. That will simplify the backward compatibility later > > on. > > I'm getting the feeling that TCON-TOP / TVE-TOP is the glue layer that > binds everything together, and provides signal routing, kind of like > DE-TOP on A64. So the signal mux controls that were originally found > in TCON0 and TVE0 were moved out. > > The driver needs to know about that, but the graph about doesn't make > much sense directly. Without looking at the manual, I understand it to > likely be one mux between the mixers and TCONs, and one between the > TCON-TVs and HDMI. Would it make more sense to just have the graph > connections between the muxed components, and remove TCON-TOP from > it, like we had in the past? A phandle could be used to reference > the TCON-TOP for mux controls, in addition to the clocks and resets. > > For TVE, we would need something to represent each of the output pins, > so the device tree can actually describe what kind of signal, be it > each component of RGB/YUV or composite video, is wanted on each pin, > if any. This is also needed on the A20 for the Cubietruck, so we can > describe which pins are tied to the VGA connector, and which one does > R, G, or B. I guess we'll see how the DT maintainers feel about this, but my impression is that the OF graph should model the flow of data between the devices. If there's a mux somewhere, then the data is definitely going through it, and as such it should be part of the graph. Maxime
Dne četrtek, 31. maj 2018 ob 11:21:33 CEST je Maxime Ripard napisal(a): > On Thu, May 24, 2018 at 03:01:09PM -0700, Chen-Yu Tsai wrote: > > >> > > + if (tcon->quirks->needs_tcon_top) { > > >> > > + struct device_node *np; > > >> > > + > > >> > > + np = of_parse_phandle(dev->of_node, "allwinner,tcon-top", > > >> > > 0); > > >> > > + if (np) { > > >> > > + struct platform_device *pdev; > > >> > > + > > >> > > + pdev = of_find_device_by_node(np); > > >> > > + if (pdev) > > >> > > + tcon->tcon_top = > > >> > > platform_get_drvdata(pdev); > > >> > > + of_node_put(np); > > >> > > + > > >> > > + if (!tcon->tcon_top) > > >> > > + return -EPROBE_DEFER; > > >> > > + } > > >> > > + } > > >> > > + > > >> > > > >> > I might have missed it, but I've not seen the bindings additions for > > >> > that property. This shouldn't really be done that way anyway, instead > > >> > of using a direct phandle, you should be using the of-graph, with the > > >> > TCON-top sitting where it belongs in the flow of data. > > >> > > >> Just to answer to the first question, it did describe it in "[PATCH > > >> 07/15] dt- bindings: display: sun4i-drm: Add R40 HDMI pipeline". > > >> > > >> As why I designed it that way - HW representation could be described > > >> that way> >> > > >> (ASCII art makes sense when fixed width font is used to view it): > > >> / LCD0/LVDS0 > > >> > > >> / TCON-LCD0 > > >> > > >> | \ MIPI DSI > > >> > > >> mixer0 | > > >> > > >> \ / TCON-LCD1 - LCD1/LVDS1 > > >> > > >> TCON-TOP > > >> > > >> / \ TCON-TV0 - TVE0/RGB > > >> > > >> mixer1 | \ > > >> > > >> | TCON-TOP - HDMI > > >> | > > >> | / > > >> > > >> \ TCON-TV1 - TVE1/RGB > > >> > > >> This is a bit simplified, since there is also TVE-TOP, which is > > >> responsible > > >> for sharing 4 DACs between both TVE encoders. You can have two TV outs > > >> (PAL/ NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It even > > >> seems that you can arbitrarly choose which DAC is responsible for > > >> which signal, so there is a ton of possible end combinations, but I'm > > >> not 100% sure. > > >> > > >> Even though I wrote TCON-TOP twice, this is same unit in HW. R40 manual > > >> suggest more possibilities, although some of them seem wrong, like RGB > > >> feeding from LCD TCON. That is confirmed to be wrong when checking BSP > > >> code. > > >> > > >> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, TVE1 and > > >> LCD1 for pin muxing, although I'm not sure why is that needed at all, > > >> since according to R40 datasheet, TVE0 and TVE1 pins are dedicated and > > >> not on PORT D and PORT H, respectively, as TCON-TOP documentation > > >> suggest. However, HSYNC and PSYNC lines might be shared between TVE > > >> (when it works in RGB mode) and LCD. But that is just my guess since > > >> I'm not really familiar with RGB and LCD interfaces. > > >> > > >> I'm really not sure what would be the best representation in OF-graph. > > >> Can you suggest one? > > > > > > Rob might disagree on this one, but I don't see anything wrong with > > > having loops in the graph. If the TCON-TOP can be both the input and > > > output of the TCONs, then so be it, and have it described that way in > > > the graph. > > > > > > The code is already able to filter out nodes that have already been > > > added to the list of devices we need to wait for in the component > > > framework, so that should work as well. > > > > > > And we'd need to describe TVE-TOP as well, even though we don't have a > > > driver for it yet. That will simplify the backward compatibility later > > > on. > > > > I'm getting the feeling that TCON-TOP / TVE-TOP is the glue layer that > > binds everything together, and provides signal routing, kind of like > > DE-TOP on A64. So the signal mux controls that were originally found > > in TCON0 and TVE0 were moved out. > > > > The driver needs to know about that, but the graph about doesn't make > > much sense directly. Without looking at the manual, I understand it to > > likely be one mux between the mixers and TCONs, and one between the > > TCON-TVs and HDMI. Would it make more sense to just have the graph > > connections between the muxed components, and remove TCON-TOP from > > it, like we had in the past? A phandle could be used to reference > > the TCON-TOP for mux controls, in addition to the clocks and resets. > > > > For TVE, we would need something to represent each of the output pins, > > so the device tree can actually describe what kind of signal, be it > > each component of RGB/YUV or composite video, is wanted on each pin, > > if any. This is also needed on the A20 for the Cubietruck, so we can > > describe which pins are tied to the VGA connector, and which one does > > R, G, or B. > > I guess we'll see how the DT maintainers feel about this, but my > impression is that the OF graph should model the flow of data between > the devices. If there's a mux somewhere, then the data is definitely > going through it, and as such it should be part of the graph. I concur, but I spent few days thinking how to represent this sanely in graph, but I didn't find any good solution. I'll represent here my idea and please tell your opinion before I start implementing it. First, let me be clear that mixer0 and mixer1 don't have same capabilities (different number of planes, mixer0 supports writeback, mixer1 does not, etc.). Thus, it does matter which mixer is connected to which TCON/encoder. mixer0 is meant to be connected to main display and mixer1 to auxiliary. That obviously depends on end system. So, TCON TOP has 3 muxes, which have to be represented in graph. Two of them are for mixer/TCON relationship (each of them 1 input and 4 possible outputs) and one for TV TCON/HDMI pair selection (2 possible inputs, 1 output). According to current practice in sun4i-drm driver, graph has to have port 0, representing input and port 1, representing output. This would mean that graph looks something like that: tcon_top: tcon-top@1c70000 { compatible = "allwinner,sun8i-r40-tcon-top"; ... ports { #address-cells = <1>; #size-cells = <0>; tcon_top_in: port@0 { #address-cells = <1>; #size-cells = <0>; reg = <0>; tcon_top_in_mixer0: endpoint@0 { reg = <0>; remote-endpoint = <&mixer0_out_tcon_top>; }; tcon_top_in_mixer1: endpoint@1 { reg = <1>; remote-endpoint = <&mixer1_out_tcon_top>; }; tcon_top_in_tcon_tv: endpoint@2 { reg = <2>; // here is HDMI input connection, part of board DTS remote-endpoint = <board specific phandle to TV TCON output>; }; }; tcon_top_out: port@1 { #address-cells = <1>; #size-cells = <0>; reg = <1>; tcon_top_out_tcon0: endpoint@0 { reg = <0>; // here is mixer0 output connection, part of board DTS remote-endpoint = <board specific phandle to TCON>; }; tcon_top_out_tcon1: endpoint@1 { reg = <1>; // here is mixer1 output connection, part of board DTS remote-endpoint = <board specific phandle to TCON>; }; tcon_top_out_hdmi: endpoint@2 { reg = <2>; remote-endpoint = <&hdmi_in_tcon_top>; }; }; }; }; tcon_tv0: lcd-controller@1c73000 { compatible = "allwinner,sun8i-r40-tcon-tv-0"; ... ports { #address-cells = <1>; #size-cells = <0>; tcon_tv0_in: port@0 { reg = <0>; tcon_tv0_in_tcon_top: endpoint { // endpoint depends on board, part of board DTS remote-endpoint = <phandle to one of tcon_top_out_tcon>; }; }; tcon_tv0_out: port@1 { #address-cells = <1>; #size-cells = <0>; reg = <1>; // endpoints to TV TOP and TCON TOP HDMI input ... }; }; }; tcon_tv1: lcd-controller@1c74000 { compatible = "allwinner,sun8i-r40-tcon-tv-1"; ... ports { #address-cells = <1>; #size-cells = <0>; tcon_tv1_in: port@0 { reg = <0>; tcon_tv1_in_tcon_top: endpoint { // endpoint depends on board, part of board DTS remote-endpoint = <phandle to one of tcon_top_out_tcon>; }; }; tcon_tv1_out: port@1 { #address-cells = <1>; #size-cells = <0>; reg = <1>; // endpoints to TV TOP and TCON TOP HDMI input ... }; }; }; tcon_lcd0 and tcon_lcd1 would have similar connections, except that for outputs would be LCD or LVDS panels or MIPI DSI encoder. Please note that each TCON (there are 4 of them) would need to have unique compatible and have HW index stored in quirks data. It would be used by TCON TOP driver for configuring muxes. What do you think about above suggestion? Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 31, 2018 at 07:54:08PM +0200, Jernej Škrabec wrote: > Dne četrtek, 31. maj 2018 ob 11:21:33 CEST je Maxime Ripard napisal(a): > > On Thu, May 24, 2018 at 03:01:09PM -0700, Chen-Yu Tsai wrote: > > > >> > > + if (tcon->quirks->needs_tcon_top) { > > > >> > > + struct device_node *np; > > > >> > > + > > > >> > > + np = of_parse_phandle(dev->of_node, "allwinner,tcon-top", > > > >> > > 0); > > > >> > > + if (np) { > > > >> > > + struct platform_device *pdev; > > > >> > > + > > > >> > > + pdev = of_find_device_by_node(np); > > > >> > > + if (pdev) > > > >> > > + tcon->tcon_top = > > > >> > > platform_get_drvdata(pdev); > > > >> > > + of_node_put(np); > > > >> > > + > > > >> > > + if (!tcon->tcon_top) > > > >> > > + return -EPROBE_DEFER; > > > >> > > + } > > > >> > > + } > > > >> > > + > > > >> > > > > >> > I might have missed it, but I've not seen the bindings additions for > > > >> > that property. This shouldn't really be done that way anyway, instead > > > >> > of using a direct phandle, you should be using the of-graph, with the > > > >> > TCON-top sitting where it belongs in the flow of data. > > > >> > > > >> Just to answer to the first question, it did describe it in "[PATCH > > > >> 07/15] dt- bindings: display: sun4i-drm: Add R40 HDMI pipeline". > > > >> > > > >> As why I designed it that way - HW representation could be described > > > >> that way> >> > > > >> (ASCII art makes sense when fixed width font is used to view it): > > > >> / LCD0/LVDS0 > > > >> > > > >> / TCON-LCD0 > > > >> > > > >> | \ MIPI DSI > > > >> > > > >> mixer0 | > > > >> > > > >> \ / TCON-LCD1 - LCD1/LVDS1 > > > >> > > > >> TCON-TOP > > > >> > > > >> / \ TCON-TV0 - TVE0/RGB > > > >> > > > >> mixer1 | \ > > > >> > > > >> | TCON-TOP - HDMI > > > >> | > > > >> | / > > > >> > > > >> \ TCON-TV1 - TVE1/RGB > > > >> > > > >> This is a bit simplified, since there is also TVE-TOP, which is > > > >> responsible > > > >> for sharing 4 DACs between both TVE encoders. You can have two TV outs > > > >> (PAL/ NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It even > > > >> seems that you can arbitrarly choose which DAC is responsible for > > > >> which signal, so there is a ton of possible end combinations, but I'm > > > >> not 100% sure. > > > >> > > > >> Even though I wrote TCON-TOP twice, this is same unit in HW. R40 manual > > > >> suggest more possibilities, although some of them seem wrong, like RGB > > > >> feeding from LCD TCON. That is confirmed to be wrong when checking BSP > > > >> code. > > > >> > > > >> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, TVE1 and > > > >> LCD1 for pin muxing, although I'm not sure why is that needed at all, > > > >> since according to R40 datasheet, TVE0 and TVE1 pins are dedicated and > > > >> not on PORT D and PORT H, respectively, as TCON-TOP documentation > > > >> suggest. However, HSYNC and PSYNC lines might be shared between TVE > > > >> (when it works in RGB mode) and LCD. But that is just my guess since > > > >> I'm not really familiar with RGB and LCD interfaces. > > > >> > > > >> I'm really not sure what would be the best representation in OF-graph. > > > >> Can you suggest one? > > > > > > > > Rob might disagree on this one, but I don't see anything wrong with > > > > having loops in the graph. If the TCON-TOP can be both the input and > > > > output of the TCONs, then so be it, and have it described that way in > > > > the graph. > > > > > > > > The code is already able to filter out nodes that have already been > > > > added to the list of devices we need to wait for in the component > > > > framework, so that should work as well. > > > > > > > > And we'd need to describe TVE-TOP as well, even though we don't have a > > > > driver for it yet. That will simplify the backward compatibility later > > > > on. > > > > > > I'm getting the feeling that TCON-TOP / TVE-TOP is the glue layer that > > > binds everything together, and provides signal routing, kind of like > > > DE-TOP on A64. So the signal mux controls that were originally found > > > in TCON0 and TVE0 were moved out. > > > > > > The driver needs to know about that, but the graph about doesn't make > > > much sense directly. Without looking at the manual, I understand it to > > > likely be one mux between the mixers and TCONs, and one between the > > > TCON-TVs and HDMI. Would it make more sense to just have the graph > > > connections between the muxed components, and remove TCON-TOP from > > > it, like we had in the past? A phandle could be used to reference > > > the TCON-TOP for mux controls, in addition to the clocks and resets. > > > > > > For TVE, we would need something to represent each of the output pins, > > > so the device tree can actually describe what kind of signal, be it > > > each component of RGB/YUV or composite video, is wanted on each pin, > > > if any. This is also needed on the A20 for the Cubietruck, so we can > > > describe which pins are tied to the VGA connector, and which one does > > > R, G, or B. > > > > I guess we'll see how the DT maintainers feel about this, but my > > impression is that the OF graph should model the flow of data between > > the devices. If there's a mux somewhere, then the data is definitely > > going through it, and as such it should be part of the graph. > > I concur, but I spent few days thinking how to represent this sanely in graph, > but I didn't find any good solution. I'll represent here my idea and please > tell your opinion before I start implementing it. > > First, let me be clear that mixer0 and mixer1 don't have same capabilities > (different number of planes, mixer0 supports writeback, mixer1 does not, > etc.). Thus, it does matter which mixer is connected to which TCON/encoder. > mixer0 is meant to be connected to main display and mixer1 to auxiliary. That > obviously depends on end system. > > So, TCON TOP has 3 muxes, which have to be represented in graph. Two of them > are for mixer/TCON relationship (each of them 1 input and 4 possible outputs) > and one for TV TCON/HDMI pair selection (2 possible inputs, 1 output). > > According to current practice in sun4i-drm driver, graph has to have port 0, > representing input and port 1, representing output. This would mean that graph > looks something like that: > > tcon_top: tcon-top@1c70000 { > compatible = "allwinner,sun8i-r40-tcon-top"; > ... > ports { > #address-cells = <1>; > #size-cells = <0>; > > tcon_top_in: port@0 { > #address-cells = <1>; > #size-cells = <0>; > reg = <0>; > > tcon_top_in_mixer0: endpoint@0 { > reg = <0>; > remote-endpoint = <&mixer0_out_tcon_top>; > }; > > tcon_top_in_mixer1: endpoint@1 { > reg = <1>; > remote-endpoint = <&mixer1_out_tcon_top>; > }; > > tcon_top_in_tcon_tv: endpoint@2 { > reg = <2>; > // here is HDMI input connection, part of board DTS > remote-endpoint = <board specific phandle to TV TCON output>; > }; > }; > > tcon_top_out: port@1 { > #address-cells = <1>; > #size-cells = <0>; > reg = <1>; > > tcon_top_out_tcon0: endpoint@0 { > reg = <0>; > // here is mixer0 output connection, part of board DTS > remote-endpoint = <board specific phandle to TCON>; > }; > > tcon_top_out_tcon1: endpoint@1 { > reg = <1>; > // here is mixer1 output connection, part of board DTS > remote-endpoint = <board specific phandle to TCON>; > }; > > tcon_top_out_hdmi: endpoint@2 { > reg = <2>; > remote-endpoint = <&hdmi_in_tcon_top>; > }; > }; > }; > }; IIRC, each port is supposed to be one route for the data, so we would have multiple ports, one for the mixers in input and for the tcon in output, and one for the TCON in input and for the HDMI/TV in output. Rob might correct me here. > tcon_tv0: lcd-controller@1c73000 { > compatible = "allwinner,sun8i-r40-tcon-tv-0"; > ... > ports { > #address-cells = <1>; > #size-cells = <0>; > > tcon_tv0_in: port@0 { > reg = <0>; > > tcon_tv0_in_tcon_top: endpoint { > // endpoint depends on board, part of board DTS > remote-endpoint = <phandle to one of tcon_top_out_tcon>; Just curious, what would be there? > }; > }; > > tcon_tv0_out: port@1 { > #address-cells = <1>; > #size-cells = <0>; > reg = <1>; > > // endpoints to TV TOP and TCON TOP HDMI input > ... > }; > }; > }; > > tcon_tv1: lcd-controller@1c74000 { > compatible = "allwinner,sun8i-r40-tcon-tv-1"; > ... > ports { > #address-cells = <1>; > #size-cells = <0>; > > tcon_tv1_in: port@0 { > reg = <0>; > > tcon_tv1_in_tcon_top: endpoint { > // endpoint depends on board, part of board DTS > remote-endpoint = <phandle to one of tcon_top_out_tcon>; > }; > }; > > tcon_tv1_out: port@1 { > #address-cells = <1>; > #size-cells = <0>; > reg = <1>; > > // endpoints to TV TOP and TCON TOP HDMI input > ... > }; > }; > }; > > tcon_lcd0 and tcon_lcd1 would have similar connections, except that for > outputs would be LCD or LVDS panels or MIPI DSI encoder. > > Please note that each TCON (there are 4 of them) would need to have unique > compatible and have HW index stored in quirks data. It would be used by TCON > TOP driver for configuring muxes. Can't we use the port/endpoint ID instead? If the mux is the only thing that changes, the compatible has no reason to. It's the same IP, and the only thing that changes is something that is not part of that IP. Maxime
On Fri, Jun 1, 2018 at 8:29 AM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > On Thu, May 31, 2018 at 07:54:08PM +0200, Jernej Škrabec wrote: >> Dne četrtek, 31. maj 2018 ob 11:21:33 CEST je Maxime Ripard napisal(a): >> > On Thu, May 24, 2018 at 03:01:09PM -0700, Chen-Yu Tsai wrote: >> > > >> > > + if (tcon->quirks->needs_tcon_top) { >> > > >> > > + struct device_node *np; >> > > >> > > + >> > > >> > > + np = of_parse_phandle(dev->of_node, "allwinner,tcon-top", >> > > >> > > 0); >> > > >> > > + if (np) { >> > > >> > > + struct platform_device *pdev; >> > > >> > > + >> > > >> > > + pdev = of_find_device_by_node(np); >> > > >> > > + if (pdev) >> > > >> > > + tcon->tcon_top = >> > > >> > > platform_get_drvdata(pdev); >> > > >> > > + of_node_put(np); >> > > >> > > + >> > > >> > > + if (!tcon->tcon_top) >> > > >> > > + return -EPROBE_DEFER; >> > > >> > > + } >> > > >> > > + } >> > > >> > > + >> > > >> > >> > > >> > I might have missed it, but I've not seen the bindings additions for >> > > >> > that property. This shouldn't really be done that way anyway, instead >> > > >> > of using a direct phandle, you should be using the of-graph, with the >> > > >> > TCON-top sitting where it belongs in the flow of data. >> > > >> >> > > >> Just to answer to the first question, it did describe it in "[PATCH >> > > >> 07/15] dt- bindings: display: sun4i-drm: Add R40 HDMI pipeline". >> > > >> >> > > >> As why I designed it that way - HW representation could be described >> > > >> that way> >> >> > > >> (ASCII art makes sense when fixed width font is used to view it): >> > > >> / LCD0/LVDS0 >> > > >> >> > > >> / TCON-LCD0 >> > > >> >> > > >> | \ MIPI DSI >> > > >> >> > > >> mixer0 | >> > > >> >> > > >> \ / TCON-LCD1 - LCD1/LVDS1 >> > > >> >> > > >> TCON-TOP >> > > >> >> > > >> / \ TCON-TV0 - TVE0/RGB >> > > >> >> > > >> mixer1 | \ >> > > >> >> > > >> | TCON-TOP - HDMI >> > > >> | >> > > >> | / >> > > >> >> > > >> \ TCON-TV1 - TVE1/RGB >> > > >> >> > > >> This is a bit simplified, since there is also TVE-TOP, which is >> > > >> responsible >> > > >> for sharing 4 DACs between both TVE encoders. You can have two TV outs >> > > >> (PAL/ NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It even >> > > >> seems that you can arbitrarly choose which DAC is responsible for >> > > >> which signal, so there is a ton of possible end combinations, but I'm >> > > >> not 100% sure. >> > > >> >> > > >> Even though I wrote TCON-TOP twice, this is same unit in HW. R40 manual >> > > >> suggest more possibilities, although some of them seem wrong, like RGB >> > > >> feeding from LCD TCON. That is confirmed to be wrong when checking BSP >> > > >> code. >> > > >> >> > > >> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, TVE1 and >> > > >> LCD1 for pin muxing, although I'm not sure why is that needed at all, >> > > >> since according to R40 datasheet, TVE0 and TVE1 pins are dedicated and >> > > >> not on PORT D and PORT H, respectively, as TCON-TOP documentation >> > > >> suggest. However, HSYNC and PSYNC lines might be shared between TVE >> > > >> (when it works in RGB mode) and LCD. But that is just my guess since >> > > >> I'm not really familiar with RGB and LCD interfaces. >> > > >> >> > > >> I'm really not sure what would be the best representation in OF-graph. >> > > >> Can you suggest one? >> > > > >> > > > Rob might disagree on this one, but I don't see anything wrong with >> > > > having loops in the graph. If the TCON-TOP can be both the input and >> > > > output of the TCONs, then so be it, and have it described that way in >> > > > the graph. >> > > > >> > > > The code is already able to filter out nodes that have already been >> > > > added to the list of devices we need to wait for in the component >> > > > framework, so that should work as well. >> > > > >> > > > And we'd need to describe TVE-TOP as well, even though we don't have a >> > > > driver for it yet. That will simplify the backward compatibility later >> > > > on. >> > > >> > > I'm getting the feeling that TCON-TOP / TVE-TOP is the glue layer that >> > > binds everything together, and provides signal routing, kind of like >> > > DE-TOP on A64. So the signal mux controls that were originally found >> > > in TCON0 and TVE0 were moved out. >> > > >> > > The driver needs to know about that, but the graph about doesn't make >> > > much sense directly. Without looking at the manual, I understand it to >> > > likely be one mux between the mixers and TCONs, and one between the >> > > TCON-TVs and HDMI. Would it make more sense to just have the graph >> > > connections between the muxed components, and remove TCON-TOP from >> > > it, like we had in the past? A phandle could be used to reference >> > > the TCON-TOP for mux controls, in addition to the clocks and resets. >> > > >> > > For TVE, we would need something to represent each of the output pins, >> > > so the device tree can actually describe what kind of signal, be it >> > > each component of RGB/YUV or composite video, is wanted on each pin, >> > > if any. This is also needed on the A20 for the Cubietruck, so we can >> > > describe which pins are tied to the VGA connector, and which one does >> > > R, G, or B. >> > >> > I guess we'll see how the DT maintainers feel about this, but my >> > impression is that the OF graph should model the flow of data between >> > the devices. If there's a mux somewhere, then the data is definitely >> > going through it, and as such it should be part of the graph. >> >> I concur, but I spent few days thinking how to represent this sanely in graph, >> but I didn't find any good solution. I'll represent here my idea and please >> tell your opinion before I start implementing it. >> >> First, let me be clear that mixer0 and mixer1 don't have same capabilities >> (different number of planes, mixer0 supports writeback, mixer1 does not, >> etc.). Thus, it does matter which mixer is connected to which TCON/encoder. >> mixer0 is meant to be connected to main display and mixer1 to auxiliary. That >> obviously depends on end system. >> >> So, TCON TOP has 3 muxes, which have to be represented in graph. Two of them >> are for mixer/TCON relationship (each of them 1 input and 4 possible outputs) >> and one for TV TCON/HDMI pair selection (2 possible inputs, 1 output). >> >> According to current practice in sun4i-drm driver, graph has to have port 0, >> representing input and port 1, representing output. This would mean that graph >> looks something like that: >> >> tcon_top: tcon-top@1c70000 { >> compatible = "allwinner,sun8i-r40-tcon-top"; >> ... >> ports { >> #address-cells = <1>; >> #size-cells = <0>; >> >> tcon_top_in: port@0 { >> #address-cells = <1>; >> #size-cells = <0>; >> reg = <0>; >> >> tcon_top_in_mixer0: endpoint@0 { >> reg = <0>; >> remote-endpoint = <&mixer0_out_tcon_top>; >> }; >> >> tcon_top_in_mixer1: endpoint@1 { >> reg = <1>; >> remote-endpoint = <&mixer1_out_tcon_top>; >> }; >> >> tcon_top_in_tcon_tv: endpoint@2 { >> reg = <2>; >> // here is HDMI input connection, part of board DTS >> remote-endpoint = <board specific phandle to TV TCON output>; >> }; >> }; >> >> tcon_top_out: port@1 { >> #address-cells = <1>; >> #size-cells = <0>; >> reg = <1>; >> >> tcon_top_out_tcon0: endpoint@0 { >> reg = <0>; >> // here is mixer0 output connection, part of board DTS >> remote-endpoint = <board specific phandle to TCON>; >> }; >> >> tcon_top_out_tcon1: endpoint@1 { >> reg = <1>; >> // here is mixer1 output connection, part of board DTS >> remote-endpoint = <board specific phandle to TCON>; >> }; >> >> tcon_top_out_hdmi: endpoint@2 { >> reg = <2>; >> remote-endpoint = <&hdmi_in_tcon_top>; >> }; >> }; >> }; >> }; > > IIRC, each port is supposed to be one route for the data, so we would > have multiple ports, one for the mixers in input and for the tcon in > output, and one for the TCON in input and for the HDMI/TV in > output. Rob might correct me here. > >> tcon_tv0: lcd-controller@1c73000 { >> compatible = "allwinner,sun8i-r40-tcon-tv-0"; >> ... >> ports { >> #address-cells = <1>; >> #size-cells = <0>; >> >> tcon_tv0_in: port@0 { >> reg = <0>; >> >> tcon_tv0_in_tcon_top: endpoint { >> // endpoint depends on board, part of board DTS >> remote-endpoint = <phandle to one of tcon_top_out_tcon>; > > Just curious, what would be there? > >> }; >> }; >> >> tcon_tv0_out: port@1 { >> #address-cells = <1>; >> #size-cells = <0>; >> reg = <1>; >> >> // endpoints to TV TOP and TCON TOP HDMI input >> ... >> }; >> }; >> }; >> >> tcon_tv1: lcd-controller@1c74000 { >> compatible = "allwinner,sun8i-r40-tcon-tv-1"; >> ... >> ports { >> #address-cells = <1>; >> #size-cells = <0>; >> >> tcon_tv1_in: port@0 { >> reg = <0>; >> >> tcon_tv1_in_tcon_top: endpoint { >> // endpoint depends on board, part of board DTS >> remote-endpoint = <phandle to one of tcon_top_out_tcon>; >> }; >> }; >> >> tcon_tv1_out: port@1 { >> #address-cells = <1>; >> #size-cells = <0>; >> reg = <1>; >> >> // endpoints to TV TOP and TCON TOP HDMI input >> ... >> }; >> }; >> }; >> >> tcon_lcd0 and tcon_lcd1 would have similar connections, except that for >> outputs would be LCD or LVDS panels or MIPI DSI encoder. >> >> Please note that each TCON (there are 4 of them) would need to have unique >> compatible and have HW index stored in quirks data. It would be used by TCON >> TOP driver for configuring muxes. > > Can't we use the port/endpoint ID instead? If the mux is the only > thing that changes, the compatible has no reason to. It's the same IP, > and the only thing that changes is something that is not part of that > IP. I agree. Endpoint IDs should provide that information. I'm still not sure How to encode multiple in/out mux groups in a device node though. ChenYu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 01, 2018 at 09:19:43AM -0700, Chen-Yu Tsai wrote: > On Fri, Jun 1, 2018 at 8:29 AM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Thu, May 31, 2018 at 07:54:08PM +0200, Jernej Škrabec wrote: > >> Dne četrtek, 31. maj 2018 ob 11:21:33 CEST je Maxime Ripard napisal(a): > >> > On Thu, May 24, 2018 at 03:01:09PM -0700, Chen-Yu Tsai wrote: > >> > > >> > > + if (tcon->quirks->needs_tcon_top) { > >> > > >> > > + struct device_node *np; > >> > > >> > > + > >> > > >> > > + np = of_parse_phandle(dev->of_node, "allwinner,tcon-top", > >> > > >> > > 0); > >> > > >> > > + if (np) { > >> > > >> > > + struct platform_device *pdev; > >> > > >> > > + > >> > > >> > > + pdev = of_find_device_by_node(np); > >> > > >> > > + if (pdev) > >> > > >> > > + tcon->tcon_top = > >> > > >> > > platform_get_drvdata(pdev); > >> > > >> > > + of_node_put(np); > >> > > >> > > + > >> > > >> > > + if (!tcon->tcon_top) > >> > > >> > > + return -EPROBE_DEFER; > >> > > >> > > + } > >> > > >> > > + } > >> > > >> > > + > >> > > >> > > >> > > >> > I might have missed it, but I've not seen the bindings additions for > >> > > >> > that property. This shouldn't really be done that way anyway, instead > >> > > >> > of using a direct phandle, you should be using the of-graph, with the > >> > > >> > TCON-top sitting where it belongs in the flow of data. > >> > > >> > >> > > >> Just to answer to the first question, it did describe it in "[PATCH > >> > > >> 07/15] dt- bindings: display: sun4i-drm: Add R40 HDMI pipeline". > >> > > >> > >> > > >> As why I designed it that way - HW representation could be described > >> > > >> that way> >> > >> > > >> (ASCII art makes sense when fixed width font is used to view it): > >> > > >> / LCD0/LVDS0 > >> > > >> > >> > > >> / TCON-LCD0 > >> > > >> > >> > > >> | \ MIPI DSI > >> > > >> > >> > > >> mixer0 | > >> > > >> > >> > > >> \ / TCON-LCD1 - LCD1/LVDS1 > >> > > >> > >> > > >> TCON-TOP > >> > > >> > >> > > >> / \ TCON-TV0 - TVE0/RGB > >> > > >> > >> > > >> mixer1 | \ > >> > > >> > >> > > >> | TCON-TOP - HDMI > >> > > >> | > >> > > >> | / > >> > > >> > >> > > >> \ TCON-TV1 - TVE1/RGB > >> > > >> > >> > > >> This is a bit simplified, since there is also TVE-TOP, which is > >> > > >> responsible > >> > > >> for sharing 4 DACs between both TVE encoders. You can have two TV outs > >> > > >> (PAL/ NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It even > >> > > >> seems that you can arbitrarly choose which DAC is responsible for > >> > > >> which signal, so there is a ton of possible end combinations, but I'm > >> > > >> not 100% sure. > >> > > >> > >> > > >> Even though I wrote TCON-TOP twice, this is same unit in HW. R40 manual > >> > > >> suggest more possibilities, although some of them seem wrong, like RGB > >> > > >> feeding from LCD TCON. That is confirmed to be wrong when checking BSP > >> > > >> code. > >> > > >> > >> > > >> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, TVE1 and > >> > > >> LCD1 for pin muxing, although I'm not sure why is that needed at all, > >> > > >> since according to R40 datasheet, TVE0 and TVE1 pins are dedicated and > >> > > >> not on PORT D and PORT H, respectively, as TCON-TOP documentation > >> > > >> suggest. However, HSYNC and PSYNC lines might be shared between TVE > >> > > >> (when it works in RGB mode) and LCD. But that is just my guess since > >> > > >> I'm not really familiar with RGB and LCD interfaces. > >> > > >> > >> > > >> I'm really not sure what would be the best representation in OF-graph. > >> > > >> Can you suggest one? > >> > > > > >> > > > Rob might disagree on this one, but I don't see anything wrong with > >> > > > having loops in the graph. If the TCON-TOP can be both the input and > >> > > > output of the TCONs, then so be it, and have it described that way in > >> > > > the graph. > >> > > > > >> > > > The code is already able to filter out nodes that have already been > >> > > > added to the list of devices we need to wait for in the component > >> > > > framework, so that should work as well. > >> > > > > >> > > > And we'd need to describe TVE-TOP as well, even though we don't have a > >> > > > driver for it yet. That will simplify the backward compatibility later > >> > > > on. > >> > > > >> > > I'm getting the feeling that TCON-TOP / TVE-TOP is the glue layer that > >> > > binds everything together, and provides signal routing, kind of like > >> > > DE-TOP on A64. So the signal mux controls that were originally found > >> > > in TCON0 and TVE0 were moved out. > >> > > > >> > > The driver needs to know about that, but the graph about doesn't make > >> > > much sense directly. Without looking at the manual, I understand it to > >> > > likely be one mux between the mixers and TCONs, and one between the > >> > > TCON-TVs and HDMI. Would it make more sense to just have the graph > >> > > connections between the muxed components, and remove TCON-TOP from > >> > > it, like we had in the past? A phandle could be used to reference > >> > > the TCON-TOP for mux controls, in addition to the clocks and resets. > >> > > > >> > > For TVE, we would need something to represent each of the output pins, > >> > > so the device tree can actually describe what kind of signal, be it > >> > > each component of RGB/YUV or composite video, is wanted on each pin, > >> > > if any. This is also needed on the A20 for the Cubietruck, so we can > >> > > describe which pins are tied to the VGA connector, and which one does > >> > > R, G, or B. > >> > > >> > I guess we'll see how the DT maintainers feel about this, but my > >> > impression is that the OF graph should model the flow of data between > >> > the devices. If there's a mux somewhere, then the data is definitely > >> > going through it, and as such it should be part of the graph. > >> > >> I concur, but I spent few days thinking how to represent this sanely in graph, > >> but I didn't find any good solution. I'll represent here my idea and please > >> tell your opinion before I start implementing it. > >> > >> First, let me be clear that mixer0 and mixer1 don't have same capabilities > >> (different number of planes, mixer0 supports writeback, mixer1 does not, > >> etc.). Thus, it does matter which mixer is connected to which TCON/encoder. > >> mixer0 is meant to be connected to main display and mixer1 to auxiliary. That > >> obviously depends on end system. > >> > >> So, TCON TOP has 3 muxes, which have to be represented in graph. Two of them > >> are for mixer/TCON relationship (each of them 1 input and 4 possible outputs) > >> and one for TV TCON/HDMI pair selection (2 possible inputs, 1 output). > >> > >> According to current practice in sun4i-drm driver, graph has to have port 0, > >> representing input and port 1, representing output. This would mean that graph > >> looks something like that: > >> > >> tcon_top: tcon-top@1c70000 { > >> compatible = "allwinner,sun8i-r40-tcon-top"; > >> ... > >> ports { > >> #address-cells = <1>; > >> #size-cells = <0>; > >> > >> tcon_top_in: port@0 { > >> #address-cells = <1>; > >> #size-cells = <0>; > >> reg = <0>; > >> > >> tcon_top_in_mixer0: endpoint@0 { > >> reg = <0>; > >> remote-endpoint = <&mixer0_out_tcon_top>; > >> }; > >> > >> tcon_top_in_mixer1: endpoint@1 { > >> reg = <1>; > >> remote-endpoint = <&mixer1_out_tcon_top>; > >> }; > >> > >> tcon_top_in_tcon_tv: endpoint@2 { > >> reg = <2>; > >> // here is HDMI input connection, part of board DTS > >> remote-endpoint = <board specific phandle to TV TCON output>; > >> }; > >> }; > >> > >> tcon_top_out: port@1 { > >> #address-cells = <1>; > >> #size-cells = <0>; > >> reg = <1>; > >> > >> tcon_top_out_tcon0: endpoint@0 { > >> reg = <0>; > >> // here is mixer0 output connection, part of board DTS > >> remote-endpoint = <board specific phandle to TCON>; > >> }; > >> > >> tcon_top_out_tcon1: endpoint@1 { > >> reg = <1>; > >> // here is mixer1 output connection, part of board DTS > >> remote-endpoint = <board specific phandle to TCON>; > >> }; > >> > >> tcon_top_out_hdmi: endpoint@2 { > >> reg = <2>; > >> remote-endpoint = <&hdmi_in_tcon_top>; > >> }; > >> }; > >> }; > >> }; > > > > IIRC, each port is supposed to be one route for the data, so we would > > have multiple ports, one for the mixers in input and for the tcon in > > output, and one for the TCON in input and for the HDMI/TV in > > output. Rob might correct me here. > > > >> tcon_tv0: lcd-controller@1c73000 { > >> compatible = "allwinner,sun8i-r40-tcon-tv-0"; > >> ... > >> ports { > >> #address-cells = <1>; > >> #size-cells = <0>; > >> > >> tcon_tv0_in: port@0 { > >> reg = <0>; > >> > >> tcon_tv0_in_tcon_top: endpoint { > >> // endpoint depends on board, part of board DTS > >> remote-endpoint = <phandle to one of tcon_top_out_tcon>; > > > > Just curious, what would be there? > > > >> }; > >> }; > >> > >> tcon_tv0_out: port@1 { > >> #address-cells = <1>; > >> #size-cells = <0>; > >> reg = <1>; > >> > >> // endpoints to TV TOP and TCON TOP HDMI input > >> ... > >> }; > >> }; > >> }; > >> > >> tcon_tv1: lcd-controller@1c74000 { > >> compatible = "allwinner,sun8i-r40-tcon-tv-1"; > >> ... > >> ports { > >> #address-cells = <1>; > >> #size-cells = <0>; > >> > >> tcon_tv1_in: port@0 { > >> reg = <0>; > >> > >> tcon_tv1_in_tcon_top: endpoint { > >> // endpoint depends on board, part of board DTS > >> remote-endpoint = <phandle to one of tcon_top_out_tcon>; > >> }; > >> }; > >> > >> tcon_tv1_out: port@1 { > >> #address-cells = <1>; > >> #size-cells = <0>; > >> reg = <1>; > >> > >> // endpoints to TV TOP and TCON TOP HDMI input > >> ... > >> }; > >> }; > >> }; > >> > >> tcon_lcd0 and tcon_lcd1 would have similar connections, except that for > >> outputs would be LCD or LVDS panels or MIPI DSI encoder. > >> > >> Please note that each TCON (there are 4 of them) would need to have unique > >> compatible and have HW index stored in quirks data. It would be used by TCON > >> TOP driver for configuring muxes. > > > > Can't we use the port/endpoint ID instead? If the mux is the only > > thing that changes, the compatible has no reason to. It's the same IP, > > and the only thing that changes is something that is not part of that > > IP. > > I agree. Endpoint IDs should provide that information. I'm still not > sure How to encode multiple in/out mux groups in a device node though. I guess we can do that through different ports? Maxime
Dne ponedeljek, 04. junij 2018 ob 13:50:34 CEST je Maxime Ripard napisal(a): > On Fri, Jun 01, 2018 at 09:19:43AM -0700, Chen-Yu Tsai wrote: > > On Fri, Jun 1, 2018 at 8:29 AM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > On Thu, May 31, 2018 at 07:54:08PM +0200, Jernej Škrabec wrote: > > >> Dne četrtek, 31. maj 2018 ob 11:21:33 CEST je Maxime Ripard napisal(a): > > >> > On Thu, May 24, 2018 at 03:01:09PM -0700, Chen-Yu Tsai wrote: > > >> > > >> > > + if (tcon->quirks->needs_tcon_top) { > > >> > > >> > > + struct device_node *np; > > >> > > >> > > + > > >> > > >> > > + np = of_parse_phandle(dev->of_node, > > >> > > >> > > "allwinner,tcon-top", > > >> > > >> > > 0); > > >> > > >> > > + if (np) { > > >> > > >> > > + struct platform_device *pdev; > > >> > > >> > > + > > >> > > >> > > + pdev = of_find_device_by_node(np); > > >> > > >> > > + if (pdev) > > >> > > >> > > + tcon->tcon_top = > > >> > > >> > > platform_get_drvdata(pdev); > > >> > > >> > > + of_node_put(np); > > >> > > >> > > + > > >> > > >> > > + if (!tcon->tcon_top) > > >> > > >> > > + return -EPROBE_DEFER; > > >> > > >> > > + } > > >> > > >> > > + } > > >> > > >> > > + > > >> > > >> > > > >> > > >> > I might have missed it, but I've not seen the bindings > > >> > > >> > additions for > > >> > > >> > that property. This shouldn't really be done that way anyway, > > >> > > >> > instead > > >> > > >> > of using a direct phandle, you should be using the of-graph, > > >> > > >> > with the > > >> > > >> > TCON-top sitting where it belongs in the flow of data. > > >> > > >> > > >> > > >> Just to answer to the first question, it did describe it in > > >> > > >> "[PATCH > > >> > > >> 07/15] dt- bindings: display: sun4i-drm: Add R40 HDMI pipeline". > > >> > > >> > > >> > > >> As why I designed it that way - HW representation could be > > >> > > >> described > > >> > > >> that way> >> > > >> > > >> > > >> > > >> (ASCII art makes sense when fixed width font is used to view it): > > >> > > >> / LCD0/LVDS0 > > >> > > >> > > >> > > >> / TCON-LCD0 > > >> > > >> > > >> > > >> | \ MIPI DSI > > >> > > >> > > >> > > >> mixer0 | > > >> > > >> > > >> > > >> \ / TCON-LCD1 - LCD1/LVDS1 > > >> > > >> > > >> > > >> TCON-TOP > > >> > > >> > > >> > > >> / \ TCON-TV0 - TVE0/RGB > > >> > > >> > > >> > > >> mixer1 | \ > > >> > > >> > > >> > > >> | TCON-TOP - HDMI > > >> > > >> | > > >> > > >> | / > > >> > > >> > > >> > > >> \ TCON-TV1 - TVE1/RGB > > >> > > >> > > >> > > >> This is a bit simplified, since there is also TVE-TOP, which is > > >> > > >> responsible > > >> > > >> for sharing 4 DACs between both TVE encoders. You can have two > > >> > > >> TV outs > > >> > > >> (PAL/ NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It > > >> > > >> even > > >> > > >> seems that you can arbitrarly choose which DAC is responsible > > >> > > >> for > > >> > > >> which signal, so there is a ton of possible end combinations, > > >> > > >> but I'm > > >> > > >> not 100% sure. > > >> > > >> > > >> > > >> Even though I wrote TCON-TOP twice, this is same unit in HW. R40 > > >> > > >> manual > > >> > > >> suggest more possibilities, although some of them seem wrong, > > >> > > >> like RGB > > >> > > >> feeding from LCD TCON. That is confirmed to be wrong when > > >> > > >> checking BSP > > >> > > >> code. > > >> > > >> > > >> > > >> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, > > >> > > >> TVE1 and > > >> > > >> LCD1 for pin muxing, although I'm not sure why is that needed at > > >> > > >> all, > > >> > > >> since according to R40 datasheet, TVE0 and TVE1 pins are > > >> > > >> dedicated and > > >> > > >> not on PORT D and PORT H, respectively, as TCON-TOP > > >> > > >> documentation > > >> > > >> suggest. However, HSYNC and PSYNC lines might be shared between > > >> > > >> TVE > > >> > > >> (when it works in RGB mode) and LCD. But that is just my guess > > >> > > >> since > > >> > > >> I'm not really familiar with RGB and LCD interfaces. > > >> > > >> > > >> > > >> I'm really not sure what would be the best representation in > > >> > > >> OF-graph. > > >> > > >> Can you suggest one? > > >> > > > > > >> > > > Rob might disagree on this one, but I don't see anything wrong > > >> > > > with > > >> > > > having loops in the graph. If the TCON-TOP can be both the input > > >> > > > and > > >> > > > output of the TCONs, then so be it, and have it described that > > >> > > > way in > > >> > > > the graph. > > >> > > > > > >> > > > The code is already able to filter out nodes that have already > > >> > > > been > > >> > > > added to the list of devices we need to wait for in the component > > >> > > > framework, so that should work as well. > > >> > > > > > >> > > > And we'd need to describe TVE-TOP as well, even though we don't > > >> > > > have a > > >> > > > driver for it yet. That will simplify the backward compatibility > > >> > > > later > > >> > > > on. > > >> > > > > >> > > I'm getting the feeling that TCON-TOP / TVE-TOP is the glue layer > > >> > > that > > >> > > binds everything together, and provides signal routing, kind of > > >> > > like > > >> > > DE-TOP on A64. So the signal mux controls that were originally > > >> > > found > > >> > > in TCON0 and TVE0 were moved out. > > >> > > > > >> > > The driver needs to know about that, but the graph about doesn't > > >> > > make > > >> > > much sense directly. Without looking at the manual, I understand it > > >> > > to > > >> > > likely be one mux between the mixers and TCONs, and one between the > > >> > > TCON-TVs and HDMI. Would it make more sense to just have the graph > > >> > > connections between the muxed components, and remove TCON-TOP from > > >> > > it, like we had in the past? A phandle could be used to reference > > >> > > the TCON-TOP for mux controls, in addition to the clocks and > > >> > > resets. > > >> > > > > >> > > For TVE, we would need something to represent each of the output > > >> > > pins, > > >> > > so the device tree can actually describe what kind of signal, be it > > >> > > each component of RGB/YUV or composite video, is wanted on each > > >> > > pin, > > >> > > if any. This is also needed on the A20 for the Cubietruck, so we > > >> > > can > > >> > > describe which pins are tied to the VGA connector, and which one > > >> > > does > > >> > > R, G, or B. > > >> > > > >> > I guess we'll see how the DT maintainers feel about this, but my > > >> > impression is that the OF graph should model the flow of data between > > >> > the devices. If there's a mux somewhere, then the data is definitely > > >> > going through it, and as such it should be part of the graph. > > >> > > >> I concur, but I spent few days thinking how to represent this sanely in > > >> graph, but I didn't find any good solution. I'll represent here my > > >> idea and please tell your opinion before I start implementing it. > > >> > > >> First, let me be clear that mixer0 and mixer1 don't have same > > >> capabilities > > >> (different number of planes, mixer0 supports writeback, mixer1 does > > >> not, > > >> etc.). Thus, it does matter which mixer is connected to which > > >> TCON/encoder. > > >> mixer0 is meant to be connected to main display and mixer1 to > > >> auxiliary. That obviously depends on end system. > > >> > > >> So, TCON TOP has 3 muxes, which have to be represented in graph. Two of > > >> them are for mixer/TCON relationship (each of them 1 input and 4 > > >> possible outputs) and one for TV TCON/HDMI pair selection (2 possible > > >> inputs, 1 output). > > >> > > >> According to current practice in sun4i-drm driver, graph has to have > > >> port 0, representing input and port 1, representing output. This would > > >> mean that graph looks something like that: > > >> > > >> tcon_top: tcon-top@1c70000 { > > >> > > >> compatible = "allwinner,sun8i-r40-tcon-top"; > > >> ... > > >> ports { > > >> > > >> #address-cells = <1>; > > >> #size-cells = <0>; > > >> > > >> tcon_top_in: port@0 { > > >> > > >> #address-cells = <1>; > > >> #size-cells = <0>; > > >> reg = <0>; > > >> > > >> tcon_top_in_mixer0: endpoint@0 { > > >> > > >> reg = <0>; > > >> remote-endpoint = <&mixer0_out_tcon_top>; > > >> > > >> }; > > >> > > >> tcon_top_in_mixer1: endpoint@1 { > > >> > > >> reg = <1>; > > >> remote-endpoint = <&mixer1_out_tcon_top>; > > >> > > >> }; > > >> > > >> tcon_top_in_tcon_tv: endpoint@2 { > > >> > > >> reg = <2>; > > >> // here is HDMI input connection, part of > > >> board DTS > > >> remote-endpoint = <board specific phandle > > >> to TV TCON output>; > > >> > > >> }; > > >> > > >> }; > > >> > > >> tcon_top_out: port@1 { > > >> > > >> #address-cells = <1>; > > >> #size-cells = <0>; > > >> reg = <1>; > > >> > > >> tcon_top_out_tcon0: endpoint@0 { > > >> > > >> reg = <0>; > > >> // here is mixer0 output connection, part > > >> of board DTS > > >> remote-endpoint = <board specific phandle > > >> to TCON>; > > >> > > >> }; > > >> > > >> tcon_top_out_tcon1: endpoint@1 { > > >> > > >> reg = <1>; > > >> // here is mixer1 output connection, part > > >> of board DTS > > >> remote-endpoint = <board specific phandle > > >> to TCON>; > > >> > > >> }; > > >> > > >> tcon_top_out_hdmi: endpoint@2 { > > >> > > >> reg = <2>; > > >> remote-endpoint = <&hdmi_in_tcon_top>; > > >> > > >> }; > > >> > > >> }; > > >> > > >> }; > > >> > > >> }; > > > > > > IIRC, each port is supposed to be one route for the data, so we would > > > have multiple ports, one for the mixers in input and for the tcon in > > > output, and one for the TCON in input and for the HDMI/TV in > > > output. Rob might correct me here. Ok, that seems more clean approach. I'll have to extend graph traversing algorithm in sun4i_drv.c, but that's no problem. Just to be clear, you have in mind 3 pairs of ports (0/1 for mixer0 mux, 2/3 for mixer1 and 4/5 for HDMI input), right? That way each mux is represented with one pair of ports, even numbered for input and odd numbered for output. > > > > > >> tcon_tv0: lcd-controller@1c73000 { > > >> > > >> compatible = "allwinner,sun8i-r40-tcon-tv-0"; > > >> ... > > >> ports { > > >> > > >> #address-cells = <1>; > > >> #size-cells = <0>; > > >> > > >> tcon_tv0_in: port@0 { > > >> > > >> reg = <0>; > > >> > > >> tcon_tv0_in_tcon_top: endpoint { > > >> > > >> // endpoint depends on board, part of > > >> board DTS > > >> remote-endpoint = <phandle to one of > > >> tcon_top_out_tcon>; > > > > > > Just curious, what would be there? Either phandle to tcon_top_out_tcon0 or tcon_top_out_tcon1. > > > > > >> }; > > >> > > >> }; > > >> > > >> tcon_tv0_out: port@1 { > > >> > > >> #address-cells = <1>; > > >> #size-cells = <0>; > > >> reg = <1>; > > >> > > >> // endpoints to TV TOP and TCON TOP HDMI input > > >> ... > > >> > > >> }; > > >> > > >> }; > > >> > > >> }; > > >> > > >> tcon_tv1: lcd-controller@1c74000 { > > >> > > >> compatible = "allwinner,sun8i-r40-tcon-tv-1"; > > >> ... > > >> ports { > > >> > > >> #address-cells = <1>; > > >> #size-cells = <0>; > > >> > > >> tcon_tv1_in: port@0 { > > >> > > >> reg = <0>; > > >> > > >> tcon_tv1_in_tcon_top: endpoint { > > >> > > >> // endpoint depends on board, part of > > >> board DTS > > >> remote-endpoint = <phandle to one of > > >> tcon_top_out_tcon>; > > >> > > >> }; > > >> > > >> }; > > >> > > >> tcon_tv1_out: port@1 { > > >> > > >> #address-cells = <1>; > > >> #size-cells = <0>; > > >> reg = <1>; > > >> > > >> // endpoints to TV TOP and TCON TOP HDMI input > > >> ... > > >> > > >> }; > > >> > > >> }; > > >> > > >> }; > > >> > > >> tcon_lcd0 and tcon_lcd1 would have similar connections, except that for > > >> outputs would be LCD or LVDS panels or MIPI DSI encoder. > > >> > > >> Please note that each TCON (there are 4 of them) would need to have > > >> unique > > >> compatible and have HW index stored in quirks data. It would be used by > > >> TCON TOP driver for configuring muxes. > > > > > > Can't we use the port/endpoint ID instead? If the mux is the only > > > thing that changes, the compatible has no reason to. It's the same IP, > > > and the only thing that changes is something that is not part of that > > > IP. > > > > I agree. Endpoint IDs should provide that information. I'm still not > > sure How to encode multiple in/out mux groups in a device node though. > > I guess we can do that through different ports? Ok, I'll try to do something with "reg" property. Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 04, 2018 at 05:09:56PM +0200, Jernej Škrabec wrote: > Dne ponedeljek, 04. junij 2018 ob 13:50:34 CEST je Maxime Ripard napisal(a): > > On Fri, Jun 01, 2018 at 09:19:43AM -0700, Chen-Yu Tsai wrote: > > > On Fri, Jun 1, 2018 at 8:29 AM, Maxime Ripard <maxime.ripard@bootlin.com> > wrote: > > > > On Thu, May 31, 2018 at 07:54:08PM +0200, Jernej Škrabec wrote: > > > >> Dne četrtek, 31. maj 2018 ob 11:21:33 CEST je Maxime Ripard napisal(a): > > > >> > On Thu, May 24, 2018 at 03:01:09PM -0700, Chen-Yu Tsai wrote: > > > >> > > >> > > + if (tcon->quirks->needs_tcon_top) { > > > >> > > >> > > + struct device_node *np; > > > >> > > >> > > + > > > >> > > >> > > + np = of_parse_phandle(dev->of_node, > > > >> > > >> > > "allwinner,tcon-top", > > > >> > > >> > > 0); > > > >> > > >> > > + if (np) { > > > >> > > >> > > + struct platform_device *pdev; > > > >> > > >> > > + > > > >> > > >> > > + pdev = of_find_device_by_node(np); > > > >> > > >> > > + if (pdev) > > > >> > > >> > > + tcon->tcon_top = > > > >> > > >> > > platform_get_drvdata(pdev); > > > >> > > >> > > + of_node_put(np); > > > >> > > >> > > + > > > >> > > >> > > + if (!tcon->tcon_top) > > > >> > > >> > > + return -EPROBE_DEFER; > > > >> > > >> > > + } > > > >> > > >> > > + } > > > >> > > >> > > + > > > >> > > >> > > > > >> > > >> > I might have missed it, but I've not seen the bindings > > > >> > > >> > additions for > > > >> > > >> > that property. This shouldn't really be done that way anyway, > > > >> > > >> > instead > > > >> > > >> > of using a direct phandle, you should be using the of-graph, > > > >> > > >> > with the > > > >> > > >> > TCON-top sitting where it belongs in the flow of data. > > > >> > > >> > > > >> > > >> Just to answer to the first question, it did describe it in > > > >> > > >> "[PATCH > > > >> > > >> 07/15] dt- bindings: display: sun4i-drm: Add R40 HDMI pipeline". > > > >> > > >> > > > >> > > >> As why I designed it that way - HW representation could be > > > >> > > >> described > > > >> > > >> that way> >> > > > >> > > >> > > > >> > > >> (ASCII art makes sense when fixed width font is used to view > it): > > > >> > > >> / LCD0/LVDS0 > > > >> > > >> > > > >> > > >> / TCON-LCD0 > > > >> > > >> > > > >> > > >> | \ MIPI DSI > > > >> > > >> > > > >> > > >> mixer0 | > > > >> > > >> > > > >> > > >> \ / TCON-LCD1 - LCD1/LVDS1 > > > >> > > >> > > > >> > > >> TCON-TOP > > > >> > > >> > > > >> > > >> / \ TCON-TV0 - TVE0/RGB > > > >> > > >> > > > >> > > >> mixer1 | \ > > > >> > > >> > > > >> > > >> | TCON-TOP - HDMI > > > >> > > >> | > > > >> > > >> | / > > > >> > > >> > > > >> > > >> \ TCON-TV1 - TVE1/RGB > > > >> > > >> > > > >> > > >> This is a bit simplified, since there is also TVE-TOP, which is > > > >> > > >> responsible > > > >> > > >> for sharing 4 DACs between both TVE encoders. You can have two > > > >> > > >> TV outs > > > >> > > >> (PAL/ NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It > > > >> > > >> even > > > >> > > >> seems that you can arbitrarly choose which DAC is responsible > > > >> > > >> for > > > >> > > >> which signal, so there is a ton of possible end combinations, > > > >> > > >> but I'm > > > >> > > >> not 100% sure. > > > >> > > >> > > > >> > > >> Even though I wrote TCON-TOP twice, this is same unit in HW. R40 > > > >> > > >> manual > > > >> > > >> suggest more possibilities, although some of them seem wrong, > > > >> > > >> like RGB > > > >> > > >> feeding from LCD TCON. That is confirmed to be wrong when > > > >> > > >> checking BSP > > > >> > > >> code. > > > >> > > >> > > > >> > > >> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, > > > >> > > >> TVE1 and > > > >> > > >> LCD1 for pin muxing, although I'm not sure why is that needed at > > > >> > > >> all, > > > >> > > >> since according to R40 datasheet, TVE0 and TVE1 pins are > > > >> > > >> dedicated and > > > >> > > >> not on PORT D and PORT H, respectively, as TCON-TOP > > > >> > > >> documentation > > > >> > > >> suggest. However, HSYNC and PSYNC lines might be shared between > > > >> > > >> TVE > > > >> > > >> (when it works in RGB mode) and LCD. But that is just my guess > > > >> > > >> since > > > >> > > >> I'm not really familiar with RGB and LCD interfaces. > > > >> > > >> > > > >> > > >> I'm really not sure what would be the best representation in > > > >> > > >> OF-graph. > > > >> > > >> Can you suggest one? > > > >> > > > > > > >> > > > Rob might disagree on this one, but I don't see anything wrong > > > >> > > > with > > > >> > > > having loops in the graph. If the TCON-TOP can be both the input > > > >> > > > and > > > >> > > > output of the TCONs, then so be it, and have it described that > > > >> > > > way in > > > >> > > > the graph. > > > >> > > > > > > >> > > > The code is already able to filter out nodes that have already > > > >> > > > been > > > >> > > > added to the list of devices we need to wait for in the component > > > >> > > > framework, so that should work as well. > > > >> > > > > > > >> > > > And we'd need to describe TVE-TOP as well, even though we don't > > > >> > > > have a > > > >> > > > driver for it yet. That will simplify the backward compatibility > > > >> > > > later > > > >> > > > on. > > > >> > > > > > >> > > I'm getting the feeling that TCON-TOP / TVE-TOP is the glue layer > > > >> > > that > > > >> > > binds everything together, and provides signal routing, kind of > > > >> > > like > > > >> > > DE-TOP on A64. So the signal mux controls that were originally > > > >> > > found > > > >> > > in TCON0 and TVE0 were moved out. > > > >> > > > > > >> > > The driver needs to know about that, but the graph about doesn't > > > >> > > make > > > >> > > much sense directly. Without looking at the manual, I understand it > > > >> > > to > > > >> > > likely be one mux between the mixers and TCONs, and one between the > > > >> > > TCON-TVs and HDMI. Would it make more sense to just have the graph > > > >> > > connections between the muxed components, and remove TCON-TOP from > > > >> > > it, like we had in the past? A phandle could be used to reference > > > >> > > the TCON-TOP for mux controls, in addition to the clocks and > > > >> > > resets. > > > >> > > > > > >> > > For TVE, we would need something to represent each of the output > > > >> > > pins, > > > >> > > so the device tree can actually describe what kind of signal, be it > > > >> > > each component of RGB/YUV or composite video, is wanted on each > > > >> > > pin, > > > >> > > if any. This is also needed on the A20 for the Cubietruck, so we > > > >> > > can > > > >> > > describe which pins are tied to the VGA connector, and which one > > > >> > > does > > > >> > > R, G, or B. > > > >> > > > > >> > I guess we'll see how the DT maintainers feel about this, but my > > > >> > impression is that the OF graph should model the flow of data between > > > >> > the devices. If there's a mux somewhere, then the data is definitely > > > >> > going through it, and as such it should be part of the graph. > > > >> > > > >> I concur, but I spent few days thinking how to represent this sanely in > > > >> graph, but I didn't find any good solution. I'll represent here my > > > >> idea and please tell your opinion before I start implementing it. > > > >> > > > >> First, let me be clear that mixer0 and mixer1 don't have same > > > >> capabilities > > > >> (different number of planes, mixer0 supports writeback, mixer1 does > > > >> not, > > > >> etc.). Thus, it does matter which mixer is connected to which > > > >> TCON/encoder. > > > >> mixer0 is meant to be connected to main display and mixer1 to > > > >> auxiliary. That obviously depends on end system. > > > >> > > > >> So, TCON TOP has 3 muxes, which have to be represented in graph. Two of > > > >> them are for mixer/TCON relationship (each of them 1 input and 4 > > > >> possible outputs) and one for TV TCON/HDMI pair selection (2 possible > > > >> inputs, 1 output). > > > >> > > > >> According to current practice in sun4i-drm driver, graph has to have > > > >> port 0, representing input and port 1, representing output. This would > > > >> mean that graph looks something like that: > > > >> > > > >> tcon_top: tcon-top@1c70000 { > > > >> > > > >> compatible = "allwinner,sun8i-r40-tcon-top"; > > > >> ... > > > >> ports { > > > >> > > > >> #address-cells = <1>; > > > >> #size-cells = <0>; > > > >> > > > >> tcon_top_in: port@0 { > > > >> > > > >> #address-cells = <1>; > > > >> #size-cells = <0>; > > > >> reg = <0>; > > > >> > > > >> tcon_top_in_mixer0: endpoint@0 { > > > >> > > > >> reg = <0>; > > > >> remote-endpoint = <&mixer0_out_tcon_top>; > > > >> > > > >> }; > > > >> > > > >> tcon_top_in_mixer1: endpoint@1 { > > > >> > > > >> reg = <1>; > > > >> remote-endpoint = <&mixer1_out_tcon_top>; > > > >> > > > >> }; > > > >> > > > >> tcon_top_in_tcon_tv: endpoint@2 { > > > >> > > > >> reg = <2>; > > > >> // here is HDMI input connection, part of > > > >> board DTS > > > >> remote-endpoint = <board specific phandle > > > >> to TV TCON output>; > > > >> > > > >> }; > > > >> > > > >> }; > > > >> > > > >> tcon_top_out: port@1 { > > > >> > > > >> #address-cells = <1>; > > > >> #size-cells = <0>; > > > >> reg = <1>; > > > >> > > > >> tcon_top_out_tcon0: endpoint@0 { > > > >> > > > >> reg = <0>; > > > >> // here is mixer0 output connection, part > > > >> of board DTS > > > >> remote-endpoint = <board specific phandle > > > >> to TCON>; > > > >> > > > >> }; > > > >> > > > >> tcon_top_out_tcon1: endpoint@1 { > > > >> > > > >> reg = <1>; > > > >> // here is mixer1 output connection, part > > > >> of board DTS > > > >> remote-endpoint = <board specific phandle > > > >> to TCON>; > > > >> > > > >> }; > > > >> > > > >> tcon_top_out_hdmi: endpoint@2 { > > > >> > > > >> reg = <2>; > > > >> remote-endpoint = <&hdmi_in_tcon_top>; > > > >> > > > >> }; > > > >> > > > >> }; > > > >> > > > >> }; > > > >> > > > >> }; > > > > > > > > IIRC, each port is supposed to be one route for the data, so we would > > > > have multiple ports, one for the mixers in input and for the tcon in > > > > output, and one for the TCON in input and for the HDMI/TV in > > > > output. Rob might correct me here. > > Ok, that seems more clean approach. I'll have to extend graph traversing > algorithm in sun4i_drv.c, but that's no problem. > > Just to be clear, you have in mind 3 pairs of ports (0/1 for mixer0 mux, 2/3 > for mixer1 and 4/5 for HDMI input), right? That way each mux is represented > with one pair of ports, even numbered for input and odd numbered for output. Yep, unless Rob feels otherwise. Maxime
Dne ponedeljek, 04. junij 2018 ob 18:23:57 CEST je Maxime Ripard napisal(a): > On Mon, Jun 04, 2018 at 05:09:56PM +0200, Jernej Škrabec wrote: > > Dne ponedeljek, 04. junij 2018 ob 13:50:34 CEST je Maxime Ripard napisal(a): > > > On Fri, Jun 01, 2018 at 09:19:43AM -0700, Chen-Yu Tsai wrote: > > > > On Fri, Jun 1, 2018 at 8:29 AM, Maxime Ripard > > > > <maxime.ripard@bootlin.com> > > > > wrote: > > > > > On Thu, May 31, 2018 at 07:54:08PM +0200, Jernej Škrabec wrote: > > > > >> Dne četrtek, 31. maj 2018 ob 11:21:33 CEST je Maxime Ripard napisal(a): > > > > >> > On Thu, May 24, 2018 at 03:01:09PM -0700, Chen-Yu Tsai wrote: > > > > >> > > >> > > + if (tcon->quirks->needs_tcon_top) { > > > > >> > > >> > > + struct device_node *np; > > > > >> > > >> > > + > > > > >> > > >> > > + np = of_parse_phandle(dev->of_node, > > > > >> > > >> > > "allwinner,tcon-top", > > > > >> > > >> > > 0); > > > > >> > > >> > > + if (np) { > > > > >> > > >> > > + struct platform_device *pdev; > > > > >> > > >> > > + > > > > >> > > >> > > + pdev = of_find_device_by_node(np); > > > > >> > > >> > > + if (pdev) > > > > >> > > >> > > + tcon->tcon_top = > > > > >> > > >> > > platform_get_drvdata(pdev); > > > > >> > > >> > > + of_node_put(np); > > > > >> > > >> > > + > > > > >> > > >> > > + if (!tcon->tcon_top) > > > > >> > > >> > > + return -EPROBE_DEFER; > > > > >> > > >> > > + } > > > > >> > > >> > > + } > > > > >> > > >> > > + > > > > >> > > >> > > > > > >> > > >> > I might have missed it, but I've not seen the bindings > > > > >> > > >> > additions for > > > > >> > > >> > that property. This shouldn't really be done that way > > > > >> > > >> > anyway, > > > > >> > > >> > instead > > > > >> > > >> > of using a direct phandle, you should be using the > > > > >> > > >> > of-graph, > > > > >> > > >> > with the > > > > >> > > >> > TCON-top sitting where it belongs in the flow of data. > > > > >> > > >> > > > > >> > > >> Just to answer to the first question, it did describe it in > > > > >> > > >> "[PATCH > > > > >> > > >> 07/15] dt- bindings: display: sun4i-drm: Add R40 HDMI > > > > >> > > >> pipeline". > > > > >> > > >> > > > > >> > > >> As why I designed it that way - HW representation could be > > > > >> > > >> described > > > > >> > > >> that way> >> > > > > >> > > >> > > > > >> > > >> (ASCII art makes sense when fixed width font is used to view > > > > it): > > > > >> > > >> / LCD0/LVDS0 > > > > >> > > >> > > > > >> > > >> / TCON-LCD0 > > > > >> > > >> > > > > >> > > >> | \ MIPI DSI > > > > >> > > >> > > > > >> > > >> mixer0 | > > > > >> > > >> > > > > >> > > >> \ / TCON-LCD1 - LCD1/LVDS1 > > > > >> > > >> > > > > >> > > >> TCON-TOP > > > > >> > > >> > > > > >> > > >> / \ TCON-TV0 - TVE0/RGB > > > > >> > > >> > > > > >> > > >> mixer1 | \ > > > > >> > > >> > > > > >> > > >> | TCON-TOP - HDMI > > > > >> > > >> | > > > > >> > > >> | / > > > > >> > > >> > > > > >> > > >> \ TCON-TV1 - TVE1/RGB > > > > >> > > >> > > > > >> > > >> This is a bit simplified, since there is also TVE-TOP, which > > > > >> > > >> is > > > > >> > > >> responsible > > > > >> > > >> for sharing 4 DACs between both TVE encoders. You can have > > > > >> > > >> two > > > > >> > > >> TV outs > > > > >> > > >> (PAL/ NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. > > > > >> > > >> It > > > > >> > > >> even > > > > >> > > >> seems that you can arbitrarly choose which DAC is > > > > >> > > >> responsible > > > > >> > > >> for > > > > >> > > >> which signal, so there is a ton of possible end > > > > >> > > >> combinations, > > > > >> > > >> but I'm > > > > >> > > >> not 100% sure. > > > > >> > > >> > > > > >> > > >> Even though I wrote TCON-TOP twice, this is same unit in HW. > > > > >> > > >> R40 > > > > >> > > >> manual > > > > >> > > >> suggest more possibilities, although some of them seem > > > > >> > > >> wrong, > > > > >> > > >> like RGB > > > > >> > > >> feeding from LCD TCON. That is confirmed to be wrong when > > > > >> > > >> checking BSP > > > > >> > > >> code. > > > > >> > > >> > > > > >> > > >> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, > > > > >> > > >> TVE1 and > > > > >> > > >> LCD1 for pin muxing, although I'm not sure why is that > > > > >> > > >> needed at > > > > >> > > >> all, > > > > >> > > >> since according to R40 datasheet, TVE0 and TVE1 pins are > > > > >> > > >> dedicated and > > > > >> > > >> not on PORT D and PORT H, respectively, as TCON-TOP > > > > >> > > >> documentation > > > > >> > > >> suggest. However, HSYNC and PSYNC lines might be shared > > > > >> > > >> between > > > > >> > > >> TVE > > > > >> > > >> (when it works in RGB mode) and LCD. But that is just my > > > > >> > > >> guess > > > > >> > > >> since > > > > >> > > >> I'm not really familiar with RGB and LCD interfaces. > > > > >> > > >> > > > > >> > > >> I'm really not sure what would be the best representation in > > > > >> > > >> OF-graph. > > > > >> > > >> Can you suggest one? > > > > >> > > > > > > > >> > > > Rob might disagree on this one, but I don't see anything > > > > >> > > > wrong > > > > >> > > > with > > > > >> > > > having loops in the graph. If the TCON-TOP can be both the > > > > >> > > > input > > > > >> > > > and > > > > >> > > > output of the TCONs, then so be it, and have it described > > > > >> > > > that > > > > >> > > > way in > > > > >> > > > the graph. > > > > >> > > > > > > > >> > > > The code is already able to filter out nodes that have > > > > >> > > > already > > > > >> > > > been > > > > >> > > > added to the list of devices we need to wait for in the > > > > >> > > > component > > > > >> > > > framework, so that should work as well. > > > > >> > > > > > > > >> > > > And we'd need to describe TVE-TOP as well, even though we > > > > >> > > > don't > > > > >> > > > have a > > > > >> > > > driver for it yet. That will simplify the backward > > > > >> > > > compatibility > > > > >> > > > later > > > > >> > > > on. > > > > >> > > > > > > >> > > I'm getting the feeling that TCON-TOP / TVE-TOP is the glue > > > > >> > > layer > > > > >> > > that > > > > >> > > binds everything together, and provides signal routing, kind of > > > > >> > > like > > > > >> > > DE-TOP on A64. So the signal mux controls that were originally > > > > >> > > found > > > > >> > > in TCON0 and TVE0 were moved out. > > > > >> > > > > > > >> > > The driver needs to know about that, but the graph about > > > > >> > > doesn't > > > > >> > > make > > > > >> > > much sense directly. Without looking at the manual, I > > > > >> > > understand it > > > > >> > > to > > > > >> > > likely be one mux between the mixers and TCONs, and one between > > > > >> > > the > > > > >> > > TCON-TVs and HDMI. Would it make more sense to just have the > > > > >> > > graph > > > > >> > > connections between the muxed components, and remove TCON-TOP > > > > >> > > from > > > > >> > > it, like we had in the past? A phandle could be used to > > > > >> > > reference > > > > >> > > the TCON-TOP for mux controls, in addition to the clocks and > > > > >> > > resets. > > > > >> > > > > > > >> > > For TVE, we would need something to represent each of the > > > > >> > > output > > > > >> > > pins, > > > > >> > > so the device tree can actually describe what kind of signal, > > > > >> > > be it > > > > >> > > each component of RGB/YUV or composite video, is wanted on each > > > > >> > > pin, > > > > >> > > if any. This is also needed on the A20 for the Cubietruck, so > > > > >> > > we > > > > >> > > can > > > > >> > > describe which pins are tied to the VGA connector, and which > > > > >> > > one > > > > >> > > does > > > > >> > > R, G, or B. > > > > >> > > > > > >> > I guess we'll see how the DT maintainers feel about this, but my > > > > >> > impression is that the OF graph should model the flow of data > > > > >> > between > > > > >> > the devices. If there's a mux somewhere, then the data is > > > > >> > definitely > > > > >> > going through it, and as such it should be part of the graph. > > > > >> > > > > >> I concur, but I spent few days thinking how to represent this > > > > >> sanely in > > > > >> graph, but I didn't find any good solution. I'll represent here my > > > > >> idea and please tell your opinion before I start implementing it. > > > > >> > > > > >> First, let me be clear that mixer0 and mixer1 don't have same > > > > >> capabilities > > > > >> (different number of planes, mixer0 supports writeback, mixer1 does > > > > >> not, > > > > >> etc.). Thus, it does matter which mixer is connected to which > > > > >> TCON/encoder. > > > > >> mixer0 is meant to be connected to main display and mixer1 to > > > > >> auxiliary. That obviously depends on end system. > > > > >> > > > > >> So, TCON TOP has 3 muxes, which have to be represented in graph. > > > > >> Two of > > > > >> them are for mixer/TCON relationship (each of them 1 input and 4 > > > > >> possible outputs) and one for TV TCON/HDMI pair selection (2 > > > > >> possible > > > > >> inputs, 1 output). > > > > >> > > > > >> According to current practice in sun4i-drm driver, graph has to > > > > >> have > > > > >> port 0, representing input and port 1, representing output. This > > > > >> would > > > > >> mean that graph looks something like that: > > > > >> > > > > >> tcon_top: tcon-top@1c70000 { > > > > >> > > > > >> compatible = "allwinner,sun8i-r40-tcon-top"; > > > > >> ... > > > > >> ports { > > > > >> > > > > >> #address-cells = <1>; > > > > >> #size-cells = <0>; > > > > >> > > > > >> tcon_top_in: port@0 { > > > > >> > > > > >> #address-cells = <1>; > > > > >> #size-cells = <0>; > > > > >> reg = <0>; > > > > >> > > > > >> tcon_top_in_mixer0: endpoint@0 { > > > > >> > > > > >> reg = <0>; > > > > >> remote-endpoint = > > > > >> <&mixer0_out_tcon_top>; > > > > >> > > > > >> }; > > > > >> > > > > >> tcon_top_in_mixer1: endpoint@1 { > > > > >> > > > > >> reg = <1>; > > > > >> remote-endpoint = > > > > >> <&mixer1_out_tcon_top>; > > > > >> > > > > >> }; > > > > >> > > > > >> tcon_top_in_tcon_tv: endpoint@2 { > > > > >> > > > > >> reg = <2>; > > > > >> // here is HDMI input connection, > > > > >> part of > > > > >> board DTS > > > > >> remote-endpoint = <board specific > > > > >> phandle > > > > >> to TV TCON output>; > > > > >> > > > > >> }; > > > > >> > > > > >> }; > > > > >> > > > > >> tcon_top_out: port@1 { > > > > >> > > > > >> #address-cells = <1>; > > > > >> #size-cells = <0>; > > > > >> reg = <1>; > > > > >> > > > > >> tcon_top_out_tcon0: endpoint@0 { > > > > >> > > > > >> reg = <0>; > > > > >> // here is mixer0 output connection, > > > > >> part > > > > >> of board DTS > > > > >> remote-endpoint = <board specific > > > > >> phandle > > > > >> to TCON>; > > > > >> > > > > >> }; > > > > >> > > > > >> tcon_top_out_tcon1: endpoint@1 { > > > > >> > > > > >> reg = <1>; > > > > >> // here is mixer1 output connection, > > > > >> part > > > > >> of board DTS > > > > >> remote-endpoint = <board specific > > > > >> phandle > > > > >> to TCON>; > > > > >> > > > > >> }; > > > > >> > > > > >> tcon_top_out_hdmi: endpoint@2 { > > > > >> > > > > >> reg = <2>; > > > > >> remote-endpoint = > > > > >> <&hdmi_in_tcon_top>; > > > > >> > > > > >> }; > > > > >> > > > > >> }; > > > > >> > > > > >> }; > > > > >> > > > > >> }; > > > > > > > > > > IIRC, each port is supposed to be one route for the data, so we > > > > > would > > > > > have multiple ports, one for the mixers in input and for the tcon in > > > > > output, and one for the TCON in input and for the HDMI/TV in > > > > > output. Rob might correct me here. > > > > Ok, that seems more clean approach. I'll have to extend graph traversing > > algorithm in sun4i_drv.c, but that's no problem. > > > > Just to be clear, you have in mind 3 pairs of ports (0/1 for mixer0 mux, > > 2/3 for mixer1 and 4/5 for HDMI input), right? That way each mux is > > represented with one pair of ports, even numbered for input and odd > > numbered for output. > Yep, unless Rob feels otherwise. I found an issue with this concept. HDMI driver (sun8i_dw_hdmi.c) uses drm_of_find_possible_crtcs() to find connected crtcs (TCONs) to HDMI. This function assumes that crtc and encoder are directly connected through of_graph, but that is not the case with TCON TOP HDMI mux anymore. I could give TCON TOP node as an input to this function, but that won't work, since TCON TOP can have connections to other crtcs, not only that of HDMI and they will also be picked up by drm_of_find_possible_crtcs(). Any suggestion how to solve this nicely? I think creating my own version of drm_of_find_possible_crtcs() which considers that case is one way, but not very nice solution. Alternatively, we can fix possible_crtcs to BIT(0), since it always has only one input. This is done in meson_dw_hdmi.c for example. Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne četrtek, 07. junij 2018 ob 00:30:24 CEST je Jernej Škrabec napisal(a): > Dne ponedeljek, 04. junij 2018 ob 18:23:57 CEST je Maxime Ripard napisal(a): > > On Mon, Jun 04, 2018 at 05:09:56PM +0200, Jernej Škrabec wrote: > > > Dne ponedeljek, 04. junij 2018 ob 13:50:34 CEST je Maxime Ripard > > napisal(a): > > > > On Fri, Jun 01, 2018 at 09:19:43AM -0700, Chen-Yu Tsai wrote: > > > > > On Fri, Jun 1, 2018 at 8:29 AM, Maxime Ripard > > > > > <maxime.ripard@bootlin.com> > > > > > > wrote: > > > > > > On Thu, May 31, 2018 at 07:54:08PM +0200, Jernej Škrabec wrote: > > > > > >> Dne četrtek, 31. maj 2018 ob 11:21:33 CEST je Maxime Ripard > > napisal(a): > > > > > >> > On Thu, May 24, 2018 at 03:01:09PM -0700, Chen-Yu Tsai wrote: > > > > > >> > > >> > > + if (tcon->quirks->needs_tcon_top) { > > > > > >> > > >> > > + struct device_node *np; > > > > > >> > > >> > > + > > > > > >> > > >> > > + np = of_parse_phandle(dev->of_node, > > > > > >> > > >> > > "allwinner,tcon-top", > > > > > >> > > >> > > 0); > > > > > >> > > >> > > + if (np) { > > > > > >> > > >> > > + struct platform_device *pdev; > > > > > >> > > >> > > + > > > > > >> > > >> > > + pdev = of_find_device_by_node(np); > > > > > >> > > >> > > + if (pdev) > > > > > >> > > >> > > + tcon->tcon_top = > > > > > >> > > >> > > platform_get_drvdata(pdev); > > > > > >> > > >> > > + of_node_put(np); > > > > > >> > > >> > > + > > > > > >> > > >> > > + if (!tcon->tcon_top) > > > > > >> > > >> > > + return -EPROBE_DEFER; > > > > > >> > > >> > > + } > > > > > >> > > >> > > + } > > > > > >> > > >> > > + > > > > > >> > > >> > > > > > > >> > > >> > I might have missed it, but I've not seen the bindings > > > > > >> > > >> > additions for > > > > > >> > > >> > that property. This shouldn't really be done that way > > > > > >> > > >> > anyway, > > > > > >> > > >> > instead > > > > > >> > > >> > of using a direct phandle, you should be using the > > > > > >> > > >> > of-graph, > > > > > >> > > >> > with the > > > > > >> > > >> > TCON-top sitting where it belongs in the flow of data. > > > > > >> > > >> > > > > > >> > > >> Just to answer to the first question, it did describe it > > > > > >> > > >> in > > > > > >> > > >> "[PATCH > > > > > >> > > >> 07/15] dt- bindings: display: sun4i-drm: Add R40 HDMI > > > > > >> > > >> pipeline". > > > > > >> > > >> > > > > > >> > > >> As why I designed it that way - HW representation could be > > > > > >> > > >> described > > > > > >> > > >> that way> >> > > > > > >> > > >> > > > > > >> > > >> (ASCII art makes sense when fixed width font is used to > > > > > >> > > >> view > > > > > > it): > > > > > >> > > >> / LCD0/LVDS0 > > > > > >> > > >> > > > > > >> > > >> / TCON-LCD0 > > > > > >> > > >> > > > > > >> > > >> | \ MIPI DSI > > > > > >> > > >> > > > > > >> > > >> mixer0 | > > > > > >> > > >> > > > > > >> > > >> \ / TCON-LCD1 - LCD1/LVDS1 > > > > > >> > > >> > > > > > >> > > >> TCON-TOP > > > > > >> > > >> > > > > > >> > > >> / \ TCON-TV0 - TVE0/RGB > > > > > >> > > >> > > > > > >> > > >> mixer1 | \ > > > > > >> > > >> > > > > > >> > > >> | TCON-TOP - HDMI > > > > > >> > > >> | > > > > > >> > > >> | / > > > > > >> > > >> > > > > > >> > > >> \ TCON-TV1 - TVE1/RGB > > > > > >> > > >> > > > > > >> > > >> This is a bit simplified, since there is also TVE-TOP, > > > > > >> > > >> which > > > > > >> > > >> is > > > > > >> > > >> responsible > > > > > >> > > >> for sharing 4 DACs between both TVE encoders. You can have > > > > > >> > > >> two > > > > > >> > > >> TV outs > > > > > >> > > >> (PAL/ NTSC) or TVE0 as TV out and TVE1 as RGB or vice > > > > > >> > > >> versa. > > > > > >> > > >> It > > > > > >> > > >> even > > > > > >> > > >> seems that you can arbitrarly choose which DAC is > > > > > >> > > >> responsible > > > > > >> > > >> for > > > > > >> > > >> which signal, so there is a ton of possible end > > > > > >> > > >> combinations, > > > > > >> > > >> but I'm > > > > > >> > > >> not 100% sure. > > > > > >> > > >> > > > > > >> > > >> Even though I wrote TCON-TOP twice, this is same unit in > > > > > >> > > >> HW. > > > > > >> > > >> R40 > > > > > >> > > >> manual > > > > > >> > > >> suggest more possibilities, although some of them seem > > > > > >> > > >> wrong, > > > > > >> > > >> like RGB > > > > > >> > > >> feeding from LCD TCON. That is confirmed to be wrong when > > > > > >> > > >> checking BSP > > > > > >> > > >> code. > > > > > >> > > >> > > > > > >> > > >> Additionally, TCON-TOP comes in the middle of TVE0 and > > > > > >> > > >> LCD0, > > > > > >> > > >> TVE1 and > > > > > >> > > >> LCD1 for pin muxing, although I'm not sure why is that > > > > > >> > > >> needed at > > > > > >> > > >> all, > > > > > >> > > >> since according to R40 datasheet, TVE0 and TVE1 pins are > > > > > >> > > >> dedicated and > > > > > >> > > >> not on PORT D and PORT H, respectively, as TCON-TOP > > > > > >> > > >> documentation > > > > > >> > > >> suggest. However, HSYNC and PSYNC lines might be shared > > > > > >> > > >> between > > > > > >> > > >> TVE > > > > > >> > > >> (when it works in RGB mode) and LCD. But that is just my > > > > > >> > > >> guess > > > > > >> > > >> since > > > > > >> > > >> I'm not really familiar with RGB and LCD interfaces. > > > > > >> > > >> > > > > > >> > > >> I'm really not sure what would be the best representation > > > > > >> > > >> in > > > > > >> > > >> OF-graph. > > > > > >> > > >> Can you suggest one? > > > > > >> > > > > > > > > >> > > > Rob might disagree on this one, but I don't see anything > > > > > >> > > > wrong > > > > > >> > > > with > > > > > >> > > > having loops in the graph. If the TCON-TOP can be both the > > > > > >> > > > input > > > > > >> > > > and > > > > > >> > > > output of the TCONs, then so be it, and have it described > > > > > >> > > > that > > > > > >> > > > way in > > > > > >> > > > the graph. > > > > > >> > > > > > > > > >> > > > The code is already able to filter out nodes that have > > > > > >> > > > already > > > > > >> > > > been > > > > > >> > > > added to the list of devices we need to wait for in the > > > > > >> > > > component > > > > > >> > > > framework, so that should work as well. > > > > > >> > > > > > > > > >> > > > And we'd need to describe TVE-TOP as well, even though we > > > > > >> > > > don't > > > > > >> > > > have a > > > > > >> > > > driver for it yet. That will simplify the backward > > > > > >> > > > compatibility > > > > > >> > > > later > > > > > >> > > > on. > > > > > >> > > > > > > > >> > > I'm getting the feeling that TCON-TOP / TVE-TOP is the glue > > > > > >> > > layer > > > > > >> > > that > > > > > >> > > binds everything together, and provides signal routing, kind > > > > > >> > > of > > > > > >> > > like > > > > > >> > > DE-TOP on A64. So the signal mux controls that were > > > > > >> > > originally > > > > > >> > > found > > > > > >> > > in TCON0 and TVE0 were moved out. > > > > > >> > > > > > > > >> > > The driver needs to know about that, but the graph about > > > > > >> > > doesn't > > > > > >> > > make > > > > > >> > > much sense directly. Without looking at the manual, I > > > > > >> > > understand it > > > > > >> > > to > > > > > >> > > likely be one mux between the mixers and TCONs, and one > > > > > >> > > between > > > > > >> > > the > > > > > >> > > TCON-TVs and HDMI. Would it make more sense to just have the > > > > > >> > > graph > > > > > >> > > connections between the muxed components, and remove TCON-TOP > > > > > >> > > from > > > > > >> > > it, like we had in the past? A phandle could be used to > > > > > >> > > reference > > > > > >> > > the TCON-TOP for mux controls, in addition to the clocks and > > > > > >> > > resets. > > > > > >> > > > > > > > >> > > For TVE, we would need something to represent each of the > > > > > >> > > output > > > > > >> > > pins, > > > > > >> > > so the device tree can actually describe what kind of signal, > > > > > >> > > be it > > > > > >> > > each component of RGB/YUV or composite video, is wanted on > > > > > >> > > each > > > > > >> > > pin, > > > > > >> > > if any. This is also needed on the A20 for the Cubietruck, so > > > > > >> > > we > > > > > >> > > can > > > > > >> > > describe which pins are tied to the VGA connector, and which > > > > > >> > > one > > > > > >> > > does > > > > > >> > > R, G, or B. > > > > > >> > > > > > > >> > I guess we'll see how the DT maintainers feel about this, but > > > > > >> > my > > > > > >> > impression is that the OF graph should model the flow of data > > > > > >> > between > > > > > >> > the devices. If there's a mux somewhere, then the data is > > > > > >> > definitely > > > > > >> > going through it, and as such it should be part of the graph. > > > > > >> > > > > > >> I concur, but I spent few days thinking how to represent this > > > > > >> sanely in > > > > > >> graph, but I didn't find any good solution. I'll represent here > > > > > >> my > > > > > >> idea and please tell your opinion before I start implementing it. > > > > > >> > > > > > >> First, let me be clear that mixer0 and mixer1 don't have same > > > > > >> capabilities > > > > > >> (different number of planes, mixer0 supports writeback, mixer1 > > > > > >> does > > > > > >> not, > > > > > >> etc.). Thus, it does matter which mixer is connected to which > > > > > >> TCON/encoder. > > > > > >> mixer0 is meant to be connected to main display and mixer1 to > > > > > >> auxiliary. That obviously depends on end system. > > > > > >> > > > > > >> So, TCON TOP has 3 muxes, which have to be represented in graph. > > > > > >> Two of > > > > > >> them are for mixer/TCON relationship (each of them 1 input and 4 > > > > > >> possible outputs) and one for TV TCON/HDMI pair selection (2 > > > > > >> possible > > > > > >> inputs, 1 output). > > > > > >> > > > > > >> According to current practice in sun4i-drm driver, graph has to > > > > > >> have > > > > > >> port 0, representing input and port 1, representing output. This > > > > > >> would > > > > > >> mean that graph looks something like that: > > > > > >> > > > > > >> tcon_top: tcon-top@1c70000 { > > > > > >> > > > > > >> compatible = "allwinner,sun8i-r40-tcon-top"; > > > > > >> ... > > > > > >> ports { > > > > > >> > > > > > >> #address-cells = <1>; > > > > > >> #size-cells = <0>; > > > > > >> > > > > > >> tcon_top_in: port@0 { > > > > > >> > > > > > >> #address-cells = <1>; > > > > > >> #size-cells = <0>; > > > > > >> reg = <0>; > > > > > >> > > > > > >> tcon_top_in_mixer0: endpoint@0 { > > > > > >> > > > > > >> reg = <0>; > > > > > >> remote-endpoint = > > > > > >> <&mixer0_out_tcon_top>; > > > > > >> > > > > > >> }; > > > > > >> > > > > > >> tcon_top_in_mixer1: endpoint@1 { > > > > > >> > > > > > >> reg = <1>; > > > > > >> remote-endpoint = > > > > > >> <&mixer1_out_tcon_top>; > > > > > >> > > > > > >> }; > > > > > >> > > > > > >> tcon_top_in_tcon_tv: endpoint@2 { > > > > > >> > > > > > >> reg = <2>; > > > > > >> // here is HDMI input connection, > > > > > >> part of > > > > > >> board DTS > > > > > >> remote-endpoint = <board specific > > > > > >> phandle > > > > > >> to TV TCON output>; > > > > > >> > > > > > >> }; > > > > > >> > > > > > >> }; > > > > > >> > > > > > >> tcon_top_out: port@1 { > > > > > >> > > > > > >> #address-cells = <1>; > > > > > >> #size-cells = <0>; > > > > > >> reg = <1>; > > > > > >> > > > > > >> tcon_top_out_tcon0: endpoint@0 { > > > > > >> > > > > > >> reg = <0>; > > > > > >> // here is mixer0 output > > > > > >> connection, > > > > > >> part > > > > > >> of board DTS > > > > > >> remote-endpoint = <board specific > > > > > >> phandle > > > > > >> to TCON>; > > > > > >> > > > > > >> }; > > > > > >> > > > > > >> tcon_top_out_tcon1: endpoint@1 { > > > > > >> > > > > > >> reg = <1>; > > > > > >> // here is mixer1 output > > > > > >> connection, > > > > > >> part > > > > > >> of board DTS > > > > > >> remote-endpoint = <board specific > > > > > >> phandle > > > > > >> to TCON>; > > > > > >> > > > > > >> }; > > > > > >> > > > > > >> tcon_top_out_hdmi: endpoint@2 { > > > > > >> > > > > > >> reg = <2>; > > > > > >> remote-endpoint = > > > > > >> <&hdmi_in_tcon_top>; > > > > > >> > > > > > >> }; > > > > > >> > > > > > >> }; > > > > > >> > > > > > >> }; > > > > > >> > > > > > >> }; > > > > > > > > > > > > IIRC, each port is supposed to be one route for the data, so we > > > > > > would > > > > > > have multiple ports, one for the mixers in input and for the tcon > > > > > > in > > > > > > output, and one for the TCON in input and for the HDMI/TV in > > > > > > output. Rob might correct me here. > > > > > > Ok, that seems more clean approach. I'll have to extend graph traversing > > > algorithm in sun4i_drv.c, but that's no problem. > > > > > > Just to be clear, you have in mind 3 pairs of ports (0/1 for mixer0 mux, > > > 2/3 for mixer1 and 4/5 for HDMI input), right? That way each mux is > > > represented with one pair of ports, even numbered for input and odd > > > numbered for output. > > > > Yep, unless Rob feels otherwise. > > I found an issue with this concept. > > HDMI driver (sun8i_dw_hdmi.c) uses drm_of_find_possible_crtcs() to find > connected crtcs (TCONs) to HDMI. This function assumes that crtc and encoder > are directly connected through of_graph, but that is not the case with TCON > TOP HDMI mux anymore. > I could give TCON TOP node as an input to this function, but that won't > work, since TCON TOP can have connections to other crtcs, not only that of > HDMI and they will also be picked up by drm_of_find_possible_crtcs(). > > Any suggestion how to solve this nicely? I think creating my own version of > drm_of_find_possible_crtcs() which considers that case is one way, but not > very nice solution. Alternatively, we can fix possible_crtcs to BIT(0), This doesn't seem like a good solution, since it doesn't work in dual head system. I'll take first approach. Best regards, Jernej > since it always has only one input. This is done in meson_dw_hdmi.c for > example. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
This series adds support for R40 HDMI pipeline. It is a bit special than other already supported pipelines because it has additional unit called TCON TOP responsible for relationship configuration between mixers, TCONs and HDMI. Additionally, it has additional gates for DSI and TV TCONs, TV encoder clock settings and pin muxing between LCD and TV encoders. However, it seems that TCON TOP will become a norm, since newer Allwinner SoCs like H6 also have this unit. I tested different possible configurations: - mixer0 <> TCON-TV0 <> HDMI - mixer0 <> TCON-TV1 <> HDMI - mixer1 <> TCON-TV0 <> HDMI - mixer1 <> TCON-TV1 <> HDMI Please review. Best regards, Jernej Jernej Skrabec (15): clk: sunxi-ng: r40: Add minimal rate for video PLLs clk: sunxi-ng: r40: Allow setting parent rate to display related clocks clk: sunxi-ng: r40: Export video PLLs dt-bindings: display: sunxi-drm: Add TCON TOP description drm/sun4i: Add TCON TOP driver drm/sun4i: tcon: Add support for tcon-top dt-bindings: display: sun4i-drm: Add R40 HDMI pipeline drm/sun4i: DE2 mixer: Add index quirk drm/sun4i: Add support for R40 mixers drm/sun4i: Add support for R40 TV TCONs drm/sun4i: DW HDMI PHY: Add support for second PLL drm/sun4i: Add support for second clock parent to DW HDMI PHY clk driver drm/sun4i: Add support for A64 HDMI PHY ARM: dts: sun8i: r40: Add HDMI pipeline ARM: dts: sun8i: r40: Enable HDMI output on BananaPi M2 Ultra .../bindings/display/sunxi/sun4i-drm.txt | 36 ++- .../boot/dts/sun8i-r40-bananapi-m2-ultra.dts | 50 ++++ arch/arm/boot/dts/sun8i-r40.dtsi | 166 ++++++++++++ drivers/clk/sunxi-ng/ccu-sun8i-r40.c | 58 ++-- drivers/clk/sunxi-ng/ccu-sun8i-r40.h | 8 +- drivers/gpu/drm/sun4i/Makefile | 3 +- drivers/gpu/drm/sun4i/sun4i_tcon.c | 67 +++++ drivers/gpu/drm/sun4i/sun4i_tcon.h | 8 + drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 8 +- drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 61 ++++- drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c | 90 ++++-- drivers/gpu/drm/sun4i/sun8i_mixer.c | 30 +- drivers/gpu/drm/sun4i/sun8i_mixer.h | 2 + drivers/gpu/drm/sun4i/sun8i_tcon_top.c | 256 ++++++++++++++++++ drivers/gpu/drm/sun4i/sun8i_tcon_top.h | 20 ++ include/dt-bindings/clock/sun8i-r40-ccu.h | 4 + include/dt-bindings/clock/sun8i-tcon-top.h | 11 + 17 files changed, 813 insertions(+), 65 deletions(-) create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.c create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.h create mode 100644 include/dt-bindings/clock/sun8i-tcon-top.h