| Message ID | 20230213194819.608-2-quic_jesszhan@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Move TE setup to prepare_for_kickoff() | expand |
On 13/02/2023 21:48, Jessica Zhang wrote: > Currently, DPU will enable TE during prepare_commit(). However, this > will cause issues when trying to read/write to register in Nit: what issues? SError? reboot to the sahara? board reset? > 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] > > Changes in V3: > - Added function prototypes > - Reordered function definitions to make change more legible > - Removed prepare_commit() function from dpu_encoder_phys_cmd > > [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> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 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..c6feffafa13f 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 > @@ -40,6 +40,10 @@ > > #define DPU_ENC_MAX_POLL_TIMEOUT_US 2000 > > +static bool dpu_encoder_phys_cmd_is_ongoing_pptx( > + struct dpu_encoder_phys *phys_enc); This should not be necessary. > +static void dpu_encoder_phys_cmd_enable_te(struct dpu_encoder_phys *phys_enc); > + > static bool dpu_encoder_phys_cmd_is_master(struct dpu_encoder_phys *phys_enc) > { > return (phys_enc->split_role != ENC_ROLE_SLAVE); > @@ -611,6 +615,8 @@ static void dpu_encoder_phys_cmd_prepare_for_kickoff( > phys_enc->hw_pp->idx - PINGPONG_0); > } > > + dpu_encoder_phys_cmd_enable_te(phys_enc); > + And this is much cleaner and easier to spot the difference than it was in the previous patch. Thank you! With the dpu_encoder_phys_cmd_is_ongoing_pptx() prototype removed it LGTM. > 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)); > @@ -641,8 +647,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); > @@ -799,7 +804,6 @@ static void dpu_encoder_phys_cmd_trigger_start( > static void dpu_encoder_phys_cmd_init_ops( > struct dpu_encoder_phys_ops *ops) > { > - ops->prepare_commit = dpu_encoder_phys_cmd_prepare_commit; > ops->is_master = dpu_encoder_phys_cmd_is_master; > ops->atomic_mode_set = dpu_encoder_phys_cmd_atomic_mode_set; > ops->enable = dpu_encoder_phys_cmd_enable;
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..c6feffafa13f 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 @@ -40,6 +40,10 @@ #define DPU_ENC_MAX_POLL_TIMEOUT_US 2000 +static bool dpu_encoder_phys_cmd_is_ongoing_pptx( + struct dpu_encoder_phys *phys_enc); +static void dpu_encoder_phys_cmd_enable_te(struct dpu_encoder_phys *phys_enc); + static bool dpu_encoder_phys_cmd_is_master(struct dpu_encoder_phys *phys_enc) { return (phys_enc->split_role != ENC_ROLE_SLAVE); @@ -611,6 +615,8 @@ static void dpu_encoder_phys_cmd_prepare_for_kickoff( 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)); @@ -641,8 +647,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); @@ -799,7 +804,6 @@ static void dpu_encoder_phys_cmd_trigger_start( static void dpu_encoder_phys_cmd_init_ops( struct dpu_encoder_phys_ops *ops) { - ops->prepare_commit = dpu_encoder_phys_cmd_prepare_commit; ops->is_master = dpu_encoder_phys_cmd_is_master; ops->atomic_mode_set = dpu_encoder_phys_cmd_atomic_mode_set; ops->enable = dpu_encoder_phys_cmd_enable;
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] Changes in V3: - Added function prototypes - Reordered function definitions to make change more legible - Removed prepare_commit() function from dpu_encoder_phys_cmd [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> --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)