Re: [PATCH 6/6] arm64: dts: qcom: kaanapali-mtp: Enable display DSI devices

Dmitry Baryshkov
2026-03-26T04:22:27Z

On Thu, Mar 26, 2026 at 11:13:46AM +0800, yuanjiey wrote:
> On Tue, Mar 24, 2026 at 03:25:49AM +0200, Dmitry Baryshkov wrote:
> > On Sun, Mar 22, 2026 at 11:19:46PM -0700, Jingyi Wang wrote:
> > > From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
> > >
> > > Enable MDSS/DPU/DSI0 and add Novatek NT37801 panel on Kaanapali MTP
> > > board.
> > >
> > > NT37801 Spec V1.0 chapter "5.7.1 Power On Sequence" states VDDI ranges
> > > 1.65V~1.95V, but ldo12 ranges 1.2V~1.8V, so change ldo12 range to
> > > 1.65V~1.8V.
> > >
> > > pmh0110_d_e0_gpios and pmh0110_f_e0_gpios are configured for
> > > level shifters. Kaanapali need configure these pinctrl for panel
> > > function.
> > >
> > > Signed-off-by: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
> > > Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> > > ---
> > > arch/arm64/boot/dts/qcom/kaanapali-mtp.dts | 118 ++++++++++++++++++++++++++++-
> > > 1 file changed, 117 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts b/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts
> > > index d0f3909621c9..07247dc98b70 100644
> > > --- a/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts
> > > +++ b/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts
> > > @@ -375,7 +375,7 @@ vreg_l11b_1p0: ldo11 {
> > >
> > > vreg_l12b_1p8: ldo12 {
> > > regulator-name = "vreg_l12b_1p8";
> > > - regulator-min-microvolt = <1200000>;
> > > + regulator-min-microvolt = <1650000>;
> >
> > Separate commit, Fixes tag.
> >
> > > regulator-max-microvolt = <1800000>;
> > > regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
> > > regulator-allow-set-load;
> > > @@ -873,6 +873,51 @@ &lpass_vamacro {
> > > qcom,dmic-sample-rate = <4800000>;
> > > };
> > >
> > > +&mdss {
> > > + status = "okay";
> > > +};
> > > +
> > > +&mdss_dsi0 {
> > > + vdda-supply = <&vreg_l1d_1p2>;
> > > + status = "okay";
> > > +
> > > + panel@0 {
> > > + compatible = "novatek,nt37801";
> > > + reg = <0>;
> > > +
> > > + pinctrl-0 = <&sde_dsi_active &sde_te_active &sde_esync0_suspend
> >
> > Why is this esync0_suspend?
>
> I check downstream sde_esync0_suspend and sde_esync0_active is same config in kaanapali,
> maybe I need rename a proper name for these pinctrl.

Yes, please.

>
> > > + &sde_mdp_vsync_p_1p2_active &sde_mdp_vsync_p_1p8_active
> > > + &sde_disp0_rst_1p2_active &sde_disp0_rst_1p8_active>;
> > > + pinctrl-1 = <&sde_dsi_suspend &sde_te_suspend &sde_esync0_suspend
> > > + &sde_mdp_vsync_p_1p2_active &sde_mdp_vsync_p_1p8_active
> > > + &sde_disp0_rst_1p2_active &sde_disp0_rst_1p8_active>;
> > > + pinctrl-names = "default", "sleep";
> > > +

> > > @@ -970,6 +1015,42 @@ bt_default: bt-default-state {
> > > };
> > > };
> > >
> > > +&pmh0110_d_e0_gpios {
> > > + sde_mdp_vsync_p_1p2_active: sde-mdp-vsync-p-1p2-active-state {
> >
> > If these are level shifters, should they be configured as a hog? Or
> > should they be modelled as regulators?
> >
> > I'm not sure, how do we handle shifters and their supplies.
>
> I think regulator-fixed is OK, but if change these pmh0110_d_e0_gpios
> to regulator-fixed, means need to modify the panel driver to accept
> these changes, is that correct?

The problem is that it is not a panel supply.

The 100% correct, but also 80% (or 95%) overkill solution would be to
define a separate "level-shifter" device sitting between DSI host and
DSI panel and let it consume that regulator.

But, as I wrote, it's a definite overkill, unless we start getting lots
of those and require more precise control.

I'm torn between accepting this solution and asking to stuff those GPIOs
into the GPIO hog.

Probably the current solution is fine, just please add the small comment
before those pins (it's a part of the commit message, but frankly
speaking I missed it when first reviewing the patch).

>
> I see downstream also directly manipulates these paired GPIOs.

That's a bad reference. Vendor kernels are frequently full of the
not-well-thought solutions.

>
--
With best wishes
Dmitry