Re: [PATCH 3/6] drm/xe: Move xe->info.devid|revid initialization

[Gustavo Sousa <gustavo.sousa@intel.com>]

Michal Wajdeczko <michal.wajdeczko@intel.com> writes:

> The xe_info_init_early() is a place where we initialize those of
> the xe->info fields that do not require any additional hardware
> probes. Move the initialization of the devid/revid also there, but
> to avoid breaking the kunit helper, which also calls this function,
> keep their initialization separate in sub-function so we can easily
> stub it when running the kunit test.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/xe/tests/xe_pci.c      | 12 ++++++++++++
>  drivers/gpu/drm/xe/tests/xe_pci_test.h |  2 ++
>  drivers/gpu/drm/xe/xe_device.c         |  2 --
>  drivers/gpu/drm/xe/xe_pci.c            | 12 ++++++++++++
>  4 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c
> index 860409c579f8..7c3428ab4c89 100644
> --- a/drivers/gpu/drm/xe/tests/xe_pci.c
> +++ b/drivers/gpu/drm/xe/tests/xe_pci.c
> @@ -311,6 +311,17 @@ const void *xe_pci_id_gen_param(struct kunit *test, const void *prev, char *desc
>  }
>  EXPORT_SYMBOL_IF_KUNIT(xe_pci_id_gen_param);
>  
> +static void fake_init_devid(struct xe_device *xe)
> +{
> +	struct kunit *test = kunit_get_current_test();
> +	struct xe_pci_fake_data *data = test->priv;
> +
> +	if (data) {
> +		xe->info.devid = data->devid;
> +		xe->info.revid = data->revid;
> +	}
> +}
> +
>  static int fake_read_gmdid(struct xe_device *xe, enum xe_gmdid_type type,
>  			   u32 *ver, u32 *revid)
>  {
> @@ -369,6 +380,7 @@ int xe_pci_fake_device_init(struct xe_device *xe)
>  	xe->sriov.__mode = data && data->sriov_mode ?
>  			   data->sriov_mode : XE_SRIOV_MODE_NONE;
>  
> +	kunit_activate_static_stub(test, init_devid, fake_init_devid);
>  	kunit_activate_static_stub(test, read_gmdid, fake_read_gmdid);
>  	kunit_activate_static_stub(test, xe_info_probe_tile_count,
>  				   fake_xe_info_probe_tile_count);
> diff --git a/drivers/gpu/drm/xe/tests/xe_pci_test.h b/drivers/gpu/drm/xe/tests/xe_pci_test.h
> index 30505d1cbefc..46f961f75ecd 100644
> --- a/drivers/gpu/drm/xe/tests/xe_pci_test.h
> +++ b/drivers/gpu/drm/xe/tests/xe_pci_test.h
> @@ -22,6 +22,8 @@ struct xe_pci_fake_data {
>  	struct xe_step_info step;
>  	u32 graphics_verx100;
>  	u32 media_verx100;
> +	u16 devid;
> +	u8 revid;

Do we need to add those fields?

Looking through the rest of the series, I don't see us using them.  I
think we could just have fake_init_devid() do nothing for now.  We could
come back and add support for having devid and revid values when/if we
need them.

--
Gustavo Sousa

>  };
>  
>  int xe_pci_fake_device_init(struct xe_device *xe);
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index cdc7e0935c13..b498147dcf61 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -521,8 +521,6 @@ struct xe_device *xe_device_create(struct pci_dev *pdev)
>  	if (err)
>  		return ERR_PTR(err);
>  
> -	xe->info.devid = pdev->device;
> -	xe->info.revid = pdev->revision;
>  	xe->atomic_svm_timeslice_ms = 5;
>  	xe->min_run_period_lr_ms = 5;
>  
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index b1f5f0098f26..6b1a4cb7ccb0 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -727,6 +727,16 @@ static int handle_gmdid(struct xe_device *xe,
>  	return 0;
>  }
>  
> +static void init_devid(struct xe_device *xe)
> +{
> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> +
> +	KUNIT_STATIC_STUB_REDIRECT(init_devid, xe);
> +
> +	xe->info.devid = pdev->device;
> +	xe->info.revid = pdev->revision;
> +}
> +
>  /*
>   * Initialize device info content that only depends on static driver_data
>   * passed to the driver at probe time from PCI ID table.
> @@ -742,6 +752,8 @@ static int xe_info_init_early(struct xe_device *xe,
>  	xe->info.subplatform = subplatform_desc ?
>  		subplatform_desc->subplatform : XE_SUBPLATFORM_NONE;
>  
> +	init_devid(xe);
> +
>  	xe->info.dma_mask_size = desc->dma_mask_size;
>  	xe->info.va_bits = desc->va_bits;
>  	xe->info.vm_max_level = desc->vm_max_level;
> -- 
> 2.47.1