[2/2] libcamera: simple: Support RPi's unicam CSI-2 receiver
diff mbox series

Message ID 20250709134229.135949-3-uajain@igalia.com
State New
Headers show
Series
  • libcamera: simple: Enable RPi's unicam+SoftISP
Related show

Commit Message

Umang Jain July 9, 2025, 1:42 p.m. UTC
This patch enables RaspberryPi's unicam CSI-2 receiver to be able
to function and capture images via the simple pipeline handler along
with SoftISP.

Unicam CSI-2 supports capture in both packed and unpacked pixel
formats. Currently, the unicam video node is configured to use
the unpacked formats for streaming with simple pipeline handler.

Signed-off-by: Umang Jain <uajain@igalia.com>
Tested-by: Umang Jain <uajain@igalia.com> # RPi-3B + IMX219
---
 src/libcamera/pipeline/simple/simple.cpp | 1 +
 1 file changed, 1 insertion(+)

Comments

Laurent Pinchart July 9, 2025, 2:20 p.m. UTC | #1
Hi Umang,

On Wed, Jul 09, 2025 at 07:12:29PM +0530, Umang Jain wrote:
> This patch enables RaspberryPi's unicam CSI-2 receiver to be able
> to function and capture images via the simple pipeline handler along
> with SoftISP.
> 
> Unicam CSI-2 supports capture in both packed and unpacked pixel
> formats. Currently, the unicam video node is configured to use
> the unpacked formats for streaming with simple pipeline handler.
> 
> Signed-off-by: Umang Jain <uajain@igalia.com>
> Tested-by: Umang Jain <uajain@igalia.com> # RPi-3B + IMX219
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index e2a48724..60034395 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -257,6 +257,7 @@ static const SimplePipelineInfo supportedDevices[] = {
>  	{ "mxc-isi", {}, false },
>  	{ "qcom-camss", {}, true },
>  	{ "sun6i-csi", {}, false },
> +	{ "unicam", {}, true },

This raises two questions:

- Why ?

- Are you aware that, if the simple pipeline handler grabs the unicam
  device, it will break all users of the RPi pipeline handler ?

>  };
>  
>  } /* namespace */
Milan Zamazal July 9, 2025, 2:46 p.m. UTC | #2
Hi,

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> Hi Umang,
>
> On Wed, Jul 09, 2025 at 07:12:29PM +0530, Umang Jain wrote:
>> This patch enables RaspberryPi's unicam CSI-2 receiver to be able
>> to function and capture images via the simple pipeline handler along
>> with SoftISP.
>> 
>> Unicam CSI-2 supports capture in both packed and unpacked pixel
>> formats. Currently, the unicam video node is configured to use
>> the unpacked formats for streaming with simple pipeline handler.
>> 
>> Signed-off-by: Umang Jain <uajain@igalia.com>
>> Tested-by: Umang Jain <uajain@igalia.com> # RPi-3B + IMX219
>> ---
>>  src/libcamera/pipeline/simple/simple.cpp | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
>> index e2a48724..60034395 100644
>> --- a/src/libcamera/pipeline/simple/simple.cpp
>> +++ b/src/libcamera/pipeline/simple/simple.cpp
>> @@ -257,6 +257,7 @@ static const SimplePipelineInfo supportedDevices[] = {
>>  	{ "mxc-isi", {}, false },
>>  	{ "qcom-camss", {}, true },
>>  	{ "sun6i-csi", {}, false },
>> +	{ "unicam", {}, true },
>
> This raises two questions:
>
> - Why ?

I don't know what's Umang's intended purpose, but for me, having another
platform for developing and testing software ISP (especially for GPU,
but not only) would be useful.

> - Are you aware that, if the simple pipeline handler grabs the unicam
>   device, it will break all users of the RPi pipeline handler ?

The global configuration file patches make this configurable.  With
that, software ISP can be enabled in the configuration file without
changing the defaults in the code.

>>  };
>>  
>>  } /* namespace */
Umang Jain July 10, 2025, 3:18 a.m. UTC | #3
Hi Laurent,

On Wed, Jul 09, 2025 at 05:20:33PM +0300, Laurent Pinchart wrote:
> Hi Umang,
> 
> On Wed, Jul 09, 2025 at 07:12:29PM +0530, Umang Jain wrote:
> > This patch enables RaspberryPi's unicam CSI-2 receiver to be able
> > to function and capture images via the simple pipeline handler along
> > with SoftISP.
> > 
> > Unicam CSI-2 supports capture in both packed and unpacked pixel
> > formats. Currently, the unicam video node is configured to use
> > the unpacked formats for streaming with simple pipeline handler.
> > 
> > Signed-off-by: Umang Jain <uajain@igalia.com>
> > Tested-by: Umang Jain <uajain@igalia.com> # RPi-3B + IMX219
> > ---
> >  src/libcamera/pipeline/simple/simple.cpp | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index e2a48724..60034395 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -257,6 +257,7 @@ static const SimplePipelineInfo supportedDevices[] = {
> >  	{ "mxc-isi", {}, false },
> >  	{ "qcom-camss", {}, true },
> >  	{ "sun6i-csi", {}, false },
> > +	{ "unicam", {}, true },
> 
> This raises two questions:
> 
> - Why ?

My intention were to develop and test simple PH <> softISP on the RPi
platform - which is relatively common and easy to acquire platform for
developers without much prior setup. Additionally, it would increase
testing and lower the entry barrier for the contributions.

I am aware there might be alternative ways to address the similar goals
(vimc, vivid?). I have not yet explored them so I don't have anything
to compare to however, testing on a real sensor/CSI2 platform might have
its own upsides in the long run, in my opinion.

> 
> - Are you aware that, if the simple pipeline handler grabs the unicam
>   device, it will break all users of the RPi pipeline handler ?

- I was expecting this to be guarded by build configuration recipes that the
  Raspberrypi ships libcamera with. I don't think that the simple pipeline
  handler(or any other handler for that matter) will be enabled there.

- For advanced users / developers building libcamera on their own with
  -Dpipelines=all, indeed it will break. We do not have logic or even use
  cases where different pipeline handler will compete for same devices
  on a platform.

I like the idea suggested by Milan of using the global configuration
files (https://patchwork.libcamera.org/patch/23723/) to address use-cases
as per this patch. One more idea was in back of mind initially was to introduce
priority-based pipeline-handler registeration (similar to
REGISTER_CAMERA_SENSOR() macro).

> 
> >  };
> >  
> >  } /* namespace */
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index e2a48724..60034395 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -257,6 +257,7 @@  static const SimplePipelineInfo supportedDevices[] = {
 	{ "mxc-isi", {}, false },
 	{ "qcom-camss", {}, true },
 	{ "sun6i-csi", {}, false },
+	{ "unicam", {}, true },
 };
 
 } /* namespace */