| Message ID | 20241213094602.2083174-4-naush@raspberrypi.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush Thanks for the patch. On Fri, 13 Dec 2024 at 09:46, Naushir Patuck <naush@raspberrypi.com> wrote: > > This property (cfeDataBufferStrided) indicates if the CSI-2 hardware > writes to the embedded/metadata buffer directly, or if it treats the > buffer like an image buffer and strides the metadata lines. > > Unicam write this buffer strided, while the PiSP Frontend writes to it s/write/writes/ > directly. This information will be relevant to data parsers in the > helpers where the data is structured in lines. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/ipa/rpi/controller/controller.cpp | 2 ++ > src/ipa/rpi/controller/controller.h | 1 + > 2 files changed, 3 insertions(+) > > diff --git a/src/ipa/rpi/controller/controller.cpp b/src/ipa/rpi/controller/controller.cpp > index e0131018e7b2..3bce88b9281e 100644 > --- a/src/ipa/rpi/controller/controller.cpp > +++ b/src/ipa/rpi/controller/controller.cpp > @@ -39,6 +39,7 @@ static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap > .pipelineWidth = 13, > .statsInline = false, > .minPixelProcessingTime = 0s, > + .cfeDataBufferStrided = true, > } > }, > { > @@ -71,6 +72,7 @@ static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap > * frames wider than ~16,000 pixels. > */ > .minPixelProcessingTime = 1.0us / 380, > + .cfeDataBufferStrided = false, > } > }, > }; > diff --git a/src/ipa/rpi/controller/controller.h b/src/ipa/rpi/controller/controller.h > index eff520bd61de..64f93f414524 100644 > --- a/src/ipa/rpi/controller/controller.h > +++ b/src/ipa/rpi/controller/controller.h > @@ -49,6 +49,7 @@ public: > unsigned int pipelineWidth; > bool statsInline; > libcamera::utils::Duration minPixelProcessingTime; > + bool cfeDataBufferStrided; I suppose I feel ever so slightly strange seeing "cfe" in a non-PiSP-specific data structure, but honestly, I'm not bothered! Reviewed-by: David Plowman <david.plowman@raspberrypi.com> Thanks David > }; > > Controller(); > -- > 2.43.0 >
On Fri, Dec 13, 2024 at 10:11:52AM +0000, David Plowman wrote: > Hi Naush > > Thanks for the patch. > > On Fri, 13 Dec 2024 at 09:46, Naushir Patuck <naush@raspberrypi.com> wrote: > > > > This property (cfeDataBufferStrided) indicates if the CSI-2 hardware > > writes to the embedded/metadata buffer directly, or if it treats the > > buffer like an image buffer and strides the metadata lines. > > > > Unicam write this buffer strided, while the PiSP Frontend writes to it > > s/write/writes/ > > > directly. This information will be relevant to data parsers in the > > helpers where the data is structured in lines. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > --- > > src/ipa/rpi/controller/controller.cpp | 2 ++ > > src/ipa/rpi/controller/controller.h | 1 + > > 2 files changed, 3 insertions(+) > > > > diff --git a/src/ipa/rpi/controller/controller.cpp b/src/ipa/rpi/controller/controller.cpp > > index e0131018e7b2..3bce88b9281e 100644 > > --- a/src/ipa/rpi/controller/controller.cpp > > +++ b/src/ipa/rpi/controller/controller.cpp > > @@ -39,6 +39,7 @@ static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap > > .pipelineWidth = 13, > > .statsInline = false, > > .minPixelProcessingTime = 0s, > > + .cfeDataBufferStrided = true, > > } > > }, > > { > > @@ -71,6 +72,7 @@ static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap > > * frames wider than ~16,000 pixels. > > */ > > .minPixelProcessingTime = 1.0us / 380, > > + .cfeDataBufferStrided = false, > > } > > }, > > }; > > diff --git a/src/ipa/rpi/controller/controller.h b/src/ipa/rpi/controller/controller.h > > index eff520bd61de..64f93f414524 100644 > > --- a/src/ipa/rpi/controller/controller.h > > +++ b/src/ipa/rpi/controller/controller.h > > @@ -49,6 +49,7 @@ public: > > unsigned int pipelineWidth; > > bool statsInline; > > libcamera::utils::Duration minPixelProcessingTime; > > + bool cfeDataBufferStrided; > > I suppose I feel ever so slightly strange seeing "cfe" in a > non-PiSP-specific data structure, but honestly, I'm not bothered! > > Reviewed-by: David Plowman <david.plowman@raspberrypi.com> This could be named embeddedDataBufferStrided instead. Either way, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > }; > > > > Controller();
Hi Laurent and David, On Sun, 15 Dec 2024 at 15:37, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > On Fri, Dec 13, 2024 at 10:11:52AM +0000, David Plowman wrote: > > Hi Naush > > > > Thanks for the patch. > > > > On Fri, 13 Dec 2024 at 09:46, Naushir Patuck <naush@raspberrypi.com> wrote: > > > > > > This property (cfeDataBufferStrided) indicates if the CSI-2 hardware > > > writes to the embedded/metadata buffer directly, or if it treats the > > > buffer like an image buffer and strides the metadata lines. > > > > > > Unicam write this buffer strided, while the PiSP Frontend writes to it > > > > s/write/writes/ > > > > > directly. This information will be relevant to data parsers in the > > > helpers where the data is structured in lines. > > > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > > --- > > > src/ipa/rpi/controller/controller.cpp | 2 ++ > > > src/ipa/rpi/controller/controller.h | 1 + > > > 2 files changed, 3 insertions(+) > > > > > > diff --git a/src/ipa/rpi/controller/controller.cpp b/src/ipa/rpi/controller/controller.cpp > > > index e0131018e7b2..3bce88b9281e 100644 > > > --- a/src/ipa/rpi/controller/controller.cpp > > > +++ b/src/ipa/rpi/controller/controller.cpp > > > @@ -39,6 +39,7 @@ static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap > > > .pipelineWidth = 13, > > > .statsInline = false, > > > .minPixelProcessingTime = 0s, > > > + .cfeDataBufferStrided = true, > > > } > > > }, > > > { > > > @@ -71,6 +72,7 @@ static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap > > > * frames wider than ~16,000 pixels. > > > */ > > > .minPixelProcessingTime = 1.0us / 380, > > > + .cfeDataBufferStrided = false, > > > } > > > }, > > > }; > > > diff --git a/src/ipa/rpi/controller/controller.h b/src/ipa/rpi/controller/controller.h > > > index eff520bd61de..64f93f414524 100644 > > > --- a/src/ipa/rpi/controller/controller.h > > > +++ b/src/ipa/rpi/controller/controller.h > > > @@ -49,6 +49,7 @@ public: > > > unsigned int pipelineWidth; > > > bool statsInline; > > > libcamera::utils::Duration minPixelProcessingTime; > > > + bool cfeDataBufferStrided; > > > > I suppose I feel ever so slightly strange seeing "cfe" in a > > non-PiSP-specific data structure, but honestly, I'm not bothered! > > > > Reviewed-by: David Plowman <david.plowman@raspberrypi.com> > > This could be named embeddedDataBufferStrided instead. Either way, Agree that "cfe" should not be used here. How about dataBufferStrided instead - since the buffer can contain more than just the embedded data stream? Naush > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > }; > > > > > > Controller(); > > -- > Regards, > > Laurent Pinchart
diff --git a/src/ipa/rpi/controller/controller.cpp b/src/ipa/rpi/controller/controller.cpp index e0131018e7b2..3bce88b9281e 100644 --- a/src/ipa/rpi/controller/controller.cpp +++ b/src/ipa/rpi/controller/controller.cpp @@ -39,6 +39,7 @@ static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap .pipelineWidth = 13, .statsInline = false, .minPixelProcessingTime = 0s, + .cfeDataBufferStrided = true, } }, { @@ -71,6 +72,7 @@ static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap * frames wider than ~16,000 pixels. */ .minPixelProcessingTime = 1.0us / 380, + .cfeDataBufferStrided = false, } }, }; diff --git a/src/ipa/rpi/controller/controller.h b/src/ipa/rpi/controller/controller.h index eff520bd61de..64f93f414524 100644 --- a/src/ipa/rpi/controller/controller.h +++ b/src/ipa/rpi/controller/controller.h @@ -49,6 +49,7 @@ public: unsigned int pipelineWidth; bool statsInline; libcamera::utils::Duration minPixelProcessingTime; + bool cfeDataBufferStrided; }; Controller();
This property (cfeDataBufferStrided) indicates if the CSI-2 hardware writes to the embedded/metadata buffer directly, or if it treats the buffer like an image buffer and strides the metadata lines. Unicam write this buffer strided, while the PiSP Frontend writes to it directly. This information will be relevant to data parsers in the helpers where the data is structured in lines. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- src/ipa/rpi/controller/controller.cpp | 2 ++ src/ipa/rpi/controller/controller.h | 1 + 2 files changed, 3 insertions(+)