From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9A4C5C43217 for ; Tue, 22 Nov 2022 08:35:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232819AbiKVIfX (ORCPT ); Tue, 22 Nov 2022 03:35:23 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40698 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231319AbiKVIfV (ORCPT ); Tue, 22 Nov 2022 03:35:21 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 367744092E; Tue, 22 Nov 2022 00:35:20 -0800 (PST) Received: from [192.168.1.15] (91-154-32-225.elisa-laajakaista.fi [91.154.32.225]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 716612D9; Tue, 22 Nov 2022 09:35:17 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1669106118; bh=YlboxYqJIdlrRB8uXxg6m6iestgCL1+GiErR7mO0mFs=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=j8jGbyFXwpUXOcckyqpLUNJ1L68wSARaaiOrO/lF6xFvItrfAzyYe7m2X/Csj/oQb JG9iszc4aB+SQalK/L5TGWws+umdvdoJ7PAHD0nUyLSlYs6AnHKSEnyrJrEwlH9tUe yZpshEx8NCa/NWkh9etXXP8bu1HF/Witcbt81PjA= Message-ID: <3c012152-a031-65ef-1fe7-9d8e690947fa@ideasonboard.com> Date: Tue, 22 Nov 2022 10:35:15 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [PATCH v1 4/8] arm64: dts: renesas: r8a779g0: Add display related data Content-Language: en-US To: Kieran Bingham , Geert Uytterhoeven , Krzysztof Kozlowski , Laurent Pinchart , Magnus Damm , Rob Herring , devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org Cc: Andrzej Hajda , Neil Armstrong , Robert Foss , Jonas Karlman , Jernej Skrabec , Tomi Valkeinen References: <20221117122547.809644-1-tomi.valkeinen@ideasonboard.com> <20221117122547.809644-5-tomi.valkeinen@ideasonboard.com> <166869741913.50677.3537704052215375530@Monstersaurus> From: Tomi Valkeinen In-Reply-To: <166869741913.50677.3537704052215375530@Monstersaurus> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On 17/11/2022 17:03, Kieran Bingham wrote: > Quoting Tomi Valkeinen (2022-11-17 12:25:43) >> From: Tomi Valkeinen >> >> Add DT nodes for components needed to get the DSI output working: >> - FCPv >> - VSPd >> - DU >> - DSI >> >> Signed-off-by: Tomi Valkeinen >> --- >> arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 129 ++++++++++++++++++++++ >> 1 file changed, 129 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi >> index 45d8d927ad26..31d4930c5adc 100644 >> --- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi >> +++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi >> @@ -1207,6 +1207,135 @@ prr: chipid@fff00044 { >> compatible = "renesas,prr"; >> reg = <0 0xfff00044 0 4>; >> }; > > I think these nodes are supposed to be in sort order based on the > register address in memory. Ah, I didn't realize that. > Disregarding sort order, I'll review the node contents. > > I would probably s/data/nodes/ in $SUBJECT too. > > >> + >> + fcpvd0: fcp@fea10000 { >> + compatible = "renesas,fcpv"; >> + reg = <0 0xfea10000 0 0x200>; >> + clocks = <&cpg CPG_MOD 508>; >> + power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>; >> + resets = <&cpg 508>; >> + }; >> + >> + fcpvd1: fcp@fea11000 { >> + compatible = "renesas,fcpv"; >> + reg = <0 0xfea11000 0 0x200>; >> + clocks = <&cpg CPG_MOD 509>; >> + power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>; >> + resets = <&cpg 509>; >> + }; > > I'm intrigued at the length of 0x200 as I only see 3 registers up to > 0x0018 .. > > But all existing platforms with fcpv* set 0x200 ... so lets cargo cult it up... :-) > >> + >> + vspd0: vsp@fea20000 { >> + compatible = "renesas,vsp2"; >> + reg = <0 0xfea20000 0 0x5000>; > > """ > Below are the base addresses of each VSP unit. VSPX has 32Kbyte address > space. VSPD has 28Kbyte address space. > """ > > Hrm : 28K is 0x7000 > > RPf n OSD CLUT Table: H’4000 + H’0400*n to H’43fc + H’0400*n > > 0x43fc+(0x400*5) > 22524 [0x57fc] > > So this needs to be /at least/ 0x6000 (Would 0x5800 be odd?) and perhaps as it clearly states > 28k, we should just set it to 0x7000. Ok. These are identical to v3u, and I was just copying v3u's dts, assuming they're correct. We probably should fix the v3u dts files too. > >> + interrupts = ; >> + clocks = <&cpg CPG_MOD 830>; >> + power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>; >> + resets = <&cpg 830>; >> + >> + renesas,fcp = <&fcpvd0>; >> + }; >> + >> + vspd1: vsp@fea28000 { >> + compatible = "renesas,vsp2"; >> + reg = <0 0xfea28000 0 0x5000>; > > Same here of course (reg = <0 0xfea28000 0 0x7000>) > > >> + interrupts = ; >> + clocks = <&cpg CPG_MOD 831>; >> + power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>; >> + resets = <&cpg 831>; >> + >> + renesas,fcp = <&fcpvd1>; >> + }; >> + >> + du: display@feb00000 { >> + compatible = "renesas,du-r8a779g0"; >> + reg = <0 0xfeb00000 0 0x40000>; >> + interrupts = , >> + ; >> + clocks = <&cpg CPG_MOD 411>; >> + clock-names = "du.0"; >> + power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>; >> + resets = <&cpg 411>; >> + reset-names = "du.0"; >> + renesas,vsps = <&vspd0 0>, <&vspd1 0>; >> + >> + status = "disabled"; >> + >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + du_out_dsi0: endpoint { >> + remote-endpoint = <&dsi0_in>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + du_out_dsi1: endpoint { >> + remote-endpoint = <&dsi1_in>; >> + }; >> + }; >> + }; >> + }; >> + >> + dsi0: dsi-encoder@fed80000 { >> + compatible = "renesas,r8a779g0-dsi-csi2-tx"; >> + reg = <0 0xfed80000 0 0x10000>; >> + power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>; >> + clocks = <&cpg CPG_MOD 415>, >> + <&cpg CPG_CORE R8A779G0_CLK_DSIEXT>, >> + <&cpg CPG_CORE R8A779G0_CLK_DSIREF>; >> + clock-names = "fck", "dsi", "pll"; >> + resets = <&cpg 415>; > > blank line here to separate it, and highlight that it's disabled? (Like > is done for DU? Ok. >> + status = "disabled"; >> + >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + dsi0_in: endpoint { >> + remote-endpoint = <&du_out_dsi0>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + }; >> + }; >> + }; >> + >> + dsi1: dsi-encoder@fed90000 { >> + compatible = "renesas,r8a779g0-dsi-csi2-tx"; >> + reg = <0 0xfed90000 0 0x10000>; >> + power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>; >> + clocks = <&cpg CPG_MOD 416>, >> + <&cpg CPG_CORE R8A779G0_CLK_DSIEXT>, >> + <&cpg CPG_CORE R8A779G0_CLK_DSIREF>; >> + clock-names = "fck", "dsi", "pll"; >> + resets = <&cpg 416>; > > Same. > > With the VSPD register ranges increased accordingly: > > Reviewed-by: Kieran Bingham > >> + status = "disabled"; >> + >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + dsi1_in: endpoint { >> + remote-endpoint = <&du_out_dsi1>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + }; >> + }; >> + }; >> + >> }; >> >> timer { >> -- >> 2.34.1 >>