<div dir="ltr"><div dir="ltr">Hi Laurent,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jul 31, 2024 at 2:47 PM Wang, Yating <<a href="mailto:yating.wang@intel.com">yating.wang@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Laurent,<br>
<br>
Attached architecture diagram.<br>
<br>
<br>
The GPI IPA typically only performs image processing algorithms, such as "noise reduction". The parameters and image data are all from the pipelinehandler. GPU ipa lib mostly doing image parameters prepare and gpu program execution by using the level-zero API.<br>
<br>
The "Level-zero API" and " Compute Runtime" are intel GPU userspace driver, already open sourced. <a href="https://github.com/intel/compute-runtime" rel="noreferrer" target="_blank">https://github.com/intel/compute-runtime</a><br>
<br></blockquote><div><br></div><div>I synced with Yating, and I think Yating means that the work to add IPC APIs to support the OpenGL API (Level-zero API) and make sure the context is synced across processes is very large. It's not our priority now.</div><div><br></div><div>The GPU processing flow will not be used in all ipu6/ipu7 modules though. We could upstream the remaining parts first, when we migrate Intel's hal into libcamera.</div><div><br></div><div>BR,<br>Harvey</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Regards,<br>
Wang Yating<br>
<br>
-----Original Message-----<br>
From: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>> <br>
Sent: Monday, July 29, 2024 3:49 PM<br>
To: Cheng-Hao Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>><br>
Cc: Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>>; Laurent Pinchart via libcamera-devel <<a href="mailto:libcamera-devel@lists.libcamera.org" target="_blank">libcamera-devel@lists.libcamera.org</a>>; <a href="mailto:yating.wang@intel.corp-partner.google.com" target="_blank">yating.wang@intel.corp-partner.google.com</a><br>
Subject: Re: [Discussion] Multiple IPAs support<br>
<br>
Hi Harvey,<br>
<br>
On Mon, Jul 29, 2024 at 02:44:05PM +0800, Cheng-Hao Yang wrote:<br>
> On Sat, Jul 27, 2024 at 8:36 AM Laurent Pinchart wrote:<br>
> > On Wed, Jul 24, 2024 at 05:55:51PM +0800, Cheng-Hao Yang wrote:<br>
> > > On Wed, Jul 24, 2024 at 3:42 PM Kieran Bingham wrote:<br>
> > > > Quoting Cheng-Hao Yang (2024-07-24 07:36:06)<br>
> > > > > Hi folks,<br>
> > > > ><br>
> > > > > I want to discuss the support of multiple IPAs within a <br>
> > > > > pipeline handler, before I upload patches, as I think there <br>
> > > > > are different approaches. It's needed because some chromeos <br>
> > > > > models (like intelipu7) need both cpu & gpu IPAs, and they might have different mojo interfaces.<br>
> > > ><br>
> > > > That sounds like multiple distinct processing blocks within a <br>
> > > > pipeline handler.<br>
> > > ><br>
> > > > But do you mean the IPA will run 'on the GPU'? Or that it will 'control'<br>
> > > > processing on the GPU?<br>
> > ><br>
> > > In CrOS, IPA runs in a sandbox. GPU IPA has access to GPU [1]. I <br>
> > > believe it can send some requests to GPU hardware.<br>
> > ><br>
> > > [1]:<br>
> > <a href="https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main" rel="noreferrer" target="_blank">https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main</a>:<br>
> > src/platform2/camera/common/init/cros-camera-gpu-algo.conf;l=29<br>
> ><br>
> > In libcamera IPA modules are not allowed to access hardware. Only <br>
> > pipeline handlers can.<br>
> ><br>
> > > I would suspect (but probably need some block diagrams) that they <br>
> > > should<br>
> > > > be broken up into a more generic set of pipelines and the 'main <br>
> > > > pipeline handler' would then construct and connect everything.<br>
> > > ><br>
> > > > I can foresee a similar thing with the soft ISP development ... <br>
> > > > it could be added as an extra processing pipline to existing <br>
> > > > platforms. For instance it could be added to the IPU3 to support the IR sensor...<br>
> > > ><br>
> > > > So some sort of generic solution or design is likely desireable here.<br>
> > > ><br>
> > > > > Currently in the `meson.build`s, they restrict the IPA name to <br>
> > > > > be the same as a pipeline handler name [1] [2].<br>
> > > > ><br>
> > > > > Here are some approaches that I can think of:<br>
> > > > ><br>
> > > > > 1. Besides the ipa that has the same name as pipeline handler <br>
> > > > > name, build the one that has a suffix `_gpu` or so. For <br>
> > > > > example, build `ipu7` & `ipu7_gpu` for pipeline handler `ipu7`.<br>
> > > > > This approach limits the number of IPAs to be two though, and <br>
> > > > > we might need to change the design again if there are other requirements in the future.<br>
> > > ><br>
> > > > Indeed, this just sounds like a hack/shortcut to me.<br>
> > > ><br>
> > > > > 2. Build exactly the IPAs specified in the meson option: <br>
> > > > > `ipas` (or we add another meson option). IIUC, currently the <br>
> > > > > default value of meson option `ipas` contains all the possible <br>
> > > > > values, and it might be the case for most of the build configs for the current use cases.<br>
> > > > > This approach might require developers to update their build <br>
> > > > > configs accordingly.<br>
> > > ><br>
> > > > If you're building distinct IPA modules - I think this is fine. <br>
> > > > Don't worry about developers updating their build configs. That <br>
> > > > sounds like worrying that developers would have to apply patches <br>
> > > > if they want to change the code to me ...<br>
> > > ><br>
> > > > It should all be covered by the build system. And <br>
> > > > -Dipas=all/auto should 'do the right thing' in what ever is designed.<br>
> > ><br>
> > > Just to make sure we're aligned: This approach needs to update the <br>
> > > logic to use the meson option `ipas`. And yes, the previous logic <br>
> > > is confusing to<br>
> > > me: It's like filtering out the options provided by developers by <br>
> > > pipeline names.<br>
> > ><br>
> > > > > 3. Build more than one shared library in one <br>
> > > > > `src/ipa/${IPA_NAME}` directory. We can register more than one <br>
> > > > > mojom interface in a pipeline handler name [3], so that the <br>
> > > > > build can generate multiple IPA proxies/workers.<br>
> > > > > This approach doesn't need to change the meson build files or <br>
> > > > > logic. I haven't tried it myself though. Maybe there are some missing parts.<br>
> > > ><br>
> > > > I haven't looked into the details / implementation here - but <br>
> > > > this sounds more aligned to the approach I would take - where <br>
> > > > for complex processing pipelines I think there should still be a <br>
> > > > top level pipeline handler in charge, but that it can construct <br>
> > > > multiple internal processing pipes to manage the whole streams.<br>
> ><br>
> > I think you're putting the cart before the horses Kieran. Before <br>
> > even discussing technical solutions, I want to clearly understand <br>
> > why multiple IPA modules would even be needed. Harvey, what are you <br>
> > trying to achieve here ? Combining multiple processing units in a <br>
> > pipeline handler (by the look of it, that would be a hardware ISP <br>
> > and GPU-based processing for the IPU7) is absolutely fine, but why <br>
> > would that require more than one IPA module ? Let's discuss the <br>
> > high-level design first, with a clear description of the IPU7 pipeline and its requirements.<br>
><br>
> Got it. I understand that libcamera IPA modules are not supposed to <br>
> access any hardware now. However, it's not the reason that cros camera <br>
> service needs to separate IPA in different processes/sandboxes.<br>
> Instead, it's because vendors provide proprietary libraries that we <br>
> cannot inspect, so we wrap them into separated sandboxes to limit the <br>
> permissions.<br>
<br>
Can't you wrap both libraries in a single IPA module ?<br>
<br>
> Currently, Intel ipu6 & Qualcomm camx [1] have proprietary algorithms <br>
> that utilize gpu to accelerate processing.<br>
> <br>
> It's true that we could probably let the gpu proprietary algorithms <br>
> calculate the instructions to access gpu (render nodes, for instance, <br>
> I'm not a gpu expert), while we didn't find strong reasons to do so in <br>
> the beginning. Letting vendors' gpu proprietary algorithms directly <br>
> access gpu is simpler for us.<br>
> In the short term, I don't think we're changing this design.<br>
<br>
I don't want to open the door to hardware access from IPA modules as that's a slippery slope. We should at least explore alternatives first.<br>
<br>
> We could probably merge the CPU IPA into the GPU IPA, as it seems that <br>
> the GPU IPA covers all permissions that CPU IPA has. I think it was <br>
> only because we want vendors to split the functionalities cleaner.<br>
> Intel also mentioned that some of the ipu6 chromebook modules don't <br>
> support the gpu processing/flow, so such an isolation might help with <br>
> this.<br>
<br>
IPA modules are not a free pass to integrate all kind of closed-source code. They are meant to permit shipping of closed-source ISP control algorithms by isolating them from the rest of the pipeline.<br>
<br>
An ISP, or part of it, can be implemented as GPU shaders (or equivalents, depending on what API is used to interface with the GPU).<br>
More generally, a GPU can perform processing as part of the whole imaging pipeline. Control algorithms for the GPU processing can be conceptually as sensitive as ISP control algorithms for vendors, so I think it makes sense to apply the same rules and allow closed-source implementations in an IPA module, alongside other ISP control algorithms. Control of the pipeline, on the other hand, should be in the pipeline handler.<br>
<br>
There's one important difference between hardware ISPs and GPU-based processing though. With GPU-based processing, the equivalent of the ISP hardware and firmware takes the form of GPU shader code (or equivalent).<br>
We don't force ISP hardware or firmware to be open-source, so there's a case for allowing the GPU shaders to remain close. However, we require hardware ISPs to be usable by 100% open-source userspace code. This means that<br>
<br>
- the input and output parameters of the GPU processing blocks need to<br>
be documented ; and<br>
<br>
- running processing on the GPU should be done without closed-source CPU<br>
code beside the control algorithms in the IPA module.<br>
<br>
We may run into technical challenges (as well as non-technical ones,<br>
*sigh*) while implementing this. I would be happy to discuss them and help finding solutions.<br>
<br>
> [1]: <br>
> <a href="https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:sr" rel="noreferrer" target="_blank">https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:sr</a><br>
> c/platform2/camera/common/BUILD.gn;l=31<br>
> <br>
> BR,<br>
> Harvey<br>
> <br>
> > > Actually I just tried myself with a POC patch:<br>
> > > <a href="https://chromium-review.googlesource.com/c/chromiumos/third_party/" rel="noreferrer" target="_blank">https://chromium-review.googlesource.com/c/chromiumos/third_party/</a><br>
> > > libcamera/+/5737834/1/include/libcamera/ipa/meson.build#75<br>
> > ><br>
> > > Seems to be working. Just need to make the map into a list instead.<br>
> > ><br>
> > > > I would envisage that sometimes those internal pipes might have <br>
> > > > their own IPA interface:<br>
> > > ><br>
> > > ><br>
> > > > ┌────────────────────────────────────────────────┐<br>
> > > > │ IPU3 Pipeline Handler │<br>
> > > > │ ┌────────────────────────────────────────────┐ │<br>
> > > > │ │ Pipe1 (Bayer) │ │<br>
> > > > │ │ ┌────────┐ ┌────────┐ ┌────────┐ │ │ ┌─────────┐<br>
> > > > │ │ │ Bayer │ │ │ │ │ │ │ │ │<br>
> > > > │ │ │ Sensor ├───►│ CIO2 ├───►│ ImgU ├─────┼─┼─► Camera1 │<br>
> > > > │ │ │ │ │ │ │ │ │ │ │ │<br>
> > > > │ │ └────────┘ └────────┘ └──┬──┬──┘ │ │ └─────────┘<br>
> > > > │ │ │ │ │ │<br>
> > > > │ │ ┌──▼──┴──┐ │ │<br>
> > > > │ │ │ │ │ │<br>
> > > > │ │ │IPU3-IPA│ │ │<br>
> > > > │ │ │ │ │ │<br>
> > > > │ │ └────────┘ │ │<br>
> > > > │ │ │ │<br>
> > > > │ └────────────────────────────────────────────┘ │<br>
> > > > │ │<br>
> > > > │ ┌────────────────────────────────────────────┐ │<br>
> > > > │ │ Pipe2 (IR) │ │<br>
> > > > │ │ ┌────────┐ ┌────────┐ ┌────────┐ │ │ ┌─────────┐<br>
> > > > │ │ │ IR │ │ │ │ │ │ │ │ │<br>
> > > > │ │ │ Sensor ├───►│ CIO2 ├───►│Soft-ISP├─────┼─┼─► Camera2 │<br>
> > > > │ │ │ │ │ │ │ │ │ │ │ │<br>
> > > > │ │ └────────┘ └────────┘ └──┬──▲──┘ │ │ └─────────┘<br>
> > > > │ │ │ │ │ │<br>
> > > > │ │ ┌──▼──┴──┐ │ │<br>
> > > > │ │ │ │ │ │<br>
> > > > │ │ │Soft-IPA│ │ │<br>
> > > > │ │ │ │ │ │<br>
> > > > │ │ └────────┘ │ │<br>
> > > > │ │ │ │<br>
> > > > │ └────────────────────────────────────────────┘ │<br>
> > > > │ │<br>
> > > > └────────────────────────────────────────────────┘<br>
> > > ><br>
> > > > I've used the IPU3 IR cam as an example above because that's <br>
> > > > hardware I have and know and I don't currently have visibility <br>
> > > > of what design you are facing. If it sort of matches your issue <br>
> > > > then great, if not - then it would help to know more about the <br>
> > > > underlying pipelines you are trying to construct.<br>
> > ><br>
> > > Do you mean that we should try to use only one IPA in each <br>
> > > pipeline handler (, in your case, "pipe1" and "pipe2"), and there's another "main pipeline handler"<br>
> > > (, which itself doesn't own an IPA) calling "pipe1" and "pipe2", <br>
> > > depending on the requests?<br>
> > ><br>
> > > Yating, from Intel, told me that the GPU IPA supports an optional <br>
> > > feature of image processing, which will be used in a stream (that doesn't need to call CPU IPA).<br>
> > > However, I believe the source is from the same sensor. Please <br>
> > > correct me if I'm wrong :)<br>
> > ><br>
> > > > > Let me know your opinions. Thanks!<br>
> > > > ><br>
> > > > > [1]: <br>
> > > > > <a href="https://git.libcamera.org/libcamera/libcamera.git/tree/include" rel="noreferrer" target="_blank">https://git.libcamera.org/libcamera/libcamera.git/tree/include</a><br>
> > > > > /libcamera/ipa/meson.build#n88<br>
> > > > > [2]: <br>
> > > > > <a href="https://git.libcamera.org/libcamera/libcamera.git/tree/src/ipa" rel="noreferrer" target="_blank">https://git.libcamera.org/libcamera/libcamera.git/tree/src/ipa</a><br>
> > > > > /meson.build#n47<br>
> > > > > [3]: <br>
> > > > > <a href="https://git.libcamera.org/libcamera/libcamera.git/tree/include" rel="noreferrer" target="_blank">https://git.libcamera.org/libcamera/libcamera.git/tree/include</a><br>
> > > > > /libcamera/ipa/meson.build#n64<br>
<br>
--<br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>