| Message ID | 20240116094935.9988-3-quic_riteshk@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | add display and panel on qcm6490 idp | expand |
On Tue, 16 Jan 2024 at 11:49, Ritesh Kumar <quic_riteshk@quicinc.com> wrote: > > Enable Display Subsystem with Novatek NT36672E Panel > on qcm6490 idp platform. Is this panel always present on the IDP board or is it an optional addon, like the panels for all the RBn boards? > > Signed-off-by: Ritesh Kumar <quic_riteshk@quicinc.com> > --- > arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 100 +++++++++++++++++++++++ > 1 file changed, 100 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts > index 2a6e4907c5ee..efa5252130a1 100644 > --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts > +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts > @@ -9,6 +9,7 @@ > #define PM7250B_SID 8 > #define PM7250B_SID1 9 > > +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h> > #include <dt-bindings/regulator/qcom,rpmh-regulator.h> > #include "sc7280.dtsi" > #include "pm7250b.dtsi" > @@ -38,6 +39,25 @@ > stdout-path = "serial0:115200n8"; > }; > > + lcd_disp_bias: lcd-disp-bias-regulator { > + compatible = "regulator-fixed"; > + regulator-name = "lcd_disp_bias"; > + regulator-min-microvolt = <5500000>; > + regulator-max-microvolt = <5500000>; > + gpio = <&pm7250b_gpios 2 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + pinctrl-names = "default"; > + pinctrl-0 = <&lcd_disp_bias_en>; > + }; > + > + pm8350c_pwm_backlight: backlight { > + compatible = "pwm-backlight"; > + pwms = <&pm8350c_pwm 3 65535>; > + enable-gpios = <&pm8350c_gpios 7 GPIO_ACTIVE_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pmic_lcd_bl_en>; > + }; > + > reserved-memory { > xbl_mem: xbl@80700000 { > reg = <0x0 0x80700000 0x0 0x100000>; > @@ -420,6 +440,86 @@ > }; > }; > > +&gpu { > + status = "disabled"; > +}; > + > +&mdss { > + status = "okay"; > +}; > + > +&mdss_dsi { > + vdda-supply = <&vreg_l6b_1p2>; > + status = "okay"; > + > + panel@0 { > + compatible = "novatek,nt36672e"; > + reg = <0>; > + > + reset-gpios = <&tlmm 44 GPIO_ACTIVE_HIGH>; > + > + vddi-supply = <&vreg_l8c_1p62>; > + avdd-supply = <&lcd_disp_bias>; > + avee-supply = <&lcd_disp_bias>; > + > + backlight = <&pm8350c_pwm_backlight>; > + > + port { > + panel0_in: endpoint { > + remote-endpoint = <&mdss_dsi0_out>; > + }; > + }; > + }; > +}; > + > +&mdss_dsi0_out { > + remote-endpoint = <&panel0_in>; > + data-lanes = <0 1 2 3>; > +}; > + > +&mdss_dsi_phy { > + vdds-supply = <&vreg_l10c_0p88>; > + status = "okay"; > +}; > + > +&pm7250b_gpios { > + lcd_disp_bias_en: lcd-disp-bias-en-state { > + pins = "gpio2"; > + function = "func1"; > + bias-disable; > + qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>; > + input-disable; > + output-enable; > + power-source = <0>; > + }; > +}; > + > +&pm8350c_gpios { > + pmic_lcd_bl_en: pmic-lcd-bl-en-state { > + pins = "gpio7"; > + function = "normal"; > + bias-disable; > + qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>; > + output-low; > + power-source = <0>; > + }; > + > + pmic_lcd_bl_pwm: pmic-lcd-bl-pwm-state { > + pins = "gpio8"; > + function = "func1"; > + bias-disable; > + qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>; > + output-low; > + power-source = <0>; > + }; > +}; > + > +&pm8350c_pwm { > + pinctrl-names = "default"; > + pinctrl-0 = <&pmic_lcd_bl_pwm>; > + status = "okay"; > +}; > + > &qupv3_id_0 { > status = "okay"; > }; > -- > 2.17.1 >
On Tue, 16 Jan 2024 at 14:06, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > On 1/16/24 10:49, Ritesh Kumar wrote: > > Enable Display Subsystem with Novatek NT36672E Panel > > on qcm6490 idp platform. > > > > Signed-off-by: Ritesh Kumar <quic_riteshk@quicinc.com> > > --- > > arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 100 +++++++++++++++++++++++ > > 1 file changed, 100 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts > > index 2a6e4907c5ee..efa5252130a1 100644 > > --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts > > +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts > > @@ -9,6 +9,7 @@ > > #define PM7250B_SID 8 > > #define PM7250B_SID1 9 > > > > +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h> > > #include <dt-bindings/regulator/qcom,rpmh-regulator.h> > > #include "sc7280.dtsi" > > #include "pm7250b.dtsi" > > @@ -38,6 +39,25 @@ > > stdout-path = "serial0:115200n8"; > > }; > > > > + lcd_disp_bias: lcd-disp-bias-regulator { > > + compatible = "regulator-fixed"; > > + regulator-name = "lcd_disp_bias"; > > + regulator-min-microvolt = <5500000>; > > + regulator-max-microvolt = <5500000>; > > + gpio = <&pm7250b_gpios 2 GPIO_ACTIVE_HIGH>; > > + enable-active-high; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&lcd_disp_bias_en>; > > property-n > property-names > > all throughout the patch > > > +&gpu { > > + status = "disabled"; > > +}; > > Hm.. generally we disable the GPU in the SoC DT, but that doesn't > seem to have happened here.. > > Thinking about it more, is disabling it here necessary? Does it > not fail gracefully? Missed this. I'd say, I don't see a reason to disable it at all. The GPU should be working on sc7280 / qcm4290.
On 1/23/2024 11:34 PM, Konrad Dybcio wrote: > > > On 1/23/24 16:12, Dmitry Baryshkov wrote: >> On Tue, 23 Jan 2024 at 15:43, Ritesh Kumar <quic_riteshk@quicinc.com> >> wrote: >>> >>> >>> On 1/16/2024 6:27 PM, Dmitry Baryshkov wrote: >>> >>>> On Tue, 16 Jan 2024 at 14:06, Konrad Dybcio >>>> <konrad.dybcio@linaro.org> wrote: >>>>> >>>>> >>>>> On 1/16/24 10:49, Ritesh Kumar wrote: >>>>>> Enable Display Subsystem with Novatek NT36672E Panel >>>>>> on qcm6490 idp platform. >>>>>> >>>>>> Signed-off-by: Ritesh Kumar <quic_riteshk@quicinc.com> >>>>>> --- >>>>>> arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 100 >>>>>> +++++++++++++++++++++++ >>>>>> 1 file changed, 100 insertions(+) >>>>>> >>>>>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts >>>>>> b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts >>>>>> index 2a6e4907c5ee..efa5252130a1 100644 >>>>>> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts >>>>>> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts >>>>>> @@ -9,6 +9,7 @@ >>>>>> #define PM7250B_SID 8 >>>>>> #define PM7250B_SID1 9 >>>>>> >>>>>> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h> >>>>>> #include <dt-bindings/regulator/qcom,rpmh-regulator.h> >>>>>> #include "sc7280.dtsi" >>>>>> #include "pm7250b.dtsi" >>>>>> @@ -38,6 +39,25 @@ >>>>>> stdout-path = "serial0:115200n8"; >>>>>> }; >>>>>> >>>>>> + lcd_disp_bias: lcd-disp-bias-regulator { >>>>>> + compatible = "regulator-fixed"; >>>>>> + regulator-name = "lcd_disp_bias"; >>>>>> + regulator-min-microvolt = <5500000>; >>>>>> + regulator-max-microvolt = <5500000>; >>>>>> + gpio = <&pm7250b_gpios 2 GPIO_ACTIVE_HIGH>; >>>>>> + enable-active-high; >>>>>> + pinctrl-names = "default"; >>>>>> + pinctrl-0 = <&lcd_disp_bias_en>; >>>>> property-n >>>>> property-names >>>>> >>>>> all throughout the patch >>> >>> Thanks, I will update in the new version. >>> >>>>>> +&gpu { >>>>>> + status = "disabled"; >>>>>> +}; >>>>> Hm.. generally we disable the GPU in the SoC DT, but that doesn't >>>>> seem to have happened here.. >>>>> >>>>> Thinking about it more, is disabling it here necessary? Does it >>>>> not fail gracefully? >>>> Missed this. >>>> >>>> I'd say, I don't see a reason to disable it at all. The GPU should be >>>> working on sc7280 / qcm4290. >>> >>> With GPU device node enabled, adreno_bind failure is seen as the >>> "speed_bin" was not populated on QCM6490 target which leads to display >>> bind failure. >> >> Excuse me please. The GPU node for sc7280 already has speed_bin, which >> points to qfprom + 0x1e9, bits 5 to 9. >> >> Do you mean that qcm6490 uses different speed bin location? Or >> different values for the speed bins? >> >>> Spoke with GPU team and on QCM6490 board, only CPU rendering is >>> supported for now and there is no plan to enable GPU rendering in near >>> future. >> >> This sounds like having the feature disabled for no particular reason. >> Both the kernel and Mesa have supported the Adreno 635 for quite a >> while. > > 643 [1], [2] > >> >>> In this regard, what do you suggest >>> >>> 1) Disable GPU in QCM6490 DT (as per the current patch) >>> 2) Disable GPU in the SoC DT, but enable it in other platform DTs. >>> (This >>> will prompt change in all the dt's and we don't have all the devices to >>> test) >> >> The second option definitely follows what is present on other platforms. >> >>> Please let me know your views on it. >> >> Please enable the GPU instead. > > +1 > > Konrad > > [1] > https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25408/diffs?commit_id=b1e851d66c3a3e53f1a464023f675f3f6cbd3503 > [2] > https://patches.linaro.org/project/linux-arm-msm/cover/20230926-topic-a643-v1-0-7af6937ac0a3@linaro.org/ Thanks for the help. After applying missing patches from series https://patches.linaro.org/project/linux-arm-msm/cover/20230926-topic-a643-v1-0-7af6937ac0a3@linaro.org/ in my local build, GPU is working fine. GPU disablement change is not needed. I will send new version of patch removing GPU part and addressing other review comments. Thanks, Ritesh
On Mon, 12 Feb 2024 at 14:28, Ritesh Kumar <quic_riteshk@quicinc.com> wrote: > > > On 1/23/2024 11:34 PM, Konrad Dybcio wrote: > > > > > > On 1/23/24 16:12, Dmitry Baryshkov wrote: > >> On Tue, 23 Jan 2024 at 15:43, Ritesh Kumar <quic_riteshk@quicinc.com> > >> wrote: > >>> > >>> > >>> On 1/16/2024 6:27 PM, Dmitry Baryshkov wrote: > >>> > >>>> On Tue, 16 Jan 2024 at 14:06, Konrad Dybcio > >>>> <konrad.dybcio@linaro.org> wrote: > >>>>> > >>>>> > >>>>> On 1/16/24 10:49, Ritesh Kumar wrote: > >>>>>> Enable Display Subsystem with Novatek NT36672E Panel > >>>>>> on qcm6490 idp platform. > >>>>>> > >>>>>> Signed-off-by: Ritesh Kumar <quic_riteshk@quicinc.com> > >>>>>> --- > >>>>>> arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 100 > >>>>>> +++++++++++++++++++++++ > >>>>>> 1 file changed, 100 insertions(+) > >>>>>> > >>>>>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts > >>>>>> b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts > >>>>>> index 2a6e4907c5ee..efa5252130a1 100644 > >>>>>> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts > >>>>>> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts > >>>>>> @@ -9,6 +9,7 @@ > >>>>>> #define PM7250B_SID 8 > >>>>>> #define PM7250B_SID1 9 > >>>>>> > >>>>>> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h> > >>>>>> #include <dt-bindings/regulator/qcom,rpmh-regulator.h> > >>>>>> #include "sc7280.dtsi" > >>>>>> #include "pm7250b.dtsi" > >>>>>> @@ -38,6 +39,25 @@ > >>>>>> stdout-path = "serial0:115200n8"; > >>>>>> }; > >>>>>> > >>>>>> + lcd_disp_bias: lcd-disp-bias-regulator { > >>>>>> + compatible = "regulator-fixed"; > >>>>>> + regulator-name = "lcd_disp_bias"; > >>>>>> + regulator-min-microvolt = <5500000>; > >>>>>> + regulator-max-microvolt = <5500000>; > >>>>>> + gpio = <&pm7250b_gpios 2 GPIO_ACTIVE_HIGH>; > >>>>>> + enable-active-high; > >>>>>> + pinctrl-names = "default"; > >>>>>> + pinctrl-0 = <&lcd_disp_bias_en>; > >>>>> property-n > >>>>> property-names > >>>>> > >>>>> all throughout the patch > >>> > >>> Thanks, I will update in the new version. > >>> > >>>>>> +&gpu { > >>>>>> + status = "disabled"; > >>>>>> +}; > >>>>> Hm.. generally we disable the GPU in the SoC DT, but that doesn't > >>>>> seem to have happened here.. > >>>>> > >>>>> Thinking about it more, is disabling it here necessary? Does it > >>>>> not fail gracefully? > >>>> Missed this. > >>>> > >>>> I'd say, I don't see a reason to disable it at all. The GPU should be > >>>> working on sc7280 / qcm4290. > >>> > >>> With GPU device node enabled, adreno_bind failure is seen as the > >>> "speed_bin" was not populated on QCM6490 target which leads to display > >>> bind failure. > >> > >> Excuse me please. The GPU node for sc7280 already has speed_bin, which > >> points to qfprom + 0x1e9, bits 5 to 9. > >> > >> Do you mean that qcm6490 uses different speed bin location? Or > >> different values for the speed bins? > >> > >>> Spoke with GPU team and on QCM6490 board, only CPU rendering is > >>> supported for now and there is no plan to enable GPU rendering in near > >>> future. > >> > >> This sounds like having the feature disabled for no particular reason. > >> Both the kernel and Mesa have supported the Adreno 635 for quite a > >> while. > > > > 643 [1], [2] > > > >> > >>> In this regard, what do you suggest > >>> > >>> 1) Disable GPU in QCM6490 DT (as per the current patch) > >>> 2) Disable GPU in the SoC DT, but enable it in other platform DTs. > >>> (This > >>> will prompt change in all the dt's and we don't have all the devices to > >>> test) > >> > >> The second option definitely follows what is present on other platforms. > >> > >>> Please let me know your views on it. > >> > >> Please enable the GPU instead. > > > > +1 > > > > Konrad > > > > [1] > > https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25408/diffs?commit_id=b1e851d66c3a3e53f1a464023f675f3f6cbd3503 > > [2] > > https://patches.linaro.org/project/linux-arm-msm/cover/20230926-topic-a643-v1-0-7af6937ac0a3@linaro.org/ > > Thanks for the help. After applying missing patches from series > https://patches.linaro.org/project/linux-arm-msm/cover/20230926-topic-a643-v1-0-7af6937ac0a3@linaro.org/ > in my local build, GPU is working fine. GPU disablement change is not > needed. I will send new version of patch removing GPU part and > addressing other review comments. Thank you!
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts index 2a6e4907c5ee..efa5252130a1 100644 --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts @@ -9,6 +9,7 @@ #define PM7250B_SID 8 #define PM7250B_SID1 9 +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h> #include <dt-bindings/regulator/qcom,rpmh-regulator.h> #include "sc7280.dtsi" #include "pm7250b.dtsi" @@ -38,6 +39,25 @@ stdout-path = "serial0:115200n8"; }; + lcd_disp_bias: lcd-disp-bias-regulator { + compatible = "regulator-fixed"; + regulator-name = "lcd_disp_bias"; + regulator-min-microvolt = <5500000>; + regulator-max-microvolt = <5500000>; + gpio = <&pm7250b_gpios 2 GPIO_ACTIVE_HIGH>; + enable-active-high; + pinctrl-names = "default"; + pinctrl-0 = <&lcd_disp_bias_en>; + }; + + pm8350c_pwm_backlight: backlight { + compatible = "pwm-backlight"; + pwms = <&pm8350c_pwm 3 65535>; + enable-gpios = <&pm8350c_gpios 7 GPIO_ACTIVE_HIGH>; + pinctrl-names = "default"; + pinctrl-0 = <&pmic_lcd_bl_en>; + }; + reserved-memory { xbl_mem: xbl@80700000 { reg = <0x0 0x80700000 0x0 0x100000>; @@ -420,6 +440,86 @@ }; }; +&gpu { + status = "disabled"; +}; + +&mdss { + status = "okay"; +}; + +&mdss_dsi { + vdda-supply = <&vreg_l6b_1p2>; + status = "okay"; + + panel@0 { + compatible = "novatek,nt36672e"; + reg = <0>; + + reset-gpios = <&tlmm 44 GPIO_ACTIVE_HIGH>; + + vddi-supply = <&vreg_l8c_1p62>; + avdd-supply = <&lcd_disp_bias>; + avee-supply = <&lcd_disp_bias>; + + backlight = <&pm8350c_pwm_backlight>; + + port { + panel0_in: endpoint { + remote-endpoint = <&mdss_dsi0_out>; + }; + }; + }; +}; + +&mdss_dsi0_out { + remote-endpoint = <&panel0_in>; + data-lanes = <0 1 2 3>; +}; + +&mdss_dsi_phy { + vdds-supply = <&vreg_l10c_0p88>; + status = "okay"; +}; + +&pm7250b_gpios { + lcd_disp_bias_en: lcd-disp-bias-en-state { + pins = "gpio2"; + function = "func1"; + bias-disable; + qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>; + input-disable; + output-enable; + power-source = <0>; + }; +}; + +&pm8350c_gpios { + pmic_lcd_bl_en: pmic-lcd-bl-en-state { + pins = "gpio7"; + function = "normal"; + bias-disable; + qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>; + output-low; + power-source = <0>; + }; + + pmic_lcd_bl_pwm: pmic-lcd-bl-pwm-state { + pins = "gpio8"; + function = "func1"; + bias-disable; + qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>; + output-low; + power-source = <0>; + }; +}; + +&pm8350c_pwm { + pinctrl-names = "default"; + pinctrl-0 = <&pmic_lcd_bl_pwm>; + status = "okay"; +}; + &qupv3_id_0 { status = "okay"; };
Enable Display Subsystem with Novatek NT36672E Panel on qcm6490 idp platform. Signed-off-by: Ritesh Kumar <quic_riteshk@quicinc.com> --- arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 100 +++++++++++++++++++++++ 1 file changed, 100 insertions(+)