Re: [PATCH v2 6/8] media: platform: amd: isp4 video node and buffers handling added

["Du, Bin" <bin.du@amd.com>]

Hi Sultan,

On 8/12/2025 2:33 PM, Du, Bin wrote:
> Many thanks, Sultan.
> 
> On 8/12/2025 1:51 PM, Sultan Alsawaf wrote:
>> On Mon, Aug 11, 2025 at 05:05:39PM +0800, Du, Bin wrote:
>>>>> [ old quotes trimmed out ]
>>>>
>>>> Following up on this: there were several more issues hiding in the 
>>>> driver that
>>>> needed to be fixed unfortunately. I ended up making a lot of changes 
>>>> to fix all
>>>> of the buffer lifetime issues, and I created individual commits for 
>>>> this work
>>>> because it was getting hard to keep track of all the changes.
>>>>
>>>> I think I'll have this completed tomorrow and will provide a link to 
>>>> my tree
>>>> with my commits. Hopefully your internal branch hasn't diverged too 
>>>> much from
>>>> this patchset submission, otherwise there may be some conflicts to 
>>>> resolve.
>>>>
>>>> Sultan
>>>
>>> Really appreciate your time and effort. Yes, big code changes happen 
>>> in our
>>> internal branch to address upstream comments, but i believe your commits
>>> will be extremely valuable. We can check and test after fixing possbile
>>> conflict.
>>
>> I've pushed my work here:
>>
>>    https://github.com/kerneltoast/kernel_x86_laptop.git v6.16-drm-tip- 
>> isp4-for-amd
>>
>> Please see my changes since commit 
>> 48d18b6e58c6dadbc79374773736924d2f532da5 on
>> that branch, up to commit 5cdacf4ff108d83869414a00465e7a612bcd04b1.
>>
>> There are 29 patches:
>>
>> Sultan Alsawaf (29):
>>        media: amd: isp4: Use amdgpu helper functions for ISP buffers
>>        media: amd: isp4: Remove -mpreferred-stack-boundary=4 cflag
>>        media: amd: isp4: Remove -DCONFIG_COMPAT from ccflags-y
>>        media: amd: isp4: Remove the myriad of redundant ccflags includes
>>        media: amd: isp4: Pass warning flags through cc-option
>>        media: amd: isp4: Clean up amd_capture-objs formatting
>>        media: amd: isp4: Don't set CONFIG_AMD_ISP4 to y by default
>>        media: amd: isp4: Clean up AMD_ISP4 Kconfig dependencies
>>        media: amd: Fix Kconfig/Makefile directory structure
>>        media: amd: isp4: Remove superfluous NULL pointer checks
>>        media: amd: isp4: Remove superfluous void pointer casts
>>        media: amd: isp4: Remove superfluous memset in 
>> isp4vid_vb2_map_dmabuf()
>>        media: amd: isp4: Don't read refcount counter directly
>>        media: amd: isp4: Add missing refcount tracking to mmap memop
>>        media: amd: isp4: Don't put or unmap the dmabuf when detaching
>>        media: amd: isp4: Don't increment refcount when dmabuf export 
>> fails
>>        media: amd: isp4: Fix possible use-after-free in isp4vid_vb2_put()
>>        media: amd: isp4: Always export a new dmabuf from get_dmabuf memop
>>        media: amd: isp4: Fix implicit dmabuf lifetime tracking
>>        media: amd: isp4: Fix possible use-after-free when putting 
>> implicit dmabuf
>>        media: amd: isp4: Remove 'refcount > 0' warning in 
>> isp4vid_vb2_put()
>>        media: amd: isp4: Fix comment in isp4vid_vb2_dmabuf_ops_release()
>>        media: amd: isp4: Simplify isp4vid_get_dmabuf() arguments
>>        media: amd: isp4: Add comment to CONFIG_HAS_DMA #endif
>>        media: amd: isp4: Move up buf->vaddr check in isp4vid_get_dmabuf()
>>        media: amd: isp4: Remove unused userptr memops
>>        media: amd: isp4: Make isp4vid_vb2_memops static
>>        media: amd: isp4: Add missing cleanup on error in 
>> isp4vid_vb2_alloc()
>>        media: amd: isp4: Release queued buffers on error in 
>> start_streaming
>>
>>   drivers/media/platform/amd/Kconfig               |  18 +-
>>   drivers/media/platform/amd/Makefile              |   6 +-
>>   drivers/media/platform/amd/isp4/Kconfig          |  14 ++
>>   drivers/media/platform/amd/isp4/Makefile         |  42 +---
>>   drivers/media/platform/amd/isp4/isp4.c           |   9 +-
>>   drivers/media/platform/amd/isp4/isp4.h           |  11 -
>>   drivers/media/platform/amd/isp4/isp4_debug.c     |   4 +-
>>   drivers/media/platform/amd/isp4/isp4_interface.c |  46 +---
>>   drivers/media/platform/amd/isp4/isp4_interface.h |   6 +-
>>   drivers/media/platform/amd/isp4/isp4_subdev.c    |   8 +-
>>   drivers/media/platform/amd/isp4/isp4_subdev.h    |   4 +-
>>   drivers/media/platform/amd/isp4/isp4_video.c     | 297 +++++ 
>> +-----------------
>>   drivers/media/platform/amd/isp4/isp4_video.h     |   4 +-
>>   13 files changed, 122 insertions(+), 347 deletions(-)
>>
>> At least 1 of them should be redundant for your internal branch 
>> (9f394a7af2cf
>> "media: amd: isp4: Use amdgpu helper functions for ISP buffers").
>>
>> Cheers,
>> Sultan
> 
> Wow, amazing, fantastic work. Will compare to our latest internal branch 
> to see what might be missing and try to merge, then have some test. Will 
> let you know the result, it might cost some time. Thanks again.
> 

We cherry-picked your buffer related changes and did some quick test, it 
works well. Will merge them into our internal branch and check other 
changes later. Really appreciate for your time, effort and good work.

-- 
Regards,
Bin