| Message ID | 1650419169-13760-17-git-send-email-quic_abhinavk@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add writeback block support for DPU | expand |
On 20/04/2022 04:46, Abhinav Kumar wrote: > kms_writeback test cases also verify with a null fb for the > writeback connector job. In addition there are also other > commit paths which can result in kickoffs without a valid > framebuffer like while closing the fb which results in the > callback to drm_atomic_helper_dirtyfb() which internally > triggers a commit. > > Add protection in the dpu driver to ensure that commits for > writeback encoders without a valid fb are gracefully skipped. > > changes in v2: > - rename dpu_encoder_has_valid_fb to dpu_encoder_is_valid_for_commit > > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 +++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 21 +++++++++++++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 1 + > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 12 ++++++++++++ > 5 files changed, 49 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index 7763558..d65e124 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -869,6 +869,13 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) > > DPU_ATRACE_BEGIN("crtc_commit"); > > + drm_for_each_encoder_mask(encoder, crtc->dev, > + crtc->state->encoder_mask) { > + if (!dpu_encoder_is_valid_for_commit(encoder)) { > + DRM_DEBUG_ATOMIC("invalid FB not kicking off crtc\n"); > + goto end; > + } > + } > /* > * Encoder will flush/start now, unless it has a tx pending. If so, it > * may delay and flush at an irq event (e.g. ppdone) > @@ -891,6 +898,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) > dpu_encoder_kickoff(encoder); > > reinit_completion(&dpu_crtc->frame_done_comp); > + > +end: > DPU_ATRACE_END("crtc_commit"); > } > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index b1475dd..d07e3ee 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -1850,6 +1850,27 @@ void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc) > dpu_encoder_prep_dsc(dpu_enc, dpu_enc->dsc); > } > > +bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc) > +{ > + struct dpu_encoder_virt *dpu_enc; > + unsigned int i; > + struct dpu_encoder_phys *phys; > + > + dpu_enc = to_dpu_encoder_virt(drm_enc); > + > + if (drm_enc->encoder_type == DRM_MODE_ENCODER_VIRTUAL) { > + for (i = 0; i < dpu_enc->num_phys_encs; i++) { > + phys = dpu_enc->phys_encs[i]; > + if (phys->ops.is_valid_for_commit && !phys->ops.is_valid_for_commit(phys)) { > + DPU_DEBUG("invalid FB not kicking off\n"); > + return false; > + } > + } > + } > + > + return true; > +} > + > void dpu_encoder_kickoff(struct drm_encoder *drm_enc) > { > struct dpu_encoder_virt *dpu_enc; > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > index 6ceec1d..781d41c 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > @@ -196,4 +196,10 @@ void dpu_encoder_prepare_wb_job(struct drm_encoder *drm_enc, > void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc, > struct drm_writeback_job *job); > > +/** > + * dpu_encoder_is_valid_for_commit - check if encode has valid parameters for commit. > + * @drm_enc: Pointer to drm encoder structure > + */ > +bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc); > + > #endif /* __DPU_ENCODER_H__ */ > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > index 5452f98..04d037e 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > @@ -142,6 +142,7 @@ struct dpu_encoder_phys_ops { > struct drm_writeback_job *job); > void (*cleanup_wb_job)(struct dpu_encoder_phys *phys_enc, > struct drm_writeback_job *job); > + bool (*is_valid_for_commit)(struct dpu_encoder_phys *phys_enc); > }; > > /** > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > index 128317fe..9acbce0 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > @@ -667,6 +667,16 @@ static void dpu_encoder_phys_wb_cleanup_wb_job(struct dpu_encoder_phys *phys_enc > wb_enc->wb_conn = NULL; > } > > +static bool dpu_encoder_phys_wb_is_valid_for_commit(struct dpu_encoder_phys *phys_enc) > +{ > + struct dpu_encoder_phys_wb *wb_enc = to_dpu_encoder_phys_wb(phys_enc); > + > + if (wb_enc->wb_job) > + return true; > + else > + return false; > +} > + > /** > * dpu_encoder_phys_wb_init_ops - initialize writeback operations > * @ops: Pointer to encoder operation table > @@ -687,6 +697,8 @@ static void dpu_encoder_phys_wb_init_ops(struct dpu_encoder_phys_ops *ops) > ops->prepare_wb_job = dpu_encoder_phys_wb_prepare_wb_job; > ops->cleanup_wb_job = dpu_encoder_phys_wb_cleanup_wb_job; > ops->irq_control = dpu_encoder_phys_wb_irq_ctrl; > + ops->is_valid_for_commit = dpu_encoder_phys_wb_is_valid_for_commit; > + > } > > /**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 7763558..d65e124 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -869,6 +869,13 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) DPU_ATRACE_BEGIN("crtc_commit"); + drm_for_each_encoder_mask(encoder, crtc->dev, + crtc->state->encoder_mask) { + if (!dpu_encoder_is_valid_for_commit(encoder)) { + DRM_DEBUG_ATOMIC("invalid FB not kicking off crtc\n"); + goto end; + } + } /* * Encoder will flush/start now, unless it has a tx pending. If so, it * may delay and flush at an irq event (e.g. ppdone) @@ -891,6 +898,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) dpu_encoder_kickoff(encoder); reinit_completion(&dpu_crtc->frame_done_comp); + +end: DPU_ATRACE_END("crtc_commit"); } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index b1475dd..d07e3ee 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1850,6 +1850,27 @@ void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc) dpu_encoder_prep_dsc(dpu_enc, dpu_enc->dsc); } +bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc) +{ + struct dpu_encoder_virt *dpu_enc; + unsigned int i; + struct dpu_encoder_phys *phys; + + dpu_enc = to_dpu_encoder_virt(drm_enc); + + if (drm_enc->encoder_type == DRM_MODE_ENCODER_VIRTUAL) { + for (i = 0; i < dpu_enc->num_phys_encs; i++) { + phys = dpu_enc->phys_encs[i]; + if (phys->ops.is_valid_for_commit && !phys->ops.is_valid_for_commit(phys)) { + DPU_DEBUG("invalid FB not kicking off\n"); + return false; + } + } + } + + return true; +} + void dpu_encoder_kickoff(struct drm_encoder *drm_enc) { struct dpu_encoder_virt *dpu_enc; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h index 6ceec1d..781d41c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h @@ -196,4 +196,10 @@ void dpu_encoder_prepare_wb_job(struct drm_encoder *drm_enc, void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc, struct drm_writeback_job *job); +/** + * dpu_encoder_is_valid_for_commit - check if encode has valid parameters for commit. + * @drm_enc: Pointer to drm encoder structure + */ +bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc); + #endif /* __DPU_ENCODER_H__ */ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h index 5452f98..04d037e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -142,6 +142,7 @@ struct dpu_encoder_phys_ops { struct drm_writeback_job *job); void (*cleanup_wb_job)(struct dpu_encoder_phys *phys_enc, struct drm_writeback_job *job); + bool (*is_valid_for_commit)(struct dpu_encoder_phys *phys_enc); }; /** diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c index 128317fe..9acbce0 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c @@ -667,6 +667,16 @@ static void dpu_encoder_phys_wb_cleanup_wb_job(struct dpu_encoder_phys *phys_enc wb_enc->wb_conn = NULL; } +static bool dpu_encoder_phys_wb_is_valid_for_commit(struct dpu_encoder_phys *phys_enc) +{ + struct dpu_encoder_phys_wb *wb_enc = to_dpu_encoder_phys_wb(phys_enc); + + if (wb_enc->wb_job) + return true; + else + return false; +} + /** * dpu_encoder_phys_wb_init_ops - initialize writeback operations * @ops: Pointer to encoder operation table @@ -687,6 +697,8 @@ static void dpu_encoder_phys_wb_init_ops(struct dpu_encoder_phys_ops *ops) ops->prepare_wb_job = dpu_encoder_phys_wb_prepare_wb_job; ops->cleanup_wb_job = dpu_encoder_phys_wb_cleanup_wb_job; ops->irq_control = dpu_encoder_phys_wb_irq_ctrl; + ops->is_valid_for_commit = dpu_encoder_phys_wb_is_valid_for_commit; + } /**
kms_writeback test cases also verify with a null fb for the writeback connector job. In addition there are also other commit paths which can result in kickoffs without a valid framebuffer like while closing the fb which results in the callback to drm_atomic_helper_dirtyfb() which internally triggers a commit. Add protection in the dpu driver to ensure that commits for writeback encoders without a valid fb are gracefully skipped. changes in v2: - rename dpu_encoder_has_valid_fb to dpu_encoder_is_valid_for_commit Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 +++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 21 +++++++++++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 12 ++++++++++++ 5 files changed, 49 insertions(+)