[libcamera-devel] [PATCH v2 2/2] pipeline: raspberrypi: Allow registration of multiple cameras

[Sakari Ailus]

Hi Laurent,

On Tue, Nov 23, 2021 at 06:57:54PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Tue, Nov 23, 2021 at 06:28:56PM +0200, Sakari Ailus wrote:
> > Hi Laurent, Naushir,
> > 
> > On Mon, Nov 22, 2021 at 03:00:40PM +0200, Laurent Pinchart wrote:
> > > > -bool PipelineHandlerRPi::registerCameras()
> > > > +int PipelineHandlerRPi::registerCameras(MediaDevice *unicam, MediaDevice *isp,
> > > > +					const std::string &deviceId)
> > > >  {
> > > >  	std::unique_ptr<RPiCameraData> data = std::make_unique<RPiCameraData>(this);
> > > > +
> > > >  	if (!data->dmaHeap_.isValid())
> > > > -		return false;
> > > > +		return -ENOMEM;
> > > > +
> > > > +	MediaEntity *unicamImage = unicam->getEntityByName("unicam" + deviceId + "-image");
> > > > +	MediaEntity *ispOutput0 = isp->getEntityByName("bcm2835-isp" + deviceId + "-output0");
> > > > +	MediaEntity *ispCapture1 = isp->getEntityByName("bcm2835-isp" + deviceId + "-capture1");
> > > > +	MediaEntity *ispCapture2 = isp->getEntityByName("bcm2835-isp" + deviceId + "-capture2");
> > > > +	MediaEntity *ispCapture3 = isp->getEntityByName("bcm2835-isp" + deviceId + "-capture3");
> > > 
> > > I'm wondering if wildcards would be nice here too, but maybe a simpler
> > > option would be to not include the device ID in the entity names in the
> > > driver ? We have a bus info field that the kernel reports for the media
> > > device, to differentiate between multiple instances of the same device.
> > > I suppose we are missing guidelines on the kernel side regarding entity
> > > naming. Sakari, any opinion on this ?
> > 
> > I think it'd be best to be able to match multiple fields, not just entity
> > name. But I guess a regex goes a long way already.
> > 
> > There are some (unwritten?) rules for naming devices and they mostly work.
> 
> This is the part I'd like your opinion on. I can read written rules
> myself, but have harder access to the ones that are only in your brain
> :-)
> 
> From an MC point of view, my understanding is that the driver and model
> names should not be instance-specific (the model can vary between
> different SoCs that integrate different versions/models of the same IP,
> but shouldn't differ between multiple instances of the same IP in a
> given SoC, at least if those instances are identical). For entity names,

Agreed.

> we have a (probably unwritten) requirement for all entities in a given
> media device to have different names. What's your opinion about entity
> names when we have different media devices that correspond to multiple
> instances of the same IP core ? For instance, with two completely
> independent CSI-2 receivers, exposed as two separate media graphs, would
> you name the CSI-2 RX subdevs differently or identically ? If named
> differently, how would they differ ?

I don't think there's any benefit from having globally unique media entity
names, they simply need to be unique within a media device.

Camera sensor entities have been named as:

	sensor name [subdev name if more than one ]i2c-address

I guess that works fine as-is if you're targetting for a given system but
that is seldom the case anymore. Still it should be possible to recognise
different sensors from this if you know what you're looking for. Regular
expressions should help here.

> > What's deviceId here? Would you need that as these are on separate MC
> > devices? (I suppose it's up to the driver thought.)
> 
> In this specific case it's an index (equal to 0 or 1) that identifies a
> particular instance of the ISP (there's a single ISP at the hardware
> level, exposed as two virtual instances by the firmware).

I suppose the two still are present in the same media device?

-- 
Kind regards,

Sakari Ailus