[PATCH 0/8] R-Car DU: Use drm bridge API

[PATCH 0/8] R-Car DU: Use drm bridge API

[Laurent Pinchart]

Hello,

(This time with the proper dri-devel mailing list address, sorry for the
noise)

This patch series replaces the custom external encoders support implementation
in the R-Car DU driver with code based on the DRM bridge API.

While the overall diffstat isn't impressive, the rcar-du-drm driver gets
notably thinner in the process:

        9 files changed, 38 insertions(+), 358 deletions(-)

This is offset by a reusable driver for LVDS encoders along with the
corresponding DT bindings (+ 277 lines).

Patch 1/8 adds the new LVDS encoder driver. It supports "dumb" LVDS encoders
only, similarly to the dumb-vga-dac driver. One notable difference, though, is
that LVDS encoders can't be purely passive, and thus require at least one
power supply (and usually multiple of them) and have a few control GPIOs (most
notably to control reset, power down, clock polarity and/or LVDS slew rate).
However, on many systems those encoders are integrated in such a way that the
control pins are pulled up or down appropriately and the power supplies are
either always on or shared with other display components that make them
operate as if they were always on. For that reason a common drivers for those
systems is useful, with simple DT bindings that don't try to cover any device-
specific control pin or power supply.

To ensure backward compatibility most LVDS encoders should *not* use the
common simple "lvds-encoder" compatible string, but should instead define a
device-specific compatible string that can then be added to the lvds-encoder
driver (patch 3/8). This way, when the need to control pins or supplies will
arise, a new driver can be developed matching on the device-specific
compatible string, which will then be removed from the simple driver. Existing
systems will migrate transparently without requiring a change to their device
tree.

A similar reasoning applies to VGA DACs, leading to the addition of the
"adi,adv7123" compatible string to the dumb-vga-dac driver's OF match table in
patch 2/8.

Patch 4/8 adds a new type field to the drm_bridge structure to inform bridge
users of the bridge type. This is useful for display driver to create a DRM
encoder of the appropriate type without having to resort to heuristics.
Patches 5/8 and 6/8 update all bridge drivers to initialize the new field to
the appropriate value.

Patches 7/8 and 8/8 finally migrate the rcar-du-drm driver to the DRM bridge
API, removing the custom VGA DAC implementation in patch 7/8 and the table of
bridge compatible strings used to find the encoder type in patch 8/8.

Cc: devicetree@vger.kernel.org

Laurent Pinchart (8):
  drm: bridge: Add LVDS encoder driver
  drm: bridge: vga-dac: Add adi,adv7123 compatible string
  drm: bridge: lvds-encoder: Add thine,thc63lvdm83d compatible string
  drm: Add encoder_type field to the drm_bridge structure
  drm: bridge: Set bridges' encoder type
  drm: Set on-chip bridges' encoder type
  drm: rcar-du: Replace manual VGA DAC implementation with DRM bridge
  drm: rcar-du: Initialize encoder's type based on the bridge's type

 .../bindings/display/bridge/lvds-transmitter.txt   |  64 +++++++
 drivers/gpu/drm/bridge/Kconfig                     |   8 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c       |   1 +
 drivers/gpu/drm/bridge/analogix-anx78xx.c          |   1 +
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |   1 +
 drivers/gpu/drm/bridge/dumb-vga-dac.c              |   2 +
 drivers/gpu/drm/bridge/dw-hdmi.c                   |   2 +
 drivers/gpu/drm/bridge/lvds-encoder.c              | 204 +++++++++++++++++++++
 drivers/gpu/drm/bridge/nxp-ptn3460.c               |   2 +
 drivers/gpu/drm/bridge/parade-ps8622.c             |   2 +
 drivers/gpu/drm/bridge/sii902x.c                   |   2 +
 drivers/gpu/drm/bridge/tc358767.c                  |   2 +
 drivers/gpu/drm/exynos/exynos_drm_mic.c            |   2 +
 drivers/gpu/drm/mediatek/mtk_hdmi.c                |   2 +
 drivers/gpu/drm/rcar-du/Kconfig                    |   6 -
 drivers/gpu/drm/rcar-du/Makefile                   |   5 +-
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c          |  67 +++----
 drivers/gpu/drm/rcar-du/rcar_du_encoder.h          |   3 -
 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c          | 137 --------------
 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.h          |  35 ----
 drivers/gpu/drm/rcar-du/rcar_du_kms.c              |  38 +---
 drivers/gpu/drm/rcar-du/rcar_du_vgacon.c           |  82 ---------
 drivers/gpu/drm/rcar-du/rcar_du_vgacon.h           |  23 ---
 drivers/gpu/drm/sti/sti_dvo.c                      |   2 +
 include/drm/drm_bridge.h                           |   8 +
 26 files changed, 344 insertions(+), 358 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
 create mode 100644 drivers/gpu/drm/bridge/lvds-encoder.c
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.h
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_vgacon.c
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_vgacon.h

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/8] drm: bridge: Add LVDS encoder driver

[Laurent Pinchart]

The LVDS encoder driver is a DRM bridge driver that supports the
parallel to LVDS encoders that don't require any configuration. The
driver thus doesn't interact with the device, but creates an LVDS
connector for the panel and exposes its size and timing based on
information retrieved from DT.

Cc: devicetree@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../bindings/display/bridge/lvds-transmitter.txt   |  64 +++++++
 drivers/gpu/drm/bridge/Kconfig                     |   8 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/lvds-encoder.c              | 203 +++++++++++++++++++++
 4 files changed, 276 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
 create mode 100644 drivers/gpu/drm/bridge/lvds-encoder.c

diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
new file mode 100644
index 000000000000..fd39ad34c383
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
@@ -0,0 +1,64 @@
+Parallel to LVDS Encoder
+------------------------
+
+This binding supports the parallel to LVDS encoders that don't require any
+configuration.
+
+LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. Multiple
+incompatible data link layers have been used over time to transmit image data
+to LVDS panels. This binding targets devices compatible with the following
+specifications only.
+
+[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, February
+1999 (Version 1.0), Japan Electronic Industry Development Association (JEIDA)
+[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
+Semiconductor
+[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
+Electronics Standards Association (VESA)
+
+Those devices have been marketed under the FPD-Link and FlatLink brand names
+among others.
+
+
+Required properties:
+
+- compatible: Must be "lvds-encoder"
+
+Required nodes:
+
+This device has two video ports. Their connections are modeled using the OF
+graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 for parallel input
+- Video port 1 for LVDS output
+
+
+Example
+-------
+
+lvds-encoder {
+	compatible = "lvds-encoder";
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			lvds_enc_in: endpoint {
+				remote-endpoint = <&display_out_rgb>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+
+			lvds_enc_out: endpoint {
+				remote-endpoint = <&lvds_panel_in>;
+			};
+		};
+	};
+};
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 10e12e74fc9f..5dafad7817ad 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -24,6 +24,14 @@ config DRM_DUMB_VGA_DAC
 	help
 	  Support for RGB to VGA DAC based bridges
 
+config DRM_LVDS_ENCODER
+	tristate "Transparent parallel to LVDS encoder support"
+	depends on OF
+	select DRM_KMS_HELPER
+	help
+	  Support for transparent parallel to LVDS encoders that don't require
+	  any configuration.
+
 config DRM_DW_HDMI
 	tristate
 	select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index cdf3a3cf765d..bbaf583581ac 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -2,6 +2,7 @@ ccflags-y := -Iinclude/drm
 
 obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
 obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
+obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
 obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
 obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c
new file mode 100644
index 000000000000..33e8025c8a6d
--- /dev/null
+++ b/drivers/gpu/drm/bridge/lvds-encoder.c
@@ -0,0 +1,203 @@
+/*
+ * Copyright (C) 2016 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_encoder.h>
+#include <drm/drm_modeset_helper_vtables.h>
+
+#include <linux/of_graph.h>
+
+#include <video/display_timing.h>
+#include <video/of_display_timing.h>
+#include <video/videomode.h>
+
+struct lvds_encoder {
+	struct device *dev;
+
+	struct drm_bridge bridge;
+	struct drm_connector connector;
+
+	struct {
+		unsigned int width_mm;
+		unsigned int height_mm;
+		struct videomode mode;
+	} panel;
+};
+
+static inline struct lvds_encoder *
+drm_bridge_to_lvds_encoder(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct lvds_encoder, bridge);
+}
+
+static inline struct lvds_encoder *
+drm_connector_to_lvds_encoder(struct drm_connector *connector)
+{
+	return container_of(connector, struct lvds_encoder, connector);
+}
+
+static int lvds_connector_get_modes(struct drm_connector *connector)
+{
+	struct lvds_encoder *lvds = drm_connector_to_lvds_encoder(connector);
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_create(connector->dev);
+	if (mode == NULL)
+		return 0;
+
+	mode->type = DRM_MODE_TYPE_PREFERRED | DRM_MODE_TYPE_DRIVER;
+
+	drm_display_mode_from_videomode(&lvds->panel.mode, mode);
+
+	drm_mode_probed_add(connector, mode);
+
+	return 1;
+}
+
+static const struct drm_connector_helper_funcs lvds_connector_helper_funcs = {
+	.get_modes = lvds_connector_get_modes,
+};
+
+static enum drm_connector_status
+lvds_connector_detect(struct drm_connector *connector, bool force)
+{
+	return connector_status_connected;
+}
+
+static const struct drm_connector_funcs lvds_connector_funcs = {
+	.dpms = drm_atomic_helper_connector_dpms,
+	.reset = drm_atomic_helper_connector_reset,
+	.detect = lvds_connector_detect,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int lvds_encoder_attach(struct drm_bridge *bridge)
+{
+	struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
+	struct drm_connector *connector = &lvds->connector;
+	int ret;
+
+	if (!bridge->encoder) {
+		DRM_ERROR("Missing encoder\n");
+		return -ENODEV;
+	}
+
+	connector->display_info.width_mm = lvds->panel.width_mm;
+	connector->display_info.height_mm = lvds->panel.height_mm;
+
+	drm_connector_helper_add(connector, &lvds_connector_helper_funcs);
+
+	ret = drm_connector_init(bridge->dev, connector, &lvds_connector_funcs,
+				 DRM_MODE_CONNECTOR_LVDS);
+	if (ret) {
+		DRM_ERROR("Failed to initialize connector\n");
+		return ret;
+	}
+
+	drm_mode_connector_attach_encoder(&lvds->connector, bridge->encoder);
+
+	return 0;
+}
+
+static const struct drm_bridge_funcs lvds_encoder_bridge_funcs = {
+	.attach = lvds_encoder_attach,
+};
+
+static int lvds_encoder_probe(struct platform_device *pdev)
+{
+	struct lvds_encoder *lvds;
+	struct display_timing timing;
+	struct device_node *port;
+	struct device_node *endpoint;
+	struct device_node *panel;
+	int ret;
+
+	lvds = devm_kzalloc(&pdev->dev, sizeof(*lvds), GFP_KERNEL);
+	if (!lvds)
+		return -ENOMEM;
+
+	lvds->dev = &pdev->dev;
+	platform_set_drvdata(pdev, lvds);
+
+	lvds->bridge.funcs = &lvds_encoder_bridge_funcs;
+	lvds->bridge.of_node = pdev->dev.of_node;
+	lvds->bridge.encoder_type = DRM_MODE_ENCODER_LVDS;
+
+	/* Locate the panel DT node. */
+	port = of_graph_get_port_by_id(pdev->dev.of_node, 1);
+	if (!port) {
+		dev_dbg(&pdev->dev, "port 1 not found\n");
+		return -ENXIO;
+	}
+
+	endpoint = of_get_child_by_name(port, "endpoint");
+	of_node_put(port);
+	if (!endpoint) {
+		dev_dbg(&pdev->dev, "no endpoint for port 1\n");
+		return -ENXIO;
+	}
+
+	panel = of_graph_get_remote_port_parent(endpoint);
+	of_node_put(endpoint);
+	if (!panel) {
+		dev_dbg(&pdev->dev, "no remote endpoint for port 1\n");
+		return -ENXIO;
+	}
+
+	/* Parse and store panel information. */
+	ret = of_get_display_timing(panel, "panel-timing", &timing);
+	of_node_put(panel);
+	if (ret < 0) {
+		dev_dbg(&pdev->dev, "unable to parse panel timings\n");
+		return ret;
+	}
+
+	videomode_from_timing(&timing, &lvds->panel.mode);
+
+	of_property_read_u32(panel, "width-mm", &lvds->panel.width_mm);
+	of_property_read_u32(panel, "height-mm", &lvds->panel.height_mm);
+
+	/* Register the bridge. */
+	return drm_bridge_add(&lvds->bridge);
+}
+
+static int lvds_encoder_remove(struct platform_device *pdev)
+{
+	struct lvds_encoder *encoder = platform_get_drvdata(pdev);
+
+	drm_bridge_remove(&encoder->bridge);
+
+	return 0;
+}
+
+static const struct of_device_id lvds_encoder_match[] = {
+	{ .compatible = "lvds-encoder" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, lvds_encoder_match);
+
+struct platform_driver lvds_encoder_driver = {
+	.probe	= lvds_encoder_probe,
+	.remove	= lvds_encoder_remove,
+	.driver		= {
+		.name		= "lvds-encoder",
+		.of_match_table	= lvds_encoder_match,
+	},
+};
+module_platform_driver(lvds_encoder_driver);
+
+MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
+MODULE_DESCRIPTION("Transparent parallel to LVDS encoder");
+MODULE_LICENSE("GPL");
-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/8] drm: bridge: Add LVDS encoder driver

[Archit Taneja]

On 10/19/2016 07:55 PM, Laurent Pinchart wrote:
> The LVDS encoder driver is a DRM bridge driver that supports the
> parallel to LVDS encoders that don't require any configuration. The
> driver thus doesn't interact with the device, but creates an LVDS
> connector for the panel and exposes its size and timing based on
> information retrieved from DT.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../bindings/display/bridge/lvds-transmitter.txt   |  64 +++++++
>  drivers/gpu/drm/bridge/Kconfig                     |   8 +
>  drivers/gpu/drm/bridge/Makefile                    |   1 +
>  drivers/gpu/drm/bridge/lvds-encoder.c              | 203 +++++++++++++++++++++
>  4 files changed, 276 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
>  create mode 100644 drivers/gpu/drm/bridge/lvds-encoder.c
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> new file mode 100644
> index 000000000000..fd39ad34c383
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> @@ -0,0 +1,64 @@
> +Parallel to LVDS Encoder
> +------------------------
> +
> +This binding supports the parallel to LVDS encoders that don't require any
> +configuration.
> +
> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. Multiple
> +incompatible data link layers have been used over time to transmit image data
> +to LVDS panels. This binding targets devices compatible with the following
> +specifications only.
> +
> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, February
> +1999 (Version 1.0), Japan Electronic Industry Development Association (JEIDA)
> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
> +Semiconductor
> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
> +Electronics Standards Association (VESA)
> +
> +Those devices have been marketed under the FPD-Link and FlatLink brand names
> +among others.
> +
> +
> +Required properties:
> +
> +- compatible: Must be "lvds-encoder"
> +
> +Required nodes:
> +
> +This device has two video ports. Their connections are modeled using the OF
> +graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> +
> +- Video port 0 for parallel input
> +- Video port 1 for LVDS output
> +
> +
> +Example
> +-------
> +
> +lvds-encoder {
> +	compatible = "lvds-encoder";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		port@0 {
> +			reg = <0>;
> +
> +			lvds_enc_in: endpoint {
> +				remote-endpoint = <&display_out_rgb>;
> +			};
> +		};
> +
> +		port@1 {
> +			reg = <1>;
> +
> +			lvds_enc_out: endpoint {
> +				remote-endpoint = <&lvds_panel_in>;
> +			};
> +		};
> +	};
> +};
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 10e12e74fc9f..5dafad7817ad 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -24,6 +24,14 @@ config DRM_DUMB_VGA_DAC
>  	help
>  	  Support for RGB to VGA DAC based bridges
>
> +config DRM_LVDS_ENCODER
> +	tristate "Transparent parallel to LVDS encoder support"
> +	depends on OF
> +	select DRM_KMS_HELPER
> +	help
> +	  Support for transparent parallel to LVDS encoders that don't require
> +	  any configuration.
> +
>  config DRM_DW_HDMI
>  	tristate
>  	select DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index cdf3a3cf765d..bbaf583581ac 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -2,6 +2,7 @@ ccflags-y := -Iinclude/drm
>
>  obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
>  obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
> +obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
>  obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
>  obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
>  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c
> new file mode 100644
> index 000000000000..33e8025c8a6d
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/lvds-encoder.c
> @@ -0,0 +1,203 @@
> +/*
> + * Copyright (C) 2016 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_connector.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_encoder.h>
> +#include <drm/drm_modeset_helper_vtables.h>
> +
> +#include <linux/of_graph.h>
> +
> +#include <video/display_timing.h>
> +#include <video/of_display_timing.h>
> +#include <video/videomode.h>
> +
> +struct lvds_encoder {
> +	struct device *dev;
> +
> +	struct drm_bridge bridge;
> +	struct drm_connector connector;
> +
> +	struct {
> +		unsigned int width_mm;
> +		unsigned int height_mm;
> +		struct videomode mode;
> +	} panel;

I don't see why we need create our own panel instance
in the encoder driver. We should use the drm_panel
API here rather than managing panel specific stuff in
this driver.

The parade-ps8622 and nxp-ptn3460 bridge drivers already
do this.

Thanks,
Archit

> +};
> +
> +static inline struct lvds_encoder *
> +drm_bridge_to_lvds_encoder(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct lvds_encoder, bridge);
> +}
> +
> +static inline struct lvds_encoder *
> +drm_connector_to_lvds_encoder(struct drm_connector *connector)
> +{
> +	return container_of(connector, struct lvds_encoder, connector);
> +}
> +
> +static int lvds_connector_get_modes(struct drm_connector *connector)
> +{
> +	struct lvds_encoder *lvds = drm_connector_to_lvds_encoder(connector);
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_create(connector->dev);
> +	if (mode == NULL)
> +		return 0;
> +
> +	mode->type = DRM_MODE_TYPE_PREFERRED | DRM_MODE_TYPE_DRIVER;
> +
> +	drm_display_mode_from_videomode(&lvds->panel.mode, mode);
> +
> +	drm_mode_probed_add(connector, mode);
> +
> +	return 1;
> +}
> +
> +static const struct drm_connector_helper_funcs lvds_connector_helper_funcs = {
> +	.get_modes = lvds_connector_get_modes,
> +};
> +
> +static enum drm_connector_status
> +lvds_connector_detect(struct drm_connector *connector, bool force)
> +{
> +	return connector_status_connected;
> +}
> +
> +static const struct drm_connector_funcs lvds_connector_funcs = {
> +	.dpms = drm_atomic_helper_connector_dpms,
> +	.reset = drm_atomic_helper_connector_reset,
> +	.detect = lvds_connector_detect,
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = drm_connector_cleanup,
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static int lvds_encoder_attach(struct drm_bridge *bridge)
> +{
> +	struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
> +	struct drm_connector *connector = &lvds->connector;
> +	int ret;
> +
> +	if (!bridge->encoder) {
> +		DRM_ERROR("Missing encoder\n");
> +		return -ENODEV;
> +	}
> +
> +	connector->display_info.width_mm = lvds->panel.width_mm;
> +	connector->display_info.height_mm = lvds->panel.height_mm;
> +
> +	drm_connector_helper_add(connector, &lvds_connector_helper_funcs);
> +
> +	ret = drm_connector_init(bridge->dev, connector, &lvds_connector_funcs,
> +				 DRM_MODE_CONNECTOR_LVDS);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize connector\n");
> +		return ret;
> +	}
> +
> +	drm_mode_connector_attach_encoder(&lvds->connector, bridge->encoder);
> +
> +	return 0;
> +}
> +
> +static const struct drm_bridge_funcs lvds_encoder_bridge_funcs = {
> +	.attach = lvds_encoder_attach,
> +};
> +
> +static int lvds_encoder_probe(struct platform_device *pdev)
> +{
> +	struct lvds_encoder *lvds;
> +	struct display_timing timing;
> +	struct device_node *port;
> +	struct device_node *endpoint;
> +	struct device_node *panel;
> +	int ret;
> +
> +	lvds = devm_kzalloc(&pdev->dev, sizeof(*lvds), GFP_KERNEL);
> +	if (!lvds)
> +		return -ENOMEM;
> +
> +	lvds->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, lvds);
> +
> +	lvds->bridge.funcs = &lvds_encoder_bridge_funcs;
> +	lvds->bridge.of_node = pdev->dev.of_node;
> +	lvds->bridge.encoder_type = DRM_MODE_ENCODER_LVDS;
> +
> +	/* Locate the panel DT node. */
> +	port = of_graph_get_port_by_id(pdev->dev.of_node, 1);
> +	if (!port) {
> +		dev_dbg(&pdev->dev, "port 1 not found\n");
> +		return -ENXIO;
> +	}
> +
> +	endpoint = of_get_child_by_name(port, "endpoint");
> +	of_node_put(port);
> +	if (!endpoint) {
> +		dev_dbg(&pdev->dev, "no endpoint for port 1\n");
> +		return -ENXIO;
> +	}
> +
> +	panel = of_graph_get_remote_port_parent(endpoint);
> +	of_node_put(endpoint);
> +	if (!panel) {
> +		dev_dbg(&pdev->dev, "no remote endpoint for port 1\n");
> +		return -ENXIO;
> +	}
> +
> +	/* Parse and store panel information. */
> +	ret = of_get_display_timing(panel, "panel-timing", &timing);
> +	of_node_put(panel);
> +	if (ret < 0) {
> +		dev_dbg(&pdev->dev, "unable to parse panel timings\n");
> +		return ret;
> +	}
> +
> +	videomode_from_timing(&timing, &lvds->panel.mode);
> +
> +	of_property_read_u32(panel, "width-mm", &lvds->panel.width_mm);
> +	of_property_read_u32(panel, "height-mm", &lvds->panel.height_mm);
> +
> +	/* Register the bridge. */
> +	return drm_bridge_add(&lvds->bridge);
> +}
> +
> +static int lvds_encoder_remove(struct platform_device *pdev)
> +{
> +	struct lvds_encoder *encoder = platform_get_drvdata(pdev);
> +
> +	drm_bridge_remove(&encoder->bridge);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id lvds_encoder_match[] = {
> +	{ .compatible = "lvds-encoder" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, lvds_encoder_match);
> +
> +struct platform_driver lvds_encoder_driver = {
> +	.probe	= lvds_encoder_probe,
> +	.remove	= lvds_encoder_remove,
> +	.driver		= {
> +		.name		= "lvds-encoder",
> +		.of_match_table	= lvds_encoder_match,
> +	},
> +};
> +module_platform_driver(lvds_encoder_driver);
> +
> +MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
> +MODULE_DESCRIPTION("Transparent parallel to LVDS encoder");
> +MODULE_LICENSE("GPL");
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/8] drm: bridge: Add LVDS encoder driver

[Laurent Pinchart]

Hi Archit,

On Friday 21 Oct 2016 10:51:59 Archit Taneja wrote:
> On 10/19/2016 07:55 PM, Laurent Pinchart wrote:
> > The LVDS encoder driver is a DRM bridge driver that supports the
> > parallel to LVDS encoders that don't require any configuration. The
> > driver thus doesn't interact with the device, but creates an LVDS
> > connector for the panel and exposes its size and timing based on
> > information retrieved from DT.
> > 
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  .../bindings/display/bridge/lvds-transmitter.txt   |  64 +++++++
> >  drivers/gpu/drm/bridge/Kconfig                     |   8 +
> >  drivers/gpu/drm/bridge/Makefile                    |   1 +
> >  drivers/gpu/drm/bridge/lvds-encoder.c              | 203 ++++++++++++++++
> >  4 files changed, 276 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> >  create mode 100644 drivers/gpu/drm/bridge/lvds-encoder.c
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> > b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> > new file mode 100644
> > index 000000000000..fd39ad34c383
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> > @@ -0,0 +1,64 @@
> > +Parallel to LVDS Encoder
> > +------------------------
> > +
> > +This binding supports the parallel to LVDS encoders that don't require
> > any
> > +configuration.
> > +
> > +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> > Multiple +incompatible data link layers have been used over time to
> > transmit image data +to LVDS panels. This binding targets devices
> > compatible with the following +specifications only.
> > +
> > +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> > February
> > +1999 (Version 1.0), Japan Electronic Industry Development Association
> > (JEIDA) +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95),
> > National +Semiconductor
> > +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
> > +Electronics Standards Association (VESA)
> > +
> > +Those devices have been marketed under the FPD-Link and FlatLink brand
> > names +among others.
> > +
> > +
> > +Required properties:
> > +
> > +- compatible: Must be "lvds-encoder"
> > +
> > +Required nodes:
> > +
> > +This device has two video ports. Their connections are modeled using the
> > OF +graph bindings specified in
> > Documentation/devicetree/bindings/graph.txt. +
> > +- Video port 0 for parallel input
> > +- Video port 1 for LVDS output
> > +
> > +
> > +Example
> > +-------
> > +
> > +lvds-encoder {
> > +	compatible = "lvds-encoder";
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +
> > +	ports {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		port@0 {
> > +			reg = <0>;
> > +
> > +			lvds_enc_in: endpoint {
> > +				remote-endpoint = <&display_out_rgb>;
> > +			};
> > +		};
> > +
> > +		port@1 {
> > +			reg = <1>;
> > +
> > +			lvds_enc_out: endpoint {
> > +				remote-endpoint = <&lvds_panel_in>;
> > +			};
> > +		};
> > +	};
> > +};
> > diff --git a/drivers/gpu/drm/bridge/Kconfig
> > b/drivers/gpu/drm/bridge/Kconfig index 10e12e74fc9f..5dafad7817ad 100644
> > --- a/drivers/gpu/drm/bridge/Kconfig
> > +++ b/drivers/gpu/drm/bridge/Kconfig
> > @@ -24,6 +24,14 @@ config DRM_DUMB_VGA_DAC
> > 
> >  	help
> >  	
> >  	  Support for RGB to VGA DAC based bridges
> > 
> > +config DRM_LVDS_ENCODER
> > +	tristate "Transparent parallel to LVDS encoder support"
> > +	depends on OF
> > +	select DRM_KMS_HELPER
> > +	help
> > +	  Support for transparent parallel to LVDS encoders that don't require
> > +	  any configuration.
> > +
> > 
> >  config DRM_DW_HDMI
> >  
> >  	tristate
> >  	select DRM_KMS_HELPER
> > 
> > diff --git a/drivers/gpu/drm/bridge/Makefile
> > b/drivers/gpu/drm/bridge/Makefile index cdf3a3cf765d..bbaf583581ac 100644
> > --- a/drivers/gpu/drm/bridge/Makefile
> > +++ b/drivers/gpu/drm/bridge/Makefile
> > @@ -2,6 +2,7 @@ ccflags-y := -Iinclude/drm
> > 
> >  obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> >  obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
> > 
> > +obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
> > 
> >  obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
> >  obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
> >  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> > 
> > diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c
> > b/drivers/gpu/drm/bridge/lvds-encoder.c new file mode 100644
> > index 000000000000..33e8025c8a6d
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/lvds-encoder.c
> > @@ -0,0 +1,203 @@
> > +/*
> > + * Copyright (C) 2016 Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of
> > + * the License, or (at your option) any later version.
> > + */
> > +
> > +#include <drm/drmP.h>
> > +#include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_connector.h>
> > +#include <drm/drm_crtc_helper.h>
> > +#include <drm/drm_encoder.h>
> > +#include <drm/drm_modeset_helper_vtables.h>
> > +
> > +#include <linux/of_graph.h>
> > +
> > +#include <video/display_timing.h>
> > +#include <video/of_display_timing.h>
> > +#include <video/videomode.h>
> > +
> > +struct lvds_encoder {
> > +	struct device *dev;
> > +
> > +	struct drm_bridge bridge;
> > +	struct drm_connector connector;
> > +
> > +	struct {
> > +		unsigned int width_mm;
> > +		unsigned int height_mm;
> > +		struct videomode mode;
> > +	} panel;
> 
> I don't see why we need create our own panel instance
> in the encoder driver. We should use the drm_panel
> API here rather than managing panel specific stuff in
> this driver.

It's not a panel instance but a structure that holds information about the 
connected panel. This is temporary until the LVDS panel DT bindings I've 
submitted get accepted.

> The parade-ps8622 and nxp-ptn3460 bridge drivers already
> do this.
> 
> Thanks,
> Archit
> 
> > +};
> > +
> > +static inline struct lvds_encoder *
> > +drm_bridge_to_lvds_encoder(struct drm_bridge *bridge)
> > +{
> > +	return container_of(bridge, struct lvds_encoder, bridge);
> > +}
> > +
> > +static inline struct lvds_encoder *
> > +drm_connector_to_lvds_encoder(struct drm_connector *connector)
> > +{
> > +	return container_of(connector, struct lvds_encoder, connector);
> > +}
> > +
> > +static int lvds_connector_get_modes(struct drm_connector *connector)
> > +{
> > +	struct lvds_encoder *lvds = drm_connector_to_lvds_encoder(connector);
> > +	struct drm_display_mode *mode;
> > +
> > +	mode = drm_mode_create(connector->dev);
> > +	if (mode == NULL)
> > +		return 0;
> > +
> > +	mode->type = DRM_MODE_TYPE_PREFERRED | DRM_MODE_TYPE_DRIVER;
> > +
> > +	drm_display_mode_from_videomode(&lvds->panel.mode, mode);
> > +
> > +	drm_mode_probed_add(connector, mode);
> > +
> > +	return 1;
> > +}
> > +
> > +static const struct drm_connector_helper_funcs
> > lvds_connector_helper_funcs = { +	.get_modes = lvds_connector_get_modes,
> > +};
> > +
> > +static enum drm_connector_status
> > +lvds_connector_detect(struct drm_connector *connector, bool force)
> > +{
> > +	return connector_status_connected;
> > +}
> > +
> > +static const struct drm_connector_funcs lvds_connector_funcs = {
> > +	.dpms = drm_atomic_helper_connector_dpms,
> > +	.reset = drm_atomic_helper_connector_reset,
> > +	.detect = lvds_connector_detect,
> > +	.fill_modes = drm_helper_probe_single_connector_modes,
> > +	.destroy = drm_connector_cleanup,
> > +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> > +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> > +};
> > +
> > +static int lvds_encoder_attach(struct drm_bridge *bridge)
> > +{
> > +	struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
> > +	struct drm_connector *connector = &lvds->connector;
> > +	int ret;
> > +
> > +	if (!bridge->encoder) {
> > +		DRM_ERROR("Missing encoder\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	connector->display_info.width_mm = lvds->panel.width_mm;
> > +	connector->display_info.height_mm = lvds->panel.height_mm;
> > +
> > +	drm_connector_helper_add(connector, &lvds_connector_helper_funcs);
> > +
> > +	ret = drm_connector_init(bridge->dev, connector, 
&lvds_connector_funcs,
> > +				 DRM_MODE_CONNECTOR_LVDS);
> > +	if (ret) {
> > +		DRM_ERROR("Failed to initialize connector\n");
> > +		return ret;
> > +	}
> > +
> > +	drm_mode_connector_attach_encoder(&lvds->connector, bridge->encoder);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct drm_bridge_funcs lvds_encoder_bridge_funcs = {
> > +	.attach = lvds_encoder_attach,
> > +};
> > +
> > +static int lvds_encoder_probe(struct platform_device *pdev)
> > +{
> > +	struct lvds_encoder *lvds;
> > +	struct display_timing timing;
> > +	struct device_node *port;
> > +	struct device_node *endpoint;
> > +	struct device_node *panel;
> > +	int ret;
> > +
> > +	lvds = devm_kzalloc(&pdev->dev, sizeof(*lvds), GFP_KERNEL);
> > +	if (!lvds)
> > +		return -ENOMEM;
> > +
> > +	lvds->dev = &pdev->dev;
> > +	platform_set_drvdata(pdev, lvds);
> > +
> > +	lvds->bridge.funcs = &lvds_encoder_bridge_funcs;
> > +	lvds->bridge.of_node = pdev->dev.of_node;
> > +	lvds->bridge.encoder_type = DRM_MODE_ENCODER_LVDS;
> > +
> > +	/* Locate the panel DT node. */
> > +	port = of_graph_get_port_by_id(pdev->dev.of_node, 1);
> > +	if (!port) {
> > +		dev_dbg(&pdev->dev, "port 1 not found\n");
> > +		return -ENXIO;
> > +	}
> > +
> > +	endpoint = of_get_child_by_name(port, "endpoint");
> > +	of_node_put(port);
> > +	if (!endpoint) {
> > +		dev_dbg(&pdev->dev, "no endpoint for port 1\n");
> > +		return -ENXIO;
> > +	}
> > +
> > +	panel = of_graph_get_remote_port_parent(endpoint);
> > +	of_node_put(endpoint);
> > +	if (!panel) {
> > +		dev_dbg(&pdev->dev, "no remote endpoint for port 1\n");
> > +		return -ENXIO;
> > +	}
> > +
> > +	/* Parse and store panel information. */
> > +	ret = of_get_display_timing(panel, "panel-timing", &timing);
> > +	of_node_put(panel);
> > +	if (ret < 0) {
> > +		dev_dbg(&pdev->dev, "unable to parse panel timings\n");
> > +		return ret;
> > +	}
> > +
> > +	videomode_from_timing(&timing, &lvds->panel.mode);
> > +
> > +	of_property_read_u32(panel, "width-mm", &lvds->panel.width_mm);
> > +	of_property_read_u32(panel, "height-mm", &lvds->panel.height_mm);
> > +
> > +	/* Register the bridge. */
> > +	return drm_bridge_add(&lvds->bridge);
> > +}
> > +
> > +static int lvds_encoder_remove(struct platform_device *pdev)
> > +{
> > +	struct lvds_encoder *encoder = platform_get_drvdata(pdev);
> > +
> > +	drm_bridge_remove(&encoder->bridge);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id lvds_encoder_match[] = {
> > +	{ .compatible = "lvds-encoder" },
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, lvds_encoder_match);
> > +
> > +struct platform_driver lvds_encoder_driver = {
> > +	.probe	= lvds_encoder_probe,
> > +	.remove	= lvds_encoder_remove,
> > +	.driver		= {
> > +		.name		= "lvds-encoder",
> > +		.of_match_table	= lvds_encoder_match,
> > +	},
> > +};
> > +module_platform_driver(lvds_encoder_driver);
> > +
> > +MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
> > +MODULE_DESCRIPTION("Transparent parallel to LVDS encoder");
> > +MODULE_LICENSE("GPL");

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/8] drm: bridge: Add LVDS encoder driver

[Rob Herring]

On Wed, Oct 19, 2016 at 05:25:36PM +0300, Laurent Pinchart wrote:
> The LVDS encoder driver is a DRM bridge driver that supports the
> parallel to LVDS encoders that don't require any configuration. The
> driver thus doesn't interact with the device, but creates an LVDS
> connector for the panel and exposes its size and timing based on
> information retrieved from DT.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../bindings/display/bridge/lvds-transmitter.txt   |  64 +++++++

It's preferred that bindings are separate patches. That makes the 
filtered DT only repo commits more sane.

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/gpu/drm/bridge/Kconfig                     |   8 +
>  drivers/gpu/drm/bridge/Makefile                    |   1 +
>  drivers/gpu/drm/bridge/lvds-encoder.c              | 203 +++++++++++++++++++++
>  4 files changed, 276 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
>  create mode 100644 drivers/gpu/drm/bridge/lvds-encoder.c
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/8] drm: bridge: vga-dac: Add adi,adv7123 compatible string

[Laurent Pinchart]

The ADV7123 is a transparent VGA DAC. Unlike dumb VGA DACs it can be
controlled through a power save pin, and requires a power supply.
However, on most boards where the device is used neither the power save
signal nor the power supply are controllable.

To avoid developing a separate device-specific driver add an
"adi,adv7123" compatible entry to the dumb-vga-dac driver. This will
allow supporting most ADV7123-based boards easily, while allowing future
development of an adv7123 driver when needed without breaking backward
compatibility.

Cc: devicetree@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index afec232185a7..b33e3f829e4f 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -204,6 +204,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
 
 static const struct of_device_id dumb_vga_match[] = {
 	{ .compatible = "dumb-vga-dac" },
+	{ .compatible = "adi,adv7123" },
 	{},
 };
 MODULE_DEVICE_TABLE(of, dumb_vga_match);
-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/8] drm: bridge: vga-dac: Add adi,adv7123 compatible string

[Archit Taneja]

On 10/19/2016 07:55 PM, Laurent Pinchart wrote:
> The ADV7123 is a transparent VGA DAC. Unlike dumb VGA DACs it can be
> controlled through a power save pin, and requires a power supply.
> However, on most boards where the device is used neither the power save
> signal nor the power supply are controllable.
>
> To avoid developing a separate device-specific driver add an
> "adi,adv7123" compatible entry to the dumb-vga-dac driver. This will
> allow supporting most ADV7123-based boards easily, while allowing future
> development of an adv7123 driver when needed without breaking backward
> compatibility.

Shouldn't we have a DT binding doc for ADV7123, even if it's sharing
the dumb vga driver for now?

Same query for the Thine LVDS encoder.

Thanks,
Archit

>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index afec232185a7..b33e3f829e4f 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -204,6 +204,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
>
>  static const struct of_device_id dumb_vga_match[] = {
>  	{ .compatible = "dumb-vga-dac" },
> +	{ .compatible = "adi,adv7123" },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, dumb_vga_match);
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/8] drm: bridge: vga-dac: Add adi,adv7123 compatible string

