diff mbox series

[1/6] drivers: bus: mhi: Added shared-dma-pool support for mhi_dev

Message ID 20240718061344.575653-2-quic_gokulsri@quicinc.com
State New
Headers show
Series add improvements to mhi driver | expand

Commit Message

Gokul Sriram Palanisamy July 18, 2024, 6:13 a.m. UTC
When using default memory for coherent memory allocation without
reservation, memory gets fragmented after several mhi
register/unregister cycles and no coherent reservation was possible.

Client driver registering MHI shall reserve a dedicated region as
shared-dma-pool for mhi to help avoid this situation. On boards
which doesn't reserve this memory, it will continue to allocate
memory from default memory.

DMA pool is reserved for coherent allocations of size SZ_512K
(mhi_cntrl->seg_len) to avoid fragmentation and always ensure
allocations of SZ_512K succeeds. Allocations of lower order from the
reserved memory would lead to fragmentation on multiple alloc/frees.
So use dma_alloc_coherent from mhi_cntrl->cntrl_dev for allocations
lower than mhi_cntrl->seg_len. If coherent pool is not reserved, all
reservations go through mhi_cntrl->cntrl_dev.

Co-developed-by: Vignesh Viswanathan <quic_viswanat@quicinc.com>
Signed-off-by: Vignesh Viswanathan <quic_viswanat@quicinc.com>
Signed-off-by: Gokul Sriram Palanisamy <quic_gokulsri@quicinc.com>
---
 drivers/bus/mhi/host/boot.c     | 19 ++++++------
 drivers/bus/mhi/host/init.c     | 51 +++++++++++++++++++++++++++++++++
 drivers/bus/mhi/host/internal.h | 26 +++++++++++++++++
 include/linux/mhi.h             |  5 ++++
 4 files changed, 91 insertions(+), 10 deletions(-)

Comments

