| Message ID | 20240225-fd-dpu-debug-timeout-v3-3-252f2b21cdcc@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm/dpu: debug commit_done timeouts | expand |
On 2/25/2024 6:12 AM, Dmitry Baryshkov wrote: > In order to debug commit_done timeouts, capture the devcoredump state > when the first timeout occurs after the encoder has been enabled. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > This looks fine now. Once we discuss patch 2, I can ack this. > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 30f349c8a1e5..3cae07bf0b9b 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -126,6 +126,8 @@ enum dpu_enc_rc_states { > * @base: drm_encoder base class for registration with DRM > * @enc_spinlock: Virtual-Encoder-Wide Spin Lock for IRQ purposes > * @enabled: True if the encoder is active, protected by enc_lock > + * @commit_done_timedout: True if there has been a timeout on commit after > + * enabling the encoder. > * @num_phys_encs: Actual number of physical encoders contained. > * @phys_encs: Container of physical encoders managed. > * @cur_master: Pointer to the current master in this mode. Optimization > @@ -172,6 +174,7 @@ struct dpu_encoder_virt { > spinlock_t enc_spinlock; > > bool enabled; > + bool commit_done_timedout; > > unsigned int num_phys_encs; > struct dpu_encoder_phys *phys_encs[MAX_PHYS_ENCODERS_PER_VIRTUAL]; > @@ -1226,6 +1229,8 @@ static void dpu_encoder_virt_atomic_enable(struct drm_encoder *drm_enc, > else if (disp_info->intf_type == INTF_DSI) > dpu_enc->wide_bus_en = msm_dsi_wide_bus_enabled(priv->dsi[index]); > > + dpu_enc->commit_done_timedout = false; > + > mutex_lock(&dpu_enc->enc_lock); > cur_mode = &dpu_enc->base.crtc->state->adjusted_mode; > > @@ -2436,6 +2441,10 @@ int dpu_encoder_wait_for_commit_done(struct drm_encoder *drm_enc) > DPU_ATRACE_BEGIN("wait_for_commit_done"); > ret = phys->ops.wait_for_commit_done(phys); > DPU_ATRACE_END("wait_for_commit_done"); > + if (ret == -ETIMEDOUT && !dpu_enc->commit_done_timedout) { > + dpu_enc->commit_done_timedout = true; > + msm_disp_snapshot_state(drm_enc->dev); > + } > if (ret) > return ret; > } >
On 2/25/2024 11:57 AM, Abhinav Kumar wrote: > > > On 2/25/2024 6:12 AM, Dmitry Baryshkov wrote: >> In order to debug commit_done timeouts, capture the devcoredump state >> when the first timeout occurs after the encoder has been enabled. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> > > This looks fine now. Once we discuss patch 2, I can ack this. > Not entirely onboard with patch 2, but lets see how that code evolves Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> index 30f349c8a1e5..3cae07bf0b9b 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> @@ -126,6 +126,8 @@ enum dpu_enc_rc_states { >> * @base: drm_encoder base class for registration with DRM >> * @enc_spinlock: Virtual-Encoder-Wide Spin Lock for IRQ purposes >> * @enabled: True if the encoder is active, protected by >> enc_lock >> + * @commit_done_timedout: True if there has been a timeout on commit >> after >> + * enabling the encoder. >> * @num_phys_encs: Actual number of physical encoders contained. >> * @phys_encs: Container of physical encoders managed. >> * @cur_master: Pointer to the current master in this mode. >> Optimization >> @@ -172,6 +174,7 @@ struct dpu_encoder_virt { >> spinlock_t enc_spinlock; >> bool enabled; >> + bool commit_done_timedout; >> unsigned int num_phys_encs; >> struct dpu_encoder_phys *phys_encs[MAX_PHYS_ENCODERS_PER_VIRTUAL]; >> @@ -1226,6 +1229,8 @@ static void >> dpu_encoder_virt_atomic_enable(struct drm_encoder *drm_enc, >> else if (disp_info->intf_type == INTF_DSI) >> dpu_enc->wide_bus_en = >> msm_dsi_wide_bus_enabled(priv->dsi[index]); >> + dpu_enc->commit_done_timedout = false; >> + >> mutex_lock(&dpu_enc->enc_lock); >> cur_mode = &dpu_enc->base.crtc->state->adjusted_mode; >> @@ -2436,6 +2441,10 @@ int dpu_encoder_wait_for_commit_done(struct >> drm_encoder *drm_enc) >> DPU_ATRACE_BEGIN("wait_for_commit_done"); >> ret = phys->ops.wait_for_commit_done(phys); >> DPU_ATRACE_END("wait_for_commit_done"); >> + if (ret == -ETIMEDOUT && !dpu_enc->commit_done_timedout) { >> + dpu_enc->commit_done_timedout = true; >> + msm_disp_snapshot_state(drm_enc->dev); >> + } >> if (ret) >> return ret; >> } >>
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 30f349c8a1e5..3cae07bf0b9b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -126,6 +126,8 @@ enum dpu_enc_rc_states { * @base: drm_encoder base class for registration with DRM * @enc_spinlock: Virtual-Encoder-Wide Spin Lock for IRQ purposes * @enabled: True if the encoder is active, protected by enc_lock + * @commit_done_timedout: True if there has been a timeout on commit after + * enabling the encoder. * @num_phys_encs: Actual number of physical encoders contained. * @phys_encs: Container of physical encoders managed. * @cur_master: Pointer to the current master in this mode. Optimization @@ -172,6 +174,7 @@ struct dpu_encoder_virt { spinlock_t enc_spinlock; bool enabled; + bool commit_done_timedout; unsigned int num_phys_encs; struct dpu_encoder_phys *phys_encs[MAX_PHYS_ENCODERS_PER_VIRTUAL]; @@ -1226,6 +1229,8 @@ static void dpu_encoder_virt_atomic_enable(struct drm_encoder *drm_enc, else if (disp_info->intf_type == INTF_DSI) dpu_enc->wide_bus_en = msm_dsi_wide_bus_enabled(priv->dsi[index]); + dpu_enc->commit_done_timedout = false; + mutex_lock(&dpu_enc->enc_lock); cur_mode = &dpu_enc->base.crtc->state->adjusted_mode; @@ -2436,6 +2441,10 @@ int dpu_encoder_wait_for_commit_done(struct drm_encoder *drm_enc) DPU_ATRACE_BEGIN("wait_for_commit_done"); ret = phys->ops.wait_for_commit_done(phys); DPU_ATRACE_END("wait_for_commit_done"); + if (ret == -ETIMEDOUT && !dpu_enc->commit_done_timedout) { + dpu_enc->commit_done_timedout = true; + msm_disp_snapshot_state(drm_enc->dev); + } if (ret) return ret; }
In order to debug commit_done timeouts, capture the devcoredump state when the first timeout occurs after the encoder has been enabled. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 9 +++++++++ 1 file changed, 9 insertions(+)