Hi Jacopo, On Tue, Dec 01, 2020 at 10:26:05AM +0100, Jacopo Mondi wrote: > Hi Hans, Sakari, > > On Tue, Nov 10, 2020 at 06:40:31PM +0100, Jacopo Mondi wrote: > > Hello, > > new iteration following > > v3: https://patchwork.linuxtv.org/project/linux-media/list/?series=3768 > > > > Major changelog: > > - Use v4l2_dev.release and drop manual ref-counting as suggested by Dafna > > - Address Hans' comments on queue setup and metadata format handling function > > - s/MEDIA_BUS_FMT_SENSOR_DATA/MEDIA_BUS_FMT_CUSTOM_SENSOR_DATA as suggested by > > Hans and rebase on Dafna's fixed metadata format patch > > - Add a TODO file to explain why the driver is in staging. > > - Conditionally register the unicam-embedded video device node on the presence > > of the sensor's metadata source pad. > > > > The media graph, when connected to a sensor that does not report metadata > > looks like: > > > > Media controller API version 5.10.0 > > > > Media device information > > ------------------------ > > driver unicam > > model unicam > > serial > > bus info platform:fe801000.csi > > hw revision 0x0 > > driver version 5.10.0 > > > > Device topology > > - entity 1: ov5647 10-0036 (1 pad, 1 link) > > type V4L2 subdev subtype Sensor flags 0 > > device node name /dev/v4l-subdev0 > > pad0: Source > > [fmt:SBGGR10_1X10/640x480 field:none colorspace:srgb > > crop.bounds:(16,16)/2592x1944 > > crop:(32,16)/2560x1920] > > -> "unicam-image":0 [ENABLED,IMMUTABLE] > > > > - entity 3: unicam-image (1 pad, 1 link) > > type Node subtype V4L flags 1 > > device node name /dev/video0 > > pad0: Sink > > <- "ov5647 10-0036":0 [ENABLED,IMMUTABLE] > > > > > > If the sensor reports an additional metadata pad: > > > > Media controller API version 5.10.0 > > > > Media device information > > ------------------------ > > driver unicam > > model unicam > > serial > > bus info platform:fe801000.csi > > hw revision 0x0 > > driver version 5.10.0 > > > > Device topology > > - entity 1: imx219 10-0010 (2 pads, 2 links) > > type V4L2 subdev subtype Sensor flags 0 > > device node name /dev/v4l-subdev0 > > pad0: Source > > [fmt:SRGGB10_1X10/3280x2464 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:full-range > > crop:(0,0)/3280x2464] > > -> "unicam-image":0 [ENABLED,IMMUTABLE] > > pad1: Source > > [fmt:unknown/16384x1 field:none > > crop:(0,0)/3280x2464] > > -> "unicam-embedded":0 [ENABLED,IMMUTABLE] > > > > - entity 4: unicam-image (1 pad, 1 link) > > type Node subtype V4L flags 1 > > device node name /dev/video0 > > pad0: Sink > > <- "imx219 10-0010":0 [ENABLED,IMMUTABLE] > > > > - entity 10: unicam-embedded (1 pad, 1 link) > > type Node subtype V4L flags 0 > > device node name /dev/video1 > > pad0: Sink > > <- "imx219 10-0010":1 [ENABLED,IMMUTABLE] > > > > Conditionally registering the metadata video node allows to simplify the > > code in the driver as well, removing the 'sensor_embedded_data' flag. > > > > An additional note: this version will break the libcamera pipeline handler > > which assume the unicam-embedded video device node to always be there. > > > > From Dave's reply to Dafna's comments I get instead that for the existing > > applications ecosystem, having the metadata node not registered if the sensor > > does not support it is not an issue. > > I think I've closed comments received on v3. > > Do you think this series is ready for being collected ? I'll try to look into this later today / this week. The problem with the approach appears, based on a quick glance, to be that it creates an additional way (to the more generic approach in VC support patchset) to support embedded data, including duplicated sensor driver support. That appears as a dead end. But let me look into the details first. -- Kind regards, Sakari Ailus
