mbox series

[v2,00/12] Enable Display for SM8350

Message ID 20221115133105.980877-1-robert.foss@linaro.org
Headers show
Series Enable Display for SM8350 | expand

Message

Robert Foss Nov. 15, 2022, 1:30 p.m. UTC
Dependencies:
https://lore.kernel.org/all/20221102231309.583587-1-dmitry.baryshkov@linaro.org/
https://lore.kernel.org/all/20221024164225.3236654-1-dmitry.baryshkov@linaro.org/
https://lore.kernel.org/all/20221104130324.1024242-5-dmitry.baryshkov@linaro.org/

Branch:
https://git.linaro.org/people/robert.foss/linux.git/log/?h=sm8350_dsi_v2

This series implements display support for SM8350 and
enables HDMI output for the SM8350-HDK platform.


Changes from v1:
 - Added R-b tags from v1
 - Added qcom,sm8350-dpu binding patch
 - Added qcom,sm8350-mdss binding patch
 - Corrected sm8350.dtsi according to new dpu/mdss bindings
 - Bjorn: Removed regulator-always-on property from lt9611_1v2 regulator
 - Bjorn: Moved lt9611 pinctl pins into a common node
 - Bjorn/Krzysztof: Moved status property to last in node
 - Krzysztof: Changed hdmi-out to hdmi-connector
 - Krzysztof: Fixed regulator node name
 - Krzysztof: Changed &mdss to status=disabled as default
 - Krzysztof: Changed &mdss_mdp node name to display-controller
 - Krzysztof: Fixed opp-table node name
 - Krzysztof: Fixed phy node name
 - Dmitry: Split commit containing dpu & mdss compatibles string
 - Dmitry: Added msm_mdss_enable case
 - Dmitry: Fixed dpu ctl features
 


Robert Foss (12):
  dt-bindings: display: msm: Add qcom,sm8350-dpu binding
  dt-bindings: display: msm: Add qcom,sm8350-mdss binding
  drm/msm/dpu: Refactor sc7280_pp location
  drm/msm/dpu: Add SM8350 to hw catalog
  drm/msm/dpu: Add support for SM8350
  drm/msm: Add support for SM8350
  arm64: dts: qcom: sm8350: Add &tlmm gpio-line-names
  arm64: dts: qcom: sm8350: Remove mmxc power-domain-name
  arm64: dts: qcom: sm8350: Use 2 interconnect cells
  arm64: dts: qcom: sm8350: Add display system nodes
  arm64: dts: qcom: sm8350-hdk: Enable display & dsi nodes
  arm64: dts: qcom: sm8350-hdk: Enable lt9611uxc dsi-hdmi bridge

 .../bindings/display/msm/qcom,sm8350-dpu.yaml | 120 +++++++
 .../display/msm/qcom,sm8350-mdss.yaml         | 240 +++++++++++++
 arch/arm64/boot/dts/qcom/sm8350-hdk.dts       | 332 ++++++++++++++++++
 arch/arm64/boot/dts/qcom/sm8350.dtsi          | 226 +++++++++++-
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    | 210 ++++++++++-
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |   1 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       |   1 +
 drivers/gpu/drm/msm/msm_mdss.c                |   4 +
 8 files changed, 1108 insertions(+), 26 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/msm/qcom,sm8350-dpu.yaml
 create mode 100644 Documentation/devicetree/bindings/display/msm/qcom,sm8350-mdss.yaml

Comments

Konrad Dybcio Nov. 15, 2022, 1:33 p.m. UTC | #1
On 15/11/2022 14:30, Robert Foss wrote:
> The sc7280_pp declaration is not located by the other _pp
> declarations, but rather hidden around the _merge_3d
> declarations. Let's fix this to avoid confusion.
> 
> Signed-off-by: Robert Foss <robert.foss@linaro.org>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
This is already merged.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=1a5b5372e3b0a4cc65a0cbb724b1b0859f4ac63c

