linux-kernel - Re: [PATCH v7 0/6] Exynos: Simple QoS for exynos-bus using interconnect

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9285e2d4-f2fa-92f2-ba25-832f32f77d78@linaro.org>
Date:   Tue, 3 Nov 2020 10:29:32 +0200
From:   Georgi Djakov <georgi.djakov@...aro.org>
To:     Chanwoo Choi <cw00.choi@...sung.com>,
        Sylwester Nawrocki <s.nawrocki@...sung.com>
Cc:     krzk@...nel.org, devicetree@...r.kernel.org, robh+dt@...nel.org,
        a.swigon@...sung.com, myungjoo.ham@...sung.com,
        inki.dae@...sung.com, sw0312.kim@...sung.com,
        b.zolnierkie@...sung.com, m.szyprowski@...sung.com,
        linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
        linux-samsung-soc@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v7 0/6] Exynos: Simple QoS for exynos-bus using
 interconnect

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,
Georgi

> 
> PS. The passive governor has a bug related to PM_QOS interface.
> So, I posted the patch[4].
> 
> 
> [1] interconnect_graph
> root@...alhost:~# 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@...alhost:~# 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@...alhost:~# 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/
> 
>