From mboxrd@z Thu Jan 1 00:00:00 1970 From: Artur =?UTF-8?Q?=C5=9Awigo=C5=84?= Subject: Re: [RFC PATCH v2 00/11] Simple QoS for exynos-bus driver using interconnect Date: Wed, 25 Sep 2019 08:37:11 +0200 Message-ID: References: <20190919142236.4071-1-a.swigon@samsung.com> <0cceb46a-fca1-62ae-c2cc-fb0f9b9bd132@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: <0cceb46a-fca1-62ae-c2cc-fb0f9b9bd132@samsung.com> Sender: linux-kernel-owner@vger.kernel.org To: Chanwoo Choi , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, dri-devel@lists.freedesktop.org Cc: a.swigon@samsung.com, myungjoo.ham@samsung.com, inki.dae@samsung.com, sw0312.kim@samsung.com, georgi.djakov@linaro.org, leonard.crestez@nxp.com, m.szyprowski@samsung.com, b.zolnierkie@samsung.com, krzk@kernel.org, "cpgs (cpgs@samsung.com)" , k.konieczny@samsung.com List-Id: linux-samsung-soc@vger.kernel.org On Wed, 2019-09-25 at 15:12 +0900, Chanwoo Choi wrote: > Hi, > > On 19. 9. 25. 오후 2:47, Artur Świgoń wrote: > > Hi, > > > > On Fri, 2019-09-20 at 11:14 +0900, Chanwoo Choi wrote: > > > Hi Artur, > > > > > > I tried to just build this patch on mainline kernel or linux-next. > > > But, when I applied them, merge conflict happens. You didn't develop > > > them on latest version. Please rebase them based on latest mainline kernel. > > > > I developed on top of next-20190918 on which I applied > > https://patchwork.kernel.org/cover/11149497/ as I mentioned in the cover > > letter. The dev_pm_qos patches and my RFC have just cleanly rebased together on > > top of next-20190920. Could you check if you have the dev_pm_qos patches (v5, > > the version number is missing in this one; link above) and if so, where does the > > conflict appear? > > I faced on the merge conflict of drivers/devfreq/exynos-bus.c. > I think that It is not related to to dev_pm_qos patch. I think that it is actually related to the specific version of dev_pm_qos (v5) that I used because patch 08/08 of dev_pm_qos series modifies exynos_bus_probe() in drivers/devfreq/exynos-bus.c (https://patchwork.kernel.org/patch/11149507/). I will rebase the next RFC (v3) on latest dev_pm_qos patches from Leonard and the latest Linux-next kernel. > Maybe, Kamil's patches[1] changed the many things of exynos-bus.c > If your test branch doesn't contain following patches, > you need to rebase your patches based on latest mainline kernel > from Linus Torvald. > [1] https://patchwork.kernel.org/cover/11083663/ > - [RESEND PATCH v5 0/4] add coupled regulators for Exynos5422/5800 Yes, requiring Kamil's patches is one of the changes in this RFC (v2), since they are already merged. > Today, I tried to apply these patch again based on latest mainline kernel. > The merge conflict happen still. > > - merge conflict log > Applying: devfreq: exynos-bus: Extract exynos_bus_profile_init() > error: patch failed: drivers/devfreq/exynos-bus.c:334 > error: drivers/devfreq/exynos-bus.c: patch does not apply > Patch failed at 0001 devfreq: exynos-bus: Extract exynos_bus_profile_init() > > > > > > > On 19. 9. 20. 오전 10:07, Chanwoo Choi wrote: > > > > Hi Artur, > > > > > > > > On v1, I mentioned that we need to discuss how to change > > > > the v2 for this. But, I have not received any reply from you on v1. > > > > And, without your reply from v1, you just send v2. > > > > > > > > I think that it is not proper development sequence. > > > > I have spent many times to review your patches > > > > and also I'll review your patches. You have to take care > > > > the reply of reviewer and and keep the basic rule > > > > of mailing contribution for discussion. > > > > > > > > On 19. 9. 19. 오후 11:22, 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 is achieved by > > > > > using dev_pm_qos_*() API[5]. 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. > > > > > > > > > > This patchset depends on [5]. > > > > > > > > > > --- Changes since v1 [6]: > > > > > * Rebase on [4] (coupled regulators). > > > > > * Rebase on [5] (dev_pm_qos for devfreq). > > > > > * Use dev_pm_qos_*() API[5] instead of overriding frequency in > > > > > exynos_bus_target(). > > > > > * Use IDR for node ID allocation. > > > > > * Avoid goto in functions extracted in patches 01 & 02 (cf. patch 04). > > > > > * Reverse order of multiplication and division in > > > > > mixer_set_memory_bandwidth() (patch 11) to avoid integer overflow. > > > > > > > > > > --- > > > > > 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/ (original issue) > > > > > [4] https://patchwork.kernel.org/cover/11083663/ (coupled regulators; merged) > > > > > [5] https://patchwork.kernel.org/cover/11149497/ (dev_pm_qos for devfreq) > > > > > [6] https://patchwork.kernel.org/cover/11054417/ (v1 of this RFC) > > > > > > > > > > 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 > > > > > interconnect: Export of_icc_get_from_provider() > > > > > interconnect: Relax requirement in of_icc_get_from_provider() > > > > > interconnect: 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 | 319 +++++++++++++----- > > > > > drivers/gpu/drm/exynos/exynos_mixer.c | 71 +++- > > > > > drivers/interconnect/core.c | 12 +- > > > > > include/linux/interconnect-provider.h | 6 + > > > > > 6 files changed, 327 insertions(+), 92 deletions(-) > > > > >