[PATCH 00/11] drm/sun4i: Add A83T HDMI support

[PATCH 00/11] drm/sun4i: Add A83T HDMI support

[Jernej Skrabec]

This patch series implements support for A83T DW HDMI and PHY. It is based
upon Maxime Ripard's "drm/sun4i: Add A83t LVDS support" patch series which
can be found here:
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-December/550035.html

While exactly this combination of HDMI controller and PHY is not common in
Allwinner SoCs, this patch series nevertheless makes groundwork for other
SoCs, which have same DW HDMI IP block, but different PHYs, like H3 and H5.

All patches can also be found on github:
https://github.com/jernejsk/linux-1/commits/a83t_hdmi

Please take a look.

Best regards,
Jernej

Jernej Skrabec (11):
  clk: sunxi-ng: Don't set k if width is 0 for nkmp plls
  clk: sunxi-ng: a83t: Add M divider to TCON1 clock
  drm/bridge/synopsys: dw-hdmi: Enable workaround for v1.32a
  drm/bridge/synopsys: dw-hdmi: Export some PHY related functions
  drm/bridge/synopsys: dw-hdmi: Add deinit callback
  dt-bindings: display: sun4i-drm: Add A83T HDMI pipeline
  drm/sun4i: Add support for A83T second TCON
  drm/sun4i: Add support for A83T second DE2 mixer
  drm/sun4i: Implement A83T HDMI driver
  ARM: dts: sun8i: a83t: Add HDMI display pipeline
  ARM: dts: sun8i: a83t: Enable HDMI on BananaPi M3

 .../bindings/display/sunxi/sun4i-drm.txt           | 188 ++++++++++-
 arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts       |  29 ++
 arch/arm/boot/dts/sun8i-a83t.dtsi                  | 108 +++++-
 drivers/clk/sunxi-ng/ccu-sun8i-a83t.c              |   4 +-
 drivers/clk/sunxi-ng/ccu_nkmp.c                    |  21 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c          |  56 +++-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.h          |   2 +
 drivers/gpu/drm/sun4i/Kconfig                      |   9 +
 drivers/gpu/drm/sun4i/Makefile                     |   1 +
 drivers/gpu/drm/sun4i/sun4i_tcon.c                 |  46 ++-
 drivers/gpu/drm/sun4i/sun4i_tcon.h                 |   1 +
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c              | 367 +++++++++++++++++++++
 drivers/gpu/drm/sun4i/sun8i_mixer.c                |  11 +
 include/drm/bridge/dw_hdmi.h                       |  11 +
 14 files changed, 808 insertions(+), 46 deletions(-)
 create mode 100644 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c

-- 
2.15.1

[PATCH 01/11] clk: sunxi-ng: Don't set k if width is 0 for nkmp plls

[Jernej Skrabec]

For example, A83T have nmp plls which are modelled as nkmp plls. Since k
is not specified, it has offset 0, shift 0 and lowest value 1. This
means that LSB bit is always set to 1, which may change clock rate.

Fix that by applying k factor only if k width is greater than 0.

Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
---
 drivers/clk/sunxi-ng/ccu_nkmp.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu_nkmp.c b/drivers/clk/sunxi-ng/ccu_nkmp.c
index e58c95787f94..709f528af2b3 100644
--- a/drivers/clk/sunxi-ng/ccu_nkmp.c
+++ b/drivers/clk/sunxi-ng/ccu_nkmp.c
@@ -81,7 +81,7 @@ static unsigned long ccu_nkmp_recalc_rate(struct clk_hw *hw,
 					unsigned long parent_rate)
 {
 	struct ccu_nkmp *nkmp = hw_to_ccu_nkmp(hw);
-	unsigned long n, m, k, p;
+	unsigned long n, m, k = 1, p;
 	u32 reg;
 
 	reg = readl(nkmp->common.base + nkmp->common.reg);
@@ -92,11 +92,13 @@ static unsigned long ccu_nkmp_recalc_rate(struct clk_hw *hw,
 	if (!n)
 		n++;
 
-	k = reg >> nkmp->k.shift;
-	k &= (1 << nkmp->k.width) - 1;
-	k += nkmp->k.offset;
-	if (!k)
-		k++;
+	if (nkmp->k.width) {
+		k = reg >> nkmp->k.shift;
+		k &= (1 << nkmp->k.width) - 1;
+		k += nkmp->k.offset;
+		if (!k)
+			k++;
+	}
 
 	m = reg >> nkmp->m.shift;
 	m &= (1 << nkmp->m.width) - 1;
@@ -153,12 +155,15 @@ static int ccu_nkmp_set_rate(struct clk_hw *hw, unsigned long rate,
 
 	reg = readl(nkmp->common.base + nkmp->common.reg);
 	reg &= ~GENMASK(nkmp->n.width + nkmp->n.shift - 1, nkmp->n.shift);
-	reg &= ~GENMASK(nkmp->k.width + nkmp->k.shift - 1, nkmp->k.shift);
+	if (nkmp->k.width)
+		reg &= ~GENMASK(nkmp->k.width + nkmp->k.shift - 1,
+				nkmp->k.shift);
 	reg &= ~GENMASK(nkmp->m.width + nkmp->m.shift - 1, nkmp->m.shift);
 	reg &= ~GENMASK(nkmp->p.width + nkmp->p.shift - 1, nkmp->p.shift);
 
 	reg |= (_nkmp.n - nkmp->n.offset) << nkmp->n.shift;
-	reg |= (_nkmp.k - nkmp->k.offset) << nkmp->k.shift;
+	if (nkmp->k.width)
+		reg |= (_nkmp.k - nkmp->k.offset) << nkmp->k.shift;
 	reg |= (_nkmp.m - nkmp->m.offset) << nkmp->m.shift;
 	reg |= ilog2(_nkmp.p) << nkmp->p.shift;
 
-- 
2.15.1

[PATCH 02/11] clk: sunxi-ng: a83t: Add M divider to TCON1 clock

[Jernej Skrabec]

TCON1 also has M divider, contrary to TCON0.

Fixes: 05359be1176b ("clk: sunxi-ng: Add driver for A83T CCU")

Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
---
 drivers/clk/sunxi-ng/ccu-sun8i-a83t.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
index 04a9c33f53f0..7d08015b980d 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
@@ -504,8 +504,8 @@ static SUNXI_CCU_MUX_WITH_GATE(tcon0_clk, "tcon0", tcon0_parents,
 				 0x118, 24, 3, BIT(31), CLK_SET_RATE_PARENT);
 
 static const char * const tcon1_parents[] = { "pll-video1" };
-static SUNXI_CCU_MUX_WITH_GATE(tcon1_clk, "tcon1", tcon1_parents,
-				 0x11c, 24, 3, BIT(31), CLK_SET_RATE_PARENT);
+static SUNXI_CCU_M_WITH_MUX_GATE(tcon1_clk, "tcon1", tcon1_parents,
+				 0x11c, 0, 4, 24, 2, BIT(31), CLK_SET_RATE_PARENT);
 
 static SUNXI_CCU_GATE(csi_misc_clk, "csi-misc", "osc24M", 0x130, BIT(16), 0);
 
-- 
2.15.1

[PATCH 03/11] drm/bridge/synopsys: dw-hdmi: Enable workaround for v1.32a

[Jernej Skrabec]

Allwinner SoCs have dw hdmi controller v1.32a which exhibits same
magenta line issue as i.MX6Q and i.MX6DL. Enable workaround for it.

Tests show that one iteration is enough.

Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index a38db40ce990..7ca14d7325b5 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1634,9 +1634,10 @@ static void dw_hdmi_clear_overflow(struct dw_hdmi *hdmi)
 	 * then write one of the FC registers several times.
 	 *
 	 * The number of iterations matters and depends on the HDMI TX revision
-	 * (and possibly on the platform). So far only i.MX6Q (v1.30a) and
-	 * i.MX6DL (v1.31a) have been identified as needing the workaround, with
-	 * 4 and 1 iterations respectively.
+	 * (and possibly on the platform). So far i.MX6Q (v1.30a), i.MX6DL
+	 * (v1.31a) and multiple Allwinner SoCs (v1.32a) have been identified
+	 * as needing the workaround, with 4 iterations for v1.30a and 1
+	 * iteration for others.
 	 */
 
 	switch (hdmi->version) {
@@ -1644,6 +1645,7 @@ static void dw_hdmi_clear_overflow(struct dw_hdmi *hdmi)
 		count = 4;
 		break;
 	case 0x131a:
+	case 0x132a:
 		count = 1;
 		break;
 	default:
-- 
2.15.1

[PATCH 04/11] drm/bridge/synopsys: dw-hdmi: Export some PHY related functions

[Jernej Skrabec]

Parts of PHY code could be useful also for custom PHYs. For example,
Allwinner A83T has custom PHY which is probably Synopsys gen2 PHY
with few additional memory mapped registers, so most of the Synopsys PHY
related code could be reused.

It turns out that even completely custom HDMI PHYs, such as the one
found in Allwinner H3, can reuse some of those functions. This would
suggest that (some?) functions exported in this commit are actually part
of generic PHY interface and not really specific to Synopsys PHYs.

Export useful PHY functions.

Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45 ++++++++++++++++++++++---------
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.h |  2 ++
 include/drm/bridge/dw_hdmi.h              | 10 +++++++
 3 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 7ca14d7325b5..67467d0b683a 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1037,19 +1037,21 @@ static void dw_hdmi_phy_enable_svsret(struct dw_hdmi *hdmi, u8 enable)
 			 HDMI_PHY_CONF0_SVSRET_MASK);
 }
 
-static void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable)
+void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable)
 {
 	hdmi_mask_writeb(hdmi, enable, HDMI_PHY_CONF0,
 			 HDMI_PHY_CONF0_GEN2_PDDQ_OFFSET,
 			 HDMI_PHY_CONF0_GEN2_PDDQ_MASK);
 }
+EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_pddq);
 
-static void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable)
+void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable)
 {
 	hdmi_mask_writeb(hdmi, enable, HDMI_PHY_CONF0,
 			 HDMI_PHY_CONF0_GEN2_TXPWRON_OFFSET,
 			 HDMI_PHY_CONF0_GEN2_TXPWRON_MASK);
 }
+EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_txpwron);
 
 static void dw_hdmi_phy_sel_data_en_pol(struct dw_hdmi *hdmi, u8 enable)
 {
@@ -1065,6 +1067,23 @@ static void dw_hdmi_phy_sel_interface_control(struct dw_hdmi *hdmi, u8 enable)
 			 HDMI_PHY_CONF0_SELDIPIF_MASK);
 }
 
+void dw_hdmi_phy_gen2_reset(struct dw_hdmi *hdmi, u8 enable)
+{
+	hdmi_mask_writeb(hdmi, enable, HDMI_MC_PHYRSTZ,
+			 HDMI_MC_PHYRSTZ_PHYRSTZ_OFFSET,
+			 HDMI_MC_PHYRSTZ_PHYRSTZ_MASK);
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_reset);
+
+void dw_hdmi_phy_set_slave_addr(struct dw_hdmi *hdmi)
+{
+	hdmi_phy_test_clear(hdmi, 1);
+	hdmi_writeb(hdmi, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2,
+		    HDMI_PHY_I2CM_SLAVE_ADDR);
+	hdmi_phy_test_clear(hdmi, 0);
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_phy_set_slave_addr);
+
 static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
 {
 	const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
@@ -1204,15 +1223,12 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi)
 		dw_hdmi_phy_enable_svsret(hdmi, 1);
 
 	/* PHY reset. The reset signal is active high on Gen2 PHYs. */
-	hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_PHYRSTZ, HDMI_MC_PHYRSTZ);
-	hdmi_writeb(hdmi, 0, HDMI_MC_PHYRSTZ);
+	dw_hdmi_phy_gen2_reset(hdmi, 1);
+	dw_hdmi_phy_gen2_reset(hdmi, 0);
 
 	hdmi_writeb(hdmi, HDMI_MC_HEACPHY_RST_ASSERT, HDMI_MC_HEACPHY_RST);
 
-	hdmi_phy_test_clear(hdmi, 1);
-	hdmi_writeb(hdmi, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2,
-		    HDMI_PHY_I2CM_SLAVE_ADDR);
-	hdmi_phy_test_clear(hdmi, 0);
+	dw_hdmi_phy_set_slave_addr(hdmi);
 
 	/* Write to the PHY as configured by the platform */
 	if (pdata->configure_phy)
@@ -1251,15 +1267,16 @@ static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi, void *data)
 	dw_hdmi_phy_power_off(hdmi);
 }
 
-static enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
-						      void *data)
+enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
+					       void *data)
 {
 	return hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_HPD ?
 		connector_status_connected : connector_status_disconnected;
 }
+EXPORT_SYMBOL_GPL(dw_hdmi_phy_read_hpd);
 
-static void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
-				   bool force, bool disabled, bool rxsense)
+void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
+			    bool force, bool disabled, bool rxsense)
 {
 	u8 old_mask = hdmi->phy_mask;
 
@@ -1271,8 +1288,9 @@ static void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
 	if (old_mask != hdmi->phy_mask)
 		hdmi_writeb(hdmi, hdmi->phy_mask, HDMI_PHY_MASK0);
 }
+EXPORT_SYMBOL_GPL(dw_hdmi_phy_update_hpd);
 
-static void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data)
+void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data)
 {
 	/*
 	 * Configure the PHY RX SENSE and HPD interrupts polarities and clear
@@ -1291,6 +1309,7 @@ static void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data)
 	hdmi_writeb(hdmi, ~(HDMI_IH_PHY_STAT0_HPD | HDMI_IH_PHY_STAT0_RX_SENSE),
 		    HDMI_IH_MUTE_PHY_STAT0);
 }
+EXPORT_SYMBOL_GPL(dw_hdmi_phy_setup_hpd);
 
 static const struct dw_hdmi_phy_ops dw_hdmi_synopsys_phy_ops = {
 	.init = dw_hdmi_phy_init,
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
index 9d90eb9c46e5..fd150430d0b3 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
@@ -950,6 +950,8 @@ enum {
 
 /* MC_PHYRSTZ field values */
 	HDMI_MC_PHYRSTZ_PHYRSTZ = 0x01,
+	HDMI_MC_PHYRSTZ_PHYRSTZ_OFFSET = 0x00,
+	HDMI_MC_PHYRSTZ_PHYRSTZ_MASK = 0x01,
 
 /* MC_HEACPHY_RST field values */
 	HDMI_MC_HEACPHY_RST_ASSERT = 0x1,
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 182f83283e24..f5cca4362154 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -159,5 +159,15 @@ void dw_hdmi_audio_disable(struct dw_hdmi *hdmi);
 /* PHY configuration */
 void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
 			   unsigned char addr);
+enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
+					       void *data);
+void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
+			    bool force, bool disabled, bool rxsense);
+void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data);
+
+void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable);
+void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable);
+void dw_hdmi_phy_gen2_reset(struct dw_hdmi *hdmi, u8 enable);
+void dw_hdmi_phy_set_slave_addr(struct dw_hdmi *hdmi);
 
 #endif /* __IMX_HDMI_H__ */
-- 
2.15.1

[PATCH 05/11] drm/bridge/synopsys: dw-hdmi: Add deinit callback

[Jernej Skrabec]

Some SoCs, like Allwinner A83T, have to do additional cleanup when
HDMI driver unloads. When using DW HDMI through DRM bridge API, there is
no place to store driver's private data so it can be accessed in unbind
function. Because of that, add deinit function which is called at the
very end, so drivers can do a proper cleanup.

Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 +++
 include/drm/bridge/dw_hdmi.h              | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 67467d0b683a..a6fe7a323c83 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2592,6 +2592,9 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
 		i2c_del_adapter(&hdmi->i2c->adap);
 	else
 		i2c_put_adapter(hdmi->ddc);
