Re: [RFC net-next 2/3] net: airoha: npu: attach a coherent pool in the NPU window

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4109 bytes --]

> The EN7581 NPU can only address host DRAM at 0x80000000-0xbfffffff;
> higher addresses alias the lower 1 GiB. The coherent buffers allocated
> on the NPU device are all shared with the NPU: since mt76 uses the NPU
> device as dma_dev, these are the NPU TX/RX descriptor rings, the HW-RRO
> ba_bitmap/addr_elem/msdu_pg/session buffers and the NPU TXD buffers.
> On a 2 GiB board nothing keeps them below 0xc0000000; they normally
> land there only because of early-boot allocation order. With low DRAM
> reserved away on W1700K the SoC hangs right after mt7996 firmware load.
> 
> If the NPU node has an optional "dma" reserved-memory region, attach it
> as the device coherent pool so all of these come from the window. The
> next patch moves the mailbox buffers to coherent memory as well.
> 
> Unlike a restricted-dma-pool this leaves streaming mappings alone.
> Packet buffers are passed through to the WiFi DMA engine: on W1700K
> with 96% of TX skbs and ~20% of RX page_pool buffers above 0xc0000000,
> host-path traffic and sha256 file round-trips stay intact, at the same
> throughput as without a pool, whereas a restricted-dma-pool (swiotlb
> bouncing) costs ~22% of host-path TX.
> 
> A device coherent pool has no fallback once full; mt76 and the mailbox
> use ~10.7 MiB with MT7996, so the DT region should be 16 MiB.
> 
> This patch was written with the help of Claude (Anthropic, claude-
> opus-5-5).
> 
> Link: https://lore.kernel.org/r/atCRsquzA1UxSpCc@mediatek.com
> Tested-by: Daniel Pawlik <pawlik.dan@gmail.com>
> Signed-off-by: Chris Scott <gilroyscott1970@gmail.com>
> ---
>  drivers/net/ethernet/airoha/airoha_npu.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/net/ethernet/airoha/airoha_npu.c b/drivers/net/ethernet/airoha/airoha_npu.c
> index 4d3195eb00..069fd60680 100644
> --- a/drivers/net/ethernet/airoha/airoha_npu.c
> +++ b/drivers/net/ethernet/airoha/airoha_npu.c
> @@ -693,6 +693,11 @@ static const struct regmap_config regmap_config = {
>  	.disable_locking	= true,
>  };
>  
> +static void airoha_npu_rmem_release(void *data)
> +{
> +	of_reserved_mem_device_release(data);
> +}
> +
>  static int airoha_npu_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -776,6 +781,19 @@ static int airoha_npu_probe(struct platform_device *pdev)
>  	if (err)
>  		return err;
>  
> +	if (of_property_match_string(dev->of_node, "memory-region-names",
> +				     "dma") >= 0) {
> +		err = of_reserved_mem_device_init_by_name(dev, dev->of_node,
> +							  "dma");
> +		if (err)
> +			return err;
> +
> +		err = devm_add_action_or_reset(dev, airoha_npu_rmem_release,
> +					       dev);
> +		if (err)
> +			return err;
> +	}

nit: what about creating a dedicated routine for both firmware and dma
memory regions. Something like:

diff --git a/drivers/net/ethernet/airoha/airoha_npu.c b/drivers/net/ethernet/airoha/airoha_npu.c
index 4d3195eb00f7..9c9c754f82c7 100644
--- a/drivers/net/ethernet/airoha/airoha_npu.c
+++ b/drivers/net/ethernet/airoha/airoha_npu.c
@@ -693,6 +693,23 @@ static const struct regmap_config regmap_config = {
 	.disable_locking	= true,
 };
 
+static int airoha_npu_reserve_mem_region(struct device *dev,
+					 struct resource *res)
+{
+	int err;
+
+	err = of_reserved_mem_region_to_resource(dev->of_node, 0, res);
+	if (err)
+		return err;
+
+	if (of_property_match_string(dev->of_node, "memory-region-names",
+				     "dma" >= 0)) {
+		...
+	}
+
+	return 0;
+}
+
 static int airoha_npu_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -729,7 +746,7 @@ static int airoha_npu_probe(struct platform_device *pdev)
 	if (IS_ERR(npu->regmap))
 		return PTR_ERR(npu->regmap);
 
-	err = of_reserved_mem_region_to_resource(dev->of_node, 0, &res);
+	err = airoha_npu_reserve_mem_region(dev, &res);
 	if (err)
 		return err;
 

Regards,
Lorenzo

> +
>  	err = airoha_npu_run_firmware(dev, base, &res);
>  	if (err)
>  		return err;
> -- 
> 2.53.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]