Re: [PATCH v2] drm/i915: Add GuC submission interface version query

2024-02-13 Thread Souza, Jose
On Fri, 2024-02-09 at 09:06 +, Tvrtko Ursulin wrote:
> On 08/02/2024 17:55, Souza, Jose wrote:
> > On Thu, 2024-02-08 at 07:19 -0800, José Roberto de Souza wrote:
> > > On Thu, 2024-02-08 at 14:59 +, Tvrtko Ursulin wrote:
> > > > On 08/02/2024 14:30, Souza, Jose wrote:
> > > > > On Thu, 2024-02-08 at 08:25 +, Tvrtko Ursulin wrote:
> > > > > > From: Tvrtko Ursulin 
> > > > > > 
> > > > > > Add a new query to the GuC submission interface version.
> > > > > > 
> > > > > > Mesa intends to use this information to check for old firmware 
> > > > > > versions
> > > > > > with a known bug where using the render and compute command 
> > > > > > streamers
> > > > > > simultaneously can cause GPU hangs due issues in firmware 
> > > > > > scheduling.
> > > > > > 
> > > > > > Based on patches from Vivaik and Joonas.
> > > > > > 
> > > > > > Compile tested only.
> > > > > > 
> > > > > > v2:
> > > > > >* Added branch version.
> > > > > 
> > > > > Reviewed-by: José Roberto de Souza 
> > > > > Tested-by: José Roberto de Souza 
> > > > > UMD: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25233
> > > > 
> > > > Thanks, but please we also need to close down on the branch number
> > > > situation. I.e. be sure what is the failure mode in shipping Mesa with
> > > > the change as it stands in the MR linked. What platforms could start
> > > > failing and when, depending on GuC FW release eventualities.
> > > 
> > > yes, I have asked John Harrison for a documentation link about the 
> > > firmware versioning.
> > 
> > Got the documentation link, MR updated.
> > Will ask for reviews in Mesa side.
> 
> Is it then understood and accepted that should GuC ever update the 
> branch number on any given platform, that platform, for all deployed 
> Mesa's in the field, will automatically revert to no async queues and so 
> cause a silent performance regression?

yes, understood and accepted.
The Mesa MR is now reviewed, thank you Sagar.

