Re: [PATCH RFC v2] drm/rockchip: vop2: Add clock rate mode check

[Alexey Charkov]

On Fri, Jun 5, 2026 at 7:58 PM Alexey Charkov <alchark@xxxxxxxxx> wrote:
>
> On Fri, Apr 10, 2026 at 2:37 AM Sebastian Reichel
> <sebastian.reichel@xxxxxxxxxxxxx> wrote:
> >
> > The display might offer modes, which exceed the maximum clock rate of a
> > video output. This usually happens for displays that offer refresh rates
> > above 60 Hz. This results in no picture (or a broken one) being displayed
> > without manual intervention. Fix this by teaching the driver about the
> > maximum achievable clock rates for each video port.
> >
> > The information about the maximum clock rates for each video channel and
> > the tip about multiple pixels being processed per clock were provided by
> > Andy Yan and roughly checked against the information available in the
> > datasheet (which specifies limits like "2560x1600@60Hz with 10-bit"
> > instead of a specific pixel rate).
> >
> > For the video ports supporting a 600 MHz input clock, there is some
> > logic to handle up to 4 pixels in parallel when needed resulting in
> > the extra multiplier.
> >
> > Suggested-by: Andy Yan <andy.yan@xxxxxxxxxxxxxx>
> > Link: https://lore.kernel.org/linux-rockchip/1528d788.186b.19d08ed974c.Coremail.andyshrk@xxxxxxx/
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
> > ---
> > I've kept the RFC tag, as I'm not sure about the 4x parallel pixel
> > processing. IIUIC all of the video ports with a maximum of 600 MHz
> > input clock support it, considering they can go to 4K @ 120Hz,
> > which is above 1.2GHz while Andy mentioned a max. support clock rate
> > of 600 MHz.
> > ---
> > Changes in v2:
> > - Link to v1: https://lore.kernel.org/r/20260217-vop2-clk-rate-check-v1-1-989b569119ba@xxxxxxxxxxxxx
> > - based on v7.0-rc7
> > - rename max_clock_rate into max_pixel_clock_rate to distinguish from
> >   input clock
> > - update max clock rates to the numbers provided by Andy Yan with
> >   extra 4x multiplier for 4K 120Hz VPs
> > ---
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c |  3 +++
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop2.h |  1 +
> >  drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 10 ++++++++++
> >  3 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > index a195f5c819a2..35a0edda5375 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > @@ -1434,6 +1434,9 @@ static enum drm_mode_status vop2_crtc_mode_valid(struct drm_crtc *crtc,
> >         if (mode->hdisplay > vp->data->max_output.width)
> >                 return MODE_BAD_HVALUE;
> >
> > +       if (mode->clock > vp->data->max_pixel_clock_rate / 1000)
> > +               return MODE_CLOCK_HIGH;
> > +
> >         return MODE_OK;
> >  }
> >
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> > index 9124191899ba..fd46913f3346 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
> > @@ -225,6 +225,7 @@ struct vop2_video_port_data {
> >         u16 gamma_lut_len;
> >         u16 cubic_lut_len;
> >         struct vop_rect max_output;
> > +       u32 max_pixel_clock_rate;
> >         const u8 pre_scan_max_dly[4];
> >         unsigned int offset;
> >         /**
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> > index f3950e8476a7..6ae3d506c476 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> > @@ -559,18 +559,21 @@ static const struct vop2_video_port_data rk3568_vop_video_ports[] = {
> >                 .gamma_lut_len = 1024,
> >                 .cubic_lut_len = 9 * 9 * 9,
> >                 .max_output = { 4096, 2304 },
> > +               .max_pixel_clock_rate = 600000000U,
> >                 .pre_scan_max_dly = { 69, 53, 53, 42 },
> >                 .offset = 0xc00,
> >         }, {
> >                 .id = 1,
> >                 .gamma_lut_len = 1024,
> >                 .max_output = { 2048, 1536 },
> > +               .max_pixel_clock_rate = 200000000U,
> >                 .pre_scan_max_dly = { 40, 40, 40, 40 },
> >                 .offset = 0xd00,
> >         }, {
> >                 .id = 2,
> >                 .gamma_lut_len = 1024,
> >                 .max_output = { 1920, 1080 },
> > +               .max_pixel_clock_rate = 150000000U,
> >                 .pre_scan_max_dly = { 40, 40, 40, 40 },
> >                 .offset = 0xe00,
> >         },
> > @@ -775,6 +778,7 @@ static const struct vop2_video_port_data rk3576_vop_video_ports[] = {
> >                 .gamma_lut_len = 1024,
> >                 .cubic_lut_len = 9 * 9 * 9, /* 9x9x9 */
> >                 .max_output = { 4096, 2304 },
> > +               .max_pixel_clock_rate = 600000000U * 4,
>
> Hi Sebastian,
>
> I've tested it on a bunch of different displays with RK3576 while also
> giving VP0 an unlimited PLL to work with (and also expanding its rates
> table for each of the displays' EDID preferred pixel clocks), and I'm
> pretty sure this should be * 2 here, rather than * 4. That would also
> align with ".pixel_rate = 2" just below.
>
> In its current form, some of the advanced displays with 240 Hz modes
> by default choose modes leading to visual artifacts indicative of
> overclocking strain of the graphics pipeline.
>
> The rest look plausible, so with that change please feel free to include:
>
> Tested-by: Alexey Charkov <alchark@xxxxxxxxxxx> # RK3576

Sorry, wrong mailbox.

Tested-by: Alexey Charkov <alchark@xxxxxxxxxxx> # RK3576

Best regards,
Alexey