[Freedreno] [PATCH] drm/msm/mdp4: flush vblank event on disable

[Abhinav Kumar]

On 11/27/2023 3:17 PM, Dmitry Baryshkov wrote:
> On Tue, 28 Nov 2023 at 00:00, Abhinav Kumar <quic_abhinavk at quicinc.com> wrote:
>>
>>
>>
>> On 11/27/2023 1:54 PM, Dmitry Baryshkov wrote:
>>> Flush queued events when disabling the crtc. This avoids timeouts when
>>> we come back and wait for dependencies (like the previous frame's
>>> flip_done).
>>>
>>> Fixes: c8afe684c95c ("drm/msm: basic KMS driver for snapdragon")
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>> ---
>>>    drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 9 +++++++++
>>>    1 file changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
>>> index 169f9de4a12a..3100957225a7 100644
>>> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
>>> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
>>> @@ -269,6 +269,7 @@ static void mdp4_crtc_atomic_disable(struct drm_crtc *crtc,
>>>    {
>>>        struct mdp4_crtc *mdp4_crtc = to_mdp4_crtc(crtc);
>>>        struct mdp4_kms *mdp4_kms = get_kms(crtc);
>>> +     unsigned long flags;
>>>
>>>        DBG("%s", mdp4_crtc->name);
>>>
>>> @@ -281,6 +282,14 @@ static void mdp4_crtc_atomic_disable(struct drm_crtc *crtc,
>>>        mdp_irq_unregister(&mdp4_kms->base, &mdp4_crtc->err);
>>>        mdp4_disable(mdp4_kms);
>>>
>>> +     if (crtc->state->event && !crtc->state->active) {
>>> +             WARN_ON(mdp4_crtc->event);
>>
>> Do you need a WARN_ON() here? Just wondering how often this might happen
>> and spam.
> 
> Well, it is atomic_disable(), so it happens only on modesets or output
> disable. Also, this is a port of mdp5 code, see commit e765ea77b0fd
> ("drm/msm: Issue queued events when disabling crtc"). We don't have
> reports from MDP5 code, so there should be no comparable reports for
> MDP4.
> 

Ok, I thought this was ported from dpu code, even that has the same 
logic but not the WARN_ON().

But I am fine with this,

Reviewed-by: Abhinav Kumar <quic_abhinavk at quicinc.com>


>> otherwise LGTM.
>>
>>> +             spin_lock_irqsave(&mdp4_kms->dev->event_lock, flags);
>>> +             drm_crtc_send_vblank_event(crtc, crtc->state->event);
>>> +             crtc->state->event = NULL;
>>> +             spin_unlock_irqrestore(&mdp4_kms->dev->event_lock, flags);
>>> +     }
>>> +
>>>        mdp4_crtc->enabled = false;
>>>    }
>>>
> 
> 
>