On Wed, 25 Dec 2024 at 11:34, Damon Ding <damon.ding@xxxxxxxxxxxxxx> wrote:
>
> Hi Dmitry,
>
> On 2024/12/20 13:38, Dmitry Baryshkov wrote:
> > On Fri, 20 Dec 2024 at 04:38, Damon Ding <damon.ding@xxxxxxxxxxxxxx> wrote:
> >>
> >> Hi Dmitry,
> >>
> >> On 2024/12/20 8:20, Dmitry Baryshkov wrote:
> >>> On Thu, Dec 19, 2024 at 04:06:03PM +0800, Damon Ding wrote:
> >>>> Add the necessary DT changes to enable eDP0 on RK3588S EVB1 board:
> >>>> - Add edp-panel node
> >>>> - Set pinctrl of pwm12 for backlight
> >>>> - Enable edp0/hdptxphy0/vp2
> >>>>
> >>>> Signed-off-by: Damon Ding <damon.ding@xxxxxxxxxxxxxx>
> >>>>
> >>>> ---
> >>>>
> >>>> Changes in v2:
> >>>> - Remove brightness-levels and default-brightness-level properties in
> >>>> backlight node.
> >>>> - Add the detail DT changes to commit message.
> >>>>
> >>>> Changes in v3:
> >>>> - Use aux-bus instead of platform bus for edp-panel.
> >>>> ---
> >>>> .../boot/dts/rockchip/rk3588s-evb1-v10.dts | 52 +++++++++++++++++++
> >>>> 1 file changed, 52 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
> >>>> index bc4077575beb..9547ab18e596 100644
> >>>> --- a/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
> >>>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-evb1-v10.dts
> >>>> @@ -9,6 +9,7 @@
> >>>> #include <dt-bindings/gpio/gpio.h>
> >>>> #include <dt-bindings/input/input.h>
> >>>> #include <dt-bindings/pinctrl/rockchip.h>
> >>>> +#include <dt-bindings/soc/rockchip,vop2.h>
> >>>> #include <dt-bindings/usb/pd.h>
> >>>> #include "rk3588s.dtsi"
> >>>>
> >>>> @@ -238,6 +239,41 @@ &combphy2_psu {
> >>>> status = "okay";
> >>>> };
> >>>>
> >>>> +&edp0 {
> >>>> + force-hpd;
> >>>> + status = "okay";
> >>>> +
> >>>> + aux-bus {
> >>>> + panel {
> >>>> + compatible = "lg,lp079qx1-sp0v";
> >>>
> >>> Why do you need the particular compat string here? Can you use the
> >>> generic "edp-panel" instead? What if the user swaps the panel?
> >>>
> >>
> >> The eDP panels used in conjunction with the RK3588S EVB1 have broken
> >> identification, which is one of the valid reasons for using a particular
> >> compat string. So the generic_edp_panel_probe() can not return success
> >> when using the "edp-panel".
> >
> > Broken how? I don't see such info in the commit message.
> >
>
> The log related to the broken identification may be like:
>
> [ 0.623793] panel-simple-dp-aux aux-fdec0000.edp: Unknown panel ETC
> 0x0000, using conservative timings
According to [1] the ETC / 0x0000 is a correct identification for that
panel. I'd suggest adding the timings to the driver instead.
[1] https://www.elecok.com/data_sheet/107657/LP079QX1-SP0V_7.9%22_a-Si_TFT-LCD%2CPanel_for_LG_Display(EN).pdf?download=true
>
> The eDP panel used in RK3588S EVB1 is indeed the LP079QX1_SP0V model, it
> should be also reasonable to use the "lg,lp079qx1-sp0v".
>
> And I will mention all of the above in the commit message for the next
> version.
>
> >>
> >>>> + backlight = <&backlight>;
> >>>> + power-supply = <&vcc3v3_lcd_edp>;
> >>>> +
> >>>> + port {
> >>>> + panel_in_edp: endpoint {
> >>>> + remote-endpoint = <&edp_out_panel>;
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> +};
> >>>> +
> >>>> +&edp0_in {
> >>>> + edp0_in_vp2: endpoint {
> >>>> + remote-endpoint = <&vp2_out_edp0>;
> >>>> + };
> >>>> +};
> >>>> +
> >>>> +&edp0_out {
> >>>> + edp_out_panel: endpoint {
> >>>> + remote-endpoint = <&panel_in_edp>;
> >>>> + };
> >>>> +};
> >>>> +
> >>>> +&hdptxphy0 {
> >>>> + status = "okay";
> >>>> +};
> >>>> +
> >>>> &i2c3 {
> >>>> status = "okay";
> >>>>
> >>>> @@ -399,6 +435,7 @@ usbc0_int: usbc0-int {
> >>>> };
> >>>>
> >>>> &pwm12 {
> >>>> + pinctrl-0 = <&pwm12m1_pins>;
> >>>> status = "okay";
> >>>> };
> >>>>
> >>>> @@ -1168,3 +1205,18 @@ usbdp_phy0_dp_altmode_mux: endpoint@1 {
> >>>> };
> >>>> };
> >>>> };
> >>>> +
> >>>> +&vop_mmu {
> >>>> + status = "okay";
> >>>> +};
> >>>> +
> >>>> +&vop {
> >>>> + status = "okay";
> >>>> +};
> >>>> +
> >>>> +&vp2 {
> >>>> + vp2_out_edp0: endpoint@ROCKCHIP_VOP2_EP_EDP0 {
> >>>> + reg = <ROCKCHIP_VOP2_EP_EDP0>;
> >>>> + remote-endpoint = <&edp0_in_vp2>;
> >>>> + };
> >>>> +};
> >>>> --
> >>>> 2.34.1
> >>>>
>
> Best regards
> Damon
>
--
With best wishes
Dmitry
