| Message ID | 20220321102702.1753800-1-naush@raspberrypi.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Naush, everyone On Mon, 21 Mar 2022 at 10:27, Naushir Patuck via libcamera-devel <libcamera-devel@lists.libcamera.org> wrote: > > Hi, > > For version 3 of this series, I've removed the patch to add a timer that waits > for 5s after stop() to de-allocate buffers. David and me have had a chat about > this and we think for now it is best that we have fully deterministic behavior for > buffer allocations, and avoid CMA fragmentation on the Raspberry Pi platforms. Yes, I think I'm happier with the idea that bugs will be easier to reproduce if they don't depend on how fast people type stuff at the console! David > Apart from that, I have renamed the reallocate flag based on Kieran's suggestion. > > Regards, > Naush > > Naushir Patuck (5): > pipeline: raspberrypi: Avoid over-allocation for ISP Output 1 > pipeline: raspberrypi: Move freeBuffers() to the RPiCameraData class > pipeline: raspberrypi: Free buffers in the RPiCamera destructor and > re-configure > pipeline: raspberrypi: Repurpose RPi::Stream::reset() > libcamera: v4l2_videodevice: Empty the V4L2 buffer cache on > streamOff() > > include/libcamera/internal/v4l2_videodevice.h | 1 + > .../pipeline/raspberrypi/raspberrypi.cpp | 58 ++++++++++++------- > .../pipeline/raspberrypi/rpi_stream.cpp | 13 ++--- > .../pipeline/raspberrypi/rpi_stream.h | 2 +- > src/libcamera/v4l2_videodevice.cpp | 16 +++++ > 5 files changed, 61 insertions(+), 29 deletions(-) > > -- > 2.25.1 >
On Mon, Mar 21, 2022 at 10:26:57AM +0000, Naushir Patuck via libcamera-devel wrote: > Hi, > > For version 3 of this series, I've removed the patch to add a timer that waits > for 5s after stop() to de-allocate buffers. David and me have had a chat about > this and we think for now it is best that we have fully deterministic behavior for > buffer allocations, and avoid CMA fragmentation on the Raspberry Pi platforms. For what it's worth, I agree about the deterministic behaviour. We can work on top of this series to add a hint to the stop() function to tell the pipeline handler if streaming is expected to be restarted soon. > Apart from that, I have renamed the reallocate flag based on Kieran's suggestion. > > Regards, > Naush > > Naushir Patuck (5): > pipeline: raspberrypi: Avoid over-allocation for ISP Output 1 > pipeline: raspberrypi: Move freeBuffers() to the RPiCameraData class > pipeline: raspberrypi: Free buffers in the RPiCamera destructor and > re-configure > pipeline: raspberrypi: Repurpose RPi::Stream::reset() > libcamera: v4l2_videodevice: Empty the V4L2 buffer cache on > streamOff() > > include/libcamera/internal/v4l2_videodevice.h | 1 + > .../pipeline/raspberrypi/raspberrypi.cpp | 58 ++++++++++++------- > .../pipeline/raspberrypi/rpi_stream.cpp | 13 ++--- > .../pipeline/raspberrypi/rpi_stream.h | 2 +- > src/libcamera/v4l2_videodevice.cpp | 16 +++++ > 5 files changed, 61 insertions(+), 29 deletions(-)
Hi Laurent, On Mon, 21 Mar 2022 at 13:02, Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > On Mon, Mar 21, 2022 at 10:26:57AM +0000, Naushir Patuck via > libcamera-devel wrote: > > Hi, > > > > For version 3 of this series, I've removed the patch to add a timer that > waits > > for 5s after stop() to de-allocate buffers. David and me have had a > chat about > > this and we think for now it is best that we have fully deterministic > behavior for > > buffer allocations, and avoid CMA fragmentation on the Raspberry Pi > platforms. > > For what it's worth, I agree about the deterministic behaviour. We can > work on top of this series to add a hint to the stop() function to tell > the pipeline handler if streaming is expected to be restarted soon. > I had a think about this briefly over the weekend, and I wonder if it might be better to have a new API function freeResources() type call instead of a flag in stop()? This would allow the application to separate out the two actions if it so desires. Naush > > > Apart from that, I have renamed the reallocate flag based on Kieran's > suggestion. > > > > Regards, > > Naush > > > > Naushir Patuck (5): > > pipeline: raspberrypi: Avoid over-allocation for ISP Output 1 > > pipeline: raspberrypi: Move freeBuffers() to the RPiCameraData class > > pipeline: raspberrypi: Free buffers in the RPiCamera destructor and > > re-configure > > pipeline: raspberrypi: Repurpose RPi::Stream::reset() > > libcamera: v4l2_videodevice: Empty the V4L2 buffer cache on > > streamOff() > > > > include/libcamera/internal/v4l2_videodevice.h | 1 + > > .../pipeline/raspberrypi/raspberrypi.cpp | 58 ++++++++++++------- > > .../pipeline/raspberrypi/rpi_stream.cpp | 13 ++--- > > .../pipeline/raspberrypi/rpi_stream.h | 2 +- > > src/libcamera/v4l2_videodevice.cpp | 16 +++++ > > 5 files changed, 61 insertions(+), 29 deletions(-) > > -- > Regards, > > Laurent Pinchart >
On Mon, Mar 21, 2022 at 01:06:04PM +0000, Naushir Patuck wrote: > On Mon, 21 Mar 2022 at 13:02, Laurent Pinchart wrote: > > On Mon, Mar 21, 2022 at 10:26:57AM +0000, Naushir Patuck via > > libcamera-devel wrote: > > > Hi, > > > > > > For version 3 of this series, I've removed the patch to add a timer that waits > > > for 5s after stop() to de-allocate buffers. David and me have had a chat about > > > this and we think for now it is best that we have fully deterministic behavior for > > > buffer allocations, and avoid CMA fragmentation on the Raspberry Pi platforms. > > > > For what it's worth, I agree about the deterministic behaviour. We can > > work on top of this series to add a hint to the stop() function to tell > > the pipeline handler if streaming is expected to be restarted soon. > > I had a think about this briefly over the weekend, and I wonder if it might be better to > have a new API function freeResources() type call instead of a flag in stop()? This would > allow the application to separate out the two actions if it so desires. Possibly. I think we need to consider what other actions a pipeline handler may take as a result of application hints (in the form of flags, or as a separate API call). If an application plans to restart the camera soon, various optimizations are possibly, such as not cutting power to the devices. There could be more too, and not everything may fall under the banner of freeing resources. > > > Apart from that, I have renamed the reallocate flag based on Kieran's suggestion. > > > > > > Regards, > > > Naush > > > > > > Naushir Patuck (5): > > > pipeline: raspberrypi: Avoid over-allocation for ISP Output 1 > > > pipeline: raspberrypi: Move freeBuffers() to the RPiCameraData class > > > pipeline: raspberrypi: Free buffers in the RPiCamera destructor and > > > re-configure > > > pipeline: raspberrypi: Repurpose RPi::Stream::reset() > > > libcamera: v4l2_videodevice: Empty the V4L2 buffer cache on > > > streamOff() > > > > > > include/libcamera/internal/v4l2_videodevice.h | 1 + > > > .../pipeline/raspberrypi/raspberrypi.cpp | 58 ++++++++++++------- > > > .../pipeline/raspberrypi/rpi_stream.cpp | 13 ++--- > > > .../pipeline/raspberrypi/rpi_stream.h | 2 +- > > > src/libcamera/v4l2_videodevice.cpp | 16 +++++ > > > 5 files changed, 61 insertions(+), 29 deletions(-)
Hi, For version 3 of this series, I've removed the patch to add a timer that waits for 5s after stop() to de-allocate buffers. David and me have had a chat about this and we think for now it is best that we have fully deterministic behavior for buffer allocations, and avoid CMA fragmentation on the Raspberry Pi platforms. Apart from that, I have renamed the reallocate flag based on Kieran's suggestion. Regards, Naush Naushir Patuck (5): pipeline: raspberrypi: Avoid over-allocation for ISP Output 1 pipeline: raspberrypi: Move freeBuffers() to the RPiCameraData class pipeline: raspberrypi: Free buffers in the RPiCamera destructor and re-configure pipeline: raspberrypi: Repurpose RPi::Stream::reset() libcamera: v4l2_videodevice: Empty the V4L2 buffer cache on streamOff() include/libcamera/internal/v4l2_videodevice.h | 1 + .../pipeline/raspberrypi/raspberrypi.cpp | 58 ++++++++++++------- .../pipeline/raspberrypi/rpi_stream.cpp | 13 ++--- .../pipeline/raspberrypi/rpi_stream.h | 2 +- src/libcamera/v4l2_videodevice.cpp | 16 +++++ 5 files changed, 61 insertions(+), 29 deletions(-)