Re: [PATCH] drm/msm/dp: promote irq_hpd handle to handle link trainign correctly

[khsieh@codeaurora.org]

On 2020-11-02 11:29, Stephen Boyd wrote:
> Subject has a typo in "training".
> 
> Quoting Kuogee Hsieh (2020-10-30 16:23:24)
>> Some dongles, such as Apple, required link training done at irq_hpd
> 
> s/required/require/
> 
>> request instead of plugin request. This patch promote irq_hpd hanlder
> 
> s/hanlder/handler/
> 
>> to handle link training and setup hpd_state correctly.
>> 
>> Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org>
>> ---
> 
> Any Fixes tag?
> 
>>  drivers/gpu/drm/msm/dp/dp_display.c | 20 ++++++++++++++++++--
>>  1 file changed, 18 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
>> b/drivers/gpu/drm/msm/dp/dp_display.c
>> index 13b66266cd69..55627530957c 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -483,10 +485,24 @@ static int dp_display_usbpd_attention_cb(struct 
>> device *dev)
>>                 return -ENODEV;
>>         }
>> 
>> +       hpd = dp->usbpd;
>> +
>>         /* check for any test request issued by sink */
>>         rc = dp_link_process_request(dp->link);
>> -       if (!rc)
>> -               dp_display_handle_irq_hpd(dp);
>> +       if (!rc) {
>> +               sink_request = dp->link->sink_request;
>> +               if (sink_request & DS_PORT_STATUS_CHANGED) {
>> +                       dp->hpd_state = ST_CONNECT_PENDING;
>> +                       hpd->hpd_high = 1;
>> +               }
>> +
>> +               rc = dp_display_handle_irq_hpd(dp);
>> +
>> +               if (rc && sink_request & DS_PORT_STATUS_CHANGED) {
> 
> Can you add parenthesis around this?
> 
> 		if (rc && (sink_request & DS_PORT_STATUS_CHANGED)) {
> 
> 
> I honestly don't know what's going on in this patch. It talks about
> making link training happen during irq hpd handler but this is the
> attention handler and we're checking port status changed? This is
> related? The code is really not clear.
irq_hpd request is generated by sinker to ask host attention that 
something has changed.
POST_STATUS_CHNAGED bit set  by sinker to indicated link had loss of 
sync. Therefore
host need to restart link retaining to fix the link loss of sync 
problem.

> 
>> +                       hpd->hpd_high = 0;
>> +                       dp->hpd_state = ST_DISCONNECTED;
>> +               }
>> +       }
>> 
>>         return rc;
>>  }
>> 
>> base-commit: 0e162b10644605428cd2596c12f8ed410cf9d2d9
> 
> What commit is this?