Re: [PATCH v4 1/4] drm/rockchip: vop: limit maximium resolution to hardware capabilities

[Sascha Hauer <s.hauer@pengutronix.de>]

On Tue, Feb 07, 2023 at 10:46:49AM +0000, Jonas Karlman wrote:
> Hi Sascha,
> On 2023-02-07 09:44, Sascha Hauer wrote:
> > The different VOP variants support different maximum resolutions. Reject
> > resolutions that are not supported by a specific variant.
> > 
> > This hasn't been a problem in the upstream driver so far as 1920x1080
> > has been the maximum resolution supported by the HDMI driver and that
> > resolution is supported by all VOP variants. Now with higher resolutions
> > supported in the HDMI driver we have to limit the resolutions to the
> > ones supported by the VOP.
> > 
> > The actual maximum resolutions are taken from the Rockchip downstream
> > Kernel.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> > 
> > Notes:
> >     Changes since v3:
> >     - new patch
> > 
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 15 ++++++++++
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  2 ++
> >  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 31 +++++++++++++++++++++
> >  3 files changed, 48 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > index fa1f4ee6d1950..96b6bd8d17803 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > @@ -1174,6 +1174,20 @@ static void vop_crtc_disable_vblank(struct drm_crtc *crtc)
> >  	spin_unlock_irqrestore(&vop->irq_lock, flags);
> >  }
> >  
> > +static enum drm_mode_status vop_crtc_mode_valid(struct drm_crtc *crtc,
> > +						const struct drm_display_mode *mode)
> > +{
> > +	struct vop *vop = to_vop(crtc);
> > +
> > +	if (vop->data->max_xres && mode->hdisplay > vop->data->max_xres)
> > +		return MODE_BAD_HVALUE;
> > +
> > +	if (vop->data->max_yres && mode->vdisplay > vop->data->max_yres)
> > +		return MODE_BAD_VVALUE;
> > +
> > +	return MODE_OK;
> > +}
> > +
> >  static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
> >  				const struct drm_display_mode *mode,
> >  				struct drm_display_mode *adjusted_mode)
> > @@ -1585,6 +1599,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
> >  }
> >  
> >  static const struct drm_crtc_helper_funcs vop_crtc_helper_funcs = {
> > +	.mode_valid = vop_crtc_mode_valid,
> >  	.mode_fixup = vop_crtc_mode_fixup,
> >  	.atomic_check = vop_crtc_atomic_check,
> >  	.atomic_begin = vop_crtc_atomic_begin,
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > index 8502849833d93..5c4875ca3f270 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > @@ -225,6 +225,8 @@ struct vop_data {
> >  	const struct vop_win_data *win;
> >  	unsigned int win_size;
> >  	unsigned int lut_size;
> > +	unsigned int max_xres;
> > +	unsigned int max_yres;
> 
> I would suggest using the same struct vop_rect max_input/output as the
> vop2 driver instead of handling this differently between the two.

As it was me who introduced struct vop_rect in the vop2 driver I
probably have to agree ;)

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-5555 |