+
+	if (hdmi->plat_data->deinit)
+		hdmi->plat_data->deinit(hdmi->plat_data);
 }
 
 /* -----------------------------------------------------------------------------
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index f5cca4362154..a3218d3da61b 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -124,6 +124,7 @@ struct dw_hdmi_phy_ops {
 
 struct dw_hdmi_plat_data {
 	struct regmap *regm;
+	void (*deinit)(const struct dw_hdmi_plat_data *pdata);
 	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
 					   const struct drm_display_mode *mode);
 	unsigned long input_bus_format;
-- 
2.15.1

[PATCH 06/11] dt-bindings: display: sun4i-drm: Add A83T HDMI pipeline

[Jernej Skrabec]

This commit adds all necessary compatibles and descriptions needed to
implement A83T HDMI pipeline.

Mixer is already properly described, so only compatible is added.

However, A83T TCON1, which is connected to HDMI, doesn't have channel 0,
contrary to all TCONs currently described. Because of that, TCON
documentation is extended.

A83T features Synopsys DW HDMI controller with a custom PHY which looks
like Synopsys Gen2 PHY with few additions. Since there is no
documentation, needed properties were found out through experimentation
and reading BSP code.

At the end, example is added for newer SoCs, which features DE2 and DW
HDMI.

Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
---
 .../bindings/display/sunxi/sun4i-drm.txt           | 188 ++++++++++++++++++++-
 1 file changed, 181 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
index 9f073af4c711..3eca258096a5 100644
--- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
+++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
@@ -64,6 +64,40 @@ Required properties:
     first port should be the input endpoint. The second should be the
     output, usually to an HDMI connector.
 
+DWC HDMI TX Encoder
+-----------------------------
+
+The HDMI transmitter is a Synopsys DesignWare HDMI 1.4 TX controller IP
+with Allwinner's own PHY IP. It supports audio and video outputs and CEC.
+
+These DT bindings follow the Synopsys DWC HDMI TX bindings defined in
+Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt with the
+following device-specific properties.
+
+Required properties:
+
+  - compatible: value must be one of:
+    * "allwinner,sun8i-a83t-dw-hdmi"
+  - reg: two pairs of base address and size of memory-mapped region, first
+    for controller and second for PHY
+    registers.
+  - reg-io-width: See dw_hdmi.txt. Shall be 1.
+  - interrupts: HDMI interrupt number
+  - clocks: phandles to the clocks feeding the HDMI encoder
+    * iahb: the HDMI bus clock
+    * isfr: the HDMI register clock
+    * tmds: the HDMI tmds clock
+  - clock-names: the clock names mentioned above
+  - resets: phandles to the reset controllers driving the encoder
+    * ctrl: the reset line for the controller
+    * phy: the reset line for the PHY
+  - reset-names: the reset names mentioned above
+
+  - ports: A ports node with endpoint definitions as defined in
+    Documentation/devicetree/bindings/media/video-interfaces.txt. The
+    first port should be the input endpoint. The second should be the
+    output, usually to an HDMI connector.
+
 TV Encoder
 ----------
 
@@ -94,18 +128,17 @@ Required properties:
    * allwinner,sun7i-a20-tcon
    * allwinner,sun8i-a33-tcon
    * allwinner,sun8i-a83t-tcon-lcd
+   * allwinner,sun8i-a83t-tcon-tv
    * allwinner,sun8i-v3s-tcon
  - reg: base address and size of memory-mapped region
  - interrupts: interrupt associated to this IP
- - clocks: phandles to the clocks feeding the TCON. Three are needed:
+ - clocks: phandles to the clocks feeding the TCON. One is needed:
    - 'ahb': the interface clocks
-   - 'tcon-ch0': The clock driving the TCON channel 0
  - resets: phandles to the reset controllers driving the encoder
    - "lcd": the reset line for the TCON channel 0
 
  - clock-names: the clock names mentioned above
  - reset-names: the reset names mentioned above
- - clock-output-names: Name of the pixel clock created
 
 - ports: A ports node with endpoint definitions as defined in
   Documentation/devicetree/bindings/media/video-interfaces.txt. The
@@ -119,11 +152,31 @@ Required properties:
   channel the endpoint is associated to. If that property is not
   present, the endpoint number will be used as the channel number.
 
-On SoCs other than the A33 and V3s, there is one more clock required:
+Following compatibles:
+ * allwinner,sun4i-a10-tcon
+ * allwinner,sun5i-a13-tcon
+ * allwinner,sun6i-a31-tcon
+ * allwinner,sun6i-a31s-tcon
+ * allwinner,sun7i-a20-tcon
+ * allwinner,sun8i-a33-tcon
+ * allwinner,sun8i-a83t-tcon-lcd
+ * allwinner,sun8i-v3s-tcon
+have additional required properties:
+ - 'tcon-ch0': The clock driving the TCON channel 0
+ - clock-output-names: Name of the pixel clock created
+
+For following compatibles:
+ * allwinner,sun4i-a10-tcon
+ * allwinner,sun5i-a13-tcon
+ * allwinner,sun6i-a31-tcon
+ * allwinner,sun6i-a31s-tcon
+ * allwinner,sun7i-a20-tcon
+ * allwinner,sun8i-a83t-tcon-tv
+there is one more clock required:
    - 'tcon-ch1': The clock driving the TCON channel 1
 
-On SoCs that support LVDS (all SoCs but the A13, H3, H5 and V3s), you
-need one more reset line:
+On TCONs that support LVDS (all TCONs except the ones found on A13, H3, H5, V3s
+and TCON1 on A83T), you need one more reset line:
    - 'lvds': The reset line driving the LVDS logic
 
 And on the SoCs newer than the A31 (sun6i and sun8i families), you
@@ -227,6 +280,7 @@ supported.
 Required properties:
   - compatible: value must be one of:
     * allwinner,sun8i-a83t-de2-mixer-0
+    * allwinner,sun8i-a83t-de2-mixer-1
     * allwinner,sun8i-v3s-de2-mixer
   - reg: base address and size of the memory-mapped region.
   - clocks: phandles to the clocks feeding the mixer
@@ -262,7 +316,7 @@ Required properties:
   - allwinner,pipelines: list of phandle to the display engine
     frontends (DE 1.0) or mixers (DE 2.0) available.
 
-Example:
+Example 1:
 
 panel: panel {
 	compatible = "olimex,lcd-olinuxino-43-ts";
@@ -461,3 +515,123 @@ display-engine {
 	compatible = "allwinner,sun5i-a13-display-engine";
 	allwinner,pipelines = <&fe0>;
 };
+
+Example 2:
+
+connector {
+	compatible = "hdmi-connector";
+	type = "a";
+
+	port {
+		hdmi_con_in: endpoint {
+			remote-endpoint = <&hdmi_out_con>;
+		};
+	};
+};
+
+de: display-engine {
+	compatible = "allwinner,sun8i-a83t-display-engine";
+	allwinner,pipelines = <&mixer1>;
+};
+
+mixer1: mixer@1200000 {
+	compatible = "allwinner,sun8i-a83t-de2-mixer-1";
+	reg = <0x01200000 0x100000>;
+	clocks = <&display_clocks CLK_BUS_MIXER1>,
+		 <&display_clocks CLK_MIXER1>;
+	clock-names = "bus",
+		      "mod";
+	resets = <&display_clocks RST_WB>;
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		mixer1_out: port@1 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <1>;
+
+			mixer1_out_tcon1: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&tcon1_in_mixer1>;
+			};
+		};
+	};
+};
+
+tcon1: lcd-controller@1c0d000 {
+	compatible = "allwinner,sun8i-a83t-tcon-tv";
+	reg = <0x01c0d000 0x1000>;
+	interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&ccu CLK_BUS_TCON1>, <&ccu CLK_TCON1>;
+	clock-names = "ahb", "tcon-ch1";
+	resets = <&ccu RST_BUS_TCON1>;
+	reset-names = "lcd";
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		tcon1_in: port@0 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0>;
+
+			tcon1_in_mixer1: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&mixer1_out_tcon1>;
+			};
+		};
+
+		tcon1_out: port@1 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <1>;
+
+			tcon1_out_hdmi: endpoint@1 {
+				reg = <1>;
+				remote-endpoint = <&hdmi_in_tcon1>;
+			};
+		};
+	};
+};
+
+hdmi: hdmi@1ee0000 {
+	compatible = "allwinner,sun8i-a83t-dw-hdmi";
+	reg = <0x01ee0000 0x10000>,
+	      <0x01ef0000 0x10000>;
+	reg-io-width = <1>;
+	interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&ccu CLK_BUS_HDMI>, <&ccu CLK_HDMI_SLOW>,
+		 <&ccu CLK_HDMI>;
+	clock-names = "iahb", "isfr", "tmds";
+	resets = <&ccu RST_BUS_HDMI0>, <&ccu RST_BUS_HDMI1>;
+	reset-names = "phy", "ctrl";
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		hdmi_in: port@0 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0>;
+
+			hdmi_in_tcon1: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&tcon1_out_hdmi>;
+			};
+		};
+
+		hdmi_out: port@1 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <1>;
+
+			hdmi_out_con: endpoint {
+				remote-endpoint = <&hdmi_con_in>;
+			};
+		};
+	};
+};
-- 
2.15.1

[PATCH 07/11] drm/sun4i: Add support for A83T second TCON

[Jernej Skrabec]

This TCON doesn't have channel 0, so quirk has_channel_0 is added in the
process.

Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 46 ++++++++++++++++++++++++++++----------
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
 2 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index b78fed809992..adfa39f372cf 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -84,6 +84,7 @@ static void sun4i_tcon_channel_set_status(struct sun4i_tcon *tcon, int channel,
 
 	switch (channel) {
 	case 0:
+		WARN_ON(!tcon->quirks->has_channel_0);
 		regmap_update_bits(tcon->regs, SUN4I_TCON0_CTL_REG,
 				   SUN4I_TCON0_CTL_TCON_ENABLE,
 				   enabled ? SUN4I_TCON0_CTL_TCON_ENABLE : 0);
@@ -276,6 +277,8 @@ static void sun4i_tcon0_mode_set_lvds(struct sun4i_tcon *tcon,
 	u8 clk_delay;
 	u32 reg, val = 0;
 
+	WARN_ON(!tcon->quirks->has_channel_0);
+
 	tcon->dclk_min_div = 7;
 	tcon->dclk_max_div = 7;
 	sun4i_tcon0_mode_set_common(tcon, mode);
@@ -344,6 +347,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
 	u8 clk_delay;
 	u32 val = 0;
 
+	WARN_ON(!tcon->quirks->has_channel_0);
+
 	tcon->dclk_min_div = 6;
 	tcon->dclk_max_div = 127;
 	sun4i_tcon0_mode_set_common(tcon, mode);
@@ -570,10 +575,12 @@ static int sun4i_tcon_init_clocks(struct device *dev,
 	}
 	clk_prepare_enable(tcon->clk);
 
-	tcon->sclk0 = devm_clk_get(dev, "tcon-ch0");
-	if (IS_ERR(tcon->sclk0)) {
-		dev_err(dev, "Couldn't get the TCON channel 0 clock\n");
-		return PTR_ERR(tcon->sclk0);
+	if (tcon->quirks->has_channel_0) {
+		tcon->sclk0 = devm_clk_get(dev, "tcon-ch0");
+		if (IS_ERR(tcon->sclk0)) {
+			dev_err(dev, "Couldn't get the TCON channel 0 clock\n");
+			return PTR_ERR(tcon->sclk0);
+		}
 	}
 
 	if (tcon->quirks->has_channel_1) {
@@ -930,10 +937,12 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master,
 		goto err_free_clocks;
 	}
 
-	ret = sun4i_dclk_create(dev, tcon);
-	if (ret) {
-		dev_err(dev, "Couldn't create our TCON dot clock\n");
-		goto err_free_clocks;
+	if (tcon->quirks->has_channel_0) {
+		ret = sun4i_dclk_create(dev, tcon);
+		if (ret) {
+			dev_err(dev, "Couldn't create our TCON dot clock\n");
+			goto err_free_clocks;
+		}
 	}
 
 	ret = sun4i_tcon_init_irq(dev, tcon);
@@ -991,7 +1000,8 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master,
 	return 0;
 
 err_free_dotclock:
-	sun4i_dclk_free(tcon);
+	if (tcon->quirks->has_channel_0)
+		sun4i_dclk_free(tcon);
 err_free_clocks:
 	sun4i_tcon_free_clocks(tcon);
 err_assert_reset:
@@ -1005,7 +1015,8 @@ static void sun4i_tcon_unbind(struct device *dev, struct device *master,
 	struct sun4i_tcon *tcon = dev_get_drvdata(dev);
 
 	list_del(&tcon->list);
-	sun4i_dclk_free(tcon);
+	if (tcon->quirks->has_channel_0)
+		sun4i_dclk_free(tcon);
 	sun4i_tcon_free_clocks(tcon);
 }
 
@@ -1102,16 +1113,19 @@ static int sun6i_tcon_set_mux(struct sun4i_tcon *tcon,
 }
 
 static const struct sun4i_tcon_quirks sun4i_a10_quirks = {
+	.has_channel_0		= true,
 	.has_channel_1		= true,
 	.set_mux		= sun4i_a10_tcon_set_mux,
 };
 
 static const struct sun4i_tcon_quirks sun5i_a13_quirks = {
+	.has_channel_0		= true,
 	.has_channel_1		= true,
 	.set_mux		= sun5i_a13_tcon_set_mux,
 };
 
 static const struct sun4i_tcon_quirks sun6i_a31_quirks = {
+	.has_channel_0		= true,
 	.has_channel_1		= true,
 	.has_lvds_alt		= true,
 	.needs_de_be_mux	= true,
@@ -1119,26 +1133,33 @@ static const struct sun4i_tcon_quirks sun6i_a31_quirks = {
 };
 
 static const struct sun4i_tcon_quirks sun6i_a31s_quirks = {
+	.has_channel_0		= true,
 	.has_channel_1		= true,
 	.needs_de_be_mux	= true,
 };
 
 static const struct sun4i_tcon_quirks sun7i_a20_quirks = {
+	.has_channel_0		= true,
 	.has_channel_1		= true,
 	/* Same display pipeline structure as A10 */
 	.set_mux		= sun4i_a10_tcon_set_mux,
 };
 
 static const struct sun4i_tcon_quirks sun8i_a33_quirks = {
+	.has_channel_0		= true,
 	.has_lvds_alt		= true,
 };
 
 static const struct sun4i_tcon_quirks sun8i_a83t_lcd_quirks = {
-	/* nothing is supported */
+	.has_channel_0		= true,
+};
+
+static const struct sun4i_tcon_quirks sun8i_a83t_tv_quirks = {
+	.has_channel_1		= true,
 };
 
 static const struct sun4i_tcon_quirks sun8i_v3s_quirks = {
-	/* nothing is supported */
+	.has_channel_0		= true,
 };
 
 /* sun4i_drv uses this list to check if a device node is a TCON */
@@ -1150,6 +1171,7 @@ const struct of_device_id sun4i_tcon_of_table[] = {
 	{ .compatible = "allwinner,sun7i-a20-tcon", .data = &sun7i_a20_quirks },
 	{ .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks },
 	{ .compatible = "allwinner,sun8i-a83t-tcon-lcd", .data = &sun8i_a83t_lcd_quirks },
+	{ .compatible = "allwinner,sun8i-a83t-tcon-tv", .data = &sun8i_a83t_tv_quirks },
 	{ .compatible = "allwinner,sun8i-v3s-tcon", .data = &sun8i_v3s_quirks },
 	{ }
 };
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index b761c7b823c5..78d55e7cd2b3 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -172,6 +172,7 @@
 struct sun4i_tcon;
 
 struct sun4i_tcon_quirks {
+	bool	has_channel_0;	/* a83t does not have channel 0 on second TCON */
 	bool	has_channel_1;	/* a33 does not have channel 1 */
 	bool	has_lvds_alt;	/* Does the LVDS clock have a parent other than the TCON clock? */
 	bool	needs_de_be_mux; /* sun6i needs mux to select backend */
-- 
2.15.1

[PATCH 08/11] drm/sun4i: Add support for A83T second DE2 mixer

[Jernej Skrabec]

It supports 1 VI and 1 UI plane and HW scaling on both planes.

Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index 2cbb2de6d39c..9b0256d31a61 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -485,6 +485,13 @@ static const struct sun8i_mixer_cfg sun8i_a83t_mixer0_cfg = {
 	.vi_num		= 1,
 };
 
+static const struct sun8i_mixer_cfg sun8i_a83t_mixer1_cfg = {
+	.ccsc		= 1,
+	.scaler_mask	= 0x3,
+	.ui_num		= 1,
+	.vi_num		= 1,
+};
+
 static const struct sun8i_mixer_cfg sun8i_v3s_mixer_cfg = {
 	.vi_num = 2,
 	.ui_num = 1,
@@ -498,6 +505,10 @@ static const struct of_device_id sun8i_mixer_of_table[] = {
 		.compatible = "allwinner,sun8i-a83t-de2-mixer-0",
 		.data = &sun8i_a83t_mixer0_cfg,
 	},
+	{
+		.compatible = "allwinner,sun8i-a83t-de2-mixer-1",
+		.data = &sun8i_a83t_mixer1_cfg,
+	},
 	{
 		.compatible = "allwinner,sun8i-v3s-de2-mixer",
 		.data = &sun8i_v3s_mixer_cfg,
-- 
2.15.1

[PATCH 09/11] drm/sun4i: Implement A83T HDMI driver

[Jernej Skrabec]

A83T has DW HDMI IP block with a custom PHY similar to Synopsys gen2
HDMI PHY.

Only video output was tested, while HW also supports audio and CEC.
Support for them will be added later.

Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
---
 drivers/gpu/drm/sun4i/Kconfig         |   9 +
 drivers/gpu/drm/sun4i/Makefile        |   1 +
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 367 ++++++++++++++++++++++++++++++++++
 3 files changed, 377 insertions(+)
 create mode 100644 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c

diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig
index 882d85db9053..7327da3bc94f 100644
--- a/drivers/gpu/drm/sun4i/Kconfig
+++ b/drivers/gpu/drm/sun4i/Kconfig
@@ -40,6 +40,15 @@ config DRM_SUN4I_BACKEND
 	  do some alpha blending and feed graphics to TCON. If M is
 	  selected the module will be called sun4i-backend.
 
+config DRM_SUN8I_DW_HDMI
+	tristate "Support for Allwinner version of DesignWare HDMI"
+	depends on DRM_SUN4I
+	select DRM_DW_HDMI
+	help
+	  Choose this option if you have an Allwinner SoC with the
+	  DesignWare HDMI controller with custom HDMI PHY. If M is
+	  selected the module will be called sun8i_dw_hdmi.
+
 config DRM_SUN8I_MIXER
 	tristate "Support for Allwinner Display Engine 2.0 Mixer"
 	default MACH_SUN8I
diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
index 2b37a6abbb1d..d633d2b816fd 100644
--- a/drivers/gpu/drm/sun4i/Makefile
+++ b/drivers/gpu/drm/sun4i/Makefile
@@ -26,4 +26,5 @@ obj-$(CONFIG_DRM_SUN4I)		+= sun6i_drc.o
 
 obj-$(CONFIG_DRM_SUN4I_BACKEND)	+= sun4i-backend.o
 obj-$(CONFIG_DRM_SUN4I_HDMI)	+= sun4i-drm-hdmi.o
+obj-$(CONFIG_DRM_SUN8I_DW_HDMI)	+= sun8i_dw_hdmi.o
 obj-$(CONFIG_DRM_SUN8I_MIXER)	+= sun8i-mixer.o
diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
new file mode 100644
index 000000000000..462bb81f11ed
--- /dev/null
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
@@ -0,0 +1,367 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2017 Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
+ */
+
+#include <linux/clk.h>
+#include <linux/component.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#include <drm/drm_of.h>
+#include <drm/drmP.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_edid.h>
+#include <drm/bridge/dw_hdmi.h>
+
+#define SUN8I_HDMI_PHY_REG_UNK1		0x0000
+#define SUN8I_HDMI_PHY_REG_POL		0x0001
+#define SUN8I_HDMI_PHY_REG_UNK2		0x0002
+#define SUN8I_HDMI_PHY_REG_UNK3		0x0003
+#define SUN8I_HDMI_PHY_REG_UNK4		0x0007
+
+#define SUN8I_HDMI_PHY_REG_READ_EN	0x0010
+#define SUN8I_HDMI_PHY_REG_READ_EN_MAGIC	0x54524545
+
+#define SUN8I_HDMI_PHY_REG_UNSCRAMBLE	0x0014
+#define SUN8I_HDMI_PHY_REG_UNSCRAMBLE_MAGIC	0x42494E47
+
+#define to_sun8i_dw_hdmi(x)	container_of(x, struct sun8i_dw_hdmi, x)
+
+struct sun8i_dw_hdmi {
+	struct clk *clk_ahb;
+	struct clk *clk_sfr;
+	struct clk *clk_tmds;
+	struct device *dev;
+	struct drm_encoder encoder;
+	void __iomem *phy_base;
+	struct dw_hdmi_plat_data plat_data;
+	struct reset_control *rst_ctrl;
+	struct reset_control *rst_phy;
+};
+
+static void sun8i_dw_hdmi_encoder_mode_set(struct drm_encoder *encoder,
+					   struct drm_display_mode *mode,
+					   struct drm_display_mode *adj_mode)
+{
+	struct sun8i_dw_hdmi *hdmi = to_sun8i_dw_hdmi(encoder);
+
+	clk_set_rate(hdmi->clk_tmds, mode->crtc_clock * 1000);
+}
+
+static const struct drm_encoder_helper_funcs
+sun8i_dw_hdmi_encoder_helper_funcs = {
+	.mode_set = sun8i_dw_hdmi_encoder_mode_set,
+};
+
+static void sun8i_dw_hdmi_init(struct sun8i_dw_hdmi *hdmi)
+{
+	/* enable read access to HDMI controller */
+	writel(SUN8I_HDMI_PHY_REG_READ_EN_MAGIC,
+	       hdmi->phy_base + SUN8I_HDMI_PHY_REG_READ_EN);
+
+	/*
+	 * HDMI PHY settings are taken as-is from Allwinner BSP code.
+	 * There is no documentation.
+	 */
+	writeb(0x01, hdmi->phy_base + SUN8I_HDMI_PHY_REG_UNK1);
+	writeb(0x00, hdmi->phy_base + SUN8I_HDMI_PHY_REG_POL);
+	writeb(0x69, hdmi->phy_base + SUN8I_HDMI_PHY_REG_UNK2);
+	writeb(0x00, hdmi->phy_base + SUN8I_HDMI_PHY_REG_UNK3);
+
+	/* unscramble register offsets */
+	writel(SUN8I_HDMI_PHY_REG_UNSCRAMBLE_MAGIC,
+	       hdmi->phy_base + SUN8I_HDMI_PHY_REG_UNSCRAMBLE);
+}
+
+static const struct drm_encoder_funcs sun8i_dw_hdmi_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+static int sun8i_dw_hdmi_phy_init(struct dw_hdmi *dw_hdmi, void *data,
+				  struct drm_display_mode *mode)
+{
+	struct sun8i_dw_hdmi *hdmi = (struct sun8i_dw_hdmi *)data;
+	u8 val = 0;
+
+	if ((mode->flags & DRM_MODE_FLAG_NHSYNC) &&
+	    (mode->flags & DRM_MODE_FLAG_NHSYNC)) {
+		val = 0x03;
+	}
+
+	writeb(0xa0, hdmi->phy_base + SUN8I_HDMI_PHY_REG_UNK4);
+
+	writeb(val, hdmi->phy_base + SUN8I_HDMI_PHY_REG_POL);
+
+	dw_hdmi_phy_gen2_reset(dw_hdmi, 1);
+
+	dw_hdmi_phy_gen2_txpwron(dw_hdmi, 0);
+	dw_hdmi_phy_gen2_pddq(dw_hdmi, 1);
+
+	dw_hdmi_phy_gen2_reset(dw_hdmi, 0);
+	dw_hdmi_phy_gen2_pddq(dw_hdmi, 0);
+
+	dw_hdmi_phy_set_slave_addr(dw_hdmi);
+
+	if (mode->crtc_clock <= 27000) {
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x01e0, 0x06);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x0000, 0x15);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x08da, 0x10);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x0007, 0x19);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x0318, 0x0e);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x8009, 0x09);
+	} else if (mode->crtc_clock <= 74250) {
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x0540, 0x06);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x0005, 0x15);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x0000, 0x10);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x0007, 0x19);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x02b5, 0x0e);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x8009, 0x09);
+	} else if (mode->crtc_clock <= 148500) {
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x04a0, 0x06);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x000a, 0x15);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x0000, 0x10);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x0002, 0x19);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x0021, 0x0e);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x8029, 0x09);
+	} else {
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x0000, 0x06);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x000f, 0x15);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x0000, 0x10);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x0002, 0x19);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x0000, 0x0e);
+		dw_hdmi_phy_i2c_write(dw_hdmi, 0x802b, 0x09);
+	}
+
+	dw_hdmi_phy_i2c_write(dw_hdmi, 0x0000, 0x1e);
+	dw_hdmi_phy_i2c_write(dw_hdmi, 0x0000, 0x13);
+	dw_hdmi_phy_i2c_write(dw_hdmi, 0x0000, 0x17);
+
+	dw_hdmi_phy_gen2_txpwron(dw_hdmi, 1);
+
+	return 0;
+};
+
+static void sun8i_dw_hdmi_phy_disable(struct dw_hdmi *dw_hdmi, void *data)
+{
+	struct sun8i_dw_hdmi *hdmi = (struct sun8i_dw_hdmi *)data;
+
+	dw_hdmi_phy_gen2_txpwron(dw_hdmi, 0);
+	dw_hdmi_phy_gen2_pddq(dw_hdmi, 1);
+
+	writeb(0x20, hdmi->phy_base + SUN8I_HDMI_PHY_REG_UNK4);
+}
+
+static const struct dw_hdmi_phy_ops sun8i_dw_hdmi_phy_ops = {
+	.init = &sun8i_dw_hdmi_phy_init,
+	.disable = &sun8i_dw_hdmi_phy_disable,
+	.read_hpd = &dw_hdmi_phy_read_hpd,
+	.update_hpd = &dw_hdmi_phy_update_hpd,
+	.setup_hpd = &dw_hdmi_phy_setup_hpd,
+};
+
+static enum drm_mode_status
+sun8i_dw_hdmi_mode_valid(struct drm_connector *connector,
+			 const struct drm_display_mode *mode)
+{
+	if (mode->clock > 297000)
+		return MODE_BAD;
+
+	return MODE_OK;
+}
+
+static void sun8i_dw_hdmi_deinit(const struct dw_hdmi_plat_data *plat_data)
+{
+	struct sun8i_dw_hdmi *hdmi = to_sun8i_dw_hdmi(plat_data);
+
+	clk_disable_unprepare(hdmi->clk_tmds);
+	clk_disable_unprepare(hdmi->clk_sfr);
+	clk_disable_unprepare(hdmi->clk_ahb);
+
+	reset_control_assert(hdmi->rst_phy);
+	reset_control_assert(hdmi->rst_ctrl);
+}
+
+static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master,
+			      void *data)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct dw_hdmi_plat_data *plat_data;
+	struct drm_device *drm = data;
+	struct drm_encoder *encoder;
+	struct sun8i_dw_hdmi *hdmi;
+	struct resource *res;
+	int ret;
+
+	if (!pdev->dev.of_node)
+		return -ENODEV;
+
+	hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
+	if (!hdmi)
+		return -ENOMEM;
+
+	plat_data = &hdmi->plat_data;
+	hdmi->dev = &pdev->dev;
+	encoder = &hdmi->encoder;
+
+	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
+	/*
+	 * If we failed to find the CRTC(s) which this encoder is
+	 * supposed to be connected to, it's because the CRTC has
+	 * not been registered yet.  Defer probing, and hope that
+	 * the required CRTC is added later.
+	 */
+	if (encoder->possible_crtcs == 0)
+		return -EPROBE_DEFER;
+
+	/* resource 0 is the memory region for the core controller */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	hdmi->phy_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(hdmi->phy_base))
+		return PTR_ERR(hdmi->phy_base);
+
+	hdmi->clk_ahb = devm_clk_get(dev, "iahb");
+	if (IS_ERR(hdmi->clk_ahb)) {
+		dev_err(dev, "Could not get iahb clock\n");
+		return PTR_ERR(hdmi->clk_ahb);
+	}
+
+	hdmi->clk_sfr = devm_clk_get(dev, "isfr");
+	if (IS_ERR(hdmi->clk_sfr)) {
+		dev_err(dev, "Could not get isfr clock\n");
+		return PTR_ERR(hdmi->clk_sfr);
+	}
+
+	hdmi->clk_tmds = devm_clk_get(dev, "tmds");
+	if (IS_ERR(hdmi->clk_tmds)) {
+		dev_err(dev, "Could not get tmds clock\n");
+		return PTR_ERR(hdmi->clk_tmds);
+	}
+
+	hdmi->rst_ctrl = devm_reset_control_get(dev, "ctrl");
+	if (IS_ERR(hdmi->rst_ctrl)) {
+		dev_err(dev, "Could not get ctrl reset control\n");
+		return PTR_ERR(hdmi->rst_ctrl);
+	}
+
+	hdmi->rst_phy = devm_reset_control_get(dev, "phy");
+	if (IS_ERR(hdmi->rst_phy)) {
+		dev_err(dev, "Could not get phy reset control\n");
+		return PTR_ERR(hdmi->rst_phy);
+	}
+
+	ret = reset_control_deassert(hdmi->rst_ctrl);
+	if (ret) {
+		dev_err(dev, "Could not deassert ctrl reset control\n");
+		return ret;
+	}
+
+	ret = reset_control_deassert(hdmi->rst_phy);
+	if (ret) {
+		dev_err(dev, "Could not deassert phy reset control\n");
+		goto err_assert_ctrl_reset;
+	}
+
+	ret = clk_prepare_enable(hdmi->clk_ahb);
+	if (ret) {
+		dev_err(dev, "Cannot enable ahb clock: %d\n", ret);
+		goto err_assert_phy_reset;
+	}
+
+	ret = clk_prepare_enable(hdmi->clk_sfr);
+	if (ret) {
+		dev_err(dev, "Cannot enable isfr clock: %d\n", ret);
+		goto err_ahb_clk;
+	}
+
+	/* A83T defaults to 1188 MHz, which is a bit high */
+	clk_set_rate(hdmi->clk_tmds, 297000000);
+
+	ret = clk_prepare_enable(hdmi->clk_tmds);
+	if (ret) {
+		dev_err(dev, "Cannot enable tmds clock: %d\n", ret);
+		goto err_sfr_clk;
+	}
+
+	drm_encoder_helper_add(encoder, &sun8i_dw_hdmi_encoder_helper_funcs);
+	drm_encoder_init(drm, encoder, &sun8i_dw_hdmi_encoder_funcs,
+			 DRM_MODE_ENCODER_TMDS, NULL);
+
+	sun8i_dw_hdmi_init(hdmi);
+
+	plat_data->deinit = &sun8i_dw_hdmi_deinit;
+	plat_data->mode_valid = &sun8i_dw_hdmi_mode_valid;
+	plat_data->phy_ops = &sun8i_dw_hdmi_phy_ops;
+	plat_data->phy_name = "sun8i_dw_hdmi_phy";
+	plat_data->phy_data = hdmi;
+
+	ret = dw_hdmi_bind(pdev, encoder, plat_data);
+
+	/*
+	 * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
+	 * which would have called the encoder cleanup.  Do it manually.
+	 */
+	if (ret)
+		goto cleanup_encoder;
+
+	return 0;
+
+cleanup_encoder:
+	drm_encoder_cleanup(encoder);
+	clk_disable_unprepare(hdmi->clk_tmds);
+err_sfr_clk:
+	clk_disable_unprepare(hdmi->clk_sfr);
+err_ahb_clk:
+	clk_disable_unprepare(hdmi->clk_ahb);
+err_assert_phy_reset:
+	reset_control_assert(hdmi->rst_phy);
+err_assert_ctrl_reset:
+	reset_control_assert(hdmi->rst_ctrl);
+
+	return ret;
+}
+
+static void sun8i_dw_hdmi_unbind(struct device *dev, struct device *master,
+				 void *data)
+{
+	return dw_hdmi_unbind(dev);
+}
+
+static const struct component_ops sun8i_dw_hdmi_ops = {
+	.bind	= sun8i_dw_hdmi_bind,
+	.unbind	= sun8i_dw_hdmi_unbind,
+};
+
+static int sun8i_dw_hdmi_probe(struct platform_device *pdev)
+{
+	return component_add(&pdev->dev, &sun8i_dw_hdmi_ops);
+}
+
+static int sun8i_dw_hdmi_remove(struct platform_device *pdev)
+{
+	component_del(&pdev->dev, &sun8i_dw_hdmi_ops);
+
+	return 0;
+}
+
+static const struct of_device_id sun8i_dw_hdmi_dt_ids[] = {
+	{ .compatible = "allwinner,sun8i-a83t-dw-hdmi" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, sun8i_dw_hdmi_dt_ids);
+
+struct platform_driver sun8i_dw_hdmi_pltfm_driver = {
+	.probe  = sun8i_dw_hdmi_probe,
+	.remove = sun8i_dw_hdmi_remove,
+	.driver = {
+		.name = "sun8i-dw-hdmi",
+		.of_match_table = sun8i_dw_hdmi_dt_ids,
+	},
+};
+module_platform_driver(sun8i_dw_hdmi_pltfm_driver);
+
+MODULE_AUTHOR("Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>");
+MODULE_DESCRIPTION("Allwinner DW HDMI bridge");
+MODULE_LICENSE("GPL");
-- 
2.15.1

[PATCH 10/11] ARM: dts: sun8i: a83t: Add HDMI display pipeline

[Jernej Skrabec]

This commit adds all bits necessary for HDMI on A83T - mixer1, tcon1,
hdmi and hdmi pinctrl entries.

Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
---
 arch/arm/boot/dts/sun8i-a83t.dtsi | 108 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 7f4955a5fab7..601d1eb5460e 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -155,7 +155,7 @@
 
 	de: display-engine {
 		compatible = "allwinner,sun8i-a83t-display-engine";
-		allwinner,pipelines = <&mixer0>;
+		allwinner,pipelines = <&mixer0>, <&mixer1>;
 		status = "disabled";
 	};
 
@@ -208,6 +208,32 @@
 			};
 		};
 
+		mixer1: mixer@1200000 {
+			compatible = "allwinner,sun8i-a83t-de2-mixer-1";
+			reg = <0x01200000 0x100000>;
+			clocks = <&display_clocks CLK_BUS_MIXER1>,
+				 <&display_clocks CLK_MIXER1>;
+			clock-names = "bus",
+				      "mod";
+			resets = <&display_clocks RST_WB>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				mixer1_out: port@1 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <1>;
+
+					mixer1_out_tcon1: endpoint@0 {
+						reg = <0>;
+						remote-endpoint = <&tcon1_in_mixer1>;
+					};
+				};
+			};
+		};
+
 		syscon: syscon@1c00000 {
 			compatible = "allwinner,sun8i-a83t-system-controller",
 				"syscon";
@@ -256,6 +282,43 @@
 			};
 		};
 
+		tcon1: lcd-controller@1c0d000 {
+			compatible = "allwinner,sun8i-a83t-tcon-tv";
+			reg = <0x01c0d000 0x1000>;
+			interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_TCON1>, <&ccu CLK_TCON1>;
+			clock-names = "ahb", "tcon-ch1";
+			resets = <&ccu RST_BUS_TCON1>;
+			reset-names = "lcd";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				tcon1_in: port@0 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <0>;
+
+					tcon1_in_mixer1: endpoint@0 {
+						reg = <0>;
+						remote-endpoint = <&mixer1_out_tcon1>;
+					};
+				};
+
+				tcon1_out: port@1 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <1>;
+
+					tcon1_out_hdmi: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&hdmi_in_tcon1>;
+					};
+				};
+			};
+		};
+
 		mmc0: mmc@1c0f000 {
 			compatible = "allwinner,sun8i-a83t-mmc",
 				     "allwinner,sun7i-a20-mmc";
@@ -427,6 +490,11 @@
 				drive-strength = <40>;
 			};
 
+			hdmi_pins: hdmi-pins {
+				pins = "PH6", "PH7", "PH8";
+				function = "hdmi";
+			};
+
 			i2c0_pins: i2c0-pins {
 				pins = "PH0", "PH1";
 				function = "i2c0";
@@ -685,6 +753,44 @@
 			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
 		};
 
+		hdmi: hdmi@1ee0000 {
+			compatible = "allwinner,sun8i-a83t-dw-hdmi";
+			reg = <0x01ee0000 0x10000>,
+			      <0x01ef0000 0x10000>;
+			reg-io-width = <1>;
+			interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_HDMI>, <&ccu CLK_HDMI_SLOW>,
+				 <&ccu CLK_HDMI>;
+			clock-names = "iahb", "isfr", "tmds";
+			resets = <&ccu RST_BUS_HDMI0>, <&ccu RST_BUS_HDMI1>;
+			reset-names = "phy", "ctrl";
+			pinctrl-names = "default";
+			pinctrl-0 = <&hdmi_pins>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				hdmi_in: port@0 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <0>;
+
+					hdmi_in_tcon1: endpoint@0 {
+						reg = <0>;
+						remote-endpoint = <&tcon1_out_hdmi>;
+					};
+				};
+
+				hdmi_out: port@1 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					reg = <1>;
+				};
+			};
+		};
+
 		r_intc: interrupt-controller@1f00c00 {
 			compatible = "allwinner,sun8i-a83t-r-intc",
 				     "allwinner,sun6i-a31-r-intc";
-- 
2.15.1

[PATCH 11/11] ARM: dts: sun8i: a83t: Enable HDMI on BananaPi M3

[Jernej Skrabec]

BananaPi M3 includes HDMI connector, so add support for it.

Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
---
 arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 29 ++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
index 6550bf0e594b..2002d249e14c 100644
--- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
@@ -60,6 +60,17 @@
 		stdout-path = "serial0:115200n8";
 	};
 
+	connector {
+		compatible = "hdmi-connector";
+		type = "a";
+
+		port {
+			hdmi_con_in: endpoint {
+				remote-endpoint = <&hdmi_out_con>;
+			};
+		};
+	};
+
 	reg_usb1_vbus: reg-usb1-vbus {
 		compatible = "regulator-fixed";
 		regulator-name = "usb1-vbus";
@@ -82,6 +93,10 @@
 	};
 };
 
+&de {
+	status = "okay";
+};
+
 &ehci0 {
 	/* Terminus Tech FE 1.1s 4-port USB 2.0 hub here */
 	status = "okay";
@@ -100,6 +115,16 @@
 	status = "okay";
 };
 
