[PATCH 00/11] ARM: dts: qcom: msm8974: add support for external display

[PATCH 00/11] ARM: dts: qcom: msm8974: add support for external display

[Brian Masney]

This patch series begins to add support for the external display over
HDMI that is supported on msm8974 SoCs. I'm testing this series on the
Nexus 5, and I'm able to communicate with the HDMI bridge via the
analogix-anx78xx driver, however the external display is not working
yet.

When I plug in the HDMI cable, the monitor detects that a device is
hooked up, but nothing is shown on the external monitor. The hot plug
detect GPIO (hpd-gpios) on the analogix-anx78xx bridge and MSM HDMI
drivers do not change state when the slimport adapter or HDMI cable is
plugged in or removed. I wonder if a regulator is not enabled somewhere?
I have a comment in patch 10 regarding 'hpd-gdsc-supply' that may
potentially be an issue.

I'm still digging in on this, however I'd appreciate any feedback if
anyone has time. Most of these patches are ready now, so I marked the
ones that aren't ready with 'PATCH RFC'.

I'm using an Analogix Semiconductor SP6001 SlimPort Micro-USB to 4K HDMI
Adapter to connect my phone to an external display via a standard HDMI
cable. This works just fine with the downstream MSM kernel using
Android.

Brian Masney (11):
  dt-bindings: drm/bridge: analogix-anx78xx: add new variants
  drm/bridge: analogix-anx78xx: add new variants
  drm/bridge: analogix-anx78xx: silence -EPROBE_DEFER warnings
  drm/bridge: analogix-anx78xx: convert to i2c_new_dummy_device
  drm/bridge: analogix-anx78xx: correct value of TX_P0
  drm/bridge: analogix-anx78xx: add support for avdd33 regulator
  ARM: qcom_defconfig: add CONFIG_DRM_ANALOGIX_ANX78XX
  drm/msm/hdmi: silence -EPROBE_DEFER warning
  ARM: dts: qcom: pm8941: add 5vs2 regulator node
  ARM: dts: qcom: msm8974: add HDMI nodes
  ARM: dts: qcom: msm8974-hammerhead: add support for external display

 .../bindings/display/bridge/anx7814.txt       |   6 +-
 .../qcom-msm8974-lge-nexus5-hammerhead.dts    | 140 ++++++++++++++++++
 arch/arm/boot/dts/qcom-msm8974.dtsi           |  80 ++++++++++
 arch/arm/boot/dts/qcom-pm8941.dtsi            |  10 ++
 arch/arm/configs/qcom_defconfig               |   1 +
 drivers/gpu/drm/bridge/analogix-anx78xx.c     |  60 +++++++-
 drivers/gpu/drm/bridge/analogix-anx78xx.h     |   2 +-
 drivers/gpu/drm/msm/hdmi/hdmi_phy.c           |   8 +-
 8 files changed, 295 insertions(+), 12 deletions(-)

-- 
2.21.0

[PATCH 01/11] dt-bindings: drm/bridge: analogix-anx78xx: add new variants

[Brian Masney]

Add support for the analogix,anx7808, analogix,anx7812, and
analogix,anx7818 variants.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 .../devicetree/bindings/display/bridge/anx7814.txt          | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/anx7814.txt b/Documentation/devicetree/bindings/display/bridge/anx7814.txt
index dbd7c84ee584..17258747fff6 100644
--- a/Documentation/devicetree/bindings/display/bridge/anx7814.txt
+++ b/Documentation/devicetree/bindings/display/bridge/anx7814.txt
@@ -6,7 +6,11 @@ designed for portable devices.
 
 Required properties:
 
- - compatible		: "analogix,anx7814"
+ - compatible		: Must be one of:
+			  "analogix,anx7808"
+			  "analogix,anx7812"
+			  "analogix,anx7814"
+			  "analogix,anx7818"
  - reg			: I2C address of the device
  - interrupts		: Should contain the INTP interrupt
  - hpd-gpios		: Which GPIO to use for hpd
-- 
2.21.0

[PATCH 02/11] drm/bridge: analogix-anx78xx: add new variants

[Brian Masney]

