lkml.org 
[lkml]   [2022]   [Apr]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2] arm64: dts: qcom: sc7280: eDP for herobrine boards
From
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
> *

\
 
 \ /
  Last update: 2022-04-27 23:51    [W:0.072 / U:0.180 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site