Re: [PATCH v4 0/3] drm/tidss: Add OLDI bridge support

[Aradhya Bhatia <aradhya.bhatia@linux.dev>]

Hi Alexander,

Thank you for testing and reviewing the patches!

On 19/03/25 23:30, Sverdlin, Alexander wrote:
> Thank you for the patches, Aradhya!
> 
> On Sun, 2024-11-24 at 20:06 +0530, Aradhya Bhatia wrote:
>> Regardless, I'd appreciate it if somebody can test it, and report back if they
>> observe any issues.
> 
> I've tried to test the patchset with necessary pre-requisites and DT additions
> with a single channel LVDS pannel and while I'm not successful yet, I've also noticed
> the following warning:
> 
> tidss 30200000.dss: vp0: Clock rate 24285714 differs over 5% from requested 37000000
> 
> even though later the clock seems to be correctly set up:
> 
> $ cat /sys/kernel/debug/clk/clk_summary 
> 
>                                  enable  prepare  protect                                duty  hardware                            connection
>    clock                          count    count    count        rate   accuracy phase  cycle    enable   consumer                         id
> ---------------------------------------------------------------------------------------------------------------------------------------------
>  clk:186:6                           1       1        0        250000000   0          0     50000      Y   30200000.dss                    fck                      
>                                                                                                            deviceless                      no_connection_id         
>  clk:186:4                           0       0        0        0           0          0     50000      Y   deviceless                      no_connection_id         
>  clk:186:3                           0       0        0        170000000   0          0     50000      Y   deviceless                      no_connection_id         
>     clk:186:2                        0       0        0        170000000   0          0     50000      Y      30200000.dss                    vp2                      
>                                                                                                               deviceless                      no_connection_id         
>  clk:186:0                           1       1        0        259090909   0          0     50000      Y   oldi@0                          serial                   
>                                                                                                            deviceless                      no_connection_id         
>     clock-divider-oldi               1       1        0        37012987    0          0     50000      Y      30200000.dss                    vp1                      
> 
> Looks like "clock-divider-oldi" doesn't propagate clk_set_rate() to the parent,
> but the parent is being set later independently?
> 

Yes, you are right. The "clock-divider-oldi" does not propagate the
clk_set_rate() to the parent.

The actual parent clock is now owned by the oldi bridge driver, and that
is what sets the clock rate (7 * pixel freq). The equivalent action from
tidss vp (DRM crtc) is a no op in cases of OLDI display pipeline.

Usually, the DRM crtc is enabled first, before _any_ of the DRM bridges
get pre_enabled or enabled.

Since, OLDI is a DRM bridge, the OLDI enable (and by extension the
actual clk_set_rate()) takes place _after_ the DRM crtc has been
enabled (which is why you see the parent being set later on).

DRM crtc enable is where tidss vp attempts (and fails) to set the clock
rate, causing the warning you see initially.


I have attempted to re-order the bridge pre_enable and crtc enable calls
in these patches[0], separately.

While you have mentioned that you did add the prerequisites, could you
confirm that you applied the (now older) dependency patch mentioned in
the v4 cover-letter[1]?
Ideally, you should not observe these concerns if [1] were successfully
applied.

More importantly, if you are already on latest linux-next, I would
request you to use v6 of this OLDI series[2], along with the latest
dependency patches[0], as the older dependency patch is simply not
applicable on latest kernel anymore! =)

I'd appreciate it if you are able to test the latest revisions on your
single-link setup, and report back any issue you see! Thank you! =)


-- 
Regards
Aradhya



[0]: Pre Requisite patches that re-order crtc/encoder/bridge sequences
(latest revision).

a. ("drm/atomic-helper: Refactor crtc & encoder-bridge op loops into
separate functions")
https://lore.kernel.org/all/20250226155737.565931-3-aradhya.bhatia@linux.dev/

b. ("drm/atomic-helper: Separate out bridge pre_enable/post_disable from
enable/disable")
https://lore.kernel.org/all/20250226155737.565931-4-aradhya.bhatia@linux.dev/

c. ("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
https://lore.kernel.org/all/20250226155737.565931-5-aradhya.bhatia@linux.dev/


[1]: Dependency patch mentioned in v4 OLDI series.
("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
https://lore.kernel.org/all/20240622110929.3115714-11-a-bhatia1@ti.com/


[2]: Latest OLDI series (v6)
https://lore.kernel.org/all/20250226181300.756610-1-aradhya.bhatia@linux.dev/