Add support for the 7808 variant. While we're here, the of match table
was missing support for the 7812 and 7818 variants, so add them as well.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/gpu/drm/bridge/analogix-anx78xx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index 3c7cc5af735c..9acdbedf1245 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -1301,6 +1301,7 @@ static const struct regmap_config anx78xx_regmap_config = {
 };
 
 static const u16 anx78xx_chipid_list[] = {
+	0x7808,
 	0x7812,
 	0x7814,
 	0x7818,
@@ -1463,7 +1464,10 @@ MODULE_DEVICE_TABLE(i2c, anx78xx_id);
 
 #if IS_ENABLED(CONFIG_OF)
 static const struct of_device_id anx78xx_match_table[] = {
+	{ .compatible = "analogix,anx7808", },
+	{ .compatible = "analogix,anx7812", },
 	{ .compatible = "analogix,anx7814", },
+	{ .compatible = "analogix,anx7818", },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, anx78xx_match_table);
-- 
2.21.0

[PATCH 03/11] drm/bridge: analogix-anx78xx: silence -EPROBE_DEFER warnings

[Brian Masney]

Silence two warning messages that occur due to -EPROBE_DEFER errors to
help cleanup the system boot log.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/gpu/drm/bridge/analogix-anx78xx.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index 9acdbedf1245..62dfced91384 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -715,7 +715,9 @@ static int anx78xx_init_pdata(struct anx78xx *anx78xx)
 	/* 1.0V digital core power regulator  */
 	pdata->dvdd10 = devm_regulator_get(dev, "dvdd10");
 	if (IS_ERR(pdata->dvdd10)) {
-		DRM_ERROR("DVDD10 regulator not found\n");
+		if (PTR_ERR(pdata->dvdd10) != -EPROBE_DEFER)
+			DRM_ERROR("DVDD10 regulator not found\n");
+
 		return PTR_ERR(pdata->dvdd10);
 	}
 
@@ -1333,7 +1335,9 @@ static int anx78xx_i2c_probe(struct i2c_client *client,
 
 	err = anx78xx_init_pdata(anx78xx);
 	if (err) {
-		DRM_ERROR("Failed to initialize pdata: %d\n", err);
+		if (err != -EPROBE_DEFER)
+			DRM_ERROR("Failed to initialize pdata: %d\n", err);
+
 		return err;
 	}
 
-- 
2.21.0

[PATCH 04/11] drm/bridge: analogix-anx78xx: convert to i2c_new_dummy_device

[Brian Masney]

The i2c_new_dummy() function is deprecated since it returns NULL on
error. Change this to use the recommended replacement
i2c_new_dummy_device() that returns an error code that can be read with
PTR_ERR() and friends.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/gpu/drm/bridge/analogix-anx78xx.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index 62dfced91384..8daee6b1fa88 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -1355,15 +1355,18 @@ static int anx78xx_i2c_probe(struct i2c_client *client,
 
 	/* Map slave addresses of ANX7814 */
 	for (i = 0; i < I2C_NUM_ADDRESSES; i++) {
-		anx78xx->i2c_dummy[i] = i2c_new_dummy(client->adapter,
-						anx78xx_i2c_addresses[i] >> 1);
-		if (!anx78xx->i2c_dummy[i]) {
-			err = -ENOMEM;
-			DRM_ERROR("Failed to reserve I2C bus %02x\n",
-				  anx78xx_i2c_addresses[i]);
+		struct i2c_client *i2c_dummy;
+
+		i2c_dummy = i2c_new_dummy_device(client->adapter,
+						 anx78xx_i2c_addresses[i] >> 1);
+		if (IS_ERR(i2c_dummy)) {
+			err = PTR_ERR(i2c_dummy);
+			DRM_ERROR("Failed to reserve I2C bus %02x: %d\n",
+				  anx78xx_i2c_addresses[i], err);
 			goto err_unregister_i2c;
 		}
 
+		anx78xx->i2c_dummy[i] = i2c_dummy;
 		anx78xx->map[i] = devm_regmap_init_i2c(anx78xx->i2c_dummy[i],
 						       &anx78xx_regmap_config);
 		if (IS_ERR(anx78xx->map[i])) {
-- 
2.21.0

[PATCH 05/11] drm/bridge: analogix-anx78xx: correct value of TX_P0

[Brian Masney]

When attempting to configure this driver on a Nexus 5 phone (msm8974),
setting up the dummy i2c bus for TX_P0 would fail due to an -EBUSY
error. The downstream MSM kernel sources [1] shows that the proper value
for TX_P0 is 0x78, not 0x70, so correct the value to allow device
probing to succeed.

[1] https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/drivers/video/slimport/slimport_tx_reg.h

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/gpu/drm/bridge/analogix-anx78xx.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.h b/drivers/gpu/drm/bridge/analogix-anx78xx.h
index 25e063bcecbc..bc511fc605c9 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.h
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.h
@@ -6,7 +6,7 @@
 #ifndef __ANX78xx_H
 #define __ANX78xx_H
 
-#define TX_P0				0x70
+#define TX_P0				0x78
 #define TX_P1				0x7a
 #define TX_P2				0x72
 
-- 
2.21.0

[PATCH RFC 06/11] drm/bridge: analogix-anx78xx: add support for avdd33 regulator

[Brian Masney]

Add support for the avdd33 regulator to the analogix-anx78xx driver.
Note that the regulator is currently enabled during driver probe and
disabled when the driver is removed. This is currently how the
downstream MSM kernel sources do this.

Let's not merge this upstream for the mean time until I get the external
display fully working on the Nexus 5 and then I can submit proper
support then that powers down this regulator in the power off function.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/gpu/drm/bridge/analogix-anx78xx.c | 33 +++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index 8daee6b1fa88..48adf010816c 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -48,6 +48,7 @@ static const u8 anx78xx_i2c_addresses[] = {
 
 struct anx78xx_platform_data {
 	struct regulator *dvdd10;
+	struct regulator *avdd33;
 	struct gpio_desc *gpiod_hpd;
 	struct gpio_desc *gpiod_pd;
 	struct gpio_desc *gpiod_reset;
@@ -707,10 +708,42 @@ static int anx78xx_start(struct anx78xx *anx78xx)
 	return err;
 }
 
+static void anx78xx_disable_regulator_action(void *_data)
+{
+	struct anx78xx_platform_data *pdata = _data;
+
+	regulator_disable(pdata->avdd33);
+}
+
 static int anx78xx_init_pdata(struct anx78xx *anx78xx)
 {
 	struct anx78xx_platform_data *pdata = &anx78xx->pdata;
 	struct device *dev = &anx78xx->client->dev;
+	int err;
+
+	/* 3.3V digital core power regulator  */
+	pdata->avdd33 = devm_regulator_get(dev, "avdd33");
+	if (IS_ERR(pdata->avdd33)) {
+		err = PTR_ERR(pdata->avdd33);
+		if (err != -EPROBE_DEFER)
+			DRM_ERROR("avdd33 regulator not found\n");
+
+		return err;
+	}
+
+	err = regulator_enable(pdata->avdd33);
+	if (err) {
+		DRM_ERROR("Failed to enable avdd33 regulator: %d\n", err);
+		return err;
+	}
+
+	err = devm_add_action(dev, anx78xx_disable_regulator_action,
+			      pdata);
+	if (err < 0) {
+		dev_err(dev, "Failed to setup regulator cleanup action %d\n",
+			err);
+		return err;
+	}
 
 	/* 1.0V digital core power regulator  */
 	pdata->dvdd10 = devm_regulator_get(dev, "dvdd10");
-- 
2.21.0

[PATCH 07/11] ARM: qcom_defconfig: add CONFIG_DRM_ANALOGIX_ANX78XX

[Brian Masney]

Add CONFIG_DRM_ANALOGIX_ANX78XX as a module so that the external display
can be used on the Nexus 5 phones.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 arch/arm/configs/qcom_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig
index 34433bf5885d..139e6610f034 100644
--- a/arch/arm/configs/qcom_defconfig
+++ b/arch/arm/configs/qcom_defconfig
@@ -148,6 +148,7 @@ CONFIG_REGULATOR_QCOM_SPMI=y
 CONFIG_MEDIA_SUPPORT=y
 CONFIG_DRM=y
 CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_DRM_ANALOGIX_ANX78XX=m
 CONFIG_FB=y
 CONFIG_FRAMEBUFFER_CONSOLE=y
 # CONFIG_LCD_CLASS_DEVICE is not set
-- 
2.21.0

[PATCH 08/11] drm/msm/hdmi: silence -EPROBE_DEFER warning

[Brian Masney]

Silence a warning message due to an -EPROBE_DEFER error to help cleanup
the system boot log.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/gpu/drm/msm/hdmi/hdmi_phy.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_phy.c b/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
index 1697e61f9c2f..8a38d4b95102 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
@@ -29,8 +29,12 @@ static int msm_hdmi_phy_resource_init(struct hdmi_phy *phy)
 		reg = devm_regulator_get(dev, cfg->reg_names[i]);
 		if (IS_ERR(reg)) {
 			ret = PTR_ERR(reg);
-			DRM_DEV_ERROR(dev, "failed to get phy regulator: %s (%d)\n",
-				cfg->reg_names[i], ret);
+			if (ret != -EPROBE_DEFER) {
+				DRM_DEV_ERROR(dev,
+					      "failed to get phy regulator: %s (%d)\n",
+					      cfg->reg_names[i], ret);
+			}
+
 			return ret;
 		}
 
-- 
2.21.0

[PATCH 09/11] ARM: dts: qcom: pm8941: add 5vs2 regulator node

[Brian Masney]

pm8941 is missing the 5vs2 regulator node so let's add it since its
needed to get the external display working. This regulator was already
configured in the interrupts property on the parent node.

Note that this regulator is referred to as mvs2 in the downstream MSM
kernel sources.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 arch/arm/boot/dts/qcom-pm8941.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi b/arch/arm/boot/dts/qcom-pm8941.dtsi
index f198480c8ef4..c1f2012d1c8b 100644
--- a/arch/arm/boot/dts/qcom-pm8941.dtsi
+++ b/arch/arm/boot/dts/qcom-pm8941.dtsi
@@ -178,6 +178,16 @@
 				qcom,vs-soft-start-strength = <0>;
 				regulator-initial-mode = <1>;
 			};
+
+			pm8941_5vs2: 5vs2 {
+				regulator-enable-ramp-delay = <1000>;
+				regulator-pull-down;
+				regulator-over-current-protection;
+				qcom,ocp-max-retries = <10>;
+				qcom,ocp-retry-delay = <30>;
+				qcom,vs-soft-start-strength = <0>;
+				regulator-initial-mode = <1>;
+			};
 		};
 	};
 };
-- 
2.21.0

[PATCH RFC 10/11] ARM: dts: qcom: msm8974: add HDMI nodes

[Brian Masney]

Add HDMI tx and phy nodes to support an external display that can be
connected over the SlimPort. This is based on work from Jonathan Marek.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
The hdmi-tx node in the downstream MSM sources:
https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-mdss.dtsi#L101

 arch/arm/boot/dts/qcom-msm8974.dtsi | 80 +++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 369e58f64145..35c51336a9d4 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -1139,6 +1139,13 @@
 
 					port@0 {
 						reg = <0>;
+						mdp5_intf3_out: endpoint {
+							remote-endpoint = <&hdmi_in>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
 						mdp5_intf1_out: endpoint {
 							remote-endpoint = <&dsi0_in>;
 						};
@@ -1216,6 +1223,79 @@
 				clocks = <&mmcc MDSS_AHB_CLK>;
 				clock-names = "iface";
 			};
+
+			hdmi: hdmi-tx@fd922100 {
+				status = "disabled";
+
+				compatible = "qcom,hdmi-tx-8974";
+				reg = <0xfd922100 0x35c>,
+				      <0xfc4b8000 0x60f0>;
+				reg-names = "core_physical",
+				            "qfprom_physical";
+
+				interrupt-parent = <&mdss>;
+				interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
+
+				power-domains = <&mmcc MDSS_GDSC>;
+
+				clocks = <&mmcc MDSS_MDP_CLK>,
+				         <&mmcc MDSS_AHB_CLK>,
+				         <&mmcc MDSS_HDMI_CLK>,
+				         <&mmcc MDSS_HDMI_AHB_CLK>,
+				         <&mmcc MDSS_EXTPCLK_CLK>;
+				clock-names = "mdp_core",
+				              "iface",
+				              "core",
+				              "alt_iface",
+				              "extp";
+
+				hpd-5v-supply = <&pm8941_5vs2>;
+				core-vdda-supply = <&pm8941_l12>;
+				core-vcc-supply = <&pm8941_s3>;
+
+				/*
+				 * FIXME - drivers/gpu/drm/msm/hdmi/hdmi.c via hpd_reg_names_8x74
+				 * looks for hpd-gdsc-supply. What should be used here? Shouldn't
+				 * this functionality be provided by the power-domains above?
+				 */
+
+				phys = <&hdmi_phy>;
+				phy-names = "hdmi_phy";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						hdmi_in: endpoint {
+							remote-endpoint = <&mdp5_intf3_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+					};
+				};
+			};
+
+			hdmi_phy: hdmi-phy@fd922500 {
+				status = "disabled";
+
+				compatible = "qcom,hdmi-phy-8974";
+				reg = <0xfd922500 0x7c>;
+				reg-names = "hdmi_phy";
+
+				clocks = <&mmcc MDSS_AHB_CLK>,
+				         <&mmcc MDSS_HDMI_AHB_CLK>;
+				clock-names = "iface",
+				              "alt_iface";
+
+				core-vdda-supply = <&pm8941_l12>;
+				vddio-supply = <&pm8941_s3>;
+
+				#phy-cells = <0>;
+			};
 		};
 	};
 
-- 
2.21.0

[PATCH RFC 11/11] ARM: dts: qcom: msm8974-hammerhead: add support for external display

[Brian Masney]

Add HDMI nodes and other supporting infrastructure in order to support
the external display. This is based on work from Jonathan Marek.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
The hdmi-tx node in the downstream MSM sources:
https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-mdss.dtsi#L101

 .../qcom-msm8974-lge-nexus5-hammerhead.dts    | 140 ++++++++++++++++++
 1 file changed, 140 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
index 3487daf98e81..83416b6d6634 100644
--- a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
+++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
@@ -234,6 +234,34 @@
 		pinctrl-names = "default";
 		pinctrl-0 = <&wlan_regulator_pin>;
 	};
+
+	anx_avdd33: avdd33 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "avdd-3p3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&pm8941_gpios 26 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&anx_avdd33_pin>;
+	};
+
+	anx_vdd10: vdd10 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "vdd-1p0";
+		regulator-min-microvolt = <1000000>;
+		regulator-max-microvolt = <1000000>;
+
+		gpio = <&pm8941_gpios 8 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&anx_vdd10_pin>;
+	};
 };
 
 &soc {
@@ -355,6 +383,40 @@
 				bias-disable;
 			};
 		};
