Hi Steve,
On 10/28/2017 10:36 PM, Steve Longerbeam wrote:
> This is a set of patches that improve support for more complex OF
> graphs. Currently the imx-media driver only supports a single device
> with a single port connected directly to either the CSI muxes or the
> MIPI CSI-2 receiver input ports. There can't be a multi-port device in
> between. This patch set removes those limitations.
>
> For an example taken from automotive, a camera sensor or decoder could
> be literally a remote device accessible over a FPD-III link, via TI
> DS90Ux9xx deserializer/serializer pairs. This patch set would support
> such OF graphs.
>
> There are still some assumptions and restrictions, regarding the equivalence
> of device-tree ports, port parents, and endpoints to media pads, entities,
> and links that have been enumerated in the TODO file.
Before I merge this patch series I wanted to know if Sakari's async work
that has now been merged (see https://www.spinics.net/lists/linux-media/msg124082.html)
affects this patch series.
It still applies cleanly, but I wondered if the subnotifier improvements
would simplify this driver.
Of course, any such simplification can also be done after this series has
been applied, but I don't know what your thoughts are on this.
Regards,
Hans
> This patch set supersedes the following patches submitted earlier:
>
> "[PATCH v2] media: staging/imx: do not return error in link_notify for unknown sources"
> "[PATCH RFC] media: staging/imx: fix complete handler"
>
> Tested by: Steve Longerbeam <[email protected]>
> on SabreLite with the OV5640
>
> Tested-by: Philipp Zabel <[email protected]>
> on Nitrogen6X with the TC358743.
>
> Tested-by: Russell King <[email protected]>
> with the IMX219
>
>
> Steve Longerbeam (9):
> media: staging/imx: get CSI bus type from nearest upstream entity
> media: staging/imx: remove static media link arrays
> media: staging/imx: of: allow for recursing downstream
> media: staging/imx: remove devname string from imx_media_subdev
> media: staging/imx: pass fwnode handle to find/add async subdev
> media: staging/imx: remove static subdev arrays
> media: staging/imx: convert static vdev lists to list_head
> media: staging/imx: reorder function prototypes
> media: staging/imx: update TODO
>
> drivers/staging/media/imx/TODO | 63 +++-
> drivers/staging/media/imx/imx-ic-prp.c | 4 +-
> drivers/staging/media/imx/imx-media-capture.c | 2 +
> drivers/staging/media/imx/imx-media-csi.c | 187 +++++-----
> drivers/staging/media/imx/imx-media-dev.c | 400 ++++++++++------------
> drivers/staging/media/imx/imx-media-internal-sd.c | 253 +++++++-------
> drivers/staging/media/imx/imx-media-of.c | 278 ++++++++-------
> drivers/staging/media/imx/imx-media-utils.c | 122 +++----
> drivers/staging/media/imx/imx-media.h | 187 ++++------
> 9 files changed, 722 insertions(+), 774 deletions(-)
>
Hi Hans,
On 12/04/2017 05:44 AM, Hans Verkuil wrote:
> Hi Steve,
>
> On 10/28/2017 10:36 PM, Steve Longerbeam wrote:
>> This is a set of patches that improve support for more complex OF
>> graphs. Currently the imx-media driver only supports a single device
>> with a single port connected directly to either the CSI muxes or the
>> MIPI CSI-2 receiver input ports. There can't be a multi-port device in
>> between. This patch set removes those limitations.
>>
>> For an example taken from automotive, a camera sensor or decoder could
>> be literally a remote device accessible over a FPD-III link, via TI
>> DS90Ux9xx deserializer/serializer pairs. This patch set would support
>> such OF graphs.
>>
>> There are still some assumptions and restrictions, regarding the equivalence
>> of device-tree ports, port parents, and endpoints to media pads, entities,
>> and links that have been enumerated in the TODO file.
> Before I merge this patch series I wanted to know if Sakari's async work
> that has now been merged (see https://www.spinics.net/lists/linux-media/msg124082.html)
> affects this patch series.
>
> It still applies cleanly, but I wondered if the subnotifier improvements
> would simplify this driver.
Indeed it does, it negates the need to recursively walk the OF graph to
discover
and register the fwnodes with the async notifier, as imx-media is
currently doing.
Of course it does require adding some code to the sub-devices, to call
v4l2_async_notifier_parse_fwnode_endpoints_by_port() to register remote
fwnodes attached to the sub-device's ports, and then
v4l2_async_subdev_notifier_register() to register a sub-notifier.
>
> Of course, any such simplification can also be done after this series has
> been applied, but I don't know what your thoughts are on this.
I do prefer the sub-notifier approach to discovering fwnodes, so yes I would
like to switch to this.
Since it is a distinct change (using sub-notifiers instead of recursive
graph
walk), I would prefer to get this series applied first, and then switch to
sub-notifiers as distinct patches afterwards.
Let me submit a v2 of this series first however. There are some minor
changes I
would like to make so that the up-coming sub-notifier patches are cleaner.
Steve
>
>> This patch set supersedes the following patches submitted earlier:
>>
>> "[PATCH v2] media: staging/imx: do not return error in link_notify for unknown sources"
>> "[PATCH RFC] media: staging/imx: fix complete handler"
>>
>> Tested by: Steve Longerbeam <[email protected]>
>> on SabreLite with the OV5640
>>
>> Tested-by: Philipp Zabel <[email protected]>
>> on Nitrogen6X with the TC358743.
>>
>> Tested-by: Russell King <[email protected]>
>> with the IMX219
>>
>>
>> Steve Longerbeam (9):
>> media: staging/imx: get CSI bus type from nearest upstream entity
>> media: staging/imx: remove static media link arrays
>> media: staging/imx: of: allow for recursing downstream
>> media: staging/imx: remove devname string from imx_media_subdev
>> media: staging/imx: pass fwnode handle to find/add async subdev
>> media: staging/imx: remove static subdev arrays
>> media: staging/imx: convert static vdev lists to list_head
>> media: staging/imx: reorder function prototypes
>> media: staging/imx: update TODO
>>
>> drivers/staging/media/imx/TODO | 63 +++-
>> drivers/staging/media/imx/imx-ic-prp.c | 4 +-
>> drivers/staging/media/imx/imx-media-capture.c | 2 +
>> drivers/staging/media/imx/imx-media-csi.c | 187 +++++-----
>> drivers/staging/media/imx/imx-media-dev.c | 400 ++++++++++------------
>> drivers/staging/media/imx/imx-media-internal-sd.c | 253 +++++++-------
>> drivers/staging/media/imx/imx-media-of.c | 278 ++++++++-------
>> drivers/staging/media/imx/imx-media-utils.c | 122 +++----
>> drivers/staging/media/imx/imx-media.h | 187 ++++------
>> 9 files changed, 722 insertions(+), 774 deletions(-)
>>
On 12/07/2017 03:23 PM, Steve Longerbeam wrote:
> Hi Hans,
>
>
> On 12/04/2017 05:44 AM, Hans Verkuil wrote:
>
> <snip>
>>
>> Of course, any such simplification can also be done after this series
>> has
>> been applied, but I don't know what your thoughts are on this.
>
> I do prefer the sub-notifier approach to discovering fwnodes, so yes I
> would
> like to switch to this.
>
> Since it is a distinct change (using sub-notifiers instead of
> recursive graph
> walk), I would prefer to get this series applied first, and then
> switch to
> sub-notifiers as distinct patches afterwards.
>
> Let me submit a v2 of this series first however. There are some minor
> changes I
> would like to make so that the up-coming sub-notifier patches are
> cleaner.
>
Hans,
Never mind, the sub-notifier patches should be fairly clean. I won't
submit a
v2 of this series unless it is requested by reviewers.
Steve