Konrad
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> index 4dac90ee5b8a..8f2d634f7b6b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -1294,6 +1294,13 @@ static const struct dpu_pingpong_cfg sm8150_pp[] = {
>   			-1),
>   };
>   
> +static const struct dpu_pingpong_cfg sc7280_pp[] = {
> +	PP_BLK("pingpong_0", PINGPONG_0, 0x59000, 0, sc7280_pp_sblk, -1, -1),
> +	PP_BLK("pingpong_1", PINGPONG_1, 0x6a000, 0, sc7280_pp_sblk, -1, -1),
> +	PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, 0, sc7280_pp_sblk, -1, -1),
> +	PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1),
> +};
> +
>   static struct dpu_pingpong_cfg qcm2290_pp[] = {
>   	PP_BLK("pingpong_0", PINGPONG_0, 0x70000, 0, sdm845_pp_sblk,
>   		DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> @@ -1352,13 +1359,6 @@ static const struct dpu_merge_3d_cfg sm8450_merge_3d[] = {
>   	MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x65f00),
>   };
>   
> -static const struct dpu_pingpong_cfg sc7280_pp[] = {
> -	PP_BLK("pingpong_0", PINGPONG_0, 0x59000, 0, sc7280_pp_sblk, -1, -1),
> -	PP_BLK("pingpong_1", PINGPONG_1, 0x6a000, 0, sc7280_pp_sblk, -1, -1),
> -	PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, 0, sc7280_pp_sblk, -1, -1),
> -	PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1),
> -};
> -
>   /*************************************************************
>    * DSC sub blocks config
>    *************************************************************/
Konrad Dybcio Nov. 15, 2022, 1:47 p.m. UTC | #2
On 15/11/2022 14:31, Robert Foss wrote:
> Add mdss, mdss_mdp, dsi0, dsi0_phy nodes. With these
> nodes the display subsystem is configured to support
> one DSI output.
> 
> Signed-off-by: Robert Foss <robert.foss@linaro.org>
> ---
>   arch/arm64/boot/dts/qcom/sm8350.dtsi | 197 ++++++++++++++++++++++++++-
>   1 file changed, 193 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> index 434f8e8b12c1..5c98e5cf5ad0 100644
> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> @@ -3,6 +3,7 @@
>    * Copyright (c) 2020, Linaro Limited
>    */
>   
> +#include <dt-bindings/interconnect/qcom,sm8350.h>
>   #include <dt-bindings/interrupt-controller/arm-gic.h>
>   #include <dt-bindings/clock/qcom,dispcc-sm8350.h>
>   #include <dt-bindings/clock/qcom,gcc-sm8350.h>
> @@ -2536,14 +2537,201 @@ usb_2_dwc3: usb@a800000 {
>   			};
>   		};
>   
> +		mdss: mdss@ae00000 {
> +			compatible = "qcom,sm8350-mdss";
> +			reg = <0 0x0ae00000 0 0x1000>;
> +			reg-names = "mdss";
> +
> +			interconnects = <&mmss_noc MASTER_MDP0 0 &mc_virt SLAVE_EBI1 0>,
> +					<&mmss_noc MASTER_MDP1 0 &mc_virt SLAVE_EBI1 0>;
> +			interconnect-names = "mdp0-mem", "mdp1-mem";
> +
> +			power-domains = <&dispcc MDSS_GDSC>;
> +			resets = <&dispcc DISP_CC_MDSS_CORE_BCR>;
> +
> +			clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +				 <&gcc GCC_DISP_HF_AXI_CLK>,
> +				 <&gcc GCC_DISP_SF_AXI_CLK>,
> +				 <&dispcc DISP_CC_MDSS_MDP_CLK>;
> +			clock-names = "iface", "bus", "nrt_bus", "core";
> +
> +			interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-controller;
> +			#interrupt-cells = <1>;
> +
> +			iommus = <&apps_smmu 0x820 0x402>;
> +
> +			status = "disabled";
> +
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> +
> +			mdss_mdp: display-controller@ae01000 {
> +				compatible = "qcom,sm8350-dpu";
> +				reg = <0 0x0ae01000 0 0x8f000>,
> +				      <0 0x0aeb0000 0 0x2008>;
> +				reg-names = "mdp", "vbif";
> +
> +				clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
> +					<&gcc GCC_DISP_SF_AXI_CLK>,
> +					<&dispcc DISP_CC_MDSS_AHB_CLK>,
> +					<&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
> +					<&dispcc DISP_CC_MDSS_MDP_CLK>,
> +					<&dispcc DISP_CC_MDSS_VSYNC_CLK>;
> +				clock-names = "bus",
> +					      "nrt_bus",
> +					      "iface",
> +					      "lut",
> +					      "core",
> +					      "vsync";
> +
> +				assigned-clocks = <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
> +				assigned-clock-rates = <19200000>;
> +
> +				operating-points-v2 = <&mdp_opp_table>;
> +				power-domains = <&rpmhpd SM8350_MMCX>;
> +
> +				interrupt-parent = <&mdss>;
> +				interrupts = <0>;
> +
> +				status = "disabled";
It doesn't make sense to disable mdp separately, as mdss is essentially 
useless without it.

> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +						dpu_intf1_out: endpoint {
> +							remote-endpoint = <&dsi0_in>;
> +						};
> +					};
> +				};
> +
> +				mdp_opp_table: opp-table {
> +					compatible = "operating-points-v2";
> +
> +					opp-200000000 {
> +						opp-hz = /bits/ 64 <200000000>;
> +						required-opps = <&rpmhpd_opp_low_svs>;
> +					};
> +
> +					opp-300000000 {
> +						opp-hz = /bits/ 64 <300000000>;
> +						required-opps = <&rpmhpd_opp_svs>;
> +					};
> +
> +					opp-345000000 {
> +						opp-hz = /bits/ 64 <345000000>;
> +						required-opps = <&rpmhpd_opp_svs_l1>;
> +					};
> +
> +					opp-460000000 {
> +						opp-hz = /bits/ 64 <460000000>;
> +						required-opps = <&rpmhpd_opp_nom>;
> +					};
> +				};
> +			};
> +
> +			dsi0: dsi@ae94000 {
> +				compatible = "qcom,mdss-dsi-ctrl";
> +				reg = <0 0x0ae94000 0 0x400>;
> +				reg-names = "dsi_ctrl";
> +
> +				interrupt-parent = <&mdss>;
> +				interrupts = <4>;
> +
> +				clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
> +					 <&dispcc DISP_CC_MDSS_BYTE0_INTF_CLK>,
> +					 <&dispcc DISP_CC_MDSS_PCLK0_CLK>,
> +					 <&dispcc DISP_CC_MDSS_ESC0_CLK>,
> +					 <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +					 <&gcc GCC_DISP_HF_AXI_CLK>;
> +				clock-names = "byte",
> +					      "byte_intf",
> +					      "pixel",
> +					      "core",
> +					      "iface",
> +					      "bus";
> +
> +				assigned-clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK_SRC>,
> +						  <&dispcc DISP_CC_MDSS_PCLK0_CLK_SRC>;
> +				assigned-clock-parents = <&dsi0_phy 0>,
> +							 <&dsi0_phy 1>;
> +
> +				operating-points-v2 = <&dsi_opp_table>;
> +				power-domains = <&rpmhpd SM8350_MMCX>;
> +
> +				phys = <&dsi0_phy>;
> +				phy-names = "dsi";
I think that was dropped as of late.

> +
> +				status = "disabled";
> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +						dsi0_in: endpoint {
> +							remote-endpoint = <&dpu_intf1_out>;
> +						};
> +					};
> +
> +					port@1 {
> +						reg = <1>;
> +						dsi0_out: endpoint {
> +						};
> +					};
> +				};
> +			};
> +
> +			dsi0_phy: phy@ae94400 {
> +				compatible = "qcom,dsi-phy-5nm-8350";
> +				reg = <0 0x0ae94400 0 0x200>,
> +				      <0 0x0ae94600 0 0x280>,
> +				      <0 0x0ae94900 0 0x260>;
> +				reg-names = "dsi_phy",
> +					    "dsi_phy_lane",
> +					    "dsi_pll";
> +
> +				#clock-cells = <1>;
> +				#phy-cells = <0>;
> +
> +				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +					 <&rpmhcc RPMH_CXO_CLK>;
> +				clock-names = "iface", "ref";
> +
> +				status = "disabled";
> +
> +				dsi_opp_table: dsi-opp-table {
> +					compatible = "operating-points-v2";
> +
> +					opp-187500000 {
> +						opp-hz = /bits/ 64 <187500000>;
> +						required-opps = <&rpmhpd_opp_low_svs>;
> +					};
> +
> +					opp-300000000 {
> +						opp-hz = /bits/ 64 <300000000>;
> +						required-opps = <&rpmhpd_opp_svs>;
> +					};
> +
> +					opp-358000000 {
> +						opp-hz = /bits/ 64 <358000000>;
> +						required-opps = <&rpmhpd_opp_svs_l1>;
> +					};
> +				};
> +			};
> +		};
> +
>   		dispcc: clock-controller@af00000 {
>   			compatible = "qcom,sm8350-dispcc";
>   			reg = <0 0x0af00000 0 0x10000>;
>   			clocks = <&rpmhcc RPMH_CXO_CLK>,
> -				 <0>,
> -				 <0>,
> -				 <0>,
> -				 <0>,
> +				 <&dsi0_phy 0>, <&dsi0_phy 1>,
> +				 <0>, <0>,
>   				 <0>,
>   				 <0>;
>   			clock-names = "bi_tcxo",
> @@ -2558,6 +2746,7 @@ dispcc: clock-controller@af00000 {
>   			#power-domain-cells = <1>;
>   
>   			power-domains = <&rpmhpd SM8350_MMCX>;
> +			required-opps = <&rpmhpd_opp_turbo>;
A turbo vote is required for it to function? Seems a bit high..

Konrad
>   		};
>   
>   		adsp: remoteproc@17300000 {
Krzysztof Kozlowski Nov. 15, 2022, 3:23 p.m. UTC | #3
On 15/11/2022 14:30, Robert Foss wrote:
> Dependencies:
> https://lore.kernel.org/all/20221102231309.583587-1-dmitry.baryshkov@linaro.org/
> https://lore.kernel.org/all/20221024164225.3236654-1-dmitry.baryshkov@linaro.org/
> https://lore.kernel.org/all/20221104130324.1024242-5-dmitry.baryshkov@linaro.org/
> 
> Branch:
> https://git.linaro.org/people/robert.foss/linux.git/log/?h=sm8350_dsi_v2
> 
> This series implements display support for SM8350 and
> enables HDMI output for the SM8350-HDK platform.
> 

I received two of these patchsets... Which one is valid? Folks also
review in both...

Best regards,
Krzysztof
Rob Herring (Arm) Nov. 16, 2022, 2:59 p.m. UTC | #4
On Tue, 15 Nov 2022 14:30:54 +0100, Robert Foss wrote:
> Mobile Display Subsystem (MDSS) encapsulates sub-blocks
> like DPU display controller, DSI etc. Add YAML schema for DPU device
> tree bindings
> 
> Signed-off-by: Robert Foss <robert.foss@linaro.org>
> ---
>  .../bindings/display/msm/qcom,sm8350-dpu.yaml | 120 ++++++++++++++++++
>  1 file changed, 120 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/msm/qcom,sm8350-dpu.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>
Abhinav Kumar Nov. 25, 2022, 6:05 a.m. UTC | #5
On 11/15/2022 5:33 AM, Konrad Dybcio wrote:
> 
> 
> On 15/11/2022 14:30, Robert Foss wrote:
>> The sc7280_pp declaration is not located by the other _pp
>> declarations, but rather hidden around the _merge_3d
>> declarations. Let's fix this to avoid confusion.
>>
>> Signed-off-by: Robert Foss <robert.foss@linaro.org>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
> This is already merged.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=1a5b5372e3b0a4cc65a0cbb724b1b0859f4ac63c 
> 
> 
> Konrad

Its part of linux-next but a PR hasnt been sent with it.

That being said, since this particular change has been taken separately, 
this series should now be rebased without this change and addressing 
some of the other comments given by konrad.

>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 14 +++++++-------
>>   1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> index 4dac90ee5b8a..8f2d634f7b6b 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> @@ -1294,6 +1294,13 @@ static const struct dpu_pingpong_cfg 
>> sm8150_pp[] = {
>>               -1),
>>   };
>> +static const struct dpu_pingpong_cfg sc7280_pp[] = {
>> +    PP_BLK("pingpong_0", PINGPONG_0, 0x59000, 0, sc7280_pp_sblk, -1, 
>> -1),
>> +    PP_BLK("pingpong_1", PINGPONG_1, 0x6a000, 0, sc7280_pp_sblk, -1, 
>> -1),
>> +    PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, 0, sc7280_pp_sblk, -1, 
>> -1),
>> +    PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, 
>> -1),
>> +};
>> +
>>   static struct dpu_pingpong_cfg qcm2290_pp[] = {
>>       PP_BLK("pingpong_0", PINGPONG_0, 0x70000, 0, sdm845_pp_sblk,
>>           DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
>> @@ -1352,13 +1359,6 @@ static const struct dpu_merge_3d_cfg 
>> sm8450_merge_3d[] = {
>>       MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x65f00),
>>   };
>> -static const struct dpu_pingpong_cfg sc7280_pp[] = {
>> -    PP_BLK("pingpong_0", PINGPONG_0, 0x59000, 0, sc7280_pp_sblk, -1, 
>> -1),
>> -    PP_BLK("pingpong_1", PINGPONG_1, 0x6a000, 0, sc7280_pp_sblk, -1, 
>> -1),
>> -    PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, 0, sc7280_pp_sblk, -1, 
>> -1),
>> -    PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, 
>> -1),
>> -};
>> -
>>   /*************************************************************
>>    * DSC sub blocks config
>>    *************************************************************/
Robert Foss Nov. 29, 2022, 4:47 p.m. UTC | #6
On Tue, 15 Nov 2022 at 14:47, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 15/11/2022 14:31, Robert Foss wrote:
> > Add mdss, mdss_mdp, dsi0, dsi0_phy nodes. With these
> > nodes the display subsystem is configured to support
> > one DSI output.
> >
> > Signed-off-by: Robert Foss <robert.foss@linaro.org>
> > ---
> >   arch/arm64/boot/dts/qcom/sm8350.dtsi | 197 ++++++++++++++++++++++++++-
> >   1 file changed, 193 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> > index 434f8e8b12c1..5c98e5cf5ad0 100644
> > --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> > @@ -3,6 +3,7 @@
> >    * Copyright (c) 2020, Linaro Limited
> >    */
> >
> > +#include <dt-bindings/interconnect/qcom,sm8350.h>
> >   #include <dt-bindings/interrupt-controller/arm-gic.h>
> >   #include <dt-bindings/clock/qcom,dispcc-sm8350.h>
> >   #include <dt-bindings/clock/qcom,gcc-sm8350.h>
> > @@ -2536,14 +2537,201 @@ usb_2_dwc3: usb@a800000 {
> >                       };
> >               };
> >
> > +             mdss: mdss@ae00000 {
> > +                     compatible = "qcom,sm8350-mdss";
> > +                     reg = <0 0x0ae00000 0 0x1000>;
> > +                     reg-names = "mdss";
> > +
> > +                     interconnects = <&mmss_noc MASTER_MDP0 0 &mc_virt SLAVE_EBI1 0>,
> > +                                     <&mmss_noc MASTER_MDP1 0 &mc_virt SLAVE_EBI1 0>;
> > +                     interconnect-names = "mdp0-mem", "mdp1-mem";
> > +
> > +                     power-domains = <&dispcc MDSS_GDSC>;
> > +                     resets = <&dispcc DISP_CC_MDSS_CORE_BCR>;
> > +
> > +                     clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
> > +                              <&gcc GCC_DISP_HF_AXI_CLK>,
> > +                              <&gcc GCC_DISP_SF_AXI_CLK>,
> > +                              <&dispcc DISP_CC_MDSS_MDP_CLK>;
> > +                     clock-names = "iface", "bus", "nrt_bus", "core";
> > +
> > +                     interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
> > +                     interrupt-controller;
> > +                     #interrupt-cells = <1>;
> > +
> > +                     iommus = <&apps_smmu 0x820 0x402>;
> > +
> > +                     status = "disabled";
> > +
> > +                     #address-cells = <2>;
> > +                     #size-cells = <2>;
> > +                     ranges;
> > +
> > +                     mdss_mdp: display-controller@ae01000 {
> > +                             compatible = "qcom,sm8350-dpu";
> > +                             reg = <0 0x0ae01000 0 0x8f000>,
> > +                                   <0 0x0aeb0000 0 0x2008>;
> > +                             reg-names = "mdp", "vbif";
> > +
> > +                             clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
> > +                                     <&gcc GCC_DISP_SF_AXI_CLK>,
> > +                                     <&dispcc DISP_CC_MDSS_AHB_CLK>,
> > +                                     <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
> > +                                     <&dispcc DISP_CC_MDSS_MDP_CLK>,
> > +                                     <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
> > +                             clock-names = "bus",
> > +                                           "nrt_bus",
> > +                                           "iface",
> > +                                           "lut",
> > +                                           "core",
> > +                                           "vsync";
> > +
> > +                             assigned-clocks = <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
> > +                             assigned-clock-rates = <19200000>;
> > +
> > +                             operating-points-v2 = <&mdp_opp_table>;
> > +                             power-domains = <&rpmhpd SM8350_MMCX>;
> > +
> > +                             interrupt-parent = <&mdss>;
> > +                             interrupts = <0>;
> > +
> > +                             status = "disabled";
> It doesn't make sense to disable mdp separately, as mdss is essentially
> useless without it.

Ack

>
> > +
> > +                             ports {
> > +                                     #address-cells = <1>;
> > +                                     #size-cells = <0>;
> > +
> > +                                     port@0 {
> > +                                             reg = <0>;
> > +                                             dpu_intf1_out: endpoint {
> > +                                                     remote-endpoint = <&dsi0_in>;
> > +                                             };
> > +                                     };
> > +                             };
> > +
> > +                             mdp_opp_table: opp-table {
> > +                                     compatible = "operating-points-v2";
> > +
> > +                                     opp-200000000 {
> > +                                             opp-hz = /bits/ 64 <200000000>;
> > +                                             required-opps = <&rpmhpd_opp_low_svs>;
> > +                                     };
> > +
> > +                                     opp-300000000 {
> > +                                             opp-hz = /bits/ 64 <300000000>;
> > +                                             required-opps = <&rpmhpd_opp_svs>;
> > +                                     };
> > +
> > +                                     opp-345000000 {
> > +                                             opp-hz = /bits/ 64 <345000000>;
> > +                                             required-opps = <&rpmhpd_opp_svs_l1>;
> > +                                     };
> > +
> > +                                     opp-460000000 {
> > +                                             opp-hz = /bits/ 64 <460000000>;
> > +                                             required-opps = <&rpmhpd_opp_nom>;
> > +                                     };
> > +                             };
> > +                     };
> > +
> > +                     dsi0: dsi@ae94000 {
> > +                             compatible = "qcom,mdss-dsi-ctrl";
> > +                             reg = <0 0x0ae94000 0 0x400>;
> > +                             reg-names = "dsi_ctrl";
> > +
> > +                             interrupt-parent = <&mdss>;
> > +                             interrupts = <4>;
> > +
> > +                             clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
> > +                                      <&dispcc DISP_CC_MDSS_BYTE0_INTF_CLK>,
> > +                                      <&dispcc DISP_CC_MDSS_PCLK0_CLK>,
> > +                                      <&dispcc DISP_CC_MDSS_ESC0_CLK>,
> > +                                      <&dispcc DISP_CC_MDSS_AHB_CLK>,
> > +                                      <&gcc GCC_DISP_HF_AXI_CLK>;
> > +                             clock-names = "byte",
> > +                                           "byte_intf",
> > +                                           "pixel",
> > +                                           "core",
> > +                                           "iface",
> > +                                           "bus";
> > +
> > +                             assigned-clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK_SRC>,
> > +                                               <&dispcc DISP_CC_MDSS_PCLK0_CLK_SRC>;
> > +                             assigned-clock-parents = <&dsi0_phy 0>,
> > +                                                      <&dsi0_phy 1>;
> > +
> > +                             operating-points-v2 = <&dsi_opp_table>;
> > +                             power-domains = <&rpmhpd SM8350_MMCX>;
> > +
> > +                             phys = <&dsi0_phy>;
> > +                             phy-names = "dsi";
> I think that was dropped as of late.

Ack

>
> > +
> > +                             status = "disabled";
> > +
> > +                             ports {
> > +                                     #address-cells = <1>;
> > +                                     #size-cells = <0>;
> > +
> > +                                     port@0 {
> > +                                             reg = <0>;
> > +                                             dsi0_in: endpoint {
> > +                                                     remote-endpoint = <&dpu_intf1_out>;
> > +                                             };
> > +                                     };
> > +
> > +                                     port@1 {
> > +                                             reg = <1>;
> > +                                             dsi0_out: endpoint {
> > +                                             };
> > +                                     };
> > +                             };
> > +                     };
> > +
> > +                     dsi0_phy: phy@ae94400 {
> > +                             compatible = "qcom,dsi-phy-5nm-8350";
> > +                             reg = <0 0x0ae94400 0 0x200>,
> > +                                   <0 0x0ae94600 0 0x280>,
> > +                                   <0 0x0ae94900 0 0x260>;
> > +                             reg-names = "dsi_phy",
> > +                                         "dsi_phy_lane",
> > +                                         "dsi_pll";
> > +
> > +                             #clock-cells = <1>;
> > +                             #phy-cells = <0>;
> > +
> > +                             clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
> > +                                      <&rpmhcc RPMH_CXO_CLK>;
> > +                             clock-names = "iface", "ref";
> > +
> > +                             status = "disabled";
> > +
> > +                             dsi_opp_table: dsi-opp-table {
> > +                                     compatible = "operating-points-v2";
> > +
> > +                                     opp-187500000 {
> > +                                             opp-hz = /bits/ 64 <187500000>;
> > +                                             required-opps = <&rpmhpd_opp_low_svs>;
> > +                                     };
> > +
> > +                                     opp-300000000 {
> > +                                             opp-hz = /bits/ 64 <300000000>;
> > +                                             required-opps = <&rpmhpd_opp_svs>;
> > +                                     };
> > +
> > +                                     opp-358000000 {
> > +                                             opp-hz = /bits/ 64 <358000000>;
> > +                                             required-opps = <&rpmhpd_opp_svs_l1>;
> > +                                     };
> > +                             };
> > +                     };
> > +             };
> > +
> >               dispcc: clock-controller@af00000 {
> >                       compatible = "qcom,sm8350-dispcc";
> >                       reg = <0 0x0af00000 0 0x10000>;
> >                       clocks = <&rpmhcc RPMH_CXO_CLK>,
> > -                              <0>,
> > -                              <0>,
> > -                              <0>,
> > -                              <0>,
> > +                              <&dsi0_phy 0>, <&dsi0_phy 1>,
> > +                              <0>, <0>,
> >                                <0>,
> >                                <0>;
> >                       clock-names = "bi_tcxo",
> > @@ -2558,6 +2746,7 @@ dispcc: clock-controller@af00000 {
> >                       #power-domain-cells = <1>;
> >
> >                       power-domains = <&rpmhpd SM8350_MMCX>;
> > +                     required-opps = <&rpmhpd_opp_turbo>;
> A turbo vote is required for it to function? Seems a bit high..

Dmitry hit a snag using &rpmhpd_opp_low_svs, so this was a dummy
value. I can't replicate that issue, but am having a conversation with
him off-list about this.

On my sm8350-hdk board &rpmhpd_opp_low_svs is working fine.

>
> Konrad
> >               };
> >
> >               adsp: remoteproc@17300000 {
Dmitry Baryshkov Nov. 29, 2022, 7:59 p.m. UTC | #7
On 29/11/2022 18:47, Robert Foss wrote:
> On Tue, 15 Nov 2022 at 14:47, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>>
>>
>> On 15/11/2022 14:31, Robert Foss wrote:
>>> Add mdss, mdss_mdp, dsi0, dsi0_phy nodes. With these
>>> nodes the display subsystem is configured to support
>>> one DSI output.
>>>
>>> Signed-off-by: Robert Foss <robert.foss@linaro.org>
>>> ---
>>>    arch/arm64/boot/dts/qcom/sm8350.dtsi | 197 ++++++++++++++++++++++++++-
>>>    1 file changed, 193 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
>>> index 434f8e8b12c1..5c98e5cf5ad0 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
>>> @@ -3,6 +3,7 @@
>>>     * Copyright (c) 2020, Linaro Limited
>>>     */
>>>
>>> +#include <dt-bindings/interconnect/qcom,sm8350.h>
>>>    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>    #include <dt-bindings/clock/qcom,dispcc-sm8350.h>
>>>    #include <dt-bindings/clock/qcom,gcc-sm8350.h>
>>> @@ -2536,14 +2537,201 @@ usb_2_dwc3: usb@a800000 {
>>>                        };
>>>                };
>>>
>>> +             mdss: mdss@ae00000 {
>>> +                     compatible = "qcom,sm8350-mdss";
>>> +                     reg = <0 0x0ae00000 0 0x1000>;
>>> +                     reg-names = "mdss";
>>> +
>>> +                     interconnects = <&mmss_noc MASTER_MDP0 0 &mc_virt SLAVE_EBI1 0>,
>>> +                                     <&mmss_noc MASTER_MDP1 0 &mc_virt SLAVE_EBI1 0>;
>>> +                     interconnect-names = "mdp0-mem", "mdp1-mem";
>>> +
>>> +                     power-domains = <&dispcc MDSS_GDSC>;
>>> +                     resets = <&dispcc DISP_CC_MDSS_CORE_BCR>;
>>> +
>>> +                     clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
>>> +                              <&gcc GCC_DISP_HF_AXI_CLK>,
>>> +                              <&gcc GCC_DISP_SF_AXI_CLK>,
>>> +                              <&dispcc DISP_CC_MDSS_MDP_CLK>;
>>> +                     clock-names = "iface", "bus", "nrt_bus", "core";
>>> +
>>> +                     interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
>>> +                     interrupt-controller;
>>> +                     #interrupt-cells = <1>;
>>> +
>>> +                     iommus = <&apps_smmu 0x820 0x402>;
>>> +
>>> +                     status = "disabled";
>>> +
>>> +                     #address-cells = <2>;
>>> +                     #size-cells = <2>;
>>> +                     ranges;
>>> +
>>> +                     mdss_mdp: display-controller@ae01000 {
>>> +                             compatible = "qcom,sm8350-dpu";
>>> +                             reg = <0 0x0ae01000 0 0x8f000>,
>>> +                                   <0 0x0aeb0000 0 0x2008>;
>>> +                             reg-names = "mdp", "vbif";
>>> +
>>> +                             clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
>>> +                                     <&gcc GCC_DISP_SF_AXI_CLK>,
>>> +                                     <&dispcc DISP_CC_MDSS_AHB_CLK>,
>>> +                                     <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
>>> +                                     <&dispcc DISP_CC_MDSS_MDP_CLK>,
>>> +                                     <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
>>> +                             clock-names = "bus",
>>> +                                           "nrt_bus",
>>> +                                           "iface",
>>> +                                           "lut",
>>> +                                           "core",
>>> +                                           "vsync";
>>> +
>>> +                             assigned-clocks = <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
>>> +                             assigned-clock-rates = <19200000>;
>>> +
>>> +                             operating-points-v2 = <&mdp_opp_table>;
>>> +                             power-domains = <&rpmhpd SM8350_MMCX>;
>>> +
>>> +                             interrupt-parent = <&mdss>;
>>> +                             interrupts = <0>;
>>> +
>>> +                             status = "disabled";
>> It doesn't make sense to disable mdp separately, as mdss is essentially
>> useless without it.
> 
> Ack
> 
>>
>>> +
>>> +                             ports {
>>> +                                     #address-cells = <1>;
>>> +                                     #size-cells = <0>;
>>> +
>>> +                                     port@0 {
>>> +                                             reg = <0>;
>>> +                                             dpu_intf1_out: endpoint {
>>> +                                                     remote-endpoint = <&dsi0_in>;
>>> +                                             };
>>> +                                     };
>>> +                             };
>>> +
>>> +                             mdp_opp_table: opp-table {
>>> +                                     compatible = "operating-points-v2";
>>> +
>>> +                                     opp-200000000 {
>>> +                                             opp-hz = /bits/ 64 <200000000>;
>>> +                                             required-opps = <&rpmhpd_opp_low_svs>;
>>> +                                     };
>>> +
>>> +                                     opp-300000000 {
>>> +                                             opp-hz = /bits/ 64 <300000000>;
>>> +                                             required-opps = <&rpmhpd_opp_svs>;
>>> +                                     };
>>> +
>>> +                                     opp-345000000 {
>>> +                                             opp-hz = /bits/ 64 <345000000>;
>>> +                                             required-opps = <&rpmhpd_opp_svs_l1>;
>>> +                                     };
>>> +
>>> +                                     opp-460000000 {
>>> +                                             opp-hz = /bits/ 64 <460000000>;
>>> +                                             required-opps = <&rpmhpd_opp_nom>;
>>> +                                     };
>>> +                             };
>>> +                     };
>>> +
>>> +                     dsi0: dsi@ae94000 {
>>> +                             compatible = "qcom,mdss-dsi-ctrl";
>>> +                             reg = <0 0x0ae94000 0 0x400>;
>>> +                             reg-names = "dsi_ctrl";
>>> +
>>> +                             interrupt-parent = <&mdss>;
>>> +                             interrupts = <4>;
>>> +
>>> +                             clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
>>> +                                      <&dispcc DISP_CC_MDSS_BYTE0_INTF_CLK>,
>>> +                                      <&dispcc DISP_CC_MDSS_PCLK0_CLK>,
>>> +                                      <&dispcc DISP_CC_MDSS_ESC0_CLK>,
>>> +                                      <&dispcc DISP_CC_MDSS_AHB_CLK>,
>>> +                                      <&gcc GCC_DISP_HF_AXI_CLK>;
>>> +                             clock-names = "byte",
>>> +                                           "byte_intf",
>>> +                                           "pixel",
>>> +                                           "core",
>>> +                                           "iface",
>>> +                                           "bus";
>>> +
>>> +                             assigned-clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK_SRC>,
>>> +                                               <&dispcc DISP_CC_MDSS_PCLK0_CLK_SRC>;
>>> +                             assigned-clock-parents = <&dsi0_phy 0>,
>>> +                                                      <&dsi0_phy 1>;
>>> +
>>> +                             operating-points-v2 = <&dsi_opp_table>;
>>> +                             power-domains = <&rpmhpd SM8350_MMCX>;
>>> +
>>> +                             phys = <&dsi0_phy>;
>>> +                             phy-names = "dsi";
>> I think that was dropped as of late.
> 
> Ack
> 
>>
>>> +
>>> +                             status = "disabled";
>>> +
>>> +                             ports {
>>> +                                     #address-cells = <1>;
>>> +                                     #size-cells = <0>;
>>> +
>>> +                                     port@0 {
>>> +                                             reg = <0>;
>>> +                                             dsi0_in: endpoint {
>>> +                                                     remote-endpoint = <&dpu_intf1_out>;
>>> +                                             };
>>> +                                     };
>>> +
>>> +                                     port@1 {
>>> +                                             reg = <1>;
>>> +                                             dsi0_out: endpoint {
>>> +                                             };
>>> +                                     };
>>> +                             };
>>> +                     };
>>> +
>>> +                     dsi0_phy: phy@ae94400 {
>>> +                             compatible = "qcom,dsi-phy-5nm-8350";
>>> +                             reg = <0 0x0ae94400 0 0x200>,
>>> +                                   <0 0x0ae94600 0 0x280>,
>>> +                                   <0 0x0ae94900 0 0x260>;
>>> +                             reg-names = "dsi_phy",
>>> +                                         "dsi_phy_lane",
>>> +                                         "dsi_pll";
>>> +
>>> +                             #clock-cells = <1>;
>>> +                             #phy-cells = <0>;
>>> +
>>> +                             clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
>>> +                                      <&rpmhcc RPMH_CXO_CLK>;
>>> +                             clock-names = "iface", "ref";
>>> +
>>> +                             status = "disabled";
>>> +
>>> +                             dsi_opp_table: dsi-opp-table {
>>> +                                     compatible = "operating-points-v2";
>>> +
>>> +                                     opp-187500000 {
>>> +                                             opp-hz = /bits/ 64 <187500000>;
>>> +                                             required-opps = <&rpmhpd_opp_low_svs>;
>>> +                                     };
>>> +
>>> +                                     opp-300000000 {
>>> +                                             opp-hz = /bits/ 64 <300000000>;
>>> +                                             required-opps = <&rpmhpd_opp_svs>;
>>> +                                     };
>>> +
>>> +                                     opp-358000000 {
>>> +                                             opp-hz = /bits/ 64 <358000000>;
>>> +                                             required-opps = <&rpmhpd_opp_svs_l1>;
>>> +                                     };
>>> +                             };
>>> +                     };
>>> +             };
>>> +
>>>                dispcc: clock-controller@af00000 {
>>>                        compatible = "qcom,sm8350-dispcc";
>>>                        reg = <0 0x0af00000 0 0x10000>;
>>>                        clocks = <&rpmhcc RPMH_CXO_CLK>,
>>> -                              <0>,
>>> -                              <0>,
>>> -                              <0>,
>>> -                              <0>,
>>> +                              <&dsi0_phy 0>, <&dsi0_phy 1>,
>>> +                              <0>, <0>,
>>>                                 <0>,
>>>                                 <0>;
>>>                        clock-names = "bi_tcxo",
>>> @@ -2558,6 +2746,7 @@ dispcc: clock-controller@af00000 {
>>>                        #power-domain-cells = <1>;
>>>
>>>                        power-domains = <&rpmhpd SM8350_MMCX>;
>>> +                     required-opps = <&rpmhpd_opp_turbo>;
>> A turbo vote is required for it to function? Seems a bit high..
> 
> Dmitry hit a snag using &rpmhpd_opp_low_svs, so this was a dummy
> value. I can't replicate that issue, but am having a conversation with
> him off-list about this.
> 
> On my sm8350-hdk board &rpmhpd_opp_low_svs is working fine.

Maybe this is related to the bootloader setting up the mode or maybe it 
was caused by the fact that I have the drm_msm set up as built-in rather 
than a module.

> 
>>
>> Konrad
>>>                };
>>>
>>>                adsp: remoteproc@17300000 {