Messages in this thread | | | Date | Wed, 27 Apr 2022 14:51:14 -0700 | Subject | Re: [PATCH v2] arm64: dts: qcom: sc7280: eDP for herobrine boards | From | Abhinav Kumar <> |
| |
I have reviewed all the display bits in this and they are in-line with
https://patchwork.kernel.org/project/linux-arm-msm/patch/1647452154-16361-3-git-send-email-quic_sbillaka@quicinc.com/
which is a working reference for this.
Hence,
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
On 4/26/2022 12:41 PM, Douglas Anderson wrote: > Add eDP support to herobrine boards, splitting up amongst the > different files as makes sense. Rationale for the current split of > things: > * The eDP connector itself is on qcard. However, not all devices with > a qcard will use an eDP panel. Some might use MIPI and, presumably, > someone could build a device with qcard that had no display at all. > * The qcard provides a PWM for backlight that goes to the eDP > connector. This PWM is also provided to the board and it's expected > that it would be used as the backlight PWM even for herobrine > devices with MIPI displays. > * It's currently assumed that all herobrine boards will have some sort > of display, either MIPI or eDP (but not both). > * We will assume herobrine-rev1 has eDP. The schematics allow for a > MIPI panel to be hooked up but, aside from some testing, nobody is > doing this and most boards don't have all the parts stuffed for > it. The two panels would also share a PWM for backlight, which is > weird. > * herobrine-villager and herobrine-hoglin (crd) also have eDP. > * herobrine-hoglin (crd) has slightly different regulator setup for > the backlight. It's expected that this is unique to this board. See > comments in the dts file. > * There are some regulators that are defined in the qcard schematic > but provided by the board like "vreg_edp_bl" and > "vreg_edp_3p3". While we could put references to these regulators > straight in the qcard.dtsi file, this would force someone using > qcard that didn't provide those regulators to provide a dummy or do > an ugly /delete-node/. Instead, we'll add references in > herobrine.dtsi. > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > This patch most directly depends on ("arm64: dts: qcom: pm8350c: Add > pwm support") [1] and it won't even compile without that. To be > functional, of course, we also need the driver support for the PWM to > land. > > In order for this patch to function we also need Sankeerth's eDP > series [2] to land. > > To be schema happy, this patch also relies on my patch to add > regulators into the schema for the PHY an the eDP controller [3]. > Several other device tree files reference these supplies without being > in the schema, though, so we'll have to decide whether to block on > those bindings landing. It does sound as if the regulator names may > not be exactly correct right now, though. :( > > In theory, I could break this patch up into separate patches adding > the basic support to the qcard, then to herobrine.dtsi, and then one > patch each for herobrine-r1, villager, and crd. Doing so didn't seem > to make sense to me--I think it's easier to make sense of the change > as one patch. However, if someone feels strongly that it should be > broken up I'm happy to do so. > > I've managed to get the display on my herobrine-rev1 up and running on > today's linuxnext (next-20220422) with this series. For whatever > reason the eDP PHY wouldn't probe unless I hacked `fw_devlink=off` in > the config. I don't believe that problem is related to this patch, > though. > > [1] https://lore.kernel.org/r/1645509309-16142-4-git-send-email-quic_c_skakit@quicinc.com > [2] https://lore.kernel.org/r/1650887072-16652-1-git-send-email-quic_sbillaka@quicinc.com/ > [3] https://lore.kernel.org/r/20220425210643.2420919-1-dianders@chromium.org > > Changes in v2: > - Commit message and comment cleanups from Stephen. > > .../boot/dts/qcom/sc7280-herobrine-crd.dts | 40 ++++++++++++++ > .../qcom/sc7280-herobrine-herobrine-r1.dts | 8 +++ > .../dts/qcom/sc7280-herobrine-villager-r0.dts | 8 +++ > .../arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 24 +++++++++ > arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi | 54 +++++++++++++++++++ > 5 files changed, 134 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts > index b06f61e9b90b..a4ac33c4fd59 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts > @@ -12,6 +12,27 @@ > / { > model = "Qualcomm Technologies, Inc. sc7280 CRD platform (rev5+)"; > compatible = "google,hoglin", "qcom,sc7280"; > + > + /* FIXED REGULATORS */ > + > + /* > + * On most herobrine boards PPVAR_SYS directly provides VREG_EDP_BL. > + * However, on CRD there's an extra regulator in the way. Since this > + * is expected to be uncommon, we'll leave the "vreg_edp_bl" label > + * in the baseboard herobrine.dtsi point at "ppvar_sys" and then > + * make a "_crd" specific version here. > + */ > + vreg_edp_bl_crd: vreg-edp-bl-crd-regulator { > + compatible = "regulator-fixed"; > + regulator-name = "vreg_edp_bl_crd"; > + > + gpio = <&pm8350c_gpios 6 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + pinctrl-names = "default"; > + pinctrl-0 = <&edp_bl_reg_en>; > + > + vin-supply = <&ppvar_sys>; > + }; > }; > > /* ADDITIONS TO NODES DEFINED IN PARENT DEVICE TREE FILES */ > @@ -81,6 +102,14 @@ ap_ts: touchscreen@5c { > }; > }; > > +&mdss_edp { > + status = "okay"; > +}; > + > +&mdss_edp_phy { > + status = "okay"; > +}; > + > /* For nvme */ > &pcie1 { > status = "okay"; > @@ -91,6 +120,10 @@ &pcie1_phy { > status = "okay"; > }; > > +&pm8350c_pwm_backlight { > + power-supply = <&vreg_edp_bl_crd>; > +}; > + > /* For eMMC */ > &sdhc_1 { > status = "okay"; > @@ -121,6 +154,13 @@ &pm8350c_gpios { > "PMIC_EDP_BL_EN", > "PMIC_EDP_BL_PWM", > ""; > + > + edp_bl_reg_en: edp-bl-reg-en { > + pins = "gpio6"; > + function = "normal"; > + bias-disable; > + qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>; > + }; > }; > > &tlmm { > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts > index 29c4ca095294..b69ca09d9bfb 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts > @@ -100,6 +100,14 @@ ap_ts: touchscreen@5c { > }; > }; > > +&mdss_edp { > + status = "okay"; > +}; > + > +&mdss_edp_phy { > + status = "okay"; > +}; > + > /* For nvme */ > &pcie1 { > status = "okay"; > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager-r0.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager-r0.dts > index 6c2b9a14535a..d3d6ffad4eff 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager-r0.dts > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager-r0.dts > @@ -58,6 +58,14 @@ &ap_sar_sensor1 { > status = "okay"; > }; > > +&mdss_edp { > + status = "okay"; > +}; > + > +&mdss_edp_phy { > + status = "okay"; > +}; > + > /* For nvme */ > &pcie1 { > status = "okay"; > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi > index d58045dd7334..9cb1bc8ed6b5 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi > @@ -367,6 +367,11 @@ &vreg_l2c_1p8 { > > /* ADDITIONS TO NODES DEFINED IN PARENT DEVICE TREE FILES */ > > +&edp_panel { > + /* Our board provides power to the qcard for the eDP panel. */ > + power-supply = <&vreg_edp_3p3>; > +}; > + > ap_sar_sensor_i2c: &i2c1 { > clock-frequency = <400000>; > status = "disabled"; > @@ -420,6 +425,14 @@ tpm@50 { > }; > }; > > +&mdss { > + status = "okay"; > +}; > + > +&mdss_mdp { > + status = "okay"; > +}; > + > /* NVMe drive, enabled on a per-board basis */ > &pcie1 { > pinctrl-names = "default"; > @@ -429,6 +442,17 @@ &pcie1 { > vddpe-3v3-supply = <&pp3300_ssd>; > }; > > +&pm8350c_pwm { > + status = "okay"; > +}; > + > +&pm8350c_pwm_backlight { > + status = "okay"; > + > + /* Our board provides power to the qcard for the backlight */ > + power-supply = <&vreg_edp_bl>; > +}; > + > &pmk8350_rtc { > status = "disabled"; > }; > diff --git a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi > index b833ba1e8f4a..fbd9f9fa9d3a 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi > @@ -29,6 +29,16 @@ aliases { > serial0 = &uart5; > serial1 = &uart7; > }; > + > + pm8350c_pwm_backlight: backlight { > + compatible = "pwm-backlight"; > + status = "disabled"; > + > + enable-gpios = <&pm8350c_gpios 7 GPIO_ACTIVE_HIGH>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pmic_edp_bl_en>; > + pwms = <&pm8350c_pwm 3 65535>; > + }; > }; > > &apps_rsc { > @@ -293,11 +303,50 @@ &ipa { > modem-init; > }; > > +/* NOTE: Not all Qcards have eDP connector stuffed */ > +&mdss_edp { > + vdda-0p9-supply = <&vdd_a_edp_0_0p9>; > + vdda-1p2-supply = <&vdd_a_edp_0_1p2>; > + > + aux-bus { > + edp_panel: panel { > + compatible = "edp-panel"; > + > + backlight = <&pm8350c_pwm_backlight>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + port@0 { > + reg = <0>; > + edp_panel_in: endpoint { > + remote-endpoint = <&mdss_edp_out>; > + }; > + }; > + }; > + }; > + }; > +}; > + > +&mdss_edp_out { > + remote-endpoint = <&edp_panel_in>; > +}; > + > +&mdss_edp_phy { > + vdda-pll-supply = <&vdd_a_edp_0_0p9>; > + vdda-phy-supply = <&vdd_a_edp_0_1p2>; > +}; > + > &pcie1_phy { > vdda-phy-supply = <&vreg_l10c_0p88>; > vdda-pll-supply = <&vreg_l6b_1p2>; > }; > > +&pm8350c_pwm { > + pinctrl-names = "default"; > + pinctrl-0 = <&pmic_edp_bl_pwm>; > +}; > + > &pmk8350_vadc { > pmk8350-die-temp@3 { > reg = <PMK8350_ADC7_DIE_TEMP>; > @@ -383,6 +432,11 @@ &usb_2_hsphy { > * baseboard or board device tree, not here. > */ > > +/* No external pull for eDP HPD, so set the internal one. */ > +&edp_hot_plug_det { > + bias-pull-down; > +}; > + > /* > * For ts_i2c > *
| |