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

[Abhinav Kumar <abhinavk-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

On 2018-10-09 06:59, Sean Paul wrote:
> 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
> 
Alright, in that case

Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>

>> 
>> >  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);
>> >  	}
>> >  }
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno