Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Robin Gong <yibin.gong@nxp.com>
To:     Benjamin Bara - SKIDATA <Benjamin.Bara@skidata.com>
CC:     "timur@kernel.org" <timur@kernel.org>,
        "nicoleotsuka@gmail.com" <nicoleotsuka@gmail.com>,
        "vkoul@kernel.org" <vkoul@kernel.org>,
        "dan.j.williams@intel.com" <dan.j.williams@intel.com>,
        dl-linux-imx <linux-imx@nxp.com>,
        "shawnguo@kernel.org" <shawnguo@kernel.org>,
        "kernel@pengutronix.de" <kernel@pengutronix.de>,
        "dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
        Richard Leitner - SKIDATA <Richard.Leitner@skidata.com>,
        "jiada_wang@mentor.com" <jiada_wang@mentor.com>
Subject: RE: pcm|dmaengine|imx-sdma race condition on i.MX6
Thread-Topic: pcm|dmaengine|imx-sdma race condition on i.MX6
Thread-Index: AQHWcWQbD9eqMlwY2U27XyctTVPmVak3Rd/wgASnXICAAByKsIAAKTYAgAFgnmA=
Date:   Tue, 18 Aug 2020 10:41:30 +0000
Message-ID: <VE1PR04MB66389F7AC7FD9DC401C57183895C0@VE1PR04MB6638.eurprd04.prod.outlook.com>
References: <20200813112258.GA327172@pcleri>
 <VE1PR04MB6638EE5BDBE2C65FF50B7DB889400@VE1PR04MB6638.eurprd04.prod.outlook.com>
 <61498763c60e488a825e8dd270732b62@skidata.com>
 <VE1PR04MB6638AC2A3AE852C3047E7B97895F0@VE1PR04MB6638.eurprd04.prod.outlook.com>
 <a64ae27d9f1348ecae6adc74969cc88c@skidata.com>
In-Reply-To: <a64ae27d9f1348ecae6adc74969cc88c@skidata.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 856834b5-50de-4df2-378b-08d8436344ca
X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Aug 2020 10:41:30.5573
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: TAW9XMXR+g43WM1tdMQV7/pu99rX9fLD5kTeRlR6p5JepmWoOJGrHGMfACgfey3yGw0xTTGpveIR6Z5EzYmm7A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB6016
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2020/08/17 19:38 Benjamin Bara - SKIDATA <Benjamin.Bara@skidata.com> wro=
te:
> > -----Original Message-----
> > From: Robin Gong <yibin.gong@nxp.com>
> > Sent: Montag, 17. August 2020 11:23
> > busy_wait is not good I think, would you please have a try with the
> > attached patch which is based on
> > https://eur01.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Flkm=
l
> > .org%2Flkml%2F2020%2F8%2F11%2F111&amp;data=3D02%7C01%7Cyibin.gong
> %40nxp.
> >
> com%7C96a66f37340648e998f108d842a2057e%7C686ea1d3bc2b4c6fa92cd99
> c5c301
> >
> 635%7C0%7C0%7C637332610926324334&amp;sdata=3Dvn80kNlIY%2BB9v9cOlXJ
> patNkn
> > YAMtVx6v7yhfvAE%2FRM%3D&amp;reserved=3D0? The basic idea is to keep
> the
> > freed descriptor into another list for freeing in later
> > terminate_worker instead of freeing directly all in terminate_worker
> > by vchan_get_all_descriptors which may break next descriptor coming
> > soon
>=20
> The idea sounds good, but with this attempt we are still not sure that th=
e 1ms
> (the ultimate reason why this is a problem) is awaited between DMA disabl=
ing
> and re-enabling.
The original 1ms delay is for ensuring sdma channel stop indeed, otherwise,=
 sdma may
still access IPs's fifo like uart/sai... during last Water-Mark-Level size =
transfer. The worst
is some IP such as uart may lead to sdma hang after UCR2_RXEN/ UCR2_TXEN di=
sabled
("Timeout waiting for CH0 ready" would be caught). So I would suggest synch=
ronizing
dma after channel terminated. But for PCM system's limitation, seems no cho=
ice but
terminate async. If sdma could access audio fifo without hang after PCM dri=
ver terminate
dma channel and rx/tx data buffers are not illegal, maybe 1ms is not a must
because only garbage data harmless touched by sdma and ignored by PCM drive=
r.
Current sdma driver with my patches could ensure below:
  -- The last terminated transfer will be stopped before the next quick tra=
nsfer start.
    because load context(sdma_load_context) done by channel0 which is the
    lowest priority. In other words, calling successfully dmaengine_prep_* =
in the
    next transfer means new normal transfer without any last terminated tra=
nsfer
    impact.
  -- No potential interrupt after terminated could be handled before next t=
ransfer
    start because 'sdmac->desc' has been set NULL in sdma_terminate_all.

>=20
> If we are allowed to leave the atomic PCM context on each trigger, synchr=
onize
> the DMA and then enter it back again, everything is fine.
> This might be the most performant and elegant solution.
> However, since we are in an atomic context for a reason, it might not be
> wanted by the PCM system that the DMA termination completion of the
> previous context happens within the next call, but we are not sure about =
that.
> In this case, a busy wait is not a good solution, but a necessary one, or=
 at least
> the only valid solution we are aware of.
>=20
> Anyhow, based on my understanding, either the start or the stop trigger h=
as to
> wait the 1ms (or whats left of it).