PixelOS-AOSP / official_devices

50 stars 59 forks source link

New maintainer #358

Closed ahmedtohamy1 closed 6 months ago

ahmedtohamy1 commented 7 months ago

Name and Codename of the device you want to apply for

Mi 10T (Apollo)

Device Tree sources

https://github.com/Apollo-Trees/device_xiaomi_sm8250-common
https://github.com/Apollo-Trees/vendor_xiaomi_sm8250-common
https://github.com/ahmedtohamy1/device_xiaomi_apollo
https://github.com/ahmedtohamy1/vendor_xiaomi_apollo
https://github.com/EmanuelCN/kernel_xiaomi_sm8250/

What ROM's do you maintain officially/unofficially

evolution

How long have you been building Custom ROMs

since 2017 
https://4pda.to/forum/index.php?showtopic=822107&st=7740#entry68554791

Any Exceptions/special concern?

This is the second time I apply (first one was a year ago), And its obv for my userbase which likes pixelos soo much also do I. so they kept insisting me on applying for apollo to maintain it officially, Aiming to add more to team and project and lets see.

List of changes/patches applied to source, if any

No

Contact

Telegram username

Ahmed_tohamy

Link to your unofficial PixelOS Build Post other than XDA

https://t.me/AgmadSpace/240
ahmedtohamy1 commented 7 months ago

Switched to https://github.com/projects-nexus/nexus_kernel_xiaomi_sm8250 to met vantom recommendations about ksu

ahmedtohamy1 commented 6 months ago

Status update: Qpr2 booted https://t.me/AgmadSpace/269 but after some source commit and qpr2 dt side fixes Source commit is: https://github.com/ricedroidOSS/android_build/commit/338d60c8a12bfe2294636c2f44d4fde26039025c

I believe thats commit was there before qpr2

basamaryan commented 6 months ago

@ahmedtohamy1 No, it was not there before QPR2 and it will not be there on QPR2. You'll have to change the configuration in your tree instead.

ahmedtohamy1 commented 6 months ago

@basamaryan fair enough just revised alioth official common tree and it doesnt have it as mine, so reverted.

https://github.com/Apollo-Trees/device_xiaomi_sm8250-common/commit/18fefb36e3660a61e07b8dc9cab2e6a01ac2c480

ahmedtohamy1 commented 6 months ago

@basamaryan but nah i was right it was there in qpr1 https://github.com/PixelOS-AOSP/build/commit/8c1845c3594faefc10988992dba42dea9f2a382c

cyberknight777 commented 6 months ago

You will not inline a kernel that ships KernelSU and/or its hooks.

ahmedtohamy1 commented 6 months ago

@cyberknight777 i already declared that i switched to https://github.com/projects-nexus/nexus_kernel_xiaomi_sm8250

Which is already used in alioth official builds after i discussed that topic with vantom

cyberknight777 commented 6 months ago

Use the kernel in github.com/PixelOS-Devices. Not the one in nexus-devices as that ships the KernelSU driver and hooks.

ahmedtohamy1 commented 6 months ago

@cyberknight777 oki got it, will use. Any other reviews about dts itself?

kawaaii commented 6 months ago

Which kernel are you using again, if you don't mind me asking?

