[PATCH 3/6] ipa: rpi: Add a HW property to determine if the data buffer is strided

[Naushir Patuck]

Hi Laurent and David,

On Sun, 15 Dec 2024 at 15:37, Laurent Pinchart
<laurent.pinchart at 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 at 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 at 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 at 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 at ideasonboard.com>
>
> > >         };
> > >
> > >         Controller();
>
> --
> Regards,
>
> Laurent Pinchart