[Laurent Pinchart]

Hi Archit,

On Friday 21 Oct 2016 10:43:34 Archit Taneja wrote:
> On 10/19/2016 07:55 PM, Laurent Pinchart wrote:
> > The ADV7123 is a transparent VGA DAC. Unlike dumb VGA DACs it can be
> > controlled through a power save pin, and requires a power supply.
> > However, on most boards where the device is used neither the power save
> > signal nor the power supply are controllable.
> > 
> > To avoid developing a separate device-specific driver add an
> > "adi,adv7123" compatible entry to the dumb-vga-dac driver. This will
> > allow supporting most ADV7123-based boards easily, while allowing future
> > development of an adv7123 driver when needed without breaking backward
> > compatibility.
> 
> Shouldn't we have a DT binding doc for ADV7123, even if it's sharing
> the dumb vga driver for now?

Documentation/devicetree/bindings/display/bridge/adi,adv7123.txt

> Same query for the Thine LVDS encoder.

Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt

> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > b/drivers/gpu/drm/bridge/dumb-vga-dac.c index afec232185a7..b33e3f829e4f
> > 100644
> > --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > @@ -204,6 +204,7 @@ static int dumb_vga_remove(struct platform_device
> > *pdev)> 
> >  static const struct of_device_id dumb_vga_match[] = {
> >  
> >  	{ .compatible = "dumb-vga-dac" },
> > +	{ .compatible = "adi,adv7123" },
> >  	{},
> >  };
> >  MODULE_DEVICE_TABLE(of, dumb_vga_match);

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/8] drm: bridge: vga-dac: Add adi,adv7123 compatible string

