[v3,0/3] libcamera: dma_buffer_allocator: Add support for using udmabuf to alloc dma-buffers
mbox series

Message ID 20240603111259.54321-1-hdegoede@redhat.com
Headers show
Series
  • libcamera: dma_buffer_allocator: Add support for using udmabuf to alloc dma-buffers
Related show

Message

Hans de Goede June 3, 2024, 11:12 a.m. UTC
Hi All,

Here is v3 of my patch-series to add /dev/udmabuf dma-buffer allocation
support. This is based on: https://patchwork.libcamera.org/patch/18922/

Changes in v3:
- Rebase on top of latest master which includes Milan's "[PATCH v6 0/5]
  Software ISP levels cleanup" series and Laurent's "[PATCH 0/3] libcamera:
  Update to the new upstream subdev routing API" series
- Drop include/linux related patches (included in Laurent's merged series)
- Wrap comments and error message logging at 80 chars
- Style fixes to some comments
- Drop the unnecessary checking of the created dma-buf size

Changes in v2:
- New patch: Add linux/udmabuf.h to libcamera's local kernel headers
- New patch: Rename DmaHeap class to DmaBufAllocator
- libcamera: DmaBufAllocator: Support allocating from /dev/udmabuf :
  - Reword the commit message
  - Add a new DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf type for udmabuf
  - Drop unnecessary size != size check
  - Reword log messages to be more like the DMA heap alloc path
  - Move UniqueFD(ret) up so as to not leak the fd on errors
-New patch: software_isp: Allow using dma-buffers from /dev/udmabuf

I did not push out a branch for this to libcamera-softisp because
of fdo gitlab maintenance. Note CI was happy with v2 and I see no reason
why v3 would be different.

Regards,

Hans


Hans de Goede (3):
  libcamera: Rename DmaHeap class to DmaBufAllocator
  libcamera: DmaBufAllocator: Support allocating from /dev/udmabuf
  libcamera: software_isp: Allow using dma-buffers from /dev/udmabuf

 .../libcamera/internal/dma_buf_allocator.h    |  42 ++++
 include/libcamera/internal/dma_heaps.h        |  38 ---
 include/libcamera/internal/meson.build        |   2 +-
 .../internal/software_isp/software_isp.h      |   4 +-
 src/libcamera/dma_buf_allocator.cpp           | 233 ++++++++++++++++++
 src/libcamera/dma_heaps.cpp                   | 165 -------------
 src/libcamera/meson.build                     |   2 +-
 src/libcamera/pipeline/rpi/vc4/vc4.cpp        |   4 +-
 src/libcamera/software_isp/software_isp.cpp   |   6 +-
 9 files changed, 285 insertions(+), 211 deletions(-)
 create mode 100644 include/libcamera/internal/dma_buf_allocator.h
 delete mode 100644 include/libcamera/internal/dma_heaps.h
 create mode 100644 src/libcamera/dma_buf_allocator.cpp
 delete mode 100644 src/libcamera/dma_heaps.cpp

Comments

Kieran Bingham June 3, 2024, 11:59 a.m. UTC | #1
Quoting Hans de Goede (2024-06-03 12:12:56)
> Hi All,
> 
> Here is v3 of my patch-series to add /dev/udmabuf dma-buffer allocation
> support. This is based on: https://patchwork.libcamera.org/patch/18922/
> 
> Changes in v3:
> - Rebase on top of latest master which includes Milan's "[PATCH v6 0/5]
>   Software ISP levels cleanup" series and Laurent's "[PATCH 0/3] libcamera:
>   Update to the new upstream subdev routing API" series
> - Drop include/linux related patches (included in Laurent's merged series)
> - Wrap comments and error message logging at 80 chars
> - Style fixes to some comments
> - Drop the unnecessary checking of the created dma-buf size
> 
> Changes in v2:
> - New patch: Add linux/udmabuf.h to libcamera's local kernel headers
> - New patch: Rename DmaHeap class to DmaBufAllocator
> - libcamera: DmaBufAllocator: Support allocating from /dev/udmabuf :
>   - Reword the commit message
>   - Add a new DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf type for udmabuf
>   - Drop unnecessary size != size check
>   - Reword log messages to be more like the DMA heap alloc path
>   - Move UniqueFD(ret) up so as to not leak the fd on errors
> -New patch: software_isp: Allow using dma-buffers from /dev/udmabuf
> 
> I did not push out a branch for this to libcamera-softisp because
> of fdo gitlab maintenance. Note CI was happy with v2 and I see no reason
> why v3 would be different.

Thanks,

I think we should probably wait for FDO to finish their maintenance
anyway, but this looks ready to merge indeed.

I'll run my script that will kick this through CI and then merge when
that's completed.

Though I think I might have to manually fix the patches as they apply
with the double '---' trailers. I think git-am currently doesn't know to
skip those ?

--
Kieran


> 
> Regards,
> 
> Hans
> 
> 
> Hans de Goede (3):
>   libcamera: Rename DmaHeap class to DmaBufAllocator
>   libcamera: DmaBufAllocator: Support allocating from /dev/udmabuf
>   libcamera: software_isp: Allow using dma-buffers from /dev/udmabuf
> 
>  .../libcamera/internal/dma_buf_allocator.h    |  42 ++++
>  include/libcamera/internal/dma_heaps.h        |  38 ---
>  include/libcamera/internal/meson.build        |   2 +-
>  .../internal/software_isp/software_isp.h      |   4 +-
>  src/libcamera/dma_buf_allocator.cpp           | 233 ++++++++++++++++++
>  src/libcamera/dma_heaps.cpp                   | 165 -------------
>  src/libcamera/meson.build                     |   2 +-
>  src/libcamera/pipeline/rpi/vc4/vc4.cpp        |   4 +-
>  src/libcamera/software_isp/software_isp.cpp   |   6 +-
>  9 files changed, 285 insertions(+), 211 deletions(-)
>  create mode 100644 include/libcamera/internal/dma_buf_allocator.h
>  delete mode 100644 include/libcamera/internal/dma_heaps.h
>  create mode 100644 src/libcamera/dma_buf_allocator.cpp
>  delete mode 100644 src/libcamera/dma_heaps.cpp
> 
> -- 
> 2.45.1
>
Hans de Goede June 3, 2024, 12:04 p.m. UTC | #2
Hi,

On 6/3/24 1:59 PM, Kieran Bingham wrote:
> Quoting Hans de Goede (2024-06-03 12:12:56)
>> Hi All,
>>
>> Here is v3 of my patch-series to add /dev/udmabuf dma-buffer allocation
>> support. This is based on: https://patchwork.libcamera.org/patch/18922/
>>
>> Changes in v3:
>> - Rebase on top of latest master which includes Milan's "[PATCH v6 0/5]
>>   Software ISP levels cleanup" series and Laurent's "[PATCH 0/3] libcamera:
>>   Update to the new upstream subdev routing API" series
>> - Drop include/linux related patches (included in Laurent's merged series)
>> - Wrap comments and error message logging at 80 chars
>> - Style fixes to some comments
>> - Drop the unnecessary checking of the created dma-buf size
>>
>> Changes in v2:
>> - New patch: Add linux/udmabuf.h to libcamera's local kernel headers
>> - New patch: Rename DmaHeap class to DmaBufAllocator
>> - libcamera: DmaBufAllocator: Support allocating from /dev/udmabuf :
>>   - Reword the commit message
>>   - Add a new DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf type for udmabuf
>>   - Drop unnecessary size != size check
>>   - Reword log messages to be more like the DMA heap alloc path
>>   - Move UniqueFD(ret) up so as to not leak the fd on errors
>> -New patch: software_isp: Allow using dma-buffers from /dev/udmabuf
>>
>> I did not push out a branch for this to libcamera-softisp because
>> of fdo gitlab maintenance. Note CI was happy with v2 and I see no reason
>> why v3 would be different.
> 
> Thanks,
> 
> I think we should probably wait for FDO to finish their maintenance
> anyway, but this looks ready to merge indeed.
> 
> I'll run my script that will kick this through CI and then merge when
> that's completed.
> 
> Though I think I might have to manually fix the patches as they apply
> with the double '---' trailers. I think git-am currently doesn't know to
> skip those ?

That is weird, by default git am should skip anything under the first
'---' cut-line for the commit message and it does do this for me.

Maybe you have some local .gitconfig setting influencing this ?

Regards,

Hans



>> Hans de Goede (3):
>>   libcamera: Rename DmaHeap class to DmaBufAllocator
>>   libcamera: DmaBufAllocator: Support allocating from /dev/udmabuf
>>   libcamera: software_isp: Allow using dma-buffers from /dev/udmabuf
>>
>>  .../libcamera/internal/dma_buf_allocator.h    |  42 ++++
>>  include/libcamera/internal/dma_heaps.h        |  38 ---
>>  include/libcamera/internal/meson.build        |   2 +-
>>  .../internal/software_isp/software_isp.h      |   4 +-
>>  src/libcamera/dma_buf_allocator.cpp           | 233 ++++++++++++++++++
>>  src/libcamera/dma_heaps.cpp                   | 165 -------------
>>  src/libcamera/meson.build                     |   2 +-
>>  src/libcamera/pipeline/rpi/vc4/vc4.cpp        |   4 +-
>>  src/libcamera/software_isp/software_isp.cpp   |   6 +-
>>  9 files changed, 285 insertions(+), 211 deletions(-)
>>  create mode 100644 include/libcamera/internal/dma_buf_allocator.h
>>  delete mode 100644 include/libcamera/internal/dma_heaps.h
>>  create mode 100644 src/libcamera/dma_buf_allocator.cpp
>>  delete mode 100644 src/libcamera/dma_heaps.cpp
>>
>> -- 
>> 2.45.1
>>
>
Kieran Bingham June 3, 2024, 12:07 p.m. UTC | #3
Quoting Hans de Goede (2024-06-03 13:04:23)
> Hi,
> 
> On 6/3/24 1:59 PM, Kieran Bingham wrote:
> > Quoting Hans de Goede (2024-06-03 12:12:56)
> >> Hi All,
> >>
> >> Here is v3 of my patch-series to add /dev/udmabuf dma-buffer allocation
> >> support. This is based on: https://patchwork.libcamera.org/patch/18922/
> >>
> >> Changes in v3:
> >> - Rebase on top of latest master which includes Milan's "[PATCH v6 0/5]
> >>   Software ISP levels cleanup" series and Laurent's "[PATCH 0/3] libcamera:
> >>   Update to the new upstream subdev routing API" series
> >> - Drop include/linux related patches (included in Laurent's merged series)
> >> - Wrap comments and error message logging at 80 chars
> >> - Style fixes to some comments
> >> - Drop the unnecessary checking of the created dma-buf size
> >>
> >> Changes in v2:
> >> - New patch: Add linux/udmabuf.h to libcamera's local kernel headers
> >> - New patch: Rename DmaHeap class to DmaBufAllocator
> >> - libcamera: DmaBufAllocator: Support allocating from /dev/udmabuf :
> >>   - Reword the commit message
> >>   - Add a new DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf type for udmabuf
> >>   - Drop unnecessary size != size check
> >>   - Reword log messages to be more like the DMA heap alloc path
> >>   - Move UniqueFD(ret) up so as to not leak the fd on errors
> >> -New patch: software_isp: Allow using dma-buffers from /dev/udmabuf
> >>
> >> I did not push out a branch for this to libcamera-softisp because
> >> of fdo gitlab maintenance. Note CI was happy with v2 and I see no reason
> >> why v3 would be different.
> > 
> > Thanks,
> > 
> > I think we should probably wait for FDO to finish their maintenance
> > anyway, but this looks ready to merge indeed.
> > 
> > I'll run my script that will kick this through CI and then merge when
> > that's completed.
> > 
> > Though I think I might have to manually fix the patches as they apply
> > with the double '---' trailers. I think git-am currently doesn't know to
> > skip those ?
> 
> That is weird, by default git am should skip anything under the first
> '---' cut-line for the commit message and it does do this for me.
> 
> Maybe you have some local .gitconfig setting influencing this ?

I've just applied with 'git-pw series apply 4352 -s' and it did the
right thing!

Sorry for the noise - I was probably mis-remembering something that I've
hit in the past.

--
Kieran


> 
> Regards,
> 
> Hans
> 
> 
> 
> >> Hans de Goede (3):
> >>   libcamera: Rename DmaHeap class to DmaBufAllocator
> >>   libcamera: DmaBufAllocator: Support allocating from /dev/udmabuf
> >>   libcamera: software_isp: Allow using dma-buffers from /dev/udmabuf
> >>
> >>  .../libcamera/internal/dma_buf_allocator.h    |  42 ++++
> >>  include/libcamera/internal/dma_heaps.h        |  38 ---
> >>  include/libcamera/internal/meson.build        |   2 +-
> >>  .../internal/software_isp/software_isp.h      |   4 +-
> >>  src/libcamera/dma_buf_allocator.cpp           | 233 ++++++++++++++++++
> >>  src/libcamera/dma_heaps.cpp                   | 165 -------------
> >>  src/libcamera/meson.build                     |   2 +-
> >>  src/libcamera/pipeline/rpi/vc4/vc4.cpp        |   4 +-
> >>  src/libcamera/software_isp/software_isp.cpp   |   6 +-
> >>  9 files changed, 285 insertions(+), 211 deletions(-)
> >>  create mode 100644 include/libcamera/internal/dma_buf_allocator.h
> >>  delete mode 100644 include/libcamera/internal/dma_heaps.h
> >>  create mode 100644 src/libcamera/dma_buf_allocator.cpp
> >>  delete mode 100644 src/libcamera/dma_heaps.cpp
> >>
> >> -- 
> >> 2.45.1
> >>
> > 
>