[Intel-xe] [PATCH 2/3] drm/xe/kunit: Add stub to peek_gmdid

[Matt Roper]

On Fri, Nov 17, 2023 at 03:14:10PM -0800, Lucas De Marchi wrote:
> Currently it's not possible to test the WAs for platforms using gmdid
> since they don't have the IP information on the descriptor struct. In
> order to allow that, add a stub function for peek_gmdid() that is
> activated when the test executes, replacing the iomap and read of the
> real register.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
>  drivers/gpu/drm/xe/tests/xe_pci.c      | 19 +++++++++++++++++++
>  drivers/gpu/drm/xe/tests/xe_pci_test.h |  6 ++++++
>  drivers/gpu/drm/xe/xe_pci.c            |  2 ++
>  drivers/gpu/drm/xe/xe_step.h           |  2 ++
>  4 files changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c
> index d1bc029f7a13..ef4f2ae140f5 100644
> --- a/drivers/gpu/drm/xe/tests/xe_pci.c
> +++ b/drivers/gpu/drm/xe/tests/xe_pci.c
> @@ -9,6 +9,7 @@
>  
>  #include <kunit/test-bug.h>
>  #include <kunit/test.h>
> +#include <kunit/test-bug.h>
>  #include <kunit/visibility.h>
>  
>  struct kunit_test_data {
> @@ -107,6 +108,22 @@ void xe_call_for_each_media_ip(xe_media_fn xe_fn)
>  }
>  EXPORT_SYMBOL_IF_KUNIT(xe_call_for_each_media_ip);
>  
> +static void fake_peek_gmdid(struct xe_device *xe, u32 gmdid_offset, u32 *ver,
> +			    u32 *revid)
> +{
> +	struct kunit *test = kunit_get_current_test();
> +	struct xe_pci_fake_data *data = test->priv;
> +	const u32 MEDIA_BASE = 0x380000;
> +
> +	if (gmdid_offset & MEDIA_BASE) {

This works for the two offsets we might get today, but seems like a
strange way to write this.  Something like like ">" instead of "&" might
be more intuitive?

> +		*ver = data->media_verx100;
> +		*revid = xe_step_to_gmdid(data->media_step);
> +	} else {
> +		*ver = data->graphics_verx100;
> +		*revid = xe_step_to_gmdid(data->graphics_step);
> +	}
> +}
> +
>  int xe_pci_fake_device_init(struct xe_device *xe)
>  {
>  	struct kunit *test = kunit_get_current_test();
> @@ -145,6 +162,8 @@ int xe_pci_fake_device_init(struct xe_device *xe)
>  		return -ENODEV;
>  
>  done:
> +	kunit_activate_static_stub(test, peek_gmdid, fake_peek_gmdid);
> +
>  	xe_info_init(xe, desc, subplatform_desc);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/xe/tests/xe_pci_test.h b/drivers/gpu/drm/xe/tests/xe_pci_test.h
> index b4b3fb2df09c..811ffe5bd9fd 100644
> --- a/drivers/gpu/drm/xe/tests/xe_pci_test.h
> +++ b/drivers/gpu/drm/xe/tests/xe_pci_test.h
> @@ -6,6 +6,8 @@
>  #ifndef _XE_PCI_TEST_H_
>  #define _XE_PCI_TEST_H_
>  
> +#include <linux/types.h>
> +
>  #include "xe_platform_types.h"
>  
>  struct xe_device;
> @@ -23,6 +25,10 @@ void xe_call_for_each_media_ip(xe_media_fn xe_fn);
>  struct xe_pci_fake_data {
>  	enum xe_platform platform;
>  	enum xe_subplatform subplatform;
> +	u32 graphics_verx100;
> +	u32 media_verx100;
> +	u32 graphics_step;
> +	u32 media_step;
>  };
>  
>  int xe_pci_fake_device_init(struct xe_device *xe);
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 682ba188e456..7c85e88f707c 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -5,6 +5,7 @@
>  
>  #include "xe_pci.h"
>  
> +#include <kunit/static_stub.h>
>  #include <linux/device/driver.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
> @@ -442,6 +443,7 @@ find_subplatform(const struct xe_device *xe, const struct xe_device_desc *desc)
>  
>  static void peek_gmdid(struct xe_device *xe, u32 gmdid_offset, u32 *ver, u32 *revid)
>  {
> +	KUNIT_STATIC_STUB_REDIRECT(peek_gmdid, xe, gmdid_offset, ver, revid);

It looks like this expands to a "do {...} while" so does it need to go
after the local variable declarations?


Matt

>  	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>  	void __iomem *map = pci_iomap_range(pdev, 0, gmdid_offset, sizeof(u32));
>  	u32 val;
> diff --git a/drivers/gpu/drm/xe/xe_step.h b/drivers/gpu/drm/xe/xe_step.h
> index a384b640f2af..686cb59200c2 100644
> --- a/drivers/gpu/drm/xe/xe_step.h
> +++ b/drivers/gpu/drm/xe/xe_step.h
> @@ -16,6 +16,8 @@ struct xe_step_info xe_step_pre_gmdid_get(struct xe_device *xe);
>  struct xe_step_info xe_step_gmdid_get(struct xe_device *xe,
>  				      u32 graphics_gmdid_revid,
>  				      u32 media_gmdid_revid);
> +static inline u32 xe_step_to_gmdid(enum xe_step step) { return step - STEP_A0; }
> +
>  const char *xe_step_name(enum xe_step step);
>  
>  #endif
> -- 
> 2.40.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation