* [PATCH AUTOSEL 4.19 190/252] ide: remove set but not used variable 'hwif'
From: Sasha Levin @ 2020-02-14 16:10 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, Wang Hai, linux-ide, Hulk Robot, linuxppc-dev,
David S . Miller
In-Reply-To: <20200214161147.15842-1-sashal@kernel.org>
From: Wang Hai <wanghai38@huawei.com>
[ Upstream commit 98949a1946d70771789def0c9dbc239497f9f138 ]
Fix the following gcc warning:
drivers/ide/pmac.c: In function pmac_ide_setup_device:
drivers/ide/pmac.c:1027:14: warning: variable hwif set but not used
[-Wunused-but-set-variable]
Fixes: d58b0c39e32f ("powerpc/macio: Rework hotplug media bay support")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Wang Hai <wanghai38@huawei.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/ide/pmac.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/ide/pmac.c b/drivers/ide/pmac.c
index 203ed4adc04ae..7db083ec5ee06 100644
--- a/drivers/ide/pmac.c
+++ b/drivers/ide/pmac.c
@@ -1024,7 +1024,6 @@ static int pmac_ide_setup_device(pmac_ide_hwif_t *pmif, struct ide_hw *hw)
struct device_node *np = pmif->node;
const int *bidp;
struct ide_host *host;
- ide_hwif_t *hwif;
struct ide_hw *hws[] = { hw };
struct ide_port_info d = pmac_port_info;
int rc;
@@ -1080,7 +1079,7 @@ static int pmac_ide_setup_device(pmac_ide_hwif_t *pmif, struct ide_hw *hw)
rc = -ENOMEM;
goto bail;
}
- hwif = pmif->hwif = host->ports[0];
+ pmif->hwif = host->ports[0];
if (on_media_bay(pmif)) {
/* Fixup bus ID for media bay */
--
2.20.1
^ permalink raw reply related
* [PATCH 3/3] rtnl: Extract table, priority and pref
From: Daniel Wagner @ 2020-02-14 18:52 UTC (permalink / raw)
To: ell
In-Reply-To: <20200214185253.32658-1-wagi@monom.org>
[-- Attachment #1: Type: text/plain, Size: 5216 bytes --]
---
ell/rtnl.c | 38 ++++++++++++++++++++++++++++++--------
ell/rtnl.h | 8 ++++----
unit/test-rtnl.c | 16 ++++++++++------
3 files changed, 44 insertions(+), 18 deletions(-)
diff --git a/ell/rtnl.c b/ell/rtnl.c
index c94531144eb4..4600a7e42001 100644
--- a/ell/rtnl.c
+++ b/ell/rtnl.c
@@ -69,12 +69,14 @@ static size_t rta_add_data(void *rta_buf, unsigned short type, void *data,
}
static void l_rtnl_route_extract(const struct rtmsg *rtmsg, uint32_t len,
- int family, uint32_t *ifindex, char **dst,
- char **gateway, char **src)
+ int family, uint32_t *table, uint32_t *ifindex,
+ uint32_t *priority, uint8_t *pref,
+ char **dst, char **gateway, char **src)
{
struct rtattr *attr;
char buf[INET6_ADDRSTRLEN];
+ /* Not extracted at the moment: RTA_CACHEINFO for IPv6 */
for (attr = RTM_RTA(rtmsg); RTA_OK(attr, len);
attr = RTA_NEXT(attr, len)) {
switch (attr->rta_type) {
@@ -101,6 +103,24 @@ static void l_rtnl_route_extract(const struct rtmsg *rtmsg, uint32_t len,
inet_ntop(family, RTA_DATA(attr), buf, sizeof(buf));
*src = l_strdup(buf);
+ break;
+ case RTA_TABLE:
+ if (!table)
+ break;
+
+ *table = *((uint32_t *) RTA_DATA(attr));
+ break;
+ case RTA_PRIORITY:
+ if (!priority)
+ break;
+
+ *priority = *((uint32_t *) RTA_DATA(attr));
+ break;
+ case RTA_PREF:
+ if (!pref)
+ break;
+
+ *pref = *((uint8_t *) RTA_DATA(attr));
break;
case RTA_OIF:
if (!ifindex)
@@ -335,10 +355,11 @@ uint32_t l_rtnl_ifaddr4_delete(struct l_netlink *rtnl, int ifindex,
}
void l_rtnl_route4_extract(const struct rtmsg *rtmsg, uint32_t len,
- uint32_t *ifindex, char **dst, char **gateway,
- char **src)
+ uint32_t *table, uint32_t *ifindex,
+ char **dst, char **gateway, char **src)
{
- l_rtnl_route_extract(rtmsg, len, AF_INET, ifindex, dst, gateway, src);
+ l_rtnl_route_extract(rtmsg, len, AF_INET, table, ifindex,
+ NULL, NULL, dst, gateway, src);
}
uint32_t l_rtnl_route4_dump(struct l_netlink *rtnl,
@@ -568,10 +589,11 @@ uint32_t l_rtnl_ifaddr6_delete(struct l_netlink *rtnl, int ifindex,
}
void l_rtnl_route6_extract(const struct rtmsg *rtmsg, uint32_t len,
- uint32_t *ifindex, char **dst, char **gateway,
- char **src)
+ uint32_t *table, uint32_t *ifindex,
+ char **dst, char **gateway, char **src)
{
- l_rtnl_route_extract(rtmsg, len, AF_INET6, ifindex, dst, gateway, src);
+ l_rtnl_route_extract(rtmsg, len, AF_INET6, table, ifindex,
+ NULL, NULL, dst, gateway, src);
}
uint32_t l_rtnl_route6_dump(struct l_netlink *rtnl,
diff --git a/ell/rtnl.h b/ell/rtnl.h
index 2be2eaee99e2..ea283c1c5a6e 100644
--- a/ell/rtnl.h
+++ b/ell/rtnl.h
@@ -63,8 +63,8 @@ uint32_t l_rtnl_ifaddr4_delete(struct l_netlink *rtnl, int ifindex,
l_netlink_destroy_func_t destroy);
void l_rtnl_route4_extract(const struct rtmsg *rtmsg, uint32_t len,
- uint32_t *ifindex, char **dst, char **gateway,
- char **src);
+ uint32_t *table, uint32_t *ifindex,
+ char **dst, char **gateway, char **src);
uint32_t l_rtnl_route4_dump(struct l_netlink *rtnl,
l_netlink_command_func_t cb, void *user_data,
l_netlink_destroy_func_t destroy);
@@ -98,8 +98,8 @@ uint32_t l_rtnl_ifaddr6_delete(struct l_netlink *rtnl, int ifindex,
void *user_data,
l_netlink_destroy_func_t destroy);
void l_rtnl_route6_extract(const struct rtmsg *rtmsg, uint32_t len,
- uint32_t *ifindex, char **dst, char **gateway,
- char **src);
+ uint32_t *table, uint32_t *ifindex,
+ char **dst, char **gateway, char **src);
uint32_t l_rtnl_route6_dump(struct l_netlink *rtnl,
l_netlink_command_func_t cb, void *user_data,
l_netlink_destroy_func_t destroy);
diff --git a/unit/test-rtnl.c b/unit/test-rtnl.c
index 37db73dc6f2f..107f69a76da1 100644
--- a/unit/test-rtnl.c
+++ b/unit/test-rtnl.c
@@ -114,14 +114,16 @@ static void route4_dump_cb(int error,
{
const struct rtmsg *rtmsg = data;
char *dst = NULL, *gateway = NULL, *src = NULL;
- uint32_t idx;
+ uint32_t table, ifindex;
test_assert(!error);
test_assert(type == RTM_NEWROUTE);
- l_rtnl_route4_extract(rtmsg, len, &idx, &dst, &gateway, &src);
+ l_rtnl_route4_extract(rtmsg, len, &table, &ifindex,
+ &dst, &gateway, &src);
- l_info("idx %d dst %s gateway %s src %s", idx, dst, gateway, src);
+ l_info("table %d ifindex %d dst %s gateway %s src %s",
+ table, ifindex, dst, gateway, src);
l_free(dst);
l_free(gateway);
@@ -145,14 +147,16 @@ static void route6_dump_cb(int error,
{
const struct rtmsg *rtmsg = data;
char *dst = NULL, *gateway = NULL, *src = NULL;
- uint32_t idx;
+ uint32_t table, ifindex;
test_assert(!error);
test_assert(type == RTM_NEWROUTE);
- l_rtnl_route6_extract(rtmsg, len, &idx, &dst, &gateway, &src);
+ l_rtnl_route6_extract(rtmsg, len, &table, &ifindex,
+ &dst, &gateway, &src);
- l_info("idx %d dst %s gateway %s src %s", idx, dst, gateway, src);
+ l_info("table %d ifindex %d dst %s gateway %s src %s",
+ table, ifindex, dst, gateway, src);
l_free(dst);
l_free(gateway);
--
2.25.0
^ permalink raw reply related
* [PATCH 2/3] unit: Add unit test for l_rtnl_route6_{dump|extract}
From: Daniel Wagner @ 2020-02-14 18:52 UTC (permalink / raw)
To: ell
In-Reply-To: <20200214185253.32658-1-wagi@monom.org>
[-- Attachment #1: Type: text/plain, Size: 1580 bytes --]
---
unit/test-rtnl.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/unit/test-rtnl.c b/unit/test-rtnl.c
index c0659c4378da..37db73dc6f2f 100644
--- a/unit/test-rtnl.c
+++ b/unit/test-rtnl.c
@@ -139,6 +139,37 @@ static void test_route4_dump(struct l_netlink *rtnl, void *user_data)
NULL, route4_dump_destroy_cb));
}
+static void route6_dump_cb(int error,
+ uint16_t type, const void *data,
+ uint32_t len, void *user_data)
+{
+ const struct rtmsg *rtmsg = data;
+ char *dst = NULL, *gateway = NULL, *src = NULL;
+ uint32_t idx;
+
+ test_assert(!error);
+ test_assert(type == RTM_NEWROUTE);
+
+ l_rtnl_route6_extract(rtmsg, len, &idx, &dst, &gateway, &src);
+
+ l_info("idx %d dst %s gateway %s src %s", idx, dst, gateway, src);
+
+ l_free(dst);
+ l_free(gateway);
+ l_free(src);
+}
+
+static void route6_dump_destroy_cb(void *user_data)
+{
+ test_next();
+}
+
+static void test_route6_dump(struct l_netlink *rtnl, void *user_data)
+{
+ test_assert(l_rtnl_route6_dump(rtnl, route6_dump_cb,
+ NULL, route6_dump_destroy_cb));
+}
+
static void ifaddr4_dump_cb(int error,
uint16_t type, const void *data,
uint32_t len, void *user_data)
@@ -211,6 +242,7 @@ int main(int argc, char *argv[])
return -1;
test_add("Dump IPv4 routing table", test_route4_dump, NULL);
+ test_add("Dump IPv6 routing table", test_route6_dump, NULL);
test_add("Dump IPv4 addresses", test_ifaddr4_dump, NULL);
test_add("Dump IPv6 addresses", test_ifaddr6_dump, NULL);
--
2.25.0
^ permalink raw reply related
* [PATCH 1/3] rtnl: Add l_rtnl_route6_dump() and l_rtnl_route6_extract()
From: Daniel Wagner @ 2020-02-14 18:52 UTC (permalink / raw)
To: ell
In-Reply-To: <20200214185253.32658-1-wagi@monom.org>
[-- Attachment #1: Type: text/plain, Size: 4421 bytes --]
Add missing function for handling IPv6. Now we for both protocol
versions the same functionality.
While at it, factor out the l_rtnl_route4_extract to use inet_ntop
which can handle both address families.
---
ell/rtnl.c | 104 +++++++++++++++++++++++++++++++++--------------------
ell/rtnl.h | 6 ++++
2 files changed, 72 insertions(+), 38 deletions(-)
diff --git a/ell/rtnl.c b/ell/rtnl.c
index ced9baab66b9..c94531144eb4 100644
--- a/ell/rtnl.c
+++ b/ell/rtnl.c
@@ -68,6 +68,50 @@ static size_t rta_add_data(void *rta_buf, unsigned short type, void *data,
return RTA_SPACE(data_len);
}
+static void l_rtnl_route_extract(const struct rtmsg *rtmsg, uint32_t len,
+ int family, uint32_t *ifindex, char **dst,
+ char **gateway, char **src)
+{
+ struct rtattr *attr;
+ char buf[INET6_ADDRSTRLEN];
+
+ for (attr = RTM_RTA(rtmsg); RTA_OK(attr, len);
+ attr = RTA_NEXT(attr, len)) {
+ switch (attr->rta_type) {
+ case RTA_DST:
+ if (!dst)
+ break;
+
+ inet_ntop(family, RTA_DATA(attr), buf, sizeof(buf));
+ *dst = l_strdup(buf);
+
+ break;
+ case RTA_GATEWAY:
+ if (!gateway)
+ break;
+
+ inet_ntop(family, RTA_DATA(attr), buf, sizeof(buf));
+ *gateway = l_strdup(buf);
+
+ break;
+ case RTA_PREFSRC:
+ if (!src)
+ break;
+
+ inet_ntop(family, RTA_DATA(attr), buf, sizeof(buf));
+ *src = l_strdup(buf);
+
+ break;
+ case RTA_OIF:
+ if (!ifindex)
+ break;
+
+ *ifindex = *((uint32_t *) RTA_DATA(attr));
+ break;
+ }
+ }
+}
+
uint32_t l_rtnl_set_linkmode_and_operstate(struct l_netlink *rtnl, int ifindex,
uint8_t linkmode, uint8_t operstate,
l_netlink_command_func_t cb,
@@ -294,44 +338,7 @@ void l_rtnl_route4_extract(const struct rtmsg *rtmsg, uint32_t len,
uint32_t *ifindex, char **dst, char **gateway,
char **src)
{
- struct in_addr in_addr;
- struct rtattr *attr;
-
- for (attr = RTM_RTA(rtmsg); RTA_OK(attr, len);
- attr = RTA_NEXT(attr, len)) {
- switch (attr->rta_type) {
- case RTA_DST:
- if (!dst)
- break;
-
- in_addr = *((struct in_addr *) RTA_DATA(attr));
- *dst = l_strdup(inet_ntoa(in_addr));
-
- break;
- case RTA_GATEWAY:
- if (!gateway)
- break;
-
- in_addr = *((struct in_addr *) RTA_DATA(attr));
- *gateway = l_strdup(inet_ntoa(in_addr));
-
- break;
- case RTA_PREFSRC:
- if (!src)
- break;
-
- in_addr = *((struct in_addr *) RTA_DATA(attr));
- *src = l_strdup(inet_ntoa(in_addr));
-
- break;
- case RTA_OIF:
- if (!ifindex)
- break;
-
- *ifindex = *((uint32_t *) RTA_DATA(attr));
- break;
- }
- }
+ l_rtnl_route_extract(rtmsg, len, AF_INET, ifindex, dst, gateway, src);
}
uint32_t l_rtnl_route4_dump(struct l_netlink *rtnl,
@@ -560,6 +567,27 @@ uint32_t l_rtnl_ifaddr6_delete(struct l_netlink *rtnl, int ifindex,
ip, cb, user_data, destroy);
}
+void l_rtnl_route6_extract(const struct rtmsg *rtmsg, uint32_t len,
+ uint32_t *ifindex, char **dst, char **gateway,
+ char **src)
+{
+ l_rtnl_route_extract(rtmsg, len, AF_INET6, ifindex, dst, gateway, src);
+}
+
+uint32_t l_rtnl_route6_dump(struct l_netlink *rtnl,
+ l_netlink_command_func_t cb, void *user_data,
+ l_netlink_destroy_func_t destroy)
+{
+ struct rtmsg rtmsg;
+
+ memset(&rtmsg, 0, sizeof(struct rtmsg));
+ rtmsg.rtm_family = AF_INET6;
+
+ return l_netlink_send(rtnl, RTM_GETROUTE, NLM_F_DUMP, &rtmsg,
+ sizeof(struct rtmsg), cb, user_data,
+ destroy);
+}
+
static uint32_t l_rtnl_route6_change(struct l_netlink *rtnl,
uint16_t nlmsg_type, int ifindex,
const char *gateway,
diff --git a/ell/rtnl.h b/ell/rtnl.h
index a6e6d1323015..2be2eaee99e2 100644
--- a/ell/rtnl.h
+++ b/ell/rtnl.h
@@ -97,6 +97,12 @@ uint32_t l_rtnl_ifaddr6_delete(struct l_netlink *rtnl, int ifindex,
l_netlink_command_func_t cb,
void *user_data,
l_netlink_destroy_func_t destroy);
+void l_rtnl_route6_extract(const struct rtmsg *rtmsg, uint32_t len,
+ uint32_t *ifindex, char **dst, char **gateway,
+ char **src);
+uint32_t l_rtnl_route6_dump(struct l_netlink *rtnl,
+ l_netlink_command_func_t cb, void *user_data,
+ l_netlink_destroy_func_t destroy);
uint32_t l_rtnl_route6_add_gateway(struct l_netlink *rtnl, int ifindex,
const char *gateway,
uint32_t priority_offset,
--
2.25.0
^ permalink raw reply related
* [PATCH 0/3] rtnl: add missing APIs
From: Daniel Wagner @ 2020-02-14 18:52 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 646 bytes --]
The first two patches make the API for both protocol version
complete. The last patch adds the table, priority and preference to
the extract functions, which is a very uselful information.
This series is based on the cleanup series send before.
Daniel Wagner (3):
rtnl: Add l_rtnl_route6_dump() and l_rtnl_route6_extract()
unit: Add unit test for l_rtnl_route6_{dump|extract}
rtnl: Extract table, priority and pref
ell/rtnl.c | 130 ++++++++++++++++++++++++++++++++---------------
ell/rtnl.h | 10 +++-
unit/test-rtnl.c | 42 +++++++++++++--
3 files changed, 137 insertions(+), 45 deletions(-)
--
2.25.0
^ permalink raw reply
* Re: [PATCH v5 7/8] multifd: Add multifd-zstd-level parameter
From: Dr. David Alan Gilbert @ 2020-02-14 18:50 UTC (permalink / raw)
To: Markus Armbruster
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Juan Quintela, qemu-devel, Paolo Bonzini
In-Reply-To: <87imk91rkx.fsf@dusky.pond.sub.org>
* Markus Armbruster (armbru@redhat.com) wrote:
> Juan Quintela <quintela@redhat.com> writes:
>
> > Markus Armbruster <armbru@redhat.com> wrote:
> >> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> >>
> >>> * Juan Quintela (quintela@redhat.com) wrote:
> >>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >>>> ---
> >>>> migration/migration.c | 15 +++++++++++++++
> >>>> monitor/hmp-cmds.c | 4 ++++
> >>>> qapi/migration.json | 29 ++++++++++++++++++++++++++---
> >>>> 3 files changed, 45 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/migration/migration.c b/migration/migration.c
> >>>> index 3b081e8147..b690500545 100644
> >>>> --- a/migration/migration.c
> >>>> +++ b/migration/migration.c
> >>>> @@ -91,6 +91,8 @@
> >>>> #define DEFAULT_MIGRATE_MULTIFD_METHOD MULTIFD_METHOD_NONE
> >>>> /*0: means nocompress, 1: best speed, ... 9: best compress ratio */
> >>>> #define DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL 1
> >>>> +/* 0: means nocompress, 1: best speed, ... 20: best compress ratio */
> >>>> +#define DEFAULT_MIGRATE_MULTIFD_ZSTD_LEVEL 1
> >>>>
> >>>> /* Background transfer rate for postcopy, 0 means unlimited, note
> >>>> * that page requests can still exceed this limit.
> >>>> @@ -805,6 +807,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
> >>>> params->multifd_method = s->parameters.multifd_method;
> >>>> params->has_multifd_zlib_level = true;
> >>>> params->multifd_zlib_level = s->parameters.multifd_zlib_level;
> >>>> + params->has_multifd_zstd_level = true;
> >>>> + params->multifd_zstd_level = s->parameters.multifd_zstd_level;
> >>>
> >>> Do we really want different 'multifd_...._level's or just one
> >>> 'multifd_compress_level' - or even just reuse the existing
> >>> 'compress-level' parameter.
> >>
> >> compress-level,
> >
> > possible values: 1 to 9 deprecated
> >
> >> multifd-zlib-level
> >
> > possible values: 1 to 9, default 1
> > (zlib default is -1, let the lib decide)
> >
> > , and multifd-zstd-level apply
> >
> > possible values: 1 to 19, default 1
> > (zstd default is 3)
> >
> >> "normal" live migration compression, multifd zlib live migration
> >> compression, and multifd zstd live migration compression, respectively.
> >>
> >> Any live migration can only use one of the three compressions.
> >>
> >> Correct?
> >
> > Yeap.
> >
> >>> The only tricky thing about combining them is how to handle
> >>> the difference in allowed ranges; When would the right time be
> >>> to check it?
> >>>
> >>> Markus/Eric: Any idea?
> >>
> >> To have an informed opinion, I'd have to dig through the migration
> >> code.
> >
> > Problem is _how_ to setup them. if we setup zstd compression method,
> > put the value at 19, and then setup zlib compression method, what should
> > we do?
> >
> > Truncate to 9?
> > Give one error?
> > Don't allow the zlib setup?
> >
> > Too complicated.
>
> The interface pretends the parameters are all independent: you get to
> set them one by one.
>
> They are in fact not independent, and this now leads to difficulties.
>
> To avoid them, the interface should let you specify a desired
> configuration all at once, and its implementation should then do what it
> takes to get from here to there.
>
> Example: current state is multifd compression method "zstd", compression
> level is 19. User wants to switch to method "zlib" level 9 the obvious
> way
>
> With old interface:
> step 1: set method to "zlib"
> step 2: set level to 9
> Problem: after step 1, we have method "zlib" with invalid level 19.
> Workaround: swap the steps.
>
> Note that the workaround only works because the set of levels both
> methods support is non-empty. We might still come up with more
> complicated parameter combinations where that is not the case.
>
> With new interface:
> set compression to "zlib" level 9
>
> The new interface require us to specify a QAPI type capable of holding
> the complete migration configuration.
>
> The stupid way is to throw all migration parameters into a struct, and
> make the ones optional that apply only when others have certain values.
> Thus, the "applies only when" bits are semantical, documented in
> comments, and enforced by C code.
I realised we already have that stupid struct! It's just
MigrationParameters - it has all the parameters as optional values,
and QMP's MigrateSetParameters already takes that - so you can already
provide both the type and the level at the same time; although there's
no semantic correlation between them.
Dave
> With a bit more care, we can make "applies only when" syntactical
> instead. Examples:
>
> @max-bandwidth and @downtime-limit always apply. They go straight
> into the struct.
>
> @tls-creds, @tls-hostname, @tls-authz apply only when TLS is enabled
> by setting @tls-creds. Have an optional member @tls, which is a
> struct with mandatory member @creds, optional members @hostname,
> @authz.
>
> @multifd-zlib-level applies when @multifd-method is "zlib", and
> @multifd-zstd-level applies when it's "zstd". Have a union
> @multifd-compression, cases "none", "zlib" and "zstd", where each
> case's members are the parameters applying in that case.
>
> Please note the purpose of these examples is to show how things can be
> done in the schema. I'm not trying to tell you how these specific
> things *should* be done.
>
> The resulting type is perfectly suited as return value of a query
> command. It's awkward as argument of a "specify desired configuration"
> command, because it requires the user to specify *complete*
> configuration. If we want to support omitting the parts of it we don't
> want to change, we have to make more members optional. Imprecise for
> the query, where we now have to specify "always present" in comments.
> Usually less bad than duplicating a complex type.
>
> >> Documentation of admissible range will become a bit awkward, too.
> >>
> >> Too many migration parameters...
> >
> > Sure, but the other option is taking a value and live with it.
> > I am all for leaving the library default and call it a day.
> >
> > Later, Juan.
>
> Hope this helps some.
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply
* Re: [PATCH 09/11] drm, cgroup: Introduce lgpu as DRM cgroup resource
From: Kenny Ho @ 2020-02-14 18:51 UTC (permalink / raw)
To: Daniel Vetter
Cc: juan.zuniga-anaya, Greathouse, Joseph, Kenny Ho, Kuehling, Felix,
jsparks, amd-gfx mailing list, lkaplan, Alex Deucher, nirmoy.das,
Maling list - DRI developers, Jason Ekstrand, Tejun Heo, cgroups,
Christian König, damon.mcdougall
In-Reply-To: <20200214183401.GY2363188@phenom.ffwll.local>
On Fri, Feb 14, 2020 at 1:34 PM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> I think guidance from Tejun in previos discussions was pretty clear that
> he expects cgroups to be both a) standardized and c) sufficient clear
> meaning that end-users have a clear understanding of what happens when
> they change the resource allocation.
>
> I'm not sure lgpu here, at least as specified, passes either.
I disagree (at least on the characterization of the feedback
provided.) I believe this series satisfied the sprite of Tejun's
guidance so far (the weight knob for lgpu, for example, was
specifically implemented base on his input.) But, I will let Tejun
speak for himself after he considered the implementation in detail.
Regards,
Kenny
> But I also
> don't have much clue, so pulled Jason in - he understands how this all
> gets reflected to userspace apis a lot better than me.
> -Daniel
>
>
> >
> > > If it's carving up compute power, what's actually being carved up? Time? Execution units/waves/threads? Even if that's the case, what advantage does it give to have it in terms of a fixed set of lgpus where each cgroup gets to pick a fixed set. Does affinity matter that much? Why not just say how many waves the GPU supports and that they have to be allocated in chunks of 16 waves (pulling a number out of thin air) and let the cgroup specify how many waves it wants.
> > >
> > > Don't get me wrong here. I'm all for the notion of being able to use cgroups to carve up GPU compute resources. However, this sounds to me like the most AMD-specific solution possible. We (Intel) could probably do some sort of carving up as well but we'd likely want to do it with preemption and time-slicing rather than handing out specific EUs.
> >
> > This has been discussed in the RFC before
> > (https://www.spinics.net/lists/cgroups/msg23469.html.) As mentioned
> > before, the idea of a compute unit is hardly an AMD specific thing as
> > it is in the OpenCL standard and part of the architecture of many
> > different vendors. In addition, the interface presented here supports
> > Intel's use case. What you described is what I considered as the
> > "anonymous resources" view of the lgpu. What you/Intel can do, is to
> > register your device to drmcg to have 100 lgpu and users can specify
> > simply by count. So if they want to allocate 5% for a cgroup, they
> > would set count=5. Per the documentation in this patch: "Some DRM
> > devices may only support lgpu as anonymous resources. In such case,
> > the significance of the position of the set bits in list will be
> > ignored." What Intel does with the user expressed configuration of "5
> > out of 100" is entirely up to Intel (time slice if you like, change to
> > specific EUs later if you like, or make it driver configurable to
> > support both if you like.)
> >
> > Regards,
> > Kenny
> >
> > >
> > > On Fri, Feb 14, 2020 at 9:57 AM Kenny Ho <Kenny.Ho@amd.com> wrote:
> > >>
> > >> drm.lgpu
> > >> A read-write nested-keyed file which exists on all cgroups.
> > >> Each entry is keyed by the DRM device's major:minor.
> > >>
> > >> lgpu stands for logical GPU, it is an abstraction used to
> > >> subdivide a physical DRM device for the purpose of resource
> > >> management. This file stores user configuration while the
> > >> drm.lgpu.effective reflects the actual allocation after
> > >> considering the relationship between the cgroups and their
> > >> configurations.
> > >>
> > >> The lgpu is a discrete quantity that is device specific (i.e.
> > >> some DRM devices may have 64 lgpus while others may have 100
> > >> lgpus.) The lgpu is a single quantity that can be allocated
> > >> in three different ways denoted by the following nested keys.
> > >>
> > >> ===== ==============================================
> > >> weight Allocate by proportion in relationship with
> > >> active sibling cgroups
> > >> count Allocate by amount statically, treat lgpu as
> > >> anonymous resources
> > >> list Allocate statically, treat lgpu as named
> > >> resource
> > >> ===== ==============================================
> > >>
> > >> For example:
> > >> 226:0 weight=100 count=256 list=0-255
> > >> 226:1 weight=100 count=4 list=0,2,4,6
> > >> 226:2 weight=100 count=32 list=32-63
> > >> 226:3 weight=100 count=0 list=
> > >> 226:4 weight=500 count=0 list=
> > >>
> > >> lgpu is represented by a bitmap and uses the bitmap_parselist
> > >> kernel function so the list key input format is a
> > >> comma-separated list of decimal numbers and ranges.
> > >>
> > >> Consecutively set bits are shown as two hyphen-separated decimal
> > >> numbers, the smallest and largest bit numbers set in the range.
> > >> Optionally each range can be postfixed to denote that only parts
> > >> of it should be set. The range will divided to groups of
> > >> specific size.
> > >> Syntax: range:used_size/group_size
> > >> Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
> > >>
> > >> The count key is the hamming weight / hweight of the bitmap.
> > >>
> > >> Weight, count and list accept the max and default keywords.
> > >>
> > >> Some DRM devices may only support lgpu as anonymous resources.
> > >> In such case, the significance of the position of the set bits
> > >> in list will be ignored.
> > >>
> > >> The weight quantity is only in effect when static allocation
> > >> is not used (by setting count=0) for this cgroup. The weight
> > >> quantity distributes lgpus that are not statically allocated by
> > >> the siblings. For example, given siblings cgroupA, cgroupB and
> > >> cgroupC for a DRM device that has 64 lgpus, if cgroupA occupies
> > >> 0-63, no lgpu is available to be distributed by weight.
> > >> Similarly, if cgroupA has list=0-31 and cgroupB has list=16-63,
> > >> cgroupC will be starved if it tries to allocate by weight.
> > >>
> > >> On the other hand, if cgroupA has weight=100 count=0, cgroupB
> > >> has list=16-47, and cgroupC has weight=100 count=0, then 32
> > >> lgpus are available to be distributed evenly between cgroupA
> > >> and cgroupC. In drm.lgpu.effective, cgroupA will have
> > >> list=0-15 and cgroupC will have list=48-63.
> > >>
> > >> This lgpu resource supports the 'allocation' and 'weight'
> > >> resource distribution model.
> > >>
> > >> drm.lgpu.effective
> > >> A read-only nested-keyed file which exists on all cgroups.
> > >> Each entry is keyed by the DRM device's major:minor.
> > >>
> > >> lgpu stands for logical GPU, it is an abstraction used to
> > >> subdivide a physical DRM device for the purpose of resource
> > >> management. This file reflects the actual allocation after
> > >> considering the relationship between the cgroups and their
> > >> configurations in drm.lgpu.
> > >>
> > >> Change-Id: Idde0ef9a331fd67bb9c7eb8ef9978439e6452488
> > >> Signed-off-by: Kenny Ho <Kenny.Ho@amd.com>
> > >> ---
> > >> Documentation/admin-guide/cgroup-v2.rst | 80 ++++++
> > >> include/drm/drm_cgroup.h | 3 +
> > >> include/linux/cgroup_drm.h | 22 ++
> > >> kernel/cgroup/drm.c | 324 +++++++++++++++++++++++-
> > >> 4 files changed, 427 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> > >> index ce5dc027366a..d8a41956e5c7 100644
> > >> --- a/Documentation/admin-guide/cgroup-v2.rst
> > >> +++ b/Documentation/admin-guide/cgroup-v2.rst
> > >> @@ -2120,6 +2120,86 @@ DRM Interface Files
> > >> Set largest allocation for /dev/dri/card1 to 4MB
> > >> echo "226:1 4m" > drm.buffer.peak.max
> > >>
> > >> + drm.lgpu
> > >> + A read-write nested-keyed file which exists on all cgroups.
> > >> + Each entry is keyed by the DRM device's major:minor.
> > >> +
> > >> + lgpu stands for logical GPU, it is an abstraction used to
> > >> + subdivide a physical DRM device for the purpose of resource
> > >> + management. This file stores user configuration while the
> > >> + drm.lgpu.effective reflects the actual allocation after
> > >> + considering the relationship between the cgroups and their
> > >> + configurations.
> > >> +
> > >> + The lgpu is a discrete quantity that is device specific (i.e.
> > >> + some DRM devices may have 64 lgpus while others may have 100
> > >> + lgpus.) The lgpu is a single quantity that can be allocated
> > >> + in three different ways denoted by the following nested keys.
> > >> +
> > >> + ===== ==============================================
> > >> + weight Allocate by proportion in relationship with
> > >> + active sibling cgroups
> > >> + count Allocate by amount statically, treat lgpu as
> > >> + anonymous resources
> > >> + list Allocate statically, treat lgpu as named
> > >> + resource
> > >> + ===== ==============================================
> > >> +
> > >> + For example:
> > >> + 226:0 weight=100 count=256 list=0-255
> > >> + 226:1 weight=100 count=4 list=0,2,4,6
> > >> + 226:2 weight=100 count=32 list=32-63
> > >> + 226:3 weight=100 count=0 list=
> > >> + 226:4 weight=500 count=0 list=
> > >> +
> > >> + lgpu is represented by a bitmap and uses the bitmap_parselist
> > >> + kernel function so the list key input format is a
> > >> + comma-separated list of decimal numbers and ranges.
> > >> +
> > >> + Consecutively set bits are shown as two hyphen-separated decimal
> > >> + numbers, the smallest and largest bit numbers set in the range.
> > >> + Optionally each range can be postfixed to denote that only parts
> > >> + of it should be set. The range will divided to groups of
> > >> + specific size.
> > >> + Syntax: range:used_size/group_size
> > >> + Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
> > >> +
> > >> + The count key is the hamming weight / hweight of the bitmap.
> > >> +
> > >> + Weight, count and list accept the max and default keywords.
> > >> +
> > >> + Some DRM devices may only support lgpu as anonymous resources.
> > >> + In such case, the significance of the position of the set bits
> > >> + in list will be ignored.
> > >> +
> > >> + The weight quantity is only in effect when static allocation
> > >> + is not used (by setting count=0) for this cgroup. The weight
> > >> + quantity distributes lgpus that are not statically allocated by
> > >> + the siblings. For example, given siblings cgroupA, cgroupB and
> > >> + cgroupC for a DRM device that has 64 lgpus, if cgroupA occupies
> > >> + 0-63, no lgpu is available to be distributed by weight.
> > >> + Similarly, if cgroupA has list=0-31 and cgroupB has list=16-63,
> > >> + cgroupC will be starved if it tries to allocate by weight.
> > >> +
> > >> + On the other hand, if cgroupA has weight=100 count=0, cgroupB
> > >> + has list=16-47, and cgroupC has weight=100 count=0, then 32
> > >> + lgpus are available to be distributed evenly between cgroupA
> > >> + and cgroupC. In drm.lgpu.effective, cgroupA will have
> > >> + list=0-15 and cgroupC will have list=48-63.
> > >> +
> > >> + This lgpu resource supports the 'allocation' and 'weight'
> > >> + resource distribution model.
> > >> +
> > >> + drm.lgpu.effective
> > >> + A read-only nested-keyed file which exists on all cgroups.
> > >> + Each entry is keyed by the DRM device's major:minor.
> > >> +
> > >> + lgpu stands for logical GPU, it is an abstraction used to
> > >> + subdivide a physical DRM device for the purpose of resource
> > >> + management. This file reflects the actual allocation after
> > >> + considering the relationship between the cgroups and their
> > >> + configurations in drm.lgpu.
> > >> +
> > >> GEM Buffer Ownership
> > >> ~~~~~~~~~~~~~~~~~~~~
> > >>
> > >> diff --git a/include/drm/drm_cgroup.h b/include/drm/drm_cgroup.h
> > >> index 2b41d4d22e33..619a110cc748 100644
> > >> --- a/include/drm/drm_cgroup.h
> > >> +++ b/include/drm/drm_cgroup.h
> > >> @@ -17,6 +17,9 @@ struct drmcg_props {
> > >>
> > >> s64 bo_limits_total_allocated_default;
> > >> s64 bo_limits_peak_allocated_default;
> > >> +
> > >> + int lgpu_capacity;
> > >> + DECLARE_BITMAP(lgpu_slots, MAX_DRMCG_LGPU_CAPACITY);
> > >> };
> > >>
> > >> void drmcg_bind(struct drm_minor (*(*acq_dm)(unsigned int minor_id)),
> > >> diff --git a/include/linux/cgroup_drm.h b/include/linux/cgroup_drm.h
> > >> index eae400f3d9b4..bb09704e7f71 100644
> > >> --- a/include/linux/cgroup_drm.h
> > >> +++ b/include/linux/cgroup_drm.h
> > >> @@ -11,10 +11,14 @@
> > >> /* limit defined per the way drm_minor_alloc operates */
> > >> #define MAX_DRM_DEV (64 * DRM_MINOR_RENDER)
> > >>
> > >> +#define MAX_DRMCG_LGPU_CAPACITY 256
> > >> +
> > >> enum drmcg_res_type {
> > >> DRMCG_TYPE_BO_TOTAL,
> > >> DRMCG_TYPE_BO_PEAK,
> > >> DRMCG_TYPE_BO_COUNT,
> > >> + DRMCG_TYPE_LGPU,
> > >> + DRMCG_TYPE_LGPU_EFF,
> > >> __DRMCG_TYPE_LAST,
> > >> };
> > >>
> > >> @@ -32,6 +36,24 @@ struct drmcg_device_resource {
> > >> s64 bo_limits_peak_allocated;
> > >>
> > >> s64 bo_stats_count_allocated;
> > >> +
> > >> + /**
> > >> + * Logical GPU
> > >> + *
> > >> + * *_cfg are properties configured by users
> > >> + * *_eff are the effective properties being applied to the hardware
> > >> + * *_stg is used to calculate _eff before applying to _eff
> > >> + * after considering the entire hierarchy
> > >> + */
> > >> + DECLARE_BITMAP(lgpu_stg, MAX_DRMCG_LGPU_CAPACITY);
> > >> + /* user configurations */
> > >> + s64 lgpu_weight_cfg;
> > >> + DECLARE_BITMAP(lgpu_cfg, MAX_DRMCG_LGPU_CAPACITY);
> > >> + /* effective lgpu for the cgroup after considering
> > >> + * relationship with other cgroup
> > >> + */
> > >> + s64 lgpu_count_eff;
> > >> + DECLARE_BITMAP(lgpu_eff, MAX_DRMCG_LGPU_CAPACITY);
> > >> };
> > >>
> > >> /**
> > >> diff --git a/kernel/cgroup/drm.c b/kernel/cgroup/drm.c
> > >> index 5fcbbc13fa1c..a4e88a3704bb 100644
> > >> --- a/kernel/cgroup/drm.c
> > >> +++ b/kernel/cgroup/drm.c
> > >> @@ -9,6 +9,7 @@
> > >> #include <linux/seq_file.h>
> > >> #include <linux/mutex.h>
> > >> #include <linux/kernel.h>
> > >> +#include <linux/bitmap.h>
> > >> #include <linux/cgroup_drm.h>
> > >> #include <drm/drm_file.h>
> > >> #include <drm/drm_drv.h>
> > >> @@ -41,6 +42,10 @@ enum drmcg_file_type {
> > >> DRMCG_FTYPE_DEFAULT,
> > >> };
> > >>
> > >> +#define LGPU_LIMITS_NAME_LIST "list"
> > >> +#define LGPU_LIMITS_NAME_COUNT "count"
> > >> +#define LGPU_LIMITS_NAME_WEIGHT "weight"
> > >> +
> > >> /**
> > >> * drmcg_bind - Bind DRM subsystem to cgroup subsystem
> > >> * @acq_dm: function pointer to the drm_minor_acquire function
> > >> @@ -98,6 +103,13 @@ static inline int init_drmcg_single(struct drmcg *drmcg, struct drm_device *dev)
> > >> ddr->bo_limits_peak_allocated =
> > >> dev->drmcg_props.bo_limits_peak_allocated_default;
> > >>
> > >> + bitmap_copy(ddr->lgpu_cfg, dev->drmcg_props.lgpu_slots,
> > >> + MAX_DRMCG_LGPU_CAPACITY);
> > >> + bitmap_copy(ddr->lgpu_stg, dev->drmcg_props.lgpu_slots,
> > >> + MAX_DRMCG_LGPU_CAPACITY);
> > >> +
> > >> + ddr->lgpu_weight_cfg = CGROUP_WEIGHT_DFL;
> > >> +
> > >> return 0;
> > >> }
> > >>
> > >> @@ -121,6 +133,120 @@ static inline void drmcg_update_cg_tree(struct drm_device *dev)
> > >> mutex_unlock(&cgroup_mutex);
> > >> }
> > >>
> > >> +static void drmcg_calculate_effective_lgpu(struct drm_device *dev,
> > >> + const unsigned long *free_static,
> > >> + const unsigned long *free_weighted,
> > >> + struct drmcg *parent_drmcg)
> > >> +{
> > >> + int capacity = dev->drmcg_props.lgpu_capacity;
> > >> + DECLARE_BITMAP(lgpu_unused, MAX_DRMCG_LGPU_CAPACITY);
> > >> + DECLARE_BITMAP(lgpu_by_weight, MAX_DRMCG_LGPU_CAPACITY);
> > >> + struct drmcg_device_resource *parent_ddr;
> > >> + struct drmcg_device_resource *ddr;
> > >> + int minor = dev->primary->index;
> > >> + struct cgroup_subsys_state *pos;
> > >> + struct drmcg *child;
> > >> + s64 weight_sum = 0;
> > >> + s64 unused;
> > >> +
> > >> + parent_ddr = parent_drmcg->dev_resources[minor];
> > >> +
> > >> + if (bitmap_empty(parent_ddr->lgpu_cfg, capacity))
> > >> + /* no static cfg, use weight for calculating the effective */
> > >> + bitmap_copy(parent_ddr->lgpu_stg, free_weighted, capacity);
> > >> + else
> > >> + /* lgpu statically configured, use the overlap as effective */
> > >> + bitmap_and(parent_ddr->lgpu_stg, free_static,
> > >> + parent_ddr->lgpu_cfg, capacity);
> > >> +
> > >> + /* calculate lgpu available for distribution by weight for children */
> > >> + bitmap_copy(lgpu_unused, parent_ddr->lgpu_stg, capacity);
> > >> + css_for_each_child(pos, &parent_drmcg->css) {
> > >> + child = css_to_drmcg(pos);
> > >> + ddr = child->dev_resources[minor];
> > >> +
> > >> + if (bitmap_empty(ddr->lgpu_cfg, capacity))
> > >> + /* no static allocation, participate in weight dist */
> > >> + weight_sum += ddr->lgpu_weight_cfg;
> > >> + else
> > >> + /* take out statically allocated lgpu by siblings */
> > >> + bitmap_andnot(lgpu_unused, lgpu_unused, ddr->lgpu_cfg,
> > >> + capacity);
> > >> + }
> > >> +
> > >> + unused = bitmap_weight(lgpu_unused, capacity);
> > >> +
> > >> + css_for_each_child(pos, &parent_drmcg->css) {
> > >> + child = css_to_drmcg(pos);
> > >> + ddr = child->dev_resources[minor];
> > >> +
> > >> + bitmap_zero(lgpu_by_weight, capacity);
> > >> + /* no static allocation, participate in weight distribution */
> > >> + if (bitmap_empty(ddr->lgpu_cfg, capacity)) {
> > >> + int c;
> > >> + int p = 0;
> > >> +
> > >> + for (c = ddr->lgpu_weight_cfg * unused / weight_sum;
> > >> + c > 0; c--) {
> > >> + p = find_next_bit(lgpu_unused, capacity, p);
> > >> + if (p < capacity) {
> > >> + clear_bit(p, lgpu_unused);
> > >> + set_bit(p, lgpu_by_weight);
> > >> + }
> > >> + }
> > >> +
> > >> + }
> > >> +
> > >> + drmcg_calculate_effective_lgpu(dev, parent_ddr->lgpu_stg,
> > >> + lgpu_by_weight, child);
> > >> + }
> > >> +}
> > >> +
> > >> +static void drmcg_apply_effective_lgpu(struct drm_device *dev)
> > >> +{
> > >> + int capacity = dev->drmcg_props.lgpu_capacity;
> > >> + int minor = dev->primary->index;
> > >> + struct drmcg_device_resource *ddr;
> > >> + struct cgroup_subsys_state *pos;
> > >> + struct drmcg *drmcg;
> > >> +
> > >> + if (root_drmcg == NULL) {
> > >> + WARN_ON(root_drmcg == NULL);
> > >> + return;
> > >> + }
> > >> +
> > >> + rcu_read_lock();
> > >> +
> > >> + /* process the entire cgroup tree from root to simplify the algorithm */
> > >> + drmcg_calculate_effective_lgpu(dev, dev->drmcg_props.lgpu_slots,
> > >> + dev->drmcg_props.lgpu_slots, root_drmcg);
> > >> +
> > >> + /* apply changes to effective only if there is a change */
> > >> + css_for_each_descendant_pre(pos, &root_drmcg->css) {
> > >> + drmcg = css_to_drmcg(pos);
> > >> + ddr = drmcg->dev_resources[minor];
> > >> +
> > >> + if (!bitmap_equal(ddr->lgpu_stg, ddr->lgpu_eff, capacity)) {
> > >> + bitmap_copy(ddr->lgpu_eff, ddr->lgpu_stg, capacity);
> > >> + ddr->lgpu_count_eff =
> > >> + bitmap_weight(ddr->lgpu_eff, capacity);
> > >> + }
> > >> + }
> > >> + rcu_read_unlock();
> > >> +}
> > >> +
> > >> +static void drmcg_apply_effective(enum drmcg_res_type type,
> > >> + struct drm_device *dev, struct drmcg *changed_drmcg)
> > >> +{
> > >> + switch (type) {
> > >> + case DRMCG_TYPE_LGPU:
> > >> + drmcg_apply_effective_lgpu(dev);
> > >> + break;
> > >> + default:
> > >> + break;
> > >> + }
> > >> +}
> > >> +
> > >> /**
> > >> * drmcg_register_dev - register a DRM device for usage in drm cgroup
> > >> * @dev: DRM device
> > >> @@ -143,7 +269,13 @@ void drmcg_register_dev(struct drm_device *dev)
> > >> {
> > >> dev->driver->drmcg_custom_init(dev, &dev->drmcg_props);
> > >>
> > >> + WARN_ON(dev->drmcg_props.lgpu_capacity !=
> > >> + bitmap_weight(dev->drmcg_props.lgpu_slots,
> > >> + MAX_DRMCG_LGPU_CAPACITY));
> > >> +
> > >> drmcg_update_cg_tree(dev);
> > >> +
> > >> + drmcg_apply_effective(DRMCG_TYPE_LGPU, dev, root_drmcg);
> > >> }
> > >> mutex_unlock(&drmcg_mutex);
> > >> }
> > >> @@ -297,7 +429,8 @@ static void drmcg_print_stats(struct drmcg_device_resource *ddr,
> > >> }
> > >>
> > >> static void drmcg_print_limits(struct drmcg_device_resource *ddr,
> > >> - struct seq_file *sf, enum drmcg_res_type type)
> > >> + struct seq_file *sf, enum drmcg_res_type type,
> > >> + struct drm_device *dev)
> > >> {
> > >> if (ddr == NULL) {
> > >> seq_puts(sf, "\n");
> > >> @@ -311,6 +444,25 @@ static void drmcg_print_limits(struct drmcg_device_resource *ddr,
> > >> case DRMCG_TYPE_BO_PEAK:
> > >> seq_printf(sf, "%lld\n", ddr->bo_limits_peak_allocated);
> > >> break;
> > >> + case DRMCG_TYPE_LGPU:
> > >> + seq_printf(sf, "%s=%lld %s=%d %s=%*pbl\n",
> > >> + LGPU_LIMITS_NAME_WEIGHT,
> > >> + ddr->lgpu_weight_cfg,
> > >> + LGPU_LIMITS_NAME_COUNT,
> > >> + bitmap_weight(ddr->lgpu_cfg,
> > >> + dev->drmcg_props.lgpu_capacity),
> > >> + LGPU_LIMITS_NAME_LIST,
> > >> + dev->drmcg_props.lgpu_capacity,
> > >> + ddr->lgpu_cfg);
> > >> + break;
> > >> + case DRMCG_TYPE_LGPU_EFF:
> > >> + seq_printf(sf, "%s=%lld %s=%*pbl\n",
> > >> + LGPU_LIMITS_NAME_COUNT,
> > >> + ddr->lgpu_count_eff,
> > >> + LGPU_LIMITS_NAME_LIST,
> > >> + dev->drmcg_props.lgpu_capacity,
> > >> + ddr->lgpu_eff);
> > >> + break;
> > >> default:
> > >> seq_puts(sf, "\n");
> > >> break;
> > >> @@ -329,6 +481,17 @@ static void drmcg_print_default(struct drmcg_props *props,
> > >> seq_printf(sf, "%lld\n",
> > >> props->bo_limits_peak_allocated_default);
> > >> break;
> > >> + case DRMCG_TYPE_LGPU:
> > >> + seq_printf(sf, "%s=%d %s=%d %s=%*pbl\n",
> > >> + LGPU_LIMITS_NAME_WEIGHT,
> > >> + CGROUP_WEIGHT_DFL,
> > >> + LGPU_LIMITS_NAME_COUNT,
> > >> + bitmap_weight(props->lgpu_slots,
> > >> + props->lgpu_capacity),
> > >> + LGPU_LIMITS_NAME_LIST,
> > >> + props->lgpu_capacity,
> > >> + props->lgpu_slots);
> > >> + break;
> > >> default:
> > >> seq_puts(sf, "\n");
> > >> break;
> > >> @@ -358,7 +521,7 @@ static int drmcg_seq_show_fn(int id, void *ptr, void *data)
> > >> drmcg_print_stats(ddr, sf, type);
> > >> break;
> > >> case DRMCG_FTYPE_LIMIT:
> > >> - drmcg_print_limits(ddr, sf, type);
> > >> + drmcg_print_limits(ddr, sf, type, minor->dev);
> > >> break;
> > >> case DRMCG_FTYPE_DEFAULT:
> > >> drmcg_print_default(&minor->dev->drmcg_props, sf, type);
> > >> @@ -415,6 +578,115 @@ static int drmcg_process_limit_s64_val(char *sval, bool is_mem,
> > >> return rc;
> > >> }
> > >>
> > >> +static void drmcg_nested_limit_parse(struct kernfs_open_file *of,
> > >> + struct drm_device *dev, char *attrs)
> > >> +{
> > >> + DECLARE_BITMAP(tmp_bitmap, MAX_DRMCG_LGPU_CAPACITY);
> > >> + DECLARE_BITMAP(chk_bitmap, MAX_DRMCG_LGPU_CAPACITY);
> > >> + enum drmcg_res_type type =
> > >> + DRMCG_CTF_PRIV2RESTYPE(of_cft(of)->private);
> > >> + struct drmcg *drmcg = css_to_drmcg(of_css(of));
> > >> + struct drmcg_props *props = &dev->drmcg_props;
> > >> + char *cft_name = of_cft(of)->name;
> > >> + int minor = dev->primary->index;
> > >> + char *nested = strstrip(attrs);
> > >> + struct drmcg_device_resource *ddr =
> > >> + drmcg->dev_resources[minor];
> > >> + char *attr;
> > >> + char sname[256];
> > >> + char sval[256];
> > >> + s64 val;
> > >> + int rc;
> > >> +
> > >> + while (nested != NULL) {
> > >> + attr = strsep(&nested, " ");
> > >> +
> > >> + if (sscanf(attr, "%255[^=]=%255[^=]", sname, sval) != 2)
> > >> + continue;
> > >> +
> > >> + switch (type) {
> > >> + case DRMCG_TYPE_LGPU:
> > >> + if (strncmp(sname, LGPU_LIMITS_NAME_LIST, 256) &&
> > >> + strncmp(sname, LGPU_LIMITS_NAME_COUNT, 256) &&
> > >> + strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256))
> > >> + continue;
> > >> +
> > >> + if (strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256) &&
> > >> + (!strcmp("max", sval) ||
> > >> + !strcmp("default", sval))) {
> > >> + bitmap_copy(ddr->lgpu_cfg, props->lgpu_slots,
> > >> + props->lgpu_capacity);
> > >> +
> > >> + continue;
> > >> + }
> > >> +
> > >> + if (strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256) == 0) {
> > >> + rc = drmcg_process_limit_s64_val(sval,
> > >> + false, CGROUP_WEIGHT_DFL,
> > >> + CGROUP_WEIGHT_MAX, &val);
> > >> +
> > >> + if (rc || val < CGROUP_WEIGHT_MIN ||
> > >> + val > CGROUP_WEIGHT_MAX) {
> > >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> > >> + minor);
> > >> + continue;
> > >> + }
> > >> +
> > >> + ddr->lgpu_weight_cfg = val;
> > >> + continue;
> > >> + }
> > >> +
> > >> + if (strncmp(sname, LGPU_LIMITS_NAME_COUNT, 256) == 0) {
> > >> + rc = drmcg_process_limit_s64_val(sval,
> > >> + false, props->lgpu_capacity,
> > >> + props->lgpu_capacity, &val);
> > >> +
> > >> + if (rc || val < 0) {
> > >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> > >> + minor);
> > >> + continue;
> > >> + }
> > >> +
> > >> + bitmap_zero(tmp_bitmap,
> > >> + MAX_DRMCG_LGPU_CAPACITY);
> > >> + bitmap_set(tmp_bitmap, 0, val);
> > >> + }
> > >> +
> > >> + if (strncmp(sname, LGPU_LIMITS_NAME_LIST, 256) == 0) {
> > >> + rc = bitmap_parselist(sval, tmp_bitmap,
> > >> + MAX_DRMCG_LGPU_CAPACITY);
> > >> +
> > >> + if (rc) {
> > >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> > >> + minor);
> > >> + continue;
> > >> + }
> > >> +
> > >> + bitmap_andnot(chk_bitmap, tmp_bitmap,
> > >> + props->lgpu_slots,
> > >> + MAX_DRMCG_LGPU_CAPACITY);
> > >> +
> > >> + /* user setting does not intersect with
> > >> + * available lgpu */
> > >> + if (!bitmap_empty(chk_bitmap,
> > >> + MAX_DRMCG_LGPU_CAPACITY)) {
> > >> + drmcg_pr_cft_err(drmcg, 0, cft_name,
> > >> + minor);
> > >> + continue;
> > >> + }
> > >> + }
> > >> +
> > >> + bitmap_copy(ddr->lgpu_cfg, tmp_bitmap,
> > >> + props->lgpu_capacity);
> > >> +
> > >> + break; /* DRMCG_TYPE_LGPU */
> > >> + default:
> > >> + break;
> > >> + } /* switch (type) */
> > >> + }
> > >> +}
> > >> +
> > >> +
> > >> /**
> > >> * drmcg_limit_write - parse cgroup interface files to obtain user config
> > >> *
> > >> @@ -499,9 +771,15 @@ static ssize_t drmcg_limit_write(struct kernfs_open_file *of, char *buf,
> > >>
> > >> ddr->bo_limits_peak_allocated = val;
> > >> break;
> > >> + case DRMCG_TYPE_LGPU:
> > >> + drmcg_nested_limit_parse(of, dm->dev, sattr);
> > >> + break;
> > >> default:
> > >> break;
> > >> }
> > >> +
> > >> + drmcg_apply_effective(type, dm->dev, drmcg);
> > >> +
> > >> mutex_unlock(&dm->dev->drmcg_mutex);
> > >>
> > >> mutex_lock(&drmcg_mutex);
> > >> @@ -560,12 +838,51 @@ struct cftype files[] = {
> > >> .private = DRMCG_CTF_PRIV(DRMCG_TYPE_BO_COUNT,
> > >> DRMCG_FTYPE_STATS),
> > >> },
> > >> + {
> > >> + .name = "lgpu",
> > >> + .seq_show = drmcg_seq_show,
> > >> + .write = drmcg_limit_write,
> > >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU,
> > >> + DRMCG_FTYPE_LIMIT),
> > >> + },
> > >> + {
> > >> + .name = "lgpu.default",
> > >> + .seq_show = drmcg_seq_show,
> > >> + .flags = CFTYPE_ONLY_ON_ROOT,
> > >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU,
> > >> + DRMCG_FTYPE_DEFAULT),
> > >> + },
> > >> + {
> > >> + .name = "lgpu.effective",
> > >> + .seq_show = drmcg_seq_show,
> > >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU_EFF,
> > >> + DRMCG_FTYPE_LIMIT),
> > >> + },
> > >> { } /* terminate */
> > >> };
> > >>
> > >> +static int drmcg_online_fn(int id, void *ptr, void *data)
> > >> +{
> > >> + struct drm_minor *minor = ptr;
> > >> + struct drmcg *drmcg = data;
> > >> +
> > >> + if (minor->type != DRM_MINOR_PRIMARY)
> > >> + return 0;
> > >> +
> > >> + drmcg_apply_effective(DRMCG_TYPE_LGPU, minor->dev, drmcg);
> > >> +
> > >> + return 0;
> > >> +}
> > >> +
> > >> +static int drmcg_css_online(struct cgroup_subsys_state *css)
> > >> +{
> > >> + return drm_minor_for_each(&drmcg_online_fn, css_to_drmcg(css));
> > >> +}
> > >> +
> > >> struct cgroup_subsys drm_cgrp_subsys = {
> > >> .css_alloc = drmcg_css_alloc,
> > >> .css_free = drmcg_css_free,
> > >> + .css_online = drmcg_css_online,
> > >> .early_init = false,
> > >> .legacy_cftypes = files,
> > >> .dfl_cftypes = files,
> > >> @@ -585,6 +902,9 @@ void drmcg_device_early_init(struct drm_device *dev)
> > >> dev->drmcg_props.bo_limits_total_allocated_default = S64_MAX;
> > >> dev->drmcg_props.bo_limits_peak_allocated_default = S64_MAX;
> > >>
> > >> + dev->drmcg_props.lgpu_capacity = MAX_DRMCG_LGPU_CAPACITY;
> > >> + bitmap_fill(dev->drmcg_props.lgpu_slots, MAX_DRMCG_LGPU_CAPACITY);
> > >> +
> > >> drmcg_update_cg_tree(dev);
> > >> }
> > >> EXPORT_SYMBOL(drmcg_device_early_init);
> > >> --
> > >> 2.25.0
> > >>
> > >> _______________________________________________
> > >> dri-devel mailing list
> > >> dri-devel@lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH 09/11] drm, cgroup: Introduce lgpu as DRM cgroup resource
From: Kenny Ho @ 2020-02-14 18:51 UTC (permalink / raw)
To: Daniel Vetter
Cc: juan.zuniga-anaya, Greathouse, Joseph, Kenny Ho, Kuehling, Felix,
jsparks, amd-gfx mailing list, lkaplan, Alex Deucher, nirmoy.das,
Maling list - DRI developers, Jason Ekstrand, Tejun Heo, cgroups,
Christian König, damon.mcdougall
In-Reply-To: <20200214183401.GY2363188@phenom.ffwll.local>
On Fri, Feb 14, 2020 at 1:34 PM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> I think guidance from Tejun in previos discussions was pretty clear that
> he expects cgroups to be both a) standardized and c) sufficient clear
> meaning that end-users have a clear understanding of what happens when
> they change the resource allocation.
>
> I'm not sure lgpu here, at least as specified, passes either.
I disagree (at least on the characterization of the feedback
provided.) I believe this series satisfied the sprite of Tejun's
guidance so far (the weight knob for lgpu, for example, was
specifically implemented base on his input.) But, I will let Tejun
speak for himself after he considered the implementation in detail.
Regards,
Kenny
> But I also
> don't have much clue, so pulled Jason in - he understands how this all
> gets reflected to userspace apis a lot better than me.
> -Daniel
>
>
> >
> > > If it's carving up compute power, what's actually being carved up? Time? Execution units/waves/threads? Even if that's the case, what advantage does it give to have it in terms of a fixed set of lgpus where each cgroup gets to pick a fixed set. Does affinity matter that much? Why not just say how many waves the GPU supports and that they have to be allocated in chunks of 16 waves (pulling a number out of thin air) and let the cgroup specify how many waves it wants.
> > >
> > > Don't get me wrong here. I'm all for the notion of being able to use cgroups to carve up GPU compute resources. However, this sounds to me like the most AMD-specific solution possible. We (Intel) could probably do some sort of carving up as well but we'd likely want to do it with preemption and time-slicing rather than handing out specific EUs.
> >
> > This has been discussed in the RFC before
> > (https://www.spinics.net/lists/cgroups/msg23469.html.) As mentioned
> > before, the idea of a compute unit is hardly an AMD specific thing as
> > it is in the OpenCL standard and part of the architecture of many
> > different vendors. In addition, the interface presented here supports
> > Intel's use case. What you described is what I considered as the
> > "anonymous resources" view of the lgpu. What you/Intel can do, is to
> > register your device to drmcg to have 100 lgpu and users can specify
> > simply by count. So if they want to allocate 5% for a cgroup, they
> > would set count=5. Per the documentation in this patch: "Some DRM
> > devices may only support lgpu as anonymous resources. In such case,
> > the significance of the position of the set bits in list will be
> > ignored." What Intel does with the user expressed configuration of "5
> > out of 100" is entirely up to Intel (time slice if you like, change to
> > specific EUs later if you like, or make it driver configurable to
> > support both if you like.)
> >
> > Regards,
> > Kenny
> >
> > >
> > > On Fri, Feb 14, 2020 at 9:57 AM Kenny Ho <Kenny.Ho@amd.com> wrote:
> > >>
> > >> drm.lgpu
> > >> A read-write nested-keyed file which exists on all cgroups.
> > >> Each entry is keyed by the DRM device's major:minor.
> > >>
> > >> lgpu stands for logical GPU, it is an abstraction used to
> > >> subdivide a physical DRM device for the purpose of resource
> > >> management. This file stores user configuration while the
> > >> drm.lgpu.effective reflects the actual allocation after
> > >> considering the relationship between the cgroups and their
> > >> configurations.
> > >>
> > >> The lgpu is a discrete quantity that is device specific (i.e.
> > >> some DRM devices may have 64 lgpus while others may have 100
> > >> lgpus.) The lgpu is a single quantity that can be allocated
> > >> in three different ways denoted by the following nested keys.
> > >>
> > >> ===== ==============================================
> > >> weight Allocate by proportion in relationship with
> > >> active sibling cgroups
> > >> count Allocate by amount statically, treat lgpu as
> > >> anonymous resources
> > >> list Allocate statically, treat lgpu as named
> > >> resource
> > >> ===== ==============================================
> > >>
> > >> For example:
> > >> 226:0 weight=100 count=256 list=0-255
> > >> 226:1 weight=100 count=4 list=0,2,4,6
> > >> 226:2 weight=100 count=32 list=32-63
> > >> 226:3 weight=100 count=0 list=
> > >> 226:4 weight=500 count=0 list=
> > >>
> > >> lgpu is represented by a bitmap and uses the bitmap_parselist
> > >> kernel function so the list key input format is a
> > >> comma-separated list of decimal numbers and ranges.
> > >>
> > >> Consecutively set bits are shown as two hyphen-separated decimal
> > >> numbers, the smallest and largest bit numbers set in the range.
> > >> Optionally each range can be postfixed to denote that only parts
> > >> of it should be set. The range will divided to groups of
> > >> specific size.
> > >> Syntax: range:used_size/group_size
> > >> Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
> > >>
> > >> The count key is the hamming weight / hweight of the bitmap.
> > >>
> > >> Weight, count and list accept the max and default keywords.
> > >>
> > >> Some DRM devices may only support lgpu as anonymous resources.
> > >> In such case, the significance of the position of the set bits
> > >> in list will be ignored.
> > >>
> > >> The weight quantity is only in effect when static allocation
> > >> is not used (by setting count=0) for this cgroup. The weight
> > >> quantity distributes lgpus that are not statically allocated by
> > >> the siblings. For example, given siblings cgroupA, cgroupB and
> > >> cgroupC for a DRM device that has 64 lgpus, if cgroupA occupies
> > >> 0-63, no lgpu is available to be distributed by weight.
> > >> Similarly, if cgroupA has list=0-31 and cgroupB has list=16-63,
> > >> cgroupC will be starved if it tries to allocate by weight.
> > >>
> > >> On the other hand, if cgroupA has weight=100 count=0, cgroupB
> > >> has list=16-47, and cgroupC has weight=100 count=0, then 32
> > >> lgpus are available to be distributed evenly between cgroupA
> > >> and cgroupC. In drm.lgpu.effective, cgroupA will have
> > >> list=0-15 and cgroupC will have list=48-63.
> > >>
> > >> This lgpu resource supports the 'allocation' and 'weight'
> > >> resource distribution model.
> > >>
> > >> drm.lgpu.effective
> > >> A read-only nested-keyed file which exists on all cgroups.
> > >> Each entry is keyed by the DRM device's major:minor.
> > >>
> > >> lgpu stands for logical GPU, it is an abstraction used to
> > >> subdivide a physical DRM device for the purpose of resource
> > >> management. This file reflects the actual allocation after
> > >> considering the relationship between the cgroups and their
> > >> configurations in drm.lgpu.
> > >>
> > >> Change-Id: Idde0ef9a331fd67bb9c7eb8ef9978439e6452488
> > >> Signed-off-by: Kenny Ho <Kenny.Ho@amd.com>
> > >> ---
> > >> Documentation/admin-guide/cgroup-v2.rst | 80 ++++++
> > >> include/drm/drm_cgroup.h | 3 +
> > >> include/linux/cgroup_drm.h | 22 ++
> > >> kernel/cgroup/drm.c | 324 +++++++++++++++++++++++-
> > >> 4 files changed, 427 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> > >> index ce5dc027366a..d8a41956e5c7 100644
> > >> --- a/Documentation/admin-guide/cgroup-v2.rst
> > >> +++ b/Documentation/admin-guide/cgroup-v2.rst
> > >> @@ -2120,6 +2120,86 @@ DRM Interface Files
> > >> Set largest allocation for /dev/dri/card1 to 4MB
> > >> echo "226:1 4m" > drm.buffer.peak.max
> > >>
> > >> + drm.lgpu
> > >> + A read-write nested-keyed file which exists on all cgroups.
> > >> + Each entry is keyed by the DRM device's major:minor.
> > >> +
> > >> + lgpu stands for logical GPU, it is an abstraction used to
> > >> + subdivide a physical DRM device for the purpose of resource
> > >> + management. This file stores user configuration while the
> > >> + drm.lgpu.effective reflects the actual allocation after
> > >> + considering the relationship between the cgroups and their
> > >> + configurations.
> > >> +
> > >> + The lgpu is a discrete quantity that is device specific (i.e.
> > >> + some DRM devices may have 64 lgpus while others may have 100
> > >> + lgpus.) The lgpu is a single quantity that can be allocated
> > >> + in three different ways denoted by the following nested keys.
> > >> +
> > >> + ===== ==============================================
> > >> + weight Allocate by proportion in relationship with
> > >> + active sibling cgroups
> > >> + count Allocate by amount statically, treat lgpu as
> > >> + anonymous resources
> > >> + list Allocate statically, treat lgpu as named
> > >> + resource
> > >> + ===== ==============================================
> > >> +
> > >> + For example:
> > >> + 226:0 weight=100 count=256 list=0-255
> > >> + 226:1 weight=100 count=4 list=0,2,4,6
> > >> + 226:2 weight=100 count=32 list=32-63
> > >> + 226:3 weight=100 count=0 list=
> > >> + 226:4 weight=500 count=0 list=
> > >> +
> > >> + lgpu is represented by a bitmap and uses the bitmap_parselist
> > >> + kernel function so the list key input format is a
> > >> + comma-separated list of decimal numbers and ranges.
> > >> +
> > >> + Consecutively set bits are shown as two hyphen-separated decimal
> > >> + numbers, the smallest and largest bit numbers set in the range.
> > >> + Optionally each range can be postfixed to denote that only parts
> > >> + of it should be set. The range will divided to groups of
> > >> + specific size.
> > >> + Syntax: range:used_size/group_size
> > >> + Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
> > >> +
> > >> + The count key is the hamming weight / hweight of the bitmap.
> > >> +
> > >> + Weight, count and list accept the max and default keywords.
> > >> +
> > >> + Some DRM devices may only support lgpu as anonymous resources.
> > >> + In such case, the significance of the position of the set bits
> > >> + in list will be ignored.
> > >> +
> > >> + The weight quantity is only in effect when static allocation
> > >> + is not used (by setting count=0) for this cgroup. The weight
> > >> + quantity distributes lgpus that are not statically allocated by
> > >> + the siblings. For example, given siblings cgroupA, cgroupB and
> > >> + cgroupC for a DRM device that has 64 lgpus, if cgroupA occupies
> > >> + 0-63, no lgpu is available to be distributed by weight.
> > >> + Similarly, if cgroupA has list=0-31 and cgroupB has list=16-63,
> > >> + cgroupC will be starved if it tries to allocate by weight.
> > >> +
> > >> + On the other hand, if cgroupA has weight=100 count=0, cgroupB
> > >> + has list=16-47, and cgroupC has weight=100 count=0, then 32
> > >> + lgpus are available to be distributed evenly between cgroupA
> > >> + and cgroupC. In drm.lgpu.effective, cgroupA will have
> > >> + list=0-15 and cgroupC will have list=48-63.
> > >> +
> > >> + This lgpu resource supports the 'allocation' and 'weight'
> > >> + resource distribution model.
> > >> +
> > >> + drm.lgpu.effective
> > >> + A read-only nested-keyed file which exists on all cgroups.
> > >> + Each entry is keyed by the DRM device's major:minor.
> > >> +
> > >> + lgpu stands for logical GPU, it is an abstraction used to
> > >> + subdivide a physical DRM device for the purpose of resource
> > >> + management. This file reflects the actual allocation after
> > >> + considering the relationship between the cgroups and their
> > >> + configurations in drm.lgpu.
> > >> +
> > >> GEM Buffer Ownership
> > >> ~~~~~~~~~~~~~~~~~~~~
> > >>
> > >> diff --git a/include/drm/drm_cgroup.h b/include/drm/drm_cgroup.h
> > >> index 2b41d4d22e33..619a110cc748 100644
> > >> --- a/include/drm/drm_cgroup.h
> > >> +++ b/include/drm/drm_cgroup.h
> > >> @@ -17,6 +17,9 @@ struct drmcg_props {
> > >>
> > >> s64 bo_limits_total_allocated_default;
> > >> s64 bo_limits_peak_allocated_default;
> > >> +
> > >> + int lgpu_capacity;
> > >> + DECLARE_BITMAP(lgpu_slots, MAX_DRMCG_LGPU_CAPACITY);
> > >> };
> > >>
> > >> void drmcg_bind(struct drm_minor (*(*acq_dm)(unsigned int minor_id)),
> > >> diff --git a/include/linux/cgroup_drm.h b/include/linux/cgroup_drm.h
> > >> index eae400f3d9b4..bb09704e7f71 100644
> > >> --- a/include/linux/cgroup_drm.h
> > >> +++ b/include/linux/cgroup_drm.h
> > >> @@ -11,10 +11,14 @@
> > >> /* limit defined per the way drm_minor_alloc operates */
> > >> #define MAX_DRM_DEV (64 * DRM_MINOR_RENDER)
> > >>
> > >> +#define MAX_DRMCG_LGPU_CAPACITY 256
> > >> +
> > >> enum drmcg_res_type {
> > >> DRMCG_TYPE_BO_TOTAL,
> > >> DRMCG_TYPE_BO_PEAK,
> > >> DRMCG_TYPE_BO_COUNT,
> > >> + DRMCG_TYPE_LGPU,
> > >> + DRMCG_TYPE_LGPU_EFF,
> > >> __DRMCG_TYPE_LAST,
> > >> };
> > >>
> > >> @@ -32,6 +36,24 @@ struct drmcg_device_resource {
> > >> s64 bo_limits_peak_allocated;
> > >>
> > >> s64 bo_stats_count_allocated;
> > >> +
> > >> + /**
> > >> + * Logical GPU
> > >> + *
> > >> + * *_cfg are properties configured by users
> > >> + * *_eff are the effective properties being applied to the hardware
> > >> + * *_stg is used to calculate _eff before applying to _eff
> > >> + * after considering the entire hierarchy
> > >> + */
> > >> + DECLARE_BITMAP(lgpu_stg, MAX_DRMCG_LGPU_CAPACITY);
> > >> + /* user configurations */
> > >> + s64 lgpu_weight_cfg;
> > >> + DECLARE_BITMAP(lgpu_cfg, MAX_DRMCG_LGPU_CAPACITY);
> > >> + /* effective lgpu for the cgroup after considering
> > >> + * relationship with other cgroup
> > >> + */
> > >> + s64 lgpu_count_eff;
> > >> + DECLARE_BITMAP(lgpu_eff, MAX_DRMCG_LGPU_CAPACITY);
> > >> };
> > >>
> > >> /**
> > >> diff --git a/kernel/cgroup/drm.c b/kernel/cgroup/drm.c
> > >> index 5fcbbc13fa1c..a4e88a3704bb 100644
> > >> --- a/kernel/cgroup/drm.c
> > >> +++ b/kernel/cgroup/drm.c
> > >> @@ -9,6 +9,7 @@
> > >> #include <linux/seq_file.h>
> > >> #include <linux/mutex.h>
> > >> #include <linux/kernel.h>
> > >> +#include <linux/bitmap.h>
> > >> #include <linux/cgroup_drm.h>
> > >> #include <drm/drm_file.h>
> > >> #include <drm/drm_drv.h>
> > >> @@ -41,6 +42,10 @@ enum drmcg_file_type {
> > >> DRMCG_FTYPE_DEFAULT,
> > >> };
> > >>
> > >> +#define LGPU_LIMITS_NAME_LIST "list"
> > >> +#define LGPU_LIMITS_NAME_COUNT "count"
> > >> +#define LGPU_LIMITS_NAME_WEIGHT "weight"
> > >> +
> > >> /**
> > >> * drmcg_bind - Bind DRM subsystem to cgroup subsystem
> > >> * @acq_dm: function pointer to the drm_minor_acquire function
> > >> @@ -98,6 +103,13 @@ static inline int init_drmcg_single(struct drmcg *drmcg, struct drm_device *dev)
> > >> ddr->bo_limits_peak_allocated =
> > >> dev->drmcg_props.bo_limits_peak_allocated_default;
> > >>
> > >> + bitmap_copy(ddr->lgpu_cfg, dev->drmcg_props.lgpu_slots,
> > >> + MAX_DRMCG_LGPU_CAPACITY);
> > >> + bitmap_copy(ddr->lgpu_stg, dev->drmcg_props.lgpu_slots,
> > >> + MAX_DRMCG_LGPU_CAPACITY);
> > >> +
> > >> + ddr->lgpu_weight_cfg = CGROUP_WEIGHT_DFL;
> > >> +
> > >> return 0;
> > >> }
> > >>
> > >> @@ -121,6 +133,120 @@ static inline void drmcg_update_cg_tree(struct drm_device *dev)
> > >> mutex_unlock(&cgroup_mutex);
> > >> }
> > >>
> > >> +static void drmcg_calculate_effective_lgpu(struct drm_device *dev,
> > >> + const unsigned long *free_static,
> > >> + const unsigned long *free_weighted,
> > >> + struct drmcg *parent_drmcg)
> > >> +{
> > >> + int capacity = dev->drmcg_props.lgpu_capacity;
> > >> + DECLARE_BITMAP(lgpu_unused, MAX_DRMCG_LGPU_CAPACITY);
> > >> + DECLARE_BITMAP(lgpu_by_weight, MAX_DRMCG_LGPU_CAPACITY);
> > >> + struct drmcg_device_resource *parent_ddr;
> > >> + struct drmcg_device_resource *ddr;
> > >> + int minor = dev->primary->index;
> > >> + struct cgroup_subsys_state *pos;
> > >> + struct drmcg *child;
> > >> + s64 weight_sum = 0;
> > >> + s64 unused;
> > >> +
> > >> + parent_ddr = parent_drmcg->dev_resources[minor];
> > >> +
> > >> + if (bitmap_empty(parent_ddr->lgpu_cfg, capacity))
> > >> + /* no static cfg, use weight for calculating the effective */
> > >> + bitmap_copy(parent_ddr->lgpu_stg, free_weighted, capacity);
> > >> + else
> > >> + /* lgpu statically configured, use the overlap as effective */
> > >> + bitmap_and(parent_ddr->lgpu_stg, free_static,
> > >> + parent_ddr->lgpu_cfg, capacity);
> > >> +
> > >> + /* calculate lgpu available for distribution by weight for children */
> > >> + bitmap_copy(lgpu_unused, parent_ddr->lgpu_stg, capacity);
> > >> + css_for_each_child(pos, &parent_drmcg->css) {
> > >> + child = css_to_drmcg(pos);
> > >> + ddr = child->dev_resources[minor];
> > >> +
> > >> + if (bitmap_empty(ddr->lgpu_cfg, capacity))
> > >> + /* no static allocation, participate in weight dist */
> > >> + weight_sum += ddr->lgpu_weight_cfg;
> > >> + else
> > >> + /* take out statically allocated lgpu by siblings */
> > >> + bitmap_andnot(lgpu_unused, lgpu_unused, ddr->lgpu_cfg,
> > >> + capacity);
> > >> + }
> > >> +
> > >> + unused = bitmap_weight(lgpu_unused, capacity);
> > >> +
> > >> + css_for_each_child(pos, &parent_drmcg->css) {
> > >> + child = css_to_drmcg(pos);
> > >> + ddr = child->dev_resources[minor];
> > >> +
> > >> + bitmap_zero(lgpu_by_weight, capacity);
> > >> + /* no static allocation, participate in weight distribution */
> > >> + if (bitmap_empty(ddr->lgpu_cfg, capacity)) {
> > >> + int c;
> > >> + int p = 0;
> > >> +
> > >> + for (c = ddr->lgpu_weight_cfg * unused / weight_sum;
> > >> + c > 0; c--) {
> > >> + p = find_next_bit(lgpu_unused, capacity, p);
> > >> + if (p < capacity) {
> > >> + clear_bit(p, lgpu_unused);
> > >> + set_bit(p, lgpu_by_weight);
> > >> + }
> > >> + }
> > >> +
> > >> + }
> > >> +
> > >> + drmcg_calculate_effective_lgpu(dev, parent_ddr->lgpu_stg,
> > >> + lgpu_by_weight, child);
> > >> + }
> > >> +}
> > >> +
> > >> +static void drmcg_apply_effective_lgpu(struct drm_device *dev)
> > >> +{
> > >> + int capacity = dev->drmcg_props.lgpu_capacity;
> > >> + int minor = dev->primary->index;
> > >> + struct drmcg_device_resource *ddr;
> > >> + struct cgroup_subsys_state *pos;
> > >> + struct drmcg *drmcg;
> > >> +
> > >> + if (root_drmcg == NULL) {
> > >> + WARN_ON(root_drmcg == NULL);
> > >> + return;
> > >> + }
> > >> +
> > >> + rcu_read_lock();
> > >> +
> > >> + /* process the entire cgroup tree from root to simplify the algorithm */
> > >> + drmcg_calculate_effective_lgpu(dev, dev->drmcg_props.lgpu_slots,
> > >> + dev->drmcg_props.lgpu_slots, root_drmcg);
> > >> +
> > >> + /* apply changes to effective only if there is a change */
> > >> + css_for_each_descendant_pre(pos, &root_drmcg->css) {
> > >> + drmcg = css_to_drmcg(pos);
> > >> + ddr = drmcg->dev_resources[minor];
> > >> +
> > >> + if (!bitmap_equal(ddr->lgpu_stg, ddr->lgpu_eff, capacity)) {
> > >> + bitmap_copy(ddr->lgpu_eff, ddr->lgpu_stg, capacity);
> > >> + ddr->lgpu_count_eff =
> > >> + bitmap_weight(ddr->lgpu_eff, capacity);
> > >> + }
> > >> + }
> > >> + rcu_read_unlock();
> > >> +}
> > >> +
> > >> +static void drmcg_apply_effective(enum drmcg_res_type type,
> > >> + struct drm_device *dev, struct drmcg *changed_drmcg)
> > >> +{
> > >> + switch (type) {
> > >> + case DRMCG_TYPE_LGPU:
> > >> + drmcg_apply_effective_lgpu(dev);
> > >> + break;
> > >> + default:
> > >> + break;
> > >> + }
> > >> +}
> > >> +
> > >> /**
> > >> * drmcg_register_dev - register a DRM device for usage in drm cgroup
> > >> * @dev: DRM device
> > >> @@ -143,7 +269,13 @@ void drmcg_register_dev(struct drm_device *dev)
> > >> {
> > >> dev->driver->drmcg_custom_init(dev, &dev->drmcg_props);
> > >>
> > >> + WARN_ON(dev->drmcg_props.lgpu_capacity !=
> > >> + bitmap_weight(dev->drmcg_props.lgpu_slots,
> > >> + MAX_DRMCG_LGPU_CAPACITY));
> > >> +
> > >> drmcg_update_cg_tree(dev);
> > >> +
> > >> + drmcg_apply_effective(DRMCG_TYPE_LGPU, dev, root_drmcg);
> > >> }
> > >> mutex_unlock(&drmcg_mutex);
> > >> }
> > >> @@ -297,7 +429,8 @@ static void drmcg_print_stats(struct drmcg_device_resource *ddr,
> > >> }
> > >>
> > >> static void drmcg_print_limits(struct drmcg_device_resource *ddr,
> > >> - struct seq_file *sf, enum drmcg_res_type type)
> > >> + struct seq_file *sf, enum drmcg_res_type type,
> > >> + struct drm_device *dev)
> > >> {
> > >> if (ddr == NULL) {
> > >> seq_puts(sf, "\n");
> > >> @@ -311,6 +444,25 @@ static void drmcg_print_limits(struct drmcg_device_resource *ddr,
> > >> case DRMCG_TYPE_BO_PEAK:
> > >> seq_printf(sf, "%lld\n", ddr->bo_limits_peak_allocated);
> > >> break;
> > >> + case DRMCG_TYPE_LGPU:
> > >> + seq_printf(sf, "%s=%lld %s=%d %s=%*pbl\n",
> > >> + LGPU_LIMITS_NAME_WEIGHT,
> > >> + ddr->lgpu_weight_cfg,
> > >> + LGPU_LIMITS_NAME_COUNT,
> > >> + bitmap_weight(ddr->lgpu_cfg,
> > >> + dev->drmcg_props.lgpu_capacity),
> > >> + LGPU_LIMITS_NAME_LIST,
> > >> + dev->drmcg_props.lgpu_capacity,
> > >> + ddr->lgpu_cfg);
> > >> + break;
> > >> + case DRMCG_TYPE_LGPU_EFF:
> > >> + seq_printf(sf, "%s=%lld %s=%*pbl\n",
> > >> + LGPU_LIMITS_NAME_COUNT,
> > >> + ddr->lgpu_count_eff,
> > >> + LGPU_LIMITS_NAME_LIST,
> > >> + dev->drmcg_props.lgpu_capacity,
> > >> + ddr->lgpu_eff);
> > >> + break;
> > >> default:
> > >> seq_puts(sf, "\n");
> > >> break;
> > >> @@ -329,6 +481,17 @@ static void drmcg_print_default(struct drmcg_props *props,
> > >> seq_printf(sf, "%lld\n",
> > >> props->bo_limits_peak_allocated_default);
> > >> break;
> > >> + case DRMCG_TYPE_LGPU:
> > >> + seq_printf(sf, "%s=%d %s=%d %s=%*pbl\n",
> > >> + LGPU_LIMITS_NAME_WEIGHT,
> > >> + CGROUP_WEIGHT_DFL,
> > >> + LGPU_LIMITS_NAME_COUNT,
> > >> + bitmap_weight(props->lgpu_slots,
> > >> + props->lgpu_capacity),
> > >> + LGPU_LIMITS_NAME_LIST,
> > >> + props->lgpu_capacity,
> > >> + props->lgpu_slots);
> > >> + break;
> > >> default:
> > >> seq_puts(sf, "\n");
> > >> break;
> > >> @@ -358,7 +521,7 @@ static int drmcg_seq_show_fn(int id, void *ptr, void *data)
> > >> drmcg_print_stats(ddr, sf, type);
> > >> break;
> > >> case DRMCG_FTYPE_LIMIT:
> > >> - drmcg_print_limits(ddr, sf, type);
> > >> + drmcg_print_limits(ddr, sf, type, minor->dev);
> > >> break;
> > >> case DRMCG_FTYPE_DEFAULT:
> > >> drmcg_print_default(&minor->dev->drmcg_props, sf, type);
> > >> @@ -415,6 +578,115 @@ static int drmcg_process_limit_s64_val(char *sval, bool is_mem,
> > >> return rc;
> > >> }
> > >>
> > >> +static void drmcg_nested_limit_parse(struct kernfs_open_file *of,
> > >> + struct drm_device *dev, char *attrs)
> > >> +{
> > >> + DECLARE_BITMAP(tmp_bitmap, MAX_DRMCG_LGPU_CAPACITY);
> > >> + DECLARE_BITMAP(chk_bitmap, MAX_DRMCG_LGPU_CAPACITY);
> > >> + enum drmcg_res_type type =
> > >> + DRMCG_CTF_PRIV2RESTYPE(of_cft(of)->private);
> > >> + struct drmcg *drmcg = css_to_drmcg(of_css(of));
> > >> + struct drmcg_props *props = &dev->drmcg_props;
> > >> + char *cft_name = of_cft(of)->name;
> > >> + int minor = dev->primary->index;
> > >> + char *nested = strstrip(attrs);
> > >> + struct drmcg_device_resource *ddr =
> > >> + drmcg->dev_resources[minor];
> > >> + char *attr;
> > >> + char sname[256];
> > >> + char sval[256];
> > >> + s64 val;
> > >> + int rc;
> > >> +
> > >> + while (nested != NULL) {
> > >> + attr = strsep(&nested, " ");
> > >> +
> > >> + if (sscanf(attr, "%255[^=]=%255[^=]", sname, sval) != 2)
> > >> + continue;
> > >> +
> > >> + switch (type) {
> > >> + case DRMCG_TYPE_LGPU:
> > >> + if (strncmp(sname, LGPU_LIMITS_NAME_LIST, 256) &&
> > >> + strncmp(sname, LGPU_LIMITS_NAME_COUNT, 256) &&
> > >> + strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256))
> > >> + continue;
> > >> +
> > >> + if (strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256) &&
> > >> + (!strcmp("max", sval) ||
> > >> + !strcmp("default", sval))) {
> > >> + bitmap_copy(ddr->lgpu_cfg, props->lgpu_slots,
> > >> + props->lgpu_capacity);
> > >> +
> > >> + continue;
> > >> + }
> > >> +
> > >> + if (strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256) == 0) {
> > >> + rc = drmcg_process_limit_s64_val(sval,
> > >> + false, CGROUP_WEIGHT_DFL,
> > >> + CGROUP_WEIGHT_MAX, &val);
> > >> +
> > >> + if (rc || val < CGROUP_WEIGHT_MIN ||
> > >> + val > CGROUP_WEIGHT_MAX) {
> > >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> > >> + minor);
> > >> + continue;
> > >> + }
> > >> +
> > >> + ddr->lgpu_weight_cfg = val;
> > >> + continue;
> > >> + }
> > >> +
> > >> + if (strncmp(sname, LGPU_LIMITS_NAME_COUNT, 256) == 0) {
> > >> + rc = drmcg_process_limit_s64_val(sval,
> > >> + false, props->lgpu_capacity,
> > >> + props->lgpu_capacity, &val);
> > >> +
> > >> + if (rc || val < 0) {
> > >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> > >> + minor);
> > >> + continue;
> > >> + }
> > >> +
> > >> + bitmap_zero(tmp_bitmap,
> > >> + MAX_DRMCG_LGPU_CAPACITY);
> > >> + bitmap_set(tmp_bitmap, 0, val);
> > >> + }
> > >> +
> > >> + if (strncmp(sname, LGPU_LIMITS_NAME_LIST, 256) == 0) {
> > >> + rc = bitmap_parselist(sval, tmp_bitmap,
> > >> + MAX_DRMCG_LGPU_CAPACITY);
> > >> +
> > >> + if (rc) {
> > >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> > >> + minor);
> > >> + continue;
> > >> + }
> > >> +
> > >> + bitmap_andnot(chk_bitmap, tmp_bitmap,
> > >> + props->lgpu_slots,
> > >> + MAX_DRMCG_LGPU_CAPACITY);
> > >> +
> > >> + /* user setting does not intersect with
> > >> + * available lgpu */
> > >> + if (!bitmap_empty(chk_bitmap,
> > >> + MAX_DRMCG_LGPU_CAPACITY)) {
> > >> + drmcg_pr_cft_err(drmcg, 0, cft_name,
> > >> + minor);
> > >> + continue;
> > >> + }
> > >> + }
> > >> +
> > >> + bitmap_copy(ddr->lgpu_cfg, tmp_bitmap,
> > >> + props->lgpu_capacity);
> > >> +
> > >> + break; /* DRMCG_TYPE_LGPU */
> > >> + default:
> > >> + break;
> > >> + } /* switch (type) */
> > >> + }
> > >> +}
> > >> +
> > >> +
> > >> /**
> > >> * drmcg_limit_write - parse cgroup interface files to obtain user config
> > >> *
> > >> @@ -499,9 +771,15 @@ static ssize_t drmcg_limit_write(struct kernfs_open_file *of, char *buf,
> > >>
> > >> ddr->bo_limits_peak_allocated = val;
> > >> break;
> > >> + case DRMCG_TYPE_LGPU:
> > >> + drmcg_nested_limit_parse(of, dm->dev, sattr);
> > >> + break;
> > >> default:
> > >> break;
> > >> }
> > >> +
> > >> + drmcg_apply_effective(type, dm->dev, drmcg);
> > >> +
> > >> mutex_unlock(&dm->dev->drmcg_mutex);
> > >>
> > >> mutex_lock(&drmcg_mutex);
> > >> @@ -560,12 +838,51 @@ struct cftype files[] = {
> > >> .private = DRMCG_CTF_PRIV(DRMCG_TYPE_BO_COUNT,
> > >> DRMCG_FTYPE_STATS),
> > >> },
> > >> + {
> > >> + .name = "lgpu",
> > >> + .seq_show = drmcg_seq_show,
> > >> + .write = drmcg_limit_write,
> > >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU,
> > >> + DRMCG_FTYPE_LIMIT),
> > >> + },
> > >> + {
> > >> + .name = "lgpu.default",
> > >> + .seq_show = drmcg_seq_show,
> > >> + .flags = CFTYPE_ONLY_ON_ROOT,
> > >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU,
> > >> + DRMCG_FTYPE_DEFAULT),
> > >> + },
> > >> + {
> > >> + .name = "lgpu.effective",
> > >> + .seq_show = drmcg_seq_show,
> > >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU_EFF,
> > >> + DRMCG_FTYPE_LIMIT),
> > >> + },
> > >> { } /* terminate */
> > >> };
> > >>
> > >> +static int drmcg_online_fn(int id, void *ptr, void *data)
> > >> +{
> > >> + struct drm_minor *minor = ptr;
> > >> + struct drmcg *drmcg = data;
> > >> +
> > >> + if (minor->type != DRM_MINOR_PRIMARY)
> > >> + return 0;
> > >> +
> > >> + drmcg_apply_effective(DRMCG_TYPE_LGPU, minor->dev, drmcg);
> > >> +
> > >> + return 0;
> > >> +}
> > >> +
> > >> +static int drmcg_css_online(struct cgroup_subsys_state *css)
> > >> +{
> > >> + return drm_minor_for_each(&drmcg_online_fn, css_to_drmcg(css));
> > >> +}
> > >> +
> > >> struct cgroup_subsys drm_cgrp_subsys = {
> > >> .css_alloc = drmcg_css_alloc,
> > >> .css_free = drmcg_css_free,
> > >> + .css_online = drmcg_css_online,
> > >> .early_init = false,
> > >> .legacy_cftypes = files,
> > >> .dfl_cftypes = files,
> > >> @@ -585,6 +902,9 @@ void drmcg_device_early_init(struct drm_device *dev)
> > >> dev->drmcg_props.bo_limits_total_allocated_default = S64_MAX;
> > >> dev->drmcg_props.bo_limits_peak_allocated_default = S64_MAX;
> > >>
> > >> + dev->drmcg_props.lgpu_capacity = MAX_DRMCG_LGPU_CAPACITY;
> > >> + bitmap_fill(dev->drmcg_props.lgpu_slots, MAX_DRMCG_LGPU_CAPACITY);
> > >> +
> > >> drmcg_update_cg_tree(dev);
> > >> }
> > >> EXPORT_SYMBOL(drmcg_device_early_init);
> > >> --
> > >> 2.25.0
> > >>
> > >> _______________________________________________
> > >> dri-devel mailing list
> > >> dri-devel@lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 09/11] drm, cgroup: Introduce lgpu as DRM cgroup resource
From: Kenny Ho @ 2020-02-14 18:51 UTC (permalink / raw)
To: Daniel Vetter
Cc: Jason Ekstrand, Kenny Ho, cgroups-u79uwXL29TY76Z2rM5mHXA,
Maling list - DRI developers, amd-gfx mailing list, Tejun Heo,
Alex Deucher, Christian König, Kuehling, Felix,
Greathouse, Joseph, jsparks-WVYJKLFxKCc, lkaplan-WVYJKLFxKCc,
nirmoy.das-5C7GfCeVMHo, damon.mcdougall-5C7GfCeVMHo,
juan.zuniga-anaya-5C7GfCeVMHo
In-Reply-To: <20200214183401.GY2363188-dv86pmgwkMBes7Z6vYuT8azUEOm+Xw19@public.gmane.org>
On Fri, Feb 14, 2020 at 1:34 PM Daniel Vetter <daniel-/w4YWyX8dFk@public.gmane.org> wrote:
>
> I think guidance from Tejun in previos discussions was pretty clear that
> he expects cgroups to be both a) standardized and c) sufficient clear
> meaning that end-users have a clear understanding of what happens when
> they change the resource allocation.
>
> I'm not sure lgpu here, at least as specified, passes either.
I disagree (at least on the characterization of the feedback
provided.) I believe this series satisfied the sprite of Tejun's
guidance so far (the weight knob for lgpu, for example, was
specifically implemented base on his input.) But, I will let Tejun
speak for himself after he considered the implementation in detail.
Regards,
Kenny
> But I also
> don't have much clue, so pulled Jason in - he understands how this all
> gets reflected to userspace apis a lot better than me.
> -Daniel
>
>
> >
> > > If it's carving up compute power, what's actually being carved up? Time? Execution units/waves/threads? Even if that's the case, what advantage does it give to have it in terms of a fixed set of lgpus where each cgroup gets to pick a fixed set. Does affinity matter that much? Why not just say how many waves the GPU supports and that they have to be allocated in chunks of 16 waves (pulling a number out of thin air) and let the cgroup specify how many waves it wants.
> > >
> > > Don't get me wrong here. I'm all for the notion of being able to use cgroups to carve up GPU compute resources. However, this sounds to me like the most AMD-specific solution possible. We (Intel) could probably do some sort of carving up as well but we'd likely want to do it with preemption and time-slicing rather than handing out specific EUs.
> >
> > This has been discussed in the RFC before
> > (https://www.spinics.net/lists/cgroups/msg23469.html.) As mentioned
> > before, the idea of a compute unit is hardly an AMD specific thing as
> > it is in the OpenCL standard and part of the architecture of many
> > different vendors. In addition, the interface presented here supports
> > Intel's use case. What you described is what I considered as the
> > "anonymous resources" view of the lgpu. What you/Intel can do, is to
> > register your device to drmcg to have 100 lgpu and users can specify
> > simply by count. So if they want to allocate 5% for a cgroup, they
> > would set count=5. Per the documentation in this patch: "Some DRM
> > devices may only support lgpu as anonymous resources. In such case,
> > the significance of the position of the set bits in list will be
> > ignored." What Intel does with the user expressed configuration of "5
> > out of 100" is entirely up to Intel (time slice if you like, change to
> > specific EUs later if you like, or make it driver configurable to
> > support both if you like.)
> >
> > Regards,
> > Kenny
> >
> > >
> > > On Fri, Feb 14, 2020 at 9:57 AM Kenny Ho <Kenny.Ho-5C7GfCeVMHo@public.gmane.org> wrote:
> > >>
> > >> drm.lgpu
> > >> A read-write nested-keyed file which exists on all cgroups.
> > >> Each entry is keyed by the DRM device's major:minor.
> > >>
> > >> lgpu stands for logical GPU, it is an abstraction used to
> > >> subdivide a physical DRM device for the purpose of resource
> > >> management. This file stores user configuration while the
> > >> drm.lgpu.effective reflects the actual allocation after
> > >> considering the relationship between the cgroups and their
> > >> configurations.
> > >>
> > >> The lgpu is a discrete quantity that is device specific (i.e.
> > >> some DRM devices may have 64 lgpus while others may have 100
> > >> lgpus.) The lgpu is a single quantity that can be allocated
> > >> in three different ways denoted by the following nested keys.
> > >>
> > >> ===== ==============================================
> > >> weight Allocate by proportion in relationship with
> > >> active sibling cgroups
> > >> count Allocate by amount statically, treat lgpu as
> > >> anonymous resources
> > >> list Allocate statically, treat lgpu as named
> > >> resource
> > >> ===== ==============================================
> > >>
> > >> For example:
> > >> 226:0 weight=100 count=256 list=0-255
> > >> 226:1 weight=100 count=4 list=0,2,4,6
> > >> 226:2 weight=100 count=32 list=32-63
> > >> 226:3 weight=100 count=0 list=
> > >> 226:4 weight=500 count=0 list=
> > >>
> > >> lgpu is represented by a bitmap and uses the bitmap_parselist
> > >> kernel function so the list key input format is a
> > >> comma-separated list of decimal numbers and ranges.
> > >>
> > >> Consecutively set bits are shown as two hyphen-separated decimal
> > >> numbers, the smallest and largest bit numbers set in the range.
> > >> Optionally each range can be postfixed to denote that only parts
> > >> of it should be set. The range will divided to groups of
> > >> specific size.
> > >> Syntax: range:used_size/group_size
> > >> Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
> > >>
> > >> The count key is the hamming weight / hweight of the bitmap.
> > >>
> > >> Weight, count and list accept the max and default keywords.
> > >>
> > >> Some DRM devices may only support lgpu as anonymous resources.
> > >> In such case, the significance of the position of the set bits
> > >> in list will be ignored.
> > >>
> > >> The weight quantity is only in effect when static allocation
> > >> is not used (by setting count=0) for this cgroup. The weight
> > >> quantity distributes lgpus that are not statically allocated by
> > >> the siblings. For example, given siblings cgroupA, cgroupB and
> > >> cgroupC for a DRM device that has 64 lgpus, if cgroupA occupies
> > >> 0-63, no lgpu is available to be distributed by weight.
> > >> Similarly, if cgroupA has list=0-31 and cgroupB has list=16-63,
> > >> cgroupC will be starved if it tries to allocate by weight.
> > >>
> > >> On the other hand, if cgroupA has weight=100 count=0, cgroupB
> > >> has list=16-47, and cgroupC has weight=100 count=0, then 32
> > >> lgpus are available to be distributed evenly between cgroupA
> > >> and cgroupC. In drm.lgpu.effective, cgroupA will have
> > >> list=0-15 and cgroupC will have list=48-63.
> > >>
> > >> This lgpu resource supports the 'allocation' and 'weight'
> > >> resource distribution model.
> > >>
> > >> drm.lgpu.effective
> > >> A read-only nested-keyed file which exists on all cgroups.
> > >> Each entry is keyed by the DRM device's major:minor.
> > >>
> > >> lgpu stands for logical GPU, it is an abstraction used to
> > >> subdivide a physical DRM device for the purpose of resource
> > >> management. This file reflects the actual allocation after
> > >> considering the relationship between the cgroups and their
> > >> configurations in drm.lgpu.
> > >>
> > >> Change-Id: Idde0ef9a331fd67bb9c7eb8ef9978439e6452488
> > >> Signed-off-by: Kenny Ho <Kenny.Ho-5C7GfCeVMHo@public.gmane.org>
> > >> ---
> > >> Documentation/admin-guide/cgroup-v2.rst | 80 ++++++
> > >> include/drm/drm_cgroup.h | 3 +
> > >> include/linux/cgroup_drm.h | 22 ++
> > >> kernel/cgroup/drm.c | 324 +++++++++++++++++++++++-
> > >> 4 files changed, 427 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> > >> index ce5dc027366a..d8a41956e5c7 100644
> > >> --- a/Documentation/admin-guide/cgroup-v2.rst
> > >> +++ b/Documentation/admin-guide/cgroup-v2.rst
> > >> @@ -2120,6 +2120,86 @@ DRM Interface Files
> > >> Set largest allocation for /dev/dri/card1 to 4MB
> > >> echo "226:1 4m" > drm.buffer.peak.max
> > >>
> > >> + drm.lgpu
> > >> + A read-write nested-keyed file which exists on all cgroups.
> > >> + Each entry is keyed by the DRM device's major:minor.
> > >> +
> > >> + lgpu stands for logical GPU, it is an abstraction used to
> > >> + subdivide a physical DRM device for the purpose of resource
> > >> + management. This file stores user configuration while the
> > >> + drm.lgpu.effective reflects the actual allocation after
> > >> + considering the relationship between the cgroups and their
> > >> + configurations.
> > >> +
> > >> + The lgpu is a discrete quantity that is device specific (i.e.
> > >> + some DRM devices may have 64 lgpus while others may have 100
> > >> + lgpus.) The lgpu is a single quantity that can be allocated
> > >> + in three different ways denoted by the following nested keys.
> > >> +
> > >> + ===== ==============================================
> > >> + weight Allocate by proportion in relationship with
> > >> + active sibling cgroups
> > >> + count Allocate by amount statically, treat lgpu as
> > >> + anonymous resources
> > >> + list Allocate statically, treat lgpu as named
> > >> + resource
> > >> + ===== ==============================================
> > >> +
> > >> + For example:
> > >> + 226:0 weight=100 count=256 list=0-255
> > >> + 226:1 weight=100 count=4 list=0,2,4,6
> > >> + 226:2 weight=100 count=32 list=32-63
> > >> + 226:3 weight=100 count=0 list=
> > >> + 226:4 weight=500 count=0 list=
> > >> +
> > >> + lgpu is represented by a bitmap and uses the bitmap_parselist
> > >> + kernel function so the list key input format is a
> > >> + comma-separated list of decimal numbers and ranges.
> > >> +
> > >> + Consecutively set bits are shown as two hyphen-separated decimal
> > >> + numbers, the smallest and largest bit numbers set in the range.
> > >> + Optionally each range can be postfixed to denote that only parts
> > >> + of it should be set. The range will divided to groups of
> > >> + specific size.
> > >> + Syntax: range:used_size/group_size
> > >> + Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
> > >> +
> > >> + The count key is the hamming weight / hweight of the bitmap.
> > >> +
> > >> + Weight, count and list accept the max and default keywords.
> > >> +
> > >> + Some DRM devices may only support lgpu as anonymous resources.
> > >> + In such case, the significance of the position of the set bits
> > >> + in list will be ignored.
> > >> +
> > >> + The weight quantity is only in effect when static allocation
> > >> + is not used (by setting count=0) for this cgroup. The weight
> > >> + quantity distributes lgpus that are not statically allocated by
> > >> + the siblings. For example, given siblings cgroupA, cgroupB and
> > >> + cgroupC for a DRM device that has 64 lgpus, if cgroupA occupies
> > >> + 0-63, no lgpu is available to be distributed by weight.
> > >> + Similarly, if cgroupA has list=0-31 and cgroupB has list=16-63,
> > >> + cgroupC will be starved if it tries to allocate by weight.
> > >> +
> > >> + On the other hand, if cgroupA has weight=100 count=0, cgroupB
> > >> + has list=16-47, and cgroupC has weight=100 count=0, then 32
> > >> + lgpus are available to be distributed evenly between cgroupA
> > >> + and cgroupC. In drm.lgpu.effective, cgroupA will have
> > >> + list=0-15 and cgroupC will have list=48-63.
> > >> +
> > >> + This lgpu resource supports the 'allocation' and 'weight'
> > >> + resource distribution model.
> > >> +
> > >> + drm.lgpu.effective
> > >> + A read-only nested-keyed file which exists on all cgroups.
> > >> + Each entry is keyed by the DRM device's major:minor.
> > >> +
> > >> + lgpu stands for logical GPU, it is an abstraction used to
> > >> + subdivide a physical DRM device for the purpose of resource
> > >> + management. This file reflects the actual allocation after
> > >> + considering the relationship between the cgroups and their
> > >> + configurations in drm.lgpu.
> > >> +
> > >> GEM Buffer Ownership
> > >> ~~~~~~~~~~~~~~~~~~~~
> > >>
> > >> diff --git a/include/drm/drm_cgroup.h b/include/drm/drm_cgroup.h
> > >> index 2b41d4d22e33..619a110cc748 100644
> > >> --- a/include/drm/drm_cgroup.h
> > >> +++ b/include/drm/drm_cgroup.h
> > >> @@ -17,6 +17,9 @@ struct drmcg_props {
> > >>
> > >> s64 bo_limits_total_allocated_default;
> > >> s64 bo_limits_peak_allocated_default;
> > >> +
> > >> + int lgpu_capacity;
> > >> + DECLARE_BITMAP(lgpu_slots, MAX_DRMCG_LGPU_CAPACITY);
> > >> };
> > >>
> > >> void drmcg_bind(struct drm_minor (*(*acq_dm)(unsigned int minor_id)),
> > >> diff --git a/include/linux/cgroup_drm.h b/include/linux/cgroup_drm.h
> > >> index eae400f3d9b4..bb09704e7f71 100644
> > >> --- a/include/linux/cgroup_drm.h
> > >> +++ b/include/linux/cgroup_drm.h
> > >> @@ -11,10 +11,14 @@
> > >> /* limit defined per the way drm_minor_alloc operates */
> > >> #define MAX_DRM_DEV (64 * DRM_MINOR_RENDER)
> > >>
> > >> +#define MAX_DRMCG_LGPU_CAPACITY 256
> > >> +
> > >> enum drmcg_res_type {
> > >> DRMCG_TYPE_BO_TOTAL,
> > >> DRMCG_TYPE_BO_PEAK,
> > >> DRMCG_TYPE_BO_COUNT,
> > >> + DRMCG_TYPE_LGPU,
> > >> + DRMCG_TYPE_LGPU_EFF,
> > >> __DRMCG_TYPE_LAST,
> > >> };
> > >>
> > >> @@ -32,6 +36,24 @@ struct drmcg_device_resource {
> > >> s64 bo_limits_peak_allocated;
> > >>
> > >> s64 bo_stats_count_allocated;
> > >> +
> > >> + /**
> > >> + * Logical GPU
> > >> + *
> > >> + * *_cfg are properties configured by users
> > >> + * *_eff are the effective properties being applied to the hardware
> > >> + * *_stg is used to calculate _eff before applying to _eff
> > >> + * after considering the entire hierarchy
> > >> + */
> > >> + DECLARE_BITMAP(lgpu_stg, MAX_DRMCG_LGPU_CAPACITY);
> > >> + /* user configurations */
> > >> + s64 lgpu_weight_cfg;
> > >> + DECLARE_BITMAP(lgpu_cfg, MAX_DRMCG_LGPU_CAPACITY);
> > >> + /* effective lgpu for the cgroup after considering
> > >> + * relationship with other cgroup
> > >> + */
> > >> + s64 lgpu_count_eff;
> > >> + DECLARE_BITMAP(lgpu_eff, MAX_DRMCG_LGPU_CAPACITY);
> > >> };
> > >>
> > >> /**
> > >> diff --git a/kernel/cgroup/drm.c b/kernel/cgroup/drm.c
> > >> index 5fcbbc13fa1c..a4e88a3704bb 100644
> > >> --- a/kernel/cgroup/drm.c
> > >> +++ b/kernel/cgroup/drm.c
> > >> @@ -9,6 +9,7 @@
> > >> #include <linux/seq_file.h>
> > >> #include <linux/mutex.h>
> > >> #include <linux/kernel.h>
> > >> +#include <linux/bitmap.h>
> > >> #include <linux/cgroup_drm.h>
> > >> #include <drm/drm_file.h>
> > >> #include <drm/drm_drv.h>
> > >> @@ -41,6 +42,10 @@ enum drmcg_file_type {
> > >> DRMCG_FTYPE_DEFAULT,
> > >> };
> > >>
> > >> +#define LGPU_LIMITS_NAME_LIST "list"
> > >> +#define LGPU_LIMITS_NAME_COUNT "count"
> > >> +#define LGPU_LIMITS_NAME_WEIGHT "weight"
> > >> +
> > >> /**
> > >> * drmcg_bind - Bind DRM subsystem to cgroup subsystem
> > >> * @acq_dm: function pointer to the drm_minor_acquire function
> > >> @@ -98,6 +103,13 @@ static inline int init_drmcg_single(struct drmcg *drmcg, struct drm_device *dev)
> > >> ddr->bo_limits_peak_allocated =
> > >> dev->drmcg_props.bo_limits_peak_allocated_default;
> > >>
> > >> + bitmap_copy(ddr->lgpu_cfg, dev->drmcg_props.lgpu_slots,
> > >> + MAX_DRMCG_LGPU_CAPACITY);
> > >> + bitmap_copy(ddr->lgpu_stg, dev->drmcg_props.lgpu_slots,
> > >> + MAX_DRMCG_LGPU_CAPACITY);
> > >> +
> > >> + ddr->lgpu_weight_cfg = CGROUP_WEIGHT_DFL;
> > >> +
> > >> return 0;
> > >> }
> > >>
> > >> @@ -121,6 +133,120 @@ static inline void drmcg_update_cg_tree(struct drm_device *dev)
> > >> mutex_unlock(&cgroup_mutex);
> > >> }
> > >>
> > >> +static void drmcg_calculate_effective_lgpu(struct drm_device *dev,
> > >> + const unsigned long *free_static,
> > >> + const unsigned long *free_weighted,
> > >> + struct drmcg *parent_drmcg)
> > >> +{
> > >> + int capacity = dev->drmcg_props.lgpu_capacity;
> > >> + DECLARE_BITMAP(lgpu_unused, MAX_DRMCG_LGPU_CAPACITY);
> > >> + DECLARE_BITMAP(lgpu_by_weight, MAX_DRMCG_LGPU_CAPACITY);
> > >> + struct drmcg_device_resource *parent_ddr;
> > >> + struct drmcg_device_resource *ddr;
> > >> + int minor = dev->primary->index;
> > >> + struct cgroup_subsys_state *pos;
> > >> + struct drmcg *child;
> > >> + s64 weight_sum = 0;
> > >> + s64 unused;
> > >> +
> > >> + parent_ddr = parent_drmcg->dev_resources[minor];
> > >> +
> > >> + if (bitmap_empty(parent_ddr->lgpu_cfg, capacity))
> > >> + /* no static cfg, use weight for calculating the effective */
> > >> + bitmap_copy(parent_ddr->lgpu_stg, free_weighted, capacity);
> > >> + else
> > >> + /* lgpu statically configured, use the overlap as effective */
> > >> + bitmap_and(parent_ddr->lgpu_stg, free_static,
> > >> + parent_ddr->lgpu_cfg, capacity);
> > >> +
> > >> + /* calculate lgpu available for distribution by weight for children */
> > >> + bitmap_copy(lgpu_unused, parent_ddr->lgpu_stg, capacity);
> > >> + css_for_each_child(pos, &parent_drmcg->css) {
> > >> + child = css_to_drmcg(pos);
> > >> + ddr = child->dev_resources[minor];
> > >> +
> > >> + if (bitmap_empty(ddr->lgpu_cfg, capacity))
> > >> + /* no static allocation, participate in weight dist */
> > >> + weight_sum += ddr->lgpu_weight_cfg;
> > >> + else
> > >> + /* take out statically allocated lgpu by siblings */
> > >> + bitmap_andnot(lgpu_unused, lgpu_unused, ddr->lgpu_cfg,
> > >> + capacity);
> > >> + }
> > >> +
> > >> + unused = bitmap_weight(lgpu_unused, capacity);
> > >> +
> > >> + css_for_each_child(pos, &parent_drmcg->css) {
> > >> + child = css_to_drmcg(pos);
> > >> + ddr = child->dev_resources[minor];
> > >> +
> > >> + bitmap_zero(lgpu_by_weight, capacity);
> > >> + /* no static allocation, participate in weight distribution */
> > >> + if (bitmap_empty(ddr->lgpu_cfg, capacity)) {
> > >> + int c;
> > >> + int p = 0;
> > >> +
> > >> + for (c = ddr->lgpu_weight_cfg * unused / weight_sum;
> > >> + c > 0; c--) {
> > >> + p = find_next_bit(lgpu_unused, capacity, p);
> > >> + if (p < capacity) {
> > >> + clear_bit(p, lgpu_unused);
> > >> + set_bit(p, lgpu_by_weight);
> > >> + }
> > >> + }
> > >> +
> > >> + }
> > >> +
> > >> + drmcg_calculate_effective_lgpu(dev, parent_ddr->lgpu_stg,
> > >> + lgpu_by_weight, child);
> > >> + }
> > >> +}
> > >> +
> > >> +static void drmcg_apply_effective_lgpu(struct drm_device *dev)
> > >> +{
> > >> + int capacity = dev->drmcg_props.lgpu_capacity;
> > >> + int minor = dev->primary->index;
> > >> + struct drmcg_device_resource *ddr;
> > >> + struct cgroup_subsys_state *pos;
> > >> + struct drmcg *drmcg;
> > >> +
> > >> + if (root_drmcg == NULL) {
> > >> + WARN_ON(root_drmcg == NULL);
> > >> + return;
> > >> + }
> > >> +
> > >> + rcu_read_lock();
> > >> +
> > >> + /* process the entire cgroup tree from root to simplify the algorithm */
> > >> + drmcg_calculate_effective_lgpu(dev, dev->drmcg_props.lgpu_slots,
> > >> + dev->drmcg_props.lgpu_slots, root_drmcg);
> > >> +
> > >> + /* apply changes to effective only if there is a change */
> > >> + css_for_each_descendant_pre(pos, &root_drmcg->css) {
> > >> + drmcg = css_to_drmcg(pos);
> > >> + ddr = drmcg->dev_resources[minor];
> > >> +
> > >> + if (!bitmap_equal(ddr->lgpu_stg, ddr->lgpu_eff, capacity)) {
> > >> + bitmap_copy(ddr->lgpu_eff, ddr->lgpu_stg, capacity);
> > >> + ddr->lgpu_count_eff =
> > >> + bitmap_weight(ddr->lgpu_eff, capacity);
> > >> + }
> > >> + }
> > >> + rcu_read_unlock();
> > >> +}
> > >> +
> > >> +static void drmcg_apply_effective(enum drmcg_res_type type,
> > >> + struct drm_device *dev, struct drmcg *changed_drmcg)
> > >> +{
> > >> + switch (type) {
> > >> + case DRMCG_TYPE_LGPU:
> > >> + drmcg_apply_effective_lgpu(dev);
> > >> + break;
> > >> + default:
> > >> + break;
> > >> + }
> > >> +}
> > >> +
> > >> /**
> > >> * drmcg_register_dev - register a DRM device for usage in drm cgroup
> > >> * @dev: DRM device
> > >> @@ -143,7 +269,13 @@ void drmcg_register_dev(struct drm_device *dev)
> > >> {
> > >> dev->driver->drmcg_custom_init(dev, &dev->drmcg_props);
> > >>
> > >> + WARN_ON(dev->drmcg_props.lgpu_capacity !=
> > >> + bitmap_weight(dev->drmcg_props.lgpu_slots,
> > >> + MAX_DRMCG_LGPU_CAPACITY));
> > >> +
> > >> drmcg_update_cg_tree(dev);
> > >> +
> > >> + drmcg_apply_effective(DRMCG_TYPE_LGPU, dev, root_drmcg);
> > >> }
> > >> mutex_unlock(&drmcg_mutex);
> > >> }
> > >> @@ -297,7 +429,8 @@ static void drmcg_print_stats(struct drmcg_device_resource *ddr,
> > >> }
> > >>
> > >> static void drmcg_print_limits(struct drmcg_device_resource *ddr,
> > >> - struct seq_file *sf, enum drmcg_res_type type)
> > >> + struct seq_file *sf, enum drmcg_res_type type,
> > >> + struct drm_device *dev)
> > >> {
> > >> if (ddr == NULL) {
> > >> seq_puts(sf, "\n");
> > >> @@ -311,6 +444,25 @@ static void drmcg_print_limits(struct drmcg_device_resource *ddr,
> > >> case DRMCG_TYPE_BO_PEAK:
> > >> seq_printf(sf, "%lld\n", ddr->bo_limits_peak_allocated);
> > >> break;
> > >> + case DRMCG_TYPE_LGPU:
> > >> + seq_printf(sf, "%s=%lld %s=%d %s=%*pbl\n",
> > >> + LGPU_LIMITS_NAME_WEIGHT,
> > >> + ddr->lgpu_weight_cfg,
> > >> + LGPU_LIMITS_NAME_COUNT,
> > >> + bitmap_weight(ddr->lgpu_cfg,
> > >> + dev->drmcg_props.lgpu_capacity),
> > >> + LGPU_LIMITS_NAME_LIST,
> > >> + dev->drmcg_props.lgpu_capacity,
> > >> + ddr->lgpu_cfg);
> > >> + break;
> > >> + case DRMCG_TYPE_LGPU_EFF:
> > >> + seq_printf(sf, "%s=%lld %s=%*pbl\n",
> > >> + LGPU_LIMITS_NAME_COUNT,
> > >> + ddr->lgpu_count_eff,
> > >> + LGPU_LIMITS_NAME_LIST,
> > >> + dev->drmcg_props.lgpu_capacity,
> > >> + ddr->lgpu_eff);
> > >> + break;
> > >> default:
> > >> seq_puts(sf, "\n");
> > >> break;
> > >> @@ -329,6 +481,17 @@ static void drmcg_print_default(struct drmcg_props *props,
> > >> seq_printf(sf, "%lld\n",
> > >> props->bo_limits_peak_allocated_default);
> > >> break;
> > >> + case DRMCG_TYPE_LGPU:
> > >> + seq_printf(sf, "%s=%d %s=%d %s=%*pbl\n",
> > >> + LGPU_LIMITS_NAME_WEIGHT,
> > >> + CGROUP_WEIGHT_DFL,
> > >> + LGPU_LIMITS_NAME_COUNT,
> > >> + bitmap_weight(props->lgpu_slots,
> > >> + props->lgpu_capacity),
> > >> + LGPU_LIMITS_NAME_LIST,
> > >> + props->lgpu_capacity,
> > >> + props->lgpu_slots);
> > >> + break;
> > >> default:
> > >> seq_puts(sf, "\n");
> > >> break;
> > >> @@ -358,7 +521,7 @@ static int drmcg_seq_show_fn(int id, void *ptr, void *data)
> > >> drmcg_print_stats(ddr, sf, type);
> > >> break;
> > >> case DRMCG_FTYPE_LIMIT:
> > >> - drmcg_print_limits(ddr, sf, type);
> > >> + drmcg_print_limits(ddr, sf, type, minor->dev);
> > >> break;
> > >> case DRMCG_FTYPE_DEFAULT:
> > >> drmcg_print_default(&minor->dev->drmcg_props, sf, type);
> > >> @@ -415,6 +578,115 @@ static int drmcg_process_limit_s64_val(char *sval, bool is_mem,
> > >> return rc;
> > >> }
> > >>
> > >> +static void drmcg_nested_limit_parse(struct kernfs_open_file *of,
> > >> + struct drm_device *dev, char *attrs)
> > >> +{
> > >> + DECLARE_BITMAP(tmp_bitmap, MAX_DRMCG_LGPU_CAPACITY);
> > >> + DECLARE_BITMAP(chk_bitmap, MAX_DRMCG_LGPU_CAPACITY);
> > >> + enum drmcg_res_type type =
> > >> + DRMCG_CTF_PRIV2RESTYPE(of_cft(of)->private);
> > >> + struct drmcg *drmcg = css_to_drmcg(of_css(of));
> > >> + struct drmcg_props *props = &dev->drmcg_props;
> > >> + char *cft_name = of_cft(of)->name;
> > >> + int minor = dev->primary->index;
> > >> + char *nested = strstrip(attrs);
> > >> + struct drmcg_device_resource *ddr =
> > >> + drmcg->dev_resources[minor];
> > >> + char *attr;
> > >> + char sname[256];
> > >> + char sval[256];
> > >> + s64 val;
> > >> + int rc;
> > >> +
> > >> + while (nested != NULL) {
> > >> + attr = strsep(&nested, " ");
> > >> +
> > >> + if (sscanf(attr, "%255[^=]=%255[^=]", sname, sval) != 2)
> > >> + continue;
> > >> +
> > >> + switch (type) {
> > >> + case DRMCG_TYPE_LGPU:
> > >> + if (strncmp(sname, LGPU_LIMITS_NAME_LIST, 256) &&
> > >> + strncmp(sname, LGPU_LIMITS_NAME_COUNT, 256) &&
> > >> + strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256))
> > >> + continue;
> > >> +
> > >> + if (strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256) &&
> > >> + (!strcmp("max", sval) ||
> > >> + !strcmp("default", sval))) {
> > >> + bitmap_copy(ddr->lgpu_cfg, props->lgpu_slots,
> > >> + props->lgpu_capacity);
> > >> +
> > >> + continue;
> > >> + }
> > >> +
> > >> + if (strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256) == 0) {
> > >> + rc = drmcg_process_limit_s64_val(sval,
> > >> + false, CGROUP_WEIGHT_DFL,
> > >> + CGROUP_WEIGHT_MAX, &val);
> > >> +
> > >> + if (rc || val < CGROUP_WEIGHT_MIN ||
> > >> + val > CGROUP_WEIGHT_MAX) {
> > >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> > >> + minor);
> > >> + continue;
> > >> + }
> > >> +
> > >> + ddr->lgpu_weight_cfg = val;
> > >> + continue;
> > >> + }
> > >> +
> > >> + if (strncmp(sname, LGPU_LIMITS_NAME_COUNT, 256) == 0) {
> > >> + rc = drmcg_process_limit_s64_val(sval,
> > >> + false, props->lgpu_capacity,
> > >> + props->lgpu_capacity, &val);
> > >> +
> > >> + if (rc || val < 0) {
> > >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> > >> + minor);
> > >> + continue;
> > >> + }
> > >> +
> > >> + bitmap_zero(tmp_bitmap,
> > >> + MAX_DRMCG_LGPU_CAPACITY);
> > >> + bitmap_set(tmp_bitmap, 0, val);
> > >> + }
> > >> +
> > >> + if (strncmp(sname, LGPU_LIMITS_NAME_LIST, 256) == 0) {
> > >> + rc = bitmap_parselist(sval, tmp_bitmap,
> > >> + MAX_DRMCG_LGPU_CAPACITY);
> > >> +
> > >> + if (rc) {
> > >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> > >> + minor);
> > >> + continue;
> > >> + }
> > >> +
> > >> + bitmap_andnot(chk_bitmap, tmp_bitmap,
> > >> + props->lgpu_slots,
> > >> + MAX_DRMCG_LGPU_CAPACITY);
> > >> +
> > >> + /* user setting does not intersect with
> > >> + * available lgpu */
> > >> + if (!bitmap_empty(chk_bitmap,
> > >> + MAX_DRMCG_LGPU_CAPACITY)) {
> > >> + drmcg_pr_cft_err(drmcg, 0, cft_name,
> > >> + minor);
> > >> + continue;
> > >> + }
> > >> + }
> > >> +
> > >> + bitmap_copy(ddr->lgpu_cfg, tmp_bitmap,
> > >> + props->lgpu_capacity);
> > >> +
> > >> + break; /* DRMCG_TYPE_LGPU */
> > >> + default:
> > >> + break;
> > >> + } /* switch (type) */
> > >> + }
> > >> +}
> > >> +
> > >> +
> > >> /**
> > >> * drmcg_limit_write - parse cgroup interface files to obtain user config
> > >> *
> > >> @@ -499,9 +771,15 @@ static ssize_t drmcg_limit_write(struct kernfs_open_file *of, char *buf,
> > >>
> > >> ddr->bo_limits_peak_allocated = val;
> > >> break;
> > >> + case DRMCG_TYPE_LGPU:
> > >> + drmcg_nested_limit_parse(of, dm->dev, sattr);
> > >> + break;
> > >> default:
> > >> break;
> > >> }
> > >> +
> > >> + drmcg_apply_effective(type, dm->dev, drmcg);
> > >> +
> > >> mutex_unlock(&dm->dev->drmcg_mutex);
> > >>
> > >> mutex_lock(&drmcg_mutex);
> > >> @@ -560,12 +838,51 @@ struct cftype files[] = {
> > >> .private = DRMCG_CTF_PRIV(DRMCG_TYPE_BO_COUNT,
> > >> DRMCG_FTYPE_STATS),
> > >> },
> > >> + {
> > >> + .name = "lgpu",
> > >> + .seq_show = drmcg_seq_show,
> > >> + .write = drmcg_limit_write,
> > >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU,
> > >> + DRMCG_FTYPE_LIMIT),
> > >> + },
> > >> + {
> > >> + .name = "lgpu.default",
> > >> + .seq_show = drmcg_seq_show,
> > >> + .flags = CFTYPE_ONLY_ON_ROOT,
> > >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU,
> > >> + DRMCG_FTYPE_DEFAULT),
> > >> + },
> > >> + {
> > >> + .name = "lgpu.effective",
> > >> + .seq_show = drmcg_seq_show,
> > >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU_EFF,
> > >> + DRMCG_FTYPE_LIMIT),
> > >> + },
> > >> { } /* terminate */
> > >> };
> > >>
> > >> +static int drmcg_online_fn(int id, void *ptr, void *data)
> > >> +{
> > >> + struct drm_minor *minor = ptr;
> > >> + struct drmcg *drmcg = data;
> > >> +
> > >> + if (minor->type != DRM_MINOR_PRIMARY)
> > >> + return 0;
> > >> +
> > >> + drmcg_apply_effective(DRMCG_TYPE_LGPU, minor->dev, drmcg);
> > >> +
> > >> + return 0;
> > >> +}
> > >> +
> > >> +static int drmcg_css_online(struct cgroup_subsys_state *css)
> > >> +{
> > >> + return drm_minor_for_each(&drmcg_online_fn, css_to_drmcg(css));
> > >> +}
> > >> +
> > >> struct cgroup_subsys drm_cgrp_subsys = {
> > >> .css_alloc = drmcg_css_alloc,
> > >> .css_free = drmcg_css_free,
> > >> + .css_online = drmcg_css_online,
> > >> .early_init = false,
> > >> .legacy_cftypes = files,
> > >> .dfl_cftypes = files,
> > >> @@ -585,6 +902,9 @@ void drmcg_device_early_init(struct drm_device *dev)
> > >> dev->drmcg_props.bo_limits_total_allocated_default = S64_MAX;
> > >> dev->drmcg_props.bo_limits_peak_allocated_default = S64_MAX;
> > >>
> > >> + dev->drmcg_props.lgpu_capacity = MAX_DRMCG_LGPU_CAPACITY;
> > >> + bitmap_fill(dev->drmcg_props.lgpu_slots, MAX_DRMCG_LGPU_CAPACITY);
> > >> +
> > >> drmcg_update_cg_tree(dev);
> > >> }
> > >> EXPORT_SYMBOL(drmcg_device_early_init);
> > >> --
> > >> 2.25.0
> > >>
> > >> _______________________________________________
> > >> dri-devel mailing list
> > >> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH 02/19] target/arm: Rename isar_feature_aa32_simd_r32
From: Philippe Mathieu-Daudé @ 2020-02-14 18:50 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: peter.maydell
In-Reply-To: <20200214181547.21408-3-richard.henderson@linaro.org>
On 2/14/20 7:15 PM, Richard Henderson wrote:
> The old name, isar_feature_aa32_fp_d32, does not reflect
> the MVFR0 field name, SIMDReg.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/cpu.h | 2 +-
> target/arm/translate-vfp.inc.c | 52 +++++++++++++++++-----------------
> 2 files changed, 27 insertions(+), 27 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply
* Re: [RFC patch 07/19] bpf: Provide BPF_PROG_RUN_PIN_ON_CPU() macro
From: Mathieu Desnoyers @ 2020-02-14 18:50 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LKML, David Miller, bpf, netdev, Alexei Starovoitov,
Daniel Borkmann, Sebastian Sewior, Peter Zijlstra, Clark Williams,
Steven Rostedt, Juri Lelli, Ingo Molnar
In-Reply-To: <20200214161503.595780887@linutronix.de>
On 14-Feb-2020 02:39:24 PM, Thomas Gleixner wrote:
[...]
> +#define BPF_PROG_RUN_PIN_ON_CPU(prog, ctx) ({ \
> + u32 ret; \
> + migrate_disable(); \
> + ret = __BPF_PROG_RUN(prog, ctx, bpf_dispatcher_nopfunc); \
> + migrate_enable(); \
> + ret; })
Does it really have to be a statement expression with a local variable ?
If so, we should consider renaming "ret" to "__ret" to minimize the
chances of a caller issuing BPF_PROG_RUN_PIN_ON_CPU with "ret" as
prog or ctx argument, which would lead to unexpected results.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [v2] arm64: dts: sc7180: add dsi controller and phy entries for idp dts
From: Matthias Kaehlcke @ 2020-02-14 18:49 UTC (permalink / raw)
To: Harigovindan P
Cc: devicetree, linux-arm-msm, linux-kernel, dri-devel, seanpaul,
kalyan_t, hoegsberg, freedreno
In-Reply-To: <20200211113735.6840-1-harigovi@codeaurora.org>
On Tue, Feb 11, 2020 at 05:07:35PM +0530, Harigovindan P wrote:
> subject: arm64: dts: sc7180: add dsi controller and phy entries for idp dts
nit: 'dts' at the end is redundant, the prefixes make it clear that this
is about DT entries.
Also the message isn't really concise. The main entries for the DSI
controller and the PHY are in sc7180.dtsi. I would suggest to drop
any mentions of DSI controller and PHYs, and just say something like
'Add nodes for IDP display'. In the body you could mention that the
display is the Visionox RM69299.
> Adding dsi controller and phy entries for idp dt.
>
> Signed-off-by: Harigovindan P <harigovi@codeaurora.org>
> ---
>
> Changes in v1:
> - Added dsi controller and dsi phy entries for idp dts
Changes in v1 is pointless, it's the first patch
> Changes in v2:
> - Adding dependency patchwork series
> - Removing suspend configuration
> - Adding blank before curly brace
>
> This patch depends on following patchwork series:
>
> https://patchwork.kernel.org/patch/11364687/
> https://patchwork.kernel.org/patch/11366303/
>
> arch/arm64/boot/dts/qcom/sc7180-idp.dts | 55 +++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> index 388f50ad4fde..6ccf8c3603ab 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> @@ -7,6 +7,7 @@
>
> /dts-v1/;
>
> +#include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> #include "sc7180.dtsi"
> #include "pm6150.dtsi"
> @@ -232,6 +233,49 @@ vreg_bob: bob {
> };
> };
>
> +&dsi0 {
> + status = "okay";
> +
> + vdda-supply = <&vreg_l3c_1p2>;
> +
> + panel@0 {
> + compatible = "visionox,rm69299-1080p-display";
> + reg = <0>;
> +
> + vdda-supply = <&vreg_l8c_1p8>;
> + vdd3p3-supply = <&vreg_l18a_2p8>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&disp_pins>;
> +
> + reset-gpios = <&pm6150l_gpio 3 GPIO_ACTIVE_HIGH>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + panel0_in: endpoint {
> + remote-endpoint = <&dsi0_out>;
> + };
> + };
> + };
> + };
> +
> + ports {
> + port@1 {
> + endpoint {
> + remote-endpoint = <&panel0_in>;
> + data-lanes = <0 1 2 3>;
> + };
> + };
> + };
> +};
> +
> +&dsi_phy {
> + status = "okay";
> +};
> +
> &qspi {
> status = "okay";
> pinctrl-names = "default";
> @@ -289,6 +333,17 @@ &usb_1_qmpphy {
>
> /* PINCTRL - additions to nodes defined in sc7180.dtsi */
>
> +&pm6150l_gpio {
> + disp_pins: disp-pins {
> + pins = "gpio3";
> + function = "func1";
> + qcom,drive-strength = <2>;
> + power-source = <0>;
> + bias-disable;
> + output-low;
> + };
> +};
> +
> &qspi_clk {
> pinconf {
> pins = "gpio63";
To get the display actually to work you also need this:
diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
index 88919da1510b03..fdbcb56dfa81f9 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
@@ -276,6 +276,14 @@
status = "okay";
};
+&mdp {
+ status = "okay";
+};
+
+&mdss {
+ status = "okay";
+};
+
&qspi {
status = "okay";
pinctrl-names = "default";
Maybe just add this to this patch?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 137/252] soc: fsl: qe: remove set but not used variable 'mm_gc'
From: Sasha Levin @ 2020-02-14 16:09 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, Chen Zhou, YueHaibing, Li Yang, Hulk Robot,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200214161147.15842-1-sashal@kernel.org>
From: YueHaibing <yuehaibing@huawei.com>
[ Upstream commit 6e62bd36e9ad85a22d92b1adce6a0336ea549733 ]
drivers/soc/fsl/qe/gpio.c: In function qe_pin_request:
drivers/soc/fsl/qe/gpio.c:163:26: warning: variable mm_gc set but not used [-Wunused-but-set-variable]
commit 1e714e54b5ca ("powerpc: qe_lib-gpio: use gpiochip data pointer")
left behind this unused variable.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Signed-off-by: Li Yang <leoyang.li@nxp.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/soc/fsl/qe/gpio.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/soc/fsl/qe/gpio.c b/drivers/soc/fsl/qe/gpio.c
index 51b3a47b5a559..2aa4088a63e6c 100644
--- a/drivers/soc/fsl/qe/gpio.c
+++ b/drivers/soc/fsl/qe/gpio.c
@@ -164,7 +164,6 @@ struct qe_pin *qe_pin_request(struct device_node *np, int index)
{
struct qe_pin *qe_pin;
struct gpio_chip *gc;
- struct of_mm_gpio_chip *mm_gc;
struct qe_gpio_chip *qe_gc;
int err;
unsigned long flags;
@@ -190,7 +189,6 @@ struct qe_pin *qe_pin_request(struct device_node *np, int index)
goto err0;
}
- mm_gc = to_of_mm_gpio_chip(gc);
qe_gc = gpiochip_get_data(gc);
spin_lock_irqsave(&qe_gc->lock, flags);
--
2.20.1
^ permalink raw reply related
* Re: [v2] arm64: dts: sc7180: add dsi controller and phy entries for idp dts
From: Matthias Kaehlcke @ 2020-02-14 18:49 UTC (permalink / raw)
To: Harigovindan P
Cc: dri-devel, linux-arm-msm, freedreno, devicetree, linux-kernel,
robdclark, seanpaul, hoegsberg, kalyan_t, nganji
In-Reply-To: <20200211113735.6840-1-harigovi@codeaurora.org>
On Tue, Feb 11, 2020 at 05:07:35PM +0530, Harigovindan P wrote:
> subject: arm64: dts: sc7180: add dsi controller and phy entries for idp dts
nit: 'dts' at the end is redundant, the prefixes make it clear that this
is about DT entries.
Also the message isn't really concise. The main entries for the DSI
controller and the PHY are in sc7180.dtsi. I would suggest to drop
any mentions of DSI controller and PHYs, and just say something like
'Add nodes for IDP display'. In the body you could mention that the
display is the Visionox RM69299.
> Adding dsi controller and phy entries for idp dt.
>
> Signed-off-by: Harigovindan P <harigovi@codeaurora.org>
> ---
>
> Changes in v1:
> - Added dsi controller and dsi phy entries for idp dts
Changes in v1 is pointless, it's the first patch
> Changes in v2:
> - Adding dependency patchwork series
> - Removing suspend configuration
> - Adding blank before curly brace
>
> This patch depends on following patchwork series:
>
> https://patchwork.kernel.org/patch/11364687/
> https://patchwork.kernel.org/patch/11366303/
>
> arch/arm64/boot/dts/qcom/sc7180-idp.dts | 55 +++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> index 388f50ad4fde..6ccf8c3603ab 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> @@ -7,6 +7,7 @@
>
> /dts-v1/;
>
> +#include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> #include "sc7180.dtsi"
> #include "pm6150.dtsi"
> @@ -232,6 +233,49 @@ vreg_bob: bob {
> };
> };
>
> +&dsi0 {
> + status = "okay";
> +
> + vdda-supply = <&vreg_l3c_1p2>;
> +
> + panel@0 {
> + compatible = "visionox,rm69299-1080p-display";
> + reg = <0>;
> +
> + vdda-supply = <&vreg_l8c_1p8>;
> + vdd3p3-supply = <&vreg_l18a_2p8>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&disp_pins>;
> +
> + reset-gpios = <&pm6150l_gpio 3 GPIO_ACTIVE_HIGH>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + panel0_in: endpoint {
> + remote-endpoint = <&dsi0_out>;
> + };
> + };
> + };
> + };
> +
> + ports {
> + port@1 {
> + endpoint {
> + remote-endpoint = <&panel0_in>;
> + data-lanes = <0 1 2 3>;
> + };
> + };
> + };
> +};
> +
> +&dsi_phy {
> + status = "okay";
> +};
> +
> &qspi {
> status = "okay";
> pinctrl-names = "default";
> @@ -289,6 +333,17 @@ &usb_1_qmpphy {
>
> /* PINCTRL - additions to nodes defined in sc7180.dtsi */
>
> +&pm6150l_gpio {
> + disp_pins: disp-pins {
> + pins = "gpio3";
> + function = "func1";
> + qcom,drive-strength = <2>;
> + power-source = <0>;
> + bias-disable;
> + output-low;
> + };
> +};
> +
> &qspi_clk {
> pinconf {
> pins = "gpio63";
To get the display actually to work you also need this:
diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
index 88919da1510b03..fdbcb56dfa81f9 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
@@ -276,6 +276,14 @@
status = "okay";
};
+&mdp {
+ status = "okay";
+};
+
+&mdss {
+ status = "okay";
+};
+
&qspi {
status = "okay";
pinctrl-names = "default";
Maybe just add this to this patch?
^ permalink raw reply related
* Re: [PATCH v5 2/6] clk: Ingenic: Adjust cgu code to make it compatible with X1830.
From: Paul Cercueil @ 2020-02-14 18:49 UTC (permalink / raw)
To: 周琰杰 (Zhou Yanjie)
Cc: linux-mips, linux-clk, linux-kernel, devicetree, mturquette,
sboyd, robh+dt, mark.rutland
In-Reply-To: <1581701262-110556-4-git-send-email-zhouyanjie@wanyeetech.com>
Hi Zhou,
Le sam., févr. 15, 2020 at 01:27, 周琰杰 (Zhou Yanjie)
<zhouyanjie@wanyeetech.com> a écrit :
> The PLL of X1830 Soc from Ingenic has been greatly changed,
> the bypass control is placed in another register, so now two
> registers may needed to control the PLL. To this end, the
> original "reg" was changed to "pll_reg", and a new "bypass_reg"
> was introduced. In addition, when calculating rate, the PLL of
> X1830 introduced an extra 2x multiplier, so a new "rate_multiplier"
> was introduced. And adjust the code in jz47xx-cgu.c and x1000-cgu.c,
> make it to be compatible with the new cgu code.
>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> ---
>
> Notes:
> v1->v2:
> 1.Use two fields (pll_reg & bypass_reg) instead of the 2-values
> array (reg[2]).
> 2.Remove the "pll_info->version" and add a
> "pll_info->rate_multiplier".
> 3.Fix the coding style and add more detailed commit message.
> 4.Change my Signed-off-by from "Zhou Yanjie <zhouyanjie@zoho.com>"
> to "周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>" because
> the old mailbox is in an unstable state.
>
> v2->v3:
> Adjust order from [1/5] in v2 to [2/5] in v3.
>
> v3->v4:
> Merge [3/5] in v3 into this patch.
>
> v4->v5:
> Rebase on top of kernel 5.6-rc1.
>
> drivers/clk/ingenic/cgu.c | 32
> +++++++++++++++++++++-----------
> drivers/clk/ingenic/cgu.h | 8 ++++++--
> drivers/clk/ingenic/jz4725b-cgu.c | 4 +++-
> drivers/clk/ingenic/jz4740-cgu.c | 4 +++-
> drivers/clk/ingenic/jz4770-cgu.c | 8 ++++++--
> drivers/clk/ingenic/jz4780-cgu.c | 4 +++-
> drivers/clk/ingenic/x1000-cgu.c | 8 ++++++--
> 7 files changed, 48 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
> index ab1302a..7d859e4 100644
> --- a/drivers/clk/ingenic/cgu.c
> +++ b/drivers/clk/ingenic/cgu.c
> @@ -82,7 +82,7 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned
> long parent_rate)
> BUG_ON(clk_info->type != CGU_CLK_PLL);
> pll_info = &clk_info->pll;
>
> - ctl = readl(cgu->base + pll_info->reg);
> + ctl = readl(cgu->base + pll_info->pll_reg);
>
> m = (ctl >> pll_info->m_shift) & GENMASK(pll_info->m_bits - 1, 0);
> m += pll_info->m_offset;
> @@ -90,6 +90,9 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned
> long parent_rate)
> n += pll_info->n_offset;
> od_enc = ctl >> pll_info->od_shift;
> od_enc &= GENMASK(pll_info->od_bits - 1, 0);
> +
> + ctl = readl(cgu->base + pll_info->bypass_reg);
> +
> bypass = !pll_info->no_bypass_bit &&
> !!(ctl & BIT(pll_info->bypass_bit));
>
> @@ -103,7 +106,8 @@ ingenic_pll_recalc_rate(struct clk_hw *hw,
> unsigned long parent_rate)
> BUG_ON(od == pll_info->od_max);
> od++;
>
> - return div_u64((u64)parent_rate * m, n * od);
> + return div_u64((u64)parent_rate * m * pll_info->rate_multiplier,
> + n * od);
> }
>
> static unsigned long
> @@ -136,7 +140,8 @@ ingenic_pll_calc(const struct
> ingenic_cgu_clk_info *clk_info,
> if (pod)
> *pod = od;
>
> - return div_u64((u64)parent_rate * m, n * od);
> + return div_u64((u64)parent_rate * m * pll_info->rate_multiplier,
> + n * od);
> }
>
> static inline const struct ingenic_cgu_clk_info *to_clk_info(
> @@ -180,7 +185,7 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned
> long req_rate,
> clk_info->name, req_rate, rate);
>
> spin_lock_irqsave(&cgu->lock, flags);
> - ctl = readl(cgu->base + pll_info->reg);
> + ctl = readl(cgu->base + pll_info->pll_reg);
>
> ctl &= ~(GENMASK(pll_info->m_bits - 1, 0) << pll_info->m_shift);
> ctl |= (m - pll_info->m_offset) << pll_info->m_shift;
> @@ -191,7 +196,7 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned
> long req_rate,
> ctl &= ~(GENMASK(pll_info->od_bits - 1, 0) << pll_info->od_shift);
> ctl |= pll_info->od_encoding[od - 1] << pll_info->od_shift;
>
> - writel(ctl, cgu->base + pll_info->reg);
> + writel(ctl, cgu->base + pll_info->pll_reg);
> spin_unlock_irqrestore(&cgu->lock, flags);
>
> return 0;
> @@ -209,16 +214,21 @@ static int ingenic_pll_enable(struct clk_hw *hw)
> u32 ctl;
>
> spin_lock_irqsave(&cgu->lock, flags);
> - ctl = readl(cgu->base + pll_info->reg);
> + ctl = readl(cgu->base + pll_info->bypass_reg);
>
> ctl &= ~BIT(pll_info->bypass_bit);
> +
> + writel(ctl, cgu->base + pll_info->bypass_reg);
> +
> + ctl = readl(cgu->base + pll_info->pll_reg);
> +
> ctl |= BIT(pll_info->enable_bit);
>
> - writel(ctl, cgu->base + pll_info->reg);
> + writel(ctl, cgu->base + pll_info->pll_reg);
>
> /* wait for the PLL to stabilise */
> for (i = 0; i < timeout; i++) {
> - ctl = readl(cgu->base + pll_info->reg);
> + ctl = readl(cgu->base + pll_info->pll_reg);
> if (ctl & BIT(pll_info->stable_bit))
> break;
> mdelay(1);
> @@ -242,11 +252,11 @@ static void ingenic_pll_disable(struct clk_hw
> *hw)
> u32 ctl;
>
> spin_lock_irqsave(&cgu->lock, flags);
> - ctl = readl(cgu->base + pll_info->reg);
> + ctl = readl(cgu->base + pll_info->pll_reg);
>
> ctl &= ~BIT(pll_info->enable_bit);
>
> - writel(ctl, cgu->base + pll_info->reg);
> + writel(ctl, cgu->base + pll_info->pll_reg);
> spin_unlock_irqrestore(&cgu->lock, flags);
> }
>
> @@ -258,7 +268,7 @@ static int ingenic_pll_is_enabled(struct clk_hw
> *hw)
> const struct ingenic_cgu_pll_info *pll_info = &clk_info->pll;
> u32 ctl;
>
> - ctl = readl(cgu->base + pll_info->reg);
> + ctl = readl(cgu->base + pll_info->pll_reg);
>
> return !!(ctl & BIT(pll_info->enable_bit));
> }
> diff --git a/drivers/clk/ingenic/cgu.h b/drivers/clk/ingenic/cgu.h
> index 0dc8004..f7b6908 100644
> --- a/drivers/clk/ingenic/cgu.h
> +++ b/drivers/clk/ingenic/cgu.h
> @@ -16,7 +16,9 @@
>
> /**
> * struct ingenic_cgu_pll_info - information about a PLL
> - * @reg: the offset of the PLL's control register within the CGU
> + * @pll_reg: the offset of the PLL's control register within the CGU
> + * @bypass_reg: the offset of the bypass control register within the
> CGU
> + * @rate_multiplier: the multiplier needed by pll rate calculation
> * @m_shift: the number of bits to shift the multiplier value by
> (ie. the
> * index of the lowest bit of the multiplier value in the
> PLL's
> * control register)
> @@ -43,7 +45,9 @@
> * @no_bypass_bit: if set, the PLL has no bypass functionality
> */
> struct ingenic_cgu_pll_info {
> - unsigned reg;
> + unsigned pll_reg;
I'd prefer that you don't rename 'reg' to 'pll_reg', this patch would
be ten times smaller if you don't.
-Paul
> + unsigned bypass_reg;
> + unsigned rate_multiplier;
> const s8 *od_encoding;
> u8 m_shift, m_bits, m_offset;
> u8 n_shift, n_bits, n_offset;
> diff --git a/drivers/clk/ingenic/jz4725b-cgu.c
> b/drivers/clk/ingenic/jz4725b-cgu.c
> index a3b4635..0b05167 100644
> --- a/drivers/clk/ingenic/jz4725b-cgu.c
> +++ b/drivers/clk/ingenic/jz4725b-cgu.c
> @@ -53,7 +53,9 @@ static const struct ingenic_cgu_clk_info
> jz4725b_cgu_clocks[] = {
> "pll", CGU_CLK_PLL,
> .parents = { JZ4725B_CLK_EXT, -1, -1, -1 },
> .pll = {
> - .reg = CGU_REG_CPPCR,
> + .pll_reg = CGU_REG_CPPCR,
> + .bypass_reg = CGU_REG_CPPCR,
> + .rate_multiplier = 1,
> .m_shift = 23,
> .m_bits = 9,
> .m_offset = 2,
> diff --git a/drivers/clk/ingenic/jz4740-cgu.c
> b/drivers/clk/ingenic/jz4740-cgu.c
> index 4f0e92c..78f31df 100644
> --- a/drivers/clk/ingenic/jz4740-cgu.c
> +++ b/drivers/clk/ingenic/jz4740-cgu.c
> @@ -68,7 +68,9 @@ static const struct ingenic_cgu_clk_info
> jz4740_cgu_clocks[] = {
> "pll", CGU_CLK_PLL,
> .parents = { JZ4740_CLK_EXT, -1, -1, -1 },
> .pll = {
> - .reg = CGU_REG_CPPCR,
> + .pll_reg = CGU_REG_CPPCR,
> + .bypass_reg = CGU_REG_CPPCR,
> + .rate_multiplier = 1,
> .m_shift = 23,
> .m_bits = 9,
> .m_offset = 2,
> diff --git a/drivers/clk/ingenic/jz4770-cgu.c
> b/drivers/clk/ingenic/jz4770-cgu.c
> index 956dd65..32e476d 100644
> --- a/drivers/clk/ingenic/jz4770-cgu.c
> +++ b/drivers/clk/ingenic/jz4770-cgu.c
> @@ -101,7 +101,9 @@ static const struct ingenic_cgu_clk_info
> jz4770_cgu_clocks[] = {
> "pll0", CGU_CLK_PLL,
> .parents = { JZ4770_CLK_EXT },
> .pll = {
> - .reg = CGU_REG_CPPCR0,
> + .pll_reg = CGU_REG_CPPCR0,
> + .bypass_reg = CGU_REG_CPPCR0,
> + .rate_multiplier = 1,
> .m_shift = 24,
> .m_bits = 7,
> .m_offset = 1,
> @@ -123,7 +125,9 @@ static const struct ingenic_cgu_clk_info
> jz4770_cgu_clocks[] = {
> "pll1", CGU_CLK_PLL,
> .parents = { JZ4770_CLK_EXT },
> .pll = {
> - .reg = CGU_REG_CPPCR1,
> + .pll_reg = CGU_REG_CPPCR1,
> + .bypass_reg = CGU_REG_CPPCR1,
> + .rate_multiplier = 1,
> .m_shift = 24,
> .m_bits = 7,
> .m_offset = 1,
> diff --git a/drivers/clk/ingenic/jz4780-cgu.c
> b/drivers/clk/ingenic/jz4780-cgu.c
> index ea905ff..d07fff1 100644
> --- a/drivers/clk/ingenic/jz4780-cgu.c
> +++ b/drivers/clk/ingenic/jz4780-cgu.c
> @@ -220,7 +220,9 @@ static const struct ingenic_cgu_clk_info
> jz4780_cgu_clocks[] = {
> /* PLLs */
>
> #define DEF_PLL(name) { \
> - .reg = CGU_REG_ ## name, \
> + .pll_reg = CGU_REG_ ## name, \
> + .bypass_reg = CGU_REG_ ## name, \
> + .rate_multiplier = 1, \
> .m_shift = 19, \
> .m_bits = 13, \
> .m_offset = 1, \
> diff --git a/drivers/clk/ingenic/x1000-cgu.c
> b/drivers/clk/ingenic/x1000-cgu.c
> index b22d87b..d6fe28f 100644
> --- a/drivers/clk/ingenic/x1000-cgu.c
> +++ b/drivers/clk/ingenic/x1000-cgu.c
> @@ -57,7 +57,9 @@ static const struct ingenic_cgu_clk_info
> x1000_cgu_clocks[] = {
> "apll", CGU_CLK_PLL,
> .parents = { X1000_CLK_EXCLK, -1, -1, -1 },
> .pll = {
> - .reg = CGU_REG_APLL,
> + .pll_reg = CGU_REG_APLL,
> + .bypass_reg = CGU_REG_APLL,
> + .rate_multiplier = 1,
> .m_shift = 24,
> .m_bits = 7,
> .m_offset = 1,
> @@ -78,7 +80,9 @@ static const struct ingenic_cgu_clk_info
> x1000_cgu_clocks[] = {
> "mpll", CGU_CLK_PLL,
> .parents = { X1000_CLK_EXCLK, -1, -1, -1 },
> .pll = {
> - .reg = CGU_REG_MPLL,
> + .pll_reg = CGU_REG_MPLL,
> + .bypass_reg = CGU_REG_MPLL,
> + .rate_multiplier = 1,
> .m_shift = 24,
> .m_bits = 7,
> .m_offset = 1,
> --
> 2.7.4
>
^ permalink raw reply
* Re: [Intel-gfx] [PATCH] drm/i915/psr: Force PSR probe only after full initialization
From: Ross Zwisler @ 2020-02-14 18:14 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: Jani Nikula, intel-gfx
In-Reply-To: <20200214015038.122913-1-jose.souza@intel.com>
On Thu, Feb 13, 2020 at 05:50:38PM -0800, José Roberto de Souza wrote:
> Commit 60c6a14b489b ("drm/i915/display: Force the state compute phase
> once to enable PSR") was forcing the state compute too earlier
> causing errors because not everything was initialized, so here
> moving to i915_driver_register() when everything is ready and driver
> is registering into the rest of the system.
>
> Also fixing the place where it disarm the force probe as during the
> atomic check phase errors could happen like the ones due locking and
> it would cause PSR to never be enabled if that happens.
> Leaving the disarm to the atomic commit phase, intel_psr_enable() or
> intel_psr_update() will be called even if the current state do not
> allow PSR to be enabled.
>
> Fixes: 60c6a14b489b ("drm/i915/display: Force the state compute phase once to enable PSR")
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/1151
> Reported-by: Ross Zwisler <zwisler@google.com>
Tested-by: Ross Zwisler <zwisler@google.com>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Note that when applying to the current upstream/master from Linus you have one
small conflict:
$ cat drivers/gpu/drm/i915/i915_drv.h.rej
--- drivers/gpu/drm/i915/i915_drv.h
+++ drivers/gpu/drm/i915/i915_drv.h
@@ -505,7 +505,7 @@ struct i915_psr {
bool dc3co_enabled;
u32 dc3co_exit_delay;
struct delayed_work dc3co_work;
- bool initially_probed;
+ bool force_mode_changed;
};
In Linus's tree the end of that structure looks like:
bool dc3co_enabled;
u32 dc3co_exit_delay;
struct delayed_work idle_work;
bool initially_probed;
};
Where the 'struct delayed_work' element is named differently.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [Intel-gfx] [PATCH] drm/i915: Disable preemption and sleeping while using the punit sideband
From: youling257 @ 2020-02-14 5:40 UTC (permalink / raw)
To: chris; +Cc: intel-gfx, youling257
In-Reply-To: <20190426081725.31217-1-chris@chris-wilson.co.uk>
Cherry trail no this problem? sure?
my z8350 ezpad always hang freeze on kernel 5.4/5.5/5.6.
when i bought it,Linux kernel has been 5.4.
v2: Restrict the heavy-weight wakeup to just the ISOF_PORT_PUNIT, there
is insufficient evidence to implicate a wider problem atm. Similarly,
restrict the w/a to Valleyview, as Cherryview doesn't have an angry cadre
of users.
how to make this patch support Cherry trail?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [Intel-gfx] [PATCH v2] drm/i915/psr: Force PSR probe only after full initialization
From: Ross Zwisler @ 2020-02-14 18:25 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: Jani Nikula, intel-gfx
In-Reply-To: <20200214181346.9957-1-jose.souza@intel.com>
On Fri, Feb 14, 2020 at 10:13:46AM -0800, José Roberto de Souza wrote:
> Commit 60c6a14b489b ("drm/i915/display: Force the state compute phase
> once to enable PSR") was forcing the state compute too earlier
> causing errors because not everything was initialized, so here
> moving to i915_driver_register() when everything is ready and driver
> is registering into the rest of the system.
>
> Also fixing the place where it disarm the force probe as during the
> atomic check phase errors could happen like the ones due locking and
> it would cause PSR to never be enabled if that happens.
> Leaving the disarm to the atomic commit phase, intel_psr_enable() or
> intel_psr_update() will be called even if the current state do not
> allow PSR to be enabled.
>
> v2: Check if intel_dp is null in intel_psr_force_mode_changed_set()
>
> Fixes: 60c6a14b489b ("drm/i915/display: Force the state compute phase once to enable PSR")
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/1151
> Reported-by: Ross Zwisler <zwisler@google.com>
Tested-by: Ross Zwisler <zwisler@google.com>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_rotation_crc: Speed up by not testing every format
From: Patchwork @ 2020-02-14 18:48 UTC (permalink / raw)
To: Ville Syrjala; +Cc: igt-dev
In-Reply-To: <20200214165502.21990-1-ville.syrjala@linux.intel.com>
== Series Details ==
Series: tests/kms_rotation_crc: Speed up by not testing every format
URL : https://patchwork.freedesktop.org/series/73478/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_7941 -> IGTPW_4155
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4155/index.html
Known issues
------------
Here are the changes found in IGTPW_4155 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live_gem_contexts:
- fi-cml-s: [PASS][1] -> [DMESG-FAIL][2] ([i915#877])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7941/fi-cml-s/igt@i915_selftest@live_gem_contexts.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4155/fi-cml-s/igt@i915_selftest@live_gem_contexts.html
#### Possible fixes ####
* igt@i915_selftest@live_execlists:
- fi-icl-y: [DMESG-FAIL][3] ([fdo#108569]) -> [PASS][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7941/fi-icl-y/igt@i915_selftest@live_execlists.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4155/fi-icl-y/igt@i915_selftest@live_execlists.html
* igt@kms_frontbuffer_tracking@basic:
- fi-hsw-peppy: [DMESG-WARN][5] ([i915#44]) -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7941/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4155/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[i915#44]: https://gitlab.freedesktop.org/drm/intel/issues/44
[i915#877]: https://gitlab.freedesktop.org/drm/intel/issues/877
Participating hosts (47 -> 45)
------------------------------
Additional (3): fi-bsw-kefka fi-gdg-551 fi-snb-2600
Missing (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-ctg-p8600 fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_5442 -> IGTPW_4155
CI-20190529: 20190529
CI_DRM_7941: c77a53006f446facee890cb72459da477c3c8f5f @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_4155: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4155/index.html
IGT_5442: 3f6080996885b997685f08ecb8b416b2dc485290 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4155/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply
* [PATCH] libselinux: drop error return from is_selinux_enabled documentation
From: Christian Göttsche @ 2020-02-14 18:47 UTC (permalink / raw)
To: selinux
In-Reply-To: <20200207143744.9944-1-cgzones@googlemail.com>
Since commit e3cab998b48ab293a9962faf9779d70ca339c65d ("libselinux
mountpoint changing patch.") for version 20120216 is_selinux_enabled()
does never return -1; drop mentions in the man-page and header file.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
libselinux/include/selinux/selinux.h | 2 +-
libselinux/man/man3/is_selinux_enabled.3 | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h
index 7922d96b..883d8b85 100644
--- a/libselinux/include/selinux/selinux.h
+++ b/libselinux/include/selinux/selinux.h
@@ -8,7 +8,7 @@
extern "C" {
#endif
-/* Return 1 if we are running on a SELinux kernel, or 0 if not or -1 if we get an error. */
+/* Return 1 if we are running on a SELinux kernel, or 0 otherwise. */
extern int is_selinux_enabled(void);
/* Return 1 if we are running on a SELinux MLS kernel, or 0 otherwise. */
extern int is_selinux_mls_enabled(void);
diff --git a/libselinux/man/man3/is_selinux_enabled.3 b/libselinux/man/man3/is_selinux_enabled.3
index df62c225..a887b48c 100644
--- a/libselinux/man/man3/is_selinux_enabled.3
+++ b/libselinux/man/man3/is_selinux_enabled.3
@@ -15,7 +15,6 @@ is_selinux_mls_enabled \- check whether SELinux is enabled for (Multi Level Secu
.SH "DESCRIPTION"
.BR is_selinux_enabled ()
returns 1 if SELinux is running or 0 if it is not.
-On error, \-1 is returned.
.BR is_selinux_mls_enabled ()
returns 1 if SELinux is capable of running in MLS mode or 0 if it is not. To
--
2.25.0
^ permalink raw reply related
* Re: [PATCH v2] net: davicom: dm9000: allow to pass MAC address through mac_addr module parameter
From: Paul Cercueil @ 2020-02-14 18:47 UTC (permalink / raw)
To: H. Nikolaus Schaller
Cc: Andrew Lunn, David S. Miller, Petr Štetiar, Richard Fontana,
Thomas Gleixner, Heiner Kallweit, netdev, linux-kernel,
letux-kernel, kernel
In-Reply-To: <0d6b4d383bb29ed5d4710e9706e5ad6c7f92d9da.1581696454.git.hns@goldelico.com>
Hi Nikolaus,
What I'd suggest is to write a NVMEM driver for the efuse and retrieve
the MAC address cleanly with nvmem_get_mac_address().
It shouldn't be hard to do (there's already code for it in the
non-upstream 3.18 kernel for the CI20) and you remove the dependency on
uboot.
-Paul
Le ven., févr. 14, 2020 at 17:07, H. Nikolaus Schaller
<hns@goldelico.com> a écrit :
> The MIPS Ingenic CI20 board is shipped with a quite old u-boot
> (ci20-v2013.10 see https://elinux.org/CI20_Dev_Zone). This passes
> the MAC address through dm9000.mac_addr=xx:xx:xx:xx:xx:xx
> kernel module parameter to give the board a fixed MAC address.
>
> This is not processed by the dm9000 driver which assigns a random
> MAC address on each boot, making DHCP assign a new IP address
> each time.
>
> So we add a check for the mac_addr module parameter as a last
> resort before assigning a random one. This mechanism can also
> be used outside of u-boot to provide a value through modprobe
> config.
>
> To parse the MAC address in a new function get_mac_addr() we
> use an copy adapted from the ksz884x.c driver which provides
> the same functionality.
>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> drivers/net/ethernet/davicom/dm9000.c | 42
> +++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/drivers/net/ethernet/davicom/dm9000.c
> b/drivers/net/ethernet/davicom/dm9000.c
> index 1ea3372775e6..7402030b0352 100644
> --- a/drivers/net/ethernet/davicom/dm9000.c
> +++ b/drivers/net/ethernet/davicom/dm9000.c
> @@ -1409,6 +1409,43 @@ static struct dm9000_plat_data
> *dm9000_parse_dt(struct device *dev)
> return pdata;
> }
>
> +static char *mac_addr = ":";
> +module_param(mac_addr, charp, 0);
> +MODULE_PARM_DESC(mac_addr, "MAC address");
> +
> +static void get_mac_addr(struct net_device *ndev, char *macaddr)
> +{
> + int i = 0;
> + int j = 0;
> + int got_num = 0;
> + int num = 0;
> +
> + while (j < ETH_ALEN) {
> + if (macaddr[i]) {
> + int digit;
> +
> + got_num = 1;
> + digit = hex_to_bin(macaddr[i]);
> + if (digit >= 0)
> + num = num * 16 + digit;
> + else if (':' == macaddr[i])
> + got_num = 2;
> + else
> + break;
> + } else if (got_num) {
> + got_num = 2;
> + } else {
> + break;
> + }
> + if (got_num == 2) {
> + ndev->dev_addr[j++] = (u8)num;
> + num = 0;
> + got_num = 0;
> + }
> + i++;
> + }
> +}
> +
> /*
> * Search DM9000 board, allocate space and register it
> */
> @@ -1679,6 +1716,11 @@ dm9000_probe(struct platform_device *pdev)
> ndev->dev_addr[i] = ior(db, i+DM9000_PAR);
> }
>
> + if (!is_valid_ether_addr(ndev->dev_addr)) {
> + mac_src = "param";
> + get_mac_addr(ndev, mac_addr);
> + }
> +
> if (!is_valid_ether_addr(ndev->dev_addr)) {
> inv_mac_addr = true;
> eth_hw_addr_random(ndev);
> --
> 2.23.0
>
^ permalink raw reply
* Re: [PATCH v5 1/2] mm: Add MREMAP_DONTUNMAP to mremap().
From: Brian Geffon @ 2020-02-14 18:46 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Andrew Morton, Michael S . Tsirkin, Arnd Bergmann, LKML, linux-mm,
Linux API, Andy Lutomirski, Will Deacon, Andrea Arcangeli,
Sonny Rao, Minchan Kim, Joel Fernandes, Yu Zhao, Jesse Barnes,
Nathan Chancellor, Florian Weimer
In-Reply-To: <20200214142857.kcmjiequhfl3sot2@box>
Hi Kirill,
> > - if (vm_flags & VM_LOCKED) {
> > - mm->locked_vm += new_len >> PAGE_SHIFT;
> > - *locked = true;
> > - }
> > -
>
> Ah. You moved this piece. Why?
Because we're not unmapping, do_munmap would have adjusted
mm->locked_vm by decreasing it by old_len so then we have to add back
in the new_len in the normal case (non. MREMAP_DONTUNMAP), but since
we're not doing the unmap I want to skip the increase by new_len and
just adjust accordingly. In the MREMAP_DONTUNMAP case if the VMA got
smaller then the do_munmap on that portion would have decreased it by
new_len - old_len, and the accounting is correct. In the case of an
unchanged VMA size there is nothing to do, but in the case where it
grows we're responsible for adding new_len - old_len because of the
decision to jump that block and now the accounting is right for all
cases.
If we were to leave the original block and not jump over it then we
would have to remove old_len bytes and then we're doing the same thing
but now special casing the situation where new_len < old_len because
the unmap on the removed part would have reduced it by new_len -
old_len so backing old_len would be too much and we'd have to add back
in new_len - old_len. I hope that explains it all.
By doing it this way, IMO it makes it easier to see how the locked_vm
accounting is happening because the vm_locked incrementing happens in
only one of two places based on the type of remap that is happening.
But I definitely can clean up the code a bit to drop the levels of
indentation, maybe this:
/*
* locked_vm accounting: if the mapping remained the same size
* it will have just moved and we don't need to touch locked_vm
* because we skip the do_unmap. If the mapping shrunk before
* being moved then the do_unmap on that portion will have
* adjusted vm_locked. Only if the mapping grows do we need to
* do something special; the reason is locked_vm only accounts
* for old_len, but we're now adding new_len - old_len locked
* bytes to the new mapping.
*/
if (vm_flags & VM_LOCKED && new_len > old_len) {
mm->locked_vm += (new_len - old_len) >> PAGE_SHIFT;
*locked = true;
}
/* We always clear VM_LOCKED[ONFAULT] on the old vma */
vma->vm_flags &= VM_LOCKED_CLEAR_MASK;
goto out;
}
Having only one place where locked_vm is accounted and adjusted based
on the type of remap seems like it will be easier to follow and less
error prone later. What do you think about this?
> > + if (flags & MREMAP_FIXED) {
>
> I think it has to be
>
> if (!(flags & MREMAP_DONTUNMAP)) {
>
> No?
No. Because we dropped the requirement to use MREMAP_FIXED with
MREMAP_DONTUNMAP, if we're not using MREMAP_FIXED we don't need to
unmap anything at dest if it already exists because
get_unmapped_area() below will not be using the MAP_FIXED flag either,
instead it will search for a new unmapped area. If we were to change
it then we wouldn't be able to do MREMAP_FIXED | MREMAP_DONTUNMAP, so
I think this is correct.
> > - if (flags & MREMAP_FIXED) {
> > + if (flags & MREMAP_FIXED || flags & MREMAP_DONTUNMAP) {
>
> if (flags & (MREMAP_FIXED | MREMAP_DONTUNMAP)) {
Sure, I can change that.
If you're good with all of that I can mail a new patch today.
Thanks again,
Brian
^ permalink raw reply
* [PATCH AUTOSEL 4.19 120/252] net/wan/fsl_ucc_hdlc: remove set but not used variables 'ut_info' and 'ret'
From: Sasha Levin @ 2020-02-14 16:09 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, Chen Zhou, netdev, Hulk Robot, linuxppc-dev,
David S . Miller
In-Reply-To: <20200214161147.15842-1-sashal@kernel.org>
From: Chen Zhou <chenzhou10@huawei.com>
[ Upstream commit 270fe2ceda66b6964d4c6f261d7f562a02c1c786 ]
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/net/wan/fsl_ucc_hdlc.c: In function ucc_hdlc_irq_handler:
drivers/net/wan/fsl_ucc_hdlc.c:643:23:
warning: variable ut_info set but not used [-Wunused-but-set-variable]
drivers/net/wan/fsl_ucc_hdlc.c: In function uhdlc_suspend:
drivers/net/wan/fsl_ucc_hdlc.c:880:23:
warning: variable ut_info set but not used [-Wunused-but-set-variable]
drivers/net/wan/fsl_ucc_hdlc.c: In function uhdlc_resume:
drivers/net/wan/fsl_ucc_hdlc.c:925:6:
warning: variable ret set but not used [-Wunused-but-set-variable]
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/wan/fsl_ucc_hdlc.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 9ab04ef532f34..bb560f1d9a48c 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -591,11 +591,9 @@ static irqreturn_t ucc_hdlc_irq_handler(int irq, void *dev_id)
struct ucc_hdlc_private *priv = (struct ucc_hdlc_private *)dev_id;
struct net_device *dev = priv->ndev;
struct ucc_fast_private *uccf;
- struct ucc_tdm_info *ut_info;
u32 ucce;
u32 uccm;
- ut_info = priv->ut_info;
uccf = priv->uccf;
ucce = ioread32be(uccf->p_ucce);
@@ -825,7 +823,6 @@ static void resume_clk_config(struct ucc_hdlc_private *priv)
static int uhdlc_suspend(struct device *dev)
{
struct ucc_hdlc_private *priv = dev_get_drvdata(dev);
- struct ucc_tdm_info *ut_info;
struct ucc_fast __iomem *uf_regs;
if (!priv)
@@ -837,7 +834,6 @@ static int uhdlc_suspend(struct device *dev)
netif_device_detach(priv->ndev);
napi_disable(&priv->napi);
- ut_info = priv->ut_info;
uf_regs = priv->uf_regs;
/* backup gumr guemr*/
@@ -870,7 +866,7 @@ static int uhdlc_resume(struct device *dev)
struct ucc_fast __iomem *uf_regs;
struct ucc_fast_private *uccf;
struct ucc_fast_info *uf_info;
- int ret, i;
+ int i;
u32 cecr_subblock;
u16 bd_status;
@@ -915,16 +911,16 @@ static int uhdlc_resume(struct device *dev)
/* Write to QE CECR, UCCx channel to Stop Transmission */
cecr_subblock = ucc_fast_get_qe_cr_subblock(uf_info->ucc_num);
- ret = qe_issue_cmd(QE_STOP_TX, cecr_subblock,
- (u8)QE_CR_PROTOCOL_UNSPECIFIED, 0);
+ qe_issue_cmd(QE_STOP_TX, cecr_subblock,
+ (u8)QE_CR_PROTOCOL_UNSPECIFIED, 0);
/* Set UPSMR normal mode */
iowrite32be(0, &uf_regs->upsmr);
/* init parameter base */
cecr_subblock = ucc_fast_get_qe_cr_subblock(uf_info->ucc_num);
- ret = qe_issue_cmd(QE_ASSIGN_PAGE_TO_DEVICE, cecr_subblock,
- QE_CR_PROTOCOL_UNSPECIFIED, priv->ucc_pram_offset);
+ qe_issue_cmd(QE_ASSIGN_PAGE_TO_DEVICE, cecr_subblock,
+ QE_CR_PROTOCOL_UNSPECIFIED, priv->ucc_pram_offset);
priv->ucc_pram = (struct ucc_hdlc_param __iomem *)
qe_muram_addr(priv->ucc_pram_offset);
--
2.20.1
^ permalink raw reply related
* Re: [alsa-devel] alsa-ucm-conf via portage on arm
From: Curtis Malainey @ 2020-02-14 18:45 UTC (permalink / raw)
To: Jaroslav Kysela
Cc: ALSA development, Dylan Reid, Takashi Iwai, Jimmy Cheng-Yi Chiang
In-Reply-To: <23daf96d-f854-f01f-bc4e-2afc47299c0a@perex.cz>
Not yet, that was going to be my follow up. I just wanted to make sure
there is not ALSA restriction first. I assumed there wasn't but
figured better safe than sorry. I will follow up with Lars.
On Fri, Feb 14, 2020 at 10:39 AM Jaroslav Kysela <perex@perex.cz> wrote:
>
> Dne 14. 02. 20 v 19:23 Curtis Malainey napsal(a):
> > Hello Takashi and Jaroslav,
> >
> > There appears to be a upstream package issue for alsa-ucm-conf in that
> > portage seems to have it restricted to x86 and amd64 platforms. Given
> > the nature of the ucms I figure this is an error and was hoping you
> > know who to speak to in order to resolve this. If this isn't an error
> > what can I do to help get the repo to state in which it is supported
> > on arm/arm64?
> >
> > See https://gitweb.gentoo.org/repo/gentoo.git/commit/media-libs/alsa-ucm-conf/alsa-ucm-conf-1.2.1.2.ebuild?id=dd3e0570e2465639431709bce0410b787d312bbe
> >
> > Curtis
>
> It's Gentoo specific and you see signed-off-by in this commit.
>
> Did you contact this person (Lars Wendler) from Gentoo?
>
> Jaroslav
>
> --
> Jaroslav Kysela <perex@perex.cz>
> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply
* Re: [PATCH] phy: core: Add consumer device link support
From: Andy Shevchenko @ 2020-02-14 18:46 UTC (permalink / raw)
To: Alexandre Torgue
Cc: Kishon Vijay Abraham I, youling 257, Yoshihiro Shimoda,
Greg Kroah-Hartman, Linux Kernel Mailing List, USB, saravanak
In-Reply-To: <1cd5885d-7db4-59b9-ef2d-e3556f60ca68@st.com>
On Mon, Feb 10, 2020 at 1:32 PM Alexandre Torgue
<alexandre.torgue@st.com> wrote:
> On 2/10/20 9:08 AM, Kishon Vijay Abraham I wrote:
> > On 07/02/20 12:27 PM, youling 257 wrote:
> >> test this diff, dwc3 work for my device, thanks.
> >>
> >> 2020-02-07 13:16 GMT+08:00, Kishon Vijay Abraham I <kishon@ti.com>:
> >>> On 06/02/20 7:09 PM, youling257 wrote:
> >>>> This patch cause "dwc3 dwc3.3.auto: failed to create device link to
> >>>> dwc3.3.auto.ulpi" problem.
> >>>> https://bugzilla.kernel.org/show_bug.cgi?id=206435
+1 to the report.
Please revert for v5.6 or provide a fix ASAP!
> >>>
> >>> I'm suspecting there is some sort of reverse dependency with dwc3 ULPI.
> >>> Can you try the following diff?
> >>>
> >>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> >>> index 2eb28cc2d2dc..397311dcb116 100644
> >>> --- a/drivers/phy/phy-core.c
> >>> +++ b/drivers/phy/phy-core.c
> >>> @@ -687,7 +687,7 @@ struct phy *phy_get(struct device *dev, const char
> >>> *string)
> >>>
> >>> get_device(&phy->dev);
> >>>
> >>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> >>> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
> >>> if (!link) {
> >>> dev_err(dev, "failed to create device link to %s\n",
> >>> dev_name(phy->dev.parent));
> >>> @@ -802,7 +802,7 @@ struct phy *devm_of_phy_get(struct device *dev,
> >>> struct device_node *np,
> >>> return phy;
> >>> }
> >>>
> >>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> >>> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
> >>> if (!link) {
> >>> dev_err(dev, "failed to create device link to %s\n",
> >>> dev_name(phy->dev.parent));
> >>> @@ -851,7 +851,7 @@ struct phy *devm_of_phy_get_by_index(struct device
> >>> *dev, struct device_node *np,
> >>> *ptr = phy;
> >>> devres_add(dev, ptr);
> >>>
> >>> - link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> >>> + link = device_link_add(dev, &phy->dev, DL_FLAG_SYNC_STATE_ONLY);
> >>> if (!link) {
> >>> dev_err(dev, "failed to create device link to %s\n",
> >>> dev_name(phy->dev.parent));Parent
> >
> > Can you check if this doesn't affect the suspend/resume ordering?
>
> With this fix, suspend/resume ordering is broken on my side. What do you
> think to keep the STATELESS flag and to only display a warn if
> "device_link_add" returns an error ? It's not "smart" but it could
> solved our issue.
>
> As a lot of improvements have been recently done on device link topic by
> Saravana, we could check with him what is the way to follow.
>
> Regards
> Alex
>
> >
> > Thanks
> > Kishon
> >
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.