kawaaii commented 6 months ago
  1. Sorry but we don't do that here https://github.com/Apollo-Trees/device_xiaomi_sm8250-common/commit/a9271894cefa89fcf00d99b3d0c0d71b01e86547

  2. https://github.com/Apollo-Trees/device_xiaomi_sm8250-common/commit/07755610d3261ad6da54b0e0ba653adb08fd3c6b Logs there are not really justifying on why the QTI Thermal is failing to intialize.

  3. Amend with proper authorship, not the thanks in the commit description. https://github.com/Apollo-Trees/device_xiaomi_sm8250-common/commit/ae7d3a2c0ad15f0613d52fb4ec9adb49be18d50d

  4. https://github.com/Apollo-Trees/device_xiaomi_sm8250-common/commit/e1235f59945e4c2c0a21b95d0362df6b3ee16218 How is Android.mk changes related with powerhint changes, separate both changes in two different commits.

  5. Is this change really required? What happens when you don't have it? https://github.com/Apollo-Trees/device_xiaomi_sm8250-common/commit/63395d6edaf5e7df62d0db873ab2016cf396458e

  6. Was there really any need to move back to armv8-a for TARGET_ARCH_VARIANT ? If you're talking about the build issues, it should have been fixed if you had the TARGET_2ND_ARCH_VARIANT as armv8-a. https://github.com/Apollo-Trees/device_xiaomi_sm8250-common/commit/18fefb36e3660a61e07b8dc9cab2e6a01ac2c480

  7. Why would you change SHIPPING_API_LEVEL? https://github.com/ahmedtohamy1/device_xiaomi_apollo/commit/249daa1fdd98f458342d3259c4f54c850f731490

  8. https://github.com/ahmedtohamy1/device_xiaomi_apollo/commit/74caf0051d1b1237a818895ad282f3d869b2f45d https://github.com/ahmedtohamy1/device_xiaomi_apollo/commit/f7d5951b2586208209f7d164a5e523080d375d1c Squash merge with better commit message and description. And yeah so you removed and added the same thing at last huh https://github.com/ahmedtohamy1/device_xiaomi_apollo/commit/9111fbfab9b27eebd5dc2c9ebb36023ee30d8edc

  9. https://github.com/ahmedtohamy1/device_xiaomi_apollo/commit/ec21ce37c72a6aa27ff5e9222d3a58bef7bc758f Since when does importing configs remove these?

  10. Any reasons on why you are doing this? https://github.com/ahmedtohamy1/device_xiaomi_apollo/commit/bd663e9ce76510a84eefd760a1f782a4ee1a27bc

  11. Use the shim from hardware/compat https://github.com/ahmedtohamy1/device_xiaomi_apollo/commit/44a22b832313e76b5b066b839899f8471f395f78 Also why are you using two same shims?

  12. https://github.com/Apollo-Trees/device_xiaomi_sm8250-common/commit/ba94edea0f5739b53719a86178fdaacf18b7e06f You say apollo doesn't support wrappedkey but I feel like you're missing changes for making it work. Go through https://source.android.com/docs/security/features/encryption/hw-wrapped-keys.

ahmedtohamy1 commented 6 months ago

@kawaaii for kernel currently using this https://github.com/LineageOS/android_kernel_xiaomi_sm8250 as even nexus rebasing over it and its doing fine now.

for changes u mentioned:

  1. okay dropped dolby entirely
  2. nothing more on logs thats the case on rest of mikona devices, but will drop for now and retest
  3. dropped the entire powerhint changes already was dropped locally after some testing
  4. fixed
  5. part of display hal device side wasnt building
  6. basamaryan said above it shouldnt be like that so i simply reverted, no huge impact noticed, revise comments above
  7. already using blobs from munch which are from a13 so tested upreving api and it went without issues as per logs so kept it also removed in rebased tree
  8. already rebased apollo specific tree yesterday with cleanups fixed that too https://github.com/ahmedtohamy1/device_xiaomi_apollo/commits/rebase/
  9. had that long time ago will revert anyway
  10. was old config found when i started doing this device, ik now its deprecated will drop
  11. fixed
  12. will do anyway heres the reworked trees afted ur notes: https://github.com/Apollo-Trees/device_xiaomi_sm8250-common/commits/rebase https://github.com/ahmedtohamy1/device_xiaomi_apollo/commits/rebase/
basamaryan commented 6 months ago

@ahmedtohamy1 I used to just do https://github.com/PixelOS-Devices/device_xiaomi_sm6150-common/commit/d731e4495bcc4bc1ee9883a5ad0ae4d752b0b18c but have now switched to https://review.lineageos.org/c/LineageOS/android_device_xiaomi_sm6150-common/+/387795

ahmedtohamy1 commented 6 months ago

@basamaryan got it

kawaaii commented 6 months ago
  1. nothing more on logs thats the case on rest of mikona devices, but will drop for now and retest

There is no such thing as nothing more on logs, build yourself eng build and find out what's wrong.

  1. part of display hal device side wasnt building

What part?

  1. already using blobs from munch which are from a13 so tested upreving api and it went without issues as per logs so kept it also removed in rebased tree

That's not a proper reason for upreving SHIPPING_API level to R for a device launched with Q.

kawaaii commented 6 months ago
  1. Use INTERACTION hint rather than these props. https://github.com/Apollo-Trees/device_xiaomi_sm8250-common/commit/a185a78ac224b203d7fc184fa42a6f7980c7ab19

  2. https://github.com/Apollo-Trees/device_xiaomi_sm8250-common/commit/4c975cec8d3cb78e673d730a48b929f6b4ef1b8c Can you tell me why you don't want to use AOSP default configuration?

  3. https://github.com/Apollo-Trees/device_xiaomi_sm8250-common/commit/40d5d65c19a623a5c928db1bc66eacd5ace4be48 Why in the tree? This is something to be done in source. Also why GoogleDialer?

  4. https://github.com/Apollo-Trees/device_xiaomi_sm8250-common/commit/e8d866c054f8cc8227cce8e3669e301e3d6a997c Can't you just use LLVM_BINUTILS, which is enabled by default?

  5. Just wanted to know, does your device not support FM in stock?

ahmedtohamy1 commented 6 months ago

@kawaaii 1.2.3.4. will drop no prob 5 no it doesn't

kawaaii commented 6 months ago

@kawaaii 1.2.3.4. will drop no prob 5 no it doesn't

I am actually interested in knowing why you did those changes in first place now, especially 2,3 and 4.

ahmedtohamy1 commented 6 months ago

@kawaaii

  1. Stock ones takes ages for me especially when u have many apps those acts better
  2. Gdialer was earlier only ringing without showing interface, 2 fixed it
  3. Was using sdclang which was better with this flags, aosp clang has no issue with them too
kawaaii commented 6 months ago
  1. part of display hal device side wasnt building

What part?

?

ahmedtohamy1 commented 6 months ago

@kawaaii https://github.com/CHRISL7/hardware_qcom_display/commit/42de8cb42a18a56e91e4cf6e449303476eb07770

https://github.com/LineageOS/android_vendor_qcom_opensource_interfaces/blob/77b3a98026fa129726693156dd1aae9d6019567e/display/mapperextensions/1.3/Android.bp#L14

ahmedtohamy1 commented 6 months ago

@kawaaii also stock does build it

kawaaii commented 6 months ago

@kawaaii also stock does build it

Show me stock having vendor.qti.hardware.display.mapperextensions@1.2 built, because I can just find stock having till vendor.qti.hardware.display.mapperextensions@1.1 while you claim stock has @1.2 interface as well. Am I missing something?

ahmedtohamy1 commented 6 months ago

@kawaaii also stock does build it

Show me stock having vendor.qti.hardware.display.mapperextensions@1.2 built, because I can just find stock having till vendor.qti.hardware.display.mapperextensions@1.1 while you claim stock has @1.2 interface as well. Am I missing something?

Stock has old version of it, then why not use newer too?

ahmedtohamy1 commented 6 months ago

@kawaaii Also as i told i always pick that too https://github.com/EmanuelCN/hardware_qcom-caf_display_sm8250/commit/c2087da868cf29a786bbce21c3fc0fcd02c8b82a

ahmedtohamy1 commented 6 months ago

@kawaaii so what?

whyredfire commented 6 months ago

Hey, I've read the entire conversation and things seems fine now. We might need a very few more changes but it can be fixed in the post.

Please proceed with a PR in official_devices.