From: Jacob Keller <jacob.e.keller@intel.com>
To: Frank Wunderlich <linux@fw-web.de>, Felix Fietkau <nbd@nbd.name>,
"Sean Wang" <sean.wang@mediatek.com>,
Mark Lee <Mark-MC.Lee@mediatek.com>,
"Lorenzo Bianconi" <lorenzo@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Matthias Brugger <matthias.bgg@gmail.com>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Daniel Golle <daniel@makrotopia.org>,
linux-mediatek@lists.infradead.org, bc-bocun.chen@mediatek.com,
John Crispin <john@phrozen.org>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [net v3] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific
Date: Tue, 4 Jun 2024 15:25:41 -0700 [thread overview]
Message-ID: <b2cb86f7-c16a-44d2-a7b9-eb379784ff83@intel.com> (raw)
In-Reply-To: <20240603192505.217881-1-linux@fw-web.de>
On 6/3/2024 12:25 PM, Frank Wunderlich wrote:
> @@ -1142,40 +1142,46 @@ static int mtk_init_fq_dma(struct mtk_eth *eth)
> cnt * soc->tx.desc_size,
> ð->phy_scratch_ring,
> GFP_KERNEL);
> +
> if (unlikely(!eth->scratch_ring))
> return -ENOMEM;
>
> - eth->scratch_head = kcalloc(cnt, MTK_QDMA_PAGE_SIZE, GFP_KERNEL);
> - if (unlikely(!eth->scratch_head))
> - return -ENOMEM;
> + phy_ring_tail = eth->phy_scratch_ring + soc->tx.desc_size * (cnt - 1);
>
> - dma_addr = dma_map_single(eth->dma_dev,
> - eth->scratch_head, cnt * MTK_QDMA_PAGE_SIZE,
> - DMA_FROM_DEVICE);
> - if (unlikely(dma_mapping_error(eth->dma_dev, dma_addr)))
> - return -ENOMEM;
> + for (j = 0; j < DIV_ROUND_UP(soc->tx.fq_dma_size, MTK_FQ_DMA_LENGTH); j++) {
> + len = min_t(int, cnt - j * MTK_FQ_DMA_LENGTH, MTK_FQ_DMA_LENGTH);
> + eth->scratch_head[j] = kcalloc(len, MTK_QDMA_PAGE_SIZE, GFP_KERNEL);
>
> - phy_ring_tail = eth->phy_scratch_ring + soc->tx.desc_size * (cnt - 1);
> + if (unlikely(!eth->scratch_head[j]))
> + return -ENOMEM;
>
> - for (i = 0; i < cnt; i++) {
> - dma_addr_t addr = dma_addr + i * MTK_QDMA_PAGE_SIZE;
> - struct mtk_tx_dma_v2 *txd;
> + dma_addr = dma_map_single(eth->dma_dev,
> + eth->scratch_head[j], len * MTK_QDMA_PAGE_SIZE,
> + DMA_FROM_DEVICE);
>
> - txd = eth->scratch_ring + i * soc->tx.desc_size;
> - txd->txd1 = addr;
> - if (i < cnt - 1)
> - txd->txd2 = eth->phy_scratch_ring +
> - (i + 1) * soc->tx.desc_size;
> + if (unlikely(dma_mapping_error(eth->dma_dev, dma_addr)))
> + return -ENOMEM;
>
> - txd->txd3 = TX_DMA_PLEN0(MTK_QDMA_PAGE_SIZE);
> - if (MTK_HAS_CAPS(soc->caps, MTK_36BIT_DMA))
> - txd->txd3 |= TX_DMA_PREP_ADDR64(addr);
> - txd->txd4 = 0;
> - if (mtk_is_netsys_v2_or_greater(eth)) {
> - txd->txd5 = 0;
> - txd->txd6 = 0;
> - txd->txd7 = 0;
> - txd->txd8 = 0;
> + for (i = 0; i < cnt; i++) {
> + struct mtk_tx_dma_v2 *txd;
> +
> + txd = eth->scratch_ring + (j * MTK_FQ_DMA_LENGTH + i) * soc->tx.desc_size;
> + txd->txd1 = dma_addr + i * MTK_QDMA_PAGE_SIZE;
> + if (j * MTK_FQ_DMA_LENGTH + i < cnt)
> + txd->txd2 = eth->phy_scratch_ring +
> + (j * MTK_FQ_DMA_LENGTH + i + 1) * soc->tx.desc_size;
> +
> + txd->txd3 = TX_DMA_PLEN0(MTK_QDMA_PAGE_SIZE);
> + if (MTK_HAS_CAPS(soc->caps, MTK_36BIT_DMA))
> + txd->txd3 |= TX_DMA_PREP_ADDR64(dma_addr + i * MTK_QDMA_PAGE_SIZE);
> +
> + txd->txd4 = 0;
> + if (mtk_is_netsys_v2_or_greater(eth)) {
> + txd->txd5 = 0;
> + txd->txd6 = 0;
> + txd->txd7 = 0;
> + txd->txd8 = 0;
> + }
This block of change was a bit hard to understand what was going on, but
I think I get the result is that you end up allocating different set of
scratch_head per size vs the original only having one scratch_head per
device?
Perhaps you can explain, but we're now allocating a bunch of different
scratch_head pointers.. However, in the patch, the only places that we
modify scratch_head appear to be the allocation path and the free path..
but I can't seem to understand how that would impact the users of
scratch head? I guess it changes the dma_addr which then changes the txd
values we program?
Ok.
I sort of understand whats going on here, but it was a fair bit to fully
grok this flow.
Overall, I'm no expert on the part or DMA here, but:
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
WARNING: multiple messages have this Message-ID (diff)
From: Jacob Keller <jacob.e.keller@intel.com>
To: Frank Wunderlich <linux@fw-web.de>, Felix Fietkau <nbd@nbd.name>,
"Sean Wang" <sean.wang@mediatek.com>,
Mark Lee <Mark-MC.Lee@mediatek.com>,
"Lorenzo Bianconi" <lorenzo@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Matthias Brugger <matthias.bgg@gmail.com>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com>
Cc: Frank Wunderlich <frank-w@public-files.de>,
John Crispin <john@phrozen.org>, <netdev@vger.kernel.org>,
<linux-kernel@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-mediatek@lists.infradead.org>,
<bc-bocun.chen@mediatek.com>,
Daniel Golle <daniel@makrotopia.org>
Subject: Re: [net v3] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific
Date: Tue, 4 Jun 2024 15:25:41 -0700 [thread overview]
Message-ID: <b2cb86f7-c16a-44d2-a7b9-eb379784ff83@intel.com> (raw)
In-Reply-To: <20240603192505.217881-1-linux@fw-web.de>
On 6/3/2024 12:25 PM, Frank Wunderlich wrote:
> @@ -1142,40 +1142,46 @@ static int mtk_init_fq_dma(struct mtk_eth *eth)
> cnt * soc->tx.desc_size,
> ð->phy_scratch_ring,
> GFP_KERNEL);
> +
> if (unlikely(!eth->scratch_ring))
> return -ENOMEM;
>
> - eth->scratch_head = kcalloc(cnt, MTK_QDMA_PAGE_SIZE, GFP_KERNEL);
> - if (unlikely(!eth->scratch_head))
> - return -ENOMEM;
> + phy_ring_tail = eth->phy_scratch_ring + soc->tx.desc_size * (cnt - 1);
>
> - dma_addr = dma_map_single(eth->dma_dev,
> - eth->scratch_head, cnt * MTK_QDMA_PAGE_SIZE,
> - DMA_FROM_DEVICE);
> - if (unlikely(dma_mapping_error(eth->dma_dev, dma_addr)))
> - return -ENOMEM;
> + for (j = 0; j < DIV_ROUND_UP(soc->tx.fq_dma_size, MTK_FQ_DMA_LENGTH); j++) {
> + len = min_t(int, cnt - j * MTK_FQ_DMA_LENGTH, MTK_FQ_DMA_LENGTH);
> + eth->scratch_head[j] = kcalloc(len, MTK_QDMA_PAGE_SIZE, GFP_KERNEL);
>
> - phy_ring_tail = eth->phy_scratch_ring + soc->tx.desc_size * (cnt - 1);
> + if (unlikely(!eth->scratch_head[j]))
> + return -ENOMEM;
>
> - for (i = 0; i < cnt; i++) {
> - dma_addr_t addr = dma_addr + i * MTK_QDMA_PAGE_SIZE;
> - struct mtk_tx_dma_v2 *txd;
> + dma_addr = dma_map_single(eth->dma_dev,
> + eth->scratch_head[j], len * MTK_QDMA_PAGE_SIZE,
> + DMA_FROM_DEVICE);
>
> - txd = eth->scratch_ring + i * soc->tx.desc_size;
> - txd->txd1 = addr;
> - if (i < cnt - 1)
> - txd->txd2 = eth->phy_scratch_ring +
> - (i + 1) * soc->tx.desc_size;
> + if (unlikely(dma_mapping_error(eth->dma_dev, dma_addr)))
> + return -ENOMEM;
>
> - txd->txd3 = TX_DMA_PLEN0(MTK_QDMA_PAGE_SIZE);
> - if (MTK_HAS_CAPS(soc->caps, MTK_36BIT_DMA))
> - txd->txd3 |= TX_DMA_PREP_ADDR64(addr);
> - txd->txd4 = 0;
> - if (mtk_is_netsys_v2_or_greater(eth)) {
> - txd->txd5 = 0;
> - txd->txd6 = 0;
> - txd->txd7 = 0;
> - txd->txd8 = 0;
> + for (i = 0; i < cnt; i++) {
> + struct mtk_tx_dma_v2 *txd;
> +
> + txd = eth->scratch_ring + (j * MTK_FQ_DMA_LENGTH + i) * soc->tx.desc_size;
> + txd->txd1 = dma_addr + i * MTK_QDMA_PAGE_SIZE;
> + if (j * MTK_FQ_DMA_LENGTH + i < cnt)
> + txd->txd2 = eth->phy_scratch_ring +
> + (j * MTK_FQ_DMA_LENGTH + i + 1) * soc->tx.desc_size;
> +
> + txd->txd3 = TX_DMA_PLEN0(MTK_QDMA_PAGE_SIZE);
> + if (MTK_HAS_CAPS(soc->caps, MTK_36BIT_DMA))
> + txd->txd3 |= TX_DMA_PREP_ADDR64(dma_addr + i * MTK_QDMA_PAGE_SIZE);
> +
> + txd->txd4 = 0;
> + if (mtk_is_netsys_v2_or_greater(eth)) {
> + txd->txd5 = 0;
> + txd->txd6 = 0;
> + txd->txd7 = 0;
> + txd->txd8 = 0;
> + }
This block of change was a bit hard to understand what was going on, but
I think I get the result is that you end up allocating different set of
scratch_head per size vs the original only having one scratch_head per
device?
Perhaps you can explain, but we're now allocating a bunch of different
scratch_head pointers.. However, in the patch, the only places that we
modify scratch_head appear to be the allocation path and the free path..
but I can't seem to understand how that would impact the users of
scratch head? I guess it changes the dma_addr which then changes the txd
values we program?
Ok.
I sort of understand whats going on here, but it was a fair bit to fully
grok this flow.
Overall, I'm no expert on the part or DMA here, but:
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Jacob Keller <jacob.e.keller@intel.com>
To: Frank Wunderlich <linux@fw-web.de>, Felix Fietkau <nbd@nbd.name>,
"Sean Wang" <sean.wang@mediatek.com>,
Mark Lee <Mark-MC.Lee@mediatek.com>,
"Lorenzo Bianconi" <lorenzo@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Matthias Brugger <matthias.bgg@gmail.com>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com>
Cc: Frank Wunderlich <frank-w@public-files.de>,
John Crispin <john@phrozen.org>, <netdev@vger.kernel.org>,
<linux-kernel@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-mediatek@lists.infradead.org>,
<bc-bocun.chen@mediatek.com>,
Daniel Golle <daniel@makrotopia.org>
Subject: Re: [net v3] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific
Date: Tue, 4 Jun 2024 15:25:41 -0700 [thread overview]
Message-ID: <b2cb86f7-c16a-44d2-a7b9-eb379784ff83@intel.com> (raw)
In-Reply-To: <20240603192505.217881-1-linux@fw-web.de>
On 6/3/2024 12:25 PM, Frank Wunderlich wrote:
> @@ -1142,40 +1142,46 @@ static int mtk_init_fq_dma(struct mtk_eth *eth)
> cnt * soc->tx.desc_size,
> ð->phy_scratch_ring,
> GFP_KERNEL);
> +
> if (unlikely(!eth->scratch_ring))
> return -ENOMEM;
>
> - eth->scratch_head = kcalloc(cnt, MTK_QDMA_PAGE_SIZE, GFP_KERNEL);
> - if (unlikely(!eth->scratch_head))
> - return -ENOMEM;
> + phy_ring_tail = eth->phy_scratch_ring + soc->tx.desc_size * (cnt - 1);
>
> - dma_addr = dma_map_single(eth->dma_dev,
> - eth->scratch_head, cnt * MTK_QDMA_PAGE_SIZE,
> - DMA_FROM_DEVICE);
> - if (unlikely(dma_mapping_error(eth->dma_dev, dma_addr)))
> - return -ENOMEM;
> + for (j = 0; j < DIV_ROUND_UP(soc->tx.fq_dma_size, MTK_FQ_DMA_LENGTH); j++) {
> + len = min_t(int, cnt - j * MTK_FQ_DMA_LENGTH, MTK_FQ_DMA_LENGTH);
> + eth->scratch_head[j] = kcalloc(len, MTK_QDMA_PAGE_SIZE, GFP_KERNEL);
>
> - phy_ring_tail = eth->phy_scratch_ring + soc->tx.desc_size * (cnt - 1);
> + if (unlikely(!eth->scratch_head[j]))
> + return -ENOMEM;
>
> - for (i = 0; i < cnt; i++) {
> - dma_addr_t addr = dma_addr + i * MTK_QDMA_PAGE_SIZE;
> - struct mtk_tx_dma_v2 *txd;
> + dma_addr = dma_map_single(eth->dma_dev,
> + eth->scratch_head[j], len * MTK_QDMA_PAGE_SIZE,
> + DMA_FROM_DEVICE);
>
> - txd = eth->scratch_ring + i * soc->tx.desc_size;
> - txd->txd1 = addr;
> - if (i < cnt - 1)
> - txd->txd2 = eth->phy_scratch_ring +
> - (i + 1) * soc->tx.desc_size;
> + if (unlikely(dma_mapping_error(eth->dma_dev, dma_addr)))
> + return -ENOMEM;
>
> - txd->txd3 = TX_DMA_PLEN0(MTK_QDMA_PAGE_SIZE);
> - if (MTK_HAS_CAPS(soc->caps, MTK_36BIT_DMA))
> - txd->txd3 |= TX_DMA_PREP_ADDR64(addr);
> - txd->txd4 = 0;
> - if (mtk_is_netsys_v2_or_greater(eth)) {
> - txd->txd5 = 0;
> - txd->txd6 = 0;
> - txd->txd7 = 0;
> - txd->txd8 = 0;
> + for (i = 0; i < cnt; i++) {
> + struct mtk_tx_dma_v2 *txd;
> +
> + txd = eth->scratch_ring + (j * MTK_FQ_DMA_LENGTH + i) * soc->tx.desc_size;
> + txd->txd1 = dma_addr + i * MTK_QDMA_PAGE_SIZE;
> + if (j * MTK_FQ_DMA_LENGTH + i < cnt)
> + txd->txd2 = eth->phy_scratch_ring +
> + (j * MTK_FQ_DMA_LENGTH + i + 1) * soc->tx.desc_size;
> +
> + txd->txd3 = TX_DMA_PLEN0(MTK_QDMA_PAGE_SIZE);
> + if (MTK_HAS_CAPS(soc->caps, MTK_36BIT_DMA))
> + txd->txd3 |= TX_DMA_PREP_ADDR64(dma_addr + i * MTK_QDMA_PAGE_SIZE);
> +
> + txd->txd4 = 0;
> + if (mtk_is_netsys_v2_or_greater(eth)) {
> + txd->txd5 = 0;
> + txd->txd6 = 0;
> + txd->txd7 = 0;
> + txd->txd8 = 0;
> + }
This block of change was a bit hard to understand what was going on, but
I think I get the result is that you end up allocating different set of
scratch_head per size vs the original only having one scratch_head per
device?
Perhaps you can explain, but we're now allocating a bunch of different
scratch_head pointers.. However, in the patch, the only places that we
modify scratch_head appear to be the allocation path and the free path..
but I can't seem to understand how that would impact the users of
scratch head? I guess it changes the dma_addr which then changes the txd
values we program?
Ok.
I sort of understand whats going on here, but it was a fair bit to fully
grok this flow.
Overall, I'm no expert on the part or DMA here, but:
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
next prev parent reply other threads:[~2024-06-04 22:25 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-03 19:25 [net v3] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific Frank Wunderlich
2024-06-03 19:25 ` Frank Wunderlich
2024-06-03 19:25 ` Frank Wunderlich
2024-06-04 22:25 ` Jacob Keller [this message]
2024-06-04 22:25 ` Jacob Keller
2024-06-04 22:25 ` Jacob Keller
2024-06-06 2:43 ` Bc-bocun Chen (陳柏村)
2024-06-06 2:43 ` Bc-bocun Chen (陳柏村)
2024-06-06 2:43 ` Bc-bocun Chen (陳柏村)
2024-06-05 13:10 ` patchwork-bot+netdevbpf
2024-06-05 13:10 ` patchwork-bot+netdevbpf
2024-06-05 13:10 ` patchwork-bot+netdevbpf
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=b2cb86f7-c16a-44d2-a7b9-eb379784ff83@intel.com \
--to=jacob.e.keller@intel.com \
--cc=Mark-MC.Lee@mediatek.com \
--cc=angelogioacchino.delregno@collabora.com \
--cc=bc-bocun.chen@mediatek.com \
--cc=daniel@makrotopia.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=john@phrozen.org \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux@fw-web.de \
--cc=lorenzo@kernel.org \
--cc=matthias.bgg@gmail.com \
--cc=nbd@nbd.name \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sean.wang@mediatek.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.