* [PATCH v2 01/23] media: entity: Pass entity to get_fwnode_pad operation
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 02/23] media: entity: Modify default behavior of media_entity_get_fwnode_pad Steve Longerbeam
` (23 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Add a missing pointer to the entity in the media_entity operation
get_fwnode_pad.
Fixes: ae45cd5efc120 ("[media] media: entity: Add get_fwnode_pad entity
operation")
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/media/mc/mc-entity.c | 2 +-
include/media/media-entity.h | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 7c429ce98bae..c333320f790a 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -386,7 +386,7 @@ int media_entity_get_fwnode_pad(struct media_entity *entity,
if (ret)
return ret;
- ret = entity->ops->get_fwnode_pad(&endpoint);
+ ret = entity->ops->get_fwnode_pad(entity, &endpoint);
if (ret < 0)
return ret;
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 8cb2c504a05c..cde80ad029b7 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -212,7 +212,8 @@ struct media_pad {
* mutex held.
*/
struct media_entity_operations {
- int (*get_fwnode_pad)(struct fwnode_endpoint *endpoint);
+ int (*get_fwnode_pad)(struct media_entity *entity,
+ struct fwnode_endpoint *endpoint);
int (*link_setup)(struct media_entity *entity,
const struct media_pad *local,
const struct media_pad *remote, u32 flags);
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 02/23] media: entity: Modify default behavior of media_entity_get_fwnode_pad
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 01/23] media: entity: Pass entity to get_fwnode_pad operation Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-28 12:04 ` Jacopo Mondi
2019-11-24 19:06 ` [PATCH v2 03/23] media: entity: Convert media_entity_get_fwnode_pad() args to const Steve Longerbeam
` (22 subsequent siblings)
24 siblings, 1 reply; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Modify the default behavior of media_entity_get_fwnode_pad() (when the
entity does not provide the get_fwnode_pad op) to first assume the entity
implements a 1:1 mapping between fwnode port number and media pad index.
If the 1:1 mapping is not valid, e.g. the port number falls outside the
entity's pad index range, or the pad at that port number doesn't match the
given direction_flags, fall-back to the previous behavior that searches
the entity for the first pad that matches the given direction_flags.
The previous default behavior can choose the wrong pad for entities with
multiple sink or source pads. With this change the function will choose
the correct pad index if the entity implements a 1:1 port to pad mapping
at that port.
Add some comments to the @get_fwnode_pad operation to make it more clear
under what conditions entities must implement the operation.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/media/mc/mc-entity.c | 25 ++++++++++++++++++++-----
include/media/media-entity.h | 21 +++++++++++++++------
2 files changed, 35 insertions(+), 11 deletions(-)
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index c333320f790a..47a39d9383d8 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -370,22 +370,37 @@ int media_entity_get_fwnode_pad(struct media_entity *entity,
unsigned long direction_flags)
{
struct fwnode_endpoint endpoint;
- unsigned int i;
int ret;
+ ret = fwnode_graph_parse_endpoint(fwnode, &endpoint);
+ if (ret)
+ return ret;
+
if (!entity->ops || !entity->ops->get_fwnode_pad) {
+ unsigned int i;
+
+ /*
+ * for the default case, first try a 1:1 mapping between
+ * fwnode port number and pad index.
+ */
+ ret = endpoint.port;
+ if (ret < entity->num_pads &&
+ (entity->pads[ret].flags & direction_flags))
+ return ret;
+
+ /*
+ * if that didn't work search the entity for the first
+ * pad that matches the @direction_flags.
+ */
for (i = 0; i < entity->num_pads; i++) {
if (entity->pads[i].flags & direction_flags)
return i;
}
+ /* else fail */
return -ENXIO;
}
- ret = fwnode_graph_parse_endpoint(fwnode, &endpoint);
- if (ret)
- return ret;
-
ret = entity->ops->get_fwnode_pad(entity, &endpoint);
if (ret < 0)
return ret;
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index cde80ad029b7..ed00adb4313b 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -199,6 +199,12 @@ struct media_pad {
* @get_fwnode_pad: Return the pad number based on a fwnode endpoint or
* a negative value on error. This operation can be used
* to map a fwnode to a media pad number. Optional.
+ * Entities do not need to implement this operation
+ * unless two conditions are met:
+ * - the entity has more than one sink and/or source
+ * pad, _and_
+ * - the entity does not implement a 1:1 mapping of
+ * fwnode port numbers to pad indexes.
* @link_setup: Notify the entity of link changes. The operation can
* return an error, in which case link setup will be
* cancelled. Optional.
@@ -858,21 +864,24 @@ struct media_link *media_entity_find_link(struct media_pad *source,
struct media_pad *media_entity_remote_pad(const struct media_pad *pad);
/**
- * media_entity_get_fwnode_pad - Get pad number from fwnode
+ * media_entity_get_fwnode_pad - Get pad number from an endpoint fwnode
*
* @entity: The entity
- * @fwnode: Pointer to the fwnode_handle which should be used to find the pad
+ * @fwnode: Pointer to the endpoint fwnode_handle which should be used to
+ * find the pad
* @direction_flags: Expected direction of the pad, as defined in
* :ref:`include/uapi/linux/media.h <media_header>`
* (seek for ``MEDIA_PAD_FL_*``)
*
* This function can be used to resolve the media pad number from
- * a fwnode. This is useful for devices which use more complex
- * mappings of media pads.
+ * an endpoint fwnode. This is useful for devices which use more
+ * complex mappings of media pads.
*
* If the entity does not implement the get_fwnode_pad() operation
- * then this function searches the entity for the first pad that
- * matches the @direction_flags.
+ * then this function first assumes the entity implements a 1:1 mapping
+ * between fwnode port number and media pad index. If the 1:1 mapping
+ * is not valid, then the function searches the entity for the first pad
+ * that matches the @direction_flags.
*
* Return: returns the pad number on success or a negative error code.
*/
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v2 02/23] media: entity: Modify default behavior of media_entity_get_fwnode_pad
2019-11-24 19:06 ` [PATCH v2 02/23] media: entity: Modify default behavior of media_entity_get_fwnode_pad Steve Longerbeam
@ 2019-11-28 12:04 ` Jacopo Mondi
2019-12-14 21:29 ` Steve Longerbeam
0 siblings, 1 reply; 38+ messages in thread
From: Jacopo Mondi @ 2019-11-28 12:04 UTC (permalink / raw)
To: Steve Longerbeam; +Cc: linux-media
[-- Attachment #1: Type: text/plain, Size: 5616 bytes --]
Hi Steve,
On Sun, Nov 24, 2019 at 11:06:42AM -0800, Steve Longerbeam wrote:
> Modify the default behavior of media_entity_get_fwnode_pad() (when the
> entity does not provide the get_fwnode_pad op) to first assume the entity
> implements a 1:1 mapping between fwnode port number and media pad index.
>
> If the 1:1 mapping is not valid, e.g. the port number falls outside the
> entity's pad index range, or the pad at that port number doesn't match the
> given direction_flags, fall-back to the previous behavior that searches
> the entity for the first pad that matches the given direction_flags.
>
> The previous default behavior can choose the wrong pad for entities with
> multiple sink or source pads. With this change the function will choose
> the correct pad index if the entity implements a 1:1 port to pad mapping
> at that port.
>
> Add some comments to the @get_fwnode_pad operation to make it more clear
> under what conditions entities must implement the operation.
>
I understand the rationale behind this, but this is not better than
assuming the first pad with a matching direction flag is the right
one imho.
This should help for subdevices with multiple sink/sources, but they
should really implement get_fwnode_pad() instead of relying on yet another
assumption as we had "the first pad with matching direction is the right
one" and now we also have "the pad at index endpoint.port is the right
one". As you've been testing it, how many of the current mailine
supported devices actually need this and could not:
1) Implement get_fwnode_pad() as you are doing for adv748x
2) Keep assuming the first pad with a matching flag is the correct one
Thanks
j
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
> drivers/media/mc/mc-entity.c | 25 ++++++++++++++++++++-----
> include/media/media-entity.h | 21 +++++++++++++++------
> 2 files changed, 35 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> index c333320f790a..47a39d9383d8 100644
> --- a/drivers/media/mc/mc-entity.c
> +++ b/drivers/media/mc/mc-entity.c
> @@ -370,22 +370,37 @@ int media_entity_get_fwnode_pad(struct media_entity *entity,
> unsigned long direction_flags)
> {
> struct fwnode_endpoint endpoint;
> - unsigned int i;
> int ret;
>
> + ret = fwnode_graph_parse_endpoint(fwnode, &endpoint);
> + if (ret)
> + return ret;
> +
> if (!entity->ops || !entity->ops->get_fwnode_pad) {
> + unsigned int i;
> +
> + /*
> + * for the default case, first try a 1:1 mapping between
> + * fwnode port number and pad index.
> + */
> + ret = endpoint.port;
> + if (ret < entity->num_pads &&
> + (entity->pads[ret].flags & direction_flags))
> + return ret;
> +
> + /*
> + * if that didn't work search the entity for the first
> + * pad that matches the @direction_flags.
> + */
> for (i = 0; i < entity->num_pads; i++) {
> if (entity->pads[i].flags & direction_flags)
> return i;
> }
>
> + /* else fail */
> return -ENXIO;
> }
>
> - ret = fwnode_graph_parse_endpoint(fwnode, &endpoint);
> - if (ret)
> - return ret;
> -
> ret = entity->ops->get_fwnode_pad(entity, &endpoint);
> if (ret < 0)
> return ret;
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index cde80ad029b7..ed00adb4313b 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -199,6 +199,12 @@ struct media_pad {
> * @get_fwnode_pad: Return the pad number based on a fwnode endpoint or
> * a negative value on error. This operation can be used
> * to map a fwnode to a media pad number. Optional.
> + * Entities do not need to implement this operation
> + * unless two conditions are met:
> + * - the entity has more than one sink and/or source
> + * pad, _and_
> + * - the entity does not implement a 1:1 mapping of
> + * fwnode port numbers to pad indexes.
> * @link_setup: Notify the entity of link changes. The operation can
> * return an error, in which case link setup will be
> * cancelled. Optional.
> @@ -858,21 +864,24 @@ struct media_link *media_entity_find_link(struct media_pad *source,
> struct media_pad *media_entity_remote_pad(const struct media_pad *pad);
>
> /**
> - * media_entity_get_fwnode_pad - Get pad number from fwnode
> + * media_entity_get_fwnode_pad - Get pad number from an endpoint fwnode
> *
> * @entity: The entity
> - * @fwnode: Pointer to the fwnode_handle which should be used to find the pad
> + * @fwnode: Pointer to the endpoint fwnode_handle which should be used to
> + * find the pad
> * @direction_flags: Expected direction of the pad, as defined in
> * :ref:`include/uapi/linux/media.h <media_header>`
> * (seek for ``MEDIA_PAD_FL_*``)
> *
> * This function can be used to resolve the media pad number from
> - * a fwnode. This is useful for devices which use more complex
> - * mappings of media pads.
> + * an endpoint fwnode. This is useful for devices which use more
> + * complex mappings of media pads.
> *
> * If the entity does not implement the get_fwnode_pad() operation
> - * then this function searches the entity for the first pad that
> - * matches the @direction_flags.
> + * then this function first assumes the entity implements a 1:1 mapping
> + * between fwnode port number and media pad index. If the 1:1 mapping
> + * is not valid, then the function searches the entity for the first pad
> + * that matches the @direction_flags.
> *
> * Return: returns the pad number on success or a negative error code.
> */
> --
> 2.17.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v2 02/23] media: entity: Modify default behavior of media_entity_get_fwnode_pad
2019-11-28 12:04 ` Jacopo Mondi
@ 2019-12-14 21:29 ` Steve Longerbeam
0 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-12-14 21:29 UTC (permalink / raw)
To: Jacopo Mondi; +Cc: linux-media
Hi Jacopo,
On 11/28/19 4:04 AM, Jacopo Mondi wrote:
> Hi Steve,
>
> On Sun, Nov 24, 2019 at 11:06:42AM -0800, Steve Longerbeam wrote:
>> Modify the default behavior of media_entity_get_fwnode_pad() (when the
>> entity does not provide the get_fwnode_pad op) to first assume the entity
>> implements a 1:1 mapping between fwnode port number and media pad index.
>>
>> If the 1:1 mapping is not valid, e.g. the port number falls outside the
>> entity's pad index range, or the pad at that port number doesn't match the
>> given direction_flags, fall-back to the previous behavior that searches
>> the entity for the first pad that matches the given direction_flags.
>>
>> The previous default behavior can choose the wrong pad for entities with
>> multiple sink or source pads. With this change the function will choose
>> the correct pad index if the entity implements a 1:1 port to pad mapping
>> at that port.
>>
>> Add some comments to the @get_fwnode_pad operation to make it more clear
>> under what conditions entities must implement the operation.
>>
> I understand the rationale behind this, but this is not better than
> assuming the first pad with a matching direction flag is the right
> one imho.
>
> This should help for subdevices with multiple sink/sources, but they
> should really implement get_fwnode_pad() instead of relying on yet another
> assumption as we had "the first pad with matching direction is the right
> one" and now we also have "the pad at index endpoint.port is the right
> one".
I don't see the second assumption as really an assumption. If the given
endpoint's port number falls within the entity's pad index range (it's a
valid pad index), and the pad at that index matches the requested
direction, then it _must_ be the correct pad.
> As you've been testing it, how many of the current mailine
> supported devices actually need this and could not:
> 1) Implement get_fwnode_pad() as you are doing for adv748x
> 2) Keep assuming the first pad with a matching flag is the correct one
Well, this patch reduces the number of devices that need to implement
get_fwnode_pad():
Before: devices do NOT need implement the op if they have just one pad
of a given direction (one sink pad, one source pad, or one of each).
After: devices do NOT need to implement the op if they have just one pad
of a given direction, OR all of their pad indexes match their fwnode
port numbers.
Steve
>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>> ---
>> drivers/media/mc/mc-entity.c | 25 ++++++++++++++++++++-----
>> include/media/media-entity.h | 21 +++++++++++++++------
>> 2 files changed, 35 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
>> index c333320f790a..47a39d9383d8 100644
>> --- a/drivers/media/mc/mc-entity.c
>> +++ b/drivers/media/mc/mc-entity.c
>> @@ -370,22 +370,37 @@ int media_entity_get_fwnode_pad(struct media_entity *entity,
>> unsigned long direction_flags)
>> {
>> struct fwnode_endpoint endpoint;
>> - unsigned int i;
>> int ret;
>>
>> + ret = fwnode_graph_parse_endpoint(fwnode, &endpoint);
>> + if (ret)
>> + return ret;
>> +
>> if (!entity->ops || !entity->ops->get_fwnode_pad) {
>> + unsigned int i;
>> +
>> + /*
>> + * for the default case, first try a 1:1 mapping between
>> + * fwnode port number and pad index.
>> + */
>> + ret = endpoint.port;
>> + if (ret < entity->num_pads &&
>> + (entity->pads[ret].flags & direction_flags))
>> + return ret;
>> +
>> + /*
>> + * if that didn't work search the entity for the first
>> + * pad that matches the @direction_flags.
>> + */
>> for (i = 0; i < entity->num_pads; i++) {
>> if (entity->pads[i].flags & direction_flags)
>> return i;
>> }
>>
>> + /* else fail */
>> return -ENXIO;
>> }
>>
>> - ret = fwnode_graph_parse_endpoint(fwnode, &endpoint);
>> - if (ret)
>> - return ret;
>> -
>> ret = entity->ops->get_fwnode_pad(entity, &endpoint);
>> if (ret < 0)
>> return ret;
>> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
>> index cde80ad029b7..ed00adb4313b 100644
>> --- a/include/media/media-entity.h
>> +++ b/include/media/media-entity.h
>> @@ -199,6 +199,12 @@ struct media_pad {
>> * @get_fwnode_pad: Return the pad number based on a fwnode endpoint or
>> * a negative value on error. This operation can be used
>> * to map a fwnode to a media pad number. Optional.
>> + * Entities do not need to implement this operation
>> + * unless two conditions are met:
>> + * - the entity has more than one sink and/or source
>> + * pad, _and_
>> + * - the entity does not implement a 1:1 mapping of
>> + * fwnode port numbers to pad indexes.
>> * @link_setup: Notify the entity of link changes. The operation can
>> * return an error, in which case link setup will be
>> * cancelled. Optional.
>> @@ -858,21 +864,24 @@ struct media_link *media_entity_find_link(struct media_pad *source,
>> struct media_pad *media_entity_remote_pad(const struct media_pad *pad);
>>
>> /**
>> - * media_entity_get_fwnode_pad - Get pad number from fwnode
>> + * media_entity_get_fwnode_pad - Get pad number from an endpoint fwnode
>> *
>> * @entity: The entity
>> - * @fwnode: Pointer to the fwnode_handle which should be used to find the pad
>> + * @fwnode: Pointer to the endpoint fwnode_handle which should be used to
>> + * find the pad
>> * @direction_flags: Expected direction of the pad, as defined in
>> * :ref:`include/uapi/linux/media.h <media_header>`
>> * (seek for ``MEDIA_PAD_FL_*``)
>> *
>> * This function can be used to resolve the media pad number from
>> - * a fwnode. This is useful for devices which use more complex
>> - * mappings of media pads.
>> + * an endpoint fwnode. This is useful for devices which use more
>> + * complex mappings of media pads.
>> *
>> * If the entity does not implement the get_fwnode_pad() operation
>> - * then this function searches the entity for the first pad that
>> - * matches the @direction_flags.
>> + * then this function first assumes the entity implements a 1:1 mapping
>> + * between fwnode port number and media pad index. If the 1:1 mapping
>> + * is not valid, then the function searches the entity for the first pad
>> + * that matches the @direction_flags.
>> *
>> * Return: returns the pad number on success or a negative error code.
>> */
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 03/23] media: entity: Convert media_entity_get_fwnode_pad() args to const
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 01/23] media: entity: Pass entity to get_fwnode_pad operation Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 02/23] media: entity: Modify default behavior of media_entity_get_fwnode_pad Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 04/23] media: Move v4l2_fwnode_parse_link from v4l2 to driver base Steve Longerbeam
` (21 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
The function media_entity_get_fwnode_pad() can be passed the
const local_fwnode member from a struct fwnode_endpoint, so
the fwnode argument should be a const pointer. Change the
direction_flags argument to const in the process.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/media/mc/mc-entity.c | 4 ++--
include/media/media-entity.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 47a39d9383d8..e9e090244fd4 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -366,8 +366,8 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph)
EXPORT_SYMBOL_GPL(media_graph_walk_next);
int media_entity_get_fwnode_pad(struct media_entity *entity,
- struct fwnode_handle *fwnode,
- unsigned long direction_flags)
+ const struct fwnode_handle *fwnode,
+ const unsigned long direction_flags)
{
struct fwnode_endpoint endpoint;
int ret;
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index ed00adb4313b..de7fc3676b5a 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -886,8 +886,8 @@ struct media_pad *media_entity_remote_pad(const struct media_pad *pad);
* Return: returns the pad number on success or a negative error code.
*/
int media_entity_get_fwnode_pad(struct media_entity *entity,
- struct fwnode_handle *fwnode,
- unsigned long direction_flags);
+ const struct fwnode_handle *fwnode,
+ const unsigned long direction_flags);
/**
* media_graph_walk_init - Allocate resources used by graph walk.
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 04/23] media: Move v4l2_fwnode_parse_link from v4l2 to driver base
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (2 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 03/23] media: entity: Convert media_entity_get_fwnode_pad() args to const Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 05/23] media: entity: Add functions to convert fwnode endpoints to media links Steve Longerbeam
` (20 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
There is nothing v4l2-specific about v4l2_fwnode_{parse|put}_link().
Make these functions more generally available by moving them to driver
base, with the appropriate name changes to the functions and struct.
In the process embed a 'struct fwnode_endpoint' in 'struct fwnode_link'
for both sides of the link, and make use of fwnode_graph_parse_endpoint()
to fully parse both endpoints. Rename members local_node and
remote_node to more descriptive local_port_parent and
remote_port_parent.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/base/property.c | 63 +++++++++++++++++++
drivers/media/platform/xilinx/xilinx-vipp.c | 69 +++++++++++----------
drivers/media/v4l2-core/v4l2-fwnode.c | 39 ------------
drivers/staging/media/imx/imx-media-of.c | 49 +++++++--------
include/linux/fwnode.h | 14 +++++
include/linux/property.h | 5 ++
include/media/v4l2-fwnode.h | 44 -------------
7 files changed, 141 insertions(+), 142 deletions(-)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 81bd01ed4042..dd82cd150d84 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1100,6 +1100,69 @@ int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
}
EXPORT_SYMBOL(fwnode_graph_parse_endpoint);
+/**
+ * fwnode_graph_parse_link() - parse a link between two endpoints
+ * @local_endpoint: the endpoint's fwnode at the local end of the link
+ * @link: pointer to the fwnode link data structure
+ *
+ * Fill the link structure with the parsed local and remote endpoint info
+ * and the local and remote port parent nodes.
+ *
+ * A reference is taken to both the local and remote port parent nodes,
+ * the caller must use fwnode_graph_put_link() to drop the references
+ * when done with the link.
+ *
+ * Return: 0 on success, or -ENOLINK if the remote endpoint fwnode
+ * can't be found.
+ */
+int fwnode_graph_parse_link(struct fwnode_handle *local_endpoint,
+ struct fwnode_link *link)
+{
+ struct fwnode_handle *remote_endpoint;
+ int ret;
+
+ memset(link, 0, sizeof(*link));
+
+ ret = fwnode_graph_parse_endpoint(local_endpoint, &link->local);
+ if (ret < 0)
+ return ret;
+
+ remote_endpoint = fwnode_graph_get_remote_endpoint(local_endpoint);
+ if (!remote_endpoint)
+ return -ENOLINK;
+
+ ret = fwnode_graph_parse_endpoint(remote_endpoint, &link->remote);
+ if (ret < 0) {
+ fwnode_handle_put(remote_endpoint);
+ return ret;
+ }
+
+ link->local_port_parent =
+ fwnode_graph_get_port_parent(local_endpoint);
+ link->remote_port_parent =
+ fwnode_graph_get_port_parent(remote_endpoint);
+
+ fwnode_handle_put(remote_endpoint);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(fwnode_graph_parse_link);
+
+/**
+ * fwnode_graph_put_link() - drop references to port parent nodes in a link
+ * @link: pointer to the fwnode link data structure
+ *
+ * Drop references to the local and remote port parent nodes in the link.
+ * This function must be called on every link parsed with
+ * fwnode_graph_parse_link().
+ */
+void fwnode_graph_put_link(struct fwnode_link *link)
+{
+ fwnode_handle_put(link->local_port_parent);
+ fwnode_handle_put(link->remote_port_parent);
+}
+EXPORT_SYMBOL_GPL(fwnode_graph_put_link);
+
const void *device_get_match_data(struct device *dev)
{
return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev);
diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c
index cc2856efea59..9c0dfc694478 100644
--- a/drivers/media/platform/xilinx/xilinx-vipp.c
+++ b/drivers/media/platform/xilinx/xilinx-vipp.c
@@ -74,7 +74,7 @@ static int xvip_graph_build_one(struct xvip_composite_device *xdev,
struct media_pad *local_pad;
struct media_pad *remote_pad;
struct xvip_graph_entity *ent;
- struct v4l2_fwnode_link link;
+ struct fwnode_link link;
struct fwnode_handle *ep = NULL;
int ret = 0;
@@ -89,7 +89,7 @@ static int xvip_graph_build_one(struct xvip_composite_device *xdev,
dev_dbg(xdev->dev, "processing endpoint %p\n", ep);
- ret = v4l2_fwnode_parse_link(ep, &link);
+ ret = fwnode_graph_parse_link(ep, &link);
if (ret < 0) {
dev_err(xdev->dev, "failed to parse link for %p\n",
ep);
@@ -99,54 +99,55 @@ static int xvip_graph_build_one(struct xvip_composite_device *xdev,
/* Skip sink ports, they will be processed from the other end of
* the link.
*/
- if (link.local_port >= local->num_pads) {
+ if (link.local.port >= local->num_pads) {
dev_err(xdev->dev, "invalid port number %u for %p\n",
- link.local_port, link.local_node);
- v4l2_fwnode_put_link(&link);
+ link.local.port, link.local_port_parent);
+ fwnode_graph_put_link(&link);
ret = -EINVAL;
break;
}
- local_pad = &local->pads[link.local_port];
+ local_pad = &local->pads[link.local.port];
if (local_pad->flags & MEDIA_PAD_FL_SINK) {
dev_dbg(xdev->dev, "skipping sink port %p:%u\n",
- link.local_node, link.local_port);
- v4l2_fwnode_put_link(&link);
+ link.local_port_parent, link.local.port);
+ fwnode_graph_put_link(&link);
continue;
}
/* Skip DMA engines, they will be processed separately. */
- if (link.remote_node == of_fwnode_handle(xdev->dev->of_node)) {
+ if (link.remote_port_parent ==
+ of_fwnode_handle(xdev->dev->of_node)) {
dev_dbg(xdev->dev, "skipping DMA port %p:%u\n",
- link.local_node, link.local_port);
- v4l2_fwnode_put_link(&link);
+ link.local_port_parent, link.local.port);
+ fwnode_graph_put_link(&link);
continue;
}
/* Find the remote entity. */
- ent = xvip_graph_find_entity(xdev, link.remote_node);
+ ent = xvip_graph_find_entity(xdev, link.remote_port_parent);
if (ent == NULL) {
dev_err(xdev->dev, "no entity found for %p\n",
- link.remote_node);
- v4l2_fwnode_put_link(&link);
+ link.remote_port_parent);
+ fwnode_graph_put_link(&link);
ret = -ENODEV;
break;
}
remote = ent->entity;
- if (link.remote_port >= remote->num_pads) {
+ if (link.remote.port >= remote->num_pads) {
dev_err(xdev->dev, "invalid port number %u on %p\n",
- link.remote_port, link.remote_node);
- v4l2_fwnode_put_link(&link);
+ link.remote.port, link.remote_port_parent);
+ fwnode_graph_put_link(&link);
ret = -EINVAL;
break;
}
- remote_pad = &remote->pads[link.remote_port];
+ remote_pad = &remote->pads[link.remote.port];
- v4l2_fwnode_put_link(&link);
+ fwnode_graph_put_link(&link);
/* Create the media link. */
dev_dbg(xdev->dev, "creating %s:%u -> %s:%u link\n",
@@ -191,7 +192,7 @@ static int xvip_graph_build_dma(struct xvip_composite_device *xdev)
struct media_pad *source_pad;
struct media_pad *sink_pad;
struct xvip_graph_entity *ent;
- struct v4l2_fwnode_link link;
+ struct fwnode_link link;
struct device_node *ep = NULL;
struct xvip_dma *dma;
int ret = 0;
@@ -206,7 +207,7 @@ static int xvip_graph_build_dma(struct xvip_composite_device *xdev)
dev_dbg(xdev->dev, "processing endpoint %pOF\n", ep);
- ret = v4l2_fwnode_parse_link(of_fwnode_handle(ep), &link);
+ ret = fwnode_graph_parse_link(of_fwnode_handle(ep), &link);
if (ret < 0) {
dev_err(xdev->dev, "failed to parse link for %pOF\n",
ep);
@@ -214,11 +215,11 @@ static int xvip_graph_build_dma(struct xvip_composite_device *xdev)
}
/* Find the DMA engine. */
- dma = xvip_graph_find_dma(xdev, link.local_port);
+ dma = xvip_graph_find_dma(xdev, link.local.port);
if (dma == NULL) {
dev_err(xdev->dev, "no DMA engine found for port %u\n",
- link.local_port);
- v4l2_fwnode_put_link(&link);
+ link.local.port);
+ fwnode_graph_put_link(&link);
ret = -EINVAL;
break;
}
@@ -227,20 +228,20 @@ static int xvip_graph_build_dma(struct xvip_composite_device *xdev)
dma->video.name);
/* Find the remote entity. */
- ent = xvip_graph_find_entity(xdev, link.remote_node);
+ ent = xvip_graph_find_entity(xdev, link.remote_port_parent);
if (ent == NULL) {
dev_err(xdev->dev, "no entity found for %pOF\n",
- to_of_node(link.remote_node));
- v4l2_fwnode_put_link(&link);
+ to_of_node(link.remote_port_parent));
+ fwnode_graph_put_link(&link);
ret = -ENODEV;
break;
}
- if (link.remote_port >= ent->entity->num_pads) {
+ if (link.remote.port >= ent->entity->num_pads) {
dev_err(xdev->dev, "invalid port number %u on %pOF\n",
- link.remote_port,
- to_of_node(link.remote_node));
- v4l2_fwnode_put_link(&link);
+ link.remote.port,
+ to_of_node(link.remote_port_parent));
+ fwnode_graph_put_link(&link);
ret = -EINVAL;
break;
}
@@ -249,15 +250,15 @@ static int xvip_graph_build_dma(struct xvip_composite_device *xdev)
source = &dma->video.entity;
source_pad = &dma->pad;
sink = ent->entity;
- sink_pad = &sink->pads[link.remote_port];
+ sink_pad = &sink->pads[link.remote.port];
} else {
source = ent->entity;
- source_pad = &source->pads[link.remote_port];
+ source_pad = &source->pads[link.remote.port];
sink = &dma->video.entity;
sink_pad = &dma->pad;
}
- v4l2_fwnode_put_link(&link);
+ fwnode_graph_put_link(&link);
/* Create the media link. */
dev_dbg(xdev->dev, "creating %s:%u -> %s:%u link\n",
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 192cac076761..f43f563f9e98 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -557,45 +557,6 @@ int v4l2_fwnode_endpoint_alloc_parse(struct fwnode_handle *fwnode,
}
EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_alloc_parse);
-int v4l2_fwnode_parse_link(struct fwnode_handle *__fwnode,
- struct v4l2_fwnode_link *link)
-{
- const char *port_prop = is_of_node(__fwnode) ? "reg" : "port";
- struct fwnode_handle *fwnode;
-
- memset(link, 0, sizeof(*link));
-
- fwnode = fwnode_get_parent(__fwnode);
- fwnode_property_read_u32(fwnode, port_prop, &link->local_port);
- fwnode = fwnode_get_next_parent(fwnode);
- if (is_of_node(fwnode) && of_node_name_eq(to_of_node(fwnode), "ports"))
- fwnode = fwnode_get_next_parent(fwnode);
- link->local_node = fwnode;
-
- fwnode = fwnode_graph_get_remote_endpoint(__fwnode);
- if (!fwnode) {
- fwnode_handle_put(fwnode);
- return -ENOLINK;
- }
-
- fwnode = fwnode_get_parent(fwnode);
- fwnode_property_read_u32(fwnode, port_prop, &link->remote_port);
- fwnode = fwnode_get_next_parent(fwnode);
- if (is_of_node(fwnode) && of_node_name_eq(to_of_node(fwnode), "ports"))
- fwnode = fwnode_get_next_parent(fwnode);
- link->remote_node = fwnode;
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(v4l2_fwnode_parse_link);
-
-void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link)
-{
- fwnode_handle_put(link->local_node);
- fwnode_handle_put(link->remote_node);
-}
-EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);
-
static int
v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev,
struct v4l2_async_notifier *notifier,
diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c
index 2d3efd2a6dde..736c954a8ff5 100644
--- a/drivers/staging/media/imx/imx-media-of.c
+++ b/drivers/staging/media/imx/imx-media-of.c
@@ -84,28 +84,29 @@ EXPORT_SYMBOL_GPL(imx_media_add_of_subdevs);
*/
static int create_of_link(struct imx_media_dev *imxmd,
struct v4l2_subdev *sd,
- struct v4l2_fwnode_link *link)
+ struct fwnode_link *link)
{
struct v4l2_subdev *remote, *src, *sink;
int src_pad, sink_pad;
- if (link->local_port >= sd->entity.num_pads)
+ if (link->local.port >= sd->entity.num_pads)
return -EINVAL;
- remote = imx_media_find_subdev_by_fwnode(imxmd, link->remote_node);
+ remote = imx_media_find_subdev_by_fwnode(imxmd,
+ link->remote_port_parent);
if (!remote)
return 0;
- if (sd->entity.pads[link->local_port].flags & MEDIA_PAD_FL_SINK) {
+ if (sd->entity.pads[link->local.port].flags & MEDIA_PAD_FL_SINK) {
src = remote;
- src_pad = link->remote_port;
+ src_pad = link->remote.port;
sink = sd;
- sink_pad = link->local_port;
+ sink_pad = link->local.port;
} else {
src = sd;
- src_pad = link->local_port;
+ src_pad = link->local.port;
sink = remote;
- sink_pad = link->remote_port;
+ sink_pad = link->remote.port;
}
/* make sure link doesn't already exist before creating */
@@ -126,17 +127,17 @@ static int create_of_link(struct imx_media_dev *imxmd,
int imx_media_create_of_links(struct imx_media_dev *imxmd,
struct v4l2_subdev *sd)
{
- struct v4l2_fwnode_link link;
- struct device_node *ep;
+ struct fwnode_handle *endpoint;
+ struct fwnode_link link;
int ret;
- for_each_endpoint_of_node(sd->dev->of_node, ep) {
- ret = v4l2_fwnode_parse_link(of_fwnode_handle(ep), &link);
+ fwnode_graph_for_each_endpoint(dev_fwnode(sd->dev), endpoint) {
+ ret = fwnode_graph_parse_link(endpoint, &link);
if (ret)
continue;
ret = create_of_link(imxmd, sd, &link);
- v4l2_fwnode_put_link(&link);
+ fwnode_graph_put_link(&link);
if (ret)
return ret;
}
@@ -152,35 +153,33 @@ EXPORT_SYMBOL_GPL(imx_media_create_of_links);
int imx_media_create_csi_of_links(struct imx_media_dev *imxmd,
struct v4l2_subdev *csi)
{
- struct device_node *csi_np = csi->dev->of_node;
- struct device_node *ep;
+ struct fwnode_handle *csi_np = dev_fwnode(csi->dev);
+ struct fwnode_handle *csi_ep;
- for_each_child_of_node(csi_np, ep) {
- struct fwnode_handle *fwnode, *csi_ep;
- struct v4l2_fwnode_link link;
+ fwnode_for_each_child_node(csi_np, csi_ep) {
+ struct fwnode_handle *fwnode;
+ struct fwnode_link link;
int ret;
memset(&link, 0, sizeof(link));
- link.local_node = of_fwnode_handle(csi_np);
- link.local_port = CSI_SINK_PAD;
-
- csi_ep = of_fwnode_handle(ep);
+ link.local_port_parent = csi_np;
+ link.local.port = CSI_SINK_PAD;
fwnode = fwnode_graph_get_remote_endpoint(csi_ep);
if (!fwnode)
continue;
fwnode = fwnode_get_parent(fwnode);
- fwnode_property_read_u32(fwnode, "reg", &link.remote_port);
+ fwnode_property_read_u32(fwnode, "reg", &link.remote.port);
fwnode = fwnode_get_next_parent(fwnode);
if (is_of_node(fwnode) &&
of_node_name_eq(to_of_node(fwnode), "ports"))
fwnode = fwnode_get_next_parent(fwnode);
- link.remote_node = fwnode;
+ link.remote_port_parent = fwnode;
ret = create_of_link(imxmd, csi, &link);
- fwnode_handle_put(link.remote_node);
+ fwnode_handle_put(link.remote_port_parent);
if (ret)
return ret;
}
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index ababd6bc82f3..7bafd01b28a4 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -31,6 +31,20 @@ struct fwnode_endpoint {
const struct fwnode_handle *local_fwnode;
};
+/**
+ * struct fwnode_link - a link between two fwnode graph endpoints
+ * @local: parsed local endpoint of the link
+ * @local_port_parent: port parent fwnode of local endpoint
+ * @remote: parsed remote endpoint of the link
+ * @remote_port_parent: port parent fwnode of the remote endpoint
+ */
+struct fwnode_link {
+ struct fwnode_endpoint local;
+ struct fwnode_handle *local_port_parent;
+ struct fwnode_endpoint remote;
+ struct fwnode_handle *remote_port_parent;
+};
+
#define NR_FWNODE_REFERENCE_ARGS 8
/**
diff --git a/include/linux/property.h b/include/linux/property.h
index 9b3d4ca3a73a..1dd5a939bab7 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -374,6 +374,11 @@ fwnode_graph_get_endpoint_by_id(const struct fwnode_handle *fwnode,
int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
struct fwnode_endpoint *endpoint);
+int fwnode_graph_parse_link(struct fwnode_handle *fwnode,
+ struct fwnode_link *link);
+
+void fwnode_graph_put_link(struct fwnode_link *link);
+
/* -------------------------------------------------------------------------- */
/* Software fwnode support - when HW description is incomplete or missing */
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index f6a7bcd13197..f81f8bf34526 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -109,20 +109,6 @@ struct v4l2_fwnode_endpoint {
unsigned int nr_of_link_frequencies;
};
-/**
- * struct v4l2_fwnode_link - a link between two endpoints
- * @local_node: pointer to device_node of this endpoint
- * @local_port: identifier of the port this endpoint belongs to
- * @remote_node: pointer to device_node of the remote endpoint
- * @remote_port: identifier of the port the remote endpoint belongs to
- */
-struct v4l2_fwnode_link {
- struct fwnode_handle *local_node;
- unsigned int local_port;
- struct fwnode_handle *remote_node;
- unsigned int remote_port;
-};
-
/**
* v4l2_fwnode_endpoint_parse() - parse all fwnode node properties
* @fwnode: pointer to the endpoint's fwnode handle
@@ -203,36 +189,6 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);
int v4l2_fwnode_endpoint_alloc_parse(struct fwnode_handle *fwnode,
struct v4l2_fwnode_endpoint *vep);
-/**
- * v4l2_fwnode_parse_link() - parse a link between two endpoints
- * @fwnode: pointer to the endpoint's fwnode at the local end of the link
- * @link: pointer to the V4L2 fwnode link data structure
- *
- * Fill the link structure with the local and remote nodes and port numbers.
- * The local_node and remote_node fields are set to point to the local and
- * remote port's parent nodes respectively (the port parent node being the
- * parent node of the port node if that node isn't a 'ports' node, or the
- * grand-parent node of the port node otherwise).
- *
- * A reference is taken to both the local and remote nodes, the caller must use
- * v4l2_fwnode_put_link() to drop the references when done with the
- * link.
- *
- * Return: 0 on success, or -ENOLINK if the remote endpoint fwnode can't be
- * found.
- */
-int v4l2_fwnode_parse_link(struct fwnode_handle *fwnode,
- struct v4l2_fwnode_link *link);
-
-/**
- * v4l2_fwnode_put_link() - drop references to nodes in a link
- * @link: pointer to the V4L2 fwnode link data structure
- *
- * Drop references to the local and remote nodes in the link. This function
- * must be called on every link parsed with v4l2_fwnode_parse_link().
- */
-void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);
-
/**
* typedef parse_endpoint_func - Driver's callback function to be called on
* each V4L2 fwnode endpoint.
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 05/23] media: entity: Add functions to convert fwnode endpoints to media links
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (3 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 04/23] media: Move v4l2_fwnode_parse_link from v4l2 to driver base Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-28 13:46 ` Jacopo Mondi
2019-11-24 19:06 ` [PATCH v2 06/23] media: adv748x: csi2: Implement get_fwnode_pad Steve Longerbeam
` (19 subsequent siblings)
24 siblings, 1 reply; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Adds two functions:
media_create_fwnode_pad_links(), which converts fwnode endpoints that
connect a local pad to all pads on a remote entity into media links.
media_create_fwnode_links(), which converts fwnode endpoints that
connect all pads from a local entity to all pads on a remote entity into
media links.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
Changes in v2:
- fixed/improved the prototype descriptions.
---
drivers/media/mc/mc-entity.c | 178 +++++++++++++++++++++++++++++++++++
include/media/media-entity.h | 71 ++++++++++++++
2 files changed, 249 insertions(+)
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index e9e090244fd4..45bbd6c91019 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -787,6 +787,184 @@ int media_create_pad_links(const struct media_device *mdev,
}
EXPORT_SYMBOL_GPL(media_create_pad_links);
+static int __media_create_fwnode_pad_link(struct media_pad *local_pad,
+ struct media_entity *remote,
+ const struct fwnode_handle *remote_ep,
+ const u32 flags)
+{
+ struct media_entity *local = local_pad->entity;
+ unsigned long local_dir = local_pad->flags;
+ unsigned long remote_dir = (local_dir & MEDIA_PAD_FL_SOURCE) ?
+ MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
+ struct media_entity *src, *sink;
+ int src_pad, sink_pad;
+ int remote_pad;
+ int ret;
+
+ remote_pad = media_entity_get_fwnode_pad(remote, remote_ep,
+ remote_dir);
+ if (remote_pad < 0)
+ return 0;
+
+ if (local_dir & MEDIA_PAD_FL_SOURCE) {
+ src = local;
+ src_pad = local_pad->index;
+ sink = remote;
+ sink_pad = remote_pad;
+ } else {
+ src = remote;
+ src_pad = remote_pad;
+ sink = local;
+ sink_pad = local_pad->index;
+ }
+
+ /* make sure link doesn't already exist */
+ if (media_entity_find_link(&src->pads[src_pad],
+ &sink->pads[sink_pad]))
+ return 0;
+
+ ret = media_create_pad_link(src, src_pad, sink, sink_pad, flags);
+ if (ret) {
+ dev_dbg(sink->graph_obj.mdev->dev,
+ "%s:%d -> %s:%d failed with %d\n",
+ src->name, src_pad, sink->name, sink_pad,
+ ret);
+ return ret;
+ }
+
+ dev_dbg(sink->graph_obj.mdev->dev, "%s:%d -> %s:%d\n",
+ src->name, src_pad, sink->name, sink_pad);
+
+ return 0;
+}
+
+int __media_create_fwnode_pad_links(struct media_pad *local_pad,
+ const struct fwnode_handle *local_fwnode,
+ struct media_entity *remote,
+ const struct fwnode_handle *remote_fwnode,
+ const u32 link_flags)
+{
+ struct fwnode_handle *endpoint;
+
+ fwnode_graph_for_each_endpoint(remote_fwnode, endpoint) {
+ struct fwnode_link link;
+ int ret;
+
+ ret = fwnode_graph_parse_link(endpoint, &link);
+ if (ret)
+ continue;
+
+ /*
+ * if this endpoint does not link to the local fwnode
+ * device, ignore it and continue.
+ */
+ if (link.remote_port_parent != local_fwnode) {
+ fwnode_graph_put_link(&link);
+ continue;
+ }
+
+ ret = __media_create_fwnode_pad_link(local_pad,
+ remote, endpoint,
+ link_flags);
+
+ fwnode_graph_put_link(&link);
+
+ if (ret) {
+ fwnode_handle_put(endpoint);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(__media_create_fwnode_pad_links);
+
+int media_create_fwnode_pad_links(struct media_pad *local_pad,
+ const struct fwnode_handle *local_fwnode,
+ struct media_entity *remote,
+ const struct fwnode_handle *remote_fwnode,
+ const u32 link_flags)
+{
+ struct media_device *mdev = local_pad->entity->graph_obj.mdev;
+ int ret;
+
+ mutex_lock(&mdev->graph_mutex);
+ ret = __media_create_fwnode_pad_links(local_pad, local_fwnode,
+ remote, remote_fwnode,
+ link_flags);
+ mutex_unlock(&mdev->graph_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(media_create_fwnode_pad_links);
+
+int __media_create_fwnode_links(struct media_entity *local,
+ const struct fwnode_handle *local_fwnode,
+ struct media_entity *remote,
+ const struct fwnode_handle *remote_fwnode,
+ const u32 link_flags)
+{
+ struct fwnode_handle *endpoint;
+
+ fwnode_graph_for_each_endpoint(local_fwnode, endpoint) {
+ struct fwnode_link link;
+ int local_pad;
+ int ret;
+
+ local_pad = media_entity_get_fwnode_pad(local, endpoint,
+ MEDIA_PAD_FL_SINK |
+ MEDIA_PAD_FL_SOURCE);
+ if (local_pad < 0)
+ continue;
+
+ ret = fwnode_graph_parse_link(endpoint, &link);
+ if (ret)
+ continue;
+
+ /*
+ * if this endpoint does not link to the remote fwnode
+ * device, ignore it and continue.
+ */
+ if (link.remote_port_parent != remote_fwnode) {
+ fwnode_graph_put_link(&link);
+ continue;
+ }
+
+ ret = __media_create_fwnode_pad_link(&local->pads[local_pad],
+ remote,
+ link.remote.local_fwnode,
+ link_flags);
+
+ fwnode_graph_put_link(&link);
+
+ if (ret) {
+ fwnode_handle_put(endpoint);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(__media_create_fwnode_links);
+
+int media_create_fwnode_links(struct media_entity *local,
+ const struct fwnode_handle *local_fwnode,
+ struct media_entity *remote,
+ const struct fwnode_handle *remote_fwnode,
+ const u32 link_flags)
+{
+ struct media_device *mdev = local->graph_obj.mdev;
+ int ret;
+
+ mutex_lock(&mdev->graph_mutex);
+ ret = __media_create_fwnode_links(local, local_fwnode,
+ remote, remote_fwnode, link_flags);
+ mutex_unlock(&mdev->graph_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(media_create_fwnode_links);
+
void __media_entity_remove_links(struct media_entity *entity)
{
struct media_link *link, *tmp;
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index de7fc3676b5a..100673ad83c4 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -772,6 +772,77 @@ int media_create_pad_links(const struct media_device *mdev,
u32 flags,
const bool allow_both_undefined);
+/**
+ * media_create_fwnode_pad_links() - create links between a single local pad
+ * and a remote entity, using the fwnode endpoints
+ * between them.
+ *
+ * @local_pad: Pointer to &media_pad of the local media pad.
+ * @local_fwnode: Pointer to the local device's firmware node.
+ * @remote: Pointer to &media_entity of the remote device.
+ * @remote_fwnode: Pointer to the remote device's firmware node.
+ * @link_flags: Link flags, as defined in include/uapi/linux/media.h.
+ *
+ * .. note::
+ *
+ * Before calling this function, media_entity_pads_init() and
+ * media_device_register_entity() should be called previously for
+ * both entities to be linked.
+ *
+ * Locked (via the mdev graph_mutex) and unlocked versions of this
+ * function are provided. If this function is called from v4l2-async
+ * notifier bound handlers, the locked version should be used to
+ * prevent races with other subdevices loading and binding to their
+ * notifiers in parallel. The unlocked version can for example be
+ * called from v4l2-async notifier complete handlers, after all
+ * subdevices have loaded and bound.
+ */
+int __media_create_fwnode_pad_links(struct media_pad *local_pad,
+ const struct fwnode_handle *local_fwnode,
+ struct media_entity *remote,
+ const struct fwnode_handle *remote_fwnode,
+ const u32 link_flags);
+int media_create_fwnode_pad_links(struct media_pad *local_pad,
+ const struct fwnode_handle *local_fwnode,
+ struct media_entity *remote,
+ const struct fwnode_handle *remote_fwnode,
+ const u32 link_flags);
+
+/**
+ * media_create_fwnode_links() - create links between two entities, using
+ * the fwnode endpoints between them.
+ *
+ * @local: Pointer to &media_entity of the local device.
+ * @local_fwnode: Pointer to the local device's firmware node.
+ * @remote: Pointer to &media_entity of the remote device.
+ * @remote_fwnode: Pointer to the remote device's firmware node.
+ * @link_flags: Link flags, as defined in include/uapi/linux/media.h.
+ *
+ * .. note::
+ *
+ * Before calling this function, media_entity_pads_init() and
+ * media_device_register_entity() should be called previously for
+ * both entities to be linked.
+ *
+ * Locked (via the mdev graph_mutex) and unlocked versions of this
+ * function are provided. If this function is called from v4l2-async
+ * notifier bound handlers, the locked version should be used to
+ * prevent races with other subdevices loading and binding to their
+ * notifiers in parallel. The unlocked version can for example be
+ * called from v4l2-async notifier complete handlers, after all
+ * subdevices have loaded and bound.
+ */
+int __media_create_fwnode_links(struct media_entity *local,
+ const struct fwnode_handle *local_fwnode,
+ struct media_entity *remote,
+ const struct fwnode_handle *remote_fwnode,
+ const u32 link_flags);
+int media_create_fwnode_links(struct media_entity *local,
+ const struct fwnode_handle *local_fwnode,
+ struct media_entity *remote,
+ const struct fwnode_handle *remote_fwnode,
+ const u32 link_flags);
+
void __media_entity_remove_links(struct media_entity *entity);
/**
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v2 05/23] media: entity: Add functions to convert fwnode endpoints to media links
2019-11-24 19:06 ` [PATCH v2 05/23] media: entity: Add functions to convert fwnode endpoints to media links Steve Longerbeam
@ 2019-11-28 13:46 ` Jacopo Mondi
2019-12-14 21:31 ` Steve Longerbeam
0 siblings, 1 reply; 38+ messages in thread
From: Jacopo Mondi @ 2019-11-28 13:46 UTC (permalink / raw)
To: Steve Longerbeam; +Cc: linux-media
[-- Attachment #1: Type: text/plain, Size: 10708 bytes --]
Hi Steve,
On Sun, Nov 24, 2019 at 11:06:45AM -0800, Steve Longerbeam wrote:
> Adds two functions:
>
> media_create_fwnode_pad_links(), which converts fwnode endpoints that
> connect a local pad to all pads on a remote entity into media links.
>
> media_create_fwnode_links(), which converts fwnode endpoints that
> connect all pads from a local entity to all pads on a remote entity into
> media links.
>
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
> Changes in v2:
> - fixed/improved the prototype descriptions.
> ---
> drivers/media/mc/mc-entity.c | 178 +++++++++++++++++++++++++++++++++++
> include/media/media-entity.h | 71 ++++++++++++++
> 2 files changed, 249 insertions(+)
>
> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> index e9e090244fd4..45bbd6c91019 100644
> --- a/drivers/media/mc/mc-entity.c
> +++ b/drivers/media/mc/mc-entity.c
> @@ -787,6 +787,184 @@ int media_create_pad_links(const struct media_device *mdev,
> }
> EXPORT_SYMBOL_GPL(media_create_pad_links);
>
> +static int __media_create_fwnode_pad_link(struct media_pad *local_pad,
> + struct media_entity *remote,
> + const struct fwnode_handle *remote_ep,
> + const u32 flags)
> +{
> + struct media_entity *local = local_pad->entity;
> + unsigned long local_dir = local_pad->flags;
> + unsigned long remote_dir = (local_dir & MEDIA_PAD_FL_SOURCE) ?
> + MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
> + struct media_entity *src, *sink;
> + int src_pad, sink_pad;
> + int remote_pad;
> + int ret;
> +
> + remote_pad = media_entity_get_fwnode_pad(remote, remote_ep,
> + remote_dir);
> + if (remote_pad < 0)
> + return 0;
> +
> + if (local_dir & MEDIA_PAD_FL_SOURCE) {
> + src = local;
> + src_pad = local_pad->index;
> + sink = remote;
> + sink_pad = remote_pad;
> + } else {
> + src = remote;
> + src_pad = remote_pad;
> + sink = local;
> + sink_pad = local_pad->index;
> + }
> +
> + /* make sure link doesn't already exist */
> + if (media_entity_find_link(&src->pads[src_pad],
> + &sink->pads[sink_pad]))
> + return 0;
> +
> + ret = media_create_pad_link(src, src_pad, sink, sink_pad, flags);
> + if (ret) {
> + dev_dbg(sink->graph_obj.mdev->dev,
> + "%s:%d -> %s:%d failed with %d\n",
> + src->name, src_pad, sink->name, sink_pad,
> + ret);
> + return ret;
> + }
> +
> + dev_dbg(sink->graph_obj.mdev->dev, "%s:%d -> %s:%d\n",
> + src->name, src_pad, sink->name, sink_pad);
> +
> + return 0;
> +}
> +
> +int __media_create_fwnode_pad_links(struct media_pad *local_pad,
> + const struct fwnode_handle *local_fwnode,
> + struct media_entity *remote,
> + const struct fwnode_handle *remote_fwnode,
> + const u32 link_flags)
> +{
> + struct fwnode_handle *endpoint;
> +
> + fwnode_graph_for_each_endpoint(remote_fwnode, endpoint) {
> + struct fwnode_link link;
> + int ret;
> +
> + ret = fwnode_graph_parse_link(endpoint, &link);
> + if (ret)
> + continue;
> +
> + /*
> + * if this endpoint does not link to the local fwnode
> + * device, ignore it and continue.
> + */
> + if (link.remote_port_parent != local_fwnode) {
> + fwnode_graph_put_link(&link);
> + continue;
> + }
> +
> + ret = __media_create_fwnode_pad_link(local_pad,
> + remote, endpoint,
> + link_flags);
> +
> + fwnode_graph_put_link(&link);
> +
> + if (ret) {
> + fwnode_handle_put(endpoint);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(__media_create_fwnode_pad_links);
> +
> +int media_create_fwnode_pad_links(struct media_pad *local_pad,
> + const struct fwnode_handle *local_fwnode,
> + struct media_entity *remote,
> + const struct fwnode_handle *remote_fwnode,
> + const u32 link_flags)
> +{
> + struct media_device *mdev = local_pad->entity->graph_obj.mdev;
> + int ret;
> +
> + mutex_lock(&mdev->graph_mutex);
> + ret = __media_create_fwnode_pad_links(local_pad, local_fwnode,
> + remote, remote_fwnode,
> + link_flags);
> + mutex_unlock(&mdev->graph_mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(media_create_fwnode_pad_links);
> +
> +int __media_create_fwnode_links(struct media_entity *local,
> + const struct fwnode_handle *local_fwnode,
> + struct media_entity *remote,
> + const struct fwnode_handle *remote_fwnode,
> + const u32 link_flags)
> +{
> + struct fwnode_handle *endpoint;
> +
> + fwnode_graph_for_each_endpoint(local_fwnode, endpoint) {
> + struct fwnode_link link;
> + int local_pad;
> + int ret;
> +
> + local_pad = media_entity_get_fwnode_pad(local, endpoint,
> + MEDIA_PAD_FL_SINK |
> + MEDIA_PAD_FL_SOURCE);
I wonder.. I feel like we could have saved a lot of churn if we record
the local endpoint on which we register an async device, likely in
struct v4l2_async_subdev.
At bound() time we would receive back the local endpoint on which the
just bound subdev was originally registered, we could get the remote
endpoint by parsing the fwnode_graph_link and from there we could
provide utilities like the ones you have here, by saving testing all
endpoints until we don't find one that matches the subdev which got
bound.
This would probably just work for V4L2_ASYNC_MATCH_FWNODE though, but
have you considered this solution ? It would avoid trying all the
local endpoints blindly here and it would encourage drivers to provide
a function to do the endpoint->pad_index translation (which ideally
they should, to avoid workarounds like the ones we have in
media_entity_get_fwnode_pad()
Thanks
j
> + if (local_pad < 0)
> + continue;
> +
> + ret = fwnode_graph_parse_link(endpoint, &link);
> + if (ret)
> + continue;
> +
> + /*
> + * if this endpoint does not link to the remote fwnode
> + * device, ignore it and continue.
> + */
> + if (link.remote_port_parent != remote_fwnode) {
> + fwnode_graph_put_link(&link);
> + continue;
> + }
> +
> + ret = __media_create_fwnode_pad_link(&local->pads[local_pad],
> + remote,
> + link.remote.local_fwnode,
> + link_flags);
> +
> + fwnode_graph_put_link(&link);
> +
> + if (ret) {
> + fwnode_handle_put(endpoint);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(__media_create_fwnode_links);
> +
> +int media_create_fwnode_links(struct media_entity *local,
> + const struct fwnode_handle *local_fwnode,
> + struct media_entity *remote,
> + const struct fwnode_handle *remote_fwnode,
> + const u32 link_flags)
> +{
> + struct media_device *mdev = local->graph_obj.mdev;
> + int ret;
> +
> + mutex_lock(&mdev->graph_mutex);
> + ret = __media_create_fwnode_links(local, local_fwnode,
> + remote, remote_fwnode, link_flags);
> + mutex_unlock(&mdev->graph_mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(media_create_fwnode_links);
> +
> void __media_entity_remove_links(struct media_entity *entity)
> {
> struct media_link *link, *tmp;
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index de7fc3676b5a..100673ad83c4 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -772,6 +772,77 @@ int media_create_pad_links(const struct media_device *mdev,
> u32 flags,
> const bool allow_both_undefined);
>
> +/**
> + * media_create_fwnode_pad_links() - create links between a single local pad
> + * and a remote entity, using the fwnode endpoints
> + * between them.
> + *
> + * @local_pad: Pointer to &media_pad of the local media pad.
> + * @local_fwnode: Pointer to the local device's firmware node.
> + * @remote: Pointer to &media_entity of the remote device.
> + * @remote_fwnode: Pointer to the remote device's firmware node.
> + * @link_flags: Link flags, as defined in include/uapi/linux/media.h.
> + *
> + * .. note::
> + *
> + * Before calling this function, media_entity_pads_init() and
> + * media_device_register_entity() should be called previously for
> + * both entities to be linked.
> + *
> + * Locked (via the mdev graph_mutex) and unlocked versions of this
> + * function are provided. If this function is called from v4l2-async
> + * notifier bound handlers, the locked version should be used to
> + * prevent races with other subdevices loading and binding to their
> + * notifiers in parallel. The unlocked version can for example be
> + * called from v4l2-async notifier complete handlers, after all
> + * subdevices have loaded and bound.
> + */
> +int __media_create_fwnode_pad_links(struct media_pad *local_pad,
> + const struct fwnode_handle *local_fwnode,
> + struct media_entity *remote,
> + const struct fwnode_handle *remote_fwnode,
> + const u32 link_flags);
> +int media_create_fwnode_pad_links(struct media_pad *local_pad,
> + const struct fwnode_handle *local_fwnode,
> + struct media_entity *remote,
> + const struct fwnode_handle *remote_fwnode,
> + const u32 link_flags);
> +
> +/**
> + * media_create_fwnode_links() - create links between two entities, using
> + * the fwnode endpoints between them.
> + *
> + * @local: Pointer to &media_entity of the local device.
> + * @local_fwnode: Pointer to the local device's firmware node.
> + * @remote: Pointer to &media_entity of the remote device.
> + * @remote_fwnode: Pointer to the remote device's firmware node.
> + * @link_flags: Link flags, as defined in include/uapi/linux/media.h.
> + *
> + * .. note::
> + *
> + * Before calling this function, media_entity_pads_init() and
> + * media_device_register_entity() should be called previously for
> + * both entities to be linked.
> + *
> + * Locked (via the mdev graph_mutex) and unlocked versions of this
> + * function are provided. If this function is called from v4l2-async
> + * notifier bound handlers, the locked version should be used to
> + * prevent races with other subdevices loading and binding to their
> + * notifiers in parallel. The unlocked version can for example be
> + * called from v4l2-async notifier complete handlers, after all
> + * subdevices have loaded and bound.
> + */
> +int __media_create_fwnode_links(struct media_entity *local,
> + const struct fwnode_handle *local_fwnode,
> + struct media_entity *remote,
> + const struct fwnode_handle *remote_fwnode,
> + const u32 link_flags);
> +int media_create_fwnode_links(struct media_entity *local,
> + const struct fwnode_handle *local_fwnode,
> + struct media_entity *remote,
> + const struct fwnode_handle *remote_fwnode,
> + const u32 link_flags);
> +
> void __media_entity_remove_links(struct media_entity *entity);
>
> /**
> --
> 2.17.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v2 05/23] media: entity: Add functions to convert fwnode endpoints to media links
2019-11-28 13:46 ` Jacopo Mondi
@ 2019-12-14 21:31 ` Steve Longerbeam
0 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-12-14 21:31 UTC (permalink / raw)
To: Jacopo Mondi; +Cc: linux-media
Hi Jacopo,
On 11/28/19 5:46 AM, Jacopo Mondi wrote:
> Hi Steve,
>
> On Sun, Nov 24, 2019 at 11:06:45AM -0800, Steve Longerbeam wrote:
>> Adds two functions:
>>
>> media_create_fwnode_pad_links(), which converts fwnode endpoints that
>> connect a local pad to all pads on a remote entity into media links.
>>
>> media_create_fwnode_links(), which converts fwnode endpoints that
>> connect all pads from a local entity to all pads on a remote entity into
>> media links.
>>
>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>> ---
>> Changes in v2:
>> - fixed/improved the prototype descriptions.
>> ---
>> drivers/media/mc/mc-entity.c | 178 +++++++++++++++++++++++++++++++++++
>> include/media/media-entity.h | 71 ++++++++++++++
>> 2 files changed, 249 insertions(+)
>>
>> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
>> index e9e090244fd4..45bbd6c91019 100644
>> --- a/drivers/media/mc/mc-entity.c
>> +++ b/drivers/media/mc/mc-entity.c
>> @@ -787,6 +787,184 @@ int media_create_pad_links(const struct media_device *mdev,
>> }
>> EXPORT_SYMBOL_GPL(media_create_pad_links);
>>
>> +static int __media_create_fwnode_pad_link(struct media_pad *local_pad,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_ep,
>> + const u32 flags)
>> +{
>> + struct media_entity *local = local_pad->entity;
>> + unsigned long local_dir = local_pad->flags;
>> + unsigned long remote_dir = (local_dir & MEDIA_PAD_FL_SOURCE) ?
>> + MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
>> + struct media_entity *src, *sink;
>> + int src_pad, sink_pad;
>> + int remote_pad;
>> + int ret;
>> +
>> + remote_pad = media_entity_get_fwnode_pad(remote, remote_ep,
>> + remote_dir);
>> + if (remote_pad < 0)
>> + return 0;
>> +
>> + if (local_dir & MEDIA_PAD_FL_SOURCE) {
>> + src = local;
>> + src_pad = local_pad->index;
>> + sink = remote;
>> + sink_pad = remote_pad;
>> + } else {
>> + src = remote;
>> + src_pad = remote_pad;
>> + sink = local;
>> + sink_pad = local_pad->index;
>> + }
>> +
>> + /* make sure link doesn't already exist */
>> + if (media_entity_find_link(&src->pads[src_pad],
>> + &sink->pads[sink_pad]))
>> + return 0;
>> +
>> + ret = media_create_pad_link(src, src_pad, sink, sink_pad, flags);
>> + if (ret) {
>> + dev_dbg(sink->graph_obj.mdev->dev,
>> + "%s:%d -> %s:%d failed with %d\n",
>> + src->name, src_pad, sink->name, sink_pad,
>> + ret);
>> + return ret;
>> + }
>> +
>> + dev_dbg(sink->graph_obj.mdev->dev, "%s:%d -> %s:%d\n",
>> + src->name, src_pad, sink->name, sink_pad);
>> +
>> + return 0;
>> +}
>> +
>> +int __media_create_fwnode_pad_links(struct media_pad *local_pad,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags)
>> +{
>> + struct fwnode_handle *endpoint;
>> +
>> + fwnode_graph_for_each_endpoint(remote_fwnode, endpoint) {
>> + struct fwnode_link link;
>> + int ret;
>> +
>> + ret = fwnode_graph_parse_link(endpoint, &link);
>> + if (ret)
>> + continue;
>> +
>> + /*
>> + * if this endpoint does not link to the local fwnode
>> + * device, ignore it and continue.
>> + */
>> + if (link.remote_port_parent != local_fwnode) {
>> + fwnode_graph_put_link(&link);
>> + continue;
>> + }
>> +
>> + ret = __media_create_fwnode_pad_link(local_pad,
>> + remote, endpoint,
>> + link_flags);
>> +
>> + fwnode_graph_put_link(&link);
>> +
>> + if (ret) {
>> + fwnode_handle_put(endpoint);
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(__media_create_fwnode_pad_links);
>> +
>> +int media_create_fwnode_pad_links(struct media_pad *local_pad,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags)
>> +{
>> + struct media_device *mdev = local_pad->entity->graph_obj.mdev;
>> + int ret;
>> +
>> + mutex_lock(&mdev->graph_mutex);
>> + ret = __media_create_fwnode_pad_links(local_pad, local_fwnode,
>> + remote, remote_fwnode,
>> + link_flags);
>> + mutex_unlock(&mdev->graph_mutex);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(media_create_fwnode_pad_links);
>> +
>> +int __media_create_fwnode_links(struct media_entity *local,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags)
>> +{
>> + struct fwnode_handle *endpoint;
>> +
>> + fwnode_graph_for_each_endpoint(local_fwnode, endpoint) {
>> + struct fwnode_link link;
>> + int local_pad;
>> + int ret;
>> +
>> + local_pad = media_entity_get_fwnode_pad(local, endpoint,
>> + MEDIA_PAD_FL_SINK |
>> + MEDIA_PAD_FL_SOURCE);
> I wonder.. I feel like we could have saved a lot of churn if we record
> the local endpoint on which we register an async device, likely in
> struct v4l2_async_subdev.
>
> At bound() time we would receive back the local endpoint on which the
> just bound subdev was originally registered, we could get the remote
> endpoint by parsing the fwnode_graph_link and from there we could
> provide utilities like the ones you have here, by saving testing all
> endpoints until we don't find one that matches the subdev which got
> bound.
>
> This would probably just work for V4L2_ASYNC_MATCH_FWNODE though, but
> have you considered this solution ? It would avoid trying all the
> local endpoints blindly here and it would encourage drivers to provide
> a function to do the endpoint->pad_index translation (which ideally
> they should, to avoid workarounds like the ones we have in
> media_entity_get_fwnode_pad()
It sounds like a promising idea, I'll start looking into it.
Steve
> Thanks
> j
>
>> + if (local_pad < 0)
>> + continue;
>> +
>> + ret = fwnode_graph_parse_link(endpoint, &link);
>> + if (ret)
>> + continue;
>> +
>> + /*
>> + * if this endpoint does not link to the remote fwnode
>> + * device, ignore it and continue.
>> + */
>> + if (link.remote_port_parent != remote_fwnode) {
>> + fwnode_graph_put_link(&link);
>> + continue;
>> + }
>> +
>> + ret = __media_create_fwnode_pad_link(&local->pads[local_pad],
>> + remote,
>> + link.remote.local_fwnode,
>> + link_flags);
>> +
>> + fwnode_graph_put_link(&link);
>> +
>> + if (ret) {
>> + fwnode_handle_put(endpoint);
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(__media_create_fwnode_links);
>> +
>> +int media_create_fwnode_links(struct media_entity *local,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags)
>> +{
>> + struct media_device *mdev = local->graph_obj.mdev;
>> + int ret;
>> +
>> + mutex_lock(&mdev->graph_mutex);
>> + ret = __media_create_fwnode_links(local, local_fwnode,
>> + remote, remote_fwnode, link_flags);
>> + mutex_unlock(&mdev->graph_mutex);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(media_create_fwnode_links);
>> +
>> void __media_entity_remove_links(struct media_entity *entity)
>> {
>> struct media_link *link, *tmp;
>> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
>> index de7fc3676b5a..100673ad83c4 100644
>> --- a/include/media/media-entity.h
>> +++ b/include/media/media-entity.h
>> @@ -772,6 +772,77 @@ int media_create_pad_links(const struct media_device *mdev,
>> u32 flags,
>> const bool allow_both_undefined);
>>
>> +/**
>> + * media_create_fwnode_pad_links() - create links between a single local pad
>> + * and a remote entity, using the fwnode endpoints
>> + * between them.
>> + *
>> + * @local_pad: Pointer to &media_pad of the local media pad.
>> + * @local_fwnode: Pointer to the local device's firmware node.
>> + * @remote: Pointer to &media_entity of the remote device.
>> + * @remote_fwnode: Pointer to the remote device's firmware node.
>> + * @link_flags: Link flags, as defined in include/uapi/linux/media.h.
>> + *
>> + * .. note::
>> + *
>> + * Before calling this function, media_entity_pads_init() and
>> + * media_device_register_entity() should be called previously for
>> + * both entities to be linked.
>> + *
>> + * Locked (via the mdev graph_mutex) and unlocked versions of this
>> + * function are provided. If this function is called from v4l2-async
>> + * notifier bound handlers, the locked version should be used to
>> + * prevent races with other subdevices loading and binding to their
>> + * notifiers in parallel. The unlocked version can for example be
>> + * called from v4l2-async notifier complete handlers, after all
>> + * subdevices have loaded and bound.
>> + */
>> +int __media_create_fwnode_pad_links(struct media_pad *local_pad,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags);
>> +int media_create_fwnode_pad_links(struct media_pad *local_pad,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags);
>> +
>> +/**
>> + * media_create_fwnode_links() - create links between two entities, using
>> + * the fwnode endpoints between them.
>> + *
>> + * @local: Pointer to &media_entity of the local device.
>> + * @local_fwnode: Pointer to the local device's firmware node.
>> + * @remote: Pointer to &media_entity of the remote device.
>> + * @remote_fwnode: Pointer to the remote device's firmware node.
>> + * @link_flags: Link flags, as defined in include/uapi/linux/media.h.
>> + *
>> + * .. note::
>> + *
>> + * Before calling this function, media_entity_pads_init() and
>> + * media_device_register_entity() should be called previously for
>> + * both entities to be linked.
>> + *
>> + * Locked (via the mdev graph_mutex) and unlocked versions of this
>> + * function are provided. If this function is called from v4l2-async
>> + * notifier bound handlers, the locked version should be used to
>> + * prevent races with other subdevices loading and binding to their
>> + * notifiers in parallel. The unlocked version can for example be
>> + * called from v4l2-async notifier complete handlers, after all
>> + * subdevices have loaded and bound.
>> + */
>> +int __media_create_fwnode_links(struct media_entity *local,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags);
>> +int media_create_fwnode_links(struct media_entity *local,
>> + const struct fwnode_handle *local_fwnode,
>> + struct media_entity *remote,
>> + const struct fwnode_handle *remote_fwnode,
>> + const u32 link_flags);
>> +
>> void __media_entity_remove_links(struct media_entity *entity);
>>
>> /**
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 06/23] media: adv748x: csi2: Implement get_fwnode_pad
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (4 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 05/23] media: entity: Add functions to convert fwnode endpoints to media links Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-28 11:53 ` Jacopo Mondi
2019-11-24 19:06 ` [PATCH v2 07/23] media: rcar-csi2: Fix fwnode media link creation Steve Longerbeam
` (18 subsequent siblings)
24 siblings, 1 reply; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
If the given endpoint fwnode passed to the .get_fwnode_pad() op is
the adv748x-csi2 TXA/TXB source endpoint, return the associated media
pad index ADV748X_CSI2_SOURCE. The adv748x-csi2 has no other media pads
that are associated with fwnode endpoints.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/media/i2c/adv748x/adv748x-csi2.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
index 2091cda50935..810085a1f2f0 100644
--- a/drivers/media/i2c/adv748x/adv748x-csi2.c
+++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
@@ -228,6 +228,24 @@ static const struct v4l2_subdev_ops adv748x_csi2_ops = {
.pad = &adv748x_csi2_pad_ops,
};
+/* -----------------------------------------------------------------------------
+ * media_entity_operations
+ */
+
+static int adv748x_csi2_get_fwnode_pad(struct media_entity *entity,
+ struct fwnode_endpoint *endpoint)
+{
+ struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+ struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
+
+ return endpoint->local_fwnode == tx->sd.fwnode ?
+ ADV748X_CSI2_SOURCE : -ENXIO;
+}
+
+static const struct media_entity_operations adv748x_csi2_entity_ops = {
+ .get_fwnode_pad = adv748x_csi2_get_fwnode_pad,
+};
+
/* -----------------------------------------------------------------------------
* Subdev module and controls
*/
@@ -295,6 +313,9 @@ int adv748x_csi2_init(struct adv748x_state *state, struct adv748x_csi2 *tx)
/* Register internal ops for incremental subdev registration */
tx->sd.internal_ops = &adv748x_csi2_internal_ops;
+ /* Register media_entity ops */
+ tx->sd.entity.ops = &adv748x_csi2_entity_ops;
+
tx->pads[ADV748X_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
tx->pads[ADV748X_CSI2_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v2 06/23] media: adv748x: csi2: Implement get_fwnode_pad
2019-11-24 19:06 ` [PATCH v2 06/23] media: adv748x: csi2: Implement get_fwnode_pad Steve Longerbeam
@ 2019-11-28 11:53 ` Jacopo Mondi
2019-12-14 20:43 ` Steve Longerbeam
0 siblings, 1 reply; 38+ messages in thread
From: Jacopo Mondi @ 2019-11-28 11:53 UTC (permalink / raw)
To: Steve Longerbeam; +Cc: linux-media
[-- Attachment #1: Type: text/plain, Size: 2370 bytes --]
Hi Steve,
On Sun, Nov 24, 2019 at 11:06:46AM -0800, Steve Longerbeam wrote:
> If the given endpoint fwnode passed to the .get_fwnode_pad() op is
> the adv748x-csi2 TXA/TXB source endpoint, return the associated media
> pad index ADV748X_CSI2_SOURCE. The adv748x-csi2 has no other media pads
> that are associated with fwnode endpoints.
>
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
> drivers/media/i2c/adv748x/adv748x-csi2.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
> index 2091cda50935..810085a1f2f0 100644
> --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
> +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
> @@ -228,6 +228,24 @@ static const struct v4l2_subdev_ops adv748x_csi2_ops = {
> .pad = &adv748x_csi2_pad_ops,
> };
>
> +/* -----------------------------------------------------------------------------
> + * media_entity_operations
> + */
> +
> +static int adv748x_csi2_get_fwnode_pad(struct media_entity *entity,
> + struct fwnode_endpoint *endpoint)
> +{
> + struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> + struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
> +
> + return endpoint->local_fwnode == tx->sd.fwnode ?
> + ADV748X_CSI2_SOURCE : -ENXIO;
Couldn't you check if the endpoint port is either 10 or 11, as those
are the only port numbers that provide a CSI-2 source pad ?
In that case you could drop extending get_fwnode_pad() with th entity
argument, as it is only used here (this one is actually the first user
in the whole code base of this operation)
> +}
> +
> +static const struct media_entity_operations adv748x_csi2_entity_ops = {
> + .get_fwnode_pad = adv748x_csi2_get_fwnode_pad,
> +};
> +
> /* -----------------------------------------------------------------------------
> * Subdev module and controls
> */
> @@ -295,6 +313,9 @@ int adv748x_csi2_init(struct adv748x_state *state, struct adv748x_csi2 *tx)
> /* Register internal ops for incremental subdev registration */
> tx->sd.internal_ops = &adv748x_csi2_internal_ops;
>
> + /* Register media_entity ops */
> + tx->sd.entity.ops = &adv748x_csi2_entity_ops;
> +
> tx->pads[ADV748X_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> tx->pads[ADV748X_CSI2_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
>
> --
> 2.17.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v2 06/23] media: adv748x: csi2: Implement get_fwnode_pad
2019-11-28 11:53 ` Jacopo Mondi
@ 2019-12-14 20:43 ` Steve Longerbeam
0 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-12-14 20:43 UTC (permalink / raw)
To: Jacopo Mondi; +Cc: linux-media
Hi Jacopo,
On 11/28/19 3:53 AM, Jacopo Mondi wrote:
> Hi Steve,
>
> On Sun, Nov 24, 2019 at 11:06:46AM -0800, Steve Longerbeam wrote:
>> If the given endpoint fwnode passed to the .get_fwnode_pad() op is
>> the adv748x-csi2 TXA/TXB source endpoint, return the associated media
>> pad index ADV748X_CSI2_SOURCE. The adv748x-csi2 has no other media pads
>> that are associated with fwnode endpoints.
>>
>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>> ---
>> drivers/media/i2c/adv748x/adv748x-csi2.c | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>>
>> diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
>> index 2091cda50935..810085a1f2f0 100644
>> --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
>> +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
>> @@ -228,6 +228,24 @@ static const struct v4l2_subdev_ops adv748x_csi2_ops = {
>> .pad = &adv748x_csi2_pad_ops,
>> };
>>
>> +/* -----------------------------------------------------------------------------
>> + * media_entity_operations
>> + */
>> +
>> +static int adv748x_csi2_get_fwnode_pad(struct media_entity *entity,
>> + struct fwnode_endpoint *endpoint)
>> +{
>> + struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
>> + struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
>> +
>> + return endpoint->local_fwnode == tx->sd.fwnode ?
>> + ADV748X_CSI2_SOURCE : -ENXIO;
> Couldn't you check if the endpoint port is either 10 or 11, as those
> are the only port numbers that provide a CSI-2 source pad ?
>
> In that case you could drop extending get_fwnode_pad() with th entity
> argument, as it is only used here (this one is actually the first user
> in the whole code base of this operation)
I don't think that's a very good idea. Entities that implement
.get_fwnode_pad() in the future will likely need access to themselves in
order to do the work. This implementation is a good example of that and
is a better approach to checking the port number, because it is actually
verifying that the endpoint fwnode object is owned by this device.
Steve
>
>> +}
>> +
>> +static const struct media_entity_operations adv748x_csi2_entity_ops = {
>> + .get_fwnode_pad = adv748x_csi2_get_fwnode_pad,
>> +};
>> +
>> /* -----------------------------------------------------------------------------
>> * Subdev module and controls
>> */
>> @@ -295,6 +313,9 @@ int adv748x_csi2_init(struct adv748x_state *state, struct adv748x_csi2 *tx)
>> /* Register internal ops for incremental subdev registration */
>> tx->sd.internal_ops = &adv748x_csi2_internal_ops;
>>
>> + /* Register media_entity ops */
>> + tx->sd.entity.ops = &adv748x_csi2_entity_ops;
>> +
>> tx->pads[ADV748X_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
>> tx->pads[ADV748X_CSI2_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
>>
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 07/23] media: rcar-csi2: Fix fwnode media link creation
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (5 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 06/23] media: adv748x: csi2: Implement get_fwnode_pad Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 08/23] media: cadence: csi2rx: " Steve Longerbeam
` (17 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
rcsi2_notify_bound() passes the bound subdev's match fwnode to
media_entity_get_fwnode_pad() to determine the subdev's source
pad for creating the media link to it. When the bound subdev is
the adv748x-csi2 transmitter, this is in fact correctly the endpoint
fwnode. For other subdevices this likely will not be the case, the
asd match fwnode is usually not an endpoint fwnode but rather the
port parent fwnode. So rcar-csi2 will fail to get the correct source
pad for bound subdev's other than the adv748x.
To fix and make rcar-csi2 connect more generally to other subdevices,
replace the calls to media_entity_get_fwnode_pad() and
media_create_pad_link() with a call to media_create_fwnode_pad_links().
Fixes: 769afd212b160 ("media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver")
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/media/platform/rcar-vin/rcar-csi2.c | 23 +++++++++------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index faa9fb23a2e9..df6cb6d91d7e 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -738,23 +738,20 @@ static int rcsi2_notify_bound(struct v4l2_async_notifier *notifier,
struct v4l2_async_subdev *asd)
{
struct rcar_csi2 *priv = notifier_to_csi2(notifier);
- int pad;
+ int ret;
- pad = media_entity_get_fwnode_pad(&subdev->entity, asd->match.fwnode,
- MEDIA_PAD_FL_SOURCE);
- if (pad < 0) {
- dev_err(priv->dev, "Failed to find pad for %s\n", subdev->name);
- return pad;
- }
+ ret = media_create_fwnode_pad_links(&priv->subdev.entity.pads[0],
+ dev_fwnode(priv->dev),
+ &subdev->entity,
+ dev_fwnode(subdev->dev),
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
+ if (ret)
+ return ret;
priv->remote = subdev;
- dev_dbg(priv->dev, "Bound %s pad: %d\n", subdev->name, pad);
-
- return media_create_pad_link(&subdev->entity, pad,
- &priv->subdev.entity, 0,
- MEDIA_LNK_FL_ENABLED |
- MEDIA_LNK_FL_IMMUTABLE);
+ return 0;
}
static void rcsi2_notify_unbind(struct v4l2_async_notifier *notifier,
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 08/23] media: cadence: csi2rx: Fix fwnode media link creation
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (6 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 07/23] media: rcar-csi2: Fix fwnode media link creation Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 09/23] media: sun6i: " Steve Longerbeam
` (16 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
csi2rx_async_bound() passes the bound subdev's sd->fwnode to
media_entity_get_fwnode_pad(). This is likely not an endpoint
fwnode as required by media_entity_get_fwnode_pad(), for most
subdevices it is the port parent of endpoint fwnode(s). This has only
worked before because no entities have implemented the .get_fwnode_pad()
op yet, and the default behavior of media_entity_get_fwnode_pad()
was to ignore the passed fwnode and return the first pad that matches
the given direction flags.
Fix this by replacing the calls to media_entity_get_fwnode_pad() and
media_create_pad_link() with a call to media_create_fwnode_pad_links().
Fixes: 1fc3b37f34f69 ("media: v4l: cadence: Add Cadence MIPI-CSI2 RX driver")
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/media/platform/cadence/cdns-csi2rx.c | 27 ++++++++------------
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index be9ec59774d6..d79345820225 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -83,7 +83,6 @@ struct csi2rx_priv {
/* Remote source */
struct v4l2_async_subdev asd;
struct v4l2_subdev *source_subdev;
- int source_pad;
};
static inline
@@ -251,26 +250,20 @@ static int csi2rx_async_bound(struct v4l2_async_notifier *notifier,
{
struct v4l2_subdev *subdev = notifier->sd;
struct csi2rx_priv *csi2rx = v4l2_subdev_to_csi2rx(subdev);
+ int ret;
- csi2rx->source_pad = media_entity_get_fwnode_pad(&s_subdev->entity,
- s_subdev->fwnode,
- MEDIA_PAD_FL_SOURCE);
- if (csi2rx->source_pad < 0) {
- dev_err(csi2rx->dev, "Couldn't find output pad for subdev %s\n",
- s_subdev->name);
- return csi2rx->source_pad;
- }
+ ret = media_create_fwnode_pad_links(&csi2rx->subdev.entity.pads[0],
+ dev_fwnode(csi2rx->dev),
+ &s_subdev->entity,
+ dev_fwnode(s_subdev->dev),
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
+ if (ret)
+ return ret;
csi2rx->source_subdev = s_subdev;
- dev_dbg(csi2rx->dev, "Bound %s pad: %d\n", s_subdev->name,
- csi2rx->source_pad);
-
- return media_create_pad_link(&csi2rx->source_subdev->entity,
- csi2rx->source_pad,
- &csi2rx->subdev.entity, 0,
- MEDIA_LNK_FL_ENABLED |
- MEDIA_LNK_FL_IMMUTABLE);
+ return 0;
}
static const struct v4l2_async_notifier_operations csi2rx_notifier_ops = {
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 09/23] media: sun6i: Fix fwnode media link creation
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (7 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 08/23] media: cadence: csi2rx: " Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 10/23] media: st-mipid02: " Steve Longerbeam
` (15 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
sun6i_csi_link_entity() passes the bound subdev's sd->fwnode to
media_entity_get_fwnode_pad(). This is likely not an endpoint
fwnode as required by media_entity_get_fwnode_pad(), for most
subdevices it is the port parent of endpoint fwnode(s). This has only
worked before because no entities have implemented the .get_fwnode_pad()
op yet, and the default behavior of media_entity_get_fwnode_pad()
was to ignore the passed fwnode and return the first pad that matches
the given direction flags.
Fix this by replacing the calls to media_entity_get_fwnode_pad() and
media_create_pad_link() with a call to media_create_fwnode_pad_links().
Fixes: 5cc7522d89655 ("media: sun6i: Add support for Allwinner CSI V3s")
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
.../platform/sunxi/sun6i-csi/sun6i_csi.c | 36 +++----------------
1 file changed, 5 insertions(+), 31 deletions(-)
diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
index 055eb0b8e396..c6f51554ef67 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
@@ -630,37 +630,11 @@ static int sun6i_csi_link_entity(struct sun6i_csi *csi,
struct media_entity *entity,
struct fwnode_handle *fwnode)
{
- struct media_entity *sink;
- struct media_pad *sink_pad;
- int src_pad_index;
- int ret;
-
- ret = media_entity_get_fwnode_pad(entity, fwnode, MEDIA_PAD_FL_SOURCE);
- if (ret < 0) {
- dev_err(csi->dev, "%s: no source pad in external entity %s\n",
- __func__, entity->name);
- return -EINVAL;
- }
-
- src_pad_index = ret;
-
- sink = &csi->video.vdev.entity;
- sink_pad = &csi->video.pad;
-
- dev_dbg(csi->dev, "creating %s:%u -> %s:%u link\n",
- entity->name, src_pad_index, sink->name, sink_pad->index);
- ret = media_create_pad_link(entity, src_pad_index, sink,
- sink_pad->index,
- MEDIA_LNK_FL_ENABLED |
- MEDIA_LNK_FL_IMMUTABLE);
- if (ret < 0) {
- dev_err(csi->dev, "failed to create %s:%u -> %s:%u link\n",
- entity->name, src_pad_index,
- sink->name, sink_pad->index);
- return ret;
- }
-
- return 0;
+ return media_create_fwnode_pad_links(&csi->video.pad,
+ dev_fwnode(csi->dev),
+ entity, fwnode,
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
}
static int sun6i_subdev_notify_complete(struct v4l2_async_notifier *notifier)
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 10/23] media: st-mipid02: Fix fwnode media link creation
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (8 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 09/23] media: sun6i: " Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 11/23] media: stm32-dcmi: " Steve Longerbeam
` (14 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
mipid02_async_bound() passes the bound subdev's sd->fwnode to
media_entity_get_fwnode_pad(). This is likely not an endpoint
fwnode as required by media_entity_get_fwnode_pad(), for most
subdevices it is the port parent of endpoint fwnode(s). This has only
worked before because no entities have implemented the .get_fwnode_pad()
op yet, and the default behavior of media_entity_get_fwnode_pad()
was to ignore the passed fwnode and return the first pad that matches
the given direction flags.
Fix this by replacing the calls to media_entity_get_fwnode_pad() and
media_create_pad_link() with a call to media_create_fwnode_pad_links().
Fixes: 642bb5e88fed8 ("media: st-mipid02: MIPID02 CSI-2 to PARALLEL
bridge driver")
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/media/i2c/st-mipid02.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index 003ba22334cd..bd487082d677 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -798,24 +798,16 @@ static int mipid02_async_bound(struct v4l2_async_notifier *notifier,
{
struct mipid02_dev *bridge = to_mipid02_dev(notifier->sd);
struct i2c_client *client = bridge->i2c_client;
- int source_pad;
int ret;
dev_dbg(&client->dev, "sensor_async_bound call %p", s_subdev);
- source_pad = media_entity_get_fwnode_pad(&s_subdev->entity,
- s_subdev->fwnode,
- MEDIA_PAD_FL_SOURCE);
- if (source_pad < 0) {
- dev_err(&client->dev, "Couldn't find output pad for subdev %s\n",
- s_subdev->name);
- return source_pad;
- }
-
- ret = media_create_pad_link(&s_subdev->entity, source_pad,
- &bridge->sd.entity, 0,
- MEDIA_LNK_FL_ENABLED |
- MEDIA_LNK_FL_IMMUTABLE);
+ ret = media_create_fwnode_pad_links(&bridge->sd.entity.pads[0],
+ dev_fwnode(&client->dev),
+ &s_subdev->entity,
+ dev_fwnode(s_subdev->dev),
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
if (ret) {
dev_err(&client->dev, "Couldn't create media link %d", ret);
return ret;
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 11/23] media: stm32-dcmi: Fix fwnode media link creation
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (9 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 10/23] media: st-mipid02: " Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 12/23] media: sunxi: " Steve Longerbeam
` (13 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
dcmi_graph_notify_bound() passes the bound subdev's sd->fwnode to
media_entity_get_fwnode_pad(). This is likely not an endpoint
fwnode as required by media_entity_get_fwnode_pad(), for most
subdevices it is the port parent of endpoint fwnode(s). This has only
worked before because no entities have implemented the .get_fwnode_pad()
op yet, and the default behavior of media_entity_get_fwnode_pad()
was to ignore the passed fwnode and return the first pad that matches
the given direction flags.
Fix this by replacing the calls to media_entity_get_fwnode_pad() and
media_create_pad_link() with a call to media_create_fwnode_pad_links().
Fixes: f4378baf07a2 ("media: stm32-dcmi: add support of several sub-devices")
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/media/platform/stm32/stm32-dcmi.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index 9392e3409fba..bfb7794c43a1 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -1743,7 +1743,6 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier,
{
struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier);
unsigned int ret;
- int src_pad;
dev_dbg(dcmi->dev, "Subdev \"%s\" bound\n", subdev->name);
@@ -1751,14 +1750,12 @@ static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier,
* Link this sub-device to DCMI, it could be
* a parallel camera sensor or a bridge
*/
- src_pad = media_entity_get_fwnode_pad(&subdev->entity,
- subdev->fwnode,
- MEDIA_PAD_FL_SOURCE);
-
- ret = media_create_pad_link(&subdev->entity, src_pad,
- &dcmi->vdev->entity, 0,
- MEDIA_LNK_FL_IMMUTABLE |
- MEDIA_LNK_FL_ENABLED);
+ ret = media_create_fwnode_pad_links(&dcmi->vdev->entity.pads[0],
+ dev_fwnode(dcmi->dev),
+ &subdev->entity,
+ dev_fwnode(subdev->dev),
+ MEDIA_LNK_FL_IMMUTABLE |
+ MEDIA_LNK_FL_ENABLED);
if (ret)
dev_err(dcmi->dev, "Failed to create media pad link with subdev \"%s\"\n",
subdev->name);
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 12/23] media: sunxi: Fix fwnode media link creation
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (10 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 11/23] media: stm32-dcmi: " Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 13/23] media: v4l2-fwnode: Pass notifier to v4l2_async_register_fwnode_subdev() Steve Longerbeam
` (12 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
sun4i_csi_notify_bound() passes the bound subdev's sd->fwnode to
media_entity_get_fwnode_pad(). This is likely not an endpoint
fwnode as required by media_entity_get_fwnode_pad(), for most
subdevices it is the port parent of endpoint fwnode(s). This has only
worked before because no entities have implemented the .get_fwnode_pad()
op yet, and the default behavior of media_entity_get_fwnode_pad()
was to ignore the passed fwnode and return the first pad that matches
the given direction flags.
Fix this by replacing the calls to media_entity_get_fwnode_pad() and
media_create_pad_link() with a call to media_create_fwnode_pad_links().
Fixes: 577bbf23b758 ("media: sunxi: Add A10 CSI driver")
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
.../platform/sunxi/sun4i-csi/sun4i_csi.c | 27 ++++++++-----------
.../platform/sunxi/sun4i-csi/sun4i_csi.h | 1 -
2 files changed, 11 insertions(+), 17 deletions(-)
diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
index f36dc6258900..0f117d41a19b 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
@@ -38,19 +38,21 @@ static int sun4i_csi_notify_bound(struct v4l2_async_notifier *notifier,
{
struct sun4i_csi *csi = container_of(notifier, struct sun4i_csi,
notifier);
+ struct media_pad *sink = &csi->subdev.entity.pads[CSI_SUBDEV_SINK];
+ int ret;
csi->src_subdev = subdev;
- csi->src_pad = media_entity_get_fwnode_pad(&subdev->entity,
- subdev->fwnode,
- MEDIA_PAD_FL_SOURCE);
- if (csi->src_pad < 0) {
- dev_err(csi->dev, "Couldn't find output pad for subdev %s\n",
+
+ ret = media_create_fwnode_pad_links(sink, dev_fwnode(csi->dev),
+ &subdev->entity,
+ dev_fwnode(subdev->dev),
+ MEDIA_LNK_FL_ENABLED |
+ MEDIA_LNK_FL_IMMUTABLE);
+ if (ret)
+ dev_err(csi->dev, "Couldn't create media links to subdev %s\n",
subdev->name);
- return csi->src_pad;
- }
- dev_dbg(csi->dev, "Bound %s pad: %d\n", subdev->name, csi->src_pad);
- return 0;
+ return ret;
}
static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
@@ -81,13 +83,6 @@ static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
if (ret)
goto err_clean_media;
- ret = media_create_pad_link(&csi->src_subdev->entity, csi->src_pad,
- &subdev->entity, CSI_SUBDEV_SINK,
- MEDIA_LNK_FL_ENABLED |
- MEDIA_LNK_FL_IMMUTABLE);
- if (ret)
- goto err_clean_media;
-
ret = v4l2_device_register_subdev_nodes(&csi->v4l);
if (ret < 0)
goto err_clean_media;
diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
index 001c8bde006c..1d403f9cef1a 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
@@ -140,7 +140,6 @@ struct sun4i_csi {
struct v4l2_async_subdev asd;
struct v4l2_async_notifier notifier;
struct v4l2_subdev *src_subdev;
- int src_pad;
/* V4L2 variables */
struct mutex lock;
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 13/23] media: v4l2-fwnode: Pass notifier to v4l2_async_register_fwnode_subdev()
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (11 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 12/23] media: sunxi: " Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 14/23] media: video-mux: Create media links in bound notifier Steve Longerbeam
` (11 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Instead of allocating a notifier in v4l2_async_register_fwnode_subdev(),
have the caller provide one. This allows the caller to implement
notifier ops (bind, unbind).
The caller is now responsible for first initializing its notifier with a
call to v4l2_async_notifier_init().
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/media/platform/video-mux.c | 6 +++++-
drivers/media/v4l2-core/v4l2-fwnode.c | 11 +----------
drivers/staging/media/imx/imx6-mipi-csi2.c | 5 ++++-
drivers/staging/media/imx/imx7-media-csi.c | 5 ++++-
drivers/staging/media/imx/imx7-mipi-csis.c | 5 ++++-
include/media/v4l2-fwnode.h | 12 ++++++++----
6 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c
index ddd0e338f9e4..ca1cef783646 100644
--- a/drivers/media/platform/video-mux.c
+++ b/drivers/media/platform/video-mux.c
@@ -21,6 +21,7 @@
struct video_mux {
struct v4l2_subdev subdev;
+ struct v4l2_async_notifier notifier;
struct media_pad *pads;
struct v4l2_mbus_framefmt *format_mbus;
struct mux_control *mux;
@@ -354,8 +355,11 @@ static int video_mux_async_register(struct video_mux *vmux,
for (i = 0; i < num_input_pads; i++)
ports[i] = i;
+ v4l2_async_notifier_init(&vmux->notifier);
+
ret = v4l2_async_register_fwnode_subdev(
- &vmux->subdev, sizeof(struct v4l2_async_subdev),
+ &vmux->subdev, &vmux->notifier,
+ sizeof(struct v4l2_async_subdev),
ports, num_input_pads, video_mux_parse_endpoint);
kfree(ports);
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index f43f563f9e98..d2f134caa0cf 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -1125,12 +1125,12 @@ int v4l2_async_register_subdev_sensor_common(struct v4l2_subdev *sd)
EXPORT_SYMBOL_GPL(v4l2_async_register_subdev_sensor_common);
int v4l2_async_register_fwnode_subdev(struct v4l2_subdev *sd,
+ struct v4l2_async_notifier *notifier,
size_t asd_struct_size,
unsigned int *ports,
unsigned int num_ports,
parse_endpoint_func parse_endpoint)
{
- struct v4l2_async_notifier *notifier;
struct device *dev = sd->dev;
struct fwnode_handle *fwnode;
int ret;
@@ -1142,12 +1142,6 @@ int v4l2_async_register_fwnode_subdev(struct v4l2_subdev *sd,
if (!fwnode_device_is_available(fwnode))
return -ENODEV;
- notifier = kzalloc(sizeof(*notifier), GFP_KERNEL);
- if (!notifier)
- return -ENOMEM;
-
- v4l2_async_notifier_init(notifier);
-
if (!ports) {
ret = v4l2_async_notifier_parse_fwnode_endpoints(dev, notifier,
asd_struct_size,
@@ -1172,15 +1166,12 @@ int v4l2_async_register_fwnode_subdev(struct v4l2_subdev *sd,
if (ret < 0)
goto out_unregister;
- sd->subdev_notifier = notifier;
-
return 0;
out_unregister:
v4l2_async_notifier_unregister(notifier);
out_cleanup:
v4l2_async_notifier_cleanup(notifier);
- kfree(notifier);
return ret;
}
diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
index cd3dd6e33ef0..06ed4057b426 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -35,6 +35,7 @@
struct csi2_dev {
struct device *dev;
struct v4l2_subdev sd;
+ struct v4l2_async_notifier notifier;
struct media_pad pad[CSI2_NUM_PADS];
struct clk *dphy_clk;
struct clk *pllref_clk;
@@ -636,8 +637,10 @@ static int csi2_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, &csi2->sd);
+ v4l2_async_notifier_init(&csi2->notifier);
+
ret = v4l2_async_register_fwnode_subdev(
- &csi2->sd, sizeof(struct v4l2_async_subdev),
+ &csi2->sd, &csi2->notifier, sizeof(struct v4l2_async_subdev),
&sink_port, 1, csi2_parse_endpoint);
if (ret)
goto dphy_off;
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index db30e2c70f2f..15b08bfb5aa7 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -155,6 +155,7 @@
struct imx7_csi {
struct device *dev;
struct v4l2_subdev sd;
+ struct v4l2_async_notifier notifier;
struct imx_media_video_dev *vdev;
struct imx_media_dev *imxmd;
struct media_pad pad[IMX7_CSI_PADS_NUM];
@@ -1266,7 +1267,9 @@ static int imx7_csi_probe(struct platform_device *pdev)
if (ret < 0)
goto free;
- ret = v4l2_async_register_fwnode_subdev(&csi->sd,
+ v4l2_async_notifier_init(&csi->notifier);
+
+ ret = v4l2_async_register_fwnode_subdev(&csi->sd, &csi->notifier,
sizeof(struct v4l2_async_subdev),
NULL, 0,
imx7_csi_parse_endpoint);
diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 99166afca071..bbbc4d55fa9e 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -223,6 +223,7 @@ struct csi_state {
struct device *dev;
struct media_pad pads[CSIS_PADS_NUM];
struct v4l2_subdev mipi_sd;
+ struct v4l2_async_notifier notifier;
struct v4l2_subdev *src_sd;
u8 index;
@@ -885,7 +886,9 @@ static int mipi_csis_subdev_init(struct v4l2_subdev *mipi_sd,
if (ret)
return ret;
- ret = v4l2_async_register_fwnode_subdev(mipi_sd,
+ v4l2_async_notifier_init(&state->notifier);
+
+ ret = v4l2_async_register_fwnode_subdev(mipi_sd, &state->notifier,
sizeof(struct v4l2_async_subdev),
&sink_port, 1,
mipi_csis_parse_endpoint);
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index f81f8bf34526..27a7b78149c2 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -331,6 +331,7 @@ int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
* and parses fwnode endpoints
*
* @sd: pointer to struct &v4l2_subdev
+ * @notifier: the sub-device's notifier.
* @asd_struct_size: size of the driver's async sub-device struct, including
* sizeof(struct v4l2_async_subdev). The &struct
* v4l2_async_subdev shall be the first member of
@@ -343,13 +344,15 @@ int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
* endpoint. Optional.
*
* This function is just like v4l2_async_register_subdev() with the
- * exception that calling it will also allocate a notifier for the
- * sub-device, parse the sub-device's firmware node endpoints using
- * v4l2_async_notifier_parse_fwnode_endpoints() or
+ * exception that calling it will also parse the sub-device's firmware
+ * node endpoints using v4l2_async_notifier_parse_fwnode_endpoints() or
* v4l2_async_notifier_parse_fwnode_endpoints_by_port(), and
- * registers the sub-device notifier. The sub-device is similarly
+ * registers the sub-device's notifier. The sub-device is similarly
* unregistered by calling v4l2_async_unregister_subdev().
*
+ * The caller must first initialize the notifier with a call to
+ * v4l2_async_notifier_init().
+ *
* While registered, the subdev module is marked as in-use.
*
* An error is returned if the module is no longer loaded on any attempts
@@ -357,6 +360,7 @@ int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
*/
int
v4l2_async_register_fwnode_subdev(struct v4l2_subdev *sd,
+ struct v4l2_async_notifier *notifier,
size_t asd_struct_size,
unsigned int *ports,
unsigned int num_ports,
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 14/23] media: video-mux: Create media links in bound notifier
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (12 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 13/23] media: v4l2-fwnode: Pass notifier to v4l2_async_register_fwnode_subdev() Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 15/23] media: imx: mipi csi-2: " Steve Longerbeam
` (10 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Implement a notifier bound op to register media links from the remote
sub-device's source pads to all of the video-mux sink pads.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/media/platform/video-mux.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c
index ca1cef783646..de5b0698fc1d 100644
--- a/drivers/media/platform/video-mux.c
+++ b/drivers/media/platform/video-mux.c
@@ -36,6 +36,12 @@ static const struct v4l2_mbus_framefmt video_mux_format_mbus_default = {
.field = V4L2_FIELD_NONE,
};
+static inline struct video_mux *
+notifier_to_video_mux(struct v4l2_async_notifier *n)
+{
+ return container_of(n, struct video_mux, notifier);
+}
+
static inline struct video_mux *v4l2_subdev_to_video_mux(struct v4l2_subdev *sd)
{
return container_of(sd, struct video_mux, subdev);
@@ -343,6 +349,22 @@ static int video_mux_parse_endpoint(struct device *dev,
return fwnode_device_is_available(asd->match.fwnode) ? 0 : -ENOTCONN;
}
+static int video_mux_notify_bound(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *sd,
+ struct v4l2_async_subdev *asd)
+{
+ struct video_mux *vmux = notifier_to_video_mux(notifier);
+
+ return media_create_fwnode_links(&vmux->subdev.entity,
+ dev_fwnode(vmux->subdev.dev),
+ &sd->entity,
+ dev_fwnode(sd->dev), 0);
+}
+
+static const struct v4l2_async_notifier_operations video_mux_notify_ops = {
+ .bound = video_mux_notify_bound,
+};
+
static int video_mux_async_register(struct video_mux *vmux,
unsigned int num_input_pads)
{
@@ -357,6 +379,8 @@ static int video_mux_async_register(struct video_mux *vmux,
v4l2_async_notifier_init(&vmux->notifier);
+ vmux->notifier.ops = &video_mux_notify_ops;
+
ret = v4l2_async_register_fwnode_subdev(
&vmux->subdev, &vmux->notifier,
sizeof(struct v4l2_async_subdev),
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 15/23] media: imx: mipi csi-2: Create media links in bound notifier
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (13 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 14/23] media: video-mux: Create media links in bound notifier Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 16/23] media: imx7-mipi-csis: " Steve Longerbeam
` (9 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Implement a notifier bound op to register media links from the remote
sub-device's source pad(s) to the mipi csi-2 receiver sink pad.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/staging/media/imx/imx6-mipi-csi2.c | 24 ++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
index 06ed4057b426..19275a310033 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -91,6 +91,11 @@ static inline struct csi2_dev *sd_to_dev(struct v4l2_subdev *sdev)
return container_of(sdev, struct csi2_dev, sd);
}
+static inline struct csi2_dev *notifier_to_dev(struct v4l2_async_notifier *n)
+{
+ return container_of(n, struct csi2_dev, notifier);
+}
+
/*
* The required sequence of MIPI CSI-2 startup as specified in the i.MX6
* reference manual is as follows:
@@ -556,6 +561,23 @@ static int csi2_parse_endpoint(struct device *dev,
return 0;
}
+static int csi2_notify_bound(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *sd,
+ struct v4l2_async_subdev *asd)
+{
+ struct csi2_dev *csi2 = notifier_to_dev(notifier);
+ struct media_pad *sink = &csi2->sd.entity.pads[CSI2_SINK_PAD];
+
+ return media_create_fwnode_pad_links(sink,
+ dev_fwnode(csi2->dev),
+ &sd->entity,
+ dev_fwnode(sd->dev), 0);
+}
+
+static const struct v4l2_async_notifier_operations csi2_notify_ops = {
+ .bound = csi2_notify_bound,
+};
+
static int csi2_probe(struct platform_device *pdev)
{
unsigned int sink_port = 0;
@@ -639,6 +661,8 @@ static int csi2_probe(struct platform_device *pdev)
v4l2_async_notifier_init(&csi2->notifier);
+ csi2->notifier.ops = &csi2_notify_ops;
+
ret = v4l2_async_register_fwnode_subdev(
&csi2->sd, &csi2->notifier, sizeof(struct v4l2_async_subdev),
&sink_port, 1, csi2_parse_endpoint);
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 16/23] media: imx7-mipi-csis: Create media links in bound notifier
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (14 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 15/23] media: imx: mipi csi-2: " Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 17/23] media: imx7-media-csi: " Steve Longerbeam
` (8 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Implement a notifier bound op to register media links from the remote
sub-device's source pad(s) to the mipi csi-2 receiver sink pad.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
Changes in v2:
Move notifier_to_csis_state() next to mipi_sd_to_csis_state(), remove
unnecessary inline, and rename to mipi_notifier_to_csis_state().
Suggested-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
drivers/staging/media/imx/imx7-mipi-csis.c | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index bbbc4d55fa9e..0ea6a48c2274 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -319,6 +319,12 @@ static int mipi_csis_dump_regs(struct csi_state *state)
return 0;
}
+static struct csi_state *
+mipi_notifier_to_csis_state(struct v4l2_async_notifier *n)
+{
+ return container_of(n, struct csi_state, notifier);
+}
+
static struct csi_state *mipi_sd_to_csis_state(struct v4l2_subdev *sdev)
{
return container_of(sdev, struct csi_state, mipi_sd);
@@ -850,6 +856,23 @@ static int mipi_csis_parse_endpoint(struct device *dev,
return 0;
}
+static int mipi_csis_notify_bound(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *sd,
+ struct v4l2_async_subdev *asd)
+{
+ struct csi_state *state = mipi_notifier_to_csis_state(notifier);
+ struct media_pad *sink = &state->mipi_sd.entity.pads[CSIS_PAD_SINK];
+
+ return media_create_fwnode_pad_links(sink,
+ dev_fwnode(state->mipi_sd.dev),
+ &sd->entity,
+ dev_fwnode(sd->dev), 0);
+}
+
+static const struct v4l2_async_notifier_operations mipi_csis_notify_ops = {
+ .bound = mipi_csis_notify_bound,
+};
+
static int mipi_csis_subdev_init(struct v4l2_subdev *mipi_sd,
struct platform_device *pdev,
const struct v4l2_subdev_ops *ops)
@@ -888,6 +911,8 @@ static int mipi_csis_subdev_init(struct v4l2_subdev *mipi_sd,
v4l2_async_notifier_init(&state->notifier);
+ state->notifier.ops = &mipi_csis_notify_ops;
+
ret = v4l2_async_register_fwnode_subdev(mipi_sd, &state->notifier,
sizeof(struct v4l2_async_subdev),
&sink_port, 1,
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 17/23] media: imx7-media-csi: Create media links in bound notifier
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (15 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 16/23] media: imx7-mipi-csis: " Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-26 22:49 ` Rui Miguel Silva
2019-11-24 19:06 ` [PATCH v2 18/23] media: imx: csi: Implement get_fwnode_pad Steve Longerbeam
` (7 subsequent siblings)
24 siblings, 1 reply; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Implement a notifier bound op to register media links from the remote
sub-device's source pad(s) to the CSI sink pad.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
Changes in v2:
Rename notifier_to_dev() to imx7_csi_notifier_to_dev() and remove
unnecessary inline.
Suggested-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
drivers/staging/media/imx/imx7-media-csi.c | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 15b08bfb5aa7..848d1286fbeb 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -196,6 +196,12 @@ struct imx7_csi {
struct completion last_eof_completion;
};
+static struct imx7_csi *
+imx7_csi_notifier_to_dev(struct v4l2_async_notifier *n)
+{
+ return container_of(n, struct imx7_csi, notifier);
+}
+
static u32 imx7_csi_reg_read(struct imx7_csi *csi, unsigned int offset)
{
return readl(csi->regbase + offset);
@@ -1187,6 +1193,23 @@ static int imx7_csi_parse_endpoint(struct device *dev,
return fwnode_device_is_available(asd->match.fwnode) ? 0 : -EINVAL;
}
+static int imx7_csi_notify_bound(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *sd,
+ struct v4l2_async_subdev *asd)
+{
+ struct imx7_csi *csi = imx7_csi_notifier_to_dev(notifier);
+ struct media_pad *sink = &csi->sd.entity.pads[IMX7_CSI_PAD_SINK];
+
+ return media_create_fwnode_pad_links(sink,
+ dev_fwnode(csi->sd.dev),
+ &sd->entity,
+ dev_fwnode(sd->dev), 0);
+}
+
+static const struct v4l2_async_notifier_operations imx7_csi_notify_ops = {
+ .bound = imx7_csi_notify_bound,
+};
+
static int imx7_csi_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -1269,6 +1292,8 @@ static int imx7_csi_probe(struct platform_device *pdev)
v4l2_async_notifier_init(&csi->notifier);
+ csi->notifier.ops = &imx7_csi_notify_ops;
+
ret = v4l2_async_register_fwnode_subdev(&csi->sd, &csi->notifier,
sizeof(struct v4l2_async_subdev),
NULL, 0,
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v2 17/23] media: imx7-media-csi: Create media links in bound notifier
2019-11-24 19:06 ` [PATCH v2 17/23] media: imx7-media-csi: " Steve Longerbeam
@ 2019-11-26 22:49 ` Rui Miguel Silva
0 siblings, 0 replies; 38+ messages in thread
From: Rui Miguel Silva @ 2019-11-26 22:49 UTC (permalink / raw)
To: Steve Longerbeam; +Cc: linux-media
Hi Steve,
On Sun, Nov 24, 2019 at 11:06:57AM -0800, Steve Longerbeam wrote:
> Implement a notifier bound op to register media links from the remote
> sub-device's source pad(s) to the CSI sink pad.
>
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
> Changes in v2:
> Rename notifier_to_dev() to imx7_csi_notifier_to_dev() and remove
> unnecessary inline.
> Suggested-by: Rui Miguel Silva <rmfrfs@gmail.com>
Many thanks for this.
Reviewed-by: Rui Miguel Silva <rmfrfs@gmail.com>
------
Cheers,
Rui
> ---
> drivers/staging/media/imx/imx7-media-csi.c | 25 ++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> index 15b08bfb5aa7..848d1286fbeb 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -196,6 +196,12 @@ struct imx7_csi {
> struct completion last_eof_completion;
> };
>
> +static struct imx7_csi *
> +imx7_csi_notifier_to_dev(struct v4l2_async_notifier *n)
> +{
> + return container_of(n, struct imx7_csi, notifier);
> +}
> +
> static u32 imx7_csi_reg_read(struct imx7_csi *csi, unsigned int offset)
> {
> return readl(csi->regbase + offset);
> @@ -1187,6 +1193,23 @@ static int imx7_csi_parse_endpoint(struct device *dev,
> return fwnode_device_is_available(asd->match.fwnode) ? 0 : -EINVAL;
> }
>
> +static int imx7_csi_notify_bound(struct v4l2_async_notifier *notifier,
> + struct v4l2_subdev *sd,
> + struct v4l2_async_subdev *asd)
> +{
> + struct imx7_csi *csi = imx7_csi_notifier_to_dev(notifier);
> + struct media_pad *sink = &csi->sd.entity.pads[IMX7_CSI_PAD_SINK];
> +
> + return media_create_fwnode_pad_links(sink,
> + dev_fwnode(csi->sd.dev),
> + &sd->entity,
> + dev_fwnode(sd->dev), 0);
> +}
> +
> +static const struct v4l2_async_notifier_operations imx7_csi_notify_ops = {
> + .bound = imx7_csi_notify_bound,
> +};
> +
> static int imx7_csi_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -1269,6 +1292,8 @@ static int imx7_csi_probe(struct platform_device *pdev)
>
> v4l2_async_notifier_init(&csi->notifier);
>
> + csi->notifier.ops = &imx7_csi_notify_ops;
> +
> ret = v4l2_async_register_fwnode_subdev(&csi->sd, &csi->notifier,
> sizeof(struct v4l2_async_subdev),
> NULL, 0,
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v2 18/23] media: imx: csi: Implement get_fwnode_pad
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (16 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 17/23] media: imx7-media-csi: " Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-24 19:06 ` [PATCH v2 19/23] media: imx: csi: Embed notifier in struct csi_priv Steve Longerbeam
` (6 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
The CSI does not have a 1:1 relationship between fwnode port numbers and
pad indexes. In fact the CSI fwnode device is itself a port which is the
sink, containing only a single fwnode endpoint. Implement media_entity
operation get_fwnode_pad to first verify the given endpoint is the CSI's
sink endpoint, and if so return the CSI sink pad index.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/staging/media/imx/imx-media-csi.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index b60ed4f22f6d..dc5fe25fe7b8 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1825,9 +1825,32 @@ static void csi_unregistered(struct v4l2_subdev *sd)
ipu_csi_put(priv->csi);
}
+/*
+ * The CSI has only one fwnode endpoint, at the sink pad. Verify the
+ * endpoint belongs to us, and return CSI_SINK_PAD.
+ */
+static int csi_get_fwnode_pad(struct media_entity *entity,
+ struct fwnode_endpoint *endpoint)
+{
+ struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+ struct csi_priv *priv = v4l2_get_subdevdata(sd);
+ struct fwnode_handle *csi_port = dev_fwnode(priv->dev);
+ struct fwnode_handle *csi_ep;
+ int ret;
+
+ csi_ep = fwnode_get_next_child_node(csi_port, NULL);
+
+ ret = endpoint->local_fwnode == csi_ep ? CSI_SINK_PAD : -ENXIO;
+
+ fwnode_handle_put(csi_ep);
+
+ return ret;
+}
+
static const struct media_entity_operations csi_entity_ops = {
.link_setup = csi_link_setup,
.link_validate = v4l2_subdev_link_validate,
+ .get_fwnode_pad = csi_get_fwnode_pad,
};
static const struct v4l2_subdev_core_ops csi_core_ops = {
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 19/23] media: imx: csi: Embed notifier in struct csi_priv
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (17 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 18/23] media: imx: csi: Implement get_fwnode_pad Steve Longerbeam
@ 2019-11-24 19:06 ` Steve Longerbeam
2019-11-24 19:07 ` [PATCH v2 20/23] media: imx: csi: Create media links in bound notifier Steve Longerbeam
` (5 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:06 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Embed the notifier in 'struct csi_priv', instead of dynamically allocating
it, to make it possible to retrieve csi_priv in a notifier callback op.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/staging/media/imx/imx-media-csi.c | 25 +++++++++--------------
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index dc5fe25fe7b8..3e2afdd59276 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -58,6 +58,8 @@ struct csi_priv {
struct ipu_soc *ipu;
struct v4l2_subdev sd;
struct media_pad pad[CSI_NUM_PADS];
+ struct v4l2_async_notifier notifier;
+
/* the video device at IDMAC output pad */
struct imx_media_video_dev *vdev;
struct imx_media_fim *fim;
@@ -1896,31 +1898,28 @@ static int imx_csi_parse_endpoint(struct device *dev,
static int imx_csi_async_register(struct csi_priv *priv)
{
- struct v4l2_async_notifier *notifier;
struct fwnode_handle *fwnode;
unsigned int port;
int ret;
- notifier = kzalloc(sizeof(*notifier), GFP_KERNEL);
- if (!notifier)
- return -ENOMEM;
-
- v4l2_async_notifier_init(notifier);
+ v4l2_async_notifier_init(&priv->notifier);
fwnode = dev_fwnode(priv->dev);
/* get this CSI's port id */
ret = fwnode_property_read_u32(fwnode, "reg", &port);
if (ret < 0)
- goto out_free;
+ return ret;
ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(
- priv->dev->parent, notifier, sizeof(struct v4l2_async_subdev),
+ priv->dev->parent, &priv->notifier,
+ sizeof(struct v4l2_async_subdev),
port, imx_csi_parse_endpoint);
if (ret < 0)
goto out_cleanup;
- ret = v4l2_async_subdev_notifier_register(&priv->sd, notifier);
+ ret = v4l2_async_subdev_notifier_register(&priv->sd,
+ &priv->notifier);
if (ret < 0)
goto out_cleanup;
@@ -1928,16 +1927,12 @@ static int imx_csi_async_register(struct csi_priv *priv)
if (ret < 0)
goto out_unregister;
- priv->sd.subdev_notifier = notifier;
-
return 0;
out_unregister:
- v4l2_async_notifier_unregister(notifier);
+ v4l2_async_notifier_unregister(&priv->notifier);
out_cleanup:
- v4l2_async_notifier_cleanup(notifier);
-out_free:
- kfree(notifier);
+ v4l2_async_notifier_cleanup(&priv->notifier);
return ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 20/23] media: imx: csi: Create media links in bound notifier
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (18 preceding siblings ...)
2019-11-24 19:06 ` [PATCH v2 19/23] media: imx: csi: Embed notifier in struct csi_priv Steve Longerbeam
@ 2019-11-24 19:07 ` Steve Longerbeam
2019-11-24 19:07 ` [PATCH v2 21/23] media: imx: csi: Lookup upstream endpoint with imx_media_get_pad_fwnode Steve Longerbeam
` (4 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:07 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Implement a notifier bound op to register media links from the remote
sub-device's source pad(s) to the CSI sink pad.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/staging/media/imx/imx-media-csi.c | 24 +++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 3e2afdd59276..e162f8aee164 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -120,6 +120,11 @@ static inline struct csi_priv *sd_to_dev(struct v4l2_subdev *sdev)
return container_of(sdev, struct csi_priv, sd);
}
+static inline struct csi_priv *notifier_to_dev(struct v4l2_async_notifier *n)
+{
+ return container_of(n, struct csi_priv, notifier);
+}
+
static inline bool is_parallel_bus(struct v4l2_fwnode_endpoint *ep)
{
return ep->bus_type != V4L2_MBUS_CSI2_DPHY;
@@ -1896,6 +1901,23 @@ static int imx_csi_parse_endpoint(struct device *dev,
return fwnode_device_is_available(asd->match.fwnode) ? 0 : -ENOTCONN;
}
+static int imx_csi_notify_bound(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *sd,
+ struct v4l2_async_subdev *asd)
+{
+ struct csi_priv *priv = notifier_to_dev(notifier);
+ struct media_pad *sink = &priv->sd.entity.pads[CSI_SINK_PAD];
+
+ return media_create_fwnode_pad_links(sink,
+ dev_fwnode(priv->dev->parent),
+ &sd->entity,
+ dev_fwnode(sd->dev), 0);
+}
+
+static const struct v4l2_async_notifier_operations csi_notify_ops = {
+ .bound = imx_csi_notify_bound,
+};
+
static int imx_csi_async_register(struct csi_priv *priv)
{
struct fwnode_handle *fwnode;
@@ -1904,6 +1926,8 @@ static int imx_csi_async_register(struct csi_priv *priv)
v4l2_async_notifier_init(&priv->notifier);
+ priv->notifier.ops = &csi_notify_ops;
+
fwnode = dev_fwnode(priv->dev);
/* get this CSI's port id */
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 21/23] media: imx: csi: Lookup upstream endpoint with imx_media_get_pad_fwnode
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (19 preceding siblings ...)
2019-11-24 19:07 ` [PATCH v2 20/23] media: imx: csi: Create media links in bound notifier Steve Longerbeam
@ 2019-11-24 19:07 ` Steve Longerbeam
2019-11-24 19:07 ` [PATCH v2 22/23] media: imx: Create missing links from CSI-2 receiver Steve Longerbeam
` (3 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:07 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Fix the 1:1 port-id:pad-index assumption for the upstream subdevice, by
searching the upstream subdevice's endpoints for one that maps to the
pad's index. This is carried out by a new reverse mapping function
imx_media_get_pad_fwnode().
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/staging/media/imx/imx-media-csi.c | 22 ++++----------
drivers/staging/media/imx/imx-media-utils.c | 33 +++++++++++++++++++++
drivers/staging/media/imx/imx-media.h | 1 +
drivers/staging/media/imx/imx7-media-csi.c | 25 +++++-----------
4 files changed, 47 insertions(+), 34 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index e162f8aee164..022472f0a657 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -164,7 +164,7 @@ static inline bool requires_passthrough(struct v4l2_fwnode_endpoint *ep,
static int csi_get_upstream_endpoint(struct csi_priv *priv,
struct v4l2_fwnode_endpoint *ep)
{
- struct device_node *endpoint, *port;
+ struct fwnode_handle *endpoint;
struct media_entity *src;
struct v4l2_subdev *sd;
struct media_pad *pad;
@@ -203,23 +203,13 @@ static int csi_get_upstream_endpoint(struct csi_priv *priv,
if (!pad)
return -ENODEV;
- sd = media_entity_to_v4l2_subdev(pad->entity);
+ endpoint = imx_media_get_pad_fwnode(pad);
+ if (IS_ERR(endpoint))
+ return PTR_ERR(endpoint);
- /*
- * NOTE: this assumes an OF-graph port id is the same as a
- * media pad index.
- */
- port = of_graph_get_port_by_id(sd->dev->of_node, pad->index);
- if (!port)
- return -ENODEV;
-
- endpoint = of_get_next_child(port, NULL);
- of_node_put(port);
- if (!endpoint)
- return -ENODEV;
+ v4l2_fwnode_endpoint_parse(endpoint, ep);
- v4l2_fwnode_endpoint_parse(of_fwnode_handle(endpoint), ep);
- of_node_put(endpoint);
+ fwnode_handle_put(endpoint);
return 0;
}
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 0788a1874557..547805f9379e 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -916,6 +916,39 @@ imx_media_pipeline_video_device(struct media_entity *start_entity,
}
EXPORT_SYMBOL_GPL(imx_media_pipeline_video_device);
+/*
+ * Find a fwnode endpoint that maps to the given subdevice's pad.
+ * If there are multiple endpoints that map to the pad, only the
+ * first endpoint encountered is returned.
+ *
+ * On success the refcount of the returned fwnode endpoint is
+ * incremented.
+ */
+struct fwnode_handle *imx_media_get_pad_fwnode(struct media_pad *pad)
+{
+ struct fwnode_handle *endpoint;
+ struct v4l2_subdev *sd;
+
+ if (!is_media_entity_v4l2_subdev(pad->entity))
+ return ERR_PTR(-ENODEV);
+
+ sd = media_entity_to_v4l2_subdev(pad->entity);
+
+ fwnode_graph_for_each_endpoint(dev_fwnode(sd->dev), endpoint) {
+ int pad_idx = media_entity_get_fwnode_pad(&sd->entity,
+ endpoint,
+ pad->flags);
+ if (pad_idx < 0)
+ continue;
+
+ if (pad_idx == pad->index)
+ return endpoint;
+ }
+
+ return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL_GPL(imx_media_get_pad_fwnode);
+
/*
* Turn current pipeline streaming on/off starting from entity.
*/
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index 11861191324a..3363af07a5ca 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -201,6 +201,7 @@ imx_media_pipeline_subdev(struct media_entity *start_entity, u32 grp_id,
struct video_device *
imx_media_pipeline_video_device(struct media_entity *start_entity,
enum v4l2_buf_type buftype, bool upstream);
+struct fwnode_handle *imx_media_get_pad_fwnode(struct media_pad *pad);
struct imx_media_dma_buf {
void *virt;
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 848d1286fbeb..103447cb5e01 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -439,9 +439,8 @@ static int imx7_csi_get_upstream_endpoint(struct imx7_csi *csi,
struct v4l2_fwnode_endpoint *ep,
bool skip_mux)
{
- struct device_node *endpoint, *port;
+ struct fwnode_handle *endpoint;
struct media_entity *src;
- struct v4l2_subdev *sd;
struct media_pad *pad;
if (!csi->src_sd)
@@ -463,29 +462,19 @@ static int imx7_csi_get_upstream_endpoint(struct imx7_csi *csi,
if (!pad)
return -ENODEV;
- sd = media_entity_to_v4l2_subdev(pad->entity);
-
/* To get bus type we may need to skip video mux */
if (skip_mux && src->function == MEDIA_ENT_F_VID_MUX) {
- src = &sd->entity;
+ src = pad->entity;
goto skip_video_mux;
}
- /*
- * NOTE: this assumes an OF-graph port id is the same as a
- * media pad index.
- */
- port = of_graph_get_port_by_id(sd->dev->of_node, pad->index);
- if (!port)
- return -ENODEV;
+ endpoint = imx_media_get_pad_fwnode(pad);
+ if (IS_ERR(endpoint))
+ return PTR_ERR(endpoint);
- endpoint = of_get_next_child(port, NULL);
- of_node_put(port);
- if (!endpoint)
- return -ENODEV;
+ v4l2_fwnode_endpoint_parse(endpoint, ep);
- v4l2_fwnode_endpoint_parse(of_fwnode_handle(endpoint), ep);
- of_node_put(endpoint);
+ fwnode_handle_put(endpoint);
return 0;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 22/23] media: imx: Create missing links from CSI-2 receiver
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (20 preceding siblings ...)
2019-11-24 19:07 ` [PATCH v2 21/23] media: imx: csi: Lookup upstream endpoint with imx_media_get_pad_fwnode Steve Longerbeam
@ 2019-11-24 19:07 ` Steve Longerbeam
2019-11-24 19:07 ` [PATCH v2 23/23] media: imx: TODO: Remove media link creation todos Steve Longerbeam
` (2 subsequent siblings)
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:07 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
The entities external to the i.MX6 IPU and i.MX7 now create the links
to their fwnode-endpoint connected entities in their notifier bound
callbacks. Which means imx_media_create_of_links() and
imx_media_create_csi_of_links() are no longer needed and are removed.
However there is still one case in which imx-media needs to create
fwnode-endpoint based links at probe completion. The v4l2-async framework
does not allow multiple subdevice notifiers to contain a duplicate
subdevice in their asd_list. Only the first subdev notifier that discovers
and adds that one subdevice to its asd_list will receive a bound callback
for it. Other subdevices that also have firmware endpoint connections to
this duplicate subdevice will not have it in their asd_list, and thus will
never receive a bound callback for it. In the case of imx-media, the one
duplicate subdevice in question is the i.MX6 MIPI CSI-2 receiver.
Until there is a solution to that problem, rewrite imx_media_create_links()
to add the missing links from the CSI-2 receiver to the CSIs and CSI muxes.
The function is renamed imx_media_create_csi2_links().
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
Changes in v2:
- this is a rewrite of v1 "media: imx: Use media_create_fwnode_links for
external links", which only adds the missing CSI-2 receiver links.
---
.../staging/media/imx/imx-media-dev-common.c | 60 +++++-----
drivers/staging/media/imx/imx-media-of.c | 113 ------------------
drivers/staging/media/imx/imx-media.h | 4 -
3 files changed, 31 insertions(+), 146 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-dev-common.c b/drivers/staging/media/imx/imx-media-dev-common.c
index 66b505f7e8df..8acf2f2e28c6 100644
--- a/drivers/staging/media/imx/imx-media-dev-common.c
+++ b/drivers/staging/media/imx/imx-media-dev-common.c
@@ -30,41 +30,45 @@ static int imx_media_subdev_bound(struct v4l2_async_notifier *notifier,
}
/*
- * Create the media links for all subdevs that registered.
+ * Create the missing media links from the CSI-2 receiver.
* Called after all async subdevs have bound.
*/
-static int imx_media_create_links(struct v4l2_async_notifier *notifier)
+static void imx_media_create_csi2_links(struct imx_media_dev *imxmd)
{
- struct imx_media_dev *imxmd = notifier2dev(notifier);
- struct v4l2_subdev *sd;
+ struct v4l2_subdev *sd, *csi2 = NULL;
list_for_each_entry(sd, &imxmd->v4l2_dev.subdevs, list) {
- switch (sd->grp_id) {
- case IMX_MEDIA_GRP_ID_IPU_VDIC:
- case IMX_MEDIA_GRP_ID_IPU_IC_PRP:
- case IMX_MEDIA_GRP_ID_IPU_IC_PRPENC:
- case IMX_MEDIA_GRP_ID_IPU_IC_PRPVF:
- /*
- * links have already been created for the
- * sync-registered subdevs.
- */
- break;
- case IMX_MEDIA_GRP_ID_IPU_CSI0:
- case IMX_MEDIA_GRP_ID_IPU_CSI1:
- case IMX_MEDIA_GRP_ID_CSI:
- imx_media_create_csi_of_links(imxmd, sd);
- break;
- default:
- /*
- * if this subdev has fwnode links, create media
- * links for them.
- */
- imx_media_create_of_links(imxmd, sd);
+ if (sd->grp_id == IMX_MEDIA_GRP_ID_CSI2) {
+ csi2 = sd;
break;
}
}
- return 0;
+ if (!csi2)
+ return;
+
+ list_for_each_entry(sd, &imxmd->v4l2_dev.subdevs, list) {
+ struct fwnode_handle *fwnode;
+
+ /* skip if not a CSI or a video mux */
+ if (!(sd->grp_id & IMX_MEDIA_GRP_ID_IPU_CSI) &&
+ !(sd->grp_id & IMX_MEDIA_GRP_ID_CSI) &&
+ sd->entity.function != MEDIA_ENT_F_VID_MUX)
+ continue;
+
+ fwnode = (sd->grp_id & IMX_MEDIA_GRP_ID_IPU_CSI) ?
+ dev_fwnode(sd->dev->parent) :
+ dev_fwnode(sd->dev);
+
+ /*
+ * if there are fwnode endpoint connections between
+ * the CSI-2 receiver and this CSI or video-mux,
+ * create media links for them.
+ */
+ __media_create_fwnode_links(&csi2->entity,
+ dev_fwnode(csi2->dev),
+ &sd->entity, fwnode, 0);
+ }
}
/*
@@ -196,9 +200,7 @@ int imx_media_probe_complete(struct v4l2_async_notifier *notifier)
mutex_lock(&imxmd->mutex);
- ret = imx_media_create_links(notifier);
- if (ret)
- goto unlock;
+ imx_media_create_csi2_links(imxmd);
ret = imx_media_create_pad_vdev_lists(imxmd);
if (ret)
diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c
index 736c954a8ff5..82e13e972e23 100644
--- a/drivers/staging/media/imx/imx-media-of.c
+++ b/drivers/staging/media/imx/imx-media-of.c
@@ -74,116 +74,3 @@ int imx_media_add_of_subdevs(struct imx_media_dev *imxmd,
return ret;
}
EXPORT_SYMBOL_GPL(imx_media_add_of_subdevs);
-
-/*
- * Create a single media link to/from sd using a fwnode link.
- *
- * NOTE: this function assumes an OF port node is equivalent to
- * a media pad (port id equal to media pad index), and that an
- * OF endpoint node is equivalent to a media link.
- */
-static int create_of_link(struct imx_media_dev *imxmd,
- struct v4l2_subdev *sd,
- struct fwnode_link *link)
-{
- struct v4l2_subdev *remote, *src, *sink;
- int src_pad, sink_pad;
-
- if (link->local.port >= sd->entity.num_pads)
- return -EINVAL;
-
- remote = imx_media_find_subdev_by_fwnode(imxmd,
- link->remote_port_parent);
- if (!remote)
- return 0;
-
- if (sd->entity.pads[link->local.port].flags & MEDIA_PAD_FL_SINK) {
- src = remote;
- src_pad = link->remote.port;
- sink = sd;
- sink_pad = link->local.port;
- } else {
- src = sd;
- src_pad = link->local.port;
- sink = remote;
- sink_pad = link->remote.port;
- }
-
- /* make sure link doesn't already exist before creating */
- if (media_entity_find_link(&src->entity.pads[src_pad],
- &sink->entity.pads[sink_pad]))
- return 0;
-
- v4l2_info(sd->v4l2_dev, "%s:%d -> %s:%d\n",
- src->name, src_pad, sink->name, sink_pad);
-
- return media_create_pad_link(&src->entity, src_pad,
- &sink->entity, sink_pad, 0);
-}
-
-/*
- * Create media links to/from sd using its device-tree endpoints.
- */
-int imx_media_create_of_links(struct imx_media_dev *imxmd,
- struct v4l2_subdev *sd)
-{
- struct fwnode_handle *endpoint;
- struct fwnode_link link;
- int ret;
-
- fwnode_graph_for_each_endpoint(dev_fwnode(sd->dev), endpoint) {
- ret = fwnode_graph_parse_link(endpoint, &link);
- if (ret)
- continue;
-
- ret = create_of_link(imxmd, sd, &link);
- fwnode_graph_put_link(&link);
- if (ret)
- return ret;
- }
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(imx_media_create_of_links);
-
-/*
- * Create media links to the given CSI subdevice's sink pads,
- * using its device-tree endpoints.
- */
-int imx_media_create_csi_of_links(struct imx_media_dev *imxmd,
- struct v4l2_subdev *csi)
-{
- struct fwnode_handle *csi_np = dev_fwnode(csi->dev);
- struct fwnode_handle *csi_ep;
-
- fwnode_for_each_child_node(csi_np, csi_ep) {
- struct fwnode_handle *fwnode;
- struct fwnode_link link;
- int ret;
-
- memset(&link, 0, sizeof(link));
-
- link.local_port_parent = csi_np;
- link.local.port = CSI_SINK_PAD;
-
- fwnode = fwnode_graph_get_remote_endpoint(csi_ep);
- if (!fwnode)
- continue;
-
- fwnode = fwnode_get_parent(fwnode);
- fwnode_property_read_u32(fwnode, "reg", &link.remote.port);
- fwnode = fwnode_get_next_parent(fwnode);
- if (is_of_node(fwnode) &&
- of_node_name_eq(to_of_node(fwnode), "ports"))
- fwnode = fwnode_get_next_parent(fwnode);
- link.remote_port_parent = fwnode;
-
- ret = create_of_link(imxmd, csi, &link);
- fwnode_handle_put(link.remote_port_parent);
- if (ret)
- return ret;
- }
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(imx_media_create_csi_of_links);
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index 3363af07a5ca..d462759f583c 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -244,10 +244,6 @@ void imx_media_unregister_ipu_internal_subdevs(struct imx_media_dev *imxmd);
/* imx-media-of.c */
int imx_media_add_of_subdevs(struct imx_media_dev *dev,
struct device_node *np);
-int imx_media_create_of_links(struct imx_media_dev *imxmd,
- struct v4l2_subdev *sd);
-int imx_media_create_csi_of_links(struct imx_media_dev *imxmd,
- struct v4l2_subdev *csi);
int imx_media_of_add_csi(struct imx_media_dev *imxmd,
struct device_node *csi_np);
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v2 23/23] media: imx: TODO: Remove media link creation todos
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (21 preceding siblings ...)
2019-11-24 19:07 ` [PATCH v2 22/23] media: imx: Create missing links from CSI-2 receiver Steve Longerbeam
@ 2019-11-24 19:07 ` Steve Longerbeam
2019-11-27 2:10 ` [PATCH v2 00/23] media: imx: Create media links in bound notifiers Adam Ford
2019-12-16 17:20 ` Adam Ford
24 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-24 19:07 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam
Remove the TODO items regarding media link creation, these issues are
resolved after exporting media link creation to individual entity bound
callbacks and the use of media_create_fwnode_links(),
media_create_fwnode_pad_links(), and media_entity_get_fwnode_pad().
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/staging/media/imx/TODO | 29 -----------------------------
1 file changed, 29 deletions(-)
diff --git a/drivers/staging/media/imx/TODO b/drivers/staging/media/imx/TODO
index 6f29b5ca5324..a371cdedcdb0 100644
--- a/drivers/staging/media/imx/TODO
+++ b/drivers/staging/media/imx/TODO
@@ -17,35 +17,6 @@
decided whether this feature is useful enough to make it generally
available by exporting to v4l2-core.
-- After all async subdevices have been bound, v4l2_fwnode_parse_link()
- is used to form the media links between the devices discovered in
- the OF graph.
-
- While this approach allows support for arbitrary OF graphs, there
- are some assumptions for this to work:
-
- 1. If a port owned by a device in the graph has endpoint nodes, the
- port is treated as a media pad.
-
- This presents problems for devices that don't make this port = pad
- assumption. Examples are SMIAPP compatible cameras which define only
- a single output port node, but which define multiple pads owned
- by multiple subdevices (pixel-array, binner, scaler). Or video
- decoders (entity function MEDIA_ENT_F_ATV_DECODER), which also define
- only a single output port node, but define multiple pads for video,
- VBI, and audio out.
-
- A workaround at present is to set the port reg properties to
- correspond to the media pad index that the port represents. A
- possible long-term solution is to implement a subdev API that
- maps a port id to a media pad index.
-
- 2. Every endpoint of a port owned by a device in the graph is treated
- as a media link.
-
- Which means a port must not contain mixed-use endpoints, they
- must all refer to media links between V4L2 subdevices.
-
- i.MX7: all of the above, since it uses the imx media core
- i.MX7: use Frame Interval Monitor
--
2.17.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v2 00/23] media: imx: Create media links in bound notifiers
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (22 preceding siblings ...)
2019-11-24 19:07 ` [PATCH v2 23/23] media: imx: TODO: Remove media link creation todos Steve Longerbeam
@ 2019-11-27 2:10 ` Adam Ford
2019-11-27 2:23 ` Steve Longerbeam
2019-12-16 17:20 ` Adam Ford
24 siblings, 1 reply; 38+ messages in thread
From: Adam Ford @ 2019-11-27 2:10 UTC (permalink / raw)
To: Steve Longerbeam; +Cc: linux-media
On Sun, Nov 24, 2019 at 1:08 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
> Move media link creation into the notifier bound callbacks in the
> subdevices required by imx, making use of media_entity_get_fwnode_pad().
>
> In the process, improve the behavior of media_entity_get_fwnode_pad().
> The function is also being used inconsistently by current callers, so
> that is fixed in this serieas as well.
>
> The default behavior of media_entity_get_fwnode_pad() fails when
> the entity has multiple sink and/or source pads. Wrt imx, there are
> two such entities: the imx6 csi-2 receiver, and the video mux.
Out of curiosity, what is required to get the imx drivers out of
staging and into the regular media directory?
At least for the i.MX6, the pipeline and CSI2 drivers appear to be
functioning well and have for some time.
I haven't had a chance to try this series, but on the surface, this
seems reasonable.
adam
>
> Modify the default behavior of media_entity_get_fwnode_pad() to first
> determine if the port number at the provided endpoint firmware node
> corresponds to a valid pad for the entity. That way the default behavior
> will work for any entities that implement 1:1 mappings, without requiring
> they implement the .get_fwnode_pad op.
>
> In other words, the old behavior of media_entity_get_fwnode_pad() required
> entities implement .get_fwnode_pad if they have multiple sink or source pads.
> The new behavior requires entities implement .get_fwnode_pad only if they
> have multiple sink or source pads, and do not have 1:1 port:pad mappings.
>
> Also, as part of the above work, it was discovered that all of the
> current callers of media_entity_get_fwnode_pad() are not using that
> function in a consistent way. In all but one case the driver passes the
> firmware node of the subdevice itself to the function, not one of it
> endpoint nodes as the function expects. In more detail:
>
> - Cadence MIPI-CSI2 Receiver,
> - ST Micro MIPID02 CSI-2,
> - Allwinner V3s CSI,
> - Allwinner A10 CSI,
> - STM32 DCMI:
> These drivers call media_entity_get_fwnode_pad() in the subdev bound
> notifier callback, but passes sd->fwnode to the function. This is
> usually the fwnode of the subdevice itself, not one of its endpoint
> nodes. This only works now because there are no implementations of the
> .get_fwnode_pad op. This will break as soon as the bound subdevice
> implements the .get_fwnode_pad op.
>
> - Renesas R-Car MIPI CSI-2 Receiver:
> Calls media_entity_get_fwnode_pad() in the subdev bound notifier
> callback. In this case the driver passes the async subdev descriptor
> match fwnode. Again for most subdevices, this is the fwnode of the
> subdevice itself, not one of its endpoint nodes. However on the
> Salvator-X platform, the bound subdevice happens to be the ADV748x
> CSI-2 transmitter, which does indeed place the endpoint node in the
> asd match fwnode. But the problem is that the driver is now making
> assumptions about what subdevices it is binding to.
>
> To resolve the above issues, this series adds a new function
> media_create_fwnode_pad_links(), which creates the media links from/to
> all pads on a remote entity, to/from a single specific local entity pad,
> using the fwnode endpoint connections between them. It's API makes it
> convenient to call from subdev bound notifier callbacks.
>
> Another function media_create_fwnode_links() is added that will create
> links from/to all pads on a remote entity, to/from all pads on a local
> entity. It's API also makes it convenient to call from subdev bound
> notifier callbacks, and can be called for entities that link to remote
> entities via multiple pads (for example the video-mux which has multiple
> sink pads that link to multiple bound subdevices).
>
> This series has been tested on i.MX6Q SabreSD, SabreLite, and SabreAuto
> platforms, and the Renesas Salvator-X platform.
>
> The series needs testing on i.MX7, Cadence, ST Micro MIPID02, STM32 DCMI,
> and Allwinner V3s and A10 platforms. Testing only needs to verify that the
> media graph is unchanged, no stream testing is required. If the media graph
> is broken, it means the subdevice(s) that bind to the drivers listed
> above need to implement the .get_fwnode_pad op.
>
> History:
>
> v2:
> - rename/move the notifier-to-state inlines in imx7-mipi-csis.c and
> imx7-media-csi.c, suggested by Rui Silva.
> - rewrite imx_media_create_links() to only add the missing media links
> from the imx6 MIPI CSI-2 receiver.
>
>
> Steve Longerbeam (23):
> media: entity: Pass entity to get_fwnode_pad operation
> media: entity: Modify default behavior of media_entity_get_fwnode_pad
> media: entity: Convert media_entity_get_fwnode_pad() args to const
> media: Move v4l2_fwnode_parse_link from v4l2 to driver base
> media: entity: Add functions to convert fwnode endpoints to media
> links
> media: adv748x: csi2: Implement get_fwnode_pad
> media: rcar-csi2: Fix fwnode media link creation
> media: cadence: csi2rx: Fix fwnode media link creation
> media: sun6i: Fix fwnode media link creation
> media: st-mipid02: Fix fwnode media link creation
> media: stm32-dcmi: Fix fwnode media link creation
> media: sunxi: Fix fwnode media link creation
> media: v4l2-fwnode: Pass notifier to
> v4l2_async_register_fwnode_subdev()
> media: video-mux: Create media links in bound notifier
> media: imx: mipi csi-2: Create media links in bound notifier
> media: imx7-mipi-csis: Create media links in bound notifier
> media: imx7-media-csi: Create media links in bound notifier
> media: imx: csi: Implement get_fwnode_pad
> media: imx: csi: Embed notifier in struct csi_priv
> media: imx: csi: Create media links in bound notifier
> media: imx: csi: Lookup upstream endpoint with
> imx_media_get_pad_fwnode
> media: imx: Create missing links from CSI-2 receiver
> media: imx: TODO: Remove media link creation todos
>
> drivers/base/property.c | 63 ++++++
> drivers/media/i2c/adv748x/adv748x-csi2.c | 21 ++
> drivers/media/i2c/st-mipid02.c | 20 +-
> drivers/media/mc/mc-entity.c | 209 +++++++++++++++++-
> drivers/media/platform/cadence/cdns-csi2rx.c | 27 +--
> drivers/media/platform/rcar-vin/rcar-csi2.c | 23 +-
> drivers/media/platform/stm32/stm32-dcmi.c | 15 +-
> .../platform/sunxi/sun4i-csi/sun4i_csi.c | 27 +--
> .../platform/sunxi/sun4i-csi/sun4i_csi.h | 1 -
> .../platform/sunxi/sun6i-csi/sun6i_csi.c | 36 +--
> drivers/media/platform/video-mux.c | 30 ++-
> drivers/media/platform/xilinx/xilinx-vipp.c | 69 +++---
> drivers/media/v4l2-core/v4l2-fwnode.c | 50 +----
> drivers/staging/media/imx/TODO | 29 ---
> drivers/staging/media/imx/imx-media-csi.c | 92 +++++---
> .../staging/media/imx/imx-media-dev-common.c | 60 ++---
> drivers/staging/media/imx/imx-media-of.c | 114 ----------
> drivers/staging/media/imx/imx-media-utils.c | 33 +++
> drivers/staging/media/imx/imx-media.h | 5 +-
> drivers/staging/media/imx/imx6-mipi-csi2.c | 29 ++-
> drivers/staging/media/imx/imx7-media-csi.c | 55 +++--
> drivers/staging/media/imx/imx7-mipi-csis.c | 30 ++-
> include/linux/fwnode.h | 14 ++
> include/linux/property.h | 5 +
> include/media/media-entity.h | 99 ++++++++-
> include/media/v4l2-fwnode.h | 56 +----
> 26 files changed, 735 insertions(+), 477 deletions(-)
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v2 00/23] media: imx: Create media links in bound notifiers
2019-11-27 2:10 ` [PATCH v2 00/23] media: imx: Create media links in bound notifiers Adam Ford
@ 2019-11-27 2:23 ` Steve Longerbeam
2019-11-27 2:31 ` Adam Ford
0 siblings, 1 reply; 38+ messages in thread
From: Steve Longerbeam @ 2019-11-27 2:23 UTC (permalink / raw)
To: Adam Ford; +Cc: linux-media
Hi Adam,
On 11/26/19 6:10 PM, Adam Ford wrote:
> On Sun, Nov 24, 2019 at 1:08 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>> Move media link creation into the notifier bound callbacks in the
>> subdevices required by imx, making use of media_entity_get_fwnode_pad().
>>
>> In the process, improve the behavior of media_entity_get_fwnode_pad().
>> The function is also being used inconsistently by current callers, so
>> that is fixed in this serieas as well.
>>
>> The default behavior of media_entity_get_fwnode_pad() fails when
>> the entity has multiple sink and/or source pads. Wrt imx, there are
>> two such entities: the imx6 csi-2 receiver, and the video mux.
> Out of curiosity, what is required to get the imx drivers out of
> staging and into the regular media directory?
>
> At least for the i.MX6, the pipeline and CSI2 drivers appear to be
> functioning well and have for some time.
>
> I haven't had a chance to try this series, but on the surface, this
> seems reasonable.
I may be biased, but I agree with you that the imx driver is ready to
move out of staging. And this series removes the important TODO items
that should make that happen. But I'm open to any issues that still need
to be addressed.
Steve
>
>> Modify the default behavior of media_entity_get_fwnode_pad() to first
>> determine if the port number at the provided endpoint firmware node
>> corresponds to a valid pad for the entity. That way the default behavior
>> will work for any entities that implement 1:1 mappings, without requiring
>> they implement the .get_fwnode_pad op.
>>
>> In other words, the old behavior of media_entity_get_fwnode_pad() required
>> entities implement .get_fwnode_pad if they have multiple sink or source pads.
>> The new behavior requires entities implement .get_fwnode_pad only if they
>> have multiple sink or source pads, and do not have 1:1 port:pad mappings.
>>
>> Also, as part of the above work, it was discovered that all of the
>> current callers of media_entity_get_fwnode_pad() are not using that
>> function in a consistent way. In all but one case the driver passes the
>> firmware node of the subdevice itself to the function, not one of it
>> endpoint nodes as the function expects. In more detail:
>>
>> - Cadence MIPI-CSI2 Receiver,
>> - ST Micro MIPID02 CSI-2,
>> - Allwinner V3s CSI,
>> - Allwinner A10 CSI,
>> - STM32 DCMI:
>> These drivers call media_entity_get_fwnode_pad() in the subdev bound
>> notifier callback, but passes sd->fwnode to the function. This is
>> usually the fwnode of the subdevice itself, not one of its endpoint
>> nodes. This only works now because there are no implementations of the
>> .get_fwnode_pad op. This will break as soon as the bound subdevice
>> implements the .get_fwnode_pad op.
>>
>> - Renesas R-Car MIPI CSI-2 Receiver:
>> Calls media_entity_get_fwnode_pad() in the subdev bound notifier
>> callback. In this case the driver passes the async subdev descriptor
>> match fwnode. Again for most subdevices, this is the fwnode of the
>> subdevice itself, not one of its endpoint nodes. However on the
>> Salvator-X platform, the bound subdevice happens to be the ADV748x
>> CSI-2 transmitter, which does indeed place the endpoint node in the
>> asd match fwnode. But the problem is that the driver is now making
>> assumptions about what subdevices it is binding to.
>>
>> To resolve the above issues, this series adds a new function
>> media_create_fwnode_pad_links(), which creates the media links from/to
>> all pads on a remote entity, to/from a single specific local entity pad,
>> using the fwnode endpoint connections between them. It's API makes it
>> convenient to call from subdev bound notifier callbacks.
>>
>> Another function media_create_fwnode_links() is added that will create
>> links from/to all pads on a remote entity, to/from all pads on a local
>> entity. It's API also makes it convenient to call from subdev bound
>> notifier callbacks, and can be called for entities that link to remote
>> entities via multiple pads (for example the video-mux which has multiple
>> sink pads that link to multiple bound subdevices).
>>
>> This series has been tested on i.MX6Q SabreSD, SabreLite, and SabreAuto
>> platforms, and the Renesas Salvator-X platform.
>>
>> The series needs testing on i.MX7, Cadence, ST Micro MIPID02, STM32 DCMI,
>> and Allwinner V3s and A10 platforms. Testing only needs to verify that the
>> media graph is unchanged, no stream testing is required. If the media graph
>> is broken, it means the subdevice(s) that bind to the drivers listed
>> above need to implement the .get_fwnode_pad op.
>>
>> History:
>>
>> v2:
>> - rename/move the notifier-to-state inlines in imx7-mipi-csis.c and
>> imx7-media-csi.c, suggested by Rui Silva.
>> - rewrite imx_media_create_links() to only add the missing media links
>> from the imx6 MIPI CSI-2 receiver.
>>
>>
>> Steve Longerbeam (23):
>> media: entity: Pass entity to get_fwnode_pad operation
>> media: entity: Modify default behavior of media_entity_get_fwnode_pad
>> media: entity: Convert media_entity_get_fwnode_pad() args to const
>> media: Move v4l2_fwnode_parse_link from v4l2 to driver base
>> media: entity: Add functions to convert fwnode endpoints to media
>> links
>> media: adv748x: csi2: Implement get_fwnode_pad
>> media: rcar-csi2: Fix fwnode media link creation
>> media: cadence: csi2rx: Fix fwnode media link creation
>> media: sun6i: Fix fwnode media link creation
>> media: st-mipid02: Fix fwnode media link creation
>> media: stm32-dcmi: Fix fwnode media link creation
>> media: sunxi: Fix fwnode media link creation
>> media: v4l2-fwnode: Pass notifier to
>> v4l2_async_register_fwnode_subdev()
>> media: video-mux: Create media links in bound notifier
>> media: imx: mipi csi-2: Create media links in bound notifier
>> media: imx7-mipi-csis: Create media links in bound notifier
>> media: imx7-media-csi: Create media links in bound notifier
>> media: imx: csi: Implement get_fwnode_pad
>> media: imx: csi: Embed notifier in struct csi_priv
>> media: imx: csi: Create media links in bound notifier
>> media: imx: csi: Lookup upstream endpoint with
>> imx_media_get_pad_fwnode
>> media: imx: Create missing links from CSI-2 receiver
>> media: imx: TODO: Remove media link creation todos
>>
>> drivers/base/property.c | 63 ++++++
>> drivers/media/i2c/adv748x/adv748x-csi2.c | 21 ++
>> drivers/media/i2c/st-mipid02.c | 20 +-
>> drivers/media/mc/mc-entity.c | 209 +++++++++++++++++-
>> drivers/media/platform/cadence/cdns-csi2rx.c | 27 +--
>> drivers/media/platform/rcar-vin/rcar-csi2.c | 23 +-
>> drivers/media/platform/stm32/stm32-dcmi.c | 15 +-
>> .../platform/sunxi/sun4i-csi/sun4i_csi.c | 27 +--
>> .../platform/sunxi/sun4i-csi/sun4i_csi.h | 1 -
>> .../platform/sunxi/sun6i-csi/sun6i_csi.c | 36 +--
>> drivers/media/platform/video-mux.c | 30 ++-
>> drivers/media/platform/xilinx/xilinx-vipp.c | 69 +++---
>> drivers/media/v4l2-core/v4l2-fwnode.c | 50 +----
>> drivers/staging/media/imx/TODO | 29 ---
>> drivers/staging/media/imx/imx-media-csi.c | 92 +++++---
>> .../staging/media/imx/imx-media-dev-common.c | 60 ++---
>> drivers/staging/media/imx/imx-media-of.c | 114 ----------
>> drivers/staging/media/imx/imx-media-utils.c | 33 +++
>> drivers/staging/media/imx/imx-media.h | 5 +-
>> drivers/staging/media/imx/imx6-mipi-csi2.c | 29 ++-
>> drivers/staging/media/imx/imx7-media-csi.c | 55 +++--
>> drivers/staging/media/imx/imx7-mipi-csis.c | 30 ++-
>> include/linux/fwnode.h | 14 ++
>> include/linux/property.h | 5 +
>> include/media/media-entity.h | 99 ++++++++-
>> include/media/v4l2-fwnode.h | 56 +----
>> 26 files changed, 735 insertions(+), 477 deletions(-)
>>
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 00/23] media: imx: Create media links in bound notifiers
2019-11-27 2:23 ` Steve Longerbeam
@ 2019-11-27 2:31 ` Adam Ford
0 siblings, 0 replies; 38+ messages in thread
From: Adam Ford @ 2019-11-27 2:31 UTC (permalink / raw)
To: Steve Longerbeam; +Cc: linux-media
On Tue, Nov 26, 2019 at 8:24 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
> Hi Adam,
>
> On 11/26/19 6:10 PM, Adam Ford wrote:
> > On Sun, Nov 24, 2019 at 1:08 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
> >> Move media link creation into the notifier bound callbacks in the
> >> subdevices required by imx, making use of media_entity_get_fwnode_pad().
> >>
> >> In the process, improve the behavior of media_entity_get_fwnode_pad().
> >> The function is also being used inconsistently by current callers, so
> >> that is fixed in this serieas as well.
> >>
> >> The default behavior of media_entity_get_fwnode_pad() fails when
> >> the entity has multiple sink and/or source pads. Wrt imx, there are
> >> two such entities: the imx6 csi-2 receiver, and the video mux.
> > Out of curiosity, what is required to get the imx drivers out of
> > staging and into the regular media directory?
> >
> > At least for the i.MX6, the pipeline and CSI2 drivers appear to be
> > functioning well and have for some time.
> >
> > I haven't had a chance to try this series, but on the surface, this
> > seems reasonable.
>
> I may be biased, but I agree with you that the imx driver is ready to
> move out of staging. And this series removes the important TODO items
> that should make that happen. But I'm open to any issues that still need
> to be addressed.
That's what I was thinking too.
In my mind, the entire kernel is always a work in progress and
undergoing continuing improvement, but having drivers in the 'staging'
area for years seems excessive since it's getting a lot of attention
lately.
I am going to try and test the series on my i.MX6Q after the US
Thanksgiving holiday is complete.
adam
>
> Steve
>
>
> >
> >> Modify the default behavior of media_entity_get_fwnode_pad() to first
> >> determine if the port number at the provided endpoint firmware node
> >> corresponds to a valid pad for the entity. That way the default behavior
> >> will work for any entities that implement 1:1 mappings, without requiring
> >> they implement the .get_fwnode_pad op.
> >>
> >> In other words, the old behavior of media_entity_get_fwnode_pad() required
> >> entities implement .get_fwnode_pad if they have multiple sink or source pads.
> >> The new behavior requires entities implement .get_fwnode_pad only if they
> >> have multiple sink or source pads, and do not have 1:1 port:pad mappings.
> >>
> >> Also, as part of the above work, it was discovered that all of the
> >> current callers of media_entity_get_fwnode_pad() are not using that
> >> function in a consistent way. In all but one case the driver passes the
> >> firmware node of the subdevice itself to the function, not one of it
> >> endpoint nodes as the function expects. In more detail:
> >>
> >> - Cadence MIPI-CSI2 Receiver,
> >> - ST Micro MIPID02 CSI-2,
> >> - Allwinner V3s CSI,
> >> - Allwinner A10 CSI,
> >> - STM32 DCMI:
> >> These drivers call media_entity_get_fwnode_pad() in the subdev bound
> >> notifier callback, but passes sd->fwnode to the function. This is
> >> usually the fwnode of the subdevice itself, not one of its endpoint
> >> nodes. This only works now because there are no implementations of the
> >> .get_fwnode_pad op. This will break as soon as the bound subdevice
> >> implements the .get_fwnode_pad op.
> >>
> >> - Renesas R-Car MIPI CSI-2 Receiver:
> >> Calls media_entity_get_fwnode_pad() in the subdev bound notifier
> >> callback. In this case the driver passes the async subdev descriptor
> >> match fwnode. Again for most subdevices, this is the fwnode of the
> >> subdevice itself, not one of its endpoint nodes. However on the
> >> Salvator-X platform, the bound subdevice happens to be the ADV748x
> >> CSI-2 transmitter, which does indeed place the endpoint node in the
> >> asd match fwnode. But the problem is that the driver is now making
> >> assumptions about what subdevices it is binding to.
> >>
> >> To resolve the above issues, this series adds a new function
> >> media_create_fwnode_pad_links(), which creates the media links from/to
> >> all pads on a remote entity, to/from a single specific local entity pad,
> >> using the fwnode endpoint connections between them. It's API makes it
> >> convenient to call from subdev bound notifier callbacks.
> >>
> >> Another function media_create_fwnode_links() is added that will create
> >> links from/to all pads on a remote entity, to/from all pads on a local
> >> entity. It's API also makes it convenient to call from subdev bound
> >> notifier callbacks, and can be called for entities that link to remote
> >> entities via multiple pads (for example the video-mux which has multiple
> >> sink pads that link to multiple bound subdevices).
> >>
> >> This series has been tested on i.MX6Q SabreSD, SabreLite, and SabreAuto
> >> platforms, and the Renesas Salvator-X platform.
> >>
> >> The series needs testing on i.MX7, Cadence, ST Micro MIPID02, STM32 DCMI,
> >> and Allwinner V3s and A10 platforms. Testing only needs to verify that the
> >> media graph is unchanged, no stream testing is required. If the media graph
> >> is broken, it means the subdevice(s) that bind to the drivers listed
> >> above need to implement the .get_fwnode_pad op.
> >>
> >> History:
> >>
> >> v2:
> >> - rename/move the notifier-to-state inlines in imx7-mipi-csis.c and
> >> imx7-media-csi.c, suggested by Rui Silva.
> >> - rewrite imx_media_create_links() to only add the missing media links
> >> from the imx6 MIPI CSI-2 receiver.
> >>
> >>
> >> Steve Longerbeam (23):
> >> media: entity: Pass entity to get_fwnode_pad operation
> >> media: entity: Modify default behavior of media_entity_get_fwnode_pad
> >> media: entity: Convert media_entity_get_fwnode_pad() args to const
> >> media: Move v4l2_fwnode_parse_link from v4l2 to driver base
> >> media: entity: Add functions to convert fwnode endpoints to media
> >> links
> >> media: adv748x: csi2: Implement get_fwnode_pad
> >> media: rcar-csi2: Fix fwnode media link creation
> >> media: cadence: csi2rx: Fix fwnode media link creation
> >> media: sun6i: Fix fwnode media link creation
> >> media: st-mipid02: Fix fwnode media link creation
> >> media: stm32-dcmi: Fix fwnode media link creation
> >> media: sunxi: Fix fwnode media link creation
> >> media: v4l2-fwnode: Pass notifier to
> >> v4l2_async_register_fwnode_subdev()
> >> media: video-mux: Create media links in bound notifier
> >> media: imx: mipi csi-2: Create media links in bound notifier
> >> media: imx7-mipi-csis: Create media links in bound notifier
> >> media: imx7-media-csi: Create media links in bound notifier
> >> media: imx: csi: Implement get_fwnode_pad
> >> media: imx: csi: Embed notifier in struct csi_priv
> >> media: imx: csi: Create media links in bound notifier
> >> media: imx: csi: Lookup upstream endpoint with
> >> imx_media_get_pad_fwnode
> >> media: imx: Create missing links from CSI-2 receiver
> >> media: imx: TODO: Remove media link creation todos
> >>
> >> drivers/base/property.c | 63 ++++++
> >> drivers/media/i2c/adv748x/adv748x-csi2.c | 21 ++
> >> drivers/media/i2c/st-mipid02.c | 20 +-
> >> drivers/media/mc/mc-entity.c | 209 +++++++++++++++++-
> >> drivers/media/platform/cadence/cdns-csi2rx.c | 27 +--
> >> drivers/media/platform/rcar-vin/rcar-csi2.c | 23 +-
> >> drivers/media/platform/stm32/stm32-dcmi.c | 15 +-
> >> .../platform/sunxi/sun4i-csi/sun4i_csi.c | 27 +--
> >> .../platform/sunxi/sun4i-csi/sun4i_csi.h | 1 -
> >> .../platform/sunxi/sun6i-csi/sun6i_csi.c | 36 +--
> >> drivers/media/platform/video-mux.c | 30 ++-
> >> drivers/media/platform/xilinx/xilinx-vipp.c | 69 +++---
> >> drivers/media/v4l2-core/v4l2-fwnode.c | 50 +----
> >> drivers/staging/media/imx/TODO | 29 ---
> >> drivers/staging/media/imx/imx-media-csi.c | 92 +++++---
> >> .../staging/media/imx/imx-media-dev-common.c | 60 ++---
> >> drivers/staging/media/imx/imx-media-of.c | 114 ----------
> >> drivers/staging/media/imx/imx-media-utils.c | 33 +++
> >> drivers/staging/media/imx/imx-media.h | 5 +-
> >> drivers/staging/media/imx/imx6-mipi-csi2.c | 29 ++-
> >> drivers/staging/media/imx/imx7-media-csi.c | 55 +++--
> >> drivers/staging/media/imx/imx7-mipi-csis.c | 30 ++-
> >> include/linux/fwnode.h | 14 ++
> >> include/linux/property.h | 5 +
> >> include/media/media-entity.h | 99 ++++++++-
> >> include/media/v4l2-fwnode.h | 56 +----
> >> 26 files changed, 735 insertions(+), 477 deletions(-)
> >>
> >> --
> >> 2.17.1
> >>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 00/23] media: imx: Create media links in bound notifiers
2019-11-24 19:06 [PATCH v2 00/23] media: imx: Create media links in bound notifiers Steve Longerbeam
` (23 preceding siblings ...)
2019-11-27 2:10 ` [PATCH v2 00/23] media: imx: Create media links in bound notifiers Adam Ford
@ 2019-12-16 17:20 ` Adam Ford
2020-02-02 19:30 ` Steve Longerbeam
24 siblings, 1 reply; 38+ messages in thread
From: Adam Ford @ 2019-12-16 17:20 UTC (permalink / raw)
To: Steve Longerbeam; +Cc: linux-media
On Sun, Nov 24, 2019 at 1:08 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
> Move media link creation into the notifier bound callbacks in the
> subdevices required by imx, making use of media_entity_get_fwnode_pad().
>
> In the process, improve the behavior of media_entity_get_fwnode_pad().
> The function is also being used inconsistently by current callers, so
> that is fixed in this serieas as well.
>
> The default behavior of media_entity_get_fwnode_pad() fails when
> the entity has multiple sink and/or source pads. Wrt imx, there are
> two such entities: the imx6 csi-2 receiver, and the video mux.
>
> Modify the default behavior of media_entity_get_fwnode_pad() to first
> determine if the port number at the provided endpoint firmware node
> corresponds to a valid pad for the entity. That way the default behavior
> will work for any entities that implement 1:1 mappings, without requiring
> they implement the .get_fwnode_pad op.
>
> In other words, the old behavior of media_entity_get_fwnode_pad() required
> entities implement .get_fwnode_pad if they have multiple sink or source pads.
> The new behavior requires entities implement .get_fwnode_pad only if they
> have multiple sink or source pads, and do not have 1:1 port:pad mappings.
>
> Also, as part of the above work, it was discovered that all of the
> current callers of media_entity_get_fwnode_pad() are not using that
> function in a consistent way. In all but one case the driver passes the
> firmware node of the subdevice itself to the function, not one of it
> endpoint nodes as the function expects. In more detail:
>
> - Cadence MIPI-CSI2 Receiver,
> - ST Micro MIPID02 CSI-2,
> - Allwinner V3s CSI,
> - Allwinner A10 CSI,
> - STM32 DCMI:
> These drivers call media_entity_get_fwnode_pad() in the subdev bound
> notifier callback, but passes sd->fwnode to the function. This is
> usually the fwnode of the subdevice itself, not one of its endpoint
> nodes. This only works now because there are no implementations of the
> .get_fwnode_pad op. This will break as soon as the bound subdevice
> implements the .get_fwnode_pad op.
>
> - Renesas R-Car MIPI CSI-2 Receiver:
> Calls media_entity_get_fwnode_pad() in the subdev bound notifier
> callback. In this case the driver passes the async subdev descriptor
> match fwnode. Again for most subdevices, this is the fwnode of the
> subdevice itself, not one of its endpoint nodes. However on the
> Salvator-X platform, the bound subdevice happens to be the ADV748x
> CSI-2 transmitter, which does indeed place the endpoint node in the
> asd match fwnode. But the problem is that the driver is now making
> assumptions about what subdevices it is binding to.
>
> To resolve the above issues, this series adds a new function
> media_create_fwnode_pad_links(), which creates the media links from/to
> all pads on a remote entity, to/from a single specific local entity pad,
> using the fwnode endpoint connections between them. It's API makes it
> convenient to call from subdev bound notifier callbacks.
>
> Another function media_create_fwnode_links() is added that will create
> links from/to all pads on a remote entity, to/from all pads on a local
> entity. It's API also makes it convenient to call from subdev bound
> notifier callbacks, and can be called for entities that link to remote
> entities via multiple pads (for example the video-mux which has multiple
> sink pads that link to multiple bound subdevices).
>
> This series has been tested on i.MX6Q SabreSD, SabreLite, and SabreAuto
> platforms, and the Renesas Salvator-X platform.
>
> The series needs testing on i.MX7, Cadence, ST Micro MIPID02, STM32 DCMI,
> and Allwinner V3s and A10 platforms. Testing only needs to verify that the
> media graph is unchanged, no stream testing is required. If the media graph
> is broken, it means the subdevice(s) that bind to the drivers listed
> above need to implement the .get_fwnode_pad op.
>
> History:
>
> v2:
> - rename/move the notifier-to-state inlines in imx7-mipi-csis.c and
> imx7-media-csi.c, suggested by Rui Silva.
> - rewrite imx_media_create_links() to only add the missing media links
> from the imx6 MIPI CSI-2 receiver.
>
I finally got around to testing the whole series on my i.MX6Q device
with a ov5640 sensor.
I get some error messages, and I was hoping you might have some ideas
or suggestion on what might be happening.
# gst-launch-1.0 -v v4l2src device=/dev/video4 ! kmssink name=imx-drm connector-
id=54 sync=0
Setting pipeline to PAUSED ...
Pipeline is live and does not need PREROLL ...
/GstPipeline:pipeline0/GstKMSSink:imx-drm: display-width = 1920
/GstPipeline:pipeline0/GstKMSSink:imx-drm: display-height = 1080
Setting pipeline to PLAYING ...
New clock: GstSystemClock
/GstPipeline:pipeline0/GstV4l2Src:v4l2src0.GstPad:src: caps =
video/x-raw, format=(string)YUY2, width=(int)640, height=(int)480,
framerate=(fraction)30/1, colorimetry[ 260.606952] ipu2_csi0:
pipeline start failed with -32
=(string)bt601, interlace-mode=(string)progressive
/GstPipeline:pipeline0/GstKMSSink:imx-drm.GstPad:sink: caps =
video/x-raw, format=(string)YUY2, width=(int)640, height=(int)480,
framerate=(fraction)30/1, colorimetry=(string)bt601,
interlace-mode=(string)progressive
ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Failed
to allocate required memory.
Additional debug info:
gstv4l2src.c(658): gst_v4l2src_decide_allocation ():
/GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
Buffer pool activation failed
Execution ended after 0:00:00.038167333
Setting pipeline to PAUSED ...
Setting pipeline to READY ...
Setting pipeline to NULL ...
Freeing pipeline ...
I visually verified my pipeline and tried to setup the video format
for each node in the pipeline:
media-ctl --links "'ov5640 2-0010':0->'imx6-mipi-csi2':0[1]"
media-ctl --links "'imx6-mipi-csi2':1->'ipu1_csi0_mux':0[1]"
media-ctl --links "'ipu1_csi0_mux':2->'ipu1_csi0':0[1]"
media-ctl --links "'ipu1_csi0':2->'ipu1_csi0 capture':0[1]"
media-ctl --set-v4l2 "'ov5640 2-0010':0[fmt:UYVY2X8/640x480 field:none]"
media-ctl --set-v4l2 "'imx6-mipi-csi2':0[fmt:UYVY2X8/640x480 field:none]"
media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/640x480 field:none]"
media-ctl --set-v4l2 "'ipu1_csi0_mux':0[fmt:UYVY2X8/640x480 field:none]"
media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/640x480 field:none]"
media-ctl --set-v4l2 "'ipu1_csi0':0[fmt:UYVY2X8/640x480 field:none]"
media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/640x480 field:none]"
I don't see any missing entries in the pipeline, so I was hoping you
might have some suggestions.
thanks
adam
>
> Steve Longerbeam (23):
> media: entity: Pass entity to get_fwnode_pad operation
> media: entity: Modify default behavior of media_entity_get_fwnode_pad
> media: entity: Convert media_entity_get_fwnode_pad() args to const
> media: Move v4l2_fwnode_parse_link from v4l2 to driver base
> media: entity: Add functions to convert fwnode endpoints to media
> links
> media: adv748x: csi2: Implement get_fwnode_pad
> media: rcar-csi2: Fix fwnode media link creation
> media: cadence: csi2rx: Fix fwnode media link creation
> media: sun6i: Fix fwnode media link creation
> media: st-mipid02: Fix fwnode media link creation
> media: stm32-dcmi: Fix fwnode media link creation
> media: sunxi: Fix fwnode media link creation
> media: v4l2-fwnode: Pass notifier to
> v4l2_async_register_fwnode_subdev()
> media: video-mux: Create media links in bound notifier
> media: imx: mipi csi-2: Create media links in bound notifier
> media: imx7-mipi-csis: Create media links in bound notifier
> media: imx7-media-csi: Create media links in bound notifier
> media: imx: csi: Implement get_fwnode_pad
> media: imx: csi: Embed notifier in struct csi_priv
> media: imx: csi: Create media links in bound notifier
> media: imx: csi: Lookup upstream endpoint with
> imx_media_get_pad_fwnode
> media: imx: Create missing links from CSI-2 receiver
> media: imx: TODO: Remove media link creation todos
>
> drivers/base/property.c | 63 ++++++
> drivers/media/i2c/adv748x/adv748x-csi2.c | 21 ++
> drivers/media/i2c/st-mipid02.c | 20 +-
> drivers/media/mc/mc-entity.c | 209 +++++++++++++++++-
> drivers/media/platform/cadence/cdns-csi2rx.c | 27 +--
> drivers/media/platform/rcar-vin/rcar-csi2.c | 23 +-
> drivers/media/platform/stm32/stm32-dcmi.c | 15 +-
> .../platform/sunxi/sun4i-csi/sun4i_csi.c | 27 +--
> .../platform/sunxi/sun4i-csi/sun4i_csi.h | 1 -
> .../platform/sunxi/sun6i-csi/sun6i_csi.c | 36 +--
> drivers/media/platform/video-mux.c | 30 ++-
> drivers/media/platform/xilinx/xilinx-vipp.c | 69 +++---
> drivers/media/v4l2-core/v4l2-fwnode.c | 50 +----
> drivers/staging/media/imx/TODO | 29 ---
> drivers/staging/media/imx/imx-media-csi.c | 92 +++++---
> .../staging/media/imx/imx-media-dev-common.c | 60 ++---
> drivers/staging/media/imx/imx-media-of.c | 114 ----------
> drivers/staging/media/imx/imx-media-utils.c | 33 +++
> drivers/staging/media/imx/imx-media.h | 5 +-
> drivers/staging/media/imx/imx6-mipi-csi2.c | 29 ++-
> drivers/staging/media/imx/imx7-media-csi.c | 55 +++--
> drivers/staging/media/imx/imx7-mipi-csis.c | 30 ++-
> include/linux/fwnode.h | 14 ++
> include/linux/property.h | 5 +
> include/media/media-entity.h | 99 ++++++++-
> include/media/v4l2-fwnode.h | 56 +----
> 26 files changed, 735 insertions(+), 477 deletions(-)
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v2 00/23] media: imx: Create media links in bound notifiers
2019-12-16 17:20 ` Adam Ford
@ 2020-02-02 19:30 ` Steve Longerbeam
2020-02-04 10:53 ` Adam Ford
0 siblings, 1 reply; 38+ messages in thread
From: Steve Longerbeam @ 2020-02-02 19:30 UTC (permalink / raw)
To: Adam Ford; +Cc: linux-media
Hi Adam,
On 12/16/19 9:20 AM, Adam Ford wrote:
> On Sun, Nov 24, 2019 at 1:08 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>> Move media link creation into the notifier bound callbacks in the
>> subdevices required by imx, making use of media_entity_get_fwnode_pad().
>>
>> In the process, improve the behavior of media_entity_get_fwnode_pad().
>> The function is also being used inconsistently by current callers, so
>> that is fixed in this serieas as well.
>>
>> The default behavior of media_entity_get_fwnode_pad() fails when
>> the entity has multiple sink and/or source pads. Wrt imx, there are
>> two such entities: the imx6 csi-2 receiver, and the video mux.
>>
>> Modify the default behavior of media_entity_get_fwnode_pad() to first
>> determine if the port number at the provided endpoint firmware node
>> corresponds to a valid pad for the entity. That way the default behavior
>> will work for any entities that implement 1:1 mappings, without requiring
>> they implement the .get_fwnode_pad op.
>>
>> In other words, the old behavior of media_entity_get_fwnode_pad() required
>> entities implement .get_fwnode_pad if they have multiple sink or source pads.
>> The new behavior requires entities implement .get_fwnode_pad only if they
>> have multiple sink or source pads, and do not have 1:1 port:pad mappings.
>>
>> Also, as part of the above work, it was discovered that all of the
>> current callers of media_entity_get_fwnode_pad() are not using that
>> function in a consistent way. In all but one case the driver passes the
>> firmware node of the subdevice itself to the function, not one of it
>> endpoint nodes as the function expects. In more detail:
>>
>> - Cadence MIPI-CSI2 Receiver,
>> - ST Micro MIPID02 CSI-2,
>> - Allwinner V3s CSI,
>> - Allwinner A10 CSI,
>> - STM32 DCMI:
>> These drivers call media_entity_get_fwnode_pad() in the subdev bound
>> notifier callback, but passes sd->fwnode to the function. This is
>> usually the fwnode of the subdevice itself, not one of its endpoint
>> nodes. This only works now because there are no implementations of the
>> .get_fwnode_pad op. This will break as soon as the bound subdevice
>> implements the .get_fwnode_pad op.
>>
>> - Renesas R-Car MIPI CSI-2 Receiver:
>> Calls media_entity_get_fwnode_pad() in the subdev bound notifier
>> callback. In this case the driver passes the async subdev descriptor
>> match fwnode. Again for most subdevices, this is the fwnode of the
>> subdevice itself, not one of its endpoint nodes. However on the
>> Salvator-X platform, the bound subdevice happens to be the ADV748x
>> CSI-2 transmitter, which does indeed place the endpoint node in the
>> asd match fwnode. But the problem is that the driver is now making
>> assumptions about what subdevices it is binding to.
>>
>> To resolve the above issues, this series adds a new function
>> media_create_fwnode_pad_links(), which creates the media links from/to
>> all pads on a remote entity, to/from a single specific local entity pad,
>> using the fwnode endpoint connections between them. It's API makes it
>> convenient to call from subdev bound notifier callbacks.
>>
>> Another function media_create_fwnode_links() is added that will create
>> links from/to all pads on a remote entity, to/from all pads on a local
>> entity. It's API also makes it convenient to call from subdev bound
>> notifier callbacks, and can be called for entities that link to remote
>> entities via multiple pads (for example the video-mux which has multiple
>> sink pads that link to multiple bound subdevices).
>>
>> This series has been tested on i.MX6Q SabreSD, SabreLite, and SabreAuto
>> platforms, and the Renesas Salvator-X platform.
>>
>> The series needs testing on i.MX7, Cadence, ST Micro MIPID02, STM32 DCMI,
>> and Allwinner V3s and A10 platforms. Testing only needs to verify that the
>> media graph is unchanged, no stream testing is required. If the media graph
>> is broken, it means the subdevice(s) that bind to the drivers listed
>> above need to implement the .get_fwnode_pad op.
>>
>> History:
>>
>> v2:
>> - rename/move the notifier-to-state inlines in imx7-mipi-csis.c and
>> imx7-media-csi.c, suggested by Rui Silva.
>> - rewrite imx_media_create_links() to only add the missing media links
>> from the imx6 MIPI CSI-2 receiver.
>>
> I finally got around to testing the whole series on my i.MX6Q device
> with a ov5640 sensor.
>
> I get some error messages, and I was hoping you might have some ideas
> or suggestion on what might be happening.
>
>
> # gst-launch-1.0 -v v4l2src device=/dev/video4 ! kmssink name=imx-drm connector-
> id=54 sync=0
> Setting pipeline to PAUSED ...
> Pipeline is live and does not need PREROLL ...
> /GstPipeline:pipeline0/GstKMSSink:imx-drm: display-width = 1920
> /GstPipeline:pipeline0/GstKMSSink:imx-drm: display-height = 1080
> Setting pipeline to PLAYING ...
> New clock: GstSystemClock
> /GstPipeline:pipeline0/GstV4l2Src:v4l2src0.GstPad:src: caps =
> video/x-raw, format=(string)YUY2, width=(int)640, height=(int)480,
> framerate=(fraction)30/1, colorimetry[ 260.606952] ipu2_csi0:
> pipeline start failed with -32
> =(string)bt601, interlace-mode=(string)progressive
> /GstPipeline:pipeline0/GstKMSSink:imx-drm.GstPad:sink: caps =
> video/x-raw, format=(string)YUY2, width=(int)640, height=(int)480,
> framerate=(fraction)30/1, colorimetry=(string)bt601,
> interlace-mode=(string)progressive
> ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Failed
> to allocate required memory.
> Additional debug info:
> gstv4l2src.c(658): gst_v4l2src_decide_allocation ():
> /GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
> Buffer pool activation failed
> Execution ended after 0:00:00.038167333
> Setting pipeline to PAUSED ...
> Setting pipeline to READY ...
> Setting pipeline to NULL ...
> Freeing pipeline ...
>
> I visually verified my pipeline and tried to setup the video format
> for each node in the pipeline:
>
> media-ctl --links "'ov5640 2-0010':0->'imx6-mipi-csi2':0[1]"
> media-ctl --links "'imx6-mipi-csi2':1->'ipu1_csi0_mux':0[1]"
> media-ctl --links "'ipu1_csi0_mux':2->'ipu1_csi0':0[1]"
> media-ctl --links "'ipu1_csi0':2->'ipu1_csi0 capture':0[1]"
>
> media-ctl --set-v4l2 "'ov5640 2-0010':0[fmt:UYVY2X8/640x480 field:none]"
> media-ctl --set-v4l2 "'imx6-mipi-csi2':0[fmt:UYVY2X8/640x480 field:none]"
> media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/640x480 field:none]"
> media-ctl --set-v4l2 "'ipu1_csi0_mux':0[fmt:UYVY2X8/640x480 field:none]"
> media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/640x480 field:none]"
> media-ctl --set-v4l2 "'ipu1_csi0':0[fmt:UYVY2X8/640x480 field:none]"
> media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/640x480 field:none]"
>
>
> I don't see any missing entries in the pipeline, so I was hoping you
> might have some suggestions.
I tried the exactly equivalent pipeline on quad SabreSD and don't see
any issue. But from your
ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Failed to allocate required memory.
it may be that you're platform was not able to allocate the buffer pools.
Steve
>
>> Steve Longerbeam (23):
>> media: entity: Pass entity to get_fwnode_pad operation
>> media: entity: Modify default behavior of media_entity_get_fwnode_pad
>> media: entity: Convert media_entity_get_fwnode_pad() args to const
>> media: Move v4l2_fwnode_parse_link from v4l2 to driver base
>> media: entity: Add functions to convert fwnode endpoints to media
>> links
>> media: adv748x: csi2: Implement get_fwnode_pad
>> media: rcar-csi2: Fix fwnode media link creation
>> media: cadence: csi2rx: Fix fwnode media link creation
>> media: sun6i: Fix fwnode media link creation
>> media: st-mipid02: Fix fwnode media link creation
>> media: stm32-dcmi: Fix fwnode media link creation
>> media: sunxi: Fix fwnode media link creation
>> media: v4l2-fwnode: Pass notifier to
>> v4l2_async_register_fwnode_subdev()
>> media: video-mux: Create media links in bound notifier
>> media: imx: mipi csi-2: Create media links in bound notifier
>> media: imx7-mipi-csis: Create media links in bound notifier
>> media: imx7-media-csi: Create media links in bound notifier
>> media: imx: csi: Implement get_fwnode_pad
>> media: imx: csi: Embed notifier in struct csi_priv
>> media: imx: csi: Create media links in bound notifier
>> media: imx: csi: Lookup upstream endpoint with
>> imx_media_get_pad_fwnode
>> media: imx: Create missing links from CSI-2 receiver
>> media: imx: TODO: Remove media link creation todos
>>
>> drivers/base/property.c | 63 ++++++
>> drivers/media/i2c/adv748x/adv748x-csi2.c | 21 ++
>> drivers/media/i2c/st-mipid02.c | 20 +-
>> drivers/media/mc/mc-entity.c | 209 +++++++++++++++++-
>> drivers/media/platform/cadence/cdns-csi2rx.c | 27 +--
>> drivers/media/platform/rcar-vin/rcar-csi2.c | 23 +-
>> drivers/media/platform/stm32/stm32-dcmi.c | 15 +-
>> .../platform/sunxi/sun4i-csi/sun4i_csi.c | 27 +--
>> .../platform/sunxi/sun4i-csi/sun4i_csi.h | 1 -
>> .../platform/sunxi/sun6i-csi/sun6i_csi.c | 36 +--
>> drivers/media/platform/video-mux.c | 30 ++-
>> drivers/media/platform/xilinx/xilinx-vipp.c | 69 +++---
>> drivers/media/v4l2-core/v4l2-fwnode.c | 50 +----
>> drivers/staging/media/imx/TODO | 29 ---
>> drivers/staging/media/imx/imx-media-csi.c | 92 +++++---
>> .../staging/media/imx/imx-media-dev-common.c | 60 ++---
>> drivers/staging/media/imx/imx-media-of.c | 114 ----------
>> drivers/staging/media/imx/imx-media-utils.c | 33 +++
>> drivers/staging/media/imx/imx-media.h | 5 +-
>> drivers/staging/media/imx/imx6-mipi-csi2.c | 29 ++-
>> drivers/staging/media/imx/imx7-media-csi.c | 55 +++--
>> drivers/staging/media/imx/imx7-mipi-csis.c | 30 ++-
>> include/linux/fwnode.h | 14 ++
>> include/linux/property.h | 5 +
>> include/media/media-entity.h | 99 ++++++++-
>> include/media/v4l2-fwnode.h | 56 +----
>> 26 files changed, 735 insertions(+), 477 deletions(-)
>>
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 00/23] media: imx: Create media links in bound notifiers
2020-02-02 19:30 ` Steve Longerbeam
@ 2020-02-04 10:53 ` Adam Ford
2020-02-05 0:44 ` Steve Longerbeam
0 siblings, 1 reply; 38+ messages in thread
From: Adam Ford @ 2020-02-04 10:53 UTC (permalink / raw)
To: Steve Longerbeam; +Cc: linux-media
On Sun, Feb 2, 2020 at 1:30 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
> Hi Adam,
>
> On 12/16/19 9:20 AM, Adam Ford wrote:
> > On Sun, Nov 24, 2019 at 1:08 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
> >> Move media link creation into the notifier bound callbacks in the
> >> subdevices required by imx, making use of media_entity_get_fwnode_pad().
> >>
> >> In the process, improve the behavior of media_entity_get_fwnode_pad().
> >> The function is also being used inconsistently by current callers, so
> >> that is fixed in this serieas as well.
> >>
> >> The default behavior of media_entity_get_fwnode_pad() fails when
> >> the entity has multiple sink and/or source pads. Wrt imx, there are
> >> two such entities: the imx6 csi-2 receiver, and the video mux.
> >>
> >> Modify the default behavior of media_entity_get_fwnode_pad() to first
> >> determine if the port number at the provided endpoint firmware node
> >> corresponds to a valid pad for the entity. That way the default behavior
> >> will work for any entities that implement 1:1 mappings, without requiring
> >> they implement the .get_fwnode_pad op.
> >>
> >> In other words, the old behavior of media_entity_get_fwnode_pad() required
> >> entities implement .get_fwnode_pad if they have multiple sink or source pads.
> >> The new behavior requires entities implement .get_fwnode_pad only if they
> >> have multiple sink or source pads, and do not have 1:1 port:pad mappings.
> >>
> >> Also, as part of the above work, it was discovered that all of the
> >> current callers of media_entity_get_fwnode_pad() are not using that
> >> function in a consistent way. In all but one case the driver passes the
> >> firmware node of the subdevice itself to the function, not one of it
> >> endpoint nodes as the function expects. In more detail:
> >>
> >> - Cadence MIPI-CSI2 Receiver,
> >> - ST Micro MIPID02 CSI-2,
> >> - Allwinner V3s CSI,
> >> - Allwinner A10 CSI,
> >> - STM32 DCMI:
> >> These drivers call media_entity_get_fwnode_pad() in the subdev bound
> >> notifier callback, but passes sd->fwnode to the function. This is
> >> usually the fwnode of the subdevice itself, not one of its endpoint
> >> nodes. This only works now because there are no implementations of the
> >> .get_fwnode_pad op. This will break as soon as the bound subdevice
> >> implements the .get_fwnode_pad op.
> >>
> >> - Renesas R-Car MIPI CSI-2 Receiver:
> >> Calls media_entity_get_fwnode_pad() in the subdev bound notifier
> >> callback. In this case the driver passes the async subdev descriptor
> >> match fwnode. Again for most subdevices, this is the fwnode of the
> >> subdevice itself, not one of its endpoint nodes. However on the
> >> Salvator-X platform, the bound subdevice happens to be the ADV748x
> >> CSI-2 transmitter, which does indeed place the endpoint node in the
> >> asd match fwnode. But the problem is that the driver is now making
> >> assumptions about what subdevices it is binding to.
> >>
> >> To resolve the above issues, this series adds a new function
> >> media_create_fwnode_pad_links(), which creates the media links from/to
> >> all pads on a remote entity, to/from a single specific local entity pad,
> >> using the fwnode endpoint connections between them. It's API makes it
> >> convenient to call from subdev bound notifier callbacks.
> >>
> >> Another function media_create_fwnode_links() is added that will create
> >> links from/to all pads on a remote entity, to/from all pads on a local
> >> entity. It's API also makes it convenient to call from subdev bound
> >> notifier callbacks, and can be called for entities that link to remote
> >> entities via multiple pads (for example the video-mux which has multiple
> >> sink pads that link to multiple bound subdevices).
> >>
> >> This series has been tested on i.MX6Q SabreSD, SabreLite, and SabreAuto
> >> platforms, and the Renesas Salvator-X platform.
> >>
> >> The series needs testing on i.MX7, Cadence, ST Micro MIPID02, STM32 DCMI,
> >> and Allwinner V3s and A10 platforms. Testing only needs to verify that the
> >> media graph is unchanged, no stream testing is required. If the media graph
> >> is broken, it means the subdevice(s) that bind to the drivers listed
> >> above need to implement the .get_fwnode_pad op.
> >>
> >> History:
> >>
> >> v2:
> >> - rename/move the notifier-to-state inlines in imx7-mipi-csis.c and
> >> imx7-media-csi.c, suggested by Rui Silva.
> >> - rewrite imx_media_create_links() to only add the missing media links
> >> from the imx6 MIPI CSI-2 receiver.
> >>
> > I finally got around to testing the whole series on my i.MX6Q device
> > with a ov5640 sensor.
> >
> > I get some error messages, and I was hoping you might have some ideas
> > or suggestion on what might be happening.
> >
> >
> > # gst-launch-1.0 -v v4l2src device=/dev/video4 ! kmssink name=imx-drm connector-
> > id=54 sync=0
> > Setting pipeline to PAUSED ...
> > Pipeline is live and does not need PREROLL ...
> > /GstPipeline:pipeline0/GstKMSSink:imx-drm: display-width = 1920
> > /GstPipeline:pipeline0/GstKMSSink:imx-drm: display-height = 1080
> > Setting pipeline to PLAYING ...
> > New clock: GstSystemClock
> > /GstPipeline:pipeline0/GstV4l2Src:v4l2src0.GstPad:src: caps =
> > video/x-raw, format=(string)YUY2, width=(int)640, height=(int)480,
> > framerate=(fraction)30/1, colorimetry[ 260.606952] ipu2_csi0:
> > pipeline start failed with -32
> > =(string)bt601, interlace-mode=(string)progressive
> > /GstPipeline:pipeline0/GstKMSSink:imx-drm.GstPad:sink: caps =
> > video/x-raw, format=(string)YUY2, width=(int)640, height=(int)480,
> > framerate=(fraction)30/1, colorimetry=(string)bt601,
> > interlace-mode=(string)progressive
> > ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Failed
> > to allocate required memory.
> > Additional debug info:
> > gstv4l2src.c(658): gst_v4l2src_decide_allocation ():
> > /GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
> > Buffer pool activation failed
> > Execution ended after 0:00:00.038167333
> > Setting pipeline to PAUSED ...
> > Setting pipeline to READY ...
> > Setting pipeline to NULL ...
> > Freeing pipeline ...
> >
> > I visually verified my pipeline and tried to setup the video format
> > for each node in the pipeline:
> >
> > media-ctl --links "'ov5640 2-0010':0->'imx6-mipi-csi2':0[1]"
> > media-ctl --links "'imx6-mipi-csi2':1->'ipu1_csi0_mux':0[1]"
> > media-ctl --links "'ipu1_csi0_mux':2->'ipu1_csi0':0[1]"
> > media-ctl --links "'ipu1_csi0':2->'ipu1_csi0 capture':0[1]"
> >
> > media-ctl --set-v4l2 "'ov5640 2-0010':0[fmt:UYVY2X8/640x480 field:none]"
> > media-ctl --set-v4l2 "'imx6-mipi-csi2':0[fmt:UYVY2X8/640x480 field:none]"
> > media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/640x480 field:none]"
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':0[fmt:UYVY2X8/640x480 field:none]"
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/640x480 field:none]"
> > media-ctl --set-v4l2 "'ipu1_csi0':0[fmt:UYVY2X8/640x480 field:none]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/640x480 field:none]"
> >
> >
> > I don't see any missing entries in the pipeline, so I was hoping you
> > might have some suggestions.
>
> I tried the exactly equivalent pipeline on quad SabreSD and don't see
> any issue. But from your
>
> ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Failed to allocate required memory.
>
>
> it may be that you're platform was not able to allocate the buffer pools.
I don't want to stand in the way. I'm sure its fine if it works with
the SabreSD board. Maybe I did something wrong.
adam
>
>
> Steve
>
> >
> >> Steve Longerbeam (23):
> >> media: entity: Pass entity to get_fwnode_pad operation
> >> media: entity: Modify default behavior of media_entity_get_fwnode_pad
> >> media: entity: Convert media_entity_get_fwnode_pad() args to const
> >> media: Move v4l2_fwnode_parse_link from v4l2 to driver base
> >> media: entity: Add functions to convert fwnode endpoints to media
> >> links
> >> media: adv748x: csi2: Implement get_fwnode_pad
> >> media: rcar-csi2: Fix fwnode media link creation
> >> media: cadence: csi2rx: Fix fwnode media link creation
> >> media: sun6i: Fix fwnode media link creation
> >> media: st-mipid02: Fix fwnode media link creation
> >> media: stm32-dcmi: Fix fwnode media link creation
> >> media: sunxi: Fix fwnode media link creation
> >> media: v4l2-fwnode: Pass notifier to
> >> v4l2_async_register_fwnode_subdev()
> >> media: video-mux: Create media links in bound notifier
> >> media: imx: mipi csi-2: Create media links in bound notifier
> >> media: imx7-mipi-csis: Create media links in bound notifier
> >> media: imx7-media-csi: Create media links in bound notifier
> >> media: imx: csi: Implement get_fwnode_pad
> >> media: imx: csi: Embed notifier in struct csi_priv
> >> media: imx: csi: Create media links in bound notifier
> >> media: imx: csi: Lookup upstream endpoint with
> >> imx_media_get_pad_fwnode
> >> media: imx: Create missing links from CSI-2 receiver
> >> media: imx: TODO: Remove media link creation todos
> >>
> >> drivers/base/property.c | 63 ++++++
> >> drivers/media/i2c/adv748x/adv748x-csi2.c | 21 ++
> >> drivers/media/i2c/st-mipid02.c | 20 +-
> >> drivers/media/mc/mc-entity.c | 209 +++++++++++++++++-
> >> drivers/media/platform/cadence/cdns-csi2rx.c | 27 +--
> >> drivers/media/platform/rcar-vin/rcar-csi2.c | 23 +-
> >> drivers/media/platform/stm32/stm32-dcmi.c | 15 +-
> >> .../platform/sunxi/sun4i-csi/sun4i_csi.c | 27 +--
> >> .../platform/sunxi/sun4i-csi/sun4i_csi.h | 1 -
> >> .../platform/sunxi/sun6i-csi/sun6i_csi.c | 36 +--
> >> drivers/media/platform/video-mux.c | 30 ++-
> >> drivers/media/platform/xilinx/xilinx-vipp.c | 69 +++---
> >> drivers/media/v4l2-core/v4l2-fwnode.c | 50 +----
> >> drivers/staging/media/imx/TODO | 29 ---
> >> drivers/staging/media/imx/imx-media-csi.c | 92 +++++---
> >> .../staging/media/imx/imx-media-dev-common.c | 60 ++---
> >> drivers/staging/media/imx/imx-media-of.c | 114 ----------
> >> drivers/staging/media/imx/imx-media-utils.c | 33 +++
> >> drivers/staging/media/imx/imx-media.h | 5 +-
> >> drivers/staging/media/imx/imx6-mipi-csi2.c | 29 ++-
> >> drivers/staging/media/imx/imx7-media-csi.c | 55 +++--
> >> drivers/staging/media/imx/imx7-mipi-csis.c | 30 ++-
> >> include/linux/fwnode.h | 14 ++
> >> include/linux/property.h | 5 +
> >> include/media/media-entity.h | 99 ++++++++-
> >> include/media/v4l2-fwnode.h | 56 +----
> >> 26 files changed, 735 insertions(+), 477 deletions(-)
> >>
> >> --
> >> 2.17.1
> >>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2 00/23] media: imx: Create media links in bound notifiers
2020-02-04 10:53 ` Adam Ford
@ 2020-02-05 0:44 ` Steve Longerbeam
0 siblings, 0 replies; 38+ messages in thread
From: Steve Longerbeam @ 2020-02-05 0:44 UTC (permalink / raw)
To: Adam Ford; +Cc: linux-media
Hi Adam,
On 2/4/20 2:53 AM, Adam Ford wrote:
> On Sun, Feb 2, 2020 at 1:30 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>> Hi Adam,
>>
>> On 12/16/19 9:20 AM, Adam Ford wrote:
>>> On Sun, Nov 24, 2019 at 1:08 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>>>> Move media link creation into the notifier bound callbacks in the
>>>> subdevices required by imx, making use of media_entity_get_fwnode_pad().
>>>>
>>>> In the process, improve the behavior of media_entity_get_fwnode_pad().
>>>> The function is also being used inconsistently by current callers, so
>>>> that is fixed in this serieas as well.
>>>>
>>>> The default behavior of media_entity_get_fwnode_pad() fails when
>>>> the entity has multiple sink and/or source pads. Wrt imx, there are
>>>> two such entities: the imx6 csi-2 receiver, and the video mux.
>>>>
>>>> Modify the default behavior of media_entity_get_fwnode_pad() to first
>>>> determine if the port number at the provided endpoint firmware node
>>>> corresponds to a valid pad for the entity. That way the default behavior
>>>> will work for any entities that implement 1:1 mappings, without requiring
>>>> they implement the .get_fwnode_pad op.
>>>>
>>>> In other words, the old behavior of media_entity_get_fwnode_pad() required
>>>> entities implement .get_fwnode_pad if they have multiple sink or source pads.
>>>> The new behavior requires entities implement .get_fwnode_pad only if they
>>>> have multiple sink or source pads, and do not have 1:1 port:pad mappings.
>>>>
>>>> Also, as part of the above work, it was discovered that all of the
>>>> current callers of media_entity_get_fwnode_pad() are not using that
>>>> function in a consistent way. In all but one case the driver passes the
>>>> firmware node of the subdevice itself to the function, not one of it
>>>> endpoint nodes as the function expects. In more detail:
>>>>
>>>> - Cadence MIPI-CSI2 Receiver,
>>>> - ST Micro MIPID02 CSI-2,
>>>> - Allwinner V3s CSI,
>>>> - Allwinner A10 CSI,
>>>> - STM32 DCMI:
>>>> These drivers call media_entity_get_fwnode_pad() in the subdev bound
>>>> notifier callback, but passes sd->fwnode to the function. This is
>>>> usually the fwnode of the subdevice itself, not one of its endpoint
>>>> nodes. This only works now because there are no implementations of the
>>>> .get_fwnode_pad op. This will break as soon as the bound subdevice
>>>> implements the .get_fwnode_pad op.
>>>>
>>>> - Renesas R-Car MIPI CSI-2 Receiver:
>>>> Calls media_entity_get_fwnode_pad() in the subdev bound notifier
>>>> callback. In this case the driver passes the async subdev descriptor
>>>> match fwnode. Again for most subdevices, this is the fwnode of the
>>>> subdevice itself, not one of its endpoint nodes. However on the
>>>> Salvator-X platform, the bound subdevice happens to be the ADV748x
>>>> CSI-2 transmitter, which does indeed place the endpoint node in the
>>>> asd match fwnode. But the problem is that the driver is now making
>>>> assumptions about what subdevices it is binding to.
>>>>
>>>> To resolve the above issues, this series adds a new function
>>>> media_create_fwnode_pad_links(), which creates the media links from/to
>>>> all pads on a remote entity, to/from a single specific local entity pad,
>>>> using the fwnode endpoint connections between them. It's API makes it
>>>> convenient to call from subdev bound notifier callbacks.
>>>>
>>>> Another function media_create_fwnode_links() is added that will create
>>>> links from/to all pads on a remote entity, to/from all pads on a local
>>>> entity. It's API also makes it convenient to call from subdev bound
>>>> notifier callbacks, and can be called for entities that link to remote
>>>> entities via multiple pads (for example the video-mux which has multiple
>>>> sink pads that link to multiple bound subdevices).
>>>>
>>>> This series has been tested on i.MX6Q SabreSD, SabreLite, and SabreAuto
>>>> platforms, and the Renesas Salvator-X platform.
>>>>
>>>> The series needs testing on i.MX7, Cadence, ST Micro MIPID02, STM32 DCMI,
>>>> and Allwinner V3s and A10 platforms. Testing only needs to verify that the
>>>> media graph is unchanged, no stream testing is required. If the media graph
>>>> is broken, it means the subdevice(s) that bind to the drivers listed
>>>> above need to implement the .get_fwnode_pad op.
>>>>
>>>> History:
>>>>
>>>> v2:
>>>> - rename/move the notifier-to-state inlines in imx7-mipi-csis.c and
>>>> imx7-media-csi.c, suggested by Rui Silva.
>>>> - rewrite imx_media_create_links() to only add the missing media links
>>>> from the imx6 MIPI CSI-2 receiver.
>>>>
>>> I finally got around to testing the whole series on my i.MX6Q device
>>> with a ov5640 sensor.
>>>
>>> I get some error messages, and I was hoping you might have some ideas
>>> or suggestion on what might be happening.
>>>
>>>
>>> # gst-launch-1.0 -v v4l2src device=/dev/video4 ! kmssink name=imx-drm connector-
>>> id=54 sync=0
>>> Setting pipeline to PAUSED ...
>>> Pipeline is live and does not need PREROLL ...
>>> /GstPipeline:pipeline0/GstKMSSink:imx-drm: display-width = 1920
>>> /GstPipeline:pipeline0/GstKMSSink:imx-drm: display-height = 1080
>>> Setting pipeline to PLAYING ...
>>> New clock: GstSystemClock
>>> /GstPipeline:pipeline0/GstV4l2Src:v4l2src0.GstPad:src: caps =
>>> video/x-raw, format=(string)YUY2, width=(int)640, height=(int)480,
>>> framerate=(fraction)30/1, colorimetry[ 260.606952] ipu2_csi0:
>>> pipeline start failed with -32
>>> =(string)bt601, interlace-mode=(string)progressive
>>> /GstPipeline:pipeline0/GstKMSSink:imx-drm.GstPad:sink: caps =
>>> video/x-raw, format=(string)YUY2, width=(int)640, height=(int)480,
>>> framerate=(fraction)30/1, colorimetry=(string)bt601,
>>> interlace-mode=(string)progressive
>>> ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Failed
>>> to allocate required memory.
>>> Additional debug info:
>>> gstv4l2src.c(658): gst_v4l2src_decide_allocation ():
>>> /GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
>>> Buffer pool activation failed
>>> Execution ended after 0:00:00.038167333
>>> Setting pipeline to PAUSED ...
>>> Setting pipeline to READY ...
>>> Setting pipeline to NULL ...
>>> Freeing pipeline ...
>>>
>>> I visually verified my pipeline and tried to setup the video format
>>> for each node in the pipeline:
>>>
>>> media-ctl --links "'ov5640 2-0010':0->'imx6-mipi-csi2':0[1]"
>>> media-ctl --links "'imx6-mipi-csi2':1->'ipu1_csi0_mux':0[1]"
>>> media-ctl --links "'ipu1_csi0_mux':2->'ipu1_csi0':0[1]"
>>> media-ctl --links "'ipu1_csi0':2->'ipu1_csi0 capture':0[1]"
>>>
>>> media-ctl --set-v4l2 "'ov5640 2-0010':0[fmt:UYVY2X8/640x480 field:none]"
>>> media-ctl --set-v4l2 "'imx6-mipi-csi2':0[fmt:UYVY2X8/640x480 field:none]"
>>> media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY2X8/640x480 field:none]"
>>> media-ctl --set-v4l2 "'ipu1_csi0_mux':0[fmt:UYVY2X8/640x480 field:none]"
>>> media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/640x480 field:none]"
>>> media-ctl --set-v4l2 "'ipu1_csi0':0[fmt:UYVY2X8/640x480 field:none]"
>>> media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/640x480 field:none]"
>>>
>>>
>>> I don't see any missing entries in the pipeline, so I was hoping you
>>> might have some suggestions.
>> I tried the exactly equivalent pipeline on quad SabreSD and don't see
>> any issue. But from your
>>
>> ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Failed to allocate required memory.
>>
>>
>> it may be that you're platform was not able to allocate the buffer pools.
> I don't want to stand in the way. I'm sure its fine if it works with
> the SabreSD board. Maybe I did something wrong.
Can you re-test with v3 I just posted on your i.MX6Q device with the
ov5640 sensor?
TIA,
Steve
>
>>>> Steve Longerbeam (23):
>>>> media: entity: Pass entity to get_fwnode_pad operation
>>>> media: entity: Modify default behavior of media_entity_get_fwnode_pad
>>>> media: entity: Convert media_entity_get_fwnode_pad() args to const
>>>> media: Move v4l2_fwnode_parse_link from v4l2 to driver base
>>>> media: entity: Add functions to convert fwnode endpoints to media
>>>> links
>>>> media: adv748x: csi2: Implement get_fwnode_pad
>>>> media: rcar-csi2: Fix fwnode media link creation
>>>> media: cadence: csi2rx: Fix fwnode media link creation
>>>> media: sun6i: Fix fwnode media link creation
>>>> media: st-mipid02: Fix fwnode media link creation
>>>> media: stm32-dcmi: Fix fwnode media link creation
>>>> media: sunxi: Fix fwnode media link creation
>>>> media: v4l2-fwnode: Pass notifier to
>>>> v4l2_async_register_fwnode_subdev()
>>>> media: video-mux: Create media links in bound notifier
>>>> media: imx: mipi csi-2: Create media links in bound notifier
>>>> media: imx7-mipi-csis: Create media links in bound notifier
>>>> media: imx7-media-csi: Create media links in bound notifier
>>>> media: imx: csi: Implement get_fwnode_pad
>>>> media: imx: csi: Embed notifier in struct csi_priv
>>>> media: imx: csi: Create media links in bound notifier
>>>> media: imx: csi: Lookup upstream endpoint with
>>>> imx_media_get_pad_fwnode
>>>> media: imx: Create missing links from CSI-2 receiver
>>>> media: imx: TODO: Remove media link creation todos
>>>>
>>>> drivers/base/property.c | 63 ++++++
>>>> drivers/media/i2c/adv748x/adv748x-csi2.c | 21 ++
>>>> drivers/media/i2c/st-mipid02.c | 20 +-
>>>> drivers/media/mc/mc-entity.c | 209 +++++++++++++++++-
>>>> drivers/media/platform/cadence/cdns-csi2rx.c | 27 +--
>>>> drivers/media/platform/rcar-vin/rcar-csi2.c | 23 +-
>>>> drivers/media/platform/stm32/stm32-dcmi.c | 15 +-
>>>> .../platform/sunxi/sun4i-csi/sun4i_csi.c | 27 +--
>>>> .../platform/sunxi/sun4i-csi/sun4i_csi.h | 1 -
>>>> .../platform/sunxi/sun6i-csi/sun6i_csi.c | 36 +--
>>>> drivers/media/platform/video-mux.c | 30 ++-
>>>> drivers/media/platform/xilinx/xilinx-vipp.c | 69 +++---
>>>> drivers/media/v4l2-core/v4l2-fwnode.c | 50 +----
>>>> drivers/staging/media/imx/TODO | 29 ---
>>>> drivers/staging/media/imx/imx-media-csi.c | 92 +++++---
>>>> .../staging/media/imx/imx-media-dev-common.c | 60 ++---
>>>> drivers/staging/media/imx/imx-media-of.c | 114 ----------
>>>> drivers/staging/media/imx/imx-media-utils.c | 33 +++
>>>> drivers/staging/media/imx/imx-media.h | 5 +-
>>>> drivers/staging/media/imx/imx6-mipi-csi2.c | 29 ++-
>>>> drivers/staging/media/imx/imx7-media-csi.c | 55 +++--
>>>> drivers/staging/media/imx/imx7-mipi-csis.c | 30 ++-
>>>> include/linux/fwnode.h | 14 ++
>>>> include/linux/property.h | 5 +
>>>> include/media/media-entity.h | 99 ++++++++-
>>>> include/media/v4l2-fwnode.h | 56 +----
>>>> 26 files changed, 735 insertions(+), 477 deletions(-)
>>>>
>>>> --
>>>> 2.17.1
>>>>
^ permalink raw reply [flat|nested] 38+ messages in thread