| Message ID | 20240829-concurrent-wb-v1-20-502b16ae2ebb@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/msm/dpu: Add Concurrent Writeback Support for DPU 10.x+ | expand |
On Thu, Aug 29, 2024 at 01:48:41PM GMT, Jessica Zhang wrote: > Add a helper that will handle the correct order of the encoder kickoffs > for concurrent writeback. > > For concurrent writeback, the realtime encoder must always kickoff last > as it will call the trigger flush and start. > > This avoids the following scenario where the writeback encoder > increments the pending kickoff count after the WB_DONE interrupt is > fired: > > If the realtime encoder is kicked off first, the encoder kickoff will > flush/start the encoder and increment the pending kickoff count. The > WB_DONE interrupt then fires (before the writeback encoder is kicked > off). When the writeback encoder enters its kickoff, it will skip the > flush/start (due to CWB being enabled) and hit a frame done timeout > as the frame was kicked off (and the WB_DONE interrupt fired) without > the pending kickoff count being incremented. > > In addition, the writeback timer should only start after the realtime > encoder is kicked off to ensure that we don't get timeouts when the > system has a heavy load (ex. when debug logs are enabled) > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 67 ++++++++++++++++++++++++++------ > 1 file changed, 55 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index daf5f751f584..d2f91e89eba7 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -928,6 +928,42 @@ static int _dpu_crtc_wait_for_frame_done(struct drm_crtc *crtc) > return rc; > } > > +static int dpu_crtc_kickoff_clone_mode(struct drm_crtc *crtc) > +{ > + struct drm_encoder *encoder; > + struct drm_encoder *rt_encoder = NULL, *wb_encoder; > + > + /* Find encoder for real time display */ > + drm_for_each_encoder_mask(encoder, crtc->dev, > + crtc->state->encoder_mask) { > + if (encoder->encoder_type == DRM_MODE_ENCODER_VIRTUAL) > + wb_encoder = encoder; > + else > + rt_encoder = encoder; > + } > + > + if (!rt_encoder || !wb_encoder) { > + DRM_DEBUG_ATOMIC("real time or wb encoder not found\n"); > + return -EINVAL; > + } > + > + dpu_encoder_prepare_for_kickoff(wb_encoder); > + dpu_encoder_prepare_for_kickoff(rt_encoder); > + > + /* > + * Kickoff real time encoder last as it's the encoder that > + * will do the flush > + */ > + dpu_encoder_kickoff(wb_encoder); > + dpu_encoder_kickoff(rt_encoder); > + > + /* Don't start frame done timers until the kickoffs have finished */ > + dpu_encoder_start_frame_done_timer(wb_encoder); > + dpu_encoder_start_frame_done_timer(rt_encoder); > + > + return 0; > +} > + > void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) > { > struct drm_encoder *encoder; > @@ -952,13 +988,25 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) > 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) > - */ > - drm_for_each_encoder_mask(encoder, crtc->dev, > - crtc->state->encoder_mask) > - dpu_encoder_prepare_for_kickoff(encoder); > + > + if (drm_crtc_in_clone_mode(crtc->state)) { > + if (dpu_crtc_kickoff_clone_mode(crtc)) > + goto end; > + } else { > + /* > + * 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) > + */ > + drm_for_each_encoder_mask(encoder, crtc->dev, > + crtc->state->encoder_mask) > + dpu_encoder_prepare_for_kickoff(encoder); > + > + drm_for_each_encoder_mask(encoder, crtc->dev, > + crtc->state->encoder_mask) { > + dpu_encoder_kickoff(encoder); > + dpu_encoder_start_frame_done_timer(encoder); > + } > + } > > if (atomic_inc_return(&dpu_crtc->frame_pending) == 1) { > /* acquire bandwidth and other resources */ > @@ -970,11 +1018,6 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) > > dpu_vbif_clear_errors(dpu_kms); > > - drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask) { > - dpu_encoder_kickoff(encoder); > - dpu_encoder_start_frame_done_timer(encoder); > - } > - I think I understand why you wanted to move the kickoff before the frame_pending, etc. However at the same time dpu_vbif_clear_errors() should be called before kickoff. > reinit_completion(&dpu_crtc->frame_done_comp); > > end: > > -- > 2.34.1 >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index daf5f751f584..d2f91e89eba7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -928,6 +928,42 @@ static int _dpu_crtc_wait_for_frame_done(struct drm_crtc *crtc) return rc; } +static int dpu_crtc_kickoff_clone_mode(struct drm_crtc *crtc) +{ + struct drm_encoder *encoder; + struct drm_encoder *rt_encoder = NULL, *wb_encoder; + + /* Find encoder for real time display */ + drm_for_each_encoder_mask(encoder, crtc->dev, + crtc->state->encoder_mask) { + if (encoder->encoder_type == DRM_MODE_ENCODER_VIRTUAL) + wb_encoder = encoder; + else + rt_encoder = encoder; + } + + if (!rt_encoder || !wb_encoder) { + DRM_DEBUG_ATOMIC("real time or wb encoder not found\n"); + return -EINVAL; + } + + dpu_encoder_prepare_for_kickoff(wb_encoder); + dpu_encoder_prepare_for_kickoff(rt_encoder); + + /* + * Kickoff real time encoder last as it's the encoder that + * will do the flush + */ + dpu_encoder_kickoff(wb_encoder); + dpu_encoder_kickoff(rt_encoder); + + /* Don't start frame done timers until the kickoffs have finished */ + dpu_encoder_start_frame_done_timer(wb_encoder); + dpu_encoder_start_frame_done_timer(rt_encoder); + + return 0; +} + void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) { struct drm_encoder *encoder; @@ -952,13 +988,25 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) 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) - */ - drm_for_each_encoder_mask(encoder, crtc->dev, - crtc->state->encoder_mask) - dpu_encoder_prepare_for_kickoff(encoder); + + if (drm_crtc_in_clone_mode(crtc->state)) { + if (dpu_crtc_kickoff_clone_mode(crtc)) + goto end; + } else { + /* + * 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) + */ + drm_for_each_encoder_mask(encoder, crtc->dev, + crtc->state->encoder_mask) + dpu_encoder_prepare_for_kickoff(encoder); + + drm_for_each_encoder_mask(encoder, crtc->dev, + crtc->state->encoder_mask) { + dpu_encoder_kickoff(encoder); + dpu_encoder_start_frame_done_timer(encoder); + } + } if (atomic_inc_return(&dpu_crtc->frame_pending) == 1) { /* acquire bandwidth and other resources */ @@ -970,11 +1018,6 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) dpu_vbif_clear_errors(dpu_kms); - drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask) { - dpu_encoder_kickoff(encoder); - dpu_encoder_start_frame_done_timer(encoder); - } - reinit_completion(&dpu_crtc->frame_done_comp); end:
Add a helper that will handle the correct order of the encoder kickoffs for concurrent writeback. For concurrent writeback, the realtime encoder must always kickoff last as it will call the trigger flush and start. This avoids the following scenario where the writeback encoder increments the pending kickoff count after the WB_DONE interrupt is fired: If the realtime encoder is kicked off first, the encoder kickoff will flush/start the encoder and increment the pending kickoff count. The WB_DONE interrupt then fires (before the writeback encoder is kicked off). When the writeback encoder enters its kickoff, it will skip the flush/start (due to CWB being enabled) and hit a frame done timeout as the frame was kicked off (and the WB_DONE interrupt fired) without the pending kickoff count being incremented. In addition, the writeback timer should only start after the realtime encoder is kicked off to ensure that we don't get timeouts when the system has a heavy load (ex. when debug logs are enabled) Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 67 ++++++++++++++++++++++++++------ 1 file changed, 55 insertions(+), 12 deletions(-)