Re: [PATCH v1 2/2] drm/tegra: dc: rgb: Allow changing PLLD rate on Tegra30+

[Dmitry Osipenko <digetx@gmail.com>]

06.10.2021 21:27, Dmitry Osipenko пишет:
> 06.10.2021 21:13, Thierry Reding пишет:
>> On Thu, Sep 30, 2021 at 01:28:05AM +0300, Dmitry Osipenko wrote:
>>> Asus Transformer TF700T is a Tegra30 tablet device which uses RGB->DSI
>>> bridge that requires a precise clock rate in order to operate properly.
>>> Tegra30 has a dedicated PLL for each display controller, hence the PLL
>>> rate can be changed freely. Allow PLL rate changes on Tegra30+ for RGB
>>> output. Configure the clock rate before display controller is enabled
>>> since DC itself may be running off this PLL and it's not okay to change
>>> the rate of the active PLL that doesn't support dynamic frequency
>>> switching since hardware will hang.
>>>
>>> Tested-by: Maxim Schwalm <maxim.schwalm@gmail.com> #TF700T
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  drivers/gpu/drm/tegra/dc.c  | 27 ++++++++++++--------
>>>  drivers/gpu/drm/tegra/dc.h  |  1 +
>>>  drivers/gpu/drm/tegra/rgb.c | 49 +++++++++++++++++++++++++++++++++++--
>>>  3 files changed, 65 insertions(+), 12 deletions(-)
>>
>> This seems overly complicated. I especially don't like the way how
>> clocks are looked up with clk_get_sys() and then used in the comparison.
>> Could this not be achieved by using assigned-clocks and friends
>> properties in DT?
> 
> Assigned-clocks have nothing to do with this patch. We need to check
> whether PLLD *is* already pre-assigned as the parent.
> 
> Adding properties for describing the clk parents is overly complicated,
> clk_get_sys() is a much simpler solution that doesn't involve extra DT
> changes.
> 
> BTW, assigned-clocks can't be used for display controller  because
> controller is usually turned on during boot and reparenting of active DC
> will hang machine.
> 

To make it more clear, we change the DC's "parent" in TF700T device-tree
to PLLD [1] to match the bootloader's configuration, otherwise it will
hang on boot since default "parent" in tegra30.dtsi is PLLP, and thus,
tegra_dc_rgb_probe() will try to reparent it to PLLP. Display of TF700T
won't work with PLLP anyways since it needs a specific clock rate.

[1]
https://github.com/grate-driver/linux/blob/master/arch/arm/boot/dts/tegra30-asus-tf700t.dts#L13