All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: simple_kms_helper: Handle the vblank events
@ 2016-09-25 19:41 Marek Vasut
  2016-09-25 21:00 ` Daniel Vetter
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2016-09-25 19:41 UTC (permalink / raw)
  To: dri-devel; +Cc: Marek Vasut

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);
+	}
+}
+
 static int drm_simple_kms_crtc_check(struct drm_crtc *crtc,
 				     struct drm_crtc_state *state)
 {
@@ -63,6 +80,7 @@ static void drm_simple_kms_crtc_disable(struct drm_crtc *crtc)
 }
 
 static const struct drm_crtc_helper_funcs drm_simple_kms_crtc_helper_funcs = {
+	.atomic_begin = drm_simple_kms_crtc_begin,
 	.atomic_check = drm_simple_kms_crtc_check,
 	.disable = drm_simple_kms_crtc_disable,
 	.enable = drm_simple_kms_crtc_enable,
-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH] drm: simple_kms_helper: Handle the vblank events
  2016-09-25 19:41 [PATCH] drm: simple_kms_helper: Handle the vblank events Marek Vasut
@ 2016-09-25 21:00 ` Daniel Vetter
  2016-09-26  9:41   ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2016-09-25 21:00 UTC (permalink / raw)
  To: Marek Vasut; +Cc: dri-devel

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

> +
>  static int drm_simple_kms_crtc_check(struct drm_crtc *crtc,
>  				     struct drm_crtc_state *state)
>  {
> @@ -63,6 +80,7 @@ static void drm_simple_kms_crtc_disable(struct drm_crtc *crtc)
>  }
>  
>  static const struct drm_crtc_helper_funcs drm_simple_kms_crtc_helper_funcs = {
> +	.atomic_begin = drm_simple_kms_crtc_begin,
>  	.atomic_check = drm_simple_kms_crtc_check,
>  	.disable = drm_simple_kms_crtc_disable,
>  	.enable = drm_simple_kms_crtc_enable,
> -- 
> 2.9.3
> 

-- 
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

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] drm: simple_kms_helper: Handle the vblank events
  2016-09-25 21:00 ` Daniel Vetter
@ 2016-09-26  9:41   ` Marek Vasut
  2016-09-26 12:59     ` Marek Vasut
  2016-09-27  7:50     ` Daniel Vetter
  0 siblings, 2 replies; 15+ messages in thread
From: Marek Vasut @ 2016-09-26  9:41 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

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.

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] drm: simple_kms_helper: Handle the vblank events
  2016-09-26  9:41   ` Marek Vasut
@ 2016-09-26 12:59     ` Marek Vasut
  2016-09-27  7:49       ` Daniel Vetter
  2016-09-27  7:50     ` Daniel Vetter
  1 sibling, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2016-09-26 12:59 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

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 ?

 ------------[ 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] drm: simple_kms_helper: Handle the vblank events
  2016-09-26 12:59     ` Marek Vasut
@ 2016-09-27  7:49       ` Daniel Vetter
  2016-09-27 10:29         ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2016-09-27  7:49 UTC (permalink / raw)
  To: Marek Vasut; +Cc: dri-devel

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? 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.

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.
-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



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] drm: simple_kms_helper: Handle the vblank events
  2016-09-26  9:41   ` Marek Vasut
  2016-09-26 12:59     ` Marek Vasut
@ 2016-09-27  7:50     ` Daniel Vetter
  1 sibling, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2016-09-27  7:50 UTC (permalink / raw)
  To: Marek Vasut; +Cc: dri-devel

On Mon, Sep 26, 2016 at 11:41 AM, Marek Vasut <marex@denx.de> wrote:
> 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 ?

tbh they all look a bit suspect. Most of those are probably wrong,
since I rolled them out semi-blindly just to give them some event
handling - they entirely lacked that and wouldn't ever have run on
e.g. wayland.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] drm: simple_kms_helper: Handle the vblank events
  2016-09-27  7:49       ` Daniel Vetter