+&hdmi {
+	status = "okay";
+};
+
+&hdmi_out {
+	hdmi_out_con: endpoint {
+		remote-endpoint = <&hdmi_con_in>;
+	};
+};
+
 &mdio {
 	rgmii_phy: ethernet-phy@1 {
 		compatible = "ethernet-phy-ieee802.3-c22";
@@ -306,6 +331,10 @@
 	regulator-name = "vcc-ephy";
 };
 
+&tcon1 {
+	status = "okay";
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pb_pins>;
-- 
2.15.1

Re: [PATCH 02/11] clk: sunxi-ng: a83t: Add M divider to TCON1 clock

[Chen-Yu Tsai]

On Sun, Dec 31, 2017 at 5:01 AM, Jernej Skrabec <jernej.skrabec@siol.net> wrote:
> TCON1 also has M divider, contrary to TCON0.
>
> Fixes: 05359be1176b ("clk: sunxi-ng: Add driver for A83T CCU")
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>

Added "And the mux is only 2 bits wide, instead of 3." to the commit
message and applied.

ChenYu

Re: [PATCH 06/11] dt-bindings: display: sun4i-drm: Add A83T HDMI pipeline

[Rob Herring]

On Sat, Dec 30, 2017 at 10:01:58PM +0100, Jernej Skrabec wrote:
> This commit adds all necessary compatibles and descriptions needed to
> implement A83T HDMI pipeline.
> 
> Mixer is already properly described, so only compatible is added.
> 
> However, A83T TCON1, which is connected to HDMI, doesn't have channel 0,
> contrary to all TCONs currently described. Because of that, TCON
> documentation is extended.
> 
> A83T features Synopsys DW HDMI controller with a custom PHY which looks
> like Synopsys Gen2 PHY with few additions. Since there is no
> documentation, needed properties were found out through experimentation
> and reading BSP code.
> 
> At the end, example is added for newer SoCs, which features DE2 and DW
> HDMI.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> ---
>  .../bindings/display/sunxi/sun4i-drm.txt           | 188 ++++++++++++++++++++-
>  1 file changed, 181 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> index 9f073af4c711..3eca258096a5 100644
> --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> @@ -64,6 +64,40 @@ Required properties:
>      first port should be the input endpoint. The second should be the
>      output, usually to an HDMI connector.
>  
> +DWC HDMI TX Encoder
> +-----------------------------
> +
> +The HDMI transmitter is a Synopsys DesignWare HDMI 1.4 TX controller IP
> +with Allwinner's own PHY IP. It supports audio and video outputs and CEC.
> +
> +These DT bindings follow the Synopsys DWC HDMI TX bindings defined in
> +Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt with the
> +following device-specific properties.
> +
> +Required properties:
> +
> +  - compatible: value must be one of:
> +    * "allwinner,sun8i-a83t-dw-hdmi"
> +  - reg: two pairs of base address and size of memory-mapped region, first
> +    for controller and second for PHY
> +    registers.

Seems like the phy should be a separate node and use the phy binding. 
You can use the phy binding even if you don't use the kernel phy 
framework...

> +  - reg-io-width: See dw_hdmi.txt. Shall be 1.
> +  - interrupts: HDMI interrupt number
> +  - clocks: phandles to the clocks feeding the HDMI encoder
> +    * iahb: the HDMI bus clock
> +    * isfr: the HDMI register clock
> +    * tmds: the HDMI tmds clock
> +  - clock-names: the clock names mentioned above
> +  - resets: phandles to the reset controllers driving the encoder
> +    * ctrl: the reset line for the controller
> +    * phy: the reset line for the PHY
> +  - reset-names: the reset names mentioned above
> +
> +  - ports: A ports node with endpoint definitions as defined in
> +    Documentation/devicetree/bindings/media/video-interfaces.txt. The
> +    first port should be the input endpoint. The second should be the
> +    output, usually to an HDMI connector.
> +
>  TV Encoder
>  ----------
>  
> @@ -94,18 +128,17 @@ Required properties:
>     * allwinner,sun7i-a20-tcon
>     * allwinner,sun8i-a33-tcon
>     * allwinner,sun8i-a83t-tcon-lcd
> +   * allwinner,sun8i-a83t-tcon-tv
>     * allwinner,sun8i-v3s-tcon
>   - reg: base address and size of memory-mapped region
>   - interrupts: interrupt associated to this IP
> - - clocks: phandles to the clocks feeding the TCON. Three are needed:
> + - clocks: phandles to the clocks feeding the TCON. One is needed:
>     - 'ahb': the interface clocks
> -   - 'tcon-ch0': The clock driving the TCON channel 0
>   - resets: phandles to the reset controllers driving the encoder
>     - "lcd": the reset line for the TCON channel 0
>  
>   - clock-names: the clock names mentioned above
>   - reset-names: the reset names mentioned above
> - - clock-output-names: Name of the pixel clock created
>  
>  - ports: A ports node with endpoint definitions as defined in
>    Documentation/devicetree/bindings/media/video-interfaces.txt. The
> @@ -119,11 +152,31 @@ Required properties:
>    channel the endpoint is associated to. If that property is not
>    present, the endpoint number will be used as the channel number.
>  
> -On SoCs other than the A33 and V3s, there is one more clock required:
> +Following compatibles:
> + * allwinner,sun4i-a10-tcon
> + * allwinner,sun5i-a13-tcon
> + * allwinner,sun6i-a31-tcon
> + * allwinner,sun6i-a31s-tcon
> + * allwinner,sun7i-a20-tcon
> + * allwinner,sun8i-a33-tcon
> + * allwinner,sun8i-a83t-tcon-lcd
> + * allwinner,sun8i-v3s-tcon
> +have additional required properties:
> + - 'tcon-ch0': The clock driving the TCON channel 0

tcon-ch0 is a clock name, not a property.

> + - clock-output-names: Name of the pixel clock created
> +
> +For following compatibles:
> + * allwinner,sun4i-a10-tcon
> + * allwinner,sun5i-a13-tcon
> + * allwinner,sun6i-a31-tcon
> + * allwinner,sun6i-a31s-tcon
> + * allwinner,sun7i-a20-tcon
> + * allwinner,sun8i-a83t-tcon-tv
> +there is one more clock required:
>     - 'tcon-ch1': The clock driving the TCON channel 1

Re: [PATCH 06/11] dt-bindings: display: sun4i-drm: Add A83T HDMI pipeline

[Jernej Škrabec]

Hi Rob,

Dne sreda, 03. januar 2018 ob 21:21:54 CET je Rob Herring napisal(a):
> On Sat, Dec 30, 2017 at 10:01:58PM +0100, Jernej Skrabec wrote:
> > This commit adds all necessary compatibles and descriptions needed to
> > implement A83T HDMI pipeline.
> > 
> > Mixer is already properly described, so only compatible is added.
> > 
> > However, A83T TCON1, which is connected to HDMI, doesn't have channel 0,
> > contrary to all TCONs currently described. Because of that, TCON
> > documentation is extended.
> > 
> > A83T features Synopsys DW HDMI controller with a custom PHY which looks
> > like Synopsys Gen2 PHY with few additions. Since there is no
> > documentation, needed properties were found out through experimentation
> > and reading BSP code.
> > 
> > At the end, example is added for newer SoCs, which features DE2 and DW
> > HDMI.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> > ---
> > 
> >  .../bindings/display/sunxi/sun4i-drm.txt           | 188
> >  ++++++++++++++++++++- 1 file changed, 181 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index
> > 9f073af4c711..3eca258096a5 100644
> > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > 
> > @@ -64,6 +64,40 @@ Required properties:
> >      first port should be the input endpoint. The second should be the
> >      output, usually to an HDMI connector.
> > 
> > +DWC HDMI TX Encoder
> > +-----------------------------
> > +
> > +The HDMI transmitter is a Synopsys DesignWare HDMI 1.4 TX controller IP
> > +with Allwinner's own PHY IP. It supports audio and video outputs and CEC.
> > +
> > +These DT bindings follow the Synopsys DWC HDMI TX bindings defined in
> > +Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt with the
> > +following device-specific properties.
> > +
> > +Required properties:
> > +
> > +  - compatible: value must be one of:
> > +    * "allwinner,sun8i-a83t-dw-hdmi"
> > +  - reg: two pairs of base address and size of memory-mapped region,
> > first
> > +    for controller and second for PHY
> > +    registers.
> 
> Seems like the phy should be a separate node and use the phy binding.
> You can use the phy binding even if you don't use the kernel phy
> framework...

Unfortunately, it's not so straighforward. Phy is actually accessed through 
I2C implemented in HDMI controller. Second memory region in this case has 
small influence on phy. However, it has big influence on controller. For 
example, magic number has to be written in one register in second memory 
region in order to unlock read access to any register from first memory region 
(controller). However, they shouldn't be merged to one region, because first 
memory region requires byte access while second memory region can be accessed 
per byte or word.

To complicate things more, later I want to add support for another SoC which 
has same glue layer (unlocking read access, etc.) and uses memory mapped phy 
registers in second memory region.

I think current binding is the least complicated way to represent this.

> 
> > +  - reg-io-width: See dw_hdmi.txt. Shall be 1.
> > +  - interrupts: HDMI interrupt number
> > +  - clocks: phandles to the clocks feeding the HDMI encoder
> > +    * iahb: the HDMI bus clock
> > +    * isfr: the HDMI register clock
> > +    * tmds: the HDMI tmds clock
> > +  - clock-names: the clock names mentioned above
> > +  - resets: phandles to the reset controllers driving the encoder
> > +    * ctrl: the reset line for the controller
> > +    * phy: the reset line for the PHY
> > +  - reset-names: the reset names mentioned above
> > +
> > +  - ports: A ports node with endpoint definitions as defined in
> > +    Documentation/devicetree/bindings/media/video-interfaces.txt. The
> > +    first port should be the input endpoint. The second should be the
> > +    output, usually to an HDMI connector.
> > +
> > 
> >  TV Encoder
> >  ----------
> > 
> > @@ -94,18 +128,17 @@ Required properties:
> >     * allwinner,sun7i-a20-tcon
> >     * allwinner,sun8i-a33-tcon
> >     * allwinner,sun8i-a83t-tcon-lcd
> > 
> > +   * allwinner,sun8i-a83t-tcon-tv
> > 
> >     * allwinner,sun8i-v3s-tcon
> >   
> >   - reg: base address and size of memory-mapped region
> >   - interrupts: interrupt associated to this IP
> > 
> > - - clocks: phandles to the clocks feeding the TCON. Three are needed:
> > 
> > + - clocks: phandles to the clocks feeding the TCON. One is needed:
> >     - 'ahb': the interface clocks
> > 
> > -   - 'tcon-ch0': The clock driving the TCON channel 0
> > 
> >   - resets: phandles to the reset controllers driving the encoder
> >   
> >     - "lcd": the reset line for the TCON channel 0
> >   
> >   - clock-names: the clock names mentioned above
> >   - reset-names: the reset names mentioned above
> > 
> > - - clock-output-names: Name of the pixel clock created
> > 
> >  - ports: A ports node with endpoint definitions as defined in
> >  
> >    Documentation/devicetree/bindings/media/video-interfaces.txt. The
> > 
> > @@ -119,11 +152,31 @@ Required properties:
> >    channel the endpoint is associated to. If that property is not
> >    present, the endpoint number will be used as the channel number.
> > 
> > -On SoCs other than the A33 and V3s, there is one more clock required:
> > +Following compatibles:
> > + * allwinner,sun4i-a10-tcon
> > + * allwinner,sun5i-a13-tcon
> > + * allwinner,sun6i-a31-tcon
> > + * allwinner,sun6i-a31s-tcon
> > + * allwinner,sun7i-a20-tcon
> > + * allwinner,sun8i-a33-tcon
> > + * allwinner,sun8i-a83t-tcon-lcd
> > + * allwinner,sun8i-v3s-tcon
> > +have additional required properties:
> > + - 'tcon-ch0': The clock driving the TCON channel 0
> 
> tcon-ch0 is a clock name, not a property.

right.

Best regards,
Jernej

Re: [PATCH 01/11] clk: sunxi-ng: Don't set k if width is 0 for nkmp plls

[maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8]

[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]

Hi,

On Sat, Dec 30, 2017 at 10:01:53PM +0100, Jernej Skrabec wrote:
> For example, A83T have nmp plls which are modelled as nkmp plls. Since k
> is not specified, it has offset 0, shift 0 and lowest value 1. This
> means that LSB bit is always set to 1, which may change clock rate.
> 
> Fix that by applying k factor only if k width is greater than 0.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>

This looks fine...

> ---
>  drivers/clk/sunxi-ng/ccu_nkmp.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu_nkmp.c b/drivers/clk/sunxi-ng/ccu_nkmp.c
> index e58c95787f94..709f528af2b3 100644
> --- a/drivers/clk/sunxi-ng/ccu_nkmp.c
> +++ b/drivers/clk/sunxi-ng/ccu_nkmp.c
> @@ -81,7 +81,7 @@ static unsigned long ccu_nkmp_recalc_rate(struct clk_hw *hw,
>  					unsigned long parent_rate)
>  {
>  	struct ccu_nkmp *nkmp = hw_to_ccu_nkmp(hw);
> -	unsigned long n, m, k, p;
> +	unsigned long n, m, k = 1, p;
>  	u32 reg;
>  
>  	reg = readl(nkmp->common.base + nkmp->common.reg);
> @@ -92,11 +92,13 @@ static unsigned long ccu_nkmp_recalc_rate(struct clk_hw *hw,
>  	if (!n)
>  		n++;
>  
> -	k = reg >> nkmp->k.shift;
> -	k &= (1 << nkmp->k.width) - 1;
> -	k += nkmp->k.offset;
> -	if (!k)
> -		k++;
> +	if (nkmp->k.width) {
> +		k = reg >> nkmp->k.shift;
> +		k &= (1 << nkmp->k.width) - 1;
> +		k += nkmp->k.offset;
> +		if (!k)
> +			k++;
> +	}

... but could you add a comment there to explain why you're using a
different construct than the one used for the other factors?

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 01/11] clk: sunxi-ng: Don't set k if width is 0 for nkmp plls

[Chen-Yu Tsai]

On Sun, Dec 31, 2017 at 5:01 AM, Jernej Skrabec <jernej.skrabec@siol.net> wrote:
> For example, A83T have nmp plls which are modelled as nkmp plls. Since k
> is not specified, it has offset 0, shift 0 and lowest value 1. This
> means that LSB bit is always set to 1, which may change clock rate.
>
> Fix that by applying k factor only if k width is greater than 0.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  drivers/clk/sunxi-ng/ccu_nkmp.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/sunxi-ng/ccu_nkmp.c b/drivers/clk/sunxi-ng/ccu_nkmp.c
> index e58c95787f94..709f528af2b3 100644
> --- a/drivers/clk/sunxi-ng/ccu_nkmp.c
> +++ b/drivers/clk/sunxi-ng/ccu_nkmp.c
> @@ -81,7 +81,7 @@ static unsigned long ccu_nkmp_recalc_rate(struct clk_hw *hw,
>                                         unsigned long parent_rate)
>  {
>         struct ccu_nkmp *nkmp = hw_to_ccu_nkmp(hw);
> -       unsigned long n, m, k, p;
> +       unsigned long n, m, k = 1, p;
>         u32 reg;
>
>         reg = readl(nkmp->common.base + nkmp->common.reg);
> @@ -92,11 +92,13 @@ static unsigned long ccu_nkmp_recalc_rate(struct clk_hw *hw,
>         if (!n)
>                 n++;
>
> -       k = reg >> nkmp->k.shift;
> -       k &= (1 << nkmp->k.width) - 1;
> -       k += nkmp->k.offset;
> -       if (!k)
> -               k++;
> +       if (nkmp->k.width) {
> +               k = reg >> nkmp->k.shift;
> +               k &= (1 << nkmp->k.width) - 1;
> +               k += nkmp->k.offset;
> +               if (!k)
> +                       k++;
> +       }

The conditional shouldn't be necessary. With nkmp->k.width = 0,
you'd simply get k & 0, which is 0, which then gets bumped up to 1,
unless k.offset > 1, which would be a bug.

>
>         m = reg >> nkmp->m.shift;
>         m &= (1 << nkmp->m.width) - 1;
> @@ -153,12 +155,15 @@ static int ccu_nkmp_set_rate(struct clk_hw *hw, unsigned long rate,
>
>         reg = readl(nkmp->common.base + nkmp->common.reg);
>         reg &= ~GENMASK(nkmp->n.width + nkmp->n.shift - 1, nkmp->n.shift);
> -       reg &= ~GENMASK(nkmp->k.width + nkmp->k.shift - 1, nkmp->k.shift);
> +       if (nkmp->k.width)
> +               reg &= ~GENMASK(nkmp->k.width + nkmp->k.shift - 1,
> +                               nkmp->k.shift);
>         reg &= ~GENMASK(nkmp->m.width + nkmp->m.shift - 1, nkmp->m.shift);
>         reg &= ~GENMASK(nkmp->p.width + nkmp->p.shift - 1, nkmp->p.shift);
>
>         reg |= (_nkmp.n - nkmp->n.offset) << nkmp->n.shift;
> -       reg |= (_nkmp.k - nkmp->k.offset) << nkmp->k.shift;
> +       if (nkmp->k.width)
> +               reg |= (_nkmp.k - nkmp->k.offset) << nkmp->k.shift;

I think a better way would be

        reg |= ((_nkmp.k - nkmp->k.offset) << nkmp->k.shift) &
               GENMASK(nkmp->k.width + nkmp->k.shift - 1, nkmp->k.shift);

And do this for all the factors, not just k. This pattern is what
regmap_update_bits does, which seems much safer. I wonder what
GENMASK() with a negative value would do though...

ChenYu

>         reg |= (_nkmp.m - nkmp->m.offset) << nkmp->m.shift;
>         reg |= ilog2(_nkmp.p) << nkmp->p.shift;
>
> --
> 2.15.1
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/11] drm/sun4i: Add support for A83T second TCON

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 463 bytes --]

Hi,

On Sat, Dec 30, 2017 at 10:01:59PM +0100, Jernej Skrabec wrote:
> This TCON doesn't have channel 0, so quirk has_channel_0 is added in the
> process.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>

Ideally, this should be split in two patches, one to add the new flag,
and one to add the support for the A83t TCON-TV.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 08/11] drm/sun4i: Add support for A83T second DE2 mixer

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 407 bytes --]

On Sat, Dec 30, 2017 at 10:02:00PM +0100, Jernej Skrabec wrote:
> It supports 1 VI and 1 UI plane and HW scaling on both planes.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>

Acked-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 06/11] dt-bindings: display: sun4i-drm: Add A83T HDMI pipeline

[Maxime Ripard]

On Wed, Jan 03, 2018 at 10:32:26PM +0100, Jernej Škrabec wrote:
> Hi Rob,
> 
> Dne sreda, 03. januar 2018 ob 21:21:54 CET je Rob Herring napisal(a):
> > On Sat, Dec 30, 2017 at 10:01:58PM +0100, Jernej Skrabec wrote:
> > > This commit adds all necessary compatibles and descriptions needed to
> > > implement A83T HDMI pipeline.
> > > 
> > > Mixer is already properly described, so only compatible is added.
> > > 
> > > However, A83T TCON1, which is connected to HDMI, doesn't have channel 0,
> > > contrary to all TCONs currently described. Because of that, TCON
> > > documentation is extended.
> > > 
> > > A83T features Synopsys DW HDMI controller with a custom PHY which looks
> > > like Synopsys Gen2 PHY with few additions. Since there is no
> > > documentation, needed properties were found out through experimentation
> > > and reading BSP code.
> > > 
> > > At the end, example is added for newer SoCs, which features DE2 and DW
> > > HDMI.
> > > 
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > ---
> > > 
> > >  .../bindings/display/sunxi/sun4i-drm.txt           | 188
> > >  ++++++++++++++++++++- 1 file changed, 181 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index
> > > 9f073af4c711..3eca258096a5 100644
> > > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > > 
> > > @@ -64,6 +64,40 @@ Required properties:
> > >      first port should be the input endpoint. The second should be the
> > >      output, usually to an HDMI connector.
> > > 
> > > +DWC HDMI TX Encoder
> > > +-----------------------------
> > > +
> > > +The HDMI transmitter is a Synopsys DesignWare HDMI 1.4 TX controller IP
> > > +with Allwinner's own PHY IP. It supports audio and video outputs and CEC.
> > > +
> > > +These DT bindings follow the Synopsys DWC HDMI TX bindings defined in
> > > +Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt with the
> > > +following device-specific properties.
> > > +
> > > +Required properties:
> > > +
> > > +  - compatible: value must be one of:
> > > +    * "allwinner,sun8i-a83t-dw-hdmi"
> > > +  - reg: two pairs of base address and size of memory-mapped region,
> > > first
> > > +    for controller and second for PHY
> > > +    registers.
> > 
> > Seems like the phy should be a separate node and use the phy binding.
> > You can use the phy binding even if you don't use the kernel phy
> > framework...
> 
> Unfortunately, it's not so straighforward. Phy is actually accessed through 
> I2C implemented in HDMI controller. Second memory region in this case has 
> small influence on phy. However, it has big influence on controller. For 
> example, magic number has to be written in one register in second memory 
> region in order to unlock read access to any register from first memory region 
> (controller). However, they shouldn't be merged to one region, because first 
> memory region requires byte access while second memory region can be accessed 
> per byte or word.
> 
> To complicate things more, later I want to add support for another SoC which 
> has same glue layer (unlocking read access, etc.) and uses memory mapped phy 
> registers in second memory region.
> 
> I think current binding is the least complicated way to represent this.

I agree with Rob here. I did a similar thing for the DSI patches I've
sent a few monthes ago and it turned out to not be that difficult, so
I'm sure you can come up with something :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 01/11] clk: sunxi-ng: Don't set k if width is 0 for nkmp plls

[Jernej Škrabec]

Hi,

Dne četrtek, 04. januar 2018 ob 15:45:18 CET je Chen-Yu Tsai napisal(a):
> On Sun, Dec 31, 2017 at 5:01 AM, Jernej Skrabec <jernej.skrabec@siol.net> 
wrote:
> > For example, A83T have nmp plls which are modelled as nkmp plls. Since k
> > is not specified, it has offset 0, shift 0 and lowest value 1. This
> > means that LSB bit is always set to 1, which may change clock rate.
> > 
> > Fix that by applying k factor only if k width is greater than 0.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> > 
> >  drivers/clk/sunxi-ng/ccu_nkmp.c | 21 +++++++++++++--------
> >  1 file changed, 13 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/clk/sunxi-ng/ccu_nkmp.c
> > b/drivers/clk/sunxi-ng/ccu_nkmp.c index e58c95787f94..709f528af2b3 100644
> > --- a/drivers/clk/sunxi-ng/ccu_nkmp.c
> > +++ b/drivers/clk/sunxi-ng/ccu_nkmp.c
> > @@ -81,7 +81,7 @@ static unsigned long ccu_nkmp_recalc_rate(struct clk_hw
> > *hw,> 
> >                                         unsigned long parent_rate)
> >  
> >  {
> >  
> >         struct ccu_nkmp *nkmp = hw_to_ccu_nkmp(hw);
> > 
> > -       unsigned long n, m, k, p;
> > +       unsigned long n, m, k = 1, p;
> > 
> >         u32 reg;
> >         
> >         reg = readl(nkmp->common.base + nkmp->common.reg);
> > 
> > @@ -92,11 +92,13 @@ static unsigned long ccu_nkmp_recalc_rate(struct
> > clk_hw *hw,> 
> >         if (!n)
> >         
> >                 n++;
> > 
> > -       k = reg >> nkmp->k.shift;
> > -       k &= (1 << nkmp->k.width) - 1;
> > -       k += nkmp->k.offset;
> > -       if (!k)
> > -               k++;
> > +       if (nkmp->k.width) {
> > +               k = reg >> nkmp->k.shift;
> > +               k &= (1 << nkmp->k.width) - 1;
> > +               k += nkmp->k.offset;
> > +               if (!k)
> > +                       k++;
> > +       }
> 
> The conditional shouldn't be necessary. With nkmp->k.width = 0,
> you'd simply get k & 0, which is 0, which then gets bumped up to 1,
> unless k.offset > 1, which would be a bug.
> 
> >         m = reg >> nkmp->m.shift;
> >         m &= (1 << nkmp->m.width) - 1;
> > 
> > @@ -153,12 +155,15 @@ static int ccu_nkmp_set_rate(struct clk_hw *hw,
> > unsigned long rate,> 
> >         reg = readl(nkmp->common.base + nkmp->common.reg);
> >         reg &= ~GENMASK(nkmp->n.width + nkmp->n.shift - 1, nkmp->n.shift);
> > 
> > -       reg &= ~GENMASK(nkmp->k.width + nkmp->k.shift - 1, nkmp->k.shift);
> > +       if (nkmp->k.width)
> > +               reg &= ~GENMASK(nkmp->k.width + nkmp->k.shift - 1,
> > +                               nkmp->k.shift);
> > 
> >         reg &= ~GENMASK(nkmp->m.width + nkmp->m.shift - 1, nkmp->m.shift);
> >         reg &= ~GENMASK(nkmp->p.width + nkmp->p.shift - 1, nkmp->p.shift);
> >         
> >         reg |= (_nkmp.n - nkmp->n.offset) << nkmp->n.shift;
> > 
> > -       reg |= (_nkmp.k - nkmp->k.offset) << nkmp->k.shift;
> > +       if (nkmp->k.width)
> > +               reg |= (_nkmp.k - nkmp->k.offset) << nkmp->k.shift;
> 
> I think a better way would be
> 
>         reg |= ((_nkmp.k - nkmp->k.offset) << nkmp->k.shift) &
>                GENMASK(nkmp->k.width + nkmp->k.shift - 1, nkmp->k.shift);
> 
> And do this for all the factors, not just k. This pattern is what
> regmap_update_bits does, which seems much safer. I wonder what
> GENMASK() with a negative value would do though...

You're right, GENMASK(-1, 0) equals 0 (calculated by hand, not tested). This 
seems much more elegant solution. 

Semi-related question: All nmp PLLs have much wider N range than real nkmp 
PLLs. This causes integer overflow when using nkmp formula from datasheet. 
Usually, N is 1-256 for nmp PLLs, which means that for very high N factors, it 
overflows. This also causes issue that M factor is never higher than 1.

I was wondering, if patch would be acceptable which would change this formula:

RATE = (24MHz * N * K) / (M * P)

to this:

RATE ((24MHz / M) * N * K) / P

I checked all M factors and are all in 1-4 or 1-2 range, which means it 
wouldn't have any impact for real nkmp PLLs when parent is 24 MHz clock which 
is probably always.

What do you think?

I discovered that when I tried to set A83T PLL_VIDEO to 346.5 MHz which is 
possible only when above formula is changed.

Best regards,
Jernej

> 
> ChenYu
> 
> >         reg |= (_nkmp.m - nkmp->m.offset) << nkmp->m.shift;
> >         reg |= ilog2(_nkmp.p) << nkmp->p.shift;
> > 
> > --
> > 2.15.1

Re: [PATCH 06/11] dt-bindings: display: sun4i-drm: Add A83T HDMI pipeline

[Icenowy Zheng]

于 2018年1月5日 GMT+08:00 上午2:52:10, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
>On Wed, Jan 03, 2018 at 10:32:26PM +0100, Jernej Škrabec wrote:
>> Hi Rob,
>> 
>> Dne sreda, 03. januar 2018 ob 21:21:54 CET je Rob Herring napisal(a):
>> > On Sat, Dec 30, 2017 at 10:01:58PM +0100, Jernej Skrabec wrote:
>> > > This commit adds all necessary compatibles and descriptions
>needed to
>> > > implement A83T HDMI pipeline.
>> > > 
>> > > Mixer is already properly described, so only compatible is added.
>> > > 
>> > > However, A83T TCON1, which is connected to HDMI, doesn't have
>channel 0,
>> > > contrary to all TCONs currently described. Because of that, TCON
>> > > documentation is extended.
>> > > 
>> > > A83T features Synopsys DW HDMI controller with a custom PHY which
>looks
>> > > like Synopsys Gen2 PHY with few additions. Since there is no
>> > > documentation, needed properties were found out through
>experimentation
>> > > and reading BSP code.
>> > > 
>> > > At the end, example is added for newer SoCs, which features DE2
>and DW
>> > > HDMI.
>> > > 
>> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
>> > > ---
>> > > 
>> > >  .../bindings/display/sunxi/sun4i-drm.txt           | 188
>> > >  ++++++++++++++++++++- 1 file changed, 181 insertions(+), 7
>deletions(-)
>> > > 
>> > > diff --git
>a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
>> > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
>index
>> > > 9f073af4c711..3eca258096a5 100644
>> > > ---
>a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
>> > > +++
>b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
>> > > 
>> > > @@ -64,6 +64,40 @@ Required properties:
>> > >      first port should be the input endpoint. The second should
>be the
>> > >      output, usually to an HDMI connector.
>> > > 
>> > > +DWC HDMI TX Encoder
>> > > +-----------------------------
>> > > +
>> > > +The HDMI transmitter is a Synopsys DesignWare HDMI 1.4 TX
>controller IP
>> > > +with Allwinner's own PHY IP. It supports audio and video outputs
>and CEC.
>> > > +
>> > > +These DT bindings follow the Synopsys DWC HDMI TX bindings
>defined in
>> > > +Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
>with the
>> > > +following device-specific properties.
>> > > +
>> > > +Required properties:
>> > > +
>> > > +  - compatible: value must be one of:
>> > > +    * "allwinner,sun8i-a83t-dw-hdmi"
>> > > +  - reg: two pairs of base address and size of memory-mapped
>region,
>> > > first
>> > > +    for controller and second for PHY
>> > > +    registers.
>> > 
>> > Seems like the phy should be a separate node and use the phy
>binding.
>> > You can use the phy binding even if you don't use the kernel phy
>> > framework...
>> 
>> Unfortunately, it's not so straighforward. Phy is actually accessed
>through 
>> I2C implemented in HDMI controller. Second memory region in this case
>has 
>> small influence on phy. However, it has big influence on controller.
>For 
>> example, magic number has to be written in one register in second
>memory 
>> region in order to unlock read access to any register from first
>memory region 
>> (controller). However, they shouldn't be merged to one region,
>because first 
>> memory region requires byte access while second memory region can be
>accessed 
>> per byte or word.
>> 
>> To complicate things more, later I want to add support for another
>SoC which 
>> has same glue layer (unlocking read access, etc.) and uses memory
>mapped phy 
>> registers in second memory region.
>> 
>> I think current binding is the least complicated way to represent
>this.
>
>I agree with Rob here. I did a similar thing for the DSI patches I've
>sent a few monthes ago and it turned out to not be that difficult, so
>I'm sure you can come up with something :)

In A83T/H3/A64/H5/R40 this part is not purely a PHY.
It controls the access of main controller's register (e.g. read/write
lock and register obfuscation). So it should be called a "glue"
with PHY part (and on A83T seems a pure glue) but not a simple
 PHY.

>
>Maxime
>
>-- 
>Maxime Ripard, Free Electrons
>Embedded Linux and Kernel engineering
>http://free-electrons.com
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 06/11] dt-bindings: display: sun4i-drm: Add A83T HDMI pipeline

[Icenowy Zheng]

于 2018年1月4日 GMT+08:00 上午5:32:26, "Jernej Škrabec" <jernej.skrabec-gGgVlfcn5nU@public.gmane.org> 写到:
>Hi Rob,
>
>Dne sreda, 03. januar 2018 ob 21:21:54 CET je Rob Herring napisal(a):
>> On Sat, Dec 30, 2017 at 10:01:58PM +0100, Jernej Skrabec wrote:
>> > This commit adds all necessary compatibles and descriptions needed
>to
>> > implement A83T HDMI pipeline.
>> > 
>> > Mixer is already properly described, so only compatible is added.
>> > 
>> > However, A83T TCON1, which is connected to HDMI, doesn't have
>channel 0,
>> > contrary to all TCONs currently described. Because of that, TCON
>> > documentation is extended.
>> > 
>> > A83T features Synopsys DW HDMI controller with a custom PHY which
>looks
>> > like Synopsys Gen2 PHY with few additions. Since there is no
>> > documentation, needed properties were found out through
>experimentation
>> > and reading BSP code.
>> > 
>> > At the end, example is added for newer SoCs, which features DE2 and
>DW
>> > HDMI.
>> > 
>> > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
>> > ---
>> > 
>> >  .../bindings/display/sunxi/sun4i-drm.txt           | 188
>> >  ++++++++++++++++++++- 1 file changed, 181 insertions(+), 7
>deletions(-)
>> > 
>> > diff --git
>a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
>> > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
>index
>> > 9f073af4c711..3eca258096a5 100644
>> > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
>> > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
>> > 
>> > @@ -64,6 +64,40 @@ Required properties:
>> >      first port should be the input endpoint. The second should be
>the
>> >      output, usually to an HDMI connector.
>> > 
>> > +DWC HDMI TX Encoder
>> > +-----------------------------
>> > +
>> > +The HDMI transmitter is a Synopsys DesignWare HDMI 1.4 TX
>controller IP
>> > +with Allwinner's own PHY IP. It supports audio and video outputs
>and CEC.
>> > +
>> > +These DT bindings follow the Synopsys DWC HDMI TX bindings defined
>in
>> > +Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt with
>the
>> > +following device-specific properties.
>> > +
>> > +Required properties:
>> > +
>> > +  - compatible: value must be one of:
>> > +    * "allwinner,sun8i-a83t-dw-hdmi"
>> > +  - reg: two pairs of base address and size of memory-mapped
>region,
>> > first
>> > +    for controller and second for PHY
>> > +    registers.
>> 
>> Seems like the phy should be a separate node and use the phy binding.
>> You can use the phy binding even if you don't use the kernel phy
>> framework...
>
>Unfortunately, it's not so straighforward. Phy is actually accessed
>through 
>I2C implemented in HDMI controller. Second memory region in this case
>has 
>small influence on phy. However, it has big influence on controller.

To be honest you used inaccurate word. Use "glue" directly
here may be more accurate.

>For 
>example, magic number has to be written in one register in second
>memory 
>region in order to unlock read access to any register from first memory
>region 
>(controller). However, they shouldn't be merged to one region, because
>first 
>memory region requires byte access while second memory region can be
>accessed 
>per byte or word.
>
>To complicate things more, later I want to add support for another SoC
>which 
>has same glue layer (unlocking read access, etc.) and uses memory
>mapped phy 
>registers in second memory region.
>
>I think current binding is the least complicated way to represent this.
>
>> 
>> > +  - reg-io-width: See dw_hdmi.txt. Shall be 1.
>> > +  - interrupts: HDMI interrupt number
>> > +  - clocks: phandles to the clocks feeding the HDMI encoder
>> > +    * iahb: the HDMI bus clock
>> > +    * isfr: the HDMI register clock
>> > +    * tmds: the HDMI tmds clock
>> > +  - clock-names: the clock names mentioned above
>> > +  - resets: phandles to the reset controllers driving the encoder
>> > +    * ctrl: the reset line for the controller
>> > +    * phy: the reset line for the PHY
>> > +  - reset-names: the reset names mentioned above
>> > +
>> > +  - ports: A ports node with endpoint definitions as defined in
>> > +    Documentation/devicetree/bindings/media/video-interfaces.txt.
>The
>> > +    first port should be the input endpoint. The second should be
>the
>> > +    output, usually to an HDMI connector.
>> > +
>> > 
>> >  TV Encoder
>> >  ----------
>> > 
>> > @@ -94,18 +128,17 @@ Required properties:
>> >     * allwinner,sun7i-a20-tcon
>> >     * allwinner,sun8i-a33-tcon
>> >     * allwinner,sun8i-a83t-tcon-lcd
>> > 
>> > +   * allwinner,sun8i-a83t-tcon-tv
>> > 
>> >     * allwinner,sun8i-v3s-tcon
>> >   
>> >   - reg: base address and size of memory-mapped region
>> >   - interrupts: interrupt associated to this IP
>> > 
>> > - - clocks: phandles to the clocks feeding the TCON. Three are
>needed:
>> > 
>> > + - clocks: phandles to the clocks feeding the TCON. One is needed:
>> >     - 'ahb': the interface clocks
>> > 
>> > -   - 'tcon-ch0': The clock driving the TCON channel 0
>> > 
>> >   - resets: phandles to the reset controllers driving the encoder
>> >   
>> >     - "lcd": the reset line for the TCON channel 0
>> >   
>> >   - clock-names: the clock names mentioned above
>> >   - reset-names: the reset names mentioned above
>> > 
>> > - - clock-output-names: Name of the pixel clock created
>> > 
>> >  - ports: A ports node with endpoint definitions as defined in
>> >  
>> >    Documentation/devicetree/bindings/media/video-interfaces.txt.
>The
>> > 
>> > @@ -119,11 +152,31 @@ Required properties:
>> >    channel the endpoint is associated to. If that property is not
>> >    present, the endpoint number will be used as the channel number.
>> > 
>> > -On SoCs other than the A33 and V3s, there is one more clock
>required:
>> > +Following compatibles:
>> > + * allwinner,sun4i-a10-tcon
>> > + * allwinner,sun5i-a13-tcon
>> > + * allwinner,sun6i-a31-tcon
>> > + * allwinner,sun6i-a31s-tcon
>> > + * allwinner,sun7i-a20-tcon
>> > + * allwinner,sun8i-a33-tcon
>> > + * allwinner,sun8i-a83t-tcon-lcd
>> > + * allwinner,sun8i-v3s-tcon
>> > +have additional required properties:
>> > + - 'tcon-ch0': The clock driving the TCON channel 0
>> 
>> tcon-ch0 is a clock name, not a property.
>
>right.
>
>Best regards,
>Jernej
>
>
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: Re: [PATCH 06/11] dt-bindings: display: sun4i-drm: Add A83T HDMI pipeline

[Jernej Škrabec]

Hi,

Dne petek, 05. januar 2018 ob 03:49:09 CET je Icenowy Zheng napisal(a):
> 于 2018年1月5日 GMT+08:00 上午2:52:10, Maxime Ripard <maxime.ripard@free-
electrons.com> 写到:
> >On Wed, Jan 03, 2018 at 10:32:26PM +0100, Jernej Škrabec wrote:
> >> Hi Rob,
> >> 
> >> Dne sreda, 03. januar 2018 ob 21:21:54 CET je Rob Herring napisal(a):
> >> > On Sat, Dec 30, 2017 at 10:01:58PM +0100, Jernej Skrabec wrote:
> >> > > This commit adds all necessary compatibles and descriptions
> >
> >needed to
> >
> >> > > implement A83T HDMI pipeline.
> >> > > 
> >> > > Mixer is already properly described, so only compatible is added.
> >> > > 
> >> > > However, A83T TCON1, which is connected to HDMI, doesn't have
> >
> >channel 0,
> >
> >> > > contrary to all TCONs currently described. Because of that, TCON
> >> > > documentation is extended.
> >> > > 
> >> > > A83T features Synopsys DW HDMI controller with a custom PHY which
> >
> >looks
> >
> >> > > like Synopsys Gen2 PHY with few additions. Since there is no
> >> > > documentation, needed properties were found out through
> >
> >experimentation
> >
> >> > > and reading BSP code.
> >> > > 
> >> > > At the end, example is added for newer SoCs, which features DE2
> >
> >and DW
> >
> >> > > HDMI.
> >> > > 
> >> > > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> >> > > ---
> >> > > 
> >> > >  .../bindings/display/sunxi/sun4i-drm.txt           | 188
> >> > >  ++++++++++++++++++++- 1 file changed, 181 insertions(+), 7
> >
> >deletions(-)
> >
> >> > > diff --git
> >
> >a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> >
> >> > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> >
> >index
> >
> >> > > 9f073af4c711..3eca258096a5 100644
> >> > > ---
> >
> >a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> >
> >> > > +++
> >
> >b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> >
> >> > > @@ -64,6 +64,40 @@ Required properties:
> >> > >      first port should be the input endpoint. The second should
> >
> >be the
> >
> >> > >      output, usually to an HDMI connector.
> >> > > 
> >> > > +DWC HDMI TX Encoder
> >> > > +-----------------------------
> >> > > +
> >> > > +The HDMI transmitter is a Synopsys DesignWare HDMI 1.4 TX
> >
> >controller IP
> >
> >> > > +with Allwinner's own PHY IP. It supports audio and video outputs
> >
> >and CEC.
> >
> >> > > +
> >> > > +These DT bindings follow the Synopsys DWC HDMI TX bindings
> >
> >defined in
> >
> >> > > +Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
> >
> >with the
> >
> >> > > +following device-specific properties.
> >> > > +
> >> > > +Required properties:
> >> > > +
> >> > > +  - compatible: value must be one of:
> >> > > +    * "allwinner,sun8i-a83t-dw-hdmi"
> >> > > +  - reg: two pairs of base address and size of memory-mapped
> >
> >region,
> >
> >> > > first
> >> > > +    for controller and second for PHY
> >> > > +    registers.
> >> > 
> >> > Seems like the phy should be a separate node and use the phy
> >
> >binding.
> >
> >> > You can use the phy binding even if you don't use the kernel phy
> >> > framework...
> >> 
> >> Unfortunately, it's not so straighforward. Phy is actually accessed
> >
> >through
> >
> >> I2C implemented in HDMI controller. Second memory region in this case
> >
> >has
> >
> >> small influence on phy. However, it has big influence on controller.
> >
> >For
> >
> >> example, magic number has to be written in one register in second
> >
> >memory
> >
> >> region in order to unlock read access to any register from first
> >
> >memory region
> >
> >> (controller). However, they shouldn't be merged to one region,
> >
> >because first
> >
> >> memory region requires byte access while second memory region can be
> >
> >accessed
> >
> >> per byte or word.
> >> 
> >> To complicate things more, later I want to add support for another
> >
> >SoC which
> >
> >> has same glue layer (unlocking read access, etc.) and uses memory
> >
> >mapped phy
> >
> >> registers in second memory region.
> >> 
> >> I think current binding is the least complicated way to represent
> >
> >this.
> >
> >I agree with Rob here. I did a similar thing for the DSI patches I've
> >sent a few monthes ago and it turned out to not be that difficult, so
> >I'm sure you can come up with something :)
> 
> In A83T/H3/A64/H5/R40 this part is not purely a PHY.
> It controls the access of main controller's register (e.g. read/write
> lock and register obfuscation). So it should be called a "glue"
> with PHY part (and on A83T seems a pure glue) but not a simple
>  PHY.

It's not so simple. Actually it has PHY settings also on A83T. For example, 
value at 0x01EF0001 depends on polarity. Value at 0x01EF0002 sets PHY I2C 
address. Bit 7 at 0x01EF0007 enables/disables external resistor. That is info 
I discovered/received after I sent patches, so it's not cleary marked.

Proper memory map (starts at 0x01EE0000):
0x00000 - 0x10000 -> DW HDMI controller
0x10000 - 0x10010 -> (almost?) Common PHY settings
0x10010 - 0x10020 -> Allwinner proprietary glue layer
0x10020 - 0x10040 -> Allwinner proprietary PHY (not present on A83T)

In preliminary PHY doc AW released, there are additional regs at 0x01EF0FF8 
and 0x01EF0FFC for controller ID and phy ID, but it was always 0 at A83T and 
H3.

So splitting memory in so many regions just to satisfy clean division it 
doesn't seem sane to me. Now that I checked how Maxime did it with MIPI DSI 
driver, I'm good with dividing it into two parts.

Best regards,
Jernej


> 
> >Maxime
> >
> >--
> >Maxime Ripard, Free Electrons
> >Embedded Linux and Kernel engineering
> >http://free-electrons.com
> >
> >_______________________________________________
> >linux-arm-kernel mailing list
> >linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> --
> You received this message because you are subscribed to the Google Groups
> "linux-sunxi" group. To unsubscribe from this group and stop receiving
> emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org.
> For more options, visit https://groups.google.com/d/optout.




-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: Re: [PATCH 01/11] clk: sunxi-ng: Don't set k if width is 0 for nkmp plls

[Chen-Yu Tsai]

On Fri, Jan 5, 2018 at 3:28 AM, Jernej Škrabec <jernej.skrabec-ix9DCk4F938@public.gmane.orgt> wrote:
> Hi,
>
> Dne četrtek, 04. januar 2018 ob 15:45:18 CET je Chen-Yu Tsai napisal(a):
>> On Sun, Dec 31, 2017 at 5:01 AM, Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> wrote:
>> > For example, A83T have nmp plls which are modelled as nkmp plls. Since k
>> > is not specified, it has offset 0, shift 0 and lowest value 1. This
>> > means that LSB bit is always set to 1, which may change clock rate.
>> >
>> > Fix that by applying k factor only if k width is greater than 0.
>> >
>> > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
>> > ---
>> >
>> >  drivers/clk/sunxi-ng/ccu_nkmp.c | 21 +++++++++++++--------
>> >  1 file changed, 13 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/drivers/clk/sunxi-ng/ccu_nkmp.c
>> > b/drivers/clk/sunxi-ng/ccu_nkmp.c index e58c95787f94..709f528af2b3 100644
>> > --- a/drivers/clk/sunxi-ng/ccu_nkmp.c
>> > +++ b/drivers/clk/sunxi-ng/ccu_nkmp.c
>> > @@ -81,7 +81,7 @@ static unsigned long ccu_nkmp_recalc_rate(struct clk_hw
>> > *hw,>
>> >                                         unsigned long parent_rate)
>> >
>> >  {
>> >
>> >         struct ccu_nkmp *nkmp = hw_to_ccu_nkmp(hw);
>> >
>> > -       unsigned long n, m, k, p;
>> > +       unsigned long n, m, k = 1, p;
>> >
>> >         u32 reg;
>> >
>> >         reg = readl(nkmp->common.base + nkmp->common.reg);
>> >
>> > @@ -92,11 +92,13 @@ static unsigned long ccu_nkmp_recalc_rate(struct
>> > clk_hw *hw,>
>> >         if (!n)
>> >
>> >                 n++;
>> >
>> > -       k = reg >> nkmp->k.shift;
>> > -       k &= (1 << nkmp->k.width) - 1;
>> > -       k += nkmp->k.offset;
>> > -       if (!k)
>> > -               k++;
>> > +       if (nkmp->k.width) {
>> > +               k = reg >> nkmp->k.shift;
>> > +               k &= (1 << nkmp->k.width) - 1;
>> > +               k += nkmp->k.offset;
>> > +               if (!k)
>> > +                       k++;
>> > +       }
>>
>> The conditional shouldn't be necessary. With nkmp->k.width = 0,
>> you'd simply get k & 0, which is 0, which then gets bumped up to 1,
>> unless k.offset > 1, which would be a bug.
>>
>> >         m = reg >> nkmp->m.shift;
>> >         m &= (1 << nkmp->m.width) - 1;
>> >
>> > @@ -153,12 +155,15 @@ static int ccu_nkmp_set_rate(struct clk_hw *hw,
>> > unsigned long rate,>
>> >         reg = readl(nkmp->common.base + nkmp->common.reg);
>> >         reg &= ~GENMASK(nkmp->n.width + nkmp->n.shift - 1, nkmp->n.shift);
>> >
>> > -       reg &= ~GENMASK(nkmp->k.width + nkmp->k.shift - 1, nkmp->k.shift);
>> > +       if (nkmp->k.width)
>> > +               reg &= ~GENMASK(nkmp->k.width + nkmp->k.shift - 1,
>> > +                               nkmp->k.shift);
>> >
>> >         reg &= ~GENMASK(nkmp->m.width + nkmp->m.shift - 1, nkmp->m.shift);
>> >         reg &= ~GENMASK(nkmp->p.width + nkmp->p.shift - 1, nkmp->p.shift);
>> >
>> >         reg |= (_nkmp.n - nkmp->n.offset) << nkmp->n.shift;
>> >
>> > -       reg |= (_nkmp.k - nkmp->k.offset) << nkmp->k.shift;
>> > +       if (nkmp->k.width)
>> > +               reg |= (_nkmp.k - nkmp->k.offset) << nkmp->k.shift;
>>
>> I think a better way would be
>>
>>         reg |= ((_nkmp.k - nkmp->k.offset) << nkmp->k.shift) &
>>                GENMASK(nkmp->k.width + nkmp->k.shift - 1, nkmp->k.shift);
>>
>> And do this for all the factors, not just k. This pattern is what
>> regmap_update_bits does, which seems much safer. I wonder what
>> GENMASK() with a negative value would do though...
>
> You're right, GENMASK(-1, 0) equals 0 (calculated by hand, not tested). This
> seems much more elegant solution.
>
> Semi-related question: All nmp PLLs have much wider N range than real nkmp
> PLLs. This causes integer overflow when using nkmp formula from datasheet.
> Usually, N is 1-256 for nmp PLLs, which means that for very high N factors, it
> overflows. This also causes issue that M factor is never higher than 1.

Sounds like we can't use u8 for storing the factors. At least the
intermediate values we use to calculate the rates.

>
> I was wondering, if patch would be acceptable which would change this formula:
>
> RATE = (24MHz * N * K) / (M * P)
>
> to this:
>
> RATE ((24MHz / M) * N * K) / P
>
> I checked all M factors and are all in 1-4 or 1-2 range, which means it
> wouldn't have any impact for real nkmp PLLs when parent is 24 MHz clock which
> is probably always.
>
> What do you think?

I think this is acceptable. M is normally the pre-divider, so this
actually fits how the hardware works, including possible rounding
errors.

ChenYu

> I discovered that when I tried to set A83T PLL_VIDEO to 346.5 MHz which is
> possible only when above formula is changed.
>
> Best regards,
> Jernej
>
>>
>> ChenYu
>>
>> >         reg |= (_nkmp.m - nkmp->m.offset) << nkmp->m.shift;
>> >         reg |= ilog2(_nkmp.p) << nkmp->p.shift;
>> >
>> > --
>> > 2.15.1
>
>
>
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 04/11] drm/bridge/synopsys: dw-hdmi: Export some PHY related functions

[Archit Taneja]

On 12/31/2017 02:31 AM, Jernej Skrabec wrote:
> Parts of PHY code could be useful also for custom PHYs. For example,
> Allwinner A83T has custom PHY which is probably Synopsys gen2 PHY
> with few additional memory mapped registers, so most of the Synopsys PHY
> related code could be reused.
> 
> It turns out that even completely custom HDMI PHYs, such as the one
> found in Allwinner H3, can reuse some of those functions. This would
> suggest that (some?) functions exported in this commit are actually part
> of generic PHY interface and not really specific to Synopsys PHYs.
> 
> Export useful PHY functions.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45 ++++++++++++++++++++++---------
>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.h |  2 ++
>   include/drm/bridge/dw_hdmi.h              | 10 +++++++
>   3 files changed, 44 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 7ca14d7325b5..67467d0b683a 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1037,19 +1037,21 @@ static void dw_hdmi_phy_enable_svsret(struct dw_hdmi *hdmi, u8 enable)
>   			 HDMI_PHY_CONF0_SVSRET_MASK);
>   }
>   
> -static void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable)
> +void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable)
>   {
>   	hdmi_mask_writeb(hdmi, enable, HDMI_PHY_CONF0,
>   			 HDMI_PHY_CONF0_GEN2_PDDQ_OFFSET,
>   			 HDMI_PHY_CONF0_GEN2_PDDQ_MASK);
>   }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_pddq);
>   
> -static void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable)
> +void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable)
>   {
>   	hdmi_mask_writeb(hdmi, enable, HDMI_PHY_CONF0,
>   			 HDMI_PHY_CONF0_GEN2_TXPWRON_OFFSET,
>   			 HDMI_PHY_CONF0_GEN2_TXPWRON_MASK);
>   }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_txpwron);
>   
>   static void dw_hdmi_phy_sel_data_en_pol(struct dw_hdmi *hdmi, u8 enable)
>   {
> @@ -1065,6 +1067,23 @@ static void dw_hdmi_phy_sel_interface_control(struct dw_hdmi *hdmi, u8 enable)
>   			 HDMI_PHY_CONF0_SELDIPIF_MASK);
>   }
>   
> +void dw_hdmi_phy_gen2_reset(struct dw_hdmi *hdmi, u8 enable)
> +{
> +	hdmi_mask_writeb(hdmi, enable, HDMI_MC_PHYRSTZ,
> +			 HDMI_MC_PHYRSTZ_PHYRSTZ_OFFSET,
> +			 HDMI_MC_PHYRSTZ_PHYRSTZ_MASK);
> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_reset);
> +
> +void dw_hdmi_phy_set_slave_addr(struct dw_hdmi *hdmi)
> +{
> +	hdmi_phy_test_clear(hdmi, 1);
> +	hdmi_writeb(hdmi, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2,
> +		    HDMI_PHY_I2CM_SLAVE_ADDR);
> +	hdmi_phy_test_clear(hdmi, 0);
> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_set_slave_addr);

Should this be called dw_hdmi_phy_gen2_set_slave_addr?

Looks good otherwise. Same for patches 3 and 4 in this series.

Thanks,
Archit

> +
>   static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
>   {
>   	const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
> @@ -1204,15 +1223,12 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi)
>   		dw_hdmi_phy_enable_svsret(hdmi, 1);
>   
>   	/* PHY reset. The reset signal is active high on Gen2 PHYs. */
> -	hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_PHYRSTZ, HDMI_MC_PHYRSTZ);
> -	hdmi_writeb(hdmi, 0, HDMI_MC_PHYRSTZ);
> +	dw_hdmi_phy_gen2_reset(hdmi, 1);
> +	dw_hdmi_phy_gen2_reset(hdmi, 0);
>   
>   	hdmi_writeb(hdmi, HDMI_MC_HEACPHY_RST_ASSERT, HDMI_MC_HEACPHY_RST);
>   
> -	hdmi_phy_test_clear(hdmi, 1);
> -	hdmi_writeb(hdmi, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2,
> -		    HDMI_PHY_I2CM_SLAVE_ADDR);
> -	hdmi_phy_test_clear(hdmi, 0);
> +	dw_hdmi_phy_set_slave_addr(hdmi);
>   
>   	/* Write to the PHY as configured by the platform */
>   	if (pdata->configure_phy)
> @@ -1251,15 +1267,16 @@ static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi, void *data)
>   	dw_hdmi_phy_power_off(hdmi);
>   }
>   
> -static enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
> -						      void *data)
> +enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
> +					       void *data)
>   {
>   	return hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_HPD ?
>   		connector_status_connected : connector_status_disconnected;
>   }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_read_hpd);
>   
> -static void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> -				   bool force, bool disabled, bool rxsense)
> +void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> +			    bool force, bool disabled, bool rxsense)
>   {
>   	u8 old_mask = hdmi->phy_mask;
>   
> @@ -1271,8 +1288,9 @@ static void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
>   	if (old_mask != hdmi->phy_mask)
>   		hdmi_writeb(hdmi, hdmi->phy_mask, HDMI_PHY_MASK0);
>   }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_update_hpd);
>   
> -static void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data)
> +void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data)
>   {
>   	/*
>   	 * Configure the PHY RX SENSE and HPD interrupts polarities and clear
> @@ -1291,6 +1309,7 @@ static void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data)
>   	hdmi_writeb(hdmi, ~(HDMI_IH_PHY_STAT0_HPD | HDMI_IH_PHY_STAT0_RX_SENSE),
>   		    HDMI_IH_MUTE_PHY_STAT0);
>   }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_setup_hpd);
>   
>   static const struct dw_hdmi_phy_ops dw_hdmi_synopsys_phy_ops = {
>   	.init = dw_hdmi_phy_init,
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> index 9d90eb9c46e5..fd150430d0b3 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> @@ -950,6 +950,8 @@ enum {
>   
>   /* MC_PHYRSTZ field values */
>   	HDMI_MC_PHYRSTZ_PHYRSTZ = 0x01,
> +	HDMI_MC_PHYRSTZ_PHYRSTZ_OFFSET = 0x00,
> +	HDMI_MC_PHYRSTZ_PHYRSTZ_MASK = 0x01,
>   
>   /* MC_HEACPHY_RST field values */
>   	HDMI_MC_HEACPHY_RST_ASSERT = 0x1,
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 182f83283e24..f5cca4362154 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -159,5 +159,15 @@ void dw_hdmi_audio_disable(struct dw_hdmi *hdmi);
>   /* PHY configuration */
>   void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
>   			   unsigned char addr);
> +enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
> +					       void *data);
> +void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> +			    bool force, bool disabled, bool rxsense);
> +void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data);
> +
> +void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable);
> +void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable);
> +void dw_hdmi_phy_gen2_reset(struct dw_hdmi *hdmi, u8 enable);
> +void dw_hdmi_phy_set_slave_addr(struct dw_hdmi *hdmi);
>   
>   #endif /* __IMX_HDMI_H__ */
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/11] drm/bridge/synopsys: dw-hdmi: Enable workaround for v1.32a

