| Message ID | 20230209184426.4437-2-quic_jesszhan@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Move TE setup to prepare_for_kickoff() | expand |
On 09/02/2023 20:44, 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 cb05036f2916..561406d92a1a 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 > @@ -583,39 +583,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) > { > @@ -641,8 +608,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); > @@ -700,6 +666,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)); > +} Quoting v1: Could you please move the function back to the place, so that we can see the actual difference? > + > +static void dpu_encoder_phys_cmd_prepare_commit( > + struct dpu_encoder_phys *phys_enc) > +{ > +} This is not necessary and can be dropped. There is a safety check in dpu_encoder_prepare_commit(). > + > static int _dpu_encoder_phys_cmd_wait_for_ctl_start( > struct dpu_encoder_phys *phys_enc) > {
On 2/9/2023 10:51 AM, Dmitry Baryshkov wrote: > On 09/02/2023 20:44, 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 cb05036f2916..561406d92a1a 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 >> @@ -583,39 +583,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) >> { >> @@ -641,8 +608,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); >> @@ -700,6 +666,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)); >> +} > > Quoting v1: > > Could you please move the function back to the place, so that we can see > the actual difference? Hi Dmitry, Sorry if I missed your reply to my reply in V1, but as stated in the V1 patch: the reason the diff looks like this is because prepare_for_kickoff() is defined above where the prepare_commit() and is_ongoing_pptx() were originally defined. So I had to move both function definitions to above the prepare_for_kickoff() function for the patch to compile. That being said, I'm open to any suggestions for making this patch more legible. > >> + >> +static void dpu_encoder_phys_cmd_prepare_commit( >> + struct dpu_encoder_phys *phys_enc) >> +{ >> +} > > This is not necessary and can be dropped. There is a safety check in > dpu_encoder_prepare_commit(). Acked. Thanks, Jessica Zhang > >> + >> static int _dpu_encoder_phys_cmd_wait_for_ctl_start( >> struct dpu_encoder_phys *phys_enc) >> { > > -- > With best wishes > Dmitry >
On Fri, 10 Feb 2023 at 00:31, Jessica Zhang <quic_jesszhan@quicinc.com> wrote: > > > > On 2/9/2023 10:51 AM, Dmitry Baryshkov wrote: > > On 09/02/2023 20:44, 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 cb05036f2916..561406d92a1a 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 > >> @@ -583,39 +583,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) > >> { > >> @@ -641,8 +608,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); > >> @@ -700,6 +666,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)); > >> +} > > > > Quoting v1: > > > > Could you please move the function back to the place, so that we can see > > the actual difference? > > Hi Dmitry, > > Sorry if I missed your reply to my reply in V1, but as stated in the V1 > patch: the reason the diff looks like this is because > prepare_for_kickoff() is defined above where the prepare_commit() and > is_ongoing_pptx() were originally defined. So I had to move both > function definitions to above the prepare_for_kickoff() function for the > patch to compile. You can add a function prototype in front of dpu_encoder_phys_cmd_prepare_for_kickoff(). > > That being said, I'm open to any suggestions for making this patch more > legible. > > > > >> + > >> +static void dpu_encoder_phys_cmd_prepare_commit( > >> + struct dpu_encoder_phys *phys_enc) > >> +{ > >> +} > > > > This is not necessary and can be dropped. There is a safety check in > > dpu_encoder_prepare_commit(). > > Acked. > > Thanks, > > Jessica Zhang > > > > >> + > >> static int _dpu_encoder_phys_cmd_wait_for_ctl_start( > >> struct dpu_encoder_phys *phys_enc) > >> { > > > > -- > > With best wishes > > Dmitry > >
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 cb05036f2916..561406d92a1a 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 @@ -583,39 +583,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) { @@ -641,8 +608,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); @@ -700,6 +666,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(-)