Messages in this thread | | | Date | Mon, 24 May 2021 12:57:17 -0700 | From | khsieh@codeauro ... | Subject | Re: [PATCH 3/3] drm/msm/dp: Handle aux timeouts, nacks, defers |
| |
On 2021-05-24 12:19, Stephen Boyd wrote: > Quoting khsieh@codeaurora.org (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.
| |