Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)

Jose Abreu
2017-12-02T17:11:40Z

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