[PATCH 1/3] video: add simple, transparent, bridge implementation

[PATCH 1/3] video: add simple, transparent, bridge implementation

[Ahmad Fatoum]

This enables support for simple bridges, i.e. bridges that can be
used without initialization.

This is e.g. the case with bridges that have persistent configuration,
the kernel has a full-fledged driver to configure the bridge and persist it.

The bootloader then needs to do nothing more. Having such a transparent
bridge allows reusing the kernel device tree without changing the graph
specification.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 .../video/display/bridge/simple-bridge.txt    | 41 +++++++++++++
 drivers/video/Kconfig                         |  7 +++
 drivers/video/Makefile                        |  2 +-
 drivers/video/simple-bridge.c                 | 57 +++++++++++++++++++
 4 files changed, 106 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/video/display/bridge/simple-bridge.txt
 create mode 100644 drivers/video/simple-bridge.c

diff --git a/Documentation/devicetree/bindings/video/display/bridge/simple-bridge.txt b/Documentation/devicetree/bindings/video/display/bridge/simple-bridge.txt
new file mode 100644
index 000000000000..b1485569d992
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display/bridge/simple-bridge.txt
@@ -0,0 +1,41 @@
+Simple display bridge
+=====================
+
+bridge node
+-----------
+
+Required properties:
+	- compatible : "barebox,simple-bridge".
+	- #address-cells : must be <1>
+	- #size-cells : must be <0>
+	- video interfaces: Device node should contain two video interface port
+			    nodes for input and output according to [1].
+			    - port@0 - bridge input
+			    - port@1 - bridge output
+
+[1]: dts/Bindings/media/video-interfaces.txt
+
+
+Example:
+	fpga-display-bridge@0 {
+		compatible = "barebox,simple-bridge";
+		reg = <0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			ch0_lcd_in: endpoint {
+				remote-endpoint = <&parallel_display_out>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+
+			ch0_out: endpoint {
+				remote-endpoint = <&disp1_in>;
+			};
+		};
+	};
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index a26bace176a1..b153978492a9 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -161,4 +161,11 @@ config DRIVER_VIDEO_SIMPLE_PANEL
 	  Linux Kernel implementation this one is able to understand display-timings
 	  nodes so that it's not necessary to keep a list of all known displays
 	  with their corresponding timings in barebox.
+
+config DRIVER_VIDEO_SIMPLE_BRIDGE
+	bool "Simple bridge support"
+	depends on OFTREE
+	help
+	  This enables support for simple bridges, i.e. bridges that can be
+	  used without initialization.
 endif
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 01fabe880920..2296c14ccc73 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -24,4 +24,4 @@ obj-$(CONFIG_DRIVER_VIDEO_IMX_IPUV3) += imx-ipu-v3/
 obj-$(CONFIG_DRIVER_VIDEO_EFI_GOP) += efi_gop.o
 obj-$(CONFIG_DRIVER_VIDEO_FB_SSD1307) += ssd1307fb.o
 obj-$(CONFIG_BACKLIGHT_RAVE_SP)	+= rave-sp-backlight.o
