[Intel-xe] [PATCH] drm/xe: Fix media detection for pre-GMD_ID platforms
Souza, Jose
jose.souza at intel.com
Thu Apr 27 18:53:14 UTC 2023
On Thu, 2023-04-27 at 11:44 -0700, Lucas De Marchi wrote:
> Reading the GMD_ID register on platforms before that register
> became available is not reliable. The assumption was that since the
> register was not allocated, it would return 0. But on PVC for example it
> returns garbage (or a very specific number), triggering the following
> error:
>
> xe 0000:8c:00.0: [drm] *ERROR* Hardware reports unknown media version 1025.55
>
> Fix it by stop relying on the value returned by that registers on
> platforms before GMD_ID. Instead this relies on the graphics description
> struct being already pre-set on the device: this can only ever be true
> for platforms before the GMD_ID support. In that case, GMD_ID is skipped
> and the hardcoded values are used. This should also help on early
> bring-up in case the GMD_ID returns something not expected and we need to
> temporarily hardcode values. With this, PVC doesn't trigger the error
> and goes straight to:
>
> xe 0000:8c:00.0: [drm:xe_display_info_init [xe]] No display IP, skipping
> xe 0000:8c:00.0: [drm:xe_pci_probe [xe]] XE_PVC 0bd5:002f dgfx:1 gfx:Xe_HPC (12.60) media:none (0.00) dma_m_s:52 tc:2
> xe 0000:8c:00.0: [drm:xe_pci_probe [xe]] Stepping = (G:C0, M:**, D:**, B:B3)
Reviewed-by: José Roberto de Souza <jose.souza at intel.com>
>
> Fixes: d55b52daa96c ("drm/xe: Select graphics/media descriptors from GMD_ID")
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
> drivers/gpu/drm/xe/xe_pci.c | 113 ++++++++++++++++++------------------
> 1 file changed, 55 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index f587bd9706af..35dcb8781f2a 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -39,7 +39,9 @@ struct xe_gt_desc {
> };
>
> struct xe_device_desc {
> + /* Should only ever be set for platforms without GMD_ID */
> const struct xe_graphics_desc *graphics;
> + /* Should only ever be set for platforms without GMD_ID */
> const struct xe_media_desc *media;
>
> const char *platform_name;
> @@ -408,6 +410,30 @@ static u32 peek_gmdid(struct xe_device *xe, u32 gmdid_offset)
> REG_FIELD_GET(GMD_ID_RELEASE_MASK, ver);
> }
>
> +/*
> + * Pre-GMD_ID platform: device descriptor already points to the appropriate
> + * graphics descriptor. Simply forward the description and calculate the version
> + * appropriately. "graphics" should be present in all such platforms, while
> + * media is optional.
> + */
> +static void handle_pre_gmdid(struct xe_device *xe,
> + const struct xe_device_desc *desc,
> + const struct xe_graphics_desc **graphics,
> + const struct xe_media_desc **media)
> +{
> + *graphics = desc->graphics;
> + xe->info.graphics_verx100 = (*graphics)->ver * 100 + (*graphics)->rel;
> +
> + *media = desc->media;
> + if (*media)
> + xe->info.media_verx100 = (*media)->ver * 100 + (*media)->rel;
> +
> +}
> +
> +/*
> + * GMD_ID platform: read IP version from hardware and select graphics descriptor
> + * based on the result.
> + */
> static void handle_gmdid(struct xe_device *xe,
> const struct xe_device_desc *desc,
> const struct xe_graphics_desc **graphics,
> @@ -415,69 +441,35 @@ static void handle_gmdid(struct xe_device *xe,
> {
> u32 ver;
>
> - if (desc->graphics) {
> - /*
> - * Pre-GMD_ID platform; device descriptor already points to
> - * the appropriate graphics descriptor.
> - */
> - *graphics = desc->graphics;
> - xe->info.graphics_verx100 = (*graphics)->ver * 100 + (*graphics)->rel;
> - } else {
> - /*
> - * GMD_ID platform; read IP version from hardware and select
> - * graphics descriptor based on the result.
> - */
> - ver = peek_gmdid(xe, GMD_ID.reg);
> - for (int i = 0; i < ARRAY_SIZE(graphics_ip_map); i++) {
> - if (ver == graphics_ip_map[i].ver) {
> - xe->info.graphics_verx100 = ver;
> - *graphics = graphics_ip_map[i].ip;
> -
> - break;
> - }
> - }
> + ver = peek_gmdid(xe, GMD_ID.reg);
> + for (int i = 0; i < ARRAY_SIZE(graphics_ip_map); i++) {
> + if (ver == graphics_ip_map[i].ver) {
> + xe->info.graphics_verx100 = ver;
> + *graphics = graphics_ip_map[i].ip;
>
> - if (!xe->info.graphics_verx100) {
> - drm_err(&xe->drm, "Hardware reports unknown graphics version %u.%02u\n",
> - ver / 100, ver % 100);
> + break;
> }
> }
>
> - if (desc->media) {
> - /*
> - * Pre-GMD_ID platform; device descriptor already points to
> - * the appropriate media descriptor.
> - */
> - *media = desc->media;
> - xe->info.media_verx100 = (*media)->ver * 100 + (*media)->rel;
> - } else {
> - /*
> - * GMD_ID platform; read IP version from hardware and select
> - * media descriptor based on the result.
> - *
> - * desc->media can also be NULL for a pre-GMD_ID platform that
> - * simply doesn't have media (e.g., PVC); in that case the
> - * attempt to read GMD_ID will return 0 (since there's no
> - * register at that location).
> - */
> - ver = peek_gmdid(xe, GMD_ID.reg + 0x380000);
> - if (ver == 0)
> - return;
> -
> - for (int i = 0; i < ARRAY_SIZE(media_ip_map); i++) {
> - if (ver == media_ip_map[i].ver) {
> - xe->info.media_verx100 = ver;
> - *media = media_ip_map[i].ip;
> -
> - break;
> - }
> - }
> + if (!xe->info.graphics_verx100) {
> + drm_err(&xe->drm, "Hardware reports unknown graphics version %u.%02u\n",
> + ver / 100, ver % 100);
> + }
> +
> + ver = peek_gmdid(xe, GMD_ID.reg + 0x380000);
> + for (int i = 0; i < ARRAY_SIZE(media_ip_map); i++) {
> + if (ver == media_ip_map[i].ver) {
> + xe->info.media_verx100 = ver;
> + *media = media_ip_map[i].ip;
>
> - if (!xe->info.media_verx100) {
> - drm_err(&xe->drm, "Hardware reports unknown media version %u.%02u\n",
> - ver / 100, ver % 100);
> + break;
> }
> }
> +
> + if (!xe->info.media_verx100) {
> + drm_err(&xe->drm, "Hardware reports unknown media version %u.%02u\n",
> + ver / 100, ver % 100);
> + }
> }
>
>
> @@ -492,9 +484,14 @@ static int xe_info_init(struct xe_device *xe,
>
> /*
> * If this platform supports GMD_ID, we'll detect the proper IP
> - * descriptor to use from hardware registers.
> + * descriptor to use from hardware registers. desc->graphics will only
> + * ever be set at this point for platforms before GMD_ID. In that case
> + * the IP descriptions and versions are simply derived from that.
> */
> - handle_gmdid(xe, desc, &graphics_desc, &media_desc);
> + if (desc->graphics)
> + handle_pre_gmdid(xe, desc, &graphics_desc, &media_desc);
> + else
> + handle_gmdid(xe, desc, &graphics_desc, &media_desc);
>
> /*
> * If we couldn't detect the graphics IP, that's considered a fatal
More information about the Intel-xe
mailing list