+
+		hdmi_pin: hdmi {
+			cec {
+				pins = "gpio31";
+				function = "hdmi_cec";
+			};
+
+			ddc {
+				pins = "gpio32", "gpio33";
+				function = "hdmi_ddc";
+			};
+
+			hpd {
+				pins = "gpio34";
+				function = "hdmi_hpd";
+			};
+		};
+
+		anx_msm_pin: anx {
+			irq {
+				pins = "gpio28";
+				function = "gpio";
+				drive-strength = <8>;
+				bias-pull-up;
+				input-enable;
+			};
+
+			reset {
+				pins = "gpio68";
+				function = "gpio";
+				drive-strength = <8>;
+				bias-pull-up;
+			};
+		};
 	};
 
 	sdhci@f9824900 {
@@ -440,6 +502,28 @@
 				default-brightness = <200>;
 			};
 		};
+
+		anx7808@72 {
+			compatible = "analogix,anx7808";
+			reg = <0x72>;
+			interrupts-extended = <&msmgpio 28 IRQ_TYPE_EDGE_RISING>;
+
+			hpd-gpios = <&pm8941_gpios 13 GPIO_ACTIVE_HIGH>;
+			pd-gpios = <&pm8941_gpios 14 GPIO_ACTIVE_HIGH>;
+			reset-gpios = <&msmgpio 68 GPIO_ACTIVE_LOW>;
+
+			pinctrl-names = "default";
+			pinctrl-0 = <&anx_msm_pin>, <&anx_pin>;
+
+			dvdd10-supply = <&anx_vdd10>;
+			avdd33-supply = <&anx_avdd33>;
+
+			port {
+				anx7808_in: endpoint {
+					remote-endpoint = <&hdmi_out>;
+				};
+			};
+		};
 	};
 
 	i2c@f9968000 {
@@ -621,6 +705,29 @@
 
 			vddio-supply = <&pm8941_l12>;
 		};
+
+		hdmi-tx@fd922100 {
+			status = "ok";
+
+			pinctrl-names = "default";
+			pinctrl-0 = <&hdmi_pin>;
+
+			qcom,hdmi-tx-ddc-clk = <&msmgpio 32 GPIO_ACTIVE_HIGH>;
+			qcom,hdmi-tx-ddc-data = <&msmgpio 33 GPIO_ACTIVE_HIGH>;
+			qcom,hdmi-tx-hpd = <&msmgpio 34 GPIO_ACTIVE_HIGH>;
+
+			ports {
+				port@1 {
+					hdmi_out: endpoint {
+						remote-endpoint = <&anx7808_in>;
+					};
+				};
+			};
+		};
+
+		hdmi-phy@fd922500 {
+			status = "ok";
+		};
 	};
 };
 
@@ -657,6 +764,39 @@
 				output-high;
 				line-name = "otg-gpio";
 			};
+
+			anx_pin: anx {
+				cbldet {
+					pins = "gpio13";
+					function = "normal";
+					input-enable;
+					bias-pull-down;
+					power-source = <PM8941_GPIO_S3>;
+				};
+
+				pd {
+					pins = "gpio14";
+					function = "normal";
+					bias-disable;
+					power-source = <PM8941_GPIO_S3>;
+				};
+			};
+
+			anx_avdd33_pin: anxvdd3  {
+				pins = "gpio26";
+				function = "normal";
+
+				bias-disable;
+				power-source = <PM8941_GPIO_S3>;
+			};
+
+			anx_vdd10_pin: anxvdd1 {
+				pins = "gpio8";
+				function = "normal";
+
+				bias-disable;
+				power-source = <PM8941_GPIO_S3>;
+			};
 		};
 	};
 };
-- 
2.21.0

Re: [PATCH 01/11] dt-bindings: drm/bridge: analogix-anx78xx: add new variants

[Linus Walleij]

On Thu, Aug 15, 2019 at 2:49 AM Brian Masney <masneyb@onstation.org> wrote:

> Add support for the analogix,anx7808, analogix,anx7812, and
> analogix,anx7818 variants.
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 02/11] drm/bridge: analogix-anx78xx: add new variants

[Linus Walleij]

On Thu, Aug 15, 2019 at 2:49 AM Brian Masney <masneyb@onstation.org> wrote:

> Add support for the 7808 variant. While we're here, the of match table
> was missing support for the 7812 and 7818 variants, so add them as well.
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 04/11] drm/bridge: analogix-anx78xx: convert to i2c_new_dummy_device

[Linus Walleij]

On Thu, Aug 15, 2019 at 2:49 AM Brian Masney <masneyb@onstation.org> wrote:

> The i2c_new_dummy() function is deprecated since it returns NULL on
> error. Change this to use the recommended replacement
> i2c_new_dummy_device() that returns an error code that can be read with
> PTR_ERR() and friends.
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH RFC 06/11] drm/bridge: analogix-anx78xx: add support for avdd33 regulator

[Linus Walleij]

On Thu, Aug 15, 2019 at 2:49 AM Brian Masney <masneyb@onstation.org> wrote:

> Add support for the avdd33 regulator to the analogix-anx78xx driver.
> Note that the regulator is currently enabled during driver probe and
> disabled when the driver is removed. This is currently how the
> downstream MSM kernel sources do this.
>
> Let's not merge this upstream for the mean time until I get the external
> display fully working on the Nexus 5 and then I can submit proper
> support then that powers down this regulator in the power off function.
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>

> +static void anx78xx_disable_regulator_action(void *_data)
> +{
> +       struct anx78xx_platform_data *pdata = _data;
> +
> +       regulator_disable(pdata->avdd33);
> +}
(...)
> +       err = devm_add_action(dev, anx78xx_disable_regulator_action,
> +                             pdata);

Clever idea. Good for initial support, probably later on it would
need to be reworked using runtime PM so it's not constantly
powered up.

See for example how I try to push down power dissipation
of sensors in 3d838118c6aa.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 07/11] ARM: qcom_defconfig: add CONFIG_DRM_ANALOGIX_ANX78XX

[Linus Walleij]

On Thu, Aug 15, 2019 at 2:49 AM Brian Masney <masneyb@onstation.org> wrote:

> Add CONFIG_DRM_ANALOGIX_ANX78XX as a module so that the external display
> can be used on the Nexus 5 phones.
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 05/11] drm/bridge: analogix-anx78xx: correct value of TX_P0

[Linus Walleij]

On Thu, Aug 15, 2019 at 2:49 AM Brian Masney <masneyb@onstation.org> wrote:

> When attempting to configure this driver on a Nexus 5 phone (msm8974),
> setting up the dummy i2c bus for TX_P0 would fail due to an -EBUSY
> error. The downstream MSM kernel sources [1] shows that the proper value
> for TX_P0 is 0x78, not 0x70, so correct the value to allow device
> probing to succeed.
>
> [1] https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/drivers/video/slimport/slimport_tx_reg.h
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH RFC 10/11] ARM: dts: qcom: msm8974: add HDMI nodes

[Linus Walleij]

On Thu, Aug 15, 2019 at 2:49 AM Brian Masney <masneyb@onstation.org> wrote:

> Add HDMI tx and phy nodes to support an external display that can be
> connected over the SlimPort. This is based on work from Jonathan Marek.
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH RFC 11/11] ARM: dts: qcom: msm8974-hammerhead: add support for external display

[Linus Walleij]

