mbox series

[v2,00/18] drm/rockchip: RK356x VOP2 support

Message ID 20211208151230.3695378-1-s.hauer@pengutronix.de (mailing list archive)
Headers show
Series drm/rockchip: RK356x VOP2 support | expand

Message

Sascha Hauer Dec. 8, 2021, 3:12 p.m. UTC
This is the second round of the vop2 series. There are still some issues open,
but I thought it's about time to let people see and test it. I integrated the
review feedback I got from v1. Other changes include:

All framesync waiting is gone from the driver which makes it more straight
forward. To accomplish this the port_mux setup is now static in the driver.
This means each video port has a fixed maximum number of planes which is less
flexible but much easier to handle.

I also removed much of the register mapping and shadow register handling around
struct vop_reg. This basically resembles regmap and can eventually replaced by
regmap. Some places are still left in the driver, I plan to remove those in
later versions.

I think I have found the issue why only 1080p resolutions work, this seems to
be an issue in the way the clock tree is arranged. See the last patch in this
series which points to the problem, so far I don't have a good solution for it.

As usual, all comments and feedback welcome.

Sascha

Changes since v1:
- drop all unnecessary waiting for frames within atomic modeset and plane update
- Cluster subwin support removed
- gamma support removed
- unnecessary irq_lock removed
- interrupt handling simplified
- simplified zpos handling
- drop is_alpha_support(), use fb->format->has_alpha instead
- use devm_regulator_get() rather than devm_regulator_get_optional() for hdmi regulators
- Use fixed number of planes per video port
- Drop homegrown regmap code from vop2 driver (not complete yet)
- Add separate include file for vop2 driver to not pollute the vop include

Andy Yan (1):
  drm: rockchip: Add VOP2 driver

Benjamin Gaignard (1):
  dt-bindings: display: rockchip: dw-hdmi: Add compatible for rk3568
    HDMI

Michael Riesch (1):
  arm64: dts: rockchip: enable vop2 and hdmi tx on quartz64a

Sascha Hauer (15):
  drm/rockchip: dw_hdmi: Do not leave clock enabled in error case
  drm/rockchip: dw_hdmi: rename vpll clock to reference clock
  drm/rockchip: dw_hdmi: add rk3568 support
  drm/rockchip: dw_hdmi: add regulator support
  dt-bindings: display: rockchip: dw-hdmi: Make unwedge pinctrl optional
  dt-bindings: display: rockchip: dw-hdmi: Allow "ref" as clock name
  dt-bindings: display: rockchip: dw-hdmi: Add regulator support
  arm64: dts: rockchip: rk3399: reorder hmdi clocks
  dt-bindings: display: rockchip: Add binding for VOP2
  arm64: dts: rockchip: rk356x: Add VOP2 nodes
  arm64: dts: rockchip: rk356x: Add HDMI nodes
  arm64: dts: rockchip: rk3568-evb: Enable VOP2 and hdmi
  drm/encoder: Add of_graph port to struct drm_encoder
  drm/rockchip: Make VOP driver optional
  [HACK, RFC] clk: rk3568: do not divide dclk_vop0

 .../display/rockchip/rockchip,dw-hdmi.yaml    |   14 +-
 .../display/rockchip/rockchip-vop2.yaml       |  118 +
 arch/arm64/boot/dts/rockchip/rk3399.dtsi      |    6 +-
 .../boot/dts/rockchip/rk3566-quartz64-a.dts   |   31 +
 arch/arm64/boot/dts/rockchip/rk3566.dtsi      |    4 +
 .../boot/dts/rockchip/rk3568-evb1-v10.dts     |   31 +
 arch/arm64/boot/dts/rockchip/rk3568.dtsi      |    4 +
 arch/arm64/boot/dts/rockchip/rk356x.dtsi      |   75 +
 drivers/clk/rockchip/clk-rk3568.c             |    4 +-
 drivers/gpu/drm/rockchip/Kconfig              |   14 +
 drivers/gpu/drm/rockchip/Makefile             |    4 +-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c   |  107 +-
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c   |    3 +-
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h   |    7 +-
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c    |    2 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h   |   15 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c  | 2636 +++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.h  |  625 ++++
 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c  |  505 ++++
 include/drm/drm_encoder.h                     |    2 +
 include/dt-bindings/soc/rockchip,vop2.h       |   14 +
 21 files changed, 4193 insertions(+), 28 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
 create mode 100644 include/dt-bindings/soc/rockchip,vop2.h

Comments

Johan Jonker Dec. 8, 2021, 4:59 p.m. UTC | #1
Hi,

On 12/8/21 4:12 PM, Sascha Hauer wrote:
> From: Andy Yan <andy.yan@rock-chips.com>
> 
> The VOP2 unit is found on Rockchip SoCs beginning with rk3566/rk3568.
> It replaces the VOP unit found in the older Rockchip SoCs.
> 
> This driver has been derived from the downstream Rockchip Kernel and
> heavily modified:
> 
> - All nonstandard DRM properties have been removed
> - dropped struct vop2_plane_state and pass around less data between
>   functions
> - Dropped all DRM_FORMAT_* not known on upstream
> - rework register access to get rid of excessively used macros
> - Drop all waiting for framesyncs
> 
> The driver is tested with HDMI and MIPI-DSI display on a RK3568-EVB
> board. Overlay support is tested with the modetest utility. AFBC support
> on the cluster windows is tested with weston-simple-dmabuf-egl on
> weston using the (yet to be upstreamed) panfrost driver support.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---

[..]

> +
> +static const struct of_device_id vop2_dt_match[] = {
> +	{
> +		.compatible = "rockchip,rk3568-vop",
> +		.data = &rk3568_vop
> +	}, {

> +		.compatible = "rockchip,rk3568-vop",

Maybe use:
.compatible = "rockchip,rk3566-vop",

> +		.data = &rk3566_vop
> +	}, {
> +	},

Maybe sort this list alphabetical based on compatible in case later more
SoCs are added.

rk3566
rk3568

===

The structure layout size above could be reduced for if we get more
compatible strings additions.

Example vop1:

static const struct of_device_id vop_driver_dt_match[] = {
	{ .compatible = "rockchip,rk3036-vop",
	  .data = &rk3036_vop },
	{ .compatible = "rockchip,rk3126-vop",
	  .data = &rk3126_vop },
	{ .compatible = "rockchip,px30-vop-big",
	  .data = &px30_vop_big },
	{ .compatible = "rockchip,px30-vop-lit",
	  .data = &px30_vop_lit },
	{ .compatible = "rockchip,rk3066-vop",
	  .data = &rk3066_vop },
	{ .compatible = "rockchip,rk3188-vop",
	  .data = &rk3188_vop },
	{ .compatible = "rockchip,rk3288-vop",
	  .data = &rk3288_vop },
	{ .compatible = "rockchip,rk3368-vop",
	  .data = &rk3368_vop },
	{ .compatible = "rockchip,rk3366-vop",
	  .data = &rk3366_vop },
	{ .compatible = "rockchip,rk3399-vop-big",
	  .data = &rk3399_vop_big },
	{ .compatible = "rockchip,rk3399-vop-lit",
	  .data = &rk3399_vop_lit },
	{ .compatible = "rockchip,rk3228-vop",
	  .data = &rk3228_vop },
	{ .compatible = "rockchip,rk3328-vop",
	  .data = &rk3328_vop },
	{},
};

> +};
> +MODULE_DEVICE_TABLE(of, vop2_dt_match);
> +
> +static int vop2_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +
> +	return component_add(dev, &vop2_component_ops);
> +}
> +
> +static int vop2_remove(struct platform_device *pdev)
> +{
> +	component_del(&pdev->dev, &vop2_component_ops);
> +
> +	return 0;
> +}
> +
> +struct platform_driver vop2_platform_driver = {
> +	.probe = vop2_probe,
> +	.remove = vop2_remove,
> +	.driver = {
> +		.name = "rockchip-vop2",
> +		.of_match_table = of_match_ptr(vop2_dt_match),
> +	},
> +};
>
Sascha Hauer Dec. 10, 2021, 8:55 a.m. UTC | #2
Hi Johan,

