Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 22 Jun 2023 17:12:03 +0200
From:   =?UTF-8?Q?K=C3=B6ry?= Maincent <kory.maincent@bootlin.com>
To:     Serge Semin <fancer.lancer@gmail.com>
Cc:     Cai Huoqing <cai.huoqing@linux.dev>,
        Manivannan Sadhasivam <mani@kernel.org>,
        Vinod Koul <vkoul@kernel.org>,
        Gustavo Pimentel <Gustavo.Pimentel@synopsys.com>,
        dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
        Herve Codina <herve.codina@bootlin.com>
Subject: Re: [PATCH 4/9] dmaengine: dw-edma: HDMA: Add memory barrier before
 starting the DMA transfer in remote setup
Message-ID: <20230622171203.6857b918@kmaincent-XPS-13-7390>
In-Reply-To: <ti6avu3xdrw7rjwskmemuxu4tcerfq3wd3y4c4v26pbjqjcs5h@izqmikcjsv56>
References: <20230609081654.330857-1-kory.maincent@bootlin.com>
        <20230609081654.330857-5-kory.maincent@bootlin.com>
        <20230619170201.5hbgte2optjlbx55@mobilestation.baikal.int>
        <20230619203207.694bfac6@kmaincent-XPS-13-7390>
        <tpowhctppelni47dosc27cg4vmzwdqnuvf3rukvmju2guoxzsr@wgxomqzfv6ch>
        <20230620153006.036ca3ba@kmaincent-XPS-13-7390>
        <qwkwtsjmfkmvsx4pmjetoxkjrpuwkndm6h6ntkpehxutz2h2jm@bmdzt7ywiuvs>
        <20230621151948.36125997@kmaincent-XPS-13-7390>
        <ti6avu3xdrw7rjwskmemuxu4tcerfq3wd3y4c4v26pbjqjcs5h@izqmikcjsv56>
Organization: bootlin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, 21 Jun 2023 18:56:49 +0300
Serge Semin <fancer.lancer@gmail.com> wrote:

> > Thanks for you detailed answer, this was instructive.
> > I will come back with more information if TLP flags are set.
> > FYI the PCIe board I am currently working with is the one from Brainchi=
p:
> > Here is the driver:
> > https://github.com/Brainchip-Inc/akida_dw_edma =20
>=20
> I've glanced at the driver a bit:
>=20
> 1. Nothing criminal I've noticed in the way the BARs are mapped. It's
> done as it's normally done. pcim_iomap_regions() is supposed to map
> with no additional optimization. So the caching seems irrelevant
> in this case.
>=20
> 2. The probe() method performs some device iATU config:
> akida_1500_setup_iatu() and akida_1000_setup_iatu(). I would have a
> closer look at the way the inbound MWs setup is done.
>=20
> 3. akida_1000_iatu_conf_table contains comments about the APB bus. If
> it's an internal device bus and both LPDDR and eDMA are accessible
> over the same bus, then the re-ordering may happen there. If APB means
> the well known Advanced Peripheral Bus, then it's a quite slow bus
> with respect to the system interconnect and PCIe buses. If eDMA regs
> and LL-memory buses are different then the last write to the LL-memory
> might be indeed still pending while the doorbells update arrives.
> Sending a dummy read to the LL-memory stalls the program execution
> until a response arrive (PCIe MRd TLPs are non-posted - "send and wait
> for response") which happens only after the last write to the
> LL-memory finishes. That's probably why your fix with the dummy-read
> works and why the delay you noticed is quite significant (4us).
> Though it looks quite strange to put LPDDR on such slow bus.
>=20
> 4. I would have also had a closer look at the way the outbound MW is
> configured in your PCIe host controller (whether it enables some
> optimizations like Relaxed ordering and ID-based ordering).
>=20
> In anyway I would have got in touch with the FPGA designers whether
> any of my suppositions correct (especially regarding 3.).

Alright, thanks for your instructive review!

In the HDMA driver point of view we can not know if the eDMA regs and the
LL-memory will be in same bus in whatever future implementation. Of course =
it
is the hardware designers who should be careful about having a fast bus and
memory for the LL, but wouldn't it be more cautious to have this read?
Just a small thought!

K=C3=B6ry