Re: [PATCH v15 13/13] drm/rockchip: vop2: Add support for rk3576

From: "Heiko Stübner" <heiko@sntech.de>
To: Andy Yan <andyshrk@163.com>
Cc: hjc@rock-chips.com, krzk+dt@kernel.org,
	devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
	derek.foreman@collabora.com, detlev.casanova@collabora.com,
	daniel@fooishbar.org, robh@kernel.org,
	sebastian.reichel@collabora.com,
	Andy Yan <andy.yan@rock-chips.com>,
	Michael Riesch <michael.riesch@wolfvision.net>
Subject: Re: [PATCH v15 13/13] drm/rockchip: vop2: Add support for rk3576
Date: Sun, 02 Mar 2025 20:09:52 +0100	[thread overview]
Message-ID: <2764042.X9hSmTKtgW@diego> (raw)
In-Reply-To: <20250218112901.34564-1-andyshrk@163.com>

Hi Andy,

Am Dienstag, 18. Februar 2025, 12:28:58 MEZ schrieb Andy Yan:
> From: Andy Yan <andy.yan@rock-chips.com>
> 
> VOP2 on rk3576:
> Three video ports:
> VP0 Max 4096x2160
> VP1 Max 2560x1600
> VP2 Max 1920x1080
> 
> 2 4K Cluster windows with AFBC/RFBC, line RGB and YUV
> 4 Esmart windows with line RGB/YUV support:
> Esmart0/1: 4K
> Esmart2/3: 2k, or worked together as a single 4K plane at shared
> line buffer mode.
> 
> Compared to the previous VOP, another difference is that each VP
> has its own independent vsync interrupt number.
> 
> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
> Tested-by: Michael Riesch <michael.riesch@wolfvision.net> # on RK3568
> Tested-by: Detlev Casanova <detlev.casanova@collabora.com>

some minor style things, but overall looks really good

> @@ -2665,6 +2721,32 @@ static int vop2_bind(struct device *dev, struct device *master, void *data)
>  	if (ret)
>  		return ret;
>  
> +	if (vop2->version >= VOP_VERSION_RK3576) {
> +		struct drm_crtc *crtc;
> +
> +		drm_for_each_crtc(crtc, drm) {
> +			struct vop2_video_port *vp = to_vop2_video_port(crtc);
> +			int vp_irq;
> +			const char *irq_name = devm_kasprintf(dev, GFP_KERNEL, "vp%d", vp->id);
> +
> +			if (!irq_name)
> +				return -ENOMEM;
> +
> +			vp_irq = platform_get_irq_byname(pdev, irq_name);
> +			if (vp_irq < 0) {
> +				DRM_DEV_ERROR(dev, "cannot find irq for vop2 vp%d\n", vp->id);

return dev_err_probe

> +				return vp_irq;
> +			}
> +
> +			ret = devm_request_irq(dev, vp_irq, rk3576_vp_isr, IRQF_SHARED, irq_name,
> +					       vp);
> +			if (ret) {
> +				DRM_DEV_ERROR(dev, "request irq for vop2 vp%d failed\n", vp->id);

return dev_err_probe

> +				return ret;
> +			}
> +		}
> +	}
> +
>  	ret = vop2_find_rgb_encoder(vop2);
>  	if (ret >= 0) {
>  		vop2->rgb = rockchip_rgb_init(dev, &vop2->vps[ret].crtc,

> +static void rk3576_vop2_setup_overlay(struct vop2_video_port *vp)
> +{
> +	struct vop2 *vop2 = vp->vop2;
> +	struct drm_crtc *crtc = &vp->crtc;
> +	struct drm_plane *plane;
> +
> +	vp->win_mask = 0;
> +
> +	drm_atomic_crtc_for_each_plane(plane, crtc) {
> +		struct vop2_win *win = to_vop2_win(plane);
> +
> +		win->delay = win->data->dly[VOP2_DLY_MODE_DEFAULT];
> +

nit: we probably don't need this empty line

> +		vp->win_mask |= BIT(win->data->phys_id);
> +
> +		if (vop2_cluster_window(win))
> +			vop2_setup_cluster_alpha(vop2, win);
> +	}
> +
> +	if (!vp->win_mask)
> +		return;
> +
> +	rk3576_vop2_setup_layer_mixer(vp);
> +	vop2_setup_alpha(vp);
> +	rk3576_vop2_setup_dly_for_windows(vp);
> +}

Heiko