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(-)
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
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
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
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);
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
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; }
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
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])) {
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
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/slimpo...
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
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/slimpo...
Signed-off-by: Brian Masney masneyb@onstation.org
Reviewed-by: Linus Walleij linus.walleij@linaro.org
Yours, Linus Walleij
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/slimpo...
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/dri...
[2]: https://github.com/AndroidGX/SimpleGX-MM-6.0_H815_20d/blob/master/drivers/vi...
[3]: https://github.com/commaai/android_kernel_leeco_msm8996/blob/master/drivers/...
Regards
Andrzej
#define TX_P1 0x7a #define TX_P2 0x72
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/slimpo...
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.
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/analog... https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analog...
I'm not sure why the actual i2c address isn't used in this code.
Brian
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/slimpo...
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.
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/analog... https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analog...
I'm not sure why the actual i2c address isn't used in this code.
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/slimpo...
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.
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/analog... https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analog...
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
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/slimpo...
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.
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/analog... https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analog...
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
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/slimpo...
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.
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/analog... https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analog...
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/+/4...
Regards
Andrzej
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/slimpo...
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.
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/analog... https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/analog...
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
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/slimpo...
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
Regards
Andrzej
#define TX_P1 0x7a #define TX_P2 0x72
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/slimpo...
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
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");
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
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
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
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
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; }
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
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>; + }; }; }; };
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
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/ms...
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
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/ms...
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>; + }; }; };
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
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/ms...
.../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>; + }; }; }; };
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
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(-)
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
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
dri-devel@lists.freedesktop.org
-
Andrzej Hajda -
Brian Masney -
Enric Balletbo i Serra -
Laurent Pinchart -
Linus Walleij -
Rob Herring