Hi Laurent, Gentle ping. Am I going in the right direction with this patch series? I know I need to rebase these patches. Please review these patches and provide some direction to move forward. Cheers, Biju > Subject: [PATCH v2 02/10] drm: rcar-du: Add encoder lib support > > Add RCar DU encoder lib support by moving > rcar_du_encoder_count_ports() and rcar_du_encoder_funcs to the lib > file and added > rcar_du_encoder_funcs() to share the common code between RCar and > RZ/G2L DU encoder drivers. > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > --- > v1->v2: > * Rebased on drm-misc-next and DU-next. > * Fixed the warning reported by bot. > --- > drivers/gpu/drm/rcar-du/Kconfig | 5 + > drivers/gpu/drm/rcar-du/Makefile | 2 + > drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 117 +-------------- > drivers/gpu/drm/rcar-du/rcar_du_encoder.h | 14 +- > drivers/gpu/drm/rcar-du/rcar_du_encoder_lib.c | 138 > ++++++++++++++++++ drivers/gpu/drm/rcar-du/rcar_du_encoder_lib.h | > 30 ++++ > 6 files changed, 180 insertions(+), 126 deletions(-) create mode > 100644 drivers/gpu/drm/rcar-du/rcar_du_encoder_lib.c > create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_encoder_lib.h > > diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar- > du/Kconfig index 58ffb8c2443b..369af45b5ff9 100644 > --- a/drivers/gpu/drm/rcar-du/Kconfig > +++ b/drivers/gpu/drm/rcar-du/Kconfig > @@ -71,3 +71,8 @@ config DRM_RCAR_WRITEBACK > bool > default y if ARM64 > depends on DRM_RCAR_DU > + > +config DRM_RCAR_LIB > + bool > + default y > + depends on DRM_RCAR_DU > diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar- > du/Makefile > index b8f2c82651d9..479c8eebba5a 100644 > --- a/drivers/gpu/drm/rcar-du/Makefile > +++ b/drivers/gpu/drm/rcar-du/Makefile > @@ -6,6 +6,8 @@ rcar-du-drm-y := rcar_du_crtc.o \ > rcar_du_kms.o \ > rcar_du_plane.o \ > > +rcar-du-drm-$(CONFIG_DRM_RCAR_LIB) += rcar_du_encoder_lib.o > + > rcar-du-drm-$(CONFIG_DRM_RCAR_VSP) += rcar_du_vsp.o > rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c > b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c > index b1787be31e92..444cc956f692 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c > @@ -2,7 +2,7 @@ > /* > * R-Car Display Unit Encoder > * > - * Copyright (C) 2013-2014 Renesas Electronics Corporation > + * Copyright (C) 2013-2022 Renesas Electronics Corporation > * > * Contact: Laurent Pinchart (laurent.pinchart@xxxxxxxxxxxxxxxx) > */ > @@ -10,128 +10,17 @@ > #include <linux/export.h> > #include <linux/of.h> > > -#include <drm/drm_bridge.h> > -#include <drm/drm_bridge_connector.h> > -#include <drm/drm_panel.h> > - > #include "rcar_du_drv.h" > #include "rcar_du_encoder.h" > -#include "rcar_lvds.h" > > /* ------------------------------------------------------------------ > ----------- > * Encoder > */ > > -static unsigned int rcar_du_encoder_count_ports(struct device_node > *node) -{ > - struct device_node *ports; > - struct device_node *port; > - unsigned int num_ports = 0; > - > - ports = of_get_child_by_name(node, "ports"); > - if (!ports) > - ports = of_node_get(node); > - > - for_each_child_of_node(ports, port) { > - if (of_node_name_eq(port, "port")) > - num_ports++; > - } > - > - of_node_put(ports); > - > - return num_ports; > -} > - > -static const struct drm_encoder_funcs rcar_du_encoder_funcs = { -}; > - > int rcar_du_encoder_init(struct rcar_du_device *rcdu, > enum rcar_du_output output, > struct device_node *enc_node) > { > - struct rcar_du_encoder *renc; > - struct drm_connector *connector; > - struct drm_bridge *bridge; > - int ret; > - > - /* > - * Locate the DRM bridge from the DT node. For the DPAD outputs, > if the > - * DT node has a single port, assume that it describes a panel > and > - * create a panel bridge. > - */ > - if ((output == RCAR_DU_OUTPUT_DPAD0 || > - output == RCAR_DU_OUTPUT_DPAD1) && > - rcar_du_encoder_count_ports(enc_node) == 1) { > - struct drm_panel *panel = of_drm_find_panel(enc_node); > - > - if (IS_ERR(panel)) > - return PTR_ERR(panel); > - > - bridge = devm_drm_panel_bridge_add_typed(rcdu->dev, panel, > - DRM_MODE_CONNECTOR_DPI); > - if (IS_ERR(bridge)) > - return PTR_ERR(bridge); > - } else { > - bridge = of_drm_find_bridge(enc_node); > - if (!bridge) > - return -EPROBE_DEFER; > - > - if (output == RCAR_DU_OUTPUT_LVDS0 || > - output == RCAR_DU_OUTPUT_LVDS1) > - rcdu->lvds[output - RCAR_DU_OUTPUT_LVDS0] = bridge; > - > - if (output == RCAR_DU_OUTPUT_DSI0 || > - output == RCAR_DU_OUTPUT_DSI1) > - rcdu->dsi[output - RCAR_DU_OUTPUT_DSI0] = bridge; > - } > - > - /* > - * Create and initialize the encoder. On Gen3, skip the LVDS1 > output if > - * the LVDS1 encoder is used as a companion for LVDS0 in dual- > link > - * mode, or any LVDS output if it isn't connected. The latter may > happen > - * on D3 or E3 as the LVDS encoders are needed to provide the > pixel > - * clock to the DU, even when the LVDS outputs are not used. > - */ > - if (rcdu->info->gen >= 3) { > - if (output == RCAR_DU_OUTPUT_LVDS1 && > - rcar_lvds_dual_link(bridge)) > - return -ENOLINK; > - > - if ((output == RCAR_DU_OUTPUT_LVDS0 || > - output == RCAR_DU_OUTPUT_LVDS1) && > - !rcar_lvds_is_connected(bridge)) > - return -ENOLINK; > - } > - > - dev_dbg(rcdu->dev, "initializing encoder %pOF for output %s\n", > - enc_node, rcar_du_output_name(output)); > - > - renc = drmm_encoder_alloc(&rcdu->ddev, struct rcar_du_encoder, > base, > - &rcar_du_encoder_funcs, > DRM_MODE_ENCODER_NONE, > - NULL); > - if (!renc) > - return -ENOMEM; > - > - renc->output = output; > - > - /* Attach the bridge to the encoder. */ > - ret = drm_bridge_attach(&renc->base, bridge, NULL, > - DRM_BRIDGE_ATTACH_NO_CONNECTOR); > - if (ret) { > - dev_err(rcdu->dev, > - "failed to attach bridge %pOF for output %s (%d)\n", > - bridge->of_node, rcar_du_output_name(output), ret); > - return ret; > - } > - > - /* Create the connector for the chain of bridges. */ > - connector = drm_bridge_connector_init(&rcdu->ddev, &renc->base); > - if (IS_ERR(connector)) { > - dev_err(rcdu->dev, > - "failed to created connector for output %s (%ld)\n", > - rcar_du_output_name(output), PTR_ERR(connector)); > - return PTR_ERR(connector); > - } > - > - return drm_connector_attach_encoder(connector, &renc->base); > + return rcar_du_lib_encoder_init(rcdu, output, enc_node, > + rcar_du_output_name(output)); > } > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.h > b/drivers/gpu/drm/rcar-du/rcar_du_encoder.h > index e5ec8fbb3979..d33b684fe93f 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.h > +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.h > @@ -2,7 +2,7 @@ > /* > * R-Car Display Unit Encoder > * > - * Copyright (C) 2013-2014 Renesas Electronics Corporation > + * Copyright (C) 2013-2022 Renesas Electronics Corporation > * > * Contact: Laurent Pinchart (laurent.pinchart@xxxxxxxxxxxxxxxx) > */ > @@ -10,17 +10,7 @@ > #ifndef __RCAR_DU_ENCODER_H__ > #define __RCAR_DU_ENCODER_H__ > > -#include <drm/drm_encoder.h> > - > -struct rcar_du_device; > - > -struct rcar_du_encoder { > - struct drm_encoder base; > - enum rcar_du_output output; > -}; > - > -#define to_rcar_encoder(e) \ > - container_of(e, struct rcar_du_encoder, base) > +#include "rcar_du_encoder_lib.h" > > int rcar_du_encoder_init(struct rcar_du_device *rcdu, > enum rcar_du_output output, > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder_lib.c > b/drivers/gpu/drm/rcar-du/rcar_du_encoder_lib.c > new file mode 100644 > index 000000000000..534d357cfbe2 > --- /dev/null > +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder_lib.c > @@ -0,0 +1,138 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * R-Car Display Unit Encoder Lib > + * > + * Copyright (C) 2013-2022 Renesas Electronics Corporation > + * > + * Contact: Laurent Pinchart (laurent.pinchart@xxxxxxxxxxxxxxxx) > + */ > + > +#include <linux/export.h> > +#include <linux/of.h> > + > +#include <drm/drm_bridge.h> > +#include <drm/drm_bridge_connector.h> > +#include <drm/drm_panel.h> > + > +#include "rcar_du_drv.h" > +#include "rcar_du_encoder.h" > +#include "rcar_lvds.h" > + > +/* > +--------------------------------------------------------------------- > -- > +------ > + * Encoder > + */ > + > +static unsigned int rcar_du_encoder_count_ports(struct device_node > +*node) { > + struct device_node *ports; > + struct device_node *port; > + unsigned int num_ports = 0; > + > + ports = of_get_child_by_name(node, "ports"); > + if (!ports) > + ports = of_node_get(node); > + > + for_each_child_of_node(ports, port) { > + if (of_node_name_eq(port, "port")) > + num_ports++; > + } > + > + of_node_put(ports); > + > + return num_ports; > +} > + > +static const struct drm_encoder_funcs rcar_du_encoder_funcs = { }; > + > +int rcar_du_lib_encoder_init(struct rcar_du_device *rcdu, > + enum rcar_du_output output, > + struct device_node *enc_node, > + const char *output_name) > +{ > + struct rcar_du_encoder *renc; > + struct drm_connector *connector; > + struct drm_bridge *bridge; > + int ret; > + > + /* > + * Locate the DRM bridge from the DT node. For the DPAD outputs, > if the > + * DT node has a single port, assume that it describes a panel > and > + * create a panel bridge. > + */ > + if ((output == RCAR_DU_OUTPUT_DPAD0 || > + output == RCAR_DU_OUTPUT_DPAD1) && > + rcar_du_encoder_count_ports(enc_node) == 1) { > + struct drm_panel *panel = of_drm_find_panel(enc_node); > + > + if (IS_ERR(panel)) > + return PTR_ERR(panel); > + > + bridge = devm_drm_panel_bridge_add_typed(rcdu->dev, panel, > + DRM_MODE_CONNECTOR_DPI); > + if (IS_ERR(bridge)) > + return PTR_ERR(bridge); > + } else { > + bridge = of_drm_find_bridge(enc_node); > + if (!bridge) > + return -EPROBE_DEFER; > + > + if (output == RCAR_DU_OUTPUT_LVDS0 || > + output == RCAR_DU_OUTPUT_LVDS1) > + rcdu->lvds[output - RCAR_DU_OUTPUT_LVDS0] = bridge; > + > + if (output == RCAR_DU_OUTPUT_DSI0 || > + output == RCAR_DU_OUTPUT_DSI1) > + rcdu->dsi[output - RCAR_DU_OUTPUT_DSI0] = bridge; > + } > + > + /* > + * Create and initialize the encoder. On Gen3, skip the LVDS1 > output if > + * the LVDS1 encoder is used as a companion for LVDS0 in dual- > link > + * mode, or any LVDS output if it isn't connected. The latter may > happen > + * on D3 or E3 as the LVDS encoders are needed to provide the > pixel > + * clock to the DU, even when the LVDS outputs are not used. > + */ > + if (rcdu->info->gen >= 3) { > + if (output == RCAR_DU_OUTPUT_LVDS1 && > + rcar_lvds_dual_link(bridge)) > + return -ENOLINK; > + > + if ((output == RCAR_DU_OUTPUT_LVDS0 || > + output == RCAR_DU_OUTPUT_LVDS1) && > + !rcar_lvds_is_connected(bridge)) > + return -ENOLINK; > + } > + > + dev_dbg(rcdu->dev, "initializing encoder %pOF for output %s\n", > + enc_node, rcar_du_output_name(output)); > + > + renc = drmm_encoder_alloc(&rcdu->ddev, struct rcar_du_encoder, > base, > + &rcar_du_encoder_funcs, > DRM_MODE_ENCODER_NONE, > + NULL); > + if (!renc) > + return -ENOMEM; > + > + renc->output = output; > + > + /* Attach the bridge to the encoder. */ > + ret = drm_bridge_attach(&renc->base, bridge, NULL, > + DRM_BRIDGE_ATTACH_NO_CONNECTOR); > + if (ret) { > + dev_err(rcdu->dev, > + "failed to attach bridge %pOF for output %s (%d)\n", > + bridge->of_node, output_name, ret); > + return ret; > + } > + > + /* Create the connector for the chain of bridges. */ > + connector = drm_bridge_connector_init(&rcdu->ddev, &renc->base); > + if (IS_ERR(connector)) { > + dev_err(rcdu->dev, > + "failed to created connector for output %s (%ld)\n", > + output_name, PTR_ERR(connector)); > + return PTR_ERR(connector); > + } > + > + return drm_connector_attach_encoder(connector, &renc->base); } > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder_lib.h > b/drivers/gpu/drm/rcar-du/rcar_du_encoder_lib.h > new file mode 100644 > index 000000000000..29fcc7cc12db > --- /dev/null > +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder_lib.h > @@ -0,0 +1,30 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * R-Car Display Unit Encoder Lib > + * > + * Copyright (C) 2013-2022 Renesas Electronics Corporation > + * > + * Contact: Laurent Pinchart (laurent.pinchart@xxxxxxxxxxxxxxxx) > + */ > + > +#ifndef __RCAR_DU_ENCODER_LIB_H__ > +#define __RCAR_DU_ENCODER_LIB_H__ > + > +#include <drm/drm_encoder.h> > + > +struct rcar_du_device; > + > +struct rcar_du_encoder { > + struct drm_encoder base; > + enum rcar_du_output output; > +}; > + > +#define to_rcar_encoder(e) \ > + container_of(e, struct rcar_du_encoder, base) > + > +int rcar_du_lib_encoder_init(struct rcar_du_device *rcdu, > + enum rcar_du_output output, > + struct device_node *enc_node, > + const char *output_name); > + > +#endif /* __RCAR_DU_ENCODER_LIB_H__ */ > -- > 2.25.1