| Message ID | 20170926065919.24446-1-wens@csie.org |
|---|---|
| Headers | show |
| Series | drm/sun4i: hdmi: Support HDMI controller on A31 | expand |
On Tue, Sep 26, 2017 at 06:59:07AM +0000, Chen-Yu Tsai wrote: > The 2x outputs of the 2 video PLL clocks are directly used by the > HDMI controller block. > > Export them so they can be referenced in the device tree. > > Fixes: c6e6c96d8fa6 ("clk: sunxi-ng: Add A31/A31s clocks") > Signed-off-by: Chen-Yu Tsai <wens@csie.org> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> Thanks! Maxime
On Tue, Sep 26, 2017 at 06:59:08AM +0000, Chen-Yu Tsai wrote: > The HDMI DDC clock found in the CCU is the parent of the actual DDC > clock within the HDMI controller. That clock is also named "hdmi-ddc". > > Rename the one in the CCU to "hdmi-ddc-parent". This makes more sense > than renaming the one in the HDMI controller to something else. > > Fixes: c6e6c96d8fa6 ("clk: sunxi-ng: Add A31/A31s clocks") > Signed-off-by: Chen-Yu Tsai <wens@csie.org> I'd rather stick to the datasheet names. What about "DDC" ? Maxime
Hi, On Tue, Sep 26, 2017 at 06:59:09AM +0000, Chen-Yu Tsai wrote: > On systems with 2 TCONs such as the A31, it is possible to demux the > output of the TCONs to one encoder. > > Add support for this for the A31. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > --- > drivers/gpu/drm/sun4i/sun4i_tcon.c | 61 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 61 insertions(+) > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c > index e853dfe51389..770b843a6fa9 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -14,9 +14,12 @@ > #include <drm/drm_atomic_helper.h> > #include <drm/drm_crtc.h> > #include <drm/drm_crtc_helper.h> > +#include <drm/drm_encoder.h> > #include <drm/drm_modes.h> > #include <drm/drm_of.h> > > +#include <uapi/drm/drm_mode.h> > + > #include <linux/component.h> > #include <linux/ioport.h> > #include <linux/of_address.h> > @@ -109,11 +112,69 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable) > } > EXPORT_SYMBOL(sun4i_tcon_enable_vblank); > > +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm) > +{ > + struct sun4i_drv *drv = drm->dev_private; > + struct sun4i_tcon *tcon; > + > + list_for_each_entry(tcon, &drv->tcon_list, list) > + if (tcon->id == 0) > + return tcon; > + > + dev_warn(drm->dev, > + "TCON0 not found, display output muxing may not work\n"); > + > + return tcon; > +} > + > +static int _sun6i_tcon_set_mux(struct drm_encoder *encoder) > +{ > + struct sun4i_tcon *tcon = sun4i_get_first_tcon(encoder->dev); > + int tcon_id = drm_crtc_to_sun4i_crtc(encoder->crtc)->tcon->id; > + u32 shift; > + > + DRM_DEBUG_DRIVER("Muxing encoder %s to CRTC %s (TCON %d)\n", > + encoder->name, encoder->crtc->name, tcon_id); > + > + /* Only 2 TCONs */ > + if (tcon_id >= 2) > + return -EINVAL; > + > + switch (encoder->encoder_type) { > + case DRM_MODE_ENCODER_TMDS: > + /* HDMI */ > + shift = 8; > + break; > + case DRM_MODE_ENCODER_DSI: > + /* No MIPI DSI on A31s */ > + if (of_device_is_compatible(tcon->dev->of_node, > + "allwinner,sun6i-a31s-tcon")) I'm not sure that test is needed. We won't end up in that case if we don't have a connected DSI block, which isn't going to be the case on the A31. And I guess we can tackle DSI later (when I'll send my patches...). > + return -EINVAL; > + shift = 0; > + break; > + default: > + return -EINVAL; > + } > + > + regmap_update_bits(tcon->regs, SUN4I_TCON_MUX_CTRL_REG, > + 0x3 << shift, tcon_id << shift); > + > + return 0; > +} > + > void sun4i_tcon_set_mux(struct sun4i_tcon *tcon, int channel, > struct drm_encoder *encoder) > { > + /* Get the device node of the display engine */ > + struct device_node *node = encoder->dev->dev->of_node; > u32 val; > > + if (of_device_is_compatible(node, "allwinner,sun6i-a31-display-engine") || > + of_device_is_compatible(node, "allwinner,sun6i-a31s-display-engine")) { > + _sun6i_tcon_set_mux(encoder); > + return; > + } > + I'd really like to avoid mix and matching the structure defined behaviour and those of_device_is_compatible calls spread out everywhere. You can either add a flag or a function pointer. Maxime
On Tue, Sep 26, 2017 at 06:59:10AM +0000, Chen-Yu Tsai wrote: > The HDMI driver enables the bus and mod clocks in the bind function, but > does not disable them if it then bails our due to any errors. Neither > does it disable the clocks in the unbind function. > > Fix this by adding a proper error path to the bind function, and > clk_disable_unprepare calls to the unbind function. > > Also rename the err_cleanup_connector label to err_cleanup_encoder, > since it is the encoder that gets cleaned up. > > Fixes: 9c5681011a0c ("drm/sun4i: Add HDMI support") > Signed-off-by: Chen-Yu Tsai <wens@csie.org> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> Maxime
On Tue, Sep 26, 2017 at 06:59:11AM +0000, Chen-Yu Tsai wrote: > The HDMI driver is written with readl/writel I/O to the registers. > However, to support the A31 variant, which has a different layout > for the DDC registers, it was recommended to use regfields to have > a cleaner implementation. To use regfields, we need to create an > underlying regmap. > > This patch only adds the regmap. It does not convert the existing > driver accesses to use regmap. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> Maxime
On Tue, Sep 26, 2017 at 06:59:12AM +0000, Chen-Yu Tsai wrote: > Allwinner SoCs typically have two PLLs reserved for video related usage. > At the moment we only support using the first one to feed the HDMI > transmitter block's TMDS clock. > > Let the HDMI encoder's TMDS clock go through all of its parents when > calculating possible clock rates. This allows usage of the second video > PLL as its parent. > > Note that this does not handle conflicting pixel clocks. It is entirely > possible to have an LCD panel use one pixel clock rate, only to be > overridden by the HDMI transmitter's clock rate request when the second > display pipeline is enabled. > > This should be handled by having all the clock drivers honor clock rate > ranges, and have the consumers use clk_set_rate_min/clk_set_rate_max. That, or relying on clk_set_rate_protect Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> Maxime
On Tue, Sep 26, 2017 at 06:59:15AM +0000, Chen-Yu Tsai wrote: > The HDMI controller found in earlier Allwinner SoCs have slight > differences between the A10, A10s, and the A31: > > - Need different initial values for the PLL related registers > > - Different behavior of the DDC and TMDS clocks > > - Different register layout for the DDC portion > > - Separate DDC parent clock on the A31 > > - Explicit reset control > > For the A31, the HDMI TMDS clock has a different value offset for > the divider. The HDMI DDC block is different from the one in the > other SoCs. As far as the DDC clock goes, it has no pre-divider, > as it is clocked from a slower parent clock, not the TMDS clock. > The divider offset from the register value is different. And the > clock control register is at a different offset. > > A new variant data structure is created to store pointers to the > above functions, structures, and the different initial values. > Another flag notates whether there is a separate DDC parent clock. > If not, the TMDS clock is passed to the DDC clock create function, > as before. > > Regmap fields are used to deal with the different register layout > of the DDC block. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> Maxime
On Tue, Sep 26, 2017 at 06:59:16AM +0000, Chen-Yu Tsai wrote: > The DDC block for the HDMI controller is different on the A31. > > This patch adds the register definitions. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> Maxime
On Tue, Sep 26, 2017 at 06:59:17AM +0000, Chen-Yu Tsai wrote: > The HDMI controller found in the A31 SoCs is slightly different > from the one already supported, which is found in the A10s: > > - Need different initial values for the PLL related registers > > - Different behavior of the DDC and TMDS clocks > > - Different register layout for the DDC portion > > - Separate DDC parent clock > > This patch adds support for it. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> Thanks! Maxime
On Tue, Sep 26, 2017 at 5:32 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Tue, Sep 26, 2017 at 06:59:08AM +0000, Chen-Yu Tsai wrote: >> The HDMI DDC clock found in the CCU is the parent of the actual DDC >> clock within the HDMI controller. That clock is also named "hdmi-ddc". >> >> Rename the one in the CCU to "hdmi-ddc-parent". This makes more sense >> than renaming the one in the HDMI controller to something else. >> >> Fixes: c6e6c96d8fa6 ("clk: sunxi-ng: Add A31/A31s clocks") >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > I'd rather stick to the datasheet names. What about "DDC" ? Works for me. Thanks 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 Tue, Sep 26, 2017 at 5:56 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > Hi, > > On Tue, Sep 26, 2017 at 06:59:09AM +0000, Chen-Yu Tsai wrote: >> On systems with 2 TCONs such as the A31, it is possible to demux the >> output of the TCONs to one encoder. >> >> Add support for this for the A31. >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> --- >> drivers/gpu/drm/sun4i/sun4i_tcon.c | 61 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 61 insertions(+) >> >> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> index e853dfe51389..770b843a6fa9 100644 >> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> @@ -14,9 +14,12 @@ >> #include <drm/drm_atomic_helper.h> >> #include <drm/drm_crtc.h> >> #include <drm/drm_crtc_helper.h> >> +#include <drm/drm_encoder.h> >> #include <drm/drm_modes.h> >> #include <drm/drm_of.h> >> >> +#include <uapi/drm/drm_mode.h> >> + >> #include <linux/component.h> >> #include <linux/ioport.h> >> #include <linux/of_address.h> >> @@ -109,11 +112,69 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable) >> } >> EXPORT_SYMBOL(sun4i_tcon_enable_vblank); >> >> +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm) >> +{ >> + struct sun4i_drv *drv = drm->dev_private; >> + struct sun4i_tcon *tcon; >> + >> + list_for_each_entry(tcon, &drv->tcon_list, list) >> + if (tcon->id == 0) >> + return tcon; >> + >> + dev_warn(drm->dev, >> + "TCON0 not found, display output muxing may not work\n"); >> + >> + return tcon; >> +} >> + >> +static int _sun6i_tcon_set_mux(struct drm_encoder *encoder) >> +{ >> + struct sun4i_tcon *tcon = sun4i_get_first_tcon(encoder->dev); >> + int tcon_id = drm_crtc_to_sun4i_crtc(encoder->crtc)->tcon->id; >> + u32 shift; >> + >> + DRM_DEBUG_DRIVER("Muxing encoder %s to CRTC %s (TCON %d)\n", >> + encoder->name, encoder->crtc->name, tcon_id); >> + >> + /* Only 2 TCONs */ >> + if (tcon_id >= 2) >> + return -EINVAL; >> + >> + switch (encoder->encoder_type) { >> + case DRM_MODE_ENCODER_TMDS: >> + /* HDMI */ >> + shift = 8; >> + break; >> + case DRM_MODE_ENCODER_DSI: >> + /* No MIPI DSI on A31s */ >> + if (of_device_is_compatible(tcon->dev->of_node, >> + "allwinner,sun6i-a31s-tcon")) > > I'm not sure that test is needed. > > We won't end up in that case if we don't have a connected DSI block, > which isn't going to be the case on the A31. And I guess we can tackle > DSI later (when I'll send my patches...). OK. I'll leave a comment instead. > >> + return -EINVAL; >> + shift = 0; >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + regmap_update_bits(tcon->regs, SUN4I_TCON_MUX_CTRL_REG, >> + 0x3 << shift, tcon_id << shift); >> + >> + return 0; >> +} >> + >> void sun4i_tcon_set_mux(struct sun4i_tcon *tcon, int channel, >> struct drm_encoder *encoder) >> { >> + /* Get the device node of the display engine */ >> + struct device_node *node = encoder->dev->dev->of_node; >> u32 val; >> >> + if (of_device_is_compatible(node, "allwinner,sun6i-a31-display-engine") || >> + of_device_is_compatible(node, "allwinner,sun6i-a31s-display-engine")) { >> + _sun6i_tcon_set_mux(encoder); >> + return; >> + } >> + > > I'd really like to avoid mix and matching the structure defined > behaviour and those of_device_is_compatible calls spread out > everywhere. > > You can either add a flag or a function pointer. Function pointer it is! 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
Hi everyone, This is v2 of my A31 HDMI support series. As it's been some time since the first version and I've rebased my series a couple times to merge newer changes done by others, the changelog might not be complete. Changes since v1: - Core changes to sun4i-drm to support two display pipelines have been merged into drm-misc and thus dropped from this version - Reworked DDC variant support onto new exposed I2C interface bits. - Reworked DDC variant support to use regmap_fields. - Patches to add variant support to various (TMDS, DDC, HDMI controller) sub-blocks have been merged into one patch. This series adds support for the HDMI controller found on Allwinner A31/A31s SoCs. It builds upon Maxime's work that added support for the HDMI controller on the Allwinner A10s SoC. The HDMI controllers in the older generation Allwinner SoCs is very similar. The A10/A10s/A20 all have the same hardware block, with the A10 having slightly different initial configuration values. The A31's variant splits out the DDC parent clock, has different formulas for the DDC and TMDS clocks, and a different register layout for the DDC block. Also, it does not expose the CEC pins outside of the SoC, which is unfortunate. Patch 1 exports the 2x outputs of the two video PLLs. These feed the TMDS clock directly. Patch 2 renames the A31 CCU's DDC clock, so that it doesn't conflict with the DDC clock in the HDMI block. Patch 3 adds support for the TCON demuxing feature on the A31. This is needed if the user wants to output through HDMI from the second display pipeline. Patch 4 adds proper error path cleanup to the HDMI driver. Patch 5 adds a regmap for the HDMI driver, to be used in a subsequent patch. Patch 6 allows the HDMI TMDS clock to use the second PLL as its parent, in case the first PLL is driving an incompatible dot clock. Patch 7 adds the A31 HDMI controller variant to the device tree binding. Patch 8 adds an iopoll-like polling macro for regmap_field. This is used in the next patch within the DDC part to poll for reset and I/O completion. Patch 9 adds support for different variants of the HDMI controller hardware, with the differences mentioned in the beginning of this letter. Patch 10 adds defines for the A31 specific DDC register offsets. Patch 11 adds support for the A31's HDMI controller variant. Patch 12 adds a device node for the HDMI controller on the A31. Patch 13 enable HDMI video output on three boards that I have. I also had simultaneous output on both display pipelines on the SinA31s, one with an LCD panel and the other using HDMI. After boot, both screens showed a proper console. The HDMI screen had higher resolution, so the console was limited to the upper left corner. Note that this series does not deal with conflicting pixel clocks. Assuming everyone is happy with the patches, I propose the following: 1. We sunxi maintainers will take the clk and dts patches through our tree with minimal but proper cross references. 2. Mark can either take the regmap patch on an immutable branch, which we then merge into drm-misc before applying the drm/sun4i patches, or give his Ack for us to merge that patch through drm-misc. Regards ChenYu Chen-Yu Tsai (13): clk: sunxi-ng: sun6i: Export video PLLs clk: sunxi-ng: sun6i: Rename HDMI DDC clock to avoid name collision drm/sun4i: tcon: Add support for demuxing TCON output on A31 drm/sun4i: hdmi: Disable clks in bind function error path and unbind function drm/sun4i: hdmi: create a regmap for later use drm/sun4i: hdmi: Allow using second PLL as TMDS clk parent dt-bindings: display: sun4i: Add binding for A31 HDMI controller regmap: add iopoll-like polling macro for regmap_field drm/sun4i: hdmi: Add support for controller hardware variants drm/sun4i: hdmi: Add A31 specific DDC register definitions drm/sun4i: hdmi: Add support for A31's HDMI controller ARM: dts: sun6i: Add device node for HDMI controller ARM: dts: sun6i: Enable HDMI support on some A31/A31s devices .../bindings/display/sunxi/sun4i-drm.txt | 3 + arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 21 ++ arch/arm/boot/dts/sun6i-a31.dtsi | 55 +++++ arch/arm/boot/dts/sun6i-a31s-primo81.dts | 25 +++ arch/arm/boot/dts/sun6i-a31s-sina31s.dts | 25 +++ drivers/clk/sunxi-ng/ccu-sun6i-a31.c | 3 +- drivers/clk/sunxi-ng/ccu-sun6i-a31.h | 8 +- drivers/gpu/drm/sun4i/sun4i_hdmi.h | 107 ++++++++++ drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c | 38 +++- drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 204 +++++++++++++++--- drivers/gpu/drm/sun4i/sun4i_hdmi_i2c.c | 227 +++++++++++++++------ drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 68 +++--- drivers/gpu/drm/sun4i/sun4i_tcon.c | 61 ++++++ include/dt-bindings/clock/sun6i-a31-ccu.h | 4 + include/linux/regmap.h | 39 ++++ 15 files changed, 760 insertions(+), 128 deletions(-)