Re: [PATCH] drm/msm: Grab a vblank reference when waiting for commit_done

[Sean Paul <sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>]

On Mon, Oct 08, 2018 at 06:38:11PM -0700, Abhinav Kumar wrote:
> On 2018-10-03 13:22, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > Similar to the atomic helpers, we should enable vblank while we're
> > waiting for the commit to finish. DPU needs this, MDP5 seems to work
> > fine without it.
> > 
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> As such I dont see any issue with this patch but I have a question overall
> on chrome vblank handling.
> For a video mode panel, we will keep the HW resources enabled (including
> vsync related clocks) till device
> is suspended.
> 
> The vblank_get and vblank_put are only controlling the register/deregister
> of the vblank IRQ callback which
> send the events to the userspace and anything pending on vsync completion.
> 
> Does the chrome userspace turn ON/OFF the vblank using vblank_ctrl IOCTL Or
> does it guarantee it to be?
>

No not explicitly. The issue I was seeing is that when userspace (this issue is
not specific to chrome) does a commit we get a frame_done timeout. So while the
hardware might still be active, the worker to signal frame_done is not unless
the vblank_ref is > 0.

> If so, is this patch just more of a safety check to make sure that vsync
> events remain ON till the frame is done?
> 
> Because HW wise it should be and this shouldnt be needed.

It is definitely needed, unless you want 60ms (the frame_done timeout) between
frames :-)

Sean

> 
> >  drivers/gpu/drm/msm/msm_atomic.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/msm/msm_atomic.c
> > b/drivers/gpu/drm/msm/msm_atomic.c
> > index c1f1779c980f..2b7bb6e166d3 100644
> > --- a/drivers/gpu/drm/msm/msm_atomic.c
> > +++ b/drivers/gpu/drm/msm/msm_atomic.c
> > @@ -32,7 +32,12 @@ static void msm_atomic_wait_for_commit_done(struct
> > drm_device *dev,
> >  		if (!new_crtc_state->active)
> >  			continue;
> > 
> > +		if (drm_crtc_vblank_get(crtc))
> > +			continue;
> > +
> >  		kms->funcs->wait_for_crtc_commit_done(kms, crtc);
> > +
> > +		drm_crtc_vblank_put(crtc);
> >  	}
> >  }

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno