RE: [PATCH v2 02/10] drm: rcar-du: Add encoder lib support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux