Re: [PATCH] drm: simple_kms_helper: Handle the vblank events

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Sep 27, 2016 at 02:20:49PM +0200, Marek Vasut wrote:
> On 09/27/2016 02:16 PM, Noralf Trønnes wrote:
> > 
> > Den 27.09.2016 12:29, skrev Marek Vasut:
> >> On 09/27/2016 09:49 AM, Daniel Vetter wrote:
> >>> On Mon, Sep 26, 2016 at 2:59 PM, Marek Vasut <marex@denx.de> wrote:
> >>>> On 09/26/2016 11:41 AM, Marek Vasut wrote:
> >>>>> On 09/25/2016 11:00 PM, Daniel Vetter wrote:
> >>>>>> On Sun, Sep 25, 2016 at 09:41:58PM +0200, Marek Vasut wrote:
> >>>>>>> Handle the vblank events in the simple_kms_helper driver, otherwise
> >>>>>>> the drm_atomic_helper flip_done event never happens.
> >>>>>>>
> >>>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>>>>>> Cc: Noralf Trønnes <noralf@tronnes.org>
> >>>>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
> >>>>>>> Cc: David Airlie <airlied@linux.ie>
> >>>>>>> ---
> >>>>>>>   drivers/gpu/drm/drm_simple_kms_helper.c | 18 ++++++++++++++++++
> >>>>>>>   1 file changed, 18 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c
> >>>>>>> b/drivers/gpu/drm/drm_simple_kms_helper.c
> >>>>>>> index 7b6d26e..3345b40 100644
> >>>>>>> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> >>>>>>> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> >>>>>>> @@ -34,6 +34,23 @@ static const struct drm_encoder_funcs
> >>>>>>> drm_simple_kms_encoder_funcs = {
> >>>>>>>      .destroy = drm_encoder_cleanup,
> >>>>>>>   };
> >>>>>>>
> >>>>>>> +static void drm_simple_kms_crtc_begin(struct drm_crtc *crtc,
> >>>>>>> +                                 struct drm_crtc_state *state)
> >>>>>>> +{
> >>>>>>> +   struct drm_pending_vblank_event *event = crtc->state->event;
> >>>>>>> +
> >>>>>>> +   if (event) {
> >>>>>>> +           crtc->state->event = NULL;
> >>>>>>> +
> >>>>>>> +           spin_lock_irq(&crtc->dev->event_lock);
> >>>>>>> +           if (drm_crtc_vblank_get(crtc) == 0)
> >>>>>>> +                   drm_crtc_arm_vblank_event(crtc, event);
> >>>>>>> +           else
> >>>>>>> +                   drm_crtc_send_vblank_event(crtc, event);
> >>>>>>> +           spin_unlock_irq(&crtc->dev->event_lock);
> >>>>>>> +   }
> >>>>>>> +}
> >>>>>> This should be done by drivers, in the ->update hook. At least if
> >>>>>> we want
> >>>>>> to pretend that it's semi-accurate and not racy (which the above is).
> >>>>>> -Daniel
> >>>>> Got it and wrapped into mxsfb, thanks.
> >>>> Hrm, while this works most of the time, I managed to get a page_flip
> >>>> timeout when terminating the kmscube OpenGL demo (see below). I'm not
> >>>> getting this splat with this patch applied. Any idea what this could
> >>>> mean ?
> >>> Are you on latest drm-next?
> >> No, I'm on next/master from 20160927 . Is this the right tree?
> >> https://cgit.freedesktop.org/~airlied/linux/log/?h=drm-next
> >>
> >>> There was a bug fixed for 4.9 which
> >>> resulted in the primary plane not always being added to the state,
> >>> which means that the ->update hook would not get called when the plane
> >>> is getting disabled.
> >> Ah, which patch ?
> > 
> > I believe this is the one:
> > 
> > drm/simple-helpers: Always add planes to the state update
> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/drm_simple_kms_helper.c?id=6dcf0de7ef10244b17442f47956a1d9fabe2abe1
> 
> Thanks. I have that one in my tree already.

Can you pls check that for the case where you hit the WARN the simple
pipe's ->update function is called, and that you're indeed handling the
event in all cases?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel