From patchwork Thu Jul 20 16:02:20 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Liviu Dudau X-Patchwork-Id: 108411 Delivered-To: patch@linaro.org Received: by 10.140.101.44 with SMTP id t41csp2317561qge; Thu, 20 Jul 2017 09:02:33 -0700 (PDT) X-Received: by 10.84.210.108 with SMTP id z99mr4790450plh.280.1500566553772; Thu, 20 Jul 2017 09:02:33 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1500566553; cv=none; d=google.com; s=arc-20160816; b=HsWZd7MWwHHchf/Xo0GZbVOgDX/sdpmyzROJkiUCNASqwbEXDOwVhdC+uZOIX9SLB3 GZFTZOW4bCCfAYxpsxZVx9FzwloRSP5XALuYMrxm7gJE8mMYCCtG52CjyiJgyhbehkAD Fc53Qp8maLG9ObEMAqspNQc5t/HRi/ciQGFMQu+HYSwe8yHOtCRL04pTPw3YrkHlZQlX v/pI4OsObgsQaURyDqVfsn7e4vkRratH8RWn3xnAgvZDNjNtE+JegA6zwERW4mZm9yCj tdd5s7xUKjmTQ3BZchs5iREqq8+/it/T2eKE5jFc+LmHXkU5sR1igwsabbIUOxHXPgXI OE/w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:mime-version :list-subscribe:list-help:list-post:list-archive:list-unsubscribe :list-id:precedence:cc:message-id:date:subject:to:from:delivered-to :arc-authentication-results; bh=W+PCkQyspM7PEiYDPn6Qw/zUcuschquVGHdNBwFQWoA=; b=Ru+wxQQW3NEG9s62clxAJ8gJUFOzicecdRwASg3I/OIQG4Rdy2auQbiWKIdp3qbkWq +0soi/b2/9BaEIGByUSYzsi32bH4A80rTcpCRO3Q6EI++0kgkMBWxMORyOl1zcQiDAyy hDz7dr5Eq2OFhREajvBP9f6bs6pP9uGFjFO+5QWp0CeqsOg7vpMiW1FIAAolqwsAc4yh NzdhxN6QCaEeR1RSDY9epRHV1RTEcoqMOQHpFmeezobJfgxTq5bjGmh/EMqhLEaYLUc7 rqgBGwNO2HTFXHsNKMtYsvbxuqs+iqkWFsXEcFUioXWi9ODaaB/iX94G8Rvs6AGfkLZs NtNg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of dri-devel-bounces@lists.freedesktop.org designates 131.252.210.177 as permitted sender) smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Return-Path: Received: from gabe.freedesktop.org (gabe.freedesktop.org. [131.252.210.177]) by mx.google.com with ESMTPS id t69si1755243pfi.550.2017.07.20.09.02.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 20 Jul 2017 09:02:33 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of dri-devel-bounces@lists.freedesktop.org designates 131.252.210.177 as permitted sender) client-ip=131.252.210.177; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of dri-devel-bounces@lists.freedesktop.org designates 131.252.210.177 as permitted sender) smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1AA9A6E13C; Thu, 20 Jul 2017 16:02:32 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from cam-smtp0.cambridge.arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by gabe.freedesktop.org (Postfix) with ESMTPS id 068486E13C for ; Thu, 20 Jul 2017 16:02:30 +0000 (UTC) Received: from e110455-lin.cambridge.arm.com (e110455-lin.cambridge.arm.com [10.2.131.9]) by cam-smtp0.cambridge.arm.com (8.13.8/8.13.8) with ESMTP id v6KG2KIe030091; Thu, 20 Jul 2017 17:02:21 +0100 From: Liviu Dudau To: Russell King Subject: [PATCH v3] drm/i2c: tda998x: Fix lockdep warning about possible circular dependency Date: Thu, 20 Jul 2017 17:02:20 +0100 Message-Id: <20170720160220.8228-1-Liviu.Dudau@arm.com> X-Mailer: git-send-email 2.13.2 Cc: Daniel Vetter , Russell King , LKML , DRI devel X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" When enabling lockdep debugging on Juno platform with HDLCD and TDA998x I get the following warning from the system: [ 25.990733] ====================================================== [ 25.998637] WARNING: possible circular locking dependency detected [ 26.006531] 4.13.0-rc1-00284-g28c0a682ecbf-dirty #17 Not tainted [ 26.014246] ------------------------------------------------------ [ 26.022142] kworker/1:2/140 is trying to acquire lock: [ 26.029001] (&priv->audio_mutex){+.+.+.}, at: [] tda998x_encoder_mode_set+0x12c/0x5a0 [tda998x] [ 26.041100] [ 26.041100] but task is already holding lock: [ 26.050436] (crtc_ww_class_mutex){+.+.+.}, at: [] drm_modeset_lock+0x64/0xf8 [drm] [ 26.061531] [ 26.061531] which lock already depends on the new lock. [ 26.061531] [ 26.075063] [ 26.075063] the existing dependency chain (in reverse order) is: [ 26.086031] [ 26.086031] -> #2 (crtc_ww_class_mutex){+.+.+.}: [ 26.095657] __lock_acquire+0x18a0/0x19b8 [ 26.101918] lock_acquire+0xd0/0x2b0 [ 26.107731] __ww_mutex_lock.constprop.3+0x90/0xe78 [ 26.114817] ww_mutex_lock+0x54/0xe0 [ 26.120672] drm_modeset_lock+0x64/0xf8 [drm] [ 26.127253] drm_helper_probe_single_connector_modes+0x7c/0x6b8 [drm_kms_helper] [ 26.136829] tda998x_connector_fill_modes+0x44/0xa8 [tda998x] [ 26.144797] drm_setup_crtcs+0x19c/0xba0 [drm_kms_helper] [ 26.152429] drm_fb_helper_initial_config+0x70/0x440 [drm_kms_helper] [ 26.161097] drm_fbdev_cma_init_with_funcs+0x94/0x168 [drm_kms_helper] [ 26.169857] drm_fbdev_cma_init+0x38/0x50 [drm_kms_helper] [ 26.177559] hdlcd_drm_bind+0x1f8/0x4a8 [hdlcd] [ 26.184310] try_to_bring_up_master+0x180/0x1e0 [ 26.191043] component_master_add_with_match+0xb0/0x108 [ 26.198458] hdlcd_probe+0x58/0x80 [hdlcd] [ 26.204735] platform_drv_probe+0x60/0xc0 [ 26.210913] driver_probe_device+0x23c/0x2e8 [ 26.217350] __driver_attach+0xd4/0xd8 [ 26.223256] bus_for_each_dev+0x5c/0xa8 [ 26.229232] driver_attach+0x30/0x40 [ 26.234917] bus_add_driver+0x1d8/0x248 [ 26.240831] driver_register+0x6c/0x118 [ 26.246715] __platform_driver_register+0x54/0x60 [ 26.253461] 0xffff000000e1b018 [ 26.258644] do_one_initcall+0x44/0x138 [ 26.264503] do_init_module+0x64/0x1d4 [ 26.270238] load_module+0x1f90/0x2590 [ 26.275957] SyS_finit_module+0xb0/0xc8 [ 26.281765] __sys_trace_return+0x0/0x4 [ 26.281767] [ 26.281767] -> #1 (crtc_ww_class_acquire){+.+.+.}: [ 26.281778] __lock_acquire+0x18a0/0x19b8 [ 26.281782] lock_acquire+0xd0/0x2b0 [ 26.281877] drm_modeset_acquire_init+0xa8/0xe0 [drm] [ 26.281921] drm_helper_probe_single_connector_modes+0x48/0x6b8 [drm_kms_helper] [ 26.281929] tda998x_connector_fill_modes+0x44/0xa8 [tda998x] [ 26.281970] drm_setup_crtcs+0x19c/0xba0 [drm_kms_helper] [ 26.282009] drm_fb_helper_initial_config+0x70/0x440 [drm_kms_helper] [ 26.282049] drm_fbdev_cma_init_with_funcs+0x94/0x168 [drm_kms_helper] [ 26.282088] drm_fbdev_cma_init+0x38/0x50 [drm_kms_helper] [ 26.282095] hdlcd_drm_bind+0x1f8/0x4a8 [hdlcd] [ 26.282099] try_to_bring_up_master+0x180/0x1e0 [ 26.282104] component_master_add_with_match+0xb0/0x108 [ 26.282110] hdlcd_probe+0x58/0x80 [hdlcd] [ 26.282114] platform_drv_probe+0x60/0xc0 [ 26.282117] driver_probe_device+0x23c/0x2e8 [ 26.282121] __driver_attach+0xd4/0xd8 [ 26.282124] bus_for_each_dev+0x5c/0xa8 [ 26.282127] driver_attach+0x30/0x40 [ 26.282130] bus_add_driver+0x1d8/0x248 [ 26.282134] driver_register+0x6c/0x118 [ 26.282138] __platform_driver_register+0x54/0x60 [ 26.282141] 0xffff000000e1b018 [ 26.282145] do_one_initcall+0x44/0x138 [ 26.282149] do_init_module+0x64/0x1d4 [ 26.282152] load_module+0x1f90/0x2590 [ 26.282156] SyS_finit_module+0xb0/0xc8 [ 26.282159] __sys_trace_return+0x0/0x4 [ 26.282161] [ 26.282161] -> #0 (&priv->audio_mutex){+.+.+.}: [ 26.282172] print_circular_bug+0x80/0x2e0 [ 26.282176] __lock_acquire+0x15a8/0x19b8 [ 26.282180] lock_acquire+0xd0/0x2b0 [ 26.282184] __mutex_lock+0x78/0x8e0 [ 26.282188] mutex_lock_nested+0x3c/0x50 [ 26.282196] tda998x_encoder_mode_set+0x12c/0x5a0 [tda998x] [ 26.282237] drm_atomic_helper_commit_modeset_disables+0x328/0x3a0 [drm_kms_helper] [ 26.282251] malidp_atomic_commit_tail+0x44/0x6b0 [mali_dp] [ 26.282292] commit_tail+0x4c/0x80 [drm_kms_helper] [ 26.282333] drm_atomic_helper_commit+0xe8/0x180 [drm_kms_helper] [ 26.282427] drm_atomic_commit+0x54/0x70 [drm] [ 26.282467] restore_fbdev_mode_atomic+0x1f0/0x220 [drm_kms_helper] [ 26.282507] restore_fbdev_mode+0x38/0x188 [drm_kms_helper] [ 26.282547] drm_fb_helper_restore_fbdev_mode_unlocked+0x44/0xd0 [drm_kms_helper] [ 26.282586] drm_fb_helper_set_par+0x34/0x80 [drm_kms_helper] [ 26.282625] drm_fb_helper_hotplug_event.part.19+0x94/0xb0 [drm_kms_helper] [ 26.282665] drm_fb_helper_hotplug_event+0x2c/0x48 [drm_kms_helper] [ 26.282704] drm_fbdev_cma_hotplug_event+0x24/0x30 [drm_kms_helper] [ 26.282716] malidp_output_poll_changed+0x24/0x30 [mali_dp] [ 26.282757] drm_kms_helper_hotplug_event+0x34/0x40 [drm_kms_helper] [ 26.282797] output_poll_execute+0x1a0/0x1f0 [drm_kms_helper] [ 26.282803] process_one_work+0x280/0x790 [ 26.282808] worker_thread+0x48/0x450 [ 26.282812] kthread+0x138/0x140 [ 26.282815] ret_from_fork+0x10/0x40 [ 26.282817] [ 26.282817] other info that might help us debug this: [ 26.282817] [ 26.282819] Chain exists of: [ 26.282819] &priv->audio_mutex --> crtc_ww_class_acquire --> crtc_ww_class_mutex [ 26.282819] [ 26.282830] Possible unsafe locking scenario: [ 26.282830] [ 26.282832] CPU0 CPU1 [ 26.282834] ---- ---- [ 26.282835] lock(crtc_ww_class_mutex); [ 26.282840] lock(crtc_ww_class_acquire); [ 26.282845] lock(crtc_ww_class_mutex); [ 26.282850] lock(&priv->audio_mutex); [ 26.282854] [ 26.282854] *** DEADLOCK *** [ 26.282854] [ 26.282858] 5 locks held by kworker/1:2/140: [ 26.282859] #0: ("events"){.+.+.+}, at: [] process_one_work+0x1d8/0x790 [ 26.282871] #1: ((&(&dev->mode_config.output_poll_work)->work)){+.+.+.}, at: [] process_one_work+0x1d8/0x790 [ 26.282883] #2: (&helper->lock){+.+.+.}, at: [] drm_fb_helper_restore_fbdev_mode_unlocked+0x3c/0xd0 [drm_kms_helper] [ 26.282929] #3: (crtc_ww_class_acquire){+.+.+.}, at: [] restore_fbdev_mode_atomic+0x38/0x220 [drm_kms_helper] [ 26.282976] #4: (crtc_ww_class_mutex){+.+.+.}, at: [] drm_modeset_lock+0x64/0xf8 [drm] [ 26.283077] [ 26.283077] stack backtrace: [ 26.283082] CPU: 1 PID: 140 Comm: kworker/1:2 Not tainted 4.13.0-rc1-00284-g28c0a682ecbf-dirty #17 [ 26.283084] Hardware name: ARM Juno development board (r0) (DT) [ 26.283127] Workqueue: events output_poll_execute [drm_kms_helper] [ 26.283131] Call trace: [ 26.283137] [] dump_backtrace+0x0/0x268 [ 26.283142] [] show_stack+0x24/0x30 [ 26.283146] [] dump_stack+0xbc/0xf4 [ 26.283151] [] print_circular_bug+0x1d4/0x2e0 [ 26.283155] [] __lock_acquire+0x15a8/0x19b8 [ 26.283159] [] lock_acquire+0xd0/0x2b0 [ 26.283163] [] __mutex_lock+0x78/0x8e0 [ 26.283168] [] mutex_lock_nested+0x3c/0x50 [ 26.283176] [] tda998x_encoder_mode_set+0x12c/0x5a0 [tda998x] [ 26.283217] [] drm_atomic_helper_commit_modeset_disables+0x328/0x3a0 [drm_kms_helper] [ 26.283230] [] malidp_atomic_commit_tail+0x44/0x6b0 [mali_dp] [ 26.283271] [] commit_tail+0x4c/0x80 [drm_kms_helper] [ 26.283312] [] drm_atomic_helper_commit+0xe8/0x180 [drm_kms_helper] [ 26.283406] [] drm_atomic_commit+0x54/0x70 [drm] [ 26.283447] [] restore_fbdev_mode_atomic+0x1f0/0x220 [drm_kms_helper] [ 26.283487] [] restore_fbdev_mode+0x38/0x188 [drm_kms_helper] [ 26.283526] [] drm_fb_helper_restore_fbdev_mode_unlocked+0x44/0xd0 [drm_kms_helper] [ 26.283566] [] drm_fb_helper_set_par+0x34/0x80 [drm_kms_helper] [ 26.283606] [] drm_fb_helper_hotplug_event.part.19+0x94/0xb0 [drm_kms_helper] [ 26.283645] [] drm_fb_helper_hotplug_event+0x2c/0x48 [drm_kms_helper] [ 26.283685] [] drm_fbdev_cma_hotplug_event+0x24/0x30 [drm_kms_helper] [ 26.283697] [] malidp_output_poll_changed+0x24/0x30 [mali_dp] [ 26.283738] [] drm_kms_helper_hotplug_event+0x34/0x40 [drm_kms_helper] [ 26.283779] [] output_poll_execute+0x1a0/0x1f0 [drm_kms_helper] [ 26.283784] [] process_one_work+0x280/0x790 [ 26.283788] [] worker_thread+0x48/0x450 [ 26.283792] [] kthread+0x138/0x140 [ 26.283796] [] ret_from_fork+0x10/0x40 This looks like it has been introduced by 'commit 02efac0fbfd5 ("drm/i2c: tda998x: remove complexity from tda998x_audio_get_eld()")'. Fix the warning by dropping the use of local audio_mutex and switch to taking the modeset connection_mutex in tda998x_audio_get_eld() for avoidance of race conditions with drm_helper_probe_single_connector_modes() updating the ELD data. v2: Change to taking the modeset connection_mutex rather than changing the time when audio_mutex lock was taken, as suggested by Russell King. v3: Drop the bespoke drm_modeset_acquire_ctx, it is not needed. (suggested by Daniel Vetter) Signed-off-by: Liviu Dudau Cc: Russell King Cc: Daniel Vetter Fixes: 02efac0fbfd5 ("drm/i2c: tda998x: remove complexity from tda998x_audio_get_eld()") --- drivers/gpu/drm/i2c/tda998x_drv.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index d1e7ac540199..a0c82345661a 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -926,13 +926,18 @@ static int tda998x_audio_get_eld(struct device *dev, void *data, uint8_t *buf, size_t len) { struct tda998x_priv *priv = dev_get_drvdata(dev); + struct drm_device *drm_dev = priv->connector.dev; + int ret; - mutex_lock(&priv->audio_mutex); - memcpy(buf, priv->connector.eld, - min(sizeof(priv->connector.eld), len)); - mutex_unlock(&priv->audio_mutex); + ret = drm_modeset_lock(&drm_dev->mode_config.connection_mutex, NULL); + if (!ret) { + memcpy(buf, priv->connector.eld, + min(sizeof(priv->connector.eld), len)); + } - return 0; + drm_modeset_unlock(&drm_dev->mode_config.connection_mutex); + + return ret; } static const struct hdmi_codec_ops audio_codec_ops = { @@ -983,7 +988,6 @@ static int tda998x_connector_fill_modes(struct drm_connector *connector, struct tda998x_priv *priv = conn_to_tda998x_priv(connector); int ret; - mutex_lock(&priv->audio_mutex); ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY); if (connector->edid_blob_ptr) { @@ -993,7 +997,6 @@ static int tda998x_connector_fill_modes(struct drm_connector *connector, } else { priv->sink_has_audio = false; } - mutex_unlock(&priv->audio_mutex); return ret; }