| Message ID | 20220523104758.29531-2-granquet@baylibre.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm/mediatek: Add mt8195 DisplayPort driver | expand |
On Mon, 23 May 2022 12:47:34 +0200, Guillaume Ranquet wrote: > From: Markus Schneider-Pargmann <msp@baylibre.com> > > DPINTF is similar to DPI but does not have the exact same feature set > or register layouts. > > DPINTF is the sink of the display pipeline that is connected to the > DisplayPort controller and encoder unit. It takes the same clocks as > DPI. > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com> > --- > .../bindings/display/mediatek/mediatek,dpi.yaml | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > Running 'make dtbs_check' with the schema in this patch gives the following warnings. Consider if they are expected or the schema is incorrect. These may not be new warnings. Note that it is not yet a requirement to have 0 warnings for dtbs_check. This will change in the future. Full log is available here: https://patchwork.ozlabs.org/patch/ dpi@14014000: Additional properties are not allowed ('ports' was unexpected) arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dtb arch/arm/boot/dts/mt7623n-rfb-emmc.dtb dpi@14014000: clock-names: ['pixel', 'engine', 'pll'] is too short arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dtb arch/arm/boot/dts/mt7623n-rfb-emmc.dtb dpi@14014000: clocks: [[59, 26], [59, 27], [3, 6]] is too short arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dtb dpi@14014000: clocks: [[61, 26], [61, 27], [3, 6]] is too short arch/arm/boot/dts/mt7623n-rfb-emmc.dtb dpi@14014000: compatible: ['mediatek,mt7623-dpi', 'mediatek,mt2701-dpi'] is too long arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dtb arch/arm/boot/dts/mt7623n-rfb-emmc.dtb dpi@14014000: 'port' is a required property arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dtb arch/arm/boot/dts/mt7623n-rfb-emmc.dtb dpi@1401d000: Additional properties are not allowed ('power-domains' was unexpected) arch/arm64/boot/dts/mediatek/mt8173-elm.dtb arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtb arch/arm64/boot/dts/mediatek/mt8173-elm-hana-rev7.dtb arch/arm64/boot/dts/mediatek/mt8173-evb.dtb dpi@1401d000: clock-names: ['pixel', 'engine', 'pll'] is too short arch/arm64/boot/dts/mediatek/mt8173-elm.dtb arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtb arch/arm64/boot/dts/mediatek/mt8173-elm-hana-rev7.dtb arch/arm64/boot/dts/mediatek/mt8173-evb.dtb dpi@1401d000: clocks: [[57, 40], [57, 41], [8, 8]] is too short arch/arm64/boot/dts/mediatek/mt8173-evb.dtb dpi@1401d000: clocks: [[68, 40], [68, 41], [8, 8]] is too short arch/arm64/boot/dts/mediatek/mt8173-elm.dtb arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtb arch/arm64/boot/dts/mediatek/mt8173-elm-hana-rev7.dtb
On Mon, 2022-05-23 at 12:47 +0200, Guillaume Ranquet wrote: > From: Markus Schneider-Pargmann <msp@baylibre.com> > > DPINTF is similar to DPI but does not have the exact same feature set > or register layouts. > > DPINTF is the sink of the display pipeline that is connected to the > DisplayPort controller and encoder unit. It takes the same clocks as > DPI. > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com> > --- > .../bindings/display/mediatek/mediatek,dpi.yaml | 13 ++++++++--- > -- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git > a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yam > l > b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yam > l > index dd2896a40ff0..6d9f6c11806e 100644 > --- > a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yam > l > +++ > b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yam > l > @@ -4,16 +4,16 @@ > $id: > http://devicetree.org/schemas/display/mediatek/mediatek,dpi.yaml# > $schema: http://devicetree.org/meta-schemas/core.yaml# > > -title: mediatek DPI Controller Device Tree Bindings > +title: mediatek DPI/DPINTF Controller > > maintainers: > - CK Hu <ck.hu@mediatek.com> > - Jitao shi <jitao.shi@mediatek.com> > > description: | > - The Mediatek DPI function block is a sink of the display subsystem > and > - provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a > parallel > - output bus. > + The Mediatek DPI and DPINTF function blocks are a sink of the > display > + subsystem and provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 > pixel data on a > + parallel output bus. > > properties: > compatible: > @@ -23,6 +23,7 @@ properties: > - mediatek,mt8173-dpi > - mediatek,mt8183-dpi > - mediatek,mt8192-dpi > + - mediatek,mt8195-dpintf > > reg: > maxItems: 1 > @@ -35,12 +36,14 @@ properties: > - description: Pixel Clock > - description: Engine Clock > - description: DPI PLL > + - description: Optional CK CG Clock > > clock-names: > items: > - const: pixel > - const: engine > - const: pll > + - const: ck_cg 'ck_cg' seems not a exact clock names, could you pleas check it again with DE. > > pinctrl-0: true > pinctrl-1: true > @@ -54,7 +57,7 @@ properties: > $ref: /schemas/graph.yaml#/properties/port > description: > Output port node. This port should be connected to the input > port of an > - attached HDMI or LVDS encoder chip. > + attached HDMI, LVDS or DisplayPort encoder chip. > > required: > - compatible
Il 23/05/22 12:47, Guillaume Ranquet ha scritto: > From: Markus Schneider-Pargmann <msp@baylibre.com> > > DPINTF is similar to DPI but does not have the exact same feature set > or register layouts. > > DPINTF is the sink of the display pipeline that is connected to the > DisplayPort controller and encoder unit. It takes the same clocks as > DPI. > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com> > --- > .../bindings/display/mediatek/mediatek,dpi.yaml | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > index dd2896a40ff0..6d9f6c11806e 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > @@ -4,16 +4,16 @@ > $id: http://devicetree.org/schemas/display/mediatek/mediatek,dpi.yaml# > $schema: http://devicetree.org/meta-schemas/core.yaml# > > -title: mediatek DPI Controller Device Tree Bindings > +title: mediatek DPI/DPINTF Controller > > maintainers: > - CK Hu <ck.hu@mediatek.com> > - Jitao shi <jitao.shi@mediatek.com> > > description: | > - The Mediatek DPI function block is a sink of the display subsystem and > - provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a parallel > - output bus. > + The Mediatek DPI and DPINTF function blocks are a sink of the display > + subsystem and provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a > + parallel output bus. > > properties: > compatible: > @@ -23,6 +23,7 @@ properties: > - mediatek,mt8173-dpi > - mediatek,mt8183-dpi > - mediatek,mt8192-dpi > + - mediatek,mt8195-dpintf > > reg: > maxItems: 1 > @@ -35,12 +36,14 @@ properties: > - description: Pixel Clock > - description: Engine Clock > - description: DPI PLL > + - description: Optional CK CG Clock > > clock-names: > items: > - const: pixel > - const: engine > - const: pll > + - const: ck_cg This is my understanding on how the DisplayPort Interface clocks work on 8195: The "engine" clock is for the *VPP Engine's DisplayPort ip/block*, "pll" is for TVD PLL divider selection "pixel" is the gate for the pixel clock to the connected display. "ck_cg" is useless, as that's the parent of "pixel" (and will always be)... for example, on mt8195... check clk/mediatek/clk-mt8195-vdo0.c - the CLK_VDO0_DP_INTF0_DP_INTF clock already has CLK_TOP_EDP as its parent, hence enabling the first will enable the latter. That said... you can most probably avoid adding the ck_cg clock, as if you try to turn that off while it's in use by its children, you'll be only decrementing a refcount, but no "real action" will ever take place. Regards, Angelo
Hi, On Mon, May 23, 2022 at 12:47:34PM +0200, Guillaume Ranquet wrote: > From: Markus Schneider-Pargmann <msp@baylibre.com> > > DPINTF is similar to DPI but does not have the exact same feature set > or register layouts. > > DPINTF is the sink of the display pipeline that is connected to the > DisplayPort controller and encoder unit. It takes the same clocks as > DPI. > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com> > --- > .../bindings/display/mediatek/mediatek,dpi.yaml | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > index dd2896a40ff0..6d9f6c11806e 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > @@ -4,16 +4,16 @@ > $id: http://devicetree.org/schemas/display/mediatek/mediatek,dpi.yaml# > $schema: http://devicetree.org/meta-schemas/core.yaml# > > -title: mediatek DPI Controller Device Tree Bindings > +title: mediatek DPI/DPINTF Controller > > maintainers: > - CK Hu <ck.hu@mediatek.com> > - Jitao shi <jitao.shi@mediatek.com> > > description: | > - The Mediatek DPI function block is a sink of the display subsystem and > - provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a parallel > - output bus. > + The Mediatek DPI and DPINTF function blocks are a sink of the display > + subsystem and provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a > + parallel output bus. > > properties: > compatible: > @@ -23,6 +23,7 @@ properties: > - mediatek,mt8173-dpi > - mediatek,mt8183-dpi > - mediatek,mt8192-dpi > + - mediatek,mt8195-dpintf Weren't you supposed to change it to have a separator between dp and intf? If it's no longer in your plans, the second patch should have s/dp_intf/dpintf/ Maxime
On Wed, 2022-05-25 at 13:55 +0200, AngeloGioacchino Del Regno wrote: > Il 23/05/22 12:47, Guillaume Ranquet ha scritto: > > From: Markus Schneider-Pargmann <msp@baylibre.com> > > > > DPINTF is similar to DPI but does not have the exact same feature > > set > > or register layouts. > > > > DPINTF is the sink of the display pipeline that is connected to the > > DisplayPort controller and encoder unit. It takes the same clocks > > as > > DPI. > > > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com> > > --- > > .../bindings/display/mediatek/mediatek,dpi.yaml | 13 > > ++++++++----- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git > > a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.y > > aml > > b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.y > > aml > > index dd2896a40ff0..6d9f6c11806e 100644 > > --- > > a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.y > > aml > > +++ > > b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.y > > aml > > @@ -4,16 +4,16 @@ > > $id: > > http://devicetree.org/schemas/display/mediatek/mediatek,dpi.yaml# > > $schema: http://devicetree.org/meta-schemas/core.yaml# > > > > -title: mediatek DPI Controller Device Tree Bindings > > +title: mediatek DPI/DPINTF Controller > > > > maintainers: > > - CK Hu <ck.hu@mediatek.com> > > - Jitao shi <jitao.shi@mediatek.com> > > > > description: | > > - The Mediatek DPI function block is a sink of the display > > subsystem and > > - provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a > > parallel > > - output bus. > > + The Mediatek DPI and DPINTF function blocks are a sink of the > > display > > + subsystem and provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 > > pixel data on a > > + parallel output bus. > > > > properties: > > compatible: > > @@ -23,6 +23,7 @@ properties: > > - mediatek,mt8173-dpi > > - mediatek,mt8183-dpi > > - mediatek,mt8192-dpi > > + - mediatek,mt8195-dpintf > > > > reg: > > maxItems: 1 > > @@ -35,12 +36,14 @@ properties: > > - description: Pixel Clock > > - description: Engine Clock > > - description: DPI PLL > > + - description: Optional CK CG Clock > > > > clock-names: > > items: > > - const: pixel > > - const: engine > > - const: pll > > + - const: ck_cg > > This is my understanding on how the DisplayPort Interface clocks work > on 8195: > > The "engine" clock is for the *VPP Engine's DisplayPort ip/block*, > "pll" is for TVD PLL divider selection > "pixel" is the gate for the pixel clock to the connected display. > > "ck_cg" is useless, as that's the parent of "pixel" (and will always > be)... for > example, on mt8195... check clk/mediatek/clk-mt8195-vdo0.c - the > CLK_VDO0_DP_INTF0_DP_INTF clock already has CLK_TOP_EDP as its > parent, hence > enabling the first will enable the latter. > > That said... you can most probably avoid adding the ck_cg clock, as > if you try > to turn that off while it's in use by its children, you'll be only > decrementing > a refcount, but no "real action" will ever take place. > > > Regards, > Angelo Hello Chunfeng and Angelo, ck_cg is a clock gate, and I try to remove it from drivers but it's failed to enable dp_intf. the block diagram is: 1. 26M->CLK_APMIXED_TVDPLL1(pll)->CLK_TOP_EDP(pixel)- >CLK_VDO0_DP_INTF0_DP_INTF(ck_cg)->dp_intf 2. VDOSYS clock->CLK_VDO0_DP_INTF0(engine)->dp_intf "engine" and "ck_cg" are all clock gates which control the clock source input to dp_intf. Maybe we just need to rename it? If so, what name do you think we should modify? BRs, Bo-Chen > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, 2022-05-25 at 14:49 +0200, Maxime Ripard wrote: > Hi, > > On Mon, May 23, 2022 at 12:47:34PM +0200, Guillaume Ranquet wrote: > > From: Markus Schneider-Pargmann <msp@baylibre.com> > > > > DPINTF is similar to DPI but does not have the exact same feature > > set > > or register layouts. > > > > DPINTF is the sink of the display pipeline that is connected to the > > DisplayPort controller and encoder unit. It takes the same clocks > > as > > DPI. > > > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com> > > --- > > .../bindings/display/mediatek/mediatek,dpi.yaml | 13 ++++++++- > > ---- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git > > a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.y > > aml > > b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.y > > aml > > index dd2896a40ff0..6d9f6c11806e 100644 > > --- > > a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.y > > aml > > +++ > > b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.y > > aml > > @@ -4,16 +4,16 @@ > > $id: > > http://devicetree.org/schemas/display/mediatek/mediatek,dpi.yaml# > > $schema: http://devicetree.org/meta-schemas/core.yaml# > > > > -title: mediatek DPI Controller Device Tree Bindings > > +title: mediatek DPI/DPINTF Controller > > > > maintainers: > > - CK Hu <ck.hu@mediatek.com> > > - Jitao shi <jitao.shi@mediatek.com> > > > > description: | > > - The Mediatek DPI function block is a sink of the display > > subsystem and > > - provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a > > parallel > > - output bus. > > + The Mediatek DPI and DPINTF function blocks are a sink of the > > display > > + subsystem and provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 > > pixel data on a > > + parallel output bus. > > > > properties: > > compatible: > > @@ -23,6 +23,7 @@ properties: > > - mediatek,mt8173-dpi > > - mediatek,mt8183-dpi > > - mediatek,mt8192-dpi > > + - mediatek,mt8195-dpintf > > Weren't you supposed to change it to have a separator between dp and > intf? > > If it's no longer in your plans, the second patch should have > s/dp_intf/dpintf/ > > Maxime Hello Maxime, Thank for your review. I will do this in next version. BRs, Bo-Chen > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml index dd2896a40ff0..6d9f6c11806e 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml @@ -4,16 +4,16 @@ $id: http://devicetree.org/schemas/display/mediatek/mediatek,dpi.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: mediatek DPI Controller Device Tree Bindings +title: mediatek DPI/DPINTF Controller maintainers: - CK Hu <ck.hu@mediatek.com> - Jitao shi <jitao.shi@mediatek.com> description: | - The Mediatek DPI function block is a sink of the display subsystem and - provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a parallel - output bus. + The Mediatek DPI and DPINTF function blocks are a sink of the display + subsystem and provides 8-bit RGB/YUV444 or 8/10/10-bit YUV422 pixel data on a + parallel output bus. properties: compatible: @@ -23,6 +23,7 @@ properties: - mediatek,mt8173-dpi - mediatek,mt8183-dpi - mediatek,mt8192-dpi + - mediatek,mt8195-dpintf reg: maxItems: 1 @@ -35,12 +36,14 @@ properties: - description: Pixel Clock - description: Engine Clock - description: DPI PLL + - description: Optional CK CG Clock clock-names: items: - const: pixel - const: engine - const: pll + - const: ck_cg pinctrl-0: true pinctrl-1: true @@ -54,7 +57,7 @@ properties: $ref: /schemas/graph.yaml#/properties/port description: Output port node. This port should be connected to the input port of an - attached HDMI or LVDS encoder chip. + attached HDMI, LVDS or DisplayPort encoder chip. required: - compatible