<div dir="ltr"><div dir="ltr">Hi Sakari and Laurent,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 24 Nov 2021 at 06:53, Sakari Ailus <<a href="mailto:sakari.ailus@iki.fi" target="_blank">sakari.ailus@iki.fi</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>
On Tue, Nov 23, 2021 at 06:57:54PM +0200, Laurent Pinchart wrote:<br>
> Hi Sakari,<br>
> <br>
> On Tue, Nov 23, 2021 at 06:28:56PM +0200, Sakari Ailus wrote:<br>
> > Hi Laurent, Naushir,<br>
> > <br>
> > On Mon, Nov 22, 2021 at 03:00:40PM +0200, Laurent Pinchart wrote:<br>
> > > > -bool PipelineHandlerRPi::registerCameras()<br>
> > > > +int PipelineHandlerRPi::registerCameras(MediaDevice *unicam, MediaDevice *isp,<br>
> > > > +                                       const std::string &deviceId)<br>
> > > >  {<br>
> > > >         std::unique_ptr<RPiCameraData> data = std::make_unique<RPiCameraData>(this);<br>
> > > > +<br>
> > > >         if (!data->dmaHeap_.isValid())<br>
> > > > -               return false;<br>
> > > > +               return -ENOMEM;<br>
> > > > +<br>
> > > > +       MediaEntity *unicamImage = unicam->getEntityByName("unicam" + deviceId + "-image");<br>
> > > > +       MediaEntity *ispOutput0 = isp->getEntityByName("bcm2835-isp" + deviceId + "-output0");<br>
> > > > +       MediaEntity *ispCapture1 = isp->getEntityByName("bcm2835-isp" + deviceId + "-capture1");<br>
> > > > +       MediaEntity *ispCapture2 = isp->getEntityByName("bcm2835-isp" + deviceId + "-capture2");<br>
> > > > +       MediaEntity *ispCapture3 = isp->getEntityByName("bcm2835-isp" + deviceId + "-capture3");<br>
> > > <br>
> > > I'm wondering if wildcards would be nice here too, but maybe a simpler<br>
> > > option would be to not include the device ID in the entity names in the<br>
> > > driver ? We have a bus info field that the kernel reports for the media<br>
> > > device, to differentiate between multiple instances of the same device.<br>
> > > I suppose we are missing guidelines on the kernel side regarding entity<br>
> > > naming. Sakari, any opinion on this ?<br>
> > <br>
> > I think it'd be best to be able to match multiple fields, not just entity<br>
> > name. But I guess a regex goes a long way already.<br>
> > <br>
> > There are some (unwritten?) rules for naming devices and they mostly work.<br>
> <br>
> This is the part I'd like your opinion on. I can read written rules<br>
> myself, but have harder access to the ones that are only in your brain<br>
> :-)<br>
> <br>
> From an MC point of view, my understanding is that the driver and model<br>
> names should not be instance-specific (the model can vary between<br>
> different SoCs that integrate different versions/models of the same IP,<br>
> but shouldn't differ between multiple instances of the same IP in a<br>
> given SoC, at least if those instances are identical). For entity names,<br>
<br>
Agreed.<br>
<br>
> we have a (probably unwritten) requirement for all entities in a given<br>
> media device to have different names. What's your opinion about entity<br>
> names when we have different media devices that correspond to multiple<br>
> instances of the same IP core ? For instance, with two completely<br>
> independent CSI-2 receivers, exposed as two separate media graphs, would<br>
> you name the CSI-2 RX subdevs differently or identically ? If named<br>
> differently, how would they differ ?<br>
<br>
I don't think there's any benefit from having globally unique media entity<br>
names, they simply need to be unique within a media device.<br>
<br>
Camera sensor entities have been named as:<br>
<br>
        sensor name [subdev name if more than one ]i2c-address<br>
<br>
I guess that works fine as-is if you're targetting for a given system but<br>
that is seldom the case anymore. Still it should be possible to recognise<br>
different sensors from this if you know what you're looking for. Regular<br>
expressions should help here.<br>
<br>
> > What's deviceId here? Would you need that as these are on separate MC<br>
> > devices? (I suppose it's up to the driver thought.)<br>
> <br>
> In this specific case it's an index (equal to 0 or 1) that identifies a<br>
> particular instance of the ISP (there's a single ISP at the hardware<br>
> level, exposed as two virtual instances by the firmware).<br>
<br>
I suppose the two still are present in the same media device?<br></blockquote><div><br></div><div>Thanks for all the feedback!</div><div><br></div><div>We have no problem duplicating driver and model names as well as entity</div><div>names across multiple instances of the hardware (i.e. unindexed).  In many ways</div><div>this makes my life easier during enumeration in libcamera.</div><div><br></div><div>The confusion (and subsequent discussion) started from my assumption that</div><div>the names needed to be different in order to enumerate the different instances</div><div>of the devices through libcamera.  Laurent clarified that this is not the case, and</div><div>I've got it working now.</div><div><br></div><div>So if there are no objections, we will keep driver/entity names the same across</div><div>all instances of the unicam and bcm2835_isp driver.</div><div><br></div><div>Regards,</div><div>Naush</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
-- <br>
Kind regards,<br>
<br>
Sakari Ailus<br>
</blockquote></div></div>