| Message ID | 20230208213713.1330-1-quic_jesszhan@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [RFC] drm/msm/dpu: Move TE setup to prepare_for_kickoff() | expand |
On 08/02/2023 23:37, Jessica Zhang wrote: > Currently, DPU will enable TE during prepare_commit(). However, this > will cause issues when trying to read/write to register in > get_autorefresh_config(), because the core clock rates aren't set at > that time. > > This used to work because phys_enc->hw_pp is only initialized in mode > set [1], so the first prepare_commit() will return before any register > read/write as hw_pp would be NULL. > > However, when we try to implement support for INTF TE, we will run into > the clock issue described above as hw_intf will *not* be NULL on the > first prepare_commit(). This is because the initialization of > dpu_enc->hw_intf has been moved to dpu_encoder_setup() [2]. > > To avoid this issue, let's enable TE during prepare_for_kickoff() > instead as the core clock rates are guaranteed to be set then. > > Depends on: "Implement tearcheck support on INTF block" [3] > > [1] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L1109 > [2] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2339 > [3] https://patchwork.freedesktop.org/series/112332/ > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 78 ++++++++++--------- > 1 file changed, 43 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c > index 279a0b7015ce..746250bce3d1 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c > @@ -587,39 +587,6 @@ static void dpu_encoder_phys_cmd_destroy(struct dpu_encoder_phys *phys_enc) > kfree(cmd_enc); > } > > -static void dpu_encoder_phys_cmd_prepare_for_kickoff( > - struct dpu_encoder_phys *phys_enc) > -{ > - struct dpu_encoder_phys_cmd *cmd_enc = > - to_dpu_encoder_phys_cmd(phys_enc); > - int ret; > - > - if (!phys_enc->hw_pp) { > - DPU_ERROR("invalid encoder\n"); > - return; > - } > - DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent), > - phys_enc->hw_pp->idx - PINGPONG_0, > - atomic_read(&phys_enc->pending_kickoff_cnt)); > - > - /* > - * Mark kickoff request as outstanding. If there are more than one, > - * outstanding, then we have to wait for the previous one to complete > - */ > - ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); > - if (ret) { > - /* force pending_kickoff_cnt 0 to discard failed kickoff */ > - atomic_set(&phys_enc->pending_kickoff_cnt, 0); > - DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", > - DRMID(phys_enc->parent), ret, > - phys_enc->hw_pp->idx - PINGPONG_0); > - } > - > - DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n", > - phys_enc->hw_pp->idx - PINGPONG_0, > - atomic_read(&phys_enc->pending_kickoff_cnt)); > -} > - > static bool dpu_encoder_phys_cmd_is_ongoing_pptx( > struct dpu_encoder_phys *phys_enc) > { > @@ -645,8 +612,7 @@ static bool dpu_encoder_phys_cmd_is_ongoing_pptx( > return false; > } > > -static void dpu_encoder_phys_cmd_prepare_commit( > - struct dpu_encoder_phys *phys_enc) > +static void dpu_encoder_phys_cmd_enable_te(struct dpu_encoder_phys *phys_enc) > { > struct dpu_encoder_phys_cmd *cmd_enc = > to_dpu_encoder_phys_cmd(phys_enc); > @@ -704,6 +670,48 @@ static void dpu_encoder_phys_cmd_prepare_commit( > "disabled autorefresh\n"); > } > > +static void dpu_encoder_phys_cmd_prepare_for_kickoff( > + struct dpu_encoder_phys *phys_enc) Could you please move the function back to the place, so that we can see the actual difference? > +{ > + struct dpu_encoder_phys_cmd *cmd_enc = > + to_dpu_encoder_phys_cmd(phys_enc); > + int ret; > + > + if (!phys_enc->hw_pp) { > + DPU_ERROR("invalid encoder\n"); > + return; > + } > + > + > + DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent), > + phys_enc->hw_pp->idx - PINGPONG_0, > + atomic_read(&phys_enc->pending_kickoff_cnt)); > + > + /* > + * Mark kickoff request as outstanding. If there are more than one, > + * outstanding, then we have to wait for the previous one to complete > + */ > + ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); > + if (ret) { > + /* force pending_kickoff_cnt 0 to discard failed kickoff */ > + atomic_set(&phys_enc->pending_kickoff_cnt, 0); > + DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", > + DRMID(phys_enc->parent), ret, > + phys_enc->hw_pp->idx - PINGPONG_0); > + } > + > + dpu_encoder_phys_cmd_enable_te(phys_enc); > + > + DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n", > + phys_enc->hw_pp->idx - PINGPONG_0, > + atomic_read(&phys_enc->pending_kickoff_cnt)); > +} > + > +static void dpu_encoder_phys_cmd_prepare_commit( > + struct dpu_encoder_phys *phys_enc) > +{ > +} There is no need to have the empty callback, you can skip it completely. Actually, if it is not needed anymore for the cmd encoders, you can drop the .prepare_commit from struct dpu_encoder_phys_ops. And then, by extension, dpu_encoder_prepare_commit(), dpu_kms_prepare_commit(). This sounds like a nice second patch for this rfc. > + > static int _dpu_encoder_phys_cmd_wait_for_ctl_start( > struct dpu_encoder_phys *phys_enc) > {
On 2/8/2023 2:18 PM, Dmitry Baryshkov wrote: > On 08/02/2023 23:37, Jessica Zhang wrote: >> Currently, DPU will enable TE during prepare_commit(). However, this >> will cause issues when trying to read/write to register in >> get_autorefresh_config(), because the core clock rates aren't set at >> that time. >> >> This used to work because phys_enc->hw_pp is only initialized in mode >> set [1], so the first prepare_commit() will return before any register >> read/write as hw_pp would be NULL. >> >> However, when we try to implement support for INTF TE, we will run into >> the clock issue described above as hw_intf will *not* be NULL on the >> first prepare_commit(). This is because the initialization of >> dpu_enc->hw_intf has been moved to dpu_encoder_setup() [2]. >> >> To avoid this issue, let's enable TE during prepare_for_kickoff() >> instead as the core clock rates are guaranteed to be set then. >> >> Depends on: "Implement tearcheck support on INTF block" [3] >> >> [1] >> https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L1109 >> [2] >> https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2339 >> [3] https://patchwork.freedesktop.org/series/112332/ >> >> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >> --- >> .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 78 ++++++++++--------- >> 1 file changed, 43 insertions(+), 35 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c >> index 279a0b7015ce..746250bce3d1 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c >> @@ -587,39 +587,6 @@ static void dpu_encoder_phys_cmd_destroy(struct >> dpu_encoder_phys *phys_enc) >> kfree(cmd_enc); >> } >> -static void dpu_encoder_phys_cmd_prepare_for_kickoff( >> - struct dpu_encoder_phys *phys_enc) >> -{ >> - struct dpu_encoder_phys_cmd *cmd_enc = >> - to_dpu_encoder_phys_cmd(phys_enc); >> - int ret; >> - >> - if (!phys_enc->hw_pp) { >> - DPU_ERROR("invalid encoder\n"); >> - return; >> - } >> - DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", >> DRMID(phys_enc->parent), >> - phys_enc->hw_pp->idx - PINGPONG_0, >> - atomic_read(&phys_enc->pending_kickoff_cnt)); >> - >> - /* >> - * Mark kickoff request as outstanding. If there are more than one, >> - * outstanding, then we have to wait for the previous one to >> complete >> - */ >> - ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); >> - if (ret) { >> - /* force pending_kickoff_cnt 0 to discard failed kickoff */ >> - atomic_set(&phys_enc->pending_kickoff_cnt, 0); >> - DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", >> - DRMID(phys_enc->parent), ret, >> - phys_enc->hw_pp->idx - PINGPONG_0); >> - } >> - >> - DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n", >> - phys_enc->hw_pp->idx - PINGPONG_0, >> - atomic_read(&phys_enc->pending_kickoff_cnt)); >> -} >> - >> static bool dpu_encoder_phys_cmd_is_ongoing_pptx( >> struct dpu_encoder_phys *phys_enc) >> { >> @@ -645,8 +612,7 @@ static bool dpu_encoder_phys_cmd_is_ongoing_pptx( >> return false; >> } >> -static void dpu_encoder_phys_cmd_prepare_commit( >> - struct dpu_encoder_phys *phys_enc) >> +static void dpu_encoder_phys_cmd_enable_te(struct dpu_encoder_phys >> *phys_enc) >> { >> struct dpu_encoder_phys_cmd *cmd_enc = >> to_dpu_encoder_phys_cmd(phys_enc); >> @@ -704,6 +670,48 @@ static void dpu_encoder_phys_cmd_prepare_commit( >> "disabled autorefresh\n"); >> } >> +static void dpu_encoder_phys_cmd_prepare_for_kickoff( >> + struct dpu_encoder_phys *phys_enc) > > Could you please move the function back to the place, so that we can see > the actual difference? Hi Dmitry, This function was moved because prepare_commit() and is_ongoing_pptx() (which is called in prepare_commit()) were originally defined later in the file. > >> +{ >> + struct dpu_encoder_phys_cmd *cmd_enc = >> + to_dpu_encoder_phys_cmd(phys_enc); >> + int ret; >> + >> + if (!phys_enc->hw_pp) { >> + DPU_ERROR("invalid encoder\n"); >> + return; >> + } >> + >> + >> + DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", >> DRMID(phys_enc->parent), >> + phys_enc->hw_pp->idx - PINGPONG_0, >> + atomic_read(&phys_enc->pending_kickoff_cnt)); >> + >> + /* >> + * Mark kickoff request as outstanding. If there are more than one, >> + * outstanding, then we have to wait for the previous one to >> complete >> + */ >> + ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); >> + if (ret) { >> + /* force pending_kickoff_cnt 0 to discard failed kickoff */ >> + atomic_set(&phys_enc->pending_kickoff_cnt, 0); >> + DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", >> + DRMID(phys_enc->parent), ret, >> + phys_enc->hw_pp->idx - PINGPONG_0); >> + } >> + >> + dpu_encoder_phys_cmd_enable_te(phys_enc); >> + >> + DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n", >> + phys_enc->hw_pp->idx - PINGPONG_0, >> + atomic_read(&phys_enc->pending_kickoff_cnt)); >> +} >> + >> +static void dpu_encoder_phys_cmd_prepare_commit( >> + struct dpu_encoder_phys *phys_enc) >> +{ >> +} > > There is no need to have the empty callback, you can skip it completely. > Actually, if it is not needed anymore for the cmd encoders, you can drop > the .prepare_commit from struct dpu_encoder_phys_ops. And then, by > extension, dpu_encoder_prepare_commit(), dpu_kms_prepare_commit(). This > sounds like a nice second patch for this rfc. Got it. FWIW I kept this as an empty method to match mdp4_prepare_commit() [1], but I can just add a NULL check in msm_atomic_commit_tail() and remove both instances of empty callbacks if that's preferable. [1] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c#L87 Thanks, Jessica Zhang > >> + >> static int _dpu_encoder_phys_cmd_wait_for_ctl_start( >> struct dpu_encoder_phys *phys_enc) >> { > > -- > With best wishes > Dmitry >
On 09/02/2023 01:24, Jessica Zhang wrote: > > > On 2/8/2023 2:18 PM, Dmitry Baryshkov wrote: >> On 08/02/2023 23:37, Jessica Zhang wrote: >>> Currently, DPU will enable TE during prepare_commit(). However, this >>> will cause issues when trying to read/write to register in >>> get_autorefresh_config(), because the core clock rates aren't set at >>> that time. >>> >>> This used to work because phys_enc->hw_pp is only initialized in mode >>> set [1], so the first prepare_commit() will return before any register >>> read/write as hw_pp would be NULL. >>> >>> However, when we try to implement support for INTF TE, we will run into >>> the clock issue described above as hw_intf will *not* be NULL on the >>> first prepare_commit(). This is because the initialization of >>> dpu_enc->hw_intf has been moved to dpu_encoder_setup() [2]. >>> >>> To avoid this issue, let's enable TE during prepare_for_kickoff() >>> instead as the core clock rates are guaranteed to be set then. >>> >>> Depends on: "Implement tearcheck support on INTF block" [3] >>> >>> [1] >>> https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L1109 >>> [2] >>> https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2339 >>> [3] https://patchwork.freedesktop.org/series/112332/ >>> >>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >>> --- >>> .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 78 ++++++++++--------- >>> 1 file changed, 43 insertions(+), 35 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c >>> index 279a0b7015ce..746250bce3d1 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c >>> @@ -587,39 +587,6 @@ static void dpu_encoder_phys_cmd_destroy(struct >>> dpu_encoder_phys *phys_enc) >>> kfree(cmd_enc); >>> } >>> -static void dpu_encoder_phys_cmd_prepare_for_kickoff( >>> - struct dpu_encoder_phys *phys_enc) >>> -{ >>> - struct dpu_encoder_phys_cmd *cmd_enc = >>> - to_dpu_encoder_phys_cmd(phys_enc); >>> - int ret; >>> - >>> - if (!phys_enc->hw_pp) { >>> - DPU_ERROR("invalid encoder\n"); >>> - return; >>> - } >>> - DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", >>> DRMID(phys_enc->parent), >>> - phys_enc->hw_pp->idx - PINGPONG_0, >>> - atomic_read(&phys_enc->pending_kickoff_cnt)); >>> - >>> - /* >>> - * Mark kickoff request as outstanding. If there are more than one, >>> - * outstanding, then we have to wait for the previous one to >>> complete >>> - */ >>> - ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); >>> - if (ret) { >>> - /* force pending_kickoff_cnt 0 to discard failed kickoff */ >>> - atomic_set(&phys_enc->pending_kickoff_cnt, 0); >>> - DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", >>> - DRMID(phys_enc->parent), ret, >>> - phys_enc->hw_pp->idx - PINGPONG_0); >>> - } >>> - >>> - DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n", >>> - phys_enc->hw_pp->idx - PINGPONG_0, >>> - atomic_read(&phys_enc->pending_kickoff_cnt)); >>> -} >>> - >>> static bool dpu_encoder_phys_cmd_is_ongoing_pptx( >>> struct dpu_encoder_phys *phys_enc) >>> { >>> @@ -645,8 +612,7 @@ static bool dpu_encoder_phys_cmd_is_ongoing_pptx( >>> return false; >>> } >>> -static void dpu_encoder_phys_cmd_prepare_commit( >>> - struct dpu_encoder_phys *phys_enc) >>> +static void dpu_encoder_phys_cmd_enable_te(struct dpu_encoder_phys >>> *phys_enc) >>> { >>> struct dpu_encoder_phys_cmd *cmd_enc = >>> to_dpu_encoder_phys_cmd(phys_enc); >>> @@ -704,6 +670,48 @@ static void dpu_encoder_phys_cmd_prepare_commit( >>> "disabled autorefresh\n"); >>> } >>> +static void dpu_encoder_phys_cmd_prepare_for_kickoff( >>> + struct dpu_encoder_phys *phys_enc) >> >> Could you please move the function back to the place, so that we can >> see the actual difference? > > Hi Dmitry, > > This function was moved because prepare_commit() and is_ongoing_pptx() > (which is called in prepare_commit()) were originally defined later in > the file. > >> >>> +{ >>> + struct dpu_encoder_phys_cmd *cmd_enc = >>> + to_dpu_encoder_phys_cmd(phys_enc); >>> + int ret; >>> + >>> + if (!phys_enc->hw_pp) { >>> + DPU_ERROR("invalid encoder\n"); >>> + return; >>> + } >>> + >>> + >>> + DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", >>> DRMID(phys_enc->parent), >>> + phys_enc->hw_pp->idx - PINGPONG_0, >>> + atomic_read(&phys_enc->pending_kickoff_cnt)); >>> + >>> + /* >>> + * Mark kickoff request as outstanding. If there are more than one, >>> + * outstanding, then we have to wait for the previous one to >>> complete >>> + */ >>> + ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); >>> + if (ret) { >>> + /* force pending_kickoff_cnt 0 to discard failed kickoff */ >>> + atomic_set(&phys_enc->pending_kickoff_cnt, 0); >>> + DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", >>> + DRMID(phys_enc->parent), ret, >>> + phys_enc->hw_pp->idx - PINGPONG_0); >>> + } >>> + >>> + dpu_encoder_phys_cmd_enable_te(phys_enc); >>> + >>> + DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n", >>> + phys_enc->hw_pp->idx - PINGPONG_0, >>> + atomic_read(&phys_enc->pending_kickoff_cnt)); >>> +} >>> + >>> +static void dpu_encoder_phys_cmd_prepare_commit( >>> + struct dpu_encoder_phys *phys_enc) >>> +{ >>> +} >> >> There is no need to have the empty callback, you can skip it >> completely. Actually, if it is not needed anymore for the cmd >> encoders, you can drop the .prepare_commit from struct >> dpu_encoder_phys_ops. And then, by extension, >> dpu_encoder_prepare_commit(), dpu_kms_prepare_commit(). This sounds >> like a nice second patch for this rfc. > > Got it. > > FWIW I kept this as an empty method to match mdp4_prepare_commit() [1], > but I can just add a NULL check in msm_atomic_commit_tail() and remove > both instances of empty callbacks if that's preferable. yes, please. > > [1] > https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c#L87
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index 279a0b7015ce..746250bce3d1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -587,39 +587,6 @@ static void dpu_encoder_phys_cmd_destroy(struct dpu_encoder_phys *phys_enc) kfree(cmd_enc); } -static void dpu_encoder_phys_cmd_prepare_for_kickoff( - struct dpu_encoder_phys *phys_enc) -{ - struct dpu_encoder_phys_cmd *cmd_enc = - to_dpu_encoder_phys_cmd(phys_enc); - int ret; - - if (!phys_enc->hw_pp) { - DPU_ERROR("invalid encoder\n"); - return; - } - DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent), - phys_enc->hw_pp->idx - PINGPONG_0, - atomic_read(&phys_enc->pending_kickoff_cnt)); - - /* - * Mark kickoff request as outstanding. If there are more than one, - * outstanding, then we have to wait for the previous one to complete - */ - ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); - if (ret) { - /* force pending_kickoff_cnt 0 to discard failed kickoff */ - atomic_set(&phys_enc->pending_kickoff_cnt, 0); - DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", - DRMID(phys_enc->parent), ret, - phys_enc->hw_pp->idx - PINGPONG_0); - } - - DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n", - phys_enc->hw_pp->idx - PINGPONG_0, - atomic_read(&phys_enc->pending_kickoff_cnt)); -} - static bool dpu_encoder_phys_cmd_is_ongoing_pptx( struct dpu_encoder_phys *phys_enc) { @@ -645,8 +612,7 @@ static bool dpu_encoder_phys_cmd_is_ongoing_pptx( return false; } -static void dpu_encoder_phys_cmd_prepare_commit( - struct dpu_encoder_phys *phys_enc) +static void dpu_encoder_phys_cmd_enable_te(struct dpu_encoder_phys *phys_enc) { struct dpu_encoder_phys_cmd *cmd_enc = to_dpu_encoder_phys_cmd(phys_enc); @@ -704,6 +670,48 @@ static void dpu_encoder_phys_cmd_prepare_commit( "disabled autorefresh\n"); } +static void dpu_encoder_phys_cmd_prepare_for_kickoff( + struct dpu_encoder_phys *phys_enc) +{ + struct dpu_encoder_phys_cmd *cmd_enc = + to_dpu_encoder_phys_cmd(phys_enc); + int ret; + + if (!phys_enc->hw_pp) { + DPU_ERROR("invalid encoder\n"); + return; + } + + + DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n", DRMID(phys_enc->parent), + phys_enc->hw_pp->idx - PINGPONG_0, + atomic_read(&phys_enc->pending_kickoff_cnt)); + + /* + * Mark kickoff request as outstanding. If there are more than one, + * outstanding, then we have to wait for the previous one to complete + */ + ret = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc); + if (ret) { + /* force pending_kickoff_cnt 0 to discard failed kickoff */ + atomic_set(&phys_enc->pending_kickoff_cnt, 0); + DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n", + DRMID(phys_enc->parent), ret, + phys_enc->hw_pp->idx - PINGPONG_0); + } + + dpu_encoder_phys_cmd_enable_te(phys_enc); + + DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n", + phys_enc->hw_pp->idx - PINGPONG_0, + atomic_read(&phys_enc->pending_kickoff_cnt)); +} + +static void dpu_encoder_phys_cmd_prepare_commit( + struct dpu_encoder_phys *phys_enc) +{ +} + static int _dpu_encoder_phys_cmd_wait_for_ctl_start( struct dpu_encoder_phys *phys_enc) {
Currently, DPU will enable TE during prepare_commit(). However, this will cause issues when trying to read/write to register in get_autorefresh_config(), because the core clock rates aren't set at that time. This used to work because phys_enc->hw_pp is only initialized in mode set [1], so the first prepare_commit() will return before any register read/write as hw_pp would be NULL. However, when we try to implement support for INTF TE, we will run into the clock issue described above as hw_intf will *not* be NULL on the first prepare_commit(). This is because the initialization of dpu_enc->hw_intf has been moved to dpu_encoder_setup() [2]. To avoid this issue, let's enable TE during prepare_for_kickoff() instead as the core clock rates are guaranteed to be set then. Depends on: "Implement tearcheck support on INTF block" [3] [1] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L1109 [2] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2339 [3] https://patchwork.freedesktop.org/series/112332/ Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> --- .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 78 ++++++++++--------- 1 file changed, 43 insertions(+), 35 deletions(-)