[Laurent Pinchart]

Hi Jernej,

Thank you for the patch.

On Saturday, 30 December 2017 23:01:55 EET Jernej Skrabec wrote:
> Allwinner SoCs have dw hdmi controller v1.32a which exhibits same
> magenta line issue as i.MX6Q and i.MX6DL. Enable workaround for it.
> 
> Tests show that one iteration is enough.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>

This does not break R-Car DU, so

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> a38db40ce990..7ca14d7325b5 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1634,9 +1634,10 @@ static void dw_hdmi_clear_overflow(struct dw_hdmi
> *hdmi) * then write one of the FC registers several times.
>  	 *
>  	 * The number of iterations matters and depends on the HDMI TX revision
> -	 * (and possibly on the platform). So far only i.MX6Q (v1.30a) and
> -	 * i.MX6DL (v1.31a) have been identified as needing the workaround, with
> -	 * 4 and 1 iterations respectively.
> +	 * (and possibly on the platform). So far i.MX6Q (v1.30a), i.MX6DL
> +	 * (v1.31a) and multiple Allwinner SoCs (v1.32a) have been identified
> +	 * as needing the workaround, with 4 iterations for v1.30a and 1
> +	 * iteration for others.
>  	 */
> 
>  	switch (hdmi->version) {
> @@ -1644,6 +1645,7 @@ static void dw_hdmi_clear_overflow(struct dw_hdmi
> *hdmi) count = 4;
>  		break;
>  	case 0x131a:
> +	case 0x132a:
>  		count = 1;
>  		break;
>  	default:

-- 
Regards,

Laurent Pinchart

Re: [PATCH 04/11] drm/bridge/synopsys: dw-hdmi: Export some PHY related functions

[Laurent Pinchart]

Hi Jernej,

Thank you for the patch.

On Saturday, 30 December 2017 23:01:56 EET Jernej Skrabec wrote:
> Parts of PHY code could be useful also for custom PHYs. For example,
> Allwinner A83T has custom PHY which is probably Synopsys gen2 PHY
> with few additional memory mapped registers, so most of the Synopsys PHY
> related code could be reused.
> 
> It turns out that even completely custom HDMI PHYs, such as the one
> found in Allwinner H3, can reuse some of those functions. This would
> suggest that (some?) functions exported in this commit are actually part
> of generic PHY interface and not really specific to Synopsys PHYs.

That's correct, those functions control the interface between the HDMI 
controller and the PHY. They're not specific to Synopsys PHYs, but they're 
specific to the PHY interface as designed by Synopsys.

> Export useful PHY functions.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45 ++++++++++++++++++++-------
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.h |  2 ++
>  include/drm/bridge/dw_hdmi.h              | 10 +++++++
>  3 files changed, 44 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> 7ca14d7325b5..67467d0b683a 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1037,19 +1037,21 @@ static void dw_hdmi_phy_enable_svsret(struct dw_hdmi
> *hdmi, u8 enable) HDMI_PHY_CONF0_SVSRET_MASK);
>  }
> 
> -static void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable)
> +void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable)
>  {
>  	hdmi_mask_writeb(hdmi, enable, HDMI_PHY_CONF0,
>  			 HDMI_PHY_CONF0_GEN2_PDDQ_OFFSET,
>  			 HDMI_PHY_CONF0_GEN2_PDDQ_MASK);
>  }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_pddq);
> 
> -static void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable)
> +void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable)
>  {
>  	hdmi_mask_writeb(hdmi, enable, HDMI_PHY_CONF0,
>  			 HDMI_PHY_CONF0_GEN2_TXPWRON_OFFSET,
>  			 HDMI_PHY_CONF0_GEN2_TXPWRON_MASK);
>  }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_txpwron);
> 
>  static void dw_hdmi_phy_sel_data_en_pol(struct dw_hdmi *hdmi, u8 enable)
>  {
> @@ -1065,6 +1067,23 @@ static void dw_hdmi_phy_sel_interface_control(struct
> dw_hdmi *hdmi, u8 enable) HDMI_PHY_CONF0_SELDIPIF_MASK);
>  }
> 
> +void dw_hdmi_phy_gen2_reset(struct dw_hdmi *hdmi, u8 enable)
> +{
> +	hdmi_mask_writeb(hdmi, enable, HDMI_MC_PHYRSTZ,
> +			 HDMI_MC_PHYRSTZ_PHYRSTZ_OFFSET,
> +			 HDMI_MC_PHYRSTZ_PHYRSTZ_MASK);
> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_reset);
> +
> +void dw_hdmi_phy_set_slave_addr(struct dw_hdmi *hdmi)
> +{
> +	hdmi_phy_test_clear(hdmi, 1);
> +	hdmi_writeb(hdmi, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2,
> +		    HDMI_PHY_I2CM_SLAVE_ADDR);
> +	hdmi_phy_test_clear(hdmi, 0);
> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_set_slave_addr);

Should the I2C address be passed as an argument ?

>  static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
>  {
>  	const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
> @@ -1204,15 +1223,12 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi)
>  		dw_hdmi_phy_enable_svsret(hdmi, 1);
> 
>  	/* PHY reset. The reset signal is active high on Gen2 PHYs. */
> -	hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_PHYRSTZ, HDMI_MC_PHYRSTZ);
> -	hdmi_writeb(hdmi, 0, HDMI_MC_PHYRSTZ);
> +	dw_hdmi_phy_gen2_reset(hdmi, 1);
> +	dw_hdmi_phy_gen2_reset(hdmi, 0);
> 
>  	hdmi_writeb(hdmi, HDMI_MC_HEACPHY_RST_ASSERT, HDMI_MC_HEACPHY_RST);
> 
> -	hdmi_phy_test_clear(hdmi, 1);
> -	hdmi_writeb(hdmi, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2,
> -		    HDMI_PHY_I2CM_SLAVE_ADDR);
> -	hdmi_phy_test_clear(hdmi, 0);
> +	dw_hdmi_phy_set_slave_addr(hdmi);
> 
>  	/* Write to the PHY as configured by the platform */
>  	if (pdata->configure_phy)
> @@ -1251,15 +1267,16 @@ static void dw_hdmi_phy_disable(struct dw_hdmi
> *hdmi, void *data) dw_hdmi_phy_power_off(hdmi);
>  }
> 
> -static enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
> -						      void *data)
> +enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
> +					       void *data)
>  {
>  	return hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_HPD ?
>  		connector_status_connected : connector_status_disconnected;
>  }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_read_hpd);
> 
> -static void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> -				   bool force, bool disabled, bool rxsense)
> +void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> +			    bool force, bool disabled, bool rxsense)
>  {
>  	u8 old_mask = hdmi->phy_mask;
> 
> @@ -1271,8 +1288,9 @@ static void dw_hdmi_phy_update_hpd(struct dw_hdmi
> *hdmi, void *data, if (old_mask != hdmi->phy_mask)
>  		hdmi_writeb(hdmi, hdmi->phy_mask, HDMI_PHY_MASK0);
>  }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_update_hpd);
> 
> -static void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data)
> +void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data)
>  {
>  	/*
>  	 * Configure the PHY RX SENSE and HPD interrupts polarities and clear
> @@ -1291,6 +1309,7 @@ static void dw_hdmi_phy_setup_hpd(struct dw_hdmi
> *hdmi, void *data) hdmi_writeb(hdmi, ~(HDMI_IH_PHY_STAT0_HPD |
> HDMI_IH_PHY_STAT0_RX_SENSE), HDMI_IH_MUTE_PHY_STAT0);
>  }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_setup_hpd);
> 
>  static const struct dw_hdmi_phy_ops dw_hdmi_synopsys_phy_ops = {
>  	.init = dw_hdmi_phy_init,
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h index
> 9d90eb9c46e5..fd150430d0b3 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> @@ -950,6 +950,8 @@ enum {
> 
>  /* MC_PHYRSTZ field values */
>  	HDMI_MC_PHYRSTZ_PHYRSTZ = 0x01,
> +	HDMI_MC_PHYRSTZ_PHYRSTZ_OFFSET = 0x00,
> +	HDMI_MC_PHYRSTZ_PHYRSTZ_MASK = 0x01,
> 
>  /* MC_HEACPHY_RST field values */
>  	HDMI_MC_HEACPHY_RST_ASSERT = 0x1,
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 182f83283e24..f5cca4362154 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -159,5 +159,15 @@ void dw_hdmi_audio_disable(struct dw_hdmi *hdmi);
>  /* PHY configuration */
>  void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
>  			   unsigned char addr);
> +enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
> +					       void *data);
> +void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> +			    bool force, bool disabled, bool rxsense);
> +void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data);
> +
> +void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable);
> +void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable);
> +void dw_hdmi_phy_gen2_reset(struct dw_hdmi *hdmi, u8 enable);
> +void dw_hdmi_phy_set_slave_addr(struct dw_hdmi *hdmi);
> 
>  #endif /* __IMX_HDMI_H__ */

