* [PATCH] [v7] nouveau: add command-line GSP-RM registry support
@ 2024-04-17 21:53 Timur Tabi
2024-04-18 15:57 ` Danilo Krummrich
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Timur Tabi @ 2024-04-17 21:53 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich, Dave Airlie, bskeggs, nouveau
Add the NVreg_RegistryDwords command line parameter, which allows
specifying additional registry keys to be sent to GSP-RM. This
allows additional configuration, debugging, and experimentation
with GSP-RM, which uses these keys to alter its behavior.
Note that these keys are passed as-is to GSP-RM, and Nouveau does
not parse them. This is in contrast to the Nvidia driver, which may
parse some of the keys to configure some functionality in concert with
GSP-RM. Therefore, any keys which also require action by the driver
may not function correctly when passed by Nouveau. Caveat emptor.
The name and format of NVreg_RegistryDwords is the same as used by
the Nvidia driver, to maintain compatibility.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
v7:
rebase onto drm-misc-next
rename vlen to alloc_size
.../gpu/drm/nouveau/include/nvkm/subdev/gsp.h | 6 +
.../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c | 355 ++++++++++++++++--
2 files changed, 337 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
index 6f5d376d8fcc..3fbc57b16a05 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
@@ -211,6 +211,12 @@ struct nvkm_gsp {
struct mutex mutex;;
struct idr idr;
} client_id;
+
+ /* A linked list of registry items. The registry RPC will be built from it. */
+ struct list_head registry_list;
+
+ /* The size of the registry RPC */
+ size_t registry_rpc_size;
};
static inline bool
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 9858c1438aa7..1b5d5b02c640 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -54,6 +54,8 @@
#include <nvrm/535.113.01/nvidia/kernel/inc/vgpu/rpc_global_enums.h>
#include <linux/acpi.h>
+#include <linux/ctype.h>
+#include <linux/parser.h>
#define GSP_MSG_MIN_SIZE GSP_PAGE_SIZE
#define GSP_MSG_MAX_SIZE GSP_PAGE_MIN_SIZE * 16
@@ -1080,51 +1082,356 @@ r535_gsp_rpc_unloading_guest_driver(struct nvkm_gsp *gsp, bool suspend)
return nvkm_gsp_rpc_wr(gsp, rpc, true);
}
+enum registry_type {
+ REGISTRY_TABLE_ENTRY_TYPE_DWORD = 1, /* 32-bit unsigned integer */
+ REGISTRY_TABLE_ENTRY_TYPE_BINARY = 2, /* Binary blob */
+ REGISTRY_TABLE_ENTRY_TYPE_STRING = 3, /* Null-terminated string */
+};
+
+/* An arbitrary limit to the length of a registry key */
+#define REGISTRY_MAX_KEY_LENGTH 64
+
+/**
+ * registry_list_entry - linked list member for a registry key/value
+ * @head: list_head struct
+ * @type: dword, binary, or string
+ * @klen: the length of name of the key
+ * @vlen: the length of the value
+ * @key: the key name
+ * @dword: the data, if REGISTRY_TABLE_ENTRY_TYPE_DWORD
+ * @binary: the data, if TYPE_BINARY or TYPE_STRING
+ *
+ * Every registry key/value is represented internally by this struct.
+ *
+ * Type DWORD is a simple 32-bit unsigned integer, and its value is stored in
+ * @dword.
+ *
+ * Types BINARY and STRING are variable-length binary blobs. The only real
+ * difference between BINARY and STRING is that STRING is null-terminated and
+ * is expected to contain only printable characters.
+ *
+ * Note: it is technically possible to have multiple keys with the same name
+ * but different types, but this is not useful since GSP-RM expects keys to
+ * have only one specific type.
+ */
+struct registry_list_entry {
+ struct list_head head;
+ enum registry_type type;
+ size_t klen;
+ char key[REGISTRY_MAX_KEY_LENGTH];
+ size_t vlen;
+ u32 dword; /* TYPE_DWORD */
+ u8 binary[] __counted_by(vlen); /* TYPE_BINARY or TYPE_STRING */
+};
+
+/**
+ * add_registry -- adds a registry entry
+ * @gsp: gsp pointer
+ * @key: name of the registry key
+ * @type: type of data
+ * @data: pointer to value
+ * @length: size of data, in bytes
+ *
+ * Adds a registry key/value pair to the registry database.
+ *
+ * This function collects the registry information in a linked list. After
+ * all registry keys have been added, build_registry() is used to create the
+ * RPC data structure.
+ *
+ * registry_rpc_size is a running total of the size of all registry keys.
+ * It's used to avoid an O(n) calculation of the size when the RPC is built.
+ *
+ * Returns 0 on success, or negative error code on error.
+ */
+static int add_registry(struct nvkm_gsp *gsp, const char *key,
+ enum registry_type type, const void *data, size_t length)
+{
+ struct registry_list_entry *reg;
+ const size_t nlen = strnlen(key, REGISTRY_MAX_KEY_LENGTH) + 1;
+ size_t alloc_size; /* extra bytes to alloc for binary or string value */
+
+ if (nlen > REGISTRY_MAX_KEY_LENGTH)
+ return -EINVAL;
+
+ alloc_size = (type == REGISTRY_TABLE_ENTRY_TYPE_DWORD) ? 0 : length;
+
+ reg = kmalloc(sizeof(*reg) + alloc_size, GFP_KERNEL);
+ if (!reg)
+ return -ENOMEM;
+
+ switch (type) {
+ case REGISTRY_TABLE_ENTRY_TYPE_DWORD:
+ reg->dword = *(const u32 *)(data);
+ break;
+ case REGISTRY_TABLE_ENTRY_TYPE_BINARY:
+ case REGISTRY_TABLE_ENTRY_TYPE_STRING:
+ memcpy(reg->binary, data, alloc_size);
+ break;
+ default:
+ nvkm_error(&gsp->subdev, "unrecognized registry type %u for '%s'\n",
+ type, key);
+ kfree(reg);
+ return -EINVAL;
+ }
+
+ memcpy(reg->key, key, nlen);
+ reg->klen = nlen;
+ reg->vlen = length;
+ reg->type = type;
+
+ list_add_tail(®->head, &gsp->registry_list);
+ gsp->registry_rpc_size += sizeof(PACKED_REGISTRY_ENTRY) + nlen + alloc_size;
+
+ return 0;
+}
+
+static int add_registry_num(struct nvkm_gsp *gsp, const char *key, u32 value)
+{
+ return add_registry(gsp, key, REGISTRY_TABLE_ENTRY_TYPE_DWORD,
+ &value, sizeof(u32));
+}
+
+static int add_registry_string(struct nvkm_gsp *gsp, const char *key, const char *value)
+{
+ return add_registry(gsp, key, REGISTRY_TABLE_ENTRY_TYPE_STRING,
+ value, strlen(value) + 1);
+}
+
+/**
+ * build_registry -- create the registry RPC data
+ * @gsp: gsp pointer
+ * @registry: pointer to the RPC payload to fill
+ *
+ * After all registry key/value pairs have been added, call this function to
+ * build the RPC.
+ *
+ * The registry RPC looks like this:
+ *
+ * +-----------------+
+ * |NvU32 size; |
+ * |NvU32 numEntries;|
+ * +-----------------+
+ * +----------------------------------------+
+ * |PACKED_REGISTRY_ENTRY |
+ * +----------------------------------------+
+ * |Null-terminated key (string) for entry 0|
+ * +----------------------------------------+
+ * |Binary/string data value for entry 0 | (only if necessary)
+ * +----------------------------------------+
+ *
+ * +----------------------------------------+
+ * |PACKED_REGISTRY_ENTRY |
+ * +----------------------------------------+
+ * |Null-terminated key (string) for entry 1|
+ * +----------------------------------------+
+ * |Binary/string data value for entry 1 | (only if necessary)
+ * +----------------------------------------+
+ * ... (and so on, one copy for each entry)
+ *
+ *
+ * The 'data' field of an entry is either a 32-bit integer (for type DWORD)
+ * or an offset into the PACKED_REGISTRY_TABLE (for types BINARY and STRING).
+ *
+ * All memory allocated by add_registry() is released.
+ */
+static void build_registry(struct nvkm_gsp *gsp, PACKED_REGISTRY_TABLE *registry)
+{
+ struct registry_list_entry *reg, *n;
+ size_t str_offset;
+ unsigned int i = 0;
+
+ registry->numEntries = list_count_nodes(&gsp->registry_list);
+ str_offset = struct_size(registry, entries, registry->numEntries);
+
+ list_for_each_entry_safe(reg, n, &gsp->registry_list, head) {
+ registry->entries[i].type = reg->type;
+ registry->entries[i].length = reg->vlen;
+
+ /* Append the key name to the table */
+ registry->entries[i].nameOffset = str_offset;
+ memcpy((void *)registry + str_offset, reg->key, reg->klen);
+ str_offset += reg->klen;
+
+ switch (reg->type) {
+ case REGISTRY_TABLE_ENTRY_TYPE_DWORD:
+ registry->entries[i].data = reg->dword;
+ break;
+ case REGISTRY_TABLE_ENTRY_TYPE_BINARY:
+ case REGISTRY_TABLE_ENTRY_TYPE_STRING:
+ /* If the type is binary or string, also append the value */
+ memcpy((void *)registry + str_offset, reg->binary, reg->vlen);
+ registry->entries[i].data = str_offset;
+ str_offset += reg->vlen;
+ break;
+ default:
+ }
+
+ i++;
+ list_del(®->head);
+ kfree(reg);
+ }
+
+ /* Double-check that we calculated the sizes correctly */
+ WARN_ON(gsp->registry_rpc_size != str_offset);
+
+ registry->size = gsp->registry_rpc_size;
+}
+
+/**
+ * clean_registry -- clean up registry memory in case of error
+ * @gsp: gsp pointer
+ *
+ * Call this function to clean up all memory allocated by add_registry()
+ * in case of error and build_registry() is not called.
+ */
+static void clean_registry(struct nvkm_gsp *gsp)
+{
+ struct registry_list_entry *reg, *n;
+
+ list_for_each_entry_safe(reg, n, &gsp->registry_list, head) {
+ list_del(®->head);
+ kfree(reg);
+ }
+
+ gsp->registry_rpc_size = sizeof(PACKED_REGISTRY_TABLE);
+}
+
+MODULE_PARM_DESC(NVreg_RegistryDwords,
+ "A semicolon-separated list of key=integer pairs of GSP-RM registry keys");
+static char *NVreg_RegistryDwords;
+module_param(NVreg_RegistryDwords, charp, 0400);
+
/* dword only */
struct nv_gsp_registry_entries {
const char *name;
u32 value;
};
+/**
+ * r535_registry_entries - required registry entries for GSP-RM
+ *
+ * This array lists registry entries that are required for GSP-RM to
+ * function correctly.
+ *
+ * RMSecBusResetEnable - enables PCI secondary bus reset
+ * RMForcePcieConfigSave - forces GSP-RM to preserve PCI configuration
+ * registers on any PCI reset.
+ */
static const struct nv_gsp_registry_entries r535_registry_entries[] = {
{ "RMSecBusResetEnable", 1 },
{ "RMForcePcieConfigSave", 1 },
};
#define NV_GSP_REG_NUM_ENTRIES ARRAY_SIZE(r535_registry_entries)
+/**
+ * strip - strips all characters in 'reject' from 's'
+ * @s: string to strip
+ * @reject: string of characters to remove
+ *
+ * 's' is modified.
+ *
+ * Returns the length of the new string.
+ */
+static size_t strip(char *s, const char *reject)
+{
+ char *p = s, *p2 = s;
+ size_t length = 0;
+ char c;
+
+ do {
+ while ((c = *p2) && strchr(reject, c))
+ p2++;
+
+ *p++ = c = *p2++;
+ length++;
+ } while (c);
+
+ return length;
+}
+
+/**
+ * r535_gsp_rpc_set_registry - build registry RPC and call GSP-RM
+ * @gsp: gsp pointer
+ *
+ * The GSP-RM registry is a set of key/value pairs that configure some aspects
+ * of GSP-RM. The keys are strings, and the values are 32-bit integers.
+ *
+ * The registry is built from a combination of a static hard-coded list (see
+ * above) and entries passed on the driver's command line.
+ */
static int
r535_gsp_rpc_set_registry(struct nvkm_gsp *gsp)
{
PACKED_REGISTRY_TABLE *rpc;
- char *strings;
- int str_offset;
- int i;
- size_t rpc_size = struct_size(rpc, entries, NV_GSP_REG_NUM_ENTRIES);
+ unsigned int i;
+ int ret;
- /* add strings + null terminator */
- for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++)
- rpc_size += strlen(r535_registry_entries[i].name) + 1;
+ INIT_LIST_HEAD(&gsp->registry_list);
+ gsp->registry_rpc_size = sizeof(PACKED_REGISTRY_TABLE);
- rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_SET_REGISTRY, rpc_size);
- if (IS_ERR(rpc))
- return PTR_ERR(rpc);
+ for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) {
+ ret = add_registry_num(gsp, r535_registry_entries[i].name,
+ r535_registry_entries[i].value);
+ if (ret) {
+ clean_registry(gsp);
+ return ret;
+ }
+ }
- rpc->numEntries = NV_GSP_REG_NUM_ENTRIES;
+ /*
+ * The NVreg_RegistryDwords parameter is a string of key=value
+ * pairs separated by semicolons. We need to extract and trim each
+ * substring, and then parse the substring to extract the key and
+ * value.
+ */
+ if (NVreg_RegistryDwords) {
+ char *p = kstrdup(NVreg_RegistryDwords, GFP_KERNEL);
+ char *start, *next = p, *equal;
+ size_t length;
+
+ /* Remove any whitespace from the parameter string */
+ length = strip(p, " \t\n");
+
+ while ((start = strsep(&next, ";"))) {
+ long value;
+
+ equal = strchr(start, '=');
+ if (!equal || equal == start || equal[1] == 0) {
+ nvkm_error(&gsp->subdev,
+ "ignoring invalid registry string '%s'\n",
+ start);
+ continue;
+ }
- str_offset = offsetof(typeof(*rpc), entries[NV_GSP_REG_NUM_ENTRIES]);
- strings = (char *)rpc + str_offset;
- for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) {
- int name_len = strlen(r535_registry_entries[i].name) + 1;
-
- rpc->entries[i].nameOffset = str_offset;
- rpc->entries[i].type = 1;
- rpc->entries[i].data = r535_registry_entries[i].value;
- rpc->entries[i].length = 4;
- memcpy(strings, r535_registry_entries[i].name, name_len);
- strings += name_len;
- str_offset += name_len;
+ /* Truncate the key=value string to just key */
+ *equal = 0;
+
+ ret = kstrtol(equal + 1, 0, &value);
+ if (!ret) {
+ ret = add_registry_num(gsp, start, value);
+ } else {
+ /* Not a number, so treat it as a string */
+ ret = add_registry_string(gsp, start, equal + 1);
+ }
+
+ if (ret) {
+ nvkm_error(&gsp->subdev,
+ "ignoring invalid registry key/value '%s=%s'\n",
+ start, equal + 1);
+ continue;
+ }
+ }
+
+ kfree(p);
}
- rpc->size = str_offset;
+
+ rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_SET_REGISTRY, gsp->registry_rpc_size);
+ if (IS_ERR(rpc)) {
+ clean_registry(gsp);
+ return PTR_ERR(rpc);
+ }
+
+ build_registry(gsp, rpc);
return nvkm_gsp_rpc_wr(gsp, rpc, false);
}
base-commit: f7ad2ce5fd89ab5d146da8f486a310746df5dc9e
prerequisite-patch-id: 9bb653b6a53dcba4171d653e24a242461374f9fe
prerequisite-patch-id: 7093a9db84053e43f6f278bf1d159a25d14ceebf
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] [v7] nouveau: add command-line GSP-RM registry support
2024-04-17 21:53 [PATCH] [v7] nouveau: add command-line GSP-RM registry support Timur Tabi
@ 2024-04-18 15:57 ` Danilo Krummrich
2024-04-25 13:22 ` Danilo Krummrich
[not found] ` <c5ff8d3e-ecfc-4970-86c0-540b75b4be2e@ti.com>
2 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2024-04-18 15:57 UTC (permalink / raw)
To: Dave Airlie, daniel, Maarten Lankhorst, mripard, tzimmermann
Cc: Timur Tabi, Lyude Paul, bskeggs, nouveau
Hi,
this patch targets drm-misc-next but depends on commit 838ae9f45c4e ("nouveau/gsp:
Avoid addressing beyond end of rpc->entries") which is only in drm-misc-fixes.
Please let me know if you want to backmerge directly, let me hold the patch back
until or anything else.
- Danilo
On 4/17/24 23:53, Timur Tabi wrote:
> Add the NVreg_RegistryDwords command line parameter, which allows
> specifying additional registry keys to be sent to GSP-RM. This
> allows additional configuration, debugging, and experimentation
> with GSP-RM, which uses these keys to alter its behavior.
>
> Note that these keys are passed as-is to GSP-RM, and Nouveau does
> not parse them. This is in contrast to the Nvidia driver, which may
> parse some of the keys to configure some functionality in concert with
> GSP-RM. Therefore, any keys which also require action by the driver
> may not function correctly when passed by Nouveau. Caveat emptor.
>
> The name and format of NVreg_RegistryDwords is the same as used by
> the Nvidia driver, to maintain compatibility.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
> v7:
> rebase onto drm-misc-next
> rename vlen to alloc_size
>
> .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h | 6 +
> .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c | 355 ++++++++++++++++--
> 2 files changed, 337 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
> index 6f5d376d8fcc..3fbc57b16a05 100644
> --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
> +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
> @@ -211,6 +211,12 @@ struct nvkm_gsp {
> struct mutex mutex;;
> struct idr idr;
> } client_id;
> +
> + /* A linked list of registry items. The registry RPC will be built from it. */
> + struct list_head registry_list;
> +
> + /* The size of the registry RPC */
> + size_t registry_rpc_size;
> };
>
> static inline bool
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
> index 9858c1438aa7..1b5d5b02c640 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
> @@ -54,6 +54,8 @@
> #include <nvrm/535.113.01/nvidia/kernel/inc/vgpu/rpc_global_enums.h>
>
> #include <linux/acpi.h>
> +#include <linux/ctype.h>
> +#include <linux/parser.h>
>
> #define GSP_MSG_MIN_SIZE GSP_PAGE_SIZE
> #define GSP_MSG_MAX_SIZE GSP_PAGE_MIN_SIZE * 16
> @@ -1080,51 +1082,356 @@ r535_gsp_rpc_unloading_guest_driver(struct nvkm_gsp *gsp, bool suspend)
> return nvkm_gsp_rpc_wr(gsp, rpc, true);
> }
>
> +enum registry_type {
> + REGISTRY_TABLE_ENTRY_TYPE_DWORD = 1, /* 32-bit unsigned integer */
> + REGISTRY_TABLE_ENTRY_TYPE_BINARY = 2, /* Binary blob */
> + REGISTRY_TABLE_ENTRY_TYPE_STRING = 3, /* Null-terminated string */
> +};
> +
> +/* An arbitrary limit to the length of a registry key */
> +#define REGISTRY_MAX_KEY_LENGTH 64
> +
> +/**
> + * registry_list_entry - linked list member for a registry key/value
> + * @head: list_head struct
> + * @type: dword, binary, or string
> + * @klen: the length of name of the key
> + * @vlen: the length of the value
> + * @key: the key name
> + * @dword: the data, if REGISTRY_TABLE_ENTRY_TYPE_DWORD
> + * @binary: the data, if TYPE_BINARY or TYPE_STRING
> + *
> + * Every registry key/value is represented internally by this struct.
> + *
> + * Type DWORD is a simple 32-bit unsigned integer, and its value is stored in
> + * @dword.
> + *
> + * Types BINARY and STRING are variable-length binary blobs. The only real
> + * difference between BINARY and STRING is that STRING is null-terminated and
> + * is expected to contain only printable characters.
> + *
> + * Note: it is technically possible to have multiple keys with the same name
> + * but different types, but this is not useful since GSP-RM expects keys to
> + * have only one specific type.
> + */
> +struct registry_list_entry {
> + struct list_head head;
> + enum registry_type type;
> + size_t klen;
> + char key[REGISTRY_MAX_KEY_LENGTH];
> + size_t vlen;
> + u32 dword; /* TYPE_DWORD */
> + u8 binary[] __counted_by(vlen); /* TYPE_BINARY or TYPE_STRING */
> +};
> +
> +/**
> + * add_registry -- adds a registry entry
> + * @gsp: gsp pointer
> + * @key: name of the registry key
> + * @type: type of data
> + * @data: pointer to value
> + * @length: size of data, in bytes
> + *
> + * Adds a registry key/value pair to the registry database.
> + *
> + * This function collects the registry information in a linked list. After
> + * all registry keys have been added, build_registry() is used to create the
> + * RPC data structure.
> + *
> + * registry_rpc_size is a running total of the size of all registry keys.
> + * It's used to avoid an O(n) calculation of the size when the RPC is built.
> + *
> + * Returns 0 on success, or negative error code on error.
> + */
> +static int add_registry(struct nvkm_gsp *gsp, const char *key,
> + enum registry_type type, const void *data, size_t length)
> +{
> + struct registry_list_entry *reg;
> + const size_t nlen = strnlen(key, REGISTRY_MAX_KEY_LENGTH) + 1;
> + size_t alloc_size; /* extra bytes to alloc for binary or string value */
> +
> + if (nlen > REGISTRY_MAX_KEY_LENGTH)
> + return -EINVAL;
> +
> + alloc_size = (type == REGISTRY_TABLE_ENTRY_TYPE_DWORD) ? 0 : length;
> +
> + reg = kmalloc(sizeof(*reg) + alloc_size, GFP_KERNEL);
> + if (!reg)
> + return -ENOMEM;
> +
> + switch (type) {
> + case REGISTRY_TABLE_ENTRY_TYPE_DWORD:
> + reg->dword = *(const u32 *)(data);
> + break;
> + case REGISTRY_TABLE_ENTRY_TYPE_BINARY:
> + case REGISTRY_TABLE_ENTRY_TYPE_STRING:
> + memcpy(reg->binary, data, alloc_size);
> + break;
> + default:
> + nvkm_error(&gsp->subdev, "unrecognized registry type %u for '%s'\n",
> + type, key);
> + kfree(reg);
> + return -EINVAL;
> + }
> +
> + memcpy(reg->key, key, nlen);
> + reg->klen = nlen;
> + reg->vlen = length;
> + reg->type = type;
> +
> + list_add_tail(®->head, &gsp->registry_list);
> + gsp->registry_rpc_size += sizeof(PACKED_REGISTRY_ENTRY) + nlen + alloc_size;
> +
> + return 0;
> +}
> +
> +static int add_registry_num(struct nvkm_gsp *gsp, const char *key, u32 value)
> +{
> + return add_registry(gsp, key, REGISTRY_TABLE_ENTRY_TYPE_DWORD,
> + &value, sizeof(u32));
> +}
> +
> +static int add_registry_string(struct nvkm_gsp *gsp, const char *key, const char *value)
> +{
> + return add_registry(gsp, key, REGISTRY_TABLE_ENTRY_TYPE_STRING,
> + value, strlen(value) + 1);
> +}
> +
> +/**
> + * build_registry -- create the registry RPC data
> + * @gsp: gsp pointer
> + * @registry: pointer to the RPC payload to fill
> + *
> + * After all registry key/value pairs have been added, call this function to
> + * build the RPC.
> + *
> + * The registry RPC looks like this:
> + *
> + * +-----------------+
> + * |NvU32 size; |
> + * |NvU32 numEntries;|
> + * +-----------------+
> + * +----------------------------------------+
> + * |PACKED_REGISTRY_ENTRY |
> + * +----------------------------------------+
> + * |Null-terminated key (string) for entry 0|
> + * +----------------------------------------+
> + * |Binary/string data value for entry 0 | (only if necessary)
> + * +----------------------------------------+
> + *
> + * +----------------------------------------+
> + * |PACKED_REGISTRY_ENTRY |
> + * +----------------------------------------+
> + * |Null-terminated key (string) for entry 1|
> + * +----------------------------------------+
> + * |Binary/string data value for entry 1 | (only if necessary)
> + * +----------------------------------------+
> + * ... (and so on, one copy for each entry)
> + *
> + *
> + * The 'data' field of an entry is either a 32-bit integer (for type DWORD)
> + * or an offset into the PACKED_REGISTRY_TABLE (for types BINARY and STRING).
> + *
> + * All memory allocated by add_registry() is released.
> + */
> +static void build_registry(struct nvkm_gsp *gsp, PACKED_REGISTRY_TABLE *registry)
> +{
> + struct registry_list_entry *reg, *n;
> + size_t str_offset;
> + unsigned int i = 0;
> +
> + registry->numEntries = list_count_nodes(&gsp->registry_list);
> + str_offset = struct_size(registry, entries, registry->numEntries);
> +
> + list_for_each_entry_safe(reg, n, &gsp->registry_list, head) {
> + registry->entries[i].type = reg->type;
> + registry->entries[i].length = reg->vlen;
> +
> + /* Append the key name to the table */
> + registry->entries[i].nameOffset = str_offset;
> + memcpy((void *)registry + str_offset, reg->key, reg->klen);
> + str_offset += reg->klen;
> +
> + switch (reg->type) {
> + case REGISTRY_TABLE_ENTRY_TYPE_DWORD:
> + registry->entries[i].data = reg->dword;
> + break;
> + case REGISTRY_TABLE_ENTRY_TYPE_BINARY:
> + case REGISTRY_TABLE_ENTRY_TYPE_STRING:
> + /* If the type is binary or string, also append the value */
> + memcpy((void *)registry + str_offset, reg->binary, reg->vlen);
> + registry->entries[i].data = str_offset;
> + str_offset += reg->vlen;
> + break;
> + default:
> + }
> +
> + i++;
> + list_del(®->head);
> + kfree(reg);
> + }
> +
> + /* Double-check that we calculated the sizes correctly */
> + WARN_ON(gsp->registry_rpc_size != str_offset);
> +
> + registry->size = gsp->registry_rpc_size;
> +}
> +
> +/**
> + * clean_registry -- clean up registry memory in case of error
> + * @gsp: gsp pointer
> + *
> + * Call this function to clean up all memory allocated by add_registry()
> + * in case of error and build_registry() is not called.
> + */
> +static void clean_registry(struct nvkm_gsp *gsp)
> +{
> + struct registry_list_entry *reg, *n;
> +
> + list_for_each_entry_safe(reg, n, &gsp->registry_list, head) {
> + list_del(®->head);
> + kfree(reg);
> + }
> +
> + gsp->registry_rpc_size = sizeof(PACKED_REGISTRY_TABLE);
> +}
> +
> +MODULE_PARM_DESC(NVreg_RegistryDwords,
> + "A semicolon-separated list of key=integer pairs of GSP-RM registry keys");
> +static char *NVreg_RegistryDwords;
> +module_param(NVreg_RegistryDwords, charp, 0400);
> +
> /* dword only */
> struct nv_gsp_registry_entries {
> const char *name;
> u32 value;
> };
>
> +/**
> + * r535_registry_entries - required registry entries for GSP-RM
> + *
> + * This array lists registry entries that are required for GSP-RM to
> + * function correctly.
> + *
> + * RMSecBusResetEnable - enables PCI secondary bus reset
> + * RMForcePcieConfigSave - forces GSP-RM to preserve PCI configuration
> + * registers on any PCI reset.
> + */
> static const struct nv_gsp_registry_entries r535_registry_entries[] = {
> { "RMSecBusResetEnable", 1 },
> { "RMForcePcieConfigSave", 1 },
> };
> #define NV_GSP_REG_NUM_ENTRIES ARRAY_SIZE(r535_registry_entries)
>
> +/**
> + * strip - strips all characters in 'reject' from 's'
> + * @s: string to strip
> + * @reject: string of characters to remove
> + *
> + * 's' is modified.
> + *
> + * Returns the length of the new string.
> + */
> +static size_t strip(char *s, const char *reject)
> +{
> + char *p = s, *p2 = s;
> + size_t length = 0;
> + char c;
> +
> + do {
> + while ((c = *p2) && strchr(reject, c))
> + p2++;
> +
> + *p++ = c = *p2++;
> + length++;
> + } while (c);
> +
> + return length;
> +}
> +
> +/**
> + * r535_gsp_rpc_set_registry - build registry RPC and call GSP-RM
> + * @gsp: gsp pointer
> + *
> + * The GSP-RM registry is a set of key/value pairs that configure some aspects
> + * of GSP-RM. The keys are strings, and the values are 32-bit integers.
> + *
> + * The registry is built from a combination of a static hard-coded list (see
> + * above) and entries passed on the driver's command line.
> + */
> static int
> r535_gsp_rpc_set_registry(struct nvkm_gsp *gsp)
> {
> PACKED_REGISTRY_TABLE *rpc;
> - char *strings;
> - int str_offset;
> - int i;
> - size_t rpc_size = struct_size(rpc, entries, NV_GSP_REG_NUM_ENTRIES);
> + unsigned int i;
> + int ret;
>
> - /* add strings + null terminator */
> - for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++)
> - rpc_size += strlen(r535_registry_entries[i].name) + 1;
> + INIT_LIST_HEAD(&gsp->registry_list);
> + gsp->registry_rpc_size = sizeof(PACKED_REGISTRY_TABLE);
>
> - rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_SET_REGISTRY, rpc_size);
> - if (IS_ERR(rpc))
> - return PTR_ERR(rpc);
> + for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) {
> + ret = add_registry_num(gsp, r535_registry_entries[i].name,
> + r535_registry_entries[i].value);
> + if (ret) {
> + clean_registry(gsp);
> + return ret;
> + }
> + }
>
> - rpc->numEntries = NV_GSP_REG_NUM_ENTRIES;
> + /*
> + * The NVreg_RegistryDwords parameter is a string of key=value
> + * pairs separated by semicolons. We need to extract and trim each
> + * substring, and then parse the substring to extract the key and
> + * value.
> + */
> + if (NVreg_RegistryDwords) {
> + char *p = kstrdup(NVreg_RegistryDwords, GFP_KERNEL);
> + char *start, *next = p, *equal;
> + size_t length;
> +
> + /* Remove any whitespace from the parameter string */
> + length = strip(p, " \t\n");
> +
> + while ((start = strsep(&next, ";"))) {
> + long value;
> +
> + equal = strchr(start, '=');
> + if (!equal || equal == start || equal[1] == 0) {
> + nvkm_error(&gsp->subdev,
> + "ignoring invalid registry string '%s'\n",
> + start);
> + continue;
> + }
>
> - str_offset = offsetof(typeof(*rpc), entries[NV_GSP_REG_NUM_ENTRIES]);
> - strings = (char *)rpc + str_offset;
> - for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) {
> - int name_len = strlen(r535_registry_entries[i].name) + 1;
> -
> - rpc->entries[i].nameOffset = str_offset;
> - rpc->entries[i].type = 1;
> - rpc->entries[i].data = r535_registry_entries[i].value;
> - rpc->entries[i].length = 4;
> - memcpy(strings, r535_registry_entries[i].name, name_len);
> - strings += name_len;
> - str_offset += name_len;
> + /* Truncate the key=value string to just key */
> + *equal = 0;
> +
> + ret = kstrtol(equal + 1, 0, &value);
> + if (!ret) {
> + ret = add_registry_num(gsp, start, value);
> + } else {
> + /* Not a number, so treat it as a string */
> + ret = add_registry_string(gsp, start, equal + 1);
> + }
> +
> + if (ret) {
> + nvkm_error(&gsp->subdev,
> + "ignoring invalid registry key/value '%s=%s'\n",
> + start, equal + 1);
> + continue;
> + }
> + }
> +
> + kfree(p);
> }
> - rpc->size = str_offset;
> +
> + rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_SET_REGISTRY, gsp->registry_rpc_size);
> + if (IS_ERR(rpc)) {
> + clean_registry(gsp);
> + return PTR_ERR(rpc);
> + }
> +
> + build_registry(gsp, rpc);
>
> return nvkm_gsp_rpc_wr(gsp, rpc, false);
> }
>
> base-commit: f7ad2ce5fd89ab5d146da8f486a310746df5dc9e
> prerequisite-patch-id: 9bb653b6a53dcba4171d653e24a242461374f9fe
> prerequisite-patch-id: 7093a9db84053e43f6f278bf1d159a25d14ceebf
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [v7] nouveau: add command-line GSP-RM registry support
2024-04-17 21:53 [PATCH] [v7] nouveau: add command-line GSP-RM registry support Timur Tabi
2024-04-18 15:57 ` Danilo Krummrich
@ 2024-04-25 13:22 ` Danilo Krummrich
2024-04-25 16:38 ` Timur Tabi
[not found] ` <c5ff8d3e-ecfc-4970-86c0-540b75b4be2e@ti.com>
2 siblings, 1 reply; 8+ messages in thread
From: Danilo Krummrich @ 2024-04-25 13:22 UTC (permalink / raw)
To: Timur Tabi; +Cc: nouveau, Lyude Paul, Dave Airlie, bskeggs
On 4/17/24 23:53, Timur Tabi wrote:
<snip>
> +
> +/**
> + * r535_gsp_rpc_set_registry - build registry RPC and call GSP-RM
> + * @gsp: gsp pointer
> + *
> + * The GSP-RM registry is a set of key/value pairs that configure some aspects
> + * of GSP-RM. The keys are strings, and the values are 32-bit integers.
> + *
> + * The registry is built from a combination of a static hard-coded list (see
> + * above) and entries passed on the driver's command line.
> + */
> static int
> r535_gsp_rpc_set_registry(struct nvkm_gsp *gsp)
> {
> PACKED_REGISTRY_TABLE *rpc;
> - char *strings;
> - int str_offset;
> - int i;
> - size_t rpc_size = struct_size(rpc, entries, NV_GSP_REG_NUM_ENTRIES);
> + unsigned int i;
> + int ret;
>
> - /* add strings + null terminator */
> - for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++)
> - rpc_size += strlen(r535_registry_entries[i].name) + 1;
> + INIT_LIST_HEAD(&gsp->registry_list);
> + gsp->registry_rpc_size = sizeof(PACKED_REGISTRY_TABLE);
>
> - rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_SET_REGISTRY, rpc_size);
> - if (IS_ERR(rpc))
> - return PTR_ERR(rpc);
> + for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) {
> + ret = add_registry_num(gsp, r535_registry_entries[i].name,
> + r535_registry_entries[i].value);
> + if (ret) {
> + clean_registry(gsp);
> + return ret;
> + }
> + }
>
> - rpc->numEntries = NV_GSP_REG_NUM_ENTRIES;
> + /*
> + * The NVreg_RegistryDwords parameter is a string of key=value
> + * pairs separated by semicolons. We need to extract and trim each
> + * substring, and then parse the substring to extract the key and
> + * value.
> + */
> + if (NVreg_RegistryDwords) {
> + char *p = kstrdup(NVreg_RegistryDwords, GFP_KERNEL);
> + char *start, *next = p, *equal;
> + size_t length;
> +
> + /* Remove any whitespace from the parameter string */
> + length = strip(p, " \t\n");
With that, I see the following warning compiling this patch.
warning: variable ‘length’ set but not used [-Wunused-but-set-variable]
Did you intend to use the length for anything?
Also, looking at the warning made me aware of 'p' potentially being NULL.
If you agree, I can fix the warning and add the corresponding NULL check when
applying the patch.
- Danilo
> +
> + while ((start = strsep(&next, ";"))) {
> + long value;
> +
> + equal = strchr(start, '=');
> + if (!equal || equal == start || equal[1] == 0) {
> + nvkm_error(&gsp->subdev,
> + "ignoring invalid registry string '%s'\n",
> + start);
> + continue;
> + }
>
> - str_offset = offsetof(typeof(*rpc), entries[NV_GSP_REG_NUM_ENTRIES]);
> - strings = (char *)rpc + str_offset;
> - for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) {
> - int name_len = strlen(r535_registry_entries[i].name) + 1;
> -
> - rpc->entries[i].nameOffset = str_offset;
> - rpc->entries[i].type = 1;
> - rpc->entries[i].data = r535_registry_entries[i].value;
> - rpc->entries[i].length = 4;
> - memcpy(strings, r535_registry_entries[i].name, name_len);
> - strings += name_len;
> - str_offset += name_len;
> + /* Truncate the key=value string to just key */
> + *equal = 0;
> +
> + ret = kstrtol(equal + 1, 0, &value);
> + if (!ret) {
> + ret = add_registry_num(gsp, start, value);
> + } else {
> + /* Not a number, so treat it as a string */
> + ret = add_registry_string(gsp, start, equal + 1);
> + }
> +
> + if (ret) {
> + nvkm_error(&gsp->subdev,
> + "ignoring invalid registry key/value '%s=%s'\n",
> + start, equal + 1);
> + continue;
> + }
> + }
> +
> + kfree(p);
> }
> - rpc->size = str_offset;
> +
> + rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_SET_REGISTRY, gsp->registry_rpc_size);
> + if (IS_ERR(rpc)) {
> + clean_registry(gsp);
> + return PTR_ERR(rpc);
> + }
> +
> + build_registry(gsp, rpc);
>
> return nvkm_gsp_rpc_wr(gsp, rpc, false);
> }
>
> base-commit: f7ad2ce5fd89ab5d146da8f486a310746df5dc9e
> prerequisite-patch-id: 9bb653b6a53dcba4171d653e24a242461374f9fe
> prerequisite-patch-id: 7093a9db84053e43f6f278bf1d159a25d14ceebf
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [v7] nouveau: add command-line GSP-RM registry support
2024-04-25 13:22 ` Danilo Krummrich
@ 2024-04-25 16:38 ` Timur Tabi
2024-04-26 16:08 ` Danilo Krummrich
0 siblings, 1 reply; 8+ messages in thread
From: Timur Tabi @ 2024-04-25 16:38 UTC (permalink / raw)
To: dakr; +Cc: airlied, nouveau, lyude, Ben Skeggs
On Thu, 2024-04-25 at 15:22 +0200, Danilo Krummrich wrote:
> > + size_t length;
> > +
> > + /* Remove any whitespace from the parameter string */
> > + length = strip(p, " \t\n");
>
> With that, I see the following warning compiling this patch.
>
> warning: variable ‘length’ set but not used [-Wunused-but-set-variable]
>
> Did you intend to use the length for anything?
No, and I could have sworn I fixed that before sending out v7. I think I
originally intended 'length' to determine when I finished parsing the
string.
> Also, looking at the warning made me aware of 'p' potentially being NULL.
>
> If you agree, I can fix the warning and add the corresponding NULL check
> when
> applying the patch.
Yes, that would be great. You can just delete 'length'. The NULL check for
'p' should call clean_registry() before returning -ENOMEM.
Thanks for catching this.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [v7] nouveau: add command-line GSP-RM registry support
2024-04-25 16:38 ` Timur Tabi
@ 2024-04-26 16:08 ` Danilo Krummrich
2024-04-30 13:06 ` Lucas De Marchi
0 siblings, 1 reply; 8+ messages in thread
From: Danilo Krummrich @ 2024-04-26 16:08 UTC (permalink / raw)
To: Timur Tabi; +Cc: airlied, nouveau, lyude, Ben Skeggs
On 4/25/24 18:38, Timur Tabi wrote:
> On Thu, 2024-04-25 at 15:22 +0200, Danilo Krummrich wrote:
>>> + size_t length;
>>> +
>>> + /* Remove any whitespace from the parameter string */
>>> + length = strip(p, " \t\n");
>>
>> With that, I see the following warning compiling this patch.
>>
>> warning: variable ‘length’ set but not used [-Wunused-but-set-variable]
>>
>> Did you intend to use the length for anything?
>
> No, and I could have sworn I fixed that before sending out v7. I think I
> originally intended 'length' to determine when I finished parsing the
> string.
>
>> Also, looking at the warning made me aware of 'p' potentially being NULL.
>>
>> If you agree, I can fix the warning and add the corresponding NULL check
>> when
>> applying the patch.
>
> Yes, that would be great. You can just delete 'length'. The NULL check for
> 'p' should call clean_registry() before returning -ENOMEM.
Applied to drm-misc-next, thanks!
>
> Thanks for catching this.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [v7] nouveau: add command-line GSP-RM registry support
[not found] ` <c5ff8d3e-ecfc-4970-86c0-540b75b4be2e@ti.com>
@ 2024-04-29 12:40 ` Timur Tabi
0 siblings, 0 replies; 8+ messages in thread
From: Timur Tabi @ 2024-04-29 12:40 UTC (permalink / raw)
To: airlied, nouveau, lyude, Ben Skeggs, dakr, danishanwar
On Mon, 2024-04-29 at 14:49 +0530, MD Danish Anwar wrote:
> This patch seems to be breaking latest linux-next (tag: next-20240429).
> While building kernel for arm64 on latest linux-next with defconfig, I
> see build failure with below error.
>
> ❯ make -j$(nproc) ARCH=arm64 CROSS_COMPILE=aarch64-none-linux-gnu-
>
> CALL scripts/checksyscalls.sh
> CC [M] drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.o
> drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c: In function
> ‘build_registry’:
> drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1266:3: error: label at
> end of compound statement
> 1266 | default:
> | ^~~~~~~
> make[6]: *** [scripts/Makefile.build:244:
> drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.o] Error 1
I've been writing C code for 30 years, and I can't remember ever seeing this
compiler compaint.
default:
}
Seems normal to me, and it doesn't fail on x86. Try adding a "break;" between
the two lines.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [v7] nouveau: add command-line GSP-RM registry support
2024-04-26 16:08 ` Danilo Krummrich
@ 2024-04-30 13:06 ` Lucas De Marchi
2024-04-30 13:15 ` Danilo Krummrich
0 siblings, 1 reply; 8+ messages in thread
From: Lucas De Marchi @ 2024-04-30 13:06 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Timur Tabi, airlied, nouveau, lyude, Ben Skeggs,
chaitanya.kumar.borah, dri-devel
On Fri, Apr 26, 2024 at 06:08:19PM GMT, Danilo Krummrich wrote:
>On 4/25/24 18:38, Timur Tabi wrote:
>>On Thu, 2024-04-25 at 15:22 +0200, Danilo Krummrich wrote:
>>>>+ size_t length;
>>>>+
>>>>+ /* Remove any whitespace from the parameter string */
>>>>+ length = strip(p, " \t\n");
>>>
>>>With that, I see the following warning compiling this patch.
>>>
>>>warning: variable ‘length’ set but not used [-Wunused-but-set-variable]
>>>
>>>Did you intend to use the length for anything?
>>
>>No, and I could have sworn I fixed that before sending out v7. I think I
>>originally intended 'length' to determine when I finished parsing the
>>string.
>>
>>>Also, looking at the warning made me aware of 'p' potentially being NULL.
>>>
>>>If you agree, I can fix the warning and add the corresponding NULL check
>>>when
>>>applying the patch.
>>
>>Yes, that would be great. You can just delete 'length'. The NULL check for
>>'p' should call clean_registry() before returning -ENOMEM.
>
>Applied to drm-misc-next, thanks!
since this commit our CI is failing to build with the following error:
CC [M] drivers/gpu/drm/i915/display/intel_display_device.o
../drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c: In function ‘build_registry’:
../drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1266:3: error: label at end of compound statement
1266 | default:
| ^~~~~~~
CC [M] drivers/gpu/drm/amd/amdgpu/gfx_v10_0.o
HDRTEST drivers/gpu/drm/xe/compat-i915-headers/i915_reg.h
CC [M] drivers/gpu/drm/amd/amdgpu/imu_v11_0.o
make[7]: *** [../scripts/Makefile.build:244: drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.o] Error 1
make[7]: *** Waiting for unfinished jobs....
you are missing a `break;` after that default.
>
>>
>>Thanks for catching this.
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [v7] nouveau: add command-line GSP-RM registry support
2024-04-30 13:06 ` Lucas De Marchi
@ 2024-04-30 13:15 ` Danilo Krummrich
0 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2024-04-30 13:15 UTC (permalink / raw)
To: Timur Tabi, Lucas De Marchi
Cc: airlied, nouveau, lyude, Ben Skeggs, chaitanya.kumar.borah, dri-devel
On 4/30/24 15:06, Lucas De Marchi wrote:
> On Fri, Apr 26, 2024 at 06:08:19PM GMT, Danilo Krummrich wrote:
>> On 4/25/24 18:38, Timur Tabi wrote:
>>> On Thu, 2024-04-25 at 15:22 +0200, Danilo Krummrich wrote:
>>>>> + size_t length;
>>>>> +
>>>>> + /* Remove any whitespace from the parameter string */
>>>>> + length = strip(p, " \t\n");
>>>>
>>>> With that, I see the following warning compiling this patch.
>>>>
>>>> warning: variable ‘length’ set but not used [-Wunused-but-set-variable]
>>>>
>>>> Did you intend to use the length for anything?
>>>
>>> No, and I could have sworn I fixed that before sending out v7. I think I
>>> originally intended 'length' to determine when I finished parsing the
>>> string.
>>>
>>>> Also, looking at the warning made me aware of 'p' potentially being NULL.
>>>>
>>>> If you agree, I can fix the warning and add the corresponding NULL check
>>>> when
>>>> applying the patch.
>>>
>>> Yes, that would be great. You can just delete 'length'. The NULL check for
>>> 'p' should call clean_registry() before returning -ENOMEM.
>>
>> Applied to drm-misc-next, thanks!
>
>
> since this commit our CI is failing to build with the following error:
>
> CC [M] drivers/gpu/drm/i915/display/intel_display_device.o
> ../drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c: In function ‘build_registry’:
> ../drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1266:3: error: label at end of compound statement
> 1266 | default:
> | ^~~~~~~
> CC [M] drivers/gpu/drm/amd/amdgpu/gfx_v10_0.o
> HDRTEST drivers/gpu/drm/xe/compat-i915-headers/i915_reg.h
> CC [M] drivers/gpu/drm/amd/amdgpu/imu_v11_0.o
> make[7]: *** [../scripts/Makefile.build:244: drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.o] Error 1
> make[7]: *** Waiting for unfinished jobs....
>
> you are missing a `break;` after that default.
@Timur, do you want to send a quick fix for this yourself? Otherwise, I can send
one as well.
We should fix this asap.
- Danilo
>
>
>>
>>>
>>> Thanks for catching this.
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-04-30 13:15 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-17 21:53 [PATCH] [v7] nouveau: add command-line GSP-RM registry support Timur Tabi
2024-04-18 15:57 ` Danilo Krummrich
2024-04-25 13:22 ` Danilo Krummrich
2024-04-25 16:38 ` Timur Tabi
2024-04-26 16:08 ` Danilo Krummrich
2024-04-30 13:06 ` Lucas De Marchi
2024-04-30 13:15 ` Danilo Krummrich
[not found] ` <c5ff8d3e-ecfc-4970-86c0-540b75b4be2e@ti.com>
2024-04-29 12:40 ` Timur Tabi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).