Hi Heiko, On 4/25/24 9:55 PM, Heiko Stuebner wrote:
From: Heiko Stuebner <heiko.stuebner@xxxxxxxxx> The rk3588 VOP2 has 4 video-ports, yet the driver currently only configures the first 3, as used on the rk3568.
I'm wondering whether we should update the drawing at the top of the driver then?
Add another block to configure the vp3 as well, if applicable. Fixes: 5a028e8f062f ("drm/rockchip: vop2: Add support for rk3588") Signed-off-by: Heiko Stuebner <heiko.stuebner@xxxxxxxxx> --- drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 12 ++++++++++++ drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 1 + 2 files changed, 13 insertions(+) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c index 523880a4e8e74..1a327a9ed7ee4 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c @@ -2303,6 +2303,7 @@ static void vop2_setup_alpha(struct vop2_video_port *vp) static void vop2_setup_layer_mixer(struct vop2_video_port *vp) { struct vop2 *vop2 = vp->vop2; + const struct vop2_data *vop2_data = vop2->data; struct drm_plane *plane; u32 layer_sel = 0; u32 port_sel; @@ -2344,6 +2345,17 @@ static void vop2_setup_layer_mixer(struct vop2_video_port *vp) else port_sel |= FIELD_PREP(RK3568_OVL_PORT_SET__PORT2_MUX, 8);+ /* configure vp3 */+ if (vop2_data->soc_id == 3588) {
I think it'd be smarter to check against vop2->data->nr_vps >= 4; so that we don't need to maintain a list of SoCs that support a specific number of video ports.
+ struct vop2_video_port *vp3 = &vop2->vps[3];
This is always possible because vps is statically allocated for 4 items, c.f. struct vop2_video_port vps[ROCKCHIP_MAX_CRTC]; so we don't necessarily need it in this specific location and can group it with the others. Cosmetic suggestion though.
Otherwise, the change itself makes sense to me, so: Reviewed-by: Quentin Schulz <quentin.schulz@xxxxxxxxx> Thanks! Quentin
