[PATCH 0/5] Add support for Xiaomi Poco F1 EBBG variant

[PATCH 0/5] Add support for Xiaomi Poco F1 EBBG variant

[Joel Selvaraj]

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

 arch/arm64/boot/dts/qcom/Makefile                      |  3 ++-
 ...ryllium.dts => sdm845-xiaomi-beryllium-common.dtsi} |  8 ++++----
 .../boot/dts/qcom/sdm845-xiaomi-beryllium-ebbg.dts     | 10 ++++++++++
 .../boot/dts/qcom/sdm845-xiaomi-beryllium-tianma.dts   | 10 ++++++++++
 4 files changed, 26 insertions(+), 5 deletions(-)
 rename arch/arm64/boot/dts/qcom/{sdm845-xiaomi-beryllium.dts => sdm845-xiaomi-beryllium-common.dtsi} (98%)
 create mode 100644 arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-ebbg.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-tianma.dts

-- 
2.36.1

Re: [PATCH 0/5] Add support for Xiaomi Poco F1 EBBG variant

[Konrad Dybcio]

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

Re: [PATCH 0/5] Add support for Xiaomi Poco F1 EBBG variant

[Joel Selvaraj]

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

Re: [PATCH 0/5] Add support for Xiaomi Poco F1 EBBG variant

[Krzysztof Kozlowski]

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

Re: [PATCH 0/5] Add support for Xiaomi Poco F1 EBBG variant

[Joel Selvaraj]

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

Re: [PATCH 0/5] Add support for Xiaomi Poco F1 EBBG variant

[Krzysztof Kozlowski]

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

Re: [PATCH 0/5] Add support for Xiaomi Poco F1 EBBG variant

[Pavel Machek]

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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 0/5] Add support for Xiaomi Poco F1 EBBG variant

[Caleb Connolly]

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