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 at vger.kernel.org
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas at 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 at 0 {
> > + reg = <0>;
> > +
> > + lvds_enc_in: endpoint {
> > + remote-endpoint = <&display_out_rgb>;
> > + };
> > + };
> > +
> > + port at 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 at 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 at ideasonboard.com>");
> > +MODULE_DESCRIPTION("Transparent parallel to LVDS encoder");
> > +MODULE_LICENSE("GPL");
--
Regards,
Laurent Pinchart
