mbox series

[v8,0/7] Exynos: Simple QoS for exynos-bus using interconnect

Message ID 20201104103657.18007-1-s.nawrocki@samsung.com
Headers show
Series Exynos: Simple QoS for exynos-bus using interconnect | expand

Message

Sylwester Nawrocki/Kernel \(PLT\) /SRPOL/Staff Engineer/Samsung Electronics Nov. 4, 2020, 10:36 a.m. UTC 
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

Comments

Chanwoo Choi Nov. 4, 2020, 12:30 p.m. UTC | #1 
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>
Sylwester Nawrocki/Kernel \(PLT\) /SRPOL/Staff Engineer/Samsung Electronics Nov. 4, 2020, 1:22 p.m. UTC | #2 
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
Chanwoo Choi Nov. 5, 2020, 2:20 a.m. UTC | #3 
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>