On Wed, Dec 08, 2021 at 05:59:16PM +0100, Johan Jonker wrote:
> Hi,
> 
> On 12/8/21 4:12 PM, Sascha Hauer wrote:
> > From: Andy Yan <andy.yan@rock-chips.com>
> > 
> > The VOP2 unit is found on Rockchip SoCs beginning with rk3566/rk3568.
> > It replaces the VOP unit found in the older Rockchip SoCs.
> > 
> > This driver has been derived from the downstream Rockchip Kernel and
> > heavily modified:
> > 
> > - All nonstandard DRM properties have been removed
> > - dropped struct vop2_plane_state and pass around less data between
> >   functions
> > - Dropped all DRM_FORMAT_* not known on upstream
> > - rework register access to get rid of excessively used macros
> > - Drop all waiting for framesyncs
> > 
> > The driver is tested with HDMI and MIPI-DSI display on a RK3568-EVB
> > board. Overlay support is tested with the modetest utility. AFBC support
> > on the cluster windows is tested with weston-simple-dmabuf-egl on
> > weston using the (yet to be upstreamed) panfrost driver support.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> 
> [..]
> 
> > +
> > +static const struct of_device_id vop2_dt_match[] = {
> > +	{
> > +		.compatible = "rockchip,rk3568-vop",
> > +		.data = &rk3568_vop
> > +	}, {
> 
> > +		.compatible = "rockchip,rk3568-vop",
> 
> Maybe use:
> .compatible = "rockchip,rk3566-vop",

Copy/Paste bug. Will fix.

> 
> > +		.data = &rk3566_vop
> > +	}, {
> > +	},
> 
> Maybe sort this list alphabetical based on compatible in case later more
> SoCs are added.
> 
> rk3566
> rk3568

Ok.

> 
> ===
> 
> The structure layout size above could be reduced for if we get more
> compatible strings additions.
> 
> Example vop1:
> 
> static const struct of_device_id vop_driver_dt_match[] = {
> 	{ .compatible = "rockchip,rk3036-vop",
> 	  .data = &rk3036_vop },
> 	{ .compatible = "rockchip,rk3126-vop",
> 	  .data = &rk3126_vop },
> 	{ .compatible = "rockchip,px30-vop-big",
> 	  .data = &px30_vop_big },
> 	{ .compatible = "rockchip,px30-vop-lit",
> 	  .data = &px30_vop_lit },
> 	{ .compatible = "rockchip,rk3066-vop",
> 	  .data = &rk3066_vop },
> 	{ .compatible = "rockchip,rk3188-vop",
> 	  .data = &rk3188_vop },
> 	{ .compatible = "rockchip,rk3288-vop",
> 	  .data = &rk3288_vop },
> 	{ .compatible = "rockchip,rk3368-vop",
> 	  .data = &rk3368_vop },
> 	{ .compatible = "rockchip,rk3366-vop",
> 	  .data = &rk3366_vop },
> 	{ .compatible = "rockchip,rk3399-vop-big",
> 	  .data = &rk3399_vop_big },
> 	{ .compatible = "rockchip,rk3399-vop-lit",
> 	  .data = &rk3399_vop_lit },
> 	{ .compatible = "rockchip,rk3228-vop",
> 	  .data = &rk3228_vop },
> 	{ .compatible = "rockchip,rk3328-vop",
> 	  .data = &rk3328_vop },
> 	{},

It's shorter, but ugly ;)
That's only my personal taste though, I don't care much.

Sascha