Re: [PATCH 3/7] accel/ivpu: Do not use SSID 1

From: Jeffrey Hugo <quic_jhugo@quicinc.com>
To: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>,
	<dri-devel@lists.freedesktop.org>
Cc: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com>,
	Oded Gabbay <ogabbay@kernel.org>,
	Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
Subject: Re: [PATCH 3/7] accel/ivpu: Do not use SSID 1
Date: Wed, 22 Mar 2023 08:25:09 -0600	[thread overview]
Message-ID: <78fbafe5-6ae2-89ce-eb36-83c869ff7755@quicinc.com> (raw)
In-Reply-To: <20230322091900.1982453-4-stanislaw.gruszka@linux.intel.com>

On 3/22/2023 3:18 AM, Stanislaw Gruszka wrote:
> The SSID=1 is used by the firmware as default value in
> case SSID mapping is not initialized. This allows
> detecting use of miss-configured memory contexts.
> The future FW versions may not allow using SSID=1.
> 
> Co-developed-by: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com>
> Signed-off-by: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
>   drivers/accel/ivpu/ivpu_drv.c | 4 ++--
>   drivers/accel/ivpu/ivpu_drv.h | 5 ++++-
>   2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
> index d9e311b40348..70245cf84593 100644
> --- a/drivers/accel/ivpu/ivpu_drv.c
> +++ b/drivers/accel/ivpu/ivpu_drv.c
> @@ -474,8 +474,8 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
>   
>   	vdev->hw->ops = &ivpu_hw_mtl_ops;
>   	vdev->platform = IVPU_PLATFORM_INVALID;
> -	vdev->context_xa_limit.min = IVPU_GLOBAL_CONTEXT_MMU_SSID + 1;
> -	vdev->context_xa_limit.max = IVPU_CONTEXT_LIMIT;
> +	vdev->context_xa_limit.min = IVPU_USER_CONTEXT_MIN_SSID;
> +	vdev->context_xa_limit.max = IVPU_USER_CONTEXT_MAX_SSID;
>   	atomic64_set(&vdev->unique_id_counter, 0);
>   	xa_init_flags(&vdev->context_xa, XA_FLAGS_ALLOC);
>   	xa_init_flags(&vdev->submitted_jobs_xa, XA_FLAGS_ALLOC1);
> diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
> index 1b2aa05840ad..ef12a38e06e1 100644
> --- a/drivers/accel/ivpu/ivpu_drv.h
> +++ b/drivers/accel/ivpu/ivpu_drv.h
> @@ -25,7 +25,10 @@
>   #define PCI_DEVICE_ID_MTL   0x7d1d
>   
>   #define IVPU_GLOBAL_CONTEXT_MMU_SSID 0
> -#define IVPU_CONTEXT_LIMIT	     64
> +/* SSID 1 is used by the VPU to represent invalid context */
> +#define IVPU_USER_CONTEXT_MIN_SSID   2
> +#define IVPU_USER_CONTEXT_MAX_SSID   (IVPU_USER_CONTEXT_MIN_SSID + 63)

The old MAX was 64.  Now the MAX is 65.  I'm guessing the intent is to 
keep the number of valid SSIDs constant (63), but it is unclear if SSID 
65 is valid for the FW.

Maybe the commit text could clarify this a bit?

> +
>   #define IVPU_NUM_ENGINES	     2
>   
>   #define IVPU_PLATFORM_SILICON 0