[PATCH 0/7] acell/ivpu: Fixes for 6.3

[PATCH 0/7] acell/ivpu: Fixes for 6.3

[Stanislaw Gruszka]

Various fixes intended for linux 6.3 relase.

Patch 6 is dependency for patch 7.

Stanislaw Gruszka (7):
  accel/ivpu: Do not access HW registers after unbind
  accel/ivpu: Cancel recovery work
  accel/ivpu: Do not use SSID 1
  accel/ivpu: Fix power down sequence
  accel/ivpu: Disable buttress on device removal
  accel/ivpu: Remove support for 1 tile SKUs
  accel/ivpu: Fix VPU clock calculation

 drivers/accel/ivpu/ivpu_drv.c    |  18 +++--
 drivers/accel/ivpu/ivpu_drv.h    |   7 +-
 drivers/accel/ivpu/ivpu_hw_mtl.c | 113 ++++++++++---------------------
 drivers/accel/ivpu/ivpu_job.c    |  11 ++-
 drivers/accel/ivpu/ivpu_pm.c     |  15 +++-
 drivers/accel/ivpu/ivpu_pm.h     |   1 +
 6 files changed, 78 insertions(+), 87 deletions(-)

-- 
2.25.1

[PATCH 1/7] accel/ivpu: Do not access HW registers after unbind

[Stanislaw Gruszka]

We should not access hardware after we unbind from the bus.

Use drm_dev_enter() / drm_dev_exit() to mark code sections where
hardware is accessed (and not already protected by other locks)
and drm_dev_unplug() to mark device is gone.

Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 drivers/accel/ivpu/ivpu_drv.c |  8 ++++++--
 drivers/accel/ivpu/ivpu_drv.h |  1 +
 drivers/accel/ivpu/ivpu_job.c | 11 +++++++++--
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index 231f29bb5025..ac06bbfca920 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -8,7 +8,6 @@
 #include <linux/pci.h>
 
 #include <drm/drm_accel.h>
-#include <drm/drm_drv.h>
 #include <drm/drm_file.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_ioctl.h>
