Re: [v1] dt-bindings: msm:disp: update dsi and dpu bindings
On 2020-02-04 23:52, Doug Anderson wrote:
Hi,
On Tue, Feb 4, 2020 at 6:15 AM Harigovindan P
wrote:
Updating bindings of dsi and dpu by adding and removing certain
properties.
Signed-off-by: Harigovindan P
---
Changes in v1:
- Adding "ahb" clock as a required property.
- Adding "bus", "rot", "lut" as optional properties for sc7180
device.
- Removing properties from dsi bindings that are unused.
- Removing power-domain property since DSI is the child node
of MDSS
and it will inherit supply from its parent.
Documentation/devicetree/bindings/display/msm/dpu.txt | 7 +++
Documentation/devicetree/bindings/display/msm/dsi.txt | 5 -
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt
b/Documentation/devicetree/bindings/display/msm/dpu.txt
index 551ae26..dd58472a 100644
--- a/Documentation/devicetree/bindings/display/msm/dpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/dpu.txt
@@ -19,6 +19,7 @@ Required properties:
The following clocks are required:
* "iface"
* "bus"
+ * "ahb"
This is only required for sc7180? ...or old SoCs should have had it
all along too?
* "core"
- interrupts: interrupt signal from MDSS.
- interrupt-controller: identifies the node as an interrupt
controller.
@@ -50,6 +51,8 @@ Required properties:
- clock-names: device clock names, must be in same order as clocks
property.
The following clocks are required.
* "bus"
+ For the device "qcom,sc7180-dpu":
+ * "bus" - is an optional property due to architecture change.
This is a really odd way to write it for two reasons:
* You're breaking up the flow of the list.
* This shouldn't be listed as "optional" in sc7180 but unless there is
some reason to ever provide it on sc7180. It should simply be
disallowed.
Maybe instead just:
The following clocks are required.
- * "bus"
+ * "bus" (anything other than qcom,sc7180-dpu)
We really need to get this into yaml ASAP but that'd probably be OK to
tide us over.
NOTE: when converting to yaml, ideally we'll have a separate file per
SoC to avoid crazy spaghetti, see commit 2a8aa18c1131 ("dt-bindings:
clk: qcom: Fix self-validation, split, and clean cruft") in clk-next
for an example of starting the transition to one yaml per SoC (at
least for anything majorly different).
* "iface"
* "core"
* "vsync"
@@ -70,6 +73,10 @@ Optional properties:
- assigned-clocks: list of clock specifiers for clocks needing rate
assignment
- assigned-clock-rates: list of clock frequencies sorted in the same
order as
the assigned-clocks property.
+- For the device "qcom,sc7180-dpu":
+ clock-names: optional device clocks, needed for accessing LUT
blocks.
+ * "rot"
+ * "lut"
Example:
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt
b/Documentation/devicetree/bindings/display/msm/dsi.txt
index af95586..61d659a 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi.txt
+++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
@@ -8,13 +8,10 @@ Required properties:
- reg-names: The names of register regions. The following regions are
required:
* "dsi_ctrl"
- interrupts: The interrupt signal from the DSI block.
-- power-domains: Should be < MDSS_GDSC>.
Is this supposed to be removed from all SoCs using this bindings, or
just yours?
I'll also note that you left it in the "Example:" below.
- clocks: Phandles to device clocks.
- clock-names: the following clocks are required:
- * "mdp_core"
* "iface"
* "bus"
- * "core_mmss"
As Jeffrey pointed out, you shouldn't be removing these from old SoCs.
In "drivers/gpu/drm/msm/dsi/dsi_cfg.c" you can clearly see them used.
Maybe it's time for you to do the yaml conversion and handle this
correctly per-SoC.
-Doug
Hi,
yaml files have been created and new patchset has been created for that.
https://patchwork.kernel.org/patch/11423653/
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v1] dt-bindings: msm:disp: update dsi and dpu bindings
On Tue, Feb 04, 2020 at 07:45:37PM +0530, Harigovindan P wrote: > Updating bindings of dsi and dpu by adding and removing certain > properties. Yes, the diff tells me that. The commit message should say why. This change breaks compatibility as well. > > Signed-off-by: Harigovindan P > --- > > Changes in v1: > - Adding "ahb" clock as a required property. > - Adding "bus", "rot", "lut" as optional properties for sc7180 device. > - Removing properties from dsi bindings that are unused. > - Removing power-domain property since DSI is the child node of MDSS > and it will inherit supply from its parent. > > Documentation/devicetree/bindings/display/msm/dpu.txt | 7 +++ > Documentation/devicetree/bindings/display/msm/dsi.txt | 5 - > 2 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt > b/Documentation/devicetree/bindings/display/msm/dpu.txt > index 551ae26..dd58472a 100644 > --- a/Documentation/devicetree/bindings/display/msm/dpu.txt > +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt > @@ -19,6 +19,7 @@ Required properties: >The following clocks are required: >* "iface" >* "bus" > + * "ahb" You can't just add new clocks... >* "core" > - interrupts: interrupt signal from MDSS. > - interrupt-controller: identifies the node as an interrupt controller. > @@ -50,6 +51,8 @@ Required properties: > - clock-names: device clock names, must be in same order as clocks property. >The following clocks are required. >* "bus" > + For the device "qcom,sc7180-dpu": > + * "bus" - is an optional property due to architecture change. >* "iface" >* "core" >* "vsync" > @@ -70,6 +73,10 @@ Optional properties: > - assigned-clocks: list of clock specifiers for clocks needing rate > assignment > - assigned-clock-rates: list of clock frequencies sorted in the same order as >the assigned-clocks property. > +- For the device "qcom,sc7180-dpu": > + clock-names: optional device clocks, needed for accessing LUT blocks. > + * "rot" > + * "lut" > > Example: > > diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt > b/Documentation/devicetree/bindings/display/msm/dsi.txt > index af95586..61d659a 100644 > --- a/Documentation/devicetree/bindings/display/msm/dsi.txt > +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt > @@ -8,13 +8,10 @@ Required properties: > - reg-names: The names of register regions. The following regions are > required: >* "dsi_ctrl" > - interrupts: The interrupt signal from the DSI block. > -- power-domains: Should be < MDSS_GDSC>. > - clocks: Phandles to device clocks. > - clock-names: the following clocks are required: > - * "mdp_core" >* "iface" >* "bus" > - * "core_mmss" >* "byte" >* "pixel" >* "core" > @@ -156,7 +153,6 @@ Example: > "core", > "core_mmss", > "iface", > - "mdp_core", > "pixel"; > clocks = > < MDSS_AXI_CLK>, > @@ -164,7 +160,6 @@ Example: > < MDSS_ESC0_CLK>, > < MMSS_MISC_AHB_CLK>, > < MDSS_AHB_CLK>, > - < MDSS_MDP_CLK>, > < MDSS_PCLK0_CLK>; > > assigned-clocks = > -- > 2.7.4 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v1] dt-bindings: msm:disp: update dsi and dpu bindings
Updating bindings of dsi and dpu by adding and removing certain properties. Signed-off-by: Harigovindan P --- Changes in v1: - Adding "ahb" clock as a required property. - Adding "bus", "rot", "lut" as optional properties for sc7180 device. - Removing properties from dsi bindings that are unused. - Removing power-domain property since DSI is the child node of MDSS and it will inherit supply from its parent. Documentation/devicetree/bindings/display/msm/dpu.txt | 7 +++ Documentation/devicetree/bindings/display/msm/dsi.txt | 5 - 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt index 551ae26..dd58472a 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu.txt +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt @@ -19,6 +19,7 @@ Required properties: The following clocks are required: * "iface" * "bus" + * "ahb" * "core" - interrupts: interrupt signal from MDSS. - interrupt-controller: identifies the node as an interrupt controller. @@ -50,6 +51,8 @@ Required properties: - clock-names: device clock names, must be in same order as clocks property. The following clocks are required. * "bus" + For the device "qcom,sc7180-dpu": + * "bus" - is an optional property due to architecture change. * "iface" * "core" * "vsync" @@ -70,6 +73,10 @@ Optional properties: - assigned-clocks: list of clock specifiers for clocks needing rate assignment - assigned-clock-rates: list of clock frequencies sorted in the same order as the assigned-clocks property. +- For the device "qcom,sc7180-dpu": + clock-names: optional device clocks, needed for accessing LUT blocks. + * "rot" + * "lut" Example: diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt index af95586..61d659a 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi.txt +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt @@ -8,13 +8,10 @@ Required properties: - reg-names: The names of register regions. The following regions are required: * "dsi_ctrl" - interrupts: The interrupt signal from the DSI block. -- power-domains: Should be < MDSS_GDSC>. - clocks: Phandles to device clocks. - clock-names: the following clocks are required: - * "mdp_core" * "iface" * "bus" - * "core_mmss" * "byte" * "pixel" * "core" @@ -156,7 +153,6 @@ Example: "core", "core_mmss", "iface", - "mdp_core", "pixel"; clocks = < MDSS_AXI_CLK>, @@ -164,7 +160,6 @@ Example: < MDSS_ESC0_CLK>, < MMSS_MISC_AHB_CLK>, < MDSS_AHB_CLK>, - < MDSS_MDP_CLK>, < MDSS_PCLK0_CLK>; assigned-clocks = -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [v1] dt-bindings: msm:disp: update dsi and dpu bindings
On Tue, Feb 4, 2020 at 7:15 AM Harigovindan P wrote: > > Updating bindings of dsi and dpu by adding and removing certain > properties. > > Signed-off-by: Harigovindan P > --- > > Changes in v1: > - Adding "ahb" clock as a required property. > - Adding "bus", "rot", "lut" as optional properties for sc7180 device. > - Removing properties from dsi bindings that are unused. > - Removing power-domain property since DSI is the child node of MDSS > and it will inherit supply from its parent. > > Documentation/devicetree/bindings/display/msm/dpu.txt | 7 +++ > Documentation/devicetree/bindings/display/msm/dsi.txt | 5 - > 2 files changed, 7 insertions(+), 5 deletions(-) > diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt > b/Documentation/devicetree/bindings/display/msm/dsi.txt > index af95586..61d659a 100644 > --- a/Documentation/devicetree/bindings/display/msm/dsi.txt > +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt > @@ -8,13 +8,10 @@ Required properties: > - reg-names: The names of register regions. The following regions are > required: >* "dsi_ctrl" > - interrupts: The interrupt signal from the DSI block. > -- power-domains: Should be < MDSS_GDSC>. > - clocks: Phandles to device clocks. > - clock-names: the following clocks are required: > - * "mdp_core" >* "iface" >* "bus" > - * "core_mmss" Why do you think these are unused? I see them used in the driver, and as far as I can tell these get routed to the hardware, therefore they should be described in DT. >* "byte" >* "pixel" >* "core" > @@ -156,7 +153,6 @@ Example: > "core", > "core_mmss", > "iface", > - "mdp_core", > "pixel"; > clocks = > < MDSS_AXI_CLK>, > @@ -164,7 +160,6 @@ Example: > < MDSS_ESC0_CLK>, > < MMSS_MISC_AHB_CLK>, > < MDSS_AHB_CLK>, > - < MDSS_MDP_CLK>, > < MDSS_PCLK0_CLK>; > > assigned-clocks = > -- > 2.7.4 > > ___ > Freedreno mailing list > freedr...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v1] dt-bindings: msm:disp: update dsi and dpu bindings
Hi,
On Tue, Feb 4, 2020 at 6:15 AM Harigovindan P wrote:
>
> Updating bindings of dsi and dpu by adding and removing certain
> properties.
>
> Signed-off-by: Harigovindan P
> ---
>
> Changes in v1:
> - Adding "ahb" clock as a required property.
> - Adding "bus", "rot", "lut" as optional properties for sc7180 device.
> - Removing properties from dsi bindings that are unused.
> - Removing power-domain property since DSI is the child node of MDSS
> and it will inherit supply from its parent.
>
> Documentation/devicetree/bindings/display/msm/dpu.txt | 7 +++
> Documentation/devicetree/bindings/display/msm/dsi.txt | 5 -
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt
> b/Documentation/devicetree/bindings/display/msm/dpu.txt
> index 551ae26..dd58472a 100644
> --- a/Documentation/devicetree/bindings/display/msm/dpu.txt
> +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt
> @@ -19,6 +19,7 @@ Required properties:
>The following clocks are required:
>* "iface"
>* "bus"
> + * "ahb"
This is only required for sc7180? ...or old SoCs should have had it
all along too?
>* "core"
> - interrupts: interrupt signal from MDSS.
> - interrupt-controller: identifies the node as an interrupt controller.
> @@ -50,6 +51,8 @@ Required properties:
> - clock-names: device clock names, must be in same order as clocks property.
>The following clocks are required.
>* "bus"
> + For the device "qcom,sc7180-dpu":
> + * "bus" - is an optional property due to architecture change.
This is a really odd way to write it for two reasons:
* You're breaking up the flow of the list.
* This shouldn't be listed as "optional" in sc7180 but unless there is
some reason to ever provide it on sc7180. It should simply be
disallowed.
Maybe instead just:
The following clocks are required.
- * "bus"
+ * "bus" (anything other than qcom,sc7180-dpu)
We really need to get this into yaml ASAP but that'd probably be OK to
tide us over.
NOTE: when converting to yaml, ideally we'll have a separate file per
SoC to avoid crazy spaghetti, see commit 2a8aa18c1131 ("dt-bindings:
clk: qcom: Fix self-validation, split, and clean cruft") in clk-next
for an example of starting the transition to one yaml per SoC (at
least for anything majorly different).
>* "iface"
>* "core"
>* "vsync"
> @@ -70,6 +73,10 @@ Optional properties:
> - assigned-clocks: list of clock specifiers for clocks needing rate
> assignment
> - assigned-clock-rates: list of clock frequencies sorted in the same order as
>the assigned-clocks property.
> +- For the device "qcom,sc7180-dpu":
> + clock-names: optional device clocks, needed for accessing LUT blocks.
> + * "rot"
> + * "lut"
>
> Example:
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt
> b/Documentation/devicetree/bindings/display/msm/dsi.txt
> index af95586..61d659a 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi.txt
> +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
> @@ -8,13 +8,10 @@ Required properties:
> - reg-names: The names of register regions. The following regions are
> required:
>* "dsi_ctrl"
> - interrupts: The interrupt signal from the DSI block.
> -- power-domains: Should be < MDSS_GDSC>.
Is this supposed to be removed from all SoCs using this bindings, or
just yours?
I'll also note that you left it in the "Example:" below.
> - clocks: Phandles to device clocks.
> - clock-names: the following clocks are required:
> - * "mdp_core"
>* "iface"
>* "bus"
> - * "core_mmss"
As Jeffrey pointed out, you shouldn't be removing these from old SoCs.
In "drivers/gpu/drm/msm/dsi/dsi_cfg.c" you can clearly see them used.
Maybe it's time for you to do the yaml conversion and handle this
correctly per-SoC.
-Doug
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
