Hi Philipp,
Two more comments below...
On Tue, Feb 2, 2016 at 4:10 PM, Daniel Kurtz <djkurtz@xxxxxxxxxxxx> wrote:
> On Tue, Jan 5, 2016 at 1:36 AM, Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> wrote:
>> From: CK Hu <ck.hu@xxxxxxxxxxxx>
>>
>> This patch adds the device nodes for the DISP function blocks
>> comprising the display subsystem.
>>
>> Signed-off-by: CK Hu <ck.hu@xxxxxxxxxxxx>
>> Signed-off-by: Cawa Cheng <cawa.cheng@xxxxxxxxxxxx>
>> Signed-off-by: Jie Qiu <jie.qiu@xxxxxxxxxxxx>
>> Signed-off-by: Daniel Kurtz <djkurtz@xxxxxxxxxxxx>
>> Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
>> +
>> + dsi0: dsi@1401b000 {
>> + compatible = "mediatek,mt8173-dsi";
>> + reg = <0 0x1401b000 0 0x1000>;
>> + interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_LOW>;
>> + power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
>> + clocks = <&mmsys CLK_MM_DSI0_ENGINE>,
>> + <&mmsys CLK_MM_DSI0_DIGITAL>,
>> + <&mipi_tx0>;
>> + clock-names = "engine", "digital", "hs";
>> + phys = <&mipi_tx0>;
>> + phy-names = "dphy";
I think it might work better if this was also default
status="disabled", and require the board-specific .dts to enable the
dsi*.
This would be useful, for example, boards that use only the MIPI/DSI
or only HDMI.
IMHO, it is more clear to have such boards explicit mark the supported
ports as 'status="okay"' in their .dts, rather than having to mark all
of the unused ones as disabled.
>> + };
>> +
>> + dsi1: dsi@1401c000 {
>> + compatible = "mediatek,mt8173-dsi";
>> + reg = <0 0x1401c000 0 0x1000>;
>> + interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_LOW>;
>> + power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
>> + clocks = <&mmsys CLK_MM_DSI1_ENGINE>,
>> + <&mmsys CLK_MM_DSI1_DIGITAL>,
>> + <&mipi_tx1>;
>> + clock-names = "engine", "digital", "hs";
>> + phy = <&mipi_tx1>;
>> + phy-names = "dphy";
>> + status = "disabled";
>> + };
>> +
>> + dpi0: dpi@1401d000 {
>> + compatible = "mediatek,mt8173-dpi";
>> + reg = <0 0x1401d000 0 0x1000>;
>> + interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>;
>> + power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
>> + clocks = <&mmsys CLK_MM_DPI_PIXEL>,
>> + <&mmsys CLK_MM_DPI_ENGINE>,
>> + <&apmixedsys CLK_APMIXED_TVDPLL>;
>> + clock-names = "pixel", "engine", "pll";
>> +
>> + port {
>> + dpi0_out: endpoint {
>> + remote-endpoint = <&hdmi0_in>;
>
> nit: At this point in the patch series, you haven't defined hdmi0_in yet.
> Move this port to "Add HDMI related nodes".
And, let's mark dpi0 as status="disabled"; by default, and require an
enabling in the board specific .dts.
>
>> + };
>> + };
>> };
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
