[PATCH v6 1/6] drm/xe/guc: Add log init config abi definitions

[Dong, Zhanjun]

On 2025-07-29 2:34 p.m., John Harrison wrote:
> On 7/21/2025 6:35 PM, Zhanjun Dong wrote:
>> Add GuC log init config (LIC) ABI definitions.
>>
>> Signed-off-by: Zhanjun Dong <zhanjun.dong at intel.com>
>> ---
>>   drivers/gpu/drm/xe/abi/guc_lic_abi.h | 107 +++++++++++++++++++++++++++
>>   1 file changed, 107 insertions(+)
>>   create mode 100644 drivers/gpu/drm/xe/abi/guc_lic_abi.h
>>
>> diff --git a/drivers/gpu/drm/xe/abi/guc_lic_abi.h b/drivers/gpu/drm/ 
>> xe/abi/guc_lic_abi.h
>> new file mode 100644
>> index 000000000000..a4e27da304cd
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/abi/guc_lic_abi.h
>> @@ -0,0 +1,107 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2025 Intel Corporation
>> + */
>> +
>> +#ifndef _ABI_GUC_LIC_ABI_H_
>> +#define _ABI_GUC_LIC_ABI_H_
>> +
>> +#include <linux/types.h>
>> +
>> +/** enum guc_lic_type - Log Init Config TLV IDs. */
> We use KLV in the Xe driver not TLV.
KLV is Key-Length-Value
TLV is Type-Length-Value
as the Key here is the type, that make it become TLV, is it right?

>> +enum guc_lic_type {
>> +    /**
>> +     * @GUC_LIC_TYPE_GUC_SW_VERSION: GuC firmware version. Value
>> +     * is a 32 bit number represented by guc_sw_version.
>> +     */
>> +    GUC_LIC_TYPE_GUC_SW_VERSION = 0x1,
>> +    /**
>> +     * @GUC_LIC_TYPE_GUC_DEVICE_ID: GuC device id. Value is a 32
>> +     * bit.
>> +     */
>> +    GUC_LIC_TYPE_GUC_DEVICE_ID = 0x2,
>> +    /**
>> +     * @GUC_LIC_TYPE_TSC_FREQUENCY: GuC timestamp counter
>> +     * frequency. Value is a 32 bit number representing frequency in
>> +     * kHz. This timestamp is utilized in log entries, timer and
>> +     * for engine utilization tracking.
>> +     */
>> +    GUC_LIC_TYPE_TSC_FREQUENCY = 0x3,
>> +    /**
>> +     * @GUC_LIC_TYPE_GMD_ID: HW GMD ID. Value is a 32 bit number
>> +     * representing graphics, media and display HW architecture IDs.
>> +     */
>> +    GUC_LIC_TYPE_GMD_ID = 0x4,
>> +    /**
>> +     * @GUC_LIC_TYPE_BUILD_PLATFORM_ID: GuC build platform ID.
>> +     * Value is 32 bits.
>> +     */
>> +    GUC_LIC_TYPE_BUILD_PLATFORM_ID = 0x5,
>> +};
>> +
>> +/**
>> + * struct guc_sw_version - This structure describes the full version 
>> of a
>> + * software component.
>> + */
>> +struct guc_sw_version {
>> +    /** @dw0: A 32 bits dword, contains multiple bit fields */
>> +    u32 dw0;
>> +#define GUC_SW_VERSION_PATCH_VERSION        GENMASK(7, 0)
>> +#define GUC_SW_VERSION_MINOR_VERSION        GENMASK(15, 8)
>> +#define GUC_SW_VERSION_MAJOR_VERSION        GENMASK(23, 16)
>> +#define GUC_SW_VERSION_BRANCH_ID        GENMASK(31, 24)
>> +} __packed;
> Is there any point in having this defined as a structure? Elsewhere we 
> just have a u32 in the parent structure and the bitmask definitions for 
> access. Given that this doesn't even have a parent structure, it is just 
> the data field of a KLV, it makes even less sense to have a structure 
> defined for it.
Sure, Will make it a u32.

> 
>> +
>> +/**
>> + * struct guc_lic_format_version - Log Init Config Structure Version.
>> + * Major-Minor is not a fractional number (i.e. Ver 1.3 would be older
>> + * than 1.12)
>> + */
>> +struct guc_lic_format_version {
>> +    /** @dw0: A 32 bits dword, contains multiple bit fields */
>> +    u32 dw0;
>> +    /*
>> +     *  Log-Init-Config structure minor version. Must be
>> +     * GUC_LIC_FORMAT_VERSION_MASK_MINOR
>> +     */
>> +#define GUC_LIC_FORMAT_VERSION_MASK_MINOR        GENMASK(15, 0)
>> +    /*
>> +     *  Log-Init-Config structure major version. Must be
>> +     * GUC_LIC_FORMAT_VERSION_MASK_MAJOR
>> +     */
>> +#define GUC_LIC_FORMAT_VERSION_MASK_MAJOR        GENMASK(31, 16)
>> +} __packed;
> Again, why bother with a structure containing a single dword. Just use a 
> u32 in the parent structure.
Will make it a u32.

> 
>> +
>> +/**
>> + * struct guc_lic - GuC lic (Log-Init-Config) structure.
>> + * This is populated by the GUC at log init time and is located in 
>> the log
>> + * buffer as per the Log Buffer Layout (In Memory). The array of guc log
>> + * buffer states plus this structure must not exceed 4KB
> The last sentence is an instruction for the GuC firmware implementation 
> people. And the 'as per ...' is a reference to another section in the 
> spec that you do not have access to in this header file. I would just 
> stop with 'is located in the log buffer memory allocation'.
will follow the recommendation
> John.
> 
>> + */
>> +struct guc_lic {
>> +    /**
>> +     * @magic: A magic number set by GuC to identify that this
>> +     * structure contains valid information: magic = GUC_LIC_MAGIC.
>> +     * Used to verify the information in this structure is valid.
>> +     */
>> +    u32 magic;
>> +#define GUC_LIC_MAGIC            0x8086900D
>> +
>> +    /**
>> +     * @version: The version of the this structure. Represented by
>> +     * guc_lic_format_version
>> +     */
>> +    struct guc_lic_format_version version;
>> +#define GUC_LIC_FORMAT_VERSION_MAJOR    1u
>> +#define GUC_LIC_FORMAT_VERSION_MINOR    0u
>> +
>> +    /** @dw_size: Number of Dws the `data` array contains. */
>> +    u32 dw_size;
>> +    /**
>> +     * @data: Array of dwords representing a list of LIC KLVs of
>> +     * type guc_klv_generic with keys represented by guc_lic_type
>> +     */
>> +    u32 data[] __counted_by(dw_size);
>> +} __packed;
>> +
>> +#endif
>