On Sat, Apr 12, 2014 at 10:42:52PM +0200, Daniel Vetter wrote:

On Sat, Apr 12, 2014 at 12:32 PM, Russell King - ARM Linux linux@arm.linux.org.uk wrote:

...

I've been seeing the following warning when a mode is attempted to be set after boot when no display connectors are connected. This typically happens if your platform has just a HDMI output, and the TV connected to the HDMI output is fully powered off.

This has been happening for quite some time now... I should've probably reported it earlier. I've seen it both with imx-drm and armada-drm.

WARNING: CPU: 0 PID: 846 at drivers/gpu/drm/drm_crtc.c:2074 drm_mode_set_config_internal+0xd0/0xd8() Modules linked in: rfcomm bnep bluetooth 6lowpan_iphc brcmfmac brcmutil snd_soc_fsl_spdif imx_pcm_dma imx_sdma imx_thermal imx2_wdt snd_soc_imx_spdif nfsd exportfs CPU: 0 PID: 846 Comm: Xorg Not tainted 3.14.0+ #1027 Backtrace: [<c0011cf4>] (dump_backtrace) from [<c0011e94>] (show_stack+0x18/0x1c) r6:0000081a r5:00000009 r4:00000000 r3:00000000 [<c0011e7c>] (show_stack) from [<c066b4e8>] (dump_stack+0x74/0x90) [<c066b474>] (dump_stack) from [<c0024588>] (warn_slowpath_common+0x70/0x90) r4:00000000 r3:c09344b8 [<c0024518>] (warn_slowpath_common) from [<c00245cc>] (warn_slowpath_null+0x24/0x2c) r8:ea3c3818 r7:00000001 r6:e9904500 r5:00000000 r4:ea3c3818 [<c00245a8>] (warn_slowpath_null) from [<c03474e4>] (drm_mode_set_config_internal+0xd0/0xd8) [<c0347414>] (drm_mode_set_config_internal) from [<c03497c8>] (drm_mode_setcrtc+0x378/0x4b4) r6:ea3c3400 r5:e8d78b00 r4:e9981e48 r3:00000000 [<c0349450>] (drm_mode_setcrtc) from [<c033c9d0>] (drm_ioctl+0x3f4/0x498) r10:c06b8fa0 r9:e9981e48 r8:e9981e48 r7:e9980000 r6:e98260c0 r5:000000a2 r4:ea3c3400 [<c033c5dc>] (drm_ioctl) from [<c01095b8>] (do_vfs_ioctl+0x80/0x5d4) r10:00000000 r9:e9980000 r8:00000006 r7:c06864a2 r6:be911850 r5:e9826900 r4:ea3bf100 [<c0109538>] (do_vfs_ioctl) from [<c0109b4c>] (SyS_ioctl+0x40/0x5c) r9:e9980000 r8:00000006 r7:c06864a2 r6:be911850 r5:e9826900 r4:e9826900 [<c0109b0c>] (SyS_ioctl) from [<c000e6e0>] (ret_fast_syscall+0x0/0x30) r8:c000e864 r7:00000036 r6:00000006 r5:c06864a2 r4:be911850 r3:00000001 ---[ end trace 46155b08f61d1185 ]---

In drm_mode_set_config_internal(), this check is done:

    fb = set->fb;

    ret = crtc->funcs->set_config(set);
    if (ret == 0) {
            /* crtc->fb must be updated by ->set_config, enforces this. */
            WARN_ON(fb != crtc->fb);
    }

However, drm_crtc_helper_set_config() does this:

    if (!set->mode)
            set->fb = NULL;

    if (set->fb) {

... } else { DRM_DEBUG_KMS("[CRTC:%d] [NOFB]\n", set->crtc->base.id); return drm_crtc_helper_disable(set->crtc); }

which results in a zero return with crtc->fb set to NULL, possibly being different to what set->fb was originally.

I haven't debugged this fully yet as the only side effect is the warning.

This is strange - passing a NULL mode and a non-NULL fb to ->set_config is a bug, and in the i915 ->set_config callback (intel_crtc_set_config) we even have a BUG_ON to check for that. A few release ago the fb helper code violated this which wreak a bit of havoc in the i915 modeset code until I've fixed it, but now this really should hold everywhere.

That was just a guess.

drm_mode_set_config_internal: pre: fb (null) mode (null) crtc dae72418 crtc->fb (null) drm_crtc_helper_set_config: disable drm_helper_disable_unused_functions: crtc dae72018 crtc->fb (null) drm_helper_disable_unused_functions: crtc dae72418 crtc->fb (null) drm_mode_set_config_internal: post: fb (null) mode (null) crtc->fb (null) ret 0

drm_mode_set_config_internal: pre: fb dac09680 mode db6b1900 crtc dae72018 crtc->fb (null) drm_helper_disable_unused_functions: crtc dae72018 crtc->fb dac09680 drm_helper_disable_unused_functions: crtc dae72418 crtc->fb (null) drm_crtc_helper_set_config: success drm_mode_set_config_internal: post: fb dac09680 mode db6b1900 crtc->fb (null) ret 0 ------------[ cut here ]------------ WARNING: CPU: 0 PID: 866 at drivers/gpu/drm/drm_crtc.c:2075 drm_mode_set_config_internal+0x11c/0x130()

'fb' is set->fb, 'mode' is set->mode.

So what happens is that a mode is attempted to be set with no connectors. We get to the

if (mode_changed) {

bit in drm_crtc_helper_set_config(). I haven't checked whether drm_helper_crtc_in_use() returns true, I guess it doesn't. We then fall down to drm_helper_disable_unused_functions() which disables the CRTC we're trying to set the mode for, setting crtc->fb to NULL.

We then return success, which results in the check for crtc->fb == set->fb, which fails.

On Sat, Apr 12, 2014 at 11:49:44PM +0200, Daniel Vetter wrote:

On Sat, Apr 12, 2014 at 11:20 PM, Russell King - ARM Linux linux@arm.linux.org.uk wrote:

...

So what happens is that a mode is attempted to be set with no connectors.

That case is the other nonsense use of ->set_config I've curbed with all the fb helper fixes.

BUG_ON(set->fb && set->num_connectors == 0);

in drm_crtc_helper_set_config should catch this abuse of trying to set a mode with no connectors. So I'm still confused what exactly is going on in your box.

Here's the DRM debug enabled:

drm_mode_set_config_internal: pre: fb d8407080 mode dab0ab00 crtc dae72018 crtc->fb (null) [drm:drm_crtc_helper_set_config], [drm:drm_crtc_helper_set_config], [CRTC:3] [FB:11] #connectors=1 (x y) (0 0) [drm:drm_crtc_helper_set_config], crtc has no fb, full mode set [drm:drm_crtc_helper_set_config], encoder changed, full mode switch [drm:drm_crtc_helper_set_config], crtc changed, full mode switch [drm:drm_crtc_helper_set_config], [CONNECTOR:9:HDMI-A-1] to [CRTC:3] [drm:drm_crtc_helper_set_config], attempting to set mode from userspace [drm:drm_mode_debug_printmodeline], Modeline 12:"1360x768" 0 84750 1360 1432 1568 1776 768 771 781 798 0x0 0x6 [drm:drm_crtc_helper_set_mode], [CRTC:3] [drm:drm_crtc_helper_set_mode], [ENCODER:8:TMDS-8] set [MODE:12:1360x768] [drm:drm_calc_timestamping_constants], crtc 3: hwmode: htotal 1776, vtotal 798, vdisplay 768 [drm:drm_calc_timestamping_constants], crtc 3: clock 84750 kHz framedur 16722690 linedur 20955, pixeldur 11 [drm:drm_crtc_helper_set_config], Setting connector DPMS state to on [drm:drm_crtc_helper_set_config], [CONNECTOR:9:HDMI-A-1] set DPMS on drm_helper_disable_unused_functions: crtc dae72018 crtc->fb d8407080 drm_helper_disable_unused_functions: crtc dae72418 crtc->fb (null) drm_crtc_helper_set_config: success drm_mode_set_config_internal: post: fb d8407080 mode dab0ab00 crtc->fb (null) ret 0

This shows that the mode was being set with one connector present, and seemed to be successful, but drm_helper_disable_unused_functions() still tried to disable it.

However, adding more debugging to drm_helper_disable_unused_functions() reveals the problem:

drm_helper_disable_unused_functions: connector 9 status 2 drm_helper_disable_unused_functions: encoder 8 not in use drm_helper_disable_unused_functions: crtc 3 crtc->fb da898500 drm_helper_disable_unused_functions: crtc 6 crtc->fb (null)

So we try to set a mode on CRTC 3 using encoder 8 and connector 9. However connector 9 is disconnected, so the connector has its encoder disassociated. The encoder is now not used, so the encoder is disabled. This then means that the CRTC is not being used by any encoder, so the CRTC gets disabled.

On Sun, Apr 13, 2014 at 10:44:28AM +0200, Daniel Vetter wrote:

On Sat, Apr 12, 2014 at 11:04:31PM +0100, Russell King - ARM Linux wrote:

...

So we try to set a mode on CRTC 3 using encoder 8 and connector 9. However connector 9 is disconnected, so the connector has its encoder disassociated. The encoder is now not used, so the encoder is disabled. This then means that the CRTC is not being used by any encoder, so the CRTC gets disabled.

That's a bit strange since userspace normally shouldn't attempt to set a mode when nothing is connected, at least if you don't force a specific configuration. In any case it sounds like you want

commit e3d6ddb35f6221859b6054879d186e13a3af351e Author: Daniel Vetter daniel.vetter@ffwll.ch Date: Tue Apr 1 22:15:00 2014 +0200

drm/crtc-helper: don't disable disconnected outputs

I wonder whether we should put a cc: stable onto that one (maybe after a bit more testing) since apparently userspace can force the kernel to do stupid things if we auto-disable disconnected outputs.

Yes, adding that change makes the warning go away. Thanks.

Tested-by: Russell King rmk+kernel@arm.linux.org.uk

It may be worth including some extra information in the commit.