| Message ID | 20240627083938.554370-1-j-luthra@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Jai, 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@ti.com> Probably worth having a Tested-by: Tag here as well. Just provide one on the thread, if you have tested it. Reviewed-by: Umang Jain <umang.jain@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 },
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@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@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 },
On 27.06.24 16:40, Laurent Pinchart wrote: > I'm thinking in particular of losing the ability to capture images > from YUV sensors That shouldn't be the case - only capturing raw bayer should be affected (I had to explicitly disable non-bayer formats on the PP for postmarket to enforce usage of the softISP).
Hi Laurent, Umang, On Jun 27, 2024 at 17:40:27 +0300, Laurent Pinchart wrote: > 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@ti.com> > > > > Probably worth having a Tested-by: Tag here as well. Just provide one on > > the thread, if you have tested it. Ah okay. I thought those tags are for others and not for patch submitter. But yes I have tested it so, Tested-by: Jai Luthra <j-luthra@ti.com> > > 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), I have tested this with OV5640, which supports ending both raw bayer formats, and converted YUV/RGB using the in-sensor ISP. With the SwISP enabled, cam tool lets me capture both the YUV/XBGR/RGB565 directly coming from sensor, or RGB/BGR converted from the raw bayer. Here are the logs of `cam -c1 -I`: https://0x0.st/XmOS.txt > 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. This is true, and I personally do use cam tool to capture raw bayer data, as it is more convenient compared to using v4l2-ctl/yavta - given it sets the routes and formats on all subdevs for me. It would be great to be able to be able to choose that through either CLI options or the config file support Milan proposed. But I wouldn't call missing that a regression, as actual end-users of a camera almost always want a usable RGB image :) > > > Reviewed-by: Umang Jain <umang.jain@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
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 },
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@ti.com> --- src/libcamera/pipeline/simple/simple.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)