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@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
Alright, in that case
Reviewed-by: Abhinav Kumar <abhinavk@xxxxxxxxxxxxxx>
> 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);
> }
> }
