Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758034AbcK3QEX (ORCPT ); Wed, 30 Nov 2016 11:04:23 -0500 Received: from mail-wm0-f47.google.com ([74.125.82.47]:34872 "EHLO mail-wm0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757811AbcK3QEM (ORCPT ); Wed, 30 Nov 2016 11:04:12 -0500 Subject: Re: [PATCH v2 3/4] dt-bindings: display: add Amlogic Meson DRM Bindings To: Laurent Pinchart References: <1480520625-13269-1-git-send-email-narmstrong@baylibre.com> <1480520625-13269-4-git-send-email-narmstrong@baylibre.com> <13104644.f9boR54smD@avalon> Cc: airlied@linux.ie, khilman@baylibre.com, carlo@caione.org, dri-devel@lists.freedesktop.org, linux-amlogic@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, victor.wan@amlogic.com, jerry.cao@amlogic.com, Xing.Xu@amlogic.com, devicetree@vger.kernel.org, daniel@ffwll.ch From: Neil Armstrong Organization: Baylibre Message-ID: Date: Wed, 30 Nov 2016 17:04:06 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <13104644.f9boR54smD@avalon> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3458 Lines: 103 Hi Laurent, On 11/30/2016 04:56 PM, Laurent Pinchart wrote: > Hi Neil, > > Thank you for the patch. > > On Wednesday 30 Nov 2016 16:43:44 Neil Armstrong wrote: >> Signed-off-by: Neil Armstrong >> --- >> .../bindings/display/meson/meson-drm.txt | 101 ++++++++++++++++++ >> 1 file changed, 101 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/display/meson/meson-drm.txt >> >> diff --git a/Documentation/devicetree/bindings/display/meson/meson-drm.txt >> b/Documentation/devicetree/bindings/display/meson/meson-drm.txt new file >> mode 100644 >> index 0000000..e52869a >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/display/meson/meson-drm.txt >> @@ -0,0 +1,101 @@ [...] >> + >> +Device Tree Bindings: >> +--------------------- >> + >> +VPU: Video Processing Unit >> +-------------------------- >> + >> +Required properties: >> +- compatible: value should be different for each SoC family as : >> + - GXBB (S905) : "amlogic,meson-gxbb-vpu" >> + - GXL (S905X, S905D) : "amlogic,meson-gxl-vpu" >> + - GXM (S912) : "amlogic,meson-gxm-vpu" >> + followed by the common "amlogic,meson-gx-vpu" >> +- reg: base address and size of he following memory-mapped regions : >> + - vpu >> + - hhi >> + - dmc >> +- reg-names: should contain the names of the previous memory regions >> +- interrupts: should contain the VENC Vsync interrupt number >> + >> +- ports: A ports node with endpoint definitions as defined in >> + Documentation/devicetree/bindings/media/video-interfaces.txt. >> + The first port should be connected to a CVBS connector endpoint if >> available. > > This is a bit vague, I propose clarifying it with a description similar to the > one in the renesas,du.txt bindings. > > Required nodes: > > The connections to the VPU output video ports are modeled using the OF graph > bindings specified in Documentation/devicetree/bindings/graph.txt. > > The following table lists for each supported model the port number > corresponding to each DU output. > > Port 0 Port1 Port2 Port3 > ----------------------------------------------------------------------------- > R8A7779 (H1) DPAD 0 DPAD 1 - - > R8A7790 (H2) DPAD LVDS 0 LVDS 1 - > R8A7791 (M2-W) DPAD LVDS 0 - - > R8A7792 (V2H) DPAD 0 DPAD 1 - - > R8A7793 (M2-N) DPAD LVDS 0 - - > R8A7794 (E2) DPAD 0 DPAD 1 - - > R8A7795 (H3) DPAD HDMI 0 HDMI 1 LVDS > R8A7796 (M3-W) DPAD HDMI LVDS - > > (You should obviously replace the table with Amlogic data) > > It doesn't matter if the current driver implementation only supports CVBS, the > DT bindings can already document the other ports. > Ok, it's a pretty table ! Will integrate this. >> + >> +Example: >> + >> +tv: connector { >> + compatible = "composite-video-connector"; >> + label = "cvbs"; > > I'd remove the label here, as it doesn't bring any additional information. > Unless the board you're using has a label for the connector, in case that > label should be used. Indeed, I already removed it in the dts. > > Apart from that, > > Reviewed-by: Laurent Pinchart > [...] Thanks for the review, Neil