search

PixelOS-AOSP / official_devices

46 stars 59 forks source link

New maintainer #391

Closed Tkiliay closed 1 month ago

Tkiliay commented 1 month ago

Name and Codename of the device you want to apply for

grus

Device Tree sources

https://github.com/Tkiliay/android_device_xiaomi_grus
https://github.com/Tkiliay/android_device_xiaomi_sdm710-common
https://github.com/Tkiliay/proprietary_vendor_xiaomi_grus
https://github.com/Tkiliay/proprietary_vendor_xiaomi_sdm710-common
https://github.com/Tkiliay/android_kernel_xiaomi_sdm710

What ROM's do you maintain officially/unofficially

None

How long have you been building Custom ROMs

Two years

Any Exceptions/special concern?

No

List of changes/patches applied to source, if any

No

Contact

Telegram username

Tkiliay

Link to your XDA thread of unofficial build

https://xdaforums.com/t/rom-14-unofficial-dynamic-pixelos-for-mi-9-se-grus-aosp-stable.4682888

Link to your unofficial PixelOS Build Post other than XDA

https://t.me/toys_updates/216
kawaaii commented 1 month ago

Trees look fine however we've got something to ask before hand.

  1. https://github.com/Tkiliay/android_device_xiaomi_sdm710-common/commit/cf98cd227067f3fee0501468339f59634d428541 Why did you do that? Do you have enough reasoning on why you did that change? Got statistics related to it that strongly proves that statsd was the reason for battery drains and not something else that was causing it?

  2. https://github.com/Tkiliay/android_device_xiaomi_sdm710-common/commit/6ea08d78f24e760ead9199b9350fe6fd442ed8c8 We do that already from source side.

  3. https://github.com/Tkiliay/android_device_xiaomi_sdm710-common/commit/47df943653448c343ada4ac80507faf8c084744b https://github.com/Tkiliay/android_device_xiaomi_grus/commit/99a2ea31c67e6951c22f3e779c99008a93d9c9f3 Reword to "prebuilt megvii blobs"

  4. https://github.com/Tkiliay/android_device_xiaomi_grus/commit/fe9e43fb02e61f6fdfec5ba6e360073eb6205e58 What change have you done here that fixes the so called "GCam buffer"? Show us the change that happened in the blob.

Tkiliay commented 1 month ago

Trees look fine however we've got something to ask before hand.

  1. Tkiliay/android_device_xiaomi_sdm710-common@cf98cd2 Why did you do that? Do you have enough reasoning on why you did that change? Got statistics related to it that strongly proves that statsd was the reason for battery drains and not something else that was causing it?
  2. Tkiliay/android_device_xiaomi_sdm710-common@6ea08d7 We do that already from source side.
  3. Tkiliay/android_device_xiaomi_sdm710-common@47df943 Tkiliay/android_device_xiaomi_grus@99a2ea3 Reword to "prebuilt megvii blobs"
  4. Tkiliay/android_device_xiaomi_grus@fe9e43f What change have you done here that fixes the so called "GCam buffer"? Show us the change that happened in the blob.

According to my own test, statsd will always consume part of the CPU, turning it off is beneficial to deep sleep and daily BB. Changes about 2,3 have been pushed. Please review again.

Reference about statsd: https://github.com/AOSPA/android_device_xiaomi_surya/commit/b66c243ca7c195c2cce285285c50e59ca950c790

Blob patch： The value 0x7D0 at 000863B0 only works in the miui camera buffer and causes interface lag in gcam, so changing it (4F F4 FA 62 -> 4F F0 00 02) to remove that buffer.

before: MOV.W R2, #0x7D0 ; unsigned int

after: MOV.W R2, #0 ; unsigned int

kawaaii commented 1 month ago

Blob patch： The value 0x7D0 at 000863B0 only works in the miui camera buffer and causes interface lag in gcam, so changing it (4F F4 FA 62 -> 4F F0 00 02) to remove that buffer.

before: MOV.W R2, #0x7D0 ; unsigned int

after: MOV.W R2, #0 ; unsigned int

Show you modifying the blobs using extract-utils instead of just patching it directly.

Reference about statsd: https://github.com/AOSPA/android_device_xiaomi_surya/commit/b66c243ca7c195c2cce285285c50e59ca950c790

That change is from the times of Android 12.