> 
> Regards,
> 
> Tvrtko
> 
> > 
> > > 
> > > > 
> > > > Regards,
> > > > 
> > > > Tvrtko
> > > > 
> > > > > > Signed-off-by: Tvrtko Ursulin 
> > > > > > Cc: Kenneth Graunke 
> > > > > > Cc: Jose Souza 
> > > > > > Cc: Sagar Ghuge 
> > > > > > Cc: Paulo Zanoni 
> > > > > > Cc: John Harrison 
> > > > > > Cc: Rodrigo Vivi 
> > > > > > Cc: Jani Nikula 
> > > > > > Cc: Tvrtko Ursulin 
> > > > > > Cc: Vivaik Balasubrawmanian 
> > > > > > Cc: Joonas Lahtinen 
> > > > > > ---
> > > > > >drivers/gpu/drm/i915/i915_query.c | 33 
> > > > > > +++
> > > > > >include/uapi/drm/i915_drm.h   | 12 +++
> > > > > >2 files changed, 45 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_query.c 
> > > > > > b/drivers/gpu/drm/i915/i915_query.c
> > > > > > index 00871ef99792..d4dba1240b40 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_query.c
> > > > > > +++ b/drivers/gpu/drm/i915/i915_query.c
> > > > > > @@ -551,6 +551,38 @@ static int query_hwconfig_blob(struct 
> > > > > > drm_i915_private *i915,
> > > > > > return hwconfig->size;
> > > > > >}
> > > > > >
> > > > > > +static int
> > > > > > +query_guc_submission_version(struct drm_i915_private *i915,
> > > > > > +struct drm_i915_query_item *query)
> > > > > > +{
> > > > > > +   struct drm_i915_query_guc_submission_version __user *query_ptr =
> > > > > > +   
> > > > > > u64_to_user_ptr(query->data_ptr);
> > > > > > +   struct drm_i915_query_guc_submission_version ver;
> > > > > > +   struct intel_guc *guc = &to_gt(i915)->uc.guc;
> > > > > > +   const size_t size = sizeof(ver);
> > > > > > +   int ret;
> > > > > > +
> > > > > > +   if (!intel_uc_uses_guc_submission(&to_gt(i915)->uc))
> > > > > > +   return -ENODEV;
> > > > > > +
> > > > > > +   ret = copy_query_item(&ver, size, size, query);
> > > > > > +   if (ret != 0)
> > > > > > +   return ret;
> > > > > > +
> > > > > > +   if (ver.branch || ver.major || ver.minor || ver.patch)
> > > > > > +   return -EINVAL;
> > > > > > +
> > > > > > +   ver.branch = 0;
> > > > > > +   ver.major = guc->submission_version.major;
> > > > > > +   ver.minor = guc->submission_version.minor;
> > > > > > +   ver.patch = guc->submission_version.patch;
> > > > > > +
> > > > > > +   if (copy_to_user(query_ptr, &ver, size))
> > > > > > +   return -EFAULT;
> > > > > > +
> > > > > > +   return 0;
> > > > > > +}
> > > > > > +
> > > > > >static int (* const i915_query_funcs[])(struct drm_i915_private 
> > > > > > *dev_priv,
> > > > > > struct drm_i915_query_item 
> > > > > > *query_item) = {
> > > > > > query_topology_info,
> > > > > > @@ -559,6 +591,7 @@ static int (* const i915_query_funcs[])(struct 
> > > > > > drm_i915_private *dev_priv,
> > > > > > query_memregion_info,
> > > > > > query_hwconfig_blob,
> > > > > > query_geometry_subslices,
> > > > > > +   query_guc_submission_version

Re: [PATCH v2] drm/i915: Add GuC submission interface version query

2024-02-09 Thread Tvrtko Ursulin



On 08/02/2024 17:55, Souza, Jose wrote:

On Thu, 2024-02-08 at 07:19 -0800, José Roberto de Souza wrote:

On Thu, 2024-02-08 at 14:59 +, Tvrtko Ursulin wrote:

On 08/02/2024 14:30, Souza, Jose wrote:

On Thu, 2024-02-08 at 08:25 +, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Add a new query to the GuC submission interface version.

Mesa intends to use this information to check for old firmware versions
with a known bug where using the render and compute command streamers
simultaneously can cause GPU hangs due issues in firmware scheduling.

Based on patches from Vivaik and Joonas.

Compile tested only.

v2:
   * Added branch version.


Reviewed-by: José Roberto de Souza 
Tested-by: José Roberto de Souza 
UMD: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25233


Thanks, but please we also need to close down on the branch number
situation. I.e. be sure what is the failure mode in shipping Mesa with
the change as it stands in the MR linked. What platforms could start
failing and when, depending on GuC FW release eventualities.


yes, I have asked John Harrison for a documentation link about the firmware 
versioning.


Got the documentation link, MR updated.
Will ask for reviews in Mesa side.


Is it then understood and accepted that should GuC ever update the 
branch number on any given platform, that platform, for all deployed 
Mesa's in the field, will automatically revert to no async queues and so 
cause a silent performance regression?


Regards,

Tvrtko







Regards,

Tvrtko


Signed-off-by: Tvrtko Ursulin 
Cc: Kenneth Graunke 
Cc: Jose Souza 
Cc: Sagar Ghuge 
Cc: Paulo Zanoni 
Cc: John Harrison 
Cc: Rodrigo Vivi 
Cc: Jani Nikula 
Cc: Tvrtko Ursulin 
Cc: Vivaik Balasubrawmanian 
Cc: Joonas Lahtinen 
---
   drivers/gpu/drm/i915/i915_query.c | 33 +++
   include/uapi/drm/i915_drm.h   | 12 +++
   2 files changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_query.c 
b/drivers/gpu/drm/i915/i915_query.c
index 00871ef99792..d4dba1240b40 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -551,6 +551,38 @@ static int query_hwconfig_blob(struct drm_i915_private 
*i915,
return hwconfig->size;
   }
   
+static int

+query_guc_submission_version(struct drm_i915_private *i915,
+struct drm_i915_query_item *query)
+{
+   struct drm_i915_query_guc_submission_version __user *query_ptr =
+   u64_to_user_ptr(query->data_ptr);
+   struct drm_i915_query_guc_submission_version ver;
+   struct intel_guc *guc = &to_gt(i915)->uc.guc;
+   const size_t size = sizeof(ver);
+   int ret;
+
+   if (!intel_uc_uses_guc_submission(&to_gt(i915)->uc))
+   return -ENODEV;
+
+   ret = copy_query_item(&ver, size, size, query);
+   if (ret != 0)
+   return ret;
+
+   if (ver.branch || ver.major || ver.minor || ver.patch)
+   return -EINVAL;
+
+   ver.branch = 0;
+   ver.major = guc->submission_version.major;
+   ver.minor = guc->submission_version.minor;
+   ver.patch = guc->submission_version.patch;
+
+   if (copy_to_user(query_ptr, &ver, size))
+   return -EFAULT;
+
+   return 0;
+}
+
   static int (* const i915_query_funcs[])(struct drm_i915_private *dev_priv,
struct drm_i915_query_item *query_item) 
= {
query_topology_info,
@@ -559,6 +591,7 @@ static int (* const i915_query_funcs[])(struct 
drm_i915_private *dev_priv,
query_memregion_info,
query_hwconfig_blob,
query_geometry_subslices,
+   query_guc_submission_version,
   };
   
   int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 550c496ce76d..84fb7f7ea834 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -3038,6 +3038,7 @@ struct drm_i915_query_item {
 *  - %DRM_I915_QUERY_MEMORY_REGIONS (see struct 
drm_i915_query_memory_regions)
 *  - %DRM_I915_QUERY_HWCONFIG_BLOB (see `GuC HWCONFIG blob uAPI`)
 *  - %DRM_I915_QUERY_GEOMETRY_SUBSLICES (see struct 
drm_i915_query_topology_info)
+*  - %DRM_I915_QUERY_GUC_SUBMISSION_VERSION (see struct 
drm_i915_query_guc_submission_version)
 */
__u64 query_id;
   #define DRM_I915_QUERY_TOPOLOGY_INFO 1
@@ -3046,6 +3047,7 @@ struct drm_i915_query_item {
   #define DRM_I915_QUERY_MEMORY_REGIONS4
   #define DRM_I915_QUERY_HWCONFIG_BLOB 5
   #define DRM_I915_QUERY_GEOMETRY_SUBSLICES6
+#define DRM_I915_QUERY_GUC_SUBMISSION_VERSION  7
   /* Must be kept compact -- no holes and well documented */
   
   	/**

@@ -3591,6 +3593,16 @@ struct drm_i915_query_memory_regions {
struct drm_i915_memory_region_info regions[];
   };
   
+/**

+* struct drm_i915_query_guc_submissi

Re: [PATCH v2] drm/i915: Add GuC submission interface version query

2024-02-08 Thread Souza, Jose
On Thu, 2024-02-08 at 07:19 -0800, José Roberto de Souza wrote:
> On Thu, 2024-02-08 at 14:59 +, Tvrtko Ursulin wrote:
> > On 08/02/2024 14:30, Souza, Jose wrote:
> > > On Thu, 2024-02-08 at 08:25 +, Tvrtko Ursulin wrote:
> > > > From: Tvrtko Ursulin 
> > > > 
> > > > Add a new query to the GuC submission interface version.
> > > > 
> > > > Mesa intends to use this information to check for old firmware versions
> > > > with a known bug where using the render and compute command streamers
> > > > simultaneously can cause GPU hangs due issues in firmware scheduling.
> > > > 
> > > > Based on patches from Vivaik and Joonas.
> > > > 
> > > > Compile tested only.
> > > > 
> > > > v2:
> > > >   * Added branch version.
> > > 
> > > Reviewed-by: José Roberto de Souza 
> > > Tested-by: José Roberto de Souza 
> > > UMD: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25233
> > 
> > Thanks, but please we also need to close down on the branch number 
> > situation. I.e. be sure what is the failure mode in shipping Mesa with 
> > the change as it stands in the MR linked. What platforms could start 
> > failing and when, depending on GuC FW release eventualities.
> 
> yes, I have asked John Harrison for a documentation link about the firmware 
> versioning.

Got the documentation link, MR updated.
Will ask for reviews in Mesa side.

> 
> > 
> > Regards,
> > 
> > Tvrtko
> > 
> > > > Signed-off-by: Tvrtko Ursulin 
> > > > Cc: Kenneth Graunke 
> > > > Cc: Jose Souza 
> > > > Cc: Sagar Ghuge 
> > > > Cc: Paulo Zanoni 
> > > > Cc: John Harrison 
> > > > Cc: Rodrigo Vivi 
> > > > Cc: Jani Nikula 
> > > > Cc: Tvrtko Ursulin 
> > > > Cc: Vivaik Balasubrawmanian 
> > > > Cc: Joonas Lahtinen 
> > > > ---
> > > >   drivers/gpu/drm/i915/i915_query.c | 33 +++
> > > >   include/uapi/drm/i915_drm.h   | 12 +++
> > > >   2 files changed, 45 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_query.c 
> > > > b/drivers/gpu/drm/i915/i915_query.c
> > > > index 00871ef99792..d4dba1240b40 100644
> > > > --- a/drivers/gpu/drm/i915/i915_query.c
> > > > +++ b/drivers/gpu/drm/i915/i915_query.c
> > > > @@ -551,6 +551,38 @@ static int query_hwconfig_blob(struct 
> > > > drm_i915_private *i915,
> > > > return hwconfig->size;
> > > >   }
> > > >   
> > > > +static int
> > > > +query_guc_submission_version(struct drm_i915_private *i915,
> > > > +struct drm_i915_query_item *query)
> > > > +{
> > > > +   struct drm_i915_query_guc_submission_version __user *query_ptr =
> > > > +   
> > > > u64_to_user_ptr(query->data_ptr);
> > > > +   struct drm_i915_query_guc_submission_version ver;
> > > > +   struct intel_guc *guc = &to_gt(i915)->uc.guc;
> > > > +   const size_t size = sizeof(ver);
> > > > +   int ret;
> > > > +
> > > > +   if (!intel_uc_uses_guc_submission(&to_gt(i915)->uc))
> > > > +   return -ENODEV;
> > > > +
> > > > +   ret = copy_query_item(&ver, size, size, query);
> > > > +   if (ret != 0)
> > > > +   return ret;
> > > > +
> > > > +   if (ver.branch || ver.major || ver.minor || ver.patch)
> > > > +   return -EINVAL;
> > > > +
> > > > +   ver.branch = 0;
> > > > +   ver.major = guc->submission_version.major;
> > > > +   ver.minor = guc->submission_version.minor;
> > > > +   ver.patch = guc->submission_version.patch;
> > > > +
> > > > +   if (copy_to_user(query_ptr, &ver, size))
> > > > +   return -EFAULT;
> > > > +
> > > > +   return 0;
> > > > +}
> > > > +
> > > >   static int (* const i915_query_funcs[])(struct drm_i915_private 
> > > > *dev_priv,
> > > > struct drm_i915_query_item 
> > > > *query_item) = {
> > > > query_topology_info,
> > > > @@ -559,6 +591,7 @@ static int (* const i915_query_funcs[])(struct 
> > > > drm_i915_private *dev_priv,
> > > > query_memregion_info,
> > > > query_hwconfig_blob,
> > > > query_geometry_subslices,
> > > > +   query_guc_submission_version,
> > > >   };
> > > >   
> > > >   int i915_query_ioctl(struct drm_device *dev, void *data, struct 
> > > > drm_file *file)
> > > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > > > index 550c496ce76d..84fb7f7ea834 100644
> > > > --- a/include/uapi/drm/i915_drm.h
> > > > +++ b/include/uapi/drm/i915_drm.h
> > > > @@ -3038,6 +3038,7 @@ struct drm_i915_query_item {
> > > >  *  - %DRM_I915_QUERY_MEMORY_REGIONS (see struct 
> > > > drm_i915_query_memory_regions)
> > > >  *  - %DRM_I915_QUERY_HWCONFIG_BLOB (see `GuC HWCONFIG blob 
> > > > uAPI`)
> > > >  *  - %DRM_I915_QUERY_GEOMETRY_SUBSLICES (see struct 
> > > > drm_i915_query_topology_info)
> > > > +*  - %DRM_I915_QUERY_GUC_SUBMISSION_VERSION (see struct 
> > > > drm_i915_query_guc_submission_version)
> > > >  */
> > > >

Re: [PATCH v2] drm/i915: Add GuC submission interface version query

2024-02-08 Thread Souza, Jose
On Thu, 2024-02-08 at 14:59 +, Tvrtko Ursulin wrote:
> On 08/02/2024 14:30, Souza, Jose wrote:
> > On Thu, 2024-02-08 at 08:25 +, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin 
> > > 
> > > Add a new query to the GuC submission interface version.
> > > 
> > > Mesa intends to use this information to check for old firmware versions
> > > with a known bug where using the render and compute command streamers
> > > simultaneously can cause GPU hangs due issues in firmware scheduling.
> > > 
> > > Based on patches from Vivaik and Joonas.
> > > 
> > > Compile tested only.
> > > 
> > > v2:
> > >   * Added branch version.
> > 
> > Reviewed-by: José Roberto de Souza 
> > Tested-by: José Roberto de Souza 
> > UMD: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25233
> 
> Thanks, but please we also need to close down on the branch number 
> situation. I.e. be sure what is the failure mode in shipping Mesa with 
> the change as it stands in the MR linked. What platforms could start 
> failing and when, depending on GuC FW release eventualities.

yes, I have asked John Harrison for a documentation link about the firmware 
versioning.

> 
> Regards,
> 
> Tvrtko
> 
> > > Signed-off-by: Tvrtko Ursulin 
> > > Cc: Kenneth Graunke 
> > > Cc: Jose Souza 
> > > Cc: Sagar Ghuge 
> > > Cc: Paulo Zanoni 
> > > Cc: John Harrison 
> > > Cc: Rodrigo Vivi 
> > > Cc: Jani Nikula 
> > > Cc: Tvrtko Ursulin 
> > > Cc: Vivaik Balasubrawmanian 
> > > Cc: Joonas Lahtinen 
> > > ---
> > >   drivers/gpu/drm/i915/i915_query.c | 33 +++
> > >   include/uapi/drm/i915_drm.h   | 12 +++
> > >   2 files changed, 45 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_query.c 
> > > b/drivers/gpu/drm/i915/i915_query.c
> > > index 00871ef99792..d4dba1240b40 100644
> > > --- a/drivers/gpu/drm/i915/i915_query.c
> > > +++ b/drivers/gpu/drm/i915/i915_query.c
> > > @@ -551,6 +551,38 @@ static int query_hwconfig_blob(struct 
> > > drm_i915_private *i915,
> > >   return hwconfig->size;
> > >   }
> > >   
> > > +static int
> > > +query_guc_submission_version(struct drm_i915_private *i915,
> > > +  struct drm_i915_query_item *query)
> > > +{
> > > + struct drm_i915_query_guc_submission_version __user *query_ptr =
> > > + u64_to_user_ptr(query->data_ptr);
> > > + struct drm_i915_query_guc_submission_version ver;
> > > + struct intel_guc *guc = &to_gt(i915)->uc.guc;
> > > + const size_t size = sizeof(ver);
> > > + int ret;
> > > +
> > > + if (!intel_uc_uses_guc_submission(&to_gt(i915)->uc))
> > > + return -ENODEV;
> > > +
> > > + ret = copy_query_item(&ver, size, size, query);
> > > + if (ret != 0)
> > > + return ret;
> > > +
> > > + if (ver.branch || ver.major || ver.minor || ver.patch)
> > > + return -EINVAL;
> > > +
> > > + ver.branch = 0;
> > > + ver.major = guc->submission_version.major;
> > > + ver.minor = guc->submission_version.minor;
> > > + ver.patch = guc->submission_version.patch;
> > > +
> > > + if (copy_to_user(query_ptr, &ver, size))
> > > + return -EFAULT;
> > > +
> > > + return 0;
> > > +}
> > > +
> > >   static int (* const i915_query_funcs[])(struct drm_i915_private 
> > > *dev_priv,
> > >   struct drm_i915_query_item 
> > > *query_item) = {
> > >   query_topology_info,
> > > @@ -559,6 +591,7 @@ static int (* const i915_query_funcs[])(struct 
> > > drm_i915_private *dev_priv,
> > >   query_memregion_info,
> > >   query_hwconfig_blob,
> > >   query_geometry_subslices,
> > > + query_guc_submission_version,
> > >   };
> > >   
> > >   int i915_query_ioctl(struct drm_device *dev, void *data, struct 
> > > drm_file *file)
> > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > > index 550c496ce76d..84fb7f7ea834 100644
> > > --- a/include/uapi/drm/i915_drm.h
> > > +++ b/include/uapi/drm/i915_drm.h
> > > @@ -3038,6 +3038,7 @@ struct drm_i915_query_item {
> > >*  - %DRM_I915_QUERY_MEMORY_REGIONS (see struct 
> > > drm_i915_query_memory_regions)
> > >*  - %DRM_I915_QUERY_HWCONFIG_BLOB (see `GuC HWCONFIG blob 
> > > uAPI`)
> > >*  - %DRM_I915_QUERY_GEOMETRY_SUBSLICES (see struct 
> > > drm_i915_query_topology_info)
> > > +  *  - %DRM_I915_QUERY_GUC_SUBMISSION_VERSION (see struct 
> > > drm_i915_query_guc_submission_version)
> > >*/
> > >   __u64 query_id;
> > >   #define DRM_I915_QUERY_TOPOLOGY_INFO1
> > > @@ -3046,6 +3047,7 @@ struct drm_i915_query_item {
> > >   #define DRM_I915_QUERY_MEMORY_REGIONS   4
> > >   #define DRM_I915_QUERY_HWCONFIG_BLOB5
> > >   #define DRM_I915_QUERY_GEOMETRY_SUBSLICES   6
> > > +#define DRM_I915_QUERY_GUC_SUBMISSION_VERSION7
> > >   /* Must be kept compact -- no holes and well documented */
> > >   
> > >   /**
> > > @@ -3591,6 +3593,16 @@ struct drm_i915_que

Re: [PATCH v2] drm/i915: Add GuC submission interface version query

2024-02-08 Thread Tvrtko Ursulin



On 08/02/2024 14:30, Souza, Jose wrote:

On Thu, 2024-02-08 at 08:25 +, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Add a new query to the GuC submission interface version.

Mesa intends to use this information to check for old firmware versions
with a known bug where using the render and compute command streamers
simultaneously can cause GPU hangs due issues in firmware scheduling.

Based on patches from Vivaik and Joonas.

Compile tested only.

v2:
  * Added branch version.


Reviewed-by: José Roberto de Souza 
Tested-by: José Roberto de Souza 
UMD: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25233


Thanks, but please we also need to close down on the branch number 
situation. I.e. be sure what is the failure mode in shipping Mesa with 
the change as it stands in the MR linked. What platforms could start 
failing and when, depending on GuC FW release eventualities.


Regards,

Tvrtko


Signed-off-by: Tvrtko Ursulin 
Cc: Kenneth Graunke 
Cc: Jose Souza 
Cc: Sagar Ghuge 
Cc: Paulo Zanoni 
Cc: John Harrison 
Cc: Rodrigo Vivi 
Cc: Jani Nikula 
Cc: Tvrtko Ursulin 
Cc: Vivaik Balasubrawmanian 
Cc: Joonas Lahtinen 
---
  drivers/gpu/drm/i915/i915_query.c | 33 +++
  include/uapi/drm/i915_drm.h   | 12 +++
  2 files changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_query.c 
b/drivers/gpu/drm/i915/i915_query.c
index 00871ef99792..d4dba1240b40 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -551,6 +551,38 @@ static int query_hwconfig_blob(struct drm_i915_private 
*i915,
return hwconfig->size;
  }
  
+static int

+query_guc_submission_version(struct drm_i915_private *i915,
+struct drm_i915_query_item *query)
+{
+   struct drm_i915_query_guc_submission_version __user *query_ptr =
+   u64_to_user_ptr(query->data_ptr);
+   struct drm_i915_query_guc_submission_version ver;
+   struct intel_guc *guc = &to_gt(i915)->uc.guc;
+   const size_t size = sizeof(ver);
+   int ret;
+
+   if (!intel_uc_uses_guc_submission(&to_gt(i915)->uc))
+   return -ENODEV;
+
+   ret = copy_query_item(&ver, size, size, query);
+   if (ret != 0)
+   return ret;
+
+   if (ver.branch || ver.major || ver.minor || ver.patch)
+   return -EINVAL;
+
+   ver.branch = 0;
+   ver.major = guc->submission_version.major;
+   ver.minor = guc->submission_version.minor;
+   ver.patch = guc->submission_version.patch;
+
+   if (copy_to_user(query_ptr, &ver, size))
+   return -EFAULT;
+
+   return 0;
+}
+
  static int (* const i915_query_funcs[])(struct drm_i915_private *dev_priv,
struct drm_i915_query_item *query_item) 
= {
query_topology_info,
@@ -559,6 +591,7 @@ static int (* const i915_query_funcs[])(struct 
drm_i915_private *dev_priv,
query_memregion_info,
query_hwconfig_blob,
query_geometry_subslices,
+   query_guc_submission_version,
  };
  
  int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 550c496ce76d..84fb7f7ea834 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -3038,6 +3038,7 @@ struct drm_i915_query_item {
 *  - %DRM_I915_QUERY_MEMORY_REGIONS (see struct 
drm_i915_query_memory_regions)
 *  - %DRM_I915_QUERY_HWCONFIG_BLOB (see `GuC HWCONFIG blob uAPI`)
 *  - %DRM_I915_QUERY_GEOMETRY_SUBSLICES (see struct 
drm_i915_query_topology_info)
+*  - %DRM_I915_QUERY_GUC_SUBMISSION_VERSION (see struct 
drm_i915_query_guc_submission_version)
 */
__u64 query_id;
  #define DRM_I915_QUERY_TOPOLOGY_INFO  1
@@ -3046,6 +3047,7 @@ struct drm_i915_query_item {
  #define DRM_I915_QUERY_MEMORY_REGIONS 4
  #define DRM_I915_QUERY_HWCONFIG_BLOB  5
  #define DRM_I915_QUERY_GEOMETRY_SUBSLICES 6
+#define DRM_I915_QUERY_GUC_SUBMISSION_VERSION  7
  /* Must be kept compact -- no holes and well documented */
  
  	/**

@@ -3591,6 +3593,16 @@ struct drm_i915_query_memory_regions {
struct drm_i915_memory_region_info regions[];
  };
  
+/**

+* struct drm_i915_query_guc_submission_version - query GuC submission 
interface version
+*/
+struct drm_i915_query_guc_submission_version {
+   __u32 branch;
+   __u32 major;
+   __u32 minor;
+   __u32 patch;
+};
+
  /**
   * DOC: GuC HWCONFIG blob uAPI
   *




Re: [PATCH v2] drm/i915: Add GuC submission interface version query

2024-02-08 Thread Souza, Jose
On Thu, 2024-02-08 at 08:25 +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin 
> 
> Add a new query to the GuC submission interface version.
> 
> Mesa intends to use this information to check for old firmware versions
> with a known bug where using the render and compute command streamers
> simultaneously can cause GPU hangs due issues in firmware scheduling.
> 
> Based on patches from Vivaik and Joonas.
> 
> Compile tested only.
> 
> v2:
>  * Added branch version.

Reviewed-by: José Roberto de Souza 
Tested-by: José Roberto de Souza 
UMD: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25233

> 
> Signed-off-by: Tvrtko Ursulin 
> Cc: Kenneth Graunke 
> Cc: Jose Souza 
> Cc: Sagar Ghuge 
> Cc: Paulo Zanoni 
> Cc: John Harrison 
> Cc: Rodrigo Vivi 
> Cc: Jani Nikula 
> Cc: Tvrtko Ursulin 
> Cc: Vivaik Balasubrawmanian 
> Cc: Joonas Lahtinen 
> ---
>  drivers/gpu/drm/i915/i915_query.c | 33 +++
>  include/uapi/drm/i915_drm.h   | 12 +++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_query.c 
> b/drivers/gpu/drm/i915/i915_query.c
> index 00871ef99792..d4dba1240b40 100644
> --- a/drivers/gpu/drm/i915/i915_query.c
> +++ b/drivers/gpu/drm/i915/i915_query.c
> @@ -551,6 +551,38 @@ static int query_hwconfig_blob(struct drm_i915_private 
> *i915,
>   return hwconfig->size;
>  }
>  
> +static int
> +query_guc_submission_version(struct drm_i915_private *i915,
> +  struct drm_i915_query_item *query)
> +{
> + struct drm_i915_query_guc_submission_version __user *query_ptr =
> + u64_to_user_ptr(query->data_ptr);
> + struct drm_i915_query_guc_submission_version ver;
> + struct intel_guc *guc = &to_gt(i915)->uc.guc;
> + const size_t size = sizeof(ver);
> + int ret;
> +
> + if (!intel_uc_uses_guc_submission(&to_gt(i915)->uc))
> + return -ENODEV;
> +
> + ret = copy_query_item(&ver, size, size, query);
> + if (ret != 0)
> + return ret;
> +
> + if (ver.branch || ver.major || ver.minor || ver.patch)
> + return -EINVAL;
> +
> + ver.branch = 0;
> + ver.major = guc->submission_version.major;
> + ver.minor = guc->submission_version.minor;
> + ver.patch = guc->submission_version.patch;
> +
> + if (copy_to_user(query_ptr, &ver, size))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
>  static int (* const i915_query_funcs[])(struct drm_i915_private *dev_priv,
>   struct drm_i915_query_item *query_item) 
> = {
>   query_topology_info,
> @@ -559,6 +591,7 @@ static int (* const i915_query_funcs[])(struct 
> drm_i915_private *dev_priv,
>   query_memregion_info,
>   query_hwconfig_blob,
>   query_geometry_subslices,
> + query_guc_submission_version,
>  };
>  
>  int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file 
> *file)
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 550c496ce76d..84fb7f7ea834 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -3038,6 +3038,7 @@ struct drm_i915_query_item {
>*  - %DRM_I915_QUERY_MEMORY_REGIONS (see struct 
> drm_i915_query_memory_regions)
>*  - %DRM_I915_QUERY_HWCONFIG_BLOB (see `GuC HWCONFIG blob uAPI`)
>*  - %DRM_I915_QUERY_GEOMETRY_SUBSLICES (see struct 
> drm_i915_query_topology_info)
> +  *  - %DRM_I915_QUERY_GUC_SUBMISSION_VERSION (see struct 
> drm_i915_query_guc_submission_version)
>*/
>   __u64 query_id;
>  #define DRM_I915_QUERY_TOPOLOGY_INFO 1
> @@ -3046,6 +3047,7 @@ struct drm_i915_query_item {
>  #define DRM_I915_QUERY_MEMORY_REGIONS4
>  #define DRM_I915_QUERY_HWCONFIG_BLOB 5
>  #define DRM_I915_QUERY_GEOMETRY_SUBSLICES6
> +#define DRM_I915_QUERY_GUC_SUBMISSION_VERSION7
>  /* Must be kept compact -- no holes and well documented */
>  
>   /**
> @@ -3591,6 +3593,16 @@ struct drm_i915_query_memory_regions {
>   struct drm_i915_memory_region_info regions[];
>  };
>  
> +/**
> +* struct drm_i915_query_guc_submission_version - query GuC submission 
> interface version
> +*/
> +struct drm_i915_query_guc_submission_version {
> + __u32 branch;
> + __u32 major;
> + __u32 minor;
> + __u32 patch;
> +};
> +
>  /**
>   * DOC: GuC HWCONFIG blob uAPI
>   *



[PATCH v2] drm/i915: Add GuC submission interface version query

2024-02-08 Thread Tvrtko Ursulin
From: Tvrtko Ursulin 

Add a new query to the GuC submission interface version.

Mesa intends to use this information to check for old firmware versions
with a known bug where using the render and compute command streamers
simultaneously can cause GPU hangs due issues in firmware scheduling.

Based on patches from Vivaik and Joonas.

Compile tested only.

v2:
 * Added branch version.

Signed-off-by: Tvrtko Ursulin 
Cc: Kenneth Graunke 
Cc: Jose Souza 
Cc: Sagar Ghuge 
Cc: Paulo Zanoni 
Cc: John Harrison 
Cc: Rodrigo Vivi 
Cc: Jani Nikula 
Cc: Tvrtko Ursulin 
Cc: Vivaik Balasubrawmanian 
Cc: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/i915_query.c | 33 +++
 include/uapi/drm/i915_drm.h   | 12 +++
 2 files changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_query.c 
b/drivers/gpu/drm/i915/i915_query.c
index 00871ef99792..d4dba1240b40 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -551,6 +551,38 @@ static int query_hwconfig_blob(struct drm_i915_private 
*i915,
return hwconfig->size;
 }
 
+static int
+query_guc_submission_version(struct drm_i915_private *i915,
+struct drm_i915_query_item *query)
+{
+   struct drm_i915_query_guc_submission_version __user *query_ptr =
+   u64_to_user_ptr(query->data_ptr);
+   struct drm_i915_query_guc_submission_version ver;
+   struct intel_guc *guc = &to_gt(i915)->uc.guc;
+   const size_t size = sizeof(ver);
+   int ret;
+
+   if (!intel_uc_uses_guc_submission(&to_gt(i915)->uc))
+   return -ENODEV;
+
+   ret = copy_query_item(&ver, size, size, query);
+   if (ret != 0)
+   return ret;
+
+   if (ver.branch || ver.major || ver.minor || ver.patch)
+   return -EINVAL;
+
+   ver.branch = 0;
+   ver.major = guc->submission_version.major;
+   ver.minor = guc->submission_version.minor;
+   ver.patch = guc->submission_version.patch;
+
+   if (copy_to_user(query_ptr, &ver, size))
+   return -EFAULT;
+
+   return 0;
+}
+
 static int (* const i915_query_funcs[])(struct drm_i915_private *dev_priv,
struct drm_i915_query_item *query_item) 
= {
query_topology_info,
@@ -559,6 +591,7 @@ static int (* const i915_query_funcs[])(struct 
drm_i915_private *dev_priv,
query_memregion_info,
query_hwconfig_blob,
query_geometry_subslices,
+   query_guc_submission_version,
 };
 
 int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 550c496ce76d..84fb7f7ea834 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -3038,6 +3038,7 @@ struct drm_i915_query_item {
 *  - %DRM_I915_QUERY_MEMORY_REGIONS (see struct 
drm_i915_query_memory_regions)
 *  - %DRM_I915_QUERY_HWCONFIG_BLOB (see `GuC HWCONFIG blob uAPI`)
 *  - %DRM_I915_QUERY_GEOMETRY_SUBSLICES (see struct 
drm_i915_query_topology_info)
+*  - %DRM_I915_QUERY_GUC_SUBMISSION_VERSION (see struct 
drm_i915_query_guc_submission_version)
 */
__u64 query_id;
 #define DRM_I915_QUERY_TOPOLOGY_INFO   1
@@ -3046,6 +3047,7 @@ struct drm_i915_query_item {
 #define DRM_I915_QUERY_MEMORY_REGIONS  4
 #define DRM_I915_QUERY_HWCONFIG_BLOB   5
 #define DRM_I915_QUERY_GEOMETRY_SUBSLICES  6
+#define DRM_I915_QUERY_GUC_SUBMISSION_VERSION  7
 /* Must be kept compact -- no holes and well documented */
 
/**
@@ -3591,6 +3593,16 @@ struct drm_i915_query_memory_regions {
struct drm_i915_memory_region_info regions[];
 };
 
+/**
+* struct drm_i915_query_guc_submission_version - query GuC submission 
interface version
+*/
+struct drm_i915_query_guc_submission_version {
+   __u32 branch;
+   __u32 major;
+   __u32 minor;
+   __u32 patch;
+};
+
 /**
  * DOC: GuC HWCONFIG blob uAPI
  *
-- 
2.40.1