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: 9435591 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 EAF3460238 for ; Fri, 18 Nov 2016 00:53:34 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DB8AF294EE for ; Fri, 18 Nov 2016 00:53:34 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D0385296FD; Fri, 18 Nov 2016 00:53:34 +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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 607CD294EE for ; Fri, 18 Nov 2016 00:53:34 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1BBAE6E390; Fri, 18 Nov 2016 00:53:33 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk [IPv6:2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by gabe.freedesktop.org (Postfix) with ESMTPS id E3C7F89944 for ; Fri, 18 Nov 2016 00:01:27 +0000 (UTC) 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-Mailman-Approved-At: Fri, 18 Nov 2016 00:52:29 +0000 Cc: dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org 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: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" 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;