Message ID | 20191124190703.12138-1-slongerbeam@gmail.com (mailing list archive) |
---|---|
Headers | show |
Series | media: imx: Create media links in bound notifiers | expand |
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 >
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 >>
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 > >> >
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 >
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 >>
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 > >> >
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 >>>>