According to my own test, statsd will always consume part of the CPU, turning it off is beneficial to deep sleep and daily BB.

You do know what's statsd is used for right?

Tkiliay commented 1 month ago

Blob patch： The value 0x7D0 at 000863B0 only works in the miui camera buffer and causes interface lag in gcam, so changing it (4F F4 FA 62 -> 4F F0 00 02) to remove that buffer. before: MOV.W R2, #0x7D0 ; unsigned int after: MOV.W R2, #0 ; unsigned int

Show you modifying the blobs using extract-utils instead of just patching it directly.

Reference about statsd: AOSPA/android_device_xiaomi_surya@b66c243

That change is from the times of Android 12.

According to my own test, statsd will always consume part of the CPU, turning it off is beneficial to deep sleep and daily BB.

You do know what's statsd is used for right?

Solved. https://github.com/Tkiliay/android_device_xiaomi_grus/commit/e90e0ded0332ef62d81519708be450f2528b2e64

Re-checked the statsd documentation. This issue no longer exists in a14. The commit has been deleted.

kawaaii commented 1 month ago

https://github.com/Tkiliay/android_device_xiaomi_sdm710-common/commit/17dd3cbf5a5821b3eea3311ace36d30ec01da750 You don't even ship half of those blobs.

https://github.com/Tkiliay/android_device_xiaomi_sdm710-common/commit/3d74657167639922a42195d7876798256d6cb959 Is this really something that you require? As per the commit description, it's related with ThermalSettingsFragment which you don't even ship.

Squash https://github.com/Tkiliay/android_device_xiaomi_grus/commit/fe9e43fb02e61f6fdfec5ba6e360073eb6205e58 and https://github.com/Tkiliay/android_device_xiaomi_grus/commit/e90e0ded0332ef62d81519708be450f2528b2e64

Fix authorship on commit https://github.com/Tkiliay/android_device_xiaomi_grus/commit/5ec21488aaff845fc56f228a3fb4b445ac667458.

Tkiliay commented 1 month ago

Tkiliay/android_device_xiaomi_sdm710-common@17dd3cb You don't even ship half of those blobs.

Tkiliay/android_device_xiaomi_sdm710-common@3d74657 Is this really something that you require? As per the commit description, it's related with ThermalSettingsFragment which you don't even ship.

Squash Tkiliay/android_device_xiaomi_grus@fe9e43f and Tkiliay/android_device_xiaomi_grus@e90e0de

Fix authorship on commit Tkiliay/android_device_xiaomi_grus@5ec2148.

Authorship has been fixed. Device tree has been cleaned up. Although the sdm710's parts do not have ThermalSettingsFragment, but it will also trigger denials about SettingsLib in the log. This commit was picked from 8250 and the commit information was not modified for sdm710.

kawaaii commented 1 month ago

You are reducing max cached process here https://github.com/Tkiliay/android_device_xiaomi_sdm710-common/commit/77acc0e5ff4bd5424e64c28e1af8d5e2989ca610, as per recent changes, it's now been changed to 1024, and we don't do override using SimpleDeviceConfig either.

kawaaii commented 1 month ago

https://github.com/Tkiliay/android_device_xiaomi_sdm710-common/commit/31a9679cc3a4b1c79e45da750638baf048a83c43 No, we don't recommend switching back to ICE.

Tkiliay commented 1 month ago

You are reducing max cached process here Tkiliay/android_device_xiaomi_sdm710-common@77acc0e, as per recent changes, it's now been changed to 1024.

Changes have been deleted

Tkiliay commented 1 month ago

Tkiliay/android_device_xiaomi_sdm710-common@31a9679 No, we don't recommend switching back to ICE.

There seems to be a problem with sdm710's support for FBEv1. Using FBEV1 will cause up to 60% drop in storage read speed. At the same time, the key service will take up a lot of CPU and cause the interface to lag.

Tkiliay commented 1 month ago

Tkiliay/android_device_xiaomi_sdm710-common@31a9679 No, we don't recommend switching back to ICE.

Is fbev1 a necessary condition?

cyberknight777 commented 1 month ago

Tkiliay/android_device_xiaomi_sdm710-common@31a9679 No, we don't recommend switching back to ICE.

There seems to be a problem with sdm710's support for FBEv1. Using FBEV1 will cause up to 60% drop in storage read speed. At the same time, the key service will take up a lot of CPU and cause the interface to lag.

Where's the perfetto trace and stats for your claim? A friend of mine on Realme XT has no issues with even FBEv2. Also what's stopping you from moving to R kernel assuming you aren't on it already?

cyberknight777 commented 1 month ago

Tkiliay/android_device_xiaomi_sdm710-common@31a9679 No, we don't recommend switching back to ICE.

Is fbev1 a necessary condition?

Yes. Preferably FBEv2.

Tkiliay commented 1 month ago

Tkiliay/android_device_xiaomi_sdm710-common@31a9679 No, we don't recommend switching back to ICE.

There seems to be a problem with sdm710's support for FBEv1. Using FBEV1 will cause up to 60% drop in storage read speed. At the same time, the key service will take up a lot of CPU and cause the interface to lag.

Where's the perfetto trace and stats for your claim? A friend of mine on Realme XT has no issues with even FBEv2. Also what's stopping you from moving to R kernel assuming you aren't on it already?

Test: Use the command line to copy a 4G file to the data directory and observe the copy speed. At the same time, use app to test and compare the version using ice and stock.

This is an issue I discussed with other developers before. It also contains the software test results. https://github.com/B--B/android_device_xiaomi_sdm710-common/issues/2

The device tree has been switched to FBEV1, but if ICE encryption can be used, the daily use experience will be better.

As for the R Kernel you mentioned, I don't seem to understand it. Can you describe it in more detail?

cyberknight777 commented 1 month ago

Tkiliay/android_device_xiaomi_sdm710-common@31a9679 No, we don't recommend switching back to ICE.

There seems to be a problem with sdm710's support for FBEv1. Using FBEV1 will cause up to 60% drop in storage read speed. At the same time, the key service will take up a lot of CPU and cause the interface to lag.

Where's the perfetto trace and stats for your claim? A friend of mine on Realme XT has no issues with even FBEv2. Also what's stopping you from moving to R kernel assuming you aren't on it already?

Test: Use the command line to copy a 4G file to the data directory and observe the copy speed. At the same time, use app to test and compare the version using ice and stock.

This is an issue I discussed with other developers before. It also contains the software test results. https://github.com/B--B/android_device_xiaomi_sdm710-common/issues/2

The device tree has been switched to FBEV1, but if ICE encryption can be used, the daily use experience will be better.

As for the R Kernel you mentioned, I don't seem to understand it. Can you describe it in more detail?

Hm alright then. ICE is fine. R kernel as in A11 base kernel, with LA.UM.9.1 tag.

Tkiliay commented 1 month ago

Tkiliay/android_device_xiaomi_sdm710-common@31a9679 No, we don't recommend switching back to ICE.

There seems to be a problem with sdm710's support for FBEv1. Using FBEV1 will cause up to 60% drop in storage read speed. At the same time, the key service will take up a lot of CPU and cause the interface to lag.

Where's the perfetto trace and stats for your claim? A friend of mine on Realme XT has no issues with even FBEv2. Also what's stopping you from moving to R kernel assuming you aren't on it already?

Test: Use the command line to copy a 4G file to the data directory and observe the copy speed. At the same time, use app to test and compare the version using ice and stock. This is an issue I discussed with other developers before. It also contains the software test results. https://github.com/B--B/android_device_xiaomi_sdm710-common/issues/2 The device tree has been switched to FBEV1, but if ICE encryption can be used, the daily use experience will be better. As for the R Kernel you mentioned, I don't seem to understand it. Can you describe it in more detail?

Hm alright then. ICE is fine. R kernel as in A11 base kernel, with LA.UM.9.1 tag.

My kernel is based on los's sdm710 and it has the latest caf tag.

Tkiliay commented 1 month ago

@cyberknight777 @kawaaii Any other requirements?

kawaaii commented 1 month ago

https://github.com/Tkiliay/android_device_xiaomi_grus/commit/4c1d15c8f9755619bfd1d72162aa5b1e2e4ebb6d Write in commit description regarding what was changed and why it was changed.

https://github.com/Tkiliay/android_device_xiaomi_grus/commit/75127147bf43533692b4af03985b034924e2d82c Unrequired

Tkiliay commented 1 month ago

Tkiliay/android_device_xiaomi_grus@4c1d15c Write in commit description regarding what was changed and why it was changed.

Tkiliay/android_device_xiaomi_grus@7512714 Unrequired

The first commit's msg has been added.

Why is the second one not needed? It was removed in https://github.com/Tkiliay/android_device_xiaomi_grus/commit/1652103af976286e37b688d44ecd44f850b82b5b, but it was not included when vendor.prop was added later, so vendor.prop obviously useless without it.

kawaaii commented 1 month ago

Why is the second one not needed? It was removed in https://github.com/Tkiliay/android_device_xiaomi_grus/commit/1652103af976286e37b688d44ecd44f850b82b5b, but it was not included when vendor.prop was added later, so vendor.prop obviously useless without it.

https://github.com/PixelOS-AOSP/build/blob/fourteen/core/sysprop.mk#L362

cyberknight777 commented 1 month ago

As for kernel:

1) https://github.com/Tkiliay/android_kernel_xiaomi_sdm710/commit/9fe5d4edf5f3c69b88be5e03bcdf6d1aac146d35

2) https://github.com/Tkiliay/android_kernel_xiaomi_sdm710/commit/29c307acad92c3cebedbeb27dec4db689d34466b

3) https://github.com/Tkiliay/android_kernel_xiaomi_sdm710/commit/a3c61a1bf34cfd70fb73acf97345ebb8b4f0ee2e

4) https://github.com/Tkiliay/android_kernel_xiaomi_sdm710/commit/40a38d06d5d8cfea0b7990dd962897d1c7da2a28

5) https://github.com/Tkiliay/android_kernel_xiaomi_sdm710/commit/441ec1825a59a7fc27e9100d65a289edd0824d24

6) https://github.com/Tkiliay/android_kernel_xiaomi_sdm710/commit/eb02166b3da8c2597c62f5b152d422275b11c3fa

Otherwise your kernel looks fine to me.

Tkiliay commented 1 month ago

Why is the second one not needed? It was removed in Tkiliay/android_device_xiaomi_grus@1652103, but it was not included when vendor.prop was added later, so vendor.prop obviously useless without it.

https://github.com/PixelOS-AOSP/build/blob/fourteen/core/sysprop.mk#L362

Changes have been removed

Tkiliay commented 1 month ago

As for kernel:

  1. Tkiliay/android_kernel_xiaomi_sdm710@9fe5d4e
  • That commit is kinda skeptical. It has been known to randomly break notifs, cause black screen of deaths and such.
  1. Tkiliay/android_kernel_xiaomi_sdm710@29c307a
  • Draco's commits were made for ancient devices and generally do not apply for newer devices. (yes i know your device isn't new but it isn't as ancient as the devices this commit was originally made for)
  1. Tkiliay/android_kernel_xiaomi_sdm710@a3c61a1
  1. Tkiliay/android_kernel_xiaomi_sdm710@40a38d0
  1. Tkiliay/android_kernel_xiaomi_sdm710@441ec18
  • Misleading commit message.
  1. Tkiliay/android_kernel_xiaomi_sdm710@eb02166

Otherwise your kernel looks fine to me.

The kernel has been updated. Please review again.

cyberknight777 commented 1 month ago

As for kernel:

  1. Tkiliay/android_kernel_xiaomi_sdm710@9fe5d4e
  • That commit is kinda skeptical. It has been known to randomly break notifs, cause black screen of deaths and such.
  1. Tkiliay/android_kernel_xiaomi_sdm710@29c307a
  • Draco's commits were made for ancient devices and generally do not apply for newer devices. (yes i know your device isn't new but it isn't as ancient as the devices this commit was originally made for)
  1. Tkiliay/android_kernel_xiaomi_sdm710@a3c61a1
  1. Tkiliay/android_kernel_xiaomi_sdm710@40a38d0
  1. Tkiliay/android_kernel_xiaomi_sdm710@441ec18
  • Misleading commit message.
  1. Tkiliay/android_kernel_xiaomi_sdm710@eb02166

Otherwise your kernel looks fine to me.

The kernel has been updated. Please review again.

https://github.com/Tkiliay/android_kernel_xiaomi_sdm710/commit/3ed67aeb75f9431b97e0baa1e58dad7e1f2ad752 remove redundant whitespace after CONFIG_ZRAM_DEDUP and fix commit prefix, should be arm64: configs