On Wed, 23 Feb 2022 at 21:27, Kuogee Hsieh quic_khsieh@quicinc.com wrote:
On 2/23/2022 10:22 AM, Dmitry Baryshkov wrote:
On 23/02/2022 20:21, Kuogee Hsieh wrote:
On 2/18/2022 6:22 PM, Dmitry Baryshkov wrote:
On Sat, 19 Feb 2022 at 03:55, Stephen Boyd swboyd@chromium.org wrote:
Quoting Dmitry Baryshkov (2022-02-18 14:32:53)
On 19/02/2022 00:31, Kuogee Hsieh wrote: > On 2/11/2022 2:40 PM, Dmitry Baryshkov wrote: >> There is little point in having both connector and root bridge >> implementation in the same driver. Move connector's >> functionality to the >> bridge to let next bridge in chain to override it. >> >> Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org > This patch break primary (edp) display > > -- right half of screen garbled > > -- screen shift vertically > > below are error messages seen -- > > [ 36.679216] panel-edp soc@0:edp_panel: No display modes > [ 36.687272] panel-edp soc@0:edp_panel: No display modes > [ 40.593709] panel-edp soc@0:edp_panel: No display modes > [ 40.600285] panel-edp soc@0:edp_panel: No display modes So, before the patch the drm core was getting modes from the drm_connector (which means, modes from drm driver itself). With this patch the panel-edp tries to get modes.
Could you please check, why panel_edp_get_modes() fails? Assuming that you use platform panel-edp binding (rather than 'edp-panel') could you please check you have either of the following:
- ddc bus for EDID?
I don't see anywhere where the ddc pointer is set for the dp bridge in msm_dp_bridge_init(). Is that required though? I'd think simple panel is still being used here so reading EDID isn't required.
I meant the 'ddc-i2c-bus' property for the corresponding eDP panel.
- either num_timing or num_modes in your panel desc.
After reading the panel-edp's code I don't have another cause for panel_edp_get_modes(). It should either have a DDC bus specified using the mentioned device tree property, or it should have specified the timings.
Kuogee, which platform were you using when testing this patch? Could you please share the dts fragment?
I cherry-picked your patches on top of our internal release which is usually have some (or many) patches behind msm-next.
where is "ddc-i2c-bus" located?
In the panel device node.
Can you please share it too?
&soc { edp_power_supply: edp_power { compatible = "regulator-fixed"; regulator-name = "edp_backlight_power";
regulator-always-on; regulator-boot-on; }; edp_backlight: edp_backlight { compatible = "pwm-backlight"; pwms = <&pm8350c_pwm 3 65535>; power-supply = <&edp_power_supply>; enable-gpio = <&pm8350c_gpios 7 GPIO_ACTIVE_HIGH>; pinctrl-names = "default"; pinctrl-0 = <&backlight_pwm_default>; }; edp_panel: edp_panel { compatible = "sharp_lq140m1jw46";
I'd assume that the panel is supported by the patch https://patchwork.kernel.org/project/linux-arm-msm/patch/1644494255-6632-5-g... and the compatible value is just an old value. Provided that the panel description defines modes, I'd ask for some debug from the panel_edp_get_modes(). At least let's see why panel_edp_get_non_edid_modes() / panel_edp_get_display_modes() returns no modes.
Regarding the ddc bus, if you have separate i2c bus connected to this panel, the ddc-i2c-bus = <&i2c_N>; property should go to this device node.
pinctrl-names = "default"; pinctrl-0 = <&edp_hot_plug_det>,<&edp_panel_power_default>;
power-supply = <&edp_power_supply>; backlight = <&edp_backlight>; ports { #address-cells = <1>; #size-cells = <0>; port@0 { reg = <0>; edp_panel_in: endpoint { remote-endpoint = <&edp_out>; }; }; }; };};
msm_edp: edp@aea0000 { compatible = "qcom,sc7280-edp"; reg = <0 0xaea0000 0 0x200>, <0 0xaea0200 0 0x200>, <0 0xaea0400 0 0xc00>, <0 0xaea1000 0 0x400>; interrupt-parent = <&mdss>; interrupts = <14>; clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GCC_EDP_CLKREF_EN>, <&dispccDISP_CC_MDSS_AHB_CLK>, <&dispcc DISP_CC_MDSS_EDP_AUX_CLK>, <&dispcc DISP_CC_MDSS_EDP_LINK_CLK>, <&dispcc DISP_CC_MDSS_EDP_LINK_INTF_CLK>, <&dispcc DISP_CC_MDSS_EDP_PIXEL_CLK>; clock-names = "core_xo", "core_ref", "core_iface", "core_aux", "ctrl_link", "ctrl_link_iface", "stream_pixel"; #clock-cells = <1>; assigned-clocks = <&dispcc DISP_CC_MDSS_EDP_LINK_CLK_SRC>, <&dispcc DISP_CC_MDSS_EDP_PIXEL_CLK_SRC>; assigned-clock-parents = <&edp_phy 0>, <&edp_phy 1>;
phys = <&edp_phy>; phy-names = "dp"; operating-points-v2 = <&edp_opp_table>; power-domains = <&rpmhpd SC7280_CX>; #address-cells = <1>; #size-cells = <0>; status = "disabled"; ports { #address-cells = <1>; #size-cells = <0>; port@0 { reg = <0>; edp_in: endpoint {remote-endpoint = <&dpu_intf5_out>; }; }; };
edp_opp_table: opp-table { compatible ="operating-points-v2";
opp-160000000 { opp-hz = /bits/ 64<160000000>; required-opps = <&rpmhpd_opp_low_svs>; };
opp-270000000 { opp-hz = /bits/ 64<270000000>; required-opps = <&rpmhpd_opp_svs>; };
opp-540000000 { opp-hz = /bits/ 64<540000000>; required-opps = <&rpmhpd_opp_nom>; };
opp-810000000 { opp-hz = /bits/ 64<810000000>; required-opps = <&rpmhpd_opp_nom>; }; }; };