diff mbox

[PATCHv3,2/8] devfreq: exynos: Add documentation for generic exynos memory bus frequency driver

Message ID 1420681257-3078-3-git-send-email-cw00.choi@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chanwoo Choi Jan. 8, 2015, 1:40 a.m. UTC
This patch adds the documentation for generic exynos memory bus frequency
driver.

Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Kukjin Kim <kgene@kernel.org>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 .../devicetree/bindings/devfreq/exynos-busfreq.txt | 184 +++++++++++++++++++++
 1 file changed, 184 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/devfreq/exynos-busfreq.txt

Comments

Rob Herring Jan. 8, 2015, 9:18 p.m. UTC | #1
Adding Viresh.

On Wed, Jan 7, 2015 at 7:40 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> This patch adds the documentation for generic exynos memory bus frequency
> driver.
>
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Kukjin Kim <kgene@kernel.org>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  .../devicetree/bindings/devfreq/exynos-busfreq.txt | 184 +++++++++++++++++++++
>  1 file changed, 184 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/devfreq/exynos-busfreq.txt
>
> diff --git a/Documentation/devicetree/bindings/devfreq/exynos-busfreq.txt b/Documentation/devicetree/bindings/devfreq/exynos-busfreq.txt
> new file mode 100644
> index 0000000..c601e88
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/devfreq/exynos-busfreq.txt
> @@ -0,0 +1,184 @@
> +
> +* Generic Exynos Memory Bus device
> +
> +The Samsung Exynos SoCs have many memory buses for data transfer between DRAM
> +memory and MMC/sub-IP in SoC. Almost Exynos SoCs have the common architecture
> +for memory buses. Generally, Exynos SoC express the memory bus by using memory
> +bus group and block. The memory bus group has one more memory bus blocks and
> +OPP table (including frequency and voltage for DVFS), regulator, devfreq-event
> +devices. Each memory bus block has a clock for own memory bus speen and
> +frequency table for DVFS. There are a little different among Exynos SoCs
> +because each Exynos SoC has the different sub-IP and differnt memory bus.
> +So, this difference should be specified in devicetree file.
> +
> +Required properties for memory bus group:
> +- compatible: Should be "samsung,exynos-memory-bus".
> +- operating-points: the OPP table including frequency/voltage information to
> +                  support DVFS (Dynamic Voltage/Frequency Scaling) feature.
> +- devfreq-events: the devfreq-event device to monitor the curret state of
> +                  memory bus group.

I don't understand what goes in here.

> +- vdd-mem-supply: the regulator to provide memory bus group with the voltage.
> +
> +Required properties for memory bus block:
> +- clock-names : the name of clock used by the memory bus, "memory-bus".
> +- clocks : phandles for clock specified in "clock-names" property.
> +- #clock-cells: should be 1.
> +- frequency: the frequency table to support DVFS feature.

So you have just defined a new OPP table format. We already have one
and Viresh is working to create a more extendable one. He asked about
what's needed in devfreq, so Viresh here you go. :)

> +
> +Example1 : Memory bus group/block in exynos3250.dtsi are listed below.
> +       Exynos3250 has two memory bus group (MIF, INT group). MIF memory bus
> +       group includes one memory bus block between DRAM and eMMC. Also, INT
> +       memory bus group includes eight memory bus blocks which support each
> +       sub-IPs between DRAM and sub-IPs.
> +
> +       memory_bus_mif: memory_bus@0 {
> +               compatible = "samsung,exynos-memory-bus";
> +
> +               operating-points = <
> +                       400000 875000
> +                       200000 800000
> +                       133000 800000
> +                       100000 800000
> +                       50000  800000>;
> +               status = "disabled";

Why is this not part of the DDR controller or /memory node?

> +               blocks {
> +                       dmc_block: memory_bus_block1 {
> +                               clocks = <&cmu_dmc CLK_DIV_DMC>;
> +                               clock-names = "memory-bus";
> +                               frequency = <
> +                                       400000
> +                                       200000
> +                                       133000
> +                                       100000
> +                                       50000>;

This is just duplicated from the operating-points table.

> +                       };
> +               };
> +       };
> +
> +       memory_bus_int: memory_bus@1 {
> +               compatible = "samsung,exynos-memory-bus";
> +
> +               operating-points = <
> +                       400000 950000
> +                       200000 950000
> +                       133000 925000
> +                       100000 850000
> +                       80000  850000
> +                       50000  850000>;
> +
> +               status = "disabled";
> +
> +               blocks {
> +                       peri_block: memory_bus_block1 {

Why is this and the following nodes not part of the respective
peripheral nodes or buses. If you need more hierarchy in your bus add
that to DT first. I'm sure just a flat "simple-bus" was done which
doesn't reflect the actual bus and now you need it to.

> +                               clocks = <&cmu CLK_DIV_ACLK_100>;
> +                               clock-names = "memory-bus";
> +                               frequency = <
> +                                       100000
> +                                       100000
> +                                       100000
> +                                       100000
> +                                       50000
> +                                       50000>;
> +                       };

This just looks like constraints on the clock frequency. This should
be added in a standard way to the clock binding.


> +
> +                       display_block: memory_bus_block2 {
> +                               clocks = <&cmu CLK_DIV_ACLK_160>;
> +                               clock-names = "memory-bus";
> +                               frequency = <
> +                                       200000
> +                                       160000
> +                                       100000
> +                                       80000
> +                                       80000
> +                                       50000>;
> +                       };
> +
> +                       isp_block: memory_bus_block3 {
> +                               clocks = <&cmu CLK_DIV_ACLK_200>;
> +                               clock-names = "memory-bus";
> +                               frequency = <
> +                                       200000
> +                                       200000
> +                                       100000
> +                                       80000
> +                                       50000
> +                                       50000>;
> +                       };
> +
> +                       gps_block: memory_bus_block4 {
> +                               clocks = <&cmu CLK_DIV_ACLK_266>;
> +                               clock-names = "memory-bus";
> +                               frequency = <
> +                                       300000
> +                                       200000
> +                                       133000
> +                                       100000
> +                                       50000
> +                                       50000>;
> +                       };
> +
> +                       mcuisp_block: memory_bus_block5 {
> +                               clocks = <&cmu CLK_DIV_ACLK_400_MCUISP>;
> +                               clock-names = "memory-bus";
> +                               frequency = <
> +                                       400000
> +                                       200000
> +                                       50000
> +                                       50000
> +                                       50000
> +                                       50000>;
> +                       };
> +
> +                       leftbus_block: memory_bus_block6 {
> +                               clocks = <&cmu CLK_DIV_GDL>;
> +                               clock-names = "memory-bus";
> +                               frequency = <
> +                                       200000
> +                                       200000
> +                                       133000
> +                                       100000
> +                                       100000
> +                                       100000>;
> +                       };
> +
> +                       rightbus_block: memory_bus_block7 {
> +                               clocks = <&cmu CLK_DIV_GDR>;
> +                               clock-names = "memory-bus";
> +                               frequency = <
> +                                       200000
> +                                       200000
> +                                       133000
> +                                       100000
> +                                       100000
> +                                       100000>;
> +                       };
> +
> +                       mfc_block: memory_bus_block8 {
> +                               clocks = <&cmu CLK_SCLK_MFC>;
> +                               clock-names = "memory-bus";
> +                               frequency = <
> +                                       200000
> +                                       200000
> +                                       200000
> +                                       133000
> +                                       100000
> +                                       80000>;
> +                       };
> +               };
> +       };
> +
> +Example2 : Usage case to handle the frequency/voltage of memory bus on runtime
> +       in exynos3250-rinato.dts are listed below.
> +
> +       &memory_bus_mif {
> +               devfreq-events = <&ppmu_dmc0_3>, <&ppmu_dmc1_3>;
> +               vdd-mem-supply = <&buck1_reg>;
> +               status = "okay";
> +       };
> +
> +       &memory_bus_int {
> +               devfreq-events = <&ppmu_leftbus_3>, <&ppmu_rightbus_3>;
> +               vdd-mem-supply = <&buck3_reg>;
> +               status = "okay";
> +       };
> --
> 1.8.5.5
>
Chanwoo Choi Jan. 9, 2015, 2:42 a.m. UTC | #2
Hi Rob,

First of all, thanks for your review.

On 01/09/2015 06:18 AM, Rob Herring wrote:
> Adding Viresh.
> 
> On Wed, Jan 7, 2015 at 7:40 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>> This patch adds the documentation for generic exynos memory bus frequency
>> driver.
>>
>> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>> Cc: Kukjin Kim <kgene@kernel.org>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>>  .../devicetree/bindings/devfreq/exynos-busfreq.txt | 184 +++++++++++++++++++++
>>  1 file changed, 184 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/devfreq/exynos-busfreq.txt
>>
>> diff --git a/Documentation/devicetree/bindings/devfreq/exynos-busfreq.txt b/Documentation/devicetree/bindings/devfreq/exynos-busfreq.txt
>> new file mode 100644
>> index 0000000..c601e88
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/devfreq/exynos-busfreq.txt
>> @@ -0,0 +1,184 @@
>> +
>> +* Generic Exynos Memory Bus device
>> +
>> +The Samsung Exynos SoCs have many memory buses for data transfer between DRAM
>> +memory and MMC/sub-IP in SoC. Almost Exynos SoCs have the common architecture
>> +for memory buses. Generally, Exynos SoC express the memory bus by using memory
>> +bus group and block. The memory bus group has one more memory bus blocks and
>> +OPP table (including frequency and voltage for DVFS), regulator, devfreq-event
>> +devices. Each memory bus block has a clock for own memory bus speen and
>> +frequency table for DVFS. There are a little different among Exynos SoCs
>> +because each Exynos SoC has the different sub-IP and differnt memory bus.
>> +So, this difference should be specified in devicetree file.
>> +
>> +Required properties for memory bus group:
>> +- compatible: Should be "samsung,exynos-memory-bus".
>> +- operating-points: the OPP table including frequency/voltage information to
>> +                  support DVFS (Dynamic Voltage/Frequency Scaling) feature.
>> +- devfreq-events: the devfreq-event device to monitor the curret state of
>> +                  memory bus group.
> 
> I don't understand what goes in here.

CPUFREQ use the cpu utilization data to decide the current state of CPU
by CPUFREQ governor. 

Exynos busfreq with DEVFREQ must need the data to monitor the current state
of memory bus of Exynos SoC. So, the devfreq-events provide the current state
of memory bus like as cpu utilization. Exynos busfreq could decide the state
of memory bus by using the devfreq-events.

The summary of devfreq-event device is as following: (https://lkml.org/lkml/2015/1/7/795)
: This patch add new devfreq_event class for devfreq_event device which provide
raw data (e.g., memory bus utilization/GPU utilization). This raw data from
devfreq_event data would be used for the governor of devfreq subsystem.
- devfreq_event device : Provide raw data for governor of existing devfreq device
- devfreq device       : Monitor device state and change frequency/voltage of device
                         using the raw data from devfreq_event device

> 
>> +- vdd-mem-supply: the regulator to provide memory bus group with the voltage.
>> +
>> +Required properties for memory bus block:
>> +- clock-names : the name of clock used by the memory bus, "memory-bus".
>> +- clocks : phandles for clock specified in "clock-names" property.
>> +- #clock-cells: should be 1.
>> +- frequency: the frequency table to support DVFS feature.
> 
> So you have just defined a new OPP table format. We already have one
> and Viresh is working to create a more extendable one. He asked about
> what's needed in devfreq, so Viresh here you go. :)
> 
>> +
>> +Example1 : Memory bus group/block in exynos3250.dtsi are listed below.
>> +       Exynos3250 has two memory bus group (MIF, INT group). MIF memory bus
>> +       group includes one memory bus block between DRAM and eMMC. Also, INT
>> +       memory bus group includes eight memory bus blocks which support each
>> +       sub-IPs between DRAM and sub-IPs.
>> +
>> +       memory_bus_mif: memory_bus@0 {
>> +               compatible = "samsung,exynos-memory-bus";
>> +
>> +               operating-points = <
>> +                       400000 875000
>> +                       200000 800000
>> +                       133000 800000
>> +                       100000 800000
>> +                       50000  800000>;
>> +               status = "disabled";
> 
> Why is this not part of the DDR controller or /memory node?

Because the memory bus node in this patch was dependent on only Exynos SoC.
I didn't check the memory bus of another SoC except for Exynos SoC.

> 
>> +               blocks {
>> +                       dmc_block: memory_bus_block1 {
>> +                               clocks = <&cmu_dmc CLK_DIV_DMC>;
>> +                               clock-names = "memory-bus";
>> +                               frequency = <
>> +                                       400000
>> +                                       200000
>> +                                       133000
>> +                                       100000
>> +                                       50000>;
> 
> This is just duplicated from the operating-points table.

First of all, I explain the meaning of 'memory bus group' and 'memory bus block'.
- The 'memory bus group' includes various memory bus group and has one regulatot for power.
Also, it must need the devfreq-events device to monitor the current state of memory buses.
- The 'memory bus block' has only one clock for memory bus and different clock rate.
And the memory bus blocks which are included in same 'memory bus group' use the same power rail (regulator).

This patch adds 'frequency' table for each memory bus block because
each memory bus block has different clock rate. Only one operating-point table
cannot support various memory bus blocks.

So, I added the frequency table to each memory bus block.

> 
>> +                       };
>> +               };
>> +       };
>> +
>> +       memory_bus_int: memory_bus@1 {
>> +               compatible = "samsung,exynos-memory-bus";
>> +
>> +               operating-points = <
>> +                       400000 950000
>> +                       200000 950000
>> +                       133000 925000
>> +                       100000 850000
>> +                       80000  850000
>> +                       50000  850000>;
>> +
>> +               status = "disabled";
>> +
>> +               blocks {
>> +                       peri_block: memory_bus_block1 {
> 
> Why is this and the following nodes not part of the respective
> peripheral nodes or buses. If you need more hierarchy in your bus add
> that to DT first.

I explain the hierarchy of Exynos memory buses.

This patch divide the memory bus group according to power rail (regulator).
- MIF (Memory Interface ) memory bus group uses the VDD_MIF regulator.
- INT (Internal) memory bus group uses the VDD_INT regulator.

For example,.
Each memory bus group contains only one power rail(regulator) and one more memory bus blocks as follwing:

- MIF memory bus group                     
power rail(VDD_MIF)-->|--- memory bus for DMC (Dynamic Memory Controller) block (dmc clock)


- INT memory bus group
                      |--- memory bus for PERI block (aclk_100 clock)
                      |
                      |--- memory bus for DISPLAY block (aclk_160 clock)
                      |  
                      |--- memory bus for ISP block (aclk_200 clock)
                      |
                      |--- memory bus for GPS block (aclk_266 clock)
power rail(VDD_INT)-->|
                      |--- memory bus for MCUISP block (aclk_400_mcuisp clock)
                      |
                      |--- memory bus for Leftbus block (gdl clock)
                      |
                      |--- memory bus for Rightbus block (gdr clock)
                      |
                      |--- memory bus for MFC block (mfc clock)


> I'm sure just a flat "simple-bus" was done which
> doesn't reflect the actual bus and now you need it to.

I'm not sure that the memory bus concept of this patch
would support the memory bus of all SoCs. So, I didn't modify common bus driver.

If I misunderstand about your question, please explain in more detail about your opinion.

> 
>> +                               clocks = <&cmu CLK_DIV_ACLK_100>;
>> +                               clock-names = "memory-bus";
>> +                               frequency = <
>> +                                       100000
>> +                                       100000
>> +                                       100000
>> +                                       100000
>> +                                       50000
>> +                                       50000>;
>> +                       };
> 
> This just looks like constraints on the clock frequency. This should
> be added in a standard way to the clock binding.

I explained the 'frequency' table of each memory bus block on upper reply.

> 
> 
>> +
>> +                       display_block: memory_bus_block2 {
>> +                               clocks = <&cmu CLK_DIV_ACLK_160>;
>> +                               clock-names = "memory-bus";
>> +                               frequency = <
>> +                                       200000
>> +                                       160000
>> +                                       100000
>> +                                       80000
>> +                                       80000
>> +                                       50000>;
>> +                       };
>> +
>> +                       isp_block: memory_bus_block3 {
>> +                               clocks = <&cmu CLK_DIV_ACLK_200>;
>> +                               clock-names = "memory-bus";
>> +                               frequency = <
>> +                                       200000
>> +                                       200000
>> +                                       100000
>> +                                       80000
>> +                                       50000
>> +                                       50000>;
>> +                       };
>> +
>> +                       gps_block: memory_bus_block4 {
>> +                               clocks = <&cmu CLK_DIV_ACLK_266>;
>> +                               clock-names = "memory-bus";
>> +                               frequency = <
>> +                                       300000
>> +                                       200000
>> +                                       133000
>> +                                       100000
>> +                                       50000
>> +                                       50000>;
>> +                       };
>> +
>> +                       mcuisp_block: memory_bus_block5 {
>> +                               clocks = <&cmu CLK_DIV_ACLK_400_MCUISP>;
>> +                               clock-names = "memory-bus";
>> +                               frequency = <
>> +                                       400000
>> +                                       200000
>> +                                       50000
>> +                                       50000
>> +                                       50000
>> +                                       50000>;
>> +                       };
>> +
>> +                       leftbus_block: memory_bus_block6 {
>> +                               clocks = <&cmu CLK_DIV_GDL>;
>> +                               clock-names = "memory-bus";
>> +                               frequency = <
>> +                                       200000
>> +                                       200000
>> +                                       133000
>> +                                       100000
>> +                                       100000
>> +                                       100000>;
>> +                       };
>> +
>> +                       rightbus_block: memory_bus_block7 {
>> +                               clocks = <&cmu CLK_DIV_GDR>;
>> +                               clock-names = "memory-bus";
>> +                               frequency = <
>> +                                       200000
>> +                                       200000
>> +                                       133000
>> +                                       100000
>> +                                       100000
>> +                                       100000>;
>> +                       };
>> +
>> +                       mfc_block: memory_bus_block8 {
>> +                               clocks = <&cmu CLK_SCLK_MFC>;
>> +                               clock-names = "memory-bus";
>> +                               frequency = <
>> +                                       200000
>> +                                       200000
>> +                                       200000
>> +                                       133000
>> +                                       100000
>> +                                       80000>;
>> +                       };
>> +               };
>> +       };
>> +
>> +Example2 : Usage case to handle the frequency/voltage of memory bus on runtime
>> +       in exynos3250-rinato.dts are listed below.
>> +
>> +       &memory_bus_mif {
>> +               devfreq-events = <&ppmu_dmc0_3>, <&ppmu_dmc1_3>;
>> +               vdd-mem-supply = <&buck1_reg>;
>> +               status = "okay";
>> +       };
>> +
>> +       &memory_bus_int {
>> +               devfreq-events = <&ppmu_leftbus_3>, <&ppmu_rightbus_3>;
>> +               vdd-mem-supply = <&buck3_reg>;
>> +               status = "okay";
>> +       };
>> --

Best Regards,
Chanwoo Choi
Viresh Kumar Jan. 20, 2015, 7:19 a.m. UTC | #3
On 9 January 2015 at 02:48, Rob Herring <robherring2@gmail.com> wrote:
> Adding Viresh.

Sorry for being too late, I was very busy with other cpufreq stuff I was doing
and saved this thread for later as it required me to understand it properly..

>> +Required properties for memory bus block:
>> +- clock-names : the name of clock used by the memory bus, "memory-bus".
>> +- clocks : phandles for clock specified in "clock-names" property.
>> +- #clock-cells: should be 1.
>> +- frequency: the frequency table to support DVFS feature.
>
> So you have just defined a new OPP table format. We already have one
> and Viresh is working to create a more extendable one. He asked about
> what's needed in devfreq, so Viresh here you go. :)

I failed to understand what's new here, probably I need more clarity on
what we are doing here..

So, this is what I see from OPPs point of view, everything else stripped out.

>> +       memory_bus_int: memory_bus@1 {

>> +               operating-points = <
>> +                       400000 950000
>> +                       200000 950000
>> +                       133000 925000
>> +                       100000 850000
>> +                       80000  850000
>> +                       50000  850000>;

So these are the OPPs your "groups" support and below ones are
the frequencies that each block will support. Right ?

>> +               blocks {

>> +                               frequency = <
>> +                                       100000
>> +                                       100000
>> +                                       100000
>> +                                       100000

Why this replication here ?

>> +                                       50000
>> +                                       50000>;
>> +                       };

How are the above two tables (operating-points and frequency) related
here? What about the voltages at which these frequencies are possible ?

>> +                       display_block: memory_bus_block2 {

>> +                               frequency = <
>> +                                       200000
>> +                                       160000
>> +                                       100000
>> +                                       80000
>> +                                       80000
>> +                                       50000>;
>> +                       };

>> +                       isp_block: memory_bus_block3 {

>> +                               frequency = <
>> +                                       200000
>> +                                       200000
>> +                                       100000
>> +                                       80000
>> +                                       50000
>> +                                       50000>;
>> +                       };

>> +                       gps_block: memory_bus_block4 {

>> +                               frequency = <
>> +                                       300000
>> +                                       200000
>> +                                       133000
>> +                                       100000
>> +                                       50000
>> +                                       50000>;
>> +                       };

same for others as well..
Chanwoo Choi Jan. 20, 2015, 8:23 a.m. UTC | #4
Hi Viresh,

I explained the relation between memory bus group and memory bus block on following patch[1].
- [1] https://lkml.org/lkml/2015/1/8/642

On 01/20/2015 04:19 PM, Viresh Kumar wrote:
> On 9 January 2015 at 02:48, Rob Herring <robherring2@gmail.com> wrote:
>> Adding Viresh.
> 
> Sorry for being too late, I was very busy with other cpufreq stuff I was doing
> and saved this thread for later as it required me to understand it properly..
> 
>>> +Required properties for memory bus block:
>>> +- clock-names : the name of clock used by the memory bus, "memory-bus".
>>> +- clocks : phandles for clock specified in "clock-names" property.
>>> +- #clock-cells: should be 1.
>>> +- frequency: the frequency table to support DVFS feature.
>>
>> So you have just defined a new OPP table format. We already have one
>> and Viresh is working to create a more extendable one. He asked about
>> what's needed in devfreq, so Viresh here you go. :)
> 
> I failed to understand what's new here, probably I need more clarity on
> what we are doing here..
> 
> So, this is what I see from OPPs point of view, everything else stripped out.
> 
>>> +       memory_bus_int: memory_bus@1 {
> 
>>> +               operating-points = <
>>> +                       400000 950000
>>> +                       200000 950000
>>> +                       133000 925000
>>> +                       100000 850000
>>> +                       80000  850000
>>> +                       50000  850000>;
> 
> So these are the OPPs your "groups" support and below ones are
> the frequencies that each block will support. Right ?

Right.
But, the frequency of OPPs is only used for devfreq ondemand governor.
After deciding the proper frequency of memory bus on ondemand governor,
exynos-bus.c (exynos memory bus frequency driver) use the frequency table
of memory bus blocks on below to change the clock rate of memory bus block.

> 
>>> +               blocks {
> 
>>> +                               frequency = <
>>> +                                       100000
>>> +                                       100000
>>> +                                       100000
>>> +                                       100000
> 
> Why this replication here ?

Firs of all,
I explain the hierarchy of Exynos memory buses.

For example of Exynos3250 memory bus,
This patch divide the memory bus group according to power rail (regulator).
- MIF (Memory Interface ) memory bus group uses the VDD_MIF regulator.
- INT (Internal) memory bus group uses the VDD_INT regulator.

Each memory bus group contains only one power rail(regulator) and one more memory bus blocks as follwing:

- MIF memory bus group                     
power rail(VDD_MIF)-->|--- memory bus for DMC (Dynamic Memory Controller) block (dmc clock)

- INT memory bus group
                      |--- memory bus for PERI block (aclk_100 clock)
                      |
                      |--- memory bus for DISPLAY block (aclk_160 clock)
                      |  
                      |--- memory bus for ISP block (aclk_200 clock)
                      |
                      |--- memory bus for GPS block (aclk_266 clock)
power rail(VDD_INT)-->|
                      |--- memory bus for MCUISP block (aclk_400_mcuisp clock)
                      |
                      |--- memory bus for Leftbus block (gdl clock)
                      |
                      |--- memory bus for Rightbus block (gdr clock)
                      |
                      |--- memory bus for MFC block (mfc clock)


Exynos3250 has following table for INT memory bus group:
All clocks of INT memory bus group have to contain the same entry count
againt the number of 'virtual freqw'. So, each memory bus clock could have duplicate clocks.

------------------------------------------------------------------------
Level|virtual freq|PERI's clk|Display's clk|ISP's clk|GPS's clk| voltage|
------------------------------------------------------------------------
L6   |400000      |100000    |200000       |200000   |300000   | 95000  |
L5   |200000      |100000    |160000       |200000   |200000   | 95000  |
L4   |133000      |100000    |100000       |100000   |133000   | 92500  |
L3   |100000      |100000    |80000        |80000    |100000   | 85000  |
L2   |80000       |50000     |80000        |50000    |50000    | 85000  |
L1   |50000       |50000     |50000        |50000    |50000    | 85000  |
-------------------------------------------------------------------------
(Except for mcuisp, leftbus, rightbus, mfc block)

This table is used for devfreq ondemand governor as following:
1. ondemand governor in devfreq use the 'virtual freq' to devcide the proper
   frequency for memory bus. 
2. ondemand governor executes the *_target() function to set clock rate and voltage.
3. *_target() function in exynos-bus.c changes the clock rate of {PERIS|Display|ISP|GPS} clk
   according to decided 'Level' by devfreq ondemand governor.

> 
>>> +                                       50000
>>> +                                       50000>;
>>> +                       };
> 
> How are the above two tables (operating-points and frequency) related
> here? What about the voltages at which these frequencies are possible ?

I explained it on the upper.

> 
>>> +                       display_block: memory_bus_block2 {
> 
>>> +                               frequency = <
>>> +                                       200000
>>> +                                       160000
>>> +                                       100000
>>> +                                       80000
>>> +                                       80000
>>> +                                       50000>;
>>> +                       };
> 
>>> +                       isp_block: memory_bus_block3 {
> 
>>> +                               frequency = <
>>> +                                       200000
>>> +                                       200000
>>> +                                       100000
>>> +                                       80000
>>> +                                       50000
>>> +                                       50000>;
>>> +                       };
> 
>>> +                       gps_block: memory_bus_block4 {
> 
>>> +                               frequency = <
>>> +                                       300000
>>> +                                       200000
>>> +                                       133000
>>> +                                       100000
>>> +                                       50000
>>> +                                       50000>;
>>> +                       };
> 
> same for others as well..

I explained it on the upper.

Best Regards,
Chanwoo Choi
Viresh Kumar Jan. 20, 2015, 11:22 a.m. UTC | #5
On 20 January 2015 at 13:53, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> But, the frequency of OPPs is only used for devfreq ondemand governor.
> After deciding the proper frequency of memory bus on ondemand governor,
> exynos-bus.c (exynos memory bus frequency driver) use the frequency table
> of memory bus blocks on below to change the clock rate of memory bus block.

> Firs of all,
> I explain the hierarchy of Exynos memory buses.
>
> For example of Exynos3250 memory bus,
> This patch divide the memory bus group according to power rail (regulator).
> - MIF (Memory Interface ) memory bus group uses the VDD_MIF regulator.
> - INT (Internal) memory bus group uses the VDD_INT regulator.
>
> Each memory bus group contains only one power rail(regulator) and one more memory bus blocks as follwing:
>
> - MIF memory bus group
> power rail(VDD_MIF)-->|--- memory bus for DMC (Dynamic Memory Controller) block (dmc clock)
>
> - INT memory bus group
>                       |--- memory bus for PERI block (aclk_100 clock)
>                       |
>                       |--- memory bus for DISPLAY block (aclk_160 clock)
>                       |
>                       |--- memory bus for ISP block (aclk_200 clock)
>                       |
>                       |--- memory bus for GPS block (aclk_266 clock)
> power rail(VDD_INT)-->|
>                       |--- memory bus for MCUISP block (aclk_400_mcuisp clock)
>                       |
>                       |--- memory bus for Leftbus block (gdl clock)
>                       |
>                       |--- memory bus for Rightbus block (gdr clock)
>                       |
>                       |--- memory bus for MFC block (mfc clock)
>
>
> Exynos3250 has following table for INT memory bus group:
> All clocks of INT memory bus group have to contain the same entry count
> againt the number of 'virtual freqw'. So, each memory bus clock could have duplicate clocks.
>
> ------------------------------------------------------------------------
> Level|virtual freq|PERI's clk|Display's clk|ISP's clk|GPS's clk| voltage|
> ------------------------------------------------------------------------
> L6   |400000      |100000    |200000       |200000   |300000   | 95000  |
> L5   |200000      |100000    |160000       |200000   |200000   | 95000  |
> L4   |133000      |100000    |100000       |100000   |133000   | 92500  |
> L3   |100000      |100000    |80000        |80000    |100000   | 85000  |
> L2   |80000       |50000     |80000        |50000    |50000    | 85000  |
> L1   |50000       |50000     |50000        |50000    |50000    | 85000  |
> -------------------------------------------------------------------------
> (Except for mcuisp, leftbus, rightbus, mfc block)
>
> This table is used for devfreq ondemand governor as following:
> 1. ondemand governor in devfreq use the 'virtual freq' to devcide the proper
>    frequency for memory bus.
> 2. ondemand governor executes the *_target() function to set clock rate and voltage.
> 3. *_target() function in exynos-bus.c changes the clock rate of {PERIS|Display|ISP|GPS} clk
>    according to decided 'Level' by devfreq ondemand governor.

I see..

Please confirm if my understanding is correct.
- mem-bus-group: all blocks sharing a regulator
- mem-bus-block: individual blocks managing their clock rate

What you do in kernel is register group as a device (with virtual OPPs)
and then manage blocks within the driver. And so you need to do this
dummy mapping of virtual to physical frequencies.

It *may* not have done it this way, if I was to design this driver.

Each bus-block is a separately controllable device and so must be registered
separately. In that case all bus-blocks will have separate OPP tables and
all this dummy-v2p mapping will go away.

I believe that you are over complicating stuff without any need..

--
viresh
Chanwoo Choi Jan. 20, 2015, 11:37 a.m. UTC | #6
On 01/20/2015 08:22 PM, Viresh Kumar wrote:
> On 20 January 2015 at 13:53, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>> But, the frequency of OPPs is only used for devfreq ondemand governor.
>> After deciding the proper frequency of memory bus on ondemand governor,
>> exynos-bus.c (exynos memory bus frequency driver) use the frequency table
>> of memory bus blocks on below to change the clock rate of memory bus block.
> 
>> Firs of all,
>> I explain the hierarchy of Exynos memory buses.
>>
>> For example of Exynos3250 memory bus,
>> This patch divide the memory bus group according to power rail (regulator).
>> - MIF (Memory Interface ) memory bus group uses the VDD_MIF regulator.
>> - INT (Internal) memory bus group uses the VDD_INT regulator.
>>
>> Each memory bus group contains only one power rail(regulator) and one more memory bus blocks as follwing:
>>
>> - MIF memory bus group
>> power rail(VDD_MIF)-->|--- memory bus for DMC (Dynamic Memory Controller) block (dmc clock)
>>
>> - INT memory bus group
>>                       |--- memory bus for PERI block (aclk_100 clock)
>>                       |
>>                       |--- memory bus for DISPLAY block (aclk_160 clock)
>>                       |
>>                       |--- memory bus for ISP block (aclk_200 clock)
>>                       |
>>                       |--- memory bus for GPS block (aclk_266 clock)
>> power rail(VDD_INT)-->|
>>                       |--- memory bus for MCUISP block (aclk_400_mcuisp clock)
>>                       |
>>                       |--- memory bus for Leftbus block (gdl clock)
>>                       |
>>                       |--- memory bus for Rightbus block (gdr clock)
>>                       |
>>                       |--- memory bus for MFC block (mfc clock)
>>
>>
>> Exynos3250 has following table for INT memory bus group:
>> All clocks of INT memory bus group have to contain the same entry count
>> againt the number of 'virtual freqw'. So, each memory bus clock could have duplicate clocks.
>>
>> ------------------------------------------------------------------------
>> Level|virtual freq|PERI's clk|Display's clk|ISP's clk|GPS's clk| voltage|
>> ------------------------------------------------------------------------
>> L6   |400000      |100000    |200000       |200000   |300000   | 95000  |
>> L5   |200000      |100000    |160000       |200000   |200000   | 95000  |
>> L4   |133000      |100000    |100000       |100000   |133000   | 92500  |
>> L3   |100000      |100000    |80000        |80000    |100000   | 85000  |
>> L2   |80000       |50000     |80000        |50000    |50000    | 85000  |
>> L1   |50000       |50000     |50000        |50000    |50000    | 85000  |
>> -------------------------------------------------------------------------
>> (Except for mcuisp, leftbus, rightbus, mfc block)
>>
>> This table is used for devfreq ondemand governor as following:
>> 1. ondemand governor in devfreq use the 'virtual freq' to devcide the proper
>>    frequency for memory bus.
>> 2. ondemand governor executes the *_target() function to set clock rate and voltage.
>> 3. *_target() function in exynos-bus.c changes the clock rate of {PERIS|Display|ISP|GPS} clk
>>    according to decided 'Level' by devfreq ondemand governor.
> 
> I see..
> 
> Please confirm if my understanding is correct.
> - mem-bus-group: all blocks sharing a regulator
> - mem-bus-block: individual blocks managing their clock rate

Right and mem-bus-blcoks are included in a mem-bus-group.

> 
> What you do in kernel is register group as a device (with virtual OPPs)
> and then manage blocks within the driver. And so you need to do this
> dummy mapping of virtual to physical frequencies.

Right.

> 
> It *may* not have done it this way, if I was to design this driver.
> 
> Each bus-block is a separately controllable device and so must be registered
> separately. In that case all bus-blocks will have separate OPP tables and
> all this dummy-v2p mapping will go away.

If each bus-block has separate regulator independently, each bus-block can be registered
separately. But, exynos bus-blocks in mem-bus-group share the same regulator.

> 
> I believe that you are over complicating stuff without any need..
> 
> --
> viresh
> 

Best Regards,
Chanwoo Choi
Viresh Kumar Jan. 21, 2015, 3:17 a.m. UTC | #7
On 20 January 2015 at 17:07, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> If each bus-block has separate regulator independently, each bus-block can be registered
> separately. But, exynos bus-blocks in mem-bus-group share the same regulator.

This can be managed easily within the driver. Just stay the highest
voltage requested.
I don't think its not manageable. But it will be much more convenient
as that will show
the correct picture and get rid of unnecessarily virtualizing things..

--
viresh
Chanwoo Choi Jan. 21, 2015, 4:20 a.m. UTC | #8
On 01/21/2015 12:17 PM, Viresh Kumar wrote:
> On 20 January 2015 at 17:07, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>> If each bus-block has separate regulator independently, each bus-block can be registered
>> separately. But, exynos bus-blocks in mem-bus-group share the same regulator.
> 
> This can be managed easily within the driver. Just stay the highest
> voltage requested.

If the clock will be stayed on highest voltage, will reduce
the considerable benefit of power-consumption.

I think it is DFS instead of DVFS. Is it right?

> I don't think its not manageable. But it will be much more convenient
> as that will show
> the correct picture and get rid of unnecessarily virtualizing things..

What is the correct picture? do you need more detailed explanation
of Exynos memory bus structure?

Thanks,
Chanwoo
Viresh Kumar Jan. 21, 2015, 4:37 a.m. UTC | #9
On 21 January 2015 at 09:50, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> If the clock will be stayed on highest voltage, will reduce
> the considerable benefit of power-consumption.

But this is exactly what you must be doing right now as well..
I think I didn't make it clear enough with an example. Let me
try..

This is how I feel it should look like:

       memory_bus_int: memory_bus@1 {
               // Regulator is shared for all below
               blocks {
                       peri_block: memory_bus_block1 {
                                operating-points = <
                                        100000 850000
                                        50000  850000>;
                       };

                       display_block: memory_bus_block2 {
                                operating-points = <
                                        200000 950000
                                        160000 950000
                                        100000 925000
                                        80000  850000
                                        50000  850000>;
                       };

                       isp_block: memory_bus_block3 {
                                operating-points = <
                                        200000 950000
                                        100000 925000
                                        80000  850000
                                        50000  850000>;
                       };

                       gps_block: memory_bus_block4 {
                                operating-points = <
                                        300000 950000
                                        200000 950000
                                        133000 925000
                                        100000 850000
                                        50000  850000>;
                       };


Now suppose these are the requirements from all the blocks
at any point of time:
- block1: 100000 850000
- block2: 100000 925000
- block3: 80000  850000
- block4: 133000 925000

Now, all of them can control freq separately and we don't need to
worry for that. But regulator is shared between them. We can check
what's the highest voltage requested at this point of time and can
switch to that.

i.e. 925000 in this case. And that's not the highest possible one.
And you will reach to similar conclusion with your current code as
well I believe.

> I think it is DFS instead of DVFS. Is it right?

No. I am still talking about DVFS, V being the highest requested one.

> What is the correct picture? do you need more detailed explanation
> of Exynos memory bus structure?

The correct picture is what i showed in the above dts example.
Chanwoo Choi Jan. 21, 2015, 6:12 a.m. UTC | #10
On 01/21/2015 01:37 PM, Viresh Kumar wrote:
> On 21 January 2015 at 09:50, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>> If the clock will be stayed on highest voltage, will reduce
>> the considerable benefit of power-consumption.
> 
> But this is exactly what you must be doing right now as well..
> I think I didn't make it clear enough with an example. Let me
> try..
> 
> This is how I feel it should look like:
> 
>        memory_bus_int: memory_bus@1 {
>                // Regulator is shared for all below
>                blocks {
>                        peri_block: memory_bus_block1 {
>                                 operating-points = <
>                                         100000 850000
>                                         50000  850000>;
>                        };
> 
>                        display_block: memory_bus_block2 {
>                                 operating-points = <
>                                         200000 950000
>                                         160000 950000
>                                         100000 925000
>                                         80000  850000
>                                         50000  850000>;
>                        };
> 
>                        isp_block: memory_bus_block3 {
>                                 operating-points = <
>                                         200000 950000
>                                         100000 925000
>                                         80000  850000
>                                         50000  850000>;
>                        };
> 
>                        gps_block: memory_bus_block4 {
>                                 operating-points = <
>                                         300000 950000
>                                         200000 950000
>                                         133000 925000
>                                         100000 850000
>                                         50000  850000>;
>                        };
> 
> 
> Now suppose these are the requirements from all the blocks
> at any point of time:
> - block1: 100000 850000
> - block2: 100000 925000
> - block3: 80000  850000
> - block4: 133000 925000
> 
> Now, all of them can control freq separately and we don't need to
> worry for that. But regulator is shared between them. We can check
> what's the highest voltage requested at this point of time and can
> switch to that.
> 
> i.e. 925000 in this case. And that's not the highest possible one.
> And you will reach to similar conclusion with your current code as
> well I believe.
> 

OK, I understand.
I'll try to update exynos memory bus according to your comment.

Thanks
Chanwoo
Viresh Kumar Jan. 21, 2015, 6:32 a.m. UTC | #11
On 21 January 2015 at 11:42, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> OK, I understand.
> I'll try to update exynos memory bus according to your comment.

Great.

@Rob: So there is nothing special required for devfreq drivers in new
OPP bindings, right ? :)
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/devfreq/exynos-busfreq.txt b/Documentation/devicetree/bindings/devfreq/exynos-busfreq.txt
new file mode 100644
index 0000000..c601e88
--- /dev/null
+++ b/Documentation/devicetree/bindings/devfreq/exynos-busfreq.txt
@@ -0,0 +1,184 @@ 
+
+* Generic Exynos Memory Bus device
+
+The Samsung Exynos SoCs have many memory buses for data transfer between DRAM
+memory and MMC/sub-IP in SoC. Almost Exynos SoCs have the common architecture
+for memory buses. Generally, Exynos SoC express the memory bus by using memory
+bus group and block. The memory bus group has one more memory bus blocks and
+OPP table (including frequency and voltage for DVFS), regulator, devfreq-event
+devices. Each memory bus block has a clock for own memory bus speen and
+frequency table for DVFS. There are a little different among Exynos SoCs
+because each Exynos SoC has the different sub-IP and differnt memory bus.
+So, this difference should be specified in devicetree file.
+
+Required properties for memory bus group:
+- compatible: Should be "samsung,exynos-memory-bus".
+- operating-points: the OPP table including frequency/voltage information to
+                  support DVFS (Dynamic Voltage/Frequency Scaling) feature.
+- devfreq-events: the devfreq-event device to monitor the curret state of
+                  memory bus group.
+- vdd-mem-supply: the regulator to provide memory bus group with the voltage.
+
+Required properties for memory bus block:
+- clock-names : the name of clock used by the memory bus, "memory-bus".
+- clocks : phandles for clock specified in "clock-names" property.
+- #clock-cells: should be 1.
+- frequency: the frequency table to support DVFS feature.
+
+Example1 : Memory bus group/block in exynos3250.dtsi are listed below.
+	Exynos3250 has two memory bus group (MIF, INT group). MIF memory bus
+	group includes one memory bus block between DRAM and eMMC. Also, INT
+	memory bus group includes eight memory bus blocks which support each
+	sub-IPs between DRAM and sub-IPs.
+
+	memory_bus_mif: memory_bus@0 {
+		compatible = "samsung,exynos-memory-bus";
+
+		operating-points = <
+			400000 875000
+			200000 800000
+			133000 800000
+			100000 800000
+			50000  800000>;
+		status = "disabled";
+
+		blocks {
+			dmc_block: memory_bus_block1 {
+				clocks = <&cmu_dmc CLK_DIV_DMC>;
+				clock-names = "memory-bus";
+				frequency = <
+					400000
+					200000
+					133000
+					100000
+					50000>;
+			};
+		};
+	};
+
+	memory_bus_int: memory_bus@1 {
+		compatible = "samsung,exynos-memory-bus";
+
+		operating-points = <
+			400000 950000
+			200000 950000
+			133000 925000
+			100000 850000
+			80000  850000
+			50000  850000>;
+
+		status = "disabled";
+
+		blocks {
+			peri_block: memory_bus_block1 {
+				clocks = <&cmu CLK_DIV_ACLK_100>;
+				clock-names = "memory-bus";
+				frequency = <
+					100000
+					100000
+					100000
+					100000
+					50000
+					50000>;
+			};
+
+			display_block: memory_bus_block2 {
+				clocks = <&cmu CLK_DIV_ACLK_160>;
+				clock-names = "memory-bus";
+				frequency = <
+					200000
+					160000
+					100000
+					80000
+					80000
+					50000>;
+			};
+
+			isp_block: memory_bus_block3 {
+				clocks = <&cmu CLK_DIV_ACLK_200>;
+				clock-names = "memory-bus";
+				frequency = <
+					200000
+					200000
+					100000
+					80000
+					50000
+					50000>;
+			};
+
+			gps_block: memory_bus_block4 {
+				clocks = <&cmu CLK_DIV_ACLK_266>;
+				clock-names = "memory-bus";
+				frequency = <
+					300000
+					200000
+					133000
+					100000
+					50000
+					50000>;
+			};
+
+			mcuisp_block: memory_bus_block5 {
+				clocks = <&cmu CLK_DIV_ACLK_400_MCUISP>;
+				clock-names = "memory-bus";
+				frequency = <
+					400000
+					200000
+					50000
+					50000
+					50000
+					50000>;
+			};
+
+			leftbus_block: memory_bus_block6 {
+				clocks = <&cmu CLK_DIV_GDL>;
+				clock-names = "memory-bus";
+				frequency = <
+					200000
+					200000
+					133000
+					100000
+					100000
+					100000>;
+			};
+
+			rightbus_block: memory_bus_block7 {
+				clocks = <&cmu CLK_DIV_GDR>;
+				clock-names = "memory-bus";
+				frequency = <
+					200000
+					200000
+					133000
+					100000
+					100000
+					100000>;
+			};
+
+			mfc_block: memory_bus_block8 {
+				clocks = <&cmu CLK_SCLK_MFC>;
+				clock-names = "memory-bus";
+				frequency = <
+					200000
+					200000
+					200000
+					133000
+					100000
+					80000>;
+			};
+		};
+	};
+
+Example2 : Usage case to handle the frequency/voltage of memory bus on runtime
+	in exynos3250-rinato.dts are listed below.
+
+	&memory_bus_mif {
+		devfreq-events = <&ppmu_dmc0_3>, <&ppmu_dmc1_3>;
+		vdd-mem-supply = <&buck1_reg>;
+		status = "okay";
+	};
+
+	&memory_bus_int {
+		devfreq-events = <&ppmu_leftbus_3>, <&ppmu_rightbus_3>;
+		vdd-mem-supply = <&buck3_reg>;
+		status = "okay";
+	};