On Thu, Aug 15, 2019 at 2:49 AM Brian Masney <masneyb@onstation.org> wrote:

> Add HDMI nodes and other supporting infrastructure in order to support
> the external display. This is based on work from Jonathan Marek.
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 09/11] ARM: dts: qcom: pm8941: add 5vs2 regulator node

[Linus Walleij]

On Thu, Aug 15, 2019 at 2:49 AM Brian Masney <masneyb@onstation.org> wrote:

> pm8941 is missing the 5vs2 regulator node so let's add it since its
> needed to get the external display working. This regulator was already
> configured in the interrupts property on the parent node.
>
> Note that this regulator is referred to as mvs2 in the downstream MSM
> kernel sources.

When I looked at it it seemed like this convention is used for power
supplies that appear on both the main PMIC and the "extra (boot? basic?
low power?) PMIC that the main 80xx PMIC has mvs1 and the
other 89xx PMIC has mvs2.

I suppose it is named "mvs" on both PMICs and this is just a rail
name so as not to confuse the schematic?

> Signed-off-by: Brian Masney <masneyb@onstation.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 03/11] drm/bridge: analogix-anx78xx: silence -EPROBE_DEFER warnings

[Linus Walleij]

On Thu, Aug 15, 2019 at 2:49 AM Brian Masney <masneyb@onstation.org> wrote:

> Silence two warning messages that occur due to -EPROBE_DEFER errors to
> help cleanup the system boot log.
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 08/11] drm/msm/hdmi: silence -EPROBE_DEFER warning

[Linus Walleij]

On Thu, Aug 15, 2019 at 2:49 AM Brian Masney <masneyb@onstation.org> wrote:

> Silence a warning message due to an -EPROBE_DEFER error to help cleanup
> the system boot log.
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 09/11] ARM: dts: qcom: pm8941: add 5vs2 regulator node

[Brian Masney]

On Thu, Aug 15, 2019 at 10:34:17AM +0200, Linus Walleij wrote:
> On Thu, Aug 15, 2019 at 2:49 AM Brian Masney <masneyb@onstation.org> wrote:
> 
> > pm8941 is missing the 5vs2 regulator node so let's add it since its
> > needed to get the external display working. This regulator was already
> > configured in the interrupts property on the parent node.
> >
> > Note that this regulator is referred to as mvs2 in the downstream MSM
> > kernel sources.
> 
> When I looked at it it seemed like this convention is used for power
> supplies that appear on both the main PMIC and the "extra (boot? basic?
> low power?) PMIC that the main 80xx PMIC has mvs1 and the
> other 89xx PMIC has mvs2.

According to the downstream MSM sources, the 5vs1 and 5vs2 rails are
both on the second pm8941 PMIC:

https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-regulator.dtsi#L18

> I suppose it is named "mvs" on both PMICs and this is just a rail
> name so as not to confuse the schematic?

That sounds reasonable.

> > Signed-off-by: Brian Masney <masneyb@onstation.org>
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Thank you!

Brian

Re: [PATCH RFC 06/11] drm/bridge: analogix-anx78xx: add support for avdd33 regulator

[Brian Masney]

On Thu, Aug 15, 2019 at 10:22:45AM +0200, Linus Walleij wrote:
> On Thu, Aug 15, 2019 at 2:49 AM Brian Masney <masneyb@onstation.org> wrote:
> 
> > Add support for the avdd33 regulator to the analogix-anx78xx driver.
> > Note that the regulator is currently enabled during driver probe and
> > disabled when the driver is removed. This is currently how the
> > downstream MSM kernel sources do this.
> >
> > Let's not merge this upstream for the mean time until I get the external
> > display fully working on the Nexus 5 and then I can submit proper
> > support then that powers down this regulator in the power off function.
> >
> > Signed-off-by: Brian Masney <masneyb@onstation.org>
> 
> > +static void anx78xx_disable_regulator_action(void *_data)
> > +{
> > +       struct anx78xx_platform_data *pdata = _data;
> > +
> > +       regulator_disable(pdata->avdd33);
> > +}
> (...)
> > +       err = devm_add_action(dev, anx78xx_disable_regulator_action,
> > +                             pdata);
> 
> Clever idea. Good for initial support, probably later on it would
> need to be reworked using runtime PM so it's not constantly
> powered up.

Yes, that's my plan. I suspect that I may have a regulator disabled
somewhere so I was planning to leave this on all the time for the time
being to match the downstream behavior until I get the hot plug detect
GPIO working.

> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Thanks,

Brian

Re: [PATCH 01/11] dt-bindings: drm/bridge: analogix-anx78xx: add new variants

[Rob Herring]

On Wed, 14 Aug 2019 20:48:44 -0400, Brian Masney wrote:
> Add support for the analogix,anx7808, analogix,anx7812, and
> analogix,anx7818 variants.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
>  .../devicetree/bindings/display/bridge/anx7814.txt          | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 00/11] ARM: dts: qcom: msm8974: add support for external display

[Andrzej Hajda]

Hi Brian,

On 15.08.2019 02:48, Brian Masney wrote:
> This patch series begins to add support for the external display over
> HDMI that is supported on msm8974 SoCs. I'm testing this series on the
> Nexus 5, and I'm able to communicate with the HDMI bridge via the
> analogix-anx78xx driver, however the external display is not working
> yet.
>
> When I plug in the HDMI cable, the monitor detects that a device is
> hooked up, but nothing is shown on the external monitor. The hot plug
> detect GPIO (hpd-gpios) on the analogix-anx78xx bridge and MSM HDMI
> drivers do not change state when the slimport adapter or HDMI cable is
> plugged in or removed. I wonder if a regulator is not enabled somewhere?
> I have a comment in patch 10 regarding 'hpd-gdsc-supply' that may
> potentially be an issue.
>
> I'm still digging in on this, however I'd appreciate any feedback if
> anyone has time. Most of these patches are ready now, so I marked the
> ones that aren't ready with 'PATCH RFC'.
>
> I'm using an Analogix Semiconductor SP6001 SlimPort Micro-USB to 4K HDMI
> Adapter to connect my phone to an external display via a standard HDMI
> cable. This works just fine with the downstream MSM kernel using
> Android.


This patchset risks to be forgotten. To avoid it, at least partially, I
can merge patches 1-5, is it OK for you?


Regards

Andrzej


>
> Brian Masney (11):
>   dt-bindings: drm/bridge: analogix-anx78xx: add new variants
>   drm/bridge: analogix-anx78xx: add new variants
>   drm/bridge: analogix-anx78xx: silence -EPROBE_DEFER warnings
>   drm/bridge: analogix-anx78xx: convert to i2c_new_dummy_device
>   drm/bridge: analogix-anx78xx: correct value of TX_P0
>   drm/bridge: analogix-anx78xx: add support for avdd33 regulator
>   ARM: qcom_defconfig: add CONFIG_DRM_ANALOGIX_ANX78XX
>   drm/msm/hdmi: silence -EPROBE_DEFER warning
>   ARM: dts: qcom: pm8941: add 5vs2 regulator node
>   ARM: dts: qcom: msm8974: add HDMI nodes
>   ARM: dts: qcom: msm8974-hammerhead: add support for external display
>
>  .../bindings/display/bridge/anx7814.txt       |   6 +-
>  .../qcom-msm8974-lge-nexus5-hammerhead.dts    | 140 ++++++++++++++++++
>  arch/arm/boot/dts/qcom-msm8974.dtsi           |  80 ++++++++++
>  arch/arm/boot/dts/qcom-pm8941.dtsi            |  10 ++
>  arch/arm/configs/qcom_defconfig               |   1 +
>  drivers/gpu/drm/bridge/analogix-anx78xx.c     |  60 +++++++-
>  drivers/gpu/drm/bridge/analogix-anx78xx.h     |   2 +-
>  drivers/gpu/drm/msm/hdmi/hdmi_phy.c           |   8 +-
>  8 files changed, 295 insertions(+), 12 deletions(-)
>

Re: [PATCH 00/11] ARM: dts: qcom: msm8974: add support for external display

[Brian Masney]

Hi Andrzej,

On Mon, Sep 16, 2019 at 10:13:58AM +0200, Andrzej Hajda wrote:
> Hi Brian,
> 
> On 15.08.2019 02:48, Brian Masney wrote:
> > This patch series begins to add support for the external display over
> > HDMI that is supported on msm8974 SoCs. I'm testing this series on the
> > Nexus 5, and I'm able to communicate with the HDMI bridge via the
> > analogix-anx78xx driver, however the external display is not working
> > yet.
> >
> > When I plug in the HDMI cable, the monitor detects that a device is
> > hooked up, but nothing is shown on the external monitor. The hot plug
> > detect GPIO (hpd-gpios) on the analogix-anx78xx bridge and MSM HDMI
> > drivers do not change state when the slimport adapter or HDMI cable is
> > plugged in or removed. I wonder if a regulator is not enabled somewhere?
> > I have a comment in patch 10 regarding 'hpd-gdsc-supply' that may
> > potentially be an issue.
> >
> > I'm still digging in on this, however I'd appreciate any feedback if
> > anyone has time. Most of these patches are ready now, so I marked the
> > ones that aren't ready with 'PATCH RFC'.
> >
> > I'm using an Analogix Semiconductor SP6001 SlimPort Micro-USB to 4K HDMI
> > Adapter to connect my phone to an external display via a standard HDMI
> > cable. This works just fine with the downstream MSM kernel using
> > Android.
> 
> 
> This patchset risks to be forgotten. To avoid it, at least partially, I
> can merge patches 1-5, is it OK for you?

