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

Sultan Alsawaf
2025-08-14T06:37:16Z

On Wed, Aug 13, 2025 at 05:42:16PM +0800, Du, Bin wrote:
> 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.

Glad to hear it! :)

Sultan