| Message ID | 20201112140931.31139-1-s.nawrocki@samsung.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Exynos: Simple QoS for exynos-bus using interconnect | expand |
On 11/12/20 16:09, 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 allows 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 patch 5/5, > it is currently added only for exynos4412 and allows to address the > mixer DMA underrun error issues [1]. Good work Sylwester! Thank you and all the reviewers! What would be the merge path for this patchset? Looks like there is no build dependency between patches. Should i take just patches 2,3 or also patch 1? Chanwoo? Thanks, Georgi
On 11/13/20 11:07, Chanwoo Choi wrote: > On 11/13/20 5:48 PM, Georgi Djakov wrote: >> On 11/12/20 16:09, 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 allows 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 patch 5/5, >>> it is currently added only for exynos4412 and allows to address the >>> mixer DMA underrun error issues [1]. >> >> Good work Sylwester! Thank you and all the reviewers! What would be the merge >> path for this patchset? Looks like there is no build dependency between patches. >> Should i take just patches 2,3 or also patch 1? Chanwoo? > > Hi Georgi, > > If you take the patch 2,3, I'll apply patch 1,4 to devfreq.git. Ok, sounds good! Thanks for clarifying! BR, Georgi
On 11/13/20 5:48 PM, Georgi Djakov wrote: > On 11/12/20 16:09, 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 allows 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 patch 5/5, >> it is currently added only for exynos4412 and allows to address the >> mixer DMA underrun error issues [1]. > > Good work Sylwester! Thank you and all the reviewers! What would be the merge > path for this patchset? Looks like there is no build dependency between patches. > Should i take just patches 2,3 or also patch 1? Chanwoo? Hi Georgi, If you take the patch 2,3, I'll apply patch 1,4 to devfreq.git. Hi Sylwester, First of all, thanks for your work to finish it for a long time. I'm very happy about finishing this work. It is very necessary feature for the QoS. Once again, thank for your work.
Hi Georgi, Chanwoo, On 13.11.2020 10:07, Chanwoo Choi wrote: > On 11/13/20 5:48 PM, Georgi Djakov wrote: >> On 11/12/20 16:09, Sylwester Nawrocki wrote: [...] >> >> Good work Sylwester! Thank you and all the reviewers! What would be the merge >> path for this patchset? Looks like there is no build dependency between patches. >> Should i take just patches 2,3 or also patch 1? Chanwoo? > > Hi Georgi, > > If you take the patch 2,3, I'll apply patch 1,4 to devfreq.git. > Hi Sylwester, > First of all, thanks for your work to finish it for a long time. > I'm very happy about finishing this work. It is very necessary feature > for the QoS. Once again, thank for your work. I would also like to thank everyone for provided feedback! As far as building is concerned the patches could be applied in any order. I think we could also apply the drm/exynos patch in same merge window. There could be runtime (or git bisect) regression only in case when INTERCONNECT is enabled and only (or as first) the dts and drm/exynos patches are applied. Hmm, maybe it's better to hold on with the drm patch, INTERCONNECT is disabled in arch/arm/configs/{multi_v7_defconfig, exynos_defconfig} but it is enabled in arch/arm64/configs/defconfig.
Hi Sylwester, On 13.11.2020 11:32, Sylwester Nawrocki wrote: > On 13.11.2020 10:07, Chanwoo Choi wrote: >> On 11/13/20 5:48 PM, Georgi Djakov wrote: >>> On 11/12/20 16:09, Sylwester Nawrocki wrote: > [...] >>> Good work Sylwester! Thank you and all the reviewers! What would be the merge >>> path for this patchset? Looks like there is no build dependency between patches. >>> Should i take just patches 2,3 or also patch 1? Chanwoo? >> Hi Georgi, >> >> If you take the patch 2,3, I'll apply patch 1,4 to devfreq.git. >> Hi Sylwester, >> First of all, thanks for your work to finish it for a long time. >> I'm very happy about finishing this work. It is very necessary feature >> for the QoS. Once again, thank for your work. > I would also like to thank everyone for provided feedback! > > As far as building is concerned the patches could be applied in any > order. I think we could also apply the drm/exynos patch in same > merge window. There could be runtime (or git bisect) regression > only in case when INTERCONNECT is enabled and only (or as first) > the dts and drm/exynos patches are applied. > > Hmm, maybe it's better to hold on with the drm patch, INTERCONNECT > is disabled in arch/arm/configs/{multi_v7_defconfig, exynos_defconfig} > but it is enabled in arch/arm64/configs/defconfig. I don't think we need to delay DRM patch. Exynos DRM mixer is not available on ARM64 SoCs, so this won't be an issue. Best regards
Hi Sylwester, On 11/12/20 11:09 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 allows 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 patch 5/5, > it is currently added only for exynos4412 and allows to address the > mixer DMA underrun error issues [1]. > > Changes since v8: > - excluded from the series already applied dts patches, > - Co-developed-by/Signed-off-by tag corrections, Ack tags added, > - the maintainers entry corrections adressing review comments, > - Kconfig/Makefile improvements/corrections, > - whitespace/indentation cleanup. > > The series has been tested on Odroid U3 board. It is based on v5.10-rc1. > > -- > Regards, > Sylwester > > 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). > > 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://protect2.fireeye.com/v1/url?k=0c5b42ec-53c07bee-0c5ac9a3-0cc47a31381a-25eb9fb6f0f852e5&q=1&e=3074012f-8e28-49a9-9773-669dbeefe1a6&u=https%3A%2F%2Fwww.spinics.net%2Flists%2Flinux-samsung-soc%2Fmsg70014.html > [3] https://protect2.fireeye.com/v1/url?k=f3fdf673-ac66cf71-f3fc7d3c-0cc47a31381a-8ab39522e9fe68a8&q=1&e=3074012f-8e28-49a9-9773-669dbeefe1a6&u=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg810722.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) > > > Sylwester Nawrocki (5): > 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 > drm: exynos: mixer: Add interconnect support > > .../devicetree/bindings/devfreq/exynos-bus.txt | 71 +++++++- > MAINTAINERS | 8 + > 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 +++++++++++++++++++++ > 9 files changed, 450 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 checked that the icc patches were merged to icc.git. So, I applied patch1 and patch4 to devfreq.git.
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 allows 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 patch 5/5, it is currently added only for exynos4412 and allows to address the mixer DMA underrun error issues [1]. Changes since v8: - excluded from the series already applied dts patches, - Co-developed-by/Signed-off-by tag corrections, Ack tags added, - the maintainers entry corrections adressing review comments, - Kconfig/Makefile improvements/corrections, - whitespace/indentation cleanup. The series has been tested on Odroid U3 board. It is based on v5.10-rc1. -- Regards, Sylwester 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). 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) Sylwester Nawrocki (5): 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 drm: exynos: mixer: Add interconnect support .../devicetree/bindings/devfreq/exynos-bus.txt | 71 +++++++- MAINTAINERS | 8 + 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 +++++++++++++++++++++ 9 files changed, 450 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