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

Message ID 20240530171600.259495-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 May 30, 2024, 5:15 p.m. UTC
Hi All,

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

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 have also pushed these patches to:
https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/udmabuf-v2

for CI, CI is happy with it except for the lint task which does not like
the kernel headers sync, which seems to be a false positive.

Regards,

Hans


Hans de Goede (5):
  update-kernel-headers: Add linux/udmabuf.h to headers to sync
  include: linux: Update kernel headers to version v6.10-rc1
  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 +-
 include/linux/README                          |   2 +-
 include/linux/drm_fourcc.h                    |  10 +-
 include/linux/media-bus-format.h              |   9 +
 include/linux/rkisp1-config.h                 |  56 ++--
 include/linux/udmabuf.h                       |  33 +++
 include/linux/v4l2-controls.h                 |   6 +
 include/linux/v4l2-mediabus.h                 |  18 +-
 include/linux/v4l2-subdev.h                   |  29 ++-
 include/linux/videodev2.h                     |  72 ++++--
 src/libcamera/dma_buf_allocator.cpp           | 240 ++++++++++++++++++
 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 +-
 utils/update-kernel-headers.sh                |   1 +
 19 files changed, 465 insertions(+), 274 deletions(-)
 create mode 100644 include/libcamera/internal/dma_buf_allocator.h
 delete mode 100644 include/libcamera/internal/dma_heaps.h
 create mode 100644 include/linux/udmabuf.h
 create mode 100644 src/libcamera/dma_buf_allocator.cpp
 delete mode 100644 src/libcamera/dma_heaps.cpp

Comments

Bryan O'Donoghue May 31, 2024, 3:02 p.m. UTC | #1
On 30/05/2024 18:15, Hans de Goede wrote:
> Hi All,
> 
> Here is v2 of my patch-series to add /dev/udmabuf dma-buffer allocation
> support. This is based on:
> https://patchwork.libcamera.org/patch/18922/
> 
> 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 have also pushed these patches to:
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/udmabuf-v2
> 
> for CI, CI is happy with it except for the lint task which does not like
> the kernel headers sync, which seems to be a false positive.
> 
> Regards,
> 
> Hans

For the series

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # Lenovo-x13s
Hans de Goede May 31, 2024, 4:45 p.m. UTC | #2
Hi,

On 5/31/24 5:02 PM, Bryan O'Donoghue wrote:
> On 30/05/2024 18:15, Hans de Goede wrote:
>> Hi All,
>>
>> Here is v2 of my patch-series to add /dev/udmabuf dma-buffer allocation
>> support. This is based on:
>> https://patchwork.libcamera.org/patch/18922/
>>
>> 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 have also pushed these patches to:
>> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/udmabuf-v2
>>
>> for CI, CI is happy with it except for the lint task which does not like
>> the kernel headers sync, which seems to be a false positive.
>>
>> Regards,
>>
>> Hans
> 
> For the series
> 
> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # Lenovo-x13s

Thank you for testing.

As for the small conflict with Milan's series I guess we'll just
wait and see which series gets applied to the master branch first
and then the other one can be rebased on top.

Regards,

Hans

>
Laurent Pinchart June 1, 2024, 11:04 p.m. UTC | #3
On Fri, May 31, 2024 at 06:45:00PM +0200, Hans de Goede wrote:
> On 5/31/24 5:02 PM, Bryan O'Donoghue wrote:
> > On 30/05/2024 18:15, Hans de Goede wrote:
> >> Hi All,
> >>
> >> Here is v2 of my patch-series to add /dev/udmabuf dma-buffer allocation
> >> support. This is based on:
> >> https://patchwork.libcamera.org/patch/18922/
> >>
> >> 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 have also pushed these patches to:
> >> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/udmabuf-v2
> >>
> >> for CI, CI is happy with it except for the lint task which does not like
> >> the kernel headers sync, which seems to be a false positive.
> >>
> >> Regards,
> >>
> >> Hans
> > 
> > For the series
> > 
> > Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # Lenovo-x13s
> 
> Thank you for testing.
> 
> As for the small conflict with Milan's series I guess we'll just
> wait and see which series gets applied to the master branch first
> and then the other one can be rebased on top.

