Message ID | MN2PR02MB702415D7BF12B7B7A41B2D38D9829@MN2PR02MB7024.namprd02.prod.outlook.com (mailing list archive) |
---|---|
Headers | show |
Series | Add support for Xiaomi Poco F1 EBBG variant | expand |
On 8.07.2022 13:12, Joel Selvaraj wrote: > There are two variants of Xiaomi Poco F1. > - Tianma variant with NOVATEK NT36672A panel + touchscreen manufactured > by Tianma > - EBBG variant with Focaltech FT8719 panel + touchscreen manufactured > by EBBG > > The current sdm845-xiaomi-beryllium.dts represents tianma panel variant. > > To add support for the EBBG variant, let's split this into 3 files, > - sdm845-xiaomi-beryllium-common.dtsi which contains all the common nodes > - sdm845-xiaomi-beryllium-tianma.dts for the tianma variant > - sdm845-xiaomi-beryllium-ebbg.dts for the ebbg variant > > Note: > ----- > Both the panels are already upstreamed and the split is based on them. > There were patches earlier for both the touchscreens, but they are not > accepted upstream yet. Once they are accepted, we will add them to > respective variants. Hi, I believe this is not the correct approach. This may work short-term, but you will have to prepare 2 separate images for the device and mistaking them may cause irreversible hw damage at worst, or lots of user complaining at best. Instead, I think it's about time we should look into implementing dynamic panel detection. Qualcomm devices do this by parsing the command line [1], as LK/XBL gives you a nice-ish string to work with that you can simply match against a label. Other vendors may use custom mechanisms, such as a resistor / GPIO to determine which panel (or generally hw config), but implementing this mechanism would make upstreaming of lots of other devices easier.. This issue concerns many phones (and well, devices in general), as they are seldom made with only one configuration due to supply chain strategies. Konrad [1] https://github.com/LineageOS/android_kernel_xiaomi_sdm845/blob/lineage-19.1/drivers/gpu/drm/msm/dsi-staging/dsi_display.c
Hi Konrad Dybcio, On 09/07/22 02:13, Konrad Dybcio wrote: > I believe this is not the correct approach. This may work short-term, but > you will have to prepare 2 separate images for the device and mistaking them > may cause irreversible hw damage at worst, or lots of user complaining at best. > Instead, I think it's about time we should look into implementing dynamic panel > detection. > > Qualcomm devices do this by parsing the command line [1], as LK/XBL > gives you a nice-ish string to work with that you can simply match > against a label. Other vendors may use custom mechanisms, such as > a resistor / GPIO to determine which panel (or generally hw config), > but implementing this mechanism would make upstreaming of lots of other > devices easier.. > > This issue concerns many phones (and well, devices in general), as > they are seldom made with only one configuration due to supply chain > strategies. Yes. I very much agree on this. It would be proper to have dynamic panel detection. But I am afraid if I can implement such a solution. It would be nice if people working on MSM DRM stack have a look at this. But, do we don't need to block this patch until such a solution is developed? > Konrad Regards Joel
On 08/07/2022 13:12, Joel Selvaraj wrote: > There are two variants of Xiaomi Poco F1. > - Tianma variant with NOVATEK NT36672A panel + touchscreen manufactured > by Tianma > - EBBG variant with Focaltech FT8719 panel + touchscreen manufactured > by EBBG > > The current sdm845-xiaomi-beryllium.dts represents tianma panel variant. > > To add support for the EBBG variant, let's split this into 3 files, > - sdm845-xiaomi-beryllium-common.dtsi which contains all the common nodes > - sdm845-xiaomi-beryllium-tianma.dts for the tianma variant > - sdm845-xiaomi-beryllium-ebbg.dts for the ebbg variant > > Note: > ----- > Both the panels are already upstreamed and the split is based on them. > There were patches earlier for both the touchscreens, but they are not > accepted upstream yet. Once they are accepted, we will add them to > respective variants. > > Joel Selvaraj (5): > arm64: dts: sdm845-xiaomi-beryllium: rename beryllium.dts into > beryllium-common.dtsi > arm64: dts: qcom: sdm845-xiaomi-beryllium-common: generalize the > display panel node > arm64: dts: qcom: sdm845-xiaomi-beryllium: introduce tianma variant > arm64: dts: qcom: sdm845-xiaomi-beryllium: introduce ebbg variant > arm64: dts: qcom: Makefile: split beryllium into tianma and ebbg > variant None of your patches reached recipients and mailing lists. Best regards, Krzysztof
Hi Krzysztof Kozlowski On 12/07/22 18:57, Krzysztof Kozlowski wrote: > None of your patches reached recipients and mailing lists. Thanks for letting me know. I didnt notice. It was shown in patchwork website and I thought it reached the mailing list too. I have RESEND the patches. This time, the cover letter (0/5) seems to be in a different thread and the rest of the patches (1 to 5/5) seems to be in a different thread. But all of them reached the mailing list though. I am not sure what is causing the issue though. Can this accepted? or do I need to resend them again? > Best regards, > Krzysztof Best Regards, Joel Selvaraj
On 13/07/2022 06:35, Joel Selvaraj wrote: > Hi Krzysztof Kozlowski > > On 12/07/22 18:57, Krzysztof Kozlowski wrote: >> None of your patches reached recipients and mailing lists. > > Thanks for letting me know. I didnt notice. It was shown in patchwork > website and I thought it reached the mailing list too. I have RESEND the > patches. This time, the cover letter (0/5) seems to be in a different > thread and the rest of the patches (1 to 5/5) seems to be in a different > thread. But all of them reached the mailing list though. I am not sure > what is causing the issue though. Can this accepted? or do I need to > resend them again? I saw your patches but not connected to cover letter. As you said, lore also misses them from cover letter: https://lore.kernel.org/all/BY5PR02MB700954C6003BC5D5B6AAB1B7D9899@BY5PR02MB7009.namprd02.prod.outlook.com/ but they are on the lists: https://lore.kernel.org/all/BY5PR02MB7009A49AD394747ACB80F746D9899@BY5PR02MB7009.namprd02.prod.outlook.com/ It's fine, but you should fix your setup. You can use whatever tools you prefer as long as you create proper result. The easiest is however to use git format-patch --cover-letter -5 && git send-email .... (and useful also git branch --edit-description && git git format-patch --cover-letter --cover-from-description=subject). Best regards, Krzysztof
Hi! > > - Tianma variant with NOVATEK NT36672A panel + touchscreen manufactured > > by Tianma > > - EBBG variant with Focaltech FT8719 panel + touchscreen manufactured > > by EBBG > > > > The current sdm845-xiaomi-beryllium.dts represents tianma panel variant. > > > > To add support for the EBBG variant, let's split this into 3 files, > > - sdm845-xiaomi-beryllium-common.dtsi which contains all the common nodes > > - sdm845-xiaomi-beryllium-tianma.dts for the tianma variant > > - sdm845-xiaomi-beryllium-ebbg.dts for the ebbg variant > > > > Note: > > ----- > > Both the panels are already upstreamed and the split is based on them. > > There were patches earlier for both the touchscreens, but they are not > > accepted upstream yet. Once they are accepted, we will add them to > > respective variants. > Hi, > > I believe this is not the correct approach. This may work short-term, but > you will have to prepare 2 separate images for the device and mistaking them > may cause irreversible hw damage at worst, or lots of user complaining at best. > Instead, I think it's about time we should look into implementing dynamic panel > detection. It is certainly better than current state. Now user will need to decide what panel he has. > Qualcomm devices do this by parsing the command line [1], as LK/XBL > gives you a nice-ish string to work with that you can simply match > against a label. Other vendors may use custom mechanisms, such as > a resistor / GPIO to determine which panel (or generally hw config), > but implementing this mechanism would make upstreaming of lots of other > devices easier.. I believe ideal solution would be bootloader passing the correct dtb to the kernel... Best regards, Pavel
On 08/07/2022 21:43, Konrad Dybcio wrote: > > > On 8.07.2022 13:12, Joel Selvaraj wrote: >> There are two variants of Xiaomi Poco F1. >> - Tianma variant with NOVATEK NT36672A panel + touchscreen manufactured >> by Tianma >> - EBBG variant with Focaltech FT8719 panel + touchscreen manufactured >> by EBBG >> >> The current sdm845-xiaomi-beryllium.dts represents tianma panel variant. >> >> To add support for the EBBG variant, let's split this into 3 files, >> - sdm845-xiaomi-beryllium-common.dtsi which contains all the common nodes >> - sdm845-xiaomi-beryllium-tianma.dts for the tianma variant >> - sdm845-xiaomi-beryllium-ebbg.dts for the ebbg variant >> >> Note: >> ----- >> Both the panels are already upstreamed and the split is based on them. >> There were patches earlier for both the touchscreens, but they are not >> accepted upstream yet. Once they are accepted, we will add them to >> respective variants. > Hi, > > I believe this is not the correct approach. This may work short-term, but > you will have to prepare 2 separate images for the device and mistaking them > may cause irreversible hw damage at worst, or lots of user complaining at best. > Instead, I think it's about time we should look into implementing dynamic panel > detection. > > Qualcomm devices do this by parsing the command line [1], as LK/XBL > gives you a nice-ish string to work with that you can simply match > against a label. Other vendors may use custom mechanisms, such as > a resistor / GPIO to determine which panel (or generally hw config), > but implementing this mechanism would make upstreaming of lots of other > devices easier.. Regarding dynamic panel detection. A mechanism for choosing DT nodes based on some generic (read: extensible) matching feature would be pretty neat.... e.g. matching cmdline: panel { compatible = "some,w3ird-panel"; /* Only attempt to probe a driver for this node if cmdline contains * this string. How this is described and the type(s) of matching to * use could be defined. */ match-if-cmdline = "msm_drm.dsi_display0=some_w3ird-panel"; }; or perhaps GPIO state: panel { compatible = "some,w3ird-panel"; /* Only attempt to probe a driver for this node if GPIO 43 on tlmm is high, * and GPIO 44 is low. */ match-if-gpios = <&tlmm 43 GPIO_ACTIVE_HIGH>, <&tlmm 44 GPIO_ACTIVE_LOW>; }; This certainly introduces the temptation to do awful things... > > This issue concerns many phones (and well, devices in general), as > they are seldom made with only one configuration due to supply chain > strategies. It would be really nice to solve this in-kernel, chainloading a bootloader sometimes kinda sucks. > > > Konrad > > [1] https://github.com/LineageOS/android_kernel_xiaomi_sdm845/blob/lineage-19.1/drivers/gpu/drm/msm/dsi-staging/dsi_display.c -- Kind Regards, Caleb