[PATCH] libcamera: pipeline: simple: Enable Soft ISP for TI CSI-RX

[Laurent Pinchart]

On Thu, Jun 27, 2024 at 07:11:22PM +0530, Umang Jain wrote:
> On 27/06/24 2:09 pm, Jai Luthra wrote:
> > The j721e-csi2rx driver pipeline uses no converters, so enable the
> > software ISP plugin support. This is handy for boards with AM62 SoC
> > (like BeaglePlay) that have no HW ISP.
> >
> > Tested with IMX519 on SK-AM62 running a kernel built with dmabuf heap
> > support.
> >
> > Signed-off-by: Jai Luthra <j-luthra at ti.com>
> 
> Probably worth having a Tested-by: Tag here as well. Just provide one on 
> the thread, if you have tested it.

I generally assume that, unless otherwise noted, patch submitters will
have arranged for code to be tested :-)

Is there a risk of regression for working use cases by enabling the soft
ISP ? I'm thinking in particular of losing the ability to capture images
from YUV sensors (if the simple pipeline handler doesn't skip the soft
ISP in that case), and the ability to capture raw images from Bayer
sensors. The latter is likely less of an issue, it could probably be
implemented a bit later if it's not there yet.

> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> 
> > ---
> >   src/libcamera/pipeline/simple/simple.cpp | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index eb36578e..812c492e 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -198,7 +198,7 @@ namespace {
> >   static const SimplePipelineInfo supportedDevices[] = {
> >   	{ "dcmipp", {}, false },
> >   	{ "imx7-csi", { { "pxp", 1 } }, false },
> > -	{ "j721e-csi2rx", {}, false },
> > +	{ "j721e-csi2rx", {}, true },
> >   	{ "mtk-seninf", { { "mtk-mdp", 3 } }, false },
> >   	{ "mxc-isi", {}, false },
> >   	{ "qcom-camss", {}, true },

-- 
Regards,

Laurent Pinchart