@ 2016-09-27 10:29         ` Marek Vasut
  2016-09-27 12:16           ` Noralf Trønnes
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2016-09-27 10:29 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

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 ?

> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] drm: simple_kms_helper: Handle the vblank events
  2016-09-27 10:29         ` Marek Vasut
@ 2016-09-27 12:16           ` Noralf Trønnes
  2016-09-27 12:20             ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Noralf Trønnes @ 2016-09-27 12:16 UTC (permalink / raw)
  To: Marek Vasut, Daniel Vetter; +Cc: dri-devel


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

Noralf.

>> 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
>>
>>
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] drm: simple_kms_helper: Handle the vblank events
  2016-09-27 12:16           ` Noralf Trønnes
@ 2016-09-27 12:20             ` Marek Vasut
  2016-09-29  9:28               ` Daniel Vetter
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2016-09-27 12:20 UTC (permalink / raw)
  To: Noralf Trønnes, Daniel Vetter; +Cc: dri-devel

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.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] drm: simple_kms_helper: Handle the vblank events
  2016-09-27 12:20             ` Marek Vasut
@ 2016-09-29  9:28               ` Daniel Vetter
  2016-09-29 21:04                 ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2016-09-29  9:28 UTC (permalink / raw)
  To: Marek Vasut; +Cc: dri-devel

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

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] drm: simple_kms_helper: Handle the vblank events
  2016-09-29  9:28               ` Daniel Vetter
@ 2016-09-29 21:04                 ` Marek Vasut
  2016-09-30  9:50                   ` Daniel Vetter
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2016-09-29 21:04 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

On 09/29/2016 11:28 AM, Daniel Vetter wrote:
> 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?

The update is called, but I check whether I have plane->state->crtc
non-NULL in the update function of mxsfb as I need to fish out the
crtc->state->event from it. This plane->state->crtc is NULL in case when
I get the backtrace. I can access the crtc via &pipe->crtc though, which
is what I should probably do, right ?

> -Daniel
> 


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

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] drm: simple_kms_helper: Handle the vblank events
  2016-09-29 21:04                 ` Marek Vasut
@ 2016-09-30  9:50                   ` Daniel Vetter
  2016-10-02 17:15                     ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2016-09-30  9:50 UTC (permalink / raw)
  To: Marek Vasut; +Cc: dri-devel

On Thu, Sep 29, 2016 at 11:04 PM, Marek Vasut <marex@denx.de> wrote:
> On 09/29/2016 11:28 AM, Daniel Vetter wrote:
>> 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?
>
> The update is called, but I check whether I have plane->state->crtc
> non-NULL in the update function of mxsfb as I need to fish out the
> crtc->state->event from it. This plane->state->crtc is NULL in case when
> I get the backtrace. I can access the crtc via &pipe->crtc though, which
> is what I should probably do, right ?

Yup. I would in general just always go through pipe->crtc ... One
thing we could look into for simple pipe helpers is to have a
drm_simple_pipe_state which contains both states, and pass that
simple_state into all callbacks. Needs a bit of trickery in the
atomic_duplicate_state hooks though, so not sure whether that's worth
it. But merging plane/crtc state for simple pipe would be nice, since
the plane/crtc itself are also merged.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] drm: simple_kms_helper: Handle the vblank events
  2016-09-30  9:50                   ` Daniel Vetter
@ 2016-10-02 17:15                     ` Marek Vasut
  2016-10-03 13:51                       ` Daniel Vetter
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2016-10-02 17:15 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

On 09/30/2016 11:50 AM, Daniel Vetter wrote:
> On Thu, Sep 29, 2016 at 11:04 PM, Marek Vasut <marex@denx.de> wrote:
>> On 09/29/2016 11:28 AM, Daniel Vetter wrote:
>>> 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?
>>
>> The update is called, but I check whether I have plane->state->crtc
>> non-NULL in the update function of mxsfb as I need to fish out the
>> crtc->state->event from it. This plane->state->crtc is NULL in case when
>> I get the backtrace. I can access the crtc via &pipe->crtc though, which
>> is what I should probably do, right ?
> 
> Yup. I would in general just always go through pipe->crtc ...

Got it, thanks.

> One thing we could look into for simple pipe helpers is to have a
> drm_simple_pipe_state which contains both states, and pass that
> simple_state into all callbacks. Needs a bit of trickery in the
> atomic_duplicate_state hooks though, so not sure whether that's worth
> it. But merging plane/crtc state for simple pipe would be nice, since
> the plane/crtc itself are also merged.

I took a brief look into this, but I don't think I see the benefit of
it. What am I missing here ?

Thanks

> -Daniel
> 


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

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] drm: simple_kms_helper: Handle the vblank events
  2016-10-02 17:15                     ` Marek Vasut
@ 2016-10-03 13:51                       ` Daniel Vetter
  2016-10-04 20:39                         ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2016-10-03 13:51 UTC (permalink / raw)
  To: Marek Vasut; +Cc: dri-devel

On Sun, Oct 2, 2016 at 7:15 PM, Marek Vasut <marex@denx.de> wrote:
>> One thing we could look into for simple pipe helpers is to have a
>> drm_simple_pipe_state which contains both states, and pass that
>> simple_state into all callbacks. Needs a bit of trickery in the
>> atomic_duplicate_state hooks though, so not sure whether that's worth
>> it. But merging plane/crtc state for simple pipe would be nice, since
>> the plane/crtc itself are also merged.
>
> I took a brief look into this, but I don't think I see the benefit of
> it. What am I missing here ?

Well if there's no benefit, then let's not do it - it was just an idea I had.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH] drm: simple_kms_helper: Handle the vblank events
  2016-10-03 13:51                       ` Daniel Vetter
@ 2016-10-04 20:39                         ` Marek Vasut
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2016-10-04 20:39 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

On 10/03/2016 03:51 PM, Daniel Vetter wrote:
> On Sun, Oct 2, 2016 at 7:15 PM, Marek Vasut <marex@denx.de> wrote:
>>> One thing we could look into for simple pipe helpers is to have a
>>> drm_simple_pipe_state which contains both states, and pass that
>>> simple_state into all callbacks. Needs a bit of trickery in the
>>> atomic_duplicate_state hooks though, so not sure whether that's worth
>>> it. But merging plane/crtc state for simple pipe would be nice, since
>>> the plane/crtc itself are also merged.
>>
>> I took a brief look into this, but I don't think I see the benefit of
>> it. What am I missing here ?
> 
> Well if there's no benefit, then let's not do it - it was just an idea I had.

But I'm a newcomer to DRM/KMS framework, most of the time I barely know
what I'm doing, so it's easily possible I missed something obvious. I
can only think of reducing the number of memory allocations a little,
but I am not convinced this is worth it.

> -Daniel
> 


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

^ permalink raw reply	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-10-04 21:14 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-25 19:41 [PATCH] drm: simple_kms_helper: Handle the vblank events Marek Vasut
2016-09-25 21:00 ` Daniel Vetter
2016-09-26  9:41   ` Marek Vasut
2016-09-26 12:59     ` Marek Vasut
2016-09-27  7:49       ` Daniel Vetter
2016-09-27 10:29         ` Marek Vasut
2016-09-27 12:16           ` Noralf Trønnes
2016-09-27 12:20             ` Marek Vasut
2016-09-29  9:28               ` Daniel Vetter
2016-09-29 21:04                 ` Marek Vasut
2016-09-30  9:50                   ` Daniel Vetter
2016-10-02 17:15                     ` Marek Vasut
2016-10-03 13:51                       ` Daniel Vetter
2016-10-04 20:39                         ` Marek Vasut
2016-09-27  7:50     ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.