Re: [PATCH 3/3] drm/msm/dp: Handle aux timeouts, nacks, defers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2021-05-24 12:19, Stephen Boyd wrote:
Quoting khsieh@xxxxxxxxxxxxxx (2021-05-24 09:33:49)
On 2021-05-07 14:25, Stephen Boyd wrote:
> @@ -367,36 +347,38 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux
> *dp_aux,
>       }
>
>       ret = dp_aux_cmd_fifo_tx(aux, msg);
> -
>       if (ret < 0) {
>               if (aux->native) {
>                       aux->retry_cnt++;
>                       if (!(aux->retry_cnt % MAX_AUX_RETRIES))
>                               dp_catalog_aux_update_cfg(aux->catalog);
>               }
> -             usleep_range(400, 500); /* at least 400us to next try */
> -             goto unlock_exit;
> -     }

1) dp_catalog_aux_update_cfg(aux->catalog) will not work without
dp_catalog_aux_reset(aux->catalog);
dp_catalog_aux_reset(aux->catalog) will reset hpd control block and
potentially cause pending hpd interrupts got lost.
Therefore I think we should not do
dp_catalog_aux_update_cfg(aux->catalog) for now.
reset aux controller will reset hpd control block probolem will be fixed
at next chipset.
after that we can add dp_catalog_aux_update_cfg(aux->catalog) followed
by dp_catalog_aux_reset(aux->catalog) back at next chipset.

Hmm ok. So the phy calibration logic that tweaks the tuning values is
never used? Why can't the phy be tuned while it is active? I don't
understand why we would ever want to reset the aux phy when changing the
settings for a retry. Either way, this is not actually changing in this
patch so it would be another patch to remove this code.
ok,


2) according to DP specification, aux read/write failed have to wait at
least 400us before next try can start.
Otherwise, DP compliant test will failed

Yes. The caller of this function, drm_dp_dpcd_access(), has the delay
already

                if (ret != 0 && ret != -ETIMEDOUT) {
                        usleep_range(AUX_RETRY_INTERVAL,
                                     AUX_RETRY_INTERVAL + 100);
                }

so this delay here is redundant.
yes, you are right. This is enough.




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux