[PATCH 1/2] drm/xe/guc: Add helpers for GuC KLVs
Michal Wajdeczko
michal.wajdeczko at intel.com
Tue Apr 9 09:35:27 UTC 2024
On 08.04.2024 22:23, Ghimiray, Himal Prasad wrote:
>
> On 08-04-2024 23:44, Michal Wajdeczko wrote:
>> Many of the GuC actions use KLVs to pass additional parameters or
>> configuration data. Add few helper functions for better reporting
>> any information related to KLVs.
>>
>> Signed-off-by: Michal Wajdeczko<michal.wajdeczko at intel.com>
>> ---
>> drivers/gpu/drm/xe/Makefile | 1 +
>> drivers/gpu/drm/xe/xe_guc_klv_helpers.c | 133 ++++++++++++++++++++++++
>> drivers/gpu/drm/xe/xe_guc_klv_helpers.h | 51 +++++++++
>> 3 files changed, 185 insertions(+)
>> create mode 100644 drivers/gpu/drm/xe/xe_guc_klv_helpers.c
>> create mode 100644 drivers/gpu/drm/xe/xe_guc_klv_helpers.h
>>
>> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>> index e5b1715f721e..ec0d1fb49b1e 100644
>> --- a/drivers/gpu/drm/xe/Makefile
>> +++ b/drivers/gpu/drm/xe/Makefile
>> @@ -98,6 +98,7 @@ xe-y += xe_bb.o \
>> xe_guc_debugfs.o \
>> xe_guc_hwconfig.o \
>> xe_guc_id_mgr.o \
>> + xe_guc_klv_helpers.o \
>> xe_guc_log.o \
>> xe_guc_pc.o \
>> xe_guc_submit.o \
>> diff --git a/drivers/gpu/drm/xe/xe_guc_klv_helpers.c
>> b/drivers/gpu/drm/xe/xe_guc_klv_helpers.c
>> new file mode 100644
>> index 000000000000..c39fda08afcb
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_guc_klv_helpers.c
>> @@ -0,0 +1,133 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2024 Intel Corporation
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <drm/drm_print.h>
>> +
>> +#include "abi/guc_klvs_abi.h"
>> +#include "xe_guc_klv_helpers.h"
>> +
>> +#define make_u64(hi, lo) ((u64)((u64)(u32)(hi) << 32 | (u32)(lo)))
>
> is this macro necessary, it's being called in one place ?
>
> ((u64)hi << 32) | lo should be enough.
IMO it is better to have a macro (with name) than a magic formula
besides, this macro here is (I hope) just temporary as either:
- it will be promoted to xe_macros.h [1], or
- something similar will be available in wordpart.h [2]
[1] https://patchwork.freedesktop.org/series/129854/
[2]
https://lore.kernel.org/lkml/20240214214654.1700-1-michal.wajdeczko@intel.com/
>
>> +
>> +/**
>> + * xe_guc_klv_key_to_string - Convert KLV key into friendly name.
>> + * @key: the `GuC KLV`_ key
>> + *
>> + * Return: name of the KLV key.
>> + */
>> +const char *xe_guc_klv_key_to_string(u16 key)
>> +{
>> + switch (key) {
>> + /* VGT POLICY keys */
>> + case GUC_KLV_VGT_POLICY_SCHED_IF_IDLE_KEY:
>> + return "sched_if_idle";
>> + case GUC_KLV_VGT_POLICY_ADVERSE_SAMPLE_PERIOD_KEY:
>> + return "sample_period";
>> + case GUC_KLV_VGT_POLICY_RESET_AFTER_VF_SWITCH_KEY:
>> + return "reset_engine";
>> + /* VF CFG keys */
>> + case GUC_KLV_VF_CFG_GGTT_START_KEY:
>> + return "ggtt_start";
>> + case GUC_KLV_VF_CFG_GGTT_SIZE_KEY:
>> + return "ggtt_size";
>> + case GUC_KLV_VF_CFG_LMEM_SIZE_KEY:
>> + return "lmem_size";
>> + case GUC_KLV_VF_CFG_NUM_CONTEXTS_KEY:
>> + return "num_contexts";
>> + case GUC_KLV_VF_CFG_TILE_MASK_KEY:
>> + return "tile_mask";
>> + case GUC_KLV_VF_CFG_NUM_DOORBELLS_KEY:
>> + return "num_doorbells";
>> + case GUC_KLV_VF_CFG_EXEC_QUANTUM_KEY:
>> + return "exec_quantum";
>> + case GUC_KLV_VF_CFG_PREEMPT_TIMEOUT_KEY:
>> + return "preempt_timeout";
>> + case GUC_KLV_VF_CFG_BEGIN_DOORBELL_ID_KEY:
>> + return "begin_db_id";
>> + case GUC_KLV_VF_CFG_BEGIN_CONTEXT_ID_KEY:
>> + return "begin_ctx_id";
>
> default:
>
> return "(unknown)";
>
> Have seen warning/errors if default is not defined in switch-case.
likely compiler was complaining because of enum being used in switch
statement, but here it is u16, so there shouldn't be any warnings
but can add 'default' case if you consider that as a blocker
>
>> + }
>> + return "(unknown)";
> remove.
>> +}
>> +
>> +/**
>> + * xe_guc_klv_print - Print content of the buffer with `GuC KLV`_.
>> + * @klvs: the buffer with KLVs
>> + * @num_dwords: number of dwords (u32) available in the buffer
>> + * @p: the &drm_printer
>> + *
>> + * The buffer may contain more than one KLV.
>> + */
>> +void xe_guc_klv_print(const u32 *klvs, u32 num_dwords, struct
>> drm_printer *p)
>> +{
>> + while (num_dwords >= GUC_KLV_LEN_MIN) {
>> + u32 key = FIELD_GET(GUC_KLV_0_KEY, klvs[0]);
>> + u32 len = FIELD_GET(GUC_KLV_0_LEN, klvs[0]);
>> +
>> + klvs += GUC_KLV_LEN_MIN;
>> + num_dwords -= GUC_KLV_LEN_MIN;
>> +
>> + if (num_dwords < len) {
>> + drm_printf(p, "{ key %#06x : truncated %zu of %zu bytes
>> %*ph } # %s\n",
>> + key, num_dwords * sizeof(u32), len * sizeof(u32),
>> + (int)(num_dwords * sizeof(u32)), klvs,
>> + xe_guc_klv_key_to_string(key));
>> + return;
>> + }
>> +
>> + switch (len) {
>> + case 0:
>> + drm_printf(p, "{ key %#06x : no value } # %s\n",
>> + key, xe_guc_klv_key_to_string(key));
>> + break;
>> + case 1:
>> + drm_printf(p, "{ key %#06x : 32b value %u } # %s\n",
>> + key, klvs[0], xe_guc_klv_key_to_string(key));
>> + break;
>> + case 2:
>> + drm_printf(p, "{ key %#06x : 64b value %#llx } # %s\n",
>> + key, make_u64(klvs[1], klvs[0]),
>> + xe_guc_klv_key_to_string(key));
>> + break;
>> + default:
>> + drm_printf(p, "{ key %#06x : %zu bytes %*ph } # %s\n",
>> + key, len * sizeof(u32), (int)(len * sizeof(u32)),
>> + klvs, xe_guc_klv_key_to_string(key));
>> + break;
>> + }
>> +
>> + klvs += len;
>> + num_dwords -= len;
>> + }
>> +
>> + /* we don't expect any leftovers, fix if KLV header is ever
>> changed */
>> + BUILD_BUG_ON(GUC_KLV_LEN_MIN > 1);
>> +}
>> +
>> +/**
>> + * xe_guc_klv_count - Count KLVs present in the buffer.
>> + * @klvs: the buffer with KLVs
>> + * @num_dwords: number of dwords (u32) in the buffer
>> + *
>> + * Return: number of recognized KLVs or
>> + * a negative error code if KLV buffer is truncated.
see here ^^^
>> + */
>> +int xe_guc_klv_count(const u32 *klvs, u32 num_dwords)
>> +{
>> + int num_klvs = 0;
>> +
>> + while (num_dwords >= GUC_KLV_LEN_MIN) {
>> + u32 len = FIELD_GET(GUC_KLV_0_LEN, klvs[0]);
>> +
>> + if (num_dwords < len + GUC_KLV_LEN_MIN)
>> + return -ENODATA;
>> +
>> + klvs += GUC_KLV_LEN_MIN + len;
>> + num_dwords -= GUC_KLV_LEN_MIN + len;
>> + num_klvs++;
>> + }
>> +
>> + return num_dwords ? -ENODATA : num_klvs;
>
> while loop can be terminated only if num_dwords < len + GUC_KLV_LEN_MIN
> or num_dwords is 0. Therefore return num_klvs; should be fine.
but then we won't catch the case with truncated buffer (which indicates
bug somewhere in code or firmware) and it is one of the function goal
and return value
>
>> +}
>> diff --git a/drivers/gpu/drm/xe/xe_guc_klv_helpers.h
>> b/drivers/gpu/drm/xe/xe_guc_klv_helpers.h
>> new file mode 100644
>> index 000000000000..b835e0ebe6db
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_guc_klv_helpers.h
>> @@ -0,0 +1,51 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2024 Intel Corporation
>> + */
>> +
>> +#ifndef _XE_GUC_KLV_HELPERS_H_
>> +#define _XE_GUC_KLV_HELPERS_H_
>> +
>> +#include <linux/types.h>
>> +
>> +struct drm_printer;
>> +
>> +const char *xe_guc_klv_key_to_string(u16 key);
>> +
>> +void xe_guc_klv_print(const u32 *klvs, u32 num_dwords, struct
>> drm_printer *p);
>> +int xe_guc_klv_count(const u32 *klvs, u32 num_dwords);
>> +
>> +/**
>> + * PREP_GUC_KLV - Prepare KLV header value based on provided key and
>> len.
>> + * @key: KLV key
>> + * @len: KLV length
>> + *
>> + * Return: value of the KLV header (u32).
>> + */
>> +#define PREP_GUC_KLV(key, len) \
>> + (FIELD_PREP(GUC_KLV_0_KEY, (key)) | \
>> + FIELD_PREP(GUC_KLV_0_LEN, (len)))
>> +
>> +/**
>> + * PREP_GUC_KLV_CONST - Prepare KLV header value based on const key
>> and len.
>> + * @key: const KLV key
>> + * @len: const KLV length
>> + *
>> + * Return: value of the KLV header (u32).
>> + */
>> +#define PREP_GUC_KLV_CONST(key, len) \
>> + (FIELD_PREP_CONST(GUC_KLV_0_KEY, (key)) | \
>> + FIELD_PREP_CONST(GUC_KLV_0_LEN, (len)))
>> +
>> +/**
>> + * PREP_GUC_KLV_TAG - Prepare KLV header value based on unique KLV
>> definition tag.
>> + * @TAG: unique tag of the KLV definition
>> + *
>> + * Combine separate KEY and LEN definitions of the KLV identified by
>> the TAG.
>> + *
>> + * Return: value of the KLV header (u32).
>> + */
>> +#define PREP_GUC_KLV_TAG(TAG) \
>> + PREP_GUC_KLV_CONST(GUC_KLV_##TAG##_KEY, GUC_KLV_##TAG##_LEN)
>> +
>> +#endif
More information about the Intel-xe
mailing list