2023-01-24 05:49:27

by Michael Riesch

[permalink] [raw]
Subject: [PATCH v3 0/6] drm/rockchip: vop2: add support for the rgb output block

Hi all,

This series adds support for the RGB output block that can be found in the
Rockchip Video Output Processor (VOP) 2. Version 2 of this series
incorporates the feedback by Dan Carpenter and Sascha Hauer. Version 3
fixes a dumb mistake pointed out by Sascha :-) Thanks for your comments!

Patches 1-4 clean up the code and make it more general.

Patch 5 activates the support for the RGB output block in the VOP2 driver.

Patch 6 adds pinctrls for the 16-bit and 18-bit RGB data lines.

Tested on a custom board featuring the RK3568 SoC with a 18-bit RGB
display.

Looking forward to your comments!

Best regards,
Michael

Michael Riesch (6):
drm/rockchip: vop2: initialize possible_crtcs properly
drm/rockchip: rgb: embed drm_encoder into rockchip_encoder
drm/rockchip: rgb: add video_port parameter to init function
drm/rockchip: vop2: use symmetric function pair
vop2_{create,destroy}_crtcs
drm/rockchip: vop2: add support for the rgb output block
arm64: dts: rockchip: add pinctrls for 16-bit/18-bit rgb interface to
rk356x

.../boot/dts/rockchip/rk3568-pinctrl.dtsi | 94 +++++++++++++++++++
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 80 ++++++++++++----
drivers/gpu/drm/rockchip/rockchip_rgb.c | 19 ++--
drivers/gpu/drm/rockchip/rockchip_rgb.h | 6 +-
5 files changed, 172 insertions(+), 29 deletions(-)


base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
--
2.30.2



2023-01-24 05:50:09

by Michael Riesch

[permalink] [raw]
Subject: [PATCH v3 2/6] drm/rockchip: rgb: embed drm_encoder into rockchip_encoder

Commit 540b8f271e53 ("drm/rockchip: Embed drm_encoder into
rockchip_decoder") provides the means to pass the endpoint ID to the
VOP2 driver, which sets the interface MUX accordingly. However, this
step has not yet been carried out for the RGB output block. Embed the
drm_encoder structure into the rockchip_encoder structure and set the
endpoint ID correctly.

Signed-off-by: Michael Riesch <[email protected]>
---
v3:
- no changes
v2:
- use endpoint id from device tree instead of hardcoded value

drivers/gpu/drm/rockchip/rockchip_rgb.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.c b/drivers/gpu/drm/rockchip/rockchip_rgb.c
index 75eb7cca3d82..5971df4302f2 100644
--- a/drivers/gpu/drm/rockchip/rockchip_rgb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
@@ -22,13 +22,11 @@
#include "rockchip_drm_vop.h"
#include "rockchip_rgb.h"

-#define encoder_to_rgb(c) container_of(c, struct rockchip_rgb, encoder)
-
struct rockchip_rgb {
struct device *dev;
struct drm_device *drm_dev;
struct drm_bridge *bridge;
- struct drm_encoder encoder;
+ struct rockchip_encoder encoder;
struct drm_connector connector;
int output_mode;
};
@@ -125,7 +123,7 @@ struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
return ERR_PTR(ret);
}

- encoder = &rgb->encoder;
+ encoder = &rgb->encoder.encoder;
encoder->possible_crtcs = drm_crtc_mask(crtc);

ret = drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_NONE);
@@ -161,6 +159,8 @@ struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
goto err_free_encoder;
}

+ rgb->encoder.crtc_endpoint_id = endpoint_id;
+
ret = drm_connector_attach_encoder(connector, encoder);
if (ret < 0) {
DRM_DEV_ERROR(drm_dev->dev,
@@ -182,6 +182,6 @@ void rockchip_rgb_fini(struct rockchip_rgb *rgb)
{
drm_panel_bridge_remove(rgb->bridge);
drm_connector_cleanup(&rgb->connector);
- drm_encoder_cleanup(&rgb->encoder);
+ drm_encoder_cleanup(&rgb->encoder.encoder);
}
EXPORT_SYMBOL_GPL(rockchip_rgb_fini);
--
2.30.2


2023-01-24 05:50:12

by Michael Riesch

[permalink] [raw]
Subject: [PATCH v3 6/6] arm64: dts: rockchip: add pinctrls for 16-bit/18-bit rgb interface to rk356x

The rk3568-pinctrl.dtsi only defines the 24-bit RGB interface. Add separate
nodes for the 16-bit and 18-bit version, respectively. While at it, split
off the clock/sync signals from the data signals.

The exact mapping of the data pins was discussed here:
https://lore.kernel.org/linux-rockchip/[email protected]/T/

Signed-off-by: Michael Riesch <[email protected]>
---
v3:
- no changes
v2:
- no changes

.../boot/dts/rockchip/rk3568-pinctrl.dtsi | 94 +++++++++++++++++++
1 file changed, 94 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-pinctrl.dtsi b/arch/arm64/boot/dts/rockchip/rk3568-pinctrl.dtsi
index 8f90c66dd9e9..0a979bfb63d9 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-pinctrl.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568-pinctrl.dtsi
@@ -3117,4 +3117,98 @@ tsadc_pin: tsadc-pin {
<0 RK_PA1 0 &pcfg_pull_none>;
};
};
+
+ lcdc {
+ /omit-if-no-ref/
+ lcdc_clock: lcdc-clock {
+ rockchip,pins =
+ /* lcdc_clk */
+ <3 RK_PA0 1 &pcfg_pull_none>,
+ /* lcdc_den */
+ <3 RK_PC3 1 &pcfg_pull_none>,
+ /* lcdc_hsync */
+ <3 RK_PC1 1 &pcfg_pull_none>,
+ /* lcdc_vsync */
+ <3 RK_PC2 1 &pcfg_pull_none>;
+ };
+
+ /omit-if-no-ref/
+ lcdc_data16: lcdc-data16 {
+ rockchip,pins =
+ /* lcdc_d3 */
+ <2 RK_PD3 1 &pcfg_pull_none>,
+ /* lcdc_d4 */
+ <2 RK_PD4 1 &pcfg_pull_none>,
+ /* lcdc_d5 */
+ <2 RK_PD5 1 &pcfg_pull_none>,
+ /* lcdc_d6 */
+ <2 RK_PD6 1 &pcfg_pull_none>,
+ /* lcdc_d7 */
+ <2 RK_PD7 1 &pcfg_pull_none>,
+ /* lcdc_d10 */
+ <3 RK_PA3 1 &pcfg_pull_none>,
+ /* lcdc_d11 */
+ <3 RK_PA4 1 &pcfg_pull_none>,
+ /* lcdc_d12 */
+ <3 RK_PA5 1 &pcfg_pull_none>,
+ /* lcdc_d13 */
+ <3 RK_PA6 1 &pcfg_pull_none>,
+ /* lcdc_d14 */
+ <3 RK_PA7 1 &pcfg_pull_none>,
+ /* lcdc_d15 */
+ <3 RK_PB0 1 &pcfg_pull_none>,
+ /* lcdc_d19 */
+ <3 RK_PB4 1 &pcfg_pull_none>,
+ /* lcdc_d20 */
+ <3 RK_PB5 1 &pcfg_pull_none>,
+ /* lcdc_d21 */
+ <3 RK_PB6 1 &pcfg_pull_none>,
+ /* lcdc_d22 */
+ <3 RK_PB7 1 &pcfg_pull_none>,
+ /* lcdc_d23 */
+ <3 RK_PC0 1 &pcfg_pull_none>;
+ };
+
+ /omit-if-no-ref/
+ lcdc_data18: lcdc-data18 {
+ rockchip,pins =
+ /* lcdc_d2 */
+ <2 RK_PD2 1 &pcfg_pull_none>,
+ /* lcdc_d3 */
+ <2 RK_PD3 1 &pcfg_pull_none>,
+ /* lcdc_d4 */
+ <2 RK_PD4 1 &pcfg_pull_none>,
+ /* lcdc_d5 */
+ <2 RK_PD5 1 &pcfg_pull_none>,
+ /* lcdc_d6 */
+ <2 RK_PD6 1 &pcfg_pull_none>,
+ /* lcdc_d7 */
+ <2 RK_PD7 1 &pcfg_pull_none>,
+ /* lcdc_d10 */
+ <3 RK_PA3 1 &pcfg_pull_none>,
+ /* lcdc_d11 */
+ <3 RK_PA4 1 &pcfg_pull_none>,
+ /* lcdc_d12 */
+ <3 RK_PA5 1 &pcfg_pull_none>,
+ /* lcdc_d13 */
+ <3 RK_PA6 1 &pcfg_pull_none>,
+ /* lcdc_d14 */
+ <3 RK_PA7 1 &pcfg_pull_none>,
+ /* lcdc_d15 */
+ <3 RK_PB0 1 &pcfg_pull_none>,
+ /* lcdc_d18 */
+ <3 RK_PB3 1 &pcfg_pull_none>,
+ /* lcdc_d19 */
+ <3 RK_PB4 1 &pcfg_pull_none>,
+ /* lcdc_d20 */
+ <3 RK_PB5 1 &pcfg_pull_none>,
+ /* lcdc_d21 */
+ <3 RK_PB6 1 &pcfg_pull_none>,
+ /* lcdc_d22 */
+ <3 RK_PB7 1 &pcfg_pull_none>,
+ /* lcdc_d23 */
+ <3 RK_PC0 1 &pcfg_pull_none>;
+ };
+ };
+
};
--
2.30.2


2023-01-24 05:50:14

by Michael Riesch

[permalink] [raw]
Subject: [PATCH v3 3/6] drm/rockchip: rgb: add video_port parameter to init function

The VOP2 driver has more than one video port, hence the hard-coded
port id will not work anymore. Add an extra parameter for the video
port id to the rockchip_rgb_init function.

Signed-off-by: Michael Riesch <[email protected]>
---
v3:
- no changes
v2:
- no changes

drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
drivers/gpu/drm/rockchip/rockchip_rgb.c | 9 +++++----
drivers/gpu/drm/rockchip/rockchip_rgb.h | 6 ++++--
3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index fa1f4ee6d195..5d18dea5c8d6 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -2221,7 +2221,7 @@ static int vop_bind(struct device *dev, struct device *master, void *data)
goto err_disable_pm_runtime;

if (vop->data->feature & VOP_FEATURE_INTERNAL_RGB) {
- vop->rgb = rockchip_rgb_init(dev, &vop->crtc, vop->drm_dev);
+ vop->rgb = rockchip_rgb_init(dev, &vop->crtc, vop->drm_dev, 0);
if (IS_ERR(vop->rgb)) {
ret = PTR_ERR(vop->rgb);
goto err_disable_pm_runtime;
diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.c b/drivers/gpu/drm/rockchip/rockchip_rgb.c
index 5971df4302f2..c677b71ae516 100644
--- a/drivers/gpu/drm/rockchip/rockchip_rgb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
@@ -72,7 +72,8 @@ struct drm_encoder_helper_funcs rockchip_rgb_encoder_helper_funcs = {

struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
struct drm_crtc *crtc,
- struct drm_device *drm_dev)
+ struct drm_device *drm_dev,
+ int video_port)
{
struct rockchip_rgb *rgb;
struct drm_encoder *encoder;
@@ -90,7 +91,7 @@ struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
rgb->dev = dev;
rgb->drm_dev = drm_dev;

- port = of_graph_get_port_by_id(dev->of_node, 0);
+ port = of_graph_get_port_by_id(dev->of_node, video_port);
if (!port)
return ERR_PTR(-EINVAL);

@@ -103,8 +104,8 @@ struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
continue;

child_count++;
- ret = drm_of_find_panel_or_bridge(dev->of_node, 0, endpoint_id,
- &panel, &bridge);
+ ret = drm_of_find_panel_or_bridge(dev->of_node, video_port,
+ endpoint_id, &panel, &bridge);
if (!ret) {
of_node_put(endpoint);
break;
diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.h b/drivers/gpu/drm/rockchip/rockchip_rgb.h
index 27b9635124bc..1bd4e20e91eb 100644
--- a/drivers/gpu/drm/rockchip/rockchip_rgb.h
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.h
@@ -8,12 +8,14 @@
#ifdef CONFIG_ROCKCHIP_RGB
struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
struct drm_crtc *crtc,
- struct drm_device *drm_dev);
+ struct drm_device *drm_dev,
+ int video_port);
void rockchip_rgb_fini(struct rockchip_rgb *rgb);
#else
static inline struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
struct drm_crtc *crtc,
- struct drm_device *drm_dev)
+ struct drm_device *drm_dev,
+ int video_port)
{
return NULL;
}
--
2.30.2


2023-01-24 05:50:44

by Michael Riesch

[permalink] [raw]
Subject: [PATCH v3 4/6] drm/rockchip: vop2: use symmetric function pair vop2_{create,destroy}_crtcs

Let the function name vop2_create_crtcs reflect that the function creates
multiple CRTCS. Also, use a symmetric function pair to create and destroy
the CRTCs and the corresponding planes.

Signed-off-by: Michael Riesch <[email protected]>
---
v3:
- no changes
v2:
- no changes

drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 31 ++++++++++----------
1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 374ef821b453..06fcdfa7b885 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -2246,7 +2246,7 @@ static struct vop2_video_port *find_vp_without_primary(struct vop2 *vop2)

#define NR_LAYERS 6

-static int vop2_create_crtc(struct vop2 *vop2)
+static int vop2_create_crtcs(struct vop2 *vop2)
{
const struct vop2_data *vop2_data = vop2->data;
struct drm_device *drm = vop2->drm;
@@ -2372,15 +2372,25 @@ static int vop2_create_crtc(struct vop2 *vop2)
return 0;
}

-static void vop2_destroy_crtc(struct drm_crtc *crtc)
+static void vop2_destroy_crtcs(struct vop2 *vop2)
{
- of_node_put(crtc->port);
+ struct drm_device *drm = vop2->drm;
+ struct list_head *crtc_list = &drm->mode_config.crtc_list;
+ struct list_head *plane_list = &drm->mode_config.plane_list;
+ struct drm_crtc *crtc, *tmpc;
+ struct drm_plane *plane, *tmpp;
+
+ list_for_each_entry_safe(plane, tmpp, plane_list, head)
+ drm_plane_cleanup(plane);

/*
* Destroy CRTC after vop2_plane_destroy() since vop2_disable_plane()
* references the CRTC.
*/
- drm_crtc_cleanup(crtc);
+ list_for_each_entry_safe(crtc, tmpc, crtc_list, head) {
+ of_node_put(crtc->port);
+ drm_crtc_cleanup(crtc);
+ }
}

