Message ID | 20240624-oneplus8-v1-0-388eecf2dff7@postmarketos.org |
---|---|
Headers | show |
Series | qcom: initial support for the OnePlus 8T | expand |
On Mon, Jun 24, 2024 at 03:30:24AM GMT, Caleb Connolly wrote: > Add bindings for the SM8250 OnePlus devices, a common devicetree, > touchscreen and display drivers, and a dts for the OnePlus 8T (kebab). > > The OnePlus 8 series is made up of 3 flagship smartphones from 2019, > featuring the Qualcomm X55 5G PCIe modem. > > This series introduces initial support for the 8T, adding drivers for > the 1080x2400 120Hz DSC panel and the Synaptics TCM Oncell touchscreen. > > The panel driver suffers from similar limitations to the LG SW43408 > panel found on the Pixel 3, namely that after toggling the reset GPIO it > is not possible to get the panel into a working state. Just to point it out: this is no longer true for SW43408. The panel wakes up and works after toggling the reset. It seems, there is an issue with one of the regulators, but not with the reset and/or panel startup. > Given the apparent prevelance of this issue, particularly with DSC > panels, I believe this is a bug in the core DSI code, and not a device > or panel specific issue. I think it is still useful to accept these > panel drivers into upstream since, from a users perspective, the panel > is fully functional just by leaving the reset GPIO alone and keeping the > regulator on. The only (theoretical) downside is worse battery life, > which is a small price to pay for a working display. >
On Mon, Jun 24, 2024 at 03:30:29AM GMT, Caleb Connolly wrote: > This is a 1080x2400 120hz panel used on the OnePlus 8T. It uses DSC but > uses non-standard DCS commands. Please add a note regarding the panel using long packets for all the commands. Also the cover letter had a mention of the panel not fully comming up after being reset, is that still true? If so, it should be mentioned in the commit message too. > Signed-off-by: Caleb Connolly <caleb@postmarketos.org> > --- > MAINTAINERS | 7 + > drivers/gpu/drm/panel/Kconfig | 9 + > drivers/gpu/drm/panel/Makefile | 1 + > drivers/gpu/drm/panel/panel-samsung-amb655x.c | 420 ++++++++++++++++++++++++++ > 4 files changed, 437 insertions(+) > +static int samsung_amb655x_on(struct samsung_amb655x *amb655x) > +{ > + struct drm_dsc_picture_parameter_set pps; > + struct mipi_dsi_device *dsi = amb655x->dsi; > + struct mipi_dsi_multi_context ctx = { .dsi = dsi }; > + struct device *dev = &dsi->dev; > + int ret; > + > + dsi->mode_flags |= MIPI_DSI_MODE_LPM; > + > + drm_dsc_pps_payload_pack(&pps, &amb655x->dsc); > + > + mipi_dsi_dcs_write_long(&ctx, 0xf0, 0x5a, 0x5a); > + mipi_dsi_dcs_write_buffer_multi(&ctx, &pps, sizeof(pps)); > + mipi_dsi_dcs_write_long(&ctx, 0x9d, 0x01); > + mipi_dsi_dcs_write_long(&ctx, 0xf0, 0xa5, 0xa5); > + > + ret = mipi_dsi_dcs_exit_sleep_mode(dsi); _multi. Shouldn't it be a long package too? > + if (ret < 0) { > + dev_err(dev, "Failed to exit sleep mode: %d\n", ret); > + return ret; > + } > + usleep_range(11000, 12000); mipi_dsi_msleep() or add mipi_dsi_usleep_range(). > + ret = mipi_dsi_dcs_set_column_address(dsi, 0x0000, 1080 - 1); _multi, adding the function as required > + if (ret < 0) { > + dev_err(dev, "Failed to set column address: %d\n", ret); > + return ret; > + } > + > + ret = mipi_dsi_dcs_set_page_address(dsi, 0x0000, 2400 - 1); _multi > + if (ret < 0) { > + dev_err(dev, "Failed to set page address: %d\n", ret); > + return ret; > + } > + > + /* FD Setting */ > + mipi_dsi_dcs_write_long(&ctx, 0xf0, 0x5a, 0x5a); > + mipi_dsi_dcs_write_long(&ctx, 0xd5, 0x8d); > + mipi_dsi_dcs_write_long(&ctx, 0xb0, 0x0a); > + mipi_dsi_dcs_write_long(&ctx, 0xd5, 0x05); > + mipi_dsi_dcs_write_long(&ctx, 0xf0, 0xa5, 0xa5); > + > + /* FFC Function */ > + mipi_dsi_dcs_write_long(&ctx, 0xfc, 0x5a, 0x5a); > + mipi_dsi_dcs_write_long(&ctx, 0xb0, 0x01); > + mipi_dsi_dcs_write_long(&ctx, 0xe4, 0xa6, 0x75, 0xa3); > + mipi_dsi_dcs_write_long(&ctx, 0xe9, > + 0x11, 0x75, 0xa6, 0x75, 0xa3, 0x4b, 0x17, 0xac, > + 0x4b, 0x17, 0xac, 0x00, 0x19, 0x19); > + mipi_dsi_dcs_write_long(&ctx, 0xfc, 0xa5, 0xa5); > + msleep(61); mipi_dsi_msleep > + > + /* Dimming Setting */ > + mipi_dsi_dcs_write_long(&ctx, 0xf0, 0x5a, 0x5a); > + mipi_dsi_dcs_write_long(&ctx, 0xb0, 0x06); > + mipi_dsi_dcs_write_long(&ctx, 0xb7, 0x01); > + mipi_dsi_dcs_write_long(&ctx, 0xb0, 0x05); > + mipi_dsi_dcs_write_long(&ctx, 0xb7, 0x13); > + mipi_dsi_dcs_write_long(&ctx, 0xb0, 0x01); > + mipi_dsi_dcs_write_long(&ctx, 0xb7, 0x4c); > + mipi_dsi_dcs_write_long(&ctx, 0xf0, 0xa5, 0xa5); > + > + mipi_dsi_dcs_write_long(&ctx, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x20); > + > + /* refresh rate Transition */ > + mipi_dsi_dcs_write_long(&ctx, 0xf0, 0x5a, 0x5a); > + /* 60 Hz */ > + //mipi_dsi_dcs_write_long(&ctx, 0x60, 0x00); > + /* 120 Hz */ > + mipi_dsi_dcs_write_long(&ctx, 0x60, 0x10); > + > + mipi_dsi_dcs_write_long(&ctx, 0xf0, 0xa5, 0xa5); > + > + /* ACL Mode */ > + mipi_dsi_dcs_write_long(&ctx, 0xf0, 0x5a, 0x5a); > + mipi_dsi_dcs_write_long(&ctx, MIPI_DCS_WRITE_POWER_SAVE, 0x00); > + mipi_dsi_dcs_write_long(&ctx, 0xf0, 0xa5, 0xa5); > + mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x20); _multi > + msleep(110); mipi_dsi_msleep() > + > + /* Enable backlight */ > + mipi_dsi_dcs_write_long(&ctx, 0x9F, 0x5A, 0x5A); > + mipi_dsi_dcs_set_display_on(dsi); _multi > + mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_ENTER_NORMAL_MODE); _multi > + mipi_dsi_dcs_write_long(&ctx, 0x9F, 0xA5, 0xA5); > + > + return ctx.accum_err; > +} > + > +static int samsung_amb655x_off(struct samsung_amb655x *amb655x) > +{ > + struct mipi_dsi_device *dsi = amb655x->dsi; > + struct mipi_dsi_multi_context ctx = { .dsi = dsi }; > + struct device *dev = &dsi->dev; > + int ret; > + > + dsi->mode_flags |= MIPI_DSI_MODE_LPM; > + > + mipi_dsi_dcs_write_long(&ctx, 0x9f, 0x5a, 0x5a); > + > + ret = mipi_dsi_dcs_set_display_on(dsi); _multi > + if (ret < 0) { > + dev_err(dev, "Failed to set display on: %d\n", ret); > + return ret; > + } > + msleep(20); mipi_dsi_msleep > + > + ret = mipi_dsi_dcs_set_display_off(dsi); _multi > + if (ret < 0) { > + dev_err(dev, "Failed to set display off: %d\n", ret); > + return ret; > + } > + msleep(20); mipi_dsi_msleep > + > + ret = mipi_dsi_dcs_enter_sleep_mode(dsi); _multi > + if (ret < 0) { > + dev_err(dev, "Failed to enter sleep mode: %d\n", ret); > + return ret; > + } > + > + mipi_dsi_dcs_write_long(&ctx, 0x9f, 0xa5, 0xa5); > + msleep(150); > + > + return ctx.accum_err; > +} > +
On 24/06/2024 07:18, Dmitry Baryshkov wrote: > On Mon, Jun 24, 2024 at 03:30:24AM GMT, Caleb Connolly wrote: >> Add bindings for the SM8250 OnePlus devices, a common devicetree, >> touchscreen and display drivers, and a dts for the OnePlus 8T (kebab). >> >> The OnePlus 8 series is made up of 3 flagship smartphones from 2019, >> featuring the Qualcomm X55 5G PCIe modem. >> >> This series introduces initial support for the 8T, adding drivers for >> the 1080x2400 120Hz DSC panel and the Synaptics TCM Oncell touchscreen. >> >> The panel driver suffers from similar limitations to the LG SW43408 >> panel found on the Pixel 3, namely that after toggling the reset GPIO it >> is not possible to get the panel into a working state. > > Just to point it out: this is no longer true for SW43408. The panel > wakes up and works after toggling the reset. It seems, there is an issue > with one of the regulators, but not with the reset and/or panel startup. Hmm ok, I've heard mixed reports then, I should try it out myself again. I'll update the cover letter to reflect this. Thanks. > >> Given the apparent prevelance of this issue, particularly with DSC >> panels, I believe this is a bug in the core DSI code, and not a device >> or panel specific issue. I think it is still useful to accept these >> panel drivers into upstream since, from a users perspective, the panel >> is fully functional just by leaving the reset GPIO alone and keeping the >> regulator on. The only (theoretical) downside is worse battery life, >> which is a small price to pay for a working display. >> >
On Mon, 24 Jun 2024 at 14:33, Caleb Connolly <caleb@postmarketos.org> wrote: > > > > On 24/06/2024 07:18, Dmitry Baryshkov wrote: > > On Mon, Jun 24, 2024 at 03:30:24AM GMT, Caleb Connolly wrote: > >> Add bindings for the SM8250 OnePlus devices, a common devicetree, > >> touchscreen and display drivers, and a dts for the OnePlus 8T (kebab). > >> > >> The OnePlus 8 series is made up of 3 flagship smartphones from 2019, > >> featuring the Qualcomm X55 5G PCIe modem. > >> > >> This series introduces initial support for the 8T, adding drivers for > >> the 1080x2400 120Hz DSC panel and the Synaptics TCM Oncell touchscreen. > >> > >> The panel driver suffers from similar limitations to the LG SW43408 > >> panel found on the Pixel 3, namely that after toggling the reset GPIO it > >> is not possible to get the panel into a working state. > > > > Just to point it out: this is no longer true for SW43408. The panel > > wakes up and works after toggling the reset. It seems, there is an issue > > with one of the regulators, but not with the reset and/or panel startup. > > Hmm ok, I've heard mixed reports then, I should try it out myself again. During MAD24 we have seen an issue with the regulators, when the panel doesn't wake up. But resetting the panel works. > > I'll update the cover letter to reflect this. Thanks. > > > >> Given the apparent prevelance of this issue, particularly with DSC > >> panels, I believe this is a bug in the core DSI code, and not a device > >> or panel specific issue. I think it is still useful to accept these > >> panel drivers into upstream since, from a users perspective, the panel > >> is fully functional just by leaving the reset GPIO alone and keeping the > >> regulator on. The only (theoretical) downside is worse battery life, > >> which is a small price to pay for a working display. > >> > >
On Mon, Jun 24, 2024 at 03:30:27AM +0200, Caleb Connolly wrote: > Add bindings for the OnePlus 8, 8 Pro, and 8T devices. > > Signed-off-by: Caleb Connolly <caleb@postmarketos.org> > --- > Documentation/devicetree/bindings/arm/qcom.yaml | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml > index d839691a900c..a41eeb8c3fc5 100644 > --- a/Documentation/devicetree/bindings/arm/qcom.yaml > +++ b/Documentation/devicetree/bindings/arm/qcom.yaml > @@ -986,8 +986,11 @@ properties: > - enum: > - qcom,qrb5165-rb5 > - qcom,sm8250-hdk > - qcom,sm8250-mtp > + - oneplus,kebab > + - oneplus,instantnoodle > + - oneplus,instantnoodlep Which one is which device in the commit msg? I can only guess that 'p' here corresponds to Pro. A comment after the compatibles would help. > - sony,pdx203-generic > - sony,pdx206-generic > - xiaomi,elish > - xiaomi,pipa > > -- > 2.45.0 >
On Mon, Jun 24, 2024 at 03:30:31AM GMT, Caleb Connolly wrote: > Initial support for USB, UFS, touchscreen, panel, wifi, and bluetooth. > Nice. > diff --git a/arch/arm64/boot/dts/qcom/sm8250-oneplus-common.dtsi b/arch/arm64/boot/dts/qcom/sm8250-oneplus-common.dtsi [..] > + vph_pwr: vph-pwr-regulator { Please keep nodes sorted by address, then node name, then label (as applicable). Perhaps making the -regulator suffix a regulator- prefix instead (to keep them grouped). > + compatible = "regulator-fixed"; > + regulator-name = "vph_pwr"; > + regulator-min-microvolt = <3700000>; > + regulator-max-microvolt = <3700000>; > + regulator-always-on; > + }; > + > + vreg_s4a_1p8: vreg-s4a-1p8 { > + compatible = "regulator-fixed"; > + regulator-name = "vreg_s4a_1p8"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-always-on; > + }; [..] > +&adsp { > + status = "okay"; Per Documentation/devicetree/bindings/dts-coding-style.rst please keep "status" as last property in your nodes. > + firmware-name = "qcom/sm8250/OnePlus/adsp.mbn"; > +}; > + [..] > +&mdss_dsi0 { > + status = "okay"; > + vdda-supply = <&vreg_l9a_1p2>; > + > + display_panel: panel@0 { > + reg = <0>; > + vddio-supply = <&vreg_l14a_1p8>; > + vdd-supply = <&vreg_l11c_3p3>; > + avdd-supply = <&panel_avdd_5p5>; How do you know that the panel will have these properties, when you don't give it a compatible here? Not a strong objection, but perhaps this should be pushed out? > + /* FIXME: There is a bug somewhere in the display stack and it isn't > + * possible to get the panel to a working state after toggling reset. > + * At best it just shows one or more vertical red lines. So for now > + * let's skip the reset GPIO. > + */ > + // reset-gpios = <&tlmm 75 GPIO_ACTIVE_LOW>; > + > + pinctrl-0 = <&panel_reset_pins &panel_vsync_pins &panel_vout_pins>; > + pinctrl-names = "default"; > + > + status = "disabled"; > + > + port { > + panel_in_0: endpoint { > + remote-endpoint = <&mdss_dsi0_out>; > + }; > + }; > + }; > + > +}; [..] > +&pm8150_gpios { > + gpio-reserved-ranges = <2 1>, <4 1>, <8 1>; How come? > +}; > + [..] > +&tlmm { > + gpio-reserved-ranges = <28 4>, <40 4>; > + > + bt_en_state: bt-default-state { > + pins = "gpio21"; > + function = "gpio"; > + drive-strength = <16>; > + output-low; > + bias-pull-up; > + }; > + > + wlan_en_state: wlan-default-state { > + wlan-en-pins { Perhaps flatten this? > + pins = "gpio20"; > + function = "gpio"; > + > + drive-strength = <16>; > + output-low; > + bias-pull-up; > + }; > + }; > + [..] > diff --git a/arch/arm64/boot/dts/qcom/sm8250-oneplus-kebab.dts b/arch/arm64/boot/dts/qcom/sm8250-oneplus-kebab.dts [..] > +&i2c13 { [..] > +}; > + > +&display_panel { 'd' < 'i' Regards, Bjorn > + compatible = "samsung,amb655x"; > + status = "okay"; > +}; > > -- > 2.45.0 >