That would be great if you could do that.

Thanks,

Brian

Re: [PATCH 05/11] drm/bridge: analogix-anx78xx: correct value of TX_P0

[Andrzej Hajda]

On 15.08.2019 02:48, Brian Masney wrote:
> When attempting to configure this driver on a Nexus 5 phone (msm8974),
> setting up the dummy i2c bus for TX_P0 would fail due to an -EBUSY
> error. The downstream MSM kernel sources [1] shows that the proper value
> for TX_P0 is 0x78, not 0x70, so correct the value to allow device
> probing to succeed.
>
> [1] https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/drivers/video/slimport/slimport_tx_reg.h
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
>  drivers/gpu/drm/bridge/analogix-anx78xx.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.h b/drivers/gpu/drm/bridge/analogix-anx78xx.h
> index 25e063bcecbc..bc511fc605c9 100644
> --- a/drivers/gpu/drm/bridge/analogix-anx78xx.h
> +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.h
> @@ -6,7 +6,7 @@
>  #ifndef __ANX78xx_H
>  #define __ANX78xx_H
>  
> -#define TX_P0				0x70
> +#define TX_P0				0x78


This bothers me little. There are no upstream users, grepping android
sources suggests that both values can be used [1][2]  (grep for "#define
TX_P0"), moreover there is code suggesting both values can be valid [3].

Could you verify datasheet which i2c slave addresses are valid for this
chip, if both I guess this patch should be reworked.


[1]:
https://android.googlesource.com/kernel/msm/+/android-msm-flo-3.4-jb-mr2/drivers/misc/slimport_anx7808/slimport_tx_reg.h

[2]:
https://github.com/AndroidGX/SimpleGX-MM-6.0_H815_20d/blob/master/drivers/video/slimport/anx7812/slimport7812_tx_reg.h

[3]:
https://github.com/commaai/android_kernel_leeco_msm8996/blob/master/drivers/video/msm/mdss/dp/slimport_custom_declare.h#L73


Regards

Andrzej


>  #define TX_P1				0x7a
>  #define TX_P2				0x72
>  

Re: [PATCH 05/11] drm/bridge: analogix-anx78xx: correct value of TX_P0

[Brian Masney]

On Mon, Sep 16, 2019 at 12:02:09PM +0200, Andrzej Hajda wrote:
> On 15.08.2019 02:48, Brian Masney wrote:
> > When attempting to configure this driver on a Nexus 5 phone (msm8974),
> > setting up the dummy i2c bus for TX_P0 would fail due to an -EBUSY
> > error. The downstream MSM kernel sources [1] shows that the proper value
> > for TX_P0 is 0x78, not 0x70, so correct the value to allow device
> > probing to succeed.
> >
> > [1] https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/drivers/video/slimport/slimport_tx_reg.h
> >
> > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > ---
> >  drivers/gpu/drm/bridge/analogix-anx78xx.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.h b/drivers/gpu/drm/bridge/analogix-anx78xx.h
> > index 25e063bcecbc..bc511fc605c9 100644
> > --- a/drivers/gpu/drm/bridge/analogix-anx78xx.h
> > +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.h
> > @@ -6,7 +6,7 @@
> >  #ifndef __ANX78xx_H
> >  #define __ANX78xx_H
> >  
> > -#define TX_P0				0x70
> > +#define TX_P0				0x78
> 
> 
> This bothers me little. There are no upstream users, grepping android
> sources suggests that both values can be used [1][2]  (grep for "#define
> TX_P0"), moreover there is code suggesting both values can be valid [3].
> 
> Could you verify datasheet which i2c slave addresses are valid for this
> chip, if both I guess this patch should be reworked.
> 
> 
> [1]:
> https://android.googlesource.com/kernel/msm/+/android-msm-flo-3.4-jb-mr2/drivers/misc/slimport_anx7808/slimport_tx_reg.h
> 
> [2]:
> https://github.com/AndroidGX/SimpleGX-MM-6.0_H815_20d/blob/master/drivers/video/slimport/anx7812/slimport7812_tx_reg.h
> 
> [3]:
> https://github.com/commaai/android_kernel_leeco_msm8996/blob/master/drivers/video/msm/mdss/dp/slimport_custom_declare.h#L73

This address is 0x78 on my Nexus 5. Given [3] above it looks like we
need to support both addresses. What do you think about moving these
addresses into device tree?

The downstream and upstream kernel sources divide these addresses by two
to get the i2c address. Here's the code in upstream:

https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analogix-anx78xx.c#L1353
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analogix-anx78xx.c#L41

I'm not sure why the actual i2c address isn't used in this code.

Brian

Re: [PATCH 05/11] drm/bridge: analogix-anx78xx: correct value of TX_P0

[Enric Balletbo i Serra]

Hi Andrzej and Brian

On 16/9/19 12:02, Andrzej Hajda wrote:
> On 15.08.2019 02:48, Brian Masney wrote:
>> When attempting to configure this driver on a Nexus 5 phone (msm8974),
>> setting up the dummy i2c bus for TX_P0 would fail due to an -EBUSY
>> error. The downstream MSM kernel sources [1] shows that the proper value
>> for TX_P0 is 0x78, not 0x70, so correct the value to allow device
>> probing to succeed.
>>
>> [1] https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/drivers/video/slimport/slimport_tx_reg.h
>>
>> Signed-off-by: Brian Masney <masneyb@onstation.org>
>> ---
>>  drivers/gpu/drm/bridge/analogix-anx78xx.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.h b/drivers/gpu/drm/bridge/analogix-anx78xx.h
>> index 25e063bcecbc..bc511fc605c9 100644
>> --- a/drivers/gpu/drm/bridge/analogix-anx78xx.h
>> +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.h
>> @@ -6,7 +6,7 @@
>>  #ifndef __ANX78xx_H
>>  #define __ANX78xx_H
>>  
>> -#define TX_P0				0x70
>> +#define TX_P0				0x78
> 
> 
> This bothers me little. There are no upstream users, grepping android
> sources suggests that both values can be used [1][2]  (grep for "#define
> TX_P0"), moreover there is code suggesting both values can be valid [3].
> 
> Could you verify datasheet which i2c slave addresses are valid for this
> chip, if both I guess this patch should be reworked.
> 

On my case the valid i2c slave address is 0x70 (from datasheet, very sorry I
can't share it) and the bridge used is an ANX7814, it could be that ANX7808 or
ANX7812 have different slave addresses?

Regards,
 Enric

> 
> [1]:
> https://android.googlesource.com/kernel/msm/+/android-msm-flo-3.4-jb-mr2/drivers/misc/slimport_anx7808/slimport_tx_reg.h
> 
> [2]:
> https://github.com/AndroidGX/SimpleGX-MM-6.0_H815_20d/blob/master/drivers/video/slimport/anx7812/slimport7812_tx_reg.h
> 
> [3]:
> https://github.com/commaai/android_kernel_leeco_msm8996/blob/master/drivers/video/msm/mdss/dp/slimport_custom_declare.h#L73
> 
> 
> Regards
> 
> Andrzej
> 
> 
>>  #define TX_P1				0x7a
>>  #define TX_P2				0x72
>>  
> 
> 

Re: [PATCH 05/11] drm/bridge: analogix-anx78xx: correct value of TX_P0

[Brian Masney]

On Mon, Sep 16, 2019 at 12:36:19PM +0200, Enric Balletbo i Serra wrote:
> Hi Andrzej and Brian
> 
> On 16/9/19 12:02, Andrzej Hajda wrote:
> > On 15.08.2019 02:48, Brian Masney wrote:
> >> When attempting to configure this driver on a Nexus 5 phone (msm8974),
> >> setting up the dummy i2c bus for TX_P0 would fail due to an -EBUSY
> >> error. The downstream MSM kernel sources [1] shows that the proper value
> >> for TX_P0 is 0x78, not 0x70, so correct the value to allow device
> >> probing to succeed.
> >>
> >> [1] https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/drivers/video/slimport/slimport_tx_reg.h
> >>
> >> Signed-off-by: Brian Masney <masneyb@onstation.org>
> >> ---
> >>  drivers/gpu/drm/bridge/analogix-anx78xx.h | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.h b/drivers/gpu/drm/bridge/analogix-anx78xx.h
> >> index 25e063bcecbc..bc511fc605c9 100644
> >> --- a/drivers/gpu/drm/bridge/analogix-anx78xx.h
> >> +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.h
> >> @@ -6,7 +6,7 @@
> >>  #ifndef __ANX78xx_H
> >>  #define __ANX78xx_H
> >>  
> >> -#define TX_P0				0x70
> >> +#define TX_P0				0x78
> > 
> > 
> > This bothers me little. There are no upstream users, grepping android
> > sources suggests that both values can be used [1][2]  (grep for "#define
> > TX_P0"), moreover there is code suggesting both values can be valid [3].
> > 
> > Could you verify datasheet which i2c slave addresses are valid for this
> > chip, if both I guess this patch should be reworked.
> > 
> 
> On my case the valid i2c slave address is 0x70 (from datasheet, very sorry I
> can't share it) and the bridge used is an ANX7814, it could be that ANX7808 or
> ANX7812 have different slave addresses?

I haven't been able to find any of the datasheets for these devices
online. Product briefs are online (such as 
https://www.analogix.com/en/system/files/ANX7808_product_brief.pdf), but
they don't provide this type of information.

Brian

Re: [PATCH 05/11] drm/bridge: analogix-anx78xx: correct value of TX_P0

[Laurent Pinchart]

Hi Brian,

On Mon, Sep 16, 2019 at 06:36:14AM -0400, Brian Masney wrote:
> On Mon, Sep 16, 2019 at 12:02:09PM +0200, Andrzej Hajda wrote:
> > On 15.08.2019 02:48, Brian Masney wrote:
> > > When attempting to configure this driver on a Nexus 5 phone (msm8974),
> > > setting up the dummy i2c bus for TX_P0 would fail due to an -EBUSY
> > > error. The downstream MSM kernel sources [1] shows that the proper value
> > > for TX_P0 is 0x78, not 0x70, so correct the value to allow device
> > > probing to succeed.
> > >
> > > [1] https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/drivers/video/slimport/slimport_tx_reg.h
> > >
> > > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > > ---
> > >  drivers/gpu/drm/bridge/analogix-anx78xx.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.h b/drivers/gpu/drm/bridge/analogix-anx78xx.h
> > > index 25e063bcecbc..bc511fc605c9 100644
> > > --- a/drivers/gpu/drm/bridge/analogix-anx78xx.h
> > > +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.h
> > > @@ -6,7 +6,7 @@
> > >  #ifndef __ANX78xx_H
> > >  #define __ANX78xx_H
> > >  
> > > -#define TX_P0				0x70
> > > +#define TX_P0				0x78
> > 
> > 
> > This bothers me little. There are no upstream users, grepping android
> > sources suggests that both values can be used [1][2]  (grep for "#define
> > TX_P0"), moreover there is code suggesting both values can be valid [3].
> > 
> > Could you verify datasheet which i2c slave addresses are valid for this
> > chip, if both I guess this patch should be reworked.
> > 
> > 
> > [1]:
> > https://android.googlesource.com/kernel/msm/+/android-msm-flo-3.4-jb-mr2/drivers/misc/slimport_anx7808/slimport_tx_reg.h
> > 
> > [2]:
> > https://github.com/AndroidGX/SimpleGX-MM-6.0_H815_20d/blob/master/drivers/video/slimport/anx7812/slimport7812_tx_reg.h
> > 
> > [3]:
> > https://github.com/commaai/android_kernel_leeco_msm8996/blob/master/drivers/video/msm/mdss/dp/slimport_custom_declare.h#L73
> 
> This address is 0x78 on my Nexus 5. Given [3] above it looks like we
> need to support both addresses. What do you think about moving these
> addresses into device tree?

Assuming that the device supports different addresses (I can't validate
that as I don't have access to the datasheet), and different addresses
need to be used on different systems, then the address to be used needs
to be provided by the firmware (DT in this case). Two options are
possible, either specifying the address explicitly in the device's DT
node, or specifying free addresses (in the form of a white list or black
list) and allocating an address from that pool. The latter has been
discussed in a BoF at the Linux Plumbers Conference last week,
https://linuxplumbersconf.org/event/4/contributions/542/.

> The downstream and upstream kernel sources divide these addresses by two
> to get the i2c address. Here's the code in upstream:
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analogix-anx78xx.c#L1353
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analogix-anx78xx.c#L41
> 
> I'm not sure why the actual i2c address isn't used in this code.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 05/11] drm/bridge: analogix-anx78xx: correct value of TX_P0

[Enric Balletbo i Serra]

Hi,

On 16/9/19 12:49, Laurent Pinchart wrote:
> Hi Brian,
> 
> On Mon, Sep 16, 2019 at 06:36:14AM -0400, Brian Masney wrote:
>> On Mon, Sep 16, 2019 at 12:02:09PM +0200, Andrzej Hajda wrote:
>>> On 15.08.2019 02:48, Brian Masney wrote:
>>>> When attempting to configure this driver on a Nexus 5 phone (msm8974),
>>>> setting up the dummy i2c bus for TX_P0 would fail due to an -EBUSY
>>>> error. The downstream MSM kernel sources [1] shows that the proper value
>>>> for TX_P0 is 0x78, not 0x70, so correct the value to allow device
>>>> probing to succeed.
>>>>
>>>> [1] https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/drivers/video/slimport/slimport_tx_reg.h
>>>>
>>>> Signed-off-by: Brian Masney <masneyb@onstation.org>
>>>> ---
>>>>  drivers/gpu/drm/bridge/analogix-anx78xx.h | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.h b/drivers/gpu/drm/bridge/analogix-anx78xx.h
>>>> index 25e063bcecbc..bc511fc605c9 100644
>>>> --- a/drivers/gpu/drm/bridge/analogix-anx78xx.h
>>>> +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.h
>>>> @@ -6,7 +6,7 @@
>>>>  #ifndef __ANX78xx_H
>>>>  #define __ANX78xx_H
>>>>  
>>>> -#define TX_P0				0x70
>>>> +#define TX_P0				0x78
>>>
>>>
>>> This bothers me little. There are no upstream users, grepping android
>>> sources suggests that both values can be used [1][2]  (grep for "#define
>>> TX_P0"), moreover there is code suggesting both values can be valid [3].
>>>
>>> Could you verify datasheet which i2c slave addresses are valid for this
>>> chip, if both I guess this patch should be reworked.
>>>
>>>
>>> [1]:
>>> https://android.googlesource.com/kernel/msm/+/android-msm-flo-3.4-jb-mr2/drivers/misc/slimport_anx7808/slimport_tx_reg.h
>>>
>>> [2]:
>>> https://github.com/AndroidGX/SimpleGX-MM-6.0_H815_20d/blob/master/drivers/video/slimport/anx7812/slimport7812_tx_reg.h
>>>
>>> [3]:
>>> https://github.com/commaai/android_kernel_leeco_msm8996/blob/master/drivers/video/msm/mdss/dp/slimport_custom_declare.h#L73
>>
>> This address is 0x78 on my Nexus 5. Given [3] above it looks like we
>> need to support both addresses. What do you think about moving these
>> addresses into device tree?
> 
> Assuming that the device supports different addresses (I can't validate
> that as I don't have access to the datasheet), and different addresses
> need to be used on different systems, then the address to be used needs
> to be provided by the firmware (DT in this case). Two options are
> possible, either specifying the address explicitly in the device's DT
> node, or specifying free addresses (in the form of a white list or black
> list) and allocating an address from that pool. The latter has been
> discussed in a BoF at the Linux Plumbers Conference last week,
> https://linuxplumbersconf.org/event/4/contributions/542/.
> 
>> The downstream and upstream kernel sources divide these addresses by two
>> to get the i2c address. Here's the code in upstream:
>>
>> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analogix-anx78xx.c#L1353
>> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analogix-anx78xx.c#L41
>>
>> I'm not sure why the actual i2c address isn't used in this code.
> 

The ANX7802/12/14/16 has a slave I2C bus that provides the interface to access
or control the chip from the AP. The I2C slave addresses used to control the
ANX7802/12/14/16 are 70h, 72h, 7Ah, 7Eh and 80h. Every address allows you to
access to different registers of the chip and AFAICS is not configurable.

I don't think these addresses should be configured via DT but for the driver itself.

My wild guess is that the ANX7808 has different addresses, but I don't have the
datasheet of this version.

Best regards,
 Enric

Re: [PATCH 00/11] ARM: dts: qcom: msm8974: add support for external display

[Andrzej Hajda]

On 16.09.2019 11:01, Brian Masney wrote:
> Hi Andrzej,
>
> On Mon, Sep 16, 2019 at 10:13:58AM +0200, Andrzej Hajda wrote:
>> Hi Brian,
>>
>> On 15.08.2019 02:48, Brian Masney wrote:
>>> This patch series begins to add support for the external display over
>>> HDMI that is supported on msm8974 SoCs. I'm testing this series on the
>>> Nexus 5, and I'm able to communicate with the HDMI bridge via the
>>> analogix-anx78xx driver, however the external display is not working
>>> yet.
>>>
>>> When I plug in the HDMI cable, the monitor detects that a device is
>>> hooked up, but nothing is shown on the external monitor. The hot plug
>>> detect GPIO (hpd-gpios) on the analogix-anx78xx bridge and MSM HDMI
>>> drivers do not change state when the slimport adapter or HDMI cable is
>>> plugged in or removed. I wonder if a regulator is not enabled somewhere?
>>> I have a comment in patch 10 regarding 'hpd-gdsc-supply' that may
>>> potentially be an issue.
>>>
>>> I'm still digging in on this, however I'd appreciate any feedback if
>>> anyone has time. Most of these patches are ready now, so I marked the
>>> ones that aren't ready with 'PATCH RFC'.
>>>
>>> I'm using an Analogix Semiconductor SP6001 SlimPort Micro-USB to 4K HDMI
>>> Adapter to connect my phone to an external display via a standard HDMI
>>> cable. This works just fine with the downstream MSM kernel using
>>> Android.
>>
>> This patchset risks to be forgotten. To avoid it, at least partially, I
>> can merge patches 1-5, is it OK for you?
> That would be great if you could do that.


I have queued 1-4 to drm-misc-next. 5th patch requires some discussion.


Regards

Andrzej


>
> Thanks,
>
> Brian
>
>

Re: [PATCH 05/11] drm/bridge: analogix-anx78xx: correct value of TX_P0

[Brian Masney]

On Mon, Sep 16, 2019 at 01:32:58PM +0200, Enric Balletbo i Serra wrote:
> Hi,
> 
> On 16/9/19 12:49, Laurent Pinchart wrote:
> > Hi Brian,
> > 
> > On Mon, Sep 16, 2019 at 06:36:14AM -0400, Brian Masney wrote:
> >> On Mon, Sep 16, 2019 at 12:02:09PM +0200, Andrzej Hajda wrote:
> >>> On 15.08.2019 02:48, Brian Masney wrote:
> >>>> When attempting to configure this driver on a Nexus 5 phone (msm8974),
> >>>> setting up the dummy i2c bus for TX_P0 would fail due to an -EBUSY
> >>>> error. The downstream MSM kernel sources [1] shows that the proper value
> >>>> for TX_P0 is 0x78, not 0x70, so correct the value to allow device
> >>>> probing to succeed.
> >>>>
> >>>> [1] https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/drivers/video/slimport/slimport_tx_reg.h
> >>>>
> >>>> Signed-off-by: Brian Masney <masneyb@onstation.org>
> >>>> ---
> >>>>  drivers/gpu/drm/bridge/analogix-anx78xx.h | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.h b/drivers/gpu/drm/bridge/analogix-anx78xx.h
> >>>> index 25e063bcecbc..bc511fc605c9 100644
> >>>> --- a/drivers/gpu/drm/bridge/analogix-anx78xx.h
> >>>> +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.h
> >>>> @@ -6,7 +6,7 @@
> >>>>  #ifndef __ANX78xx_H
> >>>>  #define __ANX78xx_H
> >>>>  
> >>>> -#define TX_P0				0x70
> >>>> +#define TX_P0				0x78
> >>>
> >>>
> >>> This bothers me little. There are no upstream users, grepping android
> >>> sources suggests that both values can be used [1][2]  (grep for "#define
> >>> TX_P0"), moreover there is code suggesting both values can be valid [3].
> >>>
> >>> Could you verify datasheet which i2c slave addresses are valid for this
> >>> chip, if both I guess this patch should be reworked.
> >>>
> >>>
> >>> [1]:
> >>> https://android.googlesource.com/kernel/msm/+/android-msm-flo-3.4-jb-mr2/drivers/misc/slimport_anx7808/slimport_tx_reg.h
> >>>
> >>> [2]:
> >>> https://github.com/AndroidGX/SimpleGX-MM-6.0_H815_20d/blob/master/drivers/video/slimport/anx7812/slimport7812_tx_reg.h
> >>>
> >>> [3]:
> >>> https://github.com/commaai/android_kernel_leeco_msm8996/blob/master/drivers/video/msm/mdss/dp/slimport_custom_declare.h#L73
> >>
> >> This address is 0x78 on my Nexus 5. Given [3] above it looks like we
> >> need to support both addresses. What do you think about moving these
> >> addresses into device tree?
> > 
> > Assuming that the device supports different addresses (I can't validate
> > that as I don't have access to the datasheet), and different addresses
> > need to be used on different systems, then the address to be used needs
> > to be provided by the firmware (DT in this case). Two options are
> > possible, either specifying the address explicitly in the device's DT
> > node, or specifying free addresses (in the form of a white list or black
> > list) and allocating an address from that pool. The latter has been
> > discussed in a BoF at the Linux Plumbers Conference last week,
> > https://linuxplumbersconf.org/event/4/contributions/542/.
> > 
> >> The downstream and upstream kernel sources divide these addresses by two
> >> to get the i2c address. Here's the code in upstream:
> >>
> >> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analogix-anx78xx.c#L1353
> >> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analogix-anx78xx.c#L41
> >>
> >> I'm not sure why the actual i2c address isn't used in this code.
> > 
> 
> The ANX7802/12/14/16 has a slave I2C bus that provides the interface to access
> or control the chip from the AP. The I2C slave addresses used to control the
> ANX7802/12/14/16 are 70h, 72h, 7Ah, 7Eh and 80h. Every address allows you to
> access to different registers of the chip and AFAICS is not configurable.
> 
> I don't think these addresses should be configured via DT but for the driver itself.
> 
> My wild guess is that the ANX7808 has different addresses, but I don't have the
> datasheet of this version.

I'm able to communicate with the 7808 on my Nexus 5 using the 0x78
address. Given that the addresses appear to be fixed per model, maybe it
makes sense to drop the address #defines and add the addresses to the
data pointer in the driver's of_match_table like so:

static const struct of_device_id anx78xx_match_table[] = {
        { .compatible = "analogix,anx7808", .data = PTR_TO_7808_ADDRS },
        { .compatible = "analogix,anx7812", .data = PTR_TO_781X_ADDRS },
        { .compatible = "analogix,anx7814", .data = PTR_TO_781X_ADDRS },
        { .compatible = "analogix,anx7818", .data = PTR_TO_781X_ADDRS },
        { /* sentinel */ },
};

Brian

Re: [PATCH 05/11] drm/bridge: analogix-anx78xx: correct value of TX_P0

[Andrzej Hajda]

On 16.09.2019 14:02, Brian Masney wrote:
> On Mon, Sep 16, 2019 at 01:32:58PM +0200, Enric Balletbo i Serra wrote:
>> Hi,
>>
>> On 16/9/19 12:49, Laurent Pinchart wrote:
>>> Hi Brian,
>>>
>>> On Mon, Sep 16, 2019 at 06:36:14AM -0400, Brian Masney wrote:
>>>> On Mon, Sep 16, 2019 at 12:02:09PM +0200, Andrzej Hajda wrote:
>>>>> On 15.08.2019 02:48, Brian Masney wrote:
>>>>>> When attempting to configure this driver on a Nexus 5 phone (msm8974),
>>>>>> setting up the dummy i2c bus for TX_P0 would fail due to an -EBUSY
>>>>>> error. The downstream MSM kernel sources [1] shows that the proper value
>>>>>> for TX_P0 is 0x78, not 0x70, so correct the value to allow device
>>>>>> probing to succeed.
>>>>>>
>>>>>> [1] https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/drivers/video/slimport/slimport_tx_reg.h
>>>>>>
>>>>>> Signed-off-by: Brian Masney <masneyb@onstation.org>
>>>>>> ---
>>>>>>  drivers/gpu/drm/bridge/analogix-anx78xx.h | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.h b/drivers/gpu/drm/bridge/analogix-anx78xx.h
>>>>>> index 25e063bcecbc..bc511fc605c9 100644
>>>>>> --- a/drivers/gpu/drm/bridge/analogix-anx78xx.h
>>>>>> +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.h
>>>>>> @@ -6,7 +6,7 @@
>>>>>>  #ifndef __ANX78xx_H
>>>>>>  #define __ANX78xx_H
>>>>>>  
>>>>>> -#define TX_P0				0x70
>>>>>> +#define TX_P0				0x78
>>>>>
>>>>> This bothers me little. There are no upstream users, grepping android
>>>>> sources suggests that both values can be used [1][2]  (grep for "#define
>>>>> TX_P0"), moreover there is code suggesting both values can be valid [3].
>>>>>
>>>>> Could you verify datasheet which i2c slave addresses are valid for this
>>>>> chip, if both I guess this patch should be reworked.
>>>>>
>>>>>
>>>>> [1]:
>>>>> https://android.googlesource.com/kernel/msm/+/android-msm-flo-3.4-jb-mr2/drivers/misc/slimport_anx7808/slimport_tx_reg.h
>>>>>
>>>>> [2]:
>>>>> https://github.com/AndroidGX/SimpleGX-MM-6.0_H815_20d/blob/master/drivers/video/slimport/anx7812/slimport7812_tx_reg.h
>>>>>
>>>>> [3]:
>>>>> https://github.com/commaai/android_kernel_leeco_msm8996/blob/master/drivers/video/msm/mdss/dp/slimport_custom_declare.h#L73
>>>> This address is 0x78 on my Nexus 5. Given [3] above it looks like we
>>>> need to support both addresses. What do you think about moving these
>>>> addresses into device tree?
>>> Assuming that the device supports different addresses (I can't validate
>>> that as I don't have access to the datasheet), and different addresses
>>> need to be used on different systems, then the address to be used needs
>>> to be provided by the firmware (DT in this case). Two options are
>>> possible, either specifying the address explicitly in the device's DT
>>> node, or specifying free addresses (in the form of a white list or black
>>> list) and allocating an address from that pool. The latter has been
>>> discussed in a BoF at the Linux Plumbers Conference last week,
>>> https://linuxplumbersconf.org/event/4/contributions/542/.
>>>
>>>> The downstream and upstream kernel sources divide these addresses by two
>>>> to get the i2c address. Here's the code in upstream:
>>>>
>>>> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analogix-anx78xx.c#L1353
>>>> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analogix-anx78xx.c#L41
>>>>
>>>> I'm not sure why the actual i2c address isn't used in this code.
>> The ANX7802/12/14/16 has a slave I2C bus that provides the interface to access
>> or control the chip from the AP. The I2C slave addresses used to control the
>> ANX7802/12/14/16 are 70h, 72h, 7Ah, 7Eh and 80h. Every address allows you to
>> access to different registers of the chip and AFAICS is not configurable.
>>
>> I don't think these addresses should be configured via DT but for the driver itself.
>>
>> My wild guess is that the ANX7808 has different addresses, but I don't have the
>> datasheet of this version.
> I'm able to communicate with the 7808 on my Nexus 5 using the 0x78
> address. Given that the addresses appear to be fixed per model, maybe it
> makes sense to drop the address #defines and add the addresses to the
> data pointer in the driver's of_match_table like so:
>
> static const struct of_device_id anx78xx_match_table[] = {
>         { .compatible = "analogix,anx7808", .data = PTR_TO_7808_ADDRS },
>         { .compatible = "analogix,anx7812", .data = PTR_TO_781X_ADDRS },
>         { .compatible = "analogix,anx7814", .data = PTR_TO_781X_ADDRS },
>         { .compatible = "analogix,anx7818", .data = PTR_TO_781X_ADDRS },
>         { /* sentinel */ },
> };
>
> Brian
>
>

I have spotted following comment on chromium's ML[1]:

> The locations are hard coded in the register spec.  Furthermore, each
> one can be changed independently--for example the Android driver puts
> 0x38 at 0x3c but leaves the rest alone.

It is not entirely clear, but IMO it suggests these addresses are
hardware configurable.


[1]:
https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/44601/2/drivers/auxdisplay/slimport.c#331


Regards

Andrzej

Re: [PATCH 05/11] drm/bridge: analogix-anx78xx: correct value of TX_P0

[Andrzej Hajda]

On 16.09.2019 14:02, Brian Masney wrote:
> On Mon, Sep 16, 2019 at 01:32:58PM +0200, Enric Balletbo i Serra wrote:
>> Hi,
>>
>> On 16/9/19 12:49, Laurent Pinchart wrote:
>>> Hi Brian,
>>>
>>> On Mon, Sep 16, 2019 at 06:36:14AM -0400, Brian Masney wrote:
>>>> On Mon, Sep 16, 2019 at 12:02:09PM +0200, Andrzej Hajda wrote:
>>>>> On 15.08.2019 02:48, Brian Masney wrote:
>>>>>> When attempting to configure this driver on a Nexus 5 phone (msm8974),
>>>>>> setting up the dummy i2c bus for TX_P0 would fail due to an -EBUSY
>>>>>> error. The downstream MSM kernel sources [1] shows that the proper value
>>>>>> for TX_P0 is 0x78, not 0x70, so correct the value to allow device
>>>>>> probing to succeed.
>>>>>>
>>>>>> [1] https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/drivers/video/slimport/slimport_tx_reg.h
>>>>>>
>>>>>> Signed-off-by: Brian Masney <masneyb@onstation.org>
>>>>>> ---
>>>>>>  drivers/gpu/drm/bridge/analogix-anx78xx.h | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.h b/drivers/gpu/drm/bridge/analogix-anx78xx.h
>>>>>> index 25e063bcecbc..bc511fc605c9 100644
>>>>>> --- a/drivers/gpu/drm/bridge/analogix-anx78xx.h
>>>>>> +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.h
>>>>>> @@ -6,7 +6,7 @@
>>>>>>  #ifndef __ANX78xx_H
>>>>>>  #define __ANX78xx_H
>>>>>>  
>>>>>> -#define TX_P0				0x70
>>>>>> +#define TX_P0				0x78
>>>>>
>>>>> This bothers me little. There are no upstream users, grepping android
>>>>> sources suggests that both values can be used [1][2]  (grep for "#define
>>>>> TX_P0"), moreover there is code suggesting both values can be valid [3].
>>>>>
>>>>> Could you verify datasheet which i2c slave addresses are valid for this
>>>>> chip, if both I guess this patch should be reworked.
>>>>>
>>>>>
>>>>> [1]:
>>>>> https://android.googlesource.com/kernel/msm/+/android-msm-flo-3.4-jb-mr2/drivers/misc/slimport_anx7808/slimport_tx_reg.h
>>>>>
>>>>> [2]:
>>>>> https://github.com/AndroidGX/SimpleGX-MM-6.0_H815_20d/blob/master/drivers/video/slimport/anx7812/slimport7812_tx_reg.h
>>>>>
>>>>> [3]:
>>>>> https://github.com/commaai/android_kernel_leeco_msm8996/blob/master/drivers/video/msm/mdss/dp/slimport_custom_declare.h#L73
>>>> This address is 0x78 on my Nexus 5. Given [3] above it looks like we
>>>> need to support both addresses. What do you think about moving these
>>>> addresses into device tree?
>>> Assuming that the device supports different addresses (I can't validate
>>> that as I don't have access to the datasheet), and different addresses
>>> need to be used on different systems, then the address to be used needs
>>> to be provided by the firmware (DT in this case). Two options are
>>> possible, either specifying the address explicitly in the device's DT
>>> node, or specifying free addresses (in the form of a white list or black
>>> list) and allocating an address from that pool. The latter has been
>>> discussed in a BoF at the Linux Plumbers Conference last week,
>>> https://linuxplumbersconf.org/event/4/contributions/542/.
>>>
>>>> The downstream and upstream kernel sources divide these addresses by two
>>>> to get the i2c address. Here's the code in upstream:
>>>>
>>>> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analogix-anx78xx.c#L1353
>>>> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analogix-anx78xx.c#L41
>>>>
>>>> I'm not sure why the actual i2c address isn't used in this code.
>> The ANX7802/12/14/16 has a slave I2C bus that provides the interface to access
>> or control the chip from the AP. The I2C slave addresses used to control the
>> ANX7802/12/14/16 are 70h, 72h, 7Ah, 7Eh and 80h. Every address allows you to
>> access to different registers of the chip and AFAICS is not configurable.
>>
>> I don't think these addresses should be configured via DT but for the driver itself.
>>
>> My wild guess is that the ANX7808 has different addresses, but I don't have the
>> datasheet of this version.
> I'm able to communicate with the 7808 on my Nexus 5 using the 0x78
> address. Given that the addresses appear to be fixed per model, maybe it
> makes sense to drop the address #defines and add the addresses to the
> data pointer in the driver's of_match_table like so:
>
> static const struct of_device_id anx78xx_match_table[] = {
>         { .compatible = "analogix,anx7808", .data = PTR_TO_7808_ADDRS },
>         { .compatible = "analogix,anx7812", .data = PTR_TO_781X_ADDRS },
>         { .compatible = "analogix,anx7814", .data = PTR_TO_781X_ADDRS },
>         { .compatible = "analogix,anx7818", .data = PTR_TO_781X_ADDRS },
>         { /* sentinel */ },
> };


With given feedback from other users and lack of datasheets for chips
(except anx7814) we can try this approach.


Regards

Andrzej


>
> Brian
>
>