ribalda: Could you also fix the driver name in the ImgU patches, and then resend?
Please also see my other reply.
The patches otherwise seem fine.
ok. I am working in the bytesperline
pinchartl: mchehab: Did you have time to look at: https://patchwork.linuxtv.org/project/linux-media/patch/20210917201521.592586-1-ribalda@chromium.org/ https://patchwork.linuxtv.org/project/linux-media/patch/20210917114930.47261-1-colin.king@canonical.com/ ?
not yet
ribalda: on a very quick look, patch looks ok, although I don't like very much to use kmemdup for copying strings... kstrdup() is generally better and would avoid things like:
+		/* Make sure there is a terminating null byte */ +		map->name[sizeof(xmap->name) - 1] = '\0';
I have a bridge device with 2 tx and 4 rx ports. I'd like to split it it to 2 tx subdevs, a mux subdev and 4 rx subdevs. I'm having some trouble understanding how I need to register the subdevs and how to hook up to the async notification mechanism.
that's similar to the adv748x, except that it has no subdevs connected to its inputs
jmondi: ^^
tomba: what have you tried so far ?
Well, I haven't tried with 4 rx subdevs yet, only with 2 tx subdevs and a "main" subdev. I now create the tx subdevs with v4l2_async_register_subdev. Then using the v4l2_subdev_internal_ops.registered, I create the main subdev with v4l2_device_register_subdev. This I can get working, and I get a pipeline, which ends at the main subdev.
If, when creating the main subdev, I try to register the async notifier, I get a deadlock. Let me check what it was.
yes, it's about list_lock
have you looked at how rcsi2_parse_dt() does it ?
it calls v4l2_async_subdev_notifier_register()
and is itself a subdev
registered with v4l2_async_register_subdev()
but is there an internal subdev after the rcar csi2?
no there isn't
so I'm thinking
I presume I could create all the subdevs here using the async mechanism, and implement the notifiers between the internal subdevs. In other words, handle it like the subdevs would be in fully separate drivers. But that's quite a bit of boilerplate.
can you create a subdev async notifier (v4l2_async_notifier_cleanup()) at probe time of your device
to match all the connected inputs
what subdev would I use for that?
you can register it with one of the two output subdevs
probably looking at which one(s) are actually connected
For the R-Car case with ADV7482 we have three drivers involved rcar-vin, rcar-csi2 and adv7482. Here rcar-vin is the simple one and registers a traditional async notifer
but you can start by hardcoding it to the first one, I wouldn't expect the second one to be connected without the first one being also connected
then you register your two output subdevs with v4l2_async_register_subdev()
and create the internal subdevs (and the links) when the output subdevs are registered
In the rcar-vin notfier only the rcar-csi2 subdev is entered
(I meant v4l2_async_subdev_notifier_register, not v4l2_async_notifier_cleanup)
neg: with VIn, we have both a device that creates multiple subdevs (adv748x) and a device that uses an subdev async notifier (v4l2_async_subdev_notifier_register()) (csi2), but we don't have a device that does both, right ?
Then rcar-csi2 that sits between the other two as a CSI-2 to parallel bridge register a subnotifer with the adv7482 in it. But here it gets tricky as the adv7482 register multiple subdevices the subnotifer in rcar-csi2 needs to look at endpoints and not device nodes
pinchartl: correct
And last the adv7482 register multiple subdevices, but modfies the registration so that each subdev is registerd using endpoint instead of device node. This works as the adv7482 has a 1:1 mapping of endpoints to subdevices
And that all adv7482 subdevices live in the same media graph. If for example there where two downstream VIN instances in two different media-graph domains this design don't work. And due to this the rcar-vin driver is overly complex as it for no real good reason force all VIN instances to be part of the same graph ;-)
there's been calls to have a single media graph for the whole system :-)
pinchartl: well, got it probing fine with the way you suggested, although I have only one TX subdev in use. Feels quite hacky, though =)
I can't disagree with that comment :-)
pinchartl: That would solve one of the complexities and I think it's a good idea. The second problem would be to allow for a single subdev to be registerd in more then one (sub)notifer
lets see what happens when I add the RX subdevs...
with an infinite amount of time, I'd rewrite v4l2 async from scratch
(in a way that would also supersede the dreadful component framework)
pinchartl: I was just about to ask about the component framework, is that even more fun?
it's worse yes
oh no. there's an update to Qt in gentoo...
sorry, was having lunch, I'll read the backlog
pinchartl: enjoy emerging
tomba: can the 4 rxes be freely routed to any of txes ?
as the adv748x which features 8 analog inputs is modeled as a single subdev (for the analog input part) with 8 sink pads and one source
jmondi: yes, they can be freely routed. I currently have the whole device as a single subdev, and I would like to keep it like that, but the async notifier mechanism doesn't work if a device has two tx ports.
tomba: is your receiver driver to which the txes are connected to matching on endpoints ?
yes
so you could theoretcially have one subdev with two source pads, but that would make handling the routing part akward, so there's no point in that :)
jmondi: even when matching on endpoints, a given subdev can be acquired once only
so if you have two independent CSI-2 RX instances matching on endpoints
and a single subdev for the CSI-2 TX with two source pads
you can't acquire the TX subdev from both RX instances
ah right, v4l2_device_register_subdev() is in the async math call path
s/math/match
jmondi: routing works fine
jmondi: sometimes v4l2-async feels like it's been written on s/match/meth/
pinchartl: hahaha. I found its documentation in Documentation/driver-api/media/v4l2-subdev.rst superb though
tomba: I meant with a single "tx" subdev with two sources. Modelling the connection with the 4 rxes in a separate subdev would have beek awkward
pinchartl: :)
so far I managed to survive V4L2 with only tea and chocolate. I hope it will continue that way
there's no shame in having to use hard drugs when dealing with other people's code
there's shame in having to use code review as an excuse to use hard drugs though
jernej: thinking out loud here, but in "media: cedrus: Fix SUNXI tile size calculation", I have this second thought that maybe I should have done that in try_fmt ?
ndufresne: cedrus_prepare_format() (where you fixed it) is called by try_fmt, so all good? 
jernej: I mean, perhaps I should have fixed the height to its padded size globally, it would less error prone
otherwise, if there is other code doing computation of buffer sizes from the height, it may need to do this roundup again
I think locally the patch is good, but I realize I haven't reviewed the code around (was a bit too focus on one thing)
I don't think this calculation is done anywhere else
ok then
jmondi: Qt upgrade complete :-)
pinchartl: just took 2 hr :)
would have been worse if qtwebengine had to be upgraded too. that one typically takes 6-7h