Re: [RFC PATCH v3 5/7] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
Hi Georgi,
On Wed, 2020-01-22 at 19:02 +0200, Georgi Djakov wrote:
> Hi Artur,
>
> On 12/20/19 13:56, Artur Świgoń wrote:
> > This patch adds interconnect functionality to the exynos-bus devfreq
> > driver.
> >
> > The SoC topology is a graph (or, more specifically, a tree) and its
> > edges are specified using the 'exynos,interconnect-parent-node' in the
> > DT. Due to unspecified relative probing order, -EPROBE_DEFER may be
> > propagated to ensure that the parent is probed before its children.
> >
> > 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 hardcoded but rather assigned at
>
> Just to note that usually the provider consists of multiple nodes and each
> node
> represents a single master or slave port on the AXI bus for example. I am not
> sure whether this represents correctly the Exynos hardware, so it's up to
> you.
>
> > runtime, in probing order (subject to the above-mentioned exception
> > regarding relative order). This approach allows for using this driver with
> > various Exynos SoCs.
>
> This sounds good. I am wondering whether such dynamic probing would be useful
> for other platforms too. Then maybe it would make sense to even have a common
> DT
> property, but we will see.
>
> Is this going to be used only together with devfreq?
Yes, this functions solely as an extension to devfreq, hence the slightly
unusual architecture (one icc_provider/icc_node per devfreq).
(Compared to a singleton icc_provider, this approach yields less code with
a very simple xlate()).
With exactly one icc_node for every devfreq device, I think I will actually
reuse the devfreq ID (as seen in the device name, e.g. the "3" in "devfreq3")
for the node ID. The devfreq framework already does the dynamic numbering
thing that I do in this patch using IDR.
> > 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.
>
> How about the case where CONFIG_INTERCONNECT=m. Looks like the build will fail
> if CONFIG_ARM_EXYNOS_BUS_DEVFREQ=y, so this dependency should be expressed in
> Kconfig.
I think adding:
depends on INTERCONNECT || !INTERCONNECT
under ARM_EXYNOS_BUS_DEVFREQ does the trick.
> >
> > Signed-off-by: Artur Świgoń
> > ---
> > drivers/devfreq/exynos-bus.c | 144 +++
> > 1 file changed, 144 insertions(+)
> >
> > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > index 9fdb188915e8..694a9581dcdb 100644
> > --- a/drivers/devfreq/exynos-bus.c
> > +++ b/drivers/devfreq/exynos-bus.c
> > @@ -14,14 +14,19 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > #define DEFAULT_SATURATION_RATIO 40
> >
> > +#define kbps_to_khz(x) ((x) / 8)
> > +
> > struct exynos_bus {
> > struct device *dev;
> >
> > @@ -35,6 +40,12 @@ struct exynos_bus {
> > struct opp_table *opp_table;
> > struct clk *clk;
> > unsigned int ratio;
> > +
> > + /* One provider per bus, one node per provider */
> > + struct icc_provider provider;
> > + struct icc_node *node;
> > +
> > + struct dev_pm_qos_request qos_req;
> > };
> >
> > /*
> > @@ -205,6 +216,39 @@ static void exynos_bus_passive_exit(struct device *dev)
> > clk_disable_unprepare(bus->clk);
> > }
> >
> > +static int exynos_bus_icc_set(struct icc_node *src, struct icc_node *dst)
> > +{
> > + struct exynos_bus *src_bus = src->data, *dst_bus = dst->data;
> > + s32 src_freq = kbps_to_khz(src->avg_bw);
> > + s32 dst_freq = kbps_to_khz(dst->avg_bw);
> > + int ret;
> > +
> > + ret = dev_pm_qos_update_request(_bus->qos_req, src_freq);
> > + if (ret < 0) {
> > + dev_err(src_bus->dev, "failed to update PM QoS request");
> > + return ret;
> > + }
> > +
> > + ret = dev_pm_qos_update_request(_bus->qos_req, dst_freq);
> > + if (ret < 0) {
> > + dev_err(dst_bus->dev, "failed to update PM QoS request");
> > + return ret;
> > + }
>
Re: [RFC PATCH v3 4/7] arm: dts: exynos: Add interconnect bindings for Exynos4412
Hi,
On Wed, 2020-01-22 at 18:54 +0200, Georgi Djakov wrote:
> Hi Artur,
>
> Thank you for your continuous work on this.
>
> On 12/20/19 13:56, Artur Świgoń wrote:
> > This patch adds the following properties to the Exynos4412 DT:
> > - exynos,interconnect-parent-node: to declare connections between
> > nodes in order to guarantee PM QoS requirements between nodes;
>
> Is this DT property documented somewhere? I believe that there should be a
> patch
> to document it somewhere in Documentation/devicetree/bindings/ before using
> it.
It will be documented in
Documentation/devicetree/bindings/devfreq/exynos-bus.txt
in the next version.
> > - #interconnect-cells: required by the interconnect framework.
> >
> > Note that #interconnect-cells is always zero and node IDs are not
> > hardcoded anywhere.
> >
> > Signed-off-by: Artur Świgoń
> > ---
> > arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > index 4ce3d77a6704..d9d70eacfcaf 100644
> > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > @@ -90,6 +90,7 @@
> > _dmc {
> > exynos,ppmu-device = <_dmc0_3>, <_dmc1_3>;
> > vdd-supply = <_reg>;
> > + #interconnect-cells = <0>;
> > status = "okay";
> > };
> >
> > @@ -106,6 +107,8 @@
> > _leftbus {
> > exynos,ppmu-device = <_leftbus_3>, <_rightbus_3>;
> > vdd-supply = <_reg>;
> > + exynos,interconnect-parent-node = <_dmc>;
> > + #interconnect-cells = <0>;
> > status = "okay";
> > };
> >
> > @@ -116,6 +119,8 @@
> >
> > _display {
> > exynos,parent-bus = <_leftbus>;
> > + exynos,interconnect-parent-node = <_leftbus>;
> > + #interconnect-cells = <0>;
> > status = "okay";
> > };
--
Artur Świgoń
Samsung R Institute Poland
Samsung Electronics
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v3 4/7] arm: dts: exynos: Add interconnect bindings for Exynos4412
On Tue, 2019-12-31 at 11:38 +0100, Krzysztof Kozlowski wrote:
> On Tue, 31 Dec 2019 at 11:23, Artur Świgoń wrote:
> > >
> > > The order of patches should reflect first of all real dependency.
> > > Whether it compiles, works at all and does not break anything. Logical
> > > dependency of "when the feature will start working" is
> > > irrelevant to DTS because DTS goes in separate way and driver is
> > > independent of it.
> >
> > The order of patches does indeed reflect real dependency. I can also reorder
> > them (preserving the dependencies) so that DTS patches go first in the
> > series
> > if this is the more preferred way.
>
> It looks wrong then. Driver should not depend on DTS. I cannot find
> the patch changing bindings (should be first in patchset) which could
> also point to this problem.
>
> It seems you added requirement for interconnect properties while it
> should be rather optional.
No, there is no requirement for interconnect properties (other than that it
simply does not make any sense to use the interconnect driver code and not the
DTS properties for it in the long run).
In case of the exynos-bus driver (code: patch 05, DTS: patch 04) if the DTS
properties ('exynos,interconnect-parent-node') are missing, the new code handles
it gracefully returning NULL from exynos_bus_icc_get_parent() (it is not an
error condition).
In case of the exynos-mixer driver (code: patch 07, DTS: patch 06) if the DTS
property ('interconnects') is missing, of_icc_get() returns NULL and the code
does
not try to set any contraints for a NULL path. Same thing happens if
CONFIG_INTERCONNECT is 'n'.
The only case when something breaks is when you try to use the interconnect
consumer (implemented in patches 06 & 07) when there is no interconnect provider
(patches 04 & 05), in which case of_icc_get() returns an error (since it cannot
find a path). From what I understand, it probably makes sense to merge any
interconnect consumers one cycle later than the provider.
> > > > I still think the order of these patches is the most logical one for
> > > > someone
> > > > reading this RFC as a whole.
> > >
> > > I am sorry but it brings only confusion. DTS is orthogonal of the
> > > driver code. You could even post the patchset without DTS (although then
> > > it would raise questions where is the user of it, but still, you
> > > could).
> > >
> > > Further, DTS describes also hardware so you could send certain DTS
> > > patches without driver implementation to describe the hardware.
> > >
> > > Driver code and DTS are kind of different worlds so mixing them up for
> > > logical review does not really make any sense.
> > >
> > > Not mentioning it is different than most of other patches on mailing
> > > lists.
> > >
> > > BTW, it is the same as bindings which should (almost) always go first as
> > > separate patches.
> >
> > Thanks for elaborating on this, I appreciate it.
> > Regarding your original concern, patches 04 & 06 are separate for several
> > reasons, one of which is that they are related to two different drivers
> > (exynos-bus vs. exynos-mixer).
>
> It's okay then (for them to be split).
>
> >
> > > >
> > > > > In certain cases dependency on DTS changes is ok:
> > > > > 1. Cleaning up deprecated properties,
> > > > > 2. Ignoring the backward compatibility for e.g. new platforms.
> > > > >
> > > > > None of these are applicable here.
> > > > >
> > > > > You need to rework it, put DTS changes at the end. This clearly shows
> > > > > that there is no wrong dependency.
> > > > >
> > > > > >
> > > > > > > Adjust the title to match the contents - you are not adding
> > > > > > > bindings but
> > > > > > > properties to bus nodes. Also the prefix is ARM: (look at recent
> > > > > > > commits).
> > > > > >
> > > > > > OK.
> > > > > >
> > > > > > > >
> > > > > > > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > > > > > > b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > > > > > > index 4ce3d77a6704..d9d70eacfcaf 100644
> > > > > > > > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > > > > > > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > >
Re: [RFC PATCH v3 4/7] arm: dts: exynos: Add interconnect bindings for Exynos4412
On Tue, 2019-12-31 at 11:02 +0100, Krzysztof Kozlowski wrote: > On Tue, Dec 31, 2019 at 10:41:47AM +0100, Artur Świgoń wrote: > > On Tue, 2019-12-31 at 10:22 +0100, Krzysztof Kozlowski wrote: > > > On Tue, Dec 31, 2019 at 08:18:01AM +0100, Artur Świgoń wrote: > > > > Hi, > > > > > > > > On Mon, 2019-12-30 at 16:44 +0100, Krzysztof Kozlowski wrote: > > > > > On Fri, Dec 20, 2019 at 12:56:50PM +0100, Artur Świgoń wrote: > > > > > > This patch adds the following properties to the Exynos4412 DT: > > > > > > - exynos,interconnect-parent-node: to declare connections between > > > > > > nodes in order to guarantee PM QoS requirements between nodes; > > > > > > - #interconnect-cells: required by the interconnect framework. > > > > > > > > > > > > Note that #interconnect-cells is always zero and node IDs are not > > > > > > hardcoded anywhere. > > > > > > > > > > > > Signed-off-by: Artur Świgoń > > > > > > --- > > > > > > arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 5 + > > > > > > 1 file changed, 5 insertions(+) > > > > > > > > > > The order of patches is confusing. Patches 4 and 6 are split - do the > > > > > depend on 5? I doubt but... > > > > > > > > Let me elaborate: > > > > > > > > The order of the patches in this series is such that every subsequent > > > > patch adds some functionality (and, of course, applying patches > > > > one-by-one > > > > yields a working kernel at every step). Specifically for patches 04--07: > > > > > > > > -- patch 04 adds interconnect _provider_ properties for Exynos4412; > > > > -- patch 05 implements interconnect provider logic (depends on patch > > > > 04); > > > > -- patch 06 adds interconnect _consumer_ properties for Exynos4412 > > > > mixer; > > > > -- patch 07 implements interconnect consumer logic (depends on patches > > > > 05 & 06); > > > > > > > > My reasoning is that this order allows to e.g., merge the interconnect > > > > provider for exynos-bus and leave the consumers for later (not limited > > > > to > > > > the mixer). I hope this makes sense. > > > > > > It is wrong. The driver should not depend on DTS changes because: > > > 1. DTS always go through separate branch and tree, so last patch > > >will have to wait up to 3 cycles (!!!), > > > 2. You break backward compatibility. > > > > It is up to the definition of "depends". The driver is _not_ broken without > > the DTS patches, but the interconnect functionality will not be available. > > > > The only requirement is that if we want to have a working interconnect > > consumer, there needs to be a working interconnet provider (and I used > > the word "depends" to specify what needs what in order to work as intended). > > > > The order of patches should reflect first of all real dependency. > Whether it compiles, works at all and does not break anything. Logical > dependency of "when the feature will start working" is > irrelevant to DTS because DTS goes in separate way and driver is > independent of it. The order of patches does indeed reflect real dependency. I can also reorder them (preserving the dependencies) so that DTS patches go first in the series if this is the more preferred way. > > I still think the order of these patches is the most logical one for someone > > reading this RFC as a whole. > > I am sorry but it brings only confusion. DTS is orthogonal of the > driver code. You could even post the patchset without DTS (although then > it would raise questions where is the user of it, but still, you > could). > > Further, DTS describes also hardware so you could send certain DTS > patches without driver implementation to describe the hardware. > > Driver code and DTS are kind of different worlds so mixing them up for > logical review does not really make any sense. > > Not mentioning it is different than most of other patches on mailing > lists. > > BTW, it is the same as bindings which should (almost) always go first as > separate patches. Thanks for elaborating on this, I appreciate it. Regarding your original concern, patches 04 & 06 are separate for several reasons, one of which is that they are related to two different drivers (exynos-bus vs. exynos-mixer).
Re: [RFC PATCH v3 4/7] arm: dts: exynos: Add interconnect bindings for Exynos4412
On Tue, 2019-12-31 at 10:22 +0100, Krzysztof Kozlowski wrote:
> On Tue, Dec 31, 2019 at 08:18:01AM +0100, Artur Świgoń wrote:
> > Hi,
> >
> > On Mon, 2019-12-30 at 16:44 +0100, Krzysztof Kozlowski wrote:
> > > On Fri, Dec 20, 2019 at 12:56:50PM +0100, Artur Świgoń wrote:
> > > > This patch adds the following properties to the Exynos4412 DT:
> > > > - exynos,interconnect-parent-node: to declare connections between
> > > > nodes in order to guarantee PM QoS requirements between nodes;
> > > > - #interconnect-cells: required by the interconnect framework.
> > > >
> > > > Note that #interconnect-cells is always zero and node IDs are not
> > > > hardcoded anywhere.
> > > >
> > > > Signed-off-by: Artur Świgoń
> > > > ---
> > > > arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 5 +
> > > > 1 file changed, 5 insertions(+)
> > >
> > > The order of patches is confusing. Patches 4 and 6 are split - do the
> > > depend on 5? I doubt but...
> >
> > Let me elaborate:
> >
> > The order of the patches in this series is such that every subsequent
> > patch adds some functionality (and, of course, applying patches one-by-one
> > yields a working kernel at every step). Specifically for patches 04--07:
> >
> > -- patch 04 adds interconnect _provider_ properties for Exynos4412;
> > -- patch 05 implements interconnect provider logic (depends on patch 04);
> > -- patch 06 adds interconnect _consumer_ properties for Exynos4412 mixer;
> > -- patch 07 implements interconnect consumer logic (depends on patches
> > 05 & 06);
> >
> > My reasoning is that this order allows to e.g., merge the interconnect
> > provider for exynos-bus and leave the consumers for later (not limited to
> > the mixer). I hope this makes sense.
>
> It is wrong. The driver should not depend on DTS changes because:
> 1. DTS always go through separate branch and tree, so last patch
>will have to wait up to 3 cycles (!!!),
> 2. You break backward compatibility.
It is up to the definition of "depends". The driver is _not_ broken without
the DTS patches, but the interconnect functionality will not be available.
The only requirement is that if we want to have a working interconnect
consumer, there needs to be a working interconnet provider (and I used
the word "depends" to specify what needs what in order to work as intended).
I still think the order of these patches is the most logical one for someone
reading this RFC as a whole.
> In certain cases dependency on DTS changes is ok:
> 1. Cleaning up deprecated properties,
> 2. Ignoring the backward compatibility for e.g. new platforms.
>
> None of these are applicable here.
>
> You need to rework it, put DTS changes at the end. This clearly shows
> that there is no wrong dependency.
>
> >
> > > Adjust the title to match the contents - you are not adding bindings but
> > > properties to bus nodes. Also the prefix is ARM: (look at recent
> > > commits).
> >
> > OK.
> >
> > > >
> > > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > > b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > > index 4ce3d77a6704..d9d70eacfcaf 100644
> > > > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > > @@ -90,6 +90,7 @@
> > > > _dmc {
> > > > exynos,ppmu-device = <_dmc0_3>, <_dmc1_3>;
> > > > vdd-supply = <_reg>;
> > > > + #interconnect-cells = <0>;
> > >
> > > This does not look like property of Odroid but Exynos4412 or Exynos4.
> >
> > Strangely enough, this file is where the 'exynos,parent-bus' (aka.
> > 'devfreq')
> > properties are located (and everything in this RFC concerns devfreq).
>
> I cannot find exynos,parent-bus in exynos4412-odroid-common.dtsi. Can
> you elaborate?
Currently a name change is being made: 'devfreq' -> 'exynos,parent-bus'
https://patchwork.kernel.org/patch/11304549/
(a dependency of this RFC; also available in devfreq-testing branch)
Best regards,
--
Artur Świgoń
Samsung R Institute Poland
Samsung Electronics
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v3 4/7] arm: dts: exynos: Add interconnect bindings for Exynos4412
Hi,
On Mon, 2019-12-30 at 16:44 +0100, Krzysztof Kozlowski wrote:
> On Fri, Dec 20, 2019 at 12:56:50PM +0100, Artur Świgoń wrote:
> > This patch adds the following properties to the Exynos4412 DT:
> > - exynos,interconnect-parent-node: to declare connections between
> > nodes in order to guarantee PM QoS requirements between nodes;
> > - #interconnect-cells: required by the interconnect framework.
> >
> > Note that #interconnect-cells is always zero and node IDs are not
> > hardcoded anywhere.
> >
> > Signed-off-by: Artur Świgoń
> > ---
> > arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 5 +
> > 1 file changed, 5 insertions(+)
>
> The order of patches is confusing. Patches 4 and 6 are split - do the
> depend on 5? I doubt but...
Let me elaborate:
The order of the patches in this series is such that every subsequent
patch adds some functionality (and, of course, applying patches one-by-one
yields a working kernel at every step). Specifically for patches 04--07:
-- patch 04 adds interconnect _provider_ properties for Exynos4412;
-- patch 05 implements interconnect provider logic (depends on patch 04);
-- patch 06 adds interconnect _consumer_ properties for Exynos4412 mixer;
-- patch 07 implements interconnect consumer logic (depends on patches
05 & 06);
My reasoning is that this order allows to e.g., merge the interconnect
provider for exynos-bus and leave the consumers for later (not limited to
the mixer). I hope this makes sense.
> Adjust the title to match the contents - you are not adding bindings but
> properties to bus nodes. Also the prefix is ARM: (look at recent
> commits).
OK.
> >
> > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > index 4ce3d77a6704..d9d70eacfcaf 100644
> > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > @@ -90,6 +90,7 @@
> > _dmc {
> > exynos,ppmu-device = <_dmc0_3>, <_dmc1_3>;
> > vdd-supply = <_reg>;
> > + #interconnect-cells = <0>;
>
> This does not look like property of Odroid but Exynos4412 or Exynos4.
Strangely enough, this file is where the 'exynos,parent-bus' (aka. 'devfreq')
properties are located (and everything in this RFC concerns devfreq).
Regards,
--
Artur Świgoń
Samsung R Institute Poland
Samsung Electronics
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v3 7/7] drm: exynos: mixer: Add interconnect support
Hi,
On Tue, 2019-12-24 at 13:56 +0900, Inki Dae wrote:
> Hi,
>
> 19. 12. 20. 오후 8:56에 Artur Świgoń 이(가) 쓴 글:
> > From: Marek Szyprowski
> >
> > This patch adds interconnect support to exynos-mixer. The mixer works
> > the same as before when CONFIG_INTERCONNECT is 'n'.
> >
> > Co-developed-by: Artur Świgoń
> > Signed-off-by: Artur Świgoń
> > Signed-off-by: Marek Szyprowski
> > ---
> > drivers/gpu/drm/exynos/exynos_mixer.c | 71 +--
> > 1 file changed, 66 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
> > b/drivers/gpu/drm/exynos/exynos_mixer.c
> > index 6cfdb95fef2f..a7e7240a055f 100644
> > --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> > +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> > @@ -13,6 +13,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -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,40 @@ 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 = >base.state->adjusted_mode;
> > + struct mixer_context *ctx = crtc->ctx;
> > + unsigned long bw, bandwidth = 0;
> > + int i, j, sub;
> > +
> > + if (!ctx->soc_path)
> > + return;
> > +
> > + for (i = 0; i < MIXER_WIN_NR; i++) {
> > + struct drm_plane *plane = >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 = bandwidth / 4 * 5;
> > +
> > + dev_dbg(ctx->dev, "exynos-mixer: safe bandwidth %ld Bps\n", bandwidth);
> > + icc_set_bw(ctx->soc_path, Bps_to_icc(bandwidth), 0);
> > +}
> > +
> > static void mixer_atomic_begin(struct exynos_drm_crtc *crtc)
> > {
> > struct mixer_context *ctx = crtc->ctx;
> > @@ -982,6 +1018,7 @@ static void mixer_atomic_flush(struct exynos_drm_crtc
> > *crtc)
> > if (!test_bit(MXR_BIT_POWERED, _ctx->flags))
> > return;
> >
> > + mixer_set_memory_bandwidth(crtc);
> > mixer_enable_sync(mixer_ctx);
> > exynos_crtc_handle_event(crtc);
> > }
> > @@ -1029,6 +1066,7 @@ static void mixer_disable(struct exynos_drm_crtc
> > *crtc)
> > for (i = 0; i < MIXER_WIN_NR; i++)
> > mixer_disable_plane(crtc, >planes[i]);
> > > +mixer_set_memory_bandwidth(crtc);
>
> Your intention is to set peak and average bandwidth to 0 at disabling mixer
> device?
Yes. In general, setting the requested bandwidth to zero means "do not override
the devfreq setting" because only constraints of type DEV_PM_QOS_MIN_FREQUENCY
are used (cf. patch 05 of this series). I will make sure to reflect that in the
commit message.
Moreover, this RFC does not really make use of the peak bandwidth (yet). It is
set to zero in this patch and ignored in patch 05 (cf. exynos_bus_icc_set()).
Only the average bandwidth is translated to a minimum frequency constraint,
overriding devfreq if necessary.
A possible modification to mixer_set_memory_bandwidth() could be:
- bandwidth = bandwidth / 4 * 5;
+ peak_bandwidth = bandwidth / 4 * 5;
in mixer_set_memory_bandwidth() plus some additional logic in
exynos_bus_icc_set().
Best regards,
--
Artur Świgoń
Samsung R Institute Poland
Samsung Electronics
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v3 6/7] arm: dts: exynos: Add interconnects to Exynos4412 mixer
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.
Signed-off-by: Artur Świgoń
---
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 48868947373e..13a679a9a107 100644
--- a/arch/arm/boot/dts/exynos4412.dtsi
+++ b/arch/arm/boot/dts/exynos4412.dtsi
@@ -771,6 +771,7 @@
clock-names = "mixer", "hdmi", "sclk_hdmi", "vp";
clocks = < CLK_MIXER>, < CLK_HDMI>,
< CLK_SCLK_HDMI>, < CLK_VP>;
+ interconnects = <_display _dmc>;
};
{
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v3 2/7] interconnect: Relax requirement in of_icc_get_from_provider()
This patch relaxes the condition in of_icc_get_from_provider() so that it is no longer required to set #interconnect-cells = <1> in the DT. In case of the devfreq driver for exynos-bus, #interconnect-cells is always zero. Signed-off-by: Artur Świgoń Acked-by: Krzysztof Kozlowski --- drivers/interconnect/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index e6035c199369..74c68898a350 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -335,7 +335,7 @@ struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec) struct icc_node *node = ERR_PTR(-EPROBE_DEFER); struct icc_provider *provider; - if (!spec || spec->args_count != 1) + if (!spec) return ERR_PTR(-EINVAL); mutex_lock(_lock); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v3 7/7] drm: exynos: mixer: Add interconnect support
From: Marek Szyprowski
This patch adds interconnect support to exynos-mixer. The mixer works
the same as before when CONFIG_INTERCONNECT is 'n'.
Co-developed-by: Artur Świgoń
Signed-off-by: Artur Świgoń
Signed-off-by: Marek Szyprowski
---
drivers/gpu/drm/exynos/exynos_mixer.c | 71 +--
1 file changed, 66 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 6cfdb95fef2f..a7e7240a055f 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -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,40 @@ 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 = >base.state->adjusted_mode;
+ struct mixer_context *ctx = crtc->ctx;
+ unsigned long bw, bandwidth = 0;
+ int i, j, sub;
+
+ if (!ctx->soc_path)
+ return;
+
+ for (i = 0; i < MIXER_WIN_NR; i++) {
+ struct drm_plane *plane = >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 = bandwidth / 4 * 5;
+
+ dev_dbg(ctx->dev, "exynos-mixer: safe bandwidth %ld Bps\n", bandwidth);
+ icc_set_bw(ctx->soc_path, Bps_to_icc(bandwidth), 0);
+}
+
static void mixer_atomic_begin(struct exynos_drm_crtc *crtc)
{
struct mixer_context *ctx = crtc->ctx;
@@ -982,6 +1018,7 @@ static void mixer_atomic_flush(struct exynos_drm_crtc
*crtc)
if (!test_bit(MXR_BIT_POWERED, _ctx->flags))
return;
+ mixer_set_memory_bandwidth(crtc);
mixer_enable_sync(mixer_ctx);
exynos_crtc_handle_event(crtc);
}
@@ -1029,6 +1066,7 @@ static void mixer_disable(struct exynos_drm_crtc *crtc)
for (i = 0; i < MIXER_WIN_NR; i++)
mixer_disable_plane(crtc, >planes[i]);
+ mixer_set_memory_bandwidth(crtc);
exynos_drm_pipe_clk_enable(crtc, false);
pm_runtime_put(ctx->dev);
@@ -1220,12 +1258,22 @@ static int mixer_probe(struct platform_device *pdev)
struct device *dev = >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(>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 +1281,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, >flags);
@@ -1242,17 +1291,29 @@ static int mixer_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, ctx);
ret = component_add(>dev, _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(>dev);
+ struct device *dev = >dev;
+ struct mixer_context *ctx = dev_get_drvda
[RFC PATCH v3 0/7] PM / devfreq: Simple QoS for exynos-bus using interconnect
The following patchset adds interconnect[1][2] framework support to the exynos-bus devfreq driver. Extending the devfreq driver with interconnect functionality started as a response to the issue referenced in [3]. The patches can be subdivided into three groups: (a) Tweaking the interconnect framework to support the exynos-bus use case (patches 01--03/07). Exporting of_icc_get_from_provider() allows to avoid hardcoding every single graph edge in the DT or driver source, and relaxing the requirement on #interconnect-cells removes the need to provide dummy node IDs in the DT. A new field in struct icc_provider is used to explicitly allow configuring node pairs from two different providers. (b) Implementing interconnect providers in the exynos-bus devfreq driver and adding required DT properties for one selected platform, namely Exynos4412 (patches 04--05/07). Due to the fact that this aims to be a generic driver for various Exynos SoCs, node IDs are generated dynamically (rather than hardcoded). (c) Implementing a sample interconnect consumer for exynos-mixer targeted at solving the issue referenced in [3], again with DT properties only for Exynos4412 (patches 06--07/07). Integration of devfreq and interconnect frameworks is achieved by using the dev_pm_qos_*() API. When CONFIG_INTERCONNECT is 'n' (such as in exynos_defconfig) all interconnect API functions are no-ops. This series depends on these three patches (merged into devfreq-next[6]): * https://patchwork.kernel.org/patch/11279087/ * https://patchwork.kernel.org/patch/11279093/ * https://patchwork.kernel.org/patch/11293765/ and on this series: * https://patchwork.kernel.org/cover/11304545/ (which does not apply cleanly on next-20191220, adding --exclude=arch/arm/boot/dts/exynos5422-odroid-core.dtsi to 'git am' is a quick workaround) --- 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 [4]: * 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. --- Artur Świgoń Samsung R 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/11054417/ (v1 of this RFC) [5] https://patchwork.kernel.org/cover/11152595/ (v2 of this RFC) [6] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-next Artur Świgoń (6): interconnect: Export of_icc_get_from_provider() interconnect: Relax requirement in of_icc_get_from_provider() interconnect: Allow inter-provider pairs to be configured arm: dts: exynos: Add interconnect bindings for 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| 5 + arch/arm/boot/dts/exynos4412.dtsi | 1 + drivers/devfreq/exynos-bus.c | 144 ++ drivers/gpu/drm/exynos/exynos_mixer.c | 71 - drivers/interconnect/core.c | 16 +- include/linux/interconnect-provider.h | 8 + 6 files changed, 232 insertions(+), 13 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v3 4/7] arm: dts: exynos: Add interconnect bindings for Exynos4412
This patch adds the following properties to the Exynos4412 DT:
- exynos,interconnect-parent-node: to declare connections between
nodes in order to guarantee PM QoS requirements between nodes;
- #interconnect-cells: required by the interconnect framework.
Note that #interconnect-cells is always zero and node IDs are not
hardcoded anywhere.
Signed-off-by: Artur Świgoń
---
arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 5 +
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index 4ce3d77a6704..d9d70eacfcaf 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -90,6 +90,7 @@
_dmc {
exynos,ppmu-device = <_dmc0_3>, <_dmc1_3>;
vdd-supply = <_reg>;
+ #interconnect-cells = <0>;
status = "okay";
};
@@ -106,6 +107,8 @@
_leftbus {
exynos,ppmu-device = <_leftbus_3>, <_rightbus_3>;
vdd-supply = <_reg>;
+ exynos,interconnect-parent-node = <_dmc>;
+ #interconnect-cells = <0>;
status = "okay";
};
@@ -116,6 +119,8 @@
_display {
exynos,parent-bus = <_leftbus>;
+ exynos,interconnect-parent-node = <_leftbus>;
+ #interconnect-cells = <0>;
status = "okay";
};
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v3 3/7] interconnect: Allow inter-provider pairs to be configured
In the exynos-bus devfreq driver every bus is probed separately and is
assigned a separate interconnect provider. However, the interconnect
framework does not call the '->set' callback for pairs of nodes which
belong to different providers.
This patch adds support for a new boolean 'inter_set' field in struct
icc_provider. Setting it to 'true' enables calling '->set' for
inter-provider node pairs. All existing users of the interconnect
framework allocate this structure with kzalloc, and are therefore
unaffected.
Signed-off-by: Artur Świgoń
---
drivers/interconnect/core.c | 11 +--
include/linux/interconnect-provider.h | 2 ++
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 74c68898a350..a28bd0f8a497 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -259,23 +259,22 @@ static int aggregate_requests(struct icc_node *node)
static int apply_constraints(struct icc_path *path)
{
struct icc_node *next, *prev = NULL;
+ struct icc_provider *p;
int ret = -EINVAL;
int i;
for (i = 0; i < path->num_nodes; i++) {
next = path->reqs[i].node;
+ p = next->provider;
- /*
-* Both endpoints should be valid master-slave pairs of the
-* same interconnect provider that will be configured.
-*/
- if (!prev || next->provider != prev->provider) {
+ /* both endpoints should be valid master-slave pairs */
+ if (!prev || (p != prev->provider && !p->inter_set)) {
prev = next;
continue;
}
/* set the constraints */
- ret = next->provider->set(prev, next);
+ ret = p->set(prev, next);
if (ret)
goto out;
diff --git a/include/linux/interconnect-provider.h
b/include/linux/interconnect-provider.h
index cc965b8fab53..b6ae0ee686c5 100644
--- a/include/linux/interconnect-provider.h
+++ b/include/linux/interconnect-provider.h
@@ -41,6 +41,7 @@ struct icc_node *of_icc_xlate_onecell(struct of_phandle_args
*spec,
* @xlate: provider-specific callback for mapping nodes from phandle arguments
* @dev: the device this interconnect provider belongs to
* @users: count of active users
+ * @inter_set: whether inter-provider pairs will be configured with @set
* @data: pointer to private data
*/
struct icc_provider {
@@ -53,6 +54,7 @@ struct icc_provider {
struct icc_node* (*xlate)(struct of_phandle_args *spec, void *data);
struct device *dev;
int users;
+ boolinter_set;
void*data;
};
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v3 5/7] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
This patch adds interconnect functionality to the exynos-bus devfreq
driver.
The SoC topology is a graph (or, more specifically, a tree) and its
edges are specified using the 'exynos,interconnect-parent-node' in the
DT. Due to unspecified relative probing order, -EPROBE_DEFER may be
propagated to ensure that the parent is probed before its children.
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 hardcoded but rather assigned at
runtime, in probing order (subject to the above-mentioned exception
regarding relative order). 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.
Signed-off-by: Artur Świgoń
---
drivers/devfreq/exynos-bus.c | 144 +++
1 file changed, 144 insertions(+)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 9fdb188915e8..694a9581dcdb 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -14,14 +14,19 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
+#include
#include
#include
#define DEFAULT_SATURATION_RATIO 40
+#define kbps_to_khz(x) ((x) / 8)
+
struct exynos_bus {
struct device *dev;
@@ -35,6 +40,12 @@ struct exynos_bus {
struct opp_table *opp_table;
struct clk *clk;
unsigned int ratio;
+
+ /* One provider per bus, one node per provider */
+ struct icc_provider provider;
+ struct icc_node *node;
+
+ struct dev_pm_qos_request qos_req;
};
/*
@@ -205,6 +216,39 @@ static void exynos_bus_passive_exit(struct device *dev)
clk_disable_unprepare(bus->clk);
}
+static int exynos_bus_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+ struct exynos_bus *src_bus = src->data, *dst_bus = dst->data;
+ s32 src_freq = kbps_to_khz(src->avg_bw);
+ s32 dst_freq = kbps_to_khz(dst->avg_bw);
+ int ret;
+
+ ret = dev_pm_qos_update_request(_bus->qos_req, src_freq);
+ if (ret < 0) {
+ dev_err(src_bus->dev, "failed to update PM QoS request");
+ return ret;
+ }
+
+ ret = dev_pm_qos_update_request(_bus->qos_req, dst_freq);
+ if (ret < 0) {
+ dev_err(dst_bus->dev, "failed to update PM QoS request");
+ return ret;
+ }
+
+ return 0;
+}
+
+static struct icc_node *exynos_bus_icc_xlate(struct of_phandle_args *spec,
+void *data)
+{
+ struct exynos_bus *bus = data;
+
+ if (spec->np != bus->dev->of_node)
+ return ERR_PTR(-EINVAL);
+
+ return bus->node;
+}
+
static int exynos_bus_parent_parse_of(struct device_node *np,
struct exynos_bus *bus)
{
@@ -419,6 +463,96 @@ static int exynos_bus_profile_init_passive(struct
exynos_bus *bus,
return 0;
}
+static struct icc_node *exynos_bus_icc_get_parent(struct exynos_bus *bus)
+{
+ struct device_node *np = bus->dev->of_node;
+ struct of_phandle_args args;
+ int num, ret;
+
+ num = of_count_phandle_with_args(np, "exynos,interconnect-parent-node",
+ "#interconnect-cells");
+ if (num != 1)
+ return NULL; /* parent nodes are optional */
+
+ ret = of_parse_phandle_with_args(np, "exynos,interconnect-parent-node",
+ "#interconnect-cells", 0, );
+ if (ret < 0)
+ return ERR_PTR(ret);
+
+ of_node_put(args.np);
+
+ return of_icc_get_from_provider();
+}
+
+static int exynos_bus_icc_init(struct exynos_bus *bus)
+{
+ static DEFINE_IDA(ida);
+
+ struct device *dev = bus->dev;
+ struct icc_provider *provider = >provider;
+ struct icc_node *node, *parent_node;
+ int id, ret;
+
+ /* Initialize the interconnect provider */
+ provider->set = exynos_bus_icc_set;
+ provider->aggregate = icc_std_aggregate;
+ provider->xlate = exynos_bus_icc_xlate;
+ provider->dev = dev;
+ provider->inter_set = true;
+ provider->data = bus;
+
+ ret = icc_provider_add(provider);
+ if (ret < 0)
+ return ret;
+
+ ret = id = ida_alloc(, GFP_KERNEL);
+ if (ret < 0)
+ goto err_id;
+
+ node = icc_node_create(id);
+ if (IS_ERR(node)) {
+ ret = PTR_ERR(node);
+ goto err_node;
+ }
+
+
[RFC PATCH v3 1/7] interconnect: Export of_icc_get_from_provider()
This patch makes the above function public (for use in exynos-bus devfreq
driver).
Signed-off-by: Artur Świgoń
Reviewed-by: Krzysztof Kozlowski
Reviewed-by: Chanwoo Choi
---
drivers/interconnect/core.c | 3 ++-
include/linux/interconnect-provider.h | 6 ++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 63c164264b73..e6035c199369 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -330,7 +330,7 @@ EXPORT_SYMBOL_GPL(of_icc_xlate_onecell);
* Returns a valid pointer to struct icc_node on success or ERR_PTR()
* on failure.
*/
-static struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec)
+struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec)
{
struct icc_node *node = ERR_PTR(-EPROBE_DEFER);
struct icc_provider *provider;
@@ -349,6 +349,7 @@ static struct icc_node *of_icc_get_from_provider(struct
of_phandle_args *spec)
return node;
}
+EXPORT_SYMBOL_GPL(of_icc_get_from_provider);
/**
* of_icc_get() - get a path handle from a DT node based on name
diff --git a/include/linux/interconnect-provider.h
b/include/linux/interconnect-provider.h
index 0c494534b4d3..cc965b8fab53 100644
--- a/include/linux/interconnect-provider.h
+++ b/include/linux/interconnect-provider.h
@@ -103,6 +103,7 @@ void icc_node_del(struct icc_node *node);
int icc_nodes_remove(struct icc_provider *provider);
int icc_provider_add(struct icc_provider *provider);
int icc_provider_del(struct icc_provider *provider);
+struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec);
#else
@@ -154,6 +155,11 @@ static inline int icc_provider_del(struct icc_provider
*provider)
return -ENOTSUPP;
}
+struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec)
+{
+ return ERR_PTR(-ENOTSUPP);
+}
+
#endif /* CONFIG_INTERCONNECT */
#endif /* __LINUX_INTERCONNECT_PROVIDER_H */
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 08/11] arm: dts: exynos: Add parents and #interconnect-cells to Exynos4412
On Wed, 2019-12-18 at 19:29 +0900, Chanwoo Choi wrote:
> Hi
>
> 2019년 12월 18일 (수) 오후 7:18, Artur Świgoń 님이 작성:
> >
> > Hi,
> >
> > On Mon, 2019-12-16 at 11:59 +0900, Chanwoo Choi wrote:
> > > Hi,
> > >
> > > On 12/16/19 9:51 AM, Chanwoo Choi wrote:
> > > > On 9/19/19 11:22 PM, Artur Świgoń wrote:
> > > > > From: Artur Świgoń
> > > > >
> > > > > This patch adds two fields to the Exynos4412 DTS:
> > > > > - parent: to declare connections between nodes that are not in a
> > > > > parent-child relation in devfreq;
> > > > > - #interconnect-cells: required by the interconnect framework.
> > > > >
> > > > > Please note that #interconnect-cells is always zero and node IDs are
> > > > > not
> > > > > hardcoded anywhere. The above-mentioned parent-child relation in
> > > > > devfreq
> > > > > means that there is a shared power line ('devfreq' property). The
> > > > > 'parent'
> > > > > property only signifies an interconnect connection.
> > > > >
> > > > > Signed-off-by: Artur Świgoń
> > > > > ---
> > > > > arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 1 +
> > > > > arch/arm/boot/dts/exynos4412.dtsi | 9 +
> > > > > 2 files changed, 10 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > > > b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > > > index ea55f377d17c..bdd61ae86103 100644
> > > > > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > > > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > > > @@ -106,6 +106,7 @@
> > > > > _leftbus {
> > > > > devfreq-events = <_leftbus_3>, <_rightbus_3>;
> > > > > vdd-supply = <_reg>;
> > > > > + parent = <_dmc>;
> > > >
> > > > As I mentioned on other reply,
> > > > I'm not sure to use the specific 'parent' property to make
> > > > the connection between buses. If possible, you better to
> > > > use the standard way like OF graph. Except for making
> > > > the connection between buses by 'parent' property,
> > > > looks good to me.
> > >
> > > I tried to think it continuously. I withdraw the my opinion
> > > using OF graph. If you make the property name like the following
> > > example, it is possible for exynos.
> > > - exynos,interconnect-parent-node = <_dmc>; or other proper name.
> > >
> > > Regardless of existing 'devfreq' property, I think you better to
> > > make the connection between buses for only interconnect as following
> > > example: This make it possible user can draw the correct tree by tracking
> > > the 'exynos,interconnect-parent-node' value.
> >
> > OK, for v3 I will add 'exynos,interconnect-parent-node' to bus_dmc,
> > bus_leftbus and bus_display as you suggested below and change the code
> > so that the 'devfreq' (or the upcoming 'exynos,parent-bus') property is
> > not taken into account.
>
> I'd like you to make the v3 based on my patches[1]
> [1]
> https://protect2.fireeye.com/url?k=3fbd62a4-6276e59a-3fbce9eb-0cc47a31309a-5329151b98fc2653=https://lkml.org/lkml/2019/12/17/21
> - [PATCH 0/9] PM / devfreq: Remove deprecated 'devfreq' and
> 'devfreq-events' properties
>
> I uploaded the patches to devfreq-testing branch[2]
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing
OK.
>
> >
> > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > index ea55f377d17c..53f87f46e161 100644
> > > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > @@ -90,6 +90,7 @@
> > > _dmc {
> > > devfreq-events = <_dmc0_3>, <_dmc1_3>;
> > > vdd-supply = <_reg>;
> > > + #interconnect-cells = <0>;
> > > status = "okay";
> > > };
> > >
> > > @@ -106,6 +107,8 @@
> > > _leftbus {
> > > devfreq-events = <_leftbus_3>, <_rightbus_3>;
> > > vdd-supply = <_reg>;
> > > + exynos,interconnect-parent-node =
Re: [RFC PATCH v2 08/11] arm: dts: exynos: Add parents and #interconnect-cells to Exynos4412
Hi,
On Mon, 2019-12-16 at 11:59 +0900, Chanwoo Choi wrote:
> Hi,
>
> On 12/16/19 9:51 AM, Chanwoo Choi wrote:
> > On 9/19/19 11:22 PM, Artur Świgoń wrote:
> > > From: Artur Świgoń
> > >
> > > This patch adds two fields to the Exynos4412 DTS:
> > > - parent: to declare connections between nodes that are not in a
> > > parent-child relation in devfreq;
> > > - #interconnect-cells: required by the interconnect framework.
> > >
> > > Please note that #interconnect-cells is always zero and node IDs are not
> > > hardcoded anywhere. The above-mentioned parent-child relation in devfreq
> > > means that there is a shared power line ('devfreq' property). The 'parent'
> > > property only signifies an interconnect connection.
> > >
> > > Signed-off-by: Artur Świgoń
> > > ---
> > > arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 1 +
> > > arch/arm/boot/dts/exynos4412.dtsi | 9 +
> > > 2 files changed, 10 insertions(+)
> > >
> > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > index ea55f377d17c..bdd61ae86103 100644
> > > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > > @@ -106,6 +106,7 @@
> > > _leftbus {
> > > devfreq-events = <_leftbus_3>, <_rightbus_3>;
> > > vdd-supply = <_reg>;
> > > + parent = <_dmc>;
> >
> > As I mentioned on other reply,
> > I'm not sure to use the specific 'parent' property to make
> > the connection between buses. If possible, you better to
> > use the standard way like OF graph. Except for making
> > the connection between buses by 'parent' property,
> > looks good to me.
>
> I tried to think it continuously. I withdraw the my opinion
> using OF graph. If you make the property name like the following
> example, it is possible for exynos.
> - exynos,interconnect-parent-node = <_dmc>; or other proper name.
>
> Regardless of existing 'devfreq' property, I think you better to
> make the connection between buses for only interconnect as following
> example: This make it possible user can draw the correct tree by tracking
> the 'exynos,interconnect-parent-node' value.
OK, for v3 I will add 'exynos,interconnect-parent-node' to bus_dmc,
bus_leftbus and bus_display as you suggested below and change the code
so that the 'devfreq' (or the upcoming 'exynos,parent-bus') property is
not taken into account.
> diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> index ea55f377d17c..53f87f46e161 100644
> --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> @@ -90,6 +90,7 @@
> _dmc {
> devfreq-events = <_dmc0_3>, <_dmc1_3>;
> vdd-supply = <_reg>;
> + #interconnect-cells = <0>;
> status = "okay";
> };
>
> @@ -106,6 +107,8 @@
> _leftbus {
> devfreq-events = <_leftbus_3>, <_rightbus_3>;
> vdd-supply = <_reg>;
> + exynos,interconnect-parent-node = <_dmc>;
> + #interconnect-cells = <0>;
> status = "okay";
> };
>
> @@ -116,6 +119,8 @@
>
> _display {
> devfreq = <_leftbus>;
> + exynos,interconnect-parent-node = <_leftbus>;
> + #interconnect-cells = <0>;
> status = "okay";
> };
>
>
> >
> >
> > > status = "okay";
> > > };
> > >
> > > diff --git a/arch/arm/boot/dts/exynos4412.dtsi
> > > b/arch/arm/boot/dts/exynos4412.dtsi
> > > index d20db2dfe8e2..a70a671acacd 100644
> > > --- a/arch/arm/boot/dts/exynos4412.dtsi
> > > +++ b/arch/arm/boot/dts/exynos4412.dtsi
> > > @@ -390,6 +390,7 @@
> > > clocks = < CLK_DIV_DMC>;
> > > clock-names = "bus";
> > > operating-points-v2 = <_dmc_opp_table>;
> > > + #interconnect-cells = <0>;
> > > status = "disabled";
> > > };
> > >
> > > @@ -398,6 +399,7 @@
> > > clocks = < CLK_DIV_ACP>;
> > > clock-names = "bus";
> > > operating-points-v2 = <_acp_opp_table>;
> > > +
Re: [RFC PATCH v2 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
On Wed, 2019-12-18 at 19:39 +0900, Chanwoo Choi wrote:
> Hi,
>
> 2019년 12월 18일 (수) 오후 7:19, Artur Świgoń 님이 작성:
> >
> > Hi,
> >
> > Thank you for the review.
> >
> > On Mon, 2019-12-16 at 09:44 +0900, Chanwoo Choi wrote:
> > > Hi,
> > >
> > > On 9/19/19 11:22 PM, Artur Świgoń wrote:
> > > > From: Artur Świgoń
> > > >
> > > > This patch adds interconnect functionality to the exynos-bus devfreq
> > > > driver.
> > > >
> > > > The SoC topology is a graph (or, more specifically, a tree) and most of
> > > > its edges are taken from the devfreq parent-child hierarchy (cf.
> > > > Documentation/devicetree/bindings/devfreq/exynos-bus.txt). Due to
> > > > unspecified relative probing order, -EPROBE_DEFER may be propagated to
> > > > guarantee that a child is probed before its parent.
> > > >
> > > > 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 hardcoded but rather assigned at
> > > > runtime, in probing order (subject to the above-mentioned exception
> > > > regarding relative order). 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.
> > > >
> > > > Signed-off-by: Artur Świgoń
> > > > ---
> > > > drivers/devfreq/exynos-bus.c | 153 +++
> > > > 1 file changed, 153 insertions(+)
> > > >
> > > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > > > index 8d44810cac69..e0232202720d 100644
> > > > --- a/drivers/devfreq/exynos-bus.c
> > > > +++ b/drivers/devfreq/exynos-bus.c
> > > > @@ -14,14 +14,19 @@
> > > > #include
> > > > #include
> > > > #include
> > > > +#include
> > > > +#include
> > > > #include
> > > > #include
> > > > #include
> > > > +#include
> > > > #include
> > > > #include
> > > >
> > > > #define DEFAULT_SATURATION_RATIO 40
> > > >
> > > > +#define icc_units_to_khz(x) ((x) / 8)
> > >
> > > icc_units_to_khz() -> kpbs_to_khz()
> >
> > OK
> >
> > > > +
> > > > struct exynos_bus {
> > > > struct device *dev;
> > > >
> > > > @@ -35,6 +40,12 @@ struct exynos_bus {
> > > > struct opp_table *opp_table;
> > > > struct clk *clk;
> > > > unsigned int ratio;
> > > > +
> > > > + /* One provider per bus, one node per provider */
> > > > + struct icc_provider provider;
> > > > + struct icc_node *node;
> > > > +
> > > > + struct dev_pm_qos_request qos_req;
> > > > };
> > > >
> > > > /*
> > > > @@ -59,6 +70,13 @@ exynos_bus_ops_edev(enable_edev);
> > > > exynos_bus_ops_edev(disable_edev);
> > > > exynos_bus_ops_edev(set_event);
> > > >
> > > > +static int exynos_bus_next_id(void)
> > > > +{
> > > > + static DEFINE_IDA(exynos_bus_icc_ida);
> > > > +
> > > > + return ida_alloc(_bus_icc_ida, GFP_KERNEL);
> > > > +}
> > > > +
> > > > static int exynos_bus_get_event(struct exynos_bus *bus,
> > > > struct devfreq_event_data *edata)
> > > > {
> > > > @@ -171,6 +189,38 @@ static void exynos_bus_passive_exit(struct device
> > > > *dev)
> > > > clk_disable_unprepare(bus->clk);
> > > > }
> > > >
> > > > +static int exynos_bus_icc_set(struct icc_node *src, struct icc_node
> > > > *dst)
> > > > +{
> > > > + struct exynos_bus *src_bus = src->data, *dst_bus = dst->data;
> > > > + s32 src_freq = icc_units_to_kh
Re: [RFC PATCH v2 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
Hi,
Thank you for the review.
On Mon, 2019-12-16 at 09:44 +0900, Chanwoo Choi wrote:
> Hi,
>
> On 9/19/19 11:22 PM, Artur Świgoń wrote:
> > From: Artur Świgoń
> >
> > This patch adds interconnect functionality to the exynos-bus devfreq
> > driver.
> >
> > The SoC topology is a graph (or, more specifically, a tree) and most of
> > its edges are taken from the devfreq parent-child hierarchy (cf.
> > Documentation/devicetree/bindings/devfreq/exynos-bus.txt). Due to
> > unspecified relative probing order, -EPROBE_DEFER may be propagated to
> > guarantee that a child is probed before its parent.
> >
> > 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 hardcoded but rather assigned at
> > runtime, in probing order (subject to the above-mentioned exception
> > regarding relative order). 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.
> >
> > Signed-off-by: Artur Świgoń
> > ---
> > drivers/devfreq/exynos-bus.c | 153 +++
> > 1 file changed, 153 insertions(+)
> >
> > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > index 8d44810cac69..e0232202720d 100644
> > --- a/drivers/devfreq/exynos-bus.c
> > +++ b/drivers/devfreq/exynos-bus.c
> > @@ -14,14 +14,19 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > #define DEFAULT_SATURATION_RATIO 40
> >
> > +#define icc_units_to_khz(x) ((x) / 8)
>
> icc_units_to_khz() -> kpbs_to_khz()
OK
> > +
> > struct exynos_bus {
> > struct device *dev;
> >
> > @@ -35,6 +40,12 @@ struct exynos_bus {
> > struct opp_table *opp_table;
> > struct clk *clk;
> > unsigned int ratio;
> > +
> > + /* One provider per bus, one node per provider */
> > + struct icc_provider provider;
> > + struct icc_node *node;
> > +
> > + struct dev_pm_qos_request qos_req;
> > };
> >
> > /*
> > @@ -59,6 +70,13 @@ exynos_bus_ops_edev(enable_edev);
> > exynos_bus_ops_edev(disable_edev);
> > exynos_bus_ops_edev(set_event);
> >
> > +static int exynos_bus_next_id(void)
> > +{
> > + static DEFINE_IDA(exynos_bus_icc_ida);
> > +
> > + return ida_alloc(_bus_icc_ida, GFP_KERNEL);
> > +}
> > +
> > static int exynos_bus_get_event(struct exynos_bus *bus,
> > struct devfreq_event_data *edata)
> > {
> > @@ -171,6 +189,38 @@ static void exynos_bus_passive_exit(struct device *dev)
> > clk_disable_unprepare(bus->clk);
> > }
> >
> > +static int exynos_bus_icc_set(struct icc_node *src, struct icc_node *dst)
> > +{
> > + struct exynos_bus *src_bus = src->data, *dst_bus = dst->data;
> > + s32 src_freq = icc_units_to_khz(src->avg_bw);
> > + s32 dst_freq = icc_units_to_khz(dst->avg_bw);
> > +
> > + dev_pm_qos_update_request(_bus->qos_req, src_freq);
>
> Have to check the return value.
> If return error, show the waring with dev_warn.
OK, I will change it to:
ret = dev_pm_qos_update_request(_bus->qos_req, src_freq);
if (ret < 0) {
dev_warn(src_bus->dev, "failed to update PM QoS request");
return ret;
}
> > + dev_pm_qos_update_request(_bus->qos_req, dst_freq);
>
> ditto.
OK (same as above).
> > +
> > + return 0;
> > +}
> > +
> > +static int exynos_bus_icc_aggregate(struct icc_node *node, u32 tag, u32
> > avg_bw,
> > + u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
> > +{
> > + *agg_avg += avg_bw;
> > + *agg_peak = max(*agg_peak, peak_bw);
> > +
> > + return 0;
> > +}
> > +
> > +static struct icc_node *exynos_bus_icc_xlate(struct of_phandle_args *spec,
> > +void *data)
> > +{
> > + struct exynos_bus *bus = data;
> > +
> > + if (spec->np != bus->dev->of_node)
> > + return E
Re: [RFC PATCH v2 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
Hi Chanwoo,
On Wed, 2019-09-25 at 16:03 +0900, Chanwoo Choi wrote:
> Hi,
>
> I need the time to dig the ICC framework
> to understand them detailed. After that, I'll review this.
Any updates on this topic?
Regardless of the purpose of this RFC, I think patches 01-04
are still beneficial to devfreq. I can rebase and post them
as a separate series if you wish.
> Basically, I agree this approach. But, I'm wondering
> the existing binding method between 'bus_leftbus' and 'bus_dmc'.
> From before, I thought that devfreq framework need to
> enhance the binding method between parent devfreq device
> and passive devfreq device instead of 'devfreq' property.
>
> On this patch, use the same binding way between
> 'bus_leftbus' and 'bus_dmc' with 'parent' property
> as following:
>
> +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> @@ -106,6 +106,7 @@
> _leftbus {
> devfreq-events = <_leftbus_3>, <_rightbus_3>;
> vdd-supply = <_reg>;
> + parent = <_dmc>;
> status = "okay";
> };
>
> I'm not sure about continuing to use this method for new feature.
> If possible, hope to replace the existing binding style
> with new method like of_graph. Actually, I don't know the correct method.
>
>
> On 19. 9. 19. 오후 11:22, Artur Świgoń wrote:
> > From: Artur Świgoń
> >
> > This patch adds interconnect functionality to the exynos-bus devfreq
> > driver.
> >
> > The SoC topology is a graph (or, more specifically, a tree) and most of
> > its edges are taken from the devfreq parent-child hierarchy (cf.
> > Documentation/devicetree/bindings/devfreq/exynos-bus.txt). Due to
> > unspecified relative probing order, -EPROBE_DEFER may be propagated to
> > guarantee that a child is probed before its parent.
> >
> > 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 hardcoded but rather assigned at
> > runtime, in probing order (subject to the above-mentioned exception
> > regarding relative order). 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.
> >
> > Signed-off-by: Artur Świgoń
> > ---
> > drivers/devfreq/exynos-bus.c | 153 +++
> > 1 file changed, 153 insertions(+)
> >
> > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > index 8d44810cac69..e0232202720d 100644
> > --- a/drivers/devfreq/exynos-bus.c
> > +++ b/drivers/devfreq/exynos-bus.c
> > @@ -14,14 +14,19 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > #define DEFAULT_SATURATION_RATIO 40
> >
> > +#define icc_units_to_khz(x) ((x) / 8)
> > +
> > struct exynos_bus {
> > struct device *dev;
> >
> > @@ -35,6 +40,12 @@ struct exynos_bus {
> > struct opp_table *opp_table;
> > struct clk *clk;
> > unsigned int ratio;
> > +
> > + /* One provider per bus, one node per provider */
> > + struct icc_provider provider;
> > + struct icc_node *node;
> > +
> > + struct dev_pm_qos_request qos_req;
> > };
> >
> > /*
> > @@ -59,6 +70,13 @@ exynos_bus_ops_edev(enable_edev);
> > exynos_bus_ops_edev(disable_edev);
> > exynos_bus_ops_edev(set_event);
> >
> > +static int exynos_bus_next_id(void)
> > +{
> > + static DEFINE_IDA(exynos_bus_icc_ida);
> > +
> > + return ida_alloc(_bus_icc_ida, GFP_KERNEL);
> > +}
> > +
> > static int exynos_bus_get_event(struct exynos_bus *bus,
> > struct devfreq_event_data *edata)
> > {
> > @@ -171,6 +189,38 @@ static void exynos_bus_passive_exit(struct device *dev)
> > clk_disable_unprepare(bus->clk);
> > }
> >
> > +static int exynos_bus_icc_set(struct icc_node *src, struct icc_node *dst)
> > +{
> > + struct exynos_bus *src_bus = src->data, *dst_bus = dst->data;
> > + s32 src_freq = icc_units_to_khz(src->avg_bw);
> > + s32 dst_freq = icc_units_to_khz(dst->avg_bw);
> > +
&g
Re: [RFC PATCH v2 04/11] devfreq: exynos-bus: Clean up code
Hi,
On Fri, 2019-09-20 at 11:22 +0900, Chanwoo Choi wrote:
> Hi Artur,
>
> On 19. 9. 19. 오후 11:22, Artur Świgoń wrote:
> > From: Artur Świgoń
> >
> > This patch adds minor improvements to the exynos-bus driver.
> >
> > Signed-off-by: Artur Świgoń
> > Reviewed-by: Krzysztof Kozlowski
> > ---
> > drivers/devfreq/exynos-bus.c | 66 ++--
> > 1 file changed, 25 insertions(+), 41 deletions(-)
> >
> > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > index 60ad4319fd80..8d44810cac69 100644
> > --- a/drivers/devfreq/exynos-bus.c
> > +++ b/drivers/devfreq/exynos-bus.c
> > @@ -15,11 +15,10 @@
> > #include
> > #include
> > #include
> > -#include
> > +#include
> > #include
> > #include
> > #include
> > -#include
> >
> > #define DEFAULT_SATURATION_RATIO 40
> >
> > @@ -178,7 +177,7 @@ static int exynos_bus_parent_parse_of(struct
> > device_node *np,
> > struct device *dev = bus->dev;
> > struct opp_table *opp_table;
> > const char *vdd = "vdd";
> > - int i, ret, count, size;
> > + int i, ret, count;
> >
> > opp_table = dev_pm_opp_set_regulators(dev, , 1);
> > if (IS_ERR(opp_table)) {
> > @@ -201,8 +200,7 @@ static int exynos_bus_parent_parse_of(struct
> > device_node *np,
> > }
> > bus->edev_count = count;
> >
> > - size = sizeof(*bus->edev) * count;
> > - bus->edev = devm_kzalloc(dev, size, GFP_KERNEL);
> > + bus->edev = devm_kcalloc(dev, count, sizeof(*bus->edev), GFP_KERNEL);
> > if (!bus->edev) {
> > ret = -ENOMEM;
> > goto err_regulator;
> > @@ -301,10 +299,9 @@ static int exynos_bus_profile_init(struct exynos_bus
> > *bus,
> > profile->exit = exynos_bus_exit;
> >
> > ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
> > - if (!ondemand_data) {
> > - ret = -ENOMEM;
> > - goto err;
> > - }
> > + if (!ondemand_data)
> > + return -ENOMEM;
> > +
> > ondemand_data->upthreshold = 40;
> > ondemand_data->downdifferential = 5;
> >
> > @@ -314,8 +311,7 @@ static int exynos_bus_profile_init(struct exynos_bus
> > *bus,
> > ondemand_data);
> > if (IS_ERR(bus->devfreq)) {
> > dev_err(dev, "failed to add devfreq device\n");
> > - ret = PTR_ERR(bus->devfreq);
> > - goto err;
> > + return PTR_ERR(bus->devfreq);
> > }
> >
> > /*
> > @@ -325,16 +321,13 @@ static int exynos_bus_profile_init(struct exynos_bus
> > *bus,
> > ret = exynos_bus_enable_edev(bus);
> > if (ret < 0) {
> > dev_err(dev, "failed to enable devfreq-event devices\n");
> > - goto err;
> > + return ret;
> > }
> >
> > ret = exynos_bus_set_event(bus);
> > - if (ret < 0) {
> > + if (ret < 0)
> > dev_err(dev, "failed to set event to devfreq-event devices\n");
> > - goto err;
>
> Instead of removing 'goto err', just return err as I commented[1] on v1.
> [1] https://lkml.org/lkml/2019/7/26/331
>
> > - }
> >
> > -err:
> > return ret;
>
> And you just keep 'return ret' or you can change it as 'return 0'.
OK, I went for:
ret = exynos_bus_set_event(bus);
if (ret < 0) {
dev_err(dev, "failed to set event to devfreq-event devices\n");
return ret;
}
return 0;
> > }
> >
> > @@ -344,7 +337,6 @@ static int exynos_bus_profile_init_passive(struct
> > exynos_bus *bus,
> > struct device *dev = bus->dev;
> > struct devfreq_passive_data *passive_data;
> > struct devfreq *parent_devfreq;
> > - int ret = 0;
> >
> > /* Initialize the struct profile and governor data for passive device */
> > profile->target = exynos_bus_target;
> > @@ -352,30 +344,26 @@ static int exynos_bus_profile_init_passive(struct
> > exynos_bus *bus,
> >
> > /* Get the instance of parent devfreq device */
> > parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0);
> > - if (IS_ERR(parent_devfreq)) {
> > - ret = -EPROBE_DEFER;
> > - goto err;
> > - }
> > + if (IS_ERR(parent_devfreq))
> > + return -EPROB
Re: [RFC PATCH v2 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
Hi,
On Wed, 2019-09-25 at 16:03 +0900, Chanwoo Choi wrote:
> Hi,
>
> I need the time to dig the ICC framework
> to understand them detailed. After that, I'll review this.
>
> Basically, I agree this approach. But, I'm wondering
> the existing binding method between 'bus_leftbus' and 'bus_dmc'.
> From before, I thought that devfreq framework need to
> enhance the binding method between parent devfreq device
> and passive devfreq device instead of 'devfreq' property.
>
> On this patch, use the same binding way between
> 'bus_leftbus' and 'bus_dmc' with 'parent' property
> as following:
>
> +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> @@ -106,6 +106,7 @@
> _leftbus {
> devfreq-events = <_leftbus_3>, <_rightbus_3>;
> vdd-supply = <_reg>;
> + parent = <_dmc>;
> status = "okay";
> };
>
> I'm not sure about continuing to use this method for new feature.
> If possible, hope to replace the existing binding style
> with new method like of_graph. Actually, I don't know the correct method.
Adding the 'parent' binding was the simplest solution to create this
RFC to show the concept of using interconnect functionality in devfreq.
I agree that a method like of_graph is probably more elegant. I am open
to suggestions.
> On 19. 9. 19. 오후 11:22, Artur Świgoń wrote:
> > From: Artur Świgoń
> >
> > This patch adds interconnect functionality to the exynos-bus devfreq
> > driver.
> >
> > The SoC topology is a graph (or, more specifically, a tree) and most of
> > its edges are taken from the devfreq parent-child hierarchy (cf.
> > Documentation/devicetree/bindings/devfreq/exynos-bus.txt). Due to
> > unspecified relative probing order, -EPROBE_DEFER may be propagated to
> > guarantee that a child is probed before its parent.
> >
> > 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 hardcoded but rather assigned at
> > runtime, in probing order (subject to the above-mentioned exception
> > regarding relative order). 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.
> >
> > Signed-off-by: Artur Świgoń
> > ---
> > drivers/devfreq/exynos-bus.c | 153 +++
> > 1 file changed, 153 insertions(+)
> >
> > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > index 8d44810cac69..e0232202720d 100644
> > --- a/drivers/devfreq/exynos-bus.c
> > +++ b/drivers/devfreq/exynos-bus.c
> > @@ -14,14 +14,19 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > #define DEFAULT_SATURATION_RATIO 40
> >
> > +#define icc_units_to_khz(x) ((x) / 8)
> > +
> > struct exynos_bus {
> > struct device *dev;
> >
> > @@ -35,6 +40,12 @@ struct exynos_bus {
> > struct opp_table *opp_table;
> > struct clk *clk;
> > unsigned int ratio;
> > +
> > + /* One provider per bus, one node per provider */
> > + struct icc_provider provider;
> > + struct icc_node *node;
> > +
> > + struct dev_pm_qos_request qos_req;
> > };
> >
> > /*
> > @@ -59,6 +70,13 @@ exynos_bus_ops_edev(enable_edev);
> > exynos_bus_ops_edev(disable_edev);
> > exynos_bus_ops_edev(set_event);
> >
> > +static int exynos_bus_next_id(void)
> > +{
> > + static DEFINE_IDA(exynos_bus_icc_ida);
> > +
> > + return ida_alloc(_bus_icc_ida, GFP_KERNEL);
> > +}
> > +
> > static int exynos_bus_get_event(struct exynos_bus *bus,
> > struct devfreq_event_data *edata)
> > {
> > @@ -171,6 +189,38 @@ static void exynos_bus_passive_exit(struct device *dev)
> > clk_disable_unprepare(bus->clk);
> > }
> >
> > +static int exynos_bus_icc_set(struct icc_node *src, struct icc_node *dst)
> > +{
> > + struct exynos_bus *src_bus = src->data, *dst_bus = dst->data;
> > + s32 src_freq = icc_units_to_khz(src->avg_bw);
> > + s32 dst_freq = ic
Re: [RFC PATCH v2 01/11] devfreq: exynos-bus: Extract exynos_bus_profile_init()
On Wed, 2019-09-25 at 15:37 +0900, Chanwoo Choi wrote:
> Hi,
>
> On 19. 9. 25. 오후 2:44, Artur Świgoń wrote:
> > Hi,
> >
> > On Fri, 2019-09-20 at 11:15 +0900, Chanwoo Choi wrote:
> > > Hi,
> > >
> > > As I already replied on v1, patch1/2/3 clean-up code
> > > for readability without any behavior changes.
> > >
> > > I think that you better to merge patch1/2/3 to one patch.
> >
> > Yes, when writing the cover letter I think I forgot to explain why I
> > decided not
> > to merge these patches. Basically, none of the diff algorithms available in
> > git
> > (I've got v2.17.1) is able to produce a readable patch with these changes
> > combined together into a single patch (functions are intermixed together in
> > the
> > output, git thinks that 'exynos_bus_probe' is a new function).
>
> After merged three patches, as you commented, looks like that
> 'exynos_bus_probe'
> is new function. Your patch style(three patches) is better than one merged
> patch.
> Keep your original patches. Thanks.
I know that having three separate patches is suboptimal, but they are more
readable
this way. I am glad you agree. I will keep them separate. Thank you for your
comments.
> >
> > Please take a look at the diff at the bottom of this message to see how
> > patches
> > 01..03 look when combined. If such patch looks acceptable to you, I can
> > merge.
> >
> > > On 19. 9. 19. 오후 11:22, Artur Świgoń wrote:
> > > > From: Artur Świgoń
> > > >
> > > > This patch adds a new static function, exynos_bus_profile_init(),
> > > > extracted
> > > > from exynos_bus_probe().
> > > >
> > > > Signed-off-by: Artur Świgoń
> > > > ---
> > > > drivers/devfreq/exynos-bus.c | 92 +---
> > > > 1 file changed, 53 insertions(+), 39 deletions(-)
> > > >
> > > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > > > index 29f422469960..78f38b7fb596 100644
> > > > --- a/drivers/devfreq/exynos-bus.c
> > > > +++ b/drivers/devfreq/exynos-bus.c
> > > > @@ -287,12 +287,62 @@ static int exynos_bus_parse_of(struct device_node
> > > > *np,
> > > > return ret;
> > > > }
> > > >
> > > > +static int exynos_bus_profile_init(struct exynos_bus *bus,
> > > > + struct devfreq_dev_profile *profile)
> > > > +{
> > > > + struct device *dev = bus->dev;
> > > > + struct devfreq_simple_ondemand_data *ondemand_data;
> > > > + int ret;
> > > > +
> > > > + /* Initialize the struct profile and governor data for parent
> > > > device */
> > > > + profile->polling_ms = 50;
> > > > + profile->target = exynos_bus_target;
> > > > + profile->get_dev_status = exynos_bus_get_dev_status;
> > > > + profile->exit = exynos_bus_exit;
> > > > +
> > > > + ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data),
> > > > GFP_KERNEL);
> > > > + if (!ondemand_data) {
> > > > + ret = -ENOMEM;
> > > > + goto err;
> > > > + }
> > > > + ondemand_data->upthreshold = 40;
> > > > + ondemand_data->downdifferential = 5;
> > > > +
> > > > + /* Add devfreq device to monitor and handle the exynos bus */
> > > > + bus->devfreq = devm_devfreq_add_device(dev, profile,
> > > > +
> > > > DEVFREQ_GOV_SIMPLE_ONDEMAND,
> > > > + ondemand_data);
> > > > + if (IS_ERR(bus->devfreq)) {
> > > > + dev_err(dev, "failed to add devfreq device\n");
> > > > + ret = PTR_ERR(bus->devfreq);
> > > > + goto err;
> > > > + }
> > > > +
> > > > + /*
> > > > +* Enable devfreq-event to get raw data which is used to
> > > > determine
> > > > +* current bus load.
> > > > +*/
> > > > + ret = exynos_bus_enable_edev(bus);
> > > > + if (ret < 0) {
> > > > + dev_err(dev, "failed to enable devfreq-event
> &g
Re: [RFC PATCH v2 00/11] Simple QoS for exynos-bus driver using interconnect
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 platfor
Re: [RFC PATCH v2 00/11] Simple QoS for exynos-bus driver using interconnect
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? > 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 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) > > > > > > Ar
Re: [RFC PATCH v2 01/11] devfreq: exynos-bus: Extract exynos_bus_profile_init()
Hi,
On Fri, 2019-09-20 at 11:15 +0900, Chanwoo Choi wrote:
> Hi,
>
> As I already replied on v1, patch1/2/3 clean-up code
> for readability without any behavior changes.
>
> I think that you better to merge patch1/2/3 to one patch.
Yes, when writing the cover letter I think I forgot to explain why I decided not
to merge these patches. Basically, none of the diff algorithms available in git
(I've got v2.17.1) is able to produce a readable patch with these changes
combined together into a single patch (functions are intermixed together in the
output, git thinks that 'exynos_bus_probe' is a new function).
Please take a look at the diff at the bottom of this message to see how patches
01..03 look when combined. If such patch looks acceptable to you, I can merge.
> On 19. 9. 19. 오후 11:22, Artur Świgoń wrote:
> > From: Artur Świgoń
> >
> > This patch adds a new static function, exynos_bus_profile_init(), extracted
> > from exynos_bus_probe().
> >
> > Signed-off-by: Artur Świgoń
> > ---
> > drivers/devfreq/exynos-bus.c | 92 +---
> > 1 file changed, 53 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > index 29f422469960..78f38b7fb596 100644
> > --- a/drivers/devfreq/exynos-bus.c
> > +++ b/drivers/devfreq/exynos-bus.c
> > @@ -287,12 +287,62 @@ static int exynos_bus_parse_of(struct device_node *np,
> > return ret;
> > }
> >
> > +static int exynos_bus_profile_init(struct exynos_bus *bus,
> > + struct devfreq_dev_profile *profile)
> > +{
> > + struct device *dev = bus->dev;
> > + struct devfreq_simple_ondemand_data *ondemand_data;
> > + int ret;
> > +
> > + /* Initialize the struct profile and governor data for parent device */
> > + profile->polling_ms = 50;
> > + profile->target = exynos_bus_target;
> > + profile->get_dev_status = exynos_bus_get_dev_status;
> > + profile->exit = exynos_bus_exit;
> > +
> > + ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
> > + if (!ondemand_data) {
> > + ret = -ENOMEM;
> > + goto err;
> > + }
> > + ondemand_data->upthreshold = 40;
> > + ondemand_data->downdifferential = 5;
> > +
> > + /* Add devfreq device to monitor and handle the exynos bus */
> > + bus->devfreq = devm_devfreq_add_device(dev, profile,
> > + DEVFREQ_GOV_SIMPLE_ONDEMAND,
> > + ondemand_data);
> > + if (IS_ERR(bus->devfreq)) {
> > + dev_err(dev, "failed to add devfreq device\n");
> > + ret = PTR_ERR(bus->devfreq);
> > + goto err;
> > + }
> > +
> > + /*
> > +* Enable devfreq-event to get raw data which is used to determine
> > +* current bus load.
> > +*/
> > + ret = exynos_bus_enable_edev(bus);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to enable devfreq-event devices\n");
> > + goto err;
> > + }
> > +
> > + ret = exynos_bus_set_event(bus);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to set event to devfreq-event devices\n");
> > + goto err;
> > + }
> > +
> > +err:
> > + return ret;
> > +}
> > +
> > static int exynos_bus_probe(struct platform_device *pdev)
> > {
> > struct device *dev = >dev;
> > struct device_node *np = dev->of_node, *node;
> > struct devfreq_dev_profile *profile;
> > - struct devfreq_simple_ondemand_data *ondemand_data;
> > struct devfreq_passive_data *passive_data;
> > struct devfreq *parent_devfreq;
> > struct exynos_bus *bus;
> > @@ -334,45 +384,9 @@ static int exynos_bus_probe(struct platform_device
> > *pdev)
> > if (passive)
> > goto passive;
> >
> > - /* Initialize the struct profile and governor data for parent device */
> > - profile->polling_ms = 50;
> > - profile->target = exynos_bus_target;
> > - profile->get_dev_status = exynos_bus_get_dev_status;
> > - profile->exit = exynos_bus_exit;
> > -
> > - ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
> > - if (!ondemand_data) {
> > - ret = -ENOMEM;
> > + ret = exynos_bus_profile_init(bus, profile);
> > + if (ret < 0)
> > goto err;
> > - }
>
[RFC PATCH v2 02/11] devfreq: exynos-bus: Extract exynos_bus_profile_init_passive()
From: Artur Świgoń
This patch adds a new static function, exynos_bus_profile_init_passive(),
extracted from exynos_bus_probe().
Signed-off-by: Artur Świgoń
---
drivers/devfreq/exynos-bus.c | 70 +---
1 file changed, 42 insertions(+), 28 deletions(-)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 78f38b7fb596..f85bed241631 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -338,13 +338,51 @@ static int exynos_bus_profile_init(struct exynos_bus *bus,
return ret;
}
+static int exynos_bus_profile_init_passive(struct exynos_bus *bus,
+ struct devfreq_dev_profile *profile)
+{
+ struct device *dev = bus->dev;
+ struct devfreq_passive_data *passive_data;
+ struct devfreq *parent_devfreq;
+ int ret = 0;
+
+ /* Initialize the struct profile and governor data for passive device */
+ profile->target = exynos_bus_target;
+ profile->exit = exynos_bus_passive_exit;
+
+ /* Get the instance of parent devfreq device */
+ parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0);
+ if (IS_ERR(parent_devfreq)) {
+ ret = -EPROBE_DEFER;
+ goto err;
+ }
+
+ passive_data = devm_kzalloc(dev, sizeof(*passive_data), GFP_KERNEL);
+ if (!passive_data) {
+ ret = -ENOMEM;
+ goto err;
+ }
+ passive_data->parent = parent_devfreq;
+
+ /* Add devfreq device for exynos bus with passive governor */
+ bus->devfreq = devm_devfreq_add_device(dev, profile,
DEVFREQ_GOV_PASSIVE,
+ passive_data);
+ if (IS_ERR(bus->devfreq)) {
+ dev_err(dev,
+ "failed to add devfreq dev with passive governor\n");
+ ret = PTR_ERR(bus->devfreq);
+ goto err;
+ }
+
+err:
+ return ret;
+}
+
static int exynos_bus_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct device_node *np = dev->of_node, *node;
struct devfreq_dev_profile *profile;
- struct devfreq_passive_data *passive_data;
- struct devfreq *parent_devfreq;
struct exynos_bus *bus;
int ret, max_state;
unsigned long min_freq, max_freq;
@@ -390,33 +428,9 @@ static int exynos_bus_probe(struct platform_device *pdev)
goto out;
passive:
- /* Initialize the struct profile and governor data for passive device */
- profile->target = exynos_bus_target;
- profile->exit = exynos_bus_passive_exit;
-
- /* Get the instance of parent devfreq device */
- parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0);
- if (IS_ERR(parent_devfreq)) {
- ret = -EPROBE_DEFER;
+ ret = exynos_bus_profile_init_passive(bus, profile);
+ if (ret < 0)
goto err;
- }
-
- passive_data = devm_kzalloc(dev, sizeof(*passive_data), GFP_KERNEL);
- if (!passive_data) {
- ret = -ENOMEM;
- goto err;
- }
- passive_data->parent = parent_devfreq;
-
- /* Add devfreq device for exynos bus with passive governor */
- bus->devfreq = devm_devfreq_add_device(dev, profile,
DEVFREQ_GOV_PASSIVE,
- passive_data);
- if (IS_ERR(bus->devfreq)) {
- dev_err(dev,
- "failed to add devfreq dev with passive governor\n");
- ret = PTR_ERR(bus->devfreq);
- goto err;
- }
out:
max_state = bus->devfreq->profile->max_state;
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v2 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
From: Artur Świgoń
This patch adds interconnect functionality to the exynos-bus devfreq
driver.
The SoC topology is a graph (or, more specifically, a tree) and most of
its edges are taken from the devfreq parent-child hierarchy (cf.
Documentation/devicetree/bindings/devfreq/exynos-bus.txt). Due to
unspecified relative probing order, -EPROBE_DEFER may be propagated to
guarantee that a child is probed before its parent.
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 hardcoded but rather assigned at
runtime, in probing order (subject to the above-mentioned exception
regarding relative order). 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.
Signed-off-by: Artur Świgoń
---
drivers/devfreq/exynos-bus.c | 153 +++
1 file changed, 153 insertions(+)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 8d44810cac69..e0232202720d 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -14,14 +14,19 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
+#include
#include
#include
#define DEFAULT_SATURATION_RATIO 40
+#define icc_units_to_khz(x) ((x) / 8)
+
struct exynos_bus {
struct device *dev;
@@ -35,6 +40,12 @@ struct exynos_bus {
struct opp_table *opp_table;
struct clk *clk;
unsigned int ratio;
+
+ /* One provider per bus, one node per provider */
+ struct icc_provider provider;
+ struct icc_node *node;
+
+ struct dev_pm_qos_request qos_req;
};
/*
@@ -59,6 +70,13 @@ exynos_bus_ops_edev(enable_edev);
exynos_bus_ops_edev(disable_edev);
exynos_bus_ops_edev(set_event);
+static int exynos_bus_next_id(void)
+{
+ static DEFINE_IDA(exynos_bus_icc_ida);
+
+ return ida_alloc(_bus_icc_ida, GFP_KERNEL);
+}
+
static int exynos_bus_get_event(struct exynos_bus *bus,
struct devfreq_event_data *edata)
{
@@ -171,6 +189,38 @@ static void exynos_bus_passive_exit(struct device *dev)
clk_disable_unprepare(bus->clk);
}
+static int exynos_bus_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+ struct exynos_bus *src_bus = src->data, *dst_bus = dst->data;
+ s32 src_freq = icc_units_to_khz(src->avg_bw);
+ s32 dst_freq = icc_units_to_khz(dst->avg_bw);
+
+ dev_pm_qos_update_request(_bus->qos_req, src_freq);
+ dev_pm_qos_update_request(_bus->qos_req, dst_freq);
+
+ return 0;
+}
+
+static int exynos_bus_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
+ u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
+{
+ *agg_avg += avg_bw;
+ *agg_peak = max(*agg_peak, peak_bw);
+
+ return 0;
+}
+
+static struct icc_node *exynos_bus_icc_xlate(struct of_phandle_args *spec,
+void *data)
+{
+ struct exynos_bus *bus = data;
+
+ if (spec->np != bus->dev->of_node)
+ return ERR_PTR(-EINVAL);
+
+ return bus->node;
+}
+
static int exynos_bus_parent_parse_of(struct device_node *np,
struct exynos_bus *bus)
{
@@ -366,6 +416,101 @@ static int exynos_bus_profile_init_passive(struct
exynos_bus *bus,
return 0;
}
+static int exynos_bus_icc_connect(struct exynos_bus *bus)
+{
+ struct device_node *np = bus->dev->of_node;
+ struct devfreq *parent_devfreq;
+ struct icc_node *parent_node = NULL;
+ struct of_phandle_args args;
+ int ret = 0;
+
+ parent_devfreq = devfreq_get_devfreq_by_phandle(bus->dev, 0);
+ if (!IS_ERR(parent_devfreq)) {
+ struct exynos_bus *parent_bus;
+
+ parent_bus = dev_get_drvdata(parent_devfreq->dev.parent);
+ parent_node = parent_bus->node;
+ } else {
+ /* Look for parent in DT */
+ int num = of_count_phandle_with_args(np, "parent",
+"#interconnect-cells");
+ if (num != 1)
+ goto out; /* 'parent' is optional */
+
+ ret = of_parse_phandle_with_args(np, "parent",
+"#interconnect-cells",
+0, );
+ if (ret < 0)
+ goto out;
+
+ of_node_put(args.np);
+
+ parent_node = of_icc_get_from_provider();
[RFC PATCH v2 04/11] devfreq: exynos-bus: Clean up code
From: Artur Świgoń
This patch adds minor improvements to the exynos-bus driver.
Signed-off-by: Artur Świgoń
Reviewed-by: Krzysztof Kozlowski
---
drivers/devfreq/exynos-bus.c | 66 ++--
1 file changed, 25 insertions(+), 41 deletions(-)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 60ad4319fd80..8d44810cac69 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -15,11 +15,10 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
-#include
#define DEFAULT_SATURATION_RATIO 40
@@ -178,7 +177,7 @@ static int exynos_bus_parent_parse_of(struct device_node
*np,
struct device *dev = bus->dev;
struct opp_table *opp_table;
const char *vdd = "vdd";
- int i, ret, count, size;
+ int i, ret, count;
opp_table = dev_pm_opp_set_regulators(dev, , 1);
if (IS_ERR(opp_table)) {
@@ -201,8 +200,7 @@ static int exynos_bus_parent_parse_of(struct device_node
*np,
}
bus->edev_count = count;
- size = sizeof(*bus->edev) * count;
- bus->edev = devm_kzalloc(dev, size, GFP_KERNEL);
+ bus->edev = devm_kcalloc(dev, count, sizeof(*bus->edev), GFP_KERNEL);
if (!bus->edev) {
ret = -ENOMEM;
goto err_regulator;
@@ -301,10 +299,9 @@ static int exynos_bus_profile_init(struct exynos_bus *bus,
profile->exit = exynos_bus_exit;
ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
- if (!ondemand_data) {
- ret = -ENOMEM;
- goto err;
- }
+ if (!ondemand_data)
+ return -ENOMEM;
+
ondemand_data->upthreshold = 40;
ondemand_data->downdifferential = 5;
@@ -314,8 +311,7 @@ static int exynos_bus_profile_init(struct exynos_bus *bus,
ondemand_data);
if (IS_ERR(bus->devfreq)) {
dev_err(dev, "failed to add devfreq device\n");
- ret = PTR_ERR(bus->devfreq);
- goto err;
+ return PTR_ERR(bus->devfreq);
}
/*
@@ -325,16 +321,13 @@ static int exynos_bus_profile_init(struct exynos_bus *bus,
ret = exynos_bus_enable_edev(bus);
if (ret < 0) {
dev_err(dev, "failed to enable devfreq-event devices\n");
- goto err;
+ return ret;
}
ret = exynos_bus_set_event(bus);
- if (ret < 0) {
+ if (ret < 0)
dev_err(dev, "failed to set event to devfreq-event devices\n");
- goto err;
- }
-err:
return ret;
}
@@ -344,7 +337,6 @@ static int exynos_bus_profile_init_passive(struct
exynos_bus *bus,
struct device *dev = bus->dev;
struct devfreq_passive_data *passive_data;
struct devfreq *parent_devfreq;
- int ret = 0;
/* Initialize the struct profile and governor data for passive device */
profile->target = exynos_bus_target;
@@ -352,30 +344,26 @@ static int exynos_bus_profile_init_passive(struct
exynos_bus *bus,
/* Get the instance of parent devfreq device */
parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0);
- if (IS_ERR(parent_devfreq)) {
- ret = -EPROBE_DEFER;
- goto err;
- }
+ if (IS_ERR(parent_devfreq))
+ return -EPROBE_DEFER;
passive_data = devm_kzalloc(dev, sizeof(*passive_data), GFP_KERNEL);
- if (!passive_data) {
- ret = -ENOMEM;
- goto err;
- }
+ if (!passive_data)
+ return -ENOMEM;
+
passive_data->parent = parent_devfreq;
/* Add devfreq device for exynos bus with passive governor */
- bus->devfreq = devm_devfreq_add_device(dev, profile,
DEVFREQ_GOV_PASSIVE,
+ bus->devfreq = devm_devfreq_add_device(dev, profile,
+ DEVFREQ_GOV_PASSIVE,
passive_data);
if (IS_ERR(bus->devfreq)) {
dev_err(dev,
"failed to add devfreq dev with passive governor\n");
- ret = PTR_ERR(bus->devfreq);
- goto err;
+ return PTR_ERR(bus->devfreq);
}
-err:
- return ret;
+ return 0;
}
static int exynos_bus_probe(struct platform_device *pdev)
@@ -393,18 +381,18 @@ static int exynos_bus_probe(struct platform_device *pdev)
return -EINVAL;
}
- bus = devm_kzalloc(>dev, sizeof(*bus), GFP_KERNEL);
+ bus = devm_kzalloc(dev, sizeof(*bus), GFP_KERNEL);
if (!bus)
return -ENOMEM;
mutex_init(>lock);
- bus->dev = >dev;
+ bus->dev
[RFC PATCH v2 10/11] arm: dts: exynos: Add interconnects to Exynos4412 mixer
From: Artur Świgoń
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.
Signed-off-by: Artur Świgoń
---
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 a70a671acacd..faaec6c40412 100644
--- a/arch/arm/boot/dts/exynos4412.dtsi
+++ b/arch/arm/boot/dts/exynos4412.dtsi
@@ -789,6 +789,7 @@
clock-names = "mixer", "hdmi", "sclk_hdmi", "vp";
clocks = < CLK_MIXER>, < CLK_HDMI>,
< CLK_SCLK_HDMI>, < CLK_VP>;
+ interconnects = <_display _dmc>;
};
{
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v2 01/11] devfreq: exynos-bus: Extract exynos_bus_profile_init()
From: Artur Świgoń
This patch adds a new static function, exynos_bus_profile_init(), extracted
from exynos_bus_probe().
Signed-off-by: Artur Świgoń
---
drivers/devfreq/exynos-bus.c | 92 +---
1 file changed, 53 insertions(+), 39 deletions(-)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 29f422469960..78f38b7fb596 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -287,12 +287,62 @@ static int exynos_bus_parse_of(struct device_node *np,
return ret;
}
+static int exynos_bus_profile_init(struct exynos_bus *bus,
+ struct devfreq_dev_profile *profile)
+{
+ struct device *dev = bus->dev;
+ struct devfreq_simple_ondemand_data *ondemand_data;
+ int ret;
+
+ /* Initialize the struct profile and governor data for parent device */
+ profile->polling_ms = 50;
+ profile->target = exynos_bus_target;
+ profile->get_dev_status = exynos_bus_get_dev_status;
+ profile->exit = exynos_bus_exit;
+
+ ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
+ if (!ondemand_data) {
+ ret = -ENOMEM;
+ goto err;
+ }
+ ondemand_data->upthreshold = 40;
+ ondemand_data->downdifferential = 5;
+
+ /* Add devfreq device to monitor and handle the exynos bus */
+ bus->devfreq = devm_devfreq_add_device(dev, profile,
+ DEVFREQ_GOV_SIMPLE_ONDEMAND,
+ ondemand_data);
+ if (IS_ERR(bus->devfreq)) {
+ dev_err(dev, "failed to add devfreq device\n");
+ ret = PTR_ERR(bus->devfreq);
+ goto err;
+ }
+
+ /*
+* Enable devfreq-event to get raw data which is used to determine
+* current bus load.
+*/
+ ret = exynos_bus_enable_edev(bus);
+ if (ret < 0) {
+ dev_err(dev, "failed to enable devfreq-event devices\n");
+ goto err;
+ }
+
+ ret = exynos_bus_set_event(bus);
+ if (ret < 0) {
+ dev_err(dev, "failed to set event to devfreq-event devices\n");
+ goto err;
+ }
+
+err:
+ return ret;
+}
+
static int exynos_bus_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct device_node *np = dev->of_node, *node;
struct devfreq_dev_profile *profile;
- struct devfreq_simple_ondemand_data *ondemand_data;
struct devfreq_passive_data *passive_data;
struct devfreq *parent_devfreq;
struct exynos_bus *bus;
@@ -334,45 +384,9 @@ static int exynos_bus_probe(struct platform_device *pdev)
if (passive)
goto passive;
- /* Initialize the struct profile and governor data for parent device */
- profile->polling_ms = 50;
- profile->target = exynos_bus_target;
- profile->get_dev_status = exynos_bus_get_dev_status;
- profile->exit = exynos_bus_exit;
-
- ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
- if (!ondemand_data) {
- ret = -ENOMEM;
+ ret = exynos_bus_profile_init(bus, profile);
+ if (ret < 0)
goto err;
- }
- ondemand_data->upthreshold = 40;
- ondemand_data->downdifferential = 5;
-
- /* Add devfreq device to monitor and handle the exynos bus */
- bus->devfreq = devm_devfreq_add_device(dev, profile,
- DEVFREQ_GOV_SIMPLE_ONDEMAND,
- ondemand_data);
- if (IS_ERR(bus->devfreq)) {
- dev_err(dev, "failed to add devfreq device\n");
- ret = PTR_ERR(bus->devfreq);
- goto err;
- }
-
- /*
-* Enable devfreq-event to get raw data which is used to determine
-* current bus load.
-*/
- ret = exynos_bus_enable_edev(bus);
- if (ret < 0) {
- dev_err(dev, "failed to enable devfreq-event devices\n");
- goto err;
- }
-
- ret = exynos_bus_set_event(bus);
- if (ret < 0) {
- dev_err(dev, "failed to set event to devfreq-event devices\n");
- goto err;
- }
goto out;
passive:
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v2 06/11] interconnect: Relax requirement in of_icc_get_from_provider()
From: Artur Świgoń This patch relaxes the condition in of_icc_get_from_provider() so that it is no longer required to set #interconnect-cells = <1> in the DT. In case of the devfreq driver for exynos-bus, #interconnect-cells is always zero. Signed-off-by: Artur Świgoń Acked-by: Krzysztof Kozlowski --- drivers/interconnect/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index 95850700e367..f357c3a78858 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -284,7 +284,7 @@ struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec) struct icc_node *node = ERR_PTR(-EPROBE_DEFER); struct icc_provider *provider; - if (!spec || spec->args_count != 1) + if (!spec) return ERR_PTR(-EINVAL); mutex_lock(_lock); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v2 05/11] interconnect: Export of_icc_get_from_provider()
From: Artur Świgoń
This patch makes the above function public (for use in exynos-bus devfreq
driver).
Signed-off-by: Artur Świgoń
Reviewed-by: Krzysztof Kozlowski
---
drivers/interconnect/core.c | 3 ++-
include/linux/interconnect-provider.h | 6 ++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 7b971228df38..95850700e367 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -279,7 +279,7 @@ EXPORT_SYMBOL_GPL(of_icc_xlate_onecell);
* Returns a valid pointer to struct icc_node on success or ERR_PTR()
* on failure.
*/
-static struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec)
+struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec)
{
struct icc_node *node = ERR_PTR(-EPROBE_DEFER);
struct icc_provider *provider;
@@ -298,6 +298,7 @@ static struct icc_node *of_icc_get_from_provider(struct
of_phandle_args *spec)
return node;
}
+EXPORT_SYMBOL_GPL(of_icc_get_from_provider);
/**
* of_icc_get() - get a path handle from a DT node based on name
diff --git a/include/linux/interconnect-provider.h
b/include/linux/interconnect-provider.h
index b16f9effa555..070e411564e1 100644
--- a/include/linux/interconnect-provider.h
+++ b/include/linux/interconnect-provider.h
@@ -100,6 +100,7 @@ void icc_node_add(struct icc_node *node, struct
icc_provider *provider);
void icc_node_del(struct icc_node *node);
int icc_provider_add(struct icc_provider *provider);
int icc_provider_del(struct icc_provider *provider);
+struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec);
#else
@@ -140,6 +141,11 @@ static inline int icc_provider_del(struct icc_provider
*provider)
return -ENOTSUPP;
}
+struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec)
+{
+ return ERR_PTR(-ENOTSUPP);
+}
+
#endif /* CONFIG_INTERCONNECT */
#endif /* __LINUX_INTERCONNECT_PROVIDER_H */
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v2 08/11] arm: dts: exynos: Add parents and #interconnect-cells to Exynos4412
From: Artur Świgoń
This patch adds two fields to the Exynos4412 DTS:
- parent: to declare connections between nodes that are not in a
parent-child relation in devfreq;
- #interconnect-cells: required by the interconnect framework.
Please note that #interconnect-cells is always zero and node IDs are not
hardcoded anywhere. The above-mentioned parent-child relation in devfreq
means that there is a shared power line ('devfreq' property). The 'parent'
property only signifies an interconnect connection.
Signed-off-by: Artur Świgoń
---
arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 1 +
arch/arm/boot/dts/exynos4412.dtsi | 9 +
2 files changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index ea55f377d17c..bdd61ae86103 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -106,6 +106,7 @@
_leftbus {
devfreq-events = <_leftbus_3>, <_rightbus_3>;
vdd-supply = <_reg>;
+ parent = <_dmc>;
status = "okay";
};
diff --git a/arch/arm/boot/dts/exynos4412.dtsi
b/arch/arm/boot/dts/exynos4412.dtsi
index d20db2dfe8e2..a70a671acacd 100644
--- a/arch/arm/boot/dts/exynos4412.dtsi
+++ b/arch/arm/boot/dts/exynos4412.dtsi
@@ -390,6 +390,7 @@
clocks = < CLK_DIV_DMC>;
clock-names = "bus";
operating-points-v2 = <_dmc_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -398,6 +399,7 @@
clocks = < CLK_DIV_ACP>;
clock-names = "bus";
operating-points-v2 = <_acp_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -406,6 +408,7 @@
clocks = < CLK_DIV_C2C>;
clock-names = "bus";
operating-points-v2 = <_dmc_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -459,6 +462,7 @@
clocks = < CLK_DIV_GDL>;
clock-names = "bus";
operating-points-v2 = <_leftbus_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -467,6 +471,7 @@
clocks = < CLK_DIV_GDR>;
clock-names = "bus";
operating-points-v2 = <_leftbus_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -475,6 +480,7 @@
clocks = < CLK_ACLK160>;
clock-names = "bus";
operating-points-v2 = <_display_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -483,6 +489,7 @@
clocks = < CLK_ACLK133>;
clock-names = "bus";
operating-points-v2 = <_fsys_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -491,6 +498,7 @@
clocks = < CLK_ACLK100>;
clock-names = "bus";
operating-points-v2 = <_peri_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -499,6 +507,7 @@
clocks = < CLK_SCLK_MFC>;
clock-names = "bus";
operating-points-v2 = <_leftbus_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v2 00/11] Simple QoS for exynos-bus driver using interconnect
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 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(-) -- 2.17.1
[RFC PATCH v2 11/11] drm: exynos: mixer: Add interconnect support
From: Marek Szyprowski
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ń
Signed-off-by: Marek Szyprowski
Signed-off-by: Artur Świgoń
---
drivers/gpu/drm/exynos/exynos_mixer.c | 71 +--
1 file changed, 66 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 7b24338fad3c..a44f3284b071 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -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,40 @@ 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 = >base.state->adjusted_mode;
+ struct mixer_context *ctx = crtc->ctx;
+ unsigned long bw, bandwidth = 0;
+ int i, j, sub;
+
+ if (!ctx->soc_path)
+ return;
+
+ for (i = 0; i < MIXER_WIN_NR; i++) {
+ struct drm_plane *plane = >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 = bandwidth / 4 * 5;
+
+ dev_dbg(ctx->dev, "exynos-mixer: safe bandwidth %ld Bps\n", bandwidth);
+ icc_set_bw(ctx->soc_path, Bps_to_icc(bandwidth), 0);
+}
+
static void mixer_atomic_begin(struct exynos_drm_crtc *crtc)
{
struct mixer_context *ctx = crtc->ctx;
@@ -982,6 +1018,7 @@ static void mixer_atomic_flush(struct exynos_drm_crtc
*crtc)
if (!test_bit(MXR_BIT_POWERED, _ctx->flags))
return;
+ mixer_set_memory_bandwidth(crtc);
mixer_enable_sync(mixer_ctx);
exynos_crtc_handle_event(crtc);
}
@@ -1029,6 +1066,7 @@ static void mixer_disable(struct exynos_drm_crtc *crtc)
for (i = 0; i < MIXER_WIN_NR; i++)
mixer_disable_plane(crtc, >planes[i]);
+ mixer_set_memory_bandwidth(crtc);
exynos_drm_pipe_clk_enable(crtc, false);
pm_runtime_put(ctx->dev);
@@ -1220,12 +1258,22 @@ static int mixer_probe(struct platform_device *pdev)
struct device *dev = >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(>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 +1281,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, >flags);
@@ -1242,17 +1291,29 @@ static int mixer_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, ctx);
ret = component_add(>dev, _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(>dev);
+ struct device *dev = >dev;
+ struct mixer_context *ctx = dev_get_drvda
[RFC PATCH v2 07/11] interconnect: Relax condition in apply_constraints()
From: Artur Świgoń
The exynos-bus devfreq driver is extended with interconnect functionality
by a subsequent patch. This patch removes a check from the interconnect
framework that prevents interconnect from working on exynos-bus, in which
every bus is a separate interconnect provider.
Signed-off-by: Artur Świgoń
---
drivers/interconnect/core.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index f357c3a78858..e8243665d5ba 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -224,11 +224,8 @@ static int apply_constraints(struct icc_path *path)
for (i = 0; i < path->num_nodes; i++) {
next = path->reqs[i].node;
- /*
-* Both endpoints should be valid master-slave pairs of the
-* same interconnect provider that will be configured.
-*/
- if (!prev || next->provider != prev->provider) {
+ /* both endpoints should be valid master-slave pairs */
+ if (!prev) {
prev = next;
continue;
}
--
2.17.1
[RFC PATCH v2 03/11] devfreq: exynos-bus: Change goto-based logic to if-else logic
From: Artur Świgoń This patch improves code readability by changing the following construct: >if (cond) >goto passive; >foo(); >goto out; >passive: >bar(); >out: into this: >if (cond) >bar(); >else > foo(); Signed-off-by: Artur Świgoń --- drivers/devfreq/exynos-bus.c | 12 +++- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c index f85bed241631..60ad4319fd80 100644 --- a/drivers/devfreq/exynos-bus.c +++ b/drivers/devfreq/exynos-bus.c @@ -420,19 +420,13 @@ static int exynos_bus_probe(struct platform_device *pdev) goto err_reg; if (passive) - goto passive; + ret = exynos_bus_profile_init_passive(bus, profile); + else + ret = exynos_bus_profile_init(bus, profile); - ret = exynos_bus_profile_init(bus, profile); if (ret < 0) goto err; - goto out; -passive: - ret = exynos_bus_profile_init_passive(bus, profile); - if (ret < 0) - goto err; - -out: 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.17.1
Re: [RFC PATCH 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
Hi, On Wed, 2019-08-07 at 17:21 +0300, Georgi Djakov wrote: > Hi Artur, > > On 8/1/19 10:59, Artur Świgoń wrote: > > Hi Georgi, > > > > On Fri, 2019-07-26 at 11:05 +0300, Georgi Djakov wrote: > > > Hi Artur, > > > > > > On 7/23/19 15:20, Artur Świgoń wrote: > > > > This patch adds interconnect functionality to the exynos-bus devfreq > > > > driver. > > > > > > > > The SoC topology is a graph (or, more specifically, a tree) and most of > > > > its > > > > edges are taken from the devfreq parent-child hierarchy (cf. > > > > Documentation/devicetree/bindings/devfreq/exynos-bus.txt). The previous > > > > patch adds missing edges to the DT (under the name 'parent'). Due to > > > > unspecified relative probing order, -EPROBE_DEFER may be propagated to > > > > guarantee that a child is probed before its parent. > > > > > > > > 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 hardcoded but rather assigned at > > > > runtime, in probing order (subject to the above-mentioned exception > > > > regarding relative order). This approach allows for using this driver > > > > with > > > > various Exynos SoCs. > > > > > > I am not familiar with the Exynos bus topology, but it seems to me that > > > it's not > > > represented correctly. An interconnect provider with just a single node > > > (port) > > > is odd. I would expect that each provider consists of multiple master and > > > slave > > > nodes. This data would be used by a framework to understand what are the > > > links > > > and how the traffic flows between the IP blocks and through which buses. > > > > To summarize the exynos-bus topology[1] used by the devfreq driver: There > > are > > many data buses for data transfer in Samsung Exynos SoC. Every bus has its > > own > > clock. Buses often share power lines, in which case one of the buses on the > > power line is referred to as 'parent' (or as 'devfreq' in the DT). In the > > particular case of Exynos4412[1][2], the topology can be expressed as > > follows: > > > > bus_dmc > > -- bus_acp > > -- bus_c2c > > > > bus_leftbus > > -- bus_rightbus > > -- bus_display > > -- bus_fsys > > -- bus_peri > > -- bus_mfc > > > > Where bus_dmc and bus_leftbus probably could be referred to as masters, and > > the > > following indented nodes as slaves. Patch 08/11 of this RFC additionally > > adds > > the following to the DT: > > > > bus_dmc > > -- bus_leftbus > > > > Which makes the topology a valid tree. > > > > The exynos-bus concept in devfreq[3] is designed in such a way that every > > bus is > > probed separately as a platform device, and is a largely independent entity. > > > > This RFC proposes an extension to the existing devfreq driver that basically > > provides a simple QoS to ensure minimum clock frequency for selected buses > > (possibly overriding devfreq governor calculations) using the interconnect > > framework. > > > > The hierarchy is modelled in such a way that every bus is an interconnect > > node. > > On the other hand, what is considered an interconnect provider here is quite > > arbitrary, but for the reasons mentioned in the above paragraph, this RFC > > assumes that every bus is a provider of itself as a node. Using an > > alternative > > IIUC, in case we want to transfer data between the display and the memory > controller, the path would look like this: > > display --> bus_display --> bus_leftbus --> bus_dmc --> memory > > But the bus_display for example would have not one, but two nodes (ports), > right? One representing the link to the display controller and another one > representing the link to bus_leftbus? So i think that all the buses should > have at least two nodes, to represent each end of the wire. I do not think we really need that for our simple tree hierarchy. Of course, I can split every tree node into two nodes/ports (e.g., 'in' for children and 'out' for parent), but neither 'display' nor 'memory' from your diagram above are registered with the interconnect framework (only buses are). The devfreq devices used in the driver are virtual anyway. > > singleton provider approach was deemed more complicated since the 'dev' > > field in > > 'struct icc_provider' has to be set to something meaningful and we are tied > > to > > the 'samsung,exynos-bus' compatible string in the driver (and multiple > > instances > > of exynos-bus probed in indeterminate relative order). > > > > Sure, the rest makes sense to me. > > Thanks, > Georgi Regards, -- Artur Świgoń Samsung R Institute Poland Samsung Electronics
Re: [RFC PATCH 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
Hi,
Thank you for your comments.
On Tue, 2019-08-06 at 13:41 +, Leonard Crestez wrote:
> On 23.07.2019 15:21, Artur Świgoń wrote:
>
> > +static int exynos_bus_icc_aggregate(struct icc_node *node, u32 avg_bw,
> > + u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
> > +{
> > + *agg_peak = *agg_avg = peak_bw;
> > +
> > + return 0;
> > +}
>
> The only current provider aggregates "avg" with "sum" and "peak" with
> "max", any particular reason to do something different? This function
> doesn't even really do aggregation, if there is a second request for "0"
> then the first request is lost.
Yes, you're right. I adopted an oversimplified solution for the purpose of this
RFC (please bear in mind that currently only one icc_path is used, so there is
no aggregation anyway).
> I didn't find any docs but my interpretation of avg/peak is that "avg"
> is for constant traffic like a display or VPU pushing pixels and "peak"
> is for variable traffic like networking. I assume devices which make
> "peak" requests are aggregated with max because they're not expected to
> all max-out together.
That's correct (according to my understanding).
> In PATCH 11 you're making a bandwidth request based on resolution, that
> traffic is constant and guaranteed to happend while the display is on so
> it would make sense to request it as an "avg" and aggregate it with "sum".
>
> --
> Regards,
> Leonard
>
--
Artur Świgoń
Samsung R Institute Poland
Samsung Electronics
Re: [RFC PATCH 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
Hi, Thank you for your remarks. I will take them into account while preparing RFCv2. On Mon, 2019-07-29 at 10:52 +0900, Chanwoo Choi wrote: > Hi, > > On 19. 7. 23. 오후 9:20, Artur Świgoń wrote: > > This patch adds interconnect functionality to the exynos-bus devfreq > > driver. > > > > The SoC topology is a graph (or, more specifically, a tree) and most of its > > edges are taken from the devfreq parent-child hierarchy (cf. > > Documentation/devicetree/bindings/devfreq/exynos-bus.txt). The previous > > patch adds missing edges to the DT (under the name 'parent'). Due to > > unspecified relative probing order, -EPROBE_DEFER may be propagated to > > guarantee that a child is probed before its parent. > > > > 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 hardcoded but rather assigned at > > runtime, in probing order (subject to the above-mentioned exception > > regarding relative order). This approach allows for using this driver with > > various Exynos SoCs. > > > > The devfreq target() callback provided by exynos-bus now selects either the > > frequency calculated by the devfreq governor or the frequency requested via > > the interconnect API for the given node, whichever is higher. > > Basically, I agree to support the QoS requirement between devices. > But, I think that need to consider the multiple cases. > > > 1. When changing the devfreq governor by user, > For example of the connection between bus_dmc/leftbus/display on patch8, > there are possible multiple cases with various devfreq governor > which is changed on the runtime by user through sysfs interface. > > If users changes the devfreq governor as following: > Before, > - bus_dmc (simple_ondemand, available frequency 100/200/300/400 MHz) > --> bus_leftbus(simple_ondemand, available frequency 100/200/300/400 MHz) > > bus_display(passive) > > After changed governor of bus_dmc, > if the min_freq by interconnect requirement is 400Mhz, > - bus_dmc (powersave) : min_freq and max_freq and cur_freq is 100MHz > --> bus_leftbus(simple_ondemand) : cur_freq is 400Mhz > > bus_display(passive) > > The final frequency is 400MHz of bus_dmc > even if the min_freq/max_freq/cur_freq is 100MHz. > It cannot show the correct min_freq/max_freq through > devfreq sysfs interface. > > > 2. When disabling the some frequency by devfreq-thermal throttling, > This patch checks the min_freq of interconnect requirement > in the exynos_bus_target() and exynos_bus_passive_target(). > Also, it cannot show the correct min_freq/max_freq through > devfreq sysfs interface. > > For example of bus_dmc bus, > - The available frequencies are 100MHz, 200MHz, 300MHz, 400MHz > - Disable 400MHz by devfreq-thermal throttling > - min_freq is 100MHz > - max_freq is 300MHz > - min_freq of interconnect is 400MHz > > In result, the final frequency is 400MHz by exynos_bus_target() > There are no problem for working. But, the user cannot know > reason why cur_freq is 400MHz even if max_freq is 300MHz. > > Basically, update_devfreq() considers the all constraints > of min_freq/max_freq to decide the proper target frequency. > > > 3. > I think that the exynos_bus_passive_target() is used for devfreq device > using 'passive' governor. The frequency already depends on the parent device. > > If already the parent devfreq device like bus_leftbus consider > the minimum frequency of QoS requirement like interconnect, > it is not necessary. The next frequency of devfreq device > with 'passive' governor, it will apply the QoS requirement > without any additional code. > > > > > Please note that it is not an error when CONFIG_INTERCONNECT is 'n', in > > which case all interconnect API functions are no-op. > > > > Signed-off-by: Artur Świgoń > > --- > > drivers/devfreq/exynos-bus.c | 145 +++ > > 1 file changed, 145 insertions(+) > > > > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > > index 412511ca7703..12fb7c84ae50 100644 > > --- a/drivers/devfreq/exynos-bus.c > > +++ b/drivers/devfreq/exynos-bus.c > > @@ -14,6 +14,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -23,6 +24,8 @@ > > #define DEFAULT_SATURATION_RATIO 40 > > #define DEFAULT_VOLTAGE_TOLERANCE 2 > > > > +#define icc_units_to_hz(x) ((x) * 1000UL / 8) > > + > &
Re: [RFC PATCH 01/11] devfreq: exynos-bus: Extract exynos_bus_profile_init()
Hi,
On Wed, 2019-07-24 at 21:07 +0200, Krzysztof Kozlowski wrote:
> On Tue, Jul 23, 2019 at 02:20:06PM +0200, Artur Świgoń wrote:
> > This patch adds a new static function, exynos_bus_profile_init(), extracted
> > from exynos_bus_probe().
> >
> > Signed-off-by: Artur Świgoń
> > ---
> > drivers/devfreq/exynos-bus.c | 106 ---
> > 1 file changed, 60 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > index d9f377912c10..d8f1efaf2d49 100644
> > --- a/drivers/devfreq/exynos-bus.c
> > +++ b/drivers/devfreq/exynos-bus.c
> > @@ -372,12 +372,69 @@ static int exynos_bus_parse_of(struct device_node *np,
> > return ret;
> > }
> >
> > +static int exynos_bus_profile_init(struct exynos_bus *bus,
> > + struct devfreq_dev_profile *profile)
> > +{
> > + struct device *dev = bus->dev;
> > + struct devfreq_simple_ondemand_data *ondemand_data;
> > + int ret;
> > +
> > + /* Initialize the struct profile and governor data for parent device */
> > + profile->polling_ms = 50;
> > + profile->target = exynos_bus_target;
> > + profile->get_dev_status = exynos_bus_get_dev_status;
> > + profile->exit = exynos_bus_exit;
> > +
> > + ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
> > + if (!ondemand_data) {
> > + ret = -ENOMEM;
> > + goto err;
>
> Just return proper error code. Less lines, obvious code since you do not
> have any cleanup in error path.
I was advised to avoid modifying code being moved (in one patch). I do make
changes in these places in patch 04/11, i.e. change the original label 'err' to
'out'. What's your opinion on making the proposed changes to patches 01 and 02
(s/goto err/return ret/) in patch 04 instead?
> > +
> > + /* Register opp_notifier to catch the change of OPP */
> > + ret = devm_devfreq_register_opp_notifier(dev, bus->devfreq);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to register opp notifier\n");
> > + goto err;
>
> The same - return err.
>
> Best regards,
> Krzysztof
Best regards,
--
Artur Świgoń
Samsung R Institute Poland
Samsung Electronics
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
Hi Georgi, On Fri, 2019-07-26 at 11:05 +0300, Georgi Djakov wrote: > Hi Artur, > > On 7/23/19 15:20, Artur Świgoń wrote: > > This patch adds interconnect functionality to the exynos-bus devfreq > > driver. > > > > The SoC topology is a graph (or, more specifically, a tree) and most of its > > edges are taken from the devfreq parent-child hierarchy (cf. > > Documentation/devicetree/bindings/devfreq/exynos-bus.txt). The previous > > patch adds missing edges to the DT (under the name 'parent'). Due to > > unspecified relative probing order, -EPROBE_DEFER may be propagated to > > guarantee that a child is probed before its parent. > > > > 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 hardcoded but rather assigned at > > runtime, in probing order (subject to the above-mentioned exception > > regarding relative order). This approach allows for using this driver with > > various Exynos SoCs. > > I am not familiar with the Exynos bus topology, but it seems to me that it's > not > represented correctly. An interconnect provider with just a single node (port) > is odd. I would expect that each provider consists of multiple master and > slave > nodes. This data would be used by a framework to understand what are the links > and how the traffic flows between the IP blocks and through which buses. To summarize the exynos-bus topology[1] used by the devfreq driver: There are many data buses for data transfer in Samsung Exynos SoC. Every bus has its own clock. Buses often share power lines, in which case one of the buses on the power line is referred to as 'parent' (or as 'devfreq' in the DT). In the particular case of Exynos4412[1][2], the topology can be expressed as follows: bus_dmc -- bus_acp -- bus_c2c bus_leftbus -- bus_rightbus -- bus_display -- bus_fsys -- bus_peri -- bus_mfc Where bus_dmc and bus_leftbus probably could be referred to as masters, and the following indented nodes as slaves. Patch 08/11 of this RFC additionally adds the following to the DT: bus_dmc -- bus_leftbus Which makes the topology a valid tree. The exynos-bus concept in devfreq[3] is designed in such a way that every bus is probed separately as a platform device, and is a largely independent entity. This RFC proposes an extension to the existing devfreq driver that basically provides a simple QoS to ensure minimum clock frequency for selected buses (possibly overriding devfreq governor calculations) using the interconnect framework. The hierarchy is modelled in such a way that every bus is an interconnect node. On the other hand, what is considered an interconnect provider here is quite arbitrary, but for the reasons mentioned in the above paragraph, this RFC assumes that every bus is a provider of itself as a node. Using an alternative singleton provider approach was deemed more complicated since the 'dev' field in 'struct icc_provider' has to be set to something meaningful and we are tied to the 'samsung,exynos-bus' compatible string in the driver (and multiple instances of exynos-bus probed in indeterminate relative order). I'm looking forward to hearing any additional thoughts you may have on this topic. Best regards, -- Artur Świgoń Samsung R Institute Poland Samsung Electronics [1] Documentation/devicetree/bindings/devfreq/exynos-bus.txt [2] arch/arm/boot/dts/exynos4412-odroid-common.dtsi [3] drivers/devfreq/exynos-bus.c (subject of this patch)
Re: [RFC PATCH 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
On Wed, 2019-07-24 at 20:36 +0200, Krzysztof Kozlowski wrote:
> On Tue, Jul 23, 2019 at 02:20:14PM +0200, Artur Świgoń wrote:
> > This patch adds interconnect functionality to the exynos-bus devfreq
> > driver.
> >
> > The SoC topology is a graph (or, more specifically, a tree) and most of its
> > edges are taken from the devfreq parent-child hierarchy (cf.
> > Documentation/devicetree/bindings/devfreq/exynos-bus.txt). The previous
> > patch adds missing edges to the DT (under the name 'parent'). Due to
>
> Do not refer to DT patches. They will come through different tree so
> "previous" will not be correct anymore. You mentioned dependencies in
> cover letter so it is sufficient.
OK.
> > /*
> > @@ -61,6 +69,13 @@ exynos_bus_ops_edev(enable_edev);
> > exynos_bus_ops_edev(disable_edev);
> > exynos_bus_ops_edev(set_event);
> >
> > +static int exynos_bus_next_id(void)
> > +{
> > + static int exynos_bus_node_id;
> > +
> > + return exynos_bus_node_id++;
>
> This does not look robust. Use IDR for IDs.
OK.
> > +static int exynos_bus_icc_connect(struct exynos_bus *bus)
> > +{
> > + struct device_node *np = bus->dev->of_node;
> > + struct devfreq *parent_devfreq;
> > + struct icc_node *parent_node = NULL;
> > + struct of_phandle_args args;
> > + int ret = 0;
> > +
> > + parent_devfreq = devfreq_get_devfreq_by_phandle(bus->dev, 0);
> > + if (!IS_ERR(parent_devfreq)) {
> > + struct exynos_bus *parent_bus;
>
> What if someone unbinds this parent devfreq? I guess everything in
> devfreq starts exploding... however it's not the problem of this patch.
>
> Do you also need suspend/resume order (device links)? I guess the other
> side, e.g. mixer, should resume before the bus?
Actually, I think that the bus (devfreq) should resume before mixer. However,
suspend/resume order is another issue that applies to this driver regardless of
using the interconnect framework, although device links could probably also be
implemented in the interconnect framework itself.
> > + parent_bus = dev_get_drvdata(parent_devfreq->dev.parent);
> > + parent_node = parent_bus->node;
> > + } else {
> > + /* Look for parent in DT */
> > + int num = of_count_phandle_with_args(np, "parent",
> > +"#interconnect-cells");
> > + if (num != 1)
>
> You will return here 0 but isn't it an error?
It is definitely not an error when 'parent' does not exist in DT (for buses that
are parents themselves). I can extend the comment in the code to explicitly
state that.
Best regards,
--
Artur Świgoń
Samsung R Institute Poland
Samsung Electronics
Re: [RFC PATCH 08/11] arm: dts: exynos: Add parents and #interconnect-cells to Exynos4412
On Wed, 2019-07-24 at 21:24 +0200, Krzysztof Kozlowski wrote:
> On Tue, Jul 23, 2019 at 02:20:13PM +0200, Artur Świgoń wrote:
> > This patch adds two fields tp the Exynos4412 DTS:
>
> tp->to
Fixed. Thanks for catching the typo :)
> > - parent: to declare connections between nodes that are not in a
> > parent-child relation in devfreq;
>
> Is it a standard property?
> The explanation needs some improvement... why are you adding parent to a
> devices which are not child-parent?
This is not a standard property. I actually wanted to call it 'neighbor' at
first. If you take a look at [1] and search for 'Exynos4x12', you will see that
there are two power lines, and each has exactly one parent block (the rest are
modelled in devfreq as its children). In [2], the parent of each child is
indicated using the 'devfreq' property, e.g.,
_display {
devfreq = <_leftbus>;
status = "okay";
};
The single piece missing to make this topology a connected graph (for
interconnect QoS purposes) is the 'parent' property I proposed in this patch.
bus_leftbus is dependent on bus_dmc, therefore using the term 'parent' is
justified IMHO.
The explanation could be improved by adding what Chanwoo Choi <
cw00.c...@samsung.com> expressed in a parallel reply to this patch:
> - If 'devfreq' property is used between buses,
> it indicates that there are requirement of sharing of power line.
> - If 'parent' property is used between buses,
> it indicates that there are requirement of interconnect connection.
[1] Documentation/devicetree/bindings/devfreq/exynos-bus.txt
[2] arch/arm/boot/dts/exynos4412-odroid-common.dtsi (subject of this patch)
Best regards,
--
Artur Świgoń
Samsung R Institute Poland
Samsung Electronics
[RFC PATCH 00/11] Simple QoS for exynos-bus driver using interconnect
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 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(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 11/11] drm: exynos: mixer: Add interconnect support
From: Marek Szyprowski
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ń
Signed-off-by: Artur Świgoń
Signed-off-by: Marek Szyprowski
---
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
#include
#include
+#include
#include
#include
#include
@@ -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 = >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 = >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, _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, >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 = >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(>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, >flags);
@@ -1242,17 +1288,29 @@ static int mixer_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, ctx);
ret = component_add(>dev, _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(>dev);
+ struct device *dev = >dev;
+ struct mixer_context *ctx = dev_get_drvdata(dev);
- component_del(>dev, _component_ops);
+ pm_runtime_disable(dev);
+
+ component_del(dev, _component_ops);
+
+ icc_put(ctx->soc_path);
return 0;
}
--
2.17.1
[RFC PATCH 01/11] devfreq: exynos-bus: Extract exynos_bus_profile_init()
This patch adds a new static function, exynos_bus_profile_init(), extracted
from exynos_bus_probe().
Signed-off-by: Artur Świgoń
---
drivers/devfreq/exynos-bus.c | 106 ---
1 file changed, 60 insertions(+), 46 deletions(-)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index d9f377912c10..d8f1efaf2d49 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -372,12 +372,69 @@ static int exynos_bus_parse_of(struct device_node *np,
return ret;
}
+static int exynos_bus_profile_init(struct exynos_bus *bus,
+ struct devfreq_dev_profile *profile)
+{
+ struct device *dev = bus->dev;
+ struct devfreq_simple_ondemand_data *ondemand_data;
+ int ret;
+
+ /* Initialize the struct profile and governor data for parent device */
+ profile->polling_ms = 50;
+ profile->target = exynos_bus_target;
+ profile->get_dev_status = exynos_bus_get_dev_status;
+ profile->exit = exynos_bus_exit;
+
+ ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
+ if (!ondemand_data) {
+ ret = -ENOMEM;
+ goto err;
+ }
+ ondemand_data->upthreshold = 40;
+ ondemand_data->downdifferential = 5;
+
+ /* Add devfreq device to monitor and handle the exynos bus */
+ bus->devfreq = devm_devfreq_add_device(dev, profile,
+ DEVFREQ_GOV_SIMPLE_ONDEMAND,
+ ondemand_data);
+ if (IS_ERR(bus->devfreq)) {
+ dev_err(dev, "failed to add devfreq device\n");
+ ret = PTR_ERR(bus->devfreq);
+ goto err;
+ }
+
+ /* Register opp_notifier to catch the change of OPP */
+ ret = devm_devfreq_register_opp_notifier(dev, bus->devfreq);
+ if (ret < 0) {
+ dev_err(dev, "failed to register opp notifier\n");
+ goto err;
+ }
+
+ /*
+* Enable devfreq-event to get raw data which is used to determine
+* current bus load.
+*/
+ ret = exynos_bus_enable_edev(bus);
+ if (ret < 0) {
+ dev_err(dev, "failed to enable devfreq-event devices\n");
+ goto err;
+ }
+
+ ret = exynos_bus_set_event(bus);
+ if (ret < 0) {
+ dev_err(dev, "failed to set event to devfreq-event devices\n");
+ goto err;
+ }
+
+err:
+ return ret;
+}
+
static int exynos_bus_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct device_node *np = dev->of_node, *node;
struct devfreq_dev_profile *profile;
- struct devfreq_simple_ondemand_data *ondemand_data;
struct devfreq_passive_data *passive_data;
struct devfreq *parent_devfreq;
struct exynos_bus *bus;
@@ -418,52 +475,9 @@ static int exynos_bus_probe(struct platform_device *pdev)
if (ret < 0)
goto err;
- /* Initialize the struct profile and governor data for parent device */
- profile->polling_ms = 50;
- profile->target = exynos_bus_target;
- profile->get_dev_status = exynos_bus_get_dev_status;
- profile->exit = exynos_bus_exit;
-
- ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
- if (!ondemand_data) {
- ret = -ENOMEM;
+ ret = exynos_bus_profile_init(bus, profile);
+ if (ret < 0)
goto err;
- }
- ondemand_data->upthreshold = 40;
- ondemand_data->downdifferential = 5;
-
- /* Add devfreq device to monitor and handle the exynos bus */
- bus->devfreq = devm_devfreq_add_device(dev, profile,
- DEVFREQ_GOV_SIMPLE_ONDEMAND,
- ondemand_data);
- if (IS_ERR(bus->devfreq)) {
- dev_err(dev, "failed to add devfreq device\n");
- ret = PTR_ERR(bus->devfreq);
- goto err;
- }
-
- /* Register opp_notifier to catch the change of OPP */
- ret = devm_devfreq_register_opp_notifier(dev, bus->devfreq);
- if (ret < 0) {
- dev_err(dev, "failed to register opp notifier\n");
- goto err;
- }
-
- /*
-* Enable devfreq-event to get raw data which is used to determine
-* current bus load.
-*/
- ret = exynos_bus_enable_edev(bus);
- if (ret < 0) {
- dev_err(dev, "failed to enable devfreq-event devices\n");
- goto err;
- }
-
- ret = exynos_bus_set_event(bus);
- if (ret < 0) {
- dev_err(dev, "failed to set event to devfreq-event devices\n");
- goto err;
- }
goto out;
passive:
--
2.17.1
[RFC PATCH 02/11] devfreq: exynos-bus: Extract exynos_bus_profile_init_passive()
This patch adds a new static function, exynos_bus_profile_init_passive(),
extracted from exynos_bus_probe().
Signed-off-by: Artur Świgoń
---
drivers/devfreq/exynos-bus.c | 70 +---
1 file changed, 42 insertions(+), 28 deletions(-)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index d8f1efaf2d49..cf6f6cbd0f55 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -430,13 +430,51 @@ static int exynos_bus_profile_init(struct exynos_bus *bus,
return ret;
}
+static int exynos_bus_profile_init_passive(struct exynos_bus *bus,
+ struct devfreq_dev_profile *profile)
+{
+ struct device *dev = bus->dev;
+ struct devfreq *parent_devfreq;
+ struct devfreq_passive_data *passive_data;
+ int ret = 0;
+
+ /* Initialize the struct profile and governor data for passive device */
+ profile->target = exynos_bus_passive_target;
+ profile->exit = exynos_bus_passive_exit;
+
+ /* Get the instance of parent devfreq device */
+ parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0);
+ if (IS_ERR(parent_devfreq)) {
+ ret = -EPROBE_DEFER;
+ goto err;
+ }
+
+ passive_data = devm_kzalloc(dev, sizeof(*passive_data), GFP_KERNEL);
+ if (!passive_data) {
+ ret = -ENOMEM;
+ goto err;
+ }
+ passive_data->parent = parent_devfreq;
+
+ /* Add devfreq device for exynos bus with passive governor */
+ bus->devfreq = devm_devfreq_add_device(dev, profile,
DEVFREQ_GOV_PASSIVE,
+ passive_data);
+ if (IS_ERR(bus->devfreq)) {
+ dev_err(dev,
+ "failed to add devfreq dev with passive governor\n");
+ ret = PTR_ERR(bus->devfreq);
+ goto err;
+ }
+
+err:
+ return ret;
+}
+
static int exynos_bus_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct device_node *np = dev->of_node, *node;
struct devfreq_dev_profile *profile;
- struct devfreq_passive_data *passive_data;
- struct devfreq *parent_devfreq;
struct exynos_bus *bus;
int ret, max_state;
unsigned long min_freq, max_freq;
@@ -481,33 +519,9 @@ static int exynos_bus_probe(struct platform_device *pdev)
goto out;
passive:
- /* Initialize the struct profile and governor data for passive device */
- profile->target = exynos_bus_passive_target;
- profile->exit = exynos_bus_passive_exit;
-
- /* Get the instance of parent devfreq device */
- parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0);
- if (IS_ERR(parent_devfreq)) {
- ret = -EPROBE_DEFER;
+ ret = exynos_bus_profile_init_passive(bus, profile);
+ if (ret < 0)
goto err;
- }
-
- passive_data = devm_kzalloc(dev, sizeof(*passive_data), GFP_KERNEL);
- if (!passive_data) {
- ret = -ENOMEM;
- goto err;
- }
- passive_data->parent = parent_devfreq;
-
- /* Add devfreq device for exynos bus with passive governor */
- bus->devfreq = devm_devfreq_add_device(dev, profile,
DEVFREQ_GOV_PASSIVE,
- passive_data);
- if (IS_ERR(bus->devfreq)) {
- dev_err(dev,
- "failed to add devfreq dev with passive governor\n");
- ret = PTR_ERR(bus->devfreq);
- goto err;
- }
out:
max_state = bus->devfreq->profile->max_state;
--
2.17.1
[RFC PATCH 06/11] icc: Relax requirement in of_icc_get_from_provider()
This patch relaxes the condition in of_icc_get_from_provider() so that it is no longer required to set #interconnect-cells = <1> in the DT. In case of the devfreq driver for exynos-bus, #interconnect-cells is always zero. Signed-off-by: Artur Świgoń --- drivers/interconnect/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index d566ae0b66c0..2556fd6d1863 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -279,7 +279,7 @@ struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec) struct icc_node *node = ERR_PTR(-EPROBE_DEFER); struct icc_provider *provider; - if (!spec || spec->args_count != 1) + if (!spec) return ERR_PTR(-EINVAL); mutex_lock(_lock); -- 2.17.1
[RFC PATCH 08/11] arm: dts: exynos: Add parents and #interconnect-cells to Exynos4412
This patch adds two fields tp the Exynos4412 DTS:
- parent: to declare connections between nodes that are not in a
parent-child relation in devfreq;
- #interconnect-cells: required by the interconnect framework.
Please note that #interconnect-cells is always zero and node IDs are not
hardcoded anywhere.
Signed-off-by: Artur Świgoń
---
arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 1 +
arch/arm/boot/dts/exynos4412.dtsi | 9 +
2 files changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index ea55f377d17c..bdd61ae86103 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -106,6 +106,7 @@
_leftbus {
devfreq-events = <_leftbus_3>, <_rightbus_3>;
vdd-supply = <_reg>;
+ parent = <_dmc>;
status = "okay";
};
diff --git a/arch/arm/boot/dts/exynos4412.dtsi
b/arch/arm/boot/dts/exynos4412.dtsi
index d20db2dfe8e2..a70a671acacd 100644
--- a/arch/arm/boot/dts/exynos4412.dtsi
+++ b/arch/arm/boot/dts/exynos4412.dtsi
@@ -390,6 +390,7 @@
clocks = < CLK_DIV_DMC>;
clock-names = "bus";
operating-points-v2 = <_dmc_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -398,6 +399,7 @@
clocks = < CLK_DIV_ACP>;
clock-names = "bus";
operating-points-v2 = <_acp_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -406,6 +408,7 @@
clocks = < CLK_DIV_C2C>;
clock-names = "bus";
operating-points-v2 = <_dmc_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -459,6 +462,7 @@
clocks = < CLK_DIV_GDL>;
clock-names = "bus";
operating-points-v2 = <_leftbus_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -467,6 +471,7 @@
clocks = < CLK_DIV_GDR>;
clock-names = "bus";
operating-points-v2 = <_leftbus_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -475,6 +480,7 @@
clocks = < CLK_ACLK160>;
clock-names = "bus";
operating-points-v2 = <_display_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -483,6 +489,7 @@
clocks = < CLK_ACLK133>;
clock-names = "bus";
operating-points-v2 = <_fsys_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -491,6 +498,7 @@
clocks = < CLK_ACLK100>;
clock-names = "bus";
operating-points-v2 = <_peri_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
@@ -499,6 +507,7 @@
clocks = < CLK_SCLK_MFC>;
clock-names = "bus";
operating-points-v2 = <_leftbus_opp_table>;
+ #interconnect-cells = <0>;
status = "disabled";
};
--
2.17.1
[RFC PATCH 10/11] arm: dts: exynos: Add interconnects to Exynos4412 mixer
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.
Signed-off-by: Artur Świgoń
---
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 a70a671acacd..faaec6c40412 100644
--- a/arch/arm/boot/dts/exynos4412.dtsi
+++ b/arch/arm/boot/dts/exynos4412.dtsi
@@ -789,6 +789,7 @@
clock-names = "mixer", "hdmi", "sclk_hdmi", "vp";
clocks = < CLK_MIXER>, < CLK_HDMI>,
< CLK_SCLK_HDMI>, < CLK_VP>;
+ interconnects = <_display _dmc>;
};
{
--
2.17.1
[RFC PATCH 03/11] devfreq: exynos-bus: Change goto-based logic to if-else logic
This patch improves code readability by changing the following construct:
>if (cond)
>goto passive;
>foo();
>goto out;
>passive:
>bar();
>out:
into this:
>if (cond)
>bar();
>else
> foo();
Signed-off-by: Artur Świgoń
---
drivers/devfreq/exynos-bus.c | 24 +---
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index cf6f6cbd0f55..4bb83b945bf7 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -505,25 +505,19 @@ static int exynos_bus_probe(struct platform_device *pdev)
node = of_parse_phandle(dev->of_node, "devfreq", 0);
if (node) {
of_node_put(node);
- goto passive;
+ ret = exynos_bus_profile_init_passive(bus, profile);
+ if (ret < 0)
+ goto err;
} else {
ret = exynos_bus_parent_parse_of(np, bus);
+ if (ret < 0)
+ goto err;
+
+ ret = exynos_bus_profile_init(bus, profile);
+ if (ret < 0)
+ goto err;
}
- if (ret < 0)
- goto err;
-
- ret = exynos_bus_profile_init(bus, profile);
- if (ret < 0)
- goto err;
-
- goto out;
-passive:
- ret = exynos_bus_profile_init_passive(bus, profile);
- if (ret < 0)
- goto err;
-
-out:
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.17.1
[RFC PATCH 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
This patch adds interconnect functionality to the exynos-bus devfreq
driver.
The SoC topology is a graph (or, more specifically, a tree) and most of its
edges are taken from the devfreq parent-child hierarchy (cf.
Documentation/devicetree/bindings/devfreq/exynos-bus.txt). The previous
patch adds missing edges to the DT (under the name 'parent'). Due to
unspecified relative probing order, -EPROBE_DEFER may be propagated to
guarantee that a child is probed before its parent.
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 hardcoded but rather assigned at
runtime, in probing order (subject to the above-mentioned exception
regarding relative order). This approach allows for using this driver with
various Exynos SoCs.
The devfreq target() callback provided by exynos-bus now selects either the
frequency calculated by the devfreq governor or the frequency requested via
the interconnect API for the given node, whichever is higher.
Please note that it is not an error when CONFIG_INTERCONNECT is 'n', in
which case all interconnect API functions are no-op.
Signed-off-by: Artur Świgoń
---
drivers/devfreq/exynos-bus.c | 145 +++
1 file changed, 145 insertions(+)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 412511ca7703..12fb7c84ae50 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -23,6 +24,8 @@
#define DEFAULT_SATURATION_RATIO 40
#define DEFAULT_VOLTAGE_TOLERANCE 2
+#define icc_units_to_hz(x) ((x) * 1000UL / 8)
+
struct exynos_bus {
struct device *dev;
@@ -31,12 +34,17 @@ struct exynos_bus {
unsigned int edev_count;
struct mutex lock;
+ unsigned long min_freq;
unsigned long curr_freq;
struct regulator *regulator;
struct clk *clk;
unsigned int voltage_tolerance;
unsigned int ratio;
+
+ /* One provider per bus, one node per provider */
+ struct icc_provider provider;
+ struct icc_node *node;
};
/*
@@ -61,6 +69,13 @@ exynos_bus_ops_edev(enable_edev);
exynos_bus_ops_edev(disable_edev);
exynos_bus_ops_edev(set_event);
+static int exynos_bus_next_id(void)
+{
+ static int exynos_bus_node_id;
+
+ return exynos_bus_node_id++;
+}
+
static int exynos_bus_get_event(struct exynos_bus *bus,
struct devfreq_event_data *edata)
{
@@ -98,6 +113,8 @@ static int exynos_bus_target(struct device *dev, unsigned
long *freq, u32 flags)
unsigned long old_freq, new_freq, new_volt, tol;
int ret = 0;
+ *freq = max(*freq, bus->min_freq);
+
/* Get new opp-bus instance according to new bus clock */
new_opp = devfreq_recommended_opp(dev, freq, flags);
if (IS_ERR(new_opp)) {
@@ -208,6 +225,8 @@ static int exynos_bus_passive_target(struct device *dev,
unsigned long *freq,
unsigned long old_freq, new_freq;
int ret = 0;
+ *freq = max(*freq, bus->min_freq);
+
/* Get new opp-bus instance according to new bus clock */
new_opp = devfreq_recommended_opp(dev, freq, flags);
if (IS_ERR(new_opp)) {
@@ -251,6 +270,35 @@ static void exynos_bus_passive_exit(struct device *dev)
clk_disable_unprepare(bus->clk);
}
+static int exynos_bus_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+ struct exynos_bus *src_bus = src->data, *dst_bus = dst->data;
+
+ src_bus->min_freq = icc_units_to_hz(src->peak_bw);
+ dst_bus->min_freq = icc_units_to_hz(dst->peak_bw);
+
+ return 0;
+}
+
+static int exynos_bus_icc_aggregate(struct icc_node *node, u32 avg_bw,
+ u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
+{
+ *agg_peak = *agg_avg = peak_bw;
+
+ return 0;
+}
+
+static struct icc_node *exynos_bus_icc_xlate(struct of_phandle_args *spec,
+void *data)
+{
+ struct exynos_bus *bus = data;
+
+ if (spec->np != bus->dev->of_node)
+ return ERR_PTR(-EINVAL);
+
+ return bus->node;
+}
+
static int exynos_bus_parent_parse_of(struct device_node *np,
struct exynos_bus *bus)
{
@@ -469,6 +517,95 @@ static int exynos_bus_profile_init_passive(struct
exynos_bus *bus,
return ret;
}
+static int exynos_bus_icc_connect(struct exynos_bus *bus)
+{
+ struct device_node *np = bus->dev->of_node;
+ struct devfreq *parent_devfreq;
+ struct icc_node *parent_node = NULL;
+ struct of_phandle_args args;
+ int ret = 0;
+
+ parent_devfreq = devfreq_get_devfreq_by_phandle(bus->dev, 0);
+ if (!IS_ERR(parent_devfreq))
[RFC PATCH 05/11] icc: Export of_icc_get_from_provider()
This patch makes the above function public (for use in exynos-bus devfreq
driver).
Signed-off-by: Artur Świgoń
---
drivers/interconnect/core.c | 3 ++-
include/linux/interconnect-provider.h | 6 ++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 871eb4bc4efc..d566ae0b66c0 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -274,7 +274,7 @@ EXPORT_SYMBOL_GPL(of_icc_xlate_onecell);
* Returns a valid pointer to struct icc_node on success or ERR_PTR()
* on failure.
*/
-static struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec)
+struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec)
{
struct icc_node *node = ERR_PTR(-EPROBE_DEFER);
struct icc_provider *provider;
@@ -293,6 +293,7 @@ static struct icc_node *of_icc_get_from_provider(struct
of_phandle_args *spec)
return node;
}
+EXPORT_SYMBOL_GPL(of_icc_get_from_provider);
/**
* of_icc_get() - get a path handle from a DT node based on name
diff --git a/include/linux/interconnect-provider.h
b/include/linux/interconnect-provider.h
index 63caccadc2db..9ecfc518b952 100644
--- a/include/linux/interconnect-provider.h
+++ b/include/linux/interconnect-provider.h
@@ -97,6 +97,7 @@ void icc_node_add(struct icc_node *node, struct icc_provider
*provider);
void icc_node_del(struct icc_node *node);
int icc_provider_add(struct icc_provider *provider);
int icc_provider_del(struct icc_provider *provider);
+struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec);
#else
@@ -137,6 +138,11 @@ static inline int icc_provider_del(struct icc_provider
*provider)
return -ENOTSUPP;
}
+struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec)
+{
+ return ERR_PTR(-ENOTSUPP);
+}
+
#endif /* CONFIG_INTERCONNECT */
#endif /* __LINUX_INTERCONNECT_PROVIDER_H */
--
2.17.1
[RFC PATCH 07/11] icc: Relax condition in apply_constraints()
The exynos-bus devfreq driver is extended with interconnect functionality
by a subsequent patch. This patch removes a check from the interconnect
framework that prevents interconnect from working on exynos-bus, in which
every bus is a separate interconnect provider.
Signed-off-by: Artur Świgoń
---
drivers/interconnect/core.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 2556fd6d1863..bb55565d190c 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -219,11 +219,8 @@ static int apply_constraints(struct icc_path *path)
for (i = 0; i < path->num_nodes; i++) {
next = path->reqs[i].node;
- /*
-* Both endpoints should be valid master-slave pairs of the
-* same interconnect provider that will be configured.
-*/
- if (!prev || next->provider != prev->provider) {
+ /* both endpoints should be valid master-slave pairs */
+ if (!prev) {
prev = next;
continue;
}
--
2.17.1
[RFC PATCH 04/11] devfreq: exynos-bus: Clean up code
This patch adds minor improvements to the exynos-bus driver.
Signed-off-by: Artur Świgoń
---
drivers/devfreq/exynos-bus.c | 49
1 file changed, 22 insertions(+), 27 deletions(-)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 4bb83b945bf7..412511ca7703 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -15,11 +15,10 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
-#include
#define DEFAULT_SATURATION_RATIO 40
#define DEFAULT_VOLTAGE_TOLERANCE 2
@@ -256,7 +255,7 @@ static int exynos_bus_parent_parse_of(struct device_node
*np,
struct exynos_bus *bus)
{
struct device *dev = bus->dev;
- int i, ret, count, size;
+ int i, ret, count;
/* Get the regulator to provide each bus with the power */
bus->regulator = devm_regulator_get(dev, "vdd");
@@ -283,8 +282,7 @@ static int exynos_bus_parent_parse_of(struct device_node
*np,
}
bus->edev_count = count;
- size = sizeof(*bus->edev) * count;
- bus->edev = devm_kzalloc(dev, size, GFP_KERNEL);
+ bus->edev = devm_kcalloc(dev, count, sizeof(*bus->edev), GFP_KERNEL);
if (!bus->edev) {
ret = -ENOMEM;
goto err_regulator;
@@ -388,7 +386,7 @@ static int exynos_bus_profile_init(struct exynos_bus *bus,
ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
if (!ondemand_data) {
ret = -ENOMEM;
- goto err;
+ goto out;
}
ondemand_data->upthreshold = 40;
ondemand_data->downdifferential = 5;
@@ -400,14 +398,14 @@ static int exynos_bus_profile_init(struct exynos_bus *bus,
if (IS_ERR(bus->devfreq)) {
dev_err(dev, "failed to add devfreq device\n");
ret = PTR_ERR(bus->devfreq);
- goto err;
+ goto out;
}
/* Register opp_notifier to catch the change of OPP */
ret = devm_devfreq_register_opp_notifier(dev, bus->devfreq);
if (ret < 0) {
dev_err(dev, "failed to register opp notifier\n");
- goto err;
+ goto out;
}
/*
@@ -417,16 +415,16 @@ static int exynos_bus_profile_init(struct exynos_bus *bus,
ret = exynos_bus_enable_edev(bus);
if (ret < 0) {
dev_err(dev, "failed to enable devfreq-event devices\n");
- goto err;
+ goto out;
}
ret = exynos_bus_set_event(bus);
if (ret < 0) {
dev_err(dev, "failed to set event to devfreq-event devices\n");
- goto err;
+ goto out;
}
-err:
+out:
return ret;
}
@@ -446,27 +444,28 @@ static int exynos_bus_profile_init_passive(struct
exynos_bus *bus,
parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0);
if (IS_ERR(parent_devfreq)) {
ret = -EPROBE_DEFER;
- goto err;
+ goto out;
}
passive_data = devm_kzalloc(dev, sizeof(*passive_data), GFP_KERNEL);
if (!passive_data) {
ret = -ENOMEM;
- goto err;
+ goto out;
}
passive_data->parent = parent_devfreq;
/* Add devfreq device for exynos bus with passive governor */
- bus->devfreq = devm_devfreq_add_device(dev, profile,
DEVFREQ_GOV_PASSIVE,
+ bus->devfreq = devm_devfreq_add_device(dev, profile,
+ DEVFREQ_GOV_PASSIVE,
passive_data);
if (IS_ERR(bus->devfreq)) {
dev_err(dev,
"failed to add devfreq dev with passive governor\n");
ret = PTR_ERR(bus->devfreq);
- goto err;
+ goto out;
}
-err:
+out:
return ret;
}
@@ -484,11 +483,11 @@ static int exynos_bus_probe(struct platform_device *pdev)
return -EINVAL;
}
- bus = devm_kzalloc(>dev, sizeof(*bus), GFP_KERNEL);
+ bus = devm_kzalloc(dev, sizeof(*bus), GFP_KERNEL);
if (!bus)
return -ENOMEM;
mutex_init(>lock);
- bus->dev = >dev;
+ bus->dev = dev;
platform_set_drvdata(pdev, bus);
/* Parse the device-tree to get the resource information */
@@ -502,7 +501,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
goto err;
}
- node = of_parse_phandle(dev->of_node, "devfreq", 0);
+ node = of_parse_phandle(np, "devfreq", 0);
if (node) {
of_node_put(node);
