* drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-10 15:01 ` Liviu Dudau
0 siblings, 0 replies; 34+ messages in thread
From: Liviu Dudau @ 2015-11-10 15:01 UTC (permalink / raw)
To: Daniel Vetter, David Airlie, Rob Clark, DRI devel, lkml
Hello,
When booting my Juno board with the HDLCD driver that I have converted to
atomic operations I'm getting the following warning:
------------[ cut here ]------------
WARNING: at /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674
Modules linked in: hdlcd(+) clk_scpi
CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5
Hardware name: ARM Juno development board (r0) (DT)
task: ffffffc974888b00 ti: ffffffc9755dc000 task.ti: ffffffc9755dc000
PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
pc : [<ffffffc0003e6d18>] lr : [<ffffffc0003e8214>] pstate: 20000145
sp : ffffffc9755df430
x29: ffffffc9755df430 x28: ffffffc975703600
x27: 0000000000000000 x26: ffffffc976253960
x25: ffffffc976254040 x24: ffffffc000819000
x23: ffffffc000689ea0 x22: ffffffc976251800
x21: ffffffc976251800 x20: 0000000000000000
x19: ffffffc9766b1f80 x18: 00000000715fe015
x17: 0000007fb4b855b0 x16: 0000000000000220
x15: 0000000000000001 x14: 0ffffffffffffffe
x13: 0000000000000008 x12: 0101010101010101
x11: ffffffc000964000 x10: ffffffc0009d2000
x9 : 0000000000000000 x8 : ffffffc97ff5f700
x7 : ffffffc97566cb80 x6 : ffffffc9766b1700
x5 : ffffffc975665100 x4 : 0000000000000000
x3 : ffffffc976253960 x2 : ffffffc97566cd00
x1 : ffffffc976253900 x0 : 0000000000000000
---[ end trace 9fe289f798e7178e ]---
Call trace:
[<ffffffc0003e6d18>] drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
[<ffffffc0003e8214>] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
[<ffffffc0003e84c8>] drm_atomic_helper_commit+0xe0/0x150
[<ffffffc00040bcc0>] drm_atomic_commit+0x40/0x6c
[<ffffffc0003e92c0>] restore_fbdev_mode+0x294/0x2d4
[<ffffffc0003eb6ec>] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c
[<ffffffc0003eb770>] drm_fb_helper_set_par+0x2c/0x58
[<ffffffc000378048>] fbcon_init+0x4d4/0x534
[<ffffffc0003b44f4>] visual_init+0xac/0x104
[<ffffffc0003b63f8>] do_bind_con_driver+0x16c/0x398
[<ffffffc0003b6968>] do_take_over_console+0xd8/0x1f4
[<ffffffc00037811c>] do_fbcon_takeover+0x74/0xf8
[<ffffffc00037c5cc>] fbcon_event_notify+0x8a4/0x8f8
[<ffffffc0000d19f0>] notifier_call_chain+0x4c/0x88
[<ffffffc0000d1d78>] __blocking_notifier_call_chain+0x44/0x74
[<ffffffc0000d1dbc>] blocking_notifier_call_chain+0x14/0x1c
[<ffffffc00037d614>] fb_notifier_call_chain+0x1c/0x24
[<ffffffc00037f8a8>] register_framebuffer+0x1c0/0x2ac
[<ffffffc0003eb9f8>] drm_fb_helper_initial_config+0x25c/0x3ec
[<ffffffc0003ec28c>] drm_fbdev_cma_init+0x98/0x134
[<ffffffbffc006780>] hdlcd_drm_bind+0x180/0x498 [hdlcd]
[<ffffffc0004158e0>] try_to_bring_up_master.part.5+0xd4/0x118
[<ffffffc0004159e8>] component_master_add_with_match+0xc4/0x148
[<ffffffc000415a7c>] component_master_add+0x10/0x18
[<ffffffbffc0065ec>] hdlcd_probe+0x14/0x28 [hdlcd]
[<ffffffc00041c504>] platform_drv_probe+0x54/0xc0
[<ffffffc00041a9b0>] driver_probe_device+0x1ec/0x2e8
[<ffffffc00041ab48>] __driver_attach+0x9c/0xa0
[<ffffffc000418bcc>] bus_for_each_dev+0x58/0x98
[<ffffffc00041a41c>] driver_attach+0x20/0x28
[<ffffffc00041a058>] bus_add_driver+0x1c8/0x22c
[<ffffffc00041b3d0>] driver_register+0x68/0x108
[<ffffffc00041c408>] __platform_driver_register+0x4c/0x54
[<ffffffbffc00b018>] hdlcd_init+0x18/0x30 [hdlcd]
[<ffffffc00008293c>] do_one_initcall+0x90/0x1a8
[<ffffffc0001424dc>] do_init_module+0x60/0x1c8
[<ffffffc00011d364>] load_module+0x1554/0x1c98
[<ffffffc00011dc64>] SyS_finit_module+0x7c/0x88
[<ffffffc000085cb0>] el0_svc_naked+0x24/0x28
The line that triggers the warning is:
674: WARN_ON(!connector->encoder->crtc);
As far as I can see the encoder->crtc value is being set to a non-NULL value only
in two places:
- in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON()
encoder->crtc = connector->state->crtc;
- in drm_crtc_helper_set_config(drm_mode_set *set):
encoder->crtc = new_crtc;
Nothing in the call path from drm_atomic_commit() calls crtc_funcs->set_config() or
drm_crtc_helper_set_config() directly, so the question is if this WARN() is actually
valid.
Call path from drm_atomic_commit:
drm_atomic_helper_commit()
- drm_atomic_helper_prepare_planes()
- drm_atomic_helper_swap_state()
- drm_atomic_helper_commit_modeset_disables()
- disable_outputs()
- drm_atomic_helper_update_legacy_modeset_state()
- WARN_ON(!connector->encoder->crtc)
Best regards,
Liviu
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 34+ messages in thread
* drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-10 15:01 ` Liviu Dudau
0 siblings, 0 replies; 34+ messages in thread
From: Liviu Dudau @ 2015-11-10 15:01 UTC (permalink / raw)
To: Daniel Vetter, David Airlie, Rob Clark, DRI devel, lkml
Hello,
When booting my Juno board with the HDLCD driver that I have converted to
atomic operations I'm getting the following warning:
------------[ cut here ]------------
WARNING: at /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674
Modules linked in: hdlcd(+) clk_scpi
CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5
Hardware name: ARM Juno development board (r0) (DT)
task: ffffffc974888b00 ti: ffffffc9755dc000 task.ti: ffffffc9755dc000
PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
pc : [<ffffffc0003e6d18>] lr : [<ffffffc0003e8214>] pstate: 20000145
sp : ffffffc9755df430
x29: ffffffc9755df430 x28: ffffffc975703600
x27: 0000000000000000 x26: ffffffc976253960
x25: ffffffc976254040 x24: ffffffc000819000
x23: ffffffc000689ea0 x22: ffffffc976251800
x21: ffffffc976251800 x20: 0000000000000000
x19: ffffffc9766b1f80 x18: 00000000715fe015
x17: 0000007fb4b855b0 x16: 0000000000000220
x15: 0000000000000001 x14: 0ffffffffffffffe
x13: 0000000000000008 x12: 0101010101010101
x11: ffffffc000964000 x10: ffffffc0009d2000
x9 : 0000000000000000 x8 : ffffffc97ff5f700
x7 : ffffffc97566cb80 x6 : ffffffc9766b1700
x5 : ffffffc975665100 x4 : 0000000000000000
x3 : ffffffc976253960 x2 : ffffffc97566cd00
x1 : ffffffc976253900 x0 : 0000000000000000
---[ end trace 9fe289f798e7178e ]---
Call trace:
[<ffffffc0003e6d18>] drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
[<ffffffc0003e8214>] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
[<ffffffc0003e84c8>] drm_atomic_helper_commit+0xe0/0x150
[<ffffffc00040bcc0>] drm_atomic_commit+0x40/0x6c
[<ffffffc0003e92c0>] restore_fbdev_mode+0x294/0x2d4
[<ffffffc0003eb6ec>] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c
[<ffffffc0003eb770>] drm_fb_helper_set_par+0x2c/0x58
[<ffffffc000378048>] fbcon_init+0x4d4/0x534
[<ffffffc0003b44f4>] visual_init+0xac/0x104
[<ffffffc0003b63f8>] do_bind_con_driver+0x16c/0x398
[<ffffffc0003b6968>] do_take_over_console+0xd8/0x1f4
[<ffffffc00037811c>] do_fbcon_takeover+0x74/0xf8
[<ffffffc00037c5cc>] fbcon_event_notify+0x8a4/0x8f8
[<ffffffc0000d19f0>] notifier_call_chain+0x4c/0x88
[<ffffffc0000d1d78>] __blocking_notifier_call_chain+0x44/0x74
[<ffffffc0000d1dbc>] blocking_notifier_call_chain+0x14/0x1c
[<ffffffc00037d614>] fb_notifier_call_chain+0x1c/0x24
[<ffffffc00037f8a8>] register_framebuffer+0x1c0/0x2ac
[<ffffffc0003eb9f8>] drm_fb_helper_initial_config+0x25c/0x3ec
[<ffffffc0003ec28c>] drm_fbdev_cma_init+0x98/0x134
[<ffffffbffc006780>] hdlcd_drm_bind+0x180/0x498 [hdlcd]
[<ffffffc0004158e0>] try_to_bring_up_master.part.5+0xd4/0x118
[<ffffffc0004159e8>] component_master_add_with_match+0xc4/0x148
[<ffffffc000415a7c>] component_master_add+0x10/0x18
[<ffffffbffc0065ec>] hdlcd_probe+0x14/0x28 [hdlcd]
[<ffffffc00041c504>] platform_drv_probe+0x54/0xc0
[<ffffffc00041a9b0>] driver_probe_device+0x1ec/0x2e8
[<ffffffc00041ab48>] __driver_attach+0x9c/0xa0
[<ffffffc000418bcc>] bus_for_each_dev+0x58/0x98
[<ffffffc00041a41c>] driver_attach+0x20/0x28
[<ffffffc00041a058>] bus_add_driver+0x1c8/0x22c
[<ffffffc00041b3d0>] driver_register+0x68/0x108
[<ffffffc00041c408>] __platform_driver_register+0x4c/0x54
[<ffffffbffc00b018>] hdlcd_init+0x18/0x30 [hdlcd]
[<ffffffc00008293c>] do_one_initcall+0x90/0x1a8
[<ffffffc0001424dc>] do_init_module+0x60/0x1c8
[<ffffffc00011d364>] load_module+0x1554/0x1c98
[<ffffffc00011dc64>] SyS_finit_module+0x7c/0x88
[<ffffffc000085cb0>] el0_svc_naked+0x24/0x28
The line that triggers the warning is:
674: WARN_ON(!connector->encoder->crtc);
As far as I can see the encoder->crtc value is being set to a non-NULL value only
in two places:
- in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON()
encoder->crtc = connector->state->crtc;
- in drm_crtc_helper_set_config(drm_mode_set *set):
encoder->crtc = new_crtc;
Nothing in the call path from drm_atomic_commit() calls crtc_funcs->set_config() or
drm_crtc_helper_set_config() directly, so the question is if this WARN() is actually
valid.
Call path from drm_atomic_commit:
drm_atomic_helper_commit()
- drm_atomic_helper_prepare_planes()
- drm_atomic_helper_swap_state()
- drm_atomic_helper_commit_modeset_disables()
- disable_outputs()
- drm_atomic_helper_update_legacy_modeset_state()
- WARN_ON(!connector->encoder->crtc)
Best regards,
Liviu
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-10 15:01 ` Liviu Dudau
@ 2015-11-10 16:56 ` Thierry Reding
-1 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2015-11-10 16:56 UTC (permalink / raw)
To: Liviu Dudau; +Cc: Daniel Vetter, David Airlie, Rob Clark, DRI devel, lkml
[-- Attachment #1: Type: text/plain, Size: 342 bytes --]
On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
> Hello,
>
> When booting my Juno board with the HDLCD driver that I have converted to
> atomic operations I'm getting the following warning:
Perhaps you can provide pointers to the source code, that might make it
easier for people to spot what's going wrong.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-10 16:56 ` Thierry Reding
0 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2015-11-10 16:56 UTC (permalink / raw)
To: Liviu Dudau; +Cc: DRI devel, lkml
[-- Attachment #1.1: Type: text/plain, Size: 342 bytes --]
On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
> Hello,
>
> When booting my Juno board with the HDLCD driver that I have converted to
> atomic operations I'm getting the following warning:
Perhaps you can provide pointers to the source code, that might make it
easier for people to spot what's going wrong.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-10 16:56 ` Thierry Reding
@ 2015-11-11 16:09 ` Liviu Dudau
-1 siblings, 0 replies; 34+ messages in thread
From: Liviu Dudau @ 2015-11-11 16:09 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, David Airlie, Rob Clark, DRI devel, lkml
On Tue, Nov 10, 2015 at 05:56:15PM +0100, Thierry Reding wrote:
> On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
> > Hello,
> >
> > When booting my Juno board with the HDLCD driver that I have converted to
> > atomic operations I'm getting the following warning:
>
> Perhaps you can provide pointers to the source code, that might make it
> easier for people to spot what's going wrong.
>
> Thierry
Hi Thierry,
I have just pushed to the mailing lists my patch series. [1] [2]
If you want to checkout the code I also have a branch here:
git://git://linux-arm.org/linux-ld testing/hdlcd
Best regards,
Liviu
[1] http://lists.freedesktop.org/archives/dri-devel/2015-November/094172.html
[2] http://lists.freedesktop.org/archives/dri-devel/2015-November/094177.html
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-11 16:09 ` Liviu Dudau
0 siblings, 0 replies; 34+ messages in thread
From: Liviu Dudau @ 2015-11-11 16:09 UTC (permalink / raw)
To: Thierry Reding; +Cc: DRI devel, lkml
On Tue, Nov 10, 2015 at 05:56:15PM +0100, Thierry Reding wrote:
> On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
> > Hello,
> >
> > When booting my Juno board with the HDLCD driver that I have converted to
> > atomic operations I'm getting the following warning:
>
> Perhaps you can provide pointers to the source code, that might make it
> easier for people to spot what's going wrong.
>
> Thierry
Hi Thierry,
I have just pushed to the mailing lists my patch series. [1] [2]
If you want to checkout the code I also have a branch here:
git://git://linux-arm.org/linux-ld testing/hdlcd
Best regards,
Liviu
[1] http://lists.freedesktop.org/archives/dri-devel/2015-November/094172.html
[2] http://lists.freedesktop.org/archives/dri-devel/2015-November/094177.html
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-10 16:56 ` Thierry Reding
(?)
(?)
@ 2015-11-12 6:27 ` Mark yao
2015-11-12 6:34 ` Mark yao
2015-11-12 12:26 ` Thierry Reding
-1 siblings, 2 replies; 34+ messages in thread
From: Mark yao @ 2015-11-12 6:27 UTC (permalink / raw)
To: Thierry Reding, Liviu Dudau; +Cc: lkml, DRI devel
[-- Attachment #1.1: Type: text/plain, Size: 3186 bytes --]
On 2015年11月11日 00:56, Thierry Reding wrote:
> On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
>> Hello,
>>
>> When booting my Juno board with the HDLCD driver that I have converted to
>> atomic operations I'm getting the following warning:
> Perhaps you can provide pointers to the source code, that might make it
> easier for people to spot what's going wrong.
>
> Thierry
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Thierry
I encountered the same problem as Liviu.
I'm coverting rockchip drm to atomic api, when booting with hdmi
connected, get under warning:
[ 1.300156] WARNING: CPU: 0 PID: 26 at
drivers/gpu/drm/drm_atomic_helper.c:674
drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8()
[ 1.300161] Modules linked in:
[ 1.300171] CPU: 0 PID: 26 Comm: kworker/0:1 Not tainted 4.3.0-rc5+ #160
[ 1.300174] Hardware name: Rockchip (Device Tree)
[ 1.300189] Workqueue: events output_poll_execute
[ 1.300224] [<c0015e54>] (unwind_backtrace) from [<c00123cc>]
(show_stack+0x10/0x14)
[ 1.300241] [<c00123cc>] (show_stack) from [<c01a5980>]
(dump_stack+0x6c/0x88)
[ 1.300255] [<c01a5980>] (dump_stack) from [<c0024050>]
(warn_slowpath_common+0x80/0xa8)
[ 1.300265] [<c0024050>] (warn_slowpath_common) from [<c0024090>]
(warn_slowpath_null+0x18/0x1c)
[ 1.300277] [<c0024090>] (warn_slowpath_null) from [<c0221184>]
(drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8)
[ 1.300293] [<c0221184>]
(drm_atomic_helper_update_legacy_modeset_state) from [<c0221548>]
(drm_atomic_helper_commit_modeset_disables+0x208/0x364)
[ 1.300305] [<c0221548>] (drm_atomic_helper_commit_modeset_disables)
from [<c0222248>] (drm_atomic_helper_commit+0xf8/0x140)
[ 1.300320] [<c0222248>] (drm_atomic_helper_commit) from [<c02404cc>]
(drm_atomic_commit+0x50/0x60)
[ 1.300333] [<c02404cc>] (drm_atomic_commit) from [<c0223180>]
(restore_fbdev_mode+0xf4/0x278)
[ 1.300345] [<c0223180>] (restore_fbdev_mode) from [<c0224164>]
(drm_fb_helper_restore_fbdev_mode_unlocked+0x30/0x68)
[ 1.300357] [<c0224164>] (drm_fb_helper_restore_fbdev_mode_unlocked)
from [<c02241d8>] (drm_fb_helper_set_par+0x3c/0x54)
[ 1.300368] [<c02241d8>] (drm_fb_helper_set_par) from [<c022411c>]
(drm_fb_helper_hotplug_event+0xe4/0xfc)
[ 1.300382] [<c022411c>] (drm_fb_helper_hotplug_event) from
[<c021ade8>] (drm_kms_helper_hotplug_event+0x24/0x28)
[ 1.300396] [<c021ade8>] (drm_kms_helper_hotplug_event) from
[<c021af20>] (output_poll_execute+0x134/0x18c)
[ 1.300413] [<c021af20>] (output_poll_execute) from [<c00377c0>]
(process_one_work+0x1e0/0x318)
[ 1.300426] [<c00377c0>] (process_one_work) from [<c0037c70>]
(worker_thread+0x378/0x4c4)
[ 1.300438] [<c0037c70>] (worker_thread) from [<c003c69c>]
(kthread+0xdc/0xf0)
[ 1.300450] [<c003c69c>] (kthread) from [<c000f5d8>]
(ret_from_fork+0x14/0x3c)
I can't found who can set encoder->crtc value before atomic_commit,
what's going wrong?
--
Ｍark Yao
[-- Attachment #1.2: Type: text/html, Size: 4570 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-12 6:27 ` Mark yao
@ 2015-11-12 6:34 ` Mark yao
2015-11-12 10:34 ` Liviu Dudau
2015-11-12 12:26 ` Thierry Reding
1 sibling, 1 reply; 34+ messages in thread
From: Mark yao @ 2015-11-12 6:34 UTC (permalink / raw)
To: Thierry Reding, Liviu Dudau; +Cc: lkml, DRI devel
[-- Attachment #1.1: Type: text/plain, Size: 12243 bytes --]
On 2015年11月12日 14:27, Mark yao wrote:
> On 2015年11月11日 00:56, Thierry Reding wrote:
>> On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
>>> Hello,
>>>
>>> When booting my Juno board with the HDLCD driver that I have converted to
>>> atomic operations I'm getting the following warning:
>> Perhaps you can provide pointers to the source code, that might make it
>> easier for people to spot what's going wrong.
>>
>> Thierry
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> Hi Thierry
> I encountered the same problem as Liviu.
> I'm coverting rockchip drm to atomic api, when booting with hdmi
> connected, get under warning:
>
> [ 1.300156] WARNING: CPU: 0 PID: 26 at
> drivers/gpu/drm/drm_atomic_helper.c:674
> drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8()
> [ 1.300161] Modules linked in:
> [ 1.300171] CPU: 0 PID: 26 Comm: kworker/0:1 Not tainted 4.3.0-rc5+
> #160
> [ 1.300174] Hardware name: Rockchip (Device Tree)
> [ 1.300189] Workqueue: events output_poll_execute
> [ 1.300224] [<c0015e54>] (unwind_backtrace) from [<c00123cc>]
> (show_stack+0x10/0x14)
> [ 1.300241] [<c00123cc>] (show_stack) from [<c01a5980>]
> (dump_stack+0x6c/0x88)
> [ 1.300255] [<c01a5980>] (dump_stack) from [<c0024050>]
> (warn_slowpath_common+0x80/0xa8)
> [ 1.300265] [<c0024050>] (warn_slowpath_common) from [<c0024090>]
> (warn_slowpath_null+0x18/0x1c)
> [ 1.300277] [<c0024090>] (warn_slowpath_null) from [<c0221184>]
> (drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8)
> [ 1.300293] [<c0221184>]
> (drm_atomic_helper_update_legacy_modeset_state) from [<c0221548>]
> (drm_atomic_helper_commit_modeset_disables+0x208/0x364)
> [ 1.300305] [<c0221548>]
> (drm_atomic_helper_commit_modeset_disables) from [<c0222248>]
> (drm_atomic_helper_commit+0xf8/0x140)
> [ 1.300320] [<c0222248>] (drm_atomic_helper_commit) from
> [<c02404cc>] (drm_atomic_commit+0x50/0x60)
> [ 1.300333] [<c02404cc>] (drm_atomic_commit) from [<c0223180>]
> (restore_fbdev_mode+0xf4/0x278)
> [ 1.300345] [<c0223180>] (restore_fbdev_mode) from [<c0224164>]
> (drm_fb_helper_restore_fbdev_mode_unlocked+0x30/0x68)
> [ 1.300357] [<c0224164>]
> (drm_fb_helper_restore_fbdev_mode_unlocked) from [<c02241d8>]
> (drm_fb_helper_set_par+0x3c/0x54)
> [ 1.300368] [<c02241d8>] (drm_fb_helper_set_par) from [<c022411c>]
> (drm_fb_helper_hotplug_event+0xe4/0xfc)
> [ 1.300382] [<c022411c>] (drm_fb_helper_hotplug_event) from
> [<c021ade8>] (drm_kms_helper_hotplug_event+0x24/0x28)
> [ 1.300396] [<c021ade8>] (drm_kms_helper_hotplug_event) from
> [<c021af20>] (output_poll_execute+0x134/0x18c)
> [ 1.300413] [<c021af20>] (output_poll_execute) from [<c00377c0>]
> (process_one_work+0x1e0/0x318)
> [ 1.300426] [<c00377c0>] (process_one_work) from [<c0037c70>]
> (worker_thread+0x378/0x4c4)
> [ 1.300438] [<c0037c70>] (worker_thread) from [<c003c69c>]
> (kthread+0xdc/0xf0)
> [ 1.300450] [<c003c69c>] (kthread) from [<c000f5d8>]
> (ret_from_fork+0x14/0x3c)
>
> I can't found who can set encoder->crtc value before atomic_commit,
> what's going wrong?
>
> --
> Ｍark Yao
Hi
here is the message before warning happen (filtering by "dmesg |
grep drm"), Hope this helps:
[ 1.245700] [drm:drm_minor_register]
[ 1.245925] [drm:drm_minor_register] new minor registered 64
[ 1.245934] [drm:drm_minor_register]
[ 1.245942] [drm:drm_minor_register]
[ 1.246099] [drm:drm_minor_register] new minor registered 0
[ 1.272968] rockchip-drm display-subsystem: bound ff940000.vop (ops
vop_component_ops)
[ 1.294790] rockchip-drm display-subsystem: bound ff930000.vop (ops
vop_component_ops)
[ 1.295086] rockchip-drm display-subsystem: bound ff980000.hdmi (ops
dw_hdmi_rockchip_ops)
[ 1.295218] [drm:drm_sysfs_connector_add] adding "HDMI-A-1" to sysfs
[ 1.295222] [drm:drm_sysfs_hotplug_event] generating hotplug event
[ 1.295268] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[ 1.295270] [drm] No driver support for vblank timestamp query.
[ 1.295290] [drm:drm_helper_probe_single_connector_modes_merge_bits]
[CONNECTOR:29:HDMI-A-1]
[ 1.295304] [drm:drm_helper_probe_single_connector_modes_merge_bits]
[CONNECTOR:29:HDMI-A-1] status updated from 3 to 1
[ 1.295470] [drm:drm_do_probe_ddc_edid] drm: skipping non-existent
adapter rk3x-i2c
[ 1.295513] [drm:drm_mode_debug_printmodeline] Modeline 30:"640x480"
0 25175 640 656 752 800 480 490 492 525 0x40 0xa
[ 1.295518] [drm:drm_mode_prune_invalid] Not using 640x480 mode: BAD
[ 1.295531] [drm:drm_mode_debug_printmodeline] Modeline 33:"848x480"
0 33750 848 864 976 1088 480 486 494 517 0x40 0x5
[ 1.295535] [drm:drm_mode_prune_invalid] Not using 848x480 mode: BAD
[ 1.295543] [drm:drm_helper_probe_single_connector_modes_merge_bits]
[CONNECTOR:29:HDMI-A-1] probed modes :
[ 1.295555] [drm:drm_mode_debug_printmodeline] Modeline 34:"1024x768"
60 65000 1024 1048 1184 1344 768 771 777 806 0x40 0xa
[ 1.295564] [drm:drm_mode_debug_printmodeline] Modeline 32:"800x600"
60 40000 800 840 968 1056 600 601 605 628 0x40 0x5
[ 1.295573] [drm:drm_mode_debug_printmodeline] Modeline 31:"800x600"
56 36000 800 824 896 1024 600 601 603 625 0x40 0x5
[ 1.295581] [drm:drm_setup_crtcs]
[ 1.295594] [drm:drm_enable_connectors] connector 29 enabled? yes
[ 1.295601] [drm:drm_target_preferred] looking for cmdline mode on
connector 29
[ 1.295606] [drm:drm_target_preferred] looking for preferred mode on
connector 29 0
[ 1.295609] [drm:drm_target_preferred] found mode 1024x768
[ 1.295614] [drm:drm_setup_crtcs] picking CRTCs for 4096x4096 config
[ 1.295623] [drm:drm_setup_crtcs] desired mode 1024x768 set on crtc
20 (0,0)
[ 1.299403] [drm:rockchip_drm_fbdev_create] FB [1024x768]-24
kvaddr=f0121000 offset=0 size=3145728
[ 1.299512] [drm:drm_sysfs_hotplug_event] generating hotplug event
[ 1.299540] [drm:drm_fb_helper_hotplug_event]
[ 1.299546] [drm:drm_helper_probe_single_connector_modes_merge_bits]
[CONNECTOR:29:HDMI-A-1]
[ 1.299712] [drm:drm_do_probe_ddc_edid] drm: skipping non-existent
adapter rk3x-i2c
[ 1.299750] [drm:drm_mode_debug_printmodeline] Modeline 35:"640x480"
0 25175 640 656 752 800 480 490 492 525 0x40 0xa
[ 1.299755] [drm:drm_mode_prune_invalid] Not using 640x480 mode: BAD
[ 1.299764] [drm:drm_mode_debug_printmodeline] Modeline 38:"848x480"
0 33750 848 864 976 1088 480 486 494 517 0x40 0x5
[ 1.299769] [drm:drm_mode_prune_invalid] Not using 848x480 mode: BAD
[ 1.299776] [drm:drm_helper_probe_single_connector_modes_merge_bits]
[CONNECTOR:29:HDMI-A-1] probed modes :
[ 1.299786] [drm:drm_mode_debug_printmodeline] Modeline 34:"1024x768"
60 65000 1024 1048 1184 1344 768 771 777 806 0x40 0xa
[ 1.299795] [drm:drm_mode_debug_printmodeline] Modeline 32:"800x600"
60 40000 800 840 968 1056 600 601 605 628 0x40 0x5
[ 1.299804] [drm:drm_mode_debug_printmodeline] Modeline 31:"800x600"
56 36000 800 824 896 1024 600 601 603 625 0x40 0x5
[ 1.299811] [drm:drm_setup_crtcs]
[ 1.299818] [drm:drm_enable_connectors] connector 29 enabled? yes
[ 1.299822] [drm:drm_target_preferred] looking for cmdline mode on
connector 29
[ 1.299827] [drm:drm_target_preferred] looking for preferred mode on
connector 29 0
[ 1.299831] [drm:drm_target_preferred] found mode 1024x768
[ 1.299836] [drm:drm_setup_crtcs] picking CRTCs for 4096x4096 config
[ 1.299842] [drm:drm_setup_crtcs] desired mode 1024x768 set on crtc
20 (0,0)
[ 1.299857] [drm:drm_atomic_state_init] Allocated atomic state ee38ebc0
[ 1.299868] [drm:drm_atomic_get_plane_state] Added [PLANE:18]
ee38eac0 state to ee38ebc0
[ 1.299875] [drm:drm_atomic_get_plane_state] Added [PLANE:19]
ee38ea80 state to ee38ebc0
[ 1.299880] [drm:drm_atomic_set_crtc_for_plane] Link plane state
ee38ea80 to [NOCRTC]
[ 1.299885] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane
state ee38ea80
[ 1.299892] [drm:drm_atomic_get_plane_state] Added [PLANE:21]
ee38ea40 state to ee38ebc0
[ 1.299897] [drm:drm_atomic_set_crtc_for_plane] Link plane state
ee38ea40 to [NOCRTC]
[ 1.299901] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane
state ee38ea40
[ 1.299907] [drm:drm_atomic_get_plane_state] Added [PLANE:22]
ee38ea00 state to ee38ebc0
[ 1.299911] [drm:drm_atomic_set_crtc_for_plane] Link plane state
ee38ea00 to [NOCRTC]
[ 1.299916] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane
state ee38ea00
[ 1.299922] [drm:drm_atomic_get_plane_state] Added [PLANE:23]
ee38e9c0 state to ee38ebc0
[ 1.299929] [drm:drm_atomic_get_plane_state] Added [PLANE:24]
ee38e980 state to ee38ebc0
[ 1.299933] [drm:drm_atomic_set_crtc_for_plane] Link plane state
ee38e980 to [NOCRTC]
[ 1.299937] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane
state ee38e980
[ 1.299943] [drm:drm_atomic_get_plane_state] Added [PLANE:26]
ee38e940 state to ee38ebc0
[ 1.299947] [drm:drm_atomic_set_crtc_for_plane] Link plane state
ee38e940 to [NOCRTC]
[ 1.299951] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane
state ee38e940
[ 1.299958] [drm:drm_atomic_get_plane_state] Added [PLANE:27]
ee38e900 state to ee38ebc0
[ 1.299962] [drm:drm_atomic_set_crtc_for_plane] Link plane state
ee38e900 to [NOCRTC]
[ 1.299966] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane
state ee38e900
[ 1.299975] [drm:drm_atomic_get_crtc_state] Added [CRTC:20] ee340000
state to ee38ebc0
[ 1.299985] [drm:drm_atomic_set_mode_for_crtc] Set [MODE:1024x768]
for CRTC state ee340000
[ 1.299990] [drm:drm_atomic_set_crtc_for_plane] Link plane state
ee38eac0 to [CRTC:20]
[ 1.299995] [drm:drm_framebuffer_reference] ee20f640: FB ID: 33 (1)
[ 1.300000] [drm:drm_atomic_set_fb_for_plane] Set [FB:33] for plane
state ee38eac0
[ 1.300008] [drm:drm_atomic_get_connector_state] Added [CONNECTOR:29]
ee38e8c0 state to ee38ebc0
[ 1.300015] [drm:drm_atomic_add_affected_connectors] Adding all
current connectors for [CRTC:20] to ee38ebc0
[ 1.300021] [drm:drm_atomic_set_crtc_for_connector] Link connector
state ee38e8c0 to [CRTC:20]
[ 1.300033] [drm:drm_atomic_get_crtc_state] Added [CRTC:25] ee393e00
state to ee38ebc0
[ 1.300038] [drm:drm_atomic_set_mode_for_crtc] Set [NOMODE] for CRTC
state ee393e00
[ 1.300043] [drm:drm_atomic_set_crtc_for_plane] Link plane state
ee38e9c0 to [NOCRTC]
[ 1.300047] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane
state ee38e9c0
[ 1.300053] [drm:drm_atomic_add_affected_connectors] Adding all
current connectors for [CRTC:20] to ee38ebc0
[ 1.300059] [drm:drm_atomic_add_affected_connectors] Adding all
current connectors for [CRTC:25] to ee38ebc0
[ 1.300067] [drm:drm_atomic_connectors_for_crtc] State ee38ebc0 has 1
connectors for [CRTC:20]
[ 1.300071] [drm:drm_atomic_check_only] checking ee38ebc0
[ 1.300079] [drm:drm_atomic_helper_check_modeset] [CRTC:20] mode changed
[ 1.300083] [drm:drm_atomic_helper_check_modeset] [CRTC:20] enable
changed
[ 1.300088] [drm:update_connector_routing] Updating routing for
[CONNECTOR:29:HDMI-A-1]
[ 1.300095] [drm:update_connector_routing] [CONNECTOR:29:HDMI-A-1]
using [ENCODER:28:TMDS-28] on [CRTC:20]
[ 1.300100] [drm:drm_atomic_helper_check_modeset] [CRTC:20] active
changed
[ 1.300105] [drm:drm_atomic_helper_check_modeset] [CRTC:20] needs all
connectors, enable: y, active: y
[ 1.300111] [drm:drm_atomic_add_affected_connectors] Adding all
current connectors for [CRTC:20] to ee38ebc0
[ 1.300117] [drm:drm_atomic_connectors_for_crtc] State ee38ebc0 has 1
connectors for [CRTC:20]
[ 1.300130] [drm:drm_atomic_commit] commiting ee38ebc0
[ 1.300156] WARNING: CPU: 0 PID: 26 at
drivers/gpu/drm/drm_atomic_helper.c:674
drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8()
--
Ｍark Yao
[-- Attachment #1.2: Type: text/html, Size: 15437 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-10 15:01 ` Liviu Dudau
(?)
(?)
@ 2015-11-12 8:32 ` Mark yao
2015-11-12 10:36 ` Liviu Dudau
-1 siblings, 1 reply; 34+ messages in thread
From: Mark yao @ 2015-11-12 8:32 UTC (permalink / raw)
To: Liviu Dudau, Daniel Vetter, David Airlie, Rob Clark, DRI devel, lkml
[-- Attachment #1.1: Type: text/plain, Size: 5419 bytes --]
On 2015年11月10日 23:01, Liviu Dudau wrote:
> Hello,
>
> When booting my Juno board with the HDLCD driver that I have converted to
> atomic operations I'm getting the following warning:
>
> ------------[ cut here ]------------
> WARNING: at /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674
> Modules linked in: hdlcd(+) clk_scpi
>
> CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5
> Hardware name: ARM Juno development board (r0) (DT)
> task: ffffffc974888b00 ti: ffffffc9755dc000 task.ti: ffffffc9755dc000
> PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> pc : [<ffffffc0003e6d18>] lr : [<ffffffc0003e8214>] pstate: 20000145
> sp : ffffffc9755df430
> x29: ffffffc9755df430 x28: ffffffc975703600
> x27: 0000000000000000 x26: ffffffc976253960
> x25: ffffffc976254040 x24: ffffffc000819000
> x23: ffffffc000689ea0 x22: ffffffc976251800
> x21: ffffffc976251800 x20: 0000000000000000
> x19: ffffffc9766b1f80 x18: 00000000715fe015
> x17: 0000007fb4b855b0 x16: 0000000000000220
> x15: 0000000000000001 x14: 0ffffffffffffffe
> x13: 0000000000000008 x12: 0101010101010101
> x11: ffffffc000964000 x10: ffffffc0009d2000
> x9 : 0000000000000000 x8 : ffffffc97ff5f700
> x7 : ffffffc97566cb80 x6 : ffffffc9766b1700
> x5 : ffffffc975665100 x4 : 0000000000000000
> x3 : ffffffc976253960 x2 : ffffffc97566cd00
> x1 : ffffffc976253900 x0 : 0000000000000000
>
> ---[ end trace 9fe289f798e7178e ]---
> Call trace:
> [<ffffffc0003e6d18>] drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> [<ffffffc0003e8214>] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> [<ffffffc0003e84c8>] drm_atomic_helper_commit+0xe0/0x150
> [<ffffffc00040bcc0>] drm_atomic_commit+0x40/0x6c
> [<ffffffc0003e92c0>] restore_fbdev_mode+0x294/0x2d4
> [<ffffffc0003eb6ec>] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c
> [<ffffffc0003eb770>] drm_fb_helper_set_par+0x2c/0x58
> [<ffffffc000378048>] fbcon_init+0x4d4/0x534
> [<ffffffc0003b44f4>] visual_init+0xac/0x104
> [<ffffffc0003b63f8>] do_bind_con_driver+0x16c/0x398
> [<ffffffc0003b6968>] do_take_over_console+0xd8/0x1f4
> [<ffffffc00037811c>] do_fbcon_takeover+0x74/0xf8
> [<ffffffc00037c5cc>] fbcon_event_notify+0x8a4/0x8f8
> [<ffffffc0000d19f0>] notifier_call_chain+0x4c/0x88
> [<ffffffc0000d1d78>] __blocking_notifier_call_chain+0x44/0x74
> [<ffffffc0000d1dbc>] blocking_notifier_call_chain+0x14/0x1c
> [<ffffffc00037d614>] fb_notifier_call_chain+0x1c/0x24
> [<ffffffc00037f8a8>] register_framebuffer+0x1c0/0x2ac
> [<ffffffc0003eb9f8>] drm_fb_helper_initial_config+0x25c/0x3ec
> [<ffffffc0003ec28c>] drm_fbdev_cma_init+0x98/0x134
> [<ffffffbffc006780>] hdlcd_drm_bind+0x180/0x498 [hdlcd]
> [<ffffffc0004158e0>] try_to_bring_up_master.part.5+0xd4/0x118
> [<ffffffc0004159e8>] component_master_add_with_match+0xc4/0x148
> [<ffffffc000415a7c>] component_master_add+0x10/0x18
> [<ffffffbffc0065ec>] hdlcd_probe+0x14/0x28 [hdlcd]
> [<ffffffc00041c504>] platform_drv_probe+0x54/0xc0
> [<ffffffc00041a9b0>] driver_probe_device+0x1ec/0x2e8
> [<ffffffc00041ab48>] __driver_attach+0x9c/0xa0
> [<ffffffc000418bcc>] bus_for_each_dev+0x58/0x98
> [<ffffffc00041a41c>] driver_attach+0x20/0x28
> [<ffffffc00041a058>] bus_add_driver+0x1c8/0x22c
> [<ffffffc00041b3d0>] driver_register+0x68/0x108
> [<ffffffc00041c408>] __platform_driver_register+0x4c/0x54
> [<ffffffbffc00b018>] hdlcd_init+0x18/0x30 [hdlcd]
> [<ffffffc00008293c>] do_one_initcall+0x90/0x1a8
> [<ffffffc0001424dc>] do_init_module+0x60/0x1c8
> [<ffffffc00011d364>] load_module+0x1554/0x1c98
> [<ffffffc00011dc64>] SyS_finit_module+0x7c/0x88
> [<ffffffc000085cb0>] el0_svc_naked+0x24/0x28
>
>
> The line that triggers the warning is:
>
> 674: WARN_ON(!connector->encoder->crtc);
>
> As far as I can see the encoder->crtc value is being set to a non-NULL value only
> in two places:
> - in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON()
> encoder->crtc = connector->state->crtc;
> - in drm_crtc_helper_set_config(drm_mode_set *set):
> encoder->crtc = new_crtc;
>
> Nothing in the call path from drm_atomic_commit() calls crtc_funcs->set_config() or
> drm_crtc_helper_set_config() directly, so the question is if this WARN() is actually
> valid.
>
> Call path from drm_atomic_commit:
>
> drm_atomic_helper_commit()
> - drm_atomic_helper_prepare_planes()
> - drm_atomic_helper_swap_state()
> - drm_atomic_helper_commit_modeset_disables()
> - disable_outputs()
> - drm_atomic_helper_update_legacy_modeset_state()
> - WARN_ON(!connector->encoder->crtc)
>
> Best regards,
> Liviu
>
Hi Liviu
I solved this problem by following change, you can check if your
driver do the same things:
drivers/gpu/drm/bridge/dw_hdmi.c:
- hdmi->connector.encoder = encoder;
+// hdmi->connector.encoder = encoder;
drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
I found some other drivers set connector.encoder before
drm_mode_connector_attach_encoder, some are not.
drm_mode_connector_attach_encoder already describe the link of
connector and encoder,
so I think "connector.encoder = encoder" is not needed, is that right?
Thanks.
--
Ｍark Yao
[-- Attachment #1.2: Type: text/html, Size: 6206 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-12 6:34 ` Mark yao
@ 2015-11-12 10:34 ` Liviu Dudau
0 siblings, 0 replies; 34+ messages in thread
From: Liviu Dudau @ 2015-11-12 10:34 UTC (permalink / raw)
To: Mark yao; +Cc: Thierry Reding, DRI devel, lkml
On Thu, Nov 12, 2015 at 02:34:57PM +0800, Mark yao wrote:
Mark,
> On 2015年11月12日 14:27, Mark yao wrote:
>
> On 2015年11月11日 00:56, Thierry Reding wrote:
>
> On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
>
> Hello,
>
> When booting my Juno board with the HDLCD driver that I have converted to
> atomic operations I'm getting the following warning:
>
> Perhaps you can provide pointers to the source code, that might make it
> easier for people to spot what's going wrong.
>
> Thierry
>
Can you switch your email client to text mode and make it do proper reply quoting? It is
rather difficult to follow where your reply comes when you don't use HTML MUAs.
Best regards,
Liviu
> _______________________________________________
> dri-devel mailing list
> [1]dri-devel@lists.freedesktop.org
> [2]http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> Hi Thierry
> I encountered the same problem as Liviu.
> I'm coverting rockchip drm to atomic api, when booting with hdmi connected, get under warning:
>
> [ 1.300156] WARNING: CPU: 0 PID: 26 at drivers/gpu/drm/drm_atomic_helper.c:674 drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8()
> [ 1.300161] Modules linked in:
> [ 1.300171] CPU: 0 PID: 26 Comm: kworker/0:1 Not tainted 4.3.0-rc5+ #160
> [ 1.300174] Hardware name: Rockchip (Device Tree)
> [ 1.300189] Workqueue: events output_poll_execute
> [ 1.300224] [<c0015e54>] (unwind_backtrace) from [<c00123cc>] (show_stack+0x10/0x14)
> [ 1.300241] [<c00123cc>] (show_stack) from [<c01a5980>] (dump_stack+0x6c/0x88)
> [ 1.300255] [<c01a5980>] (dump_stack) from [<c0024050>] (warn_slowpath_common+0x80/0xa8)
> [ 1.300265] [<c0024050>] (warn_slowpath_common) from [<c0024090>] (warn_slowpath_null+0x18/0x1c)
> [ 1.300277] [<c0024090>] (warn_slowpath_null) from [<c0221184>] (drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8)
> [ 1.300293] [<c0221184>] (drm_atomic_helper_update_legacy_modeset_state) from [<c0221548>] (drm_atomic_helper_commit_modeset_disables+0x208/0x364)
> [ 1.300305] [<c0221548>] (drm_atomic_helper_commit_modeset_disables) from [<c0222248>] (drm_atomic_helper_commit+0xf8/0x140)
> [ 1.300320] [<c0222248>] (drm_atomic_helper_commit) from [<c02404cc>] (drm_atomic_commit+0x50/0x60)
> [ 1.300333] [<c02404cc>] (drm_atomic_commit) from [<c0223180>] (restore_fbdev_mode+0xf4/0x278)
> [ 1.300345] [<c0223180>] (restore_fbdev_mode) from [<c0224164>] (drm_fb_helper_restore_fbdev_mode_unlocked+0x30/0x68)
> [ 1.300357] [<c0224164>] (drm_fb_helper_restore_fbdev_mode_unlocked) from [<c02241d8>] (drm_fb_helper_set_par+0x3c/0x54)
> [ 1.300368] [<c02241d8>] (drm_fb_helper_set_par) from [<c022411c>] (drm_fb_helper_hotplug_event+0xe4/0xfc)
> [ 1.300382] [<c022411c>] (drm_fb_helper_hotplug_event) from [<c021ade8>] (drm_kms_helper_hotplug_event+0x24/0x28)
> [ 1.300396] [<c021ade8>] (drm_kms_helper_hotplug_event) from [<c021af20>] (output_poll_execute+0x134/0x18c)
> [ 1.300413] [<c021af20>] (output_poll_execute) from [<c00377c0>] (process_one_work+0x1e0/0x318)
> [ 1.300426] [<c00377c0>] (process_one_work) from [<c0037c70>] (worker_thread+0x378/0x4c4)
> [ 1.300438] [<c0037c70>] (worker_thread) from [<c003c69c>] (kthread+0xdc/0xf0)
> [ 1.300450] [<c003c69c>] (kthread) from [<c000f5d8>] (ret_from_fork+0x14/0x3c)
>
> I can't found who can set encoder->crtc value before atomic_commit, what's going wrong?
>
> --
> Ｍark Yao
>
> Hi
> here is the message before warning happen (filtering by "dmesg | grep drm"), Hope this helps:
>
> [ 1.245700] [drm:drm_minor_register]
> [ 1.245925] [drm:drm_minor_register] new minor registered 64
> [ 1.245934] [drm:drm_minor_register]
> [ 1.245942] [drm:drm_minor_register]
> [ 1.246099] [drm:drm_minor_register] new minor registered 0
> [ 1.272968] rockchip-drm display-subsystem: bound ff940000.vop (ops vop_component_ops)
> [ 1.294790] rockchip-drm display-subsystem: bound ff930000.vop (ops vop_component_ops)
> [ 1.295086] rockchip-drm display-subsystem: bound ff980000.hdmi (ops dw_hdmi_rockchip_ops)
> [ 1.295218] [drm:drm_sysfs_connector_add] adding "HDMI-A-1" to sysfs
> [ 1.295222] [drm:drm_sysfs_hotplug_event] generating hotplug event
> [ 1.295268] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [ 1.295270] [drm] No driver support for vblank timestamp query.
> [ 1.295290] [drm:drm_helper_probe_single_connector_modes_merge_bits] [CONNECTOR:29:HDMI-A-1]
> [ 1.295304] [drm:drm_helper_probe_single_connector_modes_merge_bits] [CONNECTOR:29:HDMI-A-1] status updated from 3 to 1
> [ 1.295470] [drm:drm_do_probe_ddc_edid] drm: skipping non-existent adapter rk3x-i2c
> [ 1.295513] [drm:drm_mode_debug_printmodeline] Modeline 30:"640x480" 0 25175 640 656 752 800 480 490 492 525 0x40 0xa
> [ 1.295518] [drm:drm_mode_prune_invalid] Not using 640x480 mode: BAD
> [ 1.295531] [drm:drm_mode_debug_printmodeline] Modeline 33:"848x480" 0 33750 848 864 976 1088 480 486 494 517 0x40 0x5
> [ 1.295535] [drm:drm_mode_prune_invalid] Not using 848x480 mode: BAD
> [ 1.295543] [drm:drm_helper_probe_single_connector_modes_merge_bits] [CONNECTOR:29:HDMI-A-1] probed modes :
> [ 1.295555] [drm:drm_mode_debug_printmodeline] Modeline 34:"1024x768" 60 65000 1024 1048 1184 1344 768 771 777 806 0x40 0xa
> [ 1.295564] [drm:drm_mode_debug_printmodeline] Modeline 32:"800x600" 60 40000 800 840 968 1056 600 601 605 628 0x40 0x5
> [ 1.295573] [drm:drm_mode_debug_printmodeline] Modeline 31:"800x600" 56 36000 800 824 896 1024 600 601 603 625 0x40 0x5
> [ 1.295581] [drm:drm_setup_crtcs]
> [ 1.295594] [drm:drm_enable_connectors] connector 29 enabled? yes
> [ 1.295601] [drm:drm_target_preferred] looking for cmdline mode on connector 29
> [ 1.295606] [drm:drm_target_preferred] looking for preferred mode on connector 29 0
> [ 1.295609] [drm:drm_target_preferred] found mode 1024x768
> [ 1.295614] [drm:drm_setup_crtcs] picking CRTCs for 4096x4096 config
> [ 1.295623] [drm:drm_setup_crtcs] desired mode 1024x768 set on crtc 20 (0,0)
> [ 1.299403] [drm:rockchip_drm_fbdev_create] FB [1024x768]-24 kvaddr=f0121000 offset=0 size=3145728
> [ 1.299512] [drm:drm_sysfs_hotplug_event] generating hotplug event
> [ 1.299540] [drm:drm_fb_helper_hotplug_event]
> [ 1.299546] [drm:drm_helper_probe_single_connector_modes_merge_bits] [CONNECTOR:29:HDMI-A-1]
> [ 1.299712] [drm:drm_do_probe_ddc_edid] drm: skipping non-existent adapter rk3x-i2c
> [ 1.299750] [drm:drm_mode_debug_printmodeline] Modeline 35:"640x480" 0 25175 640 656 752 800 480 490 492 525 0x40 0xa
> [ 1.299755] [drm:drm_mode_prune_invalid] Not using 640x480 mode: BAD
> [ 1.299764] [drm:drm_mode_debug_printmodeline] Modeline 38:"848x480" 0 33750 848 864 976 1088 480 486 494 517 0x40 0x5
> [ 1.299769] [drm:drm_mode_prune_invalid] Not using 848x480 mode: BAD
> [ 1.299776] [drm:drm_helper_probe_single_connector_modes_merge_bits] [CONNECTOR:29:HDMI-A-1] probed modes :
> [ 1.299786] [drm:drm_mode_debug_printmodeline] Modeline 34:"1024x768" 60 65000 1024 1048 1184 1344 768 771 777 806 0x40 0xa
> [ 1.299795] [drm:drm_mode_debug_printmodeline] Modeline 32:"800x600" 60 40000 800 840 968 1056 600 601 605 628 0x40 0x5
> [ 1.299804] [drm:drm_mode_debug_printmodeline] Modeline 31:"800x600" 56 36000 800 824 896 1024 600 601 603 625 0x40 0x5
> [ 1.299811] [drm:drm_setup_crtcs]
> [ 1.299818] [drm:drm_enable_connectors] connector 29 enabled? yes
> [ 1.299822] [drm:drm_target_preferred] looking for cmdline mode on connector 29
> [ 1.299827] [drm:drm_target_preferred] looking for preferred mode on connector 29 0
> [ 1.299831] [drm:drm_target_preferred] found mode 1024x768
> [ 1.299836] [drm:drm_setup_crtcs] picking CRTCs for 4096x4096 config
> [ 1.299842] [drm:drm_setup_crtcs] desired mode 1024x768 set on crtc 20 (0,0)
> [ 1.299857] [drm:drm_atomic_state_init] Allocated atomic state ee38ebc0
> [ 1.299868] [drm:drm_atomic_get_plane_state] Added [PLANE:18] ee38eac0 state to ee38ebc0
> [ 1.299875] [drm:drm_atomic_get_plane_state] Added [PLANE:19] ee38ea80 state to ee38ebc0
> [ 1.299880] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38ea80 to [NOCRTC]
> [ 1.299885] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ee38ea80
> [ 1.299892] [drm:drm_atomic_get_plane_state] Added [PLANE:21] ee38ea40 state to ee38ebc0
> [ 1.299897] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38ea40 to [NOCRTC]
> [ 1.299901] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ee38ea40
> [ 1.299907] [drm:drm_atomic_get_plane_state] Added [PLANE:22] ee38ea00 state to ee38ebc0
> [ 1.299911] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38ea00 to [NOCRTC]
> [ 1.299916] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ee38ea00
> [ 1.299922] [drm:drm_atomic_get_plane_state] Added [PLANE:23] ee38e9c0 state to ee38ebc0
> [ 1.299929] [drm:drm_atomic_get_plane_state] Added [PLANE:24] ee38e980 state to ee38ebc0
> [ 1.299933] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38e980 to [NOCRTC]
> [ 1.299937] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ee38e980
> [ 1.299943] [drm:drm_atomic_get_plane_state] Added [PLANE:26] ee38e940 state to ee38ebc0
> [ 1.299947] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38e940 to [NOCRTC]
> [ 1.299951] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ee38e940
> [ 1.299958] [drm:drm_atomic_get_plane_state] Added [PLANE:27] ee38e900 state to ee38ebc0
> [ 1.299962] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38e900 to [NOCRTC]
> [ 1.299966] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ee38e900
> [ 1.299975] [drm:drm_atomic_get_crtc_state] Added [CRTC:20] ee340000 state to ee38ebc0
> [ 1.299985] [drm:drm_atomic_set_mode_for_crtc] Set [MODE:1024x768] for CRTC state ee340000
> [ 1.299990] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38eac0 to [CRTC:20]
> [ 1.299995] [drm:drm_framebuffer_reference] ee20f640: FB ID: 33 (1)
> [ 1.300000] [drm:drm_atomic_set_fb_for_plane] Set [FB:33] for plane state ee38eac0
> [ 1.300008] [drm:drm_atomic_get_connector_state] Added [CONNECTOR:29] ee38e8c0 state to ee38ebc0
> [ 1.300015] [drm:drm_atomic_add_affected_connectors] Adding all current connectors for [CRTC:20] to ee38ebc0
> [ 1.300021] [drm:drm_atomic_set_crtc_for_connector] Link connector state ee38e8c0 to [CRTC:20]
> [ 1.300033] [drm:drm_atomic_get_crtc_state] Added [CRTC:25] ee393e00 state to ee38ebc0
> [ 1.300038] [drm:drm_atomic_set_mode_for_crtc] Set [NOMODE] for CRTC state ee393e00
> [ 1.300043] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38e9c0 to [NOCRTC]
> [ 1.300047] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ee38e9c0
> [ 1.300053] [drm:drm_atomic_add_affected_connectors] Adding all current connectors for [CRTC:20] to ee38ebc0
> [ 1.300059] [drm:drm_atomic_add_affected_connectors] Adding all current connectors for [CRTC:25] to ee38ebc0
> [ 1.300067] [drm:drm_atomic_connectors_for_crtc] State ee38ebc0 has 1 connectors for [CRTC:20]
> [ 1.300071] [drm:drm_atomic_check_only] checking ee38ebc0
> [ 1.300079] [drm:drm_atomic_helper_check_modeset] [CRTC:20] mode changed
> [ 1.300083] [drm:drm_atomic_helper_check_modeset] [CRTC:20] enable changed
> [ 1.300088] [drm:update_connector_routing] Updating routing for [CONNECTOR:29:HDMI-A-1]
> [ 1.300095] [drm:update_connector_routing] [CONNECTOR:29:HDMI-A-1] using [ENCODER:28:TMDS-28] on [CRTC:20]
> [ 1.300100] [drm:drm_atomic_helper_check_modeset] [CRTC:20] active changed
> [ 1.300105] [drm:drm_atomic_helper_check_modeset] [CRTC:20] needs all connectors, enable: y, active: y
> [ 1.300111] [drm:drm_atomic_add_affected_connectors] Adding all current connectors for [CRTC:20] to ee38ebc0
> [ 1.300117] [drm:drm_atomic_connectors_for_crtc] State ee38ebc0 has 1 connectors for [CRTC:20]
> [ 1.300130] [drm:drm_atomic_commit] commiting ee38ebc0
> [ 1.300156] WARNING: CPU: 0 PID: 26 at drivers/gpu/drm/drm_atomic_helper.c:674 drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8()
>
> --
> Ｍark Yao
>
> References
>
> Visible links
> 1. mailto:dri-devel@lists.freedesktop.org
> 2. http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-12 10:34 ` Liviu Dudau
0 siblings, 0 replies; 34+ messages in thread
From: Liviu Dudau @ 2015-11-12 10:34 UTC (permalink / raw)
To: Mark yao; +Cc: lkml, DRI devel
On Thu, Nov 12, 2015 at 02:34:57PM +0800, Mark yao wrote:
Mark,
> On 2015年11月12日 14:27, Mark yao wrote:
>
> On 2015年11月11日 00:56, Thierry Reding wrote:
>
> On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
>
> Hello,
>
> When booting my Juno board with the HDLCD driver that I have converted to
> atomic operations I'm getting the following warning:
>
> Perhaps you can provide pointers to the source code, that might make it
> easier for people to spot what's going wrong.
>
> Thierry
>
Can you switch your email client to text mode and make it do proper reply quoting? It is
rather difficult to follow where your reply comes when you don't use HTML MUAs.
Best regards,
Liviu
> _______________________________________________
> dri-devel mailing list
> [1]dri-devel@lists.freedesktop.org
> [2]http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> Hi Thierry
> I encountered the same problem as Liviu.
> I'm coverting rockchip drm to atomic api, when booting with hdmi connected, get under warning:
>
> [ 1.300156] WARNING: CPU: 0 PID: 26 at drivers/gpu/drm/drm_atomic_helper.c:674 drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8()
> [ 1.300161] Modules linked in:
> [ 1.300171] CPU: 0 PID: 26 Comm: kworker/0:1 Not tainted 4.3.0-rc5+ #160
> [ 1.300174] Hardware name: Rockchip (Device Tree)
> [ 1.300189] Workqueue: events output_poll_execute
> [ 1.300224] [<c0015e54>] (unwind_backtrace) from [<c00123cc>] (show_stack+0x10/0x14)
> [ 1.300241] [<c00123cc>] (show_stack) from [<c01a5980>] (dump_stack+0x6c/0x88)
> [ 1.300255] [<c01a5980>] (dump_stack) from [<c0024050>] (warn_slowpath_common+0x80/0xa8)
> [ 1.300265] [<c0024050>] (warn_slowpath_common) from [<c0024090>] (warn_slowpath_null+0x18/0x1c)
> [ 1.300277] [<c0024090>] (warn_slowpath_null) from [<c0221184>] (drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8)
> [ 1.300293] [<c0221184>] (drm_atomic_helper_update_legacy_modeset_state) from [<c0221548>] (drm_atomic_helper_commit_modeset_disables+0x208/0x364)
> [ 1.300305] [<c0221548>] (drm_atomic_helper_commit_modeset_disables) from [<c0222248>] (drm_atomic_helper_commit+0xf8/0x140)
> [ 1.300320] [<c0222248>] (drm_atomic_helper_commit) from [<c02404cc>] (drm_atomic_commit+0x50/0x60)
> [ 1.300333] [<c02404cc>] (drm_atomic_commit) from [<c0223180>] (restore_fbdev_mode+0xf4/0x278)
> [ 1.300345] [<c0223180>] (restore_fbdev_mode) from [<c0224164>] (drm_fb_helper_restore_fbdev_mode_unlocked+0x30/0x68)
> [ 1.300357] [<c0224164>] (drm_fb_helper_restore_fbdev_mode_unlocked) from [<c02241d8>] (drm_fb_helper_set_par+0x3c/0x54)
> [ 1.300368] [<c02241d8>] (drm_fb_helper_set_par) from [<c022411c>] (drm_fb_helper_hotplug_event+0xe4/0xfc)
> [ 1.300382] [<c022411c>] (drm_fb_helper_hotplug_event) from [<c021ade8>] (drm_kms_helper_hotplug_event+0x24/0x28)
> [ 1.300396] [<c021ade8>] (drm_kms_helper_hotplug_event) from [<c021af20>] (output_poll_execute+0x134/0x18c)
> [ 1.300413] [<c021af20>] (output_poll_execute) from [<c00377c0>] (process_one_work+0x1e0/0x318)
> [ 1.300426] [<c00377c0>] (process_one_work) from [<c0037c70>] (worker_thread+0x378/0x4c4)
> [ 1.300438] [<c0037c70>] (worker_thread) from [<c003c69c>] (kthread+0xdc/0xf0)
> [ 1.300450] [<c003c69c>] (kthread) from [<c000f5d8>] (ret_from_fork+0x14/0x3c)
>
> I can't found who can set encoder->crtc value before atomic_commit, what's going wrong?
>
> --
> Ｍark Yao
>
> Hi
> here is the message before warning happen (filtering by "dmesg | grep drm"), Hope this helps:
>
> [ 1.245700] [drm:drm_minor_register]
> [ 1.245925] [drm:drm_minor_register] new minor registered 64
> [ 1.245934] [drm:drm_minor_register]
> [ 1.245942] [drm:drm_minor_register]
> [ 1.246099] [drm:drm_minor_register] new minor registered 0
> [ 1.272968] rockchip-drm display-subsystem: bound ff940000.vop (ops vop_component_ops)
> [ 1.294790] rockchip-drm display-subsystem: bound ff930000.vop (ops vop_component_ops)
> [ 1.295086] rockchip-drm display-subsystem: bound ff980000.hdmi (ops dw_hdmi_rockchip_ops)
> [ 1.295218] [drm:drm_sysfs_connector_add] adding "HDMI-A-1" to sysfs
> [ 1.295222] [drm:drm_sysfs_hotplug_event] generating hotplug event
> [ 1.295268] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [ 1.295270] [drm] No driver support for vblank timestamp query.
> [ 1.295290] [drm:drm_helper_probe_single_connector_modes_merge_bits] [CONNECTOR:29:HDMI-A-1]
> [ 1.295304] [drm:drm_helper_probe_single_connector_modes_merge_bits] [CONNECTOR:29:HDMI-A-1] status updated from 3 to 1
> [ 1.295470] [drm:drm_do_probe_ddc_edid] drm: skipping non-existent adapter rk3x-i2c
> [ 1.295513] [drm:drm_mode_debug_printmodeline] Modeline 30:"640x480" 0 25175 640 656 752 800 480 490 492 525 0x40 0xa
> [ 1.295518] [drm:drm_mode_prune_invalid] Not using 640x480 mode: BAD
> [ 1.295531] [drm:drm_mode_debug_printmodeline] Modeline 33:"848x480" 0 33750 848 864 976 1088 480 486 494 517 0x40 0x5
> [ 1.295535] [drm:drm_mode_prune_invalid] Not using 848x480 mode: BAD
> [ 1.295543] [drm:drm_helper_probe_single_connector_modes_merge_bits] [CONNECTOR:29:HDMI-A-1] probed modes :
> [ 1.295555] [drm:drm_mode_debug_printmodeline] Modeline 34:"1024x768" 60 65000 1024 1048 1184 1344 768 771 777 806 0x40 0xa
> [ 1.295564] [drm:drm_mode_debug_printmodeline] Modeline 32:"800x600" 60 40000 800 840 968 1056 600 601 605 628 0x40 0x5
> [ 1.295573] [drm:drm_mode_debug_printmodeline] Modeline 31:"800x600" 56 36000 800 824 896 1024 600 601 603 625 0x40 0x5
> [ 1.295581] [drm:drm_setup_crtcs]
> [ 1.295594] [drm:drm_enable_connectors] connector 29 enabled? yes
> [ 1.295601] [drm:drm_target_preferred] looking for cmdline mode on connector 29
> [ 1.295606] [drm:drm_target_preferred] looking for preferred mode on connector 29 0
> [ 1.295609] [drm:drm_target_preferred] found mode 1024x768
> [ 1.295614] [drm:drm_setup_crtcs] picking CRTCs for 4096x4096 config
> [ 1.295623] [drm:drm_setup_crtcs] desired mode 1024x768 set on crtc 20 (0,0)
> [ 1.299403] [drm:rockchip_drm_fbdev_create] FB [1024x768]-24 kvaddr=f0121000 offset=0 size=3145728
> [ 1.299512] [drm:drm_sysfs_hotplug_event] generating hotplug event
> [ 1.299540] [drm:drm_fb_helper_hotplug_event]
> [ 1.299546] [drm:drm_helper_probe_single_connector_modes_merge_bits] [CONNECTOR:29:HDMI-A-1]
> [ 1.299712] [drm:drm_do_probe_ddc_edid] drm: skipping non-existent adapter rk3x-i2c
> [ 1.299750] [drm:drm_mode_debug_printmodeline] Modeline 35:"640x480" 0 25175 640 656 752 800 480 490 492 525 0x40 0xa
> [ 1.299755] [drm:drm_mode_prune_invalid] Not using 640x480 mode: BAD
> [ 1.299764] [drm:drm_mode_debug_printmodeline] Modeline 38:"848x480" 0 33750 848 864 976 1088 480 486 494 517 0x40 0x5
> [ 1.299769] [drm:drm_mode_prune_invalid] Not using 848x480 mode: BAD
> [ 1.299776] [drm:drm_helper_probe_single_connector_modes_merge_bits] [CONNECTOR:29:HDMI-A-1] probed modes :
> [ 1.299786] [drm:drm_mode_debug_printmodeline] Modeline 34:"1024x768" 60 65000 1024 1048 1184 1344 768 771 777 806 0x40 0xa
> [ 1.299795] [drm:drm_mode_debug_printmodeline] Modeline 32:"800x600" 60 40000 800 840 968 1056 600 601 605 628 0x40 0x5
> [ 1.299804] [drm:drm_mode_debug_printmodeline] Modeline 31:"800x600" 56 36000 800 824 896 1024 600 601 603 625 0x40 0x5
> [ 1.299811] [drm:drm_setup_crtcs]
> [ 1.299818] [drm:drm_enable_connectors] connector 29 enabled? yes
> [ 1.299822] [drm:drm_target_preferred] looking for cmdline mode on connector 29
> [ 1.299827] [drm:drm_target_preferred] looking for preferred mode on connector 29 0
> [ 1.299831] [drm:drm_target_preferred] found mode 1024x768
> [ 1.299836] [drm:drm_setup_crtcs] picking CRTCs for 4096x4096 config
> [ 1.299842] [drm:drm_setup_crtcs] desired mode 1024x768 set on crtc 20 (0,0)
> [ 1.299857] [drm:drm_atomic_state_init] Allocated atomic state ee38ebc0
> [ 1.299868] [drm:drm_atomic_get_plane_state] Added [PLANE:18] ee38eac0 state to ee38ebc0
> [ 1.299875] [drm:drm_atomic_get_plane_state] Added [PLANE:19] ee38ea80 state to ee38ebc0
> [ 1.299880] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38ea80 to [NOCRTC]
> [ 1.299885] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ee38ea80
> [ 1.299892] [drm:drm_atomic_get_plane_state] Added [PLANE:21] ee38ea40 state to ee38ebc0
> [ 1.299897] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38ea40 to [NOCRTC]
> [ 1.299901] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ee38ea40
> [ 1.299907] [drm:drm_atomic_get_plane_state] Added [PLANE:22] ee38ea00 state to ee38ebc0
> [ 1.299911] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38ea00 to [NOCRTC]
> [ 1.299916] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ee38ea00
> [ 1.299922] [drm:drm_atomic_get_plane_state] Added [PLANE:23] ee38e9c0 state to ee38ebc0
> [ 1.299929] [drm:drm_atomic_get_plane_state] Added [PLANE:24] ee38e980 state to ee38ebc0
> [ 1.299933] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38e980 to [NOCRTC]
> [ 1.299937] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ee38e980
> [ 1.299943] [drm:drm_atomic_get_plane_state] Added [PLANE:26] ee38e940 state to ee38ebc0
> [ 1.299947] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38e940 to [NOCRTC]
> [ 1.299951] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ee38e940
> [ 1.299958] [drm:drm_atomic_get_plane_state] Added [PLANE:27] ee38e900 state to ee38ebc0
> [ 1.299962] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38e900 to [NOCRTC]
> [ 1.299966] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ee38e900
> [ 1.299975] [drm:drm_atomic_get_crtc_state] Added [CRTC:20] ee340000 state to ee38ebc0
> [ 1.299985] [drm:drm_atomic_set_mode_for_crtc] Set [MODE:1024x768] for CRTC state ee340000
> [ 1.299990] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38eac0 to [CRTC:20]
> [ 1.299995] [drm:drm_framebuffer_reference] ee20f640: FB ID: 33 (1)
> [ 1.300000] [drm:drm_atomic_set_fb_for_plane] Set [FB:33] for plane state ee38eac0
> [ 1.300008] [drm:drm_atomic_get_connector_state] Added [CONNECTOR:29] ee38e8c0 state to ee38ebc0
> [ 1.300015] [drm:drm_atomic_add_affected_connectors] Adding all current connectors for [CRTC:20] to ee38ebc0
> [ 1.300021] [drm:drm_atomic_set_crtc_for_connector] Link connector state ee38e8c0 to [CRTC:20]
> [ 1.300033] [drm:drm_atomic_get_crtc_state] Added [CRTC:25] ee393e00 state to ee38ebc0
> [ 1.300038] [drm:drm_atomic_set_mode_for_crtc] Set [NOMODE] for CRTC state ee393e00
> [ 1.300043] [drm:drm_atomic_set_crtc_for_plane] Link plane state ee38e9c0 to [NOCRTC]
> [ 1.300047] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ee38e9c0
> [ 1.300053] [drm:drm_atomic_add_affected_connectors] Adding all current connectors for [CRTC:20] to ee38ebc0
> [ 1.300059] [drm:drm_atomic_add_affected_connectors] Adding all current connectors for [CRTC:25] to ee38ebc0
> [ 1.300067] [drm:drm_atomic_connectors_for_crtc] State ee38ebc0 has 1 connectors for [CRTC:20]
> [ 1.300071] [drm:drm_atomic_check_only] checking ee38ebc0
> [ 1.300079] [drm:drm_atomic_helper_check_modeset] [CRTC:20] mode changed
> [ 1.300083] [drm:drm_atomic_helper_check_modeset] [CRTC:20] enable changed
> [ 1.300088] [drm:update_connector_routing] Updating routing for [CONNECTOR:29:HDMI-A-1]
> [ 1.300095] [drm:update_connector_routing] [CONNECTOR:29:HDMI-A-1] using [ENCODER:28:TMDS-28] on [CRTC:20]
> [ 1.300100] [drm:drm_atomic_helper_check_modeset] [CRTC:20] active changed
> [ 1.300105] [drm:drm_atomic_helper_check_modeset] [CRTC:20] needs all connectors, enable: y, active: y
> [ 1.300111] [drm:drm_atomic_add_affected_connectors] Adding all current connectors for [CRTC:20] to ee38ebc0
> [ 1.300117] [drm:drm_atomic_connectors_for_crtc] State ee38ebc0 has 1 connectors for [CRTC:20]
> [ 1.300130] [drm:drm_atomic_commit] commiting ee38ebc0
> [ 1.300156] WARNING: CPU: 0 PID: 26 at drivers/gpu/drm/drm_atomic_helper.c:674 drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8()
>
> --
> Ｍark Yao
>
> References
>
> Visible links
> 1. mailto:dri-devel@lists.freedesktop.org
> 2. http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-12 8:32 ` Mark yao
@ 2015-11-12 10:36 ` Liviu Dudau
0 siblings, 0 replies; 34+ messages in thread
From: Liviu Dudau @ 2015-11-12 10:36 UTC (permalink / raw)
To: Mark yao; +Cc: Daniel Vetter, David Airlie, Rob Clark, DRI devel, lkml
On Thu, Nov 12, 2015 at 04:32:33PM +0800, Mark yao wrote:
> On 2015年11月10日 23:01, Liviu Dudau wrote:
>
> Hello,
>
> When booting my Juno board with the HDLCD driver that I have converted to
> atomic operations I'm getting the following warning:
>
> ------------[ cut here ]------------
> WARNING: at /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674
> Modules linked in: hdlcd(+) clk_scpi
>
> CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5
> Hardware name: ARM Juno development board (r0) (DT)
> task: ffffffc974888b00 ti: ffffffc9755dc000 task.ti: ffffffc9755dc000
> PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> pc : [<ffffffc0003e6d18>] lr : [<ffffffc0003e8214>] pstate: 20000145
> sp : ffffffc9755df430
> x29: ffffffc9755df430 x28: ffffffc975703600
> x27: 0000000000000000 x26: ffffffc976253960
> x25: ffffffc976254040 x24: ffffffc000819000
> x23: ffffffc000689ea0 x22: ffffffc976251800
> x21: ffffffc976251800 x20: 0000000000000000
> x19: ffffffc9766b1f80 x18: 00000000715fe015
> x17: 0000007fb4b855b0 x16: 0000000000000220
> x15: 0000000000000001 x14: 0ffffffffffffffe
> x13: 0000000000000008 x12: 0101010101010101
> x11: ffffffc000964000 x10: ffffffc0009d2000
> x9 : 0000000000000000 x8 : ffffffc97ff5f700
> x7 : ffffffc97566cb80 x6 : ffffffc9766b1700
> x5 : ffffffc975665100 x4 : 0000000000000000
> x3 : ffffffc976253960 x2 : ffffffc97566cd00
> x1 : ffffffc976253900 x0 : 0000000000000000
>
> ---[ end trace 9fe289f798e7178e ]---
> Call trace:
> [<ffffffc0003e6d18>] drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> [<ffffffc0003e8214>] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> [<ffffffc0003e84c8>] drm_atomic_helper_commit+0xe0/0x150
> [<ffffffc00040bcc0>] drm_atomic_commit+0x40/0x6c
> [<ffffffc0003e92c0>] restore_fbdev_mode+0x294/0x2d4
> [<ffffffc0003eb6ec>] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c
> [<ffffffc0003eb770>] drm_fb_helper_set_par+0x2c/0x58
> [<ffffffc000378048>] fbcon_init+0x4d4/0x534
> [<ffffffc0003b44f4>] visual_init+0xac/0x104
> [<ffffffc0003b63f8>] do_bind_con_driver+0x16c/0x398
> [<ffffffc0003b6968>] do_take_over_console+0xd8/0x1f4
> [<ffffffc00037811c>] do_fbcon_takeover+0x74/0xf8
> [<ffffffc00037c5cc>] fbcon_event_notify+0x8a4/0x8f8
> [<ffffffc0000d19f0>] notifier_call_chain+0x4c/0x88
> [<ffffffc0000d1d78>] __blocking_notifier_call_chain+0x44/0x74
> [<ffffffc0000d1dbc>] blocking_notifier_call_chain+0x14/0x1c
> [<ffffffc00037d614>] fb_notifier_call_chain+0x1c/0x24
> [<ffffffc00037f8a8>] register_framebuffer+0x1c0/0x2ac
> [<ffffffc0003eb9f8>] drm_fb_helper_initial_config+0x25c/0x3ec
> [<ffffffc0003ec28c>] drm_fbdev_cma_init+0x98/0x134
> [<ffffffbffc006780>] hdlcd_drm_bind+0x180/0x498 [hdlcd]
> [<ffffffc0004158e0>] try_to_bring_up_master.part.5+0xd4/0x118
> [<ffffffc0004159e8>] component_master_add_with_match+0xc4/0x148
> [<ffffffc000415a7c>] component_master_add+0x10/0x18
> [<ffffffbffc0065ec>] hdlcd_probe+0x14/0x28 [hdlcd]
> [<ffffffc00041c504>] platform_drv_probe+0x54/0xc0
> [<ffffffc00041a9b0>] driver_probe_device+0x1ec/0x2e8
> [<ffffffc00041ab48>] __driver_attach+0x9c/0xa0
> [<ffffffc000418bcc>] bus_for_each_dev+0x58/0x98
> [<ffffffc00041a41c>] driver_attach+0x20/0x28
> [<ffffffc00041a058>] bus_add_driver+0x1c8/0x22c
> [<ffffffc00041b3d0>] driver_register+0x68/0x108
> [<ffffffc00041c408>] __platform_driver_register+0x4c/0x54
> [<ffffffbffc00b018>] hdlcd_init+0x18/0x30 [hdlcd]
> [<ffffffc00008293c>] do_one_initcall+0x90/0x1a8
> [<ffffffc0001424dc>] do_init_module+0x60/0x1c8
> [<ffffffc00011d364>] load_module+0x1554/0x1c98
> [<ffffffc00011dc64>] SyS_finit_module+0x7c/0x88
> [<ffffffc000085cb0>] el0_svc_naked+0x24/0x28
>
>
> The line that triggers the warning is:
>
> 674: WARN_ON(!connector->encoder->crtc);
>
> As far as I can see the encoder->crtc value is being set to a non-NULL value only
> in two places:
> - in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON()
> encoder->crtc = connector->state->crtc;
> - in drm_crtc_helper_set_config(drm_mode_set *set):
> encoder->crtc = new_crtc;
>
> Nothing in the call path from drm_atomic_commit() calls crtc_funcs->set_config() or
> drm_crtc_helper_set_config() directly, so the question is if this WARN() is actually
> valid.
>
> Call path from drm_atomic_commit:
>
> drm_atomic_helper_commit()
> - drm_atomic_helper_prepare_planes()
> - drm_atomic_helper_swap_state()
> - drm_atomic_helper_commit_modeset_disables()
> - disable_outputs()
> - drm_atomic_helper_update_legacy_modeset_state()
> - WARN_ON(!connector->encoder->crtc)
>
> Best regards,
> Liviu
>
>
> Hi Liviu
> I solved this problem by following change, you can check if your driver do the same things:
> drivers/gpu/drm/bridge/dw_hdmi.c:
> - hdmi->connector.encoder = encoder;
> +// hdmi->connector.encoder = encoder;
>
> drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
>
> I found some other drivers set connector.encoder before drm_mode_connector_attach_encoder, some are not.
>
> drm_mode_connector_attach_encoder already describe the link of connector and encoder,
> so I think "connector.encoder = encoder" is not needed, is that right?
I'll have a look, thanks for pointing this. However, my setup uses the tda998x driver for encoder, so I will
have to go look there rather than in my driver.
Best regards,
Liviu
>
> Thanks.
>
> --
> Ｍark Yao
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-12 10:36 ` Liviu Dudau
0 siblings, 0 replies; 34+ messages in thread
From: Liviu Dudau @ 2015-11-12 10:36 UTC (permalink / raw)
To: Mark yao; +Cc: DRI devel, lkml
On Thu, Nov 12, 2015 at 04:32:33PM +0800, Mark yao wrote:
> On 2015年11月10日 23:01, Liviu Dudau wrote:
>
> Hello,
>
> When booting my Juno board with the HDLCD driver that I have converted to
> atomic operations I'm getting the following warning:
>
> ------------[ cut here ]------------
> WARNING: at /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674
> Modules linked in: hdlcd(+) clk_scpi
>
> CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5
> Hardware name: ARM Juno development board (r0) (DT)
> task: ffffffc974888b00 ti: ffffffc9755dc000 task.ti: ffffffc9755dc000
> PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> pc : [<ffffffc0003e6d18>] lr : [<ffffffc0003e8214>] pstate: 20000145
> sp : ffffffc9755df430
> x29: ffffffc9755df430 x28: ffffffc975703600
> x27: 0000000000000000 x26: ffffffc976253960
> x25: ffffffc976254040 x24: ffffffc000819000
> x23: ffffffc000689ea0 x22: ffffffc976251800
> x21: ffffffc976251800 x20: 0000000000000000
> x19: ffffffc9766b1f80 x18: 00000000715fe015
> x17: 0000007fb4b855b0 x16: 0000000000000220
> x15: 0000000000000001 x14: 0ffffffffffffffe
> x13: 0000000000000008 x12: 0101010101010101
> x11: ffffffc000964000 x10: ffffffc0009d2000
> x9 : 0000000000000000 x8 : ffffffc97ff5f700
> x7 : ffffffc97566cb80 x6 : ffffffc9766b1700
> x5 : ffffffc975665100 x4 : 0000000000000000
> x3 : ffffffc976253960 x2 : ffffffc97566cd00
> x1 : ffffffc976253900 x0 : 0000000000000000
>
> ---[ end trace 9fe289f798e7178e ]---
> Call trace:
> [<ffffffc0003e6d18>] drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> [<ffffffc0003e8214>] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> [<ffffffc0003e84c8>] drm_atomic_helper_commit+0xe0/0x150
> [<ffffffc00040bcc0>] drm_atomic_commit+0x40/0x6c
> [<ffffffc0003e92c0>] restore_fbdev_mode+0x294/0x2d4
> [<ffffffc0003eb6ec>] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c
> [<ffffffc0003eb770>] drm_fb_helper_set_par+0x2c/0x58
> [<ffffffc000378048>] fbcon_init+0x4d4/0x534
> [<ffffffc0003b44f4>] visual_init+0xac/0x104
> [<ffffffc0003b63f8>] do_bind_con_driver+0x16c/0x398
> [<ffffffc0003b6968>] do_take_over_console+0xd8/0x1f4
> [<ffffffc00037811c>] do_fbcon_takeover+0x74/0xf8
> [<ffffffc00037c5cc>] fbcon_event_notify+0x8a4/0x8f8
> [<ffffffc0000d19f0>] notifier_call_chain+0x4c/0x88
> [<ffffffc0000d1d78>] __blocking_notifier_call_chain+0x44/0x74
> [<ffffffc0000d1dbc>] blocking_notifier_call_chain+0x14/0x1c
> [<ffffffc00037d614>] fb_notifier_call_chain+0x1c/0x24
> [<ffffffc00037f8a8>] register_framebuffer+0x1c0/0x2ac
> [<ffffffc0003eb9f8>] drm_fb_helper_initial_config+0x25c/0x3ec
> [<ffffffc0003ec28c>] drm_fbdev_cma_init+0x98/0x134
> [<ffffffbffc006780>] hdlcd_drm_bind+0x180/0x498 [hdlcd]
> [<ffffffc0004158e0>] try_to_bring_up_master.part.5+0xd4/0x118
> [<ffffffc0004159e8>] component_master_add_with_match+0xc4/0x148
> [<ffffffc000415a7c>] component_master_add+0x10/0x18
> [<ffffffbffc0065ec>] hdlcd_probe+0x14/0x28 [hdlcd]
> [<ffffffc00041c504>] platform_drv_probe+0x54/0xc0
> [<ffffffc00041a9b0>] driver_probe_device+0x1ec/0x2e8
> [<ffffffc00041ab48>] __driver_attach+0x9c/0xa0
> [<ffffffc000418bcc>] bus_for_each_dev+0x58/0x98
> [<ffffffc00041a41c>] driver_attach+0x20/0x28
> [<ffffffc00041a058>] bus_add_driver+0x1c8/0x22c
> [<ffffffc00041b3d0>] driver_register+0x68/0x108
> [<ffffffc00041c408>] __platform_driver_register+0x4c/0x54
> [<ffffffbffc00b018>] hdlcd_init+0x18/0x30 [hdlcd]
> [<ffffffc00008293c>] do_one_initcall+0x90/0x1a8
> [<ffffffc0001424dc>] do_init_module+0x60/0x1c8
> [<ffffffc00011d364>] load_module+0x1554/0x1c98
> [<ffffffc00011dc64>] SyS_finit_module+0x7c/0x88
> [<ffffffc000085cb0>] el0_svc_naked+0x24/0x28
>
>
> The line that triggers the warning is:
>
> 674: WARN_ON(!connector->encoder->crtc);
>
> As far as I can see the encoder->crtc value is being set to a non-NULL value only
> in two places:
> - in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON()
> encoder->crtc = connector->state->crtc;
> - in drm_crtc_helper_set_config(drm_mode_set *set):
> encoder->crtc = new_crtc;
>
> Nothing in the call path from drm_atomic_commit() calls crtc_funcs->set_config() or
> drm_crtc_helper_set_config() directly, so the question is if this WARN() is actually
> valid.
>
> Call path from drm_atomic_commit:
>
> drm_atomic_helper_commit()
> - drm_atomic_helper_prepare_planes()
> - drm_atomic_helper_swap_state()
> - drm_atomic_helper_commit_modeset_disables()
> - disable_outputs()
> - drm_atomic_helper_update_legacy_modeset_state()
> - WARN_ON(!connector->encoder->crtc)
>
> Best regards,
> Liviu
>
>
> Hi Liviu
> I solved this problem by following change, you can check if your driver do the same things:
> drivers/gpu/drm/bridge/dw_hdmi.c:
> - hdmi->connector.encoder = encoder;
> +// hdmi->connector.encoder = encoder;
>
> drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
>
> I found some other drivers set connector.encoder before drm_mode_connector_attach_encoder, some are not.
>
> drm_mode_connector_attach_encoder already describe the link of connector and encoder,
> so I think "connector.encoder = encoder" is not needed, is that right?
I'll have a look, thanks for pointing this. However, my setup uses the tda998x driver for encoder, so I will
have to go look there rather than in my driver.
Best regards,
Liviu
>
> Thanks.
>
> --
> Ｍark Yao
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-12 10:36 ` Liviu Dudau
@ 2015-11-12 10:49 ` Mark yao
-1 siblings, 0 replies; 34+ messages in thread
From: Mark yao @ 2015-11-12 10:49 UTC (permalink / raw)
To: Liviu Dudau; +Cc: Daniel Vetter, David Airlie, Rob Clark, DRI devel, lkml
On 2015年11月12日 18:36, Liviu Dudau wrote:
> On Thu, Nov 12, 2015 at 04:32:33PM +0800, Mark yao wrote:
>> On 2015年11月10日 23:01, Liviu Dudau wrote:
>>
>> Hello,
>>
>> When booting my Juno board with the HDLCD driver that I have converted to
>> atomic operations I'm getting the following warning:
>>
>> ------------[ cut here ]------------
>> WARNING: at /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674
>> Modules linked in: hdlcd(+) clk_scpi
>>
>> CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5
>> Hardware name: ARM Juno development board (r0) (DT)
>> task: ffffffc974888b00 ti: ffffffc9755dc000 task.ti: ffffffc9755dc000
>> PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
>> LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
>> pc : [<ffffffc0003e6d18>] lr : [<ffffffc0003e8214>] pstate: 20000145
>> sp : ffffffc9755df430
>> x29: ffffffc9755df430 x28: ffffffc975703600
>> x27: 0000000000000000 x26: ffffffc976253960
>> x25: ffffffc976254040 x24: ffffffc000819000
>> x23: ffffffc000689ea0 x22: ffffffc976251800
>> x21: ffffffc976251800 x20: 0000000000000000
>> x19: ffffffc9766b1f80 x18: 00000000715fe015
>> x17: 0000007fb4b855b0 x16: 0000000000000220
>> x15: 0000000000000001 x14: 0ffffffffffffffe
>> x13: 0000000000000008 x12: 0101010101010101
>> x11: ffffffc000964000 x10: ffffffc0009d2000
>> x9 : 0000000000000000 x8 : ffffffc97ff5f700
>> x7 : ffffffc97566cb80 x6 : ffffffc9766b1700
>> x5 : ffffffc975665100 x4 : 0000000000000000
>> x3 : ffffffc976253960 x2 : ffffffc97566cd00
>> x1 : ffffffc976253900 x0 : 0000000000000000
>>
>> ---[ end trace 9fe289f798e7178e ]---
>> Call trace:
>> [<ffffffc0003e6d18>] drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
>> [<ffffffc0003e8214>] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
>> [<ffffffc0003e84c8>] drm_atomic_helper_commit+0xe0/0x150
>> [<ffffffc00040bcc0>] drm_atomic_commit+0x40/0x6c
>> [<ffffffc0003e92c0>] restore_fbdev_mode+0x294/0x2d4
>> [<ffffffc0003eb6ec>] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c
>> [<ffffffc0003eb770>] drm_fb_helper_set_par+0x2c/0x58
>> [<ffffffc000378048>] fbcon_init+0x4d4/0x534
>> [<ffffffc0003b44f4>] visual_init+0xac/0x104
>> [<ffffffc0003b63f8>] do_bind_con_driver+0x16c/0x398
>> [<ffffffc0003b6968>] do_take_over_console+0xd8/0x1f4
>> [<ffffffc00037811c>] do_fbcon_takeover+0x74/0xf8
>> [<ffffffc00037c5cc>] fbcon_event_notify+0x8a4/0x8f8
>> [<ffffffc0000d19f0>] notifier_call_chain+0x4c/0x88
>> [<ffffffc0000d1d78>] __blocking_notifier_call_chain+0x44/0x74
>> [<ffffffc0000d1dbc>] blocking_notifier_call_chain+0x14/0x1c
>> [<ffffffc00037d614>] fb_notifier_call_chain+0x1c/0x24
>> [<ffffffc00037f8a8>] register_framebuffer+0x1c0/0x2ac
>> [<ffffffc0003eb9f8>] drm_fb_helper_initial_config+0x25c/0x3ec
>> [<ffffffc0003ec28c>] drm_fbdev_cma_init+0x98/0x134
>> [<ffffffbffc006780>] hdlcd_drm_bind+0x180/0x498 [hdlcd]
>> [<ffffffc0004158e0>] try_to_bring_up_master.part.5+0xd4/0x118
>> [<ffffffc0004159e8>] component_master_add_with_match+0xc4/0x148
>> [<ffffffc000415a7c>] component_master_add+0x10/0x18
>> [<ffffffbffc0065ec>] hdlcd_probe+0x14/0x28 [hdlcd]
>> [<ffffffc00041c504>] platform_drv_probe+0x54/0xc0
>> [<ffffffc00041a9b0>] driver_probe_device+0x1ec/0x2e8
>> [<ffffffc00041ab48>] __driver_attach+0x9c/0xa0
>> [<ffffffc000418bcc>] bus_for_each_dev+0x58/0x98
>> [<ffffffc00041a41c>] driver_attach+0x20/0x28
>> [<ffffffc00041a058>] bus_add_driver+0x1c8/0x22c
>> [<ffffffc00041b3d0>] driver_register+0x68/0x108
>> [<ffffffc00041c408>] __platform_driver_register+0x4c/0x54
>> [<ffffffbffc00b018>] hdlcd_init+0x18/0x30 [hdlcd]
>> [<ffffffc00008293c>] do_one_initcall+0x90/0x1a8
>> [<ffffffc0001424dc>] do_init_module+0x60/0x1c8
>> [<ffffffc00011d364>] load_module+0x1554/0x1c98
>> [<ffffffc00011dc64>] SyS_finit_module+0x7c/0x88
>> [<ffffffc000085cb0>] el0_svc_naked+0x24/0x28
>>
>>
>> The line that triggers the warning is:
>>
>> 674: WARN_ON(!connector->encoder->crtc);
>>
>> As far as I can see the encoder->crtc value is being set to a non-NULL value only
>> in two places:
>> - in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON()
>> encoder->crtc = connector->state->crtc;
>> - in drm_crtc_helper_set_config(drm_mode_set *set):
>> encoder->crtc = new_crtc;
>>
>> Nothing in the call path from drm_atomic_commit() calls crtc_funcs->set_config() or
>> drm_crtc_helper_set_config() directly, so the question is if this WARN() is actually
>> valid.
>>
>> Call path from drm_atomic_commit:
>>
>> drm_atomic_helper_commit()
>> - drm_atomic_helper_prepare_planes()
>> - drm_atomic_helper_swap_state()
>> - drm_atomic_helper_commit_modeset_disables()
>> - disable_outputs()
>> - drm_atomic_helper_update_legacy_modeset_state()
>> - WARN_ON(!connector->encoder->crtc)
>>
>> Best regards,
>> Liviu
>>
>>
>> Hi Liviu
>> I solved this problem by following change, you can check if your driver do the same things:
>> drivers/gpu/drm/bridge/dw_hdmi.c:
>> - hdmi->connector.encoder = encoder;
>> +// hdmi->connector.encoder = encoder;
>>
>> drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
>>
>> I found some other drivers set connector.encoder before drm_mode_connector_attach_encoder, some are not.
>>
>> drm_mode_connector_attach_encoder already describe the link of connector and encoder,
>> so I think "connector.encoder = encoder" is not needed, is that right?
> I'll have a look, thanks for pointing this. However, my setup uses the tda998x driver for encoder, so I will
> have to go look there rather than in my driver.
>
> Best regards,
> Liviu
>
>> Thanks.
>>
>> --
>> Ｍark Yao
Hi Liviu
drivers/gpu/drm/i2c/tda998x_drv.c do the same things:
priv->connector.encoder = &priv->encoder;
drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
I believe must be same problem.
Thanks.
--
Ｍark Yao
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-12 10:49 ` Mark yao
0 siblings, 0 replies; 34+ messages in thread
From: Mark yao @ 2015-11-12 10:49 UTC (permalink / raw)
To: Liviu Dudau; +Cc: DRI devel, lkml
On 2015年11月12日 18:36, Liviu Dudau wrote:
> On Thu, Nov 12, 2015 at 04:32:33PM +0800, Mark yao wrote:
>> On 2015年11月10日 23:01, Liviu Dudau wrote:
>>
>> Hello,
>>
>> When booting my Juno board with the HDLCD driver that I have converted to
>> atomic operations I'm getting the following warning:
>>
>> ------------[ cut here ]------------
>> WARNING: at /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674
>> Modules linked in: hdlcd(+) clk_scpi
>>
>> CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5
>> Hardware name: ARM Juno development board (r0) (DT)
>> task: ffffffc974888b00 ti: ffffffc9755dc000 task.ti: ffffffc9755dc000
>> PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
>> LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
>> pc : [<ffffffc0003e6d18>] lr : [<ffffffc0003e8214>] pstate: 20000145
>> sp : ffffffc9755df430
>> x29: ffffffc9755df430 x28: ffffffc975703600
>> x27: 0000000000000000 x26: ffffffc976253960
>> x25: ffffffc976254040 x24: ffffffc000819000
>> x23: ffffffc000689ea0 x22: ffffffc976251800
>> x21: ffffffc976251800 x20: 0000000000000000
>> x19: ffffffc9766b1f80 x18: 00000000715fe015
>> x17: 0000007fb4b855b0 x16: 0000000000000220
>> x15: 0000000000000001 x14: 0ffffffffffffffe
>> x13: 0000000000000008 x12: 0101010101010101
>> x11: ffffffc000964000 x10: ffffffc0009d2000
>> x9 : 0000000000000000 x8 : ffffffc97ff5f700
>> x7 : ffffffc97566cb80 x6 : ffffffc9766b1700
>> x5 : ffffffc975665100 x4 : 0000000000000000
>> x3 : ffffffc976253960 x2 : ffffffc97566cd00
>> x1 : ffffffc976253900 x0 : 0000000000000000
>>
>> ---[ end trace 9fe289f798e7178e ]---
>> Call trace:
>> [<ffffffc0003e6d18>] drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
>> [<ffffffc0003e8214>] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
>> [<ffffffc0003e84c8>] drm_atomic_helper_commit+0xe0/0x150
>> [<ffffffc00040bcc0>] drm_atomic_commit+0x40/0x6c
>> [<ffffffc0003e92c0>] restore_fbdev_mode+0x294/0x2d4
>> [<ffffffc0003eb6ec>] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c
>> [<ffffffc0003eb770>] drm_fb_helper_set_par+0x2c/0x58
>> [<ffffffc000378048>] fbcon_init+0x4d4/0x534
>> [<ffffffc0003b44f4>] visual_init+0xac/0x104
>> [<ffffffc0003b63f8>] do_bind_con_driver+0x16c/0x398
>> [<ffffffc0003b6968>] do_take_over_console+0xd8/0x1f4
>> [<ffffffc00037811c>] do_fbcon_takeover+0x74/0xf8
>> [<ffffffc00037c5cc>] fbcon_event_notify+0x8a4/0x8f8
>> [<ffffffc0000d19f0>] notifier_call_chain+0x4c/0x88
>> [<ffffffc0000d1d78>] __blocking_notifier_call_chain+0x44/0x74
>> [<ffffffc0000d1dbc>] blocking_notifier_call_chain+0x14/0x1c
>> [<ffffffc00037d614>] fb_notifier_call_chain+0x1c/0x24
>> [<ffffffc00037f8a8>] register_framebuffer+0x1c0/0x2ac
>> [<ffffffc0003eb9f8>] drm_fb_helper_initial_config+0x25c/0x3ec
>> [<ffffffc0003ec28c>] drm_fbdev_cma_init+0x98/0x134
>> [<ffffffbffc006780>] hdlcd_drm_bind+0x180/0x498 [hdlcd]
>> [<ffffffc0004158e0>] try_to_bring_up_master.part.5+0xd4/0x118
>> [<ffffffc0004159e8>] component_master_add_with_match+0xc4/0x148
>> [<ffffffc000415a7c>] component_master_add+0x10/0x18
>> [<ffffffbffc0065ec>] hdlcd_probe+0x14/0x28 [hdlcd]
>> [<ffffffc00041c504>] platform_drv_probe+0x54/0xc0
>> [<ffffffc00041a9b0>] driver_probe_device+0x1ec/0x2e8
>> [<ffffffc00041ab48>] __driver_attach+0x9c/0xa0
>> [<ffffffc000418bcc>] bus_for_each_dev+0x58/0x98
>> [<ffffffc00041a41c>] driver_attach+0x20/0x28
>> [<ffffffc00041a058>] bus_add_driver+0x1c8/0x22c
>> [<ffffffc00041b3d0>] driver_register+0x68/0x108
>> [<ffffffc00041c408>] __platform_driver_register+0x4c/0x54
>> [<ffffffbffc00b018>] hdlcd_init+0x18/0x30 [hdlcd]
>> [<ffffffc00008293c>] do_one_initcall+0x90/0x1a8
>> [<ffffffc0001424dc>] do_init_module+0x60/0x1c8
>> [<ffffffc00011d364>] load_module+0x1554/0x1c98
>> [<ffffffc00011dc64>] SyS_finit_module+0x7c/0x88
>> [<ffffffc000085cb0>] el0_svc_naked+0x24/0x28
>>
>>
>> The line that triggers the warning is:
>>
>> 674: WARN_ON(!connector->encoder->crtc);
>>
>> As far as I can see the encoder->crtc value is being set to a non-NULL value only
>> in two places:
>> - in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON()
>> encoder->crtc = connector->state->crtc;
>> - in drm_crtc_helper_set_config(drm_mode_set *set):
>> encoder->crtc = new_crtc;
>>
>> Nothing in the call path from drm_atomic_commit() calls crtc_funcs->set_config() or
>> drm_crtc_helper_set_config() directly, so the question is if this WARN() is actually
>> valid.
>>
>> Call path from drm_atomic_commit:
>>
>> drm_atomic_helper_commit()
>> - drm_atomic_helper_prepare_planes()
>> - drm_atomic_helper_swap_state()
>> - drm_atomic_helper_commit_modeset_disables()
>> - disable_outputs()
>> - drm_atomic_helper_update_legacy_modeset_state()
>> - WARN_ON(!connector->encoder->crtc)
>>
>> Best regards,
>> Liviu
>>
>>
>> Hi Liviu
>> I solved this problem by following change, you can check if your driver do the same things:
>> drivers/gpu/drm/bridge/dw_hdmi.c:
>> - hdmi->connector.encoder = encoder;
>> +// hdmi->connector.encoder = encoder;
>>
>> drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
>>
>> I found some other drivers set connector.encoder before drm_mode_connector_attach_encoder, some are not.
>>
>> drm_mode_connector_attach_encoder already describe the link of connector and encoder,
>> so I think "connector.encoder = encoder" is not needed, is that right?
> I'll have a look, thanks for pointing this. However, my setup uses the tda998x driver for encoder, so I will
> have to go look there rather than in my driver.
>
> Best regards,
> Liviu
>
>> Thanks.
>>
>> --
>> Ｍark Yao
Hi Liviu
drivers/gpu/drm/i2c/tda998x_drv.c do the same things:
priv->connector.encoder = &priv->encoder;
drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
I believe must be same problem.
Thanks.
--
Ｍark Yao
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-12 10:34 ` Liviu Dudau
@ 2015-11-12 10:52 ` Mark yao
-1 siblings, 0 replies; 34+ messages in thread
From: Mark yao @ 2015-11-12 10:52 UTC (permalink / raw)
To: Liviu Dudau; +Cc: Thierry Reding, DRI devel, lkml
On 2015年11月12日 18:34, Liviu Dudau wrote:
> Can you switch your email client to text mode and make it do proper reply quoting? It is
> rather difficult to follow where your reply comes when you don't use HTML MUAs.
>
> Best regards,
> Liviu
Hi Liviu
I'm sorry about that, now I switch my email into text mode, but I
don't known whether it take effect, everything look the same.
Thanks.
--
Ｍark Yao
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-12 10:52 ` Mark yao
0 siblings, 0 replies; 34+ messages in thread
From: Mark yao @ 2015-11-12 10:52 UTC (permalink / raw)
To: Liviu Dudau; +Cc: lkml, DRI devel
On 2015年11月12日 18:34, Liviu Dudau wrote:
> Can you switch your email client to text mode and make it do proper reply quoting? It is
> rather difficult to follow where your reply comes when you don't use HTML MUAs.
>
> Best regards,
> Liviu
Hi Liviu
I'm sorry about that, now I switch my email into text mode, but I
don't known whether it take effect, everything look the same.
Thanks.
--
Ｍark Yao
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-11 16:09 ` Liviu Dudau
@ 2015-11-12 12:16 ` Thierry Reding
-1 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2015-11-12 12:16 UTC (permalink / raw)
To: Liviu Dudau; +Cc: Daniel Vetter, David Airlie, Rob Clark, DRI devel, lkml
[-- Attachment #1: Type: text/plain, Size: 2192 bytes --]
On Wed, Nov 11, 2015 at 04:09:42PM +0000, Liviu Dudau wrote:
> On Tue, Nov 10, 2015 at 05:56:15PM +0100, Thierry Reding wrote:
> > On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
> > > Hello,
> > >
> > > When booting my Juno board with the HDLCD driver that I have converted to
> > > atomic operations I'm getting the following warning:
> >
> > Perhaps you can provide pointers to the source code, that might make it
> > easier for people to spot what's going wrong.
> >
> > Thierry
>
> Hi Thierry,
>
> I have just pushed to the mailing lists my patch series. [1] [2]
>
> If you want to checkout the code I also have a branch here:
>
> git://git://linux-arm.org/linux-ld testing/hdlcd
Okay, so if I understand correctly you're using the tda998x as encoder/
connector attached to your new HDLCD driver. I think the problem isn't
so much that nothing has set up the CRTC for the encoder, but rather
that the tda998x encoder tries to statically associate the encoder with
the connector. While that might be correct in that it represents the
hardware state (i.e. there is no way to physically route it anywhere
else), the DRM logical state is that it's not connected until a complete
pipeline has been set up, in which case a CRTC would have been assigned
to the encoder.
If your setup were working correctly you'd never reach the WARN_ON
because the
if (connector->encoder) {
conditional (on line 673 in drivers/gpu/drm/drm_atomic_helper.c) would
have evaluated to false already, since logically there'd be no encoder
associated with the connector yet.
Does the patch below help? Let me know and I can produce a proper patch.
Thierry
--- >8 ---
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 896b6aaf8c4d..8b0a402190eb 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1453,7 +1453,6 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
if (ret)
goto err_sysfs;
- priv->connector.encoder = &priv->encoder;
drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
return 0;
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-12 12:16 ` Thierry Reding
0 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2015-11-12 12:16 UTC (permalink / raw)
To: Liviu Dudau; +Cc: DRI devel, lkml
[-- Attachment #1.1: Type: text/plain, Size: 2192 bytes --]
On Wed, Nov 11, 2015 at 04:09:42PM +0000, Liviu Dudau wrote:
> On Tue, Nov 10, 2015 at 05:56:15PM +0100, Thierry Reding wrote:
> > On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
> > > Hello,
> > >
> > > When booting my Juno board with the HDLCD driver that I have converted to
> > > atomic operations I'm getting the following warning:
> >
> > Perhaps you can provide pointers to the source code, that might make it
> > easier for people to spot what's going wrong.
> >
> > Thierry
>
> Hi Thierry,
>
> I have just pushed to the mailing lists my patch series. [1] [2]
>
> If you want to checkout the code I also have a branch here:
>
> git://git://linux-arm.org/linux-ld testing/hdlcd
Okay, so if I understand correctly you're using the tda998x as encoder/
connector attached to your new HDLCD driver. I think the problem isn't
so much that nothing has set up the CRTC for the encoder, but rather
that the tda998x encoder tries to statically associate the encoder with
the connector. While that might be correct in that it represents the
hardware state (i.e. there is no way to physically route it anywhere
else), the DRM logical state is that it's not connected until a complete
pipeline has been set up, in which case a CRTC would have been assigned
to the encoder.
If your setup were working correctly you'd never reach the WARN_ON
because the
if (connector->encoder) {
conditional (on line 673 in drivers/gpu/drm/drm_atomic_helper.c) would
have evaluated to false already, since logically there'd be no encoder
associated with the connector yet.
Does the patch below help? Let me know and I can produce a proper patch.
Thierry
--- >8 ---
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 896b6aaf8c4d..8b0a402190eb 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1453,7 +1453,6 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
if (ret)
goto err_sysfs;
- priv->connector.encoder = &priv->encoder;
drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
return 0;
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-12 6:27 ` Mark yao
@ 2015-11-12 12:26 ` Thierry Reding
2015-11-12 12:26 ` Thierry Reding
1 sibling, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2015-11-12 12:26 UTC (permalink / raw)
To: Mark yao; +Cc: Liviu Dudau, DRI devel, lkml
[-- Attachment #1: Type: text/plain, Size: 3972 bytes --]
On Thu, Nov 12, 2015 at 02:27:29PM +0800, Mark yao wrote:
> On 2015年11月11日 00:56, Thierry Reding wrote:
> >On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
> >>Hello,
> >>
> >>When booting my Juno board with the HDLCD driver that I have converted to
> >>atomic operations I'm getting the following warning:
> >Perhaps you can provide pointers to the source code, that might make it
> >easier for people to spot what's going wrong.
> >
> >Thierry
> >
> >
> >_______________________________________________
> >dri-devel mailing list
> >dri-devel@lists.freedesktop.org
> >http://lists.freedesktop.org/mailman/listinfo/dri-devel
> Hi Thierry
> I encountered the same problem as Liviu.
> I'm coverting rockchip drm to atomic api, when booting with hdmi
> connected, get under warning:
>
> [ 1.300156] WARNING: CPU: 0 PID: 26 at
> drivers/gpu/drm/drm_atomic_helper.c:674
> drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8()
> [ 1.300161] Modules linked in:
> [ 1.300171] CPU: 0 PID: 26 Comm: kworker/0:1 Not tainted 4.3.0-rc5+ #160
> [ 1.300174] Hardware name: Rockchip (Device Tree)
> [ 1.300189] Workqueue: events output_poll_execute
> [ 1.300224] [<c0015e54>] (unwind_backtrace) from [<c00123cc>]
> (show_stack+0x10/0x14)
> [ 1.300241] [<c00123cc>] (show_stack) from [<c01a5980>]
> (dump_stack+0x6c/0x88)
> [ 1.300255] [<c01a5980>] (dump_stack) from [<c0024050>]
> (warn_slowpath_common+0x80/0xa8)
> [ 1.300265] [<c0024050>] (warn_slowpath_common) from [<c0024090>]
> (warn_slowpath_null+0x18/0x1c)
> [ 1.300277] [<c0024090>] (warn_slowpath_null) from [<c0221184>]
> (drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8)
> [ 1.300293] [<c0221184>] (drm_atomic_helper_update_legacy_modeset_state)
> from [<c0221548>] (drm_atomic_helper_commit_modeset_disables+0x208/0x364)
> [ 1.300305] [<c0221548>] (drm_atomic_helper_commit_modeset_disables) from
> [<c0222248>] (drm_atomic_helper_commit+0xf8/0x140)
> [ 1.300320] [<c0222248>] (drm_atomic_helper_commit) from [<c02404cc>]
> (drm_atomic_commit+0x50/0x60)
> [ 1.300333] [<c02404cc>] (drm_atomic_commit) from [<c0223180>]
> (restore_fbdev_mode+0xf4/0x278)
> [ 1.300345] [<c0223180>] (restore_fbdev_mode) from [<c0224164>]
> (drm_fb_helper_restore_fbdev_mode_unlocked+0x30/0x68)
> [ 1.300357] [<c0224164>] (drm_fb_helper_restore_fbdev_mode_unlocked) from
> [<c02241d8>] (drm_fb_helper_set_par+0x3c/0x54)
> [ 1.300368] [<c02241d8>] (drm_fb_helper_set_par) from [<c022411c>]
> (drm_fb_helper_hotplug_event+0xe4/0xfc)
> [ 1.300382] [<c022411c>] (drm_fb_helper_hotplug_event) from [<c021ade8>]
> (drm_kms_helper_hotplug_event+0x24/0x28)
> [ 1.300396] [<c021ade8>] (drm_kms_helper_hotplug_event) from [<c021af20>]
> (output_poll_execute+0x134/0x18c)
> [ 1.300413] [<c021af20>] (output_poll_execute) from [<c00377c0>]
> (process_one_work+0x1e0/0x318)
> [ 1.300426] [<c00377c0>] (process_one_work) from [<c0037c70>]
> (worker_thread+0x378/0x4c4)
> [ 1.300438] [<c0037c70>] (worker_thread) from [<c003c69c>]
> (kthread+0xdc/0xf0)
> [ 1.300450] [<c003c69c>] (kthread) from [<c000f5d8>]
> (ret_from_fork+0x14/0x3c)
>
> I can't found who can set encoder->crtc value before atomic_commit, what's
> going wrong?
The explanation here is probably the same as the one I gave to Liviu.
Does the below help?
Thierry
--- >8 ---
diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index 56de9f1c95fc..ffef392ccc13 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -1648,8 +1648,6 @@ static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi)
drm_connector_init(drm, &hdmi->connector, &dw_hdmi_connector_funcs,
DRM_MODE_CONNECTOR_HDMIA);
- hdmi->connector.encoder = encoder;
-
drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
return 0;
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-12 12:26 ` Thierry Reding
0 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2015-11-12 12:26 UTC (permalink / raw)
To: Mark yao; +Cc: Liviu Dudau, lkml, DRI devel
[-- Attachment #1.1: Type: text/plain, Size: 3972 bytes --]
On Thu, Nov 12, 2015 at 02:27:29PM +0800, Mark yao wrote:
> On 2015年11月11日 00:56, Thierry Reding wrote:
> >On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
> >>Hello,
> >>
> >>When booting my Juno board with the HDLCD driver that I have converted to
> >>atomic operations I'm getting the following warning:
> >Perhaps you can provide pointers to the source code, that might make it
> >easier for people to spot what's going wrong.
> >
> >Thierry
> >
> >
> >_______________________________________________
> >dri-devel mailing list
> >dri-devel@lists.freedesktop.org
> >http://lists.freedesktop.org/mailman/listinfo/dri-devel
> Hi Thierry
> I encountered the same problem as Liviu.
> I'm coverting rockchip drm to atomic api, when booting with hdmi
> connected, get under warning:
>
> [ 1.300156] WARNING: CPU: 0 PID: 26 at
> drivers/gpu/drm/drm_atomic_helper.c:674
> drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8()
> [ 1.300161] Modules linked in:
> [ 1.300171] CPU: 0 PID: 26 Comm: kworker/0:1 Not tainted 4.3.0-rc5+ #160
> [ 1.300174] Hardware name: Rockchip (Device Tree)
> [ 1.300189] Workqueue: events output_poll_execute
> [ 1.300224] [<c0015e54>] (unwind_backtrace) from [<c00123cc>]
> (show_stack+0x10/0x14)
> [ 1.300241] [<c00123cc>] (show_stack) from [<c01a5980>]
> (dump_stack+0x6c/0x88)
> [ 1.300255] [<c01a5980>] (dump_stack) from [<c0024050>]
> (warn_slowpath_common+0x80/0xa8)
> [ 1.300265] [<c0024050>] (warn_slowpath_common) from [<c0024090>]
> (warn_slowpath_null+0x18/0x1c)
> [ 1.300277] [<c0024090>] (warn_slowpath_null) from [<c0221184>]
> (drm_atomic_helper_update_legacy_modeset_state+0x3c/0x1f8)
> [ 1.300293] [<c0221184>] (drm_atomic_helper_update_legacy_modeset_state)
> from [<c0221548>] (drm_atomic_helper_commit_modeset_disables+0x208/0x364)
> [ 1.300305] [<c0221548>] (drm_atomic_helper_commit_modeset_disables) from
> [<c0222248>] (drm_atomic_helper_commit+0xf8/0x140)
> [ 1.300320] [<c0222248>] (drm_atomic_helper_commit) from [<c02404cc>]
> (drm_atomic_commit+0x50/0x60)
> [ 1.300333] [<c02404cc>] (drm_atomic_commit) from [<c0223180>]
> (restore_fbdev_mode+0xf4/0x278)
> [ 1.300345] [<c0223180>] (restore_fbdev_mode) from [<c0224164>]
> (drm_fb_helper_restore_fbdev_mode_unlocked+0x30/0x68)
> [ 1.300357] [<c0224164>] (drm_fb_helper_restore_fbdev_mode_unlocked) from
> [<c02241d8>] (drm_fb_helper_set_par+0x3c/0x54)
> [ 1.300368] [<c02241d8>] (drm_fb_helper_set_par) from [<c022411c>]
> (drm_fb_helper_hotplug_event+0xe4/0xfc)
> [ 1.300382] [<c022411c>] (drm_fb_helper_hotplug_event) from [<c021ade8>]
> (drm_kms_helper_hotplug_event+0x24/0x28)
> [ 1.300396] [<c021ade8>] (drm_kms_helper_hotplug_event) from [<c021af20>]
> (output_poll_execute+0x134/0x18c)
> [ 1.300413] [<c021af20>] (output_poll_execute) from [<c00377c0>]
> (process_one_work+0x1e0/0x318)
> [ 1.300426] [<c00377c0>] (process_one_work) from [<c0037c70>]
> (worker_thread+0x378/0x4c4)
> [ 1.300438] [<c0037c70>] (worker_thread) from [<c003c69c>]
> (kthread+0xdc/0xf0)
> [ 1.300450] [<c003c69c>] (kthread) from [<c000f5d8>]
> (ret_from_fork+0x14/0x3c)
>
> I can't found who can set encoder->crtc value before atomic_commit, what's
> going wrong?
The explanation here is probably the same as the one I gave to Liviu.
Does the below help?
Thierry
--- >8 ---
diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index 56de9f1c95fc..ffef392ccc13 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -1648,8 +1648,6 @@ static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi)
drm_connector_init(drm, &hdmi->connector, &dw_hdmi_connector_funcs,
DRM_MODE_CONNECTOR_HDMIA);
- hdmi->connector.encoder = encoder;
-
drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
return 0;
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-12 10:49 ` Mark yao
@ 2015-11-12 13:34 ` Thierry Reding
-1 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2015-11-12 13:34 UTC (permalink / raw)
To: Mark yao; +Cc: Liviu Dudau, DRI devel, lkml
[-- Attachment #1: Type: text/plain, Size: 7848 bytes --]
On Thu, Nov 12, 2015 at 06:49:38PM +0800, Mark yao wrote:
> On 2015年11月12日 18:36, Liviu Dudau wrote:
> >On Thu, Nov 12, 2015 at 04:32:33PM +0800, Mark yao wrote:
> >> On 2015年11月10日 23:01, Liviu Dudau wrote:
> >>
> >> Hello,
> >>
> >> When booting my Juno board with the HDLCD driver that I have converted to
> >> atomic operations I'm getting the following warning:
> >>
> >> ------------[ cut here ]------------
> >> WARNING: at /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674
> >> Modules linked in: hdlcd(+) clk_scpi
> >>
> >> CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5
> >> Hardware name: ARM Juno development board (r0) (DT)
> >> task: ffffffc974888b00 ti: ffffffc9755dc000 task.ti: ffffffc9755dc000
> >> PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> >> LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> >> pc : [<ffffffc0003e6d18>] lr : [<ffffffc0003e8214>] pstate: 20000145
> >> sp : ffffffc9755df430
> >> x29: ffffffc9755df430 x28: ffffffc975703600
> >> x27: 0000000000000000 x26: ffffffc976253960
> >> x25: ffffffc976254040 x24: ffffffc000819000
> >> x23: ffffffc000689ea0 x22: ffffffc976251800
> >> x21: ffffffc976251800 x20: 0000000000000000
> >> x19: ffffffc9766b1f80 x18: 00000000715fe015
> >> x17: 0000007fb4b855b0 x16: 0000000000000220
> >> x15: 0000000000000001 x14: 0ffffffffffffffe
> >> x13: 0000000000000008 x12: 0101010101010101
> >> x11: ffffffc000964000 x10: ffffffc0009d2000
> >> x9 : 0000000000000000 x8 : ffffffc97ff5f700
> >> x7 : ffffffc97566cb80 x6 : ffffffc9766b1700
> >> x5 : ffffffc975665100 x4 : 0000000000000000
> >> x3 : ffffffc976253960 x2 : ffffffc97566cd00
> >> x1 : ffffffc976253900 x0 : 0000000000000000
> >>
> >> ---[ end trace 9fe289f798e7178e ]---
> >> Call trace:
> >> [<ffffffc0003e6d18>] drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> >> [<ffffffc0003e8214>] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> >> [<ffffffc0003e84c8>] drm_atomic_helper_commit+0xe0/0x150
> >> [<ffffffc00040bcc0>] drm_atomic_commit+0x40/0x6c
> >> [<ffffffc0003e92c0>] restore_fbdev_mode+0x294/0x2d4
> >> [<ffffffc0003eb6ec>] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c
> >> [<ffffffc0003eb770>] drm_fb_helper_set_par+0x2c/0x58
> >> [<ffffffc000378048>] fbcon_init+0x4d4/0x534
> >> [<ffffffc0003b44f4>] visual_init+0xac/0x104
> >> [<ffffffc0003b63f8>] do_bind_con_driver+0x16c/0x398
> >> [<ffffffc0003b6968>] do_take_over_console+0xd8/0x1f4
> >> [<ffffffc00037811c>] do_fbcon_takeover+0x74/0xf8
> >> [<ffffffc00037c5cc>] fbcon_event_notify+0x8a4/0x8f8
> >> [<ffffffc0000d19f0>] notifier_call_chain+0x4c/0x88
> >> [<ffffffc0000d1d78>] __blocking_notifier_call_chain+0x44/0x74
> >> [<ffffffc0000d1dbc>] blocking_notifier_call_chain+0x14/0x1c
> >> [<ffffffc00037d614>] fb_notifier_call_chain+0x1c/0x24
> >> [<ffffffc00037f8a8>] register_framebuffer+0x1c0/0x2ac
> >> [<ffffffc0003eb9f8>] drm_fb_helper_initial_config+0x25c/0x3ec
> >> [<ffffffc0003ec28c>] drm_fbdev_cma_init+0x98/0x134
> >> [<ffffffbffc006780>] hdlcd_drm_bind+0x180/0x498 [hdlcd]
> >> [<ffffffc0004158e0>] try_to_bring_up_master.part.5+0xd4/0x118
> >> [<ffffffc0004159e8>] component_master_add_with_match+0xc4/0x148
> >> [<ffffffc000415a7c>] component_master_add+0x10/0x18
> >> [<ffffffbffc0065ec>] hdlcd_probe+0x14/0x28 [hdlcd]
> >> [<ffffffc00041c504>] platform_drv_probe+0x54/0xc0
> >> [<ffffffc00041a9b0>] driver_probe_device+0x1ec/0x2e8
> >> [<ffffffc00041ab48>] __driver_attach+0x9c/0xa0
> >> [<ffffffc000418bcc>] bus_for_each_dev+0x58/0x98
> >> [<ffffffc00041a41c>] driver_attach+0x20/0x28
> >> [<ffffffc00041a058>] bus_add_driver+0x1c8/0x22c
> >> [<ffffffc00041b3d0>] driver_register+0x68/0x108
> >> [<ffffffc00041c408>] __platform_driver_register+0x4c/0x54
> >> [<ffffffbffc00b018>] hdlcd_init+0x18/0x30 [hdlcd]
> >> [<ffffffc00008293c>] do_one_initcall+0x90/0x1a8
> >> [<ffffffc0001424dc>] do_init_module+0x60/0x1c8
> >> [<ffffffc00011d364>] load_module+0x1554/0x1c98
> >> [<ffffffc00011dc64>] SyS_finit_module+0x7c/0x88
> >> [<ffffffc000085cb0>] el0_svc_naked+0x24/0x28
> >>
> >>
> >> The line that triggers the warning is:
> >>
> >> 674: WARN_ON(!connector->encoder->crtc);
> >>
> >> As far as I can see the encoder->crtc value is being set to a non-NULL value only
> >> in two places:
> >> - in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON()
> >> encoder->crtc = connector->state->crtc;
> >> - in drm_crtc_helper_set_config(drm_mode_set *set):
> >> encoder->crtc = new_crtc;
> >>
> >> Nothing in the call path from drm_atomic_commit() calls crtc_funcs->set_config() or
> >> drm_crtc_helper_set_config() directly, so the question is if this WARN() is actually
> >> valid.
> >>
> >> Call path from drm_atomic_commit:
> >>
> >> drm_atomic_helper_commit()
> >> - drm_atomic_helper_prepare_planes()
> >> - drm_atomic_helper_swap_state()
> >> - drm_atomic_helper_commit_modeset_disables()
> >> - disable_outputs()
> >> - drm_atomic_helper_update_legacy_modeset_state()
> >> - WARN_ON(!connector->encoder->crtc)
> >>
> >> Best regards,
> >> Liviu
> >>
> >>
> >> Hi Liviu
> >> I solved this problem by following change, you can check if your driver do the same things:
> >> drivers/gpu/drm/bridge/dw_hdmi.c:
> >> - hdmi->connector.encoder = encoder;
> >> +// hdmi->connector.encoder = encoder;
> >>
> >> drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
> >>
> >> I found some other drivers set connector.encoder before drm_mode_connector_attach_encoder, some are not.
> >>
> >> drm_mode_connector_attach_encoder already describe the link of connector and encoder,
> >> so I think "connector.encoder = encoder" is not needed, is that right?
> >I'll have a look, thanks for pointing this. However, my setup uses the tda998x driver for encoder, so I will
> >have to go look there rather than in my driver.
> >
> >Best regards,
> >Liviu
> >
> >> Thanks.
> >>
> >> --
> >> Ｍark Yao
> Hi Liviu
> drivers/gpu/drm/i2c/tda998x_drv.c do the same things:
> priv->connector.encoder = &priv->encoder;
> drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
>
> I believe must be same problem.
Oh, I hadn't noticed this subthread, but you came to the same conclusion
as I did. I have the below local change, which I think might be good to
have given that there are at least two drivers that got this wrong.
Thierry
--- >8 ---
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 24c5434abd1c..56d53106b32d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4983,6 +4983,20 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector,
{
int i;
+ /*
+ * In the past, drivers have attempted to model the static association
+ * of connector to encoder in simple connector/encoder devices using a
+ * direct assignment of connector->encoder = encoder. This connection
+ * is a logical one and the responsibility of the core, so drivers are
+ * expected not to mess with this.
+ *
+ * Note that the error return should've been enough here, but a large
+ * majority of drivers ignores the return value, so add in a big WARN
+ * to get people's attention.
+ */
+ if (WARN_ON(connector->encoder))
+ return -EINVAL;
+
for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
if (connector->encoder_ids[i] == 0) {
connector->encoder_ids[i] = encoder->base.id;
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-12 13:34 ` Thierry Reding
0 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2015-11-12 13:34 UTC (permalink / raw)
To: Mark yao; +Cc: Liviu Dudau, lkml, DRI devel
[-- Attachment #1.1: Type: text/plain, Size: 7848 bytes --]
On Thu, Nov 12, 2015 at 06:49:38PM +0800, Mark yao wrote:
> On 2015年11月12日 18:36, Liviu Dudau wrote:
> >On Thu, Nov 12, 2015 at 04:32:33PM +0800, Mark yao wrote:
> >> On 2015年11月10日 23:01, Liviu Dudau wrote:
> >>
> >> Hello,
> >>
> >> When booting my Juno board with the HDLCD driver that I have converted to
> >> atomic operations I'm getting the following warning:
> >>
> >> ------------[ cut here ]------------
> >> WARNING: at /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674
> >> Modules linked in: hdlcd(+) clk_scpi
> >>
> >> CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5
> >> Hardware name: ARM Juno development board (r0) (DT)
> >> task: ffffffc974888b00 ti: ffffffc9755dc000 task.ti: ffffffc9755dc000
> >> PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> >> LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> >> pc : [<ffffffc0003e6d18>] lr : [<ffffffc0003e8214>] pstate: 20000145
> >> sp : ffffffc9755df430
> >> x29: ffffffc9755df430 x28: ffffffc975703600
> >> x27: 0000000000000000 x26: ffffffc976253960
> >> x25: ffffffc976254040 x24: ffffffc000819000
> >> x23: ffffffc000689ea0 x22: ffffffc976251800
> >> x21: ffffffc976251800 x20: 0000000000000000
> >> x19: ffffffc9766b1f80 x18: 00000000715fe015
> >> x17: 0000007fb4b855b0 x16: 0000000000000220
> >> x15: 0000000000000001 x14: 0ffffffffffffffe
> >> x13: 0000000000000008 x12: 0101010101010101
> >> x11: ffffffc000964000 x10: ffffffc0009d2000
> >> x9 : 0000000000000000 x8 : ffffffc97ff5f700
> >> x7 : ffffffc97566cb80 x6 : ffffffc9766b1700
> >> x5 : ffffffc975665100 x4 : 0000000000000000
> >> x3 : ffffffc976253960 x2 : ffffffc97566cd00
> >> x1 : ffffffc976253900 x0 : 0000000000000000
> >>
> >> ---[ end trace 9fe289f798e7178e ]---
> >> Call trace:
> >> [<ffffffc0003e6d18>] drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> >> [<ffffffc0003e8214>] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> >> [<ffffffc0003e84c8>] drm_atomic_helper_commit+0xe0/0x150
> >> [<ffffffc00040bcc0>] drm_atomic_commit+0x40/0x6c
> >> [<ffffffc0003e92c0>] restore_fbdev_mode+0x294/0x2d4
> >> [<ffffffc0003eb6ec>] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c
> >> [<ffffffc0003eb770>] drm_fb_helper_set_par+0x2c/0x58
> >> [<ffffffc000378048>] fbcon_init+0x4d4/0x534
> >> [<ffffffc0003b44f4>] visual_init+0xac/0x104
> >> [<ffffffc0003b63f8>] do_bind_con_driver+0x16c/0x398
> >> [<ffffffc0003b6968>] do_take_over_console+0xd8/0x1f4
> >> [<ffffffc00037811c>] do_fbcon_takeover+0x74/0xf8
> >> [<ffffffc00037c5cc>] fbcon_event_notify+0x8a4/0x8f8
> >> [<ffffffc0000d19f0>] notifier_call_chain+0x4c/0x88
> >> [<ffffffc0000d1d78>] __blocking_notifier_call_chain+0x44/0x74
> >> [<ffffffc0000d1dbc>] blocking_notifier_call_chain+0x14/0x1c
> >> [<ffffffc00037d614>] fb_notifier_call_chain+0x1c/0x24
> >> [<ffffffc00037f8a8>] register_framebuffer+0x1c0/0x2ac
> >> [<ffffffc0003eb9f8>] drm_fb_helper_initial_config+0x25c/0x3ec
> >> [<ffffffc0003ec28c>] drm_fbdev_cma_init+0x98/0x134
> >> [<ffffffbffc006780>] hdlcd_drm_bind+0x180/0x498 [hdlcd]
> >> [<ffffffc0004158e0>] try_to_bring_up_master.part.5+0xd4/0x118
> >> [<ffffffc0004159e8>] component_master_add_with_match+0xc4/0x148
> >> [<ffffffc000415a7c>] component_master_add+0x10/0x18
> >> [<ffffffbffc0065ec>] hdlcd_probe+0x14/0x28 [hdlcd]
> >> [<ffffffc00041c504>] platform_drv_probe+0x54/0xc0
> >> [<ffffffc00041a9b0>] driver_probe_device+0x1ec/0x2e8
> >> [<ffffffc00041ab48>] __driver_attach+0x9c/0xa0
> >> [<ffffffc000418bcc>] bus_for_each_dev+0x58/0x98
> >> [<ffffffc00041a41c>] driver_attach+0x20/0x28
> >> [<ffffffc00041a058>] bus_add_driver+0x1c8/0x22c
> >> [<ffffffc00041b3d0>] driver_register+0x68/0x108
> >> [<ffffffc00041c408>] __platform_driver_register+0x4c/0x54
> >> [<ffffffbffc00b018>] hdlcd_init+0x18/0x30 [hdlcd]
> >> [<ffffffc00008293c>] do_one_initcall+0x90/0x1a8
> >> [<ffffffc0001424dc>] do_init_module+0x60/0x1c8
> >> [<ffffffc00011d364>] load_module+0x1554/0x1c98
> >> [<ffffffc00011dc64>] SyS_finit_module+0x7c/0x88
> >> [<ffffffc000085cb0>] el0_svc_naked+0x24/0x28
> >>
> >>
> >> The line that triggers the warning is:
> >>
> >> 674: WARN_ON(!connector->encoder->crtc);
> >>
> >> As far as I can see the encoder->crtc value is being set to a non-NULL value only
> >> in two places:
> >> - in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON()
> >> encoder->crtc = connector->state->crtc;
> >> - in drm_crtc_helper_set_config(drm_mode_set *set):
> >> encoder->crtc = new_crtc;
> >>
> >> Nothing in the call path from drm_atomic_commit() calls crtc_funcs->set_config() or
> >> drm_crtc_helper_set_config() directly, so the question is if this WARN() is actually
> >> valid.
> >>
> >> Call path from drm_atomic_commit:
> >>
> >> drm_atomic_helper_commit()
> >> - drm_atomic_helper_prepare_planes()
> >> - drm_atomic_helper_swap_state()
> >> - drm_atomic_helper_commit_modeset_disables()
> >> - disable_outputs()
> >> - drm_atomic_helper_update_legacy_modeset_state()
> >> - WARN_ON(!connector->encoder->crtc)
> >>
> >> Best regards,
> >> Liviu
> >>
> >>
> >> Hi Liviu
> >> I solved this problem by following change, you can check if your driver do the same things:
> >> drivers/gpu/drm/bridge/dw_hdmi.c:
> >> - hdmi->connector.encoder = encoder;
> >> +// hdmi->connector.encoder = encoder;
> >>
> >> drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
> >>
> >> I found some other drivers set connector.encoder before drm_mode_connector_attach_encoder, some are not.
> >>
> >> drm_mode_connector_attach_encoder already describe the link of connector and encoder,
> >> so I think "connector.encoder = encoder" is not needed, is that right?
> >I'll have a look, thanks for pointing this. However, my setup uses the tda998x driver for encoder, so I will
> >have to go look there rather than in my driver.
> >
> >Best regards,
> >Liviu
> >
> >> Thanks.
> >>
> >> --
> >> Ｍark Yao
> Hi Liviu
> drivers/gpu/drm/i2c/tda998x_drv.c do the same things:
> priv->connector.encoder = &priv->encoder;
> drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
>
> I believe must be same problem.
Oh, I hadn't noticed this subthread, but you came to the same conclusion
as I did. I have the below local change, which I think might be good to
have given that there are at least two drivers that got this wrong.
Thierry
--- >8 ---
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 24c5434abd1c..56d53106b32d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4983,6 +4983,20 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector,
{
int i;
+ /*
+ * In the past, drivers have attempted to model the static association
+ * of connector to encoder in simple connector/encoder devices using a
+ * direct assignment of connector->encoder = encoder. This connection
+ * is a logical one and the responsibility of the core, so drivers are
+ * expected not to mess with this.
+ *
+ * Note that the error return should've been enough here, but a large
+ * majority of drivers ignores the return value, so add in a big WARN
+ * to get people's attention.
+ */
+ if (WARN_ON(connector->encoder))
+ return -EINVAL;
+
for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
if (connector->encoder_ids[i] == 0) {
connector->encoder_ids[i] = encoder->base.id;
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-12 12:16 ` Thierry Reding
@ 2015-11-12 13:57 ` Liviu Dudau
-1 siblings, 0 replies; 34+ messages in thread
From: Liviu Dudau @ 2015-11-12 13:57 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, David Airlie, Rob Clark, DRI devel, lkml
On Thu, Nov 12, 2015 at 01:16:55PM +0100, Thierry Reding wrote:
> On Wed, Nov 11, 2015 at 04:09:42PM +0000, Liviu Dudau wrote:
> > On Tue, Nov 10, 2015 at 05:56:15PM +0100, Thierry Reding wrote:
> > > On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
> > > > Hello,
> > > >
> > > > When booting my Juno board with the HDLCD driver that I have converted to
> > > > atomic operations I'm getting the following warning:
> > >
> > > Perhaps you can provide pointers to the source code, that might make it
> > > easier for people to spot what's going wrong.
> > >
> > > Thierry
> >
> > Hi Thierry,
> >
> > I have just pushed to the mailing lists my patch series. [1] [2]
> >
> > If you want to checkout the code I also have a branch here:
> >
> > git://git://linux-arm.org/linux-ld testing/hdlcd
>
> Okay, so if I understand correctly you're using the tda998x as encoder/
> connector attached to your new HDLCD driver. I think the problem isn't
> so much that nothing has set up the CRTC for the encoder, but rather
> that the tda998x encoder tries to statically associate the encoder with
> the connector. While that might be correct in that it represents the
> hardware state (i.e. there is no way to physically route it anywhere
> else), the DRM logical state is that it's not connected until a complete
> pipeline has been set up, in which case a CRTC would have been assigned
> to the encoder.
>
> If your setup were working correctly you'd never reach the WARN_ON
> because the
>
> if (connector->encoder) {
>
> conditional (on line 673 in drivers/gpu/drm/drm_atomic_helper.c) would
> have evaluated to false already, since logically there'd be no encoder
> associated with the connector yet.
Your analysis is spot on and matches my conditions. However ...
>
> Does the patch below help? Let me know and I can produce a proper patch.
>
> Thierry
>
> --- >8 ---
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 896b6aaf8c4d..8b0a402190eb 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -1453,7 +1453,6 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
> if (ret)
> goto err_sysfs;
>
> - priv->connector.encoder = &priv->encoder;
while this patch helps (no WARN() printed) I'm not sure this is the right fix.
TDA998x is also used by armada DRM driver which has not been converted (yet) to
atomic modesetting. And judging by the code and comment of drm_connector_get_encoder()
in drm_crtc.c, having access to the encoder through the connector is the non-atomic
way of doing things:
static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *connector)
{
/* For atomic drivers only state objects are synchronously updated and
* protected by modeset locks, so check those first. */
if (connector->state)
return connector->state->best_encoder;
return connector->encoder;
}
To me, it looks like the WARN() is bogus when the atomic modesetting is used in
drm_atomic_helper_update_legacy_modeset_state(). Either we acknowledge that the "legacy"
code sets the connector in the encoder structure before complete pipeline is setup
and we remove the WARN(), or we find a better way of detecting that some sort of mixed
support is in place (i.e. atomic and non-atomic aware code running) and we clean up
in a way that is compatible with both worlds.
Best regards,
Liviu
> drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
>
> return 0;
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-12 13:57 ` Liviu Dudau
0 siblings, 0 replies; 34+ messages in thread
From: Liviu Dudau @ 2015-11-12 13:57 UTC (permalink / raw)
To: Thierry Reding; +Cc: DRI devel, lkml
On Thu, Nov 12, 2015 at 01:16:55PM +0100, Thierry Reding wrote:
> On Wed, Nov 11, 2015 at 04:09:42PM +0000, Liviu Dudau wrote:
> > On Tue, Nov 10, 2015 at 05:56:15PM +0100, Thierry Reding wrote:
> > > On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
> > > > Hello,
> > > >
> > > > When booting my Juno board with the HDLCD driver that I have converted to
> > > > atomic operations I'm getting the following warning:
> > >
> > > Perhaps you can provide pointers to the source code, that might make it
> > > easier for people to spot what's going wrong.
> > >
> > > Thierry
> >
> > Hi Thierry,
> >
> > I have just pushed to the mailing lists my patch series. [1] [2]
> >
> > If you want to checkout the code I also have a branch here:
> >
> > git://git://linux-arm.org/linux-ld testing/hdlcd
>
> Okay, so if I understand correctly you're using the tda998x as encoder/
> connector attached to your new HDLCD driver. I think the problem isn't
> so much that nothing has set up the CRTC for the encoder, but rather
> that the tda998x encoder tries to statically associate the encoder with
> the connector. While that might be correct in that it represents the
> hardware state (i.e. there is no way to physically route it anywhere
> else), the DRM logical state is that it's not connected until a complete
> pipeline has been set up, in which case a CRTC would have been assigned
> to the encoder.
>
> If your setup were working correctly you'd never reach the WARN_ON
> because the
>
> if (connector->encoder) {
>
> conditional (on line 673 in drivers/gpu/drm/drm_atomic_helper.c) would
> have evaluated to false already, since logically there'd be no encoder
> associated with the connector yet.
Your analysis is spot on and matches my conditions. However ...
>
> Does the patch below help? Let me know and I can produce a proper patch.
>
> Thierry
>
> --- >8 ---
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 896b6aaf8c4d..8b0a402190eb 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -1453,7 +1453,6 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
> if (ret)
> goto err_sysfs;
>
> - priv->connector.encoder = &priv->encoder;
while this patch helps (no WARN() printed) I'm not sure this is the right fix.
TDA998x is also used by armada DRM driver which has not been converted (yet) to
atomic modesetting. And judging by the code and comment of drm_connector_get_encoder()
in drm_crtc.c, having access to the encoder through the connector is the non-atomic
way of doing things:
static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *connector)
{
/* For atomic drivers only state objects are synchronously updated and
* protected by modeset locks, so check those first. */
if (connector->state)
return connector->state->best_encoder;
return connector->encoder;
}
To me, it looks like the WARN() is bogus when the atomic modesetting is used in
drm_atomic_helper_update_legacy_modeset_state(). Either we acknowledge that the "legacy"
code sets the connector in the encoder structure before complete pipeline is setup
and we remove the WARN(), or we find a better way of detecting that some sort of mixed
support is in place (i.e. atomic and non-atomic aware code running) and we clean up
in a way that is compatible with both worlds.
Best regards,
Liviu
> drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
>
> return 0;
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-12 13:34 ` Thierry Reding
(?)
@ 2015-11-12 14:03 ` Liviu Dudau
2015-11-12 16:12 ` Thierry Reding
-1 siblings, 1 reply; 34+ messages in thread
From: Liviu Dudau @ 2015-11-12 14:03 UTC (permalink / raw)
To: Thierry Reding; +Cc: Mark yao, DRI devel, lkml
On Thu, Nov 12, 2015 at 02:34:11PM +0100, Thierry Reding wrote:
> On Thu, Nov 12, 2015 at 06:49:38PM +0800, Mark yao wrote:
> > On 2015年11月12日 18:36, Liviu Dudau wrote:
> > >On Thu, Nov 12, 2015 at 04:32:33PM +0800, Mark yao wrote:
> > >> On 2015年11月10日 23:01, Liviu Dudau wrote:
> > >>
> > >> Hello,
> > >>
> > >> When booting my Juno board with the HDLCD driver that I have converted to
> > >> atomic operations I'm getting the following warning:
> > >>
> > >> ------------[ cut here ]------------
> > >> WARNING: at /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674
> > >> Modules linked in: hdlcd(+) clk_scpi
> > >>
> > >> CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5
> > >> Hardware name: ARM Juno development board (r0) (DT)
> > >> task: ffffffc974888b00 ti: ffffffc9755dc000 task.ti: ffffffc9755dc000
> > >> PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> > >> LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> > >> pc : [<ffffffc0003e6d18>] lr : [<ffffffc0003e8214>] pstate: 20000145
> > >> sp : ffffffc9755df430
> > >> x29: ffffffc9755df430 x28: ffffffc975703600
> > >> x27: 0000000000000000 x26: ffffffc976253960
> > >> x25: ffffffc976254040 x24: ffffffc000819000
> > >> x23: ffffffc000689ea0 x22: ffffffc976251800
> > >> x21: ffffffc976251800 x20: 0000000000000000
> > >> x19: ffffffc9766b1f80 x18: 00000000715fe015
> > >> x17: 0000007fb4b855b0 x16: 0000000000000220
> > >> x15: 0000000000000001 x14: 0ffffffffffffffe
> > >> x13: 0000000000000008 x12: 0101010101010101
> > >> x11: ffffffc000964000 x10: ffffffc0009d2000
> > >> x9 : 0000000000000000 x8 : ffffffc97ff5f700
> > >> x7 : ffffffc97566cb80 x6 : ffffffc9766b1700
> > >> x5 : ffffffc975665100 x4 : 0000000000000000
> > >> x3 : ffffffc976253960 x2 : ffffffc97566cd00
> > >> x1 : ffffffc976253900 x0 : 0000000000000000
> > >>
> > >> ---[ end trace 9fe289f798e7178e ]---
> > >> Call trace:
> > >> [<ffffffc0003e6d18>] drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> > >> [<ffffffc0003e8214>] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> > >> [<ffffffc0003e84c8>] drm_atomic_helper_commit+0xe0/0x150
> > >> [<ffffffc00040bcc0>] drm_atomic_commit+0x40/0x6c
> > >> [<ffffffc0003e92c0>] restore_fbdev_mode+0x294/0x2d4
> > >> [<ffffffc0003eb6ec>] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c
> > >> [<ffffffc0003eb770>] drm_fb_helper_set_par+0x2c/0x58
> > >> [<ffffffc000378048>] fbcon_init+0x4d4/0x534
> > >> [<ffffffc0003b44f4>] visual_init+0xac/0x104
> > >> [<ffffffc0003b63f8>] do_bind_con_driver+0x16c/0x398
> > >> [<ffffffc0003b6968>] do_take_over_console+0xd8/0x1f4
> > >> [<ffffffc00037811c>] do_fbcon_takeover+0x74/0xf8
> > >> [<ffffffc00037c5cc>] fbcon_event_notify+0x8a4/0x8f8
> > >> [<ffffffc0000d19f0>] notifier_call_chain+0x4c/0x88
> > >> [<ffffffc0000d1d78>] __blocking_notifier_call_chain+0x44/0x74
> > >> [<ffffffc0000d1dbc>] blocking_notifier_call_chain+0x14/0x1c
> > >> [<ffffffc00037d614>] fb_notifier_call_chain+0x1c/0x24
> > >> [<ffffffc00037f8a8>] register_framebuffer+0x1c0/0x2ac
> > >> [<ffffffc0003eb9f8>] drm_fb_helper_initial_config+0x25c/0x3ec
> > >> [<ffffffc0003ec28c>] drm_fbdev_cma_init+0x98/0x134
> > >> [<ffffffbffc006780>] hdlcd_drm_bind+0x180/0x498 [hdlcd]
> > >> [<ffffffc0004158e0>] try_to_bring_up_master.part.5+0xd4/0x118
> > >> [<ffffffc0004159e8>] component_master_add_with_match+0xc4/0x148
> > >> [<ffffffc000415a7c>] component_master_add+0x10/0x18
> > >> [<ffffffbffc0065ec>] hdlcd_probe+0x14/0x28 [hdlcd]
> > >> [<ffffffc00041c504>] platform_drv_probe+0x54/0xc0
> > >> [<ffffffc00041a9b0>] driver_probe_device+0x1ec/0x2e8
> > >> [<ffffffc00041ab48>] __driver_attach+0x9c/0xa0
> > >> [<ffffffc000418bcc>] bus_for_each_dev+0x58/0x98
> > >> [<ffffffc00041a41c>] driver_attach+0x20/0x28
> > >> [<ffffffc00041a058>] bus_add_driver+0x1c8/0x22c
> > >> [<ffffffc00041b3d0>] driver_register+0x68/0x108
> > >> [<ffffffc00041c408>] __platform_driver_register+0x4c/0x54
> > >> [<ffffffbffc00b018>] hdlcd_init+0x18/0x30 [hdlcd]
> > >> [<ffffffc00008293c>] do_one_initcall+0x90/0x1a8
> > >> [<ffffffc0001424dc>] do_init_module+0x60/0x1c8
> > >> [<ffffffc00011d364>] load_module+0x1554/0x1c98
> > >> [<ffffffc00011dc64>] SyS_finit_module+0x7c/0x88
> > >> [<ffffffc000085cb0>] el0_svc_naked+0x24/0x28
> > >>
> > >>
> > >> The line that triggers the warning is:
> > >>
> > >> 674: WARN_ON(!connector->encoder->crtc);
> > >>
> > >> As far as I can see the encoder->crtc value is being set to a non-NULL value only
> > >> in two places:
> > >> - in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON()
> > >> encoder->crtc = connector->state->crtc;
> > >> - in drm_crtc_helper_set_config(drm_mode_set *set):
> > >> encoder->crtc = new_crtc;
> > >>
> > >> Nothing in the call path from drm_atomic_commit() calls crtc_funcs->set_config() or
> > >> drm_crtc_helper_set_config() directly, so the question is if this WARN() is actually
> > >> valid.
> > >>
> > >> Call path from drm_atomic_commit:
> > >>
> > >> drm_atomic_helper_commit()
> > >> - drm_atomic_helper_prepare_planes()
> > >> - drm_atomic_helper_swap_state()
> > >> - drm_atomic_helper_commit_modeset_disables()
> > >> - disable_outputs()
> > >> - drm_atomic_helper_update_legacy_modeset_state()
> > >> - WARN_ON(!connector->encoder->crtc)
> > >>
> > >> Best regards,
> > >> Liviu
> > >>
> > >>
> > >> Hi Liviu
> > >> I solved this problem by following change, you can check if your driver do the same things:
> > >> drivers/gpu/drm/bridge/dw_hdmi.c:
> > >> - hdmi->connector.encoder = encoder;
> > >> +// hdmi->connector.encoder = encoder;
> > >>
> > >> drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
> > >>
> > >> I found some other drivers set connector.encoder before drm_mode_connector_attach_encoder, some are not.
> > >>
> > >> drm_mode_connector_attach_encoder already describe the link of connector and encoder,
> > >> so I think "connector.encoder = encoder" is not needed, is that right?
> > >I'll have a look, thanks for pointing this. However, my setup uses the tda998x driver for encoder, so I will
> > >have to go look there rather than in my driver.
> > >
> > >Best regards,
> > >Liviu
> > >
> > >> Thanks.
> > >>
> > >> --
> > >> Ｍark Yao
> > Hi Liviu
> > drivers/gpu/drm/i2c/tda998x_drv.c do the same things:
> > priv->connector.encoder = &priv->encoder;
> > drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
> >
> > I believe must be same problem.
>
> Oh, I hadn't noticed this subthread, but you came to the same conclusion
> as I did. I have the below local change, which I think might be good to
> have given that there are at least two drivers that got this wrong.
>
> Thierry
>
> --- >8 ---
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 24c5434abd1c..56d53106b32d 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -4983,6 +4983,20 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector,
> {
> int i;
>
> + /*
> + * In the past, drivers have attempted to model the static association
> + * of connector to encoder in simple connector/encoder devices using a
> + * direct assignment of connector->encoder = encoder. This connection
> + * is a logical one and the responsibility of the core, so drivers are
> + * expected not to mess with this.
> + *
> + * Note that the error return should've been enough here, but a large
> + * majority of drivers ignores the return value, so add in a big WARN
> + * to get people's attention.
> + */
> + if (WARN_ON(connector->encoder))
> + return -EINVAL;
> +
> for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> if (connector->encoder_ids[i] == 0) {
> connector->encoder_ids[i] = encoder->base.id;
Hi Thierry,
This patch together with your tda998x proposed patch should solve nicely the problem we are
detecting, as long as no one calls drm_connector_get_encoder() before drm_crtc_helper_set_config().
Best regards,
Liviu
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-12 14:03 ` Liviu Dudau
@ 2015-11-12 16:12 ` Thierry Reding
0 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2015-11-12 16:12 UTC (permalink / raw)
To: Liviu Dudau; +Cc: Mark yao, DRI devel, lkml
[-- Attachment #1: Type: text/plain, Size: 9053 bytes --]
On Thu, Nov 12, 2015 at 02:03:35PM +0000, Liviu Dudau wrote:
> On Thu, Nov 12, 2015 at 02:34:11PM +0100, Thierry Reding wrote:
> > On Thu, Nov 12, 2015 at 06:49:38PM +0800, Mark yao wrote:
> > > On 2015年11月12日 18:36, Liviu Dudau wrote:
> > > >On Thu, Nov 12, 2015 at 04:32:33PM +0800, Mark yao wrote:
> > > >> On 2015年11月10日 23:01, Liviu Dudau wrote:
> > > >>
> > > >> Hello,
> > > >>
> > > >> When booting my Juno board with the HDLCD driver that I have converted to
> > > >> atomic operations I'm getting the following warning:
> > > >>
> > > >> ------------[ cut here ]------------
> > > >> WARNING: at /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674
> > > >> Modules linked in: hdlcd(+) clk_scpi
> > > >>
> > > >> CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5
> > > >> Hardware name: ARM Juno development board (r0) (DT)
> > > >> task: ffffffc974888b00 ti: ffffffc9755dc000 task.ti: ffffffc9755dc000
> > > >> PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> > > >> LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> > > >> pc : [<ffffffc0003e6d18>] lr : [<ffffffc0003e8214>] pstate: 20000145
> > > >> sp : ffffffc9755df430
> > > >> x29: ffffffc9755df430 x28: ffffffc975703600
> > > >> x27: 0000000000000000 x26: ffffffc976253960
> > > >> x25: ffffffc976254040 x24: ffffffc000819000
> > > >> x23: ffffffc000689ea0 x22: ffffffc976251800
> > > >> x21: ffffffc976251800 x20: 0000000000000000
> > > >> x19: ffffffc9766b1f80 x18: 00000000715fe015
> > > >> x17: 0000007fb4b855b0 x16: 0000000000000220
> > > >> x15: 0000000000000001 x14: 0ffffffffffffffe
> > > >> x13: 0000000000000008 x12: 0101010101010101
> > > >> x11: ffffffc000964000 x10: ffffffc0009d2000
> > > >> x9 : 0000000000000000 x8 : ffffffc97ff5f700
> > > >> x7 : ffffffc97566cb80 x6 : ffffffc9766b1700
> > > >> x5 : ffffffc975665100 x4 : 0000000000000000
> > > >> x3 : ffffffc976253960 x2 : ffffffc97566cd00
> > > >> x1 : ffffffc976253900 x0 : 0000000000000000
> > > >>
> > > >> ---[ end trace 9fe289f798e7178e ]---
> > > >> Call trace:
> > > >> [<ffffffc0003e6d18>] drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> > > >> [<ffffffc0003e8214>] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> > > >> [<ffffffc0003e84c8>] drm_atomic_helper_commit+0xe0/0x150
> > > >> [<ffffffc00040bcc0>] drm_atomic_commit+0x40/0x6c
> > > >> [<ffffffc0003e92c0>] restore_fbdev_mode+0x294/0x2d4
> > > >> [<ffffffc0003eb6ec>] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c
> > > >> [<ffffffc0003eb770>] drm_fb_helper_set_par+0x2c/0x58
> > > >> [<ffffffc000378048>] fbcon_init+0x4d4/0x534
> > > >> [<ffffffc0003b44f4>] visual_init+0xac/0x104
> > > >> [<ffffffc0003b63f8>] do_bind_con_driver+0x16c/0x398
> > > >> [<ffffffc0003b6968>] do_take_over_console+0xd8/0x1f4
> > > >> [<ffffffc00037811c>] do_fbcon_takeover+0x74/0xf8
> > > >> [<ffffffc00037c5cc>] fbcon_event_notify+0x8a4/0x8f8
> > > >> [<ffffffc0000d19f0>] notifier_call_chain+0x4c/0x88
> > > >> [<ffffffc0000d1d78>] __blocking_notifier_call_chain+0x44/0x74
> > > >> [<ffffffc0000d1dbc>] blocking_notifier_call_chain+0x14/0x1c
> > > >> [<ffffffc00037d614>] fb_notifier_call_chain+0x1c/0x24
> > > >> [<ffffffc00037f8a8>] register_framebuffer+0x1c0/0x2ac
> > > >> [<ffffffc0003eb9f8>] drm_fb_helper_initial_config+0x25c/0x3ec
> > > >> [<ffffffc0003ec28c>] drm_fbdev_cma_init+0x98/0x134
> > > >> [<ffffffbffc006780>] hdlcd_drm_bind+0x180/0x498 [hdlcd]
> > > >> [<ffffffc0004158e0>] try_to_bring_up_master.part.5+0xd4/0x118
> > > >> [<ffffffc0004159e8>] component_master_add_with_match+0xc4/0x148
> > > >> [<ffffffc000415a7c>] component_master_add+0x10/0x18
> > > >> [<ffffffbffc0065ec>] hdlcd_probe+0x14/0x28 [hdlcd]
> > > >> [<ffffffc00041c504>] platform_drv_probe+0x54/0xc0
> > > >> [<ffffffc00041a9b0>] driver_probe_device+0x1ec/0x2e8
> > > >> [<ffffffc00041ab48>] __driver_attach+0x9c/0xa0
> > > >> [<ffffffc000418bcc>] bus_for_each_dev+0x58/0x98
> > > >> [<ffffffc00041a41c>] driver_attach+0x20/0x28
> > > >> [<ffffffc00041a058>] bus_add_driver+0x1c8/0x22c
> > > >> [<ffffffc00041b3d0>] driver_register+0x68/0x108
> > > >> [<ffffffc00041c408>] __platform_driver_register+0x4c/0x54
> > > >> [<ffffffbffc00b018>] hdlcd_init+0x18/0x30 [hdlcd]
> > > >> [<ffffffc00008293c>] do_one_initcall+0x90/0x1a8
> > > >> [<ffffffc0001424dc>] do_init_module+0x60/0x1c8
> > > >> [<ffffffc00011d364>] load_module+0x1554/0x1c98
> > > >> [<ffffffc00011dc64>] SyS_finit_module+0x7c/0x88
> > > >> [<ffffffc000085cb0>] el0_svc_naked+0x24/0x28
> > > >>
> > > >>
> > > >> The line that triggers the warning is:
> > > >>
> > > >> 674: WARN_ON(!connector->encoder->crtc);
> > > >>
> > > >> As far as I can see the encoder->crtc value is being set to a non-NULL value only
> > > >> in two places:
> > > >> - in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON()
> > > >> encoder->crtc = connector->state->crtc;
> > > >> - in drm_crtc_helper_set_config(drm_mode_set *set):
> > > >> encoder->crtc = new_crtc;
> > > >>
> > > >> Nothing in the call path from drm_atomic_commit() calls crtc_funcs->set_config() or
> > > >> drm_crtc_helper_set_config() directly, so the question is if this WARN() is actually
> > > >> valid.
> > > >>
> > > >> Call path from drm_atomic_commit:
> > > >>
> > > >> drm_atomic_helper_commit()
> > > >> - drm_atomic_helper_prepare_planes()
> > > >> - drm_atomic_helper_swap_state()
> > > >> - drm_atomic_helper_commit_modeset_disables()
> > > >> - disable_outputs()
> > > >> - drm_atomic_helper_update_legacy_modeset_state()
> > > >> - WARN_ON(!connector->encoder->crtc)
> > > >>
> > > >> Best regards,
> > > >> Liviu
> > > >>
> > > >>
> > > >> Hi Liviu
> > > >> I solved this problem by following change, you can check if your driver do the same things:
> > > >> drivers/gpu/drm/bridge/dw_hdmi.c:
> > > >> - hdmi->connector.encoder = encoder;
> > > >> +// hdmi->connector.encoder = encoder;
> > > >>
> > > >> drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
> > > >>
> > > >> I found some other drivers set connector.encoder before drm_mode_connector_attach_encoder, some are not.
> > > >>
> > > >> drm_mode_connector_attach_encoder already describe the link of connector and encoder,
> > > >> so I think "connector.encoder = encoder" is not needed, is that right?
> > > >I'll have a look, thanks for pointing this. However, my setup uses the tda998x driver for encoder, so I will
> > > >have to go look there rather than in my driver.
> > > >
> > > >Best regards,
> > > >Liviu
> > > >
> > > >> Thanks.
> > > >>
> > > >> --
> > > >> Ｍark Yao
> > > Hi Liviu
> > > drivers/gpu/drm/i2c/tda998x_drv.c do the same things:
> > > priv->connector.encoder = &priv->encoder;
> > > drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
> > >
> > > I believe must be same problem.
> >
> > Oh, I hadn't noticed this subthread, but you came to the same conclusion
> > as I did. I have the below local change, which I think might be good to
> > have given that there are at least two drivers that got this wrong.
> >
> > Thierry
> >
> > --- >8 ---
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 24c5434abd1c..56d53106b32d 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -4983,6 +4983,20 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector,
> > {
> > int i;
> >
> > + /*
> > + * In the past, drivers have attempted to model the static association
> > + * of connector to encoder in simple connector/encoder devices using a
> > + * direct assignment of connector->encoder = encoder. This connection
> > + * is a logical one and the responsibility of the core, so drivers are
> > + * expected not to mess with this.
> > + *
> > + * Note that the error return should've been enough here, but a large
> > + * majority of drivers ignores the return value, so add in a big WARN
> > + * to get people's attention.
> > + */
> > + if (WARN_ON(connector->encoder))
> > + return -EINVAL;
> > +
> > for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> > if (connector->encoder_ids[i] == 0) {
> > connector->encoder_ids[i] = encoder->base.id;
>
> Hi Thierry,
>
> This patch together with your tda998x proposed patch should solve
> nicely the problem we are detecting, as long as no one calls
> drm_connector_get_encoder() before drm_crtc_helper_set_config().
The only caller of drm_connector_get_encoder() is drm_mode_getconnector()
and that handles the NULL case gracefully, so I don't think there are any
issues.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-12 16:12 ` Thierry Reding
0 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2015-11-12 16:12 UTC (permalink / raw)
To: Liviu Dudau; +Cc: lkml, DRI devel
[-- Attachment #1.1: Type: text/plain, Size: 9053 bytes --]
On Thu, Nov 12, 2015 at 02:03:35PM +0000, Liviu Dudau wrote:
> On Thu, Nov 12, 2015 at 02:34:11PM +0100, Thierry Reding wrote:
> > On Thu, Nov 12, 2015 at 06:49:38PM +0800, Mark yao wrote:
> > > On 2015年11月12日 18:36, Liviu Dudau wrote:
> > > >On Thu, Nov 12, 2015 at 04:32:33PM +0800, Mark yao wrote:
> > > >> On 2015年11月10日 23:01, Liviu Dudau wrote:
> > > >>
> > > >> Hello,
> > > >>
> > > >> When booting my Juno board with the HDLCD driver that I have converted to
> > > >> atomic operations I'm getting the following warning:
> > > >>
> > > >> ------------[ cut here ]------------
> > > >> WARNING: at /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674
> > > >> Modules linked in: hdlcd(+) clk_scpi
> > > >>
> > > >> CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5
> > > >> Hardware name: ARM Juno development board (r0) (DT)
> > > >> task: ffffffc974888b00 ti: ffffffc9755dc000 task.ti: ffffffc9755dc000
> > > >> PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> > > >> LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> > > >> pc : [<ffffffc0003e6d18>] lr : [<ffffffc0003e8214>] pstate: 20000145
> > > >> sp : ffffffc9755df430
> > > >> x29: ffffffc9755df430 x28: ffffffc975703600
> > > >> x27: 0000000000000000 x26: ffffffc976253960
> > > >> x25: ffffffc976254040 x24: ffffffc000819000
> > > >> x23: ffffffc000689ea0 x22: ffffffc976251800
> > > >> x21: ffffffc976251800 x20: 0000000000000000
> > > >> x19: ffffffc9766b1f80 x18: 00000000715fe015
> > > >> x17: 0000007fb4b855b0 x16: 0000000000000220
> > > >> x15: 0000000000000001 x14: 0ffffffffffffffe
> > > >> x13: 0000000000000008 x12: 0101010101010101
> > > >> x11: ffffffc000964000 x10: ffffffc0009d2000
> > > >> x9 : 0000000000000000 x8 : ffffffc97ff5f700
> > > >> x7 : ffffffc97566cb80 x6 : ffffffc9766b1700
> > > >> x5 : ffffffc975665100 x4 : 0000000000000000
> > > >> x3 : ffffffc976253960 x2 : ffffffc97566cd00
> > > >> x1 : ffffffc976253900 x0 : 0000000000000000
> > > >>
> > > >> ---[ end trace 9fe289f798e7178e ]---
> > > >> Call trace:
> > > >> [<ffffffc0003e6d18>] drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> > > >> [<ffffffc0003e8214>] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> > > >> [<ffffffc0003e84c8>] drm_atomic_helper_commit+0xe0/0x150
> > > >> [<ffffffc00040bcc0>] drm_atomic_commit+0x40/0x6c
> > > >> [<ffffffc0003e92c0>] restore_fbdev_mode+0x294/0x2d4
> > > >> [<ffffffc0003eb6ec>] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c
> > > >> [<ffffffc0003eb770>] drm_fb_helper_set_par+0x2c/0x58
> > > >> [<ffffffc000378048>] fbcon_init+0x4d4/0x534
> > > >> [<ffffffc0003b44f4>] visual_init+0xac/0x104
> > > >> [<ffffffc0003b63f8>] do_bind_con_driver+0x16c/0x398
> > > >> [<ffffffc0003b6968>] do_take_over_console+0xd8/0x1f4
> > > >> [<ffffffc00037811c>] do_fbcon_takeover+0x74/0xf8
> > > >> [<ffffffc00037c5cc>] fbcon_event_notify+0x8a4/0x8f8
> > > >> [<ffffffc0000d19f0>] notifier_call_chain+0x4c/0x88
> > > >> [<ffffffc0000d1d78>] __blocking_notifier_call_chain+0x44/0x74
> > > >> [<ffffffc0000d1dbc>] blocking_notifier_call_chain+0x14/0x1c
> > > >> [<ffffffc00037d614>] fb_notifier_call_chain+0x1c/0x24
> > > >> [<ffffffc00037f8a8>] register_framebuffer+0x1c0/0x2ac
> > > >> [<ffffffc0003eb9f8>] drm_fb_helper_initial_config+0x25c/0x3ec
> > > >> [<ffffffc0003ec28c>] drm_fbdev_cma_init+0x98/0x134
> > > >> [<ffffffbffc006780>] hdlcd_drm_bind+0x180/0x498 [hdlcd]
> > > >> [<ffffffc0004158e0>] try_to_bring_up_master.part.5+0xd4/0x118
> > > >> [<ffffffc0004159e8>] component_master_add_with_match+0xc4/0x148
> > > >> [<ffffffc000415a7c>] component_master_add+0x10/0x18
> > > >> [<ffffffbffc0065ec>] hdlcd_probe+0x14/0x28 [hdlcd]
> > > >> [<ffffffc00041c504>] platform_drv_probe+0x54/0xc0
> > > >> [<ffffffc00041a9b0>] driver_probe_device+0x1ec/0x2e8
> > > >> [<ffffffc00041ab48>] __driver_attach+0x9c/0xa0
> > > >> [<ffffffc000418bcc>] bus_for_each_dev+0x58/0x98
> > > >> [<ffffffc00041a41c>] driver_attach+0x20/0x28
> > > >> [<ffffffc00041a058>] bus_add_driver+0x1c8/0x22c
> > > >> [<ffffffc00041b3d0>] driver_register+0x68/0x108
> > > >> [<ffffffc00041c408>] __platform_driver_register+0x4c/0x54
> > > >> [<ffffffbffc00b018>] hdlcd_init+0x18/0x30 [hdlcd]
> > > >> [<ffffffc00008293c>] do_one_initcall+0x90/0x1a8
> > > >> [<ffffffc0001424dc>] do_init_module+0x60/0x1c8
> > > >> [<ffffffc00011d364>] load_module+0x1554/0x1c98
> > > >> [<ffffffc00011dc64>] SyS_finit_module+0x7c/0x88
> > > >> [<ffffffc000085cb0>] el0_svc_naked+0x24/0x28
> > > >>
> > > >>
> > > >> The line that triggers the warning is:
> > > >>
> > > >> 674: WARN_ON(!connector->encoder->crtc);
> > > >>
> > > >> As far as I can see the encoder->crtc value is being set to a non-NULL value only
> > > >> in two places:
> > > >> - in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON()
> > > >> encoder->crtc = connector->state->crtc;
> > > >> - in drm_crtc_helper_set_config(drm_mode_set *set):
> > > >> encoder->crtc = new_crtc;
> > > >>
> > > >> Nothing in the call path from drm_atomic_commit() calls crtc_funcs->set_config() or
> > > >> drm_crtc_helper_set_config() directly, so the question is if this WARN() is actually
> > > >> valid.
> > > >>
> > > >> Call path from drm_atomic_commit:
> > > >>
> > > >> drm_atomic_helper_commit()
> > > >> - drm_atomic_helper_prepare_planes()
> > > >> - drm_atomic_helper_swap_state()
> > > >> - drm_atomic_helper_commit_modeset_disables()
> > > >> - disable_outputs()
> > > >> - drm_atomic_helper_update_legacy_modeset_state()
> > > >> - WARN_ON(!connector->encoder->crtc)
> > > >>
> > > >> Best regards,
> > > >> Liviu
> > > >>
> > > >>
> > > >> Hi Liviu
> > > >> I solved this problem by following change, you can check if your driver do the same things:
> > > >> drivers/gpu/drm/bridge/dw_hdmi.c:
> > > >> - hdmi->connector.encoder = encoder;
> > > >> +// hdmi->connector.encoder = encoder;
> > > >>
> > > >> drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
> > > >>
> > > >> I found some other drivers set connector.encoder before drm_mode_connector_attach_encoder, some are not.
> > > >>
> > > >> drm_mode_connector_attach_encoder already describe the link of connector and encoder,
> > > >> so I think "connector.encoder = encoder" is not needed, is that right?
> > > >I'll have a look, thanks for pointing this. However, my setup uses the tda998x driver for encoder, so I will
> > > >have to go look there rather than in my driver.
> > > >
> > > >Best regards,
> > > >Liviu
> > > >
> > > >> Thanks.
> > > >>
> > > >> --
> > > >> Ｍark Yao
> > > Hi Liviu
> > > drivers/gpu/drm/i2c/tda998x_drv.c do the same things:
> > > priv->connector.encoder = &priv->encoder;
> > > drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
> > >
> > > I believe must be same problem.
> >
> > Oh, I hadn't noticed this subthread, but you came to the same conclusion
> > as I did. I have the below local change, which I think might be good to
> > have given that there are at least two drivers that got this wrong.
> >
> > Thierry
> >
> > --- >8 ---
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 24c5434abd1c..56d53106b32d 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -4983,6 +4983,20 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector,
> > {
> > int i;
> >
> > + /*
> > + * In the past, drivers have attempted to model the static association
> > + * of connector to encoder in simple connector/encoder devices using a
> > + * direct assignment of connector->encoder = encoder. This connection
> > + * is a logical one and the responsibility of the core, so drivers are
> > + * expected not to mess with this.
> > + *
> > + * Note that the error return should've been enough here, but a large
> > + * majority of drivers ignores the return value, so add in a big WARN
> > + * to get people's attention.
> > + */
> > + if (WARN_ON(connector->encoder))
> > + return -EINVAL;
> > +
> > for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> > if (connector->encoder_ids[i] == 0) {
> > connector->encoder_ids[i] = encoder->base.id;
>
> Hi Thierry,
>
> This patch together with your tda998x proposed patch should solve
> nicely the problem we are detecting, as long as no one calls
> drm_connector_get_encoder() before drm_crtc_helper_set_config().
The only caller of drm_connector_get_encoder() is drm_mode_getconnector()
and that handles the NULL case gracefully, so I don't think there are any
issues.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-12 13:57 ` Liviu Dudau
@ 2015-11-12 16:28 ` Thierry Reding
-1 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2015-11-12 16:28 UTC (permalink / raw)
To: Liviu Dudau; +Cc: Daniel Vetter, David Airlie, Rob Clark, DRI devel, lkml
[-- Attachment #1: Type: text/plain, Size: 4622 bytes --]
On Thu, Nov 12, 2015 at 01:57:11PM +0000, Liviu Dudau wrote:
> On Thu, Nov 12, 2015 at 01:16:55PM +0100, Thierry Reding wrote:
> > On Wed, Nov 11, 2015 at 04:09:42PM +0000, Liviu Dudau wrote:
> > > On Tue, Nov 10, 2015 at 05:56:15PM +0100, Thierry Reding wrote:
> > > > On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
> > > > > Hello,
> > > > >
> > > > > When booting my Juno board with the HDLCD driver that I have converted to
> > > > > atomic operations I'm getting the following warning:
> > > >
> > > > Perhaps you can provide pointers to the source code, that might make it
> > > > easier for people to spot what's going wrong.
> > > >
> > > > Thierry
> > >
> > > Hi Thierry,
> > >
> > > I have just pushed to the mailing lists my patch series. [1] [2]
> > >
> > > If you want to checkout the code I also have a branch here:
> > >
> > > git://git://linux-arm.org/linux-ld testing/hdlcd
> >
> > Okay, so if I understand correctly you're using the tda998x as encoder/
> > connector attached to your new HDLCD driver. I think the problem isn't
> > so much that nothing has set up the CRTC for the encoder, but rather
> > that the tda998x encoder tries to statically associate the encoder with
> > the connector. While that might be correct in that it represents the
> > hardware state (i.e. there is no way to physically route it anywhere
> > else), the DRM logical state is that it's not connected until a complete
> > pipeline has been set up, in which case a CRTC would have been assigned
> > to the encoder.
> >
> > If your setup were working correctly you'd never reach the WARN_ON
> > because the
> >
> > if (connector->encoder) {
> >
> > conditional (on line 673 in drivers/gpu/drm/drm_atomic_helper.c) would
> > have evaluated to false already, since logically there'd be no encoder
> > associated with the connector yet.
>
> Your analysis is spot on and matches my conditions. However ...
>
> >
> > Does the patch below help? Let me know and I can produce a proper patch.
> >
> > Thierry
> >
> > --- >8 ---
> > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> > index 896b6aaf8c4d..8b0a402190eb 100644
> > --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> > @@ -1453,7 +1453,6 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
> > if (ret)
> > goto err_sysfs;
> >
> > - priv->connector.encoder = &priv->encoder;
>
> while this patch helps (no WARN() printed) I'm not sure this is the right fix.
> TDA998x is also used by armada DRM driver which has not been converted (yet) to
> atomic modesetting. And judging by the code and comment of drm_connector_get_encoder()
> in drm_crtc.c, having access to the encoder through the connector is the non-atomic
> way of doing things:
>
> static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *connector)
> {
> /* For atomic drivers only state objects are synchronously updated and
> * protected by modeset locks, so check those first. */
> if (connector->state)
> return connector->state->best_encoder;
> return connector->encoder;
> }
>
> To me, it looks like the WARN() is bogus when the atomic modesetting is used in
> drm_atomic_helper_update_legacy_modeset_state(). Either we acknowledge that the "legacy"
> code sets the connector in the encoder structure before complete pipeline is setup
> and we remove the WARN(), or we find a better way of detecting that some sort of mixed
> support is in place (i.e. atomic and non-atomic aware code running) and we clean up
> in a way that is compatible with both worlds.
I think the problem you're seeing here is specifically caused by drivers
setting up the connector->encoder explicitly. There should be no reason
to do that. The DRM core will automatically do that when setting up a
default configuration, or as a result of userspace setting up the wanted
configuration. You're also likely only seeing this the first time around
and subsequent calls will not trigger this anymore because at that point
the core will have reset connector->encoder to NULL as part of tearing
down the pipeline.
To corroborate that, the Tegra driver doesn't trigger this WARN_ON() and
also never sets up the connector->encoder manually. The same is probably
true for many other drivers that have already been converted. That said,
from a brief look it seems like many more drivers get this wrong and may
run into this WARN when they get converted to atomic.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-12 16:28 ` Thierry Reding
0 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2015-11-12 16:28 UTC (permalink / raw)
To: Liviu Dudau; +Cc: DRI devel, lkml
[-- Attachment #1.1: Type: text/plain, Size: 4622 bytes --]
On Thu, Nov 12, 2015 at 01:57:11PM +0000, Liviu Dudau wrote:
> On Thu, Nov 12, 2015 at 01:16:55PM +0100, Thierry Reding wrote:
> > On Wed, Nov 11, 2015 at 04:09:42PM +0000, Liviu Dudau wrote:
> > > On Tue, Nov 10, 2015 at 05:56:15PM +0100, Thierry Reding wrote:
> > > > On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
> > > > > Hello,
> > > > >
> > > > > When booting my Juno board with the HDLCD driver that I have converted to
> > > > > atomic operations I'm getting the following warning:
> > > >
> > > > Perhaps you can provide pointers to the source code, that might make it
> > > > easier for people to spot what's going wrong.
> > > >
> > > > Thierry
> > >
> > > Hi Thierry,
> > >
> > > I have just pushed to the mailing lists my patch series. [1] [2]
> > >
> > > If you want to checkout the code I also have a branch here:
> > >
> > > git://git://linux-arm.org/linux-ld testing/hdlcd
> >
> > Okay, so if I understand correctly you're using the tda998x as encoder/
> > connector attached to your new HDLCD driver. I think the problem isn't
> > so much that nothing has set up the CRTC for the encoder, but rather
> > that the tda998x encoder tries to statically associate the encoder with
> > the connector. While that might be correct in that it represents the
> > hardware state (i.e. there is no way to physically route it anywhere
> > else), the DRM logical state is that it's not connected until a complete
> > pipeline has been set up, in which case a CRTC would have been assigned
> > to the encoder.
> >
> > If your setup were working correctly you'd never reach the WARN_ON
> > because the
> >
> > if (connector->encoder) {
> >
> > conditional (on line 673 in drivers/gpu/drm/drm_atomic_helper.c) would
> > have evaluated to false already, since logically there'd be no encoder
> > associated with the connector yet.
>
> Your analysis is spot on and matches my conditions. However ...
>
> >
> > Does the patch below help? Let me know and I can produce a proper patch.
> >
> > Thierry
> >
> > --- >8 ---
> > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> > index 896b6aaf8c4d..8b0a402190eb 100644
> > --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> > @@ -1453,7 +1453,6 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
> > if (ret)
> > goto err_sysfs;
> >
> > - priv->connector.encoder = &priv->encoder;
>
> while this patch helps (no WARN() printed) I'm not sure this is the right fix.
> TDA998x is also used by armada DRM driver which has not been converted (yet) to
> atomic modesetting. And judging by the code and comment of drm_connector_get_encoder()
> in drm_crtc.c, having access to the encoder through the connector is the non-atomic
> way of doing things:
>
> static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *connector)
> {
> /* For atomic drivers only state objects are synchronously updated and
> * protected by modeset locks, so check those first. */
> if (connector->state)
> return connector->state->best_encoder;
> return connector->encoder;
> }
>
> To me, it looks like the WARN() is bogus when the atomic modesetting is used in
> drm_atomic_helper_update_legacy_modeset_state(). Either we acknowledge that the "legacy"
> code sets the connector in the encoder structure before complete pipeline is setup
> and we remove the WARN(), or we find a better way of detecting that some sort of mixed
> support is in place (i.e. atomic and non-atomic aware code running) and we clean up
> in a way that is compatible with both worlds.
I think the problem you're seeing here is specifically caused by drivers
setting up the connector->encoder explicitly. There should be no reason
to do that. The DRM core will automatically do that when setting up a
default configuration, or as a result of userspace setting up the wanted
configuration. You're also likely only seeing this the first time around
and subsequent calls will not trigger this anymore because at that point
the core will have reset connector->encoder to NULL as part of tearing
down the pipeline.
To corroborate that, the Tegra driver doesn't trigger this WARN_ON() and
also never sets up the connector->encoder manually. The same is probably
true for many other drivers that have already been converted. That said,
from a brief look it seems like many more drivers get this wrong and may
run into this WARN when they get converted to atomic.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-12 16:12 ` Thierry Reding
@ 2015-11-12 16:48 ` Liviu Dudau
-1 siblings, 0 replies; 34+ messages in thread
From: Liviu Dudau @ 2015-11-12 16:48 UTC (permalink / raw)
To: Thierry Reding; +Cc: Mark yao, DRI devel, lkml
On Thu, Nov 12, 2015 at 05:12:33PM +0100, Thierry Reding wrote:
> On Thu, Nov 12, 2015 at 02:03:35PM +0000, Liviu Dudau wrote:
> > On Thu, Nov 12, 2015 at 02:34:11PM +0100, Thierry Reding wrote:
> > > On Thu, Nov 12, 2015 at 06:49:38PM +0800, Mark yao wrote:
> > > > On 2015年11月12日 18:36, Liviu Dudau wrote:
> > > > >On Thu, Nov 12, 2015 at 04:32:33PM +0800, Mark yao wrote:
> > > > >> On 2015年11月10日 23:01, Liviu Dudau wrote:
> > > > >>
> > > > >> Hello,
> > > > >>
> > > > >> When booting my Juno board with the HDLCD driver that I have converted to
> > > > >> atomic operations I'm getting the following warning:
> > > > >>
> > > > >> ------------[ cut here ]------------
> > > > >> WARNING: at /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674
> > > > >> Modules linked in: hdlcd(+) clk_scpi
> > > > >>
> > > > >> CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5
> > > > >> Hardware name: ARM Juno development board (r0) (DT)
> > > > >> task: ffffffc974888b00 ti: ffffffc9755dc000 task.ti: ffffffc9755dc000
> > > > >> PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> > > > >> LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> > > > >> pc : [<ffffffc0003e6d18>] lr : [<ffffffc0003e8214>] pstate: 20000145
> > > > >> sp : ffffffc9755df430
> > > > >> x29: ffffffc9755df430 x28: ffffffc975703600
> > > > >> x27: 0000000000000000 x26: ffffffc976253960
> > > > >> x25: ffffffc976254040 x24: ffffffc000819000
> > > > >> x23: ffffffc000689ea0 x22: ffffffc976251800
> > > > >> x21: ffffffc976251800 x20: 0000000000000000
> > > > >> x19: ffffffc9766b1f80 x18: 00000000715fe015
> > > > >> x17: 0000007fb4b855b0 x16: 0000000000000220
> > > > >> x15: 0000000000000001 x14: 0ffffffffffffffe
> > > > >> x13: 0000000000000008 x12: 0101010101010101
> > > > >> x11: ffffffc000964000 x10: ffffffc0009d2000
> > > > >> x9 : 0000000000000000 x8 : ffffffc97ff5f700
> > > > >> x7 : ffffffc97566cb80 x6 : ffffffc9766b1700
> > > > >> x5 : ffffffc975665100 x4 : 0000000000000000
> > > > >> x3 : ffffffc976253960 x2 : ffffffc97566cd00
> > > > >> x1 : ffffffc976253900 x0 : 0000000000000000
> > > > >>
> > > > >> ---[ end trace 9fe289f798e7178e ]---
> > > > >> Call trace:
> > > > >> [<ffffffc0003e6d18>] drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> > > > >> [<ffffffc0003e8214>] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> > > > >> [<ffffffc0003e84c8>] drm_atomic_helper_commit+0xe0/0x150
> > > > >> [<ffffffc00040bcc0>] drm_atomic_commit+0x40/0x6c
> > > > >> [<ffffffc0003e92c0>] restore_fbdev_mode+0x294/0x2d4
> > > > >> [<ffffffc0003eb6ec>] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c
> > > > >> [<ffffffc0003eb770>] drm_fb_helper_set_par+0x2c/0x58
> > > > >> [<ffffffc000378048>] fbcon_init+0x4d4/0x534
> > > > >> [<ffffffc0003b44f4>] visual_init+0xac/0x104
> > > > >> [<ffffffc0003b63f8>] do_bind_con_driver+0x16c/0x398
> > > > >> [<ffffffc0003b6968>] do_take_over_console+0xd8/0x1f4
> > > > >> [<ffffffc00037811c>] do_fbcon_takeover+0x74/0xf8
> > > > >> [<ffffffc00037c5cc>] fbcon_event_notify+0x8a4/0x8f8
> > > > >> [<ffffffc0000d19f0>] notifier_call_chain+0x4c/0x88
> > > > >> [<ffffffc0000d1d78>] __blocking_notifier_call_chain+0x44/0x74
> > > > >> [<ffffffc0000d1dbc>] blocking_notifier_call_chain+0x14/0x1c
> > > > >> [<ffffffc00037d614>] fb_notifier_call_chain+0x1c/0x24
> > > > >> [<ffffffc00037f8a8>] register_framebuffer+0x1c0/0x2ac
> > > > >> [<ffffffc0003eb9f8>] drm_fb_helper_initial_config+0x25c/0x3ec
> > > > >> [<ffffffc0003ec28c>] drm_fbdev_cma_init+0x98/0x134
> > > > >> [<ffffffbffc006780>] hdlcd_drm_bind+0x180/0x498 [hdlcd]
> > > > >> [<ffffffc0004158e0>] try_to_bring_up_master.part.5+0xd4/0x118
> > > > >> [<ffffffc0004159e8>] component_master_add_with_match+0xc4/0x148
> > > > >> [<ffffffc000415a7c>] component_master_add+0x10/0x18
> > > > >> [<ffffffbffc0065ec>] hdlcd_probe+0x14/0x28 [hdlcd]
> > > > >> [<ffffffc00041c504>] platform_drv_probe+0x54/0xc0
> > > > >> [<ffffffc00041a9b0>] driver_probe_device+0x1ec/0x2e8
> > > > >> [<ffffffc00041ab48>] __driver_attach+0x9c/0xa0
> > > > >> [<ffffffc000418bcc>] bus_for_each_dev+0x58/0x98
> > > > >> [<ffffffc00041a41c>] driver_attach+0x20/0x28
> > > > >> [<ffffffc00041a058>] bus_add_driver+0x1c8/0x22c
> > > > >> [<ffffffc00041b3d0>] driver_register+0x68/0x108
> > > > >> [<ffffffc00041c408>] __platform_driver_register+0x4c/0x54
> > > > >> [<ffffffbffc00b018>] hdlcd_init+0x18/0x30 [hdlcd]
> > > > >> [<ffffffc00008293c>] do_one_initcall+0x90/0x1a8
> > > > >> [<ffffffc0001424dc>] do_init_module+0x60/0x1c8
> > > > >> [<ffffffc00011d364>] load_module+0x1554/0x1c98
> > > > >> [<ffffffc00011dc64>] SyS_finit_module+0x7c/0x88
> > > > >> [<ffffffc000085cb0>] el0_svc_naked+0x24/0x28
> > > > >>
> > > > >>
> > > > >> The line that triggers the warning is:
> > > > >>
> > > > >> 674: WARN_ON(!connector->encoder->crtc);
> > > > >>
> > > > >> As far as I can see the encoder->crtc value is being set to a non-NULL value only
> > > > >> in two places:
> > > > >> - in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON()
> > > > >> encoder->crtc = connector->state->crtc;
> > > > >> - in drm_crtc_helper_set_config(drm_mode_set *set):
> > > > >> encoder->crtc = new_crtc;
> > > > >>
> > > > >> Nothing in the call path from drm_atomic_commit() calls crtc_funcs->set_config() or
> > > > >> drm_crtc_helper_set_config() directly, so the question is if this WARN() is actually
> > > > >> valid.
> > > > >>
> > > > >> Call path from drm_atomic_commit:
> > > > >>
> > > > >> drm_atomic_helper_commit()
> > > > >> - drm_atomic_helper_prepare_planes()
> > > > >> - drm_atomic_helper_swap_state()
> > > > >> - drm_atomic_helper_commit_modeset_disables()
> > > > >> - disable_outputs()
> > > > >> - drm_atomic_helper_update_legacy_modeset_state()
> > > > >> - WARN_ON(!connector->encoder->crtc)
> > > > >>
> > > > >> Best regards,
> > > > >> Liviu
> > > > >>
> > > > >>
> > > > >> Hi Liviu
> > > > >> I solved this problem by following change, you can check if your driver do the same things:
> > > > >> drivers/gpu/drm/bridge/dw_hdmi.c:
> > > > >> - hdmi->connector.encoder = encoder;
> > > > >> +// hdmi->connector.encoder = encoder;
> > > > >>
> > > > >> drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
> > > > >>
> > > > >> I found some other drivers set connector.encoder before drm_mode_connector_attach_encoder, some are not.
> > > > >>
> > > > >> drm_mode_connector_attach_encoder already describe the link of connector and encoder,
> > > > >> so I think "connector.encoder = encoder" is not needed, is that right?
> > > > >I'll have a look, thanks for pointing this. However, my setup uses the tda998x driver for encoder, so I will
> > > > >have to go look there rather than in my driver.
> > > > >
> > > > >Best regards,
> > > > >Liviu
> > > > >
> > > > >> Thanks.
> > > > >>
> > > > >> --
> > > > >> Ｍark Yao
> > > > Hi Liviu
> > > > drivers/gpu/drm/i2c/tda998x_drv.c do the same things:
> > > > priv->connector.encoder = &priv->encoder;
> > > > drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
> > > >
> > > > I believe must be same problem.
> > >
> > > Oh, I hadn't noticed this subthread, but you came to the same conclusion
> > > as I did. I have the below local change, which I think might be good to
> > > have given that there are at least two drivers that got this wrong.
> > >
> > > Thierry
> > >
> > > --- >8 ---
> > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > > index 24c5434abd1c..56d53106b32d 100644
> > > --- a/drivers/gpu/drm/drm_crtc.c
> > > +++ b/drivers/gpu/drm/drm_crtc.c
> > > @@ -4983,6 +4983,20 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector,
> > > {
> > > int i;
> > >
> > > + /*
> > > + * In the past, drivers have attempted to model the static association
> > > + * of connector to encoder in simple connector/encoder devices using a
> > > + * direct assignment of connector->encoder = encoder. This connection
> > > + * is a logical one and the responsibility of the core, so drivers are
> > > + * expected not to mess with this.
> > > + *
> > > + * Note that the error return should've been enough here, but a large
> > > + * majority of drivers ignores the return value, so add in a big WARN
> > > + * to get people's attention.
> > > + */
> > > + if (WARN_ON(connector->encoder))
> > > + return -EINVAL;
> > > +
> > > for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> > > if (connector->encoder_ids[i] == 0) {
> > > connector->encoder_ids[i] = encoder->base.id;
> >
> > Hi Thierry,
> >
> > This patch together with your tda998x proposed patch should solve
> > nicely the problem we are detecting, as long as no one calls
> > drm_connector_get_encoder() before drm_crtc_helper_set_config().
>
> The only caller of drm_connector_get_encoder() is drm_mode_getconnector()
> and that handles the NULL case gracefully, so I don't think there are any
> issues.
Fine :)
I'm happy to test any patch you might have and give you my Tested-by.
Best regards,
Liviu
>
> Thierry
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-12 16:48 ` Liviu Dudau
0 siblings, 0 replies; 34+ messages in thread
From: Liviu Dudau @ 2015-11-12 16:48 UTC (permalink / raw)
To: Thierry Reding; +Cc: lkml, DRI devel
On Thu, Nov 12, 2015 at 05:12:33PM +0100, Thierry Reding wrote:
> On Thu, Nov 12, 2015 at 02:03:35PM +0000, Liviu Dudau wrote:
> > On Thu, Nov 12, 2015 at 02:34:11PM +0100, Thierry Reding wrote:
> > > On Thu, Nov 12, 2015 at 06:49:38PM +0800, Mark yao wrote:
> > > > On 2015年11月12日 18:36, Liviu Dudau wrote:
> > > > >On Thu, Nov 12, 2015 at 04:32:33PM +0800, Mark yao wrote:
> > > > >> On 2015年11月10日 23:01, Liviu Dudau wrote:
> > > > >>
> > > > >> Hello,
> > > > >>
> > > > >> When booting my Juno board with the HDLCD driver that I have converted to
> > > > >> atomic operations I'm getting the following warning:
> > > > >>
> > > > >> ------------[ cut here ]------------
> > > > >> WARNING: at /work/repositories/kernel/drivers/gpu/drm/drm_atomic_helper.c:674
> > > > >> Modules linked in: hdlcd(+) clk_scpi
> > > > >>
> > > > >> CPU: 3 PID: 1375 Comm: systemd-udevd Not tainted 4.3.0-next-20151109+ #5
> > > > >> Hardware name: ARM Juno development board (r0) (DT)
> > > > >> task: ffffffc974888b00 ti: ffffffc9755dc000 task.ti: ffffffc9755dc000
> > > > >> PC is at drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> > > > >> LR is at drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> > > > >> pc : [<ffffffc0003e6d18>] lr : [<ffffffc0003e8214>] pstate: 20000145
> > > > >> sp : ffffffc9755df430
> > > > >> x29: ffffffc9755df430 x28: ffffffc975703600
> > > > >> x27: 0000000000000000 x26: ffffffc976253960
> > > > >> x25: ffffffc976254040 x24: ffffffc000819000
> > > > >> x23: ffffffc000689ea0 x22: ffffffc976251800
> > > > >> x21: ffffffc976251800 x20: 0000000000000000
> > > > >> x19: ffffffc9766b1f80 x18: 00000000715fe015
> > > > >> x17: 0000007fb4b855b0 x16: 0000000000000220
> > > > >> x15: 0000000000000001 x14: 0ffffffffffffffe
> > > > >> x13: 0000000000000008 x12: 0101010101010101
> > > > >> x11: ffffffc000964000 x10: ffffffc0009d2000
> > > > >> x9 : 0000000000000000 x8 : ffffffc97ff5f700
> > > > >> x7 : ffffffc97566cb80 x6 : ffffffc9766b1700
> > > > >> x5 : ffffffc975665100 x4 : 0000000000000000
> > > > >> x3 : ffffffc976253960 x2 : ffffffc97566cd00
> > > > >> x1 : ffffffc976253900 x0 : 0000000000000000
> > > > >>
> > > > >> ---[ end trace 9fe289f798e7178e ]---
> > > > >> Call trace:
> > > > >> [<ffffffc0003e6d18>] drm_atomic_helper_update_legacy_modeset_state+0x204/0x20c
> > > > >> [<ffffffc0003e8214>] drm_atomic_helper_commit_modeset_disables+0x1c0/0x394
> > > > >> [<ffffffc0003e84c8>] drm_atomic_helper_commit+0xe0/0x150
> > > > >> [<ffffffc00040bcc0>] drm_atomic_commit+0x40/0x6c
> > > > >> [<ffffffc0003e92c0>] restore_fbdev_mode+0x294/0x2d4
> > > > >> [<ffffffc0003eb6ec>] drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x8c
> > > > >> [<ffffffc0003eb770>] drm_fb_helper_set_par+0x2c/0x58
> > > > >> [<ffffffc000378048>] fbcon_init+0x4d4/0x534
> > > > >> [<ffffffc0003b44f4>] visual_init+0xac/0x104
> > > > >> [<ffffffc0003b63f8>] do_bind_con_driver+0x16c/0x398
> > > > >> [<ffffffc0003b6968>] do_take_over_console+0xd8/0x1f4
> > > > >> [<ffffffc00037811c>] do_fbcon_takeover+0x74/0xf8
> > > > >> [<ffffffc00037c5cc>] fbcon_event_notify+0x8a4/0x8f8
> > > > >> [<ffffffc0000d19f0>] notifier_call_chain+0x4c/0x88
> > > > >> [<ffffffc0000d1d78>] __blocking_notifier_call_chain+0x44/0x74
> > > > >> [<ffffffc0000d1dbc>] blocking_notifier_call_chain+0x14/0x1c
> > > > >> [<ffffffc00037d614>] fb_notifier_call_chain+0x1c/0x24
> > > > >> [<ffffffc00037f8a8>] register_framebuffer+0x1c0/0x2ac
> > > > >> [<ffffffc0003eb9f8>] drm_fb_helper_initial_config+0x25c/0x3ec
> > > > >> [<ffffffc0003ec28c>] drm_fbdev_cma_init+0x98/0x134
> > > > >> [<ffffffbffc006780>] hdlcd_drm_bind+0x180/0x498 [hdlcd]
> > > > >> [<ffffffc0004158e0>] try_to_bring_up_master.part.5+0xd4/0x118
> > > > >> [<ffffffc0004159e8>] component_master_add_with_match+0xc4/0x148
> > > > >> [<ffffffc000415a7c>] component_master_add+0x10/0x18
> > > > >> [<ffffffbffc0065ec>] hdlcd_probe+0x14/0x28 [hdlcd]
> > > > >> [<ffffffc00041c504>] platform_drv_probe+0x54/0xc0
> > > > >> [<ffffffc00041a9b0>] driver_probe_device+0x1ec/0x2e8
> > > > >> [<ffffffc00041ab48>] __driver_attach+0x9c/0xa0
> > > > >> [<ffffffc000418bcc>] bus_for_each_dev+0x58/0x98
> > > > >> [<ffffffc00041a41c>] driver_attach+0x20/0x28
> > > > >> [<ffffffc00041a058>] bus_add_driver+0x1c8/0x22c
> > > > >> [<ffffffc00041b3d0>] driver_register+0x68/0x108
> > > > >> [<ffffffc00041c408>] __platform_driver_register+0x4c/0x54
> > > > >> [<ffffffbffc00b018>] hdlcd_init+0x18/0x30 [hdlcd]
> > > > >> [<ffffffc00008293c>] do_one_initcall+0x90/0x1a8
> > > > >> [<ffffffc0001424dc>] do_init_module+0x60/0x1c8
> > > > >> [<ffffffc00011d364>] load_module+0x1554/0x1c98
> > > > >> [<ffffffc00011dc64>] SyS_finit_module+0x7c/0x88
> > > > >> [<ffffffc000085cb0>] el0_svc_naked+0x24/0x28
> > > > >>
> > > > >>
> > > > >> The line that triggers the warning is:
> > > > >>
> > > > >> 674: WARN_ON(!connector->encoder->crtc);
> > > > >>
> > > > >> As far as I can see the encoder->crtc value is being set to a non-NULL value only
> > > > >> in two places:
> > > > >> - in drm_atomic_helper_update_legacy_modeset_state() after WARN_ON()
> > > > >> encoder->crtc = connector->state->crtc;
> > > > >> - in drm_crtc_helper_set_config(drm_mode_set *set):
> > > > >> encoder->crtc = new_crtc;
> > > > >>
> > > > >> Nothing in the call path from drm_atomic_commit() calls crtc_funcs->set_config() or
> > > > >> drm_crtc_helper_set_config() directly, so the question is if this WARN() is actually
> > > > >> valid.
> > > > >>
> > > > >> Call path from drm_atomic_commit:
> > > > >>
> > > > >> drm_atomic_helper_commit()
> > > > >> - drm_atomic_helper_prepare_planes()
> > > > >> - drm_atomic_helper_swap_state()
> > > > >> - drm_atomic_helper_commit_modeset_disables()
> > > > >> - disable_outputs()
> > > > >> - drm_atomic_helper_update_legacy_modeset_state()
> > > > >> - WARN_ON(!connector->encoder->crtc)
> > > > >>
> > > > >> Best regards,
> > > > >> Liviu
> > > > >>
> > > > >>
> > > > >> Hi Liviu
> > > > >> I solved this problem by following change, you can check if your driver do the same things:
> > > > >> drivers/gpu/drm/bridge/dw_hdmi.c:
> > > > >> - hdmi->connector.encoder = encoder;
> > > > >> +// hdmi->connector.encoder = encoder;
> > > > >>
> > > > >> drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
> > > > >>
> > > > >> I found some other drivers set connector.encoder before drm_mode_connector_attach_encoder, some are not.
> > > > >>
> > > > >> drm_mode_connector_attach_encoder already describe the link of connector and encoder,
> > > > >> so I think "connector.encoder = encoder" is not needed, is that right?
> > > > >I'll have a look, thanks for pointing this. However, my setup uses the tda998x driver for encoder, so I will
> > > > >have to go look there rather than in my driver.
> > > > >
> > > > >Best regards,
> > > > >Liviu
> > > > >
> > > > >> Thanks.
> > > > >>
> > > > >> --
> > > > >> Ｍark Yao
> > > > Hi Liviu
> > > > drivers/gpu/drm/i2c/tda998x_drv.c do the same things:
> > > > priv->connector.encoder = &priv->encoder;
> > > > drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder);
> > > >
> > > > I believe must be same problem.
> > >
> > > Oh, I hadn't noticed this subthread, but you came to the same conclusion
> > > as I did. I have the below local change, which I think might be good to
> > > have given that there are at least two drivers that got this wrong.
> > >
> > > Thierry
> > >
> > > --- >8 ---
> > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > > index 24c5434abd1c..56d53106b32d 100644
> > > --- a/drivers/gpu/drm/drm_crtc.c
> > > +++ b/drivers/gpu/drm/drm_crtc.c
> > > @@ -4983,6 +4983,20 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector,
> > > {
> > > int i;
> > >
> > > + /*
> > > + * In the past, drivers have attempted to model the static association
> > > + * of connector to encoder in simple connector/encoder devices using a
> > > + * direct assignment of connector->encoder = encoder. This connection
> > > + * is a logical one and the responsibility of the core, so drivers are
> > > + * expected not to mess with this.
> > > + *
> > > + * Note that the error return should've been enough here, but a large
> > > + * majority of drivers ignores the return value, so add in a big WARN
> > > + * to get people's attention.
> > > + */
> > > + if (WARN_ON(connector->encoder))
> > > + return -EINVAL;
> > > +
> > > for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> > > if (connector->encoder_ids[i] == 0) {
> > > connector->encoder_ids[i] = encoder->base.id;
> >
> > Hi Thierry,
> >
> > This patch together with your tda998x proposed patch should solve
> > nicely the problem we are detecting, as long as no one calls
> > drm_connector_get_encoder() before drm_crtc_helper_set_config().
>
> The only caller of drm_connector_get_encoder() is drm_mode_getconnector()
> and that handles the NULL case gracefully, so I don't think there are any
> issues.
Fine :)
I'm happy to test any patch you might have and give you my Tested-by.
Best regards,
Liviu
>
> Thierry
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
2015-11-12 16:28 ` Thierry Reding
@ 2015-11-16 16:30 ` Daniel Vetter
-1 siblings, 0 replies; 34+ messages in thread
From: Daniel Vetter @ 2015-11-16 16:30 UTC (permalink / raw)
To: Thierry Reding
Cc: Liviu Dudau, Daniel Vetter, David Airlie, Rob Clark, DRI devel, lkml
On Thu, Nov 12, 2015 at 05:28:01PM +0100, Thierry Reding wrote:
> On Thu, Nov 12, 2015 at 01:57:11PM +0000, Liviu Dudau wrote:
> > On Thu, Nov 12, 2015 at 01:16:55PM +0100, Thierry Reding wrote:
> > > On Wed, Nov 11, 2015 at 04:09:42PM +0000, Liviu Dudau wrote:
> > > > On Tue, Nov 10, 2015 at 05:56:15PM +0100, Thierry Reding wrote:
> > > > > On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
> > > > > > Hello,
> > > > > >
> > > > > > When booting my Juno board with the HDLCD driver that I have converted to
> > > > > > atomic operations I'm getting the following warning:
> > > > >
> > > > > Perhaps you can provide pointers to the source code, that might make it
> > > > > easier for people to spot what's going wrong.
> > > > >
> > > > > Thierry
> > > >
> > > > Hi Thierry,
> > > >
> > > > I have just pushed to the mailing lists my patch series. [1] [2]
> > > >
> > > > If you want to checkout the code I also have a branch here:
> > > >
> > > > git://git://linux-arm.org/linux-ld testing/hdlcd
> > >
> > > Okay, so if I understand correctly you're using the tda998x as encoder/
> > > connector attached to your new HDLCD driver. I think the problem isn't
> > > so much that nothing has set up the CRTC for the encoder, but rather
> > > that the tda998x encoder tries to statically associate the encoder with
> > > the connector. While that might be correct in that it represents the
> > > hardware state (i.e. there is no way to physically route it anywhere
> > > else), the DRM logical state is that it's not connected until a complete
> > > pipeline has been set up, in which case a CRTC would have been assigned
> > > to the encoder.
> > >
> > > If your setup were working correctly you'd never reach the WARN_ON
> > > because the
> > >
> > > if (connector->encoder) {
> > >
> > > conditional (on line 673 in drivers/gpu/drm/drm_atomic_helper.c) would
> > > have evaluated to false already, since logically there'd be no encoder
> > > associated with the connector yet.
> >
> > Your analysis is spot on and matches my conditions. However ...
> >
> > >
> > > Does the patch below help? Let me know and I can produce a proper patch.
> > >
> > > Thierry
> > >
> > > --- >8 ---
> > > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> > > index 896b6aaf8c4d..8b0a402190eb 100644
> > > --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> > > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> > > @@ -1453,7 +1453,6 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
> > > if (ret)
> > > goto err_sysfs;
> > >
> > > - priv->connector.encoder = &priv->encoder;
> >
> > while this patch helps (no WARN() printed) I'm not sure this is the right fix.
> > TDA998x is also used by armada DRM driver which has not been converted (yet) to
> > atomic modesetting. And judging by the code and comment of drm_connector_get_encoder()
> > in drm_crtc.c, having access to the encoder through the connector is the non-atomic
> > way of doing things:
> >
> > static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *connector)
> > {
> > /* For atomic drivers only state objects are synchronously updated and
> > * protected by modeset locks, so check those first. */
> > if (connector->state)
> > return connector->state->best_encoder;
> > return connector->encoder;
> > }
> >
> > To me, it looks like the WARN() is bogus when the atomic modesetting is used in
> > drm_atomic_helper_update_legacy_modeset_state(). Either we acknowledge that the "legacy"
> > code sets the connector in the encoder structure before complete pipeline is setup
> > and we remove the WARN(), or we find a better way of detecting that some sort of mixed
> > support is in place (i.e. atomic and non-atomic aware code running) and we clean up
> > in a way that is compatible with both worlds.
>
> I think the problem you're seeing here is specifically caused by drivers
> setting up the connector->encoder explicitly. There should be no reason
> to do that. The DRM core will automatically do that when setting up a
> default configuration, or as a result of userspace setting up the wanted
> configuration. You're also likely only seeing this the first time around
> and subsequent calls will not trigger this anymore because at that point
> the core will have reset connector->encoder to NULL as part of tearing
> down the pipeline.
>
> To corroborate that, the Tegra driver doesn't trigger this WARN_ON() and
> also never sets up the connector->encoder manually. The same is probably
> true for many other drivers that have already been converted. That said,
> from a brief look it seems like many more drivers get this wrong and may
> run into this WARN when they get converted to atomic.
Thierry's analysis is spot-on, drm_connector->encoder shouldn't ever be
set by drives who either use atomic helpers or the old legacy crtc
helpers. Which is everyone (now that i915 moved from it's own modeset code
to using atomic helpers).
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> on the above tda patch
if wrapped in an appropriate commit message.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ?
@ 2015-11-16 16:30 ` Daniel Vetter
0 siblings, 0 replies; 34+ messages in thread
From: Daniel Vetter @ 2015-11-16 16:30 UTC (permalink / raw)
To: Thierry Reding; +Cc: Liviu Dudau, lkml, DRI devel
On Thu, Nov 12, 2015 at 05:28:01PM +0100, Thierry Reding wrote:
> On Thu, Nov 12, 2015 at 01:57:11PM +0000, Liviu Dudau wrote:
> > On Thu, Nov 12, 2015 at 01:16:55PM +0100, Thierry Reding wrote:
> > > On Wed, Nov 11, 2015 at 04:09:42PM +0000, Liviu Dudau wrote:
> > > > On Tue, Nov 10, 2015 at 05:56:15PM +0100, Thierry Reding wrote:
> > > > > On Tue, Nov 10, 2015 at 03:01:03PM +0000, Liviu Dudau wrote:
> > > > > > Hello,
> > > > > >
> > > > > > When booting my Juno board with the HDLCD driver that I have converted to
> > > > > > atomic operations I'm getting the following warning:
> > > > >
> > > > > Perhaps you can provide pointers to the source code, that might make it
> > > > > easier for people to spot what's going wrong.
> > > > >
> > > > > Thierry
> > > >
> > > > Hi Thierry,
> > > >
> > > > I have just pushed to the mailing lists my patch series. [1] [2]
> > > >
> > > > If you want to checkout the code I also have a branch here:
> > > >
> > > > git://git://linux-arm.org/linux-ld testing/hdlcd
> > >
> > > Okay, so if I understand correctly you're using the tda998x as encoder/
> > > connector attached to your new HDLCD driver. I think the problem isn't
> > > so much that nothing has set up the CRTC for the encoder, but rather
> > > that the tda998x encoder tries to statically associate the encoder with
> > > the connector. While that might be correct in that it represents the
> > > hardware state (i.e. there is no way to physically route it anywhere
> > > else), the DRM logical state is that it's not connected until a complete
> > > pipeline has been set up, in which case a CRTC would have been assigned
> > > to the encoder.
> > >
> > > If your setup were working correctly you'd never reach the WARN_ON
> > > because the
> > >
> > > if (connector->encoder) {
> > >
> > > conditional (on line 673 in drivers/gpu/drm/drm_atomic_helper.c) would
> > > have evaluated to false already, since logically there'd be no encoder
> > > associated with the connector yet.
> >
> > Your analysis is spot on and matches my conditions. However ...
> >
> > >
> > > Does the patch below help? Let me know and I can produce a proper patch.
> > >
> > > Thierry
> > >
> > > --- >8 ---
> > > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> > > index 896b6aaf8c4d..8b0a402190eb 100644
> > > --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> > > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> > > @@ -1453,7 +1453,6 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data)
> > > if (ret)
> > > goto err_sysfs;
> > >
> > > - priv->connector.encoder = &priv->encoder;
> >
> > while this patch helps (no WARN() printed) I'm not sure this is the right fix.
> > TDA998x is also used by armada DRM driver which has not been converted (yet) to
> > atomic modesetting. And judging by the code and comment of drm_connector_get_encoder()
> > in drm_crtc.c, having access to the encoder through the connector is the non-atomic
> > way of doing things:
> >
> > static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *connector)
> > {
> > /* For atomic drivers only state objects are synchronously updated and
> > * protected by modeset locks, so check those first. */
> > if (connector->state)
> > return connector->state->best_encoder;
> > return connector->encoder;
> > }
> >
> > To me, it looks like the WARN() is bogus when the atomic modesetting is used in
> > drm_atomic_helper_update_legacy_modeset_state(). Either we acknowledge that the "legacy"
> > code sets the connector in the encoder structure before complete pipeline is setup
> > and we remove the WARN(), or we find a better way of detecting that some sort of mixed
> > support is in place (i.e. atomic and non-atomic aware code running) and we clean up
> > in a way that is compatible with both worlds.
>
> I think the problem you're seeing here is specifically caused by drivers
> setting up the connector->encoder explicitly. There should be no reason
> to do that. The DRM core will automatically do that when setting up a
> default configuration, or as a result of userspace setting up the wanted
> configuration. You're also likely only seeing this the first time around
> and subsequent calls will not trigger this anymore because at that point
> the core will have reset connector->encoder to NULL as part of tearing
> down the pipeline.
>
> To corroborate that, the Tegra driver doesn't trigger this WARN_ON() and
> also never sets up the connector->encoder manually. The same is probably
> true for many other drivers that have already been converted. That said,
> from a brief look it seems like many more drivers get this wrong and may
> run into this WARN when they get converted to atomic.
Thierry's analysis is spot-on, drm_connector->encoder shouldn't ever be
set by drives who either use atomic helpers or the old legacy crtc
helpers. Which is everyone (now that i915 moved from it's own modeset code
to using atomic helpers).
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> on the above tda patch
if wrapped in an appropriate commit message.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2015-11-16 16:30 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-10 15:01 drm: Bogus WARN() in drm_atomic_helper_update_legacy_modeset_state() ? Liviu Dudau
2015-11-10 15:01 ` Liviu Dudau
2015-11-10 16:56 ` Thierry Reding
2015-11-10 16:56 ` Thierry Reding
2015-11-11 16:09 ` Liviu Dudau
2015-11-11 16:09 ` Liviu Dudau
2015-11-12 12:16 ` Thierry Reding
2015-11-12 12:16 ` Thierry Reding
2015-11-12 13:57 ` Liviu Dudau
2015-11-12 13:57 ` Liviu Dudau
2015-11-12 16:28 ` Thierry Reding
2015-11-12 16:28 ` Thierry Reding
2015-11-16 16:30 ` Daniel Vetter
2015-11-16 16:30 ` Daniel Vetter
2015-11-12 6:27 ` Mark yao
2015-11-12 6:34 ` Mark yao
2015-11-12 10:34 ` Liviu Dudau
2015-11-12 10:34 ` Liviu Dudau
2015-11-12 10:52 ` Mark yao
2015-11-12 10:52 ` Mark yao
2015-11-12 12:26 ` Thierry Reding
2015-11-12 12:26 ` Thierry Reding
2015-11-12 8:32 ` Mark yao
2015-11-12 10:36 ` Liviu Dudau
2015-11-12 10:36 ` Liviu Dudau
2015-11-12 10:49 ` Mark yao
2015-11-12 10:49 ` Mark yao
2015-11-12 13:34 ` Thierry Reding
2015-11-12 13:34 ` Thierry Reding
2015-11-12 14:03 ` Liviu Dudau
2015-11-12 16:12 ` Thierry Reding
2015-11-12 16:12 ` Thierry Reding
2015-11-12 16:48 ` Liviu Dudau
2015-11-12 16:48 ` Liviu Dudau
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.