[Archit Taneja]

On 10/21/2016 03:52 PM, Laurent Pinchart wrote:
> Hi Archit,
>
> On Friday 21 Oct 2016 10:43:34 Archit Taneja wrote:
>> On 10/19/2016 07:55 PM, Laurent Pinchart wrote:
>>> The ADV7123 is a transparent VGA DAC. Unlike dumb VGA DACs it can be
>>> controlled through a power save pin, and requires a power supply.
>>> However, on most boards where the device is used neither the power save
>>> signal nor the power supply are controllable.
>>>
>>> To avoid developing a separate device-specific driver add an
>>> "adi,adv7123" compatible entry to the dumb-vga-dac driver. This will
>>> allow supporting most ADV7123-based boards easily, while allowing future
>>> development of an adv7123 driver when needed without breaking backward
>>> compatibility.
>>
>> Shouldn't we have a DT binding doc for ADV7123, even if it's sharing
>> the dumb vga driver for now?
>
> Documentation/devicetree/bindings/display/bridge/adi,adv7123.txt
>
>> Same query for the Thine LVDS encoder.
>
> Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt

Cool, didn't know these already existed :)

Thanks,
Archit

>
>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> Signed-off-by: Laurent Pinchart
>>> <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
>>> ---
>>>
>>>  drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index afec232185a7..b33e3f829e4f
>>> 100644
>>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> @@ -204,6 +204,7 @@ static int dumb_vga_remove(struct platform_device
>>> *pdev)>
>>>  static const struct of_device_id dumb_vga_match[] = {
>>>
>>>  	{ .compatible = "dumb-vga-dac" },
>>> +	{ .compatible = "adi,adv7123" },
>>>  	{},
>>>  };
>>>  MODULE_DEVICE_TABLE(of, dumb_vga_match);
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/8] drm: bridge: lvds-encoder: Add thine,thc63lvdm83d compatible string

[Laurent Pinchart]

The THC63LVDM83D is a transparent LVDS encoder. Unlike dumb LVDS
encoders it can be controlled through a few pins (power down, LVDS
swing, clock edge selection) and requires power supplies. However, on
several boards where the device is used neither the control pins nor the
power supply are controllable.

To avoid developing a separate device-specific driver add a
"thine,thc63lvdm83d" compatible entry to the lvds-encoder driver. This
will allow supporting many THC63LVDM83D-based boards easily, while
allowing future development of an thc63lvdm83d driver when needed
without breaking backward compatibility.

Cc: devicetree@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/bridge/lvds-encoder.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c
index 33e8025c8a6d..f3026ad82fe1 100644
--- a/drivers/gpu/drm/bridge/lvds-encoder.c
+++ b/drivers/gpu/drm/bridge/lvds-encoder.c
@@ -184,6 +184,7 @@ static int lvds_encoder_remove(struct platform_device *pdev)
 
 static const struct of_device_id lvds_encoder_match[] = {
 	{ .compatible = "lvds-encoder" },
+	{ .compatible = "thine,thc63lvdm83d" },
 	{},
 };
 MODULE_DEVICE_TABLE(of, lvds_encoder_match);
-- 
Regards,

Laurent Pinchart