[PATCH 4/4] drm/xe: Convert pre-GMDID IPs to struct xe_ip
Gustavo Sousa
gustavo.sousa at intel.com
Thu Feb 20 21:23:49 UTC 2025
Quoting Matt Roper (2025-02-20 18:10:50-03:00)
>On Thu, Feb 20, 2025 at 02:25:11PM -0300, Gustavo Sousa wrote:
>> We have now a struct xe_ip to fully describe an IP, but we are only
>> using that for GMDID-based IPs.
>>
>> For pre-GMDID IPs, we still describe release info (version and name) via
>> feature descriptors (struct xe_{graphics,media}_desc). Let's convert
>> those to use struct xe_ip.
>>
>> With this, we have a uniform way of describing IPs in the xe driver
>> instead of having different approaches based on whether the IPs use
>> GMDIDs or not.
>>
>> A nice side-effect of this change is that now we have an easy way to
>> lookup, in the source code, mappings between versions, names and
>> features for all supported IPs.
>>
>> Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_pci.c | 60 ++++++++++++++++---------------
>> drivers/gpu/drm/xe/xe_pci_types.h | 8 -----
>> 2 files changed, 32 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>> index 8b8111e954c1..c88c2da1e731 100644
>> --- a/drivers/gpu/drm/xe/xe_pci.c
>> +++ b/drivers/gpu/drm/xe/xe_pci.c
>> @@ -82,10 +82,6 @@ __diag_ignore_all("-Woverride-init", "Allow field overrides in table");
>> #define NOP(x) x
>>
>> static const struct xe_graphics_desc graphics_xelp = {
>> - .name = "Xe_LP",
>> - .ver = 12,
>> - .rel = 0,
>> -
>> .hw_engine_mask = BIT(XE_HW_ENGINE_RCS0) | BIT(XE_HW_ENGINE_BCS0),
>>
>> .va_bits = 48,
>> @@ -93,10 +89,6 @@ static const struct xe_graphics_desc graphics_xelp = {
>> };
>>
>> static const struct xe_graphics_desc graphics_xelpp = {
>> - .name = "Xe_LP+",
>> - .ver = 12,
>> - .rel = 10,
>> -
>> .hw_engine_mask = BIT(XE_HW_ENGINE_RCS0) | BIT(XE_HW_ENGINE_BCS0),
>>
>> .va_bits = 48,
>> @@ -109,10 +101,6 @@ static const struct xe_graphics_desc graphics_xelpp = {
>> .vm_max_level = 3
>>
>> static const struct xe_graphics_desc graphics_xehpg = {
>> - .name = "Xe_HPG",
>> - .ver = 12,
>> - .rel = 55,
>> -
>> .hw_engine_mask =
>> BIT(XE_HW_ENGINE_RCS0) | BIT(XE_HW_ENGINE_BCS0) |
>> BIT(XE_HW_ENGINE_CCS0) | BIT(XE_HW_ENGINE_CCS1) |
>> @@ -125,10 +113,6 @@ static const struct xe_graphics_desc graphics_xehpg = {
>> };
>>
>> static const struct xe_graphics_desc graphics_xehpc = {
>> - .name = "Xe_HPC",
>> - .ver = 12,
>> - .rel = 60,
>> -
>> .hw_engine_mask =
>> BIT(XE_HW_ENGINE_BCS0) | BIT(XE_HW_ENGINE_BCS1) |
>> BIT(XE_HW_ENGINE_BCS2) | BIT(XE_HW_ENGINE_BCS3) |
>> @@ -175,20 +159,12 @@ static const struct xe_graphics_desc graphics_xe2 = {
>> };
>>
>> static const struct xe_media_desc media_xem = {
>> - .name = "Xe_M",
>> - .ver = 12,
>> - .rel = 0,
>> -
>> .hw_engine_mask =
>> GENMASK(XE_HW_ENGINE_VCS7, XE_HW_ENGINE_VCS0) |
>> GENMASK(XE_HW_ENGINE_VECS3, XE_HW_ENGINE_VECS0),
>> };
>>
>> static const struct xe_media_desc media_xehpm = {
>> - .name = "Xe_HPM",
>> - .ver = 12,
>> - .rel = 55,
>> -
>> .hw_engine_mask =
>> GENMASK(XE_HW_ENGINE_VCS7, XE_HW_ENGINE_VCS0) |
>> GENMASK(XE_HW_ENGINE_VECS3, XE_HW_ENGINE_VECS0),
>> @@ -369,6 +345,13 @@ static const struct xe_device_desc ptl_desc = {
>> #undef PLATFORM
>> __diag_pop();
>>
>> +static const struct xe_ip pre_gmdid_ips[] = {
>> + { 1200, "Xe_LP", &graphics_xelp },
>> + { 1210, "Xe_LP+", &graphics_xelpp },
>> + { 1255, "Xe_HPG", &graphics_xehpg },
>> + { 1260, "Xe_HPC", &graphics_xehpc },
>> +};
>> +
>> /* GMDID-based Graphics IPs */
>> static const struct xe_ip graphics_ips[] = {
>> { 1270, "Xe_LPG", &graphics_xelpg },
>> @@ -380,6 +363,11 @@ static const struct xe_ip graphics_ips[] = {
>> { 3001, "Xe3_LPG", &graphics_xe2 },
>> };
>>
>> +static const struct xe_ip pre_gmdid_media_ips[] = {
>> + { 1200, "Xe_M", &media_xem },
>> + { 1255, "Xe_HPM", &media_xehpm },
>> +};
>> +
>> /* GMDID-based Media IPs */
>> static const struct xe_ip media_ips[] = {
>> { 1300, "Xe_LPM+", &media_xelpmp },
>> @@ -558,12 +546,28 @@ static void handle_pre_gmdid(struct xe_device *xe,
>> const struct xe_graphics_desc *graphics,
>> const struct xe_media_desc *media)
>> {
>> - xe->info.graphics_verx100 = graphics->ver * 100 + graphics->rel;
>> - xe->info.graphics_name = graphics->name;
>> + for (int i = 0; i < ARRAY_SIZE(pre_gmdid_ips); i++) {
>> + if (pre_gmdid_ips[i].desc == graphics) {
>
>Unfortunately it doesn't appear that we can consolidate now-identical
>structures (e.g., graphics_xelp and graphics_xelpp) since doing so would
>always match the first entry in the table an incorrect assign a version
>of 12.00 instead of 12.10. I guess that's not really any worse than the
>current situation of having near-identical structures that only vary in
>version though.
>
>We might want to add a ktest to ensure that we never try to point
>multiple versions at the same structure for pre-GMD_ID IPs.
Hm... I wonder if we could make graphics and media members of struct
xe_device_desc be pointers to struct xe_ip instead of
xe_{graphics,media}_desc. That would allow descriptor reuse and rule out
issues related to matching the same descriptor for different pre-GMDID
platforms.
What do you think?
--
Gustavo Sousa
>
>Anyway,
>
>Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
>
>
>> + xe->info.graphics_verx100 = pre_gmdid_ips[i].verx100;
>> + xe->info.graphics_name = pre_gmdid_ips[i].name;
>> +
>> + break;
>> + }
>> + }
>> +
>> + xe_assert(xe, xe->info.graphics_verx100);
>>
>> if (media) {
>> - xe->info.media_verx100 = media->ver * 100 + media->rel;
>> - xe->info.media_name = media->name;
>> + for (int i = 0; i < ARRAY_SIZE(pre_gmdid_media_ips); i++) {
>> + if (pre_gmdid_media_ips[i].desc == media) {
>> + xe->info.media_verx100 = pre_gmdid_media_ips[i].verx100;
>> + xe->info.media_name = pre_gmdid_media_ips[i].name;
>> +
>> + break;
>> + }
>> + }
>> +
>> + xe_assert(xe, xe->info.media_verx100);
>> } else {
>> xe->info.media_name = "none";
>> }
>> diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
>> index f46426ef8ed8..e9b9bbc138d3 100644
>> --- a/drivers/gpu/drm/xe/xe_pci_types.h
>> +++ b/drivers/gpu/drm/xe/xe_pci_types.h
>> @@ -9,10 +9,6 @@
>> #include <linux/types.h>
>>
>> struct xe_graphics_desc {
>> - const char *name;
>> - u8 ver;
>> - u8 rel;
>> -
>> u8 va_bits;
>> u8 vm_max_level;
>> u8 vram_flags;
>> @@ -28,10 +24,6 @@ struct xe_graphics_desc {
>> };
>>
>> struct xe_media_desc {
>> - const char *name;
>> - u8 ver;
>> - u8 rel;
>> -
>> u64 hw_engine_mask; /* hardware engines provided by media IP */
>>
>> u8 has_indirect_ring_state:1;
>> --
>> 2.48.1
>>
>
>--
>Matt Roper
>Graphics Software Engineer
>Linux GPU Platform Enablement
>Intel Corporation
More information about the Intel-xe
mailing list