Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v7 0/6] Exynos: Simple QoS for exynos-bus using
 interconnect
To:     Georgi Djakov <georgi.djakov@linaro.org>,
        Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc:     krzk@kernel.org, devicetree@vger.kernel.org, robh+dt@kernel.org,
        a.swigon@samsung.com, myungjoo.ham@samsung.com,
        inki.dae@samsung.com, sw0312.kim@samsung.com,
        b.zolnierkie@samsung.com, m.szyprowski@samsung.com,
        linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
        linux-samsung-soc@vger.kernel.org, dri-devel@lists.freedesktop.org
From:   Chanwoo Choi <cw00.choi@samsung.com>
Organization: Samsung Electronics
Message-ID: <3e3038da-d5de-7ea9-9cb9-082ce63af91d@samsung.com>
Date:   Tue, 3 Nov 2020 17:53:10 +0900
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101
        Thunderbird/59.0
MIME-Version: 1.0
In-Reply-To: <9285e2d4-f2fa-92f2-ba25-832f32f77d78@linaro.org>
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 101P
DLP-Filter: Pass
References: <CGME20201030125221eucas1p14e525f75c4b8dadae04144ce7684d776@eucas1p1.samsung.com>
        <20201030125149.8227-1-s.nawrocki@samsung.com>
        <b586c2b7-9ca1-e641-b70c-27493ffd05e0@samsung.com>
        <9285e2d4-f2fa-92f2-ba25-832f32f77d78@linaro.org>
Precedence: bulk

Hi Georgi,

On 11/3/20 5:29 PM, Georgi Djakov wrote:
> Hi Chanwoo and Sylwester,
> 
> On 11/3/20 09:54, Chanwoo Choi wrote:
>> Hi Sylwester,
>>
>> When I tested this patchset on Odroid-U3,
>> After setting 0 bps by interconnect[1][2],
>> the frequency of devfreq devs sustain the high frequency
>> according to the pm qos request.
>>
>> So, I try to find the cause of this situation.
>> In result, it seems that interconnect exynos driver
>> updates the pm qos request to devfreq device
>> during the kernel booting. Do you know why the exynos
>> interconnect driver request the pm qos during probe
>> without the mixer request?
> 
> That's probably because of the sync_state support, that was introduced
> recently. The icc_sync_state callback needs to be added to the driver
> (i just left a comment on that patch), and then check again if it works.
> 
> The idea of the sync_state is that there could be multiple users of a
> path and we must wait for all consumers to tell their bandwidth needs.
> Otherwise the first consumer may lower the bandwidth or disable a path
> needed for another consumer (driver), which has not probed yet. So we
> maintain a floor bandwidth until everyone has probed. By default the floor
> bandwidth is INT_MAX, but can be overridden by implementing the get_bw()
> callback.

Thanks for guide. I tested it with your comment of patch2.
It is well working without problem as I mentioned previously.

I caught the reset operation of PM QoS requested from interconnect
on kernel log. In result, after completed the kernel booting,
there is no pm qos request if hdmi cable is not connected.

[Test Result]
1. Set 622080 Bps with HDMI cable

