ruff.mobi - under conception

A piece of Librem5 on my MNT Reform2

Despite my frequent ranting at Purism - don't take me wrong, I fully appreciate what they are doing. I just didn't like their communication strategy. Which finally improved and entered a state where I can say - yea ok, I understand. Other than that - their work at GTK/Gnome (libhandy which is now libadwaita, phosh/phoc, various userspace apps) is enormous, their effort at mainlining imx8m is impressive, all together allow me now using pinephone and MNT Reform2 laptop. This article is a demonstration of how the effort in kernel helps on MNT Reform.

So here I will be trying to run Librem5 kernel on MNT Re2. MNT Re2 native image is currently pinned to Linux Kernel 5.12, which is not drasticly old, but still not a cutting edge. The reason being a relatively small set of patches which are not mainlined. Either not yet (eg. are already in linux-next), or not because (eg. they are hw specific and not polished yet). On the other hand Purism is commited to mainline as much of imx8mq soc as possible. And that covers not just a baseline (it works) but a full set (it works fully and efficiently).

Of course part of that efficiency falls to elements outside of the SoC - eg L5-specific periphery, like WIFI, WWAN, system (power and clock) controller, screen panel and sound codec, etc. This from MNT Re2 perspective makes "not because" part, since there's similar problem from this side of the SoC.

Either way, Purism declared as a current priority improving runtime power management (S0 to S2) with suspend state being a lesser priority. That sounds like a real benefit for another type of mobile platform like MNT Re2. Of course it doesn't mean Reform will automatically consume less power, the platform specific periphery doesn't magically start working by itself, but still if there is some space for improvement at SoC level - there's big chance it will be explored by Purism engineers. This is a rationale to chosing Purism's branch versus pure mainline.

So what I will be doing is following: clone the reform-system-image, replace kernel repository in mkkernel.sh from mainline pinned to 5.12 to purism pinned to byzantium, try to apply (check actually) all OOT patches (and correct those which don't apply - this is the hardest part) and re-apply kernel-config from template-kernel to the new tree. As we go we'll see what else needs to be done, but OOT patch correction is a main thing.

Before updating the script let's do the steps manually. We need to do it anyway to prove it's doable and the syntax is correct.

$ git clone https://source.mnt.re/reform/reform-system-image.git
$ cd reform-system-image/
$ cd reform2-imx8mq/
$ mv linux linux-orig
$ git clone git@source.puri.sm:Librem5/linux-next.git linux

Here we do a full clone of the linux kernel tree. Technically no need to (it's enormous), can do a shallow copy, but I didn't know yet which exactly branch to use. I also preserved existing tree - in case I need to look at the code, original or patched. The next thing I will be doing iteratively is picking a patch from ../template-kernel/patches/ folder (in alphabetical order) and trying to apply it. To prevent generating conflict files and sections I will be using patch -p1 --dry-run command which makes a soft check, which is trying to apply the patch without modifying actual file. The command git apply --check (as in the script) technically is doing the same - but not exactly, and we'll come to this later.

$ cd linux
$ ls -la template-kernel/patches/
$ patch -p1 --dry-run < \
 ../template-kernel/patches/0001-nwl-dsi-fixup-mode-only-for-LCDIF-input-not-DCSS.patch
$ vim drivers/gpu/drm/bridge/nwl-dsi.c
$ vim ../template-kernel/patches/0001-nwl-dsi-fixup-mode-only-for-LCDIF-input-not-\
DCSS.patch
$ rm ../template-kernel/patches/0001-nwl-dsi-fixup-mode-only-for-LCDIF-input-not-\
DCSS.patch

The first patch failed to apply. Partially with failed chunks, partially with "reversed patch" error. Now the "reversed patch" is a good indication the patch is already in the tree (either mainline or at least purism's) but failed chunks indicate it's not as straight forward hence need manual checks. Which I did, to find that failed chunks are due to missing context (lines before and after the patch). Otherwise apart of context all added (or removed) lines seem to be there, therefore I removed the patch and switched to next one.

$ patch -p1 --dry-run < \
   ../template-kernel/patches/0005-pci-imx6-add-support-for-internal-refclk-imx8mq.patch
$ vim ../template-kernel/patches/0005-pci-imx6-add-support-for-internal-refclk-imx8mq.patch
$ vim drivers/pci/controller/dwc/pci-imx6.c

This patch also fails, there's one chunk which cannot apply, but others look ok. First let see where the patch is trying to apply:

index 5cf1ef12f..2f1ed228d 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c

This is the file which is being modified (without a/ b/ prefixes) and the chunks are starting with @@ so let's find third occurence and see there it tries to find the code:

@@ -620,7 +658,8 @@ static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
		regmap_update_bits(imx6_pcie->iomuxc_gpr,
 				   imx6_pcie_grp_offset(imx6_pcie),
 				   IMX8MQ_GPR_PCIE_REF_USE_PAD,
-				   IMX8MQ_GPR_PCIE_REF_USE_PAD);
+				   (imx6_pcie->internal_refclk ?
+				    0 : IMX8MQ_GPR_PCIE_REF_USE_PAD));
 		break;
 	case IMX7D:
		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,

Ok so somewhere around line 620, presumably in function imx6_pcie_init_phy should be a code with regmap_update_bits which should look like the abovewhere we want to modify one line. If we look on actual file and try to find the corresponding section - we can find it at line 660:

...
        case IMX8MQ:
                /*
                 * TODO: Currently this code assumes external
                 * oscillator is being used
                 */
                regmap_update_bits(imx6_pcie->iomuxc_gpr,
                                   imx6_pcie_grp_offset(imx6_pcie),
                                   IMX8MQ_GPR_PCIE_REF_USE_PAD,
                                   IMX8MQ_GPR_PCIE_REF_USE_PAD);
                /*
                 * Regarding the datasheet, the PCIE_VPH is suggested
                 * to be 1.8V. If the PCIE_VPH is supplied by 3.3V, the
                 * VREG_BYPASS should be cleared to zero.
                 */
                if (imx6_pcie->vph &&
                    regulator_get_voltage(imx6_pcie->vph) > 3000000)
...

It's clear from this code the trailing part of the context does not match. The patch expects break and next case start, while in actual code the case section continues and exact context should be the comment right after the line the patch changes. According to the patch it uses 3 lines of context (3 lines befor and after modified section) so we need to take 3 lines of actual code (comment) and replace the trailing context with it, to make it looking like this:

@@ -620,7 +658,8 @@ static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
                regmap_update_bits(imx6_pcie->iomuxc_gpr,
                                   imx6_pcie_grp_offset(imx6_pcie),
                                   IMX8MQ_GPR_PCIE_REF_USE_PAD,
-                                  IMX8MQ_GPR_PCIE_REF_USE_PAD);
+                                  (imx6_pcie->internal_refclk ?
+                                   0 : IMX8MQ_GPR_PCIE_REF_USE_PAD));
                /*
                 * Regarding the datasheet, the PCIE_VPH is suggested
                 * to be 1.8V. If the PCIE_VPH is supplied by 3.3V, the

Now we can check the patch using dry run to ensure it applies cleanly (which it does):

$ patch -p1 --dry-run <\
  ../template-kernel/patches/0005-pci-imx6-add-support-for-internal-refclk-imx8mq.patch 
checking file drivers/pci/controller/dwc/pci-imx6.c
Hunk #1 succeeded at 65 (offset 1 line).
Hunk #2 succeeded at 612 (offset 2 lines).
Hunk #3 succeeded at 660 (offset 2 lines).
Hunk #4 succeeded at 1110 (offset -16 lines).

A little disclaimer - I'm writing this now from the memory and trying to highlight important things to note but if your tree behaves differently it means either tree has changed or my memory is fading. The above example I've rebuilt manually again, but below are mostly bash history commands.

Next patch - multiple failures again but looking at code it seems the code is already there (or rather not there, as this patch is a reversal which removes the other patch):

$ patch -p1 --dry-run < \
 ../template-kernel/patches/0009-revert-58074b08c04af1817ab34be986a80279e7267d07-\
edid.patch
$ vim ../template-kernel/patches/0009-revert-58074b08c04af1817ab34be986a80279e7267d07-\
edid.patch
$ rm ../template-kernel/patches/0009-revert-58074b08c04af1817ab34be986a80279e7267d07-\
edid.patch

Next one also looks like applied, although the target file is significantly modified so need to understand the logic (what should be the end state) rather than semantic (how patch should look like to apply).

$ patch -p1 --dry-run < ../template-kernel/patches/caam-revert-imx8m-soc-match.patch
$ vim ../template-kernel/patches/caam-revert-imx8m-soc-match.patch
$ vim drivers/soc/imx/soc-imx8m.c
$ rm ../template-kernel/patches/caam-revert-imx8m-soc-match.patch

Now the next one is the odd one, it's quite simple and patch --dry-run actually succeeds. But if you try git patch --check it actually fails. Which I figured much later. While trying to understand why I notced the patch output indicates the patch could be applied with a fuzz 6. Fuzz is an ability of patch to guess the context, i.e. if it does not match literally but looks kind of similar the patch by default accepts it while git patch check does not. The value of fuzz indicates how much dfferent the context is. So let's take a closer look here:

$ patch -p1 --dry-run < ../template-kernel/patches/caam-revert-swiotlb-origaddr.patch
Hunk #1 succeeded at 376 with fuzz 6 (offset -350 lines).
$ vim ../template-kernel/patches/caam-revert-swiotlb-origaddr.patch
...
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -726,6 +726,7 @@ void swiotlb_tbl_sync_single(struct device *hwdev, phys_addr_t tlb_addr,
 
 	if (orig_addr == INVALID_PHYS_ADDR)
 		return;
+	orig_addr += (unsigned long)tlb_addr & (IO_TLB_SIZE - 1);
 
 	validate_sync_size_and_truncate(hwdev, orig_size, &size);
 
$ vim kernel/dma/swiotlb.c
... 376G

        if (orig_addr == INVALID_PHYS_ADDR)
                return;

        tlb_offset = tlb_addr & (IO_TLB_SIZE - 1);
        orig_addr_offset = swiotlb_align_offset(dev, orig_addr);
        if (tlb_offset < orig_addr_offset) {

I am not that much patch expert to tell why this is fuzz but the other case above is fail. To me both look like fail since both trailing contexts are totally wrong. I suspect it might be due to at least one line matching (empty line following the modified line) but don't take my word on it. Either way if you follow the code's logic (not semantic) you can see that the offset is also calculated here, but stored into separate variable instead of straigt correction. And if you read the code further you'll find that after some additional checks the offset correction is applied to the orig_addr:


        orig_addr += tlb_offset;
        alloc_size -= tlb_offset;

So to me it means the patch could be dropped as well with rm ../template-kernel/patches/caam-revert-swiotlb-origaddr.patch.

The next patches seem to be already in the tree hence dropping them as well:

$ patch -p1 --dry-run < \
 ../template-kernel/patches/mnt3004-MNT-Reform-imx8mq-add-PHY_27M-clock.patch
$ vim ../template-kernel/patches/mnt3004-MNT-Reform-imx8mq-add-PHY_27M-clock.patch
$ rm ../template-kernel/patches/mnt3004-MNT-Reform-imx8mq-add-PHY_27M-clock.patch
$ patch -p1 --dry-run < \
 ../template-kernel/patches/mnt3006-MNT-Reform-imx8mq-add-PHY_27M-clock-missing-define.patch
$ vim ../template-kernel/patches/mnt3006-MNT-Reform-imx8mq-add-PHY_27M-clock-missing-\
define.patch
$ rm ../template-kernel/patches/mnt3006-MNT-Reform-imx8mq-add-PHY_27M-clock-missing-\
define.patch

The next one seems to be ok as is, applies with only one line offset. The one after also applies - but with the fuzz - so I also needed to correct it later. The fuzz now is 2 and is at leading context where 2 lines are matching but one line is slightly different (here I can understand why the fuzz is low).

$ patch -p1 --dry-run < ../template-kernel/patches/mnt4000-limit-fslsai-to-48khz.patch
$ patch -p1 --dry-run < ../template-kernel/patches/mnt4001-lcdif-fix-pcie-interference.patch
$ vim ../template-kernel/patches/mnt4001-lcdif-fix-pcie-interference.patch
@@ -212,7 +212,7 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb)

	mxsfb_set_formats(mxsfb);

-	clk_set_rate(mxsfb->clk, m->crtc_clock * 1000);
+	clk_set_rate(mxsfb->clk, m->crtc_clock * 660);

 	if (mxsfb->bridge && mxsfb->bridge->timings)
		bus_flags = mxsfb->bridge->timings->input_bus_flags;
$ vim drivers/gpu/drm/mxsfb/mxsfb_kms.c
... 251G

        mxsfb_set_formats(mxsfb, bus_format);

        clk_set_rate(mxsfb->clk, m->crtc_clock * 1000);

        if (mxsfb->bridge && mxsfb->bridge->timings)
                bus_flags = mxsfb->bridge->timings->input_bus_flags;

So here it's quite obvious - the call in context has slightly changed so we need to adjust the leading context making it looking like this:


        mxsfb_set_formats(mxsfb, bus_format);

-       clk_set_rate(mxsfb->clk, m->crtc_clock * 1000);
+       clk_set_rate(mxsfb->clk, m->crtc_clock * 660);

        if (mxsfb->bridge && mxsfb->bridge->timings)
                bus_flags = mxsfb->bridge->timings->input_bus_flags;

Other chunks in this patch apply without fuzz so we can proceed to the next one. The next claims the patch is reversed, but if we look closer it's just the first chunk wich is already there, the others are not. So we can simply drop the first chunk by removing it from the patch (the text starting from @@ and down to the next @@).

$ patch -p1 --dry-run < ../template-kernel/patches/mnt4002-imx-gpcv2-wake-smccc.patch
$ vim ../template-kernel/patches/mnt4002-imx-gpcv2-wake-smccc.patch
$ patch -p1 --dry-run < ../template-kernel/patches/mnt4002-imx-gpcv2-wake-smccc.patch

Next one is again the odd one, I was scratching my head trying to understand if new code is actually doing what is intended or not, in the end I decided it does and hence dropped the patch.

$ patch -p1 --dry-run < ../template-kernel/patches/mnt4003-emmc-clockgate.patch
$ vim ../template-kernel/patches/mnt4003-emmc-clockgate.patch
$ rm ../template-kernel/patches/mnt4003-emmc-clockgate.patch

Finally we're coming to the Big Badass Patch. The patch required quite significant effort to understand it and rework as it makes multiple changes in multiple fiels, creating some new files and whatnot. So I will only try to state the high level description without much technical details..

The patch originally introduces new HDMI driver for IMX8MQ and hooks it into DCSS subsys. In the new kernel tree the IMX8MQ HDMI driver already exists. And is (mostly) hooked into DCSS. So after numerous lengthy iterations this patch was reduced to following:

Introduce HDMI CEC driver (the in-tree driver lacks it while patch has)
Separate HDMI Audio and add HDMI CEC to kernel configuration (in-tree configures it as a single blob, patch allows enabling separate sub-components)
Add additional quirks into DCSS driver to work with HDMI (conditional use based on existence of HDMI port as per DTB)
Various dependencies introduced by patches, mostly by CEC driver (additional header file, some functions, etc.).

The new ported patch is still quite big (even though reduced by 5k+ lines) to fit in here, so you can find it in my forked tree here. I never used HDMI on reform yet so I don't even know if it works tbh :)

$ patch -p1 --dry-run < \
 ../template-kernel/patches/mnt5000-imx8mq-import-HDMI-driver-and-make-DCSS-\
compatible.patch
$ vim ../template-kernel/patches/mnt5000-imx8mq-import-HDMI-driver-and-make-DCSS-\
compatible.patch
$ vim drivers/gpu/drm/bridge/cadence/Kconfig
$ vim drivers/gpu/drm/bridge/cadence/Makefile
$ vim drivers/gpu/drm/bridge/cadence/cdns-mhdp-hdmi.c
$ vim drivers/gpu/drm/bridge/cadence/cdns-mhdp-common.h
$ vim drivers/gpu/drm/bridge/cadence/cdns-mhdp-common.c
$ vim drivers/gpu/drm/bridge/cadence/cdns-mhdp-audio.c
$ vim drivers/gpu/drm/imx/Makefile
$ vim drivers/gpu/drm/imx/dcss/dcss-dev.c
$ vim drivers/gpu/drm/imx/dcss/dcss-dev.h
$ vim drivers/gpu/drm/imx/dcss/dcss-drv.c
$ vim drivers/gpu/drm/imx/dcss/dcss-kms.c
$ vim drivers/gpu/drm/imx/dcss/dcss-kms.h
$ patch -p1 --dry-run < \
 ../template-kernel/patches/mnt5000-imx8mq-import-HDMI-driver-and-make-DCSS-\
compatible.patch

Well, not very informative and useful output, just shows my progress through the patch till I get to the point where last line succeeded. After this point I've tried executing mkkernel.sh and realized two things: first some patches didn't apply (the fuzz thing mentioned above), second the kernel config changed quite a lot so need to re-conf it. Answering all (new) questions with defaults is tedious task so after fixing fuzz and re-applying patches I just executed make menuconfig and saved the file. Which made it compatible with new kernel. Btb to reapply patches you can either execute git reset --hard . and then remove extra files, or simply remove the linux dir and let the script re-fetch it (clean method).

The next thing which fails is DTB file compilation. It complains on duplicate label dcss_dsi_out in dts file imx8mq-mnt-reform2.dts from templates-kernel. I didn't find anyting better than adding an underscore to the label so that the label name does not conflict. It needs to be added in both places, the definition and application.

If you try to find what else in the file it conflicts with - you won't, as it conflicts with a label defined in imx8mq.dtsi file included into the reform2 DTS. Which means - in new in-tree SoC-level DTSI the dcss_dsi_out connector is already defined. Although I didn't analyze it thoroughly to tell whether re-definition in reform2 DTS is rendundant or not.

After everything is said and done we can build and install the new kernel. Now that I'm using Reform2 as a daily driver I don't pull my dell to cross-compile faster so everything is done in-place. To install the kernel I made the following script:

$ vim instkrnl.sh
#!/bin/bash

SRC=$PWD/linux/arch/arm64/boot/Image
DST=/Image
if [[ -f $DST ]]
then
        mv $DST $DST.old
fi
cp $SRC $DST

VER=$(grep -aGom1 'Linux version [A-Za-z0-9.+_~-]\+' $DST | awk '{print$3}')
SRC=$PWD/kmods.tgz
DST=/lib/modules/$VER
if [[ -d $DST ]]
then
        test -d $DST.old && rm -rf $DST.old/
        mv $DST $DST.old
fi
mkdir -p $DST
tar xf $SRC -C $DST
/sbin/depmod "$VER"

So now we can reboot and:

$ uname -a
Linux mnr 5.15.14+ #3 SMP PREEMPT Fri Jan 14 11:42:33 CET 2022 aarch64 GNU/Linux

Note - since that time I've migrated to nvme so a warning that the kernel need to be installed onto boot root (sdcard or emmc) while modules to system root (eg. nvme) so make sure you put it into the right place. I will modify the install script in my next article about migration.

Bottomline: Moving to Purism L5 kernel reduced custom patchset more than twice from patch number perspective, and even more so LOC-wise. And ICYMI - the repo with ported patches is at my fork .

Link... Wed Jan 19 21:28:56 2022

Tags: