Re: [PATCH] drm/rockchip: vop2: Add mode valid callback for crtc

["Heiko Stübner" <heiko@sntech.de>]

Hi Andy,

Am Freitag, 16. Januar 2026, 01:59:49 Mitteleuropäische Normalzeit schrieb Andy Yan:
> From: Andy Yan <andy.yan@rock-chips.com>

when resending an unmodified patch, please mark the subject as
[PATCH RESEND] ....

> Filter the mode that can't output by the crtc.

In commit 8e140cb60270 ("drm/rockchip: vop: limit maximum resolution to
hardware capabilities") which introduced the similar check on VOP(1), we
had additional information, in that the VOP1 hardware does not have
an output height limit. Is the same true for VOP2 ?

Because then I'd like to extend the commit description to something like:
======= 8< =======
The different VOP variants support different maximum resolutions. Reject
resolutions that are not supported by a specific variant.

Only the output width is checked because the hardware itself does not
have a hard output height limit.
======= 8< =======

Because when someone sees the code later they might ask why the height
is not checked, so having that in the commit description allows us all to
remember why the check is this specific way :-)

Thanks
Heiko

> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index 498df0ce4680..74fba29bfff3 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -1439,6 +1439,17 @@ static void vop2_crtc_disable_vblank(struct drm_crtc *crtc)
>  	vop2_crtc_disable_irq(vp, VP_INT_FS_FIELD);
>  }
>  
> +static enum drm_mode_status vop2_crtc_mode_valid(struct drm_crtc *crtc,
> +						 const struct drm_display_mode *mode)
> +{
> +	struct vop2_video_port *vp = to_vop2_video_port(crtc);
> +
> +	if (mode->hdisplay > vp->data->max_output.width)
> +		return MODE_BAD_HVALUE;
> +
> +	return MODE_OK;
> +}
> +
>  static bool vop2_crtc_mode_fixup(struct drm_crtc *crtc,
>  				 const struct drm_display_mode *mode,
>  				 struct drm_display_mode *adj_mode)
> @@ -1884,6 +1895,7 @@ static void vop2_crtc_atomic_flush(struct drm_crtc *crtc,
>  
>  static const struct drm_crtc_helper_funcs vop2_crtc_helper_funcs = {
>  	.mode_fixup = vop2_crtc_mode_fixup,
> +	.mode_valid = vop2_crtc_mode_valid,
>  	.atomic_check = vop2_crtc_atomic_check,
>  	.atomic_begin = vop2_crtc_atomic_begin,
>  	.atomic_flush = vop2_crtc_atomic_flush,
>