-
+obj-$(CONFIG_DRIVER_VIDEO_SIMPLE_BRIDGE)	+= simple-bridge.o
diff --git a/drivers/video/simple-bridge.c b/drivers/video/simple-bridge.c
new file mode 100644
index 000000000000..0d6621df7b0c
--- /dev/null
+++ b/drivers/video/simple-bridge.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// SPDX-FileCopyright-Text: 2020 Pengutronix, Ahmad Fatoum <a.fatoum@pengutronix.de>
+
+#include <common.h>
+#include <init.h>
+#include <driver.h>
+#include <video/vpl.h>
+#include <of_graph.h>
+
+enum { BRIDGE_INPUT_PORT = 0, BRIDGE_OUTPUT_PORT = 1 };
+
+struct simple_bridge_priv {
+	struct device_d	*dev;
+	struct vpl	vpl;
+};
+
+static int simple_bridge_ioctl(struct vpl *vpl, unsigned int port,
+			       unsigned int cmd, void *ptr)
+{
+	struct simple_bridge_priv *priv = container_of(vpl, struct simple_bridge_priv, vpl);
+
+	dev_dbg(priv->dev, "ioctl(port=%d, cmd=0x%08x)\n", port, cmd);
+
+	return vpl_ioctl(vpl, BRIDGE_OUTPUT_PORT, cmd, ptr);
+}
+
+static int simple_bridge_probe(struct device_d *dev)
+{
+	struct simple_bridge_priv *priv;
+	struct device_node *port;
+
+	priv = xzalloc(sizeof(*priv));
+	priv->dev = dev;
+
+	port = of_graph_get_port_by_id(dev->device_node, BRIDGE_OUTPUT_PORT);
+	if (!port) {
+		dev_err(dev, "No remote panel found\n");
+		return -ENODEV;
+	}
+
+	priv->vpl.node = dev->device_node;
+	priv->vpl.ioctl = simple_bridge_ioctl;
+
+	return vpl_register(&priv->vpl);
+}
+
+static const struct of_device_id __maybe_unused simple_bridge_match[] = {
+	{ .compatible = "barebox,simple-bridge" },
+	{ /* sentinel */ },
+};
+
+static struct driver_d simple_bridge_driver = {
+	.name		= "simple-bridge",
+	.probe		= simple_bridge_probe,
+	.of_compatible	= DRV_OF_COMPAT(simple_bridge_match),
+};
+device_platform_driver(simple_bridge_driver);
-- 
2.28.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/3] video: ipuv3: parallel-display: support of_graph binding

[Ahmad Fatoum]

"fsl,imx-parallel-display"-compatible devices are supposed to support
two bindings to query the connected LCD's parameters:

  - A display timings sub node
  - A of_graph pointing to a panel or a bridge

So far only the first was supported. Mimic what LDB is already doing, so
we support both.

Unlike LDB, if both are specified, the display timings node takes
precedence. This is to maintain backwards compatibility.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/video/imx-ipu-v3/imx-pd.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/video/imx-ipu-v3/imx-pd.c b/drivers/video/imx-ipu-v3/imx-pd.c
index 601be35880ac..fa6314c04430 100644
--- a/drivers/video/imx-ipu-v3/imx-pd.c
+++ b/drivers/video/imx-ipu-v3/imx-pd.c
@@ -27,6 +27,8 @@
 
 #include "imx-ipu-v3.h"
 
+#define IMX_PD_OUTPUT_PORT	1
+
 struct imx_pd {
 	struct device_d *dev;
 	struct display_timings *timings;
@@ -39,7 +41,6 @@ static int imx_pd_ioctl(struct vpl *vpl, unsigned int port,
 {
 	struct imx_pd *imx_pd = container_of(vpl, struct imx_pd, vpl);
 	struct ipu_di_mode *mode;
-	struct display_timings *timings;
 
 	switch (cmd) {
 	case IMX_IPU_VPL_DI_MODE:
@@ -50,15 +51,21 @@ static int imx_pd_ioctl(struct vpl *vpl, unsigned int port,
 		return 0;
 
 	case VPL_GET_VIDEOMODES:
-		timings = data;
-
-		timings->num_modes   = imx_pd->timings->num_modes;
-		timings->native_mode = imx_pd->timings->native_mode;
-		timings->modes       = imx_pd->timings->modes;
-		timings->edid        = NULL;
-		return 0;
+		if (imx_pd->timings) {
+			struct display_timings *timings = data;
+
+			timings->num_modes   = imx_pd->timings->num_modes;
+			timings->native_mode = imx_pd->timings->native_mode;
+			timings->modes       = imx_pd->timings->modes;
+			timings->edid        = NULL;
+			return 0;
+		}
+		break;
 	}
 
+	if (!imx_pd->timings)
+		return vpl_ioctl(vpl, IMX_PD_OUTPUT_PORT, cmd, data);
+
 	return 0;
 }
 
@@ -66,6 +73,7 @@ static int imx_pd_probe(struct device_d *dev)
 {
 	struct device_node *node = dev->device_node;
 	struct imx_pd *imx_pd;
+	struct device_node *port;
 	const char *fmt;
 	int ret;
 
@@ -88,8 +96,11 @@ static int imx_pd_probe(struct device_d *dev)
 
 	imx_pd->timings = of_get_display_timings(node);
 	if (!imx_pd->timings) {
-		dev_err(dev, "No display timings panel found\n");
-		return -EINVAL;
+		port = of_graph_get_port_by_id(node, IMX_PD_OUTPUT_PORT);
+		if (!port) {
+			dev_err(dev, "Neither display timings in nor remote panel found in node\n");
+			return -EINVAL;
+		}
 	}
 
 	imx_pd->vpl.node = node;
-- 
2.28.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] video: ipuv3: parallel-display: support of_graph binding

[Sascha Hauer]

On Mon, Aug 17, 2020 at 06:53:31AM +0200, Ahmad Fatoum wrote:
> "fsl,imx-parallel-display"-compatible devices are supposed to support
> two bindings to query the connected LCD's parameters:
> 
>   - A display timings sub node
>   - A of_graph pointing to a panel or a bridge
> 
> So far only the first was supported. Mimic what LDB is already doing, so
> we support both.
> 
> Unlike LDB, if both are specified, the display timings node takes
> precedence. This is to maintain backwards compatibility.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/video/imx-ipu-v3/imx-pd.c | 31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)

Applied 2/3 and 3/3, thanks

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/3] video: simple-panel: don't error out on unhandled ioctl command

[Ahmad Fatoum]

Video devices propagate ioctls down the pipeline by calling vpl_ioctl
themselves. This also happens when propagating VPL_PREPARE.
simple-panel doesn't have anything special to do in VPL_PREPARE, so
it returned -ENOSYS so far. This leads vpl_ioctl to fail, which in
turn is propagated back through the pipeline.

For devices with multiple endpoints, vpl_ioctl passes the ioctl
to each one of them. An early exit by a simple-panel can thus cause
other endpoints to not prepare themselves leading to display
issues.

Fix this by having the simple panel ignore ioctls that aren't relevant
to it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/video/simple-panel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/simple-panel.c b/drivers/video/simple-panel.c
index 1d05153d1669..2f904a7b2bee 100644
--- a/drivers/video/simple-panel.c
+++ b/drivers/video/simple-panel.c
@@ -135,7 +135,7 @@ static int simple_panel_ioctl(struct vpl *vpl, unsigned int port,
 	case VPL_GET_VIDEOMODES:
 		return simple_panel_get_modes(panel, ptr);
 	default:
-		return -ENOSYS;
+		return 0;
 	}
 }
 
-- 
2.28.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] video: add simple, transparent, bridge implementation

[Sam Ravnborg]

Hi Ahmad.

On Mon, Aug 17, 2020 at 06:53:30AM +0200, Ahmad Fatoum wrote:
> This enables support for simple bridges, i.e. bridges that can be
> used without initialization.
> 
> This is e.g. the case with bridges that have persistent configuration,
> the kernel has a full-fledged driver to configure the bridge and persist it.
> 
> The bootloader then needs to do nothing more. Having such a transparent
> bridge allows reusing the kernel device tree without changing the graph
> specification.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Looking at this with my kernel hat on.

The kernel already have a simple-bridge.yaml binding, so another name
for the binding would be preferred - to avoid the name clash.
Naming it barebox,simple-bridge would be fine IMO.

And in the kernel we today only accept bindings in DT schema format
(.yaml). Maybe do the same in the barebox and convert this binding to DT
Schema format while at it.

	Sam

> ---
>  .../video/display/bridge/simple-bridge.txt    | 41 +++++++++++++
>  drivers/video/Kconfig                         |  7 +++
>  drivers/video/Makefile                        |  2 +-
>  drivers/video/simple-bridge.c                 | 57 +++++++++++++++++++
>  4 files changed, 106 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/video/display/bridge/simple-bridge.txt
>  create mode 100644 drivers/video/simple-bridge.c
> 
> diff --git a/Documentation/devicetree/bindings/video/display/bridge/simple-bridge.txt b/Documentation/devicetree/bindings/video/display/bridge/simple-bridge.txt
> new file mode 100644
> index 000000000000..b1485569d992
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/display/bridge/simple-bridge.txt
> @@ -0,0 +1,41 @@
> +Simple display bridge
> +=====================
> +
> +bridge node
> +-----------
> +
> +Required properties:
> +	- compatible : "barebox,simple-bridge".
> +	- #address-cells : must be <1>
> +	- #size-cells : must be <0>
> +	- video interfaces: Device node should contain two video interface port
> +			    nodes for input and output according to [1].
> +			    - port@0 - bridge input
> +			    - port@1 - bridge output
> +
> +[1]: dts/Bindings/media/video-interfaces.txt
> +
> +
> +Example:
> +	fpga-display-bridge@0 {
> +		compatible = "barebox,simple-bridge";
> +		reg = <0>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		port@0 {
> +			reg = <0>;
> +
> +			ch0_lcd_in: endpoint {
> +				remote-endpoint = <&parallel_display_out>;
> +			};
> +		};
> +
> +		port@1 {
> +			reg = <1>;
> +
> +			ch0_out: endpoint {
> +				remote-endpoint = <&disp1_in>;
> +			};
> +		};
> +	};
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index a26bace176a1..b153978492a9 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -161,4 +161,11 @@ config DRIVER_VIDEO_SIMPLE_PANEL
>  	  Linux Kernel implementation this one is able to understand display-timings
>  	  nodes so that it's not necessary to keep a list of all known displays
>  	  with their corresponding timings in barebox.
> +
> +config DRIVER_VIDEO_SIMPLE_BRIDGE
> +	bool "Simple bridge support"
> +	depends on OFTREE
> +	help
> +	  This enables support for simple bridges, i.e. bridges that can be
> +	  used without initialization.
>  endif
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 01fabe880920..2296c14ccc73 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -24,4 +24,4 @@ obj-$(CONFIG_DRIVER_VIDEO_IMX_IPUV3) += imx-ipu-v3/
>  obj-$(CONFIG_DRIVER_VIDEO_EFI_GOP) += efi_gop.o
>  obj-$(CONFIG_DRIVER_VIDEO_FB_SSD1307) += ssd1307fb.o
>  obj-$(CONFIG_BACKLIGHT_RAVE_SP)	+= rave-sp-backlight.o
> -
> +obj-$(CONFIG_DRIVER_VIDEO_SIMPLE_BRIDGE)	+= simple-bridge.o
> diff --git a/drivers/video/simple-bridge.c b/drivers/video/simple-bridge.c
> new file mode 100644
> index 000000000000..0d6621df7b0c
> --- /dev/null
> +++ b/drivers/video/simple-bridge.c
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// SPDX-FileCopyright-Text: 2020 Pengutronix, Ahmad Fatoum <a.fatoum@pengutronix.de>
> +
> +#include <common.h>
> +#include <init.h>
> +#include <driver.h>
> +#include <video/vpl.h>
> +#include <of_graph.h>
> +
> +enum { BRIDGE_INPUT_PORT = 0, BRIDGE_OUTPUT_PORT = 1 };
> +
> +struct simple_bridge_priv {
> +	struct device_d	*dev;
> +	struct vpl	vpl;
> +};
> +
> +static int simple_bridge_ioctl(struct vpl *vpl, unsigned int port,
> +			       unsigned int cmd, void *ptr)
> +{
> +	struct simple_bridge_priv *priv = container_of(vpl, struct simple_bridge_priv, vpl);
> +
> +	dev_dbg(priv->dev, "ioctl(port=%d, cmd=0x%08x)\n", port, cmd);
> +
> +	return vpl_ioctl(vpl, BRIDGE_OUTPUT_PORT, cmd, ptr);
> +}
> +
> +static int simple_bridge_probe(struct device_d *dev)
> +{
> +	struct simple_bridge_priv *priv;
> +	struct device_node *port;
> +
> +	priv = xzalloc(sizeof(*priv));
> +	priv->dev = dev;
> +
> +	port = of_graph_get_port_by_id(dev->device_node, BRIDGE_OUTPUT_PORT);
> +	if (!port) {
> +		dev_err(dev, "No remote panel found\n");
> +		return -ENODEV;
> +	}
> +
> +	priv->vpl.node = dev->device_node;
> +	priv->vpl.ioctl = simple_bridge_ioctl;
> +
> +	return vpl_register(&priv->vpl);
> +}
> +
> +static const struct of_device_id __maybe_unused simple_bridge_match[] = {
> +	{ .compatible = "barebox,simple-bridge" },
> +	{ /* sentinel */ },
> +};
> +
> +static struct driver_d simple_bridge_driver = {
> +	.name		= "simple-bridge",
> +	.probe		= simple_bridge_probe,
> +	.of_compatible	= DRV_OF_COMPAT(simple_bridge_match),
> +};
> +device_platform_driver(simple_bridge_driver);
> -- 
> 2.28.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] video: add simple, transparent, bridge implementation