Gokul Sriram Palanisamy July 28, 2024, 1:41 p.m. UTC | #1
On 7/18/2024 9:47 PM, Jeffrey Hugo wrote:
> $SUBJECT looks wrong
>
> On 7/18/2024 12:13 AM, Gokul Sriram Palanisamy wrote:
>> When using default memory for coherent memory allocation without
>> reservation, memory gets fragmented after several mhi
>> register/unregister cycles and no coherent reservation was possible.
>>
>> Client driver registering MHI shall reserve a dedicated region as
>> shared-dma-pool for mhi to help avoid this situation. On boards
>> which doesn't reserve this memory, it will continue to allocate
>> memory from default memory.
>>
>> DMA pool is reserved for coherent allocations of size SZ_512K
>> (mhi_cntrl->seg_len) to avoid fragmentation and always ensure
>> allocations of SZ_512K succeeds. Allocations of lower order from the
>> reserved memory would lead to fragmentation on multiple alloc/frees.
>> So use dma_alloc_coherent from mhi_cntrl->cntrl_dev for allocations
>> lower than mhi_cntrl->seg_len. If coherent pool is not reserved, all
>> reservations go through mhi_cntrl->cntrl_dev.
>>
>> Co-developed-by: Vignesh Viswanathan <quic_viswanat@quicinc.com>
>> Signed-off-by: Vignesh Viswanathan <quic_viswanat@quicinc.com>
>> Signed-off-by: Gokul Sriram Palanisamy <quic_gokulsri@quicinc.com>
>> ---
>>   drivers/bus/mhi/host/boot.c     | 19 ++++++------
>>   drivers/bus/mhi/host/init.c     | 51 +++++++++++++++++++++++++++++++++
>>   drivers/bus/mhi/host/internal.h | 26 +++++++++++++++++
>>   include/linux/mhi.h             |  5 ++++
>>   4 files changed, 91 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
>> index dedd29ca8db3..ca842facf820 100644
>> --- a/drivers/bus/mhi/host/boot.c
>> +++ b/drivers/bus/mhi/host/boot.c
>> @@ -303,8 +303,8 @@ void mhi_free_bhie_table(struct mhi_controller 
>> *mhi_cntrl,
>>       struct mhi_buf *mhi_buf = image_info->mhi_buf;
>>         for (i = 0; i < image_info->entries; i++, mhi_buf++)
>> -        dma_free_coherent(mhi_cntrl->cntrl_dev, mhi_buf->len,
>> -                  mhi_buf->buf, mhi_buf->dma_addr);
>> +        mhi_fw_free_coherent(mhi_cntrl, mhi_buf->len,
>> +                     mhi_buf->buf, mhi_buf->dma_addr);
>>         kfree(image_info->mhi_buf);
>>       kfree(image_info);
>> @@ -340,9 +340,9 @@ int mhi_alloc_bhie_table(struct mhi_controller 
>> *mhi_cntrl,
>>               vec_size = sizeof(struct bhi_vec_entry) * i;
>>             mhi_buf->len = vec_size;
>> -        mhi_buf->buf = dma_alloc_coherent(mhi_cntrl->cntrl_dev,
>> -                          vec_size, &mhi_buf->dma_addr,
>> -                          GFP_KERNEL);
>> +        mhi_buf->buf = mhi_fw_alloc_coherent(mhi_cntrl, vec_size,
>> +                             &mhi_buf->dma_addr,
>> +                             GFP_KERNEL);
>>           if (!mhi_buf->buf)
>>               goto error_alloc_segment;
>>       }
>> @@ -355,8 +355,8 @@ int mhi_alloc_bhie_table(struct mhi_controller 
>> *mhi_cntrl,
>>     error_alloc_segment:
>>       for (--i, --mhi_buf; i >= 0; i--, mhi_buf--)
>> -        dma_free_coherent(mhi_cntrl->cntrl_dev, mhi_buf->len,
>> -                  mhi_buf->buf, mhi_buf->dma_addr);
>> +        mhi_fw_free_coherent(mhi_cntrl, mhi_buf->len,
>> +                     mhi_buf->buf, mhi_buf->dma_addr);
>>     error_alloc_mhi_buf:
>>       kfree(img_info);
>> @@ -452,8 +452,7 @@ void mhi_fw_load_handler(struct mhi_controller 
>> *mhi_cntrl)
>>       fw_sz = firmware->size;
>>     skip_req_fw:
>> -    buf = dma_alloc_coherent(mhi_cntrl->cntrl_dev, size, &dma_addr,
>> -                 GFP_KERNEL);
>> +    buf = mhi_fw_alloc_coherent(mhi_cntrl, size, &dma_addr, 
>> GFP_KERNEL);
>>       if (!buf) {
>>           release_firmware(firmware);
>>           goto error_fw_load;
>> @@ -462,7 +461,7 @@ void mhi_fw_load_handler(struct mhi_controller 
>> *mhi_cntrl)
>>       /* Download image using BHI */
>>       memcpy(buf, fw_data, size);
>>       ret = mhi_fw_load_bhi(mhi_cntrl, dma_addr, size);
>> -    dma_free_coherent(mhi_cntrl->cntrl_dev, size, buf, dma_addr);
>> +    mhi_fw_free_coherent(mhi_cntrl, size, buf, dma_addr);
>>         /* Error or in EDL mode, we're done */
>>       if (ret) {
>> diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
>> index ce7d2e62c2f1..c1e1412c43e2 100644
>> --- a/drivers/bus/mhi/host/init.c
>> +++ b/drivers/bus/mhi/host/init.c
>> @@ -8,9 +8,12 @@
>>   #include <linux/debugfs.h>
>>   #include <linux/device.h>
>>   #include <linux/dma-direction.h>
>> +#include <linux/dma-map-ops.h>
>>   #include <linux/dma-mapping.h>
>>   #include <linux/idr.h>
>>   #include <linux/interrupt.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_reserved_mem.h>
>
> NACK.  Not all platforms that use MHI have devicetree.
>
Got it. Removed this patch. This can be addressed from client driver 
that need this feature.

Regards,

Gokul
diff mbox series

Patch

diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
index dedd29ca8db3..ca842facf820 100644
--- a/drivers/bus/mhi/host/boot.c
+++ b/drivers/bus/mhi/host/boot.c
@@ -303,8 +303,8 @@  void mhi_free_bhie_table(struct mhi_controller *mhi_cntrl,
 	struct mhi_buf *mhi_buf = image_info->mhi_buf;
 
 	for (i = 0; i < image_info->entries; i++, mhi_buf++)
-		dma_free_coherent(mhi_cntrl->cntrl_dev, mhi_buf->len,
-				  mhi_buf->buf, mhi_buf->dma_addr);
+		mhi_fw_free_coherent(mhi_cntrl, mhi_buf->len,
+				     mhi_buf->buf, mhi_buf->dma_addr);
 
 	kfree(image_info->mhi_buf);
 	kfree(image_info);
@@ -340,9 +340,9 @@  int mhi_alloc_bhie_table(struct mhi_controller *mhi_cntrl,
 			vec_size = sizeof(struct bhi_vec_entry) * i;
 
 		mhi_buf->len = vec_size;
-		mhi_buf->buf = dma_alloc_coherent(mhi_cntrl->cntrl_dev,
-						  vec_size, &mhi_buf->dma_addr,
-						  GFP_KERNEL);
+		mhi_buf->buf = mhi_fw_alloc_coherent(mhi_cntrl, vec_size,
+						     &mhi_buf->dma_addr,
+						     GFP_KERNEL);
 		if (!mhi_buf->buf)
 			goto error_alloc_segment;
 	}
@@ -355,8 +355,8 @@  int mhi_alloc_bhie_table(struct mhi_controller *mhi_cntrl,
 
 error_alloc_segment:
 	for (--i, --mhi_buf; i >= 0; i--, mhi_buf--)
-		dma_free_coherent(mhi_cntrl->cntrl_dev, mhi_buf->len,
-				  mhi_buf->buf, mhi_buf->dma_addr);
+		mhi_fw_free_coherent(mhi_cntrl, mhi_buf->len,
+				     mhi_buf->buf, mhi_buf->dma_addr);
 
 error_alloc_mhi_buf:
 	kfree(img_info);
@@ -452,8 +452,7 @@  void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	fw_sz = firmware->size;
 
 skip_req_fw:
-	buf = dma_alloc_coherent(mhi_cntrl->cntrl_dev, size, &dma_addr,
-				 GFP_KERNEL);
+	buf = mhi_fw_alloc_coherent(mhi_cntrl, size, &dma_addr, GFP_KERNEL);
 	if (!buf) {
 		release_firmware(firmware);
 		goto error_fw_load;
@@ -462,7 +461,7 @@  void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	/* Download image using BHI */
 	memcpy(buf, fw_data, size);
 	ret = mhi_fw_load_bhi(mhi_cntrl, dma_addr, size);
-	dma_free_coherent(mhi_cntrl->cntrl_dev, size, buf, dma_addr);
+	mhi_fw_free_coherent(mhi_cntrl, size, buf, dma_addr);
 
 	/* Error or in EDL mode, we're done */
 	if (ret) {
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index ce7d2e62c2f1..c1e1412c43e2 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -8,9 +8,12 @@ 
 #include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/dma-direction.h>
+#include <linux/dma-map-ops.h>
 #include <linux/dma-mapping.h>
 #include <linux/idr.h>
 #include <linux/interrupt.h>
+#include <linux/of_address.h>
+#include <linux/of_reserved_mem.h>
 #include <linux/list.h>
 #include <linux/mhi.h>
 #include <linux/mod_devicetable.h>
@@ -929,6 +932,51 @@  static int parse_config(struct mhi_controller *mhi_cntrl,
 	return ret;
 }
 
+static void mhi_init_fw_coherent_memory(struct mhi_controller *mhi_cntrl,
+					struct mhi_device *mhi_dev)
+{
+	struct reserved_mem *mhi_rmem = NULL;
+	struct device *dev = &mhi_dev->dev;
+	struct device_node *cma_node = mhi_cntrl->cma_node;
+	int ret;
+
+	dev->coherent_dma_mask = mhi_cntrl->cntrl_dev->coherent_dma_mask;
+
+	if (!cma_node) {
+		dev_err(mhi_cntrl->cntrl_dev, "mhi coherent pool is not reserved");
+		return;
+	}
+
+	mhi_rmem = of_reserved_mem_lookup(cma_node);
+	of_node_put(cma_node);
+
+	if (!mhi_rmem) {
+		dev_err(mhi_cntrl->cntrl_dev, "Failed to get DMA reserved memory");
+		return;
+	}
+
+	mhi_cntrl->cma_base = mhi_rmem->base;
+	mhi_cntrl->cma_size = mhi_rmem->size;
+
+	ret = dma_declare_coherent_memory(dev, mhi_cntrl->cma_base,
+					  mhi_cntrl->cma_base,
+					  mhi_cntrl->cma_size);
+	if (ret)
+		dev_err(mhi_cntrl->cntrl_dev, "Failed to declare dma coherent memory");
+	else
+		dev_info(mhi_cntrl->cntrl_dev, "DMA Memory initialized at %pa, size %ld MiB",
+			 &mhi_cntrl->cma_base,
+			 (unsigned long)mhi_cntrl->cma_size / SZ_1M);
+}
+
+static void mhi_deinit_fw_coherent_memory(struct mhi_controller *mhi_cntrl)
+{
+	struct mhi_device *mhi_dev = mhi_cntrl->mhi_dev;
+
+	dma_release_coherent_memory(&mhi_dev->dev);
+	mhi_dev->dev.dma_mem = NULL;
+}
+
 int mhi_register_controller(struct mhi_controller *mhi_cntrl,
 			    const struct mhi_controller_config *config)
 {
@@ -1028,6 +1076,7 @@  int mhi_register_controller(struct mhi_controller *mhi_cntrl,
 		goto error_setup_irq;
 	}
 
+	mhi_init_fw_coherent_memory(mhi_cntrl, mhi_dev);
 	mhi_dev->dev_type = MHI_DEVICE_CONTROLLER;
 	mhi_dev->mhi_cntrl = mhi_cntrl;
 	dev_set_name(&mhi_dev->dev, "mhi%d", mhi_cntrl->index);
@@ -1053,6 +1102,7 @@  int mhi_register_controller(struct mhi_controller *mhi_cntrl,
 	return 0;
 
 err_release_dev:
+	mhi_deinit_fw_coherent_memory(mhi_cntrl);
 	put_device(&mhi_dev->dev);
 error_setup_irq:
 	mhi_deinit_free_irq(mhi_cntrl);
@@ -1095,6 +1145,7 @@  void mhi_unregister_controller(struct mhi_controller *mhi_cntrl)
 	}
 	vfree(mhi_cntrl->mhi_chan);
 
+	mhi_deinit_fw_coherent_memory(mhi_cntrl);
 	device_del(&mhi_dev->dev);
 	put_device(&mhi_dev->dev);
 
diff --git a/drivers/bus/mhi/host/internal.h b/drivers/bus/mhi/host/internal.h
index aaad40a07f69..41ce100d87d2 100644
--- a/drivers/bus/mhi/host/internal.h
+++ b/drivers/bus/mhi/host/internal.h
@@ -396,6 +396,32 @@  void mhi_deinit_chan_ctxt(struct mhi_controller *mhi_cntrl,
 void mhi_reset_chan(struct mhi_controller *mhi_cntrl,
 		    struct mhi_chan *mhi_chan);
 
+static inline void *mhi_fw_alloc_coherent(struct mhi_controller *mhi_cntrl,
+					  size_t size, dma_addr_t *dma_handle,
+					  gfp_t gfp)
+{
+	if (size < mhi_cntrl->seg_len || !mhi_cntrl->cma_base) {
+		return dma_alloc_coherent(mhi_cntrl->cntrl_dev,
+					  size, dma_handle, gfp);
+	} else {
+		return dma_alloc_coherent(&mhi_cntrl->mhi_dev->dev,
+					  size, dma_handle, gfp);
+	}
+}
+
+static inline void mhi_fw_free_coherent(struct mhi_controller *mhi_cntrl,
+					size_t size, void *vaddr,
+					dma_addr_t dma_handle)
+{
+	if (size < mhi_cntrl->seg_len || !mhi_cntrl->cma_base) {
+		dma_free_coherent(mhi_cntrl->cntrl_dev, size, vaddr,
+				  dma_handle);
+	} else {
+		dma_free_coherent(&mhi_cntrl->mhi_dev->dev, size, vaddr,
+				  dma_handle);
+	}
+}
+
 /* Event processing methods */
 void mhi_ctrl_ev_task(unsigned long data);
 void mhi_ev_task(unsigned long data);
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 059dc94d20bb..c788c12039b5 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -362,6 +362,8 @@  struct mhi_controller_config {
  * @wake_set: Device wakeup set flag
  * @irq_flags: irq flags passed to request_irq (optional)
  * @mru: the default MRU for the MHI device
+ * @cma_base: Base address of the cohernet memory pool reserved
+ * @cma_size: Size of the cohernel memory pool reserved
  *
  * Fields marked as (required) need to be populated by the controller driver
  * before calling mhi_register_controller(). For the fields marked as (optional)
@@ -447,6 +449,9 @@  struct mhi_controller {
 	bool wake_set;
 	unsigned long irq_flags;
 	u32 mru;
+	struct device_node *cma_node;
+	phys_addr_t cma_base;
+	size_t cma_size;
 };
 
 /**