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" ?
<snip>
+ media-ctl -V "\"camif1\":0 [fmt:UYVY2X8/640x480]"
I agree this looks very intuitive, but technically correct for the
csi1:1 and camif1:0 pads would be a 32-bit YUV format.
(MEDIA_BUS_FMT_YUV8_1X32_PADLO doesn't exist yet).
I think it would be better to use the correct format
I'm not sure I follow you here.
The ov5640 sends UYVY2X8 on the wire, so pads "ov5640_mipi 1-0040":0
up to "ipu1_csi1":0 are correct. But the CSI writes 32-bit YUV values
into the SMFC, so the CSI output pad and the IDMAC input pad should have
a YUV8_1X32 format.
Chapter 37.4.2.3 "FCW & FCR - Format converter write and read" in the
IDMAC chapter states that all internal submodules only work on 8-bit per
component formats with four components: YUVA or RGBA.
Right, the "direct" IPU internal (that do not transfer buffers to/from
memory via IDMAC channels) should only allow the IPU internal
formats YUVA and RGBA. I get you now.
The "direct" pads now only accept MEDIA_BUS_FMT_AYUV8_1X32 and
MEDIA_BUS_FMT_ARGB8888_1X32.
Those pads are:
ipu_csi:1
ipu_vdic:1
ipu_ic_prp:0
ipu_ic_prp:1
ipu_ic_prpenc:0
ipu_ic_prpenc:1
ipu_ic_prpvf:0
ipu_ic_prpvf:1
<snip>
There does not appear to be support in the FSU for linking a write channel
to the VDIC read channels (8, 9, 10) according to VDI_SRC_SEL field. There
is support for the direct link from CSI (which I am using), but that's
not an
IDMAC channel link.
There is a PRP_SRC_SEL field, with linking from IDMAC (SMFC) channels
0..2 (and 3? it's not clear, and not clear whether this includes channel 1).
As I read it, that is 0 and 2 only, no idea why. But since there are
only 2 CSIs, that shouldn't be a problem.
ipu_csi1 is now transferring on IDMAC/SMFC channel 2 (ipu_csi0 still
at channel 0).
<snip>
+static const u32 power_off_seq[] = {
+ IMX_MEDIA_GRP_ID_IC_PP,
+ IMX_MEDIA_GRP_ID_IC_PRPVF,
+ IMX_MEDIA_GRP_ID_IC_PRPENC,
+ IMX_MEDIA_GRP_ID_SMFC,
+ IMX_MEDIA_GRP_ID_CSI,
+ IMX_MEDIA_GRP_ID_VIDMUX,
+ IMX_MEDIA_GRP_ID_SENSOR,
+ IMX_MEDIA_GRP_ID_CSI2,
+};
This seems somewhat arbitrary. Why is a power sequence needed?
The CSI-2 receiver must be powered up before the sensor, that's the
only requirement IIRC. The others have no s_power requirement. So I
can probably change this to power up in the frontend -> backend order
(IC_PP to sensor). And vice-versa for power off.
Yes, I think that should work (see below).
Actually there are problems using this, see below.
[...]
+/*
+ * Turn current pipeline power on/off starting from start_entity.
+ * Must be called with mdev->graph_mutex held.
+ */
+int imx_media_pipeline_set_power(struct imx_media_dev *imxmd,
+ struct media_entity_graph *graph,
+ struct media_entity *start_entity, bool on)
+{
+ struct media_entity *entity;
+ struct v4l2_subdev *sd;
+ int i, ret = 0;
+ u32 id;
+
+ for (i = 0; i < NUM_POWER_ENTITIES; i++) {
+ id = on ? power_on_seq[i] : power_off_seq[i];
+ entity = find_pipeline_entity(imxmd, graph, start_entity, id);
+ if (!entity)
+ continue;
+
+ sd = media_entity_to_v4l2_subdev(entity);
+
+ ret = v4l2_subdev_call(sd, core, s_power, on);
+ if (ret && ret != -ENOIOCTLCMD)
+ break;
+ }
+
+ return (ret && ret != -ENOIOCTLCMD) ? ret : 0;
+}
+EXPORT_SYMBOL_GPL(imx_media_pipeline_set_power);
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.
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.
+int imx_media_add_internal_subdevs(struct imx_media_dev *imxmd,
+ struct imx_media_subdev *csi[4])
+{
+ int ret;
+
+ /* there must be at least one CSI in first IPU */
Why?
Well yeah, imx-media doesn't necessarily need a CSI if things
like the VDIC or post-processor are being used by an output
overlay pipeline, for example. I'll fix this.
I haven't even thought that far, but there could be boards with only a
parallel sensor connected to IPU2 CSI1 and IPU1 disabled for power
saving reasons.
done! A very simple change to imx_media_add_internal_subdevs(),
and now no CSI's are necessary, but the remaining IPU internal entities
are loaded and linked just fine without them, so for example with no
CSI's in IPU2, the VDIC entity in IPU2 is still present in the graph and
could still be used in the future by a mem2mem device for instance.
Steve
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
