From patchwork Fri Nov 18 00:01:15 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Russell King (Oracle)" X-Patchwork-Id: 9435511 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8A18060469 for ; Fri, 18 Nov 2016 00:06:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7A413296F5 for ; Fri, 18 Nov 2016 00:06:15 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6E4872970B; Fri, 18 Nov 2016 00:06:15 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 4DCB02970A for ; Fri, 18 Nov 2016 00:06:13 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1c7Wet-0005pw-0z; Fri, 18 Nov 2016 00:03:59 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1c7Wen-0005Sr-Ev for linux-arm-kernel@lists.infradead.org; Fri, 18 Nov 2016 00:03:55 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=O75kg3rchLSPryDkoljXB3VeV1r7OBtyN7cgOgt8iGY=; b=gKKvikkodrdvvWBOuEFhor9IlGRmA5NRKKh9Qze8eRSsz+eODR+KHGVncJj4TMt20TytlyduXLeE36jUV3m4S99EwUfuGtRC9A9lrvBXHH8Yyrc0Z7kgs0FNbPOOtvtfMWYVjM7byNaks22hAbu6IFyJiPTkNqATt0yUu3e11d8=; Received: from n2100.armlinux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:38722) by pandora.armlinux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1c7WcI-0001c6-U3; Fri, 18 Nov 2016 00:01:19 +0000 Received: from linux by n2100.armlinux.org.uk with local (Exim 4.76) (envelope-from ) id 1c7WcF-00027q-Sd; Fri, 18 Nov 2016 00:01:15 +0000 Date: Fri, 18 Nov 2016 00:01:15 +0000 From: Russell King - ARM Linux To: Brian Starkey , Liviu Dudau , Jyri Sarha Subject: Re: [PATCH RFC 01/12] drm/i2c: tda998x: avoid race in tda998x_encoder_mode_set() Message-ID: <20161118000115.GI1041@n2100.armlinux.org.uk> References: <20161108122420.GP1041@n2100.armlinux.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20161117_160354_110424_E1693BB4 X-CRM114-Status: GOOD ( 21.92 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Nov 08, 2016 at 12:25:00PM +0000, Russell King wrote: > As priv->audio_params can now be changed at run time, we need to be more > careful about how we deal with a mode set. We must take the audio lock > while checking if there's a valid audio configuration. > > However, it's slightly worse than that - during mode set, we mute the > audio, and it must not be unmuted until we have finished the mode set. > It is possible that the audio side may start while a mode set is in > progress, so take the audio_mutex lock around the whole mode setting > procedure. > > Signed-off-by: Russell King > --- > drivers/gpu/drm/i2c/tda998x_drv.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c > index 088900d78ceb..1cc0433ce9d5 100644 > --- a/drivers/gpu/drm/i2c/tda998x_drv.c > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c > @@ -1074,13 +1074,12 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, > > tda998x_write_avi(priv, adjusted_mode); > > - if (priv->audio_params.format != AFMT_UNUSED) { > - mutex_lock(&priv->audio_mutex); > + mutex_lock(&priv->audio_mutex); > + if (priv->audio_params.format != AFMT_UNUSED) > tda998x_configure_audio(priv, > &priv->audio_params, > adjusted_mode->clock); > - mutex_unlock(&priv->audio_mutex); > - } > + mutex_unlock(&priv->audio_mutex); > } > } > I've just realised that this (and the following few changes) require this mutex to be always initialised. Currently, we only initialise the audio mutex when in DT mode, which is bad. So, I'm going to add this patch in front of this patch set: diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 088900d78ceb..168c0b4b8f47 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -1405,6 +1405,8 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) int rev_lo, rev_hi, ret; unsigned short cec_addr; + mutex_init(&priv->audio_mutex); /* Protect access from audio thread */ + priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(2) | VIP_CNTRL_0_SWAP_B(3); priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(0) | VIP_CNTRL_1_SWAP_D(1); priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(4) | VIP_CNTRL_2_SWAP_F(5); @@ -1519,8 +1521,6 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) priv->vip_cntrl_2 = video; } - mutex_init(&priv->audio_mutex); /* Protect access from audio thread */ - ret = tda998x_get_audio_ports(priv, np); if (ret) goto fail;