(unlike rcar-vin, imx capture interface doesn't crop or scale, the
connected source subdev does that, but it does pad the compose window so
it needs to know the original compose window, sorry hope I haven't lost
you). Anyway I think catching the V4L2_EVENT_SOURCE_CHANGE event is
maybe a cleaner way to update the crop bounds in rcar-vin than using an
exported function.
2. Sub-device events need to be forwarded to the VIN's that have enabled
media links to the reporting sub-device. Currently, sub-device events are
only forwarded to VIN7, because the notifier is registered only from VIN7,
and so the sub-devices are registered only to the v4l2_dev owned by VIN7. So
we have a set of patches that fix this: sub-device events get forwarded to
the VIN's that have connected media paths to that sub-device. Besides
allowing to reset the VIN crop bounds rectangle asynchronously, this also
seems to be logically the correct behavior. It also makes the user interface
a little more intuitive: userland knows which VIN it is capturing on, and
that is the same VIN that will be receiving sub-device events in the active
pipeline.
This is a problem I ponder from time to time, happy to hear I'm not the
only one :-) My view is that events are somewhat not functional in the
media controller world and should be fixed at the framework level, maybe
by moving them from the vdev to the mdev :-)
How about embedding the 'struct v4l2_device' into 'struct rvin_group'
instead of 'struct rvin_dev'? That probably makes more sense, the
v4l2_dev keeps track of media-device-wide info such as the list of
sub-devices in the graph, so it seems more appropriate to create only
one instance of v4l2_dev. That will force rcar-vin to lookup the correct
VINs to forward the event to, since v4l2_dev is no longer attached to
the VINs.
But moving events from v4l2 to media (e.g. media entities send events to
the mdev, rather than v4l2 sub-devices sending events to the v4l2_dev)
is also an interesting idea.
I think it's great that you worked on the problem but I'm a bit
concerned with addressing this on a driver basis. Maybe this is
something to bring up at the next v4l2 summit? I might be wrong here and
there already is a consensus to address this as you describe in each
driver. Do you have any insights on the topic?
I haven't been tuned into that topic, but yes I agree that events need a
better framework.
3. We have some patches under review that allow alternate -> none field
mode. That is, source sub-device is sending alternate fields (top, bottom,
top, ...), and userland is configured to receive those fields unmodified by
setting field type to none. rcar-dma then detects and reports the field type
of the captured field (top or bottom) in (struct v4l2_buffer)->field. Doing
this allows for de-interlacing the captured fields using the FDP1 mem2mem
device.
Interesting, maybe I'm missing something but would it not be a better
solution to add support for V4L2_FIELD_ALTERNATE to rcar-vin instead of
abusing V4L2_FIELD_NONE?
Well, from 6c51f646f6 ("media: rcar-vin: fix handling of single field
frames (top, bottom and alternate fields)"),
"There was never proper support in the VIN driver to deliver ALTERNATING
field format to user-space, remove this field option. The problem is
that ALTERNATING field order requires the sequence numbers of buffers
returned to userspace to reflect if fields were dropped or not,
something which is not possible with the VIN drivers capture logic."
Is that still a concern, or can alternate mode be brought back but with
possibly the above limitation?
Also, there is this paragraph in [1] regarding V4L2_FIELD_NONE, so it
sounds like using this field type in this way may not be too much abuse :)
"Images are in progressive format, not interlaced. The driver may also
indicate this order when it cannot distinguish
between|V4L2_FIELD_TOP|and|V4L2_FIELD_BOTTOM|."
[1] https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/field-order.html
There are some other miscellaneous patches that I can submit, but the above
describes the bulk of the changes.
I'm happy to see work being done on the driver and would of course like
to help you get all changes upstream so that all your use-cases would
work out-of-the-box.
Before I start on porting these patches to the media-tree, do the above
changes make general sense, or are there fundamental problems with those
ideas?
Subject: [PATCH] media: rcar-vin: Generalize vin_to_source()
Change the vin_to_source() macro to an inline function that will
retrieve the source subdevice for both media-control and non
media-control mode.
Signed-off-by: Steve Longerbeam <steve_longerbeam@xxxxxxxxxx>
---
[..]
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index 0b13b34d03e3..29d8c4a80c35 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -217,7 +217,21 @@ struct rvin_dev {
v4l2_std_id std;
};
-#define vin_to_source(vin) ((vin)->parallel->subdev)
+static inline struct v4l2_subdev *
+vin_to_source(struct rvin_dev *vin)
+{
+ if (vin->info->use_mc) {
+ struct media_pad *pad;
+
+ pad = media_entity_remote_pad(&vin->pad);
+ if (!pad)
+ return NULL;
+
+ return media_entity_to_v4l2_subdev(pad->entity);
+ }
+
+ return vin->parallel->subdev;
+}
/* Debug */
#define vin_dbg(d, fmt, arg...) dev_dbg(d->dev, fmt, ##arg)
