Re: [Freedreno] [PATCH] drm/msm: Grab a vblank reference when waiting for commit_done
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 > > 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 > --- 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 > 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
Re: [Freedreno] [PATCH] drm/msm: Grab a vblank reference when waiting for commit_done
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 > > > > 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 > > --- > 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
Re: [Freedreno] [PATCH] drm/msm: Grab a vblank reference when waiting for commit_done
On 2018-10-03 13:22, Sean Paul wrote: From: Sean Paul 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 --- 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? 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. 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
[Freedreno] [PATCH] drm/msm: Grab a vblank reference when waiting for commit_done
From: Sean Paul 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 --- 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