root@localhost:~# cat /sys/kernel/debug/devfreq/devfreq_summary;
dev                            parent_dev                     governor        timer      polling_ms  cur_freq_Hz  min_freq_Hz  max_freq_Hz
------------------------------ ------------------------------ --------------- ---------- ---------- ------------ ------------ ------------
soc:bus_dmc                    null                           simple_ondemand deferrable         50    400000000    155520000    400000000
soc:bus_acp                    soc:bus_dmc                    passive         null                0    267000000    100000000    267000000
soc:bus_c2c                    soc:bus_dmc                    passive         null                0    400000000    100000000    400000000
soc:bus_leftbus                null                           simple_ondemand deferrable         50    200000000    100000000    200000000
soc:bus_rightbus               soc:bus_leftbus                passive         null                0    200000000    100000000    200000000
soc:bus_display                soc:bus_leftbus                passive         null                0    200000000    160000000    200000000
soc:bus_fsys                   soc:bus_leftbus                passive         null                0    134000000    100000000    134000000
soc:bus_peri                   soc:bus_leftbus                passive         null                0    100000000     50000000    100000000
soc:bus_mfc                    soc:bus_leftbus                passive         null                0    200000000    100000000    200000000
root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_graph;
digraph {
        rankdir = LR
        node [shape = record]
        subgraph cluster_1 {
                label = "soc:bus_dmc"
                "2:bus_dmc" [label="2:bus_dmc
                        |avg_bw=622080kBps
                        |peak_bw=622080kBps"]
        }
        subgraph cluster_2 {
                label = "soc:bus_leftbus"
                "3:bus_leftbus" [label="3:bus_leftbus
                        |avg_bw=622080kBps
                        |peak_bw=622080kBps"]
        }
        subgraph cluster_3 {
                label = "soc:bus_display"
                "4:bus_display" [label="4:bus_display
                        |avg_bw=622080kBps
                        |peak_bw=622080kBps"]
        }
        "3:bus_leftbus" -> "2:bus_dmc"
        "4:bus_display" -> "3:bus_leftbus"
}root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_summary;
 node                                  tag          avg         peak
--------------------------------------------------------------------
bus_dmc                                          622080       622080
  12c10000.mixer                         0       622080       622080
bus_leftbus                                      622080       622080
  12c10000.mixer                         0       622080       622080
bus_display                                      622080       622080
  12c10000.mixer                         0       622080       622080


2. Set 0Bps without HDMI cable
root@localhost:~# cat /sys/kernel/debug/devfreq/devfreq_summary;
dev                            parent_dev                     governor        timer      polling_ms  cur_freq_Hz  min_freq_Hz  max_freq_Hz
------------------------------ ------------------------------ --------------- ---------- ---------- ------------ ------------ ------------
soc:bus_dmc                    null                           simple_ondemand deferrable         50    100000000    100000000    400000000
soc:bus_acp                    soc:bus_dmc                    passive         null                0    100000000    100000000    267000000
soc:bus_c2c                    soc:bus_dmc                    passive         null                0    100000000    100000000    400000000
soc:bus_leftbus                null                           simple_ondemand deferrable         50    100000000    100000000    200000000
soc:bus_rightbus               soc:bus_leftbus                passive         null                0    100000000    100000000    200000000
soc:bus_display                soc:bus_leftbus                passive         null                0    160000000    160000000    200000000
soc:bus_fsys                   soc:bus_leftbus                passive         null                0    100000000    100000000    134000000
soc:bus_peri                   soc:bus_leftbus                passive         null                0     50000000     50000000    100000000
soc:bus_mfc                    soc:bus_leftbus                passive         null                0    100000000    100000000    200000000
root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_graph;
digraph {
        rankdir = LR
        node [shape = record]
        subgraph cluster_1 {
                label = "soc:bus_dmc"
                "2:bus_dmc" [label="2:bus_dmc
                        |avg_bw=0kBps
                        |peak_bw=0kBps"]
        }
        subgraph cluster_2 {
                label = "soc:bus_leftbus"
                "3:bus_leftbus" [label="3:bus_leftbus
                        |avg_bw=0kBps
                        |peak_bw=0kBps"]
        }
        subgraph cluster_3 {
                label = "soc:bus_display"
                "4:bus_display" [label="4:bus_display
                        |avg_bw=0kBps
                        |peak_bw=0kBps"]
        }
        "3:bus_leftbus" -> "2:bus_dmc"
        "4:bus_display" -> "3:bus_leftbus"
}root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_summary;
 node                                  tag          avg         peak
--------------------------------------------------------------------
bus_dmc                                               0            0
  12c10000.mixer                         0            0            0
bus_leftbus                                           0            0
  12c10000.mixer                         0            0            0
bus_display                                           0            0
  12c10000.mixer                         0            0            0


Thanks,
Chanwoo Choi

> 
> Thanks,
> Georgi
> 
>>
>> PS. The passive governor has a bug related to PM_QOS interface.
>> So, I posted the patch[4].
>>
>>
>> [1] interconnect_graph
>> root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_graph 
>> digraph {
>>         rankdir = LR
>>         node [shape = record]
>>         subgraph cluster_1 {
>>                 label = "soc:bus_dmc"
>>                 "2:bus_dmc" [label="2:bus_dmc
>>                         |avg_bw=0kBps
>>                         |peak_bw=0kBps"]
>>         }
>>         subgraph cluster_2 {
>>                 label = "soc:bus_leftbus"
>>                 "3:bus_leftbus" [label="3:bus_leftbus
>>                         |avg_bw=0kBps
>>                         |peak_bw=0kBps"]
>>         }
>>         subgraph cluster_3 {
>>                 label = "soc:bus_display"
>>                 "4:bus_display" [label="4:bus_display
>>                         |avg_bw=0kBps
>>                         |peak_bw=0kBps"]
>>         }
>>         "3:bus_leftbus" -> "2:bus_dmc"
>>         "4:bus_display" -> "3:bus_leftbus"
>>
>>
>> [2] interconnect_summary
>> root@localhost:~# cat /sys/kernel/debug/interconnect/interconnect_summary 
>>  node                                  tag          avg         peak
>> --------------------------------------------------------------------
>> bus_dmc                                               0            0
>>   12c10000.mixer                         0            0            0
>> bus_leftbus                                           0            0
>>   12c10000.mixer                         0            0            0
>> bus_display                                           0            0
>>   12c10000.mixer                         0            0            0
>>
>>
>> [3] devfreq_summary
>> root@localhost:~# cat /sys/kernel/debug/devfreq/devfreq_summary 
>> dev                            parent_dev                     governor        timer      polling_ms  cur_freq_Hz  min_freq_Hz  max_freq_Hz
>> ------------------------------ ------------------------------ --------------- ---------- ---------- ------------ ------------ ------------
>> soc:bus_dmc                    null                           simple_ondemand deferrable         50    400000000    400000000    400000000
>> soc:bus_acp                    soc:bus_dmc                    passive         null                0    267000000    100000000    267000000
>> soc:bus_c2c                    soc:bus_dmc                    passive         null                0    400000000    100000000    400000000
>> soc:bus_leftbus                null                           simple_ondemand deferrable         50    200000000    200000000    200000000
>> soc:bus_rightbus               soc:bus_leftbus                passive         null                0    200000000    100000000    200000000
>> soc:bus_display                soc:bus_leftbus                passive         null                0    200000000    200000000    200000000
>> soc:bus_fsys                   soc:bus_leftbus                passive         null                0    134000000    100000000    134000000
>> soc:bus_peri                   soc:bus_leftbus                passive         null                0    100000000     50000000    100000000
>> soc:bus_mfc                    soc:bus_leftbus                passive         null                0    200000000    100000000    200000000
>>
>>
>> [4] PM / devfreq: passive: Update frequency when start governor
>> https://patchwork.kernel.org/project/linux-pm/patch/20201103070646.18687-1-cw00.choi@samsung.com/
>>
>>
> 
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics