* [PATCH 0/8] media: imx-pxp: add support for i.MX7D
@ 2023-01-05 13:47 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
This series adds support for the PXP found on the i.MX7D to the imx-pxp
driver.
The PXP on the i.MX7D has a few differences compared to the one on the
i.MX6ULL. Especially, it has more processing blocks and slightly different
multiplexers to route the data between the blocks. Therefore, the driver must
configure a different data path depending on the platform.
While the PXP has a version register, the reported version is the same on the
i.MX6ULL and the i.MX7D. Therefore, we cannot use the version register to
change the driver behavior, but have to use the device tree compatible. The
driver still prints the found version to the log to help bringing up the PXP
on further platforms.
The patches are inspired by some earlier patches [0] by Laurent to add PXP
support to the i.MX7d. Compared to the earlier patches, these patches add
different behavior depending on the platform. Furthermore, the patches disable
only the LUT block, but keep the rotator block enabled, as it may now be
configured via the V4L2 rotate control.
Patch 1 converts the dt-binding to yaml.
Patches 2 to 5 cleanup and refactor the driver in preparation of handling
different PXP versions.
Patches 6 and 7 add the handling of different platforms and the i.MX7d
specific configuration.
Patch 8 adds the device tree node for the PXP to the i.MX7d device tree.
Michael
[0] https://lore.kernel.org/linux-media/20200510223100.11641-1-laurent.pinchart@ideasonboard.com/
Michael Tretter (8):
media: dt-bindings: media: fsl-pxp: convert to yaml
media: imx-pxp: detect PXP version
media: imx-pxp: extract helper function to setup data path
media: imx-pxp: explicitly disable unused blocks
media: imx-pxp: disable LUT block
media: imx-pxp: make data_path_ctrl0 platform dependent
media: imx-pxp: add support for i.MX7D
ARM: dts: imx7d: add node for PXP
.../bindings/media/fsl,imx6ull-pxp.yaml | 62 ++++++++
.../devicetree/bindings/media/fsl-pxp.txt | 26 ---
arch/arm/boot/dts/imx7d.dtsi | 9 ++
drivers/media/platform/nxp/imx-pxp.c | 148 +++++++++++++++---
4 files changed, 197 insertions(+), 48 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
--
2.30.2
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH 0/8] media: imx-pxp: add support for i.MX7D
@ 2023-01-05 13:47 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
This series adds support for the PXP found on the i.MX7D to the imx-pxp
driver.
The PXP on the i.MX7D has a few differences compared to the one on the
i.MX6ULL. Especially, it has more processing blocks and slightly different
multiplexers to route the data between the blocks. Therefore, the driver must
configure a different data path depending on the platform.
While the PXP has a version register, the reported version is the same on the
i.MX6ULL and the i.MX7D. Therefore, we cannot use the version register to
change the driver behavior, but have to use the device tree compatible. The
driver still prints the found version to the log to help bringing up the PXP
on further platforms.
The patches are inspired by some earlier patches [0] by Laurent to add PXP
support to the i.MX7d. Compared to the earlier patches, these patches add
different behavior depending on the platform. Furthermore, the patches disable
only the LUT block, but keep the rotator block enabled, as it may now be
configured via the V4L2 rotate control.
Patch 1 converts the dt-binding to yaml.
Patches 2 to 5 cleanup and refactor the driver in preparation of handling
different PXP versions.
Patches 6 and 7 add the handling of different platforms and the i.MX7d
specific configuration.
Patch 8 adds the device tree node for the PXP to the i.MX7d device tree.
Michael
[0] https://lore.kernel.org/linux-media/20200510223100.11641-1-laurent.pinchart@ideasonboard.com/
Michael Tretter (8):
media: dt-bindings: media: fsl-pxp: convert to yaml
media: imx-pxp: detect PXP version
media: imx-pxp: extract helper function to setup data path
media: imx-pxp: explicitly disable unused blocks
media: imx-pxp: disable LUT block
media: imx-pxp: make data_path_ctrl0 platform dependent
media: imx-pxp: add support for i.MX7D
ARM: dts: imx7d: add node for PXP
.../bindings/media/fsl,imx6ull-pxp.yaml | 62 ++++++++
.../devicetree/bindings/media/fsl-pxp.txt | 26 ---
arch/arm/boot/dts/imx7d.dtsi | 9 ++
drivers/media/platform/nxp/imx-pxp.c | 148 +++++++++++++++---
4 files changed, 197 insertions(+), 48 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH 1/8] media: dt-bindings: media: fsl-pxp: convert to yaml
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-05 13:47 ` Michael Tretter
-1 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
Convert the bindings of the Freescale Pixel Pipeline to YAML.
The conversion drops the previously listed compatibles for several SoCs.
It is unclear, if the PXP on these SoCs is compatible to any of the PXPs
on the existing SoCs and would allow to reuse the already defined
compatibles. The missing compatibles should be brought back when the
support for the PXP on these SoCs is added.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
.../bindings/media/fsl,imx6ull-pxp.yaml | 62 +++++++++++++++++++
.../devicetree/bindings/media/fsl-pxp.txt | 26 --------
2 files changed, 62 insertions(+), 26 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
diff --git a/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
new file mode 100644
index 000000000000..e5f227b84759
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/media/fsl,imx6ull-pxp.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Freescale Pixel Pipeline
+
+maintainers:
+ - Philipp Zabel <p.zabel@pengutronix.de>
+ - Michael Tretter <m.tretter@pengutronix.de>
+
+description:
+ The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine
+ that supports scaling, colorspace conversion, alpha blending, rotation, and
+ pixel conversion via lookup table. Different versions are present on various
+ i.MX SoCs from i.MX23 to i.MX7.
+
+properties:
+ compatible:
+ enum:
+ - fsl,imx6ul-pxp
+ - fsl,imx6ull-pxp
+ - fsl,imx7d-pxp
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ minItems: 1
+ maxItems: 2
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: axi
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+
+additionalProperties: False
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx6ul-clock.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ pxp: pxp@21cc000 {
+ compatible = "fsl,imx6ull-pxp";
+ reg = <0x021cc000 0x4000>;
+ interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
+ clock-names = "axi";
+ clocks = <&clks IMX6UL_CLK_PXP>;
+ };
diff --git a/Documentation/devicetree/bindings/media/fsl-pxp.txt b/Documentation/devicetree/bindings/media/fsl-pxp.txt
deleted file mode 100644
index f8090e06530d..000000000000
--- a/Documentation/devicetree/bindings/media/fsl-pxp.txt
+++ /dev/null
@@ -1,26 +0,0 @@
-Freescale Pixel Pipeline
-========================
-
-The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine
-that supports scaling, colorspace conversion, alpha blending, rotation, and
-pixel conversion via lookup table. Different versions are present on various
-i.MX SoCs from i.MX23 to i.MX7.
-
-Required properties:
-- compatible: should be "fsl,<soc>-pxp", where SoC can be one of imx23, imx28,
- imx6dl, imx6sl, imx6sll, imx6ul, imx6sx, imx6ull, or imx7d.
-- reg: the register base and size for the device registers
-- interrupts: the PXP interrupt, two interrupts for imx6ull and imx7d.
-- clock-names: should be "axi"
-- clocks: the PXP AXI clock
-
-Example:
-
-pxp@21cc000 {
- compatible = "fsl,imx6ull-pxp";
- reg = <0x021cc000 0x4000>;
- interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
- clock-names = "axi";
- clocks = <&clks IMX6UL_CLK_PXP>;
-};
--
2.30.2
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 1/8] media: dt-bindings: media: fsl-pxp: convert to yaml
@ 2023-01-05 13:47 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
Convert the bindings of the Freescale Pixel Pipeline to YAML.
The conversion drops the previously listed compatibles for several SoCs.
It is unclear, if the PXP on these SoCs is compatible to any of the PXPs
on the existing SoCs and would allow to reuse the already defined
compatibles. The missing compatibles should be brought back when the
support for the PXP on these SoCs is added.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
.../bindings/media/fsl,imx6ull-pxp.yaml | 62 +++++++++++++++++++
.../devicetree/bindings/media/fsl-pxp.txt | 26 --------
2 files changed, 62 insertions(+), 26 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
diff --git a/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
new file mode 100644
index 000000000000..e5f227b84759
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/media/fsl,imx6ull-pxp.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Freescale Pixel Pipeline
+
+maintainers:
+ - Philipp Zabel <p.zabel@pengutronix.de>
+ - Michael Tretter <m.tretter@pengutronix.de>
+
+description:
+ The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine
+ that supports scaling, colorspace conversion, alpha blending, rotation, and
+ pixel conversion via lookup table. Different versions are present on various
+ i.MX SoCs from i.MX23 to i.MX7.
+
+properties:
+ compatible:
+ enum:
+ - fsl,imx6ul-pxp
+ - fsl,imx6ull-pxp
+ - fsl,imx7d-pxp
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ minItems: 1
+ maxItems: 2
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: axi
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+
+additionalProperties: False
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx6ul-clock.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ pxp: pxp@21cc000 {
+ compatible = "fsl,imx6ull-pxp";
+ reg = <0x021cc000 0x4000>;
+ interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
+ clock-names = "axi";
+ clocks = <&clks IMX6UL_CLK_PXP>;
+ };
diff --git a/Documentation/devicetree/bindings/media/fsl-pxp.txt b/Documentation/devicetree/bindings/media/fsl-pxp.txt
deleted file mode 100644
index f8090e06530d..000000000000
--- a/Documentation/devicetree/bindings/media/fsl-pxp.txt
+++ /dev/null
@@ -1,26 +0,0 @@
-Freescale Pixel Pipeline
-========================
-
-The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine
-that supports scaling, colorspace conversion, alpha blending, rotation, and
-pixel conversion via lookup table. Different versions are present on various
-i.MX SoCs from i.MX23 to i.MX7.
-
-Required properties:
-- compatible: should be "fsl,<soc>-pxp", where SoC can be one of imx23, imx28,
- imx6dl, imx6sl, imx6sll, imx6ul, imx6sx, imx6ull, or imx7d.
-- reg: the register base and size for the device registers
-- interrupts: the PXP interrupt, two interrupts for imx6ull and imx7d.
-- clock-names: should be "axi"
-- clocks: the PXP AXI clock
-
-Example:
-
-pxp@21cc000 {
- compatible = "fsl,imx6ull-pxp";
- reg = <0x021cc000 0x4000>;
- interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
- clock-names = "axi";
- clocks = <&clks IMX6UL_CLK_PXP>;
-};
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 2/8] media: imx-pxp: detect PXP version
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-05 13:47 ` Michael Tretter
-1 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
Different versions of the Pixel Pipeline have different blocks and their
routing may be different. Read the PXP_HW_VERSION register to determine
the version of the PXP and print it to the log for debugging purposes.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/media/platform/nxp/imx-pxp.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
index 689ae5e6ac62..05abe40558b0 100644
--- a/drivers/media/platform/nxp/imx-pxp.c
+++ b/drivers/media/platform/nxp/imx-pxp.c
@@ -10,6 +10,7 @@
* Pawel Osciak, <pawel@osciak.com>
* Marek Szyprowski, <m.szyprowski@samsung.com>
*/
+#include <linux/bitfield.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/dma-mapping.h>
@@ -52,6 +53,11 @@ MODULE_PARM_DESC(debug, "activates debug info");
#define MEM2MEM_HFLIP (1 << 0)
#define MEM2MEM_VFLIP (1 << 1)
+#define PXP_VERSION_MAJOR(version) \
+ FIELD_GET(BM_PXP_VERSION_MAJOR, version)
+#define PXP_VERSION_MINOR(version) \
+ FIELD_GET(BM_PXP_VERSION_MINOR, version)
+
#define dprintk(dev, fmt, arg...) \
v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: " fmt, __func__, ## arg)
@@ -192,6 +198,8 @@ struct pxp_dev {
struct clk *clk;
void __iomem *mmio;
+ u32 hw_version;
+
atomic_t num_inst;
struct mutex dev_mutex;
spinlock_t irqlock;
@@ -1660,6 +1668,11 @@ static int pxp_soft_reset(struct pxp_dev *dev)
return 0;
}
+static u32 pxp_read_version(struct pxp_dev *dev)
+{
+ return readl(dev->mmio + HW_PXP_VERSION);
+}
+
static int pxp_probe(struct platform_device *pdev)
{
struct pxp_dev *dev;
@@ -1705,6 +1718,11 @@ static int pxp_probe(struct platform_device *pdev)
goto err_clk;
}
+ dev->hw_version = pxp_read_version(dev);
+ dev_info(&pdev->dev, "PXP Version %d.%d\n",
+ PXP_VERSION_MAJOR(dev->hw_version),
+ PXP_VERSION_MINOR(dev->hw_version));
+
ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
if (ret)
goto err_clk;
--
2.30.2
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 2/8] media: imx-pxp: detect PXP version
@ 2023-01-05 13:47 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
Different versions of the Pixel Pipeline have different blocks and their
routing may be different. Read the PXP_HW_VERSION register to determine
the version of the PXP and print it to the log for debugging purposes.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/media/platform/nxp/imx-pxp.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
index 689ae5e6ac62..05abe40558b0 100644
--- a/drivers/media/platform/nxp/imx-pxp.c
+++ b/drivers/media/platform/nxp/imx-pxp.c
@@ -10,6 +10,7 @@
* Pawel Osciak, <pawel@osciak.com>
* Marek Szyprowski, <m.szyprowski@samsung.com>
*/
+#include <linux/bitfield.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/dma-mapping.h>
@@ -52,6 +53,11 @@ MODULE_PARM_DESC(debug, "activates debug info");
#define MEM2MEM_HFLIP (1 << 0)
#define MEM2MEM_VFLIP (1 << 1)
+#define PXP_VERSION_MAJOR(version) \
+ FIELD_GET(BM_PXP_VERSION_MAJOR, version)
+#define PXP_VERSION_MINOR(version) \
+ FIELD_GET(BM_PXP_VERSION_MINOR, version)
+
#define dprintk(dev, fmt, arg...) \
v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: " fmt, __func__, ## arg)
@@ -192,6 +198,8 @@ struct pxp_dev {
struct clk *clk;
void __iomem *mmio;
+ u32 hw_version;
+
atomic_t num_inst;
struct mutex dev_mutex;
spinlock_t irqlock;
@@ -1660,6 +1668,11 @@ static int pxp_soft_reset(struct pxp_dev *dev)
return 0;
}
+static u32 pxp_read_version(struct pxp_dev *dev)
+{
+ return readl(dev->mmio + HW_PXP_VERSION);
+}
+
static int pxp_probe(struct platform_device *pdev)
{
struct pxp_dev *dev;
@@ -1705,6 +1718,11 @@ static int pxp_probe(struct platform_device *pdev)
goto err_clk;
}
+ dev->hw_version = pxp_read_version(dev);
+ dev_info(&pdev->dev, "PXP Version %d.%d\n",
+ PXP_VERSION_MAJOR(dev->hw_version),
+ PXP_VERSION_MINOR(dev->hw_version));
+
ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
if (ret)
goto err_clk;
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 3/8] media: imx-pxp: extract helper function to setup data path
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-05 13:47 ` Michael Tretter
-1 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
The driver must configure the data path through the Pixel Pipeline.
Currently, the driver is using a fixed setup, but once there are
different pipeline configurations, it is helpful to have a dedicated
function for determining the register value for the data path.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/media/platform/nxp/imx-pxp.c | 62 +++++++++++++++++++---------
1 file changed, 42 insertions(+), 20 deletions(-)
diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
index 05abe40558b0..a957fee88829 100644
--- a/drivers/media/platform/nxp/imx-pxp.c
+++ b/drivers/media/platform/nxp/imx-pxp.c
@@ -726,6 +726,47 @@ static void pxp_setup_csc(struct pxp_ctx *ctx)
}
}
+static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
+{
+ u32 ctrl0;
+
+ ctrl0 = 0;
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0);
+
+ return ctrl0;
+}
+
+static void pxp_set_data_path(struct pxp_ctx *ctx)
+{
+ struct pxp_dev *dev = ctx->dev;
+ u32 ctrl0;
+ u32 ctrl1;
+
+ ctrl0 = pxp_data_path_ctrl0(ctx);
+
+ ctrl1 = 0;
+ ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1);
+ ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1);
+
+ writel(ctrl0, dev->mmio + HW_PXP_DATA_PATH_CTRL0);
+ writel(ctrl1, dev->mmio + HW_PXP_DATA_PATH_CTRL1);
+}
+
static int pxp_start(struct pxp_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
struct vb2_v4l2_buffer *out_vb)
{
@@ -912,26 +953,7 @@ static int pxp_start(struct pxp_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
/* bypass LUT */
writel(BM_PXP_LUT_CTRL_BYPASS, dev->mmio + HW_PXP_LUT_CTRL);
- writel(BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1)|
- BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1)|
- BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0),
- dev->mmio + HW_PXP_DATA_PATH_CTRL0);
- writel(BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1) |
- BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1),
- dev->mmio + HW_PXP_DATA_PATH_CTRL1);
+ pxp_set_data_path(ctx);
writel(0xffff, dev->mmio + HW_PXP_IRQ_MASK);
--
2.30.2
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 3/8] media: imx-pxp: extract helper function to setup data path
@ 2023-01-05 13:47 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
The driver must configure the data path through the Pixel Pipeline.
Currently, the driver is using a fixed setup, but once there are
different pipeline configurations, it is helpful to have a dedicated
function for determining the register value for the data path.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/media/platform/nxp/imx-pxp.c | 62 +++++++++++++++++++---------
1 file changed, 42 insertions(+), 20 deletions(-)
diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
index 05abe40558b0..a957fee88829 100644
--- a/drivers/media/platform/nxp/imx-pxp.c
+++ b/drivers/media/platform/nxp/imx-pxp.c
@@ -726,6 +726,47 @@ static void pxp_setup_csc(struct pxp_ctx *ctx)
}
}
+static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
+{
+ u32 ctrl0;
+
+ ctrl0 = 0;
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0);
+
+ return ctrl0;
+}
+
+static void pxp_set_data_path(struct pxp_ctx *ctx)
+{
+ struct pxp_dev *dev = ctx->dev;
+ u32 ctrl0;
+ u32 ctrl1;
+
+ ctrl0 = pxp_data_path_ctrl0(ctx);
+
+ ctrl1 = 0;
+ ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1);
+ ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1);
+
+ writel(ctrl0, dev->mmio + HW_PXP_DATA_PATH_CTRL0);
+ writel(ctrl1, dev->mmio + HW_PXP_DATA_PATH_CTRL1);
+}
+
static int pxp_start(struct pxp_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
struct vb2_v4l2_buffer *out_vb)
{
@@ -912,26 +953,7 @@ static int pxp_start(struct pxp_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
/* bypass LUT */
writel(BM_PXP_LUT_CTRL_BYPASS, dev->mmio + HW_PXP_LUT_CTRL);
- writel(BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1)|
- BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1)|
- BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0)|
- BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0),
- dev->mmio + HW_PXP_DATA_PATH_CTRL0);
- writel(BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1) |
- BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1),
- dev->mmio + HW_PXP_DATA_PATH_CTRL1);
+ pxp_set_data_path(ctx);
writel(0xffff, dev->mmio + HW_PXP_IRQ_MASK);
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 4/8] media: imx-pxp: explicitly disable unused blocks
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-05 13:47 ` Michael Tretter
-1 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
Various multiplexers in the pipeline are not used with the currently
configured data path. Disable all unused multiplexers by selecting the
"no output" (3) option.
The datasheet doesn't explicitly require this, but the PXP has been seen
to hang after processing a few hundreds of frames otherwise.
As at it, add documentation for the multiplexers that are actually
relevant for the data path.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/media/platform/nxp/imx-pxp.c | 30 +++++++++++++++++-----------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
index a957fee88829..6ffd07cda965 100644
--- a/drivers/media/platform/nxp/imx-pxp.c
+++ b/drivers/media/platform/nxp/imx-pxp.c
@@ -731,22 +731,28 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
u32 ctrl0;
ctrl0 = 0;
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(3);
+ /* Bypass Dithering x3CH */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
+ /* Select Rotation */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
+ /* Select LUT */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
+ /* Select MUX8 for LUT */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
+ /* Select CSC 2 */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(3);
+ /* Bypass Rotation 2 */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(3);
return ctrl0;
}
@@ -760,8 +766,8 @@ static void pxp_set_data_path(struct pxp_ctx *ctx)
ctrl0 = pxp_data_path_ctrl0(ctx);
ctrl1 = 0;
- ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1);
- ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1);
+ ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(3);
+ ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(3);
writel(ctrl0, dev->mmio + HW_PXP_DATA_PATH_CTRL0);
writel(ctrl1, dev->mmio + HW_PXP_DATA_PATH_CTRL1);
--
2.30.2
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 4/8] media: imx-pxp: explicitly disable unused blocks
@ 2023-01-05 13:47 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
Various multiplexers in the pipeline are not used with the currently
configured data path. Disable all unused multiplexers by selecting the
"no output" (3) option.
The datasheet doesn't explicitly require this, but the PXP has been seen
to hang after processing a few hundreds of frames otherwise.
As at it, add documentation for the multiplexers that are actually
relevant for the data path.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/media/platform/nxp/imx-pxp.c | 30 +++++++++++++++++-----------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
index a957fee88829..6ffd07cda965 100644
--- a/drivers/media/platform/nxp/imx-pxp.c
+++ b/drivers/media/platform/nxp/imx-pxp.c
@@ -731,22 +731,28 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
u32 ctrl0;
ctrl0 = 0;
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(3);
+ /* Bypass Dithering x3CH */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
+ /* Select Rotation */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
+ /* Select LUT */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
+ /* Select MUX8 for LUT */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
+ /* Select CSC 2 */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(3);
+ /* Bypass Rotation 2 */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0);
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(3);
return ctrl0;
}
@@ -760,8 +766,8 @@ static void pxp_set_data_path(struct pxp_ctx *ctx)
ctrl0 = pxp_data_path_ctrl0(ctx);
ctrl1 = 0;
- ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1);
- ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1);
+ ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(3);
+ ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(3);
writel(ctrl0, dev->mmio + HW_PXP_DATA_PATH_CTRL0);
writel(ctrl1, dev->mmio + HW_PXP_DATA_PATH_CTRL1);
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 5/8] media: imx-pxp: disable LUT block
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-05 13:47 ` Michael Tretter
-1 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
The LUT block is always configured in bypass mode.
Take it entirely out of the pipeline by disabling it and routing the
data path around the LUT.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/media/platform/nxp/imx-pxp.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
index 6ffd07cda965..1d649b9cadad 100644
--- a/drivers/media/platform/nxp/imx-pxp.c
+++ b/drivers/media/platform/nxp/imx-pxp.c
@@ -737,11 +737,10 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
/* Select Rotation */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
- /* Select LUT */
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
+ /* Bypass LUT */
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(1);
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
- /* Select MUX8 for LUT */
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(3);
/* Select CSC 2 */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
@@ -966,7 +965,7 @@ static int pxp_start(struct pxp_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
/* ungate, enable PS/AS/OUT and PXP operation */
writel(BM_PXP_CTRL_IRQ_ENABLE, dev->mmio + HW_PXP_CTRL_SET);
writel(BM_PXP_CTRL_ENABLE | BM_PXP_CTRL_ENABLE_CSC2 |
- BM_PXP_CTRL_ENABLE_LUT | BM_PXP_CTRL_ENABLE_ROTATE0 |
+ BM_PXP_CTRL_ENABLE_ROTATE0 |
BM_PXP_CTRL_ENABLE_PS_AS_OUT, dev->mmio + HW_PXP_CTRL_SET);
return 0;
--
2.30.2
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 5/8] media: imx-pxp: disable LUT block
@ 2023-01-05 13:47 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
The LUT block is always configured in bypass mode.
Take it entirely out of the pipeline by disabling it and routing the
data path around the LUT.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/media/platform/nxp/imx-pxp.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
index 6ffd07cda965..1d649b9cadad 100644
--- a/drivers/media/platform/nxp/imx-pxp.c
+++ b/drivers/media/platform/nxp/imx-pxp.c
@@ -737,11 +737,10 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
/* Select Rotation */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
- /* Select LUT */
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
+ /* Bypass LUT */
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(1);
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
- /* Select MUX8 for LUT */
- ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(3);
/* Select CSC 2 */
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
@@ -966,7 +965,7 @@ static int pxp_start(struct pxp_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
/* ungate, enable PS/AS/OUT and PXP operation */
writel(BM_PXP_CTRL_IRQ_ENABLE, dev->mmio + HW_PXP_CTRL_SET);
writel(BM_PXP_CTRL_ENABLE | BM_PXP_CTRL_ENABLE_CSC2 |
- BM_PXP_CTRL_ENABLE_LUT | BM_PXP_CTRL_ENABLE_ROTATE0 |
+ BM_PXP_CTRL_ENABLE_ROTATE0 |
BM_PXP_CTRL_ENABLE_PS_AS_OUT, dev->mmio + HW_PXP_CTRL_SET);
return 0;
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 6/8] media: imx-pxp: make data_path_ctrl0 platform dependent
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-05 13:47 ` Michael Tretter
-1 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
Unfortunately, the PXP_HW_VERSION register reports the PXP on the i.MX7D
and on the i.MX6ULL as version 3.0, although the PXP versions on these
SoCs have significant differences.
Use the compatible to configure the ctrl0 register as required dependent
on the platform.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/media/platform/nxp/imx-pxp.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
index 1d649b9cadad..4e182f80a36b 100644
--- a/drivers/media/platform/nxp/imx-pxp.c
+++ b/drivers/media/platform/nxp/imx-pxp.c
@@ -19,6 +19,7 @@
#include <linux/iopoll.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/sched.h>
#include <linux/slab.h>
@@ -191,6 +192,11 @@ static struct pxp_fmt *find_format(struct v4l2_format *f)
return &formats[k];
}
+struct pxp_ctx;
+struct pxp_pdata {
+ u32 (*data_path_ctrl0)(struct pxp_ctx *ctx);
+};
+
struct pxp_dev {
struct v4l2_device v4l2_dev;
struct video_device vfd;
@@ -199,6 +205,7 @@ struct pxp_dev {
void __iomem *mmio;
u32 hw_version;
+ const struct pxp_pdata *pdata;
atomic_t num_inst;
struct mutex dev_mutex;
@@ -726,7 +733,7 @@ static void pxp_setup_csc(struct pxp_ctx *ctx)
}
}
-static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
+static u32 pxp_imx6ull_data_path_ctrl0(struct pxp_ctx *ctx)
{
u32 ctrl0;
@@ -756,6 +763,16 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
return ctrl0;
}
+static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
+{
+ struct pxp_dev *dev = ctx->dev;
+
+ if (dev->pdata && dev->pdata->data_path_ctrl0)
+ return dev->pdata->data_path_ctrl0(ctx);
+
+ return pxp_imx6ull_data_path_ctrl0(ctx);
+}
+
static void pxp_set_data_path(struct pxp_ctx *ctx)
{
struct pxp_dev *dev = ctx->dev;
@@ -1711,6 +1728,8 @@ static int pxp_probe(struct platform_device *pdev)
if (!dev)
return -ENOMEM;
+ dev->pdata = of_device_get_match_data(&pdev->dev);
+
dev->clk = devm_clk_get(&pdev->dev, "axi");
if (IS_ERR(dev->clk)) {
ret = PTR_ERR(dev->clk);
@@ -1811,8 +1830,12 @@ static int pxp_remove(struct platform_device *pdev)
return 0;
}
+static const struct pxp_pdata pxp_imx6ull_pdata = {
+ .data_path_ctrl0 = pxp_imx6ull_data_path_ctrl0,
+};
+
static const struct of_device_id pxp_dt_ids[] = {
- { .compatible = "fsl,imx6ull-pxp", .data = NULL },
+ { .compatible = "fsl,imx6ull-pxp", .data = &pxp_imx6ull_pdata },
{ },
};
MODULE_DEVICE_TABLE(of, pxp_dt_ids);
--
2.30.2
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 6/8] media: imx-pxp: make data_path_ctrl0 platform dependent
@ 2023-01-05 13:47 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
Unfortunately, the PXP_HW_VERSION register reports the PXP on the i.MX7D
and on the i.MX6ULL as version 3.0, although the PXP versions on these
SoCs have significant differences.
Use the compatible to configure the ctrl0 register as required dependent
on the platform.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/media/platform/nxp/imx-pxp.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
index 1d649b9cadad..4e182f80a36b 100644
--- a/drivers/media/platform/nxp/imx-pxp.c
+++ b/drivers/media/platform/nxp/imx-pxp.c
@@ -19,6 +19,7 @@
#include <linux/iopoll.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/sched.h>
#include <linux/slab.h>
@@ -191,6 +192,11 @@ static struct pxp_fmt *find_format(struct v4l2_format *f)
return &formats[k];
}
+struct pxp_ctx;
+struct pxp_pdata {
+ u32 (*data_path_ctrl0)(struct pxp_ctx *ctx);
+};
+
struct pxp_dev {
struct v4l2_device v4l2_dev;
struct video_device vfd;
@@ -199,6 +205,7 @@ struct pxp_dev {
void __iomem *mmio;
u32 hw_version;
+ const struct pxp_pdata *pdata;
atomic_t num_inst;
struct mutex dev_mutex;
@@ -726,7 +733,7 @@ static void pxp_setup_csc(struct pxp_ctx *ctx)
}
}
-static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
+static u32 pxp_imx6ull_data_path_ctrl0(struct pxp_ctx *ctx)
{
u32 ctrl0;
@@ -756,6 +763,16 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
return ctrl0;
}
+static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
+{
+ struct pxp_dev *dev = ctx->dev;
+
+ if (dev->pdata && dev->pdata->data_path_ctrl0)
+ return dev->pdata->data_path_ctrl0(ctx);
+
+ return pxp_imx6ull_data_path_ctrl0(ctx);
+}
+
static void pxp_set_data_path(struct pxp_ctx *ctx)
{
struct pxp_dev *dev = ctx->dev;
@@ -1711,6 +1728,8 @@ static int pxp_probe(struct platform_device *pdev)
if (!dev)
return -ENOMEM;
+ dev->pdata = of_device_get_match_data(&pdev->dev);
+
dev->clk = devm_clk_get(&pdev->dev, "axi");
if (IS_ERR(dev->clk)) {
ret = PTR_ERR(dev->clk);
@@ -1811,8 +1830,12 @@ static int pxp_remove(struct platform_device *pdev)
return 0;
}
+static const struct pxp_pdata pxp_imx6ull_pdata = {
+ .data_path_ctrl0 = pxp_imx6ull_data_path_ctrl0,
+};
+
static const struct of_device_id pxp_dt_ids[] = {
- { .compatible = "fsl,imx6ull-pxp", .data = NULL },
+ { .compatible = "fsl,imx6ull-pxp", .data = &pxp_imx6ull_pdata },
{ },
};
MODULE_DEVICE_TABLE(of, pxp_dt_ids);
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 7/8] media: imx-pxp: add support for i.MX7D
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-05 13:47 ` Michael Tretter
-1 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
The i.MX7D needs a different data path configuration than the i.MX6ULL.
Configure the data path as close as possible to the data path on the
i.MX6ULL.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/media/platform/nxp/imx-pxp.c | 36 ++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
index 4e182f80a36b..04cc8df2a498 100644
--- a/drivers/media/platform/nxp/imx-pxp.c
+++ b/drivers/media/platform/nxp/imx-pxp.c
@@ -763,6 +763,37 @@ static u32 pxp_imx6ull_data_path_ctrl0(struct pxp_ctx *ctx)
return ctrl0;
}
+static u32 pxp_imx7d_data_path_ctrl0(struct pxp_ctx *ctx)
+{
+ u32 ctrl0;
+
+ ctrl0 = 0;
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(3);
+ /* Select Rotation 0 */
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
+ /* Select MUX11 for Rotation 0 */
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(1);
+ /* Bypass LUT */
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(1);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(3);
+ /* Select CSC 2 */
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
+ /* Select Composite Alpha Blending/Color Key 0 for CSC 2 */
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(1);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(3);
+ /* Bypass Rotation 1 */
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(3);
+
+ return ctrl0;
+}
+
static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
{
struct pxp_dev *dev = ctx->dev;
@@ -1834,8 +1865,13 @@ static const struct pxp_pdata pxp_imx6ull_pdata = {
.data_path_ctrl0 = pxp_imx6ull_data_path_ctrl0,
};
+static const struct pxp_pdata pxp_imx7d_pdata = {
+ .data_path_ctrl0 = pxp_imx7d_data_path_ctrl0,
+};
+
static const struct of_device_id pxp_dt_ids[] = {
{ .compatible = "fsl,imx6ull-pxp", .data = &pxp_imx6ull_pdata },
+ { .compatible = "fsl,imx7d-pxp", .data = &pxp_imx7d_pdata },
{ },
};
MODULE_DEVICE_TABLE(of, pxp_dt_ids);
--
2.30.2
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 7/8] media: imx-pxp: add support for i.MX7D
@ 2023-01-05 13:47 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
The i.MX7D needs a different data path configuration than the i.MX6ULL.
Configure the data path as close as possible to the data path on the
i.MX6ULL.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/media/platform/nxp/imx-pxp.c | 36 ++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
index 4e182f80a36b..04cc8df2a498 100644
--- a/drivers/media/platform/nxp/imx-pxp.c
+++ b/drivers/media/platform/nxp/imx-pxp.c
@@ -763,6 +763,37 @@ static u32 pxp_imx6ull_data_path_ctrl0(struct pxp_ctx *ctx)
return ctrl0;
}
+static u32 pxp_imx7d_data_path_ctrl0(struct pxp_ctx *ctx)
+{
+ u32 ctrl0;
+
+ ctrl0 = 0;
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(3);
+ /* Select Rotation 0 */
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
+ /* Select MUX11 for Rotation 0 */
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(1);
+ /* Bypass LUT */
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(1);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(3);
+ /* Select CSC 2 */
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
+ /* Select Composite Alpha Blending/Color Key 0 for CSC 2 */
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(1);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(3);
+ /* Bypass Rotation 1 */
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(3);
+ ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(3);
+
+ return ctrl0;
+}
+
static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
{
struct pxp_dev *dev = ctx->dev;
@@ -1834,8 +1865,13 @@ static const struct pxp_pdata pxp_imx6ull_pdata = {
.data_path_ctrl0 = pxp_imx6ull_data_path_ctrl0,
};
+static const struct pxp_pdata pxp_imx7d_pdata = {
+ .data_path_ctrl0 = pxp_imx7d_data_path_ctrl0,
+};
+
static const struct of_device_id pxp_dt_ids[] = {
{ .compatible = "fsl,imx6ull-pxp", .data = &pxp_imx6ull_pdata },
+ { .compatible = "fsl,imx7d-pxp", .data = &pxp_imx7d_pdata },
{ },
};
MODULE_DEVICE_TABLE(of, pxp_dt_ids);
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 8/8] ARM: dts: imx7d: add node for PXP
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-05 13:47 ` Michael Tretter
-1 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
The i.MX7d contains a Pixel Pipeline in version 3.0. Add the device tree
node to make it available.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
arch/arm/boot/dts/imx7d.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 7ceb7c09f7ad..728cc9413a7c 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -165,6 +165,15 @@ pcie_phy: pcie-phy@306d0000 {
reg = <0x306d0000 0x10000>;
status = "disabled";
};
+
+ pxp: pxp@30700000 {
+ compatible = "fsl,imx7d-pxp";
+ reg = <0x30700000 0x10000>;
+ interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX7D_PXP_CLK>;
+ clock-names = "axi";
+ };
};
&aips3 {
--
2.30.2
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 8/8] ARM: dts: imx7d: add node for PXP
@ 2023-01-05 13:47 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-05 13:47 UTC (permalink / raw)
To: linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel, Michael Tretter
The i.MX7d contains a Pixel Pipeline in version 3.0. Add the device tree
node to make it available.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
arch/arm/boot/dts/imx7d.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 7ceb7c09f7ad..728cc9413a7c 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -165,6 +165,15 @@ pcie_phy: pcie-phy@306d0000 {
reg = <0x306d0000 0x10000>;
status = "disabled";
};
+
+ pxp: pxp@30700000 {
+ compatible = "fsl,imx7d-pxp";
+ reg = <0x30700000 0x10000>;
+ interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX7D_PXP_CLK>;
+ clock-names = "axi";
+ };
};
&aips3 {
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH 1/8] media: dt-bindings: media: fsl-pxp: convert to yaml
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-06 3:18 ` Rob Herring
-1 siblings, 0 replies; 62+ messages in thread
From: Rob Herring @ 2023-01-06 3:18 UTC (permalink / raw)
To: Michael Tretter
Cc: Mauro Carvalho Chehab, Fabio Estevam, Rob Herring,
linux-arm-kernel, Philipp Zabel, kernel, Krzysztof Kozlowski,
linux-media, linux-imx, Laurent Pinchart, devicetree
On Thu, 05 Jan 2023 14:47:22 +0100, Michael Tretter wrote:
> Convert the bindings of the Freescale Pixel Pipeline to YAML.
>
> The conversion drops the previously listed compatibles for several SoCs.
> It is unclear, if the PXP on these SoCs is compatible to any of the PXPs
> on the existing SoCs and would allow to reuse the already defined
> compatibles. The missing compatibles should be brought back when the
> support for the PXP on these SoCs is added.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> .../bindings/media/fsl,imx6ull-pxp.yaml | 62 +++++++++++++++++++
> .../devicetree/bindings/media/fsl-pxp.txt | 26 --------
> 2 files changed, 62 insertions(+), 26 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
>
Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.
Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.
Full log is available here: https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230105134729.59542-2-m.tretter@pengutronix.de
pxp@20f0000: compatible:0: 'fsl,imx6sll-pxp' is not one of ['fsl,imx6ul-pxp', 'fsl,imx6ull-pxp', 'fsl,imx7d-pxp']
arch/arm/boot/dts/imx6sll-evk.dtb
arch/arm/boot/dts/imx6sll-kobo-clarahd.dtb
arch/arm/boot/dts/imx6sll-kobo-librah2o.dtb
pxp@20f0000: compatible: ['fsl,imx6sll-pxp', 'fsl,imx6ull-pxp'] is too long
arch/arm/boot/dts/imx6sll-evk.dtb
arch/arm/boot/dts/imx6sll-kobo-clarahd.dtb
arch/arm/boot/dts/imx6sll-kobo-librah2o.dtb
pxp@2218000: compatible:0: 'fsl,imx6sx-pxp' is not one of ['fsl,imx6ul-pxp', 'fsl,imx6ull-pxp', 'fsl,imx7d-pxp']
arch/arm/boot/dts/imx6sx-nitrogen6sx.dtb
arch/arm/boot/dts/imx6sx-sabreauto.dtb
arch/arm/boot/dts/imx6sx-sdb.dtb
arch/arm/boot/dts/imx6sx-sdb-mqs.dtb
arch/arm/boot/dts/imx6sx-sdb-reva.dtb
arch/arm/boot/dts/imx6sx-sdb-sai.dtb
arch/arm/boot/dts/imx6sx-softing-vining-2000.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-basic.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-extended.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-full.dtb
pxp@2218000: compatible: ['fsl,imx6sx-pxp', 'fsl,imx6ull-pxp'] is too long
arch/arm/boot/dts/imx6sx-nitrogen6sx.dtb
arch/arm/boot/dts/imx6sx-sabreauto.dtb
arch/arm/boot/dts/imx6sx-sdb.dtb
arch/arm/boot/dts/imx6sx-sdb-mqs.dtb
arch/arm/boot/dts/imx6sx-sdb-reva.dtb
arch/arm/boot/dts/imx6sx-sdb-sai.dtb
arch/arm/boot/dts/imx6sx-softing-vining-2000.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-basic.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-extended.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-full.dtb
pxp@2218000: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
arch/arm/boot/dts/imx6sx-nitrogen6sx.dtb
arch/arm/boot/dts/imx6sx-sabreauto.dtb
arch/arm/boot/dts/imx6sx-sdb.dtb
arch/arm/boot/dts/imx6sx-sdb-mqs.dtb
arch/arm/boot/dts/imx6sx-sdb-reva.dtb
arch/arm/boot/dts/imx6sx-sdb-sai.dtb
arch/arm/boot/dts/imx6sx-softing-vining-2000.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-basic.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-extended.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-full.dtb
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 1/8] media: dt-bindings: media: fsl-pxp: convert to yaml
@ 2023-01-06 3:18 ` Rob Herring
0 siblings, 0 replies; 62+ messages in thread
From: Rob Herring @ 2023-01-06 3:18 UTC (permalink / raw)
To: Michael Tretter
Cc: Mauro Carvalho Chehab, Fabio Estevam, Rob Herring,
linux-arm-kernel, Philipp Zabel, kernel, Krzysztof Kozlowski,
linux-media, linux-imx, Laurent Pinchart, devicetree
On Thu, 05 Jan 2023 14:47:22 +0100, Michael Tretter wrote:
> Convert the bindings of the Freescale Pixel Pipeline to YAML.
>
> The conversion drops the previously listed compatibles for several SoCs.
> It is unclear, if the PXP on these SoCs is compatible to any of the PXPs
> on the existing SoCs and would allow to reuse the already defined
> compatibles. The missing compatibles should be brought back when the
> support for the PXP on these SoCs is added.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> .../bindings/media/fsl,imx6ull-pxp.yaml | 62 +++++++++++++++++++
> .../devicetree/bindings/media/fsl-pxp.txt | 26 --------
> 2 files changed, 62 insertions(+), 26 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
>
Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.
Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.
Full log is available here: https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230105134729.59542-2-m.tretter@pengutronix.de
pxp@20f0000: compatible:0: 'fsl,imx6sll-pxp' is not one of ['fsl,imx6ul-pxp', 'fsl,imx6ull-pxp', 'fsl,imx7d-pxp']
arch/arm/boot/dts/imx6sll-evk.dtb
arch/arm/boot/dts/imx6sll-kobo-clarahd.dtb
arch/arm/boot/dts/imx6sll-kobo-librah2o.dtb
pxp@20f0000: compatible: ['fsl,imx6sll-pxp', 'fsl,imx6ull-pxp'] is too long
arch/arm/boot/dts/imx6sll-evk.dtb
arch/arm/boot/dts/imx6sll-kobo-clarahd.dtb
arch/arm/boot/dts/imx6sll-kobo-librah2o.dtb
pxp@2218000: compatible:0: 'fsl,imx6sx-pxp' is not one of ['fsl,imx6ul-pxp', 'fsl,imx6ull-pxp', 'fsl,imx7d-pxp']
arch/arm/boot/dts/imx6sx-nitrogen6sx.dtb
arch/arm/boot/dts/imx6sx-sabreauto.dtb
arch/arm/boot/dts/imx6sx-sdb.dtb
arch/arm/boot/dts/imx6sx-sdb-mqs.dtb
arch/arm/boot/dts/imx6sx-sdb-reva.dtb
arch/arm/boot/dts/imx6sx-sdb-sai.dtb
arch/arm/boot/dts/imx6sx-softing-vining-2000.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-basic.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-extended.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-full.dtb
pxp@2218000: compatible: ['fsl,imx6sx-pxp', 'fsl,imx6ull-pxp'] is too long
arch/arm/boot/dts/imx6sx-nitrogen6sx.dtb
arch/arm/boot/dts/imx6sx-sabreauto.dtb
arch/arm/boot/dts/imx6sx-sdb.dtb
arch/arm/boot/dts/imx6sx-sdb-mqs.dtb
arch/arm/boot/dts/imx6sx-sdb-reva.dtb
arch/arm/boot/dts/imx6sx-sdb-sai.dtb
arch/arm/boot/dts/imx6sx-softing-vining-2000.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-basic.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-extended.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-full.dtb
pxp@2218000: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
arch/arm/boot/dts/imx6sx-nitrogen6sx.dtb
arch/arm/boot/dts/imx6sx-sabreauto.dtb
arch/arm/boot/dts/imx6sx-sdb.dtb
arch/arm/boot/dts/imx6sx-sdb-mqs.dtb
arch/arm/boot/dts/imx6sx-sdb-reva.dtb
arch/arm/boot/dts/imx6sx-sdb-sai.dtb
arch/arm/boot/dts/imx6sx-softing-vining-2000.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-basic.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-extended.dtb
arch/arm/boot/dts/imx6sx-udoo-neo-full.dtb
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 1/8] media: dt-bindings: media: fsl-pxp: convert to yaml
2023-01-06 3:18 ` Rob Herring
@ 2023-01-06 8:23 ` Michael Tretter
-1 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-06 8:23 UTC (permalink / raw)
To: Rob Herring
Cc: Mauro Carvalho Chehab, Fabio Estevam, Rob Herring,
linux-arm-kernel, Philipp Zabel, kernel, Krzysztof Kozlowski,
linux-media, linux-imx, Laurent Pinchart, devicetree
On Thu, 05 Jan 2023 21:18:21 -0600, Rob Herring wrote:
> On Thu, 05 Jan 2023 14:47:22 +0100, Michael Tretter wrote:
> > Convert the bindings of the Freescale Pixel Pipeline to YAML.
> >
> > The conversion drops the previously listed compatibles for several SoCs.
> > It is unclear, if the PXP on these SoCs is compatible to any of the PXPs
> > on the existing SoCs and would allow to reuse the already defined
> > compatibles. The missing compatibles should be brought back when the
> > support for the PXP on these SoCs is added.
> >
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > .../bindings/media/fsl,imx6ull-pxp.yaml | 62 +++++++++++++++++++
> > .../devicetree/bindings/media/fsl-pxp.txt | 26 --------
> > 2 files changed, 62 insertions(+), 26 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> > delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
> >
>
> Running 'make dtbs_check' with the schema in this patch gives the
> following warnings. Consider if they are expected or the schema is
> incorrect. These may not be new warnings.
I am surprised that these warnings didn't show up when I ran 'make
dtbs_check'. I will check if there is something wrong with my setup.
>
> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> This will change in the future.
>
> Full log is available here: https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230105134729.59542-2-m.tretter@pengutronix.de
>
>
> pxp@20f0000: compatible:0: 'fsl,imx6sll-pxp' is not one of ['fsl,imx6ul-pxp', 'fsl,imx6ull-pxp', 'fsl,imx7d-pxp']
> arch/arm/boot/dts/imx6sll-evk.dtb
> arch/arm/boot/dts/imx6sll-kobo-clarahd.dtb
> arch/arm/boot/dts/imx6sll-kobo-librah2o.dtb
>
> pxp@20f0000: compatible: ['fsl,imx6sll-pxp', 'fsl,imx6ull-pxp'] is too long
> arch/arm/boot/dts/imx6sll-evk.dtb
> arch/arm/boot/dts/imx6sll-kobo-clarahd.dtb
> arch/arm/boot/dts/imx6sll-kobo-librah2o.dtb
This is an error in the schema. I dropped the fsl,imx6sll-pxp and
fsl,imx6sx-pxp compatibles, because I thought that they aren't used.
I will send a v2 to fix the schema.
Michael
>
> pxp@2218000: compatible:0: 'fsl,imx6sx-pxp' is not one of ['fsl,imx6ul-pxp', 'fsl,imx6ull-pxp', 'fsl,imx7d-pxp']
> arch/arm/boot/dts/imx6sx-nitrogen6sx.dtb
> arch/arm/boot/dts/imx6sx-sabreauto.dtb
> arch/arm/boot/dts/imx6sx-sdb.dtb
> arch/arm/boot/dts/imx6sx-sdb-mqs.dtb
> arch/arm/boot/dts/imx6sx-sdb-reva.dtb
> arch/arm/boot/dts/imx6sx-sdb-sai.dtb
> arch/arm/boot/dts/imx6sx-softing-vining-2000.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-basic.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-extended.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-full.dtb
>
> pxp@2218000: compatible: ['fsl,imx6sx-pxp', 'fsl,imx6ull-pxp'] is too long
> arch/arm/boot/dts/imx6sx-nitrogen6sx.dtb
> arch/arm/boot/dts/imx6sx-sabreauto.dtb
> arch/arm/boot/dts/imx6sx-sdb.dtb
> arch/arm/boot/dts/imx6sx-sdb-mqs.dtb
> arch/arm/boot/dts/imx6sx-sdb-reva.dtb
> arch/arm/boot/dts/imx6sx-sdb-sai.dtb
> arch/arm/boot/dts/imx6sx-softing-vining-2000.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-basic.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-extended.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-full.dtb
>
> pxp@2218000: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
> arch/arm/boot/dts/imx6sx-nitrogen6sx.dtb
> arch/arm/boot/dts/imx6sx-sabreauto.dtb
> arch/arm/boot/dts/imx6sx-sdb.dtb
> arch/arm/boot/dts/imx6sx-sdb-mqs.dtb
> arch/arm/boot/dts/imx6sx-sdb-reva.dtb
> arch/arm/boot/dts/imx6sx-sdb-sai.dtb
> arch/arm/boot/dts/imx6sx-softing-vining-2000.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-basic.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-extended.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-full.dtb
>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 1/8] media: dt-bindings: media: fsl-pxp: convert to yaml
@ 2023-01-06 8:23 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-06 8:23 UTC (permalink / raw)
To: Rob Herring
Cc: Mauro Carvalho Chehab, Fabio Estevam, Rob Herring,
linux-arm-kernel, Philipp Zabel, kernel, Krzysztof Kozlowski,
linux-media, linux-imx, Laurent Pinchart, devicetree
On Thu, 05 Jan 2023 21:18:21 -0600, Rob Herring wrote:
> On Thu, 05 Jan 2023 14:47:22 +0100, Michael Tretter wrote:
> > Convert the bindings of the Freescale Pixel Pipeline to YAML.
> >
> > The conversion drops the previously listed compatibles for several SoCs.
> > It is unclear, if the PXP on these SoCs is compatible to any of the PXPs
> > on the existing SoCs and would allow to reuse the already defined
> > compatibles. The missing compatibles should be brought back when the
> > support for the PXP on these SoCs is added.
> >
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > .../bindings/media/fsl,imx6ull-pxp.yaml | 62 +++++++++++++++++++
> > .../devicetree/bindings/media/fsl-pxp.txt | 26 --------
> > 2 files changed, 62 insertions(+), 26 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> > delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
> >
>
> Running 'make dtbs_check' with the schema in this patch gives the
> following warnings. Consider if they are expected or the schema is
> incorrect. These may not be new warnings.
I am surprised that these warnings didn't show up when I ran 'make
dtbs_check'. I will check if there is something wrong with my setup.
>
> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> This will change in the future.
>
> Full log is available here: https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230105134729.59542-2-m.tretter@pengutronix.de
>
>
> pxp@20f0000: compatible:0: 'fsl,imx6sll-pxp' is not one of ['fsl,imx6ul-pxp', 'fsl,imx6ull-pxp', 'fsl,imx7d-pxp']
> arch/arm/boot/dts/imx6sll-evk.dtb
> arch/arm/boot/dts/imx6sll-kobo-clarahd.dtb
> arch/arm/boot/dts/imx6sll-kobo-librah2o.dtb
>
> pxp@20f0000: compatible: ['fsl,imx6sll-pxp', 'fsl,imx6ull-pxp'] is too long
> arch/arm/boot/dts/imx6sll-evk.dtb
> arch/arm/boot/dts/imx6sll-kobo-clarahd.dtb
> arch/arm/boot/dts/imx6sll-kobo-librah2o.dtb
This is an error in the schema. I dropped the fsl,imx6sll-pxp and
fsl,imx6sx-pxp compatibles, because I thought that they aren't used.
I will send a v2 to fix the schema.
Michael
>
> pxp@2218000: compatible:0: 'fsl,imx6sx-pxp' is not one of ['fsl,imx6ul-pxp', 'fsl,imx6ull-pxp', 'fsl,imx7d-pxp']
> arch/arm/boot/dts/imx6sx-nitrogen6sx.dtb
> arch/arm/boot/dts/imx6sx-sabreauto.dtb
> arch/arm/boot/dts/imx6sx-sdb.dtb
> arch/arm/boot/dts/imx6sx-sdb-mqs.dtb
> arch/arm/boot/dts/imx6sx-sdb-reva.dtb
> arch/arm/boot/dts/imx6sx-sdb-sai.dtb
> arch/arm/boot/dts/imx6sx-softing-vining-2000.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-basic.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-extended.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-full.dtb
>
> pxp@2218000: compatible: ['fsl,imx6sx-pxp', 'fsl,imx6ull-pxp'] is too long
> arch/arm/boot/dts/imx6sx-nitrogen6sx.dtb
> arch/arm/boot/dts/imx6sx-sabreauto.dtb
> arch/arm/boot/dts/imx6sx-sdb.dtb
> arch/arm/boot/dts/imx6sx-sdb-mqs.dtb
> arch/arm/boot/dts/imx6sx-sdb-reva.dtb
> arch/arm/boot/dts/imx6sx-sdb-sai.dtb
> arch/arm/boot/dts/imx6sx-softing-vining-2000.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-basic.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-extended.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-full.dtb
>
> pxp@2218000: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
> arch/arm/boot/dts/imx6sx-nitrogen6sx.dtb
> arch/arm/boot/dts/imx6sx-sabreauto.dtb
> arch/arm/boot/dts/imx6sx-sdb.dtb
> arch/arm/boot/dts/imx6sx-sdb-mqs.dtb
> arch/arm/boot/dts/imx6sx-sdb-reva.dtb
> arch/arm/boot/dts/imx6sx-sdb-sai.dtb
> arch/arm/boot/dts/imx6sx-softing-vining-2000.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-basic.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-extended.dtb
> arch/arm/boot/dts/imx6sx-udoo-neo-full.dtb
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 1/8] media: dt-bindings: media: fsl-pxp: convert to yaml
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-06 11:35 ` Laurent Pinchart
-1 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 11:35 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:22PM +0100, Michael Tretter wrote:
> Convert the bindings of the Freescale Pixel Pipeline to YAML.
>
> The conversion drops the previously listed compatibles for several SoCs.
> It is unclear, if the PXP on these SoCs is compatible to any of the PXPs
> on the existing SoCs and would allow to reuse the already defined
> compatibles. The missing compatibles should be brought back when the
> support for the PXP on these SoCs is added.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> .../bindings/media/fsl,imx6ull-pxp.yaml | 62 +++++++++++++++++++
> .../devicetree/bindings/media/fsl-pxp.txt | 26 --------
> 2 files changed, 62 insertions(+), 26 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
>
> diff --git a/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> new file mode 100644
> index 000000000000..e5f227b84759
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/media/fsl,imx6ull-pxp.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Freescale Pixel Pipeline
> +
> +maintainers:
> + - Philipp Zabel <p.zabel@pengutronix.de>
> + - Michael Tretter <m.tretter@pengutronix.de>
> +
> +description:
> + The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine
> + that supports scaling, colorspace conversion, alpha blending, rotation, and
> + pixel conversion via lookup table. Different versions are present on various
> + i.MX SoCs from i.MX23 to i.MX7.
> +
> +properties:
> + compatible:
> + enum:
> + - fsl,imx6ul-pxp
> + - fsl,imx6ull-pxp
> + - fsl,imx7d-pxp
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 1
> + maxItems: 2
Can you make the number of items conditional on the compatible string ?
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + items:
> + - const: axi
I think this could be simplified to
clock-names:
const: axi
Up to you.
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> +
> +additionalProperties: False
s/False/false/
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/imx6ul-clock.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + pxp: pxp@21cc000 {
> + compatible = "fsl,imx6ull-pxp";
> + reg = <0x021cc000 0x4000>;
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
> + clock-names = "axi";
> + clocks = <&clks IMX6UL_CLK_PXP>;
> + };
> diff --git a/Documentation/devicetree/bindings/media/fsl-pxp.txt b/Documentation/devicetree/bindings/media/fsl-pxp.txt
> deleted file mode 100644
> index f8090e06530d..000000000000
> --- a/Documentation/devicetree/bindings/media/fsl-pxp.txt
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -Freescale Pixel Pipeline
> -========================
> -
> -The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine
> -that supports scaling, colorspace conversion, alpha blending, rotation, and
> -pixel conversion via lookup table. Different versions are present on various
> -i.MX SoCs from i.MX23 to i.MX7.
> -
> -Required properties:
> -- compatible: should be "fsl,<soc>-pxp", where SoC can be one of imx23, imx28,
> - imx6dl, imx6sl, imx6sll, imx6ul, imx6sx, imx6ull, or imx7d.
> -- reg: the register base and size for the device registers
> -- interrupts: the PXP interrupt, two interrupts for imx6ull and imx7d.
> -- clock-names: should be "axi"
> -- clocks: the PXP AXI clock
> -
> -Example:
> -
> -pxp@21cc000 {
> - compatible = "fsl,imx6ull-pxp";
> - reg = <0x021cc000 0x4000>;
> - interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> - <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
> - clock-names = "axi";
> - clocks = <&clks IMX6UL_CLK_PXP>;
> -};
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 1/8] media: dt-bindings: media: fsl-pxp: convert to yaml
@ 2023-01-06 11:35 ` Laurent Pinchart
0 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 11:35 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:22PM +0100, Michael Tretter wrote:
> Convert the bindings of the Freescale Pixel Pipeline to YAML.
>
> The conversion drops the previously listed compatibles for several SoCs.
> It is unclear, if the PXP on these SoCs is compatible to any of the PXPs
> on the existing SoCs and would allow to reuse the already defined
> compatibles. The missing compatibles should be brought back when the
> support for the PXP on these SoCs is added.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> .../bindings/media/fsl,imx6ull-pxp.yaml | 62 +++++++++++++++++++
> .../devicetree/bindings/media/fsl-pxp.txt | 26 --------
> 2 files changed, 62 insertions(+), 26 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
>
> diff --git a/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> new file mode 100644
> index 000000000000..e5f227b84759
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/media/fsl,imx6ull-pxp.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Freescale Pixel Pipeline
> +
> +maintainers:
> + - Philipp Zabel <p.zabel@pengutronix.de>
> + - Michael Tretter <m.tretter@pengutronix.de>
> +
> +description:
> + The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine
> + that supports scaling, colorspace conversion, alpha blending, rotation, and
> + pixel conversion via lookup table. Different versions are present on various
> + i.MX SoCs from i.MX23 to i.MX7.
> +
> +properties:
> + compatible:
> + enum:
> + - fsl,imx6ul-pxp
> + - fsl,imx6ull-pxp
> + - fsl,imx7d-pxp
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 1
> + maxItems: 2
Can you make the number of items conditional on the compatible string ?
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + items:
> + - const: axi
I think this could be simplified to
clock-names:
const: axi
Up to you.
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> +
> +additionalProperties: False
s/False/false/
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/imx6ul-clock.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + pxp: pxp@21cc000 {
> + compatible = "fsl,imx6ull-pxp";
> + reg = <0x021cc000 0x4000>;
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
> + clock-names = "axi";
> + clocks = <&clks IMX6UL_CLK_PXP>;
> + };
> diff --git a/Documentation/devicetree/bindings/media/fsl-pxp.txt b/Documentation/devicetree/bindings/media/fsl-pxp.txt
> deleted file mode 100644
> index f8090e06530d..000000000000
> --- a/Documentation/devicetree/bindings/media/fsl-pxp.txt
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -Freescale Pixel Pipeline
> -========================
> -
> -The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine
> -that supports scaling, colorspace conversion, alpha blending, rotation, and
> -pixel conversion via lookup table. Different versions are present on various
> -i.MX SoCs from i.MX23 to i.MX7.
> -
> -Required properties:
> -- compatible: should be "fsl,<soc>-pxp", where SoC can be one of imx23, imx28,
> - imx6dl, imx6sl, imx6sll, imx6ul, imx6sx, imx6ull, or imx7d.
> -- reg: the register base and size for the device registers
> -- interrupts: the PXP interrupt, two interrupts for imx6ull and imx7d.
> -- clock-names: should be "axi"
> -- clocks: the PXP AXI clock
> -
> -Example:
> -
> -pxp@21cc000 {
> - compatible = "fsl,imx6ull-pxp";
> - reg = <0x021cc000 0x4000>;
> - interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> - <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
> - clock-names = "axi";
> - clocks = <&clks IMX6UL_CLK_PXP>;
> -};
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 2/8] media: imx-pxp: detect PXP version
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-06 11:47 ` Laurent Pinchart
-1 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 11:47 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:23PM +0100, Michael Tretter wrote:
> Different versions of the Pixel Pipeline have different blocks and their
> routing may be different. Read the PXP_HW_VERSION register to determine
> the version of the PXP and print it to the log for debugging purposes.
Is there a specific reason you chose to read the version register
instead of basing the decision on the compatible string ?
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> drivers/media/platform/nxp/imx-pxp.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> index 689ae5e6ac62..05abe40558b0 100644
> --- a/drivers/media/platform/nxp/imx-pxp.c
> +++ b/drivers/media/platform/nxp/imx-pxp.c
> @@ -10,6 +10,7 @@
> * Pawel Osciak, <pawel@osciak.com>
> * Marek Szyprowski, <m.szyprowski@samsung.com>
> */
> +#include <linux/bitfield.h>
> #include <linux/clk.h>
> #include <linux/delay.h>
> #include <linux/dma-mapping.h>
> @@ -52,6 +53,11 @@ MODULE_PARM_DESC(debug, "activates debug info");
> #define MEM2MEM_HFLIP (1 << 0)
> #define MEM2MEM_VFLIP (1 << 1)
>
> +#define PXP_VERSION_MAJOR(version) \
> + FIELD_GET(BM_PXP_VERSION_MAJOR, version)
> +#define PXP_VERSION_MINOR(version) \
> + FIELD_GET(BM_PXP_VERSION_MINOR, version)
> +
> #define dprintk(dev, fmt, arg...) \
> v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: " fmt, __func__, ## arg)
>
> @@ -192,6 +198,8 @@ struct pxp_dev {
> struct clk *clk;
> void __iomem *mmio;
>
> + u32 hw_version;
> +
> atomic_t num_inst;
> struct mutex dev_mutex;
> spinlock_t irqlock;
> @@ -1660,6 +1668,11 @@ static int pxp_soft_reset(struct pxp_dev *dev)
> return 0;
> }
>
> +static u32 pxp_read_version(struct pxp_dev *dev)
> +{
> + return readl(dev->mmio + HW_PXP_VERSION);
> +}
> +
> static int pxp_probe(struct platform_device *pdev)
> {
> struct pxp_dev *dev;
> @@ -1705,6 +1718,11 @@ static int pxp_probe(struct platform_device *pdev)
> goto err_clk;
> }
>
> + dev->hw_version = pxp_read_version(dev);
> + dev_info(&pdev->dev, "PXP Version %d.%d\n",
As the version can't be negative, I'd use %u.%u.
> + PXP_VERSION_MAJOR(dev->hw_version),
> + PXP_VERSION_MINOR(dev->hw_version));
> +
The driver now prints two messages at probe time, it would be nice to
combine them, or remove the other one. That's a candidate for a future
patch though.
> ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> if (ret)
> goto err_clk;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 2/8] media: imx-pxp: detect PXP version
@ 2023-01-06 11:47 ` Laurent Pinchart
0 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 11:47 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:23PM +0100, Michael Tretter wrote:
> Different versions of the Pixel Pipeline have different blocks and their
> routing may be different. Read the PXP_HW_VERSION register to determine
> the version of the PXP and print it to the log for debugging purposes.
Is there a specific reason you chose to read the version register
instead of basing the decision on the compatible string ?
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> drivers/media/platform/nxp/imx-pxp.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> index 689ae5e6ac62..05abe40558b0 100644
> --- a/drivers/media/platform/nxp/imx-pxp.c
> +++ b/drivers/media/platform/nxp/imx-pxp.c
> @@ -10,6 +10,7 @@
> * Pawel Osciak, <pawel@osciak.com>
> * Marek Szyprowski, <m.szyprowski@samsung.com>
> */
> +#include <linux/bitfield.h>
> #include <linux/clk.h>
> #include <linux/delay.h>
> #include <linux/dma-mapping.h>
> @@ -52,6 +53,11 @@ MODULE_PARM_DESC(debug, "activates debug info");
> #define MEM2MEM_HFLIP (1 << 0)
> #define MEM2MEM_VFLIP (1 << 1)
>
> +#define PXP_VERSION_MAJOR(version) \
> + FIELD_GET(BM_PXP_VERSION_MAJOR, version)
> +#define PXP_VERSION_MINOR(version) \
> + FIELD_GET(BM_PXP_VERSION_MINOR, version)
> +
> #define dprintk(dev, fmt, arg...) \
> v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: " fmt, __func__, ## arg)
>
> @@ -192,6 +198,8 @@ struct pxp_dev {
> struct clk *clk;
> void __iomem *mmio;
>
> + u32 hw_version;
> +
> atomic_t num_inst;
> struct mutex dev_mutex;
> spinlock_t irqlock;
> @@ -1660,6 +1668,11 @@ static int pxp_soft_reset(struct pxp_dev *dev)
> return 0;
> }
>
> +static u32 pxp_read_version(struct pxp_dev *dev)
> +{
> + return readl(dev->mmio + HW_PXP_VERSION);
> +}
> +
> static int pxp_probe(struct platform_device *pdev)
> {
> struct pxp_dev *dev;
> @@ -1705,6 +1718,11 @@ static int pxp_probe(struct platform_device *pdev)
> goto err_clk;
> }
>
> + dev->hw_version = pxp_read_version(dev);
> + dev_info(&pdev->dev, "PXP Version %d.%d\n",
As the version can't be negative, I'd use %u.%u.
> + PXP_VERSION_MAJOR(dev->hw_version),
> + PXP_VERSION_MINOR(dev->hw_version));
> +
The driver now prints two messages at probe time, it would be nice to
combine them, or remove the other one. That's a candidate for a future
patch though.
> ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> if (ret)
> goto err_clk;
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 3/8] media: imx-pxp: extract helper function to setup data path
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-06 11:59 ` Laurent Pinchart
-1 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 11:59 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:24PM +0100, Michael Tretter wrote:
> The driver must configure the data path through the Pixel Pipeline.
>
> Currently, the driver is using a fixed setup, but once there are
> different pipeline configurations, it is helpful to have a dedicated
> function for determining the register value for the data path.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/platform/nxp/imx-pxp.c | 62 +++++++++++++++++++---------
> 1 file changed, 42 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> index 05abe40558b0..a957fee88829 100644
> --- a/drivers/media/platform/nxp/imx-pxp.c
> +++ b/drivers/media/platform/nxp/imx-pxp.c
> @@ -726,6 +726,47 @@ static void pxp_setup_csc(struct pxp_ctx *ctx)
> }
> }
>
> +static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> +{
> + u32 ctrl0;
> +
> + ctrl0 = 0;
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0);
> +
> + return ctrl0;
> +}
> +
> +static void pxp_set_data_path(struct pxp_ctx *ctx)
> +{
> + struct pxp_dev *dev = ctx->dev;
> + u32 ctrl0;
> + u32 ctrl1;
> +
> + ctrl0 = pxp_data_path_ctrl0(ctx);
> +
> + ctrl1 = 0;
> + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1);
> + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1);
> +
> + writel(ctrl0, dev->mmio + HW_PXP_DATA_PATH_CTRL0);
> + writel(ctrl1, dev->mmio + HW_PXP_DATA_PATH_CTRL1);
> +}
> +
> static int pxp_start(struct pxp_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
> struct vb2_v4l2_buffer *out_vb)
> {
> @@ -912,26 +953,7 @@ static int pxp_start(struct pxp_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
> /* bypass LUT */
> writel(BM_PXP_LUT_CTRL_BYPASS, dev->mmio + HW_PXP_LUT_CTRL);
>
> - writel(BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1)|
> - BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1)|
> - BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0),
> - dev->mmio + HW_PXP_DATA_PATH_CTRL0);
> - writel(BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1) |
> - BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1),
> - dev->mmio + HW_PXP_DATA_PATH_CTRL1);
> + pxp_set_data_path(ctx);
>
> writel(0xffff, dev->mmio + HW_PXP_IRQ_MASK);
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 3/8] media: imx-pxp: extract helper function to setup data path
@ 2023-01-06 11:59 ` Laurent Pinchart
0 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 11:59 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:24PM +0100, Michael Tretter wrote:
> The driver must configure the data path through the Pixel Pipeline.
>
> Currently, the driver is using a fixed setup, but once there are
> different pipeline configurations, it is helpful to have a dedicated
> function for determining the register value for the data path.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/platform/nxp/imx-pxp.c | 62 +++++++++++++++++++---------
> 1 file changed, 42 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> index 05abe40558b0..a957fee88829 100644
> --- a/drivers/media/platform/nxp/imx-pxp.c
> +++ b/drivers/media/platform/nxp/imx-pxp.c
> @@ -726,6 +726,47 @@ static void pxp_setup_csc(struct pxp_ctx *ctx)
> }
> }
>
> +static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> +{
> + u32 ctrl0;
> +
> + ctrl0 = 0;
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0);
> +
> + return ctrl0;
> +}
> +
> +static void pxp_set_data_path(struct pxp_ctx *ctx)
> +{
> + struct pxp_dev *dev = ctx->dev;
> + u32 ctrl0;
> + u32 ctrl1;
> +
> + ctrl0 = pxp_data_path_ctrl0(ctx);
> +
> + ctrl1 = 0;
> + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1);
> + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1);
> +
> + writel(ctrl0, dev->mmio + HW_PXP_DATA_PATH_CTRL0);
> + writel(ctrl1, dev->mmio + HW_PXP_DATA_PATH_CTRL1);
> +}
> +
> static int pxp_start(struct pxp_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
> struct vb2_v4l2_buffer *out_vb)
> {
> @@ -912,26 +953,7 @@ static int pxp_start(struct pxp_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
> /* bypass LUT */
> writel(BM_PXP_LUT_CTRL_BYPASS, dev->mmio + HW_PXP_LUT_CTRL);
>
> - writel(BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1)|
> - BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1)|
> - BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0)|
> - BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0),
> - dev->mmio + HW_PXP_DATA_PATH_CTRL0);
> - writel(BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1) |
> - BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1),
> - dev->mmio + HW_PXP_DATA_PATH_CTRL1);
> + pxp_set_data_path(ctx);
>
> writel(0xffff, dev->mmio + HW_PXP_IRQ_MASK);
>
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 4/8] media: imx-pxp: explicitly disable unused blocks
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-06 12:26 ` Laurent Pinchart
-1 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 12:26 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:25PM +0100, Michael Tretter wrote:
> Various multiplexers in the pipeline are not used with the currently
> configured data path. Disable all unused multiplexers by selecting the
> "no output" (3) option.
>
> The datasheet doesn't explicitly require this, but the PXP has been seen
> to hang after processing a few hundreds of frames otherwise.
On which platform(s) have you noticed that ?
> As at it, add documentation for the multiplexers that are actually
> relevant for the data path.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> drivers/media/platform/nxp/imx-pxp.c | 30 +++++++++++++++++-----------
> 1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> index a957fee88829..6ffd07cda965 100644
> --- a/drivers/media/platform/nxp/imx-pxp.c
> +++ b/drivers/media/platform/nxp/imx-pxp.c
> @@ -731,22 +731,28 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> u32 ctrl0;
>
> ctrl0 = 0;
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(3);
> + /* Bypass Dithering x3CH */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
> + /* Select Rotation */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
> + /* Select LUT */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
> + /* Select MUX8 for LUT */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
> + /* Select CSC 2 */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(3);
> + /* Bypass Rotation 2 */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(3);
The muxes being disabled look fine to me, but the values of MUX8, MUX12
and MUX14 look strange based on the i.MX7D reference manual. Maybe the
register values were different in previous SoCs ? I haven't found any
other relevant reference manual that document the mux values, I may have
overlooked something.
Anyway, this isn't an issue with this patch, so
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> return ctrl0;
> }
> @@ -760,8 +766,8 @@ static void pxp_set_data_path(struct pxp_ctx *ctx)
> ctrl0 = pxp_data_path_ctrl0(ctx);
>
> ctrl1 = 0;
> - ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1);
> - ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1);
> + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(3);
> + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(3);
>
> writel(ctrl0, dev->mmio + HW_PXP_DATA_PATH_CTRL0);
> writel(ctrl1, dev->mmio + HW_PXP_DATA_PATH_CTRL1);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 4/8] media: imx-pxp: explicitly disable unused blocks
@ 2023-01-06 12:26 ` Laurent Pinchart
0 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 12:26 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:25PM +0100, Michael Tretter wrote:
> Various multiplexers in the pipeline are not used with the currently
> configured data path. Disable all unused multiplexers by selecting the
> "no output" (3) option.
>
> The datasheet doesn't explicitly require this, but the PXP has been seen
> to hang after processing a few hundreds of frames otherwise.
On which platform(s) have you noticed that ?
> As at it, add documentation for the multiplexers that are actually
> relevant for the data path.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> drivers/media/platform/nxp/imx-pxp.c | 30 +++++++++++++++++-----------
> 1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> index a957fee88829..6ffd07cda965 100644
> --- a/drivers/media/platform/nxp/imx-pxp.c
> +++ b/drivers/media/platform/nxp/imx-pxp.c
> @@ -731,22 +731,28 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> u32 ctrl0;
>
> ctrl0 = 0;
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(3);
> + /* Bypass Dithering x3CH */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
> + /* Select Rotation */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
> + /* Select LUT */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
> + /* Select MUX8 for LUT */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
> + /* Select CSC 2 */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(3);
> + /* Bypass Rotation 2 */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0);
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(3);
The muxes being disabled look fine to me, but the values of MUX8, MUX12
and MUX14 look strange based on the i.MX7D reference manual. Maybe the
register values were different in previous SoCs ? I haven't found any
other relevant reference manual that document the mux values, I may have
overlooked something.
Anyway, this isn't an issue with this patch, so
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> return ctrl0;
> }
> @@ -760,8 +766,8 @@ static void pxp_set_data_path(struct pxp_ctx *ctx)
> ctrl0 = pxp_data_path_ctrl0(ctx);
>
> ctrl1 = 0;
> - ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1);
> - ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1);
> + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(3);
> + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(3);
>
> writel(ctrl0, dev->mmio + HW_PXP_DATA_PATH_CTRL0);
> writel(ctrl1, dev->mmio + HW_PXP_DATA_PATH_CTRL1);
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 5/8] media: imx-pxp: disable LUT block
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-06 12:27 ` Laurent Pinchart
-1 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 12:27 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:26PM +0100, Michael Tretter wrote:
> The LUT block is always configured in bypass mode.
>
> Take it entirely out of the pipeline by disabling it and routing the
> data path around the LUT.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/platform/nxp/imx-pxp.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> index 6ffd07cda965..1d649b9cadad 100644
> --- a/drivers/media/platform/nxp/imx-pxp.c
> +++ b/drivers/media/platform/nxp/imx-pxp.c
> @@ -737,11 +737,10 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
> /* Select Rotation */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
> - /* Select LUT */
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
> + /* Bypass LUT */
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(1);
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
> - /* Select MUX8 for LUT */
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(3);
> /* Select CSC 2 */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
> @@ -966,7 +965,7 @@ static int pxp_start(struct pxp_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
> /* ungate, enable PS/AS/OUT and PXP operation */
> writel(BM_PXP_CTRL_IRQ_ENABLE, dev->mmio + HW_PXP_CTRL_SET);
> writel(BM_PXP_CTRL_ENABLE | BM_PXP_CTRL_ENABLE_CSC2 |
> - BM_PXP_CTRL_ENABLE_LUT | BM_PXP_CTRL_ENABLE_ROTATE0 |
> + BM_PXP_CTRL_ENABLE_ROTATE0 |
> BM_PXP_CTRL_ENABLE_PS_AS_OUT, dev->mmio + HW_PXP_CTRL_SET);
>
> return 0;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 5/8] media: imx-pxp: disable LUT block
@ 2023-01-06 12:27 ` Laurent Pinchart
0 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 12:27 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:26PM +0100, Michael Tretter wrote:
> The LUT block is always configured in bypass mode.
>
> Take it entirely out of the pipeline by disabling it and routing the
> data path around the LUT.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/platform/nxp/imx-pxp.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> index 6ffd07cda965..1d649b9cadad 100644
> --- a/drivers/media/platform/nxp/imx-pxp.c
> +++ b/drivers/media/platform/nxp/imx-pxp.c
> @@ -737,11 +737,10 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
> /* Select Rotation */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
> - /* Select LUT */
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
> + /* Bypass LUT */
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(1);
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
> - /* Select MUX8 for LUT */
> - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(3);
> /* Select CSC 2 */
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
> ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
> @@ -966,7 +965,7 @@ static int pxp_start(struct pxp_ctx *ctx, struct vb2_v4l2_buffer *in_vb,
> /* ungate, enable PS/AS/OUT and PXP operation */
> writel(BM_PXP_CTRL_IRQ_ENABLE, dev->mmio + HW_PXP_CTRL_SET);
> writel(BM_PXP_CTRL_ENABLE | BM_PXP_CTRL_ENABLE_CSC2 |
> - BM_PXP_CTRL_ENABLE_LUT | BM_PXP_CTRL_ENABLE_ROTATE0 |
> + BM_PXP_CTRL_ENABLE_ROTATE0 |
> BM_PXP_CTRL_ENABLE_PS_AS_OUT, dev->mmio + HW_PXP_CTRL_SET);
>
> return 0;
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 2/8] media: imx-pxp: detect PXP version
2023-01-06 11:47 ` Laurent Pinchart
@ 2023-01-06 12:28 ` Laurent Pinchart
-1 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 12:28 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, Jan 06, 2023 at 01:47:32PM +0200, Laurent Pinchart wrote:
> Hi Michael,
>
> Thank you for the patch.
>
> On Thu, Jan 05, 2023 at 02:47:23PM +0100, Michael Tretter wrote:
> > Different versions of the Pixel Pipeline have different blocks and their
> > routing may be different. Read the PXP_HW_VERSION register to determine
> > the version of the PXP and print it to the log for debugging purposes.
>
> Is there a specific reason you chose to read the version register
> instead of basing the decision on the compatible string ?
Reading the rest of the series, you use the compatible strings later,
and never use the hw_version field. I'm tempted to propose dropping this
patch.
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > drivers/media/platform/nxp/imx-pxp.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> > index 689ae5e6ac62..05abe40558b0 100644
> > --- a/drivers/media/platform/nxp/imx-pxp.c
> > +++ b/drivers/media/platform/nxp/imx-pxp.c
> > @@ -10,6 +10,7 @@
> > * Pawel Osciak, <pawel@osciak.com>
> > * Marek Szyprowski, <m.szyprowski@samsung.com>
> > */
> > +#include <linux/bitfield.h>
> > #include <linux/clk.h>
> > #include <linux/delay.h>
> > #include <linux/dma-mapping.h>
> > @@ -52,6 +53,11 @@ MODULE_PARM_DESC(debug, "activates debug info");
> > #define MEM2MEM_HFLIP (1 << 0)
> > #define MEM2MEM_VFLIP (1 << 1)
> >
> > +#define PXP_VERSION_MAJOR(version) \
> > + FIELD_GET(BM_PXP_VERSION_MAJOR, version)
> > +#define PXP_VERSION_MINOR(version) \
> > + FIELD_GET(BM_PXP_VERSION_MINOR, version)
> > +
> > #define dprintk(dev, fmt, arg...) \
> > v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: " fmt, __func__, ## arg)
> >
> > @@ -192,6 +198,8 @@ struct pxp_dev {
> > struct clk *clk;
> > void __iomem *mmio;
> >
> > + u32 hw_version;
> > +
> > atomic_t num_inst;
> > struct mutex dev_mutex;
> > spinlock_t irqlock;
> > @@ -1660,6 +1668,11 @@ static int pxp_soft_reset(struct pxp_dev *dev)
> > return 0;
> > }
> >
> > +static u32 pxp_read_version(struct pxp_dev *dev)
> > +{
> > + return readl(dev->mmio + HW_PXP_VERSION);
> > +}
> > +
> > static int pxp_probe(struct platform_device *pdev)
> > {
> > struct pxp_dev *dev;
> > @@ -1705,6 +1718,11 @@ static int pxp_probe(struct platform_device *pdev)
> > goto err_clk;
> > }
> >
> > + dev->hw_version = pxp_read_version(dev);
> > + dev_info(&pdev->dev, "PXP Version %d.%d\n",
>
> As the version can't be negative, I'd use %u.%u.
>
> > + PXP_VERSION_MAJOR(dev->hw_version),
> > + PXP_VERSION_MINOR(dev->hw_version));
> > +
>
> The driver now prints two messages at probe time, it would be nice to
> combine them, or remove the other one. That's a candidate for a future
> patch though.
>
> > ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> > if (ret)
> > goto err_clk;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 2/8] media: imx-pxp: detect PXP version
@ 2023-01-06 12:28 ` Laurent Pinchart
0 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 12:28 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, Jan 06, 2023 at 01:47:32PM +0200, Laurent Pinchart wrote:
> Hi Michael,
>
> Thank you for the patch.
>
> On Thu, Jan 05, 2023 at 02:47:23PM +0100, Michael Tretter wrote:
> > Different versions of the Pixel Pipeline have different blocks and their
> > routing may be different. Read the PXP_HW_VERSION register to determine
> > the version of the PXP and print it to the log for debugging purposes.
>
> Is there a specific reason you chose to read the version register
> instead of basing the decision on the compatible string ?
Reading the rest of the series, you use the compatible strings later,
and never use the hw_version field. I'm tempted to propose dropping this
patch.
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > drivers/media/platform/nxp/imx-pxp.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> > index 689ae5e6ac62..05abe40558b0 100644
> > --- a/drivers/media/platform/nxp/imx-pxp.c
> > +++ b/drivers/media/platform/nxp/imx-pxp.c
> > @@ -10,6 +10,7 @@
> > * Pawel Osciak, <pawel@osciak.com>
> > * Marek Szyprowski, <m.szyprowski@samsung.com>
> > */
> > +#include <linux/bitfield.h>
> > #include <linux/clk.h>
> > #include <linux/delay.h>
> > #include <linux/dma-mapping.h>
> > @@ -52,6 +53,11 @@ MODULE_PARM_DESC(debug, "activates debug info");
> > #define MEM2MEM_HFLIP (1 << 0)
> > #define MEM2MEM_VFLIP (1 << 1)
> >
> > +#define PXP_VERSION_MAJOR(version) \
> > + FIELD_GET(BM_PXP_VERSION_MAJOR, version)
> > +#define PXP_VERSION_MINOR(version) \
> > + FIELD_GET(BM_PXP_VERSION_MINOR, version)
> > +
> > #define dprintk(dev, fmt, arg...) \
> > v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: " fmt, __func__, ## arg)
> >
> > @@ -192,6 +198,8 @@ struct pxp_dev {
> > struct clk *clk;
> > void __iomem *mmio;
> >
> > + u32 hw_version;
> > +
> > atomic_t num_inst;
> > struct mutex dev_mutex;
> > spinlock_t irqlock;
> > @@ -1660,6 +1668,11 @@ static int pxp_soft_reset(struct pxp_dev *dev)
> > return 0;
> > }
> >
> > +static u32 pxp_read_version(struct pxp_dev *dev)
> > +{
> > + return readl(dev->mmio + HW_PXP_VERSION);
> > +}
> > +
> > static int pxp_probe(struct platform_device *pdev)
> > {
> > struct pxp_dev *dev;
> > @@ -1705,6 +1718,11 @@ static int pxp_probe(struct platform_device *pdev)
> > goto err_clk;
> > }
> >
> > + dev->hw_version = pxp_read_version(dev);
> > + dev_info(&pdev->dev, "PXP Version %d.%d\n",
>
> As the version can't be negative, I'd use %u.%u.
>
> > + PXP_VERSION_MAJOR(dev->hw_version),
> > + PXP_VERSION_MINOR(dev->hw_version));
> > +
>
> The driver now prints two messages at probe time, it would be nice to
> combine them, or remove the other one. That's a candidate for a future
> patch though.
>
> > ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> > if (ret)
> > goto err_clk;
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 6/8] media: imx-pxp: make data_path_ctrl0 platform dependent
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-06 12:30 ` Laurent Pinchart
-1 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 12:30 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:27PM +0100, Michael Tretter wrote:
> Unfortunately, the PXP_HW_VERSION register reports the PXP on the i.MX7D
> and on the i.MX6ULL as version 3.0, although the PXP versions on these
> SoCs have significant differences.
>
> Use the compatible to configure the ctrl0 register as required dependent
> on the platform.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> drivers/media/platform/nxp/imx-pxp.c | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> index 1d649b9cadad..4e182f80a36b 100644
> --- a/drivers/media/platform/nxp/imx-pxp.c
> +++ b/drivers/media/platform/nxp/imx-pxp.c
> @@ -19,6 +19,7 @@
> #include <linux/iopoll.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_device.h>
> #include <linux/sched.h>
> #include <linux/slab.h>
>
> @@ -191,6 +192,11 @@ static struct pxp_fmt *find_format(struct v4l2_format *f)
> return &formats[k];
> }
>
> +struct pxp_ctx;
Please add a blank line here.
> +struct pxp_pdata {
> + u32 (*data_path_ctrl0)(struct pxp_ctx *ctx);
> +};
> +
> struct pxp_dev {
> struct v4l2_device v4l2_dev;
> struct video_device vfd;
> @@ -199,6 +205,7 @@ struct pxp_dev {
> void __iomem *mmio;
>
> u32 hw_version;
> + const struct pxp_pdata *pdata;
>
> atomic_t num_inst;
> struct mutex dev_mutex;
> @@ -726,7 +733,7 @@ static void pxp_setup_csc(struct pxp_ctx *ctx)
> }
> }
>
> -static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> +static u32 pxp_imx6ull_data_path_ctrl0(struct pxp_ctx *ctx)
> {
> u32 ctrl0;
>
> @@ -756,6 +763,16 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> return ctrl0;
> }
>
> +static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> +{
> + struct pxp_dev *dev = ctx->dev;
> +
> + if (dev->pdata && dev->pdata->data_path_ctrl0)
> + return dev->pdata->data_path_ctrl0(ctx);
> +
> + return pxp_imx6ull_data_path_ctrl0(ctx);
Do you need this fallback, given that all compatible strings give you
valid pdata ? I'd rather be explicit.
This function then becomes so small that I would inline it in the
caller.
> +}
> +
> static void pxp_set_data_path(struct pxp_ctx *ctx)
> {
> struct pxp_dev *dev = ctx->dev;
> @@ -1711,6 +1728,8 @@ static int pxp_probe(struct platform_device *pdev)
> if (!dev)
> return -ENOMEM;
>
> + dev->pdata = of_device_get_match_data(&pdev->dev);
> +
> dev->clk = devm_clk_get(&pdev->dev, "axi");
> if (IS_ERR(dev->clk)) {
> ret = PTR_ERR(dev->clk);
> @@ -1811,8 +1830,12 @@ static int pxp_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct pxp_pdata pxp_imx6ull_pdata = {
> + .data_path_ctrl0 = pxp_imx6ull_data_path_ctrl0,
> +};
> +
> static const struct of_device_id pxp_dt_ids[] = {
> - { .compatible = "fsl,imx6ull-pxp", .data = NULL },
> + { .compatible = "fsl,imx6ull-pxp", .data = &pxp_imx6ull_pdata },
> { },
> };
> MODULE_DEVICE_TABLE(of, pxp_dt_ids);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 6/8] media: imx-pxp: make data_path_ctrl0 platform dependent
@ 2023-01-06 12:30 ` Laurent Pinchart
0 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 12:30 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:27PM +0100, Michael Tretter wrote:
> Unfortunately, the PXP_HW_VERSION register reports the PXP on the i.MX7D
> and on the i.MX6ULL as version 3.0, although the PXP versions on these
> SoCs have significant differences.
>
> Use the compatible to configure the ctrl0 register as required dependent
> on the platform.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> drivers/media/platform/nxp/imx-pxp.c | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> index 1d649b9cadad..4e182f80a36b 100644
> --- a/drivers/media/platform/nxp/imx-pxp.c
> +++ b/drivers/media/platform/nxp/imx-pxp.c
> @@ -19,6 +19,7 @@
> #include <linux/iopoll.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_device.h>
> #include <linux/sched.h>
> #include <linux/slab.h>
>
> @@ -191,6 +192,11 @@ static struct pxp_fmt *find_format(struct v4l2_format *f)
> return &formats[k];
> }
>
> +struct pxp_ctx;
Please add a blank line here.
> +struct pxp_pdata {
> + u32 (*data_path_ctrl0)(struct pxp_ctx *ctx);
> +};
> +
> struct pxp_dev {
> struct v4l2_device v4l2_dev;
> struct video_device vfd;
> @@ -199,6 +205,7 @@ struct pxp_dev {
> void __iomem *mmio;
>
> u32 hw_version;
> + const struct pxp_pdata *pdata;
>
> atomic_t num_inst;
> struct mutex dev_mutex;
> @@ -726,7 +733,7 @@ static void pxp_setup_csc(struct pxp_ctx *ctx)
> }
> }
>
> -static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> +static u32 pxp_imx6ull_data_path_ctrl0(struct pxp_ctx *ctx)
> {
> u32 ctrl0;
>
> @@ -756,6 +763,16 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> return ctrl0;
> }
>
> +static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> +{
> + struct pxp_dev *dev = ctx->dev;
> +
> + if (dev->pdata && dev->pdata->data_path_ctrl0)
> + return dev->pdata->data_path_ctrl0(ctx);
> +
> + return pxp_imx6ull_data_path_ctrl0(ctx);
Do you need this fallback, given that all compatible strings give you
valid pdata ? I'd rather be explicit.
This function then becomes so small that I would inline it in the
caller.
> +}
> +
> static void pxp_set_data_path(struct pxp_ctx *ctx)
> {
> struct pxp_dev *dev = ctx->dev;
> @@ -1711,6 +1728,8 @@ static int pxp_probe(struct platform_device *pdev)
> if (!dev)
> return -ENOMEM;
>
> + dev->pdata = of_device_get_match_data(&pdev->dev);
> +
> dev->clk = devm_clk_get(&pdev->dev, "axi");
> if (IS_ERR(dev->clk)) {
> ret = PTR_ERR(dev->clk);
> @@ -1811,8 +1830,12 @@ static int pxp_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct pxp_pdata pxp_imx6ull_pdata = {
> + .data_path_ctrl0 = pxp_imx6ull_data_path_ctrl0,
> +};
> +
> static const struct of_device_id pxp_dt_ids[] = {
> - { .compatible = "fsl,imx6ull-pxp", .data = NULL },
> + { .compatible = "fsl,imx6ull-pxp", .data = &pxp_imx6ull_pdata },
> { },
> };
> MODULE_DEVICE_TABLE(of, pxp_dt_ids);
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 7/8] media: imx-pxp: add support for i.MX7D
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-06 12:32 ` Laurent Pinchart
-1 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 12:32 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:28PM +0100, Michael Tretter wrote:
> The i.MX7D needs a different data path configuration than the i.MX6ULL.
> Configure the data path as close as possible to the data path on the
> i.MX6ULL.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/platform/nxp/imx-pxp.c | 36 ++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> index 4e182f80a36b..04cc8df2a498 100644
> --- a/drivers/media/platform/nxp/imx-pxp.c
> +++ b/drivers/media/platform/nxp/imx-pxp.c
> @@ -763,6 +763,37 @@ static u32 pxp_imx6ull_data_path_ctrl0(struct pxp_ctx *ctx)
> return ctrl0;
> }
>
> +static u32 pxp_imx7d_data_path_ctrl0(struct pxp_ctx *ctx)
> +{
> + u32 ctrl0;
> +
> + ctrl0 = 0;
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(3);
> + /* Select Rotation 0 */
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
> + /* Select MUX11 for Rotation 0 */
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(1);
> + /* Bypass LUT */
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(1);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(3);
> + /* Select CSC 2 */
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
> + /* Select Composite Alpha Blending/Color Key 0 for CSC 2 */
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(1);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(3);
> + /* Bypass Rotation 1 */
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(3);
> +
> + return ctrl0;
> +}
> +
> static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> {
> struct pxp_dev *dev = ctx->dev;
> @@ -1834,8 +1865,13 @@ static const struct pxp_pdata pxp_imx6ull_pdata = {
> .data_path_ctrl0 = pxp_imx6ull_data_path_ctrl0,
> };
>
> +static const struct pxp_pdata pxp_imx7d_pdata = {
> + .data_path_ctrl0 = pxp_imx7d_data_path_ctrl0,
> +};
> +
> static const struct of_device_id pxp_dt_ids[] = {
> { .compatible = "fsl,imx6ull-pxp", .data = &pxp_imx6ull_pdata },
> + { .compatible = "fsl,imx7d-pxp", .data = &pxp_imx7d_pdata },
> { },
> };
> MODULE_DEVICE_TABLE(of, pxp_dt_ids);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 7/8] media: imx-pxp: add support for i.MX7D
@ 2023-01-06 12:32 ` Laurent Pinchart
0 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 12:32 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:28PM +0100, Michael Tretter wrote:
> The i.MX7D needs a different data path configuration than the i.MX6ULL.
> Configure the data path as close as possible to the data path on the
> i.MX6ULL.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/platform/nxp/imx-pxp.c | 36 ++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> index 4e182f80a36b..04cc8df2a498 100644
> --- a/drivers/media/platform/nxp/imx-pxp.c
> +++ b/drivers/media/platform/nxp/imx-pxp.c
> @@ -763,6 +763,37 @@ static u32 pxp_imx6ull_data_path_ctrl0(struct pxp_ctx *ctx)
> return ctrl0;
> }
>
> +static u32 pxp_imx7d_data_path_ctrl0(struct pxp_ctx *ctx)
> +{
> + u32 ctrl0;
> +
> + ctrl0 = 0;
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(3);
> + /* Select Rotation 0 */
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
> + /* Select MUX11 for Rotation 0 */
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(1);
> + /* Bypass LUT */
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(1);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(3);
> + /* Select CSC 2 */
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
> + /* Select Composite Alpha Blending/Color Key 0 for CSC 2 */
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(1);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(3);
> + /* Bypass Rotation 1 */
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(3);
> + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(3);
> +
> + return ctrl0;
> +}
> +
> static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> {
> struct pxp_dev *dev = ctx->dev;
> @@ -1834,8 +1865,13 @@ static const struct pxp_pdata pxp_imx6ull_pdata = {
> .data_path_ctrl0 = pxp_imx6ull_data_path_ctrl0,
> };
>
> +static const struct pxp_pdata pxp_imx7d_pdata = {
> + .data_path_ctrl0 = pxp_imx7d_data_path_ctrl0,
> +};
> +
> static const struct of_device_id pxp_dt_ids[] = {
> { .compatible = "fsl,imx6ull-pxp", .data = &pxp_imx6ull_pdata },
> + { .compatible = "fsl,imx7d-pxp", .data = &pxp_imx7d_pdata },
> { },
> };
> MODULE_DEVICE_TABLE(of, pxp_dt_ids);
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 1/8] media: dt-bindings: media: fsl-pxp: convert to yaml
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-06 12:34 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 62+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-06 12:34 UTC (permalink / raw)
To: Michael Tretter, linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel
On 05/01/2023 14:47, Michael Tretter wrote:
> Convert the bindings of the Freescale Pixel Pipeline to YAML.
>
> The conversion drops the previously listed compatibles for several SoCs.
> It is unclear, if the PXP on these SoCs is compatible to any of the PXPs
> on the existing SoCs and would allow to reuse the already defined
> compatibles. The missing compatibles should be brought back when the
> support for the PXP on these SoCs is added.
>
Subject: only one "media" prefix.
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> .../bindings/media/fsl,imx6ull-pxp.yaml | 62 +++++++++++++++++++
> .../devicetree/bindings/media/fsl-pxp.txt | 26 --------
> 2 files changed, 62 insertions(+), 26 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
>
> diff --git a/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> new file mode 100644
> index 000000000000..e5f227b84759
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/media/fsl,imx6ull-pxp.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
Drop qyotes from both.
> +
> +title: Freescale Pixel Pipeline
> +
> +maintainers:
> + - Philipp Zabel <p.zabel@pengutronix.de>
> + - Michael Tretter <m.tretter@pengutronix.de>
> +
> +description:
> + The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine
> + that supports scaling, colorspace conversion, alpha blending, rotation, and
> + pixel conversion via lookup table. Different versions are present on various
> + i.MX SoCs from i.MX23 to i.MX7.
> +
> +properties:
> + compatible:
> + enum:
> + - fsl,imx6ul-pxp
> + - fsl,imx6ull-pxp
> + - fsl,imx7d-pxp
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 1
> + maxItems: 2
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + items:
> + - const: axi
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
Add allOf:if:then restricting interrupts per variant.
> +
> +additionalProperties: False
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/imx6ul-clock.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 1/8] media: dt-bindings: media: fsl-pxp: convert to yaml
@ 2023-01-06 12:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 62+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-06 12:34 UTC (permalink / raw)
To: Michael Tretter, linux-media, devicetree, Philipp Zabel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Fabio Estevam, Laurent Pinchart, kernel, linux-imx,
linux-arm-kernel
On 05/01/2023 14:47, Michael Tretter wrote:
> Convert the bindings of the Freescale Pixel Pipeline to YAML.
>
> The conversion drops the previously listed compatibles for several SoCs.
> It is unclear, if the PXP on these SoCs is compatible to any of the PXPs
> on the existing SoCs and would allow to reuse the already defined
> compatibles. The missing compatibles should be brought back when the
> support for the PXP on these SoCs is added.
>
Subject: only one "media" prefix.
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> .../bindings/media/fsl,imx6ull-pxp.yaml | 62 +++++++++++++++++++
> .../devicetree/bindings/media/fsl-pxp.txt | 26 --------
> 2 files changed, 62 insertions(+), 26 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
>
> diff --git a/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> new file mode 100644
> index 000000000000..e5f227b84759
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/media/fsl,imx6ull-pxp.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
Drop qyotes from both.
> +
> +title: Freescale Pixel Pipeline
> +
> +maintainers:
> + - Philipp Zabel <p.zabel@pengutronix.de>
> + - Michael Tretter <m.tretter@pengutronix.de>
> +
> +description:
> + The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine
> + that supports scaling, colorspace conversion, alpha blending, rotation, and
> + pixel conversion via lookup table. Different versions are present on various
> + i.MX SoCs from i.MX23 to i.MX7.
> +
> +properties:
> + compatible:
> + enum:
> + - fsl,imx6ul-pxp
> + - fsl,imx6ull-pxp
> + - fsl,imx7d-pxp
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 1
> + maxItems: 2
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + items:
> + - const: axi
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
Add allOf:if:then restricting interrupts per variant.
> +
> +additionalProperties: False
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/imx6ul-clock.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 8/8] ARM: dts: imx7d: add node for PXP
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-06 12:36 ` Laurent Pinchart
-1 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 12:36 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:29PM +0100, Michael Tretter wrote:
> The i.MX7d contains a Pixel Pipeline in version 3.0. Add the device tree
> node to make it available.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> arch/arm/boot/dts/imx7d.dtsi | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
> index 7ceb7c09f7ad..728cc9413a7c 100644
> --- a/arch/arm/boot/dts/imx7d.dtsi
> +++ b/arch/arm/boot/dts/imx7d.dtsi
> @@ -165,6 +165,15 @@ pcie_phy: pcie-phy@306d0000 {
> reg = <0x306d0000 0x10000>;
> status = "disabled";
> };
> +
> + pxp: pxp@30700000 {
> + compatible = "fsl,imx7d-pxp";
Hmmm... The i.MX7S also has a PXP that seems compatible. I thus wonder
if we shouldn't move this node to imx7s.dtsi.
> + reg = <0x30700000 0x10000>;
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
Nitpicking, alignment ?
> + clocks = <&clks IMX7D_PXP_CLK>;
> + clock-names = "axi";
> + };
> };
>
> &aips3 {
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 8/8] ARM: dts: imx7d: add node for PXP
@ 2023-01-06 12:36 ` Laurent Pinchart
0 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 12:36 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
Thank you for the patch.
On Thu, Jan 05, 2023 at 02:47:29PM +0100, Michael Tretter wrote:
> The i.MX7d contains a Pixel Pipeline in version 3.0. Add the device tree
> node to make it available.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> arch/arm/boot/dts/imx7d.dtsi | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
> index 7ceb7c09f7ad..728cc9413a7c 100644
> --- a/arch/arm/boot/dts/imx7d.dtsi
> +++ b/arch/arm/boot/dts/imx7d.dtsi
> @@ -165,6 +165,15 @@ pcie_phy: pcie-phy@306d0000 {
> reg = <0x306d0000 0x10000>;
> status = "disabled";
> };
> +
> + pxp: pxp@30700000 {
> + compatible = "fsl,imx7d-pxp";
Hmmm... The i.MX7S also has a PXP that seems compatible. I thus wonder
if we shouldn't move this node to imx7s.dtsi.
> + reg = <0x30700000 0x10000>;
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
Nitpicking, alignment ?
> + clocks = <&clks IMX7D_PXP_CLK>;
> + clock-names = "axi";
> + };
> };
>
> &aips3 {
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 0/8] media: imx-pxp: add support for i.MX7D
2023-01-05 13:47 ` Michael Tretter
@ 2023-01-06 12:41 ` Laurent Pinchart
-1 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 12:41 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
On Thu, Jan 05, 2023 at 02:47:21PM +0100, Michael Tretter wrote:
> This series adds support for the PXP found on the i.MX7D to the imx-pxp
> driver.
>
> The PXP on the i.MX7D has a few differences compared to the one on the
> i.MX6ULL. Especially, it has more processing blocks and slightly different
> multiplexers to route the data between the blocks. Therefore, the driver must
> configure a different data path depending on the platform.
>
> While the PXP has a version register, the reported version is the same on the
> i.MX6ULL and the i.MX7D. Therefore, we cannot use the version register to
> change the driver behavior, but have to use the device tree compatible. The
> driver still prints the found version to the log to help bringing up the PXP
> on further platforms.
>
> The patches are inspired by some earlier patches [0] by Laurent to add PXP
> support to the i.MX7d. Compared to the earlier patches, these patches add
> different behavior depending on the platform. Furthermore, the patches disable
> only the LUT block, but keep the rotator block enabled, as it may now be
> configured via the V4L2 rotate control.
Sounds good to me.
> Patch 1 converts the dt-binding to yaml.
>
> Patches 2 to 5 cleanup and refactor the driver in preparation of handling
> different PXP versions.
>
> Patches 6 and 7 add the handling of different platforms and the i.MX7d
> specific configuration.
>
> Patch 8 adds the device tree node for the PXP to the i.MX7d device tree.
I've reviewed the whole series and comments are mostly minor. As you're
reminding me of the PXP, I'll take this as an opportunity to post
patches that I've had in my tree for way too long :-) There will be
minor conflicts with yours, so I'll first rebase them on this series,
assuming that v2 will be similar in places where conflicts occur.
> Michael
>
> [0] https://lore.kernel.org/linux-media/20200510223100.11641-1-laurent.pinchart@ideasonboard.com/
>
> Michael Tretter (8):
> media: dt-bindings: media: fsl-pxp: convert to yaml
> media: imx-pxp: detect PXP version
> media: imx-pxp: extract helper function to setup data path
> media: imx-pxp: explicitly disable unused blocks
> media: imx-pxp: disable LUT block
> media: imx-pxp: make data_path_ctrl0 platform dependent
> media: imx-pxp: add support for i.MX7D
> ARM: dts: imx7d: add node for PXP
>
> .../bindings/media/fsl,imx6ull-pxp.yaml | 62 ++++++++
> .../devicetree/bindings/media/fsl-pxp.txt | 26 ---
> arch/arm/boot/dts/imx7d.dtsi | 9 ++
> drivers/media/platform/nxp/imx-pxp.c | 148 +++++++++++++++---
> 4 files changed, 197 insertions(+), 48 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 0/8] media: imx-pxp: add support for i.MX7D
@ 2023-01-06 12:41 ` Laurent Pinchart
0 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 12:41 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
On Thu, Jan 05, 2023 at 02:47:21PM +0100, Michael Tretter wrote:
> This series adds support for the PXP found on the i.MX7D to the imx-pxp
> driver.
>
> The PXP on the i.MX7D has a few differences compared to the one on the
> i.MX6ULL. Especially, it has more processing blocks and slightly different
> multiplexers to route the data between the blocks. Therefore, the driver must
> configure a different data path depending on the platform.
>
> While the PXP has a version register, the reported version is the same on the
> i.MX6ULL and the i.MX7D. Therefore, we cannot use the version register to
> change the driver behavior, but have to use the device tree compatible. The
> driver still prints the found version to the log to help bringing up the PXP
> on further platforms.
>
> The patches are inspired by some earlier patches [0] by Laurent to add PXP
> support to the i.MX7d. Compared to the earlier patches, these patches add
> different behavior depending on the platform. Furthermore, the patches disable
> only the LUT block, but keep the rotator block enabled, as it may now be
> configured via the V4L2 rotate control.
Sounds good to me.
> Patch 1 converts the dt-binding to yaml.
>
> Patches 2 to 5 cleanup and refactor the driver in preparation of handling
> different PXP versions.
>
> Patches 6 and 7 add the handling of different platforms and the i.MX7d
> specific configuration.
>
> Patch 8 adds the device tree node for the PXP to the i.MX7d device tree.
I've reviewed the whole series and comments are mostly minor. As you're
reminding me of the PXP, I'll take this as an opportunity to post
patches that I've had in my tree for way too long :-) There will be
minor conflicts with yours, so I'll first rebase them on this series,
assuming that v2 will be similar in places where conflicts occur.
> Michael
>
> [0] https://lore.kernel.org/linux-media/20200510223100.11641-1-laurent.pinchart@ideasonboard.com/
>
> Michael Tretter (8):
> media: dt-bindings: media: fsl-pxp: convert to yaml
> media: imx-pxp: detect PXP version
> media: imx-pxp: extract helper function to setup data path
> media: imx-pxp: explicitly disable unused blocks
> media: imx-pxp: disable LUT block
> media: imx-pxp: make data_path_ctrl0 platform dependent
> media: imx-pxp: add support for i.MX7D
> ARM: dts: imx7d: add node for PXP
>
> .../bindings/media/fsl,imx6ull-pxp.yaml | 62 ++++++++
> .../devicetree/bindings/media/fsl-pxp.txt | 26 ---
> arch/arm/boot/dts/imx7d.dtsi | 9 ++
> drivers/media/platform/nxp/imx-pxp.c | 148 +++++++++++++++---
> 4 files changed, 197 insertions(+), 48 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 2/8] media: imx-pxp: detect PXP version
2023-01-06 12:28 ` Laurent Pinchart
@ 2023-01-06 14:01 ` Michael Tretter
-1 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-06 14:01 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, 06 Jan 2023 14:28:47 +0200, Laurent Pinchart wrote:
> On Fri, Jan 06, 2023 at 01:47:32PM +0200, Laurent Pinchart wrote:
> > Thank you for the patch.
> >
> > On Thu, Jan 05, 2023 at 02:47:23PM +0100, Michael Tretter wrote:
> > > Different versions of the Pixel Pipeline have different blocks and their
> > > routing may be different. Read the PXP_HW_VERSION register to determine
> > > the version of the PXP and print it to the log for debugging purposes.
> >
> > Is there a specific reason you chose to read the version register
> > instead of basing the decision on the compatible string ?
>
> Reading the rest of the series, you use the compatible strings later,
> and never use the hw_version field. I'm tempted to propose dropping this
> patch.
My first try was to use the version register, but it turned out that the
version is the same at least on i.MX6ULL and i.MX7D. I kept it to avoid that
others fall into the same trap.
>
> > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > ---
> > > drivers/media/platform/nxp/imx-pxp.c | 18 ++++++++++++++++++
> > > 1 file changed, 18 insertions(+)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> > > index 689ae5e6ac62..05abe40558b0 100644
> > > --- a/drivers/media/platform/nxp/imx-pxp.c
> > > +++ b/drivers/media/platform/nxp/imx-pxp.c
> > > @@ -10,6 +10,7 @@
> > > * Pawel Osciak, <pawel@osciak.com>
> > > * Marek Szyprowski, <m.szyprowski@samsung.com>
> > > */
> > > +#include <linux/bitfield.h>
> > > #include <linux/clk.h>
> > > #include <linux/delay.h>
> > > #include <linux/dma-mapping.h>
> > > @@ -52,6 +53,11 @@ MODULE_PARM_DESC(debug, "activates debug info");
> > > #define MEM2MEM_HFLIP (1 << 0)
> > > #define MEM2MEM_VFLIP (1 << 1)
> > >
> > > +#define PXP_VERSION_MAJOR(version) \
> > > + FIELD_GET(BM_PXP_VERSION_MAJOR, version)
> > > +#define PXP_VERSION_MINOR(version) \
> > > + FIELD_GET(BM_PXP_VERSION_MINOR, version)
> > > +
> > > #define dprintk(dev, fmt, arg...) \
> > > v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: " fmt, __func__, ## arg)
> > >
> > > @@ -192,6 +198,8 @@ struct pxp_dev {
> > > struct clk *clk;
> > > void __iomem *mmio;
> > >
> > > + u32 hw_version;
> > > +
> > > atomic_t num_inst;
> > > struct mutex dev_mutex;
> > > spinlock_t irqlock;
> > > @@ -1660,6 +1668,11 @@ static int pxp_soft_reset(struct pxp_dev *dev)
> > > return 0;
> > > }
> > >
> > > +static u32 pxp_read_version(struct pxp_dev *dev)
> > > +{
> > > + return readl(dev->mmio + HW_PXP_VERSION);
> > > +}
> > > +
> > > static int pxp_probe(struct platform_device *pdev)
> > > {
> > > struct pxp_dev *dev;
> > > @@ -1705,6 +1718,11 @@ static int pxp_probe(struct platform_device *pdev)
> > > goto err_clk;
> > > }
> > >
> > > + dev->hw_version = pxp_read_version(dev);
> > > + dev_info(&pdev->dev, "PXP Version %d.%d\n",
> >
> > As the version can't be negative, I'd use %u.%u.
> >
> > > + PXP_VERSION_MAJOR(dev->hw_version),
> > > + PXP_VERSION_MINOR(dev->hw_version));
> > > +
> >
> > The driver now prints two messages at probe time, it would be nice to
> > combine them, or remove the other one. That's a candidate for a future
> > patch though.
I would reduce the level to dev_debug. Then the version is not always printed,
but it can be easily enabled if necessary for the bringup on another platform.
Michael
> >
> > > ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> > > if (ret)
> > > goto err_clk;
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 2/8] media: imx-pxp: detect PXP version
@ 2023-01-06 14:01 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-06 14:01 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, 06 Jan 2023 14:28:47 +0200, Laurent Pinchart wrote:
> On Fri, Jan 06, 2023 at 01:47:32PM +0200, Laurent Pinchart wrote:
> > Thank you for the patch.
> >
> > On Thu, Jan 05, 2023 at 02:47:23PM +0100, Michael Tretter wrote:
> > > Different versions of the Pixel Pipeline have different blocks and their
> > > routing may be different. Read the PXP_HW_VERSION register to determine
> > > the version of the PXP and print it to the log for debugging purposes.
> >
> > Is there a specific reason you chose to read the version register
> > instead of basing the decision on the compatible string ?
>
> Reading the rest of the series, you use the compatible strings later,
> and never use the hw_version field. I'm tempted to propose dropping this
> patch.
My first try was to use the version register, but it turned out that the
version is the same at least on i.MX6ULL and i.MX7D. I kept it to avoid that
others fall into the same trap.
>
> > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > ---
> > > drivers/media/platform/nxp/imx-pxp.c | 18 ++++++++++++++++++
> > > 1 file changed, 18 insertions(+)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> > > index 689ae5e6ac62..05abe40558b0 100644
> > > --- a/drivers/media/platform/nxp/imx-pxp.c
> > > +++ b/drivers/media/platform/nxp/imx-pxp.c
> > > @@ -10,6 +10,7 @@
> > > * Pawel Osciak, <pawel@osciak.com>
> > > * Marek Szyprowski, <m.szyprowski@samsung.com>
> > > */
> > > +#include <linux/bitfield.h>
> > > #include <linux/clk.h>
> > > #include <linux/delay.h>
> > > #include <linux/dma-mapping.h>
> > > @@ -52,6 +53,11 @@ MODULE_PARM_DESC(debug, "activates debug info");
> > > #define MEM2MEM_HFLIP (1 << 0)
> > > #define MEM2MEM_VFLIP (1 << 1)
> > >
> > > +#define PXP_VERSION_MAJOR(version) \
> > > + FIELD_GET(BM_PXP_VERSION_MAJOR, version)
> > > +#define PXP_VERSION_MINOR(version) \
> > > + FIELD_GET(BM_PXP_VERSION_MINOR, version)
> > > +
> > > #define dprintk(dev, fmt, arg...) \
> > > v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: " fmt, __func__, ## arg)
> > >
> > > @@ -192,6 +198,8 @@ struct pxp_dev {
> > > struct clk *clk;
> > > void __iomem *mmio;
> > >
> > > + u32 hw_version;
> > > +
> > > atomic_t num_inst;
> > > struct mutex dev_mutex;
> > > spinlock_t irqlock;
> > > @@ -1660,6 +1668,11 @@ static int pxp_soft_reset(struct pxp_dev *dev)
> > > return 0;
> > > }
> > >
> > > +static u32 pxp_read_version(struct pxp_dev *dev)
> > > +{
> > > + return readl(dev->mmio + HW_PXP_VERSION);
> > > +}
> > > +
> > > static int pxp_probe(struct platform_device *pdev)
> > > {
> > > struct pxp_dev *dev;
> > > @@ -1705,6 +1718,11 @@ static int pxp_probe(struct platform_device *pdev)
> > > goto err_clk;
> > > }
> > >
> > > + dev->hw_version = pxp_read_version(dev);
> > > + dev_info(&pdev->dev, "PXP Version %d.%d\n",
> >
> > As the version can't be negative, I'd use %u.%u.
> >
> > > + PXP_VERSION_MAJOR(dev->hw_version),
> > > + PXP_VERSION_MINOR(dev->hw_version));
> > > +
> >
> > The driver now prints two messages at probe time, it would be nice to
> > combine them, or remove the other one. That's a candidate for a future
> > patch though.
I would reduce the level to dev_debug. Then the version is not always printed,
but it can be easily enabled if necessary for the bringup on another platform.
Michael
> >
> > > ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> > > if (ret)
> > > goto err_clk;
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 4/8] media: imx-pxp: explicitly disable unused blocks
2023-01-06 12:26 ` Laurent Pinchart
@ 2023-01-06 14:08 ` Michael Tretter
-1 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-06 14:08 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, 06 Jan 2023 14:26:40 +0200, Laurent Pinchart wrote:
> On Thu, Jan 05, 2023 at 02:47:25PM +0100, Michael Tretter wrote:
> > Various multiplexers in the pipeline are not used with the currently
> > configured data path. Disable all unused multiplexers by selecting the
> > "no output" (3) option.
> >
> > The datasheet doesn't explicitly require this, but the PXP has been seen
> > to hang after processing a few hundreds of frames otherwise.
>
> On which platform(s) have you noticed that ?
I didn't observe this myself, but took this information from the comment in
your earlier patch [0] that disables the unused multiplexers.
https://lore.kernel.org/linux-media/20200510223100.11641-2-laurent.pinchart@ideasonboard.com/
>
> > As at it, add documentation for the multiplexers that are actually
> > relevant for the data path.
> >
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > drivers/media/platform/nxp/imx-pxp.c | 30 +++++++++++++++++-----------
> > 1 file changed, 18 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> > index a957fee88829..6ffd07cda965 100644
> > --- a/drivers/media/platform/nxp/imx-pxp.c
> > +++ b/drivers/media/platform/nxp/imx-pxp.c
> > @@ -731,22 +731,28 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > u32 ctrl0;
> >
> > ctrl0 = 0;
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(3);
> > + /* Bypass Dithering x3CH */
> > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
> > + /* Select Rotation */
> > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
> > + /* Select LUT */
> > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
> > + /* Select MUX8 for LUT */
> > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
> > + /* Select CSC 2 */
> > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(3);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(3);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(3);
> > + /* Bypass Rotation 2 */
> > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(3);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(3);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(3);
>
> The muxes being disabled look fine to me, but the values of MUX8, MUX12
> and MUX14 look strange based on the i.MX7D reference manual. Maybe the
> register values were different in previous SoCs ? I haven't found any
> other relevant reference manual that document the mux values, I may have
> overlooked something.
The MUX8, MUX12 and MUX14 are documented in the i.MX6ULL reference manual
section 41.11.51 and their location and function in the data path is shown in
Figure 41-1. "PXP Architecture" on page 2490.
Michael
>
> Anyway, this isn't an issue with this patch, so
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> >
> > return ctrl0;
> > }
> > @@ -760,8 +766,8 @@ static void pxp_set_data_path(struct pxp_ctx *ctx)
> > ctrl0 = pxp_data_path_ctrl0(ctx);
> >
> > ctrl1 = 0;
> > - ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1);
> > - ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1);
> > + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(3);
> > + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(3);
> >
> > writel(ctrl0, dev->mmio + HW_PXP_DATA_PATH_CTRL0);
> > writel(ctrl1, dev->mmio + HW_PXP_DATA_PATH_CTRL1);
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 4/8] media: imx-pxp: explicitly disable unused blocks
@ 2023-01-06 14:08 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-06 14:08 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, 06 Jan 2023 14:26:40 +0200, Laurent Pinchart wrote:
> On Thu, Jan 05, 2023 at 02:47:25PM +0100, Michael Tretter wrote:
> > Various multiplexers in the pipeline are not used with the currently
> > configured data path. Disable all unused multiplexers by selecting the
> > "no output" (3) option.
> >
> > The datasheet doesn't explicitly require this, but the PXP has been seen
> > to hang after processing a few hundreds of frames otherwise.
>
> On which platform(s) have you noticed that ?
I didn't observe this myself, but took this information from the comment in
your earlier patch [0] that disables the unused multiplexers.
https://lore.kernel.org/linux-media/20200510223100.11641-2-laurent.pinchart@ideasonboard.com/
>
> > As at it, add documentation for the multiplexers that are actually
> > relevant for the data path.
> >
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > drivers/media/platform/nxp/imx-pxp.c | 30 +++++++++++++++++-----------
> > 1 file changed, 18 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> > index a957fee88829..6ffd07cda965 100644
> > --- a/drivers/media/platform/nxp/imx-pxp.c
> > +++ b/drivers/media/platform/nxp/imx-pxp.c
> > @@ -731,22 +731,28 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > u32 ctrl0;
> >
> > ctrl0 = 0;
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(3);
> > + /* Bypass Dithering x3CH */
> > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
> > + /* Select Rotation */
> > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
> > + /* Select LUT */
> > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
> > + /* Select MUX8 for LUT */
> > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
> > + /* Select CSC 2 */
> > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(3);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(3);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(3);
> > + /* Bypass Rotation 2 */
> > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0);
> > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(3);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(3);
> > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(3);
>
> The muxes being disabled look fine to me, but the values of MUX8, MUX12
> and MUX14 look strange based on the i.MX7D reference manual. Maybe the
> register values were different in previous SoCs ? I haven't found any
> other relevant reference manual that document the mux values, I may have
> overlooked something.
The MUX8, MUX12 and MUX14 are documented in the i.MX6ULL reference manual
section 41.11.51 and their location and function in the data path is shown in
Figure 41-1. "PXP Architecture" on page 2490.
Michael
>
> Anyway, this isn't an issue with this patch, so
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> >
> > return ctrl0;
> > }
> > @@ -760,8 +766,8 @@ static void pxp_set_data_path(struct pxp_ctx *ctx)
> > ctrl0 = pxp_data_path_ctrl0(ctx);
> >
> > ctrl1 = 0;
> > - ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1);
> > - ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1);
> > + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(3);
> > + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(3);
> >
> > writel(ctrl0, dev->mmio + HW_PXP_DATA_PATH_CTRL0);
> > writel(ctrl1, dev->mmio + HW_PXP_DATA_PATH_CTRL1);
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 6/8] media: imx-pxp: make data_path_ctrl0 platform dependent
2023-01-06 12:30 ` Laurent Pinchart
@ 2023-01-06 14:11 ` Michael Tretter
-1 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-06 14:11 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, 06 Jan 2023 14:30:52 +0200, Laurent Pinchart wrote:
> On Thu, Jan 05, 2023 at 02:47:27PM +0100, Michael Tretter wrote:
> > Unfortunately, the PXP_HW_VERSION register reports the PXP on the i.MX7D
> > and on the i.MX6ULL as version 3.0, although the PXP versions on these
> > SoCs have significant differences.
> >
> > Use the compatible to configure the ctrl0 register as required dependent
> > on the platform.
> >
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > drivers/media/platform/nxp/imx-pxp.c | 27 +++++++++++++++++++++++++--
> > 1 file changed, 25 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> > index 1d649b9cadad..4e182f80a36b 100644
> > --- a/drivers/media/platform/nxp/imx-pxp.c
> > +++ b/drivers/media/platform/nxp/imx-pxp.c
> > @@ -19,6 +19,7 @@
> > #include <linux/iopoll.h>
> > #include <linux/module.h>
> > #include <linux/of.h>
> > +#include <linux/of_device.h>
> > #include <linux/sched.h>
> > #include <linux/slab.h>
> >
> > @@ -191,6 +192,11 @@ static struct pxp_fmt *find_format(struct v4l2_format *f)
> > return &formats[k];
> > }
> >
> > +struct pxp_ctx;
>
> Please add a blank line here.
>
> > +struct pxp_pdata {
> > + u32 (*data_path_ctrl0)(struct pxp_ctx *ctx);
> > +};
> > +
> > struct pxp_dev {
> > struct v4l2_device v4l2_dev;
> > struct video_device vfd;
> > @@ -199,6 +205,7 @@ struct pxp_dev {
> > void __iomem *mmio;
> >
> > u32 hw_version;
> > + const struct pxp_pdata *pdata;
> >
> > atomic_t num_inst;
> > struct mutex dev_mutex;
> > @@ -726,7 +733,7 @@ static void pxp_setup_csc(struct pxp_ctx *ctx)
> > }
> > }
> >
> > -static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > +static u32 pxp_imx6ull_data_path_ctrl0(struct pxp_ctx *ctx)
> > {
> > u32 ctrl0;
> >
> > @@ -756,6 +763,16 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > return ctrl0;
> > }
> >
> > +static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > +{
> > + struct pxp_dev *dev = ctx->dev;
> > +
> > + if (dev->pdata && dev->pdata->data_path_ctrl0)
> > + return dev->pdata->data_path_ctrl0(ctx);
> > +
> > + return pxp_imx6ull_data_path_ctrl0(ctx);
>
> Do you need this fallback, given that all compatible strings give you
> valid pdata ? I'd rather be explicit.
>
> This function then becomes so small that I would inline it in the
> caller.
I was a bit paranoid that there may be cases in which pdata is not set. I will
change this to assume that pdata is always valid and just be explicit.
Michael
>
> > +}
> > +
> > static void pxp_set_data_path(struct pxp_ctx *ctx)
> > {
> > struct pxp_dev *dev = ctx->dev;
> > @@ -1711,6 +1728,8 @@ static int pxp_probe(struct platform_device *pdev)
> > if (!dev)
> > return -ENOMEM;
> >
> > + dev->pdata = of_device_get_match_data(&pdev->dev);
> > +
> > dev->clk = devm_clk_get(&pdev->dev, "axi");
> > if (IS_ERR(dev->clk)) {
> > ret = PTR_ERR(dev->clk);
> > @@ -1811,8 +1830,12 @@ static int pxp_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > +static const struct pxp_pdata pxp_imx6ull_pdata = {
> > + .data_path_ctrl0 = pxp_imx6ull_data_path_ctrl0,
> > +};
> > +
> > static const struct of_device_id pxp_dt_ids[] = {
> > - { .compatible = "fsl,imx6ull-pxp", .data = NULL },
> > + { .compatible = "fsl,imx6ull-pxp", .data = &pxp_imx6ull_pdata },
> > { },
> > };
> > MODULE_DEVICE_TABLE(of, pxp_dt_ids);
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 6/8] media: imx-pxp: make data_path_ctrl0 platform dependent
@ 2023-01-06 14:11 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-06 14:11 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, 06 Jan 2023 14:30:52 +0200, Laurent Pinchart wrote:
> On Thu, Jan 05, 2023 at 02:47:27PM +0100, Michael Tretter wrote:
> > Unfortunately, the PXP_HW_VERSION register reports the PXP on the i.MX7D
> > and on the i.MX6ULL as version 3.0, although the PXP versions on these
> > SoCs have significant differences.
> >
> > Use the compatible to configure the ctrl0 register as required dependent
> > on the platform.
> >
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > drivers/media/platform/nxp/imx-pxp.c | 27 +++++++++++++++++++++++++--
> > 1 file changed, 25 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> > index 1d649b9cadad..4e182f80a36b 100644
> > --- a/drivers/media/platform/nxp/imx-pxp.c
> > +++ b/drivers/media/platform/nxp/imx-pxp.c
> > @@ -19,6 +19,7 @@
> > #include <linux/iopoll.h>
> > #include <linux/module.h>
> > #include <linux/of.h>
> > +#include <linux/of_device.h>
> > #include <linux/sched.h>
> > #include <linux/slab.h>
> >
> > @@ -191,6 +192,11 @@ static struct pxp_fmt *find_format(struct v4l2_format *f)
> > return &formats[k];
> > }
> >
> > +struct pxp_ctx;
>
> Please add a blank line here.
>
> > +struct pxp_pdata {
> > + u32 (*data_path_ctrl0)(struct pxp_ctx *ctx);
> > +};
> > +
> > struct pxp_dev {
> > struct v4l2_device v4l2_dev;
> > struct video_device vfd;
> > @@ -199,6 +205,7 @@ struct pxp_dev {
> > void __iomem *mmio;
> >
> > u32 hw_version;
> > + const struct pxp_pdata *pdata;
> >
> > atomic_t num_inst;
> > struct mutex dev_mutex;
> > @@ -726,7 +733,7 @@ static void pxp_setup_csc(struct pxp_ctx *ctx)
> > }
> > }
> >
> > -static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > +static u32 pxp_imx6ull_data_path_ctrl0(struct pxp_ctx *ctx)
> > {
> > u32 ctrl0;
> >
> > @@ -756,6 +763,16 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > return ctrl0;
> > }
> >
> > +static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > +{
> > + struct pxp_dev *dev = ctx->dev;
> > +
> > + if (dev->pdata && dev->pdata->data_path_ctrl0)
> > + return dev->pdata->data_path_ctrl0(ctx);
> > +
> > + return pxp_imx6ull_data_path_ctrl0(ctx);
>
> Do you need this fallback, given that all compatible strings give you
> valid pdata ? I'd rather be explicit.
>
> This function then becomes so small that I would inline it in the
> caller.
I was a bit paranoid that there may be cases in which pdata is not set. I will
change this to assume that pdata is always valid and just be explicit.
Michael
>
> > +}
> > +
> > static void pxp_set_data_path(struct pxp_ctx *ctx)
> > {
> > struct pxp_dev *dev = ctx->dev;
> > @@ -1711,6 +1728,8 @@ static int pxp_probe(struct platform_device *pdev)
> > if (!dev)
> > return -ENOMEM;
> >
> > + dev->pdata = of_device_get_match_data(&pdev->dev);
> > +
> > dev->clk = devm_clk_get(&pdev->dev, "axi");
> > if (IS_ERR(dev->clk)) {
> > ret = PTR_ERR(dev->clk);
> > @@ -1811,8 +1830,12 @@ static int pxp_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > +static const struct pxp_pdata pxp_imx6ull_pdata = {
> > + .data_path_ctrl0 = pxp_imx6ull_data_path_ctrl0,
> > +};
> > +
> > static const struct of_device_id pxp_dt_ids[] = {
> > - { .compatible = "fsl,imx6ull-pxp", .data = NULL },
> > + { .compatible = "fsl,imx6ull-pxp", .data = &pxp_imx6ull_pdata },
> > { },
> > };
> > MODULE_DEVICE_TABLE(of, pxp_dt_ids);
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 8/8] ARM: dts: imx7d: add node for PXP
2023-01-06 12:36 ` Laurent Pinchart
@ 2023-01-06 14:36 ` Michael Tretter
-1 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-06 14:36 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, 06 Jan 2023 14:36:41 +0200, Laurent Pinchart wrote:
> Hi Michael,
>
> Thank you for the patch.
>
> On Thu, Jan 05, 2023 at 02:47:29PM +0100, Michael Tretter wrote:
> > The i.MX7d contains a Pixel Pipeline in version 3.0. Add the device tree
> > node to make it available.
> >
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > arch/arm/boot/dts/imx7d.dtsi | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
> > index 7ceb7c09f7ad..728cc9413a7c 100644
> > --- a/arch/arm/boot/dts/imx7d.dtsi
> > +++ b/arch/arm/boot/dts/imx7d.dtsi
> > @@ -165,6 +165,15 @@ pcie_phy: pcie-phy@306d0000 {
> > reg = <0x306d0000 0x10000>;
> > status = "disabled";
> > };
> > +
> > + pxp: pxp@30700000 {
> > + compatible = "fsl,imx7d-pxp";
>
> Hmmm... The i.MX7S also has a PXP that seems compatible. I thus wonder
> if we shouldn't move this node to imx7s.dtsi.
The i.MX7S has a PXP at the same address, but the architecture in the
reference manual (Figure 13-71. PXP Architecture, p. 3797) looks slightly
different wrt. the location of the multiplexers. The reference manual is also
conspicuously lacking documentation of the DATA_PATH_CTRL0 register.
I wouldn't risk adding the node to the imx7s.dtsi and would rather keep the
option to add a different compatible for the i.MX7S to be able to handle the
difference.
Michael
>
> > + reg = <0x30700000 0x10000>;
> > + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
>
> Nitpicking, alignment ?
>
> > + clocks = <&clks IMX7D_PXP_CLK>;
> > + clock-names = "axi";
> > + };
> > };
> >
> > &aips3 {
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 8/8] ARM: dts: imx7d: add node for PXP
@ 2023-01-06 14:36 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-06 14:36 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, 06 Jan 2023 14:36:41 +0200, Laurent Pinchart wrote:
> Hi Michael,
>
> Thank you for the patch.
>
> On Thu, Jan 05, 2023 at 02:47:29PM +0100, Michael Tretter wrote:
> > The i.MX7d contains a Pixel Pipeline in version 3.0. Add the device tree
> > node to make it available.
> >
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > arch/arm/boot/dts/imx7d.dtsi | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
> > index 7ceb7c09f7ad..728cc9413a7c 100644
> > --- a/arch/arm/boot/dts/imx7d.dtsi
> > +++ b/arch/arm/boot/dts/imx7d.dtsi
> > @@ -165,6 +165,15 @@ pcie_phy: pcie-phy@306d0000 {
> > reg = <0x306d0000 0x10000>;
> > status = "disabled";
> > };
> > +
> > + pxp: pxp@30700000 {
> > + compatible = "fsl,imx7d-pxp";
>
> Hmmm... The i.MX7S also has a PXP that seems compatible. I thus wonder
> if we shouldn't move this node to imx7s.dtsi.
The i.MX7S has a PXP at the same address, but the architecture in the
reference manual (Figure 13-71. PXP Architecture, p. 3797) looks slightly
different wrt. the location of the multiplexers. The reference manual is also
conspicuously lacking documentation of the DATA_PATH_CTRL0 register.
I wouldn't risk adding the node to the imx7s.dtsi and would rather keep the
option to add a different compatible for the i.MX7S to be able to handle the
difference.
Michael
>
> > + reg = <0x30700000 0x10000>;
> > + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
>
> Nitpicking, alignment ?
>
> > + clocks = <&clks IMX7D_PXP_CLK>;
> > + clock-names = "axi";
> > + };
> > };
> >
> > &aips3 {
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 0/8] media: imx-pxp: add support for i.MX7D
2023-01-06 12:41 ` Laurent Pinchart
@ 2023-01-06 14:42 ` Michael Tretter
-1 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-06 14:42 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, 06 Jan 2023 14:41:32 +0200, Laurent Pinchart wrote:
> Hi Michael,
>
> On Thu, Jan 05, 2023 at 02:47:21PM +0100, Michael Tretter wrote:
> > This series adds support for the PXP found on the i.MX7D to the imx-pxp
> > driver.
> >
> > The PXP on the i.MX7D has a few differences compared to the one on the
> > i.MX6ULL. Especially, it has more processing blocks and slightly different
> > multiplexers to route the data between the blocks. Therefore, the driver must
> > configure a different data path depending on the platform.
> >
> > While the PXP has a version register, the reported version is the same on the
> > i.MX6ULL and the i.MX7D. Therefore, we cannot use the version register to
> > change the driver behavior, but have to use the device tree compatible. The
> > driver still prints the found version to the log to help bringing up the PXP
> > on further platforms.
> >
> > The patches are inspired by some earlier patches [0] by Laurent to add PXP
> > support to the i.MX7d. Compared to the earlier patches, these patches add
> > different behavior depending on the platform. Furthermore, the patches disable
> > only the LUT block, but keep the rotator block enabled, as it may now be
> > configured via the V4L2 rotate control.
>
> Sounds good to me.
>
> > Patch 1 converts the dt-binding to yaml.
> >
> > Patches 2 to 5 cleanup and refactor the driver in preparation of handling
> > different PXP versions.
> >
> > Patches 6 and 7 add the handling of different platforms and the i.MX7d
> > specific configuration.
> >
> > Patch 8 adds the device tree node for the PXP to the i.MX7d device tree.
>
> I've reviewed the whole series and comments are mostly minor. As you're
> reminding me of the PXP, I'll take this as an opportunity to post
> patches that I've had in my tree for way too long :-) There will be
> minor conflicts with yours, so I'll first rebase them on this series,
> assuming that v2 will be similar in places where conflicts occur.
Thanks for the review! I will send a v2 to address your comments.
Thanks for your other patches as well. I will take a look and probably just
include them in my v2.
Michael
>
> > Michael
> >
> > [0] https://lore.kernel.org/linux-media/20200510223100.11641-1-laurent.pinchart@ideasonboard.com/
> >
> > Michael Tretter (8):
> > media: dt-bindings: media: fsl-pxp: convert to yaml
> > media: imx-pxp: detect PXP version
> > media: imx-pxp: extract helper function to setup data path
> > media: imx-pxp: explicitly disable unused blocks
> > media: imx-pxp: disable LUT block
> > media: imx-pxp: make data_path_ctrl0 platform dependent
> > media: imx-pxp: add support for i.MX7D
> > ARM: dts: imx7d: add node for PXP
> >
> > .../bindings/media/fsl,imx6ull-pxp.yaml | 62 ++++++++
> > .../devicetree/bindings/media/fsl-pxp.txt | 26 ---
> > arch/arm/boot/dts/imx7d.dtsi | 9 ++
> > drivers/media/platform/nxp/imx-pxp.c | 148 +++++++++++++++---
> > 4 files changed, 197 insertions(+), 48 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> > delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 0/8] media: imx-pxp: add support for i.MX7D
@ 2023-01-06 14:42 ` Michael Tretter
0 siblings, 0 replies; 62+ messages in thread
From: Michael Tretter @ 2023-01-06 14:42 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, 06 Jan 2023 14:41:32 +0200, Laurent Pinchart wrote:
> Hi Michael,
>
> On Thu, Jan 05, 2023 at 02:47:21PM +0100, Michael Tretter wrote:
> > This series adds support for the PXP found on the i.MX7D to the imx-pxp
> > driver.
> >
> > The PXP on the i.MX7D has a few differences compared to the one on the
> > i.MX6ULL. Especially, it has more processing blocks and slightly different
> > multiplexers to route the data between the blocks. Therefore, the driver must
> > configure a different data path depending on the platform.
> >
> > While the PXP has a version register, the reported version is the same on the
> > i.MX6ULL and the i.MX7D. Therefore, we cannot use the version register to
> > change the driver behavior, but have to use the device tree compatible. The
> > driver still prints the found version to the log to help bringing up the PXP
> > on further platforms.
> >
> > The patches are inspired by some earlier patches [0] by Laurent to add PXP
> > support to the i.MX7d. Compared to the earlier patches, these patches add
> > different behavior depending on the platform. Furthermore, the patches disable
> > only the LUT block, but keep the rotator block enabled, as it may now be
> > configured via the V4L2 rotate control.
>
> Sounds good to me.
>
> > Patch 1 converts the dt-binding to yaml.
> >
> > Patches 2 to 5 cleanup and refactor the driver in preparation of handling
> > different PXP versions.
> >
> > Patches 6 and 7 add the handling of different platforms and the i.MX7d
> > specific configuration.
> >
> > Patch 8 adds the device tree node for the PXP to the i.MX7d device tree.
>
> I've reviewed the whole series and comments are mostly minor. As you're
> reminding me of the PXP, I'll take this as an opportunity to post
> patches that I've had in my tree for way too long :-) There will be
> minor conflicts with yours, so I'll first rebase them on this series,
> assuming that v2 will be similar in places where conflicts occur.
Thanks for the review! I will send a v2 to address your comments.
Thanks for your other patches as well. I will take a look and probably just
include them in my v2.
Michael
>
> > Michael
> >
> > [0] https://lore.kernel.org/linux-media/20200510223100.11641-1-laurent.pinchart@ideasonboard.com/
> >
> > Michael Tretter (8):
> > media: dt-bindings: media: fsl-pxp: convert to yaml
> > media: imx-pxp: detect PXP version
> > media: imx-pxp: extract helper function to setup data path
> > media: imx-pxp: explicitly disable unused blocks
> > media: imx-pxp: disable LUT block
> > media: imx-pxp: make data_path_ctrl0 platform dependent
> > media: imx-pxp: add support for i.MX7D
> > ARM: dts: imx7d: add node for PXP
> >
> > .../bindings/media/fsl,imx6ull-pxp.yaml | 62 ++++++++
> > .../devicetree/bindings/media/fsl-pxp.txt | 26 ---
> > arch/arm/boot/dts/imx7d.dtsi | 9 ++
> > drivers/media/platform/nxp/imx-pxp.c | 148 +++++++++++++++---
> > 4 files changed, 197 insertions(+), 48 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> > delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 4/8] media: imx-pxp: explicitly disable unused blocks
2023-01-06 14:08 ` Michael Tretter
@ 2023-01-06 18:39 ` Laurent Pinchart
-1 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 18:39 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
On Fri, Jan 06, 2023 at 03:08:55PM +0100, Michael Tretter wrote:
> On Fri, 06 Jan 2023 14:26:40 +0200, Laurent Pinchart wrote:
> > On Thu, Jan 05, 2023 at 02:47:25PM +0100, Michael Tretter wrote:
> > > Various multiplexers in the pipeline are not used with the currently
> > > configured data path. Disable all unused multiplexers by selecting the
> > > "no output" (3) option.
> > >
> > > The datasheet doesn't explicitly require this, but the PXP has been seen
> > > to hang after processing a few hundreds of frames otherwise.
> >
> > On which platform(s) have you noticed that ?
>
> I didn't observe this myself, but took this information from the comment in
> your earlier patch [0] that disables the unused multiplexers.
>
> https://lore.kernel.org/linux-media/20200510223100.11641-2-laurent.pinchart@ideasonboard.com/
I suppose I should trust myself :-)
> > > As at it, add documentation for the multiplexers that are actually
> > > relevant for the data path.
> > >
> > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > ---
> > > drivers/media/platform/nxp/imx-pxp.c | 30 +++++++++++++++++-----------
> > > 1 file changed, 18 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> > > index a957fee88829..6ffd07cda965 100644
> > > --- a/drivers/media/platform/nxp/imx-pxp.c
> > > +++ b/drivers/media/platform/nxp/imx-pxp.c
> > > @@ -731,22 +731,28 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > > u32 ctrl0;
> > >
> > > ctrl0 = 0;
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(3);
> > > + /* Bypass Dithering x3CH */
> > > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
> > > + /* Select Rotation */
> > > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
> > > + /* Select LUT */
> > > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
> > > + /* Select MUX8 for LUT */
> > > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
> > > + /* Select CSC 2 */
> > > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(3);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(3);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(3);
> > > + /* Bypass Rotation 2 */
> > > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(3);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(3);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(3);
> >
> > The muxes being disabled look fine to me, but the values of MUX8, MUX12
> > and MUX14 look strange based on the i.MX7D reference manual. Maybe the
> > register values were different in previous SoCs ? I haven't found any
> > other relevant reference manual that document the mux values, I may have
> > overlooked something.
>
> The MUX8, MUX12 and MUX14 are documented in the i.MX6ULL reference manual
> section 41.11.51 and their location and function in the data path is shown in
> Figure 41-1. "PXP Architecture" on page 2490.
I've seen that, but as far as I can tell, the description of the
register in section 41.11.51 doesn't tell what the different MUX* field
value map to. Figure 41-1 shows that, for instance, MUX8 handles CSC2
bypass, but it doesn't tell what value corresponds to what path.
> > Anyway, this isn't an issue with this patch, so
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > >
> > > return ctrl0;
> > > }
> > > @@ -760,8 +766,8 @@ static void pxp_set_data_path(struct pxp_ctx *ctx)
> > > ctrl0 = pxp_data_path_ctrl0(ctx);
> > >
> > > ctrl1 = 0;
> > > - ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1);
> > > - ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1);
> > > + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(3);
> > > + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(3);
> > >
> > > writel(ctrl0, dev->mmio + HW_PXP_DATA_PATH_CTRL0);
> > > writel(ctrl1, dev->mmio + HW_PXP_DATA_PATH_CTRL1);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 4/8] media: imx-pxp: explicitly disable unused blocks
@ 2023-01-06 18:39 ` Laurent Pinchart
0 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 18:39 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
Hi Michael,
On Fri, Jan 06, 2023 at 03:08:55PM +0100, Michael Tretter wrote:
> On Fri, 06 Jan 2023 14:26:40 +0200, Laurent Pinchart wrote:
> > On Thu, Jan 05, 2023 at 02:47:25PM +0100, Michael Tretter wrote:
> > > Various multiplexers in the pipeline are not used with the currently
> > > configured data path. Disable all unused multiplexers by selecting the
> > > "no output" (3) option.
> > >
> > > The datasheet doesn't explicitly require this, but the PXP has been seen
> > > to hang after processing a few hundreds of frames otherwise.
> >
> > On which platform(s) have you noticed that ?
>
> I didn't observe this myself, but took this information from the comment in
> your earlier patch [0] that disables the unused multiplexers.
>
> https://lore.kernel.org/linux-media/20200510223100.11641-2-laurent.pinchart@ideasonboard.com/
I suppose I should trust myself :-)
> > > As at it, add documentation for the multiplexers that are actually
> > > relevant for the data path.
> > >
> > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > ---
> > > drivers/media/platform/nxp/imx-pxp.c | 30 +++++++++++++++++-----------
> > > 1 file changed, 18 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> > > index a957fee88829..6ffd07cda965 100644
> > > --- a/drivers/media/platform/nxp/imx-pxp.c
> > > +++ b/drivers/media/platform/nxp/imx-pxp.c
> > > @@ -731,22 +731,28 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > > u32 ctrl0;
> > >
> > > ctrl0 = 0;
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(0);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX15_SEL(3);
> > > + /* Bypass Dithering x3CH */
> > > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX14_SEL(1);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(0);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX13_SEL(3);
> > > + /* Select Rotation */
> > > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX12_SEL(0);
> > > + /* Select LUT */
> > > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX11_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(0);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX10_SEL(3);
> > > + /* Select MUX8 for LUT */
> > > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX9_SEL(1);
> > > + /* Select CSC 2 */
> > > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX8_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(0);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX7_SEL(3);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX6_SEL(3);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX5_SEL(3);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX4_SEL(3);
> > > + /* Bypass Rotation 2 */
> > > ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX3_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(0);
> > > - ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(0);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX2_SEL(3);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX1_SEL(3);
> > > + ctrl0 |= BF_PXP_DATA_PATH_CTRL0_MUX0_SEL(3);
> >
> > The muxes being disabled look fine to me, but the values of MUX8, MUX12
> > and MUX14 look strange based on the i.MX7D reference manual. Maybe the
> > register values were different in previous SoCs ? I haven't found any
> > other relevant reference manual that document the mux values, I may have
> > overlooked something.
>
> The MUX8, MUX12 and MUX14 are documented in the i.MX6ULL reference manual
> section 41.11.51 and their location and function in the data path is shown in
> Figure 41-1. "PXP Architecture" on page 2490.
I've seen that, but as far as I can tell, the description of the
register in section 41.11.51 doesn't tell what the different MUX* field
value map to. Figure 41-1 shows that, for instance, MUX8 handles CSC2
bypass, but it doesn't tell what value corresponds to what path.
> > Anyway, this isn't an issue with this patch, so
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > >
> > > return ctrl0;
> > > }
> > > @@ -760,8 +766,8 @@ static void pxp_set_data_path(struct pxp_ctx *ctx)
> > > ctrl0 = pxp_data_path_ctrl0(ctx);
> > >
> > > ctrl1 = 0;
> > > - ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(1);
> > > - ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(1);
> > > + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX17_SEL(3);
> > > + ctrl1 |= BF_PXP_DATA_PATH_CTRL1_MUX16_SEL(3);
> > >
> > > writel(ctrl0, dev->mmio + HW_PXP_DATA_PATH_CTRL0);
> > > writel(ctrl1, dev->mmio + HW_PXP_DATA_PATH_CTRL1);
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 2/8] media: imx-pxp: detect PXP version
2023-01-06 14:01 ` Michael Tretter
@ 2023-01-06 18:40 ` Laurent Pinchart
-1 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 18:40 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, Jan 06, 2023 at 03:01:41PM +0100, Michael Tretter wrote:
> On Fri, 06 Jan 2023 14:28:47 +0200, Laurent Pinchart wrote:
> > On Fri, Jan 06, 2023 at 01:47:32PM +0200, Laurent Pinchart wrote:
> > > Thank you for the patch.
> > >
> > > On Thu, Jan 05, 2023 at 02:47:23PM +0100, Michael Tretter wrote:
> > > > Different versions of the Pixel Pipeline have different blocks and their
> > > > routing may be different. Read the PXP_HW_VERSION register to determine
> > > > the version of the PXP and print it to the log for debugging purposes.
> > >
> > > Is there a specific reason you chose to read the version register
> > > instead of basing the decision on the compatible string ?
> >
> > Reading the rest of the series, you use the compatible strings later,
> > and never use the hw_version field. I'm tempted to propose dropping this
> > patch.
>
> My first try was to use the version register, but it turned out that the
> version is the same at least on i.MX6ULL and i.MX7D. I kept it to avoid that
> others fall into the same trap.
>
> >
> > > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > > ---
> > > > drivers/media/platform/nxp/imx-pxp.c | 18 ++++++++++++++++++
> > > > 1 file changed, 18 insertions(+)
> > > >
> > > > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> > > > index 689ae5e6ac62..05abe40558b0 100644
> > > > --- a/drivers/media/platform/nxp/imx-pxp.c
> > > > +++ b/drivers/media/platform/nxp/imx-pxp.c
> > > > @@ -10,6 +10,7 @@
> > > > * Pawel Osciak, <pawel@osciak.com>
> > > > * Marek Szyprowski, <m.szyprowski@samsung.com>
> > > > */
> > > > +#include <linux/bitfield.h>
> > > > #include <linux/clk.h>
> > > > #include <linux/delay.h>
> > > > #include <linux/dma-mapping.h>
> > > > @@ -52,6 +53,11 @@ MODULE_PARM_DESC(debug, "activates debug info");
> > > > #define MEM2MEM_HFLIP (1 << 0)
> > > > #define MEM2MEM_VFLIP (1 << 1)
> > > >
> > > > +#define PXP_VERSION_MAJOR(version) \
> > > > + FIELD_GET(BM_PXP_VERSION_MAJOR, version)
> > > > +#define PXP_VERSION_MINOR(version) \
> > > > + FIELD_GET(BM_PXP_VERSION_MINOR, version)
> > > > +
> > > > #define dprintk(dev, fmt, arg...) \
> > > > v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: " fmt, __func__, ## arg)
> > > >
> > > > @@ -192,6 +198,8 @@ struct pxp_dev {
> > > > struct clk *clk;
> > > > void __iomem *mmio;
> > > >
> > > > + u32 hw_version;
> > > > +
> > > > atomic_t num_inst;
> > > > struct mutex dev_mutex;
> > > > spinlock_t irqlock;
> > > > @@ -1660,6 +1668,11 @@ static int pxp_soft_reset(struct pxp_dev *dev)
> > > > return 0;
> > > > }
> > > >
> > > > +static u32 pxp_read_version(struct pxp_dev *dev)
> > > > +{
> > > > + return readl(dev->mmio + HW_PXP_VERSION);
> > > > +}
> > > > +
> > > > static int pxp_probe(struct platform_device *pdev)
> > > > {
> > > > struct pxp_dev *dev;
> > > > @@ -1705,6 +1718,11 @@ static int pxp_probe(struct platform_device *pdev)
> > > > goto err_clk;
> > > > }
> > > >
> > > > + dev->hw_version = pxp_read_version(dev);
> > > > + dev_info(&pdev->dev, "PXP Version %d.%d\n",
> > >
> > > As the version can't be negative, I'd use %u.%u.
> > >
> > > > + PXP_VERSION_MAJOR(dev->hw_version),
> > > > + PXP_VERSION_MINOR(dev->hw_version));
> > > > +
> > >
> > > The driver now prints two messages at probe time, it would be nice to
> > > combine them, or remove the other one. That's a candidate for a future
> > > patch though.
>
> I would reduce the level to dev_debug. Then the version is not always printed,
> but it can be easily enabled if necessary for the bringup on another platform.
Fine with me.
> > > > ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> > > > if (ret)
> > > > goto err_clk;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 2/8] media: imx-pxp: detect PXP version
@ 2023-01-06 18:40 ` Laurent Pinchart
0 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 18:40 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, Jan 06, 2023 at 03:01:41PM +0100, Michael Tretter wrote:
> On Fri, 06 Jan 2023 14:28:47 +0200, Laurent Pinchart wrote:
> > On Fri, Jan 06, 2023 at 01:47:32PM +0200, Laurent Pinchart wrote:
> > > Thank you for the patch.
> > >
> > > On Thu, Jan 05, 2023 at 02:47:23PM +0100, Michael Tretter wrote:
> > > > Different versions of the Pixel Pipeline have different blocks and their
> > > > routing may be different. Read the PXP_HW_VERSION register to determine
> > > > the version of the PXP and print it to the log for debugging purposes.
> > >
> > > Is there a specific reason you chose to read the version register
> > > instead of basing the decision on the compatible string ?
> >
> > Reading the rest of the series, you use the compatible strings later,
> > and never use the hw_version field. I'm tempted to propose dropping this
> > patch.
>
> My first try was to use the version register, but it turned out that the
> version is the same at least on i.MX6ULL and i.MX7D. I kept it to avoid that
> others fall into the same trap.
>
> >
> > > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > > ---
> > > > drivers/media/platform/nxp/imx-pxp.c | 18 ++++++++++++++++++
> > > > 1 file changed, 18 insertions(+)
> > > >
> > > > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> > > > index 689ae5e6ac62..05abe40558b0 100644
> > > > --- a/drivers/media/platform/nxp/imx-pxp.c
> > > > +++ b/drivers/media/platform/nxp/imx-pxp.c
> > > > @@ -10,6 +10,7 @@
> > > > * Pawel Osciak, <pawel@osciak.com>
> > > > * Marek Szyprowski, <m.szyprowski@samsung.com>
> > > > */
> > > > +#include <linux/bitfield.h>
> > > > #include <linux/clk.h>
> > > > #include <linux/delay.h>
> > > > #include <linux/dma-mapping.h>
> > > > @@ -52,6 +53,11 @@ MODULE_PARM_DESC(debug, "activates debug info");
> > > > #define MEM2MEM_HFLIP (1 << 0)
> > > > #define MEM2MEM_VFLIP (1 << 1)
> > > >
> > > > +#define PXP_VERSION_MAJOR(version) \
> > > > + FIELD_GET(BM_PXP_VERSION_MAJOR, version)
> > > > +#define PXP_VERSION_MINOR(version) \
> > > > + FIELD_GET(BM_PXP_VERSION_MINOR, version)
> > > > +
> > > > #define dprintk(dev, fmt, arg...) \
> > > > v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: " fmt, __func__, ## arg)
> > > >
> > > > @@ -192,6 +198,8 @@ struct pxp_dev {
> > > > struct clk *clk;
> > > > void __iomem *mmio;
> > > >
> > > > + u32 hw_version;
> > > > +
> > > > atomic_t num_inst;
> > > > struct mutex dev_mutex;
> > > > spinlock_t irqlock;
> > > > @@ -1660,6 +1668,11 @@ static int pxp_soft_reset(struct pxp_dev *dev)
> > > > return 0;
> > > > }
> > > >
> > > > +static u32 pxp_read_version(struct pxp_dev *dev)
> > > > +{
> > > > + return readl(dev->mmio + HW_PXP_VERSION);
> > > > +}
> > > > +
> > > > static int pxp_probe(struct platform_device *pdev)
> > > > {
> > > > struct pxp_dev *dev;
> > > > @@ -1705,6 +1718,11 @@ static int pxp_probe(struct platform_device *pdev)
> > > > goto err_clk;
> > > > }
> > > >
> > > > + dev->hw_version = pxp_read_version(dev);
> > > > + dev_info(&pdev->dev, "PXP Version %d.%d\n",
> > >
> > > As the version can't be negative, I'd use %u.%u.
> > >
> > > > + PXP_VERSION_MAJOR(dev->hw_version),
> > > > + PXP_VERSION_MINOR(dev->hw_version));
> > > > +
> > >
> > > The driver now prints two messages at probe time, it would be nice to
> > > combine them, or remove the other one. That's a candidate for a future
> > > patch though.
>
> I would reduce the level to dev_debug. Then the version is not always printed,
> but it can be easily enabled if necessary for the bringup on another platform.
Fine with me.
> > > > ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> > > > if (ret)
> > > > goto err_clk;
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 6/8] media: imx-pxp: make data_path_ctrl0 platform dependent
2023-01-06 14:11 ` Michael Tretter
@ 2023-01-06 18:42 ` Laurent Pinchart
-1 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 18:42 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, Jan 06, 2023 at 03:11:41PM +0100, Michael Tretter wrote:
> On Fri, 06 Jan 2023 14:30:52 +0200, Laurent Pinchart wrote:
> > On Thu, Jan 05, 2023 at 02:47:27PM +0100, Michael Tretter wrote:
> > > Unfortunately, the PXP_HW_VERSION register reports the PXP on the i.MX7D
> > > and on the i.MX6ULL as version 3.0, although the PXP versions on these
> > > SoCs have significant differences.
> > >
> > > Use the compatible to configure the ctrl0 register as required dependent
> > > on the platform.
> > >
> > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > ---
> > > drivers/media/platform/nxp/imx-pxp.c | 27 +++++++++++++++++++++++++--
> > > 1 file changed, 25 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> > > index 1d649b9cadad..4e182f80a36b 100644
> > > --- a/drivers/media/platform/nxp/imx-pxp.c
> > > +++ b/drivers/media/platform/nxp/imx-pxp.c
> > > @@ -19,6 +19,7 @@
> > > #include <linux/iopoll.h>
> > > #include <linux/module.h>
> > > #include <linux/of.h>
> > > +#include <linux/of_device.h>
> > > #include <linux/sched.h>
> > > #include <linux/slab.h>
> > >
> > > @@ -191,6 +192,11 @@ static struct pxp_fmt *find_format(struct v4l2_format *f)
> > > return &formats[k];
> > > }
> > >
> > > +struct pxp_ctx;
> >
> > Please add a blank line here.
> >
> > > +struct pxp_pdata {
> > > + u32 (*data_path_ctrl0)(struct pxp_ctx *ctx);
> > > +};
> > > +
> > > struct pxp_dev {
> > > struct v4l2_device v4l2_dev;
> > > struct video_device vfd;
> > > @@ -199,6 +205,7 @@ struct pxp_dev {
> > > void __iomem *mmio;
> > >
> > > u32 hw_version;
> > > + const struct pxp_pdata *pdata;
> > >
> > > atomic_t num_inst;
> > > struct mutex dev_mutex;
> > > @@ -726,7 +733,7 @@ static void pxp_setup_csc(struct pxp_ctx *ctx)
> > > }
> > > }
> > >
> > > -static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > > +static u32 pxp_imx6ull_data_path_ctrl0(struct pxp_ctx *ctx)
> > > {
> > > u32 ctrl0;
> > >
> > > @@ -756,6 +763,16 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > > return ctrl0;
> > > }
> > >
> > > +static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > > +{
> > > + struct pxp_dev *dev = ctx->dev;
> > > +
> > > + if (dev->pdata && dev->pdata->data_path_ctrl0)
> > > + return dev->pdata->data_path_ctrl0(ctx);
> > > +
> > > + return pxp_imx6ull_data_path_ctrl0(ctx);
> >
> > Do you need this fallback, given that all compatible strings give you
> > valid pdata ? I'd rather be explicit.
> >
> > This function then becomes so small that I would inline it in the
> > caller.
>
> I was a bit paranoid that there may be cases in which pdata is not set. I will
> change this to assume that pdata is always valid and just be explicit.
If you're worried that future addition of platform support could add a
compatible string with NULL .data, you could catch that in probe(). I'm
not worried personally, as it would crash on first use, so the developer
submitting the patch should notice.
> > > +}
> > > +
> > > static void pxp_set_data_path(struct pxp_ctx *ctx)
> > > {
> > > struct pxp_dev *dev = ctx->dev;
> > > @@ -1711,6 +1728,8 @@ static int pxp_probe(struct platform_device *pdev)
> > > if (!dev)
> > > return -ENOMEM;
> > >
> > > + dev->pdata = of_device_get_match_data(&pdev->dev);
> > > +
> > > dev->clk = devm_clk_get(&pdev->dev, "axi");
> > > if (IS_ERR(dev->clk)) {
> > > ret = PTR_ERR(dev->clk);
> > > @@ -1811,8 +1830,12 @@ static int pxp_remove(struct platform_device *pdev)
> > > return 0;
> > > }
> > >
> > > +static const struct pxp_pdata pxp_imx6ull_pdata = {
> > > + .data_path_ctrl0 = pxp_imx6ull_data_path_ctrl0,
> > > +};
> > > +
> > > static const struct of_device_id pxp_dt_ids[] = {
> > > - { .compatible = "fsl,imx6ull-pxp", .data = NULL },
> > > + { .compatible = "fsl,imx6ull-pxp", .data = &pxp_imx6ull_pdata },
> > > { },
> > > };
> > > MODULE_DEVICE_TABLE(of, pxp_dt_ids);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 6/8] media: imx-pxp: make data_path_ctrl0 platform dependent
@ 2023-01-06 18:42 ` Laurent Pinchart
0 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 18:42 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, Jan 06, 2023 at 03:11:41PM +0100, Michael Tretter wrote:
> On Fri, 06 Jan 2023 14:30:52 +0200, Laurent Pinchart wrote:
> > On Thu, Jan 05, 2023 at 02:47:27PM +0100, Michael Tretter wrote:
> > > Unfortunately, the PXP_HW_VERSION register reports the PXP on the i.MX7D
> > > and on the i.MX6ULL as version 3.0, although the PXP versions on these
> > > SoCs have significant differences.
> > >
> > > Use the compatible to configure the ctrl0 register as required dependent
> > > on the platform.
> > >
> > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > ---
> > > drivers/media/platform/nxp/imx-pxp.c | 27 +++++++++++++++++++++++++--
> > > 1 file changed, 25 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c
> > > index 1d649b9cadad..4e182f80a36b 100644
> > > --- a/drivers/media/platform/nxp/imx-pxp.c
> > > +++ b/drivers/media/platform/nxp/imx-pxp.c
> > > @@ -19,6 +19,7 @@
> > > #include <linux/iopoll.h>
> > > #include <linux/module.h>
> > > #include <linux/of.h>
> > > +#include <linux/of_device.h>
> > > #include <linux/sched.h>
> > > #include <linux/slab.h>
> > >
> > > @@ -191,6 +192,11 @@ static struct pxp_fmt *find_format(struct v4l2_format *f)
> > > return &formats[k];
> > > }
> > >
> > > +struct pxp_ctx;
> >
> > Please add a blank line here.
> >
> > > +struct pxp_pdata {
> > > + u32 (*data_path_ctrl0)(struct pxp_ctx *ctx);
> > > +};
> > > +
> > > struct pxp_dev {
> > > struct v4l2_device v4l2_dev;
> > > struct video_device vfd;
> > > @@ -199,6 +205,7 @@ struct pxp_dev {
> > > void __iomem *mmio;
> > >
> > > u32 hw_version;
> > > + const struct pxp_pdata *pdata;
> > >
> > > atomic_t num_inst;
> > > struct mutex dev_mutex;
> > > @@ -726,7 +733,7 @@ static void pxp_setup_csc(struct pxp_ctx *ctx)
> > > }
> > > }
> > >
> > > -static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > > +static u32 pxp_imx6ull_data_path_ctrl0(struct pxp_ctx *ctx)
> > > {
> > > u32 ctrl0;
> > >
> > > @@ -756,6 +763,16 @@ static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > > return ctrl0;
> > > }
> > >
> > > +static u32 pxp_data_path_ctrl0(struct pxp_ctx *ctx)
> > > +{
> > > + struct pxp_dev *dev = ctx->dev;
> > > +
> > > + if (dev->pdata && dev->pdata->data_path_ctrl0)
> > > + return dev->pdata->data_path_ctrl0(ctx);
> > > +
> > > + return pxp_imx6ull_data_path_ctrl0(ctx);
> >
> > Do you need this fallback, given that all compatible strings give you
> > valid pdata ? I'd rather be explicit.
> >
> > This function then becomes so small that I would inline it in the
> > caller.
>
> I was a bit paranoid that there may be cases in which pdata is not set. I will
> change this to assume that pdata is always valid and just be explicit.
If you're worried that future addition of platform support could add a
compatible string with NULL .data, you could catch that in probe(). I'm
not worried personally, as it would crash on first use, so the developer
submitting the patch should notice.
> > > +}
> > > +
> > > static void pxp_set_data_path(struct pxp_ctx *ctx)
> > > {
> > > struct pxp_dev *dev = ctx->dev;
> > > @@ -1711,6 +1728,8 @@ static int pxp_probe(struct platform_device *pdev)
> > > if (!dev)
> > > return -ENOMEM;
> > >
> > > + dev->pdata = of_device_get_match_data(&pdev->dev);
> > > +
> > > dev->clk = devm_clk_get(&pdev->dev, "axi");
> > > if (IS_ERR(dev->clk)) {
> > > ret = PTR_ERR(dev->clk);
> > > @@ -1811,8 +1830,12 @@ static int pxp_remove(struct platform_device *pdev)
> > > return 0;
> > > }
> > >
> > > +static const struct pxp_pdata pxp_imx6ull_pdata = {
> > > + .data_path_ctrl0 = pxp_imx6ull_data_path_ctrl0,
> > > +};
> > > +
> > > static const struct of_device_id pxp_dt_ids[] = {
> > > - { .compatible = "fsl,imx6ull-pxp", .data = NULL },
> > > + { .compatible = "fsl,imx6ull-pxp", .data = &pxp_imx6ull_pdata },
> > > { },
> > > };
> > > MODULE_DEVICE_TABLE(of, pxp_dt_ids);
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 8/8] ARM: dts: imx7d: add node for PXP
2023-01-06 14:36 ` Michael Tretter
@ 2023-01-06 18:43 ` Laurent Pinchart
-1 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 18:43 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, Jan 06, 2023 at 03:36:21PM +0100, Michael Tretter wrote:
> On Fri, 06 Jan 2023 14:36:41 +0200, Laurent Pinchart wrote:
> > Hi Michael,
> >
> > Thank you for the patch.
> >
> > On Thu, Jan 05, 2023 at 02:47:29PM +0100, Michael Tretter wrote:
> > > The i.MX7d contains a Pixel Pipeline in version 3.0. Add the device tree
> > > node to make it available.
> > >
> > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > ---
> > > arch/arm/boot/dts/imx7d.dtsi | 9 +++++++++
> > > 1 file changed, 9 insertions(+)
> > >
> > > diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
> > > index 7ceb7c09f7ad..728cc9413a7c 100644
> > > --- a/arch/arm/boot/dts/imx7d.dtsi
> > > +++ b/arch/arm/boot/dts/imx7d.dtsi
> > > @@ -165,6 +165,15 @@ pcie_phy: pcie-phy@306d0000 {
> > > reg = <0x306d0000 0x10000>;
> > > status = "disabled";
> > > };
> > > +
> > > + pxp: pxp@30700000 {
> > > + compatible = "fsl,imx7d-pxp";
> >
> > Hmmm... The i.MX7S also has a PXP that seems compatible. I thus wonder
> > if we shouldn't move this node to imx7s.dtsi.
>
> The i.MX7S has a PXP at the same address, but the architecture in the
> reference manual (Figure 13-71. PXP Architecture, p. 3797) looks slightly
> different wrt. the location of the multiplexers. The reference manual is also
> conspicuously lacking documentation of the DATA_PATH_CTRL0 register.
>
> I wouldn't risk adding the node to the imx7s.dtsi and would rather keep the
> option to add a different compatible for the i.MX7S to be able to handle the
> difference.
OK, fine with me.
> > > + reg = <0x30700000 0x10000>;
> > > + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> > > + <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
> >
> > Nitpicking, alignment ?
> >
> > > + clocks = <&clks IMX7D_PXP_CLK>;
> > > + clock-names = "axi";
> > > + };
> > > };
> > >
> > > &aips3 {
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 8/8] ARM: dts: imx7d: add node for PXP
@ 2023-01-06 18:43 ` Laurent Pinchart
0 siblings, 0 replies; 62+ messages in thread
From: Laurent Pinchart @ 2023-01-06 18:43 UTC (permalink / raw)
To: Michael Tretter
Cc: linux-media, devicetree, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Fabio Estevam, kernel,
linux-imx, linux-arm-kernel
On Fri, Jan 06, 2023 at 03:36:21PM +0100, Michael Tretter wrote:
> On Fri, 06 Jan 2023 14:36:41 +0200, Laurent Pinchart wrote:
> > Hi Michael,
> >
> > Thank you for the patch.
> >
> > On Thu, Jan 05, 2023 at 02:47:29PM +0100, Michael Tretter wrote:
> > > The i.MX7d contains a Pixel Pipeline in version 3.0. Add the device tree
> > > node to make it available.
> > >
> > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > ---
> > > arch/arm/boot/dts/imx7d.dtsi | 9 +++++++++
> > > 1 file changed, 9 insertions(+)
> > >
> > > diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
> > > index 7ceb7c09f7ad..728cc9413a7c 100644
> > > --- a/arch/arm/boot/dts/imx7d.dtsi
> > > +++ b/arch/arm/boot/dts/imx7d.dtsi
> > > @@ -165,6 +165,15 @@ pcie_phy: pcie-phy@306d0000 {
> > > reg = <0x306d0000 0x10000>;
> > > status = "disabled";
> > > };
> > > +
> > > + pxp: pxp@30700000 {
> > > + compatible = "fsl,imx7d-pxp";
> >
> > Hmmm... The i.MX7S also has a PXP that seems compatible. I thus wonder
> > if we shouldn't move this node to imx7s.dtsi.
>
> The i.MX7S has a PXP at the same address, but the architecture in the
> reference manual (Figure 13-71. PXP Architecture, p. 3797) looks slightly
> different wrt. the location of the multiplexers. The reference manual is also
> conspicuously lacking documentation of the DATA_PATH_CTRL0 register.
>
> I wouldn't risk adding the node to the imx7s.dtsi and would rather keep the
> option to add a different compatible for the i.MX7S to be able to handle the
> difference.
OK, fine with me.
> > > + reg = <0x30700000 0x10000>;
> > > + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> > > + <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
> >
> > Nitpicking, alignment ?
> >
> > > + clocks = <&clks IMX7D_PXP_CLK>;
> > > + clock-names = "axi";
> > > + };
> > > };
> > >
> > > &aips3 {
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 62+ messages in thread
end of thread, other threads:[~2023-01-06 18:44 UTC | newest]
Thread overview: 62+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-05 13:47 [PATCH 0/8] media: imx-pxp: add support for i.MX7D Michael Tretter
2023-01-05 13:47 ` Michael Tretter
2023-01-05 13:47 ` [PATCH 1/8] media: dt-bindings: media: fsl-pxp: convert to yaml Michael Tretter
2023-01-05 13:47 ` Michael Tretter
2023-01-06 3:18 ` Rob Herring
2023-01-06 3:18 ` Rob Herring
2023-01-06 8:23 ` Michael Tretter
2023-01-06 8:23 ` Michael Tretter
2023-01-06 11:35 ` Laurent Pinchart
2023-01-06 11:35 ` Laurent Pinchart
2023-01-06 12:34 ` Krzysztof Kozlowski
2023-01-06 12:34 ` Krzysztof Kozlowski
2023-01-05 13:47 ` [PATCH 2/8] media: imx-pxp: detect PXP version Michael Tretter
2023-01-05 13:47 ` Michael Tretter
2023-01-06 11:47 ` Laurent Pinchart
2023-01-06 11:47 ` Laurent Pinchart
2023-01-06 12:28 ` Laurent Pinchart
2023-01-06 12:28 ` Laurent Pinchart
2023-01-06 14:01 ` Michael Tretter
2023-01-06 14:01 ` Michael Tretter
2023-01-06 18:40 ` Laurent Pinchart
2023-01-06 18:40 ` Laurent Pinchart
2023-01-05 13:47 ` [PATCH 3/8] media: imx-pxp: extract helper function to setup data path Michael Tretter
2023-01-05 13:47 ` Michael Tretter
2023-01-06 11:59 ` Laurent Pinchart
2023-01-06 11:59 ` Laurent Pinchart
2023-01-05 13:47 ` [PATCH 4/8] media: imx-pxp: explicitly disable unused blocks Michael Tretter
2023-01-05 13:47 ` Michael Tretter
2023-01-06 12:26 ` Laurent Pinchart
2023-01-06 12:26 ` Laurent Pinchart
2023-01-06 14:08 ` Michael Tretter
2023-01-06 14:08 ` Michael Tretter
2023-01-06 18:39 ` Laurent Pinchart
2023-01-06 18:39 ` Laurent Pinchart
2023-01-05 13:47 ` [PATCH 5/8] media: imx-pxp: disable LUT block Michael Tretter
2023-01-05 13:47 ` Michael Tretter
2023-01-06 12:27 ` Laurent Pinchart
2023-01-06 12:27 ` Laurent Pinchart
2023-01-05 13:47 ` [PATCH 6/8] media: imx-pxp: make data_path_ctrl0 platform dependent Michael Tretter
2023-01-05 13:47 ` Michael Tretter
2023-01-06 12:30 ` Laurent Pinchart
2023-01-06 12:30 ` Laurent Pinchart
2023-01-06 14:11 ` Michael Tretter
2023-01-06 14:11 ` Michael Tretter
2023-01-06 18:42 ` Laurent Pinchart
2023-01-06 18:42 ` Laurent Pinchart
2023-01-05 13:47 ` [PATCH 7/8] media: imx-pxp: add support for i.MX7D Michael Tretter
2023-01-05 13:47 ` Michael Tretter
2023-01-06 12:32 ` Laurent Pinchart
2023-01-06 12:32 ` Laurent Pinchart
2023-01-05 13:47 ` [PATCH 8/8] ARM: dts: imx7d: add node for PXP Michael Tretter
2023-01-05 13:47 ` Michael Tretter
2023-01-06 12:36 ` Laurent Pinchart
2023-01-06 12:36 ` Laurent Pinchart
2023-01-06 14:36 ` Michael Tretter
2023-01-06 14:36 ` Michael Tretter
2023-01-06 18:43 ` Laurent Pinchart
2023-01-06 18:43 ` Laurent Pinchart
2023-01-06 12:41 ` [PATCH 0/8] media: imx-pxp: add support for i.MX7D Laurent Pinchart
2023-01-06 12:41 ` Laurent Pinchart
2023-01-06 14:42 ` Michael Tretter
2023-01-06 14:42 ` Michael Tretter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.