@@ -118,6 +117,10 @@ static int ivpu_get_param_ioctl(struct drm_device *dev, void *data, struct drm_f
 	struct pci_dev *pdev = to_pci_dev(vdev->drm.dev);
 	struct drm_ivpu_param *args = data;
 	int ret = 0;
+	int idx;
+
+	if (!drm_dev_enter(dev, &idx))
+		return -ENODEV;
 
 	switch (args->param) {
 	case DRM_IVPU_PARAM_DEVICE_ID:
@@ -171,6 +174,7 @@ static int ivpu_get_param_ioctl(struct drm_device *dev, void *data, struct drm_f
 		break;
 	}
 
+	drm_dev_exit(idx);
 	return ret;
 }
 
@@ -622,7 +626,7 @@ static void ivpu_remove(struct pci_dev *pdev)
 {
 	struct ivpu_device *vdev = pci_get_drvdata(pdev);
 
-	drm_dev_unregister(&vdev->drm);
+	drm_dev_unplug(&vdev->drm);
 	ivpu_dev_fini(vdev);
 }
 
diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
index f47b4965db2e..1b2aa05840ad 100644
--- a/drivers/accel/ivpu/ivpu_drv.h
+++ b/drivers/accel/ivpu/ivpu_drv.h
@@ -7,6 +7,7 @@
 #define __IVPU_DRV_H__
 
 #include <drm/drm_device.h>
+#include <drm/drm_drv.h>
 #include <drm/drm_managed.h>
 #include <drm/drm_mm.h>
 #include <drm/drm_print.h>
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index 94068aedf97c..910301fae435 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -489,12 +489,12 @@ ivpu_job_prepare_bos_for_submit(struct drm_file *file, struct ivpu_job *job, u32
 
 int ivpu_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 {
-	int ret = 0;
 	struct ivpu_file_priv *file_priv = file->driver_priv;
 	struct ivpu_device *vdev = file_priv->vdev;
 	struct drm_ivpu_submit *params = data;
 	struct ivpu_job *job;
 	u32 *buf_handles;
+	int idx, ret;
 
 	if (params->engine > DRM_IVPU_ENGINE_COPY)
 		return -EINVAL;
@@ -523,6 +523,11 @@ int ivpu_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 		goto free_handles;
 	}
 
+	if (!drm_dev_enter(&vdev->drm, &idx)) {
+		ret = -ENODEV;
+		goto free_handles;
+	}
+
 	ivpu_dbg(vdev, JOB, "Submit ioctl: ctx %u buf_count %u\n",
 		 file_priv->ctx.id, params->buffer_count);
 
@@ -530,7 +535,7 @@ int ivpu_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 	if (!job) {
 		ivpu_err(vdev, "Failed to create job\n");
 		ret = -ENOMEM;
-		goto free_handles;
+		goto dev_exit;
 	}
 
 	ret = ivpu_job_prepare_bos_for_submit(file, job, buf_handles, params->buffer_count,
@@ -548,6 +553,8 @@ int ivpu_submit_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 
 job_put:
 	job_put(job);
+dev_exit:
+	drm_dev_exit(idx);
 free_handles:
 	kfree(buf_handles);
 
-- 
2.25.1

Re: [PATCH 1/7] accel/ivpu: Do not access HW registers after unbind

[Jeffrey Hugo]

On 3/22/2023 3:18 AM, Stanislaw Gruszka wrote:
> We should not access hardware after we unbind from the bus.
> 
> Use drm_dev_enter() / drm_dev_exit() to mark code sections where
> hardware is accessed (and not already protected by other locks)
> and drm_dev_unplug() to mark device is gone.
> 
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

Looks sane.

Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

[PATCH 2/7] accel/ivpu: Cancel recovery work

[Stanislaw Gruszka]

Prevent running recovery_work after device is removed.

Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 drivers/accel/ivpu/ivpu_drv.c |  2 ++
 drivers/accel/ivpu/ivpu_pm.c  | 15 +++++++++++++--
 drivers/accel/ivpu/ivpu_pm.h  |  1 +
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index ac06bbfca920..d9e311b40348 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -580,6 +580,8 @@ static void ivpu_dev_fini(struct ivpu_device *vdev)
 	ivpu_pm_disable(vdev);
 	ivpu_shutdown(vdev);
 	ivpu_job_done_thread_fini(vdev);
+	ivpu_pm_cancel_recovery(vdev);
+
 	ivpu_ipc_fini(vdev);
 	ivpu_fw_fini(vdev);
 	ivpu_mmu_global_context_fini(vdev);
diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c
index a880f1dd857e..df2e98cc0a56 100644
--- a/drivers/accel/ivpu/ivpu_pm.c
+++ b/drivers/accel/ivpu/ivpu_pm.c
@@ -98,11 +98,17 @@ static int ivpu_resume(struct ivpu_device *vdev)
 static void ivpu_pm_recovery_work(struct work_struct *work)
 {
 	struct ivpu_pm_info *pm = container_of(work, struct ivpu_pm_info, recovery_work);
-	struct ivpu_device *vdev =  pm->vdev;
+	struct ivpu_device *vdev = pm->vdev;
 	char *evt[2] = {"IVPU_PM_EVENT=IVPU_RECOVER", NULL};
 	int ret;
 
-	ret = pci_reset_function(to_pci_dev(vdev->drm.dev));
+retry:
+	ret = pci_try_reset_function(to_pci_dev(vdev->drm.dev));
+	if (ret == -EAGAIN && !drm_dev_is_unplugged(&vdev->drm)) {
+		cond_resched();
+		goto retry;
+	}
+
 	if (ret)
 		ivpu_err(vdev, "Failed to reset VPU: %d\n", ret);
 
@@ -302,6 +308,11 @@ int ivpu_pm_init(struct ivpu_device *vdev)
 	return 0;
 }
 
+void ivpu_pm_cancel_recovery(struct ivpu_device *vdev)
+{
+	cancel_work_sync(&vdev->pm->recovery_work);
+}
+
 void ivpu_pm_enable(struct ivpu_device *vdev)
 {
 	struct device *dev = vdev->drm.dev;
diff --git a/drivers/accel/ivpu/ivpu_pm.h b/drivers/accel/ivpu/ivpu_pm.h
index dc1b3758e13f..baca98187255 100644
--- a/drivers/accel/ivpu/ivpu_pm.h
+++ b/drivers/accel/ivpu/ivpu_pm.h
@@ -21,6 +21,7 @@ struct ivpu_pm_info {
 int ivpu_pm_init(struct ivpu_device *vdev);
 void ivpu_pm_enable(struct ivpu_device *vdev);
 void ivpu_pm_disable(struct ivpu_device *vdev);
+void ivpu_pm_cancel_recovery(struct ivpu_device *vdev);
 
 int ivpu_pm_suspend_cb(struct device *dev);
 int ivpu_pm_resume_cb(struct device *dev);
-- 
2.25.1

Re: [PATCH 2/7] accel/ivpu: Cancel recovery work

[Jeffrey Hugo]

On 3/22/2023 3:18 AM, Stanislaw Gruszka wrote:
> Prevent running recovery_work after device is removed.
> 
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
>   drivers/accel/ivpu/ivpu_drv.c |  2 ++
>   drivers/accel/ivpu/ivpu_pm.c  | 15 +++++++++++++--
>   drivers/accel/ivpu/ivpu_pm.h  |  1 +
>   3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
> index ac06bbfca920..d9e311b40348 100644
> --- a/drivers/accel/ivpu/ivpu_drv.c
> +++ b/drivers/accel/ivpu/ivpu_drv.c
> @@ -580,6 +580,8 @@ static void ivpu_dev_fini(struct ivpu_device *vdev)
>   	ivpu_pm_disable(vdev);
>   	ivpu_shutdown(vdev);
>   	ivpu_job_done_thread_fini(vdev);
> +	ivpu_pm_cancel_recovery(vdev);
> +
>   	ivpu_ipc_fini(vdev);
>   	ivpu_fw_fini(vdev);
>   	ivpu_mmu_global_context_fini(vdev);
> diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c
> index a880f1dd857e..df2e98cc0a56 100644
> --- a/drivers/accel/ivpu/ivpu_pm.c
> +++ b/drivers/accel/ivpu/ivpu_pm.c
> @@ -98,11 +98,17 @@ static int ivpu_resume(struct ivpu_device *vdev)
>   static void ivpu_pm_recovery_work(struct work_struct *work)
>   {
>   	struct ivpu_pm_info *pm = container_of(work, struct ivpu_pm_info, recovery_work);
> -	struct ivpu_device *vdev =  pm->vdev;
> +	struct ivpu_device *vdev = pm->vdev;
>   	char *evt[2] = {"IVPU_PM_EVENT=IVPU_RECOVER", NULL};
>   	int ret;
>   
> -	ret = pci_reset_function(to_pci_dev(vdev->drm.dev));
> +retry:
> +	ret = pci_try_reset_function(to_pci_dev(vdev->drm.dev));
> +	if (ret == -EAGAIN && !drm_dev_is_unplugged(&vdev->drm)) {
> +		cond_resched();
> +		goto retry;
> +	}
> +
>   	if (ret)
>   		ivpu_err(vdev, "Failed to reset VPU: %d\n", ret);

I'm unsure about this now.  Yes, you did fail to reset the VPU, but is 
it an error if the device is unplugged?  Feels like this message could 
be misleading in that corner case.

> @@ -302,6 +308,11 @@ int ivpu_pm_init(struct ivpu_device *vdev)
>   	return 0;
>   }
>   
> +void ivpu_pm_cancel_recovery(struct ivpu_device *vdev)
> +{
> +	cancel_work_sync(&vdev->pm->recovery_work);
> +}
> +
>   void ivpu_pm_enable(struct ivpu_device *vdev)
>   {
>   	struct device *dev = vdev->drm.dev;
> diff --git a/drivers/accel/ivpu/ivpu_pm.h b/drivers/accel/ivpu/ivpu_pm.h
> index dc1b3758e13f..baca98187255 100644
> --- a/drivers/accel/ivpu/ivpu_pm.h
> +++ b/drivers/accel/ivpu/ivpu_pm.h
> @@ -21,6 +21,7 @@ struct ivpu_pm_info {
>   int ivpu_pm_init(struct ivpu_device *vdev);
>   void ivpu_pm_enable(struct ivpu_device *vdev);
>   void ivpu_pm_disable(struct ivpu_device *vdev);
> +void ivpu_pm_cancel_recovery(struct ivpu_device *vdev);
>   
>   int ivpu_pm_suspend_cb(struct device *dev);
>   int ivpu_pm_resume_cb(struct device *dev);

Re: [PATCH 2/7] accel/ivpu: Cancel recovery work

[Stanislaw Gruszka]

On Wed, Mar 22, 2023 at 08:21:03AM -0600, Jeffrey Hugo wrote:
> On 3/22/2023 3:18 AM, Stanislaw Gruszka wrote:
> > Prevent running recovery_work after device is removed.
> > 
> > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> > ---
> >   drivers/accel/ivpu/ivpu_drv.c |  2 ++
> >   drivers/accel/ivpu/ivpu_pm.c  | 15 +++++++++++++--
> >   drivers/accel/ivpu/ivpu_pm.h  |  1 +
> >   3 files changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
> > index ac06bbfca920..d9e311b40348 100644
> > --- a/drivers/accel/ivpu/ivpu_drv.c
> > +++ b/drivers/accel/ivpu/ivpu_drv.c
> > @@ -580,6 +580,8 @@ static void ivpu_dev_fini(struct ivpu_device *vdev)
> >   	ivpu_pm_disable(vdev);
> >   	ivpu_shutdown(vdev);
> >   	ivpu_job_done_thread_fini(vdev);
> > +	ivpu_pm_cancel_recovery(vdev);
> > +
> >   	ivpu_ipc_fini(vdev);
> >   	ivpu_fw_fini(vdev);
> >   	ivpu_mmu_global_context_fini(vdev);
> > diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c
> > index a880f1dd857e..df2e98cc0a56 100644
> > --- a/drivers/accel/ivpu/ivpu_pm.c
> > +++ b/drivers/accel/ivpu/ivpu_pm.c
> > @@ -98,11 +98,17 @@ static int ivpu_resume(struct ivpu_device *vdev)
> >   static void ivpu_pm_recovery_work(struct work_struct *work)
> >   {
> >   	struct ivpu_pm_info *pm = container_of(work, struct ivpu_pm_info, recovery_work);
> > -	struct ivpu_device *vdev =  pm->vdev;
> > +	struct ivpu_device *vdev = pm->vdev;
> >   	char *evt[2] = {"IVPU_PM_EVENT=IVPU_RECOVER", NULL};
> >   	int ret;
> > -	ret = pci_reset_function(to_pci_dev(vdev->drm.dev));
> > +retry:
> > +	ret = pci_try_reset_function(to_pci_dev(vdev->drm.dev));
> > +	if (ret == -EAGAIN && !drm_dev_is_unplugged(&vdev->drm)) {
> > +		cond_resched();
> > +		goto retry;
> > +	}
> > +
> >   	if (ret)
> >   		ivpu_err(vdev, "Failed to reset VPU: %d\n", ret);
> 
> I'm unsure about this now.  Yes, you did fail to reset the VPU, but is it an
> error if the device is unplugged?  Feels like this message could be
> misleading in that corner case.

Yes, it's not elegant to print error message on unplug.
I'll change this to:

	if (ret && ret != -EAGAIN)
   		ivpu_err(vdev, "Failed to reset VPU: %d\n", ret);

Regards
Stanislaw

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

[Stanislaw Gruszka]

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)
+
 #define IVPU_NUM_ENGINES	     2
 
 #define IVPU_PLATFORM_SILICON 0
-- 
2.25.1

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

[Jeffrey Hugo]

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

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

[Stanislaw Gruszka]

On Wed, Mar 22, 2023 at 08:25:09AM -0600, Jeffrey Hugo wrote:
> 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?

Yes SSID=65 is valid, I'll update the changelog.

Regards
Stanislaw
> 

[PATCH 4/7] accel/ivpu: Fix power down sequence

[Stanislaw Gruszka]

Remove FPGA WA on power_down to skip checking for noc quiescent state.

Put VPU in reset before powering it down and skip manipulating
registers that are reset by the VPU reset.

This fixes power down errors there VPU is powered down just after VPU
is booted.

Co-developed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 drivers/accel/ivpu/ivpu_hw_mtl.c | 37 ++------------------------------
 1 file changed, 2 insertions(+), 35 deletions(-)

diff --git a/drivers/accel/ivpu/ivpu_hw_mtl.c b/drivers/accel/ivpu/ivpu_hw_mtl.c
index 62bfaa9081c4..70ca6de78060 100644
--- a/drivers/accel/ivpu/ivpu_hw_mtl.c
+++ b/drivers/accel/ivpu/ivpu_hw_mtl.c
@@ -403,11 +403,6 @@ static int ivpu_boot_host_ss_axi_enable(struct ivpu_device *vdev)
 	return ivpu_boot_host_ss_axi_drive(vdev, true);
 }
 
-static int ivpu_boot_host_ss_axi_disable(struct ivpu_device *vdev)
-{
-	return ivpu_boot_host_ss_axi_drive(vdev, false);
-}
-
 static int ivpu_boot_host_ss_top_noc_drive(struct ivpu_device *vdev, bool enable)
 {
 	int ret;
@@ -441,11 +436,6 @@ static int ivpu_boot_host_ss_top_noc_enable(struct ivpu_device *vdev)
 	return ivpu_boot_host_ss_top_noc_drive(vdev, true);
 }
 
-static int ivpu_boot_host_ss_top_noc_disable(struct ivpu_device *vdev)
-{
-	return ivpu_boot_host_ss_top_noc_drive(vdev, false);
-}
-
 static void ivpu_boot_pwr_island_trickle_drive(struct ivpu_device *vdev, bool enable)
 {
 	u32 val = REGV_RD32(MTL_VPU_HOST_SS_AON_PWR_ISLAND_TRICKLE_EN0);
@@ -504,16 +494,6 @@ static void ivpu_boot_dpu_active_drive(struct ivpu_device *vdev, bool enable)
 	REGV_WR32(MTL_VPU_HOST_SS_AON_DPU_ACTIVE, val);
 }
 
-static int ivpu_boot_pwr_domain_disable(struct ivpu_device *vdev)
-{
-	ivpu_boot_dpu_active_drive(vdev, false);
-	ivpu_boot_pwr_island_isolation_drive(vdev, true);
-	ivpu_boot_pwr_island_trickle_drive(vdev, false);
-	ivpu_boot_pwr_island_drive(vdev, false);
-
-	return ivpu_boot_wait_for_pwr_island_status(vdev, 0x0);
-}
-
 static int ivpu_boot_pwr_domain_enable(struct ivpu_device *vdev)
 {
 	int ret;
@@ -797,21 +777,8 @@ static int ivpu_hw_mtl_power_down(struct ivpu_device *vdev)
 {
 	int ret = 0;
 
-	/* FPGA requires manual clearing of IP_Reset bit by enabling quiescent state */
-	if (ivpu_is_fpga(vdev)) {
-		if (ivpu_boot_host_ss_top_noc_disable(vdev)) {
-			ivpu_err(vdev, "Failed to disable TOP NOC\n");
-			ret = -EIO;
-		}
-
-		if (ivpu_boot_host_ss_axi_disable(vdev)) {
-			ivpu_err(vdev, "Failed to disable AXI\n");
-			ret = -EIO;
-		}
-	}
-
-	if (ivpu_boot_pwr_domain_disable(vdev)) {
-		ivpu_err(vdev, "Failed to disable power domain\n");
+	if (ivpu_hw_mtl_reset(vdev)) {
+		ivpu_err(vdev, "Failed to reset the VPU\n");
 		ret = -EIO;
 	}
 
-- 
2.25.1

Re: [PATCH 4/7] accel/ivpu: Fix power down sequence

[Jeffrey Hugo]

On 3/22/2023 3:18 AM, Stanislaw Gruszka wrote:
> Remove FPGA WA on power_down to skip checking for noc quiescent state.

WA is workaround, right?  Maybe spell it out here?

> Put VPU in reset before powering it down and skip manipulating
> registers that are reset by the VPU reset.
> 
> This fixes power down errors there VPU is powered down just after VPU
> is booted.

there -> where ?

> Co-developed-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

With the above addressed,
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

[PATCH 5/7] accel/ivpu: Disable buttress on device removal

[Stanislaw Gruszka]

Use pci_set_power_state() to disable buttress when device is removed.
This is workaround of hardware bug that hangs the system.

Additionally not disabling buttress prevents CPU enter deeper Pkg-C
states when the driver is unloaded or fail to probe.

Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 drivers/accel/ivpu/ivpu_drv.c    | 4 ++++
 drivers/accel/ivpu/ivpu_drv.h    | 1 +
 drivers/accel/ivpu/ivpu_hw_mtl.c | 1 +
 3 files changed, 6 insertions(+)

diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index 70245cf84593..6a320a73e3cc 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -569,6 +569,8 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
 	ivpu_mmu_global_context_fini(vdev);
 err_power_down:
 	ivpu_hw_power_down(vdev);
+	if (IVPU_WA(d3hot_after_power_off))
+		pci_set_power_state(to_pci_dev(vdev->drm.dev), PCI_D3hot);
 err_xa_destroy:
 	xa_destroy(&vdev->submitted_jobs_xa);
 	xa_destroy(&vdev->context_xa);
@@ -579,6 +581,8 @@ static void ivpu_dev_fini(struct ivpu_device *vdev)
 {
 	ivpu_pm_disable(vdev);
 	ivpu_shutdown(vdev);
+	if (IVPU_WA(d3hot_after_power_off))
+		pci_set_power_state(to_pci_dev(vdev->drm.dev), PCI_D3hot);
 	ivpu_job_done_thread_fini(vdev);
 	ivpu_pm_cancel_recovery(vdev);
 
diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
index ef12a38e06e1..d3013fbd13b3 100644
--- a/drivers/accel/ivpu/ivpu_drv.h
+++ b/drivers/accel/ivpu/ivpu_drv.h
@@ -74,6 +74,7 @@
 struct ivpu_wa_table {
 	bool punit_disabled;
 	bool clear_runtime_mem;
+	bool d3hot_after_power_off;
 };
 
 struct ivpu_hw_info;
diff --git a/drivers/accel/ivpu/ivpu_hw_mtl.c b/drivers/accel/ivpu/ivpu_hw_mtl.c
index 70ca6de78060..133ba33d2866 100644
--- a/drivers/accel/ivpu/ivpu_hw_mtl.c
+++ b/drivers/accel/ivpu/ivpu_hw_mtl.c
@@ -101,6 +101,7 @@ static void ivpu_hw_wa_init(struct ivpu_device *vdev)
 {
 	vdev->wa.punit_disabled = ivpu_is_fpga(vdev);
 	vdev->wa.clear_runtime_mem = false;
+	vdev->wa.d3hot_after_power_off = true;
 }
 
 static void ivpu_hw_timeouts_init(struct ivpu_device *vdev)
-- 
2.25.1

Re: [PATCH 5/7] accel/ivpu: Disable buttress on device removal

[Jeffrey Hugo]

On 3/22/2023 3:18 AM, Stanislaw Gruszka wrote:
> Use pci_set_power_state() to disable buttress when device is removed.
> This is workaround of hardware bug that hangs the system.
> 
> Additionally not disabling buttress prevents CPU enter deeper Pkg-C
> states when the driver is unloaded or fail to probe.
> 
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

[PATCH 6/7] accel/ivpu: Remove support for 1 tile SKUs

[Stanislaw Gruszka]

The support for single tile SKUs was dropped from MTL.
Note that we can still boot the VPU with 1-tile work point
config - this is independent from number of tiles present
in the VPU.

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_hw_mtl.c | 59 ++++++++++----------------------
 1 file changed, 18 insertions(+), 41 deletions(-)

diff --git a/drivers/accel/ivpu/ivpu_hw_mtl.c b/drivers/accel/ivpu/ivpu_hw_mtl.c
index 133ba33d2866..98c8a4aa25f0 100644
--- a/drivers/accel/ivpu/ivpu_hw_mtl.c
+++ b/drivers/accel/ivpu/ivpu_hw_mtl.c
@@ -12,20 +12,20 @@
 #include "ivpu_mmu.h"
 #include "ivpu_pm.h"
 
-#define TILE_FUSE_ENABLE_BOTH	     0x0
-#define TILE_FUSE_ENABLE_UPPER	     0x1
-#define TILE_FUSE_ENABLE_LOWER	     0x2
-
-#define TILE_SKU_BOTH_MTL	     0x3630
-#define TILE_SKU_LOWER_MTL	     0x3631
-#define TILE_SKU_UPPER_MTL	     0x3632
+#define TILE_FUSE_ENABLE_BOTH        0x0
+#define TILE_SKU_BOTH_MTL            0x3630
 
 /* Work point configuration values */
-#define WP_CONFIG_1_TILE_5_3_RATIO   0x0101
-#define WP_CONFIG_1_TILE_4_3_RATIO   0x0102
-#define WP_CONFIG_2_TILE_5_3_RATIO   0x0201
-#define WP_CONFIG_2_TILE_4_3_RATIO   0x0202
-#define WP_CONFIG_0_TILE_PLL_OFF     0x0000
+#define CONFIG_1_TILE                0x01
+#define CONFIG_2_TILE                0x02
+#define PLL_RATIO_5_3                0x01
+#define PLL_RATIO_4_3                0x02
+#define WP_CONFIG(tile, ratio)       (((tile) << 8) | (ratio))
+#define WP_CONFIG_1_TILE_5_3_RATIO   WP_CONFIG(CONFIG_1_TILE, PLL_RATIO_5_3)
+#define WP_CONFIG_1_TILE_4_3_RATIO   WP_CONFIG(CONFIG_1_TILE, PLL_RATIO_4_3)
+#define WP_CONFIG_2_TILE_5_3_RATIO   WP_CONFIG(CONFIG_2_TILE, PLL_RATIO_5_3)
+#define WP_CONFIG_2_TILE_4_3_RATIO   WP_CONFIG(CONFIG_2_TILE, PLL_RATIO_4_3)
+#define WP_CONFIG_0_TILE_PLL_OFF     WP_CONFIG(0, 0)
 
 #define PLL_REF_CLK_FREQ	     (50 * 1000000)
 #define PLL_SIMULATION_FREQ	     (10 * 1000000)
@@ -219,7 +219,8 @@ static int ivpu_pll_drive(struct ivpu_device *vdev, bool enable)
 		config = 0;
 	}
 
-	ivpu_dbg(vdev, PM, "PLL workpoint request: %d Hz\n", PLL_RATIO_TO_FREQ(target_ratio));
+	ivpu_dbg(vdev, PM, "PLL workpoint request: config 0x%04x pll ratio 0x%x\n",
+		 config, target_ratio);
 
 	ret = ivpu_pll_cmd_send(vdev, hw->pll.min_ratio, hw->pll.max_ratio, target_ratio, config);
 	if (ret) {
@@ -610,34 +611,10 @@ static int ivpu_boot_d0i3_drive(struct ivpu_device *vdev, bool enable)
 static int ivpu_hw_mtl_info_init(struct ivpu_device *vdev)
 {
 	struct ivpu_hw_info *hw = vdev->hw;
-	u32 tile_fuse;
-
-	tile_fuse = REGB_RD32(MTL_BUTTRESS_TILE_FUSE);
-	if (!REG_TEST_FLD(MTL_BUTTRESS_TILE_FUSE, VALID, tile_fuse))
-		ivpu_warn(vdev, "Tile Fuse: Invalid (0x%x)\n", tile_fuse);
-
-	hw->tile_fuse = REG_GET_FLD(MTL_BUTTRESS_TILE_FUSE, SKU, tile_fuse);
-	switch (hw->tile_fuse) {
-	case TILE_FUSE_ENABLE_LOWER:
-		hw->sku = TILE_SKU_LOWER_MTL;
-		hw->config = WP_CONFIG_1_TILE_5_3_RATIO;
-		ivpu_dbg(vdev, MISC, "Tile Fuse: Enable Lower\n");
-		break;
-	case TILE_FUSE_ENABLE_UPPER:
-		hw->sku = TILE_SKU_UPPER_MTL;
-		hw->config = WP_CONFIG_1_TILE_4_3_RATIO;
-		ivpu_dbg(vdev, MISC, "Tile Fuse: Enable Upper\n");
-		break;
-	case TILE_FUSE_ENABLE_BOTH:
-		hw->sku = TILE_SKU_BOTH_MTL;
-		hw->config = WP_CONFIG_2_TILE_5_3_RATIO;
-		ivpu_dbg(vdev, MISC, "Tile Fuse: Enable Both\n");
-		break;
-	default:
-		hw->config = WP_CONFIG_0_TILE_PLL_OFF;
-		ivpu_dbg(vdev, MISC, "Tile Fuse: Disable\n");
-		break;
-	}
+
+	hw->tile_fuse = TILE_FUSE_ENABLE_BOTH;
+	hw->sku = TILE_SKU_BOTH_MTL;
+	hw->config = WP_CONFIG_2_TILE_4_3_RATIO;
 
 	ivpu_pll_init_frequency_ratios(vdev);
 
-- 
2.25.1

Re: [PATCH 6/7] accel/ivpu: Remove support for 1 tile SKUs

[Jeffrey Hugo]

On 3/22/2023 3:18 AM, Stanislaw Gruszka wrote:
> The support for single tile SKUs was dropped from MTL.
> Note that we can still boot the VPU with 1-tile work point
> config - this is independent from number of tiles present
> in the VPU.
> 
> 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>

Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

[PATCH 7/7] accel/ivpu: Fix VPU clock calculation

[Stanislaw Gruszka]

VPU cpu clock frequency depends on the workpoint configuration
that was granted by the punit. Previously driver was passing
incorrect frequency to the VPU firmware.

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_hw_mtl.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/accel/ivpu/ivpu_hw_mtl.c b/drivers/accel/ivpu/ivpu_hw_mtl.c
index 98c8a4aa25f0..382ec127be8e 100644
--- a/drivers/accel/ivpu/ivpu_hw_mtl.c
+++ b/drivers/accel/ivpu/ivpu_hw_mtl.c
@@ -29,7 +29,6 @@
 
 #define PLL_REF_CLK_FREQ	     (50 * 1000000)
 #define PLL_SIMULATION_FREQ	     (10 * 1000000)
-#define PLL_RATIO_TO_FREQ(x)	     ((x) * PLL_REF_CLK_FREQ)
 #define PLL_DEFAULT_EPP_VALUE	     0x80
 
 #define TIM_SAFE_ENABLE		     0xf1d0dead
@@ -789,6 +788,19 @@ static void ivpu_hw_mtl_wdt_disable(struct ivpu_device *vdev)
 	REGV_WR32(MTL_VPU_CPU_SS_TIM_GEN_CONFIG, val);
 }
 
+static u32 ivpu_hw_mtl_pll_to_freq(u32 ratio, u32 config)
+{
+	u32 pll_clock = PLL_REF_CLK_FREQ * ratio;
+	u32 cpu_clock;
+
+	if ((config & 0xff) == PLL_RATIO_4_3)
+		cpu_clock = pll_clock * 2 / 4;
+	else
+		cpu_clock = pll_clock * 2 / 5;
+
+	return cpu_clock;
+}
+
 /* Register indirect accesses */
 static u32 ivpu_hw_mtl_reg_pll_freq_get(struct ivpu_device *vdev)
 {
@@ -800,7 +812,7 @@ static u32 ivpu_hw_mtl_reg_pll_freq_get(struct ivpu_device *vdev)
 	if (!ivpu_is_silicon(vdev))
 		return PLL_SIMULATION_FREQ;
 
-	return PLL_RATIO_TO_FREQ(pll_curr_ratio);
+	return ivpu_hw_mtl_pll_to_freq(pll_curr_ratio, vdev->hw->config);
 }
 
 static u32 ivpu_hw_mtl_reg_telemetry_offset_get(struct ivpu_device *vdev)
-- 
2.25.1

Re: [PATCH 7/7] accel/ivpu: Fix VPU clock calculation

[Jeffrey Hugo]

On 3/22/2023 3:19 AM, Stanislaw Gruszka wrote:
> VPU cpu clock frequency depends on the workpoint configuration
> that was granted by the punit. Previously driver was passing
> incorrect frequency to the VPU firmware.

This looks like past tense.  I believe the preference is to use the 
present tense for commit text.  Something like "the driver calculates 
the wrong frequency because it ignores the workpoint config and this 
causes X.  Fix this by using the workpoint config in the freq calculations".

Should this have a fixes tag?  Sounds like this addresses a bug.

> 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_hw_mtl.c | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/accel/ivpu/ivpu_hw_mtl.c b/drivers/accel/ivpu/ivpu_hw_mtl.c
> index 98c8a4aa25f0..382ec127be8e 100644
> --- a/drivers/accel/ivpu/ivpu_hw_mtl.c
> +++ b/drivers/accel/ivpu/ivpu_hw_mtl.c
> @@ -29,7 +29,6 @@
>   
>   #define PLL_REF_CLK_FREQ	     (50 * 1000000)
>   #define PLL_SIMULATION_FREQ	     (10 * 1000000)
> -#define PLL_RATIO_TO_FREQ(x)	     ((x) * PLL_REF_CLK_FREQ)
>   #define PLL_DEFAULT_EPP_VALUE	     0x80
>   
>   #define TIM_SAFE_ENABLE		     0xf1d0dead
> @@ -789,6 +788,19 @@ static void ivpu_hw_mtl_wdt_disable(struct ivpu_device *vdev)
>   	REGV_WR32(MTL_VPU_CPU_SS_TIM_GEN_CONFIG, val);
>   }
>   
> +static u32 ivpu_hw_mtl_pll_to_freq(u32 ratio, u32 config)
> +{
> +	u32 pll_clock = PLL_REF_CLK_FREQ * ratio;
> +	u32 cpu_clock;
> +
> +	if ((config & 0xff) == PLL_RATIO_4_3)
> +		cpu_clock = pll_clock * 2 / 4;
> +	else
> +		cpu_clock = pll_clock * 2 / 5;
> +
> +	return cpu_clock;
> +}
> +
>   /* Register indirect accesses */
>   static u32 ivpu_hw_mtl_reg_pll_freq_get(struct ivpu_device *vdev)
>   {
> @@ -800,7 +812,7 @@ static u32 ivpu_hw_mtl_reg_pll_freq_get(struct ivpu_device *vdev)
>   	if (!ivpu_is_silicon(vdev))
>   		return PLL_SIMULATION_FREQ;
>   
> -	return PLL_RATIO_TO_FREQ(pll_curr_ratio);
> +	return ivpu_hw_mtl_pll_to_freq(pll_curr_ratio, vdev->hw->config);
>   }
>   
>   static u32 ivpu_hw_mtl_reg_telemetry_offset_get(struct ivpu_device *vdev)

Re: [PATCH 7/7] accel/ivpu: Fix VPU clock calculation

[Stanislaw Gruszka]

On Wed, Mar 22, 2023 at 08:52:56AM -0600, Jeffrey Hugo wrote:
> On 3/22/2023 3:19 AM, Stanislaw Gruszka wrote:
> > VPU cpu clock frequency depends on the workpoint configuration
> > that was granted by the punit. Previously driver was passing
> > incorrect frequency to the VPU firmware.
> 
> This looks like past tense.  I believe the preference is to use the present
> tense for commit text.  Something like "the driver calculates the wrong
> frequency because it ignores the workpoint config and this causes X.  Fix
> this by using the workpoint config in the freq calculations".

Will do.

> Should this have a fixes tag?  Sounds like this addresses a bug.

Not sure how this is done, but seems all my previous patches for ivpu
have Fixes tag in commit message, even if I did not post the tag in
the patches. Seems to be a feature of drm git tooling and can be
easily done by committer ?

Regards
Stanislaw

Re: [PATCH 7/7] accel/ivpu: Fix VPU clock calculation

[Stanislaw Gruszka]

On Wed, Mar 22, 2023 at 05:28:48PM +0100, Stanislaw Gruszka wrote:
> On Wed, Mar 22, 2023 at 08:52:56AM -0600, Jeffrey Hugo wrote:
> > On 3/22/2023 3:19 AM, Stanislaw Gruszka wrote:
> > > VPU cpu clock frequency depends on the workpoint configuration
> > > that was granted by the punit. Previously driver was passing
> > > incorrect frequency to the VPU firmware.
> > 
> > This looks like past tense.  I believe the preference is to use the present
> > tense for commit text.  Something like "the driver calculates the wrong
> > frequency because it ignores the workpoint config and this causes X.  Fix
> > this by using the workpoint config in the freq calculations".
> 
> Will do.
> 
> > Should this have a fixes tag?  Sounds like this addresses a bug.
> 
> Not sure how this is done, but seems all my previous patches for ivpu
> have Fixes tag in commit message, even if I did not post the tag in
> the patches. Seems to be a feature of drm git tooling and can be
> easily done by committer ?

Tags were added by Jacek,  I'll add them to the patchset then.

Regards
Stanislaw