[Ahmad Fatoum]

Hi Sam,

On 8/17/20 8:38 AM, Sam Ravnborg wrote:
> Hi Ahmad.
> 
> On Mon, Aug 17, 2020 at 06:53:30AM +0200, Ahmad Fatoum wrote:
>> This enables support for simple bridges, i.e. bridges that can be
>> used without initialization.
>>
>> This is e.g. the case with bridges that have persistent configuration,
>> the kernel has a full-fledged driver to configure the bridge and persist it.
>>
>> The bootloader then needs to do nothing more. Having such a transparent
>> bridge allows reusing the kernel device tree without changing the graph
>> specification.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> Looking at this with my kernel hat on.
> 
> The kernel already have a simple-bridge.yaml binding, so another name
> for the binding would be preferred - to avoid the name clash.
> Naming it barebox,simple-bridge would be fine IMO.

Oh, didn't notice the rename. It looks like the kernel simple bridge
does everything I want. When I work on this again, I'll look into
using that binding instead. Thanks!

> And in the kernel we today only accept bindings in DT schema format
> (.yaml). Maybe do the same in the barebox and convert this binding to DT
> Schema format while at it.

having make dtbs and dtbs_check as barebox make targets is on my todo list.
For now, I don't see the utility in having yaml bindings when they aren't
easily tested.

Cheers,
Ahmad

