Re: [PATCH] drm/tidss: Decouple max_pclk from tidss feats to remove clock dependency

[Jayesh Choudhary]

Hello Maxime,

On 18/06/25 14:02, Maxime Ripard wrote:
> Hi,
>
> On Wed, Jun 18, 2025 at 01:28:04PM +0530, Jayesh Choudhary wrote:
> > TIDSS hardware by itself does not have variable max_pclk for each VP.
> > Each VP supports a fixed maximum pixel clock. K2 devices and AM62*
> > devices uses "ultra-light" version where each VP supports a max of
> > 300MHz whereas J7* devices uses TIDSS where all VP can support a
> > max pclk of 600MHz.
> > The limitation that has been modeled till now comes from the clock
> > (PLL can only be programmed to a particular max value). Due to this
> > we end up using different compatible for each SoC when the clocking
> > architecture changes for VPs, even when the hardware is essentially
> > the same.
> > max_pclk cannot be entirely removed since the display controller
> > should tell if a particular mode clock can be supported or not in crtc's
> > "mode_valid()" call. So remove "max_pclk_khz" from the static display
> > feat and add it to "tidss_device" structure which would be modified in
> > runtime. In mode_valid() call, check if a best frequency match for mode
> > clock can be found or not using "clk_round_rate()". Based on that,
> > propagate "max_pclk" and check max_clk again only if the requested mode
> > clock is greater than saved value. (As the preferred display mode is
> > usually the max resolution, driver ends up checking the maximum clock
> > the first time itself which is used in subsequent checks)
> > Since TIDSS display controller provides clock tolerance of 5%, we use
> > this while checking the max_pclk. Also, move up "dispc_pclk_diff()"
> > before it is called.
> >
> > This will make the existing compatibles reusable.
> >
> > Signed-off-by: Jayesh Choudhary <j-choudhary@xxxxxx>
>
> We've generally used 0.5% in drivers with similar requirements, because
> that's what VESA requires for some mode generation techniques (and
> what's required by HDMI too iirc).
>
> We've never had to revisit it on any driver afaik, so unless you have a
> specific reason to do so, I'd really prefer if you stuck with that as
> well.

I agree with 0.5% but I was just trying to keep it in sync with existing
tidss tolerance.

In tidss_crtc.c when we set the clk rate in crtc_atomic_enable() using
dispc_vp_set_clk_rate(), we are giving it 5% tolerance:
https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/tidss/tidss_dispc.c#n1446

So I kept it 5%.

Warm Regards,
Jayesh


> Maxime