* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
@ 2017-11-27 9:00 ` Laurent Pinchart
0 siblings, 0 replies; 24+ messages in thread
From: Laurent Pinchart @ 2017-11-27 9:00 UTC (permalink / raw)
To: Archit Taneja
Cc: Laurent Pinchart, linux-kernel, dri-devel, Nick Bowler,
Dave Airlie, Linus Torvalds
Hi Archit,
Thank you for handling this, and sorry for missing the original bug report
(and for breaking this in the first place).
On Monday, 27 November 2017 06:05:03 EET Archit Taneja wrote:
> On 11/16/2017 11:58 AM, Nick Bowler wrote:
> > On 2017-11-05 11:41 -0500, Nick Bowler wrote:
> >> On 2017-11-02, Nick Bowler <nbowler@draconx.ca> 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@ideasonboard.com>
> >> 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().
Another difference is that the PWDN and TMDS signals, in theory needed for
Gen1 PHYs only, are not set anymore for Gen2 PHYs. Nick, could you test the
following change to see if it makes a difference ?
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/
bridge/synopsys/dw-hdmi.c
index b172139502d6..1c18ff1bf24a 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1104,14 +1104,14 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
unsigned int i;
u8 val;
- 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);
+ /* Toggle TMDS enable. */
+ dw_hdmi_phy_enable_tmds(hdmi, 0);
+ dw_hdmi_phy_enable_tmds(hdmi, 1);
+
+ if (phy->gen == 1)
return 0;
- }
dw_hdmi_phy_gen2_txpwron(hdmi, 1);
dw_hdmi_phy_gen2_pddq(hdmi, 0);
> 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?
The driver should print a "PHY PLL failed to lock" error message to the kernel
log in that case. Nick, does that happen on your system ?
> 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?
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
2017-11-27 9:00 ` Laurent Pinchart
(?)
@ 2017-11-27 16:34 ` Nick Bowler
-1 siblings, 0 replies; 24+ messages in thread
From: Nick Bowler @ 2017-11-27 16:34 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Archit Taneja, linux-kernel, dri-devel, Laurent Pinchart,
Linus Torvalds, Dave Airlie, Andrzej Hajda
Hi,
On 11/27/17, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> The driver should print a "PHY PLL failed to lock" error message to the
> kernel log in that case. Nick, does that happen on your system ?
I will try to test the other things later today, but after bootup there
were no messages whatsoever printed to the kernel log during the test
procedure.
Cheers,
Nick
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
2017-11-27 9:00 ` Laurent Pinchart
(?)
(?)
@ 2017-11-28 3:30 ` Nick Bowler
2017-12-01 0:11 ` Nick Bowler
-1 siblings, 1 reply; 24+ messages in thread
From: Nick Bowler @ 2017-11-28 3:30 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Archit Taneja, linux-kernel, dri-devel, Laurent Pinchart,
Linus Torvalds, Dave Airlie, Andrzej Hajda
[-- Attachment #1: Type: text/plain, Size: 4883 bytes --]
On 2017-11-27 11:00 +0200, Laurent Pinchart wrote:
> On Monday, 27 November 2017 06:05:03 EET Archit Taneja wrote:
> > On 2017-11-05 11:41 -0500, Nick Bowler wrote:
[...]
> > > Bisection implicates the following commit:
> > >
> > > 181e0ef092a4952aa523c5b9cb21394cf43bcd46 is the first bad commit
> > > commit 181e0ef092a4952aa523c5b9cb21394cf43bcd46
> > > Author: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > Date: Mon Mar 6 01:35:57 2017 +0200
> > >
> > > drm: bridge: dw-hdmi: Fix the PHY power up sequence
[...]
> >
> > 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().
>
> Another difference is that the PWDN and TMDS signals, in theory needed for
> Gen1 PHYs only, are not set anymore for Gen2 PHYs. Nick, could you test the
> following change to see if it makes a difference ?
I do not notice any difference with this change applied on top of Linux
4.15-rc1.
A note about the test setup: I had to remove the test equipment so I
no longer have any information about the video mode from the sink side
(like in the photos). Thus, with the current setup, I am using the
presense or absense of audio to determine whether the issue is present
or not.
The test procedure is: boot up, start music, then hotplug the hdmi four
times. If sound is heard after all four connections, PASS; otherwise FAIL.
- I retested on 4.15-rc1 to confirm that the issue is still present (it is).
- I applied the functional revert from earlier on top of 4.15-rc1 and the
problem is fixed.
- Returning to 4.15-rc1 and applying this next patch -- the issue is
present again (no change in behaviour compared to 4.15-rc1).
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/
> bridge/synopsys/dw-hdmi.c
> index b172139502d6..1c18ff1bf24a 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1104,14 +1104,14 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
> unsigned int i;
> u8 val;
>
> - 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);
> + /* Toggle TMDS enable. */
> + dw_hdmi_phy_enable_tmds(hdmi, 0);
> + dw_hdmi_phy_enable_tmds(hdmi, 1);
> +
> + if (phy->gen == 1)
> return 0;
> - }
>
> dw_hdmi_phy_gen2_txpwron(hdmi, 1);
> dw_hdmi_phy_gen2_pddq(hdmi, 0);
>
> > 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?
[...]
> > As an experiment, could you forcefully return 0 instead of -ETIMEDOUT and
> > see if things return back to normal?
I did both of these tests at once by applying the below patch on top of
4.15-rc1. There is no change in behaviour compared to 4.15-rc1 (except
for the added printouts).
With this, every time after inserting the cable the following is printed:
[ 128.002965] dwhdmi-rockchip ff980000.hdmi: 0: HDMI_PHY_STAT0: f2
[ 128.004614] dwhdmi-rockchip ff980000.hdmi: 1: HDMI_PHY_STAT0: f3
[ 128.013752] dwhdmi-rockchip ff980000.hdmi: 0: HDMI_PHY_STAT0: f2
[ 128.015605] dwhdmi-rockchip ff980000.hdmi: 1: HDMI_PHY_STAT0: f3
And there is no difference in output between working and non-working
cases. I've attached the full log; I manually logged extra messages
to give context from the test procedure:
"hdmi (not) working" - after bootup or connecting the cable
(indicating test pass/fail)
"hdmi disconnect" - after unplugging the cable.
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bf14214fa464..0358f6020fb4 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1118,7 +1118,11 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
/* Wait for PHY PLL lock */
for (i = 0; i < 5; ++i) {
- val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
+ val = hdmi_readb(hdmi, HDMI_PHY_STAT0);
+
+ dev_info(hdmi->dev, "%u: HDMI_PHY_STAT0: %.2hhx\n", i, val);
+
+ val &= HDMI_PHY_TX_PHY_LOCK;
if (val)
break;
@@ -1127,7 +1131,7 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
if (!val) {
dev_err(hdmi->dev, "PHY PLL failed to lock\n");
- return -ETIMEDOUT;
+ return 0;
}
dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
Let me know if there's anything else I should try.
Thanks,
Nick
[-- Attachment #2: aidos-hdmi-stat0.log.gz --]
[-- Type: application/octet-stream, Size: 7010 bytes --]
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
2017-11-28 3:30 ` Nick Bowler
@ 2017-12-01 0:11 ` Nick Bowler
2017-12-02 17:11 ` Jose Abreu
2017-12-04 19:06 ` Laurent Pinchart
0 siblings, 2 replies; 24+ messages in thread
From: Nick Bowler @ 2017-12-01 0:11 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Archit Taneja, linux-kernel, dri-devel, Laurent Pinchart,
Linus Torvalds, Dave Airlie, Andrzej Hajda
Hi,
On 2017-11-27 22:30 -0500, Nick Bowler wrote:
> A note about the test setup: I had to remove the test equipment so I
> no longer have any information about the video mode from the sink side
> (like in the photos). Thus, with the current setup, I am using the
> presense or absense of audio to determine whether the issue is present
> or not.
>
> The test procedure is: boot up, start music, then hotplug the hdmi four
> times. If sound is heard after all four connections, PASS; otherwise FAIL.
>
> - I retested on 4.15-rc1 to confirm that the issue is still present (it is).
>
> - I applied the functional revert from earlier on top of 4.15-rc1 and the
> problem is fixed.
>
> - Returning to 4.15-rc1 and applying [Laurent Pinchart's] patch --
> the issue is present again (no change in behaviour compared to
> 4.15-rc1).
Another data point... the following patch appears sufficient to restore
working behaviour.
Cheers,
Nick
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bf14214fa464..3118fbd8433d 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1101,8 +1101,6 @@ 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;
if (phy->gen == 1) {
dw_hdmi_phy_enable_powerdown(hdmi, false);
@@ -1116,21 +1114,6 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
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) {
- val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
- if (val)
- break;
-
- usleep_range(1000, 2000);
- }
-
- if (!val) {
- dev_err(hdmi->dev, "PHY PLL failed to lock\n");
- return -ETIMEDOUT;
- }
-
- dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
return 0;
}
--
2.13.6
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
2017-12-01 0:11 ` Nick Bowler
@ 2017-12-02 17:11 ` Jose Abreu
2017-12-04 19:06 ` Laurent Pinchart
1 sibling, 0 replies; 24+ messages in thread
From: Jose Abreu @ 2017-12-02 17:11 UTC (permalink / raw)
To: Nick Bowler, Laurent Pinchart
Cc: Laurent Pinchart, linux-kernel, dri-devel, Dave Airlie, Linus Torvalds
Hi Nick,
On 01-12-2017 00:11, Nick Bowler wrote:
> Hi,
>
> On 2017-11-27 22:30 -0500, Nick Bowler wrote:
>> A note about the test setup: I had to remove the test equipment so I
>> no longer have any information about the video mode from the sink side
>> (like in the photos). Thus, with the current setup, I am using the
>> presense or absense of audio to determine whether the issue is present
>> or not.
>>
>> The test procedure is: boot up, start music, then hotplug the hdmi four
>> times. If sound is heard after all four connections, PASS; otherwise FAIL.
>>
>> - I retested on 4.15-rc1 to confirm that the issue is still present (it is).
>>
>> - I applied the functional revert from earlier on top of 4.15-rc1 and the
>> problem is fixed.
>>
>> - Returning to 4.15-rc1 and applying [Laurent Pinchart's] patch --
>> the issue is present again (no change in behaviour compared to
>> 4.15-rc1).
> Another data point... the following patch appears sufficient to restore
> working behaviour.
>
> Cheers,
> Nick
>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 -----------------
> 1 file changed, 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index bf14214fa464..3118fbd8433d 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1101,8 +1101,6 @@ 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;
>
> if (phy->gen == 1) {
> dw_hdmi_phy_enable_powerdown(hdmi, false);
> @@ -1116,21 +1114,6 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
> 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) {
> - val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
> - if (val)
> - break;
> -
> - usleep_range(1000, 2000);
> - }
> -
> - if (!val) {
> - dev_err(hdmi->dev, "PHY PLL failed to lock\n");
> - return -ETIMEDOUT;
> - }
> -
> - dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
> return 0;
> }
>
I don't think you can do this. The phy pll lock check is
recommended and can indicate hw failure. Can you please check if
this untested, uncompiled patch makes it work correctly ?
------------------>8---------------
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bf14214..456fc54 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1669,7 +1669,7 @@ static void
hdmi_disable_overflow_interrupts(struct dw_hdmi *hdmi)
static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct
drm_display_mode *mode)
{
- int ret;
+ int ret, vsync_len = mode->vsync_end - mode->vsync_start;
hdmi_disable_overflow_interrupts(hdmi);
@@ -1722,6 +1722,14 @@ static int dw_hdmi_setup(struct dw_hdmi
*hdmi, struct drm_display_mode *mode)
return ret;
hdmi->phy.enabled = true;
+ /* Reset all clock domains */
+ hdmi_writeb(hdmi, 0x00, HDMI_MC_SWRSTZ);
+
+ /* Rewrite vsync register to latch previous written values */
+ if (mode->flags & DRM_MODE_FLAG_INTERLACE)
+ vsync_len /= 2;
+ hdmi_writeb(hdmi, vsync_len, HDMI_FC_VSYNCINWIDTH);
+
/* HDMI Initialization Step B.3 */
dw_hdmi_enable_video_path(hdmi);
------------------>8---------------
I would expect this patch to end your wrong image issue but the
audio part may be a different problem.
Best Regards,
Jose Miguel Abreu
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
@ 2017-12-02 17:11 ` Jose Abreu
0 siblings, 0 replies; 24+ messages in thread
From: Jose Abreu @ 2017-12-02 17:11 UTC (permalink / raw)
To: Nick Bowler, Laurent Pinchart
Cc: Dave Airlie, Laurent Pinchart, Linus Torvalds, linux-kernel, dri-devel
Hi Nick,
On 01-12-2017 00:11, Nick Bowler wrote:
> Hi,
>
> On 2017-11-27 22:30 -0500, Nick Bowler wrote:
>> A note about the test setup: I had to remove the test equipment so I
>> no longer have any information about the video mode from the sink side
>> (like in the photos). Thus, with the current setup, I am using the
>> presense or absense of audio to determine whether the issue is present
>> or not.
>>
>> The test procedure is: boot up, start music, then hotplug the hdmi four
>> times. If sound is heard after all four connections, PASS; otherwise FAIL.
>>
>> - I retested on 4.15-rc1 to confirm that the issue is still present (it is).
>>
>> - I applied the functional revert from earlier on top of 4.15-rc1 and the
>> problem is fixed.
>>
>> - Returning to 4.15-rc1 and applying [Laurent Pinchart's] patch --
>> the issue is present again (no change in behaviour compared to
>> 4.15-rc1).
> Another data point... the following patch appears sufficient to restore
> working behaviour.
>
> Cheers,
> Nick
>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 -----------------
> 1 file changed, 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index bf14214fa464..3118fbd8433d 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1101,8 +1101,6 @@ 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;
>
> if (phy->gen == 1) {
> dw_hdmi_phy_enable_powerdown(hdmi, false);
> @@ -1116,21 +1114,6 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
> 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) {
> - val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
> - if (val)
> - break;
> -
> - usleep_range(1000, 2000);
> - }
> -
> - if (!val) {
> - dev_err(hdmi->dev, "PHY PLL failed to lock\n");
> - return -ETIMEDOUT;
> - }
> -
> - dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
> return 0;
> }
>
I don't think you can do this. The phy pll lock check is
recommended and can indicate hw failure. Can you please check if
this untested, uncompiled patch makes it work correctly ?
------------------>8---------------
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bf14214..456fc54 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1669,7 +1669,7 @@ static void
hdmi_disable_overflow_interrupts(struct dw_hdmi *hdmi)
static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct
drm_display_mode *mode)
{
- int ret;
+ int ret, vsync_len = mode->vsync_end - mode->vsync_start;
hdmi_disable_overflow_interrupts(hdmi);
@@ -1722,6 +1722,14 @@ static int dw_hdmi_setup(struct dw_hdmi
*hdmi, struct drm_display_mode *mode)
return ret;
hdmi->phy.enabled = true;
+ /* Reset all clock domains */
+ hdmi_writeb(hdmi, 0x00, HDMI_MC_SWRSTZ);
+
+ /* Rewrite vsync register to latch previous written values */
+ if (mode->flags & DRM_MODE_FLAG_INTERLACE)
+ vsync_len /= 2;
+ hdmi_writeb(hdmi, vsync_len, HDMI_FC_VSYNCINWIDTH);
+
/* HDMI Initialization Step B.3 */
dw_hdmi_enable_video_path(hdmi);
------------------>8---------------
I would expect this patch to end your wrong image issue but the
audio part may be a different problem.
Best Regards,
Jose Miguel Abreu
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
2017-12-02 17:11 ` Jose Abreu
(?)
@ 2017-12-03 5:20 ` Nick Bowler
2017-12-04 10:04 ` Jose Abreu
-1 siblings, 1 reply; 24+ messages in thread
From: Nick Bowler @ 2017-12-03 5:20 UTC (permalink / raw)
To: Jose Abreu
Cc: Laurent Pinchart, Laurent Pinchart, linux-kernel, dri-devel,
Dave Airlie, Linus Torvalds
Hi Jose,
On 2017-12-02 17:11 +0000, Jose Abreu wrote:
> On 01-12-2017 00:11, Nick Bowler wrote:
> > Another data point... the following patch appears sufficient to
> > restore working behaviour.
[...]
> I don't think you can do this. The phy pll lock check is
> recommended and can indicate hw failure. Can you please check if
> this untested, uncompiled patch makes it work correctly ?
Your patch changes things. With this applied on top of 4.15-rc1
it is failing 100% of the time instead of only half of the time.
I brought the original test equipment back to the setup so I can
see the video and pink bar again. The symptoms remain the same
(unexpected size, pink bar, and no audio).
PS: your patch seems to have been line wrapped which made it a
bit annoying to apply.
> ------------------>8---------------
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index bf14214..456fc54 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1669,7 +1669,7 @@ static void
> hdmi_disable_overflow_interrupts(struct dw_hdmi *hdmi)
>
> static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct
> drm_display_mode *mode)
> {
> - int ret;
> + int ret, vsync_len = mode->vsync_end - mode->vsync_start;
>
> hdmi_disable_overflow_interrupts(hdmi);
>
> @@ -1722,6 +1722,14 @@ static int dw_hdmi_setup(struct dw_hdmi
> *hdmi, struct drm_display_mode *mode)
> return ret;
> hdmi->phy.enabled = true;
>
> + /* Reset all clock domains */
> + hdmi_writeb(hdmi, 0x00, HDMI_MC_SWRSTZ);
> +
> + /* Rewrite vsync register to latch previous written values */
> + if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> + vsync_len /= 2;
> + hdmi_writeb(hdmi, vsync_len, HDMI_FC_VSYNCINWIDTH);
> +
> /* HDMI Initialization Step B.3 */
> dw_hdmi_enable_video_path(hdmi);
>
> ------------------>8---------------
>
> I would expect this patch to end your wrong image issue but the
> audio part may be a different problem.
It is very consistent: pink bar <=> no audio.
My suspicion is that the audio problem is just the wrong video mode
on the sink side messing things up, but I have no way of confirming
that (that I know of).
Thanks,
Nick
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
2017-12-03 5:20 ` Nick Bowler
@ 2017-12-04 10:04 ` Jose Abreu
0 siblings, 0 replies; 24+ messages in thread
From: Jose Abreu @ 2017-12-04 10:04 UTC (permalink / raw)
To: Nick Bowler, Jose Abreu
Cc: Laurent Pinchart, Laurent Pinchart, linux-kernel, dri-devel,
Dave Airlie, Linus Torvalds
On 03-12-2017 05:20, Nick Bowler wrote:
>
> Your patch changes things. With this applied on top of 4.15-rc1
> it is failing 100% of the time instead of only half of the time.
Ok, it was a long shot anyway.
>
> I brought the original test equipment back to the setup so I can
> see the video and pink bar again. The symptoms remain the same
> (unexpected size, pink bar, and no audio).
>
Can you tell me which test equipment are you using?
> It is very consistent: pink bar <=> no audio.
>
> My suspicion is that the audio problem is just the wrong video mode
> on the sink side messing things up, but I have no way of confirming
> that (that I know of).
Hmmm, my first thought was that audio is being configured first
because of the phy lock wait time, I've seen this happening before.
Lets try this:
- Disable all alsa clients (e.g. pulseaudio, ...) so that no one
tries to configure audio.
- Plug out/in the cable until the issue appears
- When the issue appears use aplay to play audio through the HDMI
output
- Repeat several times with different audio rates and with no
resample (you can use the plughw interface in aplay).
>
> Thanks,
> Nick
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
@ 2017-12-04 10:04 ` Jose Abreu
0 siblings, 0 replies; 24+ messages in thread
From: Jose Abreu @ 2017-12-04 10:04 UTC (permalink / raw)
To: Nick Bowler, Jose Abreu
Cc: Laurent Pinchart, linux-kernel, dri-devel, Laurent Pinchart,
Dave Airlie, Linus Torvalds
On 03-12-2017 05:20, Nick Bowler wrote:
>
> Your patch changes things. With this applied on top of 4.15-rc1
> it is failing 100% of the time instead of only half of the time.
Ok, it was a long shot anyway.
>
> I brought the original test equipment back to the setup so I can
> see the video and pink bar again. The symptoms remain the same
> (unexpected size, pink bar, and no audio).
>
Can you tell me which test equipment are you using?
> It is very consistent: pink bar <=> no audio.
>
> My suspicion is that the audio problem is just the wrong video mode
> on the sink side messing things up, but I have no way of confirming
> that (that I know of).
Hmmm, my first thought was that audio is being configured first
because of the phy lock wait time, I've seen this happening before.
Lets try this:
- Disable all alsa clients (e.g. pulseaudio, ...) so that no one
tries to configure audio.
- Plug out/in the cable until the issue appears
- When the issue appears use aplay to play audio through the HDMI
output
- Repeat several times with different audio rates and with no
resample (you can use the plughw interface in aplay).
>
> Thanks,
> Nick
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
2017-12-04 10:04 ` Jose Abreu
(?)
@ 2017-12-04 18:33 ` Nick Bowler
2017-12-05 6:18 ` Nick Bowler
-1 siblings, 1 reply; 24+ messages in thread
From: Nick Bowler @ 2017-12-04 18:33 UTC (permalink / raw)
To: Jose Abreu
Cc: Laurent Pinchart, Laurent Pinchart, linux-kernel, dri-devel,
Dave Airlie, Linus Torvalds
On 2017-12-04 10:04 +0000, Jose Abreu wrote:
> On 03-12-2017 05:20, Nick Bowler wrote:
> > I brought the original test equipment back to the setup so I can
> > see the video and pink bar again. The symptoms remain the same
> > (unexpected size, pink bar, and no audio).
> >
>
> Can you tell me which test equipment are you using?
I am using an XRGB-Mini Framemeister as the sink device for testing
which isn't "real" test equipment but does show details of the video
and audio mode (which the monitor I have does not do).
The normal setup of this laptop has a small "audio splitter" as
the sink, which among other things includes HDMI input and output
ports and an S/PDIF audio output.
These devices can be connected together in various ways and the
results seem to be consistent in any configuration.
> > It is very consistent: pink bar <=> no audio.
> >
> > My suspicion is that the audio problem is just the wrong video mode
> > on the sink side messing things up, but I have no way of confirming
> > that (that I know of).
>
> Hmmm, my first thought was that audio is being configured first
> because of the phy lock wait time, I've seen this happening before.
>
> Lets try this:
> - Disable all alsa clients (e.g. pulseaudio, ...) so that no one
> tries to configure audio.
> - Plug out/in the cable until the issue appears
> - When the issue appears use aplay to play audio through the HDMI
> output
> - Repeat several times with different audio rates and with no
> resample (you can use the plughw interface in aplay).
OK, I will give it a try later this evening.
Cheers,
Nick
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
2017-12-04 18:33 ` Nick Bowler
@ 2017-12-05 6:18 ` Nick Bowler
0 siblings, 0 replies; 24+ messages in thread
From: Nick Bowler @ 2017-12-05 6:18 UTC (permalink / raw)
To: Jose Abreu
Cc: Laurent Pinchart, Laurent Pinchart, linux-kernel, dri-devel,
Dave Airlie, Linus Torvalds
On 2017-12-04 13:33 -0500, Nick Bowler wrote:
> On 2017-12-04 10:04 +0000, Jose Abreu wrote:
> > Hmmm, my first thought was that audio is being configured first
> > because of the phy lock wait time, I've seen this happening before.
> >
> > Lets try this:
> > - Disable all alsa clients (e.g. pulseaudio, ...) so that no one
> > tries to configure audio.
> > - Plug out/in the cable until the issue appears
> > - When the issue appears use aplay to play audio through the HDMI
> > output
> > - Repeat several times with different audio rates and with no
> > resample (you can use the plughw interface in aplay).
>
> OK, I will give it a try later this evening.
Using the above sequence on unpatched 4.15-rc1 it seems there is no
sound when starting audio output after the pink bar is visible.
However I am not confident of the results here, restarting aplay
with different sample rates (or even restarting with the same rate)
is causing some weird effects on my setup so I want to check the test
setup with some different source devices.
Cheers,
Nick
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
2017-12-01 0:11 ` Nick Bowler
@ 2017-12-04 19:06 ` Laurent Pinchart
2017-12-04 19:06 ` Laurent Pinchart
1 sibling, 0 replies; 24+ messages in thread
From: Laurent Pinchart @ 2017-12-04 19:06 UTC (permalink / raw)
To: Nick Bowler
Cc: Archit Taneja, linux-kernel, dri-devel, Laurent Pinchart,
Linus Torvalds, Dave Airlie, Andrzej Hajda
Hi Nick,
On Friday, 1 December 2017 02:11:46 EET Nick Bowler wrote:
> On 2017-11-27 22:30 -0500, Nick Bowler wrote:
> > A note about the test setup: I had to remove the test equipment so I
> > no longer have any information about the video mode from the sink side
> > (like in the photos). Thus, with the current setup, I am using the
> > presense or absense of audio to determine whether the issue is present
> > or not.
> >
> > The test procedure is: boot up, start music, then hotplug the hdmi four
> > times. If sound is heard after all four connections, PASS; otherwise
> > FAIL.
> >
> > - I retested on 4.15-rc1 to confirm that the issue is still present (it
> > is).
> >
> > - I applied the functional revert from earlier on top of 4.15-rc1 and the
> > problem is fixed.
> >
> > - Returning to 4.15-rc1 and applying [Laurent Pinchart's] patch --
> > the issue is present again (no change in behaviour compared to
> > 4.15-rc1).
>
> Another data point... the following patch appears sufficient to restore
> working behaviour.
>
> Cheers,
> Nick
>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 -----------------
> 1 file changed, 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> bf14214fa464..3118fbd8433d 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1101,8 +1101,6 @@ 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;
>
> if (phy->gen == 1) {
> dw_hdmi_phy_enable_powerdown(hdmi, false);
> @@ -1116,21 +1114,6 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
> 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) {
> - val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
> - if (val)
> - break;
> -
> - usleep_range(1000, 2000);
> - }
> -
> - if (!val) {
> - dev_err(hdmi->dev, "PHY PLL failed to lock\n");
> - return -ETIMEDOUT;
> - }
> -
> - dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
As you reported that the PLL lock failure message is not printed, the failure
can only come from either the extra delay introduced by the above loop, or
from reading the HDMI_PHY_STAT0 register.
How many iterations of the for loop execute before the condition becomes true
?
> return 0;
> }
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
@ 2017-12-04 19:06 ` Laurent Pinchart
0 siblings, 0 replies; 24+ messages in thread
From: Laurent Pinchart @ 2017-12-04 19:06 UTC (permalink / raw)
To: Nick Bowler
Cc: Laurent Pinchart, linux-kernel, dri-devel, Dave Airlie, Linus Torvalds
Hi Nick,
On Friday, 1 December 2017 02:11:46 EET Nick Bowler wrote:
> On 2017-11-27 22:30 -0500, Nick Bowler wrote:
> > A note about the test setup: I had to remove the test equipment so I
> > no longer have any information about the video mode from the sink side
> > (like in the photos). Thus, with the current setup, I am using the
> > presense or absense of audio to determine whether the issue is present
> > or not.
> >
> > The test procedure is: boot up, start music, then hotplug the hdmi four
> > times. If sound is heard after all four connections, PASS; otherwise
> > FAIL.
> >
> > - I retested on 4.15-rc1 to confirm that the issue is still present (it
> > is).
> >
> > - I applied the functional revert from earlier on top of 4.15-rc1 and the
> > problem is fixed.
> >
> > - Returning to 4.15-rc1 and applying [Laurent Pinchart's] patch --
> > the issue is present again (no change in behaviour compared to
> > 4.15-rc1).
>
> Another data point... the following patch appears sufficient to restore
> working behaviour.
>
> Cheers,
> Nick
>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 -----------------
> 1 file changed, 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> bf14214fa464..3118fbd8433d 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1101,8 +1101,6 @@ 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;
>
> if (phy->gen == 1) {
> dw_hdmi_phy_enable_powerdown(hdmi, false);
> @@ -1116,21 +1114,6 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
> 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) {
> - val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
> - if (val)
> - break;
> -
> - usleep_range(1000, 2000);
> - }
> -
> - if (!val) {
> - dev_err(hdmi->dev, "PHY PLL failed to lock\n");
> - return -ETIMEDOUT;
> - }
> -
> - dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
As you reported that the PLL lock failure message is not printed, the failure
can only come from either the extra delay introduced by the above loop, or
from reading the HDMI_PHY_STAT0 register.
How many iterations of the for loop execute before the condition becomes true
?
> return 0;
> }
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
2017-12-04 19:06 ` Laurent Pinchart
(?)
@ 2017-12-04 19:30 ` Nick Bowler
2017-12-04 19:34 ` Laurent Pinchart
-1 siblings, 1 reply; 24+ messages in thread
From: Nick Bowler @ 2017-12-04 19:30 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Archit Taneja, linux-kernel, dri-devel, Laurent Pinchart,
Linus Torvalds, Dave Airlie, Andrzej Hajda
Hi,
On 2017-12-04 21:06 +0200, Laurent Pinchart wrote:
> As you reported that the PLL lock failure message is not printed, the
> failure can only come from either the extra delay introduced by the
> above loop, or from reading the HDMI_PHY_STAT0 register.
>
> How many iterations of the for loop execute before the condition
> becomes true?
Judging from the log posted elsethread (where I added extra printouts),
it seems to consistently become true on the second iteration.
I will try to rule out read side effects by replacing the polling loop
with an unconditional delay.
Cheers,
--
Nick Bowler
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
2017-12-04 19:30 ` Nick Bowler
@ 2017-12-04 19:34 ` Laurent Pinchart
2017-12-05 3:22 ` Nick Bowler
0 siblings, 1 reply; 24+ messages in thread
From: Laurent Pinchart @ 2017-12-04 19:34 UTC (permalink / raw)
To: Nick Bowler
Cc: Archit Taneja, linux-kernel, dri-devel, Laurent Pinchart,
Linus Torvalds, Dave Airlie, Andrzej Hajda
Hi Nick,
On Monday, 4 December 2017 21:30:01 EET Nick Bowler wrote:
> On 2017-12-04 21:06 +0200, Laurent Pinchart wrote:
> > As you reported that the PLL lock failure message is not printed, the
> > failure can only come from either the extra delay introduced by the
> > above loop, or from reading the HDMI_PHY_STAT0 register.
> >
> > How many iterations of the for loop execute before the condition
> > becomes true?
>
> Judging from the log posted elsethread (where I added extra printouts),
> it seems to consistently become true on the second iteration.
>
> I will try to rule out read side effects by replacing the polling loop
> with an unconditional delay.
You're reading my mind :-)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
2017-12-04 19:34 ` Laurent Pinchart
@ 2017-12-05 3:22 ` Nick Bowler
2017-12-05 9:41 ` Laurent Pinchart
0 siblings, 1 reply; 24+ messages in thread
From: Nick Bowler @ 2017-12-05 3:22 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Archit Taneja, linux-kernel, dri-devel, Laurent Pinchart,
Linus Torvalds, Dave Airlie, Andrzej Hajda, Jose Abreu
On 2017-12-04 21:34 +0200, Laurent Pinchart wrote:
> On Monday, 4 December 2017 21:30:01 EET Nick Bowler wrote:
> > On 2017-12-04 21:06 +0200, Laurent Pinchart wrote:
> > > As you reported that the PLL lock failure message is not printed, the
> > > failure can only come from either the extra delay introduced by the
> > > above loop, or from reading the HDMI_PHY_STAT0 register.
> > >
> > > How many iterations of the for loop execute before the condition
> > > becomes true?
> >
> > Judging from the log posted elsethread (where I added extra printouts),
> > it seems to consistently become true on the second iteration.
> >
> > I will try to rule out read side effects by replacing the polling loop
> > with an unconditional delay.
>
> You're reading my mind :-)
I did this test by applying the following patch on 4.15-rc1, and the
problem remains. So it appears the delay is responsible somehow.
Cheers,
Nick
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bf14214fa464..4aec4d5c130e 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1101,8 +1101,6 @@ 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;
if (phy->gen == 1) {
dw_hdmi_phy_enable_powerdown(hdmi, false);
@@ -1116,21 +1114,7 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
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) {
- val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
- if (val)
- break;
-
- usleep_range(1000, 2000);
- }
-
- if (!val) {
- dev_err(hdmi->dev, "PHY PLL failed to lock\n");
- return -ETIMEDOUT;
- }
-
- dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
+ usleep_range(1000, 2000);
return 0;
}
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
2017-12-05 3:22 ` Nick Bowler
@ 2017-12-05 9:41 ` Laurent Pinchart
0 siblings, 0 replies; 24+ messages in thread
From: Laurent Pinchart @ 2017-12-05 9:41 UTC (permalink / raw)
To: Nick Bowler
Cc: Archit Taneja, linux-kernel, dri-devel, Laurent Pinchart,
Linus Torvalds, Dave Airlie, Andrzej Hajda, Jose Abreu
Hi Nick,
On Tuesday, 5 December 2017 05:22:28 EET Nick Bowler wrote:
> On 2017-12-04 21:34 +0200, Laurent Pinchart wrote:
> > On Monday, 4 December 2017 21:30:01 EET Nick Bowler wrote:
> >> On 2017-12-04 21:06 +0200, Laurent Pinchart wrote:
> >>> As you reported that the PLL lock failure message is not printed, the
> >>> failure can only come from either the extra delay introduced by the
> >>> above loop, or from reading the HDMI_PHY_STAT0 register.
> >>>
> >>> How many iterations of the for loop execute before the condition
> >>> becomes true?
> >>
> >> Judging from the log posted elsethread (where I added extra printouts),
> >> it seems to consistently become true on the second iteration.
> >>
> >> I will try to rule out read side effects by replacing the polling loop
> >> with an unconditional delay.
> >
> > You're reading my mind :-)
>
> I did this test by applying the following patch on 4.15-rc1, and the
> problem remains. So it appears the delay is responsible somehow.
That's interesting. I think it also means we really need to find the root
cause, as otherwise your system would be susceptible to random malfunction if
the scheduler ends up interrupting the power on sequence. That might not
happen frequently, but would be much harder to debug.
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> bf14214fa464..4aec4d5c130e 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1101,8 +1101,6 @@ 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;
>
> if (phy->gen == 1) {
> dw_hdmi_phy_enable_powerdown(hdmi, false);
> @@ -1116,21 +1114,7 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
> 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) {
> - val = hdmi_readb(hdmi, HDMI_PHY_STAT0) &
> HDMI_PHY_TX_PHY_LOCK; - if (val)
> - break;
> -
> - usleep_range(1000, 2000);
> - }
> -
> - if (!val) {
> - dev_err(hdmi->dev, "PHY PLL failed to lock\n");
> - return -ETIMEDOUT;
> - }
> -
> - dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
> + usleep_range(1000, 2000);
> return 0;
> }
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)
@ 2017-12-05 9:41 ` Laurent Pinchart
0 siblings, 0 replies; 24+ messages in thread
From: Laurent Pinchart @ 2017-12-05 9:41 UTC (permalink / raw)
To: Nick Bowler
Cc: Jose Abreu, Laurent Pinchart, linux-kernel, dri-devel,
Dave Airlie, Linus Torvalds
Hi Nick,
On Tuesday, 5 December 2017 05:22:28 EET Nick Bowler wrote:
> On 2017-12-04 21:34 +0200, Laurent Pinchart wrote:
> > On Monday, 4 December 2017 21:30:01 EET Nick Bowler wrote:
> >> On 2017-12-04 21:06 +0200, Laurent Pinchart wrote:
> >>> As you reported that the PLL lock failure message is not printed, the
> >>> failure can only come from either the extra delay introduced by the
> >>> above loop, or from reading the HDMI_PHY_STAT0 register.
> >>>
> >>> How many iterations of the for loop execute before the condition
> >>> becomes true?
> >>
> >> Judging from the log posted elsethread (where I added extra printouts),
> >> it seems to consistently become true on the second iteration.
> >>
> >> I will try to rule out read side effects by replacing the polling loop
> >> with an unconditional delay.
> >
> > You're reading my mind :-)
>
> I did this test by applying the following patch on 4.15-rc1, and the
> problem remains. So it appears the delay is responsible somehow.
That's interesting. I think it also means we really need to find the root
cause, as otherwise your system would be susceptible to random malfunction if
the scheduler ends up interrupting the power on sequence. That might not
happen frequently, but would be much harder to debug.
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> bf14214fa464..4aec4d5c130e 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1101,8 +1101,6 @@ 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;
>
> if (phy->gen == 1) {
> dw_hdmi_phy_enable_powerdown(hdmi, false);
> @@ -1116,21 +1114,7 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
> 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) {
> - val = hdmi_readb(hdmi, HDMI_PHY_STAT0) &
> HDMI_PHY_TX_PHY_LOCK; - if (val)
> - break;
> -
> - usleep_range(1000, 2000);
> - }
> -
> - if (!val) {
> - dev_err(hdmi->dev, "PHY PLL failed to lock\n");
> - return -ETIMEDOUT;
> - }
> -
> - dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
> + usleep_range(1000, 2000);
> return 0;
> }
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 24+ messages in thread