Re: [PATCH 1/1] media: ipu6: Drop unused ipu6_dma_get_sgtable()

[Bingbu Cao <bingbu.cao@linux.intel.com>]

On 2/26/25 3:58 PM, Sakari Ailus wrote:
> Hi Bingbu,
> 
> On Wed, Feb 26, 2025 at 10:38:25AM +0800, Bingbu Cao wrote:
>> Sakari,
>>
>> On 2/25/25 5:45 PM, Sakari Ailus wrote:
>>> ipu6_dma_get_sgtable() is now unused. Drop it.
>>>
>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>> ---
>>>  drivers/media/pci/intel/ipu6/ipu6-dma.c | 33 -------------------------
>>>  drivers/media/pci/intel/ipu6/ipu6-dma.h |  3 ---
>>>  2 files changed, 36 deletions(-)
>>>
>>> diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.c b/drivers/media/pci/intel/ipu6/ipu6-dma.c
>>> index 44e24da33907..052e396a28af 100644
>>> --- a/drivers/media/pci/intel/ipu6/ipu6-dma.c
>>> +++ b/drivers/media/pci/intel/ipu6/ipu6-dma.c
>>> @@ -455,36 +455,3 @@ void ipu6_dma_unmap_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
>>>  	ipu6_dma_unmap_sg(sys, sgt->sgl, sgt->nents, dir, attrs);
>>>  }
>>>  EXPORT_SYMBOL_NS_GPL(ipu6_dma_unmap_sgtable, "INTEL_IPU6");
>>> -
>>> -/*
>>> - * Create scatter-list for the already allocated DMA buffer
>>> - */
>>> -int ipu6_dma_get_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
>>> -			 void *cpu_addr, dma_addr_t handle, size_t size,
>>> -			 unsigned long attrs)
>>> -{
>>> -	struct device *dev = &sys->auxdev.dev;
>>> -	struct ipu6_mmu *mmu = sys->mmu;
>>> -	struct vm_info *info;
>>> -	int n_pages;
>>> -	int ret = 0;
>>> -
>>> -	info = get_vm_info(mmu, handle);
>>> -	if (!info)
>>> -		return -EFAULT;
>>> -
>>> -	if (!info->vaddr)
>>> -		return -EFAULT;
>>> -
>>> -	if (WARN_ON(!info->pages))
>>> -		return -ENOMEM;
>>> -
>>> -	n_pages = PFN_UP(size);
>>> -
>>> -	ret = sg_alloc_table_from_pages(sgt, info->pages, n_pages, 0, size,
>>> -					GFP_KERNEL);
>>> -	if (ret)
>>> -		dev_warn(dev, "get sgt table failed\n");
>>> -
>>> -	return ret;
>>> -}
>>> diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.h b/drivers/media/pci/intel/ipu6/ipu6-dma.h
>>> index b51244add9e6..2882850d9366 100644
>>> --- a/drivers/media/pci/intel/ipu6/ipu6-dma.h
>>> +++ b/drivers/media/pci/intel/ipu6/ipu6-dma.h
>>> @@ -43,7 +43,4 @@ int ipu6_dma_map_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
>>>  			 enum dma_data_direction dir, unsigned long attrs);
>>>  void ipu6_dma_unmap_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
>>>  			    enum dma_data_direction dir, unsigned long attrs);
>>> -int ipu6_dma_get_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
>>> -			 void *cpu_addr, dma_addr_t handle, size_t size,
>>> -			 unsigned long attrs);
>>
>> ipu6_dma_get_sgtable() is used in downstream PS driver, I don't think it
>> is necessary to remove it as it will be hard to integrate with downstream PS
>> driver.
> 
> When will there be an upstream PSYS driver?
> 
> I suppose you can just as well apply a revert of this patch to your
> downstream tree?
>

Sakari,

I don't think all the OS vendors like this way, it make the downstream
development harder. It's a trade-off, I cannot see removing offer any
benefits that its side-effect.


-- 
Best regards,
Bingbu Cao