Re: [PATCH 22/22] drm: rockchip: Add VOP2 driver
Hi Andy, On Tue, Jan 04, 2022 at 07:07:23PM +0800, Andy Yan wrote: > > > I thinks we should be very carefully about switch to regmap. > > Most of the registers are take effect by frame sync(that is you write the > config done bit and when vsync interrupt come), > > Not only windows register, but also the SYS_CTRL, post processor(VP0/1/2), > OVERLAY, hdr and so on. That's why I marked these as nonvolatile, so that reading the registers comes from the cache. I may have missed some registers, these could be added when needed. > > + ret = vop2_win_init(vop2); > > + if (ret) > > + return ret; > Do you have a count about how much time the function vop2_win_init cost ? No. Why does that matter? 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- |
Re: [PATCH 22/22] drm: rockchip: Add VOP2 driver
Hi Sascha:
Please add Series-version for all the patch series. add also Series-changes.
I saw you have Series-changes in cover-letter, but we usually include
them in patch.
Here is a link you can take for reference[0]
[0]https://source.denx.de/u-boot/u-boot/-/tree/master/tools/patman
On 12/20/21 7:06 PM, Sascha Hauer wrote:
From: Andy Yan
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
---
drivers/gpu/drm/rockchip/Kconfig |6 +
drivers/gpu/drm/rockchip/Makefile|1 +
drivers/gpu/drm/rockchip/rockchip_drm_drv.c |1 +
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 | 2768 ++
drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 480 +++
drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 285 ++
9 files changed, 3564 insertions(+), 1 deletion(-)
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
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index b9b156308460a..4ff0043f0ee70 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -28,6 +28,12 @@ config ROCKCHIP_VOP
This selects support for the VOP driver. You should enable it
on all older SoCs up to RK3399.
+config ROCKCHIP_VOP2
+ bool "Rockchip VOP2 driver"
+ help
+ This selects support for the VOP2 driver. You should enable it
+ on all newer SoCs beginning form RK3568.
+
config ROCKCHIP_ANALOGIX_DP
bool "Rockchip specific extensions for Analogix DP driver"
depends on ROCKCHIP_VOP
diff --git a/drivers/gpu/drm/rockchip/Makefile
b/drivers/gpu/drm/rockchip/Makefile
index cd6e7bb5ce9c5..29848caef5c21 100644
--- a/drivers/gpu/drm/rockchip/Makefile
+++ b/drivers/gpu/drm/rockchip/Makefile
@@ -7,6 +7,7 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
rockchip_drm_gem.o
rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
+rockchipdrm-$(CONFIG_ROCKCHIP_VOP2) += rockchip_drm_vop2.o rockchip_vop2_reg.o
rockchipdrm-$(CONFIG_ROCKCHIP_VOP) += rockchip_drm_vop.o rockchip_vop_reg.o
rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 64fa5fd62c01a..2bd9acb265e5a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -474,6 +474,7 @@ static int __init rockchip_drm_init(void)
num_rockchip_sub_drivers = 0;
ADD_ROCKCHIP_SUB_DRIVER(vop_platform_driver, CONFIG_ROCKCHIP_VOP);
+ ADD_ROCKCHIP_SUB_DRIVER(vop2_platform_driver, CONFIG_ROCKCHIP_VOP2);
ADD_ROCKCHIP_SUB_DRIVER(rockchip_lvds_driver,
CONFIG_ROCKCHIP_LVDS);
ADD_ROCKCHIP_SUB_DRIVER(rockchip_dp_driver,
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index aa0909e8edf93..fd6994f21817e 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -18,7 +18,7 @@
#define ROCKCHIP_MAX_FB_BUFFER 3
#define ROCKCHIP_MAX_CONNECTOR2
-#define ROCKCHIP_MAX_CRTC 2
+#define ROCKCHIP_MAX_CRTC 4
struct drm_device;
struct drm_connector;
@@ -31,6 +31,9 @@ struct rockchip_crtc_state {
int output_bpc;
int output_flags;
bool enable_afbc;
+ uint32_t bus_format;
+ u32 bus_flags;
+ int color_space;
};
#define to_rockchip_crtc_state(s) \
container_of(s, struct rockchip_crtc_state, base)
@@ -65,4 +68,6 @@ extern struct platform_driver rockchip_dp_driver;
extern struct platform_driver rockchip_lvds_driver;
extern struct platform_driver vop_platform_driver;
extern struct platform_driver rk3066_hdmi_driver;
+extern struct platform_driver vop2_platfo
Re: [PATCH 22/22] drm: rockchip: Add VOP2 driver
Hi Nicolas,
Thanks for the review. I have changed the points I don't comment on here
according to your suggestions.
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
>
> Alphabetically sort these includes? Not sure on what the
> convention is in the DRM subsystem.
Me neither. The first random file I looked at in drivers/gpu/drm/ has
the includes sorted alphabetically, so I'll do the same.
>
> > +static void vop2_cfg_done(struct vop2_video_port *vp)
> > +{
> > + struct vop2 *vop2 = vp->vop2;
> > + uint32_t val;
> > +
> > + val = vop2_readl(vop2, RK3568_REG_CFG_DONE);
> > +
> > + val &= 0x7;
>
> GENMASK(2, 0) instead of 0x7, should that be a constant somewhere?
>
> > +
> > + vop2_writel(vop2, RK3568_REG_CFG_DONE,
> > + val | BIT(vp->id) | RK3568_REG_CFG_DONE__GLB_CFG_DONE_EN);
Replaced that with the following which doesn't need a mask:
regmap_set_bits(vop2->map, RK3568_REG_CFG_DONE,
BIT(vp->id) |
RK3568_REG_CFG_DONE__GLB_CFG_DONE_EN);
> > +}
> > +
> > +static void vop2_win_disable(struct vop2_win *win)
> > +{
> > + vop2_win_write(win, VOP2_WIN_ENABLE, 0);
> > +
> > + if (vop2_cluster_window(win))
> > + vop2_win_write(win, VOP2_WIN_CLUSTER_ENABLE, 0);
> > +}
> > +
> > +static uint32_t vop2_afbc_transform_offset(struct drm_plane_state *pstate,
> > + bool afbc_half_block_en)
> > +{
> > + struct drm_rect *src = &pstate->src;
> > + struct drm_framebuffer *fb = pstate->fb;
> > + uint32_t bpp = fb->format->cpp[0] * 8;
> > + uint32_t vir_width = (fb->pitches[0] << 3) / bpp;
> > + uint32_t width = drm_rect_width(src) >> 16;
> > + uint32_t height = drm_rect_height(src) >> 16;
> > + uint32_t act_xoffset = src->x1 >> 16;
> > + uint32_t act_yoffset = src->y1 >> 16;
> > + uint32_t align16_crop = 0;
> > + uint32_t align64_crop = 0;
> > + uint32_t height_tmp = 0;
> > + uint32_t transform_tmp = 0;
> > + uint8_t transform_xoffset = 0;
> > + uint8_t transform_yoffset = 0;
> > + uint8_t top_crop = 0;
> > + uint8_t top_crop_line_num = 0;
> > + uint8_t bottom_crop_line_num = 0;
> > +
> > + /* 16 pixel align */
> > + if (height & 0xf)
> > + align16_crop = 16 - (height & 0xf);
> > +
> > + height_tmp = height + align16_crop;
> > +
> > + /* 64 pixel align */
> > + if (height_tmp & 0x3f)
> > + align64_crop = 64 - (height_tmp & 0x3f);
> > +
> > + top_crop_line_num = top_crop << 2;
> > + if (top_crop == 0)
> > + bottom_crop_line_num = align16_crop + align64_crop;
> > + else if (top_crop == 1)
> > + bottom_crop_line_num = align16_crop + align64_crop + 12;
> > + else if (top_crop == 2)
> > + bottom_crop_line_num = align16_crop + align64_crop + 8;
>
> top_crop == 0 is always taken from what I can gather, rest is
> dead code. Is this intentional? It doesn't seem intentional.
It's the same in the downstream driver. I don't know what top_crop is
good for. I just removed the dead code for now.
> > + transform_xoffset = transform_tmp & 0xf;
> > + transform_tmp = vir_width - width - act_xoffset;
> > + transform_yoffset = transform_tmp & 0xf;
> > + break;
> > + case 0:
> > + transform_tmp = act_xoffset;
> > + transform_xoffset = transform_tmp & 0xf;
> > + transform_tmp = top_crop_line_num + act_yoffset;
> > +
> > + if (afbc_half_block_en)
> > + transform_yoffset = transform_tmp & 0x7;
> > + else
> > + transform_yoffset = transform_tmp & 0xf;
> > +
> > + break;
> > + }
> > +
> > + return (transform_xoffset & 0xf) | ((transform_yoffset & 0xf) << 16);
> > +}
>
> 0xf, 0x3f, 0x7 might be more clear as GENMASK.
> if (afbc_half_block_en) branches can be moved out of cases and
> assign a transform mask variable instead.
Sure. Other than that the function can be simplified a bit more.
>
> > +
> > +/*
> > + * A Cluster window has 2048 x 16 line buffer, which can
> > + * works at 2048 x 16(Full) or 4096 x 8 (Half) mode.
> > + * for Cluster_lb_mode register:
> > + * 0: half mode, for plane input width range 2048 ~ 4096
> > + * 1: half mode, for cluster work at 2 * 2048 plane mode
> > + * 2: half mode, for rotate_90/270 mode
> > + *
> > + */
> > +static int vop2_get_cluster_lb_mode(struct vop2_win *win, struct
> > drm_plane_state *pstate)
> > +{
> > + if ((pstate->rotation & DRM_MODE_ROTATE_270) || (pstate->rotation &
> > DRM_MODE_ROTATE_90))
> > + return 2;
> > + else
> > + return 0;
> > +}
>
> What about 1?
1 is used in the downstream driver for cluster sub windows. These are
currently not supported, I don't know yet where this is leading to.
>
> > +
> > +/*
> > + * bli_sd_factor = (src - 1) / (dst - 1) << 12;
> > + * avg_sd_factor:
> > + * bli_su_factor:
> > + * bic_su_fa
Re: [PATCH 22/22] drm: rockchip: Add VOP2 driver
On Dienstag, 21. Dezember 2021 14:44:39 CET Nicolas Frattaroli wrote: > On Montag, 20. Dezember 2021 12:06:30 CET Sascha Hauer wrote: > > From: Andy Yan > > > > 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 > > --- > > Hi Sascha, > > quick partial review of the code in-line. > > For reference, I debugged locking issues with the kernel lock > debug config options and assert_spin_locked in the reg write > functions, as well as some manual deduction. > As a small follow-up, I've completely mapped out the calls to vop2_writel, vop2_readl, vop2_vp_write and vop2_win_write and coloured in whether they were called with the lock held or not. The conclusion is startling: Most of the code absolutely does not care about the reg_lock. Here's the graph as an SVG: https://overviewer.org/~pillow/up/6800427ef3/vop2_callgraph_modified.svg vop2_isr here needs to be paid special attention, as it also acquires a different spinlock, and we want to avoid deadlocks. Perhaps we should precisely define which lock must be held for what registers, such that the vop2_isr can write its interrupt related registers without acquiring the "big" reg_lock. I'm also not entirely sure whether I should assume vop2_readl needs to be called with the lock held. This needs some investigating both in terms of whether the hardware presents a writel as an atomic write of a long, and whether the code assumes the state between readl calls is ever a consistent view. Regards, Nicolas Frattaroli
Re: [PATCH 22/22] drm: rockchip: Add VOP2 driver
On Montag, 20. Dezember 2021 12:06:30 CET Sascha Hauer wrote:
> From: Andy Yan
>
> 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
> ---
Hi Sascha,
quick partial review of the code in-line.
For reference, I debugged locking issues with the kernel lock
debug config options and assert_spin_locked in the reg write
functions, as well as some manual deduction.
> drivers/gpu/drm/rockchip/Kconfig |6 +
> drivers/gpu/drm/rockchip/Makefile|1 +
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c |1 +
> 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 | 2768 ++
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 480 +++
> drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 285 ++
> 9 files changed, 3564 insertions(+), 1 deletion(-)
> 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
>
> diff --git a/drivers/gpu/drm/rockchip/Kconfig
> b/drivers/gpu/drm/rockchip/Kconfig
> index b9b156308460a..4ff0043f0ee70 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -28,6 +28,12 @@ config ROCKCHIP_VOP
> This selects support for the VOP driver. You should enable it
> on all older SoCs up to RK3399.
>
> +config ROCKCHIP_VOP2
> + bool "Rockchip VOP2 driver"
> + help
> + This selects support for the VOP2 driver. You should enable it
> + on all newer SoCs beginning form RK3568.
> +
> config ROCKCHIP_ANALOGIX_DP
> bool "Rockchip specific extensions for Analogix DP driver"
> depends on ROCKCHIP_VOP
> diff --git a/drivers/gpu/drm/rockchip/Makefile
> b/drivers/gpu/drm/rockchip/Makefile
> index cd6e7bb5ce9c5..29848caef5c21 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -7,6 +7,7 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
> rockchip_drm_gem.o
> rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>
> +rockchipdrm-$(CONFIG_ROCKCHIP_VOP2) += rockchip_drm_vop2.o
> rockchip_vop2_reg.o
> rockchipdrm-$(CONFIG_ROCKCHIP_VOP) += rockchip_drm_vop.o rockchip_vop_reg.o
> rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
> rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 64fa5fd62c01a..2bd9acb265e5a 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -474,6 +474,7 @@ static int __init rockchip_drm_init(void)
>
> num_rockchip_sub_drivers = 0;
> ADD_ROCKCHIP_SUB_DRIVER(vop_platform_driver, CONFIG_ROCKCHIP_VOP);
> + ADD_ROCKCHIP_SUB_DRIVER(vop2_platform_driver, CONFIG_ROCKCHIP_VOP2);
> ADD_ROCKCHIP_SUB_DRIVER(rockchip_lvds_driver,
> CONFIG_ROCKCHIP_LVDS);
> ADD_ROCKCHIP_SUB_DRIVER(rockchip_dp_driver,
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index aa0909e8edf93..fd6994f21817e 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -18,7 +18,7 @@
>
> #define ROCKCHIP_MAX_FB_BUFFER 3
> #define ROCKCHIP_MAX_CONNECTOR 2
> -#define ROCKCHIP_MAX_CRTC2
> +#define ROCKCHIP_MAX_CRTC4
>
> struct drm_device;
> struct drm_connector;
> @@ -31,6 +31,9 @@ struct rockchip_crtc_state {
> int output_bpc;
> int output_flags;
> bool enable_afbc;
> + uint32_t bus_format;
> + u32 bus_flags;
> + int color_space;
> };
> #define to_rockchip_crtc_state(s) \
> container_of(s, struct rockchip_crtc_state, base)
> @@ -65,4 +68,6 @@ extern struct platform_driver rockchip_dp_driver;
> extern struct platform_driver rockchip_lvds_driver;
> extern struct platform_driver vop_platform_driver;
Re: [PATCH 22/22] drm: rockchip: Add VOP2 driver
Hi Sascha,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on rockchip/for-next]
[also build test WARNING on tegra-drm/drm/tegra/for-next v5.16-rc6]
[cannot apply to drm/drm-next drm-intel/for-linux-next drm-tip/drm-tip
drm-exynos/exynos-drm-next airlied/drm-next next-20211220]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Sascha-Hauer/drm-rockchip-RK356x-VOP2-support/20211220-190821
base:
https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git
for-next
config: arc-allyesconfig
(https://download.01.org/0day-ci/archive/20211221/202112210748.vruldmgp-...@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/ade6380669a79670b48d440d8b7a00986a5d7ca8
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Sascha-Hauer/drm-rockchip-RK356x-VOP2-support/20211220-190821
git checkout ade6380669a79670b48d440d8b7a00986a5d7ca8
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross
O=build_dir ARCH=arc SHELL=/bin/bash drivers/gpu/drm/rockchip/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c: In function
'vop2_cluster_init':
>> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2525:1: warning: the frame size
>> of 1100 bytes is larger than 1024 bytes [-Wframe-larger-than=]
2525 | };
| ^
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c: In function 'vop2_esmart_init':
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2597:1: warning: the frame size
of 1100 bytes is larger than 1024 bytes [-Wframe-larger-than=]
2597 | };
| ^
vim +2525 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
2450
2451 static int vop2_cluster_init(struct vop2_win *win)
2452 {
2453 struct vop2 *vop2 = win->vop2;
2454 int i;
2455 struct reg_field vop2_cluster_regs[VOP2_WIN_MAX_REG] = {
2456 [VOP2_WIN_ENABLE] = REG_FIELD(RK3568_CLUSTER_WIN_CTRL0,
0, 0),
2457 [VOP2_WIN_FORMAT] = REG_FIELD(RK3568_CLUSTER_WIN_CTRL0,
1, 5),
2458 [VOP2_WIN_RB_SWAP] =
REG_FIELD(RK3568_CLUSTER_WIN_CTRL0, 14, 14),
2459 [VOP2_WIN_DITHER_UP] =
REG_FIELD(RK3568_CLUSTER_WIN_CTRL0, 18, 18),
2460 [VOP2_WIN_ACT_INFO] =
REG_FIELD(RK3568_CLUSTER_WIN_ACT_INFO, 0, 31),
2461 [VOP2_WIN_DSP_INFO] =
REG_FIELD(RK3568_CLUSTER_WIN_DSP_INFO, 0, 31),
2462 [VOP2_WIN_DSP_ST] =
REG_FIELD(RK3568_CLUSTER_WIN_DSP_ST, 0, 31),
2463 [VOP2_WIN_YRGB_MST] =
REG_FIELD(RK3568_CLUSTER_WIN_YRGB_MST, 0, 31),
2464 [VOP2_WIN_UV_MST] =
REG_FIELD(RK3568_CLUSTER_WIN_CBR_MST, 0, 31),
2465 [VOP2_WIN_YUV_CLIP] =
REG_FIELD(RK3568_CLUSTER_WIN_CTRL0, 19, 19),
2466 [VOP2_WIN_YRGB_VIR] = REG_FIELD(RK3568_CLUSTER_WIN_VIR,
0, 15),
2467 [VOP2_WIN_UV_VIR] = REG_FIELD(RK3568_CLUSTER_WIN_VIR,
16, 31),
2468 [VOP2_WIN_Y2R_EN] = REG_FIELD(RK3568_CLUSTER_WIN_CTRL0,
8, 8),
2469 [VOP2_WIN_R2Y_EN] = REG_FIELD(RK3568_CLUSTER_WIN_CTRL0,
9, 9),
2470 [VOP2_WIN_CSC_MODE] =
REG_FIELD(RK3568_CLUSTER_WIN_CTRL0, 10, 11),
2471
2472 /* Scale */
2473 [VOP2_WIN_SCALE_YRGB_X] =
REG_FIELD(RK3568_CLUSTER_WIN_SCL_FACTOR_YRGB, 0, 15),
2474 [VOP2_WIN_SCALE_YRGB_Y] =
REG_FIELD(RK3568_CLUSTER_WIN_SCL_FACTOR_YRGB, 16, 31),
2475 [VOP2_WIN_YRGB_VER_SCL_MODE] =
REG_FIELD(RK3568_CLUSTER_WIN_CTRL1, 14, 15),
2476 [VOP2_WIN_YRGB_HOR_SCL_MODE] =
REG_FIELD(RK3568_CLUSTER_WIN_CTRL1, 12, 13),
2477 [VOP2_WIN_BIC_COE_SEL] =
REG_FIELD(RK3568_CLUSTER_WIN_CTRL1, 2, 3),
2478 [VOP2_WIN_VSD_YRGB_GT2] =
REG_FIELD(RK3568_CLUSTER_WIN_CTRL1, 28, 28),
2479 [VOP2_WIN_VSD_YRGB_GT4] =
REG_FIELD(RK3568_CLUSTER_WIN_CTRL1, 29, 29),
2480
2481 /* cluster regs */
2482 [VOP2_WIN_AFBC_ENABLE] = REG_FIELD(RK3568_CLUSTER_CTRL,
1, 1),
2483 [VOP2_WIN_CLUSTER_ENABLE] =
REG_FIELD(RK3568_CLUSTER_CTRL, 0, 0),
2484 [VOP2_WIN_CLUSTER_LB_MODE] =
REG_FIELD(RK3568_CLUSTER_CTRL, 4, 7),
2485
2486 /* afbc regs */
2487
Re: [PATCH 22/22] drm: rockchip: Add VOP2 driver
On Mon, Dec 20, 2021 at 03:16:55PM +0100, Nicolas Frattaroli wrote:
> On Montag, 20. Dezember 2021 12:06:30 CET Sascha Hauer wrote:
> > From: Andy Yan
> >
> > 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
> > ---
>
> Hi Sascha,
>
> sadly I'm getting
>
> [1.668856] rockchip-drm display-subsystem: [drm] *ERROR* failed to get
> vop2 register byname
> [1.669621] rockchip-drm display-subsystem: failed to bind fe04.vop
> (ops vop2_component_ops): -22
> [1.670584] rockchip-drm display-subsystem: master bind failed: -22
> [1.671164] dwhdmi-rockchip: probe of fe0a.hdmi failed with error -22
>
> on a Quartz64 Model A.
>
>
> > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> > + if (!res) {
> > + drm_err(vop2->drm, "failed to get vop2 register byname\n");
> > + return -EINVAL;
> > + }
>
> This seems to be the code that triggers it.
>
> Any ideas as to what could be causing this?
Gnaa, this was a last minute change I did and thought it was too trivial
to fail :(
The binding previously had:
reg = <0x0 0xfe04 0x0 0x3000>, <0x0 0xfe044000 0x0 0x1000>;
reg-names = "regs", "gamma_lut";
I thought that I can merge these two regions into one bigger region.
Apart from the fact that I got it wrong (The driver still requests a
resource "regs" as you found out), it doesnt work, because the space
between those two regions is not unused, the iommu is in between them.
I'll have to revert that change for the next round. Please use the
attached fixup patch in the meantime.
Sascha
---8<--
>From 9b3dee3d8584f5a9cc24f21ff8c5895465c4b3ae Mon Sep 17 00:00:00 2001
From: Sascha Hauer
Date: Mon, 20 Dec 2021 19:49:14 +0100
Subject: [PATCH] fixup! arm64: dts: rockchip: rk356x: Add VOP2 nodes
---
arch/arm64/boot/dts/rockchip/rk356x.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 6d93b8fcee179..cce4b789d6d58 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -453,7 +453,8 @@ gmac1_mtl_tx_setup: tx-queues-config {
};
vop: vop@fe04 {
- reg = <0x0 0xfe04 0x0 0x5000>;
+ reg = <0x0 0xfe04 0x0 0x3000>, <0x0 0xfe044000 0x0 0x1000>;
+ reg-names = "regs", "gamma_lut";
interrupts = ;
clocks = <&cru ACLK_VOP>, <&cru HCLK_VOP>, <&cru DCLK_VOP0>,
<&cru DCLK_VOP1>, <&cru DCLK_VOP2>;
clock-names = "aclk_vop", "hclk_vop", "dclk_vp0", "dclk_vp1",
"dclk_vp2";
--
2.30.2
--
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- |
Re: [PATCH 22/22] drm: rockchip: Add VOP2 driver
On Montag, 20. Dezember 2021 15:16:55 CET Nicolas Frattaroli wrote:
> On Montag, 20. Dezember 2021 12:06:30 CET Sascha Hauer wrote:
> > From: Andy Yan
> >
> > 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
> > ---
>
> Hi Sascha,
>
> sadly I'm getting
>
> [1.668856] rockchip-drm display-subsystem: [drm] *ERROR* failed to get
> vop2 register byname
> [1.669621] rockchip-drm display-subsystem: failed to bind fe04.vop
> (ops vop2_component_ops): -22
> [1.670584] rockchip-drm display-subsystem: master bind failed: -22
> [1.671164] dwhdmi-rockchip: probe of fe0a.hdmi failed with error -22
>
> on a Quartz64 Model A.
>
>
> > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> > + if (!res) {
> > + drm_err(vop2->drm, "failed to get vop2 register byname\n");
> > + return -EINVAL;
> > + }
>
> This seems to be the code that triggers it.
>
> Any ideas as to what could be causing this?
>
> Regards,
> Nicolas Frattaroli
>
A small follow-up:
We're trying to get IORESOURCE_MEM by the name "regs", but nothing
has a reg-names property in the device tree changes you sent in.
How did you test this on your system?
Also, adding the reg-names = "regs"; to the vop node doesn't make
it probe, it just fails with
[1.668560] rockchip-vop2 fe04.vop: can't request region for resource
[mem 0xfe04-0xfe044fff]
then.
We're also trying to get gamma_lut the same way, but your cover letter states
> drop unnecessary gamma_lut registers from vop2
which makes me wonder: is this the correct series of patches you sent in?
Regards,
Nicolas Frattaroli
Re: [PATCH 22/22] drm: rockchip: Add VOP2 driver
On Montag, 20. Dezember 2021 12:06:30 CET Sascha Hauer wrote:
> From: Andy Yan
>
> 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
> ---
Hi Sascha,
sadly I'm getting
[1.668856] rockchip-drm display-subsystem: [drm] *ERROR* failed to get vop2
register byname
[1.669621] rockchip-drm display-subsystem: failed to bind fe04.vop (ops
vop2_component_ops): -22
[1.670584] rockchip-drm display-subsystem: master bind failed: -22
[1.671164] dwhdmi-rockchip: probe of fe0a.hdmi failed with error -22
on a Quartz64 Model A.
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> + if (!res) {
> + drm_err(vop2->drm, "failed to get vop2 register byname\n");
> + return -EINVAL;
> + }
This seems to be the code that triggers it.
Any ideas as to what could be causing this?
Regards,
Nicolas Frattaroli
[PATCH 22/22] drm: rockchip: Add VOP2 driver
From: Andy Yan
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
---
drivers/gpu/drm/rockchip/Kconfig |6 +
drivers/gpu/drm/rockchip/Makefile|1 +
drivers/gpu/drm/rockchip/rockchip_drm_drv.c |1 +
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 | 2768 ++
drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 480 +++
drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 285 ++
9 files changed, 3564 insertions(+), 1 deletion(-)
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
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index b9b156308460a..4ff0043f0ee70 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -28,6 +28,12 @@ config ROCKCHIP_VOP
This selects support for the VOP driver. You should enable it
on all older SoCs up to RK3399.
+config ROCKCHIP_VOP2
+ bool "Rockchip VOP2 driver"
+ help
+ This selects support for the VOP2 driver. You should enable it
+ on all newer SoCs beginning form RK3568.
+
config ROCKCHIP_ANALOGIX_DP
bool "Rockchip specific extensions for Analogix DP driver"
depends on ROCKCHIP_VOP
diff --git a/drivers/gpu/drm/rockchip/Makefile
b/drivers/gpu/drm/rockchip/Makefile
index cd6e7bb5ce9c5..29848caef5c21 100644
--- a/drivers/gpu/drm/rockchip/Makefile
+++ b/drivers/gpu/drm/rockchip/Makefile
@@ -7,6 +7,7 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
rockchip_drm_gem.o
rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
+rockchipdrm-$(CONFIG_ROCKCHIP_VOP2) += rockchip_drm_vop2.o rockchip_vop2_reg.o
rockchipdrm-$(CONFIG_ROCKCHIP_VOP) += rockchip_drm_vop.o rockchip_vop_reg.o
rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 64fa5fd62c01a..2bd9acb265e5a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -474,6 +474,7 @@ static int __init rockchip_drm_init(void)
num_rockchip_sub_drivers = 0;
ADD_ROCKCHIP_SUB_DRIVER(vop_platform_driver, CONFIG_ROCKCHIP_VOP);
+ ADD_ROCKCHIP_SUB_DRIVER(vop2_platform_driver, CONFIG_ROCKCHIP_VOP2);
ADD_ROCKCHIP_SUB_DRIVER(rockchip_lvds_driver,
CONFIG_ROCKCHIP_LVDS);
ADD_ROCKCHIP_SUB_DRIVER(rockchip_dp_driver,
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index aa0909e8edf93..fd6994f21817e 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -18,7 +18,7 @@
#define ROCKCHIP_MAX_FB_BUFFER 3
#define ROCKCHIP_MAX_CONNECTOR 2
-#define ROCKCHIP_MAX_CRTC 2
+#define ROCKCHIP_MAX_CRTC 4
struct drm_device;
struct drm_connector;
@@ -31,6 +31,9 @@ struct rockchip_crtc_state {
int output_bpc;
int output_flags;
bool enable_afbc;
+ uint32_t bus_format;
+ u32 bus_flags;
+ int color_space;
};
#define to_rockchip_crtc_state(s) \
container_of(s, struct rockchip_crtc_state, base)
@@ -65,4 +68,6 @@ extern struct platform_driver rockchip_dp_driver;
extern struct platform_driver rockchip_lvds_driver;
extern struct platform_driver vop_platform_driver;
extern struct platform_driver rk3066_hdmi_driver;
+extern struct platform_driver vop2_platform_driver;
+
#endif /* _ROCKCHIP_DRM_DRV_H_ */
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index 3aa37e177667e..0d2cb4f3922b8 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -134,4 +134,6 @@ void rockchip_drm_mode_config_init(struct drm_device *dev)