> 
> 	Sam
> 
>> ---
>>  .../video/display/bridge/simple-bridge.txt    | 41 +++++++++++++
>>  drivers/video/Kconfig                         |  7 +++
>>  drivers/video/Makefile                        |  2 +-
>>  drivers/video/simple-bridge.c                 | 57 +++++++++++++++++++
>>  4 files changed, 106 insertions(+), 1 deletion(-)
>>  create mode 100644 Documentation/devicetree/bindings/video/display/bridge/simple-bridge.txt
>>  create mode 100644 drivers/video/simple-bridge.c
>>
>> diff --git a/Documentation/devicetree/bindings/video/display/bridge/simple-bridge.txt b/Documentation/devicetree/bindings/video/display/bridge/simple-bridge.txt
>> new file mode 100644
>> index 000000000000..b1485569d992
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/video/display/bridge/simple-bridge.txt
>> @@ -0,0 +1,41 @@
>> +Simple display bridge
>> +=====================
>> +
>> +bridge node
>> +-----------
>> +
>> +Required properties:
>> +	- compatible : "barebox,simple-bridge".
>> +	- #address-cells : must be <1>
>> +	- #size-cells : must be <0>
>> +	- video interfaces: Device node should contain two video interface port
>> +			    nodes for input and output according to [1].
>> +			    - port@0 - bridge input
>> +			    - port@1 - bridge output
>> +
>> +[1]: dts/Bindings/media/video-interfaces.txt
>> +
>> +
>> +Example:
>> +	fpga-display-bridge@0 {
>> +		compatible = "barebox,simple-bridge";
>> +		reg = <0>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		port@0 {
>> +			reg = <0>;
>> +
>> +			ch0_lcd_in: endpoint {
>> +				remote-endpoint = <&parallel_display_out>;
>> +			};
>> +		};
>> +
>> +		port@1 {
>> +			reg = <1>;
>> +
>> +			ch0_out: endpoint {
>> +				remote-endpoint = <&disp1_in>;
>> +			};
>> +		};
>> +	};
>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
>> index a26bace176a1..b153978492a9 100644
>> --- a/drivers/video/Kconfig
>> +++ b/drivers/video/Kconfig
>> @@ -161,4 +161,11 @@ config DRIVER_VIDEO_SIMPLE_PANEL
>>  	  Linux Kernel implementation this one is able to understand display-timings
>>  	  nodes so that it's not necessary to keep a list of all known displays
>>  	  with their corresponding timings in barebox.
>> +
>> +config DRIVER_VIDEO_SIMPLE_BRIDGE
>> +	bool "Simple bridge support"
>> +	depends on OFTREE
>> +	help
>> +	  This enables support for simple bridges, i.e. bridges that can be
>> +	  used without initialization.
>>  endif
>> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
>> index 01fabe880920..2296c14ccc73 100644
>> --- a/drivers/video/Makefile
>> +++ b/drivers/video/Makefile
>> @@ -24,4 +24,4 @@ obj-$(CONFIG_DRIVER_VIDEO_IMX_IPUV3) += imx-ipu-v3/
>>  obj-$(CONFIG_DRIVER_VIDEO_EFI_GOP) += efi_gop.o
>>  obj-$(CONFIG_DRIVER_VIDEO_FB_SSD1307) += ssd1307fb.o
>>  obj-$(CONFIG_BACKLIGHT_RAVE_SP)	+= rave-sp-backlight.o
>> -
>> +obj-$(CONFIG_DRIVER_VIDEO_SIMPLE_BRIDGE)	+= simple-bridge.o
>> diff --git a/drivers/video/simple-bridge.c b/drivers/video/simple-bridge.c
>> new file mode 100644
>> index 000000000000..0d6621df7b0c
>> --- /dev/null
>> +++ b/drivers/video/simple-bridge.c
>> @@ -0,0 +1,57 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +// SPDX-FileCopyright-Text: 2020 Pengutronix, Ahmad Fatoum <a.fatoum@pengutronix.de>
>> +
>> +#include <common.h>
>> +#include <init.h>
>> +#include <driver.h>
>> +#include <video/vpl.h>
>> +#include <of_graph.h>
>> +
>> +enum { BRIDGE_INPUT_PORT = 0, BRIDGE_OUTPUT_PORT = 1 };
>> +
>> +struct simple_bridge_priv {
>> +	struct device_d	*dev;
>> +	struct vpl	vpl;
>> +};
>> +
>> +static int simple_bridge_ioctl(struct vpl *vpl, unsigned int port,
>> +			       unsigned int cmd, void *ptr)
>> +{
>> +	struct simple_bridge_priv *priv = container_of(vpl, struct simple_bridge_priv, vpl);
>> +
>> +	dev_dbg(priv->dev, "ioctl(port=%d, cmd=0x%08x)\n", port, cmd);
>> +
>> +	return vpl_ioctl(vpl, BRIDGE_OUTPUT_PORT, cmd, ptr);
>> +}
>> +
>> +static int simple_bridge_probe(struct device_d *dev)
>> +{
>> +	struct simple_bridge_priv *priv;
>> +	struct device_node *port;
>> +
>> +	priv = xzalloc(sizeof(*priv));
>> +	priv->dev = dev;
>> +
>> +	port = of_graph_get_port_by_id(dev->device_node, BRIDGE_OUTPUT_PORT);
>> +	if (!port) {
>> +		dev_err(dev, "No remote panel found\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	priv->vpl.node = dev->device_node;
>> +	priv->vpl.ioctl = simple_bridge_ioctl;
>> +
>> +	return vpl_register(&priv->vpl);
>> +}
>> +
>> +static const struct of_device_id __maybe_unused simple_bridge_match[] = {
>> +	{ .compatible = "barebox,simple-bridge" },
>> +	{ /* sentinel */ },
>> +};
>> +
>> +static struct driver_d simple_bridge_driver = {
>> +	.name		= "simple-bridge",
>> +	.probe		= simple_bridge_probe,
>> +	.of_compatible	= DRV_OF_COMPAT(simple_bridge_match),
>> +};
>> +device_platform_driver(simple_bridge_driver);
>> -- 
>> 2.28.0
>>
>>
>> _______________________________________________
>> barebox mailing list
>> barebox@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] video: add simple, transparent, bridge implementation

[Sam Ravnborg]

Hi Ahmad.

On Thu, Aug 20, 2020 at 02:24:56PM +0200, Ahmad Fatoum wrote:
> Hi Sam,
> 
> On 8/17/20 8:38 AM, Sam Ravnborg wrote:
> > Hi Ahmad.
> > 
> > On Mon, Aug 17, 2020 at 06:53:30AM +0200, Ahmad Fatoum wrote:
> >> This enables support for simple bridges, i.e. bridges that can be
> >> used without initialization.
> >>
> >> This is e.g. the case with bridges that have persistent configuration,
> >> the kernel has a full-fledged driver to configure the bridge and persist it.
> >>
> >> The bootloader then needs to do nothing more. Having such a transparent
> >> bridge allows reusing the kernel device tree without changing the graph
> >> specification.
> >>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > 
> > Looking at this with my kernel hat on.
> > 
> > The kernel already have a simple-bridge.yaml binding, so another name
> > for the binding would be preferred - to avoid the name clash.
> > Naming it barebox,simple-bridge would be fine IMO.
> 
> Oh, didn't notice the rename. It looks like the kernel simple bridge
> does everything I want. When I work on this again, I'll look into
> using that binding instead. Thanks!
> 
> > And in the kernel we today only accept bindings in DT schema format
> > (.yaml). Maybe do the same in the barebox and convert this binding to DT
> > Schema format while at it.
> 
> having make dtbs and dtbs_check as barebox make targets is on my todo list.
> For now, I don't see the utility in having yaml bindings when they aren't
> easily tested.
You are coloring me confused here.

.txt based bindings are not testable and syntax errros needs to be
spotted manually. Futrthermore there is very little in description of
the syntax.

.yaml bindings are very simple to test - there is full infrastructure in
the kernel. And there is semi formal specification of the syntax. And
this is the syntax to be used for all new bindings.

Tooling is simple - barebox tooling is not needed:

cp foobar.yaml ${kernel}/Documentation/bindings/
make dt_binding_check DT_SCHEMA_FILES=foobar.yaml

I do not know what is the right approach in barebox, but as I wrote
above the arguments confused me.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] video: add simple, transparent, bridge implementation

[Ahmad Fatoum]

Hello,

On 8/20/20 3:33 PM, Sam Ravnborg wrote:
>>> And in the kernel we today only accept bindings in DT schema format
>>> (.yaml). Maybe do the same in the barebox and convert this binding to DT
>>> Schema format while at it.
>>
>> having make dtbs and dtbs_check as barebox make targets is on my todo list.
>> For now, I don't see the utility in having yaml bindings when they aren't
>> easily tested.
> You are coloring me confused here.
> 
> .txt based bindings are not testable and syntax errros needs to be
> spotted manually. Futrthermore there is very little in description of
> the syntax.
> 
> .yaml bindings are very simple to test - there is full infrastructure in
> the kernel. And there is semi formal specification of the syntax. And
> this is the syntax to be used for all new bindings.

I am not used to writing yaml bindings. I think the effort is better
invested, when I know the bindings are actively put to use by having
a target that can be run that automatically tests everything.

When we have that, I intend to migrate existing barebox-specific bindings
to YAML, so we can spot the errors.

> Tooling is simple - barebox tooling is not needed:
> 
> cp foobar.yaml ${kernel}/Documentation/bindings/
> make dt_binding_check DT_SCHEMA_FILES=foobar.yaml
> 
> I do not know what is the right approach in barebox, but as I wrote
> above the arguments confused me.
> 
> 	Sam
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox