Add support for the eDP panel on sc7280 CRD platform. The eDP panel does
not need HPD line for connect disconnect. So, this series will report eDP
as always connected. The driver needs to register for IRQ_HPD only for eDP.
This support will be added later.
These changes are dependent on the following series:
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=586263&archive=both&state=*
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=560587&state=%2A&archive=both
Sankeerth Billakanti (5):
dt-bindings: display: simple: Add sharp LQ140M1JW46 panel
arm64: dts: qcom: sc7280: Add support for eDP panel on CRD
arm64: dts: qcom: sc7280: rename edp_out label to mdss_edp_out
drm/panel-edp: Add eDP sharp panel support
drm/msm/dp: Add driver support to utilize drm panel
.../bindings/display/panel/panel-simple.yaml | 2 +
arch/arm64/boot/dts/qcom/sc7280-crd.dts | 120 +++++++++++++++++++++
arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +-
drivers/gpu/drm/msm/dp/dp_display.c | 6 ++
drivers/gpu/drm/msm/dp/dp_drm.c | 62 +++++++++--
drivers/gpu/drm/msm/dp/dp_parser.h | 3 +
drivers/gpu/drm/panel/panel-edp.c | 44 ++++++++
7 files changed, 228 insertions(+), 11 deletions(-)
--
2.7.4
Add support for sharp LQ140M1JW46 display panel. It is a 14" eDP panel
with 1920x1080 display resolution.
Signed-off-by: Sankeerth Billakanti <[email protected]>
Acked-by: Rob Herring <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
---
Changes in v4:
None
Changes in v3:
None
Documentation/devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 9cf5588..1eb9dd4 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -284,6 +284,8 @@ properties:
- sharp,lq101k1ly04
# Sharp 12.3" (2400x1600 pixels) TFT LCD panel
- sharp,lq123p1jx31
+ # Sharp 14" (1920x1080 pixels) TFT LCD panel
+ - sharp,lq140m1jw46
# Sharp LS020B1DD01D 2.0" HQVGA TFT LCD panel
- sharp,ls020b1dd01d
# Shelly SCA07010-BFN-LNN 7.0" WVGA TFT LCD panel
--
2.7.4
Enable the eDP display panel support without HPD on sc7280 platform.
Signed-off-by: Sankeerth Billakanti <[email protected]>
---
Changes in v4:
- Create new patch for name changes
- Remove output-low
Changes in v3:
- Sort the nodes alphabetically
- Use - instead of _ as node names
- Place the backlight and panel nodes under root
- Change the name of edp_out to mdss_edp_out
- Change the names of regulator nodes
- Delete unused properties in the board file
Changes in v2:
- Sort node references alphabetically
- Improve readability
- Move the pwm pinctrl to pwm node
- Move the regulators to root
- Define backlight power
- Remove dummy regulator node
- Cleanup pinctrl definitions
arch/arm64/boot/dts/qcom/sc7280-crd.dts | 120 ++++++++++++++++++++++++++++++++
1 file changed, 120 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
index e2efbdd..6dba5ac 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
@@ -21,6 +21,59 @@
chosen {
stdout-path = "serial0:115200n8";
};
+
+ backlight_3v3_regulator: backlight-3v3-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "backlight_3v3_regulator";
+
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+
+ gpio = <&pm8350c_gpios 7 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&edp_bl_power>;
+ };
+
+ edp_3v3_regulator: edp-3v3-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "edp_3v3_regulator";
+
+ 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>;
+ };
+
+ edp_backlight: edp-backlight {
+ compatible = "pwm-backlight";
+
+ power-supply = <&backlight_3v3_regulator>;
+ pwms = <&pm8350c_pwm 3 65535>;
+ };
+
+ edp_panel: edp-panel {
+ compatible = "sharp,lq140m1jw46";
+
+ power-supply = <&edp_3v3_regulator>;
+ backlight = <&edp_backlight>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ edp_panel_in: endpoint {
+ remote-endpoint = <&edp_out>;
+ };
+ };
+ };
+ };
};
&apps_rsc {
@@ -76,6 +129,44 @@ ap_ts_pen_1v8: &i2c13 {
};
};
+&edp_out {
+ remote-endpoint = <&edp_panel_in>;
+};
+
+&mdss {
+ status = "okay";
+};
+
+&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>;
+};
+
+&mdss_edp {
+ status = "okay";
+
+ vdda-1p2-supply = <&vreg_l6b_1p2>;
+ vdda-0p9-supply = <&vreg_l10c_0p8>;
+ /delete-property/ pinctrl-names;
+ /delete-property/ pinctrl-0;
+};
+
+&mdss_edp_phy {
+ status = "okay";
+
+ vdda-1p2-supply = <&vreg_l6b_1p2>;
+ vdda-0p9-supply = <&vreg_l10c_0p8>;
+};
+
+&mdss_mdp {
+ status = "okay";
+};
+
&nvme_3v3_regulator {
gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>;
};
@@ -84,7 +175,36 @@ ap_ts_pen_1v8: &i2c13 {
pins = "gpio51";
};
+&pm8350c_gpios {
+ edp_bl_power: edp-bl-power {
+ pins = "gpio7";
+ function = "normal";
+ qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
+ bias-pull-down;
+ };
+
+ edp_bl_pwm: edp-bl-pwm {
+ pins = "gpio8";
+ function = "func1";
+ qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
+ bias-pull-down;
+ };
+};
+
+&pm8350c_pwm {
+ status = "okay";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&edp_bl_pwm>;
+};
+
&tlmm {
+ edp_panel_power: edp-panel-power {
+ pins = "gpio80";
+ function = "gpio";
+ bias-pull-down;
+ };
+
tp_int_odl: tp-int-odl {
pins = "gpio7";
function = "gpio";
--
2.7.4
On Thu 10 Feb 05:57 CST 2022, Sankeerth Billakanti wrote:
> Enable the eDP display panel support without HPD on sc7280 platform.
>
> Signed-off-by: Sankeerth Billakanti <[email protected]>
> ---
>
> Changes in v4:
> - Create new patch for name changes
> - Remove output-low
>
> Changes in v3:
> - Sort the nodes alphabetically
> - Use - instead of _ as node names
> - Place the backlight and panel nodes under root
> - Change the name of edp_out to mdss_edp_out
> - Change the names of regulator nodes
> - Delete unused properties in the board file
>
>
> Changes in v2:
> - Sort node references alphabetically
> - Improve readability
> - Move the pwm pinctrl to pwm node
> - Move the regulators to root
> - Define backlight power
> - Remove dummy regulator node
> - Cleanup pinctrl definitions
>
> arch/arm64/boot/dts/qcom/sc7280-crd.dts | 120 ++++++++++++++++++++++++++++++++
> 1 file changed, 120 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> index e2efbdd..6dba5ac 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> @@ -21,6 +21,59 @@
> chosen {
> stdout-path = "serial0:115200n8";
> };
> +
> + backlight_3v3_regulator: backlight-3v3-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "backlight_3v3_regulator";
> +
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> +
> + gpio = <&pm8350c_gpios 7 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&edp_bl_power>;
> + };
> +
> + edp_3v3_regulator: edp-3v3-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "edp_3v3_regulator";
> +
> + 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>;
> + };
> +
> + edp_backlight: edp-backlight {
> + compatible = "pwm-backlight";
> +
> + power-supply = <&backlight_3v3_regulator>;
> + pwms = <&pm8350c_pwm 3 65535>;
> + };
> +
> + edp_panel: edp-panel {
> + compatible = "sharp,lq140m1jw46";
> +
> + power-supply = <&edp_3v3_regulator>;
> + backlight = <&edp_backlight>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + edp_panel_in: endpoint {
> + remote-endpoint = <&edp_out>;
> + };
> + };
> + };
> + };
> };
>
> &apps_rsc {
> @@ -76,6 +129,44 @@ ap_ts_pen_1v8: &i2c13 {
> };
> };
>
> +&edp_out {
> + remote-endpoint = <&edp_panel_in>;
> +};
> +
> +&mdss {
> + status = "okay";
> +};
> +
> +&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>;
> +};
> +
> +&mdss_edp {
> + status = "okay";
> +
> + vdda-1p2-supply = <&vreg_l6b_1p2>;
> + vdda-0p9-supply = <&vreg_l10c_0p8>;
> + /delete-property/ pinctrl-names;
> + /delete-property/ pinctrl-0;
If the first device to enable &mdss_edp overwrites pinctrl-{names,0} in
&mdss_dp and removes the properties in &mdss_edp, I think that's a sign
that they should not be in the .dtsi in the first place.
> +};
> +
> +&mdss_edp_phy {
> + status = "okay";
> +
> + vdda-1p2-supply = <&vreg_l6b_1p2>;
> + vdda-0p9-supply = <&vreg_l10c_0p8>;
> +};
> +
> +&mdss_mdp {
> + status = "okay";
> +};
> +
> &nvme_3v3_regulator {
> gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>;
> };
> @@ -84,7 +175,36 @@ ap_ts_pen_1v8: &i2c13 {
> pins = "gpio51";
> };
>
> +&pm8350c_gpios {
> + edp_bl_power: edp-bl-power {
> + pins = "gpio7";
> + function = "normal";
> + qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
> + bias-pull-down;
Why do you pull down these two pins? They are both outputs.
> + };
> +
> + edp_bl_pwm: edp-bl-pwm {
> + pins = "gpio8";
> + function = "func1";
> + qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
> + bias-pull-down;
> + };
> +};
> +
> +&pm8350c_pwm {
As stated previously, this will prevent me from merging this patch until
the LPG/PWM support has been accepted.
As such I would recommend that you drop the backlight parts of this
patch until that has landed - so we can merge the rest of this in the
meantime.
> + status = "okay";
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&edp_bl_pwm>;
> +};
> +
> &tlmm {
> + edp_panel_power: edp-panel-power {
> + pins = "gpio80";
> + function = "gpio";
> + bias-pull-down;
Same here, why is this pulled down?
Thanks,
Bjorn
> + };
> +
> tp_int_odl: tp-int-odl {
> pins = "gpio7";
> function = "gpio";
> --
> 2.7.4
>
Hi,
On Thu, Feb 10, 2022 at 3:58 AM Sankeerth Billakanti
<[email protected]> wrote:
>
> Add support for sharp LQ140M1JW46 display panel. It is a 14" eDP panel
> with 1920x1080 display resolution.
>
> Signed-off-by: Sankeerth Billakanti <[email protected]>
> Acked-by: Rob Herring <[email protected]>
> Reviewed-by: Stephen Boyd <[email protected]>
> ---
>
> Changes in v4:
> None
>
> Changes in v3:
> None
>
> Documentation/devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
> 1 file changed, 2 insertions(+)
Reviewed-by: Douglas Anderson <[email protected]>
Hi,
On Wed, Feb 16, 2022 at 11:26 AM Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Thu, Feb 10, 2022 at 3:58 AM Sankeerth Billakanti
> <[email protected]> wrote:
> >
> > Add support for sharp LQ140M1JW46 display panel. It is a 14" eDP panel
> > with 1920x1080 display resolution.
> >
> > Signed-off-by: Sankeerth Billakanti <[email protected]>
> > Acked-by: Rob Herring <[email protected]>
> > Reviewed-by: Stephen Boyd <[email protected]>
> > ---
> >
> > Changes in v4:
> > None
> >
> > Changes in v3:
> > None
> >
> > Documentation/devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
> > 1 file changed, 2 insertions(+)
>
> Reviewed-by: Douglas Anderson <[email protected]>
...and pushed to drm-misc-next:
122365cfe9de dt-bindings: display: simple: Add sharp LQ140M1JW46 panel
So v5 shouldn't include this patch.
-Doug
Hi,
On Thu, Feb 10, 2022 at 3:58 AM Sankeerth Billakanti
<[email protected]> wrote:
>
> +&pm8350c_gpios {
> + edp_bl_power: edp-bl-power {
> + pins = "gpio7";
> + function = "normal";
> + qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
As far as I can tell you're lacking:
#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
...which is needed to use PMIC_GPIO_STRENGTH_LOW. I'm curious how
you're compiling?
Hi,
On Thu, Feb 10, 2022 at 3:58 AM Sankeerth Billakanti
<[email protected]> wrote:
>
> + backlight_3v3_regulator: backlight-3v3-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "backlight_3v3_regulator";
> +
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> +
> + gpio = <&pm8350c_gpios 7 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&edp_bl_power>;
> + };
So I'm pretty sure that this is wrong and what you had on a previous
patch was more correct. Specifically the PMIC's GPIO 7 truly _is_ an
enable pin for the backlight. In the schematics I see it's named as
"PMIC_EDP_BL_EN" and is essentially the same net as "EDP_BL_EN". This
is distinct from the backlight _regulator_ that is named VREG_EDP_BP.
I believe the VREG_EDP_BP is essentially sourced directly from
PPVAR_SYS. That's how it works on herobrine and I believe that CRD is
the same. You currently don't model ppvar_sys, but it's basically just
a variable-voltage rail that could be provided somewhat directly from
the battery or could be provided from Type C components. I believe
that the panel backlight is designed to handle this fairly wide
voltage range and it's done this way to get the best efficiency.
So personally I'd prefer if you do something like herobrine and model
PPVAR_SYS. Then the backlight can use ppvar_sys as its regulator and
you can go back to providing this as an "enable" pin for the
backlight.
I know, technically it doesn't _really_ matter, but it's nice to model
it more correctly.
On Thu 17 Feb 17:03 PST 2022, Doug Anderson wrote:
> Hi,
>
> On Thu, Feb 10, 2022 at 3:58 AM Sankeerth Billakanti
> <[email protected]> wrote:
> >
> > + backlight_3v3_regulator: backlight-3v3-regulator {
> > + compatible = "regulator-fixed";
> > + regulator-name = "backlight_3v3_regulator";
> > +
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > +
> > + gpio = <&pm8350c_gpios 7 GPIO_ACTIVE_HIGH>;
> > + enable-active-high;
> > +
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&edp_bl_power>;
> > + };
>
> So I'm pretty sure that this is wrong and what you had on a previous
> patch was more correct. Specifically the PMIC's GPIO 7 truly _is_ an
> enable pin for the backlight. In the schematics I see it's named as
> "PMIC_EDP_BL_EN" and is essentially the same net as "EDP_BL_EN". This
> is distinct from the backlight _regulator_ that is named VREG_EDP_BP.
> I believe the VREG_EDP_BP is essentially sourced directly from
> PPVAR_SYS. That's how it works on herobrine and I believe that CRD is
> the same. You currently don't model ppvar_sys, but it's basically just
> a variable-voltage rail that could be provided somewhat directly from
> the battery or could be provided from Type C components. I believe
> that the panel backlight is designed to handle this fairly wide
> voltage range and it's done this way to get the best efficiency.
>
> So personally I'd prefer if you do something like herobrine and model
> PPVAR_SYS. Then the backlight can use ppvar_sys as its regulator and
> you can go back to providing this as an "enable" pin for the
> backlight.
>
> I know, technically it doesn't _really_ matter, but it's nice to model
> it more correctly.
While I've not seen your schematics, the proposal does look similar to
what I have on sc8180x, where there's a power rail, the BL_EN and a pwm
signal.
If that's the case I think representing BL_EN using the enable-gpios
property directly in the pwm-backlight node seems more appropriate (with
power-supply being the actual thing that powers the backlight).
If however gpio 7 is wired to something like the enable-pin on an actual
LDO the proposal here seems reasonable, but it seems unlikely that the
output of that would be named "backlight_3v3_regulator"?
Regards,
Bjorn
Hi,
On Thu, Feb 10, 2022 at 4:04 PM Bjorn Andersson
<[email protected]> wrote:
>
> > +&mdss_edp {
> > + status = "okay";
> > +
> > + vdda-1p2-supply = <&vreg_l6b_1p2>;
> > + vdda-0p9-supply = <&vreg_l10c_0p8>;
> > + /delete-property/ pinctrl-names;
> > + /delete-property/ pinctrl-0;
>
> If the first device to enable &mdss_edp overwrites pinctrl-{names,0} in
> &mdss_dp and removes the properties in &mdss_edp, I think that's a sign
> that they should not be in the .dtsi in the first place.
Actually, I just looked more carefully here. I think the
"/delete-property" for edp_hpd here is just wrong. I'm pretty sure
that the HPD signal is hooked up on CRD and we actually need it. If
somehow deleting the property helps you then it's probably just
hacking around a bug and relying on the panel to be always powered on,
or something.
I think this gets into some of the stuff in your final patch in this
series. I found that, on my hardware, the panel doesn't come up at all
with that final patch. When I go back to how things were working in an
earlier version of your series, though, I can get things working a
little better (though still not perfect).
-Doug