From: Matthias Kaehlcke <mka@chromium.org>
To: Harigovindan P <harigovi@codeaurora.org>
Cc: dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org,
freedreno@lists.freedesktop.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, robdclark@gmail.com,
seanpaul@chromium.org, hoegsberg@chromium.org,
kalyan_t@codeaurora.org, nganji@codeaurora.org
Subject: Re: [v2] arm64: dts: sc7180: add dsi controller and phy entries for idp dts
Date: Wed, 6 May 2020 12:25:47 -0700 [thread overview]
Message-ID: <20200506192547.GY4525@google.com> (raw)
In-Reply-To: <20200214184937.GA15781@google.com>
On Fri, Feb 14, 2020 at 10:49:37AM -0800, Matthias Kaehlcke wrote:
> On Tue, Feb 11, 2020 at 05:07:35PM +0530, Harigovindan P wrote:
>
> > subject: arm64: dts: sc7180: add dsi controller and phy entries for idp dts
>
> nit: 'dts' at the end is redundant, the prefixes make it clear that this
> is about DT entries.
>
> Also the message isn't really concise. The main entries for the DSI
> controller and the PHY are in sc7180.dtsi. I would suggest to drop
> any mentions of DSI controller and PHYs, and just say something like
> 'Add nodes for IDP display'. In the body you could mention that the
> display is the Visionox RM69299.
>
> > Adding dsi controller and phy entries for idp dt.
> >
> > Signed-off-by: Harigovindan P <harigovi@codeaurora.org>
> > ---
> >
> > Changes in v1:
> > - Added dsi controller and dsi phy entries for idp dts
>
> Changes in v1 is pointless, it's the first patch
>
> > Changes in v2:
> > - Adding dependency patchwork series
> > - Removing suspend configuration
> > - Adding blank before curly brace
> >
> > This patch depends on following patchwork series:
> >
> > https://patchwork.kernel.org/patch/11364687/
> > https://patchwork.kernel.org/patch/11366303/
> >
> > arch/arm64/boot/dts/qcom/sc7180-idp.dts | 55 +++++++++++++++++++++++++
> > 1 file changed, 55 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > index 388f50ad4fde..6ccf8c3603ab 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > @@ -7,6 +7,7 @@
> >
> > /dts-v1/;
> >
> > +#include <dt-bindings/gpio/gpio.h>
> > #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> > #include "sc7180.dtsi"
> > #include "pm6150.dtsi"
> > @@ -232,6 +233,49 @@ vreg_bob: bob {
> > };
> > };
> >
> > +&dsi0 {
> > + status = "okay";
> > +
> > + vdda-supply = <&vreg_l3c_1p2>;
> > +
> > + panel@0 {
> > + compatible = "visionox,rm69299-1080p-display";
> > + reg = <0>;
> > +
> > + vdda-supply = <&vreg_l8c_1p8>;
> > + vdd3p3-supply = <&vreg_l18a_2p8>;
> > +
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&disp_pins>;
> > +
> > + reset-gpios = <&pm6150l_gpio 3 GPIO_ACTIVE_HIGH>;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + port@0 {
> > + reg = <0>;
> > + panel0_in: endpoint {
> > + remote-endpoint = <&dsi0_out>;
> > + };
> > + };
> > + };
> > + };
> > +
> > + ports {
> > + port@1 {
> > + endpoint {
> > + remote-endpoint = <&panel0_in>;
> > + data-lanes = <0 1 2 3>;
> > + };
> > + };
> > + };
> > +};
> > +
> > +&dsi_phy {
> > + status = "okay";
> > +};
> > +
> > &qspi {
> > status = "okay";
> > pinctrl-names = "default";
> > @@ -289,6 +333,17 @@ &usb_1_qmpphy {
> >
> > /* PINCTRL - additions to nodes defined in sc7180.dtsi */
> >
> > +&pm6150l_gpio {
> > + disp_pins: disp-pins {
> > + pins = "gpio3";
> > + function = "func1";
> > + qcom,drive-strength = <2>;
> > + power-source = <0>;
> > + bias-disable;
> > + output-low;
> > + };
> > +};
> > +
> > &qspi_clk {
> > pinconf {
> > pins = "gpio63";
>
> To get the display actually to work you also need this:
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> index 88919da1510b03..fdbcb56dfa81f9 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> @@ -276,6 +276,14 @@
> status = "okay";
> };
>
> +&mdp {
> + status = "okay";
> +};
> +
> +&mdss {
> + status = "okay";
> +};
> +
> &qspi {
> status = "okay";
> pinctrl-names = "default";
>
> Maybe just add this to this patch?
ping
the display driver landed in drm-misc, but this patch still needs a
respin.
WARNING: multiple messages have this Message-ID (diff)
From: Matthias Kaehlcke <mka@chromium.org>
To: Harigovindan P <harigovi@codeaurora.org>
Cc: devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
seanpaul@chromium.org, kalyan_t@codeaurora.org,
hoegsberg@chromium.org, freedreno@lists.freedesktop.org
Subject: Re: [v2] arm64: dts: sc7180: add dsi controller and phy entries for idp dts
Date: Wed, 6 May 2020 12:25:47 -0700 [thread overview]
Message-ID: <20200506192547.GY4525@google.com> (raw)
In-Reply-To: <20200214184937.GA15781@google.com>
On Fri, Feb 14, 2020 at 10:49:37AM -0800, Matthias Kaehlcke wrote:
> On Tue, Feb 11, 2020 at 05:07:35PM +0530, Harigovindan P wrote:
>
> > subject: arm64: dts: sc7180: add dsi controller and phy entries for idp dts
>
> nit: 'dts' at the end is redundant, the prefixes make it clear that this
> is about DT entries.
>
> Also the message isn't really concise. The main entries for the DSI
> controller and the PHY are in sc7180.dtsi. I would suggest to drop
> any mentions of DSI controller and PHYs, and just say something like
> 'Add nodes for IDP display'. In the body you could mention that the
> display is the Visionox RM69299.
>
> > Adding dsi controller and phy entries for idp dt.
> >
> > Signed-off-by: Harigovindan P <harigovi@codeaurora.org>
> > ---
> >
> > Changes in v1:
> > - Added dsi controller and dsi phy entries for idp dts
>
> Changes in v1 is pointless, it's the first patch
>
> > Changes in v2:
> > - Adding dependency patchwork series
> > - Removing suspend configuration
> > - Adding blank before curly brace
> >
> > This patch depends on following patchwork series:
> >
> > https://patchwork.kernel.org/patch/11364687/
> > https://patchwork.kernel.org/patch/11366303/
> >
> > arch/arm64/boot/dts/qcom/sc7180-idp.dts | 55 +++++++++++++++++++++++++
> > 1 file changed, 55 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > index 388f50ad4fde..6ccf8c3603ab 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > @@ -7,6 +7,7 @@
> >
> > /dts-v1/;
> >
> > +#include <dt-bindings/gpio/gpio.h>
> > #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> > #include "sc7180.dtsi"
> > #include "pm6150.dtsi"
> > @@ -232,6 +233,49 @@ vreg_bob: bob {
> > };
> > };
> >
> > +&dsi0 {
> > + status = "okay";
> > +
> > + vdda-supply = <&vreg_l3c_1p2>;
> > +
> > + panel@0 {
> > + compatible = "visionox,rm69299-1080p-display";
> > + reg = <0>;
> > +
> > + vdda-supply = <&vreg_l8c_1p8>;
> > + vdd3p3-supply = <&vreg_l18a_2p8>;
> > +
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&disp_pins>;
> > +
> > + reset-gpios = <&pm6150l_gpio 3 GPIO_ACTIVE_HIGH>;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + port@0 {
> > + reg = <0>;
> > + panel0_in: endpoint {
> > + remote-endpoint = <&dsi0_out>;
> > + };
> > + };
> > + };
> > + };
> > +
> > + ports {
> > + port@1 {
> > + endpoint {
> > + remote-endpoint = <&panel0_in>;
> > + data-lanes = <0 1 2 3>;
> > + };
> > + };
> > + };
> > +};
> > +
> > +&dsi_phy {
> > + status = "okay";
> > +};
> > +
> > &qspi {
> > status = "okay";
> > pinctrl-names = "default";
> > @@ -289,6 +333,17 @@ &usb_1_qmpphy {
> >
> > /* PINCTRL - additions to nodes defined in sc7180.dtsi */
> >
> > +&pm6150l_gpio {
> > + disp_pins: disp-pins {
> > + pins = "gpio3";
> > + function = "func1";
> > + qcom,drive-strength = <2>;
> > + power-source = <0>;
> > + bias-disable;
> > + output-low;
> > + };
> > +};
> > +
> > &qspi_clk {
> > pinconf {
> > pins = "gpio63";
>
> To get the display actually to work you also need this:
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> index 88919da1510b03..fdbcb56dfa81f9 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> @@ -276,6 +276,14 @@
> status = "okay";
> };
>
> +&mdp {
> + status = "okay";
> +};
> +
> +&mdss {
> + status = "okay";
> +};
> +
> &qspi {
> status = "okay";
> pinctrl-names = "default";
>
> Maybe just add this to this patch?
ping
the display driver landed in drm-misc, but this patch still needs a
respin.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-05-06 19:25 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-11 11:37 [v2] arm64: dts: sc7180: add dsi controller and phy entries for idp dts Harigovindan P
2020-02-11 11:37 ` Harigovindan P
2020-02-14 18:49 ` Matthias Kaehlcke
2020-02-14 18:49 ` Matthias Kaehlcke
2020-05-06 19:25 ` Matthias Kaehlcke [this message]
2020-05-06 19:25 ` Matthias Kaehlcke
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200506192547.GY4525@google.com \
--to=mka@chromium.org \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=harigovi@codeaurora.org \
--cc=hoegsberg@chromium.org \
--cc=kalyan_t@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nganji@codeaurora.org \
--cc=robdclark@gmail.com \
--cc=seanpaul@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.