Convert rockchip64 patches into series, introduce LE patches

[paolosabatino]

Description

This PR follows the discussion of the previous PR about introducing LibreELEC patches into rockchip64 family patch tree.

There is no Jira reference number yet, since this is yet a proposal. If there is general consensus, a ticket will be opened and linked here.

As like as @AdamLantos did, I first let LibreELEC patches apply and then armbian ones, so if an armbian patch clashes, I remove it and keep the libreelec variant. This is so because LibreELEC patches usually are already queued for being upstreamed into mainline kernel and then downloaded from https://patchwork.kernel.org

This time, following the requests coming from the previous PR, LibreELEC patches have been splitted from mailbox format into single patches. It should be easier to review and change them in case a patch is mainstreamed.

A small readme.txt file in libreelec series directory has been created with a minimal documentation about where the patches come from, the technique used to split the mailbox format and, obviously, give credits to the original authors.

Also patchsets have been separated using the same patch series logic which is already used for sunxi families, so librelec and amrbian patches stay in their own directories.

Since patch series application logic does not remove existing files if a patch wants to create a file from scratch, as "basic" patch logic do, I reworked the existing patches that were leveraging that behaviour to keep the final result exactly as it was before. This is so for:

• helios64 board file
• nanopi-r4s board file
• roc-pc-plus board file
• YT-85xx motorcomm driver

All other patches have been adapted, so at the current moment all of them apply clean and all of them have been ported to the new structure. Duplicate ones, as already stated, have been removed.

TODO: I still have to review the device tree overlays for correctness

How Has This Been Tested?

• [x] All patches apply cleanly
• [x] Built image and tested on rk3318 board
• [x] Built image and tested on rk3328 board (Rockpi E)
• [x] Built image and tested on rk3399 board (Orange PI 4 LTS)

Checklist:

• [x] My code follows the style guidelines of this project
• [x] I have performed a self-review of my own code
• [x] I have commented my code, particularly in hard-to-understand areas
• [x] I have made corresponding changes to the documentation
• [x] My changes generate no new warnings
• [x] Any dependent changes have been merged and published in downstream modules

[Tonymac32]

I just saw this pop up, did we address the nebulous blob of LE changes problem? Is there any way to maintain this that isn't "pull in new LE blob and make sure Armbian patches get out of the way of the storm"? Despite the rather impassioned arguments from the other thread, I sincerely don't see this as our problem/worth our time to constantly work around.

@igorpecovnik perhaps we need a patch mechanism like the "extrwifi" that was designed to encapsulate that pile of unavoidable dog crap. Over half the people using Rockchip64 have no use for any of this PR's improvements, and would probably love to shut them off when they invariably complicate small tasks like building images.

Quite frankly I still maintain that the LE patches need to make room for ours, not the other way around as being done here. Add them last, and if they don't work shut them off. Otherwise we are adding massive unpredictable decency hell to this project.

As always, this is my opinion. I can always complain somewhere else if I find it impossible to do basic tasks with Armbian. I see IAV took the liberty of approving the PR with nothing more than a test on a single board, so have at it.

[Tonymac32]

That said, @paolosabatino thank you for all the work and documentation. I'll review it as soon as I can.

[paolosabatino]

There are rk3288 related patches which are useless in the rockchip64 family, can we just clean them?

Surely we can; but when we will "sync" with libreelec sources again, they will reappear and will need to remove them manually again. Even if they are unneeded and unnecessary for the rockchip64 case, it is less work to do now and later; with less work also comes less risks of breaking something if someone removes wrong patches, and also commit history should be also easier to navigate.

[catalinii]

Tested this on rockpro64 and looks good

[igorpecovnik]

perhaps we need a patch mechanism

You mean like "include media patches series=yes" ? Technically this should be possible and useful, but I would propose we deal with few other critical changes such as armbian-next first, then re-design this "on paper", then code things in. Even implementation looks simple on a first sight, it is possible to make one step further, two back. Or to not improve things which can also be improved while changing the way patches are organised.

If we have patches organised better, like this commit is doing, its already a first step towards having a control in some simple manner. But it also has to be trivial to understand outside this group of people.

[The-going]

All patches in the series must be applied and extracted using the git <am\format-patch> command.

Thus, the user will be able to create an independent full-fledged git repository from a series of patches and continue to work with it. Subsequently extracting a new series from it. The user will be able to rebase it when updating the upstream kernel using standard git mechanisms. Then he will be able to extract the "clean" patches as a series using a script. The user will be able to take a series of Debian patches from the original debian package of 3 quilt format. And apply them using the git quiltimport command. See git quiltimport -h.

[paolosabatino]

