* [PATCH v8 0/7] Exynos: Simple QoS for exynos-bus using interconnect [not found] <CGME20201104103713eucas1p2d21b6f936aa18725ae4b4878f3be0a8e@eucas1p2.samsung.com> @ 2020-11-04 10:36 ` Sylwester Nawrocki [not found] ` <CGME20201104103718eucas1p1c103f1a96499b03c72e5457ac2542c3d@eucas1p1.samsung.com> ` (7 more replies) 0 siblings, 8 replies; 23+ messages in thread From: Sylwester Nawrocki @ 2020-11-04 10:36 UTC (permalink / raw) To: georgi.djakov, cw00.choi, krzk Cc: devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel, s.nawrocki This patchset adds interconnect API support for the Exynos SoC "samsung, exynos-bus" compatible devices, which already have their corresponding exynos-bus driver in the devfreq subsystem. Complementing the devfreq driver with an interconnect functionality allows to ensure the QoS requirements of devices accessing the system memory (e.g. video processing devices) are fulfilled and aallows to avoid issues like the one discussed in thread [1]. This patch series adds implementation of the interconnect provider per each "samsung,exynos-bus" compatible DT node, with one interconnect node per provider. The interconnect code which was previously added as a part of the devfreq driver has been converted to a separate platform driver. In the devfreq a corresponding virtual child platform device is registered. Integration of devfreq and interconnect frameworks is achieved through the PM QoS API. A sample interconnect consumer for exynos-mixer is added in patches 6/7, 7/7, it is currently added only for exynos4412 and allows to address the mixer DMA underrun error issues [1]. Changes since v7: - drivers/interconnect/exynos renamed to drivers/interconnect/samsung, - added INTERCONNECT_SAMSUNG Kconfig symbol, - added missing driver sync_state callback, - improved the DT binding description, - added a patch adding maintainers entry, - updated comment in patch 7/7, typo fix (patch 1/7). The series has been tested on Odroid U3 board. It is based on v5.10-rc1. -- Regards, Sylwester Changes since v6: - the interconnect consumer DT bindings are now used to describe dependencies of the interconnects (samsung,exynos-bus nodes), - bus-width property replaced with samsung,data-clk-ratio, - adaptation to recent changes in the interconnect code (of_icc_get_from_provider(), icc_node_add()). Changes since v5: - addition of "bus-width: DT property, which specifies data width of the interconnect bus (patches 1...2/6), - addition of synchronization of the interconnect bandwidth setting with VSYNC (patch 6/6). Changes since v3 [4] (v4 skipped to align with patchset [1]), detailed changes are listed in patches: - conversion to a separate interconnect (platform) driver, - an update of the DT binding documenting new optional properties: #interconnect-cells, samsung,interconnect-parent in "samsung,exynos-bus" nodes, - new DT properties added to the SoC, rather than to the board specific files. Changes since v2 [5]: - Use icc_std_aggregate(). - Implement a different modification of apply_constraints() in drivers/interconnect/core.c (patch 03). - Use 'exynos,interconnect-parent-node' in the DT instead of 'devfreq'/'parent', depending on the bus. - Rebase on DT patches that deprecate the 'devfreq' DT property. - Improve error handling, including freeing generated IDs on failure. - Remove exynos_bus_icc_connect() and add exynos_bus_icc_get_parent(). Changes since v1 [6]: - Rebase on coupled regulators patches. - Use dev_pm_qos_*() API instead of overriding frequency in exynos_bus_target(). - Use IDR for node ID allocation. - Reverse order of multiplication and division in mixer_set_memory_bandwidth() (patch 07) to avoid integer overflow. References: [1] https://patchwork.kernel.org/patch/10861757/ (original issue) [2] https://www.spinics.net/lists/linux-samsung-soc/msg70014.html [3] https://www.spinics.net/lists/arm-kernel/msg810722.html [4] https://lore.kernel.org/linux-pm/20191220115653.6487-1-a.swigon@samsung.com [5] https://patchwork.kernel.org/cover/11054417/ (v1 of this RFC) [6] https://patchwork.kernel.org/cover/11152595/ (v2 of this RFC) Artur Świgoń (1): ARM: dts: exynos: Add interconnects to Exynos4412 mixer Sylwester Nawrocki (6): dt-bindings: devfreq: Add documentation for the interconnect properties interconnect: Add generic interconnect driver for Exynos SoCs MAINTAINERS: Add entry for Samsung interconnect drivers PM / devfreq: exynos-bus: Add registration of interconnect child device ARM: dts: exynos: Add interconnect properties to Exynos4412 bus nodes drm: exynos: mixer: Add interconnect support .../devicetree/bindings/devfreq/exynos-bus.txt | 71 +++++++- MAINTAINERS | 7 + arch/arm/boot/dts/exynos4412.dtsi | 7 + drivers/devfreq/exynos-bus.c | 17 ++ drivers/gpu/drm/exynos/exynos_mixer.c | 146 ++++++++++++++- drivers/interconnect/Kconfig | 1 + drivers/interconnect/Makefile | 1 + drivers/interconnect/samsung/Kconfig | 13 ++ drivers/interconnect/samsung/Makefile | 4 + drivers/interconnect/samsung/exynos.c | 199 +++++++++++++++++++++ 10 files changed, 456 insertions(+), 10 deletions(-) create mode 100644 drivers/interconnect/samsung/Kconfig create mode 100644 drivers/interconnect/samsung/Makefile create mode 100644 drivers/interconnect/samsung/exynos.c -- 2.7.4 ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <CGME20201104103718eucas1p1c103f1a96499b03c72e5457ac2542c3d@eucas1p1.samsung.com>]
* [PATCH v8 1/7] dt-bindings: devfreq: Add documentation for the interconnect properties [not found] ` <CGME20201104103718eucas1p1c103f1a96499b03c72e5457ac2542c3d@eucas1p1.samsung.com> @ 2020-11-04 10:36 ` Sylwester Nawrocki 2020-11-04 12:25 ` Krzysztof Kozlowski ` (2 more replies) 0 siblings, 3 replies; 23+ messages in thread From: Sylwester Nawrocki @ 2020-11-04 10:36 UTC (permalink / raw) To: georgi.djakov, cw00.choi, krzk Cc: devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel, s.nawrocki Add documentation for new optional properties in the exynos bus nodes: interconnects, #interconnect-cells, samsung,data-clock-ratio. These properties allow to specify the SoC interconnect structure which then allows the interconnect consumer devices to request specific bandwidth requirements. Signed-off-by: Artur Świgoń <a.swigon@samsung.com> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> --- Changes for v8: - updated description of the interconnects property, - fixed typo in samsung,data-clk-ratio property description. Changes for v7: - bus-width property replaced with samsung,data-clock-ratio, - the interconnect consumer bindings used instead of vendor specific properties Changes for v6: - added dts example of bus hierarchy definition and the interconnect consumer, - added new bus-width property. Changes for v5: - exynos,interconnect-parent-node renamed to samsung,interconnect-parent --- .../devicetree/bindings/devfreq/exynos-bus.txt | 71 +++++++++++++++++++++- 1 file changed, 69 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt index e71f752..bcaa2c0 100644 --- a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt +++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt @@ -51,6 +51,19 @@ Optional properties only for parent bus device: - exynos,saturation-ratio: the percentage value which is used to calibrate the performance count against total cycle count. +Optional properties for the interconnect functionality (QoS frequency +constraints): +- #interconnect-cells: should be 0. +- interconnects: as documented in ../interconnect.txt, describes a path at the + higher level interconnects used by this interconnect provider. + If this interconnect provider is directly linked to a top level interconnect + provider the property contains only one phandle. The provider extends + the interconnect graph by linking its node to a node registered by provider + pointed to by first phandle in the 'interconnects' property. + +- samsung,data-clock-ratio: ratio of the data throughput in B/s to minimum data + clock frequency in Hz, default value is 8 when this property is missing. + Detailed correlation between sub-blocks and power line according to Exynos SoC: - In case of Exynos3250, there are two power line as following: VDD_MIF |--- DMC @@ -135,7 +148,7 @@ Detailed correlation between sub-blocks and power line according to Exynos SoC: |--- PERIC (Fixed clock rate) |--- FSYS (Fixed clock rate) -Example1: +Example 1: Show the AXI buses of Exynos3250 SoC. Exynos3250 divides the buses to power line (regulator). The MIF (Memory Interface) AXI bus is used to transfer data between DRAM and CPU and uses the VDD_MIF regulator. @@ -184,7 +197,7 @@ Example1: |L5 |200000 |200000 |400000 |300000 | ||1000000 | ---------------------------------------------------------- -Example2 : +Example 2: The bus of DMC (Dynamic Memory Controller) block in exynos3250.dtsi is listed below: @@ -419,3 +432,57 @@ Example2 : devfreq = <&bus_leftbus>; status = "okay"; }; + +Example 3: + An interconnect path "bus_display -- bus_leftbus -- bus_dmc" on + Exynos4412 SoC with video mixer as an interconnect consumer device. + + soc { + bus_dmc: bus_dmc { + compatible = "samsung,exynos-bus"; + clocks = <&clock CLK_DIV_DMC>; + clock-names = "bus"; + operating-points-v2 = <&bus_dmc_opp_table>; + samsung,data-clock-ratio = <4>; + #interconnect-cells = <0>; + }; + + bus_leftbus: bus_leftbus { + compatible = "samsung,exynos-bus"; + clocks = <&clock CLK_DIV_GDL>; + clock-names = "bus"; + operating-points-v2 = <&bus_leftbus_opp_table>; + #interconnect-cells = <0>; + interconnects = <&bus_dmc>; + }; + + bus_display: bus_display { + compatible = "samsung,exynos-bus"; + clocks = <&clock CLK_ACLK160>; + clock-names = "bus"; + operating-points-v2 = <&bus_display_opp_table>; + #interconnect-cells = <0>; + interconnects = <&bus_leftbus &bus_dmc>; + }; + + bus_dmc_opp_table: opp_table1 { + compatible = "operating-points-v2"; + /* ... */ + } + + bus_leftbus_opp_table: opp_table3 { + compatible = "operating-points-v2"; + /* ... */ + }; + + bus_display_opp_table: opp_table4 { + compatible = "operating-points-v2"; + /* .. */ + }; + + &mixer { + compatible = "samsung,exynos4212-mixer"; + interconnects = <&bus_display &bus_dmc>; + /* ... */ + }; + }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v8 1/7] dt-bindings: devfreq: Add documentation for the interconnect properties 2020-11-04 10:36 ` [PATCH v8 1/7] dt-bindings: devfreq: Add documentation for the interconnect properties Sylwester Nawrocki @ 2020-11-04 12:25 ` Krzysztof Kozlowski 2020-11-04 12:28 ` Chanwoo Choi 2020-11-10 14:26 ` Rob Herring 2 siblings, 0 replies; 23+ messages in thread From: Krzysztof Kozlowski @ 2020-11-04 12:25 UTC (permalink / raw) To: Sylwester Nawrocki Cc: georgi.djakov, cw00.choi, devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel On Wed, Nov 04, 2020 at 11:36:51AM +0100, Sylwester Nawrocki wrote: > Add documentation for new optional properties in the exynos bus nodes: > interconnects, #interconnect-cells, samsung,data-clock-ratio. > These properties allow to specify the SoC interconnect structure which > then allows the interconnect consumer devices to request specific > bandwidth requirements. > > Signed-off-by: Artur Świgoń <a.swigon@samsung.com> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > Changes for v8: > - updated description of the interconnects property, > - fixed typo in samsung,data-clk-ratio property description. > > Changes for v7: > - bus-width property replaced with samsung,data-clock-ratio, > - the interconnect consumer bindings used instead of vendor specific > properties > > Changes for v6: > - added dts example of bus hierarchy definition and the interconnect > consumer, > - added new bus-width property. > > Changes for v5: > - exynos,interconnect-parent-node renamed to samsung,interconnect-parent > --- > .../devicetree/bindings/devfreq/exynos-bus.txt | 71 +++++++++++++++++++++- > 1 file changed, 69 insertions(+), 2 deletions(-) I already acked it and there are no signigicant changes from v7. Acked-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v8 1/7] dt-bindings: devfreq: Add documentation for the interconnect properties 2020-11-04 10:36 ` [PATCH v8 1/7] dt-bindings: devfreq: Add documentation for the interconnect properties Sylwester Nawrocki 2020-11-04 12:25 ` Krzysztof Kozlowski @ 2020-11-04 12:28 ` Chanwoo Choi 2020-11-10 14:26 ` Rob Herring 2 siblings, 0 replies; 23+ messages in thread From: Chanwoo Choi @ 2020-11-04 12:28 UTC (permalink / raw) To: Sylwester Nawrocki Cc: Georgi Djakov, Chanwoo Choi, Krzysztof Kozlowski, devicetree, linux-samsung-soc, Bartlomiej Zolnierkiewicz, Linux PM list, Seung-Woo Kim, Artur Świgoń, Rob Herring, linux-kernel, MyungJoo Ham, dri-devel, linux-arm-kernel, Marek Szyprowski Hi Sylwester, On Wed, Nov 4, 2020 at 7:37 PM Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > > Add documentation for new optional properties in the exynos bus nodes: > interconnects, #interconnect-cells, samsung,data-clock-ratio. > These properties allow to specify the SoC interconnect structure which > then allows the interconnect consumer devices to request specific > bandwidth requirements. > > Signed-off-by: Artur Świgoń <a.swigon@samsung.com> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > Changes for v8: > - updated description of the interconnects property, > - fixed typo in samsung,data-clk-ratio property description. > > Changes for v7: > - bus-width property replaced with samsung,data-clock-ratio, > - the interconnect consumer bindings used instead of vendor specific > properties > > Changes for v6: > - added dts example of bus hierarchy definition and the interconnect > consumer, > - added new bus-width property. > > Changes for v5: > - exynos,interconnect-parent-node renamed to samsung,interconnect-parent > --- > .../devicetree/bindings/devfreq/exynos-bus.txt | 71 +++++++++++++++++++++- > 1 file changed, 69 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt > index e71f752..bcaa2c0 100644 > --- a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt > +++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt > @@ -51,6 +51,19 @@ Optional properties only for parent bus device: > - exynos,saturation-ratio: the percentage value which is used to calibrate > the performance count against total cycle count. > > +Optional properties for the interconnect functionality (QoS frequency > +constraints): > +- #interconnect-cells: should be 0. > +- interconnects: as documented in ../interconnect.txt, describes a path at the > + higher level interconnects used by this interconnect provider. > + If this interconnect provider is directly linked to a top level interconnect > + provider the property contains only one phandle. The provider extends > + the interconnect graph by linking its node to a node registered by provider > + pointed to by first phandle in the 'interconnects' property. > + > +- samsung,data-clock-ratio: ratio of the data throughput in B/s to minimum data > + clock frequency in Hz, default value is 8 when this property is missing. > + > Detailed correlation between sub-blocks and power line according to Exynos SoC: > - In case of Exynos3250, there are two power line as following: > VDD_MIF |--- DMC > @@ -135,7 +148,7 @@ Detailed correlation between sub-blocks and power line according to Exynos SoC: > |--- PERIC (Fixed clock rate) > |--- FSYS (Fixed clock rate) > > -Example1: > +Example 1: > Show the AXI buses of Exynos3250 SoC. Exynos3250 divides the buses to > power line (regulator). The MIF (Memory Interface) AXI bus is used to > transfer data between DRAM and CPU and uses the VDD_MIF regulator. > @@ -184,7 +197,7 @@ Example1: > |L5 |200000 |200000 |400000 |300000 | ||1000000 | > ---------------------------------------------------------- > > -Example2 : > +Example 2: > The bus of DMC (Dynamic Memory Controller) block in exynos3250.dtsi > is listed below: > > @@ -419,3 +432,57 @@ Example2 : > devfreq = <&bus_leftbus>; > status = "okay"; > }; > + > +Example 3: > + An interconnect path "bus_display -- bus_leftbus -- bus_dmc" on > + Exynos4412 SoC with video mixer as an interconnect consumer device. > + > + soc { > + bus_dmc: bus_dmc { > + compatible = "samsung,exynos-bus"; > + clocks = <&clock CLK_DIV_DMC>; > + clock-names = "bus"; > + operating-points-v2 = <&bus_dmc_opp_table>; > + samsung,data-clock-ratio = <4>; > + #interconnect-cells = <0>; > + }; > + > + bus_leftbus: bus_leftbus { > + compatible = "samsung,exynos-bus"; > + clocks = <&clock CLK_DIV_GDL>; > + clock-names = "bus"; > + operating-points-v2 = <&bus_leftbus_opp_table>; > + #interconnect-cells = <0>; > + interconnects = <&bus_dmc>; > + }; > + > + bus_display: bus_display { > + compatible = "samsung,exynos-bus"; > + clocks = <&clock CLK_ACLK160>; > + clock-names = "bus"; > + operating-points-v2 = <&bus_display_opp_table>; > + #interconnect-cells = <0>; > + interconnects = <&bus_leftbus &bus_dmc>; > + }; > + > + bus_dmc_opp_table: opp_table1 { > + compatible = "operating-points-v2"; > + /* ... */ > + } > + > + bus_leftbus_opp_table: opp_table3 { > + compatible = "operating-points-v2"; > + /* ... */ > + }; > + > + bus_display_opp_table: opp_table4 { > + compatible = "operating-points-v2"; > + /* .. */ > + }; > + > + &mixer { > + compatible = "samsung,exynos4212-mixer"; > + interconnects = <&bus_display &bus_dmc>; > + /* ... */ > + }; > + }; > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel Acked-by: Chanwoo Choi <cw00.choi@samsung.com> Thanks for your work. -- Best Regards, Chanwoo Choi ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v8 1/7] dt-bindings: devfreq: Add documentation for the interconnect properties 2020-11-04 10:36 ` [PATCH v8 1/7] dt-bindings: devfreq: Add documentation for the interconnect properties Sylwester Nawrocki 2020-11-04 12:25 ` Krzysztof Kozlowski 2020-11-04 12:28 ` Chanwoo Choi @ 2020-11-10 14:26 ` Rob Herring 2 siblings, 0 replies; 23+ messages in thread From: Rob Herring @ 2020-11-10 14:26 UTC (permalink / raw) To: Sylwester Nawrocki Cc: krzk, sw0312.kim, myungjoo.ham, devicetree, georgi.djakov, dri-devel, cw00.choi, b.zolnierkie, linux-pm, linux-kernel, robh+dt, linux-samsung-soc, linux-arm-kernel, m.szyprowski, a.swigon On Wed, 04 Nov 2020 11:36:51 +0100, Sylwester Nawrocki wrote: > Add documentation for new optional properties in the exynos bus nodes: > interconnects, #interconnect-cells, samsung,data-clock-ratio. > These properties allow to specify the SoC interconnect structure which > then allows the interconnect consumer devices to request specific > bandwidth requirements. > > Signed-off-by: Artur Świgoń <a.swigon@samsung.com> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > Changes for v8: > - updated description of the interconnects property, > - fixed typo in samsung,data-clk-ratio property description. > > Changes for v7: > - bus-width property replaced with samsung,data-clock-ratio, > - the interconnect consumer bindings used instead of vendor specific > properties > > Changes for v6: > - added dts example of bus hierarchy definition and the interconnect > consumer, > - added new bus-width property. > > Changes for v5: > - exynos,interconnect-parent-node renamed to samsung,interconnect-parent > --- > .../devicetree/bindings/devfreq/exynos-bus.txt | 71 +++++++++++++++++++++- > 1 file changed, 69 insertions(+), 2 deletions(-) > Acked-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <CGME20201104103720eucas1p1014217e751a681796ed508af22c6bb12@eucas1p1.samsung.com>]
* [PATCH v8 2/7] interconnect: Add generic interconnect driver for Exynos SoCs [not found] ` <CGME20201104103720eucas1p1014217e751a681796ed508af22c6bb12@eucas1p1.samsung.com> @ 2020-11-04 10:36 ` Sylwester Nawrocki 2020-11-04 12:28 ` Chanwoo Choi 2020-11-04 12:37 ` Krzysztof Kozlowski 0 siblings, 2 replies; 23+ messages in thread From: Sylwester Nawrocki @ 2020-11-04 10:36 UTC (permalink / raw) To: georgi.djakov, cw00.choi, krzk Cc: devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel, s.nawrocki This patch adds a generic interconnect driver for Exynos SoCs in order to provide interconnect functionality for each "samsung,exynos-bus" compatible device. The SoC topology is a graph (or more specifically, a tree) and its edges are described by specifying in the 'interconnects' property the interconnect consumer path for each interconnect provider DT node. Each bus is now an interconnect provider and an interconnect node as well (cf. Documentation/interconnect/interconnect.rst), i.e. every bus registers itself as a node. Node IDs are not hard coded but rather assigned dynamically at runtime. This approach allows for using this driver with various Exynos SoCs. Frequencies requested via the interconnect API for a given node are propagated to devfreq using dev_pm_qos_update_request(). Please note that it is not an error when CONFIG_INTERCONNECT is 'n', in which case all interconnect API functions are no-op. The samsung,data-clk-ratio DT property is used to specify the ratio of the interconect bandwidth to the minimum data clock frequency for each bus. Due to unspecified relative probing order, -EPROBE_DEFER may be propagated to ensure that the parent is probed before its children. Signed-off-by: Artur Świgoń <a.swigon@samsung.com> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> --- Changes for v8: - renamed drivers/interconnect/exynos to drivers/interconnect/samsung, - added missing driver sync_state callback assignment. Changes for v7: - adjusted to the DT property changes: "interconnects" instead of "samsung,interconnect-parent", "samsung,data-clk-ratio" instead of "bus-width", - adaptation to of_icc_get_from_provider() function changes in v5.10-rc1. Changes for v6: - corrected of_node dereferencing in exynos_icc_get_parent() function, - corrected initialization of icc_node->name so as to avoid direct of_node->name dereferencing, - added parsing of bus-width DT property. Changes for v5: - adjust to renamed exynos,interconnect-parent-node property, - use automatically generated platform device id as the interconect node id instead of a now unavailable devfreq->id field, - add icc_ prefix to some variables to make the code more self-commenting, - use icc_nodes_remove() instead of icc_node_del() + icc_node_destroy(), - adjust to exynos,interconnect-parent-node property rename to samsung,interconnect-parent, - converted to a separate platform driver in drivers/interconnect. --- drivers/interconnect/Kconfig | 1 + drivers/interconnect/Makefile | 1 + drivers/interconnect/samsung/Kconfig | 13 +++ drivers/interconnect/samsung/Makefile | 4 + drivers/interconnect/samsung/exynos.c | 199 ++++++++++++++++++++++++++++++++++ 5 files changed, 218 insertions(+) create mode 100644 drivers/interconnect/samsung/Kconfig create mode 100644 drivers/interconnect/samsung/Makefile create mode 100644 drivers/interconnect/samsung/exynos.c diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig index 5b7204e..d637a89 100644 --- a/drivers/interconnect/Kconfig +++ b/drivers/interconnect/Kconfig @@ -13,5 +13,6 @@ if INTERCONNECT source "drivers/interconnect/imx/Kconfig" source "drivers/interconnect/qcom/Kconfig" +source "drivers/interconnect/samsung/Kconfig" endif diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile index d203520..c2f9e9d 100644 --- a/drivers/interconnect/Makefile +++ b/drivers/interconnect/Makefile @@ -6,3 +6,4 @@ icc-core-objs := core.o bulk.o obj-$(CONFIG_INTERCONNECT) += icc-core.o obj-$(CONFIG_INTERCONNECT_IMX) += imx/ obj-$(CONFIG_INTERCONNECT_QCOM) += qcom/ +obj-$(CONFIG_INTERCONNECT_SAMSUNG) += samsung/ \ No newline at end of file diff --git a/drivers/interconnect/samsung/Kconfig b/drivers/interconnect/samsung/Kconfig new file mode 100644 index 0000000..508ed64 --- /dev/null +++ b/drivers/interconnect/samsung/Kconfig @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: GPL-2.0-only +config INTERCONNECT_SAMSUNG + bool "Samsung interconnect drivers" + depends on ARCH_EXYNOS || COMPILE_TEST + help + Interconnect drivers for Samsung SoCs. + + +config INTERCONNECT_EXYNOS + tristate "Exynos generic interconnect driver" + depends on INTERCONNECT_SAMSUNG + help + Generic interconnect driver for Exynos SoCs. diff --git a/drivers/interconnect/samsung/Makefile b/drivers/interconnect/samsung/Makefile new file mode 100644 index 0000000..e19d1df --- /dev/null +++ b/drivers/interconnect/samsung/Makefile @@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0 +exynos-interconnect-objs := exynos.o + +obj-$(CONFIG_INTERCONNECT_EXYNOS) += exynos-interconnect.o diff --git a/drivers/interconnect/samsung/exynos.c b/drivers/interconnect/samsung/exynos.c new file mode 100644 index 0000000..6559d8c --- /dev/null +++ b/drivers/interconnect/samsung/exynos.c @@ -0,0 +1,199 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Exynos generic interconnect provider driver + * + * Copyright (c) 2020 Samsung Electronics Co., Ltd. + * + * Authors: Artur Świgoń <a.swigon@samsung.com> + * Sylwester Nawrocki <s.nawrocki@samsung.com> + */ +#include <linux/device.h> +#include <linux/interconnect-provider.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/pm_qos.h> +#include <linux/slab.h> + +#define EXYNOS_ICC_DEFAULT_BUS_CLK_RATIO 8 + +struct exynos_icc_priv { + struct device *dev; + + /* One interconnect node per provider */ + struct icc_provider provider; + struct icc_node *node; + + struct dev_pm_qos_request qos_req; + u32 bus_clk_ratio; +}; + +static struct icc_node *exynos_icc_get_parent(struct device_node *np) +{ + struct of_phandle_args args; + struct icc_node_data *icc_node_data; + struct icc_node *icc_node; + int num, ret; + + num = of_count_phandle_with_args(np, "interconnects", + "#interconnect-cells"); + if (num < 1) + return NULL; /* parent nodes are optional */ + + /* Get the interconnect target node */ + ret = of_parse_phandle_with_args(np, "interconnects", + "#interconnect-cells", 0, &args); + if (ret < 0) + return ERR_PTR(ret); + + icc_node_data = of_icc_get_from_provider(&args); + of_node_put(args.np); + + if (IS_ERR(icc_node_data)) + return ERR_CAST(icc_node_data); + + icc_node = icc_node_data->node; + kfree(icc_node_data); + + return icc_node; +} + +static int exynos_generic_icc_set(struct icc_node *src, struct icc_node *dst) +{ + struct exynos_icc_priv *src_priv = src->data, *dst_priv = dst->data; + s32 src_freq = max(src->avg_bw, src->peak_bw) / src_priv->bus_clk_ratio; + s32 dst_freq = max(dst->avg_bw, dst->peak_bw) / dst_priv->bus_clk_ratio; + int ret; + + ret = dev_pm_qos_update_request(&src_priv->qos_req, src_freq); + if (ret < 0) { + dev_err(src_priv->dev, "failed to update PM QoS of %s (src)\n", + src->name); + return ret; + } + + ret = dev_pm_qos_update_request(&dst_priv->qos_req, dst_freq); + if (ret < 0) { + dev_err(dst_priv->dev, "failed to update PM QoS of %s (dst)\n", + dst->name); + return ret; + } + + return 0; +} + +static struct icc_node *exynos_generic_icc_xlate(struct of_phandle_args *spec, + void *data) +{ + struct exynos_icc_priv *priv = data; + + if (spec->np != priv->dev->parent->of_node) + return ERR_PTR(-EINVAL); + + return priv->node; +} + +static int exynos_generic_icc_remove(struct platform_device *pdev) +{ + struct exynos_icc_priv *priv = platform_get_drvdata(pdev); + struct icc_node *parent_node, *node = priv->node; + + parent_node = exynos_icc_get_parent(priv->dev->parent->of_node); + if (parent_node && !IS_ERR(parent_node)) + icc_link_destroy(node, parent_node); + + icc_nodes_remove(&priv->provider); + icc_provider_del(&priv->provider); + + return 0; +} + +static int exynos_generic_icc_probe(struct platform_device *pdev) +{ + struct device *bus_dev = pdev->dev.parent; + struct exynos_icc_priv *priv; + struct icc_provider *provider; + struct icc_node *icc_node, *icc_parent_node; + int ret; + + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->dev = &pdev->dev; + platform_set_drvdata(pdev, priv); + + provider = &priv->provider; + + provider->set = exynos_generic_icc_set; + provider->aggregate = icc_std_aggregate; + provider->xlate = exynos_generic_icc_xlate; + provider->dev = bus_dev; + provider->inter_set = true; + provider->data = priv; + + ret = icc_provider_add(provider); + if (ret < 0) + return ret; + + icc_node = icc_node_create(pdev->id); + if (IS_ERR(icc_node)) { + ret = PTR_ERR(icc_node); + goto err_prov_del; + } + + priv->node = icc_node; + icc_node->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%pOFn", + bus_dev->of_node); + if (of_property_read_u32(bus_dev->of_node, "samsung,data-clock-ratio", + &priv->bus_clk_ratio)) + priv->bus_clk_ratio = EXYNOS_ICC_DEFAULT_BUS_CLK_RATIO; + + /* + * Register a PM QoS request for the parent (devfreq) device. + */ + ret = dev_pm_qos_add_request(bus_dev, &priv->qos_req, + DEV_PM_QOS_MIN_FREQUENCY, 0); + if (ret < 0) + goto err_node_del; + + icc_node->data = priv; + icc_node_add(icc_node, provider); + + icc_parent_node = exynos_icc_get_parent(bus_dev->of_node); + if (IS_ERR(icc_parent_node)) { + ret = PTR_ERR(icc_parent_node); + goto err_pmqos_del; + } + if (icc_parent_node) { + ret = icc_link_create(icc_node, icc_parent_node->id); + if (ret < 0) + goto err_pmqos_del; + } + + return 0; + +err_pmqos_del: + dev_pm_qos_remove_request(&priv->qos_req); +err_node_del: + icc_nodes_remove(provider); +err_prov_del: + icc_provider_del(provider); + return ret; +} + +static struct platform_driver exynos_generic_icc_driver = { + .driver = { + .name = "exynos-generic-icc", + .sync_state = icc_sync_state, + }, + .probe = exynos_generic_icc_probe, + .remove = exynos_generic_icc_remove, +}; +module_platform_driver(exynos_generic_icc_driver); + +MODULE_DESCRIPTION("Exynos generic interconnect driver"); +MODULE_AUTHOR("Artur Świgoń <a.swigon@samsung.com>"); +MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:exynos-generic-icc"); -- 2.7.4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v8 2/7] interconnect: Add generic interconnect driver for Exynos SoCs 2020-11-04 10:36 ` [PATCH v8 2/7] interconnect: Add generic interconnect driver for Exynos SoCs Sylwester Nawrocki @ 2020-11-04 12:28 ` Chanwoo Choi 2020-11-04 12:37 ` Krzysztof Kozlowski 1 sibling, 0 replies; 23+ messages in thread From: Chanwoo Choi @ 2020-11-04 12:28 UTC (permalink / raw) To: Sylwester Nawrocki Cc: Georgi Djakov, Chanwoo Choi, Krzysztof Kozlowski, devicetree, linux-samsung-soc, Bartlomiej Zolnierkiewicz, Linux PM list, Seung-Woo Kim, Artur Świgoń, Rob Herring, linux-kernel, MyungJoo Ham, dri-devel, linux-arm-kernel, Marek Szyprowski Hi Sylwester, On Wed, Nov 4, 2020 at 7:37 PM Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > > This patch adds a generic interconnect driver for Exynos SoCs in order > to provide interconnect functionality for each "samsung,exynos-bus" > compatible device. > > The SoC topology is a graph (or more specifically, a tree) and its > edges are described by specifying in the 'interconnects' property > the interconnect consumer path for each interconnect provider DT node. > > Each bus is now an interconnect provider and an interconnect node as > well (cf. Documentation/interconnect/interconnect.rst), i.e. every bus > registers itself as a node. Node IDs are not hard coded but rather > assigned dynamically at runtime. This approach allows for using this > driver with various Exynos SoCs. > > Frequencies requested via the interconnect API for a given node are > propagated to devfreq using dev_pm_qos_update_request(). Please note > that it is not an error when CONFIG_INTERCONNECT is 'n', in which > case all interconnect API functions are no-op. > > The samsung,data-clk-ratio DT property is used to specify the ratio > of the interconect bandwidth to the minimum data clock frequency > for each bus. > > Due to unspecified relative probing order, -EPROBE_DEFER may be > propagated to ensure that the parent is probed before its children. > > Signed-off-by: Artur Świgoń <a.swigon@samsung.com> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > Changes for v8: > - renamed drivers/interconnect/exynos to drivers/interconnect/samsung, > - added missing driver sync_state callback assignment. > > Changes for v7: > - adjusted to the DT property changes: "interconnects" instead > of "samsung,interconnect-parent", "samsung,data-clk-ratio" > instead of "bus-width", > - adaptation to of_icc_get_from_provider() function changes > in v5.10-rc1. > > Changes for v6: > - corrected of_node dereferencing in exynos_icc_get_parent() > function, > - corrected initialization of icc_node->name so as to avoid > direct of_node->name dereferencing, > - added parsing of bus-width DT property. > > Changes for v5: > - adjust to renamed exynos,interconnect-parent-node property, > - use automatically generated platform device id as the interconect > node id instead of a now unavailable devfreq->id field, > - add icc_ prefix to some variables to make the code more self-commenting, > - use icc_nodes_remove() instead of icc_node_del() + icc_node_destroy(), > - adjust to exynos,interconnect-parent-node property rename to > samsung,interconnect-parent, > - converted to a separate platform driver in drivers/interconnect. > --- > drivers/interconnect/Kconfig | 1 + > drivers/interconnect/Makefile | 1 + > drivers/interconnect/samsung/Kconfig | 13 +++ > drivers/interconnect/samsung/Makefile | 4 + > drivers/interconnect/samsung/exynos.c | 199 ++++++++++++++++++++++++++++++++++ > 5 files changed, 218 insertions(+) > create mode 100644 drivers/interconnect/samsung/Kconfig > create mode 100644 drivers/interconnect/samsung/Makefile > create mode 100644 drivers/interconnect/samsung/exynos.c > > diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig > index 5b7204e..d637a89 100644 > --- a/drivers/interconnect/Kconfig > +++ b/drivers/interconnect/Kconfig > @@ -13,5 +13,6 @@ if INTERCONNECT > > source "drivers/interconnect/imx/Kconfig" > source "drivers/interconnect/qcom/Kconfig" > +source "drivers/interconnect/samsung/Kconfig" > > endif > diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile > index d203520..c2f9e9d 100644 > --- a/drivers/interconnect/Makefile > +++ b/drivers/interconnect/Makefile > @@ -6,3 +6,4 @@ icc-core-objs := core.o bulk.o > obj-$(CONFIG_INTERCONNECT) += icc-core.o > obj-$(CONFIG_INTERCONNECT_IMX) += imx/ > obj-$(CONFIG_INTERCONNECT_QCOM) += qcom/ > +obj-$(CONFIG_INTERCONNECT_SAMSUNG) += samsung/ > \ No newline at end of file > diff --git a/drivers/interconnect/samsung/Kconfig b/drivers/interconnect/samsung/Kconfig > new file mode 100644 > index 0000000..508ed64 > --- /dev/null > +++ b/drivers/interconnect/samsung/Kconfig > @@ -0,0 +1,13 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +config INTERCONNECT_SAMSUNG > + bool "Samsung interconnect drivers" > + depends on ARCH_EXYNOS || COMPILE_TEST > + help > + Interconnect drivers for Samsung SoCs. > + > + > +config INTERCONNECT_EXYNOS > + tristate "Exynos generic interconnect driver" > + depends on INTERCONNECT_SAMSUNG > + help > + Generic interconnect driver for Exynos SoCs. > diff --git a/drivers/interconnect/samsung/Makefile b/drivers/interconnect/samsung/Makefile > new file mode 100644 > index 0000000..e19d1df > --- /dev/null > +++ b/drivers/interconnect/samsung/Makefile > @@ -0,0 +1,4 @@ > +# SPDX-License-Identifier: GPL-2.0 > +exynos-interconnect-objs := exynos.o > + > +obj-$(CONFIG_INTERCONNECT_EXYNOS) += exynos-interconnect.o > diff --git a/drivers/interconnect/samsung/exynos.c b/drivers/interconnect/samsung/exynos.c > new file mode 100644 > index 0000000..6559d8c > --- /dev/null > +++ b/drivers/interconnect/samsung/exynos.c > @@ -0,0 +1,199 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Exynos generic interconnect provider driver > + * > + * Copyright (c) 2020 Samsung Electronics Co., Ltd. > + * > + * Authors: Artur Świgoń <a.swigon@samsung.com> > + * Sylwester Nawrocki <s.nawrocki@samsung.com> > + */ > +#include <linux/device.h> > +#include <linux/interconnect-provider.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/pm_qos.h> > +#include <linux/slab.h> > + > +#define EXYNOS_ICC_DEFAULT_BUS_CLK_RATIO 8 > + > +struct exynos_icc_priv { > + struct device *dev; > + > + /* One interconnect node per provider */ > + struct icc_provider provider; > + struct icc_node *node; > + > + struct dev_pm_qos_request qos_req; > + u32 bus_clk_ratio; > +}; > + > +static struct icc_node *exynos_icc_get_parent(struct device_node *np) > +{ > + struct of_phandle_args args; > + struct icc_node_data *icc_node_data; > + struct icc_node *icc_node; > + int num, ret; > + > + num = of_count_phandle_with_args(np, "interconnects", > + "#interconnect-cells"); > + if (num < 1) > + return NULL; /* parent nodes are optional */ > + > + /* Get the interconnect target node */ > + ret = of_parse_phandle_with_args(np, "interconnects", > + "#interconnect-cells", 0, &args); > + if (ret < 0) > + return ERR_PTR(ret); > + > + icc_node_data = of_icc_get_from_provider(&args); > + of_node_put(args.np); > + > + if (IS_ERR(icc_node_data)) > + return ERR_CAST(icc_node_data); > + > + icc_node = icc_node_data->node; > + kfree(icc_node_data); > + > + return icc_node; > +} > + > +static int exynos_generic_icc_set(struct icc_node *src, struct icc_node *dst) > +{ > + struct exynos_icc_priv *src_priv = src->data, *dst_priv = dst->data; > + s32 src_freq = max(src->avg_bw, src->peak_bw) / src_priv->bus_clk_ratio; > + s32 dst_freq = max(dst->avg_bw, dst->peak_bw) / dst_priv->bus_clk_ratio; > + int ret; > + > + ret = dev_pm_qos_update_request(&src_priv->qos_req, src_freq); > + if (ret < 0) { > + dev_err(src_priv->dev, "failed to update PM QoS of %s (src)\n", > + src->name); > + return ret; > + } > + > + ret = dev_pm_qos_update_request(&dst_priv->qos_req, dst_freq); > + if (ret < 0) { > + dev_err(dst_priv->dev, "failed to update PM QoS of %s (dst)\n", > + dst->name); > + return ret; > + } > + > + return 0; > +} > + > +static struct icc_node *exynos_generic_icc_xlate(struct of_phandle_args *spec, > + void *data) > +{ > + struct exynos_icc_priv *priv = data; > + > + if (spec->np != priv->dev->parent->of_node) > + return ERR_PTR(-EINVAL); > + > + return priv->node; > +} > + > +static int exynos_generic_icc_remove(struct platform_device *pdev) > +{ > + struct exynos_icc_priv *priv = platform_get_drvdata(pdev); > + struct icc_node *parent_node, *node = priv->node; > + > + parent_node = exynos_icc_get_parent(priv->dev->parent->of_node); > + if (parent_node && !IS_ERR(parent_node)) > + icc_link_destroy(node, parent_node); > + > + icc_nodes_remove(&priv->provider); > + icc_provider_del(&priv->provider); > + > + return 0; > +} > + > +static int exynos_generic_icc_probe(struct platform_device *pdev) > +{ > + struct device *bus_dev = pdev->dev.parent; > + struct exynos_icc_priv *priv; > + struct icc_provider *provider; > + struct icc_node *icc_node, *icc_parent_node; > + int ret; > + > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->dev = &pdev->dev; > + platform_set_drvdata(pdev, priv); > + > + provider = &priv->provider; > + > + provider->set = exynos_generic_icc_set; > + provider->aggregate = icc_std_aggregate; > + provider->xlate = exynos_generic_icc_xlate; > + provider->dev = bus_dev; > + provider->inter_set = true; > + provider->data = priv; > + > + ret = icc_provider_add(provider); > + if (ret < 0) > + return ret; > + > + icc_node = icc_node_create(pdev->id); > + if (IS_ERR(icc_node)) { > + ret = PTR_ERR(icc_node); > + goto err_prov_del; > + } > + > + priv->node = icc_node; > + icc_node->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%pOFn", > + bus_dev->of_node); > + if (of_property_read_u32(bus_dev->of_node, "samsung,data-clock-ratio", > + &priv->bus_clk_ratio)) > + priv->bus_clk_ratio = EXYNOS_ICC_DEFAULT_BUS_CLK_RATIO; > + > + /* > + * Register a PM QoS request for the parent (devfreq) device. > + */ > + ret = dev_pm_qos_add_request(bus_dev, &priv->qos_req, > + DEV_PM_QOS_MIN_FREQUENCY, 0); > + if (ret < 0) > + goto err_node_del; > + > + icc_node->data = priv; > + icc_node_add(icc_node, provider); > + > + icc_parent_node = exynos_icc_get_parent(bus_dev->of_node); > + if (IS_ERR(icc_parent_node)) { > + ret = PTR_ERR(icc_parent_node); > + goto err_pmqos_del; > + } > + if (icc_parent_node) { > + ret = icc_link_create(icc_node, icc_parent_node->id); > + if (ret < 0) > + goto err_pmqos_del; > + } > + > + return 0; > + > +err_pmqos_del: > + dev_pm_qos_remove_request(&priv->qos_req); > +err_node_del: > + icc_nodes_remove(provider); > +err_prov_del: > + icc_provider_del(provider); > + return ret; > +} > + > +static struct platform_driver exynos_generic_icc_driver = { > + .driver = { > + .name = "exynos-generic-icc", > + .sync_state = icc_sync_state, > + }, > + .probe = exynos_generic_icc_probe, > + .remove = exynos_generic_icc_remove, > +}; > +module_platform_driver(exynos_generic_icc_driver); > + > +MODULE_DESCRIPTION("Exynos generic interconnect driver"); > +MODULE_AUTHOR("Artur Świgoń <a.swigon@samsung.com>"); > +MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>"); > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("platform:exynos-generic-icc"); > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> -- Best Regards, Chanwoo Choi ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v8 2/7] interconnect: Add generic interconnect driver for Exynos SoCs 2020-11-04 10:36 ` [PATCH v8 2/7] interconnect: Add generic interconnect driver for Exynos SoCs Sylwester Nawrocki 2020-11-04 12:28 ` Chanwoo Choi @ 2020-11-04 12:37 ` Krzysztof Kozlowski 2020-11-04 13:22 ` Sylwester Nawrocki 1 sibling, 1 reply; 23+ messages in thread From: Krzysztof Kozlowski @ 2020-11-04 12:37 UTC (permalink / raw) To: Sylwester Nawrocki Cc: georgi.djakov, cw00.choi, devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel On Wed, Nov 04, 2020 at 11:36:52AM +0100, Sylwester Nawrocki wrote: > This patch adds a generic interconnect driver for Exynos SoCs in order > to provide interconnect functionality for each "samsung,exynos-bus" > compatible device. > > The SoC topology is a graph (or more specifically, a tree) and its > edges are described by specifying in the 'interconnects' property > the interconnect consumer path for each interconnect provider DT node. > > Each bus is now an interconnect provider and an interconnect node as > well (cf. Documentation/interconnect/interconnect.rst), i.e. every bus > registers itself as a node. Node IDs are not hard coded but rather > assigned dynamically at runtime. This approach allows for using this > driver with various Exynos SoCs. > > Frequencies requested via the interconnect API for a given node are > propagated to devfreq using dev_pm_qos_update_request(). Please note > that it is not an error when CONFIG_INTERCONNECT is 'n', in which > case all interconnect API functions are no-op. > > The samsung,data-clk-ratio DT property is used to specify the ratio > of the interconect bandwidth to the minimum data clock frequency > for each bus. > > Due to unspecified relative probing order, -EPROBE_DEFER may be > propagated to ensure that the parent is probed before its children. > > Signed-off-by: Artur Świgoń <a.swigon@samsung.com> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > Changes for v8: > - renamed drivers/interconnect/exynos to drivers/interconnect/samsung, > - added missing driver sync_state callback assignment. > > Changes for v7: > - adjusted to the DT property changes: "interconnects" instead > of "samsung,interconnect-parent", "samsung,data-clk-ratio" > instead of "bus-width", > - adaptation to of_icc_get_from_provider() function changes > in v5.10-rc1. > > Changes for v6: > - corrected of_node dereferencing in exynos_icc_get_parent() > function, > - corrected initialization of icc_node->name so as to avoid > direct of_node->name dereferencing, > - added parsing of bus-width DT property. > > Changes for v5: > - adjust to renamed exynos,interconnect-parent-node property, > - use automatically generated platform device id as the interconect > node id instead of a now unavailable devfreq->id field, > - add icc_ prefix to some variables to make the code more self-commenting, > - use icc_nodes_remove() instead of icc_node_del() + icc_node_destroy(), > - adjust to exynos,interconnect-parent-node property rename to > samsung,interconnect-parent, > - converted to a separate platform driver in drivers/interconnect. > --- > drivers/interconnect/Kconfig | 1 + > drivers/interconnect/Makefile | 1 + > drivers/interconnect/samsung/Kconfig | 13 +++ > drivers/interconnect/samsung/Makefile | 4 + > drivers/interconnect/samsung/exynos.c | 199 ++++++++++++++++++++++++++++++++++ > 5 files changed, 218 insertions(+) > create mode 100644 drivers/interconnect/samsung/Kconfig > create mode 100644 drivers/interconnect/samsung/Makefile > create mode 100644 drivers/interconnect/samsung/exynos.c > > diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig > index 5b7204e..d637a89 100644 > --- a/drivers/interconnect/Kconfig > +++ b/drivers/interconnect/Kconfig > @@ -13,5 +13,6 @@ if INTERCONNECT > > source "drivers/interconnect/imx/Kconfig" > source "drivers/interconnect/qcom/Kconfig" > +source "drivers/interconnect/samsung/Kconfig" > > endif > diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile > index d203520..c2f9e9d 100644 > --- a/drivers/interconnect/Makefile > +++ b/drivers/interconnect/Makefile > @@ -6,3 +6,4 @@ icc-core-objs := core.o bulk.o > obj-$(CONFIG_INTERCONNECT) += icc-core.o > obj-$(CONFIG_INTERCONNECT_IMX) += imx/ > obj-$(CONFIG_INTERCONNECT_QCOM) += qcom/ > +obj-$(CONFIG_INTERCONNECT_SAMSUNG) += samsung/ > \ No newline at end of file This needs a fix. > diff --git a/drivers/interconnect/samsung/Kconfig b/drivers/interconnect/samsung/Kconfig > new file mode 100644 > index 0000000..508ed64 > --- /dev/null > +++ b/drivers/interconnect/samsung/Kconfig > @@ -0,0 +1,13 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +config INTERCONNECT_SAMSUNG > + bool "Samsung interconnect drivers" "Samsung SoC interconnect drivers" > + depends on ARCH_EXYNOS || COMPILE_TEST Don't the depend on INTERCONNECT? > + help > + Interconnect drivers for Samsung SoCs. > + > + One line break > +config INTERCONNECT_EXYNOS > + tristate "Exynos generic interconnect driver" > + depends on INTERCONNECT_SAMSUNG How about: default y if ARCH_EXYNOS > + help > + Generic interconnect driver for Exynos SoCs. > diff --git a/drivers/interconnect/samsung/Makefile b/drivers/interconnect/samsung/Makefile > new file mode 100644 > index 0000000..e19d1df > --- /dev/null > +++ b/drivers/interconnect/samsung/Makefile > @@ -0,0 +1,4 @@ > +# SPDX-License-Identifier: GPL-2.0 > +exynos-interconnect-objs := exynos.o What is this line for? Best regards, Krzysztof > + > +obj-$(CONFIG_INTERCONNECT_EXYNOS) += exynos-interconnect.o > diff --git a/drivers/interconnect/samsung/exynos.c b/drivers/interconnect/samsung/exynos.c > new file mode 100644 > index 0000000..6559d8c ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v8 2/7] interconnect: Add generic interconnect driver for Exynos SoCs 2020-11-04 12:37 ` Krzysztof Kozlowski @ 2020-11-04 13:22 ` Sylwester Nawrocki 2020-11-04 14:06 ` Krzysztof Kozlowski 0 siblings, 1 reply; 23+ messages in thread From: Sylwester Nawrocki @ 2020-11-04 13:22 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: georgi.djakov, cw00.choi, devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel On 04.11.2020 13:37, Krzysztof Kozlowski wrote: > On Wed, Nov 04, 2020 at 11:36:52AM +0100, Sylwester Nawrocki wrote: >> diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile >> index d203520..c2f9e9d 100644 >> --- a/drivers/interconnect/Makefile >> +++ b/drivers/interconnect/Makefile >> @@ -6,3 +6,4 @@ icc-core-objs := core.o bulk.o >> obj-$(CONFIG_INTERCONNECT) += icc-core.o >> obj-$(CONFIG_INTERCONNECT_IMX) += imx/ >> obj-$(CONFIG_INTERCONNECT_QCOM) += qcom/ >> +obj-$(CONFIG_INTERCONNECT_SAMSUNG) += samsung/ >> \ No newline at end of file > > This needs a fix. Corrected, thanks for pointing out. >> diff --git a/drivers/interconnect/samsung/Kconfig b/drivers/interconnect/samsung/Kconfig >> new file mode 100644 >> index 0000000..508ed64 >> --- /dev/null >> +++ b/drivers/interconnect/samsung/Kconfig >> @@ -0,0 +1,13 @@ >> +# SPDX-License-Identifier: GPL-2.0-only >> +config INTERCONNECT_SAMSUNG >> + bool "Samsung interconnect drivers" > > "Samsung SoC interconnect drivers" Changed. >> + depends on ARCH_EXYNOS || COMPILE_TEST > > Don't the depend on INTERCONNECT? This file gets included only if INTERCONNECT is enabled, see higher level Kconfig file. >> + help >> + Interconnect drivers for Samsung SoCs. >> + >> + > > One line break Fixed. >> +config INTERCONNECT_EXYNOS >> + tristate "Exynos generic interconnect driver" >> + depends on INTERCONNECT_SAMSUNG > > How about: > default y if ARCH_EXYNOS OK, added. >> + help >> + Generic interconnect driver for Exynos SoCs. >> diff --git a/drivers/interconnect/samsung/Makefile b/drivers/interconnect/samsung/Makefile >> new file mode 100644 >> index 0000000..e19d1df >> --- /dev/null >> +++ b/drivers/interconnect/samsung/Makefile >> @@ -0,0 +1,4 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +exynos-interconnect-objs := exynos.o > > What is this line for? That allows to change the module name, so it's exynos-interconnect.ko rather than just exynos.c. It's done similarly for other SoCs in the subsystem. >> +obj-$(CONFIG_INTERCONNECT_EXYNOS) += exynos-interconnect.o -- Regards, Sylwester ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v8 2/7] interconnect: Add generic interconnect driver for Exynos SoCs 2020-11-04 13:22 ` Sylwester Nawrocki @ 2020-11-04 14:06 ` Krzysztof Kozlowski 0 siblings, 0 replies; 23+ messages in thread From: Krzysztof Kozlowski @ 2020-11-04 14:06 UTC (permalink / raw) To: Sylwester Nawrocki Cc: georgi.djakov, cw00.choi, devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel On Wed, Nov 04, 2020 at 02:22:37PM +0100, Sylwester Nawrocki wrote: > On 04.11.2020 13:37, Krzysztof Kozlowski wrote: > > On Wed, Nov 04, 2020 at 11:36:52AM +0100, Sylwester Nawrocki wrote: > > >> diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile > >> index d203520..c2f9e9d 100644 > >> --- a/drivers/interconnect/Makefile > >> +++ b/drivers/interconnect/Makefile > >> @@ -6,3 +6,4 @@ icc-core-objs := core.o bulk.o > >> obj-$(CONFIG_INTERCONNECT) += icc-core.o > >> obj-$(CONFIG_INTERCONNECT_IMX) += imx/ > >> obj-$(CONFIG_INTERCONNECT_QCOM) += qcom/ > >> +obj-$(CONFIG_INTERCONNECT_SAMSUNG) += samsung/ > >> \ No newline at end of file > > > > This needs a fix. > > Corrected, thanks for pointing out. > > >> diff --git a/drivers/interconnect/samsung/Kconfig b/drivers/interconnect/samsung/Kconfig > >> new file mode 100644 > >> index 0000000..508ed64 > >> --- /dev/null > >> +++ b/drivers/interconnect/samsung/Kconfig > >> @@ -0,0 +1,13 @@ > >> +# SPDX-License-Identifier: GPL-2.0-only > >> +config INTERCONNECT_SAMSUNG > >> + bool "Samsung interconnect drivers" > > > > "Samsung SoC interconnect drivers" > > Changed. > > >> + depends on ARCH_EXYNOS || COMPILE_TEST > > > > Don't the depend on INTERCONNECT? > > This file gets included only if INTERCONNECT is enabled, see > higher level Kconfig file. I missed the include part, looks good. > > >> + help > >> + Interconnect drivers for Samsung SoCs. > >> + > >> + > > > > One line break > > Fixed. > > >> +config INTERCONNECT_EXYNOS > >> + tristate "Exynos generic interconnect driver" > >> + depends on INTERCONNECT_SAMSUNG > > > > How about: > > default y if ARCH_EXYNOS > > OK, added. > > >> + help > >> + Generic interconnect driver for Exynos SoCs. > >> diff --git a/drivers/interconnect/samsung/Makefile b/drivers/interconnect/samsung/Makefile > >> new file mode 100644 > >> index 0000000..e19d1df > >> --- /dev/null > >> +++ b/drivers/interconnect/samsung/Makefile > >> @@ -0,0 +1,4 @@ > >> +# SPDX-License-Identifier: GPL-2.0 > >> +exynos-interconnect-objs := exynos.o > > > > What is this line for? > > That allows to change the module name, so it's exynos-interconnect.ko > rather than just exynos.c. It's done similarly for other SoCs in > the subsystem. Thanks, makes sense. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <CGME20201104103722eucas1p1db939995e60d0bf2cd581070c14379f5@eucas1p1.samsung.com>]
* [PATCH v8 3/7] MAINTAINERS: Add entry for Samsung interconnect drivers [not found] ` <CGME20201104103722eucas1p1db939995e60d0bf2cd581070c14379f5@eucas1p1.samsung.com> @ 2020-11-04 10:36 ` Sylwester Nawrocki 2020-11-04 12:27 ` Krzysztof Kozlowski 2020-11-04 12:30 ` Chanwoo Choi 0 siblings, 2 replies; 23+ messages in thread From: Sylwester Nawrocki @ 2020-11-04 10:36 UTC (permalink / raw) To: georgi.djakov, cw00.choi, krzk Cc: devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel, s.nawrocki Add maintainers entry for the Samsung interconnect drivers, this currently includes Exynos SoC generic interconnect driver. Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> --- Changes since v7: - new patch. --- MAINTAINERS | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index e73636b..4bbafef 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9156,6 +9156,13 @@ F: include/dt-bindings/interconnect/ F: include/linux/interconnect-provider.h F: include/linux/interconnect.h +SAMSUNG INTERCONNECT DRIVERS +M: Sylwester Nawrocki <s.nawrocki@samsung.com> +M: Artur Swigoń <a.swigon@samsung.com> +L: linux-pm@vger.kernel.org +S: Supported +F: drivers/interconnect/samsung + INVENSENSE ICM-426xx IMU DRIVER M: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> L: linux-iio@vger.kernel.org -- 2.7.4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v8 3/7] MAINTAINERS: Add entry for Samsung interconnect drivers 2020-11-04 10:36 ` [PATCH v8 3/7] MAINTAINERS: Add entry for Samsung interconnect drivers Sylwester Nawrocki @ 2020-11-04 12:27 ` Krzysztof Kozlowski 2020-11-04 13:27 ` Sylwester Nawrocki 2020-11-04 12:30 ` Chanwoo Choi 1 sibling, 1 reply; 23+ messages in thread From: Krzysztof Kozlowski @ 2020-11-04 12:27 UTC (permalink / raw) To: Sylwester Nawrocki Cc: georgi.djakov, cw00.choi, devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel On Wed, Nov 04, 2020 at 11:36:53AM +0100, Sylwester Nawrocki wrote: > Add maintainers entry for the Samsung interconnect drivers, this currently > includes Exynos SoC generic interconnect driver. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > Changes since v7: > - new patch. > --- > MAINTAINERS | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index e73636b..4bbafef 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9156,6 +9156,13 @@ F: include/dt-bindings/interconnect/ > F: include/linux/interconnect-provider.h > F: include/linux/interconnect.h > > +SAMSUNG INTERCONNECT DRIVERS Does not look like ordered alphabetically. > +M: Sylwester Nawrocki <s.nawrocki@samsung.com> > +M: Artur Swigoń <a.swigon@samsung.com> > +L: linux-pm@vger.kernel.org Also: L: linux-samsung-soc@vger.kernel.org > +S: Supported > +F: drivers/interconnect/samsung Add trailing /. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v8 3/7] MAINTAINERS: Add entry for Samsung interconnect drivers 2020-11-04 12:27 ` Krzysztof Kozlowski @ 2020-11-04 13:27 ` Sylwester Nawrocki 0 siblings, 0 replies; 23+ messages in thread From: Sylwester Nawrocki @ 2020-11-04 13:27 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: georgi.djakov, cw00.choi, devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel On 04.11.2020 13:27, Krzysztof Kozlowski wrote: > On Wed, Nov 04, 2020 at 11:36:53AM +0100, Sylwester Nawrocki wrote: >> Add maintainers entry for the Samsung interconnect drivers, this currently >> includes Exynos SoC generic interconnect driver. >> >> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index e73636b..4bbafef 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -9156,6 +9156,13 @@ F: include/dt-bindings/interconnect/ >> F: include/linux/interconnect-provider.h >> F: include/linux/interconnect.h >> >> +SAMSUNG INTERCONNECT DRIVERS > > Does not look like ordered alphabetically. >> +M: Sylwester Nawrocki <s.nawrocki@samsung.com> >> +M: Artur Swigoń <a.swigon@samsung.com> >> +L: linux-pm@vger.kernel.org > > Also: > L: linux-samsung-soc@vger.kernel.org >> +S: Supported >> +F: drivers/interconnect/samsung > > Add trailing /. Fixed all issues, thanks for your review! -- Regards, Sylwester ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v8 3/7] MAINTAINERS: Add entry for Samsung interconnect drivers 2020-11-04 10:36 ` [PATCH v8 3/7] MAINTAINERS: Add entry for Samsung interconnect drivers Sylwester Nawrocki 2020-11-04 12:27 ` Krzysztof Kozlowski @ 2020-11-04 12:30 ` Chanwoo Choi 1 sibling, 0 replies; 23+ messages in thread From: Chanwoo Choi @ 2020-11-04 12:30 UTC (permalink / raw) To: Sylwester Nawrocki Cc: Georgi Djakov, Chanwoo Choi, Krzysztof Kozlowski, devicetree, linux-samsung-soc, Bartlomiej Zolnierkiewicz, Linux PM list, Seung-Woo Kim, Artur Świgoń, Rob Herring, linux-kernel, MyungJoo Ham, dri-devel, linux-arm-kernel, Marek Szyprowski Hi Sylwester, On Wed, Nov 4, 2020 at 7:37 PM Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > > Add maintainers entry for the Samsung interconnect drivers, this currently > includes Exynos SoC generic interconnect driver. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > Changes since v7: > - new patch. > --- > MAINTAINERS | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index e73636b..4bbafef 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9156,6 +9156,13 @@ F: include/dt-bindings/interconnect/ > F: include/linux/interconnect-provider.h > F: include/linux/interconnect.h > > +SAMSUNG INTERCONNECT DRIVERS > +M: Sylwester Nawrocki <s.nawrocki@samsung.com> > +M: Artur Swigoń <a.swigon@samsung.com> > +L: linux-pm@vger.kernel.org > +S: Supported > +F: drivers/interconnect/samsung > + > INVENSENSE ICM-426xx IMU DRIVER > M: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> > L: linux-iio@vger.kernel.org > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> -- Best Regards, Chanwoo Choi ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <CGME20201104103726eucas1p2e8f7c32f13b4232d925e901284ada13d@eucas1p2.samsung.com>]
* [PATCH v8 4/7] PM / devfreq: exynos-bus: Add registration of interconnect child device [not found] ` <CGME20201104103726eucas1p2e8f7c32f13b4232d925e901284ada13d@eucas1p2.samsung.com> @ 2020-11-04 10:36 ` Sylwester Nawrocki 0 siblings, 0 replies; 23+ messages in thread From: Sylwester Nawrocki @ 2020-11-04 10:36 UTC (permalink / raw) To: georgi.djakov, cw00.choi, krzk Cc: devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel, s.nawrocki This patch adds registration of a child platform device for the exynos interconnect driver. It is assumed that the interconnect provider will only be needed when #interconnect-cells property is present in the bus DT node, hence the child device will be created only when such a property is present. Acked-by: Krzysztof Kozlowski <krzk@kernel.org> Acked-by: Chanwoo Choi <cw00.choi@samsung.com> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> --- Changes for v8, v7, v6: - none. Changes for v5: - new patch. --- drivers/devfreq/exynos-bus.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c index 1e684a4..ee300ee 100644 --- a/drivers/devfreq/exynos-bus.c +++ b/drivers/devfreq/exynos-bus.c @@ -24,6 +24,7 @@ struct exynos_bus { struct device *dev; + struct platform_device *icc_pdev; struct devfreq *devfreq; struct devfreq_event_dev **edev; @@ -156,6 +157,8 @@ static void exynos_bus_exit(struct device *dev) if (ret < 0) dev_warn(dev, "failed to disable the devfreq-event devices\n"); + platform_device_unregister(bus->icc_pdev); + dev_pm_opp_of_remove_table(dev); clk_disable_unprepare(bus->clk); if (bus->opp_table) { @@ -168,6 +171,8 @@ static void exynos_bus_passive_exit(struct device *dev) { struct exynos_bus *bus = dev_get_drvdata(dev); + platform_device_unregister(bus->icc_pdev); + dev_pm_opp_of_remove_table(dev); clk_disable_unprepare(bus->clk); } @@ -432,6 +437,18 @@ static int exynos_bus_probe(struct platform_device *pdev) if (ret < 0) goto err; + /* Create child platform device for the interconnect provider */ + if (of_get_property(dev->of_node, "#interconnect-cells", NULL)) { + bus->icc_pdev = platform_device_register_data( + dev, "exynos-generic-icc", + PLATFORM_DEVID_AUTO, NULL, 0); + + if (IS_ERR(bus->icc_pdev)) { + ret = PTR_ERR(bus->icc_pdev); + goto err; + } + } + max_state = bus->devfreq->profile->max_state; min_freq = (bus->devfreq->profile->freq_table[0] / 1000); max_freq = (bus->devfreq->profile->freq_table[max_state - 1] / 1000); -- 2.7.4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
[parent not found: <CGME20201104103726eucas1p248b51b25f5ee42898bf03e9cb2229c5d@eucas1p2.samsung.com>]
* [PATCH v8 5/7] ARM: dts: exynos: Add interconnect properties to Exynos4412 bus nodes [not found] ` <CGME20201104103726eucas1p248b51b25f5ee42898bf03e9cb2229c5d@eucas1p2.samsung.com> @ 2020-11-04 10:36 ` Sylwester Nawrocki 2020-11-04 12:31 ` Chanwoo Choi 2020-11-10 18:11 ` Krzysztof Kozlowski 0 siblings, 2 replies; 23+ messages in thread From: Sylwester Nawrocki @ 2020-11-04 10:36 UTC (permalink / raw) To: georgi.djakov, cw00.choi, krzk Cc: devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel, s.nawrocki This patch adds the following properties for Exynos4412 interconnect bus nodes: - interconnects: to declare connections between nodes in order to guarantee PM QoS requirements between nodes, - #interconnect-cells: required by the interconnect framework, - samsung,data-clk-ratio: which allows to specify minimum data clock frequency corresponding to requested bandwidth for each bus. Note that #interconnect-cells is always zero and node IDs are not hardcoded anywhere. Signed-off-by: Artur Świgoń <a.swigon@samsung.com> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> --- Changes for v8: - none. Changes for v7: - adjusted to the DT property changes: "interconnects" instead of "samsung,interconnect-parent", "samsung,data-clk-ratio" instead of "bus-width". Changes for v6: - added bus-width property in bus_dmc node. Changes for v5: - adjust to renamed exynos,interconnect-parent-node property, - add properties in common exynos4412.dtsi file rather than in Odroid specific odroid4412-odroid-common.dtsi. --- arch/arm/boot/dts/exynos4412.dtsi | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi index e76881d..4999e68 100644 --- a/arch/arm/boot/dts/exynos4412.dtsi +++ b/arch/arm/boot/dts/exynos4412.dtsi @@ -383,6 +383,8 @@ clocks = <&clock CLK_DIV_DMC>; clock-names = "bus"; operating-points-v2 = <&bus_dmc_opp_table>; + samsung,data-clock-ratio = <4>; + #interconnect-cells = <0>; status = "disabled"; }; @@ -452,6 +454,8 @@ clocks = <&clock CLK_DIV_GDL>; clock-names = "bus"; operating-points-v2 = <&bus_leftbus_opp_table>; + interconnects = <&bus_dmc>; + #interconnect-cells = <0>; status = "disabled"; }; @@ -468,6 +472,8 @@ clocks = <&clock CLK_ACLK160>; clock-names = "bus"; operating-points-v2 = <&bus_display_opp_table>; + interconnects = <&bus_leftbus &bus_dmc>; + #interconnect-cells = <0>; status = "disabled"; }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v8 5/7] ARM: dts: exynos: Add interconnect properties to Exynos4412 bus nodes 2020-11-04 10:36 ` [PATCH v8 5/7] ARM: dts: exynos: Add interconnect properties to Exynos4412 bus nodes Sylwester Nawrocki @ 2020-11-04 12:31 ` Chanwoo Choi 2020-11-10 18:11 ` Krzysztof Kozlowski 1 sibling, 0 replies; 23+ messages in thread From: Chanwoo Choi @ 2020-11-04 12:31 UTC (permalink / raw) To: Sylwester Nawrocki Cc: Georgi Djakov, Chanwoo Choi, Krzysztof Kozlowski, devicetree, linux-samsung-soc, Bartlomiej Zolnierkiewicz, Linux PM list, Seung-Woo Kim, Artur Świgoń, Rob Herring, linux-kernel, MyungJoo Ham, dri-devel, linux-arm-kernel, Marek Szyprowski Hi Sylwester, On Wed, Nov 4, 2020 at 7:37 PM Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > > This patch adds the following properties for Exynos4412 interconnect > bus nodes: > - interconnects: to declare connections between nodes in order to > guarantee PM QoS requirements between nodes, > - #interconnect-cells: required by the interconnect framework, > - samsung,data-clk-ratio: which allows to specify minimum data clock > frequency corresponding to requested bandwidth for each bus. > > Note that #interconnect-cells is always zero and node IDs are not > hardcoded anywhere. > > Signed-off-by: Artur Świgoń <a.swigon@samsung.com> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > Changes for v8: > - none. > > Changes for v7: > - adjusted to the DT property changes: "interconnects" instead > of "samsung,interconnect-parent", "samsung,data-clk-ratio" > instead of "bus-width". > > Changes for v6: > - added bus-width property in bus_dmc node. > > Changes for v5: > - adjust to renamed exynos,interconnect-parent-node property, > - add properties in common exynos4412.dtsi file rather than > in Odroid specific odroid4412-odroid-common.dtsi. > --- > arch/arm/boot/dts/exynos4412.dtsi | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi > index e76881d..4999e68 100644 > --- a/arch/arm/boot/dts/exynos4412.dtsi > +++ b/arch/arm/boot/dts/exynos4412.dtsi > @@ -383,6 +383,8 @@ > clocks = <&clock CLK_DIV_DMC>; > clock-names = "bus"; > operating-points-v2 = <&bus_dmc_opp_table>; > + samsung,data-clock-ratio = <4>; > + #interconnect-cells = <0>; > status = "disabled"; > }; > > @@ -452,6 +454,8 @@ > clocks = <&clock CLK_DIV_GDL>; > clock-names = "bus"; > operating-points-v2 = <&bus_leftbus_opp_table>; > + interconnects = <&bus_dmc>; > + #interconnect-cells = <0>; > status = "disabled"; > }; > > @@ -468,6 +472,8 @@ > clocks = <&clock CLK_ACLK160>; > clock-names = "bus"; > operating-points-v2 = <&bus_display_opp_table>; > + interconnects = <&bus_leftbus &bus_dmc>; > + #interconnect-cells = <0>; > status = "disabled"; > }; > > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> -- Best Regards, Chanwoo Choi ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v8 5/7] ARM: dts: exynos: Add interconnect properties to Exynos4412 bus nodes 2020-11-04 10:36 ` [PATCH v8 5/7] ARM: dts: exynos: Add interconnect properties to Exynos4412 bus nodes Sylwester Nawrocki 2020-11-04 12:31 ` Chanwoo Choi @ 2020-11-10 18:11 ` Krzysztof Kozlowski 1 sibling, 0 replies; 23+ messages in thread From: Krzysztof Kozlowski @ 2020-11-10 18:11 UTC (permalink / raw) To: Sylwester Nawrocki Cc: georgi.djakov, cw00.choi, devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel On Wed, Nov 04, 2020 at 11:36:55AM +0100, Sylwester Nawrocki wrote: > This patch adds the following properties for Exynos4412 interconnect > bus nodes: > - interconnects: to declare connections between nodes in order to > guarantee PM QoS requirements between nodes, > - #interconnect-cells: required by the interconnect framework, > - samsung,data-clk-ratio: which allows to specify minimum data clock > frequency corresponding to requested bandwidth for each bus. > > Note that #interconnect-cells is always zero and node IDs are not > hardcoded anywhere. > > Signed-off-by: Artur Świgoń <a.swigon@samsung.com> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > Changes for v8: > - none. > > Changes for v7: > - adjusted to the DT property changes: "interconnects" instead > of "samsung,interconnect-parent", "samsung,data-clk-ratio" > instead of "bus-width". > > Changes for v6: > - added bus-width property in bus_dmc node. > > Changes for v5: > - adjust to renamed exynos,interconnect-parent-node property, > - add properties in common exynos4412.dtsi file rather than > in Odroid specific odroid4412-odroid-common.dtsi. > --- > arch/arm/boot/dts/exynos4412.dtsi | 6 ++++++ Thanks, applied with Chanwoo's tags. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <CGME20201104103728eucas1p2f671f29ed9eb06d4c6c991b073be092e@eucas1p2.samsung.com>]
* [PATCH v8 6/7] ARM: dts: exynos: Add interconnects to Exynos4412 mixer [not found] ` <CGME20201104103728eucas1p2f671f29ed9eb06d4c6c991b073be092e@eucas1p2.samsung.com> @ 2020-11-04 10:36 ` Sylwester Nawrocki 2020-11-10 18:12 ` Krzysztof Kozlowski 0 siblings, 1 reply; 23+ messages in thread From: Sylwester Nawrocki @ 2020-11-04 10:36 UTC (permalink / raw) To: georgi.djakov, cw00.choi, krzk Cc: devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel, s.nawrocki From: Artur Świgoń <a.swigon@samsung.com> This patch adds an 'interconnects' property to Exynos4412 DTS in order to declare the interconnect path used by the mixer. Please note that the 'interconnect-names' property is not needed when there is only one path in 'interconnects', in which case calling of_icc_get() with a NULL name simply returns the right path. Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> Signed-off-by: Artur Świgoń <a.swigon@samsung.com> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> --- Changes for v8...v5: - none. --- arch/arm/boot/dts/exynos4412.dtsi | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi index 4999e68..d07739e 100644 --- a/arch/arm/boot/dts/exynos4412.dtsi +++ b/arch/arm/boot/dts/exynos4412.dtsi @@ -779,6 +779,7 @@ clock-names = "mixer", "hdmi", "sclk_hdmi", "vp"; clocks = <&clock CLK_MIXER>, <&clock CLK_HDMI>, <&clock CLK_SCLK_HDMI>, <&clock CLK_VP>; + interconnects = <&bus_display &bus_dmc>; }; &pmu { -- 2.7.4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v8 6/7] ARM: dts: exynos: Add interconnects to Exynos4412 mixer 2020-11-04 10:36 ` [PATCH v8 6/7] ARM: dts: exynos: Add interconnects to Exynos4412 mixer Sylwester Nawrocki @ 2020-11-10 18:12 ` Krzysztof Kozlowski 0 siblings, 0 replies; 23+ messages in thread From: Krzysztof Kozlowski @ 2020-11-10 18:12 UTC (permalink / raw) To: Sylwester Nawrocki Cc: georgi.djakov, cw00.choi, devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel On Wed, Nov 04, 2020 at 11:36:56AM +0100, Sylwester Nawrocki wrote: > From: Artur Świgoń <a.swigon@samsung.com> > > This patch adds an 'interconnects' property to Exynos4412 DTS in order to > declare the interconnect path used by the mixer. Please note that the > 'interconnect-names' property is not needed when there is only one path in > 'interconnects', in which case calling of_icc_get() with a NULL name simply > returns the right path. > > Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> > Signed-off-by: Artur Świgoń <a.swigon@samsung.com> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > Changes for v8...v5: > - none. > --- > arch/arm/boot/dts/exynos4412.dtsi | 1 + > 1 file changed, 1 insertion(+) Thanks, applied with Chanwoo's tags. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <CGME20201104103730eucas1p2964e5910a1319fc1c931db8f02a447de@eucas1p2.samsung.com>]
* [PATCH v8 7/7] drm: exynos: mixer: Add interconnect support [not found] ` <CGME20201104103730eucas1p2964e5910a1319fc1c931db8f02a447de@eucas1p2.samsung.com> @ 2020-11-04 10:36 ` Sylwester Nawrocki 2020-11-04 12:31 ` Chanwoo Choi 0 siblings, 1 reply; 23+ messages in thread From: Sylwester Nawrocki @ 2020-11-04 10:36 UTC (permalink / raw) To: georgi.djakov, cw00.choi, krzk Cc: devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel, s.nawrocki This patch adds interconnect support to exynos-mixer. The mixer works the same as before when CONFIG_INTERCONNECT is 'n'. For proper operation of the video mixer block we need to ensure the interconnect busses like DMC or LEFTBUS provide enough bandwidth so as to avoid DMA buffer underruns in the mixer block. I.e we need to prevent those busses from operating in low perfomance OPPs when the mixer is running. In this patch the bus bandwidth request is done through the interconnect API, the bandwidth value is calculated from selected DRM mode, i.e. video plane width, height, refresh rate and pixel format. The bandwidth setting is synchronized with VSYNC when we are switching to lower bandwidth. This is required to ensure enough bandwidth for the device since new settings are normally being applied in the hardware synchronously with VSYNC. Acked-by: Krzysztof Kozlowski <krzk@kernel.org> Co-developed-by: Artur Świgoń <a.swigon@samsung.com> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> --- Changes for v8: - updated comment in mixer_probe() Changes for v7: - fixed incorrect setting of the ICC bandwidth when the mixer is disabled, now the bandwidth is set explicitly to 0 in such case. Changes for v6: - the icc_set_bw() call is now only done when calculated value for a crtc changes, this avoids unnecessary calls per each video frame - added synchronization of the interconnect bandwidth setting with the mixer VSYNC in order to avoid buffer underflow when we lower the interconnect bandwidth when the hardware still operates with previous mode settings that require higher bandwidth. This fixed IOMMU faults observed e.g. during switching from two planes to a single plane operation. Changes for v5: - renamed soc_path variable to icc_path --- drivers/gpu/drm/exynos/exynos_mixer.c | 146 ++++++++++++++++++++++++++++++++-- 1 file changed, 138 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index af192e5..8c1509e 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -13,6 +13,7 @@ #include <linux/component.h> #include <linux/delay.h> #include <linux/i2c.h> +#include <linux/interconnect.h> #include <linux/interrupt.h> #include <linux/irq.h> #include <linux/kernel.h> @@ -73,6 +74,7 @@ enum mixer_flag_bits { MXR_BIT_INTERLACE, MXR_BIT_VP_ENABLED, MXR_BIT_HAS_SCLK, + MXR_BIT_REQUEST_BW, }; static const uint32_t mixer_formats[] = { @@ -99,6 +101,13 @@ struct mixer_context { struct exynos_drm_plane planes[MIXER_WIN_NR]; unsigned long flags; + struct icc_path *icc_path; + /* memory bandwidth on the interconnect bus in B/s */ + unsigned long icc_bandwidth; + /* mutex protecting @icc_bandwidth */ + struct mutex icc_lock; + struct work_struct work; + int irq; void __iomem *mixer_regs; void __iomem *vp_regs; @@ -754,6 +763,9 @@ static irqreturn_t mixer_irq_handler(int irq, void *arg) val |= MXR_INT_CLEAR_VSYNC; val &= ~MXR_INT_STATUS_VSYNC; + if (test_and_clear_bit(MXR_BIT_REQUEST_BW, &ctx->flags)) + schedule_work(&ctx->work); + /* interlace scan need to check shadow register */ if (test_bit(MXR_BIT_INTERLACE, &ctx->flags) && !mixer_is_synced(ctx)) @@ -934,6 +946,76 @@ static void mixer_disable_vblank(struct exynos_drm_crtc *crtc) mixer_reg_writemask(mixer_ctx, MXR_INT_EN, 0, MXR_INT_EN_VSYNC); } +/** + * mixer_get_memory_bandwidth - calculate memory bandwidth for current crtc mode + * @crtc: a crtc with DRM mode to calculate the bandwidth for + * + * Return: memory bandwidth in B/s + * + * This function returns memory bandwidth calculated as a sum of amount of data + * per second for each plane. The calculation is based on maximum possible pixel + * resolution for a plane so as to avoid different bandwidth request per each + * video frame. The formula used for calculation for each plane is: + * + * bw = width * height * frame_rate / interlace / (hor_subs * vert_subs) + * + * where: + * - width, height - (DRM mode) video frame width and height in pixels, + * - frame_rate - DRM mode frame refresh rate, + * - interlace: 1 - in case of progressive and 2 in case of interlaced video, + * - hor_subs, vert_subs - accordingly horizontal and vertical pixel + * subsampling for a plane. + */ +static unsigned int mixer_get_memory_bandwidth(struct exynos_drm_crtc *crtc) +{ + struct drm_display_mode *mode = &crtc->base.state->adjusted_mode; + struct mixer_context *ctx = crtc->ctx; + unsigned long bw, bandwidth = 0; + int i, j, sub; + + for (i = 0; i < MIXER_WIN_NR; i++) { + struct drm_plane *plane = &ctx->planes[i].base; + const struct drm_format_info *format; + + if (plane->state && plane->state->crtc && plane->state->fb) { + format = plane->state->fb->format; + bw = mode->hdisplay * mode->vdisplay * + drm_mode_vrefresh(mode); + if (mode->flags & DRM_MODE_FLAG_INTERLACE) + bw /= 2; + for (j = 0; j < format->num_planes; j++) { + sub = j ? (format->vsub * format->hsub) : 1; + bandwidth += format->cpp[j] * bw / sub; + } + } + } + + return bandwidth; +} + +static void mixer_set_icc_bandwidth(struct mixer_context *ctx, + unsigned long bandwidth) +{ + u32 avg_bw, peak_bw; + + /* add 20% safety margin */ + bandwidth = bandwidth / 4 * 5; + + avg_bw = peak_bw = Bps_to_icc(bandwidth); + icc_set_bw(ctx->icc_path, avg_bw, peak_bw); + + dev_dbg(ctx->dev, "safe bandwidth %lu Bps\n", bandwidth); +} + +static void mixer_icc_request_fn(struct work_struct *work) +{ + struct mixer_context *ctx = container_of(work, struct mixer_context, + work); + mutex_lock(&ctx->icc_lock); + mixer_set_icc_bandwidth(ctx, ctx->icc_bandwidth); + mutex_unlock(&ctx->icc_lock); +} + static void mixer_atomic_begin(struct exynos_drm_crtc *crtc) { struct mixer_context *ctx = crtc->ctx; @@ -980,12 +1062,35 @@ static void mixer_disable_plane(struct exynos_drm_crtc *crtc, static void mixer_atomic_flush(struct exynos_drm_crtc *crtc) { - struct mixer_context *mixer_ctx = crtc->ctx; + struct mixer_context *ctx = crtc->ctx; + int bw, prev_bw; - if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags)) + if (!test_bit(MXR_BIT_POWERED, &ctx->flags)) return; - mixer_enable_sync(mixer_ctx); + /* + * Request necessary bandwidth on the interconnects. If new + * bandwidth is greater than current value set the new value + * immediately. Otherwise request lower bandwidth only after + * VSYNC, after the HW has actually switched to new video + * frame settings. + */ + if (ctx->icc_path) { + bw = mixer_get_memory_bandwidth(crtc); + + mutex_lock(&ctx->icc_lock); + prev_bw = ctx->icc_bandwidth; + ctx->icc_bandwidth = bw; + + if (bw > prev_bw) + mixer_set_icc_bandwidth(ctx, bw); + else if (bw < prev_bw) + set_bit(MXR_BIT_REQUEST_BW, &ctx->flags); + + mutex_unlock(&ctx->icc_lock); + } + + mixer_enable_sync(ctx); exynos_crtc_handle_event(crtc); } @@ -1036,6 +1141,8 @@ static void mixer_atomic_disable(struct exynos_drm_crtc *crtc) pm_runtime_put(ctx->dev); + mixer_set_icc_bandwidth(ctx, 0); + clear_bit(MXR_BIT_POWERED, &ctx->flags); } @@ -1223,19 +1330,33 @@ static int mixer_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; const struct mixer_drv_data *drv; struct mixer_context *ctx; + struct icc_path *path; int ret; + /* + * Returns NULL if CONFIG_INTERCONNECT is disabled or if "interconnects" + * property does not exist. May return ERR_PTR(-EPROBE_DEFER). + */ + path = of_icc_get(dev, NULL); + if (IS_ERR(path)) + return PTR_ERR(path); + ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); if (!ctx) { DRM_DEV_ERROR(dev, "failed to alloc mixer context.\n"); - return -ENOMEM; + ret = -ENOMEM; + goto err; } drv = of_device_get_match_data(dev); + INIT_WORK(&ctx->work, mixer_icc_request_fn); + mutex_init(&ctx->icc_lock); + ctx->pdev = pdev; ctx->dev = dev; ctx->mxr_ver = drv->version; + ctx->icc_path = path; if (drv->is_vp_enabled) __set_bit(MXR_BIT_VP_ENABLED, &ctx->flags); @@ -1247,17 +1368,26 @@ static int mixer_probe(struct platform_device *pdev) pm_runtime_enable(dev); ret = component_add(&pdev->dev, &mixer_component_ops); - if (ret) + if (ret) { pm_runtime_disable(dev); - + goto err; + } + return 0; +err: + icc_put(path); return ret; } static int mixer_remove(struct platform_device *pdev) { - pm_runtime_disable(&pdev->dev); + struct device *dev = &pdev->dev; + struct mixer_context *ctx = dev_get_drvdata(dev); + + pm_runtime_disable(dev); + + component_del(dev, &mixer_component_ops); - component_del(&pdev->dev, &mixer_component_ops); + icc_put(ctx->icc_path); return 0; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v8 7/7] drm: exynos: mixer: Add interconnect support 2020-11-04 10:36 ` [PATCH v8 7/7] drm: exynos: mixer: Add interconnect support Sylwester Nawrocki @ 2020-11-04 12:31 ` Chanwoo Choi 0 siblings, 0 replies; 23+ messages in thread From: Chanwoo Choi @ 2020-11-04 12:31 UTC (permalink / raw) To: Sylwester Nawrocki Cc: Georgi Djakov, Chanwoo Choi, Krzysztof Kozlowski, devicetree, linux-samsung-soc, Bartlomiej Zolnierkiewicz, Linux PM list, Seung-Woo Kim, Artur Świgoń, Rob Herring, linux-kernel, MyungJoo Ham, dri-devel, linux-arm-kernel, Marek Szyprowski Hi Sylwester, On Wed, Nov 4, 2020 at 7:37 PM Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > > This patch adds interconnect support to exynos-mixer. The mixer works > the same as before when CONFIG_INTERCONNECT is 'n'. > > For proper operation of the video mixer block we need to ensure the > interconnect busses like DMC or LEFTBUS provide enough bandwidth so > as to avoid DMA buffer underruns in the mixer block. I.e we need to > prevent those busses from operating in low perfomance OPPs when > the mixer is running. > In this patch the bus bandwidth request is done through the interconnect > API, the bandwidth value is calculated from selected DRM mode, i.e. > video plane width, height, refresh rate and pixel format. > > The bandwidth setting is synchronized with VSYNC when we are switching > to lower bandwidth. This is required to ensure enough bandwidth for > the device since new settings are normally being applied in the hardware > synchronously with VSYNC. > > Acked-by: Krzysztof Kozlowski <krzk@kernel.org> > Co-developed-by: Artur Świgoń <a.swigon@samsung.com> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > Changes for v8: > - updated comment in mixer_probe() > > Changes for v7: > - fixed incorrect setting of the ICC bandwidth when the mixer is > disabled, now the bandwidth is set explicitly to 0 in such case. > > Changes for v6: > - the icc_set_bw() call is now only done when calculated value for > a crtc changes, this avoids unnecessary calls per each video frame > - added synchronization of the interconnect bandwidth setting with > the mixer VSYNC in order to avoid buffer underflow when we lower > the interconnect bandwidth when the hardware still operates with > previous mode settings that require higher bandwidth. This fixed > IOMMU faults observed e.g. during switching from two planes to > a single plane operation. > > Changes for v5: > - renamed soc_path variable to icc_path > --- > drivers/gpu/drm/exynos/exynos_mixer.c | 146 ++++++++++++++++++++++++++++++++-- > 1 file changed, 138 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c > index af192e5..8c1509e 100644 > --- a/drivers/gpu/drm/exynos/exynos_mixer.c > +++ b/drivers/gpu/drm/exynos/exynos_mixer.c > @@ -13,6 +13,7 @@ > #include <linux/component.h> > #include <linux/delay.h> > #include <linux/i2c.h> > +#include <linux/interconnect.h> > #include <linux/interrupt.h> > #include <linux/irq.h> > #include <linux/kernel.h> > @@ -73,6 +74,7 @@ enum mixer_flag_bits { > MXR_BIT_INTERLACE, > MXR_BIT_VP_ENABLED, > MXR_BIT_HAS_SCLK, > + MXR_BIT_REQUEST_BW, > }; > > static const uint32_t mixer_formats[] = { > @@ -99,6 +101,13 @@ struct mixer_context { > struct exynos_drm_plane planes[MIXER_WIN_NR]; > unsigned long flags; > > + struct icc_path *icc_path; > + /* memory bandwidth on the interconnect bus in B/s */ > + unsigned long icc_bandwidth; > + /* mutex protecting @icc_bandwidth */ > + struct mutex icc_lock; > + struct work_struct work; > + > int irq; > void __iomem *mixer_regs; > void __iomem *vp_regs; > @@ -754,6 +763,9 @@ static irqreturn_t mixer_irq_handler(int irq, void *arg) > val |= MXR_INT_CLEAR_VSYNC; > val &= ~MXR_INT_STATUS_VSYNC; > > + if (test_and_clear_bit(MXR_BIT_REQUEST_BW, &ctx->flags)) > + schedule_work(&ctx->work); > + > /* interlace scan need to check shadow register */ > if (test_bit(MXR_BIT_INTERLACE, &ctx->flags) > && !mixer_is_synced(ctx)) > @@ -934,6 +946,76 @@ static void mixer_disable_vblank(struct exynos_drm_crtc *crtc) > mixer_reg_writemask(mixer_ctx, MXR_INT_EN, 0, MXR_INT_EN_VSYNC); > } > > +/** > + * mixer_get_memory_bandwidth - calculate memory bandwidth for current crtc mode > + * @crtc: a crtc with DRM mode to calculate the bandwidth for > + * > + * Return: memory bandwidth in B/s > + * > + * This function returns memory bandwidth calculated as a sum of amount of data > + * per second for each plane. The calculation is based on maximum possible pixel > + * resolution for a plane so as to avoid different bandwidth request per each > + * video frame. The formula used for calculation for each plane is: > + * > + * bw = width * height * frame_rate / interlace / (hor_subs * vert_subs) > + * > + * where: > + * - width, height - (DRM mode) video frame width and height in pixels, > + * - frame_rate - DRM mode frame refresh rate, > + * - interlace: 1 - in case of progressive and 2 in case of interlaced video, > + * - hor_subs, vert_subs - accordingly horizontal and vertical pixel > + * subsampling for a plane. > + */ > +static unsigned int mixer_get_memory_bandwidth(struct exynos_drm_crtc *crtc) > +{ > + struct drm_display_mode *mode = &crtc->base.state->adjusted_mode; > + struct mixer_context *ctx = crtc->ctx; > + unsigned long bw, bandwidth = 0; > + int i, j, sub; > + > + for (i = 0; i < MIXER_WIN_NR; i++) { > + struct drm_plane *plane = &ctx->planes[i].base; > + const struct drm_format_info *format; > + > + if (plane->state && plane->state->crtc && plane->state->fb) { > + format = plane->state->fb->format; > + bw = mode->hdisplay * mode->vdisplay * > + drm_mode_vrefresh(mode); > + if (mode->flags & DRM_MODE_FLAG_INTERLACE) > + bw /= 2; > + for (j = 0; j < format->num_planes; j++) { > + sub = j ? (format->vsub * format->hsub) : 1; > + bandwidth += format->cpp[j] * bw / sub; > + } > + } > + } > + > + return bandwidth; > +} > + > +static void mixer_set_icc_bandwidth(struct mixer_context *ctx, > + unsigned long bandwidth) > +{ > + u32 avg_bw, peak_bw; > + > + /* add 20% safety margin */ > + bandwidth = bandwidth / 4 * 5; > + > + avg_bw = peak_bw = Bps_to_icc(bandwidth); > + icc_set_bw(ctx->icc_path, avg_bw, peak_bw); > + > + dev_dbg(ctx->dev, "safe bandwidth %lu Bps\n", bandwidth); > +} > + > +static void mixer_icc_request_fn(struct work_struct *work) > +{ > + struct mixer_context *ctx = container_of(work, struct mixer_context, > + work); > + mutex_lock(&ctx->icc_lock); > + mixer_set_icc_bandwidth(ctx, ctx->icc_bandwidth); > + mutex_unlock(&ctx->icc_lock); > +} > + > static void mixer_atomic_begin(struct exynos_drm_crtc *crtc) > { > struct mixer_context *ctx = crtc->ctx; > @@ -980,12 +1062,35 @@ static void mixer_disable_plane(struct exynos_drm_crtc *crtc, > > static void mixer_atomic_flush(struct exynos_drm_crtc *crtc) > { > - struct mixer_context *mixer_ctx = crtc->ctx; > + struct mixer_context *ctx = crtc->ctx; > + int bw, prev_bw; > > - if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags)) > + if (!test_bit(MXR_BIT_POWERED, &ctx->flags)) > return; > > - mixer_enable_sync(mixer_ctx); > + /* > + * Request necessary bandwidth on the interconnects. If new > + * bandwidth is greater than current value set the new value > + * immediately. Otherwise request lower bandwidth only after > + * VSYNC, after the HW has actually switched to new video > + * frame settings. > + */ > + if (ctx->icc_path) { > + bw = mixer_get_memory_bandwidth(crtc); > + > + mutex_lock(&ctx->icc_lock); > + prev_bw = ctx->icc_bandwidth; > + ctx->icc_bandwidth = bw; > + > + if (bw > prev_bw) > + mixer_set_icc_bandwidth(ctx, bw); > + else if (bw < prev_bw) > + set_bit(MXR_BIT_REQUEST_BW, &ctx->flags); > + > + mutex_unlock(&ctx->icc_lock); > + } > + > + mixer_enable_sync(ctx); > exynos_crtc_handle_event(crtc); > } > > @@ -1036,6 +1141,8 @@ static void mixer_atomic_disable(struct exynos_drm_crtc *crtc) > > pm_runtime_put(ctx->dev); > > + mixer_set_icc_bandwidth(ctx, 0); > + > clear_bit(MXR_BIT_POWERED, &ctx->flags); > } > > @@ -1223,19 +1330,33 @@ static int mixer_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > const struct mixer_drv_data *drv; > struct mixer_context *ctx; > + struct icc_path *path; > int ret; > > + /* > + * Returns NULL if CONFIG_INTERCONNECT is disabled or if "interconnects" > + * property does not exist. May return ERR_PTR(-EPROBE_DEFER). > + */ > + path = of_icc_get(dev, NULL); > + if (IS_ERR(path)) > + return PTR_ERR(path); > + > ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); > if (!ctx) { > DRM_DEV_ERROR(dev, "failed to alloc mixer context.\n"); > - return -ENOMEM; > + ret = -ENOMEM; > + goto err; > } > > drv = of_device_get_match_data(dev); > > + INIT_WORK(&ctx->work, mixer_icc_request_fn); > + mutex_init(&ctx->icc_lock); > + > ctx->pdev = pdev; > ctx->dev = dev; > ctx->mxr_ver = drv->version; > + ctx->icc_path = path; > > if (drv->is_vp_enabled) > __set_bit(MXR_BIT_VP_ENABLED, &ctx->flags); > @@ -1247,17 +1368,26 @@ static int mixer_probe(struct platform_device *pdev) > pm_runtime_enable(dev); > > ret = component_add(&pdev->dev, &mixer_component_ops); > - if (ret) > + if (ret) { > pm_runtime_disable(dev); > - > + goto err; > + } > + return 0; > +err: > + icc_put(path); > return ret; > } > > static int mixer_remove(struct platform_device *pdev) > { > - pm_runtime_disable(&pdev->dev); > + struct device *dev = &pdev->dev; > + struct mixer_context *ctx = dev_get_drvdata(dev); > + > + pm_runtime_disable(dev); > + > + component_del(dev, &mixer_component_ops); > > - component_del(&pdev->dev, &mixer_component_ops); > + icc_put(ctx->icc_path); > > return 0; > } > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> -- Best Regards, Chanwoo Choi ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v8 0/7] Exynos: Simple QoS for exynos-bus using interconnect 2020-11-04 10:36 ` [PATCH v8 0/7] Exynos: Simple QoS for exynos-bus using interconnect Sylwester Nawrocki ` (6 preceding siblings ...) [not found] ` <CGME20201104103730eucas1p2964e5910a1319fc1c931db8f02a447de@eucas1p2.samsung.com> @ 2020-11-05 2:20 ` Chanwoo Choi 7 siblings, 0 replies; 23+ messages in thread From: Chanwoo Choi @ 2020-11-05 2:20 UTC (permalink / raw) To: Sylwester Nawrocki, georgi.djakov, krzk Cc: devicetree, robh+dt, a.swigon, myungjoo.ham, inki.dae, sw0312.kim, b.zolnierkie, m.szyprowski, linux-kernel, linux-pm, linux-samsung-soc, dri-devel, linux-arm-kernel Hi Sylwester, On 11/4/20 7:36 PM, Sylwester Nawrocki wrote: > This patchset adds interconnect API support for the Exynos SoC "samsung, > exynos-bus" compatible devices, which already have their corresponding > exynos-bus driver in the devfreq subsystem. Complementing the devfreq > driver with an interconnect functionality allows to ensure the QoS > requirements of devices accessing the system memory (e.g. video processing > devices) are fulfilled and aallows to avoid issues like the one discussed > in thread [1]. > > This patch series adds implementation of the interconnect provider per each > "samsung,exynos-bus" compatible DT node, with one interconnect node per > provider. The interconnect code which was previously added as a part of > the devfreq driver has been converted to a separate platform driver. > In the devfreq a corresponding virtual child platform device is registered. > Integration of devfreq and interconnect frameworks is achieved through > the PM QoS API. > > A sample interconnect consumer for exynos-mixer is added in patches 6/7, > 7/7, it is currently added only for exynos4412 and allows to address the > mixer DMA underrun error issues [1]. > > Changes since v7: > - drivers/interconnect/exynos renamed to drivers/interconnect/samsung, > - added INTERCONNECT_SAMSUNG Kconfig symbol, > - added missing driver sync_state callback, > - improved the DT binding description, > - added a patch adding maintainers entry, > - updated comment in patch 7/7, typo fix (patch 1/7). > > The series has been tested on Odroid U3 board. It is based on v5.10-rc1. > > -- > Regards, > Sylwester > > Changes since v6: > - the interconnect consumer DT bindings are now used to describe dependencies > of the interconnects (samsung,exynos-bus nodes), > - bus-width property replaced with samsung,data-clk-ratio, > - adaptation to recent changes in the interconnect code > (of_icc_get_from_provider(), icc_node_add()). > > Changes since v5: > - addition of "bus-width: DT property, which specifies data width > of the interconnect bus (patches 1...2/6), > - addition of synchronization of the interconnect bandwidth setting > with VSYNC (patch 6/6). > > Changes since v3 [4] (v4 skipped to align with patchset [1]), detailed > changes are listed in patches: > - conversion to a separate interconnect (platform) driver, > - an update of the DT binding documenting new optional properties: > #interconnect-cells, samsung,interconnect-parent in "samsung,exynos-bus" > nodes, > - new DT properties added to the SoC, rather than to the board specific > files. > > Changes since v2 [5]: > - Use icc_std_aggregate(). > - Implement a different modification of apply_constraints() in > drivers/interconnect/core.c (patch 03). > - Use 'exynos,interconnect-parent-node' in the DT instead of > 'devfreq'/'parent', depending on the bus. > - Rebase on DT patches that deprecate the 'devfreq' DT property. > - Improve error handling, including freeing generated IDs on failure. > - Remove exynos_bus_icc_connect() and add exynos_bus_icc_get_parent(). > > Changes since v1 [6]: > - Rebase on coupled regulators patches. > - Use dev_pm_qos_*() API instead of overriding frequency in > exynos_bus_target(). > - Use IDR for node ID allocation. > - Reverse order of multiplication and division in > mixer_set_memory_bandwidth() (patch 07) to avoid integer overflow. > > > References: > [1] https://patchwork.kernel.org/patch/10861757/ (original issue) > [2] https://www.spinics.net/lists/linux-samsung-soc/msg70014.html > [3] https://www.spinics.net/lists/arm-kernel/msg810722.html > [4] https://lore.kernel.org/linux-pm/20191220115653.6487-1-a.swigon@samsung.com > [5] https://patchwork.kernel.org/cover/11054417/ (v1 of this RFC) > [6] https://patchwork.kernel.org/cover/11152595/ (v2 of this RFC) > > > Artur Świgoń (1): > ARM: dts: exynos: Add interconnects to Exynos4412 mixer > > Sylwester Nawrocki (6): > dt-bindings: devfreq: Add documentation for the interconnect > properties > interconnect: Add generic interconnect driver for Exynos SoCs > MAINTAINERS: Add entry for Samsung interconnect drivers > PM / devfreq: exynos-bus: Add registration of interconnect child > device > ARM: dts: exynos: Add interconnect properties to Exynos4412 bus nodes > drm: exynos: mixer: Add interconnect support > > .../devicetree/bindings/devfreq/exynos-bus.txt | 71 +++++++- > MAINTAINERS | 7 + > arch/arm/boot/dts/exynos4412.dtsi | 7 + > drivers/devfreq/exynos-bus.c | 17 ++ > drivers/gpu/drm/exynos/exynos_mixer.c | 146 ++++++++++++++- > drivers/interconnect/Kconfig | 1 + > drivers/interconnect/Makefile | 1 + > drivers/interconnect/samsung/Kconfig | 13 ++ > drivers/interconnect/samsung/Makefile | 4 + > drivers/interconnect/samsung/exynos.c | 199 +++++++++++++++++++++ > 10 files changed, 456 insertions(+), 10 deletions(-) > create mode 100644 drivers/interconnect/samsung/Kconfig > create mode 100644 drivers/interconnect/samsung/Makefile > create mode 100644 drivers/interconnect/samsung/exynos.c > I tested it. It is well working. Thanks. Tested-by: Chanwoo Choi <cw00.choi@samsung.com> -- Best Regards, Chanwoo Choi Samsung Electronics ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2020-11-10 18:12 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20201104103713eucas1p2d21b6f936aa18725ae4b4878f3be0a8e@eucas1p2.samsung.com> 2020-11-04 10:36 ` [PATCH v8 0/7] Exynos: Simple QoS for exynos-bus using interconnect Sylwester Nawrocki [not found] ` <CGME20201104103718eucas1p1c103f1a96499b03c72e5457ac2542c3d@eucas1p1.samsung.com> 2020-11-04 10:36 ` [PATCH v8 1/7] dt-bindings: devfreq: Add documentation for the interconnect properties Sylwester Nawrocki 2020-11-04 12:25 ` Krzysztof Kozlowski 2020-11-04 12:28 ` Chanwoo Choi 2020-11-10 14:26 ` Rob Herring [not found] ` <CGME20201104103720eucas1p1014217e751a681796ed508af22c6bb12@eucas1p1.samsung.com> 2020-11-04 10:36 ` [PATCH v8 2/7] interconnect: Add generic interconnect driver for Exynos SoCs Sylwester Nawrocki 2020-11-04 12:28 ` Chanwoo Choi 2020-11-04 12:37 ` Krzysztof Kozlowski 2020-11-04 13:22 ` Sylwester Nawrocki 2020-11-04 14:06 ` Krzysztof Kozlowski [not found] ` <CGME20201104103722eucas1p1db939995e60d0bf2cd581070c14379f5@eucas1p1.samsung.com> 2020-11-04 10:36 ` [PATCH v8 3/7] MAINTAINERS: Add entry for Samsung interconnect drivers Sylwester Nawrocki 2020-11-04 12:27 ` Krzysztof Kozlowski 2020-11-04 13:27 ` Sylwester Nawrocki 2020-11-04 12:30 ` Chanwoo Choi [not found] ` <CGME20201104103726eucas1p2e8f7c32f13b4232d925e901284ada13d@eucas1p2.samsung.com> 2020-11-04 10:36 ` [PATCH v8 4/7] PM / devfreq: exynos-bus: Add registration of interconnect child device Sylwester Nawrocki [not found] ` <CGME20201104103726eucas1p248b51b25f5ee42898bf03e9cb2229c5d@eucas1p2.samsung.com> 2020-11-04 10:36 ` [PATCH v8 5/7] ARM: dts: exynos: Add interconnect properties to Exynos4412 bus nodes Sylwester Nawrocki 2020-11-04 12:31 ` Chanwoo Choi 2020-11-10 18:11 ` Krzysztof Kozlowski [not found] ` <CGME20201104103728eucas1p2f671f29ed9eb06d4c6c991b073be092e@eucas1p2.samsung.com> 2020-11-04 10:36 ` [PATCH v8 6/7] ARM: dts: exynos: Add interconnects to Exynos4412 mixer Sylwester Nawrocki 2020-11-10 18:12 ` Krzysztof Kozlowski [not found] ` <CGME20201104103730eucas1p2964e5910a1319fc1c931db8f02a447de@eucas1p2.samsung.com> 2020-11-04 10:36 ` [PATCH v8 7/7] drm: exynos: mixer: Add interconnect support Sylwester Nawrocki 2020-11-04 12:31 ` Chanwoo Choi 2020-11-05 2:20 ` [PATCH v8 0/7] Exynos: Simple QoS for exynos-bus using interconnect Chanwoo Choi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).