Hi Jacopo, On Mon, May 27, 2024 at 12:18:54PM +0200, Jacopo Mondi wrote: > Hi Sakari, Laurent > > On Mon, May 27, 2024 at 06:44:25AM GMT, Sakari Ailus wrote: > > Hi Laurent, > > > > On Mon, May 27, 2024 at 04:19:11AM +0300, Laurent Pinchart wrote: > > > > One API-based solution could be moving the IOCTL interface to MC device > > > > node only. This wouldn't be a small change so I'm not proposing doing that > > > > now. > > > > > > I think we could also use the request API. It is a bit more cumbersome > > > to use from a userspace point of view, but this driver is meant to be > > > used from libcamera, so we can isolate applications from the extra > > > burden. > > > > > > We will need to add support for formats in the request API (or rather > > > for requests in the format ioctls). > > > > > > From a kernel point of view, the helpers used by the codec drivers may > > > not be suitable for ISP drivers, but I don't think it would be very > > > difficult to implement other helpers is needed, isolating the ISP driver > > > from the complexity of the request API. > > > > > > This doesn't preclude developing a better userspace API with ioctls on > > > the MC device node only at a later point. If the above-mentioned kernel > > > helpers are done right, transitioning to a new userspace API will have > > > minimal impact on drivers. > > > > This is indeed the third feasible option. I agree. The work on the > > framework side might not be that much either. > > > > For the time being, I would like to move forward and merge the version > of the driver with a single enabled context. > > As you can see the driver multiples the contexts by creating two > groups > > The number of groups is defined by > > /* > * We want to support 2 independent instances allowing 2 simultaneous users > * of the ISP-BE (of course they share hardware, platform resources and mutex). > * Each such instance comprises a group of device nodes representing input > * and output queues, and a media controller device node to describe them. > */ > #define PISPBE_NUM_NODE_GROUPS 2 > > Can I simply set this to 1 or should the driver be reworked to remove > the group concept completely (it will be quite some rework). > > You can guess what my preference is, and considering we want to > experiment with a different API the group part will possibily need to > be reintroduced. I believe you can also guess what my preference is. :-) I don't expect we'll need the number of groups in the upstream driver. Before deciding what to do here I'd like to arrive to a conclusion on how this gets addressed so the desired functionality is available in the upstream driver. Maybe the Request API is indeed something to consider here. I wonder what Laurent thinks. > > Also if we simply set PISPBE_NUM_NODE_GROUPS=1, the downstream RPi > kernel will solely need to have 1 patch that restores the value to 2 > to be able to use the mainline kernel driver instead of keeping their > multi-context downstream version in use until multi-context is > finalized in mainline. -- Kind regards, Sakari Ailus