@The-going git amis how I applied the LibreELEC patches on vanilla v5.19 kernel and then "extracted" back git git format-patch as single patch files for each commit. The process has been described in a readme.txt in the libreelec-series directory.

I didn't use your script: I did it manually to better grasp the process, so the readme contains the bare bones of the process.

Yesterday I had time to review every single of the 93 libreelec patches: all of them are correctly formatted as kernel developers want. This means all of them looks like regular emails with a proper and intelligible description in front of the code diff; a good amount of them (more than half of them) have been submitted in the kernel mailing lists, so they are already orbiting the kernel and may be mainlined sooner or later (or never...). I don't see dealing with those patches really challenging, most of the time when they don't apply anymore, they just require to be renamed to .disabled and things will keep going on.

Some other concerns are really solid: for example just yesterday it happened that @amazingfate pushed a change in a libreelec patch for rockchip (armhf): https://github.com/armbian/build/pull/4111 - this is not bad per-se and not blaming him (absolutely!), but this is the perfect example of what can happen: on next sync that I will do, that "fix" will be overwritten with fresh libreelec patches, so how to handle with these "external patches" in general is a serious issue. So, some questions pop in my mind:

• how to deal with "external patches" in general? I think that also the megous series in sunxi directory are in the same class, aren't them?
• do we need "family supervisors" that handles the pull requests?
• clear rules on how and what to patch?

These questions don't necessarily need an answer, we can just not deal with them at all: if a push to a patch part of "external patches" happens, it may happen that it will just be zapped away sooner or later. :man_shrugging:

[The-going]

to be renamed to .disabled

You don't have to rename patches. Just put (-) minus in the first position before the patch name in the series.conf file in this way: https://github.com/armbian/build/blob/79439fcbf1b7db8fe3d478983b0144db89b5b081/patch/kernel/archive/sunxi-5.15/series.conf#L75-L80 these three patches will not apply.

[The-going]

• how to deal with "external patches" in general? I think that also the megous series in sunxi directory are in the same class, aren't them?

There will always be a handmade element in this process. Paolo, you did everything right by applying LibreELEC patches to the kernel repository. Now that the application and extraction process has become transparent to you, you can use the mk_format_patch script.

I extract megous patches using this script from an already existing git repository. Due to the fact that the working branch is the result of merging from many other thematic branches and the time of creating commits differs, the order of extracted patches is chaotic. I sort them manually first, just edit the series file. Then I apply it sequentially in a loop using a small script to the corresponding branch of the repository kernel.org . Then I extract them again using the mk_format_patch script to the target directory. These are already clean patches for the pull request.

Next comes a series of armbian patches. It is also extracted from my local repository using the mk_format_patch script. And can be used to apply and create a git repository.

That's the whole point.

We extract patches as a series for distribution from an already existing repository. Next, we use them when we compile and build the package. Or we can use them to create a git repository and start working with this repository.

[The-going]

P.S. I didn't actually reinvent the wheel. Ubuntu and openSUSE are such different distributions doing the same process using scripts written in python and ruby. I just shifted their logic to bash language.

[Tonymac32]

Guys, I'm fine with these patches as long as they've been tested on each SoC we support for building and booting. The rest of this discussion, including my less than patient feedback, is future consideration we can debate in forum, and discuss in committee whenever those come to be.

[igorpecovnik]

/rebase

[igorpecovnik]

I'm fine with these patches as long as they've been tested on each SoC we support for building and booting

rk3328 and rk3399 are apparently tested. What else?

and discuss in committee whenever those come to be.

This will take some time before ready ... means we are good with this PR, while otherwise we should vote on such changes prior on persuading them?

[Tonymac32]

Patches that are irrelevant to large numbers of users and introduce an order of magnitude of complexity should in fact be reviewed/discussed by committee to determine their actual value.

That said, committee involvement should be for the discussion of using series of patches of dubious value to other users, or how to implement them so others can shut them off. Or how to settle debates where a supported board is rendered less capable by a patch set and removal of the patches would be the easiest solution. Or how to implement kernels by function (similar to minimal vs desktop images). That is where committee is of value.

[igorpecovnik]

That is where committee is of value.

Yes, I see the value the same way. Just we are at the concept level and before decision making this way won't run butter smooth, we still have to move things further. The longer we wait, the biggest will be the pile of things that require such intervention.

Midway? Keeping the LE patches disabled by default, but keep things organised this way?

[catalinii]

IMHO, I think there are 4 problems here:

1. Patches as these are released to a large number of boards potentially breaking just a few of them while the others are largely fine.

I have never used current kernel for my rockpro64 because edge worked well for me. Not sure how many users are actually using them.

One suggestion would be to keep the current kernel as edge - 1 (eg 5.19 is edge, current is 5.18) and don't publish edge kernels (or mark them as beta)

Of course this can be done more granualar and split per set of boards to ensure a PR blast radius is smaller, but this would probably add another layer of complexity to the project.

I think this is a problem for each PR just the risks are higher for larger PRs such as this one.

2)Organization of the actual patches: There are probably a lot of ways to manage this, but the this to me this is a short term problem as many of those patches are landing in mainline.

I think of a family of boards become to complex (with a lot of patches) this can wait for a armbian board decision and be done as a separate PR.

3. the value that this PR brings. To me personally this patch is a high value as it prevents me from having a separate SBC running LE. I know not all users feel the same but this would be a nice addition even for the ones that want to try kodi just for a while.

4. Updating the patches for a new kernel version. I do think this patch brings operational overhead when switching to a new kernel version and the possibility of breaking boards. But leaving aside there some individuals which can do this currently, this can be improved by either documentation or scripts. This will be a problem will always be there and any automation will just make it easier.

I would not have a set of patches enabled or disabled as this will add another layer of complexity in testing boards (board X works with patch Y disabled while board Z does not work with patch Y enabled). As we have git history we can always revive an old patch.

Just my 2c.

[paolosabatino]

As long as kernel 6.0 has been release, I try to revive the discussion on this trying to answer @catalinii with my point of view on the problems:

1. yes, this is true. But it is true for any other patch that is going to touch shared files, drivers or device trees; so the problem is a bit bigger than this patchset and IMHO is caused by missing rules and control that made the rockchip64 family patches grow somehow unordered. An example: this patch, if removed, has the potential to break several rk3328 boards: this patch artificially keeps the GPU voltage high because GPU and memory controller share the "logic" voltage regulator; since some boards are hardwired to run memory clock as high as 933 MHz (for DDR3-1866), you can't afford to use lower voltages or they will break. This patch causes ALL rk3328 boards to feed the logic plane with a higher voltage, consume more current and heat more.
2. those patches may or may not actually land in mainline; some are taken from kernel mailing lists, some others are private contributions, but all of them are decently documented and explain what they do; the format proposed in this PR is a middle way to be able to keep them aligned with libreelec updates in an easier manner;
3. I share this opinion. These introduce several features (rkvdec hevc decoder, iep, YCbCr planes and V4L2 formats, resolutions >= 2k) and fixes to the HDMI, DRM and clock subsystems/device trees. In general, I see that these patches make the SBCs more exploitable for serious projects and fun, with heavy contributions to the multimedia part.
4. Well, you should consider that ANY patch could break any board. Actually any kernel upgrade could break any board (not to tell about u-boot upgrades :exploding_head: ). Maintainers are there for the purpose of keeping things working, aren't they?

[paolosabatino]

Just a quick update: I applied the LE patches for 5.19 over the 6.0 kernel and there were a couple of failures. One is trivial to fix (a hunk changed), another one (hevc rkvdec) had a couple of header files moved and required a bit more attention.

On the other side, yesterday LE folks already rebased the patches upon newer kernel.

[paolosabatino]

Rebase against current armbian master, made a testing image for rk3318/rk3328 tvbox community, let's see also their feedback.

[paolosabatino]

I wish to report at least one success story from an user with rk3328 (or rk3318) tv box. Kernel 5.19 broke something on his HDMI that was previously working on 5.15 on two similar boards. A custom kernel built from this branch restored HDMI functionality.

https://forum.armbian.com/topic/17597-csc-armbian-for-rk3318rk3328-tv-box-boards/?do=findComment&comment=149482

He also did a quick test with several USB peripherals:

https://forum.armbian.com/topic/17597-csc-armbian-for-rk3318rk3328-tv-box-boards/?do=findComment&comment=149605

And even posted dmesg (looks clean to me):

https://forum.armbian.com/topic/17597-csc-armbian-for-rk3318rk3328-tv-box-boards/?do=findComment&comment=149821

[schwar3kat]

@paolosabatino What does the large diff for YT-85xx motorcomm driver do? (driver-for-Motorcomm-YT85xx+PHYs.patch)

My interest; the current patch includes a lot of PHY devices not upstream including YT8531C used by Orange Pi R1 Plus LTS board which I added, along with the current patch which replaced a similar patch without YT8531C. So I have a desire to keep them working.

[paolosabatino]

@schwar3kat that large diff is just the application of the original driver-for-Motorcomm-YT85xx+PHYs.patch file from master branch over the mainline motorcomm driver.

This is needed because there is a different behaviour from armbian scripts when a patch is inside a series or outside series: you can see here for some reference.

Anyway to sum up, the result is exactly the same as the master branch, just the patch needs to be different; you should not worry though, this branch has been killed in action. Amen.

[paolosabatino]

I close this, as long as it is not up to date anymore·