Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RK3588 edge kernel: Add support for Linux 6.10 + introduce stable current branch (6.8) #6699

Merged
merged 15 commits into from
Jun 12, 2024

Conversation

ColorfulRhino
Copy link
Collaborator

@ColorfulRhino ColorfulRhino commented Jun 6, 2024

Description

The general improvements for RK3588 on 6.9 and 6.10 weren't as many as we've hoped, but still this is what was mainlined:

  • USB3 DRD support, including PHY usbdp
  • HDMI PHY
  • GPU

We already had most of those features patched in via patches, but this gives us the chance to finally be able to remove those and stay closer to mainline.
-> PR summary: (+5,895 - 28,098) ~= 22,000 lines less 👍 (meaning less hassles in the future, less maintenance needed) (not anymore since this PR now introduces a current kernel branch for RK3588)

In addition to that, I have also updated our patch to support RK3588 thermal sensors and management, enabling and improving the use of cooling fans. This patch also includes updates for CpuFreq and OPP.

We had an old patch that patched in the HDMI Controller driver, but this patch does not build on 6.10. Fortunately, I was able to include a new, improved version for this driver, which is as recent as 1st of June. This driver patch set did not work, thus I fixed the old patch that wouldn't build on 6.10.

Since out "general-add-overlay-compilation-support.patch" became obsolete in Linux 6.9 (see AR-2352) , @paolosabatino developed an improved way on adding overlays as first seen in #6690. I have adapted this patch for 6.10, which also paves the way for other kernel bumps to 6.10 in the future.

In addition to that, there are some fixes included for Orange Pi 5 and Khadas Edge 2 patches, which were failing on 6.10.

Please see the commit messages for details.

How Has This Been Tested?

  • Compile success: ./compile.sh BOARD=nanopc-cm3588-nas BRANCH=edge RELEASE=trixie EXPERT=yes KERNEL_CONFIGURE=no BUILD_MINIMAL=no BUILD_DESKTOP=no
  • HDMI does work, tested on NanoPi R6C (no desktop, only command line)
  • More general testing is needed

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

@github-actions github-actions bot added size/large PR with 250 lines or more Hardware Hardware related like kernel, U-Boot, ... labels Jun 6, 2024
@ColorfulRhino ColorfulRhino added the 08 Milestone: Third quarter release label Jun 6, 2024
@efectn
Copy link
Member

efectn commented Jun 6, 2024

I can check edge2 and opi5 patches weekend

@EvilOlaf
Copy link
Member

EvilOlaf commented Jun 6, 2024

I can test opi5+ hdmi by chance

@amazingfate
Copy link
Contributor

armsom-sige7 has been upstreamed to v6.10. I will push a commit later to delete the dts under dt.

@ColorfulRhino
Copy link
Collaborator Author

armsom-sige7 has been upstreamed to v6.10. I will push a commit later to delete the dts under dt.

Doesn't the current dts in the dt folder have more stuff like hdmi0 and such? I think maintaining a dts file instead of patches fiddling with the mainline dts is easier. For example fixing the Orange Pi 5 patch for 6.10 cost me quite a lot of time. If it's in the dt folder, there won't be any conflicts in the future :)

@rpardini
Copy link
Member

rpardini commented Jun 6, 2024

I think maintaining a dts file instead of patches fiddling with the mainline dts is easier. For example fixing the Orange Pi 5 patch for 6.10 cost me quite a lot of time. If it's in the dt folder, there won't be any conflicts in the future :)

I think if a given file is upstream, the correct thing is to convert the bare-DT into a patch.

Otherwise, there's a tendency that although "there's no conflicts", in practice/runtime there might be problems -- the bare-DT will simply overwrite the mainline one, and we will have a very hard time understanding what the hell is going on. eg: suppose Amazingfate forgot this, we might be a couple months before we even realized that board had an old DT vs a new kernel.

