[PATCH] drm/msm/dpu: Capture dpu snapshot when frame_done_timer timeouts

[Paloma Arellano]

On 11/28/2023 12:24 PM, Dmitry Baryshkov wrote:
> On Tue, 28 Nov 2023 at 19:43, Paloma Arellano <quic_parellan at quicinc.com> wrote:
>>
>> On 11/27/2023 5:48 PM, Dmitry Baryshkov wrote:
>>> On Tue, 28 Nov 2023 at 03:12, Paloma Arellano <quic_parellan at quicinc.com> wrote:
>>>> Trigger a devcoredump to dump dpu registers and capture the drm atomic
>>>> state when the frame_done_timer timeouts.
>>>>
>>>> Signed-off-by: Paloma Arellano <quic_parellan at quicinc.com>
>>>> ---
>>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 +++++++++++--
>>>>    1 file changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>>> index 1cf7ff6caff4..5cf7594feb5a 100644
>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>>> @@ -191,6 +191,7 @@ struct dpu_encoder_virt {
>>>>           void *crtc_frame_event_cb_data;
>>>>
>>>>           atomic_t frame_done_timeout_ms;
>>>> +       atomic_t frame_done_timeout_cnt;
>>>>           struct timer_list frame_done_timer;
>>>>
>>>>           struct msm_display_info disp_info;
>>>> @@ -1204,6 +1205,8 @@ static void dpu_encoder_virt_atomic_enable(struct drm_encoder *drm_enc,
>>>>
>>>>           dpu_enc->dsc = dpu_encoder_get_dsc_config(drm_enc);
>>>>
>>>> +       atomic_set(&dpu_enc->frame_done_timeout_cnt, 0);
>>>> +
>>>>           if (disp_info->intf_type == INTF_DP)
>>>>                   dpu_enc->wide_bus_en = msm_dp_wide_bus_available(priv->dp[index]);
>>>>           else if (disp_info->intf_type == INTF_DSI)
>>>> @@ -2115,11 +2118,12 @@ static int _dpu_encoder_status_show(struct seq_file *s, void *data)
>>>>           for (i = 0; i < dpu_enc->num_phys_encs; i++) {
>>>>                   struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
>>>>
>>>> -               seq_printf(s, "intf:%d  wb:%d  vsync:%8d     underrun:%8d    ",
>>>> +               seq_printf(s, "intf:%d  wb:%d  vsync:%8d     underrun:%8d    frame_done_cnt:%d",
>>>>                                   phys->hw_intf ? phys->hw_intf->idx - INTF_0 : -1,
>>>>                                   phys->hw_wb ? phys->hw_wb->idx - WB_0 : -1,
>>>>                                   atomic_read(&phys->vsync_cnt),
>>>> -                               atomic_read(&phys->underrun_cnt));
>>>> +                               atomic_read(&phys->underrun_cnt),
>>>> +                               atomic_read(&dpu_enc->frame_done_timeout_cnt));
>>>>
>>>>                   seq_printf(s, "mode: %s\n", dpu_encoder_helper_get_intf_type(phys->intf_mode));
>>>>           }
>>>> @@ -2341,6 +2345,10 @@ static void dpu_encoder_frame_done_timeout(struct timer_list *t)
>>>>
>>>>           DPU_ERROR_ENC(dpu_enc, "frame done timeout\n");
>>>>
>>>> +       atomic_inc(&dpu_enc->frame_done_timeout_cnt);
>>>> +       if (atomic_read(&dpu_enc->frame_done_timeout_cnt) == 1)
>>>> +               msm_disp_snapshot_state(drm_enc->dev);
>>> atomic_inc_and_test(), please
>> Hi Dmitry,
>>
>> We only want to create a snapshot for the first instance in which the
>> timer timeouts. atomic_int_and_test() increments the value and then
>> returns whether it has a value of zero or not. FWIW I think I should
>> change it to 'atomic_add_return(1, &dpu_enc->frame_done_timeout_cnt)' so
>> that we can check only when this value equals one.
> Works for me too.
>
> I suggested atomic_inc_test() because then we can let devcoredump take
> care of duplicate events.

Ack

-Paloma

>
>> Thank you,
>>
>> Paloma
>>
>>>> +
>>>>           event = DPU_ENCODER_FRAME_EVENT_ERROR;
>>>>           trace_dpu_enc_frame_done_timeout(DRMID(drm_enc), event);
>>>>           dpu_enc->crtc_frame_event_cb(dpu_enc->crtc_frame_event_cb_data, event);
>>>> @@ -2392,6 +2400,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
>>>>                   goto fail;
>>>>
>>>>           atomic_set(&dpu_enc->frame_done_timeout_ms, 0);
>>>> +       atomic_set(&dpu_enc->frame_done_timeout_cnt, 0);
>>>>           timer_setup(&dpu_enc->frame_done_timer,
>>>>                           dpu_encoder_frame_done_timeout, 0);
>>>>
>>>> --
>>>> 2.41.0
>>>>
>
>