Quoting Kuogee Hsieh (2021-04-12 10:02:51)

At pm_resume check link sisnk_count before update is_connected status

s/sisnk_count/sink_count/

base on HPD real time link status. Also print out error message only when either EV_CONNECT_PENDING_TIMEOUT or EV_DISCONNECT_PENDING_TIMEOUT happen.

Can you add "why"? I think the why is something like "link status is different from display connected status in the case of something like an Apple dongle where the type-c plug can be connected, and therefore the link is connected, but no sink is connected until an HDMI cable is plugged into the dongle". This still doesn't explain why it's important to check at resume time though.

Signed-off-by: Kuogee Hsieh khsieh@codeaurora.org

Any Fixes tag?

drivers/gpu/drm/msm/dp/dp_display.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 5a39da6..4992a049 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -587,7 +587,7 @@ static int dp_connect_pending_timeout(struct dp_display_private *dp, u32 data)

    state = dp->hpd_state;
    if (state == ST_CONNECT_PENDING) {

•           dp_display_enable(dp, 0);
  

•           DRM_ERROR("EV_CONNECT_PENDING_TIMEOUT error\n");
  

Can we get rid of these messages?

            dp->hpd_state = ST_CONNECTED;
    }

@@ -670,7 +670,7 @@ static int dp_disconnect_pending_timeout(struct dp_display_private *dp, u32 data

    state =  dp->hpd_state;
    if (state == ST_DISCONNECT_PENDING) {

•           dp_display_disable(dp, 0);
  

•           DRM_ERROR("EV_DISCONNECT_PENDING_TIMEOUT error\n");
  

And this one? If it happens it will just sit in the logs when probably the user can't do anything about it. Timeouts are just a fact of life.

            dp->hpd_state = ST_DISCONNECTED;
    }

@@ -1272,7 +1272,7 @@ static int dp_pm_resume(struct device *dev)

    status = dp_catalog_link_is_connected(dp->catalog);

•   if (status)
  

•   if (status && dp->link->sink_count)
  

Can we add a comment above this if? Otherwise it doesn't make much sense why sink_count is important.

/* * Only consider the display as connected, and send a connected * notification to userspace in * dp_display_send_hpd_notification(), if there's actually a * sink connected. Otherwise, the link could be up/connected or * in the process of being established, but there isn't actually * anything to display to on the other side yet. */

            dp->dp_display.is_connected = true;
    else
            dp->dp_display.is_connected = false;