It has happened in the past that we had patches that rewrote most of a DT on purpose, but I think those are mostly gone by now. (Helios64, if memory doesn't fail me, and others).

At the same time, being able to quickly overwrite a DT without fiddling with patches is useful for development and trying out things, etc -- so I hesitate to prevent overwrites from happening in the patching tool. Maybe a large warning would be enough?


Either way, thanks for the work! -- I shall cherry pick right now & test on nanopct6

@ColorfulRhino
Copy link
Collaborator Author

I think if a given file is upstream, the correct thing is to convert the bare-DT into a patch.

Otherwise, there's a tendency that although "there's no conflicts", in practice/runtime there might be problems -- the bare-DT will simply overwrite the mainline one, and we will have a very hard time understanding what the hell is going on. eg: suppose Amazingfate forgot this, we might be a couple months before we even realized that board had an old DT vs a new kernel.

Your reasonign does make sense! Even though I'm still not a huge fan of patches in general. Maybe I just need to figure out a faster workflow to create or apply patches :) One of my biggest time losers is that my IDE can't edit kernel files in the cache/sources dir when using kernel-patch since the cache folder is only writable as root user. And I can't sudo git apply/format-patch because the patch will reference the wrong folder structure (e.g. scripts/checkpatch.pl vs cache/sources/linux-kernel-worktree/6.10__rockchip-rk3588__arm64/scripts/checkpatch.pl). 🙃

At the same time, being able to quickly overwrite a DT without fiddling with patches is useful for development and trying out things, etc -- so I hesitate to prevent overwrites from happening in the patching tool. Maybe a large warning would be enough?

Yes, definitely don't remove it, I use the overwriting a lot :)

@rpardini
Copy link
Member

rpardini commented Jun 7, 2024

Cherry picked and built. Boots. Seems stable. Haven't tested any HDMI / GPU or such since my T6 runs headless for now.

(The T6 needs a blob bump to be stable, will PR separately since it's in board file).

@ColorfulRhino
Copy link
Collaborator Author

Cherry picked and built. Boots. Seems stable. Haven't tested any HDMI / GPU or such since my T6 runs headless for now.

Awesome, thanks for testing!
@amazingfate does HDMI work on Sige7?

(The T6 needs a blob bump to be stable, will PR separately since it's in board file).

U-Boot blob?

@EvilOlaf
Copy link
Member

EvilOlaf commented Jun 7, 2024

OPi5+ does not boot. Does not stop always on the same step. I assume this is because stuff is executed in parallel and order might change randomly.

Two examples:
http://paste.armbian.com/yuyabocopa.yaml (HDMI, ethernet, mouse, keyboard and NVMe attached)
http://paste.armbian.com/eyabijeqiz.yaml (I disconnected HDMI and ethernet for this one beforehand to see if things change. Spoiler: it did not)

@amazingfate
Copy link
Contributor

armsom sige7 also not boot

[    1.481918] Internal error: Oops: 0000000096000004 [#1] PREEMModules linked in:
[    1.482768] CPU: 1 PID: 21 Comm: cpuhp/1 ardware name: ArmSoM Sige7 (DT)
[    1.483865] pstate: a0400009 pc : blk_mq_hctx_notify_online+0x34/0xb0
[    1.484941] lr : c33bd60
[    1.485675] x29: ffff80008233bd60 x28: 00000000000000 x25: 00000000000000ec x24: 0000000000000000
[    1.486962] x23[    1.487606] x20: ffff0002fee2aca0 x19: ffff0001047ad000 x18: 000f2b5503510 x15: 0000000000000204
[    1.488892] x14: 0000000489535] x11: ffffffffffffffff x10: 0000000000000052 x9 : 1fffe00600 x6 : 0000000000000001
online+0x34/0xb05 : ffff800081c9d000  : 0000000000000000 x1 : 0000000000000000 x0 : ffff000107f42120
[    1.492749]  cpuhp_invoke_callback+0x2d8/0x10]  smpboot_thread_fn+0x23c/0x268
[    1.493883]  kthread+0x112] Code: 79422a61 9100e040 f941a842 8b011041 (f9400421) 

@amazingfate
Copy link
Contributor

After disabling the hdmi dts patch, I can boot now. So the hdmi driver patch should have issues.

@ColorfulRhino
Copy link
Collaborator Author

OPi5+ does not boot

armsom sige7 also not boot

After disabling the hdmi dts patch, I can boot now. So the hdmi driver patch should have issues.

Thanks for testing!

I have discovered the same. Booting a board without the HDMI nodes enabled in the dts file OR removing the patch series (0161-0174) does not have any problem.

So the patched in HDMI controller driver might have some problems. I may investigate more about this issue, but @amazingfate is much more versed in HDMI stuff than me since I don't use HDMI stuff much.

@ColorfulRhino
Copy link
Collaborator Author

ColorfulRhino commented Jun 7, 2024

I tried building the current Collabora 6.10 kernel for comparison, and their kernel doesn't even build, at least not with Armbian (because warning is treated as error?)

Collabora rk3588-hdmi-bridge-v6.10-rc1:

[🔨]   drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c: In function 'rockchip_hdmi_parse_dt':
[🔨]   drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c:280:40: error: implicit declaration of function 'devm_gpiod_get_optional'; did you mean 'devm_clk_get_optional'? [-Werror=implicit-function-declaration]
[🔨]     280 |                 hdmi->qp_enable_gpio = devm_gpiod_get_optional(hdmi->dev, "enable",
[🔨]         |                                        ^~~~~~~~~~~~~~~~~~~~~~~
[🔨]         |                                        devm_clk_get_optional
[🔨]   drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c:281:64: error: 'GPIOD_OUT_HIGH' undeclared (first use in this function)
[🔨]     281 |                                                                GPIOD_OUT_HIGH);
[🔨]         |                                                                ^~~~~~~~~~~~~~
[🔨]   drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c:281:64: note: each undeclared identifier is reported only once for each function it appears in
[🔨]   drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c: In function 'dw_hdmi_rockchip_encoder_enable':
[🔨]   drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c:364:17: error: implicit declaration of function 'gpiod_set_value' [-Werror=implicit-function-declaration]
[🔨]     364 |                 gpiod_set_value(hdmi->qp_enable_gpio, 1);
[🔨]         |                 ^~~~~~~~~~~~~~~
[🔨]   cc1: some warnings being treated as errors
[🔨]   make[7]: *** [scripts/Makefile.build:244: drivers/gpu/drm/rockchip/dw_hdmi-rockchip.o] Error 1
[🔨]   make[6]: *** [scripts/Makefile.build:485: drivers/gpu/drm/rockchip] Error 2
[🔨]   make[5]: *** [scripts/Makefile.build:485: drivers/gpu/drm] Error 2
[🔨]   make[4]: *** [scripts/Makefile.build:485: drivers/gpu] Error 2

Soooooo things might be complicated...

@rpardini
Copy link
Member

rpardini commented Jun 9, 2024

Yeah:

  • On the T6, both 6.8.y (previous state) and this 6.10-rc2 boots. Couldn't get working HDMI (vs a 4k TV) with 6.10, but my HDMI cabling skills are terrible.
  • On the CM3588, 6.8.y boot, but this (6.10-rc2) hangs at boot about some HDMI stuff -- haven't captured the bootlog yet. @ColorfulRhino

On my fork, I've done some work to move things around (using archive/rockchip-rk3588-6.8) in a way we could keep the 6.8.y as some oldedge BRANCH while we work this. Yes I know "another nonstandard branch" but it's useful. Should I PR it?

@rpardini
Copy link
Member

rpardini commented Jun 9, 2024

On my fork, I've done some work to move things around (using archive/rockchip-rk3588-6.8) in a way we could keep the 6.8.y as some oldedge BRANCH while we work this. Yes I know "another nonstandard branch" but it's useful. Should I PR it?

Heck, we could even call it current.
Or, keep 6.8 as edge and call this 6.10-rc2 bleedingedge.
The possibilities are endless lol

@rpardini
Copy link
Member

rpardini commented Jun 9, 2024

(The T6 needs a blob bump to be stable, will PR separately since it's in board file).

U-Boot blob?

BL31/DDR, see #6711 -- I realize now that #6696 could have been avoided with new blobs, as they would match what's shipped from FriendlyELEC factory. It also doesn't hurt, though.

@ColorfulRhino
Copy link
Collaborator Author

ColorfulRhino commented Jun 9, 2024

On my fork, I've done some work to move things around (using archive/rockchip-rk3588-6.8) in a way we could keep the 6.8.y as some oldedge BRANCH while we work this. Yes I know "another nonstandard branch" but it's useful. Should I PR it?

Heck, we could even call it current. Or, keep 6.8 as edge and call this 6.10-rc2 bleedingedge. The possibilities are endless lol

No no no no no, let's better keep things simple and similar accross all Armbian kernels. 😀

If at all, I believe we should establish even more consistency accross boards and families, rather than spread them apart.
As in, if you choose current kernel, you will always be sure to receive a mainline LTS kernel (so current equals LTS stable) and edge is always the latest supported/maintained kernel version (even if that sometimes means that current and edge are on the same version. While edge may have some experimental stuff, current should be more stable.

That's mostly how it currently is I believe.
But I don't think we really need an "experimental" kernel in the main Armbian branch. The only thing holding 6.10 back is the HDMI stuff. I would be totally fine with shipping 6.10 without HDMI support, but the thing is that we shipped 6.8 with HDMI, so some users who don't follow the project might be confused.

@ColorfulRhino
Copy link
Collaborator Author

Update:

I have fixed HDMI :)

Tested with NanoPi R6C (command line over HDMI only, no desktop)

@ColorfulRhino
Copy link
Collaborator Author

@rpardini I have included your changes regarding patch dir and config renaming from #6717, but in two commits since I found your commit history a bit confusing, especially going via oldedge :)
Please let me know if that's okay for you 😄 I believe the result is the same.

@ColorfulRhino ColorfulRhino added the Needs review Seeking for review label Jun 12, 2024
@rpardini
Copy link
Member

I have fixed HDMI :)

🥳

Please let me know if that's okay for you

Excellent, was my intention all along.

I believe the result is the same.

It's better. oldedge was a middle state that shouldn't be there. Thanks!

ColorfulRhino and others added 15 commits June 12, 2024 17:53
- Bump mainline kernel from 6.10-rc1 to 6.10-rc2
- Remove patches which are now mainlined
- Re-number "fix-initial-PERST-GPIO-value" patch as per number
  ordering seen in 0000.patching_config.yaml
- Rewrite kernel config
"general-add-overlay-compilation-support.patch" became obsolete in
Linux 6.9 (see AR-2352 [1]). Fix this problem:

- Rename *.dts sources in overlay directory to *.dtso
- Change "target +=" line to "dts-y +=" in overlay Makefile
- Remove "always +=" line in overlay Makefile
- Add .scr compilation support in kernel scripts/Makefile.lib
- Patch kernel scripts/Makefile.dtbinst to avoid flattening overlay
  directory

For the last two points, see
general-add-overlay-compilation-support.patch

Credits for this fix go to @paolosabatino

[1] https://armbian.atlassian.net/browse/AR-2352
This simplifies the process updating to newer kernel versions and makes
it easier to see and edit the actual dts file being used.
Patches are from the Linux Rockchip Mailing List, submitted by Alexey
Charkov. [1]

Notable improvements from changelog:
- Moved the TSADC enablement to per-board .dts/.dtsi files
- Dropped extra "inefficient" OPPs (same voltage - lower frequencies)
- Dropped second passive cooling trips altogether to keep things simple
- Added a cooling map for passive GPU cooling (in a separate patch)
- Added regulator coupling for EVB1 and QuartzPro64

Also enable automatic fan control on Rock 5B.

[1] https://lore.kernel.org/linux-rockchip/[email protected]/
The old driver didn't build on 6.10, so the kernel build couldn't finish.
Fix this driver.
This enables powerkey functionality for the PrangePi 5 Plus and other devices using the RK805
Since the `edge` kernel branch often uses RC kernels, introduce a more stable `current` branch.
This branch should not be RC kernels and it also should move to an LTS kernel once released.
Move kernel patch folder to archive/rockchip-rk3588-${KERNEL_MAJOR_MINOR} which is the default.
Also rename kernel config to linux-rockchip-rk3588-${KERNEL_MAJOR_MINOR}
Copy link
Member

@rpardini rpardini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets merge before more conflicts come in from main.
We can always fix edge more during next -rcX's and current should exactly as edge before this.

@ColorfulRhino ColorfulRhino changed the title RK3588 edge kernel: Add support for Linux 6.10 RK3588 edge kernel: Add support for Linux 6.10 + introduce stable current branch (6.8) Jun 12, 2024
@ColorfulRhino ColorfulRhino merged commit 994f376 into armbian:main Jun 12, 2024
7 checks passed
@ColorfulRhino
Copy link
Collaborator Author

Lets merge before more conflicts come in from main.

Merged 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
08 Milestone: Third quarter release Hardware Hardware related like kernel, U-Boot, ... size/large PR with 250 lines or more
Development

Successfully merging this pull request may close these issues.

5 participants