Re: [v4] arm64: dts: sc7180: add display dt nodes

[harigovi@codeaurora.org]

On 2020-01-29 02:02, Matthias Kaehlcke wrote:
> Hi,
> 
> On Tue, Jan 28, 2020 at 06:54:44PM +0530, Harigovindan P wrote:
>> Add display, DSI hardware DT nodes for sc7180.
>> 
>> Signed-off-by: Harigovindan P <harigovi@codeaurora.org>
>> ---
>> 
>> Changes in v1:
>> 	-Added display DT nodes for sc7180
>> Changes in v2:
>> 	-Renamed node names
>> 	-Corrected code alignments
>> 	-Removed extra new line
>> 	-Added DISP AHB clock for register access
>> 	under display_subsystem node for global settings
>> Changes in v3:
>> 	-Modified node names
>> 	-Modified hard coded values
>> 	-Removed mdss reg entry
>> Changes in v4:
>> 	-Reverting mdp node name
>> 	-Setting status to disabled in main SOC dtsi file
>> 	-Replacing _ to - for node names
>> 	-Adding clock dependency patch link
>> 	-Splitting idp dt file to a separate patch
>> 
>> This patch has dependency on the below series
>> https://lkml.org/lkml/2019/12/27/73
>>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 128 
>> +++++++++++++++++++++++++++++++++++
>>  1 file changed, 128 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi 
>> b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> index 3bc3f64..c3883af 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> @@ -1184,6 +1184,134 @@
>>  			#power-domain-cells = <1>;
>>  		};
>> 
>> +		mdss: mdss@ae00000 {
>> +			compatible = "qcom,sc7180-mdss";
>> +			reg = <0 0x0ae00000 0 0x1000>;
>> +			reg-names = "mdss";
>> +
>> +			power-domains = <&dispcc MDSS_GDSC>;
>> +
>> +			clocks = <&gcc GCC_DISP_AHB_CLK>,
>> +				 <&gcc GCC_DISP_HF_AXI_CLK>,
>> +				 <&dispcc DISP_CC_MDSS_AHB_CLK>,
>> +				 <&dispcc DISP_CC_MDSS_MDP_CLK>;
>> +			clock-names = "iface", "gcc_bus", "ahb", "core";
>> +
>> +			assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>;
>> +			assigned-clock-rates = <300000000>;
>> +
>> +			interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
>> +			interrupt-controller;
>> +			#interrupt-cells = <1>;
>> +
>> +			iommus = <&apps_smmu 0x800 0x2>;
>> +
>> +			#address-cells = <2>;
>> +			#size-cells = <2>;
>> +			ranges;
>> +
>> +			mdss_mdp: mdp@ae01000 {
>> +				compatible = "qcom,sc7180-dpu";
>> +				reg = <0 0x0ae01000 0 0x8f000>,
>> +				      <0 0x0aeb0000 0 0x2008>,
>> +				      <0 0x0af03000 0 0x16>;
>> +				reg-names = "mdp", "vbif", "disp_cc";
>> +
>> +				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
>> +					 <&dispcc DISP_CC_MDSS_ROT_CLK>,
>> +					 <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
>> +					 <&dispcc DISP_CC_MDSS_MDP_CLK>,
>> +					 <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
>> +				clock-names = "iface", "rot", "lut", "core",
>> +					      "vsync";
>> +				assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>,
>> +						  <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
>> +				assigned-clock-rates = <300000000>,
>> +						       <19200000>;
> 
> The clock rate for DISP_CC_MDSS_MDP_CLK is already specified in the
> parent node, do we really want/need to specify it twice?

Hi,

The parent device ( MDSS ) configures global HW settings which needs MDP 
CLK to be turned on with a default rate.
mdp device handles the composition, and it will compute the frequency 
needed as per the layer stack in the composition, hence we can have 
multiple rates on the CLK.