I think I'll merge Milan's serie first, as it has been fully reviewed,
while this series depends on a patch I have resent today.
Laurent Pinchart June 1, 2024, 11:38 p.m. UTC | #4
On Sun, Jun 02, 2024 at 02:04:18AM +0300, Laurent Pinchart wrote:
> On Fri, May 31, 2024 at 06:45:00PM +0200, Hans de Goede wrote:
> > On 5/31/24 5:02 PM, Bryan O'Donoghue wrote:
> > > On 30/05/2024 18:15, Hans de Goede wrote:
> > >> Hi All,
> > >>
> > >> Here is v2 of my patch-series to add /dev/udmabuf dma-buffer allocation
> > >> support. This is based on:
> > >> https://patchwork.libcamera.org/patch/18922/
> > >>
> > >> 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 have also pushed these patches to:
> > >> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/udmabuf-v2
> > >>
> > >> for CI, CI is happy with it except for the lint task which does not like
> > >> the kernel headers sync, which seems to be a false positive.
> > >>
> > >> Regards,
> > >>
> > >> Hans
> > > 
> > > For the series
> > > 
> > > Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # Lenovo-x13s
> > 
> > Thank you for testing.
> > 
> > As for the small conflict with Milan's series I guess we'll just
> > wait and see which series gets applied to the master branch first
> > and then the other one can be rebased on top.
> 
> I think I'll merge Milan's serie first, as it has been fully reviewed,
> while this series depends on a patch I have resent today.

I've finished reviewing this series. Comments are minor, I expect v3
will be the final version. Could you base it on top of Milan's series ?
Laurent Pinchart June 3, 2024, 9:51 a.m. UTC | #5
On Sun, Jun 02, 2024 at 02:38:09AM +0300, Laurent Pinchart wrote:
> On Sun, Jun 02, 2024 at 02:04:18AM +0300, Laurent Pinchart wrote:
> > On Fri, May 31, 2024 at 06:45:00PM +0200, Hans de Goede wrote:
> > > On 5/31/24 5:02 PM, Bryan O'Donoghue wrote:
> > > > On 30/05/2024 18:15, Hans de Goede wrote:
> > > >> Hi All,
> > > >>
> > > >> Here is v2 of my patch-series to add /dev/udmabuf dma-buffer allocation
> > > >> support. This is based on:
> > > >> https://patchwork.libcamera.org/patch/18922/
> > > >>
> > > >> 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 have also pushed these patches to:
> > > >> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/udmabuf-v2
> > > >>
> > > >> for CI, CI is happy with it except for the lint task which does not like
> > > >> the kernel headers sync, which seems to be a false positive.
> > > >>
> > > >> Regards,
> > > >>
> > > >> Hans
> > > > 
> > > > For the series
> > > > 
> > > > Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # Lenovo-x13s
> > > 
> > > Thank you for testing.
> > > 
> > > As for the small conflict with Milan's series I guess we'll just
> > > wait and see which series gets applied to the master branch first
> > > and then the other one can be rebased on top.
> > 
> > I think I'll merge Milan's serie first, as it has been fully reviewed,
> > while this series depends on a patch I have resent today.
> 
> I've finished reviewing this series. Comments are minor, I expect v3
> will be the final version. Could you base it on top of Milan's series ?

Milan's series is now in the libcamera master branch, so you can base v3
on top of that. The kernel headers update has also been merged.
Hans de Goede June 3, 2024, 10:06 a.m. UTC | #6
Hi,

On 6/3/24 11:51 AM, Laurent Pinchart wrote:
> On Sun, Jun 02, 2024 at 02:38:09AM +0300, Laurent Pinchart wrote:
>> On Sun, Jun 02, 2024 at 02:04:18AM +0300, Laurent Pinchart wrote:
>>> On Fri, May 31, 2024 at 06:45:00PM +0200, Hans de Goede wrote:
>>>> On 5/31/24 5:02 PM, Bryan O'Donoghue wrote:
>>>>> On 30/05/2024 18:15, Hans de Goede wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> Here is v2 of my patch-series to add /dev/udmabuf dma-buffer allocation
>>>>>> support. This is based on:
>>>>>> https://patchwork.libcamera.org/patch/18922/
>>>>>>
>>>>>> 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 have also pushed these patches to:
>>>>>> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/udmabuf-v2
>>>>>>
>>>>>> for CI, CI is happy with it except for the lint task which does not like
>>>>>> the kernel headers sync, which seems to be a false positive.
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> Hans
>>>>>
>>>>> For the series
>>>>>
>>>>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # Lenovo-x13s
>>>>
>>>> Thank you for testing.
>>>>
>>>> As for the small conflict with Milan's series I guess we'll just
>>>> wait and see which series gets applied to the master branch first
>>>> and then the other one can be rebased on top.
>>>
>>> I think I'll merge Milan's serie first, as it has been fully reviewed,
>>> while this series depends on a patch I have resent today.
>>
>> I've finished reviewing this series. Comments are minor, I expect v3
>> will be the final version. Could you base it on top of Milan's series ?
> 
> Milan's series is now in the libcamera master branch, so you can base v3
> on top of that. The kernel headers update has also been merged.

Yes I noticed when I just rebased, thank you.

I'm preparing a v3 of this series now.

Regards,

Hans