On Tue, 28 Nov 2023 at 00:00, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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.
> 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;
> > }
> >
--
With best wishes
Dmitry
