The following patchset adds interconnect[1][2] framework support to the exynos-bus devfreq driver. Extending the devfreq driver with interconnect capabilities started as a response to the issue referenced in [3]. The patches can be subdivided into four logical groups:
(a) Refactoring the existing devfreq driver in order to improve readability and accommodate for adding new code (patches 01--04/11).
(b) Tweaking the interconnect framework to support the exynos-bus use case (patches 05--07/11). Exporting of_icc_get_from_provider() allows us to avoid hardcoding every single graph edge in the DT or driver source, and relaxing the requirement contained in that function removes the need to provide dummy node IDs in the DT. Adjusting the logic in apply_constraints() (drivers/interconnect/core.c) accounts for the fact that every bus is a separate entity and therefore a separate interconnect provider, albeit constituting a part of a larger hierarchy.
(c) Implementing interconnect providers in the exynos-bus devfreq driver and adding required DT properties for one selected platform, namely Exynos4412 (patches 08--09/11). Due to the fact that this aims to be a generic driver for various Exynos SoCs, node IDs are generated dynamically rather than hardcoded. This has been determined to be a simpler approach, but depends on changes described in (b).
(d) Implementing a sample interconnect consumer for exynos-mixer targeted at the issue referenced in [3], again with DT info only for Exynos4412 (patches 10--11/11).
Integration of devfreq and interconnect functionalities comes down to one extra line in the devfreq target() callback, which selects either the frequency calculated by the devfreq governor, or the one requested with the interconnect API, whichever is higher. All new code works equally well when CONFIG_INTERCONNECT is 'n' (as in exynos_defconfig) in which case all interconnect API functions are no-ops.
--- Artur Świgoń Samsung R&D Institute Poland Samsung Electronics
--- References: [1] Documentation/interconnect/interconnect.rst [2] Documentation/devicetree/bindings/interconnect/interconnect.txt [3] https://patchwork.kernel.org/patch/10861757
Artur Świgoń (10): devfreq: exynos-bus: Extract exynos_bus_profile_init() devfreq: exynos-bus: Extract exynos_bus_profile_init_passive() devfreq: exynos-bus: Change goto-based logic to if-else logic devfreq: exynos-bus: Clean up code icc: Export of_icc_get_from_provider() icc: Relax requirement in of_icc_get_from_provider() icc: Relax condition in apply_constraints() arm: dts: exynos: Add parents and #interconnect-cells to Exynos4412 devfreq: exynos-bus: Add interconnect functionality to exynos-bus arm: dts: exynos: Add interconnects to Exynos4412 mixer
Marek Szyprowski (1): drm: exynos: mixer: Add interconnect support
.../boot/dts/exynos4412-odroid-common.dtsi | 1 + arch/arm/boot/dts/exynos4412.dtsi | 10 + drivers/devfreq/exynos-bus.c | 296 ++++++++++++++---- drivers/gpu/drm/exynos/exynos_mixer.c | 68 +++- drivers/interconnect/core.c | 12 +- include/linux/interconnect-provider.h | 6 + 6 files changed, 314 insertions(+), 79 deletions(-)
From: Marek Szyprowski m.szyprowski@samsung.com
This patch adds interconnect support to exynos-mixer. Please note that the mixer works the same as before when CONFIG_INTERCONNECT is 'n'.
Co-developed-by: Artur Świgoń a.swigon@partner.samsung.com Signed-off-by: Artur Świgoń a.swigon@partner.samsung.com Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/gpu/drm/exynos/exynos_mixer.c | 68 +++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 7b24338fad3c..fb763854b300 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> @@ -97,6 +98,7 @@ struct mixer_context { struct exynos_drm_crtc *crtc; struct exynos_drm_plane planes[MIXER_WIN_NR]; unsigned long flags; + struct icc_path *soc_path;
int irq; void __iomem *mixer_regs; @@ -931,6 +933,37 @@ static void mixer_disable_vblank(struct exynos_drm_crtc *crtc) mixer_reg_writemask(mixer_ctx, MXR_INT_EN, 0, MXR_INT_EN_VSYNC); }
+static void mixer_set_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; + } + } + } + + /* add 20% safety margin */ + bandwidth = 5UL * bandwidth / 4; + + pr_info("exynos-mixer: safe bandwidth %ld Bps\n", bandwidth); + icc_set_bw(ctx->soc_path, 0, Bps_to_icc(bandwidth)); +} + static void mixer_atomic_begin(struct exynos_drm_crtc *crtc) { struct mixer_context *ctx = crtc->ctx; @@ -982,6 +1015,7 @@ static void mixer_atomic_flush(struct exynos_drm_crtc *crtc) if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags)) return;
+ mixer_set_memory_bandwidth(crtc); mixer_enable_sync(mixer_ctx); exynos_crtc_handle_event(crtc); } @@ -1029,6 +1063,7 @@ static void mixer_disable(struct exynos_drm_crtc *crtc) for (i = 0; i < MIXER_WIN_NR; i++) mixer_disable_plane(crtc, &ctx->planes[i]);
+ mixer_set_memory_bandwidth(crtc); exynos_drm_pipe_clk_enable(crtc, false);
pm_runtime_put(ctx->dev); @@ -1220,12 +1255,22 @@ 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. + * 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); @@ -1233,6 +1278,7 @@ static int mixer_probe(struct platform_device *pdev) ctx->pdev = pdev; ctx->dev = dev; ctx->mxr_ver = drv->version; + ctx->soc_path = path;
if (drv->is_vp_enabled) __set_bit(MXR_BIT_VP_ENABLED, &ctx->flags); @@ -1242,17 +1288,29 @@ static int mixer_probe(struct platform_device *pdev) platform_set_drvdata(pdev, ctx);
ret = component_add(&pdev->dev, &mixer_component_ops); - if (!ret) - pm_runtime_enable(dev); + if (ret < 0) + goto err; + + pm_runtime_enable(dev); + + 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);
- component_del(&pdev->dev, &mixer_component_ops); + pm_runtime_disable(dev); + + component_del(dev, &mixer_component_ops); + + icc_put(ctx->soc_path);
return 0; }
On Tue, Jul 23, 2019 at 02:20:16PM +0200, Artur Świgoń wrote:
From: Marek Szyprowski m.szyprowski@samsung.com
This patch adds interconnect support to exynos-mixer. Please note that the mixer works the same as before when CONFIG_INTERCONNECT is 'n'.
Co-developed-by: Artur Świgoń a.swigon@partner.samsung.com Signed-off-by: Artur Świgoń a.swigon@partner.samsung.com Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/exynos/exynos_mixer.c | 68 +++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 7b24338fad3c..fb763854b300 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> @@ -97,6 +98,7 @@ struct mixer_context { struct exynos_drm_crtc *crtc; struct exynos_drm_plane planes[MIXER_WIN_NR]; unsigned long flags;
struct icc_path *soc_path;
int irq; void __iomem *mixer_regs;
@@ -931,6 +933,37 @@ static void mixer_disable_vblank(struct exynos_drm_crtc *crtc) mixer_reg_writemask(mixer_ctx, MXR_INT_EN, 0, MXR_INT_EN_VSYNC); }
+static void mixer_set_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;
Early exit if !ctx->soc_path, no need to figure out the bandwidth. Optionally check it before calling mixer_set_memory_bandwidth() - should not hurt readability.
- 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;}}- }
- /* add 20% safety margin */
- bandwidth = 5UL * bandwidth / 4;
- pr_info("exynos-mixer: safe bandwidth %ld Bps\n", bandwidth);
dev_dbg()
Best regards, Krzysztof
On Tue, Jul 23, 2019 at 02:20:05PM +0200, Artur Świgoń wrote:
The following patchset adds interconnect[1][2] framework support to the exynos-bus devfreq driver. Extending the devfreq driver with interconnect capabilities started as a response to the issue referenced in [3]. The patches can be subdivided into four logical groups:
Nice work! Good to see proper solution :)
Best regards, Krzysztof
Hi Artur.
The patch1-4 in this series depend on other patches[1] on mainline. On next v2 version, please make some patches based on patches[1] in order to prevent the merge conflict.
[1] [RESEND PATCH v5 0/4] add coupled regulators for Exynos5422/5800 - https://lkml.org/lkml/2019/8/8/217
Also, as I commented, we better to discuss it before sending the v2.
On 19. 7. 23. 오후 9:20, Artur Świgoń wrote:
The following patchset adds interconnect[1][2] framework support to the exynos-bus devfreq driver. Extending the devfreq driver with interconnect capabilities started as a response to the issue referenced in [3]. The patches can be subdivided into four logical groups:
(a) Refactoring the existing devfreq driver in order to improve readability and accommodate for adding new code (patches 01--04/11).
(b) Tweaking the interconnect framework to support the exynos-bus use case (patches 05--07/11). Exporting of_icc_get_from_provider() allows us to avoid hardcoding every single graph edge in the DT or driver source, and relaxing the requirement contained in that function removes the need to provide dummy node IDs in the DT. Adjusting the logic in apply_constraints() (drivers/interconnect/core.c) accounts for the fact that every bus is a separate entity and therefore a separate interconnect provider, albeit constituting a part of a larger hierarchy.
(c) Implementing interconnect providers in the exynos-bus devfreq driver and adding required DT properties for one selected platform, namely Exynos4412 (patches 08--09/11). Due to the fact that this aims to be a generic driver for various Exynos SoCs, node IDs are generated dynamically rather than hardcoded. This has been determined to be a simpler approach, but depends on changes described in (b).
(d) Implementing a sample interconnect consumer for exynos-mixer targeted at the issue referenced in [3], again with DT info only for Exynos4412 (patches 10--11/11).
Integration of devfreq and interconnect functionalities comes down to one extra line in the devfreq target() callback, which selects either the frequency calculated by the devfreq governor, or the one requested with the interconnect API, whichever is higher. All new code works equally well when CONFIG_INTERCONNECT is 'n' (as in exynos_defconfig) in which case all interconnect API functions are no-ops.
Artur Świgoń Samsung R&D Institute Poland Samsung Electronics
References: [1] Documentation/interconnect/interconnect.rst [2] Documentation/devicetree/bindings/interconnect/interconnect.txt [3] https://patchwork.kernel.org/patch/10861757
Artur Świgoń (10): devfreq: exynos-bus: Extract exynos_bus_profile_init() devfreq: exynos-bus: Extract exynos_bus_profile_init_passive() devfreq: exynos-bus: Change goto-based logic to if-else logic devfreq: exynos-bus: Clean up code icc: Export of_icc_get_from_provider() icc: Relax requirement in of_icc_get_from_provider() icc: Relax condition in apply_constraints() arm: dts: exynos: Add parents and #interconnect-cells to Exynos4412 devfreq: exynos-bus: Add interconnect functionality to exynos-bus arm: dts: exynos: Add interconnects to Exynos4412 mixer
Marek Szyprowski (1): drm: exynos: mixer: Add interconnect support
.../boot/dts/exynos4412-odroid-common.dtsi | 1 + arch/arm/boot/dts/exynos4412.dtsi | 10 + drivers/devfreq/exynos-bus.c | 296 ++++++++++++++---- drivers/gpu/drm/exynos/exynos_mixer.c | 68 +++- drivers/interconnect/core.c | 12 +- include/linux/interconnect-provider.h | 6 + 6 files changed, 314 insertions(+), 79 deletions(-)
Hi Artur,
On 19. 8. 13. 오후 3:17, Chanwoo Choi wrote:
Hi Artur.
The patch1-4 in this series depend on other patches[1] on mainline. On next v2 version, please make some patches based on patches[1] in order to prevent the merge conflict.
[1] [RESEND PATCH v5 0/4] add coupled regulators for Exynos5422/5800
Add correct reference url as following: - https://lkml.org/lkml/2019/8/8/217
Also, as I commented, we better to discuss it before sending the v2.
On 19. 7. 23. 오후 9:20, Artur Świgoń wrote:
The following patchset adds interconnect[1][2] framework support to the exynos-bus devfreq driver. Extending the devfreq driver with interconnect capabilities started as a response to the issue referenced in [3]. The patches can be subdivided into four logical groups:
(a) Refactoring the existing devfreq driver in order to improve readability and accommodate for adding new code (patches 01--04/11).
(b) Tweaking the interconnect framework to support the exynos-bus use case (patches 05--07/11). Exporting of_icc_get_from_provider() allows us to avoid hardcoding every single graph edge in the DT or driver source, and relaxing the requirement contained in that function removes the need to provide dummy node IDs in the DT. Adjusting the logic in apply_constraints() (drivers/interconnect/core.c) accounts for the fact that every bus is a separate entity and therefore a separate interconnect provider, albeit constituting a part of a larger hierarchy.
(c) Implementing interconnect providers in the exynos-bus devfreq driver and adding required DT properties for one selected platform, namely Exynos4412 (patches 08--09/11). Due to the fact that this aims to be a generic driver for various Exynos SoCs, node IDs are generated dynamically rather than hardcoded. This has been determined to be a simpler approach, but depends on changes described in (b).
(d) Implementing a sample interconnect consumer for exynos-mixer targeted at the issue referenced in [3], again with DT info only for Exynos4412 (patches 10--11/11).
Integration of devfreq and interconnect functionalities comes down to one extra line in the devfreq target() callback, which selects either the frequency calculated by the devfreq governor, or the one requested with the interconnect API, whichever is higher. All new code works equally well when CONFIG_INTERCONNECT is 'n' (as in exynos_defconfig) in which case all interconnect API functions are no-ops.
Artur Świgoń Samsung R&D Institute Poland Samsung Electronics
References: [1] Documentation/interconnect/interconnect.rst [2] Documentation/devicetree/bindings/interconnect/interconnect.txt [3] https://patchwork.kernel.org/patch/10861757
Artur Świgoń (10): devfreq: exynos-bus: Extract exynos_bus_profile_init() devfreq: exynos-bus: Extract exynos_bus_profile_init_passive() devfreq: exynos-bus: Change goto-based logic to if-else logic devfreq: exynos-bus: Clean up code icc: Export of_icc_get_from_provider() icc: Relax requirement in of_icc_get_from_provider() icc: Relax condition in apply_constraints() arm: dts: exynos: Add parents and #interconnect-cells to Exynos4412 devfreq: exynos-bus: Add interconnect functionality to exynos-bus arm: dts: exynos: Add interconnects to Exynos4412 mixer
Marek Szyprowski (1): drm: exynos: mixer: Add interconnect support
.../boot/dts/exynos4412-odroid-common.dtsi | 1 + arch/arm/boot/dts/exynos4412.dtsi | 10 + drivers/devfreq/exynos-bus.c | 296 ++++++++++++++---- drivers/gpu/drm/exynos/exynos_mixer.c | 68 +++- drivers/interconnect/core.c | 12 +- include/linux/interconnect-provider.h | 6 + 6 files changed, 314 insertions(+), 79 deletions(-)
dri-devel@lists.freedesktop.org
-
Artur Świgoń -
Chanwoo Choi -
Krzysztof Kozlowski