Change in coreboot[master]: sc7180: Remove the delay to force hpd detection and always disable HPD

[Ravi kumar (Code Review)]

Ravi kumar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45706 )

Change subject: sc7180: Remove the delay to force hpd detection and always disable HPD
......................................................................

sc7180: Remove the delay to force hpd detection and always disable HPD

HPD on this bridge chip is a bit useless. This is an eDP bridge so the HPD is
an internal signal that's only there to signal that the panel is done powering up.
But the bridge chip debounces this signal by between 100 ms and 400 ms (depending on process,
voltage, and temperate). One particular panel asserted HPD 84 ms after it was powered on
meaning that we saw HPD 284 ms after power on. Assume that the panel driver will have the
hardcoded delay in its prepare and always disable HPD.

Change-Id: Iea7dd75b57fa55ec182c0bee09b0f35208357892
Signed-off-by: Vinod Polimera 
---
M src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c
1 file changed, 2 insertions(+), 30 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/45706/1

diff --git a/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c b/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c
index e0058c4..a73d6d7 100644
--- a/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c
+++ b/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c
@@ -426,35 +426,6 @@

}

-static enum cb_err sn65dsi86_bridge_get_plug_in_status(uint8_t bus, uint8_t chip)
-{
-	int val;
-	uint8_t buf;
-
-	val = i2c_readb(bus, chip, SN_HPD_DISABLE_REG, &buf);
-	if (val == 0 && (buf & HPD_DISABLE))
-		return CB_SUCCESS;
-
-	return CB_ERR;
-}
-
-/*
- * support bridge HPD function some hardware versions do not support bridge hdp,
- * we use 360ms to try to get the hpd single now, if we can not get bridge hpd single,
- * it will delay 360ms, also meet the bridge power timing request, to be compatible
- * all of the hardware versions
- */
-static void sn65dsi86_bridge_wait_hpd(uint8_t bus, uint8_t chip)
-{
-	if (wait_ms(400, sn65dsi86_bridge_get_plug_in_status(bus, chip)))
-		return;
-
-	printk(BIOS_WARNING, "HPD detection failed, force hpd\n");
-
-	/* Force HPD */
-	i2c_write_field(bus, chip, SN_HPD_DISABLE_REG, HPD_DISABLE, 1, 0);
-}
-
 static void sn65dsi86_bridge_assr_config(uint8_t bus, uint8_t chip, int enable)
 {
    if (enable)
@@ -476,7 +447,8 @@

void sn65dsi86_bridge_init(uint8_t bus, uint8_t chip, enum dp_pll_clk_src ref_clk)
 {
-	sn65dsi86_bridge_wait_hpd(bus, chip);
+	/* disable HPD */
+	i2c_write_field(bus, chip, SN_HPD_DISABLE_REG, HPD_DISABLE, 1, 0);

/* set refclk to 19.2 MHZ */
    i2c_write_field(bus, chip, SN_DPPLL_SRC_REG, ref_clk, 7, 1);

[Douglas Anderson (Code Review)]

Douglas Anderson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45706 )

Change subject: sc7180: Remove the delay to force hpd detection and always disable HPD
......................................................................

Patch Set 1: Code-Review-1

...but then you need a delay.  See https://review.coreboot.org/c/coreboot/+/45744

[Douglas Anderson (Code Review)]

Douglas Anderson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45706 )

Change subject: sc7180: Remove the delay to force hpd detection and always disable HPD
......................................................................

Patch Set 1: -Code-Review

Ah, Julius points out that there's always a 250 ms delay...

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45706 )

Change subject: sc7180: Remove the delay to force hpd detection and always disable HPD
......................................................................

Patch Set 1: Code-Review+2

[Julius Werner (Code Review)]

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45706 )

Change subject: sc7180: Remove the delay to force hpd detection and always disable HPD
......................................................................

sc7180: Remove the delay to force hpd detection and always disable HPD

HPD on this bridge chip is a bit useless. This is an eDP bridge so the HPD is
an internal signal that's only there to signal that the panel is done powering up.
But the bridge chip debounces this signal by between 100 ms and 400 ms (depending on process,
voltage, and temperate). One particular panel asserted HPD 84 ms after it was powered on
meaning that we saw HPD 284 ms after power on. Assume that the panel driver will have the
hardcoded delay in its prepare and always disable HPD.

Change-Id: Iea7dd75b57fa55ec182c0bee09b0f35208357892
Signed-off-by: Vinod Polimera 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/45706
Tested-by: build bot (Jenkins) 
Reviewed-by: Julius Werner 
---
M src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c
1 file changed, 2 insertions(+), 30 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Julius Werner: Looks good to me, approved

diff --git a/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c b/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c
index effa841..5a6eb49 100644
--- a/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c
+++ b/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c
@@ -431,35 +431,6 @@
    printk(BIOS_ERR, "ERROR: Link training failed 10 times\n");
 }

-static enum cb_err sn65dsi86_bridge_get_plug_in_status(uint8_t bus, uint8_t chip)
-{
-	int val;
-	uint8_t buf;
-
-	val = i2c_readb(bus, chip, SN_HPD_DISABLE_REG, &buf);
-	if (val == 0 && (buf & HPD_DISABLE))
-		return CB_SUCCESS;
-
-	return CB_ERR;
-}
-
-/*
- * support bridge HPD function some hardware versions do not support bridge hdp,
- * we use 360ms to try to get the hpd single now, if we can not get bridge hpd single,
- * it will delay 360ms, also meet the bridge power timing request, to be compatible
- * all of the hardware versions
- */
-static void sn65dsi86_bridge_wait_hpd(uint8_t bus, uint8_t chip)
-{
-	if (wait_ms(400, sn65dsi86_bridge_get_plug_in_status(bus, chip)))
-		return;
-
-	printk(BIOS_WARNING, "HPD detection failed, force hpd\n");
-
-	/* Force HPD */
-	i2c_write_field(bus, chip, SN_HPD_DISABLE_REG, HPD_DISABLE, 1, 0);
-}
-
 static void sn65dsi86_bridge_assr_config(uint8_t bus, uint8_t chip, int enable)
 {
    if (enable)
@@ -481,7 +452,8 @@

void sn65dsi86_bridge_init(uint8_t bus, uint8_t chip, enum dp_pll_clk_src ref_clk)
 {
-	sn65dsi86_bridge_wait_hpd(bus, chip);
+	/* disable HPD */
+	i2c_write_field(bus, chip, SN_HPD_DISABLE_REG, HPD_DISABLE, 1, 0);

/* set refclk to 19.2 MHZ */
    i2c_write_field(bus, chip, SN_DPPLL_SRC_REG, ref_clk, 7, 1);