Hi,
On 11/16/2017 11:58 AM, Nick Bowler wrote:
Hi,
Any ideas on this issue? Are there any additional tests I can perform
to help debug this?
On 2017-11-05 11:41 -0500, Nick Bowler wrote:
I completed bisecting this issue. See below.
On 2017-11-02, Nick Bowler <nbowler@xxxxxxxxxx> wrote:
~50% of the time after a hotplug, there is a vertical pink bar on the
left of the display area and audio is not working at all. According to
the sink device the display size is 1282x720 which seems pretty wrong
(normal and working situation is 1280x720).
I posted photos of non-working versus working states here:
https://imgur.com/a/qhAZG
Unplugging and plugging the cable again will correct the issue (it seems
to, for the most part, alternate between working and not-working states,
although not always). It always works on power up with the cable initially
connected.
This is a regression from 4.11, where hotplug works perfectly every time.
Bisection implicates the following commit:
181e0ef092a4952aa523c5b9cb21394cf43bcd46 is the first bad commit
commit 181e0ef092a4952aa523c5b9cb21394cf43bcd46
Author: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
Date: Mon Mar 6 01:35:57 2017 +0200
drm: bridge: dw-hdmi: Fix the PHY power up sequence
When powering the PHY up we need to wait for the PLL to lock. This is
done by polling the TX_PHY_LOCK bit in the HDMI_PHY_STAT0 register
(interrupt-based wait could be implemented as well but is likely
overkill). The bit is asserted when the PLL locks, but the current code
incorrectly waits for the bit to be deasserted. Fix it, and while at it,
replace the udelay() with a sleep as the code never runs in
non-sleepable context.
To be consistent with the power down implementation move the poll loop
to the power off function.
The two main things the commit below does it to a) correctly wait on the
TX_PHY_LOCK bit to be asserted and b) use usleep_range() instead of udelay().
I don't see (b) being a problem. About (a), it's possible that the bit above
is interpreted differently on a rockchip SoC versus a renesas chip. Could you
print the value of HDMI_PHY_STAT0 that's read back?
If the code returns an -ETIMEDOUT, hdmi->phy.ops->init() will return an -ETIMEDOUT.
This will cause dw_hdmi_setup() to bail out early, before we get a chance to
configure the AVI infoframe and other stuff. I've seen other HDMI HW throwing up pink
strips if the AVI infoframe stuff isn't configured properly.
As an experiment, could you forcefully return 0 instead of -ETIMEDOUT and see if things
return back to normal?
Thanks,
Archit
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
Tested-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
Reviewed-by: Jose Abreu <joabreu@xxxxxxxxxxxx>
Signed-off-by: Archit Taneja <architt@xxxxxxxxxxxxxx>
Link: http://patchwork.freedesktop.org/patch/msgid/20170305233557.11945-1-laurent.pinchart+renesas@xxxxxxxxxxxxxxxx
:040000 040000 0defad9d1a61c0355f49c679b18eebae2c4b9495
5d260e6db25d6abc1211d61ec3405be99e693a23 M drivers
This commit does not revert cleanly, but on top of latest master (which has
the problem) I manually changed the relevant code back to its original state
and the problem is fixed, like this:
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bf14214fa464..6618aac95a51 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1100,37 +1100,34 @@ static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
{
- const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
- unsigned int i;
- u8 val;
+ u8 val, msec;
- if (phy->gen == 1) {
- dw_hdmi_phy_enable_powerdown(hdmi, false);
+ dw_hdmi_phy_enable_powerdown(hdmi, false);
- /* Toggle TMDS enable. */
- dw_hdmi_phy_enable_tmds(hdmi, 0);
- dw_hdmi_phy_enable_tmds(hdmi, 1);
- return 0;
- }
+ /* toggle TMDS enable */
+ dw_hdmi_phy_enable_tmds(hdmi, 0);
+ dw_hdmi_phy_enable_tmds(hdmi, 1);
+ /* gen2 tx power on */
dw_hdmi_phy_gen2_txpwron(hdmi, 1);
dw_hdmi_phy_gen2_pddq(hdmi, 0);
/* Wait for PHY PLL lock */
- for (i = 0; i < 5; ++i) {
+ msec = 5;
+ do {
val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
- if (val)
+ if (!val)
break;
- usleep_range(1000, 2000);
- }
+ if (msec == 0) {
+ dev_err(hdmi->dev, "PHY PLL not locked\n");
+ return -ETIMEDOUT;
+ }
- if (!val) {
- dev_err(hdmi->dev, "PHY PLL failed to lock\n");
- return -ETIMEDOUT;
- }
+ udelay(1000);
+ msec--;
+ } while (1);
- dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
return 0;
}
Thanks,
Nick
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