-- 
Regards,

Laurent Pinchart

Re: [PATCH 03/11] drm/bridge/synopsys: dw-hdmi: Enable workaround for v1.32a

[Neil Armstrong]

Hi,

I think this question is for Jose.

On 09/01/2018 13:56, Laurent Pinchart wrote:
> Hi Jernej,
> 
> Thank you for the patch.
> 
> On Saturday, 30 December 2017 23:01:55 EET Jernej Skrabec wrote:
>> Allwinner SoCs have dw hdmi controller v1.32a which exhibits same
>> magenta line issue as i.MX6Q and i.MX6DL. Enable workaround for it.

We observe the same issue on Amlogic SoCs with the dw hdmi controller v2.01a.

Rockchip seems to also use count=1 for 0x200a, 0x201a and 0x211a in
https://github.com/rockchip-linux/kernel/commit/cafa8ebd5f4df41425d6f2f61633d5bc64f20e65

Changelog is :
The issue can be worked around by issuing a TMDS software reset and
then write one of the FC registers several times. After tested, the
number of iterations of RK3399/RK3328(v2.11a), RK3368(v2.01a),
RK3288(v2.00a) is one.

Can you confirm it is necessary ?

Neil

>>
>> Tests show that one iteration is enough.
>>
>> Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> 
> This does not break R-Car DU, so
> 
> Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> 
>> ---
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
>> a38db40ce990..7ca14d7325b5 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -1634,9 +1634,10 @@ static void dw_hdmi_clear_overflow(struct dw_hdmi
>> *hdmi) * then write one of the FC registers several times.
>>  	 *
>>  	 * The number of iterations matters and depends on the HDMI TX revision
>> -	 * (and possibly on the platform). So far only i.MX6Q (v1.30a) and
>> -	 * i.MX6DL (v1.31a) have been identified as needing the workaround, with
>> -	 * 4 and 1 iterations respectively.
>> +	 * (and possibly on the platform). So far i.MX6Q (v1.30a), i.MX6DL
>> +	 * (v1.31a) and multiple Allwinner SoCs (v1.32a) have been identified
>> +	 * as needing the workaround, with 4 iterations for v1.30a and 1
>> +	 * iteration for others.
>>  	 */
>>
>>  	switch (hdmi->version) {
>> @@ -1644,6 +1645,7 @@ static void dw_hdmi_clear_overflow(struct dw_hdmi
>> *hdmi) count = 4;
>>  		break;
>>  	case 0x131a:
>> +	case 0x132a:
>>  		count = 1;
>>  		break;
>>  	default:
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-sunxi] Re: [PATCH 01/11] clk: sunxi-ng: Don't set k if width is 0 for nkmp plls

[Jernej Škrabec]

Hi Chen-Yu,

Dne ponedeljek, 08. januar 2018 ob 10:19:47 CET je Chen-Yu Tsai napisal(a):
> On Fri, Jan 5, 2018 at 3:28 AM, Jernej Škrabec <jernej.skrabec@siol.net> 
wrote:
> > Hi,
> > 
> > Dne četrtek, 04. januar 2018 ob 15:45:18 CET je Chen-Yu Tsai napisal(a):
> >> On Sun, Dec 31, 2017 at 5:01 AM, Jernej Skrabec <jernej.skrabec@siol.net>
> > 
> > wrote:
> >> > For example, A83T have nmp plls which are modelled as nkmp plls. Since
> >> > k
> >> > is not specified, it has offset 0, shift 0 and lowest value 1. This
> >> > means that LSB bit is always set to 1, which may change clock rate.
> >> > 
> >> > Fix that by applying k factor only if k width is greater than 0.
> >> > 
> >> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> >> > ---
> >> > 
> >> >  drivers/clk/sunxi-ng/ccu_nkmp.c | 21 +++++++++++++--------
> >> >  1 file changed, 13 insertions(+), 8 deletions(-)
> >> > 
> >> > diff --git a/drivers/clk/sunxi-ng/ccu_nkmp.c
> >> > b/drivers/clk/sunxi-ng/ccu_nkmp.c index e58c95787f94..709f528af2b3
> >> > 100644
> >> > --- a/drivers/clk/sunxi-ng/ccu_nkmp.c
> >> > +++ b/drivers/clk/sunxi-ng/ccu_nkmp.c
> >> > @@ -81,7 +81,7 @@ static unsigned long ccu_nkmp_recalc_rate(struct
> >> > clk_hw
> >> > *hw,>
> >> > 
> >> >                                         unsigned long parent_rate)
> >> >  
> >> >  {
> >> >  
> >> >         struct ccu_nkmp *nkmp = hw_to_ccu_nkmp(hw);
> >> > 
> >> > -       unsigned long n, m, k, p;
> >> > +       unsigned long n, m, k = 1, p;
> >> > 
> >> >         u32 reg;
> >> >         
> >> >         reg = readl(nkmp->common.base + nkmp->common.reg);
> >> > 
> >> > @@ -92,11 +92,13 @@ static unsigned long ccu_nkmp_recalc_rate(struct
> >> > clk_hw *hw,>
> >> > 
> >> >         if (!n)
> >> >         
> >> >                 n++;
> >> > 
> >> > -       k = reg >> nkmp->k.shift;
> >> > -       k &= (1 << nkmp->k.width) - 1;
> >> > -       k += nkmp->k.offset;
> >> > -       if (!k)
> >> > -               k++;
> >> > +       if (nkmp->k.width) {
> >> > +               k = reg >> nkmp->k.shift;
> >> > +               k &= (1 << nkmp->k.width) - 1;
> >> > +               k += nkmp->k.offset;
> >> > +               if (!k)
> >> > +                       k++;
> >> > +       }
> >> 
> >> The conditional shouldn't be necessary. With nkmp->k.width = 0,
> >> you'd simply get k & 0, which is 0, which then gets bumped up to 1,
> >> unless k.offset > 1, which would be a bug.
> >> 
> >> >         m = reg >> nkmp->m.shift;
> >> >         m &= (1 << nkmp->m.width) - 1;
> >> > 
> >> > @@ -153,12 +155,15 @@ static int ccu_nkmp_set_rate(struct clk_hw *hw,
> >> > unsigned long rate,>
> >> > 
> >> >         reg = readl(nkmp->common.base + nkmp->common.reg);
> >> >         reg &= ~GENMASK(nkmp->n.width + nkmp->n.shift - 1,
> >> >         nkmp->n.shift);
> >> > 
> >> > -       reg &= ~GENMASK(nkmp->k.width + nkmp->k.shift - 1,
> >> > nkmp->k.shift);
> >> > +       if (nkmp->k.width)
> >> > +               reg &= ~GENMASK(nkmp->k.width + nkmp->k.shift - 1,
> >> > +                               nkmp->k.shift);
> >> > 
> >> >         reg &= ~GENMASK(nkmp->m.width + nkmp->m.shift - 1,
> >> >         nkmp->m.shift);
> >> >         reg &= ~GENMASK(nkmp->p.width + nkmp->p.shift - 1,
> >> >         nkmp->p.shift);
> >> >         
> >> >         reg |= (_nkmp.n - nkmp->n.offset) << nkmp->n.shift;
> >> > 
> >> > -       reg |= (_nkmp.k - nkmp->k.offset) << nkmp->k.shift;
> >> > +       if (nkmp->k.width)
> >> > +               reg |= (_nkmp.k - nkmp->k.offset) << nkmp->k.shift;
> >> 
> >> I think a better way would be
> >> 
> >>         reg |= ((_nkmp.k - nkmp->k.offset) << nkmp->k.shift) &
> >>         
> >>                GENMASK(nkmp->k.width + nkmp->k.shift - 1, nkmp->k.shift);
> >> 
> >> And do this for all the factors, not just k. This pattern is what
> >> regmap_update_bits does, which seems much safer. I wonder what
> >> GENMASK() with a negative value would do though...
> > 
> > You're right, GENMASK(-1, 0) equals 0 (calculated by hand, not tested).
> > This seems much more elegant solution.
> > 
> > Semi-related question: All nmp PLLs have much wider N range than real nkmp
> > PLLs. This causes integer overflow when using nkmp formula from datasheet.
> > Usually, N is 1-256 for nmp PLLs, which means that for very high N
> > factors, it overflows. This also causes issue that M factor is never
> > higher than 1.
> Sounds like we can't use u8 for storing the factors. At least the
> intermediate values we use to calculate the rates.

Only issue with u8 could be max field in struct ccu_mult_internal for K factor. 
But since it's not used, there is no issue. All intermediate variables in 
ccu_nkmp are wider.

> 
> > I was wondering, if patch would be acceptable which would change this
> > formula:
> > 
> > RATE = (24MHz * N * K) / (M * P)
> > 
> > to this:
> > 
> > RATE ((24MHz / M) * N * K) / P
> > 
> > I checked all M factors and are all in 1-4 or 1-2 range, which means it
> > wouldn't have any impact for real nkmp PLLs when parent is 24 MHz clock
> > which is probably always.
> > 
> > What do you think?
> 
> I think this is acceptable. M is normally the pre-divider, so this
> actually fits how the hardware works, including possible rounding
> errors.

Ok, I'll add a patch for that in v2.

Best regards,
Jernej

> 
> ChenYu
> 
> > I discovered that when I tried to set A83T PLL_VIDEO to 346.5 MHz which is
> > possible only when above formula is changed.
> > 
> > Best regards,
> > Jernej
> > 
> >> ChenYu
> >> 
> >> >         reg |= (_nkmp.m - nkmp->m.offset) << nkmp->m.shift;
> >> >         reg |= ilog2(_nkmp.p) << nkmp->p.shift;
> >> > 
> >> > --
> >> > 2.15.1
> > 
> > --
> > You received this message because you are subscribed to the Google Groups
> > "linux-sunxi" group. To unsubscribe from this group and stop receiving
> > emails from it, send an email to
> > linux-sunxi+unsubscribe@googlegroups.com. For more options, visit
> > https://groups.google.com/d/optout.

Re: [PATCH 04/11] drm/bridge/synopsys: dw-hdmi: Export some PHY related functions

[Jernej Škrabec]

Hi Archit,

Dne torek, 09. januar 2018 ob 11:43:08 CET je Archit Taneja napisal(a):
> On 12/31/2017 02:31 AM, Jernej Skrabec wrote:
> > Parts of PHY code could be useful also for custom PHYs. For example,
> > Allwinner A83T has custom PHY which is probably Synopsys gen2 PHY
> > with few additional memory mapped registers, so most of the Synopsys PHY
> > related code could be reused.
> > 
> > It turns out that even completely custom HDMI PHYs, such as the one
> > found in Allwinner H3, can reuse some of those functions. This would
> > suggest that (some?) functions exported in this commit are actually part
> > of generic PHY interface and not really specific to Synopsys PHYs.
> > 
> > Export useful PHY functions.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> > ---
> > 
> >   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45
> >   ++++++++++++++++++++++---------
> >   drivers/gpu/drm/bridge/synopsys/dw-hdmi.h |  2 ++
> >   include/drm/bridge/dw_hdmi.h              | 10 +++++++
> >   3 files changed, 44 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> > 7ca14d7325b5..67467d0b683a 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -1037,19 +1037,21 @@ static void dw_hdmi_phy_enable_svsret(struct
> > dw_hdmi *hdmi, u8 enable)> 
> >   			 HDMI_PHY_CONF0_SVSRET_MASK);
> >   
> >   }
> > 
> > -static void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable)
> > +void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable)
> > 
> >   {
> >   
> >   	hdmi_mask_writeb(hdmi, enable, HDMI_PHY_CONF0,
> >   	
> >   			 HDMI_PHY_CONF0_GEN2_PDDQ_OFFSET,
> >   			 HDMI_PHY_CONF0_GEN2_PDDQ_MASK);
> >   
> >   }
> > 
> > +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_pddq);
> > 
> > -static void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable)
> > +void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable)
> > 
> >   {
> >   
> >   	hdmi_mask_writeb(hdmi, enable, HDMI_PHY_CONF0,
> >   	
> >   			 HDMI_PHY_CONF0_GEN2_TXPWRON_OFFSET,
> >   			 HDMI_PHY_CONF0_GEN2_TXPWRON_MASK);
> >   
> >   }
> > 
> > +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_txpwron);
> > 
> >   static void dw_hdmi_phy_sel_data_en_pol(struct dw_hdmi *hdmi, u8 enable)
> >   {
> > 
> > @@ -1065,6 +1067,23 @@ static void
> > dw_hdmi_phy_sel_interface_control(struct dw_hdmi *hdmi, u8 enable)> 
> >   			 HDMI_PHY_CONF0_SELDIPIF_MASK);
> >   
> >   }
> > 
> > +void dw_hdmi_phy_gen2_reset(struct dw_hdmi *hdmi, u8 enable)
> > +{
> > +	hdmi_mask_writeb(hdmi, enable, HDMI_MC_PHYRSTZ,
> > +			 HDMI_MC_PHYRSTZ_PHYRSTZ_OFFSET,
> > +			 HDMI_MC_PHYRSTZ_PHYRSTZ_MASK);
> > +}
> > +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_reset);
> > +
> > +void dw_hdmi_phy_set_slave_addr(struct dw_hdmi *hdmi)
> > +{
> > +	hdmi_phy_test_clear(hdmi, 1);
> > +	hdmi_writeb(hdmi, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2,
> > +		    HDMI_PHY_I2CM_SLAVE_ADDR);
> > +	hdmi_phy_test_clear(hdmi, 0);
> > +}
> > +EXPORT_SYMBOL_GPL(dw_hdmi_phy_set_slave_addr);
> 
> Should this be called dw_hdmi_phy_gen2_set_slave_addr?

Probably. I will rename it in v2 to be consistent with other phy functions.

Best regards,
Jernej

> 
> Looks good otherwise. Same for patches 3 and 4 in this series.
> 
> Thanks,
> Archit
> 
> > +
> > 
> >   static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
> >   {
> >   
> >   	const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
> > 
> > @@ -1204,15 +1223,12 @@ static int hdmi_phy_configure(struct dw_hdmi
> > *hdmi)
> > 
> >   		dw_hdmi_phy_enable_svsret(hdmi, 1);
> >   	
> >   	/* PHY reset. The reset signal is active high on Gen2 PHYs. */
> > 
> > -	hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_PHYRSTZ, HDMI_MC_PHYRSTZ);
> > -	hdmi_writeb(hdmi, 0, HDMI_MC_PHYRSTZ);
> > +	dw_hdmi_phy_gen2_reset(hdmi, 1);
> > +	dw_hdmi_phy_gen2_reset(hdmi, 0);
> > 
> >   	hdmi_writeb(hdmi, HDMI_MC_HEACPHY_RST_ASSERT, HDMI_MC_HEACPHY_RST);
> > 
> > -	hdmi_phy_test_clear(hdmi, 1);
> > -	hdmi_writeb(hdmi, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2,
> > -		    HDMI_PHY_I2CM_SLAVE_ADDR);
> > -	hdmi_phy_test_clear(hdmi, 0);
> > +	dw_hdmi_phy_set_slave_addr(hdmi);
> > 
> >   	/* Write to the PHY as configured by the platform */
> >   	if (pdata->configure_phy)
> > 
> > @@ -1251,15 +1267,16 @@ static void dw_hdmi_phy_disable(struct dw_hdmi
> > *hdmi, void *data)> 
> >   	dw_hdmi_phy_power_off(hdmi);
> >   
> >   }
> > 
> > -static enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi
> > *hdmi, -						      void *data)
> > +enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
> > +					       void *data)
> > 
> >   {
> >   
> >   	return hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_HPD ?
> >   	
> >   		connector_status_connected : connector_status_disconnected;
> >   
> >   }
> > 
> > +EXPORT_SYMBOL_GPL(dw_hdmi_phy_read_hpd);
> > 
> > -static void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> > -				   bool force, bool disabled, bool rxsense)
> > +void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> > +			    bool force, bool disabled, bool rxsense)
> > 
> >   {
> >   
> >   	u8 old_mask = hdmi->phy_mask;
> > 
> > @@ -1271,8 +1288,9 @@ static void dw_hdmi_phy_update_hpd(struct dw_hdmi
> > *hdmi, void *data,> 
> >   	if (old_mask != hdmi->phy_mask)
> >   	
> >   		hdmi_writeb(hdmi, hdmi->phy_mask, HDMI_PHY_MASK0);
> >   
> >   }
> > 
> > +EXPORT_SYMBOL_GPL(dw_hdmi_phy_update_hpd);
> > 
> > -static void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data)
> > +void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data)
> > 
> >   {
> >   
> >   	/*
> >   	
> >   	 * Configure the PHY RX SENSE and HPD interrupts polarities and clear
> > 
> > @@ -1291,6 +1309,7 @@ static void dw_hdmi_phy_setup_hpd(struct dw_hdmi
> > *hdmi, void *data)> 
> >   	hdmi_writeb(hdmi, ~(HDMI_IH_PHY_STAT0_HPD |
> >   	HDMI_IH_PHY_STAT0_RX_SENSE),
> >   	
> >   		    HDMI_IH_MUTE_PHY_STAT0);
> >   
> >   }
> > 
> > +EXPORT_SYMBOL_GPL(dw_hdmi_phy_setup_hpd);
> > 
> >   static const struct dw_hdmi_phy_ops dw_hdmi_synopsys_phy_ops = {
> >   
> >   	.init = dw_hdmi_phy_init,
> > 
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h index
> > 9d90eb9c46e5..fd150430d0b3 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > @@ -950,6 +950,8 @@ enum {
> > 
> >   /* MC_PHYRSTZ field values */
> >   
> >   	HDMI_MC_PHYRSTZ_PHYRSTZ = 0x01,
> > 
> > +	HDMI_MC_PHYRSTZ_PHYRSTZ_OFFSET = 0x00,
> > +	HDMI_MC_PHYRSTZ_PHYRSTZ_MASK = 0x01,
> > 
> >   /* MC_HEACPHY_RST field values */
> >   
> >   	HDMI_MC_HEACPHY_RST_ASSERT = 0x1,
> > 
> > diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> > index 182f83283e24..f5cca4362154 100644
> > --- a/include/drm/bridge/dw_hdmi.h
> > +++ b/include/drm/bridge/dw_hdmi.h
> > @@ -159,5 +159,15 @@ void dw_hdmi_audio_disable(struct dw_hdmi *hdmi);
> > 
> >   /* PHY configuration */
> >   void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
> >   
> >   			   unsigned char addr);
> > 
> > +enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
> > +					       void *data);
> > +void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> > +			    bool force, bool disabled, bool rxsense);
> > +void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data);
> > +
> > +void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable);
> > +void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable);
> > +void dw_hdmi_phy_gen2_reset(struct dw_hdmi *hdmi, u8 enable);
> > +void dw_hdmi_phy_set_slave_addr(struct dw_hdmi *hdmi);
> > 
> >   #endif /* __IMX_HDMI_H__ */
> 
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

Re: [PATCH 04/11] drm/bridge/synopsys: dw-hdmi: Export some PHY related functions

[Jernej Škrabec]

Hi Laurent,

Dne torek, 09. januar 2018 ob 14:30:22 CET je Laurent Pinchart napisal(a):
> Hi Jernej,
> 
> Thank you for the patch.
> 
> On Saturday, 30 December 2017 23:01:56 EET Jernej Skrabec wrote:
> > Parts of PHY code could be useful also for custom PHYs. For example,
> > Allwinner A83T has custom PHY which is probably Synopsys gen2 PHY
> > with few additional memory mapped registers, so most of the Synopsys PHY
> > related code could be reused.
> > 
> > It turns out that even completely custom HDMI PHYs, such as the one
> > found in Allwinner H3, can reuse some of those functions. This would
> > suggest that (some?) functions exported in this commit are actually part
> > of generic PHY interface and not really specific to Synopsys PHYs.
> 
> That's correct, those functions control the interface between the HDMI
> controller and the PHY. They're not specific to Synopsys PHYs, but they're
> specific to the PHY interface as designed by Synopsys.

