[Intel-gfx] [PATCH] drm/i915/guc: Add GuC Load time to debugfs
Calculate the time that GuC takes to load.
This information could be very useful in
determining if GuC is taking unreasonably long time
to load in a certain platforms.
Cc: Oscar Mateo
Cc: Michal Wajdeczko
Signed-off-by: Anusha Srivatsa
---
drivers/gpu/drm/i915/i915_debugfs.c | 4
drivers/gpu/drm/i915/intel_guc_loader.c | 4
drivers/gpu/drm/i915/intel_uc.h | 3 +++
3 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 48572b157222..9283fc714705 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2379,6 +2379,10 @@ static int i915_guc_load_status_info(struct seq_file *m,
void *data)
guc_fw->major_ver_wanted, guc_fw->minor_ver_wanted);
seq_printf(m, "\tversion found: %d.%d\n",
guc_fw->major_ver_found, guc_fw->minor_ver_found);
+ seq_printf(m, "\tLoad time is %lu ms\n",
+ jiffies_to_msecs(dev_priv->guc.guc_finish_load -
+ dev_priv->guc.guc_start_load));
+
seq_printf(m, "\theader: offset is %d; size = %d\n",
guc_fw->header_offset, guc_fw->header_size);
seq_printf(m, "\tuCode: offset is %d; size = %d\n",
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 8b0ae7fce7f2..1c5059b930f9 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -194,6 +194,7 @@ static inline bool guc_ucode_response(struct
drm_i915_private *dev_priv,
static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
struct i915_vma *vma)
{
+ struct intel_guc *guc = &dev_priv->guc;
struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
unsigned long offset;
struct sg_table *sg = vma->pages;
@@ -226,6 +227,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private
*dev_priv,
/* Finally start the DMA */
I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));
+ guc->guc_start_load = jiffies;
/*
* Wait for the DMA to complete & the GuC to start up.
@@ -240,6 +242,8 @@ static int guc_ucode_xfer_dma(struct drm_i915_private
*dev_priv,
DRM_DEBUG_DRIVER("DMA status 0x%x, GuC status 0x%x\n",
I915_READ(DMA_CTRL), status);
+ guc->guc_finish_load = jiffies;
+
if ((status & GS_BOOTROM_MASK) == GS_BOOTROM_RSA_FAILED) {
DRM_ERROR("GuC firmware signature verification failed\n");
ret = -ENOEXEC;
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 22ae52b17b0f..3d5a15ed1995 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -210,6 +210,9 @@ struct intel_guc {
/* GuC's FW specific notify function */
void (*notify)(struct intel_guc *guc);
+
+ unsigned long guc_start_load;
+ unsigned long guc_finish_load;
};
struct intel_huc {
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Add GuC Load time to debugfs
On Thu, Aug 24, 2017 at 09:38:06PM -0700, Anusha Srivatsa wrote:
> Calculate the time that GuC takes to load.
> This information could be very useful in
> determining if GuC is taking unreasonably long time
> to load in a certain platforms.
>
> Cc: Oscar Mateo
> Cc: Michal Wajdeczko
> Signed-off-by: Anusha Srivatsa
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 4
> drivers/gpu/drm/i915/intel_guc_loader.c | 4
> drivers/gpu/drm/i915/intel_uc.h | 3 +++
> 3 files changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 48572b157222..9283fc714705 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2379,6 +2379,10 @@ static int i915_guc_load_status_info(struct seq_file
> *m, void *data)
> guc_fw->major_ver_wanted, guc_fw->minor_ver_wanted);
> seq_printf(m, "\tversion found: %d.%d\n",
> guc_fw->major_ver_found, guc_fw->minor_ver_found);
> + seq_printf(m, "\tLoad time is %lu ms\n",
> +jiffies_to_msecs(dev_priv->guc.guc_finish_load -
> +dev_priv->guc.guc_start_load));
> +
> seq_printf(m, "\theader: offset is %d; size = %d\n",
> guc_fw->header_offset, guc_fw->header_size);
> seq_printf(m, "\tuCode: offset is %d; size = %d\n",
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 8b0ae7fce7f2..1c5059b930f9 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -194,6 +194,7 @@ static inline bool guc_ucode_response(struct
> drm_i915_private *dev_priv,
> static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
> struct i915_vma *vma)
> {
> + struct intel_guc *guc = &dev_priv->guc;
> struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
> unsigned long offset;
> struct sg_table *sg = vma->pages;
> @@ -226,6 +227,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private
> *dev_priv,
>
> /* Finally start the DMA */
> I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));
> + guc->guc_start_load = jiffies;
>
> /*
>* Wait for the DMA to complete & the GuC to start up.
> @@ -240,6 +242,8 @@ static int guc_ucode_xfer_dma(struct drm_i915_private
> *dev_priv,
> DRM_DEBUG_DRIVER("DMA status 0x%x, GuC status 0x%x\n",
> I915_READ(DMA_CTRL), status);
>
> + guc->guc_finish_load = jiffies;
> +
On error/timeout we don't know if loading was finished/completed and your
calculations will be wrong. End time shall be captured before any debug logs
to more accurate. Btw, if loading time is so important, maybe it should be
also printed here as part of above DRM_DEBUG ?
> if ((status & GS_BOOTROM_MASK) == GS_BOOTROM_RSA_FAILED) {
> DRM_ERROR("GuC firmware signature verification failed\n");
> ret = -ENOEXEC;
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 22ae52b17b0f..3d5a15ed1995 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -210,6 +210,9 @@ struct intel_guc {
>
> /* GuC's FW specific notify function */
> void (*notify)(struct intel_guc *guc);
> +
> + unsigned long guc_start_load;
> + unsigned long guc_finish_load;
No need to keep both jiffies here. Calculate "load_time_in_ms" in the
loader function and store only final result. Maybe better place for
this result would be "intel_uc_fw" ? Then we can do the same for Huc.
-Michal
> };
>
> struct intel_huc {
> --
> 2.11.0
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Add GuC Load time to debugfs
Cc: Sujaritha
On 08/24/2017 09:38 PM, Anusha Srivatsa wrote:
Calculate the time that GuC takes to load.
This information could be very useful in
determining if GuC is taking unreasonably long time
to load in a certain platforms.
Cc: Oscar Mateo
Cc: Michal Wajdeczko
Signed-off-by: Anusha Srivatsa
---
drivers/gpu/drm/i915/i915_debugfs.c | 4
drivers/gpu/drm/i915/intel_guc_loader.c | 4
drivers/gpu/drm/i915/intel_uc.h | 3 +++
3 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 48572b157222..9283fc714705 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2379,6 +2379,10 @@ static int i915_guc_load_status_info(struct seq_file *m,
void *data)
guc_fw->major_ver_wanted, guc_fw->minor_ver_wanted);
seq_printf(m, "\tversion found: %d.%d\n",
guc_fw->major_ver_found, guc_fw->minor_ver_found);
+ seq_printf(m, "\tLoad time is %lu ms\n",
+ jiffies_to_msecs(dev_priv->guc.guc_finish_load -
+ dev_priv->guc.guc_start_load));
+
seq_printf(m, "\theader: offset is %d; size = %d\n",
guc_fw->header_offset, guc_fw->header_size);
seq_printf(m, "\tuCode: offset is %d; size = %d\n",
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 8b0ae7fce7f2..1c5059b930f9 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -194,6 +194,7 @@ static inline bool guc_ucode_response(struct
drm_i915_private *dev_priv,
static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
struct i915_vma *vma)
{
+ struct intel_guc *guc = &dev_priv->guc;
struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
unsigned long offset;
struct sg_table *sg = vma->pages;
@@ -226,6 +227,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private
*dev_priv,
/* Finally start the DMA */
I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));
+ guc->guc_start_load = jiffies;
/*
* Wait for the DMA to complete & the GuC to start up.
@@ -240,6 +242,8 @@ static int guc_ucode_xfer_dma(struct drm_i915_private
*dev_priv,
DRM_DEBUG_DRIVER("DMA status 0x%x, GuC status 0x%x\n",
I915_READ(DMA_CTRL), status);
+ guc->guc_finish_load = jiffies;
+
if ((status & GS_BOOTROM_MASK) == GS_BOOTROM_RSA_FAILED) {
DRM_ERROR("GuC firmware signature verification failed\n");
ret = -ENOEXEC;
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 22ae52b17b0f..3d5a15ed1995 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -210,6 +210,9 @@ struct intel_guc {
/* GuC's FW specific notify function */
void (*notify)(struct intel_guc *guc);
+
+ unsigned long guc_start_load;
+ unsigned long guc_finish_load;
};
struct intel_huc {
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Add GuC Load time to debugfs
>-Original Message-
>From: Mateo Lozano, Oscar
>Sent: Friday, August 25, 2017 8:18 AM
>To: Srivatsa, Anusha ; intel-
>g...@lists.freedesktop.org
>Cc: Wajdeczko, Michal ; Sundaresan, Sujaritha
>
>Subject: Re: [PATCH] drm/i915/guc: Add GuC Load time to debugfs
>
>Cc: Sujaritha
>
Thanks Oscar for adding Sujaritha.
Do you think the approach here is accurate?
Anusha
>On 08/24/2017 09:38 PM, Anusha Srivatsa wrote:
>> Calculate the time that GuC takes to load.
>> This information could be very useful in determining if GuC is taking
>> unreasonably long time to load in a certain platforms.
>>
>> Cc: Oscar Mateo
>> Cc: Michal Wajdeczko
>> Signed-off-by: Anusha Srivatsa
>> ---
>> drivers/gpu/drm/i915/i915_debugfs.c | 4
>> drivers/gpu/drm/i915/intel_guc_loader.c | 4
>> drivers/gpu/drm/i915/intel_uc.h | 3 +++
>> 3 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 48572b157222..9283fc714705 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -2379,6 +2379,10 @@ static int i915_guc_load_status_info(struct seq_file
>*m, void *data)
>> guc_fw->major_ver_wanted, guc_fw->minor_ver_wanted);
>> seq_printf(m, "\tversion found: %d.%d\n",
>> guc_fw->major_ver_found, guc_fw->minor_ver_found);
>> +seq_printf(m, "\tLoad time is %lu ms\n",
>> + jiffies_to_msecs(dev_priv->guc.guc_finish_load -
>> + dev_priv->guc.guc_start_load));
>> +
>> seq_printf(m, "\theader: offset is %d; size = %d\n",
>> guc_fw->header_offset, guc_fw->header_size);
>> seq_printf(m, "\tuCode: offset is %d; size = %d\n", diff --git
>> a/drivers/gpu/drm/i915/intel_guc_loader.c
>> b/drivers/gpu/drm/i915/intel_guc_loader.c
>> index 8b0ae7fce7f2..1c5059b930f9 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>> @@ -194,6 +194,7 @@ static inline bool guc_ucode_response(struct
>drm_i915_private *dev_priv,
>> static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
>>struct i915_vma *vma)
>> {
>> +struct intel_guc *guc = &dev_priv->guc;
>> struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
>> unsigned long offset;
>> struct sg_table *sg = vma->pages;
>> @@ -226,6 +227,7 @@ static int guc_ucode_xfer_dma(struct
>> drm_i915_private *dev_priv,
>>
>> /* Finally start the DMA */
>> I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE |
>START_DMA));
>> +guc->guc_start_load = jiffies;
>>
>> /*
>> * Wait for the DMA to complete & the GuC to start up.
>> @@ -240,6 +242,8 @@ static int guc_ucode_xfer_dma(struct drm_i915_private
>*dev_priv,
>> DRM_DEBUG_DRIVER("DMA status 0x%x, GuC status 0x%x\n",
>> I915_READ(DMA_CTRL), status);
>>
>> +guc->guc_finish_load = jiffies;
>> +
>> if ((status & GS_BOOTROM_MASK) == GS_BOOTROM_RSA_FAILED) {
>> DRM_ERROR("GuC firmware signature verification failed\n");
>> ret = -ENOEXEC;
>> diff --git a/drivers/gpu/drm/i915/intel_uc.h
>> b/drivers/gpu/drm/i915/intel_uc.h index 22ae52b17b0f..3d5a15ed1995
>> 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.h
>> +++ b/drivers/gpu/drm/i915/intel_uc.h
>> @@ -210,6 +210,9 @@ struct intel_guc {
>>
>> /* GuC's FW specific notify function */
>> void (*notify)(struct intel_guc *guc);
>> +
>> +unsigned long guc_start_load;
>> +unsigned long guc_finish_load;
>> };
>>
>> struct intel_huc {
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Add GuC Load time to debugfs
>-Original Message-
>From: Wajdeczko, Michal
>Sent: Friday, August 25, 2017 5:35 AM
>To: Srivatsa, Anusha
>Cc: intel-gfx@lists.freedesktop.org; Mateo Lozano, Oscar
>
>Subject: Re: [PATCH] drm/i915/guc: Add GuC Load time to debugfs
>
>On Thu, Aug 24, 2017 at 09:38:06PM -0700, Anusha Srivatsa wrote:
>> Calculate the time that GuC takes to load.
>> This information could be very useful in determining if GuC is taking
>> unreasonably long time to load in a certain platforms.
>>
>> Cc: Oscar Mateo
>> Cc: Michal Wajdeczko
>> Signed-off-by: Anusha Srivatsa
>> ---
>> drivers/gpu/drm/i915/i915_debugfs.c | 4
>> drivers/gpu/drm/i915/intel_guc_loader.c | 4
>> drivers/gpu/drm/i915/intel_uc.h | 3 +++
>> 3 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 48572b157222..9283fc714705 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -2379,6 +2379,10 @@ static int i915_guc_load_status_info(struct seq_file
>*m, void *data)
>> guc_fw->major_ver_wanted, guc_fw->minor_ver_wanted);
>> seq_printf(m, "\tversion found: %d.%d\n",
>> guc_fw->major_ver_found, guc_fw->minor_ver_found);
>> +seq_printf(m, "\tLoad time is %lu ms\n",
>> + jiffies_to_msecs(dev_priv->guc.guc_finish_load -
>> + dev_priv->guc.guc_start_load));
>> +
>> seq_printf(m, "\theader: offset is %d; size = %d\n",
>> guc_fw->header_offset, guc_fw->header_size);
>> seq_printf(m, "\tuCode: offset is %d; size = %d\n", diff --git
>> a/drivers/gpu/drm/i915/intel_guc_loader.c
>> b/drivers/gpu/drm/i915/intel_guc_loader.c
>> index 8b0ae7fce7f2..1c5059b930f9 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>> @@ -194,6 +194,7 @@ static inline bool guc_ucode_response(struct
>> drm_i915_private *dev_priv, static int guc_ucode_xfer_dma(struct
>drm_i915_private *dev_priv,
>>struct i915_vma *vma)
>> {
>> +struct intel_guc *guc = &dev_priv->guc;
>> struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
>> unsigned long offset;
>> struct sg_table *sg = vma->pages;
>> @@ -226,6 +227,7 @@ static int guc_ucode_xfer_dma(struct
>> drm_i915_private *dev_priv,
>>
>> /* Finally start the DMA */
>> I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE |
>START_DMA));
>> +guc->guc_start_load = jiffies;
>>
>> /*
>> * Wait for the DMA to complete & the GuC to start up.
>> @@ -240,6 +242,8 @@ static int guc_ucode_xfer_dma(struct drm_i915_private
>*dev_priv,
>> DRM_DEBUG_DRIVER("DMA status 0x%x, GuC status 0x%x\n",
>> I915_READ(DMA_CTRL), status);
>>
>> +guc->guc_finish_load = jiffies;
>> +
>
>On error/timeout we don't know if loading was finished/completed and your
>calculations will be wrong. End time shall be captured before any debug logs to
>more accurate. Btw, if loading time is so important, maybe it should be also
>printed here as part of above DRM_DEBUG ?
Hmmm... I thought by this time in the code the load will be over and hence we
read the stautus registers.
Yes adding as a dmesg too, will be helpful.
Anusha
>
>> if ((status & GS_BOOTROM_MASK) == GS_BOOTROM_RSA_FAILED) {
>> DRM_ERROR("GuC firmware signature verification failed\n");
>> ret = -ENOEXEC;
>> diff --git a/drivers/gpu/drm/i915/intel_uc.h
>> b/drivers/gpu/drm/i915/intel_uc.h index 22ae52b17b0f..3d5a15ed1995
>> 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.h
>> +++ b/drivers/gpu/drm/i915/intel_uc.h
>> @@ -210,6 +210,9 @@ struct intel_guc {
>>
>> /* GuC's FW specific notify function */
>> void (*notify)(struct intel_guc *guc);
>> +
>> +unsigned long guc_start_load;
>> +unsigned long guc_finish_load;
>
>No need to keep both jiffies here. Calculate "load_time_in_ms" in the loader
>function and store only final result. Maybe better place for this result would
>be
>"intel_uc_fw" ? Then we can do the same for Huc.
Adding to intel_uc_fw makes sense. But I wonder if we need a usecase to know
the huc load time nothing wrong to add though.
Thanks for your inputs!
Anusha
>
>-Michal
>
>> };
>>
>> struct intel_huc {
>> --
>> 2.11.0
>>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
