[PATCH 4/4] drm/xe: Convert pre-GMDID IPs to struct xe_ip
Gustavo Sousa
gustavo.sousa at intel.com
Thu Feb 20 21:25:14 UTC 2025
Quoting Gustavo Sousa (2025-02-20 18:23:49-03:00)
>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.
Also, with that, the for-loop wouldn't even be necessary.
--
Gustavo Sousa
>
>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