Ok, I'll update commit message.

> 
> > Export useful PHY functions.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> > ---
> > 
> >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45
> >  ++++++++++++++++++++-------
> >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.h |  2 ++
> >  include/drm/bridge/dw_hdmi.h              | 10 +++++++
> >  3 files changed, 44 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> > 7ca14d7325b5..67467d0b683a 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -1037,19 +1037,21 @@ static void dw_hdmi_phy_enable_svsret(struct
> > dw_hdmi *hdmi, u8 enable) HDMI_PHY_CONF0_SVSRET_MASK);
> > 
> >  }
> > 
> > -static void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable)
> > +void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable)
> > 
> >  {
> >  
> >  	hdmi_mask_writeb(hdmi, enable, HDMI_PHY_CONF0,
> >  	
> >  			 HDMI_PHY_CONF0_GEN2_PDDQ_OFFSET,
> >  			 HDMI_PHY_CONF0_GEN2_PDDQ_MASK);
> >  
> >  }
> > 
> > +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_pddq);
> > 
> > -static void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable)
> > +void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable)
> > 
> >  {
> >  
> >  	hdmi_mask_writeb(hdmi, enable, HDMI_PHY_CONF0,
> >  	
> >  			 HDMI_PHY_CONF0_GEN2_TXPWRON_OFFSET,
> >  			 HDMI_PHY_CONF0_GEN2_TXPWRON_MASK);
> >  
> >  }
> > 
> > +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_txpwron);
> > 
> >  static void dw_hdmi_phy_sel_data_en_pol(struct dw_hdmi *hdmi, u8 enable)
> >  {
> > 
> > @@ -1065,6 +1067,23 @@ static void
> > dw_hdmi_phy_sel_interface_control(struct
> > dw_hdmi *hdmi, u8 enable) HDMI_PHY_CONF0_SELDIPIF_MASK);
> > 
> >  }
> > 
> > +void dw_hdmi_phy_gen2_reset(struct dw_hdmi *hdmi, u8 enable)
> > +{
> > +	hdmi_mask_writeb(hdmi, enable, HDMI_MC_PHYRSTZ,
> > +			 HDMI_MC_PHYRSTZ_PHYRSTZ_OFFSET,
> > +			 HDMI_MC_PHYRSTZ_PHYRSTZ_MASK);
> > +}
> > +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_reset);
> > +
> > +void dw_hdmi_phy_set_slave_addr(struct dw_hdmi *hdmi)
> > +{
> > +	hdmi_phy_test_clear(hdmi, 1);
> > +	hdmi_writeb(hdmi, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2,
> > +		    HDMI_PHY_I2CM_SLAVE_ADDR);
> > +	hdmi_phy_test_clear(hdmi, 0);
> > +}
> > +EXPORT_SYMBOL_GPL(dw_hdmi_phy_set_slave_addr);
> 
> Should the I2C address be passed as an argument ?

Yes, I already planned to do that for v2.

Best regards,
Jernej

> 
> >  static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
> >  {
> >  
> >  	const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
> > 
> > @@ -1204,15 +1223,12 @@ static int hdmi_phy_configure(struct dw_hdmi
> > *hdmi)
> > 
> >  		dw_hdmi_phy_enable_svsret(hdmi, 1);
> >  	
> >  	/* PHY reset. The reset signal is active high on Gen2 PHYs. */
> > 
> > -	hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_PHYRSTZ, HDMI_MC_PHYRSTZ);
> > -	hdmi_writeb(hdmi, 0, HDMI_MC_PHYRSTZ);
> > +	dw_hdmi_phy_gen2_reset(hdmi, 1);
> > +	dw_hdmi_phy_gen2_reset(hdmi, 0);
> > 
> >  	hdmi_writeb(hdmi, HDMI_MC_HEACPHY_RST_ASSERT, HDMI_MC_HEACPHY_RST);
> > 
> > -	hdmi_phy_test_clear(hdmi, 1);
> > -	hdmi_writeb(hdmi, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2,
> > -		    HDMI_PHY_I2CM_SLAVE_ADDR);
> > -	hdmi_phy_test_clear(hdmi, 0);
> > +	dw_hdmi_phy_set_slave_addr(hdmi);
> > 
> >  	/* Write to the PHY as configured by the platform */
> >  	if (pdata->configure_phy)
> > 
> > @@ -1251,15 +1267,16 @@ static void dw_hdmi_phy_disable(struct dw_hdmi
> > *hdmi, void *data) dw_hdmi_phy_power_off(hdmi);
> > 
> >  }
> > 
> > -static enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi
> > *hdmi, -						      void *data)
> > +enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
> > +					       void *data)
> > 
> >  {
> >  
> >  	return hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_HPD ?
> >  	
> >  		connector_status_connected : connector_status_disconnected;
> >  
> >  }
> > 
> > +EXPORT_SYMBOL_GPL(dw_hdmi_phy_read_hpd);
> > 
> > -static void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> > -				   bool force, bool disabled, bool rxsense)
> > +void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> > +			    bool force, bool disabled, bool rxsense)
> > 
> >  {
> >  
> >  	u8 old_mask = hdmi->phy_mask;
> > 
> > @@ -1271,8 +1288,9 @@ static void dw_hdmi_phy_update_hpd(struct dw_hdmi
> > *hdmi, void *data, if (old_mask != hdmi->phy_mask)
> > 
> >  		hdmi_writeb(hdmi, hdmi->phy_mask, HDMI_PHY_MASK0);
> >  
> >  }
> > 
> > +EXPORT_SYMBOL_GPL(dw_hdmi_phy_update_hpd);
> > 
> > -static void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data)
> > +void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data)
> > 
> >  {
> >  
> >  	/*
> >  	
> >  	 * Configure the PHY RX SENSE and HPD interrupts polarities and clear
> > 
> > @@ -1291,6 +1309,7 @@ static void dw_hdmi_phy_setup_hpd(struct dw_hdmi
> > *hdmi, void *data) hdmi_writeb(hdmi, ~(HDMI_IH_PHY_STAT0_HPD |
> > HDMI_IH_PHY_STAT0_RX_SENSE), HDMI_IH_MUTE_PHY_STAT0);
> > 
> >  }
> > 
> > +EXPORT_SYMBOL_GPL(dw_hdmi_phy_setup_hpd);
> > 
> >  static const struct dw_hdmi_phy_ops dw_hdmi_synopsys_phy_ops = {
> >  
> >  	.init = dw_hdmi_phy_init,
> > 
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h index
> > 9d90eb9c46e5..fd150430d0b3 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > @@ -950,6 +950,8 @@ enum {
> > 
> >  /* MC_PHYRSTZ field values */
> >  
> >  	HDMI_MC_PHYRSTZ_PHYRSTZ = 0x01,
> > 
> > +	HDMI_MC_PHYRSTZ_PHYRSTZ_OFFSET = 0x00,
> > +	HDMI_MC_PHYRSTZ_PHYRSTZ_MASK = 0x01,
> > 
> >  /* MC_HEACPHY_RST field values */
> >  
> >  	HDMI_MC_HEACPHY_RST_ASSERT = 0x1,
> > 
> > diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> > index 182f83283e24..f5cca4362154 100644
> > --- a/include/drm/bridge/dw_hdmi.h
> > +++ b/include/drm/bridge/dw_hdmi.h
> > @@ -159,5 +159,15 @@ void dw_hdmi_audio_disable(struct dw_hdmi *hdmi);
> > 
> >  /* PHY configuration */
> >  void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
> >  
> >  			   unsigned char addr);
> > 
> > +enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
> > +					       void *data);
> > +void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
> > +			    bool force, bool disabled, bool rxsense);
> > +void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data);
> > +
> > +void dw_hdmi_phy_gen2_pddq(struct dw_hdmi *hdmi, u8 enable);
> > +void dw_hdmi_phy_gen2_txpwron(struct dw_hdmi *hdmi, u8 enable);
> > +void dw_hdmi_phy_gen2_reset(struct dw_hdmi *hdmi, u8 enable);
> > +void dw_hdmi_phy_set_slave_addr(struct dw_hdmi *hdmi);
> > 
> >  #endif /* __IMX_HDMI_H__ */
> 
> --
> Regards,
> 
> Laurent Pinchart

Re: [PATCH 04/11] drm/bridge/synopsys: dw-hdmi: Export some PHY related functions

[Laurent Pinchart]

Hello,

On Tuesday, 9 January 2018 17:58:46 EET Jernej Škrabec wrote:
> Dne torek, 09. januar 2018 ob 11:43:08 CET je Archit Taneja napisal(a):
> > On 12/31/2017 02:31 AM, Jernej Skrabec wrote:
> >> Parts of PHY code could be useful also for custom PHYs. For example,
> >> Allwinner A83T has custom PHY which is probably Synopsys gen2 PHY
> >> with few additional memory mapped registers, so most of the Synopsys PHY
> >> related code could be reused.
> >> 
> >> It turns out that even completely custom HDMI PHYs, such as the one
> >> found in Allwinner H3, can reuse some of those functions. This would
> >> suggest that (some?) functions exported in this commit are actually part
> >> of generic PHY interface and not really specific to Synopsys PHYs.
> >> 
> >> Export useful PHY functions.
> >> 
> >> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> >> ---
> >> 
> >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45 +++++++++++++++++-------
> >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.h |  2 ++
> >> include/drm/bridge/dw_hdmi.h              | 10 +++++++
> >> 3 files changed, 44 insertions(+), 13 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> >> 7ca14d7325b5..67467d0b683a 100644
> >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c

[snip]

> >> @@ -1065,6 +1067,23 @@ static void
> >> dw_hdmi_phy_sel_interface_control(struct dw_hdmi *hdmi, u8 enable)
> >>   			 HDMI_PHY_CONF0_SELDIPIF_MASK);
> >>   }
> >> 
> >> +void dw_hdmi_phy_gen2_reset(struct dw_hdmi *hdmi, u8 enable)
> >> +{
> >> +	hdmi_mask_writeb(hdmi, enable, HDMI_MC_PHYRSTZ,
> >> +			 HDMI_MC_PHYRSTZ_PHYRSTZ_OFFSET,
> >> +			 HDMI_MC_PHYRSTZ_PHYRSTZ_MASK);
> >> +}
> >> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_reset);

I don't remember the details, is the reset signal Gen2-specific ?

How about asserting and deasserting the reset signal in the same call instead 
of having to call this function twice ?

> >> +void dw_hdmi_phy_set_slave_addr(struct dw_hdmi *hdmi)
> >> +{
> >> +	hdmi_phy_test_clear(hdmi, 1);
> >> +	hdmi_writeb(hdmi, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2,
> >> +		    HDMI_PHY_I2CM_SLAVE_ADDR);
> >> +	hdmi_phy_test_clear(hdmi, 0);
> >> +}
> >> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_set_slave_addr);
> > 
> > Should this be called dw_hdmi_phy_gen2_set_slave_addr?
> 
> Probably. I will rename it in v2 to be consistent with other phy functions.

The I2C write function is called dw_hdmi_phy_i2c_write(). If we want to be 
conosistent we should either rename this one to dw_hdmi_phy_i2c_set_addr() or 
rename them both to dw_hdmi_phy_gen2_i2c_write() and 
dw_hdmi_phy_gen2_i2c_set_addr(). I think I'd prefer the former, and we could 
even drop gen2 from dw_hdmi_phy_gen2_pddq() and dw_hdmi_phy_gen2_txpwron() if 
desired.

> > Looks good otherwise. Same for patches 3 and 4 in this series.
> > 
> >> +
> >>   static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
> >>   {
> >>   	const struct dw_hdmi_phy_data *phy = hdmi->phy.data;

[snip]

-- 
Regards,

Laurent Pinchart

Re: [PATCH 04/11] drm/bridge/synopsys: dw-hdmi: Export some PHY related functions

[Jernej Škrabec]

Hi,

Dne torek, 09. januar 2018 ob 17:08:55 CET je Laurent Pinchart napisal(a):
> Hello,
> 
> On Tuesday, 9 January 2018 17:58:46 EET Jernej Škrabec wrote:
> > Dne torek, 09. januar 2018 ob 11:43:08 CET je Archit Taneja napisal(a):
> > > On 12/31/2017 02:31 AM, Jernej Skrabec wrote:
> > >> Parts of PHY code could be useful also for custom PHYs. For example,
> > >> Allwinner A83T has custom PHY which is probably Synopsys gen2 PHY
> > >> with few additional memory mapped registers, so most of the Synopsys
> > >> PHY
> > >> related code could be reused.
> > >> 
> > >> It turns out that even completely custom HDMI PHYs, such as the one
> > >> found in Allwinner H3, can reuse some of those functions. This would
> > >> suggest that (some?) functions exported in this commit are actually
> > >> part
> > >> of generic PHY interface and not really specific to Synopsys PHYs.
> > >> 
> > >> Export useful PHY functions.
> > >> 
> > >> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > >> ---
> > >> 
> > >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45 +++++++++++++++++-------
> > >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.h |  2 ++
> > >> include/drm/bridge/dw_hdmi.h              | 10 +++++++
> > >> 3 files changed, 44 insertions(+), 13 deletions(-)
> > >> 
> > >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> > >> 7ca14d7325b5..67467d0b683a 100644
> > >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> 
> [snip]
> 
> > >> @@ -1065,6 +1067,23 @@ static void
> > >> dw_hdmi_phy_sel_interface_control(struct dw_hdmi *hdmi, u8 enable)
> > >> 
> > >>   			 HDMI_PHY_CONF0_SELDIPIF_MASK);
> > >>   
> > >>   }
> > >> 
> > >> +void dw_hdmi_phy_gen2_reset(struct dw_hdmi *hdmi, u8 enable)
> > >> +{
> > >> +	hdmi_mask_writeb(hdmi, enable, HDMI_MC_PHYRSTZ,
> > >> +			 HDMI_MC_PHYRSTZ_PHYRSTZ_OFFSET,
> > >> +			 HDMI_MC_PHYRSTZ_PHYRSTZ_MASK);
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_reset);
> 
> I don't remember the details, is the reset signal Gen2-specific ?

I don't know, since there is not much documentation. Should I remove "gen2" 
from the name?

> 
> How about asserting and deasserting the reset signal in the same call
> instead of having to call this function twice ?

A83T BSP driver clears txpwron and sets pddq between asserting and deasserting 
reset. I'll test if it works with your proposal.

> 
> > >> +void dw_hdmi_phy_set_slave_addr(struct dw_hdmi *hdmi)
> > >> +{
> > >> +	hdmi_phy_test_clear(hdmi, 1);
> > >> +	hdmi_writeb(hdmi, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2,
> > >> +		    HDMI_PHY_I2CM_SLAVE_ADDR);
> > >> +	hdmi_phy_test_clear(hdmi, 0);
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_set_slave_addr);
> > > 
> > > Should this be called dw_hdmi_phy_gen2_set_slave_addr?
> > 
> > Probably. I will rename it in v2 to be consistent with other phy
> > functions.
> 
> The I2C write function is called dw_hdmi_phy_i2c_write(). If we want to be
> conosistent we should either rename this one to dw_hdmi_phy_i2c_set_addr()
> or rename them both to dw_hdmi_phy_gen2_i2c_write() and
> dw_hdmi_phy_gen2_i2c_set_addr(). I think I'd prefer the former, and we could
> even drop gen2 from dw_hdmi_phy_gen2_pddq() and dw_hdmi_phy_gen2_txpwron()
> if desired.

Ok, then I'll name it dw_hdmi_phy_i2c_set_addr(). I'll leave other names as 
they are.

Best regards,
Jernej

> 
> > > Looks good otherwise. Same for patches 3 and 4 in this series.
> > > 
> > >> +
> > >> 
> > >>   static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
> > >>   {
> > >>   
> > >>   	const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
> 
> [snip]
> 
> --
> Regards,
> 
> Laurent Pinchart

Re: [PATCH 04/11] drm/bridge/synopsys: dw-hdmi: Export some PHY related functions

[Jernej Škrabec]

Hi Laurent,

Dne torek, 09. januar 2018 ob 17:08:55 CET je Laurent Pinchart napisal(a):
> Hello,
> 
> On Tuesday, 9 January 2018 17:58:46 EET Jernej Škrabec wrote:
> > Dne torek, 09. januar 2018 ob 11:43:08 CET je Archit Taneja napisal(a):
> > > On 12/31/2017 02:31 AM, Jernej Skrabec wrote:
> > >> Parts of PHY code could be useful also for custom PHYs. For example,
> > >> Allwinner A83T has custom PHY which is probably Synopsys gen2 PHY
> > >> with few additional memory mapped registers, so most of the Synopsys
> > >> PHY
> > >> related code could be reused.
> > >> 
> > >> It turns out that even completely custom HDMI PHYs, such as the one
> > >> found in Allwinner H3, can reuse some of those functions. This would
> > >> suggest that (some?) functions exported in this commit are actually
> > >> part
> > >> of generic PHY interface and not really specific to Synopsys PHYs.
> > >> 
> > >> Export useful PHY functions.
> > >> 
> > >> Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> > >> ---
> > >> 
> > >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45 +++++++++++++++++-------
> > >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.h |  2 ++
> > >> include/drm/bridge/dw_hdmi.h              | 10 +++++++
> > >> 3 files changed, 44 insertions(+), 13 deletions(-)
> > >> 
> > >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> > >> 7ca14d7325b5..67467d0b683a 100644
> > >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> 
> [snip]
> 
> > >> @@ -1065,6 +1067,23 @@ static void
> > >> dw_hdmi_phy_sel_interface_control(struct dw_hdmi *hdmi, u8 enable)
> > >> 
> > >>   			 HDMI_PHY_CONF0_SELDIPIF_MASK);
> > >>   
> > >>   }
> > >> 
> > >> +void dw_hdmi_phy_gen2_reset(struct dw_hdmi *hdmi, u8 enable)
> > >> +{
> > >> +	hdmi_mask_writeb(hdmi, enable, HDMI_MC_PHYRSTZ,
> > >> +			 HDMI_MC_PHYRSTZ_PHYRSTZ_OFFSET,
> > >> +			 HDMI_MC_PHYRSTZ_PHYRSTZ_MASK);
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_gen2_reset);
> 
> I don't remember the details, is the reset signal Gen2-specific ?

According to this comment:

/* PHY reset. The reset signal is active high on Gen2 PHYs. */

I would guess that it is not specific to Gen2, otherwise the comment wouldn't 
be needed. I will remove "gen2" from the name.

> 
> How about asserting and deasserting the reset signal in the same call
> instead of having to call this function twice ?

It works on A83T if reset signal is asserted and deasserted immediately. I 
will change function according to your proposal.

Best regards,
Jernej

> 
> > >> +void dw_hdmi_phy_set_slave_addr(struct dw_hdmi *hdmi)
> > >> +{
> > >> +	hdmi_phy_test_clear(hdmi, 1);
> > >> +	hdmi_writeb(hdmi, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2,
> > >> +		    HDMI_PHY_I2CM_SLAVE_ADDR);
> > >> +	hdmi_phy_test_clear(hdmi, 0);
> > >> +}
> > >> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_set_slave_addr);
> > > 
> > > Should this be called dw_hdmi_phy_gen2_set_slave_addr?
> > 
> > Probably. I will rename it in v2 to be consistent with other phy
> > functions.
> 
> The I2C write function is called dw_hdmi_phy_i2c_write(). If we want to be
> conosistent we should either rename this one to dw_hdmi_phy_i2c_set_addr()
> or rename them both to dw_hdmi_phy_gen2_i2c_write() and
> dw_hdmi_phy_gen2_i2c_set_addr(). I think I'd prefer the former, and we could
> even drop gen2 from dw_hdmi_phy_gen2_pddq() and dw_hdmi_phy_gen2_txpwron()
> if desired.
> 
> > > Looks good otherwise. Same for patches 3 and 4 in this series.
> > > 
> > >> +
> > >> 
> > >>   static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
> > >>   {
> > >>   
> > >>   	const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
> 
> [snip]
> 
> --
> Regards,
> 
> Laurent Pinchart




-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.