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&a... https://patchwork.kernel.org/project/linux-arm-msm/list/?series=560587&s...
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(-)
Add support for sharp LQ140M1JW46 display panel. It is a 14" eDP panel with 1920x1080 display resolution.
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com Acked-by: Rob Herring robh@kernel.org Reviewed-by: Stephen Boyd swboyd@chromium.org ---
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
Hi,
On Thu, Feb 10, 2022 at 3:58 AM Sankeerth Billakanti quic_sbillaka@quicinc.com wrote:
Add support for sharp LQ140M1JW46 display panel. It is a 14" eDP panel with 1920x1080 display resolution.
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com Acked-by: Rob Herring robh@kernel.org Reviewed-by: Stephen Boyd swboyd@chromium.org
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 dianders@chromium.org
Hi,
On Wed, Feb 16, 2022 at 11:26 AM Doug Anderson dianders@chromium.org wrote:
Hi,
On Thu, Feb 10, 2022 at 3:58 AM Sankeerth Billakanti quic_sbillaka@quicinc.com wrote:
Add support for sharp LQ140M1JW46 display panel. It is a 14" eDP panel with 1920x1080 display resolution.
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com Acked-by: Rob Herring robh@kernel.org Reviewed-by: Stephen Boyd swboyd@chromium.org
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 dianders@chromium.org
...and pushed to drm-misc-next:
122365cfe9de dt-bindings: display: simple: Add sharp LQ140M1JW46 panel
So v5 shouldn't include this patch.
-Doug
Enable the eDP display panel support without HPD on sc7280 platform.
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com ---
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";
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 quic_sbillaka@quicinc.com
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 4:04 PM Bjorn Andersson bjorn.andersson@linaro.org 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
Hi,
On Thu, Feb 10, 2022 at 3:58 AM Sankeerth Billakanti quic_sbillaka@quicinc.com 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 quic_sbillaka@quicinc.com 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 quic_sbillaka@quicinc.com 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
Rename the edp_out label in the sc7280 platform to mdss_edp_out.
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com --- arch/arm64/boot/dts/qcom/sc7280-crd.dts | 10 +++++----- arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts index 6dba5ac..af40e14 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts @@ -69,7 +69,7 @@ port@0 { reg = <0>; edp_panel_in: endpoint { - remote-endpoint = <&edp_out>; + remote-endpoint = <&mdss_edp_out>; }; }; }; @@ -129,10 +129,6 @@ ap_ts_pen_1v8: &i2c13 { }; };
-&edp_out { - remote-endpoint = <&edp_panel_in>; -}; - &mdss { status = "okay"; }; @@ -156,6 +152,10 @@ ap_ts_pen_1v8: &i2c13 { /delete-property/ pinctrl-0; };
+&mdss_edp_out { + remote-endpoint = <&edp_panel_in>; +}; + &mdss_edp_phy { status = "okay";
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index 3572399..eca403a 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -3066,7 +3066,7 @@
port@1 { reg = <1>; - edp_out: endpoint { }; + mdss_edp_out: endpoint { }; }; };
On Thu 10 Feb 05:57 CST 2022, Sankeerth Billakanti wrote:
Rename the edp_out label in the sc7280 platform to mdss_edp_out.
Next week, or in the next product, it might not be obvious why we did this change. So please continue this sentence with something like "so that the nodes are grouped together when sorted in the dts".
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com
arch/arm64/boot/dts/qcom/sc7280-crd.dts | 10 +++++----- arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts index 6dba5ac..af40e14 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts @@ -69,7 +69,7 @@ port@0 { reg = <0>; edp_panel_in: endpoint {
remote-endpoint = <&edp_out>;
remote-endpoint = <&mdss_edp_out>; }; };@@ -129,10 +129,6 @@ ap_ts_pen_1v8: &i2c13 { }; };
-&edp_out {
You just added this node in patch 2 and now you change it immediately. If you reorder the two patches the history will be cleaner.
Thanks, Bjorn
- remote-endpoint = <&edp_panel_in>;
-};
&mdss { status = "okay"; }; @@ -156,6 +152,10 @@ ap_ts_pen_1v8: &i2c13 { /delete-property/ pinctrl-0; };
+&mdss_edp_out {
- remote-endpoint = <&edp_panel_in>;
+};
&mdss_edp_phy { status = "okay";
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index 3572399..eca403a 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -3066,7 +3066,7 @@
port@1 { reg = <1>;
edp_out: endpoint { };
mdss_edp_out: endpoint { }; }; };-- 2.7.4
Add support for the 14" sharp,lq140m1jw46 eDP panel.
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com --- 00 ff ff ff ff ff ff 00 4d 10 23 15 00 00 00 00 35 1e 01 04 a5 1f 11 78 07 de 50 a3 54 4c 99 26 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 5a 87 80 a0 70 38 4d 40 30 20 35 00 35 ae 10 00 00 18 65 38 80 a0 70 38 4d 40 30 20 35 00 35 ae 10 00 00 18 00 00 00 fd 00 30 90 a7 a7 23 01 00 00 00 00 00 00 00 00 00 00 fc 00 4c 51 31 34 30 4d 31 4a 57 34 39 0a 20 00 77
----------------
Block 0, Base EDID: EDID Structure Version & Revision: 1.4 Vendor & Product Identification: Manufacturer: SHP Model: 5411 Made in: week 53 of 2020 Basic Display Parameters & Features: Digital display Bits per primary color channel: 8 DisplayPort interface Maximum image size: 31 cm x 17 cm Gamma: 2.20 Supported color formats: RGB 4:4:4 Default (sRGB) color space is primary color space First detailed timing includes the native pixel format and preferred refresh rate Display is continuous frequency Color Characteristics: Red : 0.6396, 0.3291 Green: 0.2998, 0.5996 Blue : 0.1494, 0.0595 White: 0.3125, 0.3281 Established Timings I & II: none Standard Timings: none Detailed Timing Descriptors: DTD 1: 1920x1080 143.981 Hz 16:9 166.587 kHz 346.500 MHz (309 mm x 174 mm) Hfront 48 Hsync 32 Hback 80 Hpol N Vfront 3 Vsync 5 Vback 69 Vpol N DTD 2: 1920x1080 59.990 Hz 16:9 69.409 kHz 144.370 MHz (309 mm x 174 mm) Hfront 48 Hsync 32 Hback 80 Hpol N Vfront 3 Vsync 5 Vback 69 Vpol N Display Range Limits: Monitor ranges (Bare Limits): 48-144 Hz V, 167-167 kHz H, max dotclock 350 MHz Display Product Name: 'LQ140M1JW49' Checksum: 0x77
Changes in v4: -Add all modes from EDID -Provide EDID blob
Changes in v3: None
drivers/gpu/drm/panel/panel-edp.c | 44 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c index a394a15..f9355b2 100644 --- a/drivers/gpu/drm/panel/panel-edp.c +++ b/drivers/gpu/drm/panel/panel-edp.c @@ -1605,6 +1605,47 @@ static const struct panel_desc sharp_lq123p1jx31 = { }, };
+static const struct drm_display_mode sharp_lq140m1jw46_mode[] = { + { + .clock = 346500, + .hdisplay = 1920, + .hsync_start = 1920 + 48, + .hsync_end = 1920 + 48 + 32, + .htotal = 1920 + 48 + 32 + 80, + .vdisplay = 1080, + .vsync_start = 1080 + 3, + .vsync_end = 1080 + 3 + 5, + .vtotal = 1080 + 3 + 5 + 69, + .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC, + }, { + .clock = 144370, + .hdisplay = 1920, + .hsync_start = 1920 + 48, + .hsync_end = 1920 + 48 + 32, + .htotal = 1920 + 48 + 32 + 80, + .vdisplay = 1080, + .vsync_start = 1080 + 3, + .vsync_end = 1080 + 3 + 5, + .vtotal = 1080 + 3 + 5 + 69, + .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC, + }, +}; + +static const struct panel_desc sharp_lq140m1jw46 = { + .modes = sharp_lq140m1jw46_mode, + .num_modes = ARRAY_SIZE(sharp_lq140m1jw46_mode), + .bpc = 8, + .size = { + .width = 309, + .height = 174, + }, + .delay = { + .hpd_absent = 80, + .enable = 50, + .unprepare = 500, + }, +}; + static const struct drm_display_mode starry_kr122ea0sra_mode = { .clock = 147000, .hdisplay = 1920, @@ -1719,6 +1760,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 Thu, Feb 10, 2022 at 3:58 AM Sankeerth Billakanti quic_sbillaka@quicinc.com wrote:
Add support for the 14" sharp,lq140m1jw46 eDP panel.
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com
00 ff ff ff ff ff ff 00 4d 10 23 15 00 00 00 00 35 1e 01 04 a5 1f 11 78 07 de 50 a3 54 4c 99 26 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 5a 87 80 a0 70 38 4d 40 30 20 35 00 35 ae 10 00 00 18 65 38 80 a0 70 38 4d 40 30 20 35 00 35 ae 10 00 00 18 00 00 00 fd 00 30 90 a7 a7 23 01 00 00 00 00 00 00 00 00 00 00 fc 00 4c 51 31 34 30 4d 31 4a 57 34 39 0a 20 00 77
Block 0, Base EDID: EDID Structure Version & Revision: 1.4 Vendor & Product Identification: Manufacturer: SHP Model: 5411 Made in: week 53 of 2020 Basic Display Parameters & Features: Digital display Bits per primary color channel: 8 DisplayPort interface Maximum image size: 31 cm x 17 cm Gamma: 2.20 Supported color formats: RGB 4:4:4 Default (sRGB) color space is primary color space First detailed timing includes the native pixel format and preferred refresh rate Display is continuous frequency Color Characteristics: Red : 0.6396, 0.3291 Green: 0.2998, 0.5996 Blue : 0.1494, 0.0595 White: 0.3125, 0.3281 Established Timings I & II: none Standard Timings: none Detailed Timing Descriptors: DTD 1: 1920x1080 143.981 Hz 16:9 166.587 kHz 346.500 MHz (309 mm x 174 mm) Hfront 48 Hsync 32 Hback 80 Hpol N Vfront 3 Vsync 5 Vback 69 Vpol N DTD 2: 1920x1080 59.990 Hz 16:9 69.409 kHz 144.370 MHz (309 mm x 174 mm) Hfront 48 Hsync 32 Hback 80 Hpol N Vfront 3 Vsync 5 Vback 69 Vpol N Display Range Limits: Monitor ranges (Bare Limits): 48-144 Hz V, 167-167 kHz H, max dotclock 350 MHz Display Product Name: 'LQ140M1JW49' Checksum: 0x77
Changes in v4: -Add all modes from EDID -Provide EDID blob
Changes in v3: None
drivers/gpu/drm/panel/panel-edp.c | 44 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+)
We want to be moving to the generic edp-panel but even if we move to edp-panel there's no harm in supporting things the old way, especially as people are transitioning.
Reviewed-by: Douglas Anderson dianders@chromium.org
Hi,
On Wed, Feb 16, 2022 at 11:29 AM Doug Anderson dianders@chromium.org wrote:
Hi,
On Thu, Feb 10, 2022 at 3:58 AM Sankeerth Billakanti quic_sbillaka@quicinc.com wrote:
Add support for the 14" sharp,lq140m1jw46 eDP panel.
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com
00 ff ff ff ff ff ff 00 4d 10 23 15 00 00 00 00 35 1e 01 04 a5 1f 11 78 07 de 50 a3 54 4c 99 26 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 5a 87 80 a0 70 38 4d 40 30 20 35 00 35 ae 10 00 00 18 65 38 80 a0 70 38 4d 40 30 20 35 00 35 ae 10 00 00 18 00 00 00 fd 00 30 90 a7 a7 23 01 00 00 00 00 00 00 00 00 00 00 fc 00 4c 51 31 34 30 4d 31 4a 57 34 39 0a 20 00 77
Block 0, Base EDID: EDID Structure Version & Revision: 1.4 Vendor & Product Identification: Manufacturer: SHP Model: 5411 Made in: week 53 of 2020 Basic Display Parameters & Features: Digital display Bits per primary color channel: 8 DisplayPort interface Maximum image size: 31 cm x 17 cm Gamma: 2.20 Supported color formats: RGB 4:4:4 Default (sRGB) color space is primary color space First detailed timing includes the native pixel format and preferred refresh rate Display is continuous frequency Color Characteristics: Red : 0.6396, 0.3291 Green: 0.2998, 0.5996 Blue : 0.1494, 0.0595 White: 0.3125, 0.3281 Established Timings I & II: none Standard Timings: none Detailed Timing Descriptors: DTD 1: 1920x1080 143.981 Hz 16:9 166.587 kHz 346.500 MHz (309 mm x 174 mm) Hfront 48 Hsync 32 Hback 80 Hpol N Vfront 3 Vsync 5 Vback 69 Vpol N DTD 2: 1920x1080 59.990 Hz 16:9 69.409 kHz 144.370 MHz (309 mm x 174 mm) Hfront 48 Hsync 32 Hback 80 Hpol N Vfront 3 Vsync 5 Vback 69 Vpol N Display Range Limits: Monitor ranges (Bare Limits): 48-144 Hz V, 167-167 kHz H, max dotclock 350 MHz Display Product Name: 'LQ140M1JW49' Checksum: 0x77
Changes in v4: -Add all modes from EDID -Provide EDID blob
Changes in v3: None
drivers/gpu/drm/panel/panel-edp.c | 44 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+)
We want to be moving to the generic edp-panel but even if we move to edp-panel there's no harm in supporting things the old way, especially as people are transitioning.
Reviewed-by: Douglas Anderson dianders@chromium.org
...and pushed to drm-misc-next:
a874aba8bbc5 drm/panel-edp: Add eDP sharp panel support
So v5 shouldn't include this patch.
-Doug
Add support in the DP driver to utilize the custom eDP panels from drm/panels.
An eDP panel is always connected to the platform. So, the eDP connector can be reported as always connected. The display mode will be sourced from the panel. The panel mode will be set after the link training is completed.
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com ---
Changes in v4: - Remove obvious comments - Define separate connector_ops for eDP - Remove unnecessary checks
Changes in v3: None
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 ++ 3 files changed, 61 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 7cc4d21..5d314e6 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -1513,6 +1513,9 @@ int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder) return -EINVAL; }
+ if (dp->connector_type == DRM_MODE_CONNECTOR_eDP) + dp_hpd_plug_handle(dp_display, 0); + mutex_lock(&dp_display->event_mutex);
/* stop sentinel checking */ @@ -1577,6 +1580,9 @@ int msm_dp_display_disable(struct msm_dp *dp, struct drm_encoder *encoder)
dp_display = container_of(dp, struct dp_display_private, dp_display);
+ if (dp->connector_type == DRM_MODE_CONNECTOR_eDP) + dp_hpd_unplug_handle(dp_display, 0); + mutex_lock(&dp_display->event_mutex);
/* stop sentinel checking */ diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c index d4d360d..2436329 100644 --- a/drivers/gpu/drm/msm/dp/dp_drm.c +++ b/drivers/gpu/drm/msm/dp/dp_drm.c @@ -123,6 +123,25 @@ static enum drm_mode_status dp_connector_mode_valid( return dp_display_validate_mode(dp_disp, mode->clock); }
+static int edp_connector_get_modes(struct drm_connector *connector) +{ + struct msm_dp *dp; + + dp = to_dp_connector(connector)->dp_display; + + return drm_bridge_get_modes(dp->panel_bridge, connector); +} + +static enum drm_mode_status edp_connector_mode_valid( + struct drm_connector *connector, + struct drm_display_mode *mode) +{ + if (mode->clock > EDP_MAX_PIXEL_CLK_KHZ) + return MODE_CLOCK_HIGH; + + return MODE_OK; +} + static const struct drm_connector_funcs dp_connector_funcs = { .detect = dp_connector_detect, .fill_modes = drm_helper_probe_single_connector_modes, @@ -132,11 +151,24 @@ static const struct drm_connector_funcs dp_connector_funcs = { .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, };
+static const struct drm_connector_funcs edp_connector_funcs = { + .fill_modes = drm_helper_probe_single_connector_modes, + .destroy = drm_connector_cleanup, + .reset = drm_atomic_helper_connector_reset, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + static const struct drm_connector_helper_funcs dp_connector_helper_funcs = { .get_modes = dp_connector_get_modes, .mode_valid = dp_connector_mode_valid, };
+static const struct drm_connector_helper_funcs edp_connector_helper_funcs = { + .get_modes = edp_connector_get_modes, + .mode_valid = edp_connector_mode_valid, +}; + /* connector initialization */ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display) { @@ -154,18 +186,28 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
connector = &dp_connector->base;
- ret = drm_connector_init(dp_display->drm_dev, connector, - &dp_connector_funcs, - dp_display->connector_type); - if (ret) - return ERR_PTR(ret); + if (dp_display->connector_type == DRM_MODE_CONNECTOR_eDP) { + ret = drm_connector_init(dp_display->drm_dev, connector, + &edp_connector_funcs, DRM_MODE_CONNECTOR_eDP); + if (ret) + return ERR_PTR(ret); + + drm_connector_helper_add(connector, + &edp_connector_helper_funcs); + } else { + ret = drm_connector_init(dp_display->drm_dev, connector, + &dp_connector_funcs, + DRM_MODE_CONNECTOR_DisplayPort); + if (ret) + return ERR_PTR(ret);
- drm_connector_helper_add(connector, &dp_connector_helper_funcs); + drm_connector_helper_add(connector, &dp_connector_helper_funcs);
- /* - * Enable HPD to let hpd event is handled when cable is connected. - */ - connector->polled = DRM_CONNECTOR_POLL_HPD; + /* + * Enable HPD to let hpd event is handled when cable is connected. + */ + connector->polled = DRM_CONNECTOR_POLL_HPD; + }
drm_connector_attach_encoder(connector, dp_display->encoder);
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h index 3172da0..58c4f27 100644 --- a/drivers/gpu/drm/msm/dp/dp_parser.h +++ b/drivers/gpu/drm/msm/dp/dp_parser.h @@ -17,6 +17,9 @@ #define DP_MAX_PIXEL_CLK_KHZ 675000 #define DP_MAX_NUM_DP_LANES 4
+/* Maximum validated clock */ +#define EDP_MAX_PIXEL_CLK_KHZ 285550 + enum dp_pm_type { DP_CORE_PM, DP_CTRL_PM,
Hi,
On Thu, Feb 10, 2022 at 3:58 AM Sankeerth Billakanti quic_sbillaka@quicinc.com wrote:
Add support in the DP driver to utilize the custom eDP panels from drm/panels.
An eDP panel is always connected to the platform. So, the eDP connector can be reported as always connected. The display mode will be sourced from the panel. The panel mode will be set after the link training is completed.
Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com
Changes in v4:
- Remove obvious comments
- Define separate connector_ops for eDP
- Remove unnecessary checks
Changes in v3: None
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 ++ 3 files changed, 61 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 7cc4d21..5d314e6 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -1513,6 +1513,9 @@ int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder) return -EINVAL; }
if (dp->connector_type == DRM_MODE_CONNECTOR_eDP)dp_hpd_plug_handle(dp_display, 0);
I'm really not so sure here. You're just totally ignoring the HPD signal here which isn't right at all. The HPD signal is important for knowing if an edp panel is ready yet so you can't just ignore it. The only way this could work is if something else turns the panel on w/ plenty of time before your code runs so it has had time to get ready...
It feels like we just need to work to get this all plumbed up properly with the right power sequencing. That'll also allow us to enable the generic edp-panel stuff...
+static int edp_connector_get_modes(struct drm_connector *connector) +{
struct msm_dp *dp;dp = to_dp_connector(connector)->dp_display;return drm_bridge_get_modes(dp->panel_bridge, connector);+}
+static enum drm_mode_status edp_connector_mode_valid(
struct drm_connector *connector,struct drm_display_mode *mode)+{
if (mode->clock > EDP_MAX_PIXEL_CLK_KHZ)return MODE_CLOCK_HIGH;return MODE_OK;+}
static const struct drm_connector_funcs dp_connector_funcs = { .detect = dp_connector_detect, .fill_modes = drm_helper_probe_single_connector_modes, @@ -132,11 +151,24 @@ static const struct drm_connector_funcs dp_connector_funcs = { .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, };
+static const struct drm_connector_funcs edp_connector_funcs = {
.fill_modes = drm_helper_probe_single_connector_modes,.destroy = drm_connector_cleanup,.reset = drm_atomic_helper_connector_reset,.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,+};
static const struct drm_connector_helper_funcs dp_connector_helper_funcs = { .get_modes = dp_connector_get_modes, .mode_valid = dp_connector_mode_valid, };
+static const struct drm_connector_helper_funcs edp_connector_helper_funcs = {
.get_modes = edp_connector_get_modes,.mode_valid = edp_connector_mode_valid,+};
/* connector initialization */ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display) { @@ -154,18 +186,28 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
connector = &dp_connector->base;
ret = drm_connector_init(dp_display->drm_dev, connector,&dp_connector_funcs,dp_display->connector_type);if (ret)return ERR_PTR(ret);
if (dp_display->connector_type == DRM_MODE_CONNECTOR_eDP) {ret = drm_connector_init(dp_display->drm_dev, connector,&edp_connector_funcs, DRM_MODE_CONNECTOR_eDP);if (ret)return ERR_PTR(ret);drm_connector_helper_add(connector,&edp_connector_helper_funcs);} else {ret = drm_connector_init(dp_display->drm_dev, connector,&dp_connector_funcs,DRM_MODE_CONNECTOR_DisplayPort);if (ret)return ERR_PTR(ret);
drm_connector_helper_add(connector, &dp_connector_helper_funcs);
drm_connector_helper_add(connector, &dp_connector_helper_funcs);
This is probably not the correct way to do this. Drivers like this should really be moving _away_ from creating their own connectors. The idea is that you should just be creating bridges and then someone creates a "bridge connector" that implements the connector functions atop the bridge.
This is what Dmitry is working on [1]. Speaking of which, he really ought to be CCed on all your patches.
/** Enable HPD to let hpd event is handled when cable is connected.*/connector->polled = DRM_CONNECTOR_POLL_HPD;
/** Enable HPD to let hpd event is handled when cable is connected.*/connector->polled = DRM_CONNECTOR_POLL_HPD;} drm_connector_attach_encoder(connector, dp_display->encoder);diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h index 3172da0..58c4f27 100644 --- a/drivers/gpu/drm/msm/dp/dp_parser.h +++ b/drivers/gpu/drm/msm/dp/dp_parser.h @@ -17,6 +17,9 @@ #define DP_MAX_PIXEL_CLK_KHZ 675000 #define DP_MAX_NUM_DP_LANES 4
+/* Maximum validated clock */ +#define EDP_MAX_PIXEL_CLK_KHZ 285550
As discussed out-of-band, this isn't my favorite define. The datasheet for sc7280, which is what you're testing on / targeting, claims to support higher rates. Other users of this driver also ought to support higher rates. It might be OK short term, but it's definitely not a good long term solution.
[1] https://lore.kernel.org/all/20220211224006.1797846-5-dmitry.baryshkov@linaro...
dri-devel@lists.freedesktop.org
-
Bjorn Andersson -
Doug Anderson -
Sankeerth Billakanti