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@xxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxx> > > --- > 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
