From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: dmaengine: stm32-dma: fix residue calculation in stm32-dma From: Vinod Koul Message-Id: <20190429051310.GC3845@vkoul-mobl.Dlink> Date: Mon, 29 Apr 2019 10:43:23 +0530 To: Arnaud Pouliquen Cc: Dan Williams , Pierre-Yves MORDRET , linux-stm32@st-md-mailman.stormreply.com, linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org List-ID: T24gMjYtMDQtMTksIDE1OjQxLCBBcm5hdWQgUG91bGlxdWVuIHdyb3RlOgo+ID4+IER1cmluZyBy ZXNpZHVlIGNhbGN1bGF0aW9uLiB0aGUgRE1BIGNhbiBzd2l0Y2ggdG8gdGhlIG5leHQgc2cuIFdo ZW4KPiA+PiB0aGlzIHJhY2UgY29uZGl0aW9uIG9jY3VycywgdGhlIHJlc2lkdWUgcmV0dXJuZWQg dmFsdWUgaXMgbm90IHZhbGlkLgo+ID4+IEluZGVlZCB0aGUgcG9zaXRpb24gaW4gdGhlIHNnIHJl dHVybmVkIGJ5IHRoZSBoYXJkd2FyZSBpcyB0aGUgcG9zaXRpb24KPiA+PiBvZiB0aGUgbmV4dCBz Zywgbm90IHRoZSBjdXJyZW50IHNnLgo+ID4+IFNvbHV0aW9uIGlzIHRvIGNoZWNrIHRoZSBzZyBh ZnRlciB0aGUgY2FsY3VsYXRpb24gdG8gdmVyaWZ5IGl0Lgo+ID4+IElmIGEgdHJhbnNpdGlvbiBp cyBkZXRlY3RlZCB3ZSBjb25zaWRlciB0aGF0IHRoZSBETUEgaGFzIHN3aXRjaGVkIHRvCj4gPj4g dGhlIGJlZ2lubmluZyBvZiBuZXh0IHNnLgo+ID4gCj4gPiBOb3csIHRoYXQgc291bmRzIGxpa2Ug ZHVjdCB0YXBlLiBXaHkgc2hvdWxkIHdlIGJvdGhlciBkb2luZyB0aGF0Lgo+ID4gCj4gPiBBbHNv IGxvb2tpbmcgYmFjayBhdCB0aGUgc3RtMzJfZG1hX2Rlc2NfcmVzaWR1ZSgpIGFuZCBjYWxscyB0 byBpdCBmcm9tCj4gPiBzdG0zMl9kbWFfdHhfc3RhdHVzKCkgYW0gbm90IHN1cmUgd2UgYXJlIGRv aW5nIHRoZSByaWdodCB0aGluZwo+IFBsZWFzZSwgY291bGQgeW91IGV4cGxhaW4gd2hhdCB5b3Ug aGF2ZSBpbiBtaW5kIGhlcmU/CgpTbyB3aGVuIHdlIGNhbGwgdmNoYW5fZmluZF9kZXNjKCkgdGhh dCB0ZWxscyB1cyBpZiB0aGUgZGVzY3JpcHRvciBpcyBpbgp0aGUgaXNzdWVkIHF1ZXVlIG9yIG5v dC4uICBJZGVhbGx5IGl0IHNob3VsZCBub3QgbWF0dGVyIGlmIHdlIGhhdmUgb25lCm9yIE4gZGVz Y3JpcHRvcnMgaXNzdWVkIHRvIGhhcmR3YXJlLgoKU28gd2h5IHNob3VsZCB5b3UgYm90aGVyIGNo ZWNraW5nIGZvciBuZXh0X3NnLgoKPiA+IHdoeSBhcmUgd2UgbG9va2luZyBhdCBuZXh0X3NnIGhl cmUsIGNhbiB5b3UgZXhwbGFpbiBtZSB0aGF0IHBsZWFzZQo+IAo+IFRoaXMgc29sdXRpb24gaXMg c2ltaWxhciB0byBvbmUgaW1wbGVtZW50ZWQgaW4gdGhlIGF0X2hkbWFjLmMgZHJpdmVyCj4gKGF0 Y19nZXRfYnl0ZXNfbGVmdCBmdW5jdGlvbikuCj4gCj4gWWVzIGNvdWxkIGJlIGNvbnNpZGVyIGFz IGEgd29ya2Fyb3VuZCBmb3IgYSBoYXJkd2FyZSBpc3N1ZS4uLgo+IAo+IEluIHN0bTMyIERNQSBQ ZXJpcGhlcmFsLCB3ZSBjYW4gcmVnaXN0ZXIgdXAgdG8gMiBzZyBkZXNjcmlwdG9ycyAoc2cxICYK PiBzZzIpaW4gRE1BIHJlZ2lzdGVycywgYW5kIHVzZSBpdCBpbiBhIGN5Y2xpYyBtb2RlIChhdXRv IHJlbG9hZCkuIFRoaXMKPiBtb2RlIGlzIG1haW5seSB1c2UgZm9yIGF1ZGlvIHRyYW5zZmVyIGlu aXRpYXRlZCBieSBhbiBBTFNBIGRyaXZlci4KPiAKPiA+RnJvbSBoYXJkd2FyZSBwb2ludCBvZiB2 aWV3IHRoZSBETUEgdHJhbnNmZXJzIGZpcnN0IGJsb2NrIGJhc2VkIG9uIHNnMSwKPiB0aGVuIGl0 IHVwZGF0ZXMgcmVnaXN0ZXJzIHRvIHByZXBhcmUgc2cyIHRyYW5zZmVyLCBhbmQgdGhlbiBnZW5l cmF0ZXMgYW4KPiBJUlEgdG8gaW5mb3JtIHRoYXQgaXQgaXNzdWVzIHRoZSBuZXh0IHRyYW5zZmVy IChzZzIpLgo+IAo+IFRoZW4gZHJpdmVyIGNhbiB1cGRhdGUgc2cxIHRvIHByZXBhcmUgdGhlIHRo aXJkIHRyYW5zZmVyLi4uCj4gCj4gSW4gcGFyYWxsZWwgdGhlIGNsaWVudCBkcml2ZXIgY2FuIHJl cXVlc3RzIHN0YXR1cyB0byBnZXQgdGhlIHJlc2lkdWUgdG8KPiB1cGRhdGUgaW50ZXJuYWwgcG9p bnRlci4KPiBUaGUgaXNzdWUgaXMgaW4gdGhlIHJhY2UgY29uZGl0aW9uIGJldHdlZW4gdGhlIGNh bGwgb2YgdGhlCj4gZGV2aWNlX3R4X3N0YXR1cyBvcHMgYW5kIHRoZSB1cGRhdGUgb2YgdGhlIERN QSByZWdpc3RlciBvbiBzZyBzd2l0Y2guCgpTb3JyeSBJIGRvIG5vdCBhZ3JlZSEgWW91IGFyZSBp biBzdG0zMl9kbWFfdHhfc3RhdHVzKCkgaG9sZCB0aGUgbG9jayBhbmQKaXJxcyBhcmUgZGlzYWJs ZWQsIHNvIGV2ZW4gaWYgc2cyIHdhcyBsb2FkZWQsIHlvdSB3aWxsIG5vdCBnZXQgYW4KaW50ZXJy dXB0IGFuZCB3b250IGtub3cuIEJ5IGxvb2tpbmcgYXQgc2cxIHJlZ2lzdGVyIHlvdSB3aWxsIHNl ZSB0aGF0CnNnMSBpcyB0ZWxsaW5nIHlvdSB0aGF0IGl0IGhhcyBmaW5pc2hlZCBhbmQgcmVzaWR1 ZSBjYW4gYmUgemVyby4gVGhhdCBpcwpmaW5lIGFuZCBjb3JyZWN0IHRvIHJlcG9ydC4KCk1vc3Qg aW1wb3J0YW50IHRoaW5nIGhlcmUgaXMgdGhhdCByZXNpZGUgaXMgZm9yIF9yZXF1ZXN0ZWRfIGRl c2NyaXB0b3IKYW5kIG5vdCBfY3VycmVudF8gZGVzY3JpcHRvciwgc28gbG9va2luZyBpbnRvIHNn MiBkb2VzbnQgbm90IGZpdC4KCj4gRHVyaW5nIGEgc2hvcnQgdGltZSB0aGUgaGFyZHdhcmUgdXBk YXRlZCB0aGUgcmVnaXN0ZXJzIGNvbnRhaW5pbmcgdGhlCj4gc2cgSUQgYnV0IG5vdCB0aGUgdHJh bnNmZXIgY291bnRlcihTeE5EVFIpLiBJbiB0aGlzIGNhc2UgdGhlcmUgaXMgYQo+IG1pc21hdGNo IGJldHdlZW4gdGhlIFNnIElEIGFuZCB0aGUgYXNzb2NpYXRlZCB0cmFuc2ZlciBjb3VudGVyLgo+ IFNvIHJlc2lkdWUgY2FsY3VsYXRpb24gaXMgd3JvbmcuCj4gSWRlYSBvZiB0aGlzIHBhdGNoIGlz IHRvIHBlcmZvcm0gdGhlIGNhbGN1bGF0aW9uIGFuZCB0aGVuIHRvIGNyb3NzY2hlY2sKPiB0aGF0 IHRoZSBoYXJkd2FyZSBoYXMgbm90IHN3aXRjaGVkIHRvIHRoZSBuZXh0IHNnIGR1cmluZyB0aGUK PiBjYWxjdWxhdGlvbi4gVGhlIHdheSB0byBjcm9zc2NoZWNrIGlzIHRvIGNvbXBhcmUgdGhlIHRo ZSBzZyBJRCBiZWZvcmUKPiBhbmQgYWZ0ZXIgdGhlIGNhbGN1bGF0aW9uLgo+IAo+IEkgdGVzdGVk IHRoZSBzb2x1dGlvbiB0byBmb3JjZSBhIG5ldyByZWNhbGN1bGF0aW9uIGJ1dCBubyByZWFsIHNv bHV0aW9uCj4gdG8gdHJ1c3QgdGhlIHJlZ2lzdGVycyBkdXJpbmcgdGhpcyBwaGFzZS4gSW4gdGhp cyBjYXNlIGFuIGFwcHJveGltYXRpb24KPiBpcyB0byBjb25zaWRlciB0aGF0IHRoZSBETUEgaXMg dHJhbnNmZXJyaW5nIHRoZSBmaXJzdCBieXRlcyBvZiB0aGUgbmV4dCBzZy4KPiBTbyB3ZSByZXR1 cm4gdGhlIHJlc2lkdWUgY29ycmVzcG9uZGluZyB0byB0aGUgYmVnaW5uaW5nIG9mIHRoZSBuZXh0 IGJ1ZmZlci4KCkFuZCB0aGF0IGlzIHdyb25nIS4gVGhlIGFyZ3VtZW50IGlzICdjb29raWUnIGFu ZCB5b3UgcmV0dXJuIHJlc2lkdWUgZm9yCnRoYXQgY29va2llLgoKRm9yIGV4YW1wbGUsIGlmIHlv dSBoYXZlIGRtYSB0eG4gd2l0aCBjb29raWUgMSwgMiwgMywgNCBzdWJtaXR0ZWQsIHRoZW4gY3Vy cmVudGx5IEhXCmlzIHByb2Nlc3NpbmcgY29va2llIDIsIHRoZW4gZm9yIHR4X3N0YXR1cyBvbjoK Y29va2llIDE6IHJldHVybiBETUFfQ09NUExFVEUsIHJlc2lkdWUgMApjb29raWUgMjogcmV0dXJu IERNQV9JTl9QUk9HUkVTUywgcmVzaWR1ZSAocmVhZCBmcm9tIEhXKQpjb29raWUgMzogcmV0dXJu IERNQV9JTl9QUk9HUkVTUywgcmVzaWR1ZSB0eG4gbGVuZ3RoCmNvb2tpZSA0OiByZXR1cm4gRE1B X0lOX1BST0dSRVNTLCByZXNpZHVlIHR4biBsZW5ndGgKClRoYW5rcwo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_PASS,T_DKIMWL_WL_HIGH,USER_AGENT_MUTT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3C20CC43219 for ; Mon, 29 Apr 2019 05:13:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0784621473 for ; Mon, 29 Apr 2019 05:13:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1556514809; bh=7VmgqraXn/9VuG/nJLtKzsply2v6Qrak0T0ayd5evyU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=j2jkvz6F2lVZGAc88R060KBr8wlQl8oPs7t2Fnira9WyCTRrmHXdnxWUt1wliBNH9 T3gunQ1QyUOqXCh1Zd5F0syvotj97lSKuPnkuKRdMkMisas9gGlHn/tilwQQ8uvI3d x0BCYj5gTQcw61mGAeM2s7ZIyfwAllOLq97CAPUI= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726090AbfD2FN2 (ORCPT ); Mon, 29 Apr 2019 01:13:28 -0400 Received: from mail.kernel.org ([198.145.29.99]:60798 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725783AbfD2FN2 (ORCPT ); Mon, 29 Apr 2019 01:13:28 -0400 Received: from localhost (unknown [171.76.113.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id F15A62075E; Mon, 29 Apr 2019 05:13:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1556514806; bh=7VmgqraXn/9VuG/nJLtKzsply2v6Qrak0T0ayd5evyU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Q/ofAF2mDvIZ/HGpM13OgWf7Vxb81OYKP9Y0Hb9hZqR19GnHWy6fIY8jSfFOgkFkR WmqvjSDfKvxv2B4bUBwXNb9JcIm4nx7CYx3/qIc5e8ycMiCr5ORdFZDMKrT62+9baw 0+/170xhhnZSRbNfzmoXM2J5Etd+5Zl3iy4VePUk= Date: Mon, 29 Apr 2019 10:43:23 +0530 From: Vinod Koul To: Arnaud Pouliquen Cc: Dan Williams , Pierre-Yves MORDRET , linux-stm32@st-md-mailman.stormreply.com, linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org Subject: Re: [PATCH] dmaengine: stm32-dma: fix residue calculation in stm32-dma Message-ID: <20190429051310.GC3845@vkoul-mobl.Dlink> References: <1553689316-6231-1-git-send-email-arnaud.pouliquen@st.com> <20190426121751.GC28103@vkoul-mobl> <6894b54e-651f-1caf-d363-79d1ef0eee14@st.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline In-Reply-To: <6894b54e-651f-1caf-d363-79d1ef0eee14@st.com> User-Agent: Mutt/1.11.3 (2019-02-01) Sender: dmaengine-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dmaengine@vger.kernel.org Message-ID: <20190429051323.nQsc13Rpcq0ZhBwbjOW6Da9zx7j4FLQkIQXHgZS6C74@z> On 26-04-19, 15:41, Arnaud Pouliquen wrote: > >> During residue calculation. the DMA can switch to the next sg. When > >> this race condition occurs, the residue returned value is not valid. > >> Indeed the position in the sg returned by the hardware is the position > >> of the next sg, not the current sg. > >> Solution is to check the sg after the calculation to verify it. > >> If a transition is detected we consider that the DMA has switched to > >> the beginning of next sg. > > > > Now, that sounds like duct tape. Why should we bother doing that. > > > > Also looking back at the stm32_dma_desc_residue() and calls to it from > > stm32_dma_tx_status() am not sure we are doing the right thing > Please, could you explain what you have in mind here? So when we call vchan_find_desc() that tells us if the descriptor is in the issued queue or not.. Ideally it should not matter if we have one or N descriptors issued to hardware. So why should you bother checking for next_sg. > > why are we looking at next_sg here, can you explain me that please > > This solution is similar to one implemented in the at_hdmac.c driver > (atc_get_bytes_left function). > > Yes could be consider as a workaround for a hardware issue... > > In stm32 DMA Peripheral, we can register up to 2 sg descriptors (sg1 & > sg2)in DMA registers, and use it in a cyclic mode (auto reload). This > mode is mainly use for audio transfer initiated by an ALSA driver. > > >From hardware point of view the DMA transfers first block based on sg1, > then it updates registers to prepare sg2 transfer, and then generates an > IRQ to inform that it issues the next transfer (sg2). > > Then driver can update sg1 to prepare the third transfer... > > In parallel the client driver can requests status to get the residue to > update internal pointer. > The issue is in the race condition between the call of the > device_tx_status ops and the update of the DMA register on sg switch. Sorry I do not agree! You are in stm32_dma_tx_status() hold the lock and irqs are disabled, so even if sg2 was loaded, you will not get an interrupt and wont know. By looking at sg1 register you will see that sg1 is telling you that it has finished and residue can be zero. That is fine and correct to report. Most important thing here is that reside is for _requested_ descriptor and not _current_ descriptor, so looking into sg2 doesnt not fit. > During a short time the hardware updated the registers containing the > sg ID but not the transfer counter(SxNDTR). In this case there is a > mismatch between the Sg ID and the associated transfer counter. > So residue calculation is wrong. > Idea of this patch is to perform the calculation and then to crosscheck > that the hardware has not switched to the next sg during the > calculation. The way to crosscheck is to compare the the sg ID before > and after the calculation. > > I tested the solution to force a new recalculation but no real solution > to trust the registers during this phase. In this case an approximation > is to consider that the DMA is transferring the first bytes of the next sg. > So we return the residue corresponding to the beginning of the next buffer. And that is wrong!. The argument is 'cookie' and you return residue for that cookie. For example, if you have dma txn with cookie 1, 2, 3, 4 submitted, then currently HW is processing cookie 2, then for tx_status on: cookie 1: return DMA_COMPLETE, residue 0 cookie 2: return DMA_IN_PROGRESS, residue (read from HW) cookie 3: return DMA_IN_PROGRESS, residue txn length cookie 4: return DMA_IN_PROGRESS, residue txn length Thanks -- ~Vinod