Message ID | 20241113055524.3099340-1-chenghaoyang@chromium.org |
---|---|
Headers | show |
Series |
|
Related | show |
Quoting Harvey Yang (2024-11-13 05:54:31) > Hi folks, > > This series of patches follows the discussion with Kieran in patch [1], Aha, ok so I was a bit confused here. I didn't recall suggesting anytime we should make this into an object, but what you mean is that it follows "Roberts" implementation - not 'Mine'. I don't mind a helper object here ... Just clarifying (to my self) that this was not produced from something I said ;-) Moving syncBufferForCPU to a helper in dma_buf_allocator.h does indeed seem like a good idea to promote re-usabilty though! So lets follow the reviews on the patches ;-) -- Kieran > which adds a helper function and a helper class to make synchronizing > DMA buffers easier. > > The second patch updates debayer_cpu to utilize the new helper class. > The following mtkisp7 pipeline handler will also depend on the helper > class. > > I put the function in DmaBufAllocator and the helper class in the same > file for now. Let me know if I should put them elsewhere. > > This passes gitlab pipeline: > https://gitlab.freedesktop.org/chenghaoyang/libcamera/-/pipelines/1309296 > > v2: > - Fixed ioctl return value check when being interrupted. > > BR, > Harvey > > [1]: [PATCH v4] libcamera: debayer_cpu: Sync DMABUFs > > > Harvey Yang (2): > DmaBufAllocator: Add Dma Buffer synchronization function & helper > class > debayer_cpu: Replace syncing DMABUFs with DmaSyncer > > .../libcamera/internal/dma_buf_allocator.h | 33 +++++++ > src/libcamera/dma_buf_allocator.cpp | 90 +++++++++++++++++++ > src/libcamera/software_isp/debayer_cpu.cpp | 29 ++---- > 3 files changed, 131 insertions(+), 21 deletions(-) > > -- > 2.47.0.277.g8800431eea-goog >
Hi Kieran, On Wed, Nov 13, 2024 at 6:00 PM Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Quoting Harvey Yang (2024-11-13 05:54:31) > > Hi folks, > > > > This series of patches follows the discussion with Kieran in patch [1], > > Aha, ok so I was a bit confused here. I didn't recall suggesting anytime > we should make this into an object, but what you mean is that it follows > "Roberts" implementation - not 'Mine'. > > I don't mind a helper object here ... Just clarifying (to my self) that > this was not produced from something I said ;-) Ah I didn't mean that the current version is the conclusion of our discussion or your suggestion. Just saying that there's such a discussion, and this series is what I think a first step to work on with. Sorry if the message looks to mean otherwise :) BR, Harvey > > Moving syncBufferForCPU to a helper in dma_buf_allocator.h does indeed > seem like a good idea to promote re-usabilty though! So lets follow the > reviews on the patches ;-) > > -- > Kieran > > > which adds a helper function and a helper class to make synchronizing > > DMA buffers easier. > > > > The second patch updates debayer_cpu to utilize the new helper class. > > The following mtkisp7 pipeline handler will also depend on the helper > > class. > > > > I put the function in DmaBufAllocator and the helper class in the same > > file for now. Let me know if I should put them elsewhere. > > > > This passes gitlab pipeline: > > https://gitlab.freedesktop.org/chenghaoyang/libcamera/-/pipelines/1309296 > > > > v2: > > - Fixed ioctl return value check when being interrupted. > > > > BR, > > Harvey > > > > [1]: [PATCH v4] libcamera: debayer_cpu: Sync DMABUFs > > > > > > Harvey Yang (2): > > DmaBufAllocator: Add Dma Buffer synchronization function & helper > > class > > debayer_cpu: Replace syncing DMABUFs with DmaSyncer > > > > .../libcamera/internal/dma_buf_allocator.h | 33 +++++++ > > src/libcamera/dma_buf_allocator.cpp | 90 +++++++++++++++++++ > > src/libcamera/software_isp/debayer_cpu.cpp | 29 ++---- > > 3 files changed, 131 insertions(+), 21 deletions(-) > > > > -- > > 2.47.0.277.g8800431eea-goog > >