Hello Alexey.
On 2024-05-29 11:57, Alexey Charkov wrote:
On Wed, May 29, 2024 at 6:14 AM Dragan Simic <dsimic@xxxxxxxxxxx>
wrote:
Rename and modify the RK3588 dtsi files a bit, to make preparations
for
the ability to specify different CPU and GPU OPPs for each of the
supported
RK3588 SoC variants, including the RK3588J.
As already discussed, [1][2][3] some of the different RK3588 SoC
variants
require different OPPs, and it makes more sense to have the OPPs
already
defined when a board dts includes one of the SoC dtsi files
(rk3588.dtsi,
rk3588j.dtsi or rk3588s.dtsi), rather than requiring the board dts
file to
also include a separate rk3588*-opp.dtsi file. The choice of the SoC
variant
is already made by the inclusion of the SoC dtsi file, and it doesn't
make
much sense to, effectively, allow the board dts file to include and
use an
incompatible set of OPPs for the already selected SoC variant.
Indeed, including just one .dtsi for the correct SoC variant and not
having to bother about what other bits and pieces are required to use
the full SoC functionality sounds great! Thanks for putting this
together so promptly!
You're welcome. :)
Some considerations below.
No intended functional changes are introduced. (Still to be
additionally
verified if the patch moves forward from the RFC state.)
[1]
https://lore.kernel.org/linux-rockchip/646a33e0-5c1b-471c-8183-2c0df40ea51a@xxxxxxxxx/
[2]
https://lore.kernel.org/linux-rockchip/CABjd4Yxi=+3gkNnH3BysUzzYsji-=-yROtzEc8jM_g0roKB0-w@xxxxxxxxxxxxxx/
[3]
https://lore.kernel.org/linux-rockchip/035a274be262528012173d463e25b55f@xxxxxxxxxxx/
Signed-off-by: Dragan Simic <dsimic@xxxxxxxxxxx>
---
...inctrl.dtsi => rk3588-common-pinctrl.dtsi} | 0
Renaming the pinctrl includes seems superfluous - maybe keep them as
they were to minimize churn?
Believe it or not, the same thoughts crossed my mind. However,
after thinking a bit about it, I concluded that renaming the pinctrl
.dtsi files as well makes the overall naming of the reworked RK3588
.dtsi files more consistent.
It's also rather neat to have the "common" and "fullfat" file pairs
together in the arch/arm64/boot/dts/rockchip directory listing, when
it's sorted by the file name, which is the default in most (if not
all) environments.
.../{rk3588s.dtsi => rk3588-common.dtsi} | 2 +-
...nctrl.dtsi => rk3588-fullfat-pinctrl.dtsi} | 0
.../{rk3588.dtsi => rk3588-fullfat.dtsi} | 4 +-
To me, "fullfat" doesn't look super descriptive, albeit fun :)
Yeah, I resorted to "fullfat" as some kind of a funny option. :)
That's for the "beefy" SoC variants, so it kind of fits well. :)
How about we rename the existing rk3588.dtsi and rk3588s.dtsi to
something like rk3588-devices.dtsi and rk3588s-devices.dtsi
(preserving the inheritance between them), and then I add
rk3588s-opp.dtsi and rk3588j-opp.dtsi in a follow-up patch?
The trouble with including "devices" into the filenames is, well,
the fact that those files isn't about any devices, but about the
SoC variants. Thus, "devices" simply wouldn't fit there.
Moreover, in the envisioned scheme there should be no separate
OPP .dtsi files; the OPP data should go directly into the new
rk3588.dtsi, rk3588s.dtsi and rk3588j.dtsi files, where it actually
belongs. That's why those .dtsi files exist and are mostly empty,
at least the way I see it. I'll get back to this below.
Then we keep "thin" versions of rk3588.dtsi, rk3588s.dtsi and
rk3588j.dtsi for board files to include. The mix-and-match of
different on-chip devices and different OPPs then gets transparently
represented within those "thin" .dtsi, something like this:
rk3588.dtsi:
#include "rk3588-devices.dtsi"
#include "rk3588s-opp.dtsi"
rk3588s.dtsi:
#include "rk3588s-devices.dtsi"
#include "rk3588s-opp.dtsi"
rk3588j.dtsi:
#include "rk3588-devices.dtsi"
#include "rk3588j-opp.dtsi"
Such a layout, in general, has also crossed my mind, but my conclusion
was that having the per-SoC-variant OPP data specified directly in the
reworked rk3588.dtsi, rk3588s.dtsi and rk3588j.dtsi files is a better
option in the long term, even if we end up that way with rk3588.dtsi and
rk3588s.dtsi repeating most (if not all) of the same OPP data.
That way we'll have no roadblocks if, at some point, we end up with
having
different OPPs defined for the RK3588 and the RK3588S variants. Or
maybe
even for the RK3582, which we don't know much about yet.
arch/arm64/boot/dts/rockchip/rk3588.dtsi | 414 +--
arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 6 +-
arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 2671
+----------------
Rename detection didn't do a particularly great job here - wonder if
we can do anything about it to minimize the patch size and ensure that
the change history is preserved for git blame...
Yeah, that bothered me as well. :/ Unfortunately, I don't think that
much can be done there, but I'll try fiddling with the values for the
--find-renames parameter for git-diff(1).
