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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0d72844f-fdc3-4fcb-8590-ee8512179d8f@ti.com>
Date: Wed, 18 Jun 2025 14:19:39 +0530
From: Jayesh Choudhary <j-choudhary@...com>
To: Maxime Ripard <mripard@...nel.org>
CC: <jyri.sarha@....fi>, <dri-devel@...ts.freedesktop.org>, <devarsht@...com>,
        <tomi.valkeinen@...asonboard.com>, <maarten.lankhorst@...ux.intel.com>,
        <tzimmermann@...e.de>, <airlied@...il.com>, <simona@...ll.ch>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/tidss: Decouple max_pclk from tidss feats to remove
 clock dependency

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@...com>
> 
> 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