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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/27/2016 09:49 AM, Daniel Vetter wrote:
> On Mon, Sep 26, 2016 at 2:59 PM, Marek Vasut <marex@xxxxxxx> 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@xxxxxxx>
>>>>> Cc: Noralf Trønnes <noralf@xxxxxxxxxxx>
>>>>> Cc: Daniel Vetter <daniel@xxxxxxxx>
>>>>> Cc: David Airlie <airlied@xxxxxxxx>
>>>>> ---
>>>>>  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 ?

> At least that's my guess, the WARN_ON just tells you that no one sent
> out the event yet, but by that point it should have happened.

Yup :)

> -Daniel
> 
>>
>>  ------------[ cut here ]------------
>>  WARNING: CPU: 0 PID: 77 at drivers/gpu/drm/drm_atomic_helper.c:1579
>> drm_atomic_helper_commit_hw_done+0xa8/0xb8
>>  Modules linked in:
>>  CPU: 0 PID: 77 Comm: kworker/0:2 Tainted: G        W
>> 4.8.0-rc7-next-20160923-00033-ge3f15d6 #56
>>  Hardware name: Freescale i.MX6 SoloX (Device Tree)
>>  Workqueue: events drm_mode_rmfb_work_fn
>>  Backtrace:
>>  [<c010c180>] (dump_backtrace) from [<c010c378>] (show_stack+0x18/0x1c)
>>   r7:00000000 r6:600e0013 r5:00000000 r4:c0e2342c
>>  [<c010c360>] (show_stack) from [<c03f4ad8>] (dump_stack+0xb4/0xe8)
>>  [<c03f4a24>] (dump_stack) from [<c0124a90>] (__warn+0xd8/0x104)
>>   r9:c04bdb60 r8:0000062b r7:00000009 r6:c0c38298 r5:00000000 r4:00000000
>>  [<c01249b8>] (__warn) from [<c0124b70>] (warn_slowpath_null+0x28/0x30)
>>   r9:db43639c r8:0000062b r7:c0c38298 r6:da962e40 r5:00000000 r4:00000000
>>  [<c0124b48>] (warn_slowpath_null) from [<c04bdb60>]
>> (drm_atomic_helper_commit_hw_done+0xa8/0xb8)
>>  [<c04bdab8>] (drm_atomic_helper_commit_hw_done) from [<c04c0568>]
>> (drm_atomic_helper_commit_tail+0x44/0x60)
>>   r10:c0c3bd84 r9:c0c371cc r8:db770000 r7:db770000 r6:00000000 r5:db770000
>>   r4:daffa780 r3:daffa3c0
>>  [<c04c0524>] (drm_atomic_helper_commit_tail) from [<c04c05e8>]
>> (commit_tail+0x64/0x68)
>>   r5:00000000 r4:daffa780
>>  [<c04c0584>] (commit_tail) from [<c04c06b0>]
>> (drm_atomic_helper_commit+0xac/0xec)
>>   r5:00000000 r4:daffa780
>>  [<c04c0604>] (drm_atomic_helper_commit) from [<c04e11b4>]
>> (drm_atomic_commit+0x58/0x78)
>>   r7:db436028 r6:db770000 r5:daffa780 r4:00000000
>>  [<c04e115c>] (drm_atomic_commit) from [<c04c0be4>]
>> (drm_atomic_helper_set_config+0x80/0xa4)
>>   r6:dad37e14 r5:00000000 r4:daffa780
>>  [<c04c0b64>] (drm_atomic_helper_set_config) from [<c04d35fc>]
>> (drm_mode_set_config_internal+0x6c/0xfc)
>>   r7:db77048c r6:db770480 r5:db436028 r4:00000000
>>  [<c04d3590>] (drm_mode_set_config_internal) from [<c04d36c4>]
>> (drm_crtc_force_disable+0x38/0x40)
>>   r7:db77048c r6:db770480 r5:da962c00 r4:db436028
>>  [<c04d368c>] (drm_crtc_force_disable) from [<c04e2b60>]
>> (drm_framebuffer_remove+0xd8/0x124)
>>  [<c04e2a88>] (drm_framebuffer_remove) from [<c04e2bec>]
>> (drm_mode_rmfb_work_fn+0x40/0x50)
>>   r10:00000001 r9:00000000 r8:dbbb2a00 r7:dad37ea8 r6:dbbaf800 r5:db60be68
>>   r4:db60be48
>>  [<c04e2bac>] (drm_mode_rmfb_work_fn) from [<c013fa10>]
>> (process_one_work+0x24c/0x4e8)
>>   r5:dad12f00 r4:db60be48
>>  [<c013f7c4>] (process_one_work) from [<c01408dc>]
>> (worker_thread+0x40/0x574)
>>   r10:dad12f00 r9:dad36000 r8:c0e02100 r7:dbbaf834 r6:00000008 r5:dad12f18
>>   r4:dbbaf800
>>  [<c014089c>] (worker_thread) from [<c0146908>] (kthread+0xd8/0xf8)
>>   r10:00000000 r9:00000000 r8:00000000 r7:c014089c r6:dad12f00 r5:dad28500
>>   r4:00000000
>>  [<c0146830>] (kthread) from [<c0107cb0>] (ret_from_fork+0x14/0x24)
>>   r7:00000000 r6:00000000 r5:c0146830 r4:dad28500
>>  ---[ end trace 2b6e3065f0299ba5 ]---
>>
>>> But then, I see a few drivers (arm hdlcd, fsl-dcu,...) doing the same
>>> thing at random callbacks of CRTC . Shouldn't this event handling be
>>> consolidated into some generic function and those drivers fixed to
>>> call it from atomic update ?
>>>
>>
>>
>> --
>> Best regards,
>> Marek Vasut
> 
> 
> 


-- 
Best regards,
Marek Vasut
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux