diff mbox series

[2/2] drm/i915: Add logical mapping for video decode engines

Message ID 20220316234538.434357-2-lucas.demarchi@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/i915: Fix renamed struct field | expand

Commit Message

Lucas De Marchi March 16, 2022, 11:45 p.m. UTC 
From: Matthew Brost <matthew.brost@intel.com>

Add logical mapping for VDBOXs. This mapping is required for
split-frame workloads, which otherwise fail with

	00000000-F8C53528: [GUC] 0441-INVALID_ENGINE_SUBMIT_MASK

... if the application is using the logical id to reorder the engines and
then using it for the batch buffer submission. It's not a big problem on
media version 11 and 12 as they have only 2 instances of VCS and the
logical to physical mapping is monotonically increasing - if the
application is not using the logical id.

Changing it for the previous platforms allows the media driver
implementation for the next ones (12.50 and above) to be the same,
checking the logical id. It should also not introduce any bug for the
old versions of userspace not checking the id.

The mapping added here is the complete map needed by XEHPSDV. Previous
platforms with only 2 instances will just use a partial map and should
still work.

Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
[ Extend the mapping to media versions 11 and 12 and give proper
  justification in the commit message why ]
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

Comments

Matthew Brost March 17, 2022, 12:20 a.m. UTC | #1 
On Wed, Mar 16, 2022 at 04:45:38PM -0700, Lucas De Marchi wrote:
> From: Matthew Brost <matthew.brost@intel.com>
> 
> Add logical mapping for VDBOXs. This mapping is required for
> split-frame workloads, which otherwise fail with
> 
> 	00000000-F8C53528: [GUC] 0441-INVALID_ENGINE_SUBMIT_MASK
> 
> ... if the application is using the logical id to reorder the engines and
> then using it for the batch buffer submission. It's not a big problem on
> media version 11 and 12 as they have only 2 instances of VCS and the
> logical to physical mapping is monotonically increasing - if the
> application is not using the logical id.
> 
> Changing it for the previous platforms allows the media driver
> implementation for the next ones (12.50 and above) to be the same,
> checking the logical id. It should also not introduce any bug for the
> old versions of userspace not checking the id.
> 
> The mapping added here is the complete map needed by XEHPSDV. Previous
> platforms with only 2 instances will just use a partial map and should
> still work.
> 
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> [ Extend the mapping to media versions 11 and 12 and give proper
>   justification in the commit message why ]
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

For the changes to the patch / commit message:

Acked-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 8080479f27aa..afa2e61cf729 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -731,12 +731,24 @@ static void populate_logical_ids(struct intel_gt *gt, u8 *logical_ids,
>  
>  static void setup_logical_ids(struct intel_gt *gt, u8 *logical_ids, u8 class)
>  {
> -	int i;
> -	u8 map[MAX_ENGINE_INSTANCE + 1];
> +	/*
> +	 * Logical to physical mapping is needed for proper support
> +	 * to split-frame feature.
> +	 */
> +	if (MEDIA_VER(gt->i915) >= 11 && class == VIDEO_DECODE_CLASS) {
> +		static const u8 map[] = { 0, 2, 4, 6, 1, 3, 5, 7 };
>  
> -	for (i = 0; i < MAX_ENGINE_INSTANCE + 1; ++i)
> -		map[i] = i;
> -	populate_logical_ids(gt, logical_ids, class, map, ARRAY_SIZE(map));
> +		populate_logical_ids(gt, logical_ids, class,
> +				     map, ARRAY_SIZE(map));
> +	} else {
> +		int i;
> +		u8 map[MAX_ENGINE_INSTANCE + 1];
> +
> +		for (i = 0; i < MAX_ENGINE_INSTANCE + 1; ++i)
> +			map[i] = i;
> +		populate_logical_ids(gt, logical_ids, class,
> +				     map, ARRAY_SIZE(map));
> +	}
>  }
>  
>  /**
> -- 
> 2.35.1
>
Souza, Jose March 17, 2022, 1:42 p.m. UTC | #2 
On Wed, 2022-03-16 at 16:45 -0700, Lucas De Marchi wrote:
> From: Matthew Brost <matthew.brost@intel.com>
> 
> Add logical mapping for VDBOXs. This mapping is required for
> split-frame workloads, which otherwise fail with
> 
> 	00000000-F8C53528: [GUC] 0441-INVALID_ENGINE_SUBMIT_MASK
> 
> ... if the application is using the logical id to reorder the engines and
> then using it for the batch buffer submission. It's not a big problem on
> media version 11 and 12 as they have only 2 instances of VCS and the
> logical to physical mapping is monotonically increasing - if the
> application is not using the logical id.
> 
> Changing it for the previous platforms allows the media driver
> implementation for the next ones (12.50 and above) to be the same,
> checking the logical id. It should also not introduce any bug for the
> old versions of userspace not checking the id.
> 
> The mapping added here is the complete map needed by XEHPSDV. Previous
> platforms with only 2 instances will just use a partial map and should
> still work.
> 
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> [ Extend the mapping to media versions 11 and 12 and give proper
>   justification in the commit message why ]
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 8080479f27aa..afa2e61cf729 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -731,12 +731,24 @@ static void populate_logical_ids(struct intel_gt *gt, u8 *logical_ids,
>  
>  static void setup_logical_ids(struct intel_gt *gt, u8 *logical_ids, u8 class)
>  {
> -	int i;
> -	u8 map[MAX_ENGINE_INSTANCE + 1];
> +	/*
> +	 * Logical to physical mapping is needed for proper support
> +	 * to split-frame feature.
> +	 */
> +	if (MEDIA_VER(gt->i915) >= 11 && class == VIDEO_DECODE_CLASS) {
> +		static const u8 map[] = { 0, 2, 4, 6, 1, 3, 5, 7 };

You can drop the static.

Other than that LGTM.
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

>  
> -	for (i = 0; i < MAX_ENGINE_INSTANCE + 1; ++i)
> -		map[i] = i;
> -	populate_logical_ids(gt, logical_ids, class, map, ARRAY_SIZE(map));
> +		populate_logical_ids(gt, logical_ids, class,
> +				     map, ARRAY_SIZE(map));
> +	} else {
> +		int i;
> +		u8 map[MAX_ENGINE_INSTANCE + 1];
> +
> +		for (i = 0; i < MAX_ENGINE_INSTANCE + 1; ++i)
> +			map[i] = i;
> +		populate_logical_ids(gt, logical_ids, class,
> +				     map, ARRAY_SIZE(map));
> +	}
>  }
>  
>  /**
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 8080479f27aa..afa2e61cf729 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -731,12 +731,24 @@  static void populate_logical_ids(struct intel_gt *gt, u8 *logical_ids,
 
 static void setup_logical_ids(struct intel_gt *gt, u8 *logical_ids, u8 class)
 {
-	int i;
-	u8 map[MAX_ENGINE_INSTANCE + 1];
+	/*
+	 * Logical to physical mapping is needed for proper support
+	 * to split-frame feature.
+	 */
+	if (MEDIA_VER(gt->i915) >= 11 && class == VIDEO_DECODE_CLASS) {
+		static const u8 map[] = { 0, 2, 4, 6, 1, 3, 5, 7 };
 
-	for (i = 0; i < MAX_ENGINE_INSTANCE + 1; ++i)
-		map[i] = i;
-	populate_logical_ids(gt, logical_ids, class, map, ARRAY_SIZE(map));
+		populate_logical_ids(gt, logical_ids, class,
+				     map, ARRAY_SIZE(map));
+	} else {
+		int i;
+		u8 map[MAX_ENGINE_INSTANCE + 1];
+
+		for (i = 0; i < MAX_ENGINE_INSTANCE + 1; ++i)
+			map[i] = i;
+		populate_logical_ids(gt, logical_ids, class,
+				     map, ARRAY_SIZE(map));
+	}
 }
 
 /**