static struct reg_field vop2_cluster_regs[VOP2_WIN_MAX_REG] = {
@@ -2684,7 +2694,7 @@ static int vop2_bind(struct device *dev, struct device *master, void *data)
if (ret)
return ret;

- ret = vop2_create_crtc(vop2);
+ ret = vop2_create_crtcs(vop2);
if (ret)
return ret;

@@ -2698,19 +2708,10 @@ static int vop2_bind(struct device *dev, struct device *master, void *data)
static void vop2_unbind(struct device *dev, struct device *master, void *data)
{
struct vop2 *vop2 = dev_get_drvdata(dev);
- struct drm_device *drm = vop2->drm;
- struct list_head *plane_list = &drm->mode_config.plane_list;
- struct list_head *crtc_list = &drm->mode_config.crtc_list;
- struct drm_crtc *crtc, *tmpc;
- struct drm_plane *plane, *tmpp;

pm_runtime_disable(dev);

- list_for_each_entry_safe(plane, tmpp, plane_list, head)
- drm_plane_cleanup(plane);
-
- list_for_each_entry_safe(crtc, tmpc, crtc_list, head)
- vop2_destroy_crtc(crtc);
+ vop2_destroy_crtcs(vop2);
}

const struct component_ops vop2_component_ops = {
--
2.30.2


2023-01-25 08:29:38

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] drm/rockchip: vop2: add support for the rgb output block

On Tue, Jan 24, 2023 at 06:47:00AM +0100, Michael Riesch wrote:
> Hi all,
>
> This series adds support for the RGB output block that can be found in the
> Rockchip Video Output Processor (VOP) 2. Version 2 of this series
> incorporates the feedback by Dan Carpenter and Sascha Hauer. Version 3
> fixes a dumb mistake pointed out by Sascha :-) Thanks for your comments!
>
> Patches 1-4 clean up the code and make it more general.
>
> Patch 5 activates the support for the RGB output block in the VOP2 driver.
>
> Patch 6 adds pinctrls for the 16-bit and 18-bit RGB data lines.
>
> Tested on a custom board featuring the RK3568 SoC with a 18-bit RGB
> display.
>
> Looking forward to your comments!

For the series:

Reviewed-by: Sascha Hauer <[email protected]>

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 |

2023-01-29 13:39:29

by Heiko Stübner

[permalink] [raw]
Subject: Re: (subset) [PATCH v3 0/6] drm/rockchip: vop2: add support for the rgb output block

On Tue, 24 Jan 2023 06:47:00 +0100, Michael Riesch wrote:
> This series adds support for the RGB output block that can be found in the
> Rockchip Video Output Processor (VOP) 2. Version 2 of this series
> incorporates the feedback by Dan Carpenter and Sascha Hauer. Version 3
> fixes a dumb mistake pointed out by Sascha :-) Thanks for your comments!
>
> Patches 1-4 clean up the code and make it more general.
>
> [...]

Applied, thanks!

[6/6] arm64: dts: rockchip: add pinctrls for 16-bit/18-bit rgb interface to rk356x
commit: 381b6d432f6eb00e1faff763f55e67519af9fa23

Best regards,
--
Heiko Stuebner <[email protected]>

2023-02-05 14:56:51

by Heiko Stübner

[permalink] [raw]
Subject: Re: (subset) [PATCH v3 0/6] drm/rockchip: vop2: add support for the rgb output block

On Tue, 24 Jan 2023 06:47:00 +0100, Michael Riesch wrote:
> This series adds support for the RGB output block that can be found in the
> Rockchip Video Output Processor (VOP) 2. Version 2 of this series
> incorporates the feedback by Dan Carpenter and Sascha Hauer. Version 3
> fixes a dumb mistake pointed out by Sascha :-) Thanks for your comments!
>
> Patches 1-4 clean up the code and make it more general.
>
> [...]

Applied, thanks!

[1/6] drm/rockchip: vop2: initialize possible_crtcs properly
commit: 368419a2d429e2438bef333959732c640310bdc7
[2/6] drm/rockchip: rgb: embed drm_encoder into rockchip_encoder
commit: f8a852f1f86391127ab57b1c41fe0e62bc14f27c
[3/6] drm/rockchip: rgb: add video_port parameter to init function
commit: 03db8f25cf16c579fe75fd2230bbe64c221bfe25
[4/6] drm/rockchip: vop2: use symmetric function pair vop2_{create,destroy}_crtcs
commit: cddddc066b056e7bb629a8c4d99c9c4a8ca70a6a
[5/6] drm/rockchip: vop2: add support for the rgb output block
commit: c66c6d7c47058a72a00b50d7f5c4538e3fa49b1c

Best regards,
--
Heiko Stuebner <[email protected]>