Add support for the eDP panel on sc7280 CRD platform.
These changes are dependent on the following series: https://patchwork.kernel.org/project/linux-arm-msm/list/?series=586263&a... https://patchwork.kernel.org/project/linux-arm-msm/list/?series=560587&s...
Sankeerth Billakanti (2): ARM64: dts: qcom: enable eDP panel support for sc7280 drm/panel-edp: Add sharp panel support for sc7280
arch/arm64/boot/dts/qcom/sc7280-crd.dts | 127 ++++++++++++++++++++++++++++++++ drivers/gpu/drm/panel/panel-edp.c | 11 +++ 2 files changed, 138 insertions(+)
Enable the eDP display panel support with backlight on sc7280 platform.
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com --- arch/arm64/boot/dts/qcom/sc7280-crd.dts | 127 ++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts index cd2755c..fde6f75 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts @@ -64,6 +64,47 @@ ap_ts_pen_1v8: &i2c13 { }; };
+&mdss { + status = "okay"; +}; + +&mdss_mdp { + status = "okay"; +}; + +&mdss_edp { + status = "okay"; + + vdda-1p2-supply = <&vreg_l6b_1p2>; + vdda-0p9-supply = <&vreg_l10c_0p8>; + + ports { + port@1 { + reg = <1>; + edp_out: endpoint { + remote-endpoint = <&edp_panel_in>; + }; + }; + }; +}; + +&mdss_edp_phy { + status = "okay"; + + vdda-1p2-supply = <&vreg_l6b_1p2>; + vdda-0p9-supply = <&vreg_l10c_0p8>; +}; + +&mdss_dp { + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&dp_hot_plug_det>; + data-lanes = <0 1>; + vdda-1p2-supply = <&vreg_l6b_1p2>; + vdda-0p9-supply = <&vreg_l1b_0p8>; +}; + &nvme_3v3_regulator { gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>; }; @@ -72,7 +113,93 @@ ap_ts_pen_1v8: &i2c13 { pins = "gpio51"; };
+&pm8350c_pwm{ + status = "okay"; +}; + +&pm8350c_gpios { + backlight_pwm_default: backlight_pwm_default { + pinconf { + pins = "gpio8"; + function = "func1"; + output-low; + bias-disable; + power-source = <0>; + qcom,drive-strength = <3>; + }; + }; +}; + +&soc { + backlight_power: backlight_power { + compatible = "regulator-fixed"; + regulator-name = "backlight_power"; + regulator-always-on; + regulator-boot-on; + }; + + edp_power: edp_power { + compatible = "regulator-fixed"; + regulator-name = "edp_power"; + + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + + gpio = <&tlmm 80 GPIO_ACTIVE_HIGH>; + enable-active-high; + + pinctrl-names = "default"; + pinctrl-0 = <&edp_panel_power_default>; + }; + + edp_backlight: edp_backlight { + compatible = "pwm-backlight"; + + pwms = <&pm8350c_pwm 3 65535>; + power-supply = <&backlight_power>; + enable-gpio = <&pm8350c_gpios 7 GPIO_ACTIVE_HIGH>; + + pinctrl-names = "default"; + pinctrl-0 = <&backlight_pwm_default>; + }; + + edp_panel: edp_panel { + compatible = "sharp_lq140m1jw46"; + + pinctrl-names = "default"; + pinctrl-0 = <&edp_hot_plug_det>; + + power-supply = <&edp_power>; + backlight = <&edp_backlight>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + reg = <0>; + edp_panel_in: endpoint { + remote-endpoint = <&edp_out>; + }; + }; + }; + }; +}; + &tlmm { + edp_hot_plug_det: edp-hot-plug-det { + pins = "gpio60"; + function = "edp_hot"; + bias-pull-down; + input-enable; + }; + + edp_panel_power_default: edp_panel_power_default { + pins = "gpio80"; + function = "gpio"; + drive-strength = <2>; + output-high; + }; + tp_int_odl: tp-int-odl { pins = "gpio7"; function = "gpio";
Hi,
On Mon, Jan 24, 2022 at 10:15 AM Sankeerth Billakanti quic_sbillaka@quicinc.com wrote:
Enable the eDP display panel support with backlight on sc7280 platform.
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com
arch/arm64/boot/dts/qcom/sc7280-crd.dts | 127 ++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts index cd2755c..fde6f75 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts @@ -64,6 +64,47 @@ ap_ts_pen_1v8: &i2c13 { }; };
+&mdss {
status = "okay";+};
+&mdss_mdp {
status = "okay";+};
"mdss_mdp" sorts after "mdss_edp",
+&mdss_edp {
status = "okay";vdda-1p2-supply = <&vreg_l6b_1p2>;vdda-0p9-supply = <&vreg_l10c_0p8>;ports {port@1 {reg = <1>;edp_out: endpoint {remote-endpoint = <&edp_panel_in>;};};};
I think part of the above should be in sc7280.dtsi. Basically in sc7820.dtsi I think you should have:
ports { #address-cells = <1>; #size-cells = <0>; port@0 { reg = <0>; edp_in: endpoint { remote-endpoint = <&dpu_intf5_out>; }; }; port@1 { reg = <1>; edp_out: endpoint { }; }; };
...and then the crd dts file just needs:
&edp_out { remote-endpoint = <&edp_panel_in>; };
Right?
+};
+&mdss_edp_phy {
status = "okay";vdda-1p2-supply = <&vreg_l6b_1p2>;vdda-0p9-supply = <&vreg_l10c_0p8>;+};
+&mdss_dp {
status = "okay";pinctrl-names = "default";pinctrl-0 = <&dp_hot_plug_det>;data-lanes = <0 1>;vdda-1p2-supply = <&vreg_l6b_1p2>;vdda-0p9-supply = <&vreg_l1b_0p8>;+};
&nvme_3v3_regulator { gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>; }; @@ -72,7 +113,93 @@ ap_ts_pen_1v8: &i2c13 { pins = "gpio51"; };
+&pm8350c_pwm{
nit: space after "pwm" and before "{"
status = "okay";
Don't you need:
pinctrl-names = "default"; pinctrl-0 = <&backlight_pwm_default>;
+};
+&pm8350c_gpios {
backlight_pwm_default: backlight_pwm_default {
nit: node name should have dashes, not underscores. Also, don't include the word "default".
pinconf {pins = "gpio8";function = "func1";output-low;bias-disable;power-source = <0>;qcom,drive-strength = <3>;
Instead of 3, should be PMIC_GPIO_STRENGTH_LOW
};
Don't need the "pinconf" subnode.
};+};
+&soc {
Don't need to put the regulators under &soc. They can be top level, right?
backlight_power: backlight_power {
nit: node names should have "-", not "_"
compatible = "regulator-fixed";regulator-name = "backlight_power";regulator-always-on;regulator-boot-on;};edp_power: edp_power {
nit: node names should have "-", not "_"
compatible = "regulator-fixed";regulator-name = "edp_power";regulator-min-microvolt = <3300000>;regulator-max-microvolt = <3300000>;gpio = <&tlmm 80 GPIO_ACTIVE_HIGH>;enable-active-high;pinctrl-names = "default";pinctrl-0 = <&edp_panel_power_default>;};edp_backlight: edp_backlight {
nit: node names should have a "-", not an "_".
compatible = "pwm-backlight";pwms = <&pm8350c_pwm 3 65535>;power-supply = <&backlight_power>;enable-gpio = <&pm8350c_gpios 7 GPIO_ACTIVE_HIGH>;
So wait. Why do you have a "bogus" backlight_power regulator and then a separate enable-gpio? Why
pinctrl-names = "default";pinctrl-0 = <&backlight_pwm_default>;
This pinctrl belongs as part of the PWM, not as part of the backlight.
...but where's the pinctrl for pm8350c_gpios #7? That _should_ be here since that's the backlight enable.
};edp_panel: edp_panel {compatible = "sharp_lq140m1jw46";
Device tree compatible strings separate the manufacturer from the model with ",", not "_".
pinctrl-names = "default";pinctrl-0 = <&edp_hot_plug_det>;
I think for eDP we probably want the pinctrl to be in the sc7280.dtsi file. It seems highly likely that people will hook up the eDP HPD pin to the standard place. If they don't then boards can override it.
power-supply = <&edp_power>;backlight = <&edp_backlight>;ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;edp_panel_in: endpoint {remote-endpoint = <&edp_out>;};};
Like for DP, much of this "ports" stuff should be in sc7280.dtsi.
};};+};
&tlmm {
edp_hot_plug_det: edp-hot-plug-det {pins = "gpio60";function = "edp_hot";
The "pins" and "function" belong in sc7280.dtsi.
bias-pull-down;
This part belongs in the CRD file.
input-enable;
You don't need "input-enable".
};
edp_panel_power_default: edp_panel_power_default {
Nit: node name shouldn't have underscores. ...and remove the word "default".
Hi,
On Mon, Jan 24, 2022 at 11:04 AM Doug Anderson dianders@chromium.org wrote:
+&mdss_edp {
status = "okay";vdda-1p2-supply = <&vreg_l6b_1p2>;vdda-0p9-supply = <&vreg_l10c_0p8>;ports {port@1 {reg = <1>;edp_out: endpoint {remote-endpoint = <&edp_panel_in>;};};};I think part of the above should be in sc7280.dtsi. Basically in sc7820.dtsi I think you should have:
ports { #address-cells = <1>; #size-cells = <0>; port@0 { reg = <0>; edp_in: endpoint { remote-endpoint = <&dpu_intf5_out>; }; }; port@1 { reg = <1>; edp_out: endpoint { }; }; };
...and then the crd dts file just needs:
&edp_out { remote-endpoint = <&edp_panel_in>; };
Right?
I've attempted to do the sc7280 part of this in:
https://lore.kernel.org/r/20220202132301.v3.7.Ic84bb69c45be2fccf50e3bd17b845...
Assuming folks think that's good then you should probably base your next version atop that.
-Doug
Add eDP panel support for sc7280 CRD platform.
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com --- drivers/gpu/drm/panel/panel-edp.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c index 176ef0c..bb2e346 100644 --- a/drivers/gpu/drm/panel/panel-edp.c +++ b/drivers/gpu/drm/panel/panel-edp.c @@ -1605,6 +1605,14 @@ static const struct panel_desc sharp_lq123p1jx31 = { }, };
+static const struct panel_desc sharp_lq140m1jw46 = { + .bpc = 8, + .size = { + .width = 309, + .height = 173, + }, +}; + static const struct drm_display_mode starry_kr122ea0sra_mode = { .clock = 147000, .hdisplay = 1920, @@ -1719,6 +1727,9 @@ static const struct of_device_id platform_of_match[] = { .compatible = "sharp,lq123p1jx31", .data = &sharp_lq123p1jx31, }, { + .compatible = "sharp_lq140m1jw46", + .data = &sharp_lq140m1jw46, + }, { .compatible = "starry,kr122ea0sra", .data = &starry_kr122ea0sra, }, {
Hi,
On Mon, Jan 24, 2022 at 10:16 AM Sankeerth Billakanti quic_sbillaka@quicinc.com wrote:
Add eDP panel support for sc7280 CRD platform.
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com
drivers/gpu/drm/panel/panel-edp.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c index 176ef0c..bb2e346 100644 --- a/drivers/gpu/drm/panel/panel-edp.c +++ b/drivers/gpu/drm/panel/panel-edp.c @@ -1605,6 +1605,14 @@ static const struct panel_desc sharp_lq123p1jx31 = { }, };
+static const struct panel_desc sharp_lq140m1jw46 = {
.bpc = 8,.size = {.width = 309,.height = 173,},
Where are your delays? I very much doubt that they are all 0.
I guess you're also not putting timings in here and you're relying on these coming from the EDID? My own preference would be:
1. If you are relying on a reliable way to read the EDID of the panel then you shouldn't even need to add anything to this section of the file. You should use the "edp-panel" compatible string and then add an entry to the "edp_panels" structure.
2. If you want to support folks that don't have a reliable way to read the EDID then you can add things here, but you should add a mode.
+};
static const struct drm_display_mode starry_kr122ea0sra_mode = { .clock = 147000, .hdisplay = 1920, @@ -1719,6 +1727,9 @@ static const struct of_device_id platform_of_match[] = { .compatible = "sharp,lq123p1jx31", .data = &sharp_lq123p1jx31, }, {
.compatible = "sharp_lq140m1jw46",.data = &sharp_lq140m1jw46,
Two problems:
1. You should use a ",", not a "_" to separate the vendor from the model.
2. You need to post device tree bindings for this.
NOTE: if instead your eDP controller supports DP AUX bus then you don't need to add to this table at all and you don't need to add bindings. Instead, you'd add your EDID panel ID to the "edp_panels" structure.
dri-devel@lists.freedesktop.org
-
Doug Anderson -
Sankeerth Billakanti