| Message ID | 1618246971-28754-1-git-send-email-khsieh@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] drm/msm/dp: check sink_count before update is_connected status | expand |
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;
Quoting Kuogee Hsieh (2021-04-12 10:02:51) > At pm_resume check link sisnk_count before update is_connected status > 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. > > Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org> > --- Also please include Reported-by: Stephen Boyd <swboyd@chromium.org> in the next post.
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"); 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"); 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) dp->dp_display.is_connected = true; else dp->dp_display.is_connected = false;
At pm_resume check link sisnk_count before update is_connected status 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. Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org> --- drivers/gpu/drm/msm/dp/dp_display.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)