Hi Philipp, CK, Please see my comments inline. On Thu, Nov 19, 2015 at 2:34 AM, Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> wrote: > From: CK Hu <ck.hu@xxxxxxxxxxxx> > > Add device tree binding documentation for the display subsystem in > Mediatek MT8173 SoCs. > > Signed-off-by: CK Hu <ck.hu@xxxxxxxxxxxx> > Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> > Acked-by: Rob Herring <robh@xxxxxxxxxx> > --- > Changes since v5: > - Updated DISP_MUTEX description > - Fixed DSI and DPI documentation path > --- > .../bindings/display/mediatek/mediatek,disp.txt | 183 +++++++++++++++++++++ > .../bindings/display/mediatek/mediatek,dpi.txt | 35 ++++ > .../bindings/display/mediatek/mediatek,dsi.txt | 53 ++++++ > 3 files changed, 271 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt > create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt > > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > new file mode 100644 > index 0000000..a311019 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > @@ -0,0 +1,183 @@ > +Mediatek display subsystem > +========================== > + > +The Mediatek display subsystem consists of various DISP function blocks in the > +MMSYS register space. The connections between them can be configured by output > +and input selectors in the MMSYS_CONFIG register space. Pixel clock and start > +of frame signal are distributed to the other function blocks by a DISP_MUTEX > +function block. > + > +All DISP device tree nodes must be siblings to the central MMSYS_CONFIG node. > +For a description of the MMSYS_CONFIG binding, see > +Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt. > + > +DISP function blocks > +==================== > + > +A display stream starts at a source function block that reads pixel data from > +memory and ends with a sink function block that drives pixels on a display > +interface, or writes pixels back to memory. All DISP function blocks have > +their own register space, interrupt, and clock gate. The blocks that can > +access memory additionally have to list the IOMMU and local arbiter they are > +connected to. > + > +For a description of the display interface sink function blocks, see > +Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt and > +Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt. > + > +Required properties (all function blocks): > +- compatible: "mediatek,<chip>-disp-<function>", one of > + "mediatek,<chip>-disp-ovl" - overlay (4 layers, blending, csc) > + "mediatek,<chip>-disp-rdma" - read DMA / line buffer > + "mediatek,<chip>-disp-wdma" - write DMA > + "mediatek,<chip>-disp-color" - color processor > + "mediatek,<chip>-disp-aal" - adaptive ambient light controller > + "mediatek,<chip>-disp-gamma" - gamma correction > + "mediatek,<chip>-disp-ufoe" - data compression engine > + "mediatek,<chip>-dsi" - DSI controller, see mediatek,dsi.txt > + "mediatek,<chip>-dpi" - DPI controller, see mediatek,dpi.txt > + "mediatek,<chip>-disp-mutex" - display mutex > + "mediatek,<chip>-disp-od" - overdrive > +- reg: Physical base address and length of the function block register space > +- interrupts: The interrupt signal from the function block. Do all the blocks have only one interrupt signal? > +- clocks: device clocks > + See Documentation/devicetree/bindings/clock/clock-bindings.txt for details. What about clock-names? Also all required clocks for particular compatible strings should be listed in documentation, > +- compatible: "mediatek,<chip>-ddp" Is this a rebase error? compatible property was already described few lines above and also missing description. > + > +Required properties (DMA function blocks): > +- compatible: Should be one of > + "mediatek,<chip>-disp-ovl" > + "mediatek,<chip>-disp-rdma" > + "mediatek,<chip>-disp-wdma" Perhaps these 3 could also have some short description like the ones listed above? > +- larb: Should contain a phandle pointing to the local arbiter device as defined > + in Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt > +- iommus: required a iommu node Maybe rewrite to: "Should point to respective IOMMU block as defined in <path to IOMMU binding documentation>"? Best regards, Tomasz _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel