[PATCH 1/3] drm/xe/guc: move KLV helpers to common file
Michal Wajdeczko
michal.wajdeczko at intel.com
Thu Feb 27 23:29:43 UTC 2025
On 27.02.2025 02:05, Daniele Ceraolo Spurio wrote:
> The GuC uses the same format for all its KLVs, so having the functions
> to set them in a common file will allow us to re-use it for different
> types of KLVs and not just the WAs in ADS.
> The next patch will make use of these functions for a new H2G command.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: John Harrison <John.C.Harrison at Intel.com>
> ---
> drivers/gpu/drm/xe/xe_guc_ads.c | 89 ++++++-------------------
> drivers/gpu/drm/xe/xe_guc_klv_helpers.c | 62 +++++++++++++++++
> drivers/gpu/drm/xe/xe_guc_klv_helpers.h | 7 ++
> 3 files changed, 89 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
> index fab259adc380..ed3304a11812 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ads.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ads.c
> @@ -22,6 +22,7 @@
> #include "xe_guc.h"
> #include "xe_guc_capture.h"
> #include "xe_guc_ct.h"
> +#include "xe_guc_klv_helpers.h"
> #include "xe_hw_engine.h"
> #include "xe_lrc.h"
> #include "xe_map.h"
> @@ -283,56 +284,6 @@ static size_t calculate_golden_lrc_size(struct xe_guc_ads *ads)
> return total_size;
> }
>
> -static void guc_waklv_enable_one_word(struct xe_guc_ads *ads,
> - enum xe_guc_klv_ids klv_id,
> - u32 value,
> - u32 *offset, u32 *remain)
> -{
> - u32 size;
> - u32 klv_entry[] = {
> - /* 16:16 key/length */
> - FIELD_PREP(GUC_KLV_0_KEY, klv_id) |
> - FIELD_PREP(GUC_KLV_0_LEN, 1),
> - value,
> - /* 1 dword data */
> - };
> -
> - size = sizeof(klv_entry);
> -
> - if (*remain < size) {
> - drm_warn(&ads_to_xe(ads)->drm,
> - "w/a klv buffer too small to add klv id %d\n", klv_id);
> - } else {
> - xe_map_memcpy_to(ads_to_xe(ads), ads_to_map(ads), *offset,
> - klv_entry, size);
> - *offset += size;
> - *remain -= size;
> - }
> -}
> -
> -static void guc_waklv_enable_simple(struct xe_guc_ads *ads,
> - enum xe_guc_klv_ids klv_id, u32 *offset, u32 *remain)
> -{
> - u32 klv_entry[] = {
> - /* 16:16 key/length */
> - FIELD_PREP(GUC_KLV_0_KEY, klv_id) |
> - FIELD_PREP(GUC_KLV_0_LEN, 0),
> - /* 0 dwords data */
> - };
> - u32 size;
> -
> - size = sizeof(klv_entry);
> -
> - if (xe_gt_WARN(ads_to_gt(ads), *remain < size,
> - "w/a klv buffer too small to add klv id %d\n", klv_id))
> - return;
> -
> - xe_map_memcpy_to(ads_to_xe(ads), ads_to_map(ads), *offset,
> - klv_entry, size);
> - *offset += size;
> - *remain -= size;
> -}
> -
> static void guc_waklv_init(struct xe_guc_ads *ads)
> {
> struct xe_gt *gt = ads_to_gt(ads);
> @@ -343,22 +294,22 @@ static void guc_waklv_init(struct xe_guc_ads *ads)
> remain = guc_ads_waklv_size(ads);
>
> if (XE_WA(gt, 14019882105))
> - guc_waklv_enable_simple(ads,
> - GUC_WORKAROUND_KLV_BLOCK_INTERRUPTS_WHEN_MGSR_BLOCKED,
> - &offset, &remain);
> + xe_guc_klv_enable_simple(ads_to_guc(ads), ads_to_map(ads),
> + GUC_WORKAROUND_KLV_BLOCK_INTERRUPTS_WHEN_MGSR_BLOCKED,
> + &offset, &remain);
> if (XE_WA(gt, 18024947630))
> - guc_waklv_enable_simple(ads,
> - GUC_WORKAROUND_KLV_ID_GAM_PFQ_SHADOW_TAIL_POLLING,
> - &offset, &remain);
> + xe_guc_klv_enable_simple(ads_to_guc(ads), ads_to_map(ads),
> + GUC_WORKAROUND_KLV_ID_GAM_PFQ_SHADOW_TAIL_POLLING,
> + &offset, &remain);
> if (XE_WA(gt, 16022287689))
> - guc_waklv_enable_simple(ads,
> - GUC_WORKAROUND_KLV_ID_DISABLE_MTP_DURING_ASYNC_COMPUTE,
> - &offset, &remain);
> + xe_guc_klv_enable_simple(ads_to_guc(ads), ads_to_map(ads),
> + GUC_WORKAROUND_KLV_ID_DISABLE_MTP_DURING_ASYNC_COMPUTE,
> + &offset, &remain);
>
> if (XE_WA(gt, 14022866841))
> - guc_waklv_enable_simple(ads,
> - GUC_WA_KLV_WAKE_POWER_DOMAINS_FOR_OUTBOUND_MMIO,
> - &offset, &remain);
> + xe_guc_klv_enable_simple(ads_to_guc(ads), ads_to_map(ads),
> + GUC_WA_KLV_WAKE_POWER_DOMAINS_FOR_OUTBOUND_MMIO,
> + &offset, &remain);
>
> /*
> * On RC6 exit, GuC will write register 0xB04 with the default value provided. As of now,
> @@ -366,15 +317,15 @@ static void guc_waklv_init(struct xe_guc_ads *ads)
> * future, so GuC depends on KMD to send it the correct value.
> */
> if (XE_WA(gt, 13011645652))
> - guc_waklv_enable_one_word(ads,
> - GUC_WA_KLV_NP_RD_WRITE_TO_CLEAR_RCSM_AT_CGP_LATE_RESTORE,
> - 0xC40,
> - &offset, &remain);
> + xe_guc_klv_enable_one_word(ads_to_guc(ads), ads_to_map(ads),
> + GUC_WA_KLV_NP_RD_WRITE_TO_CLEAR_RCSM_AT_CGP_LATE_RESTORE,
> + 0xC40,
> + &offset, &remain);
>
> if (XE_WA(gt, 14022293748) || XE_WA(gt, 22019794406))
> - guc_waklv_enable_simple(ads,
> - GUC_WORKAROUND_KLV_ID_BACK_TO_BACK_RCS_ENGINE_RESET,
> - &offset, &remain);
> + xe_guc_klv_enable_simple(ads_to_guc(ads), ads_to_map(ads),
> + GUC_WORKAROUND_KLV_ID_BACK_TO_BACK_RCS_ENGINE_RESET,
> + &offset, &remain);
>
> size = guc_ads_waklv_size(ads) - remain;
> if (!size)
> diff --git a/drivers/gpu/drm/xe/xe_guc_klv_helpers.c b/drivers/gpu/drm/xe/xe_guc_klv_helpers.c
> index 146a6eda9e06..50eab2086ee3 100644
> --- a/drivers/gpu/drm/xe/xe_guc_klv_helpers.c
> +++ b/drivers/gpu/drm/xe/xe_guc_klv_helpers.c
> @@ -7,8 +7,11 @@
> #include <drm/drm_print.h>
>
> #include "abi/guc_klvs_abi.h"
> +#include "xe_gt_printk.h"
> +#include "xe_guc.h"
> #include "xe_guc_klv_helpers.h"
> #include "xe_guc_klv_thresholds_set.h"
> +#include "xe_map.h"
>
> #define make_u64(hi, lo) ((u64)((u64)(u32)(hi) << 32 | (u32)(lo)))
>
> @@ -146,3 +149,62 @@ int xe_guc_klv_count(const u32 *klvs, u32 num_dwords)
>
> return num_dwords ? -ENODATA : num_klvs;
> }
> +
> +static void emit_klv(struct xe_guc *guc, struct iosys_map *map, u32 *klv_entry,
> + u32 size, u32 *offset, u32 *remain)
s/klv_entry/klv
s/size/num_dwords
KLVs are always u32 aligned
'size/offset/remain' all should be in dwords
this will also match rest of functions in this file
> +{
> + if (xe_gt_WARN(guc_to_gt(guc), *remain < size,
do we need xe_gt_WARN in production?
maybe xe_gt_assert will suffice?
> + "klv buffer too small to add klv id 0x%04x\n",
s/klv/KLV
> + FIELD_GET(GUC_KLV_0_KEY, klv_entry[0])))
> + return;
> +
> + xe_map_memcpy_to(guc_to_xe(guc), map, *offset, klv_entry, size);
> + *offset += size;
> + *remain -= size;
> +}
> +
> +/**
> + * xe_guc_klv_enable_simple - Emits a KLV with no data to an iosys mapped buffer
> + * @guc: the guc structure
> + * @map: the iosys_map to write to
> + * @klv_id: the KLV to enable
key ? there is no Len nor Value
this will follow other functions in this file
> + * @offset: pointer to a variable holding the offset to write to
> + * @remain: pointer to a variable holding the remaining writing space available
> + *
> + * The function checks if there is enough space to emit a KLV with no data and
> + * if so writes it to memory. After the write, the offset and remain variables
> + * are respectively increased and decreased of the written size to be ready for
> + * the next emission.
> + */
> +void xe_guc_klv_enable_simple(struct xe_guc *guc, struct iosys_map *map,
> + u16 klv_id, u32 *offset, u32 *remain)
> +{
> + u32 klv_entry = PREP_GUC_KLV(klv_id, 0);
u32 klv[] = { PREP_GUC_KLV(key, 0) };
emit(... ARRAY_SIZE(klv)
> +
> + emit_klv(guc, map, &klv_entry, sizeof(klv_entry), offset, remain);
> +}
> +
> +/**
> + * xe_guc_klv_enable_one_word - Emits a KLV with 1 DW data to an iosys mapped buffer
> + * @guc: the guc structure
> + * @map: the iosys_map to write to
> + * @klv_id: the KLV to enable
s/klv_id/key
> + * @value: the data associated with the KLV
> + * @offset: pointer to a variable holding the offset to write to
> + * @remain: pointer to a variable holding the remaining writing space available
> + *
> + * The function checks if there is enough space to emit a KLV with 1 DW data and
> + * if so writes it to memory. After the write, the offset and remain variables
> + * are respectively increased and decreased of the written size to be ready for
> + * the next emission.
> + */
> +void xe_guc_klv_enable_one_word(struct xe_guc *guc, struct iosys_map *map,
> + u16 klv_id, u32 value, u32 *offset, u32 *remain)
> +{
> + u32 klv_entry[] = {
> + PREP_GUC_KLV(klv_id, 1),
> + value,
> + };
> +
> + emit_klv(guc, map, klv_entry, sizeof(klv_entry), offset, remain);
> +}
> diff --git a/drivers/gpu/drm/xe/xe_guc_klv_helpers.h b/drivers/gpu/drm/xe/xe_guc_klv_helpers.h
> index c676d21c173b..231f7c4b0947 100644
> --- a/drivers/gpu/drm/xe/xe_guc_klv_helpers.h
> +++ b/drivers/gpu/drm/xe/xe_guc_klv_helpers.h
> @@ -10,6 +10,8 @@
> #include <linux/types.h>
>
> struct drm_printer;
> +struct iosys_map;
> +struct xe_guc;
>
> const char *xe_guc_klv_key_to_string(u16 key);
>
> @@ -61,4 +63,9 @@ int xe_guc_klv_count(const u32 *klvs, u32 num_dwords);
> #define PREP_GUC_KLV_TAG(TAG) \
> PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
>
> +void xe_guc_klv_enable_simple(struct xe_guc *guc, struct iosys_map *map,
> + u16 klv_id, u32 *offset, u32 *remain);
> +void xe_guc_klv_enable_one_word(struct xe_guc *guc, struct iosys_map *map,
> + u16 klv_id, u32 value, u32 *offset, u32 *remain);
hmm, "enable" does not fit here, maybe:
xe_guc_klv_emit_empty / no_data / no_value
xe_guc_klv_emit_dword
and those should be inlines that use:
void xe_guc_klv_emit_simple(
struct xe_guc *guc,
struct iosys_map *map,
u16 key, u16 len, const u32 *klv,
u32 *offset, u32 *remain);
like:
inline xe_guc_klv_emit_simple_empty(.. key ..)
{
xe_guc_klv_emit_simple(.. key, 0, NULL, ..);
}
inline xe_guc_klv_emit_simple_dword(.. key, value ..)
{
xe_guc_klv_emit_simple(.. key, 1, &value, ..);
}
> +
> #endif
More information about the Intel-xe
mailing list