[PATCH v3] drm/msm/dp: update is_connected status base on sink count at dp_pm_resume()
khsieh at codeaurora.org
khsieh at codeaurora.org
Wed Aug 4 15:48:04 UTC 2021
On 2021-08-03 12:05, Stephen Boyd wrote:
> Quoting Kuogee Hsieh (2021-08-03 09:25:13)
>> Currently at dp_pm_resume() is_connected state is decided base on hpd
>> connection
>> status only. This will put is_connected in wrongly "true" state at the
>> scenario
>> that dongle attached to DUT but without hmdi cable connecting to it.
>> Fix this
>> problem by adding read sink count from dongle and decided is_connected
>> state base
>> on both sink count and hpd connection status.
>>
>> Changes in v2:
>> -- remove dp_get_sink_count() cand call drm_dp_read_sink_count()
>>
>> Changes in v3:
>> -- delete status local variable from dp_pm_resume()
>>
>> Fixes: d9aa6571b28ba ("drm/msm/dp: check sink_count before update
>> is_connected status")
>> Signed-off-by: Kuogee Hsieh <khsieh at codeaurora.org>
>> ---
>> drivers/gpu/drm/msm/dp/dp_display.c | 18 +++++++++++++++---
>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
>> b/drivers/gpu/drm/msm/dp/dp_display.c
>> index 78c5301..0f39256 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -1313,7 +1313,7 @@ static int dp_pm_resume(struct device *dev)
>> struct platform_device *pdev = to_platform_device(dev);
>> struct msm_dp *dp_display = platform_get_drvdata(pdev);
>> struct dp_display_private *dp;
>> - u32 status;
>> + int sink_count = 0;
>>
>> dp = container_of(dp_display, struct dp_display_private,
>> dp_display);
>>
>> @@ -1327,14 +1327,26 @@ static int dp_pm_resume(struct device *dev)
>>
>> dp_catalog_ctrl_hpd_config(dp->catalog);
>>
>> - status = dp_catalog_link_is_connected(dp->catalog);
>> + /*
>> + * set sink to normal operation mode -- D0
>> + * before dpcd read
>> + */
>> + dp_link_psm_config(dp->link, &dp->panel->link_info, false);
>> +
>> + /* if sink conencted, do dpcd read sink count */
>
> s/conencted/connected/
>
> This also just says what the code is doing. Why do we only read the
> sink
> count if the link is connected? Can we read the sink count even if the
> link isn't connected and then consider sink count as 0 if trying to
> read
> fails?
>
yes, we can do that.
But it will suffer aux time out and retry.
i think it is better to avoid this overhead by check connection first.
>> + if (dp_catalog_link_is_connected(dp->catalog)) {
>> + sink_count = drm_dp_read_sink_count(dp->aux);
>> + if (sink_count < 0)
>> + sink_count = 0;
>> + }
>>
>> + dp->link->sink_count = sink_count;
>> /*
>> * can not declared display is connected unless
>> * HDMI cable is plugged in and sink_count of
>> * dongle become 1
>> */
>> - if (status && dp->link->sink_count)
>> + if (dp->link->sink_count)
>> dp->dp_display.is_connected = true;
>> else
>> dp->dp_display.is_connected = false;
More information about the dri-devel
mailing list