Sakari,
On 2/26/25 4:39 PM, Sakari Ailus wrote:
> Hi Bingbu,
>
> On Wed, Feb 26, 2025 at 04:01:16PM +0800, Bingbu Cao wrote:
>>
>> 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@xxxxxxxxxxxxxxx>
>>>>> ---
>>>>> 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.
>
> I don't think the Linux distro maintainers like downstream drivers at all,
> independently of how many patches they consist of.
>
> You could as well move the function to the PSYS driver and use
> find_vm_area(cpu_addr) instead to find the area.
My fault, I forgot I already removed the ipu6_dma_get_sgtable() from
downstream driver months ago, so,
Reviewed-by: Bingbu Cao <bingbu.cao@xxxxxxxxx>
;)
>
--
Best regards,
Bingbu Cao
