lights: switch to AIDL HAL from somainline

[z3ntu]

The previously used android.hardware.light@2.0 is mostly just a
passthrough HAL that uses the legacy liblights that we implemented here.
The new HAL is using AIDL and implements the interface directly which is
much more what it should be.

This also fixes max brightness being (indirectly) hardcoded to 1020
instead of the actual values that devices support.

Link: https://github.com/SoMainline/android-lights-hal

--

I didn't actually build test this directly here, just in my own branch so please do so on important devices ;)

[MarijnS95]

That's an odd, author-less PR. Wasn't the plan to migrate our repo to aospm at some point, or do people really insist on having it as a folder within this git repo?

[calebccff]

yeah, I'd like the move HALs out of this folder where possible to simplify supporting more devices. Can we use it from external/

@MarijnS95 I'd be happy with moving your repo to AOSPM

[MarijnS95]

@calebccff I think I can move it here. Not sure yet what to do with the rest of my (both public and private) Android+mainline repos, they probably get more love+use under the aospm organization here.

[MarijnS95]

Anyone interested in our vibrator HAL too? Have some improvements planned but that's also what we use internally :)

[z3ntu]

Caleb's https://github.com/aospm/external_vibrator-ff works well on FP4 but if the other one works also I'm fine with either :)

[calebccff]

@calebccff I think I can move it here. Not sure yet what to do with the rest of my (both public and private) Android+mainline repos, they probably get more love+use under the aospm organization here.

to be clear, @z3ntu we want to move the repo here and then depend on it, not vendor it under this project.

[MarijnS95]

@z3ntu Nice, I almost forgot about @calebccff's repo! We should probably unify the two and stay in the mainline spirit :)

---

This was discussed with folks from @SoMainline and before diving into transferring / forking those repos into @aospm, is there any possibility to land these HALs as default or "mainline" variant upstream with AOSP directly? Seems a much better place though I reckon they'd need to have a "need/use" for it before committing to adopting and maintaining such an approach.

[calebccff]

I looked into this and I think without upstream users it's quite a tough sell. Maybe worth giving it a go though

[MarijnS95]

I looked into this and I think without upstream users it's quite a tough sell. Maybe worth giving it a go though

Aren't AOSP developers also slowly progressing towards a mainline kernel? But I guess with copypasted custom drivers including those for notification leds / backlights (and haptics, WRT the other driver).

Regardless, I didn't find much time for this now nor will have any time at all the next month. I just forked my repo into https://github.com/aospm/android-lights-hal in case you rather reference a branch from aospm and SoMainline, that at least allows to remove copy-pasted - and easily getting outdated - code to be removed from this device tree again.

We can always change this later.

[z3ntu]

@MarijnS95 Nitpick but what about using branch name like android12-dev? That's the name Google uses for their branches, see e.g. here https://android.googlesource.com/platform/frameworks/base/+refs
But I'm also totally fine with aosp-12.0 🤷

[MarijnS95]

@z3ntu All my AOSP/SODP/SoMainline "mates" use a different naming convention so I'm really fine with whatever. Rather stick to what AOSP uses indeed, but I don't think I can find the time to update them on the core repo and the manifest I have locally before I'm gone.

[MarijnS95]

Alas, we typically use these branches in conjunction with the stable _r tag releases, but it should also work fine on -dev and it's a rolling branch anyway.

[calebccff]

thanks Marijn, i think there's a chance we'd be able to get this into AOSP, if it's ok with you I'll keep looking into it and keep you updated.

[MarijnS95]

@calebccff That'd be lovely! Meanwhile we can use this here and see how that progresses. Would be nice if we can do the same for haptics as well.

[z3ntu]

So can we get this forward, I guess adding https://github.com/aospm/android-lights-hal to https://github.com/aospm/android_local_manifests and use it here?

Was this also tested on another (supported) device with aospm by anyone?

[MarijnS95]

@z3ntu yes please, that has my preference. I'm still on holiday for a few days so you're free to pick this up.

[MarijnS95]

Iirc I put these in the HAL repo, or did I forget and only do that for the vibrator HAL?

[z3ntu]

Nope, just this is there https://github.com/aospm/android-lights-hal/blob/aosp-12.0/sepolicy/file_contexts

Also we still need to figure out that one.. https://github.com/aospm/android-lights-hal/blob/aosp-12.0/lights.mk Or just change the hardcoded path there to the new one I guess?

[MarijnS95]

Whoops, forgot to address this while on holiday.

Nope, just this is there https://github.com/aospm/android-lights-hal/blob/aosp-12.0/sepolicy/file_contexts

We could add these, or at least the sysfs_lights type and generalized sysfs paths (the first of):

android_device_generic_sdm845/shared/sepolicy/file_contexts

Lines 22 to 23 in 848542a

	/sys/class/backlight(/.*)? u:object_r:sysfs_lights:s0
	/sys/devices/platform/soc@0/c440000.spmi/spmi-0/0-03/c440000.spmi:pmic@3:leds@d800/backlight u:object_r:sysfs_lights:s0

Also we still need to figure out that one.. https://github.com/aospm/android-lights-hal/blob/aosp-12.0/lights.mk Or just change the hardcoded path there to the new one I guess?

I guess no-one found a solution for the my-dir bit yet? Otherwise feel free to update it; we can also stuff these under a vendor/aospm/ folder instead of throwing everything in device/?

[z3ntu]

Otherwise feel free to update it; we can also stuff these under a vendor/aospm/ folder instead of throwing everything in device/?

I threw it into external/lights-hal in aospm/android_local_manifests#2 , at least that matches where other aospm repos like vibrator-ff are. No clue what the best option is.

[z3ntu]

Can someone with permissions please create an aosp-13.0 branch in https://github.com/aospm/android-lights-hal so I can send a PR and/or enable issues in that repo.

For Android 13 we seem to need this patch (still build and runtime test on my side)

diff --git a/Android.bp b/Android.bp
index f32feac..d149b3f 100644
--- a/Android.bp
+++ b/Android.bp
@@ -7,7 +7,7 @@ cc_binary {
     shared_libs: [
         "libbase",
         "libbinder_ndk",
-        "android.hardware.light-V1-ndk_platform",
+        "android.hardware.light-V2-ndk",
     ],
     srcs: [
         "Lights.cpp",

References:

• https://cs.android.com/android/_/android/platform/hardware/interfaces/+/27f77fefd71e486e72c9dc4150dc316de13a9153
• https://cs.android.com/android/_/android/platform/hardware/interfaces/+/ef2da6da7f08513f50f389d2ebc831f2c85e6489

[calebccff]

Android 13 is now merged into AOSP so you should be fine to open a MR for that as is. If needed we can backport stuff and make a stable branch though if needed

[MarijnS95]

@z3ntu Sure, I'll make sure to check the permissions as well. Before doing that though, should we rename the branches as per #15 (comment)?

Shall we aim for android[-]XX-dev or android[-]XX.0 or something else entirely?

[MarijnS95]

@z3ntu In any case, if I read the first commit correctly, is ndk and ndk_platform pretty much the same on Android 12 already? That might mean that we can apply this there as well instead of having an Android-13 specific change (which I think is what @calebccff is suggesting in some way?)?