From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: "Gustavo Pimentel" <gustavo.pimentel@synopsys.com>,
"Vinod Koul" <vkoul@kernel.org>,
"Jingoo Han" <jingoohan1@gmail.com>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Frank Li" <Frank.Li@nxp.com>,
"Serge Semin" <fancer.lancer@gmail.com>,
"Alexey Malahov" <Alexey.Malahov@baikalelectronics.ru>,
"Pavel Parkhomenko" <Pavel.Parkhomenko@baikalelectronics.ru>,
"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
"Rob Herring" <robh@kernel.org>,
"Krzysztof Wilczyński" <kw@linux.com>,
linux-pci@vger.kernel.org, dmaengine@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 16/25] dmaengine: dw-edma: Simplify the DebugFS context CSRs init procedure
Date: Fri, 25 Mar 2022 12:01:33 +0530 [thread overview]
Message-ID: <20220325063133.GC4675@thinkpad> (raw)
In-Reply-To: <20220325062708.GB4675@thinkpad>
On Fri, Mar 25, 2022 at 11:57:16AM +0530, Manivannan Sadhasivam wrote:
> On Thu, Mar 24, 2022 at 04:48:27AM +0300, Serge Semin wrote:
> > DW eDMA v4.70a and older have the read and write channels context CSRs
> > indirectly accessible. It means the CSRs like Channel Control, Xfer size,
> > SAR, DAR and LLP address are accessed over at a fixed MMIO address, but
> > their reference to the corresponding channel is determined by the Viewport
> > CSR. In order to have a coherent access to these registers the CSR IOs are
> > supposed to be protected with a spin-lock. DW eDMA v4.80a and newer
> > normally have unrolled Read/Write channel context registers. That is all
> > CSRs denoted before are directly mapped in the controller MMIO space.
> >
> > Since both normal and viewport-based registers are exposed via the DebugFS
> > nodes, the original code author decided to implement an algorithm based on
> > the unrolled CSRs mapping with the viewport addresses recalculation if
> > it's required. The problem is that such implementation turned to be first
> > unscalable (supports a platform with only single eDMA available since a
> > base address statically preserved) and second needlessly overcomplicated
> > (it loops over all Rd/Wr context addresses and re-calculates the viewport
> > base address on each DebugFS node access). The algorithm can be greatly
> > simplified just by adding the channel ID and it's direction fields in the
> > eDMA DebugFS node descriptor. These new parameters can be used to find a
> > CSR offset within the corresponding channel registers space. The DW eDMA
> > DebugFS node getter afterwards will also use them in order to activate the
> > respective context CSRs viewport before reading data from the specified
> > register. In case of the unrolled version of the CSRs mapping there won't
> > be any spin-lock taken/released, no viewport activation as before this
> > modification.
> >
> > Note this modification fixes the REGISTER() macros using an externally
> > defined local variable. The same problem with the rest of the macro will
> > be fixed in the next commit.
> >
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > ---
> > drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 84 +++++++++++-------------
> > 1 file changed, 38 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
> > index 7eb0147912fa..b34a68964232 100644
> > --- a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
> > +++ b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
> > @@ -15,9 +15,27 @@
> >
> > #define REGS_ADDR(name) \
> > ((void __iomem *)®s->name)
> > +
> > +#define REGS_CH_ADDR(name, _dir, _ch) \
> > + ({ \
> > + struct dw_edma_v0_ch_regs __iomem *__ch_regs; \
> > + \
> > + if ((dw)->chip->mf == EDMA_MF_EDMA_LEGACY) \
> > + __ch_regs = ®s->type.legacy.ch; \
> > + else if (_dir == EDMA_DIR_READ) \
> > + __ch_regs = ®s->type.unroll.ch[_ch].rd; \
> > + else \
> > + __ch_regs = ®s->type.unroll.ch[_ch].wr; \
> > + \
> > + (void __iomem *)&__ch_regs->name; \
> > + })
> > +
> > #define REGISTER(name) \
> > { #name, REGS_ADDR(name) }
> >
> > +#define CTX_REGISTER(name, dir, ch) \
> > + { #name, REGS_CH_ADDR(name, dir, ch), dir, ch }
>
> What is the need of "dir, ch" at the end?
>
Ignore this comment. I failed to notice that your addition.
Thanks,
Mani
> > +
> > #define WR_REGISTER(name) \
> > { #name, REGS_ADDR(wr_##name) }
> > #define RD_REGISTER(name) \
> > @@ -41,14 +59,11 @@
> > static struct dw_edma *dw;
> > static struct dw_edma_v0_regs __iomem *regs;
> >
> > -static struct {
> > - void __iomem *start;
> > - void __iomem *end;
> > -} lim[2][EDMA_V0_MAX_NR_CH];
> > -
> > struct dw_edma_debugfs_entry {
> > const char *name;
> > void __iomem *reg;
> > + enum dw_edma_dir dir;
> > + u16 ch;
> > };
> >
> > static int dw_edma_debugfs_u32_get(void *data, u64 *val)
> > @@ -58,33 +73,16 @@ static int dw_edma_debugfs_u32_get(void *data, u64 *val)
> >
> > if (dw->chip->mf == EDMA_MF_EDMA_LEGACY &&
> > reg >= (void __iomem *)®s->type.legacy.ch) {
> > - void __iomem *ptr = ®s->type.legacy.ch;
> > - u32 viewport_sel = 0;
> > unsigned long flags;
> > - u16 ch;
> > -
> > - for (ch = 0; ch < dw->wr_ch_cnt; ch++)
> > - if (lim[0][ch].start >= reg && reg < lim[0][ch].end) {
> > - ptr += (reg - lim[0][ch].start);
> > - goto legacy_sel_wr;
> > - }
> > -
> > - for (ch = 0; ch < dw->rd_ch_cnt; ch++)
> > - if (lim[1][ch].start >= reg && reg < lim[1][ch].end) {
> > - ptr += (reg - lim[1][ch].start);
> > - goto legacy_sel_rd;
> > - }
> > -
> > - return 0;
> > -legacy_sel_rd:
> > - viewport_sel = BIT(31);
> > -legacy_sel_wr:
> > - viewport_sel |= FIELD_PREP(EDMA_V0_VIEWPORT_MASK, ch);
> > + u32 viewport_sel;
> > +
> > + viewport_sel = entry->dir == EDMA_DIR_READ ? BIT(31) : 0;
> > + viewport_sel |= FIELD_PREP(EDMA_V0_VIEWPORT_MASK, entry->ch);
> >
> > raw_spin_lock_irqsave(&dw->lock, flags);
> >
> > writel(viewport_sel, ®s->type.legacy.viewport_sel);
> > - *val = readl(ptr);
> > + *val = readl(reg);
> >
> > raw_spin_unlock_irqrestore(&dw->lock, flags);
> > } else {
> > @@ -114,19 +112,19 @@ static void dw_edma_debugfs_create_x32(const struct dw_edma_debugfs_entry ini[],
> > }
> > }
> >
> > -static void dw_edma_debugfs_regs_ch(struct dw_edma_v0_ch_regs __iomem *regs,
> > +static void dw_edma_debugfs_regs_ch(enum dw_edma_dir edma_dir, u16 ch,
> > struct dentry *dir)
>
> Using "dir" for directory would be confusing since it could also refer
> direction. I'd suggest to use "dentry".
>
> Thanks,
> Mani
>
> > {
> > - const struct dw_edma_debugfs_entry debugfs_regs[] = {
> > - REGISTER(ch_control1),
> > - REGISTER(ch_control2),
> > - REGISTER(transfer_size),
> > - REGISTER(sar.lsb),
> > - REGISTER(sar.msb),
> > - REGISTER(dar.lsb),
> > - REGISTER(dar.msb),
> > - REGISTER(llp.lsb),
> > - REGISTER(llp.msb),
> > + struct dw_edma_debugfs_entry debugfs_regs[] = {
> > + CTX_REGISTER(ch_control1, edma_dir, ch),
> > + CTX_REGISTER(ch_control2, edma_dir, ch),
> > + CTX_REGISTER(transfer_size, edma_dir, ch),
> > + CTX_REGISTER(sar.lsb, edma_dir, ch),
> > + CTX_REGISTER(sar.msb, edma_dir, ch),
> > + CTX_REGISTER(dar.lsb, edma_dir, ch),
> > + CTX_REGISTER(dar.msb, edma_dir, ch),
> > + CTX_REGISTER(llp.lsb, edma_dir, ch),
> > + CTX_REGISTER(llp.msb, edma_dir, ch),
> > };
> > int nr_entries;
> >
> > @@ -191,10 +189,7 @@ static void dw_edma_debugfs_regs_wr(struct dentry *dir)
> >
> > ch_dir = debugfs_create_dir(name, regs_dir);
> >
> > - dw_edma_debugfs_regs_ch(®s->type.unroll.ch[i].wr, ch_dir);
> > -
> > - lim[0][i].start = ®s->type.unroll.ch[i].wr;
> > - lim[0][i].end = ®s->type.unroll.ch[i].padding_1[0];
> > + dw_edma_debugfs_regs_ch(EDMA_DIR_WRITE, i, ch_dir);
> > }
> > }
> >
> > @@ -256,10 +251,7 @@ static void dw_edma_debugfs_regs_rd(struct dentry *dir)
> >
> > ch_dir = debugfs_create_dir(name, regs_dir);
> >
> > - dw_edma_debugfs_regs_ch(®s->type.unroll.ch[i].rd, ch_dir);
> > -
> > - lim[1][i].start = ®s->type.unroll.ch[i].rd;
> > - lim[1][i].end = ®s->type.unroll.ch[i].padding_2[0];
> > + dw_edma_debugfs_regs_ch(EDMA_DIR_READ, i, ch_dir);
> > }
> > }
> >
> > --
> > 2.35.1
> >
next prev parent reply other threads:[~2022-03-25 6:31 UTC|newest]
Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-24 1:48 [PATCH 00/25] dmaengine: dw-edma: Add RP/EP local DMA controllers support Serge Semin
2022-03-24 1:48 ` [PATCH 01/25] dmaengine: dw-edma: Drop dma_slave_config.direction field usage Serge Semin
2022-03-24 13:30 ` Manivannan Sadhasivam
2022-04-05 11:15 ` Serge Semin
2022-03-24 1:48 ` [PATCH 02/25] dmaengine: dw-edma: Fix eDMA Rd/Wr-channels and DMA-direction semantics Serge Semin
2022-03-24 1:48 ` [PATCH 03/25] dma-direct: take dma-ranges/offsets into account in resource mapping Serge Semin
2022-03-24 11:30 ` Robin Murphy
2022-04-17 22:44 ` Serge Semin
2022-04-20 7:12 ` Christoph Hellwig
2022-04-20 8:32 ` Serge Semin
2022-04-20 8:47 ` Christoph Hellwig
2022-04-20 8:55 ` Serge Semin
2022-04-21 14:45 ` Christoph Hellwig
2022-04-21 17:35 ` Serge Semin
2022-04-21 20:51 ` Robin Murphy
2022-04-24 21:46 ` Serge Semin
2022-03-24 1:48 ` [PATCH 04/25] dmaengine: Fix dma_slave_config.dst_addr description Serge Semin
2022-03-24 14:08 ` Manivannan Sadhasivam
2022-03-31 5:38 ` Vinod Koul
2022-03-31 7:13 ` Serge Semin
2022-03-31 10:50 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 05/25] dmaengine: dw-edma: Convert ll/dt phys-address to PCIe bus/DMA address Serge Semin
2022-03-24 16:23 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 06/25] dmaengine: dw-edma: Fix missing src/dst address of the interleaved xfers Serge Semin
2022-03-24 16:26 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 07/25] dmaengine: dw-edma: Don't permit non-inc " Serge Semin
2022-03-24 17:15 ` Manivannan Sadhasivam
2022-04-17 22:59 ` Serge Semin
2022-03-24 1:48 ` [PATCH 08/25] dmaengine: dw-edma: Fix invalid interleaved xfers semantics Serge Semin
2022-03-24 1:48 ` [PATCH 09/25] dmaengine: dw-edma: Add CPU to PCIe bus address translation Serge Semin
2022-03-24 17:25 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 10/25] dmaengine: dw-edma: Add PCIe bus address getter to the remote EP glue-driver Serge Semin
2022-03-24 17:41 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 11/25] dmaengine: dw-edma: Drop chancnt initialization Serge Semin
2022-03-24 17:42 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 12/25] dmaengine: dw-edma: Fix DebugFS reg entry type Serge Semin
2022-03-24 17:48 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 13/25] dmaengine: dw-edma: Stop checking debugfs_create_*() return value Serge Semin
2022-03-24 18:12 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 14/25] dmaengine: dw-edma: Add dw_edma prefix to the DebugFS nodes descriptor Serge Semin
2022-03-24 18:14 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 15/25] dmaengine: dw-edma: Convert DebugFS descs to being kz-allocated Serge Semin
2022-03-25 6:03 ` Manivannan Sadhasivam
2022-03-25 6:42 ` Manivannan Sadhasivam
2022-04-18 7:17 ` Serge Semin
2022-03-24 1:48 ` [PATCH 16/25] dmaengine: dw-edma: Simplify the DebugFS context CSRs init procedure Serge Semin
2022-03-25 6:27 ` Manivannan Sadhasivam
2022-03-25 6:31 ` Manivannan Sadhasivam [this message]
2022-04-18 8:23 ` Serge Semin
2022-03-24 1:48 ` [PATCH 17/25] dmaengine: dw-edma: Move eDMA data pointer to DebugFS node descriptor Serge Semin
2022-03-25 6:35 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 18/25] dmaengine: dw-edma: Join Write/Read channels into a single device Serge Semin
2022-03-25 7:34 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 19/25] dmaengine: dw-edma: Use DMA-engine device DebugFS subdirectory Serge Semin
2022-03-25 7:41 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 20/25] dmaengine: dw-edma: Use non-atomic io-64 methods Serge Semin
2022-03-25 8:28 ` Manivannan Sadhasivam
2022-04-18 11:37 ` Serge Semin
2022-03-24 1:48 ` [PATCH 21/25] dmaengine: dw-edma: Drop DT-region allocation Serge Semin
2022-03-25 8:33 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 22/25] dmaengine: dw-edma: Replace chip ID number with device name Serge Semin
2022-03-25 10:02 ` Manivannan Sadhasivam
2022-04-18 12:17 ` Serge Semin
2022-03-24 1:48 ` [PATCH 23/25] dmaengine: dw-edma: Bypass dma-ranges mapping for the local setup Serge Semin
2022-03-25 18:10 ` Manivannan Sadhasivam
2022-04-18 13:36 ` Serge Semin
2022-03-24 1:48 ` [PATCH 24/25] dmaengine: dw-edma: Skip cleanup procedure if no private data found Serge Semin
2022-03-25 18:15 ` Manivannan Sadhasivam
2022-04-18 13:48 ` Serge Semin
2022-04-23 14:45 ` Manivannan Sadhasivam
2022-03-24 1:48 ` [PATCH 25/25] PCI: dwc: Add DW eDMA engine support Serge Semin
2022-03-28 14:15 ` Manivannan Sadhasivam
2022-04-19 20:54 ` Serge Semin
2022-04-23 14:40 ` Manivannan Sadhasivam
2022-04-25 5:22 ` Manivannan Sadhasivam
2022-04-28 14:05 ` Serge Semin
2022-04-28 17:09 ` Manivannan Sadhasivam
2022-04-29 16:13 ` Serge Semin
2022-04-29 17:20 ` Manivannan Sadhasivam
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220325063133.GC4675@thinkpad \
--to=manivannan.sadhasivam@linaro.org \
--cc=Alexey.Malahov@baikalelectronics.ru \
--cc=Frank.Li@nxp.com \
--cc=Pavel.Parkhomenko@baikalelectronics.ru \
--cc=Sergey.Semin@baikalelectronics.ru \
--cc=bhelgaas@google.com \
--cc=dmaengine@vger.kernel.org \
--cc=fancer.lancer@gmail.com \
--cc=gustavo.pimentel@synopsys.com \
--cc=jingoohan1@gmail.com \
--cc=kw@linux.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=robh@kernel.org \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).