Messages in this thread | | | Subject | Re: [PATCH v3 16/24] media: Add i.MX media core driver | From | Steve Longerbeam <> | Date | Mon, 23 Jan 2017 17:38:33 -0800 |
| |
On 01/23/2017 03:13 AM, Philipp Zabel wrote: > Hi Steve, > > On Sun, 2017-01-22 at 18:31 -0800, Steve Longerbeam wrote: >> On 01/16/2017 05:47 AM, Philipp Zabel wrote: >>> On Sat, 2017-01-14 at 14:46 -0800, Steve Longerbeam wrote: >>> [...] >>>>>> +Unprocessed Video Capture: >>>>>> +-------------------------- >>>>>> + >>>>>> +Send frames directly from sensor to camera interface, with no >>>>>> +conversions: >>>>>> + >>>>>> +-> ipu_smfc -> camif >>>>> I'd call this capture interface, this is not just for cameras. Or maybe >>>>> idmac if you want to mirror hardware names? >>>> Camif is so named because it is the V4L2 user interface for video >>>> capture. I suppose it could be named "capif", but that doesn't role >>>> off the tongue quite as well. >>> Agreed, capif sounds weird. I find camif a bit confusing though, because >>> Samsung S3C has a camera interface that is actually called "CAMIF". >> how about simply "capture" ? > That sounds good to me.
done.
> >>>>> This should really be handled by v4l2_pipeline_pm_use. >>>> I thought about this earlier, but v4l2_pipeline_pm_use() seems to be >>>> doing some other stuff that bothered me, at least that's what I remember. >>>> I will revisit this. >>> I have used it with a tc358743 -> mipi-csi2 pipeline, it didn't cause >>> any problems. It would be better to reuse and, if necessary, fix the >>> existing infrastructure where available. >> I tried this API, by switching to v4l2_pipeline_pm_use() in camif >> open/release, >> and switched to v4l2_pipeline_link_notify() instead of >> imx_media_link_notify() >> in the media driver's media_device_ops. >> >> This API assumes the video device has an open file handle while the media >> links are being established. This doesn't work for me, I want to be able to >> establish the links using 'media-ctl -l', and that won't work unless >> there is an >> open file handle on the video capture device node. >> >> Also, I looked into calling v4l2_pipeline_pm_use() during >> imx_media_link_notify(), >> instead of imx_media_pipeline_set_power(). Again there are problems with >> that. >> >> First, v4l2_pipeline_pm_use() acquires the graph mutex, so it can't be >> called inside >> link_notify which already acquires that lock. The header for this >> function also >> clearly states it should only be called in open/release. > So why not call it in open/release then?
er, see above (?)
> >> Second, ignoring the above locking issue for a moment, >> v4l2_pipeline_pm_use() >> will call s_power on the sensor _first_, then the mipi csi-2 s_power, >> when executing >> media-ctl -l '"ov5640 1-003c":0 -> "imx6-mipi-csi2":0[1]'. Which is the >> wrong order. >> In my version which enforces the correct power on order, the mipi csi-2 >> s_power >> is called first in that link setup, followed by the sensor. > I don't understand why you want to power up subdevs as soon as the links > are established.
Because that is the precedence, all other media drivers do pipeline power on/off at link_notify. And v4l2_pipeline_link_notify() was written as a link_notify method.
> Shouldn't that rather be done for all subdevices in the > pipeline when the corresponding capture device is opened?
that won't work. There's no guarantee the links will be established at capture device open time.
> It seems to me that powering up the pipeline should be the last step > before userspace actually starts the capture.
Well, I'm ok with moving pipeline power on/off to start/stop streaming. I would actually prefer to do it then, I only chose at link_notify because of precedence. I'll look into it.
Steve
| |