[Intel-gfx] [PATCH] drm/i915: Use uabi engines for the default engine map

[Intel-gfx] [PATCH] drm/i915: Use uabi engines for the default engine map

[Jonathan Cavitt]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Default engine map is exactly about uabi engines so no excuse not to use
the appropriate iterator to populate it.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 454e73a433c8..42a39e103d7c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1096,16 +1096,15 @@ static struct i915_gem_engines *alloc_engines(unsigned int count)
 static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
 						struct intel_sseu rcs_sseu)
 {
-	const struct intel_gt *gt = to_gt(ctx->i915);
+	const unsigned int max = I915_NUM_ENGINES;
 	struct intel_engine_cs *engine;
 	struct i915_gem_engines *e, *err;
-	enum intel_engine_id id;
 
-	e = alloc_engines(I915_NUM_ENGINES);
+	e = alloc_engines(max);
 	if (!e)
 		return ERR_PTR(-ENOMEM);
 
-	for_each_engine(engine, gt, id) {
+	for_each_uabi_engine(engine, ctx->i915) {
 		struct intel_context *ce;
 		struct intel_sseu sseu = {};
 		int ret;
@@ -1113,7 +1112,7 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
 		if (engine->legacy_idx == INVALID_ENGINE)
 			continue;
 
-		GEM_BUG_ON(engine->legacy_idx >= I915_NUM_ENGINES);
+		GEM_BUG_ON(engine->legacy_idx >= max);
 		GEM_BUG_ON(e->engines[engine->legacy_idx]);
 
 		ce = intel_context_create(engine);
-- 
2.25.1

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Use uabi engines for the default engine map (rev3)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 5886 bytes --]

== Series Details ==

Series: drm/i915: Use uabi engines for the default engine map (rev3)
URL   : https://patchwork.freedesktop.org/series/68395/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12624 -> Patchwork_68395v3
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_68395v3/index.html

Participating hosts (38 -> 38)
------------------------------

  Additional (2): fi-kbl-soraka fi-pnv-d510 
  Missing    (2): fi-ilk-650 fi-snb-2520m 

Known issues
------------

  Here are the changes found in Patchwork_68395v3 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_gttfill@basic:
    - fi-pnv-d510:        NOTRUN -> [FAIL][1] ([i915#7229])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_68395v3/fi-pnv-d510/igt@gem_exec_gttfill@basic.html

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#2190])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_68395v3/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#4613]) +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_68395v3/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-apl-guc:         [PASS][4] -> [DMESG-FAIL][5] ([i915#5334])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12624/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_68395v3/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][6] ([i915#1886])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_68395v3/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@guc:
    - fi-kbl-soraka:      NOTRUN -> [INCOMPLETE][7] ([i915#7913])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_68395v3/fi-kbl-soraka/igt@i915_selftest@live@guc.html

  * igt@kms_chamelium_frames@hdmi-crc-fast:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][8] ([fdo#109271]) +15 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_68395v3/fi-kbl-soraka/igt@kms_chamelium_frames@hdmi-crc-fast.html

  * igt@kms_psr@primary_page_flip:
    - fi-pnv-d510:        NOTRUN -> [SKIP][9] ([fdo#109271]) +44 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_68395v3/fi-pnv-d510/igt@kms_psr@primary_page_flip.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_mocs:
    - {bat-rpls-1}:       [DMESG-FAIL][10] ([i915#7059]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12624/bat-rpls-1/igt@i915_selftest@live@gt_mocs.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_68395v3/bat-rpls-1/igt@i915_selftest@live@gt_mocs.html

  * igt@i915_selftest@live@requests:
    - {bat-rpls-2}:       [INCOMPLETE][12] ([i915#6257]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12624/bat-rpls-2/igt@i915_selftest@live@requests.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_68395v3/bat-rpls-2/igt@i915_selftest@live@requests.html
    - {bat-rpls-1}:       [INCOMPLETE][14] ([i915#4983]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12624/bat-rpls-1/igt@i915_selftest@live@requests.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_68395v3/bat-rpls-1/igt@i915_selftest@live@requests.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions:
    - fi-bsw-kefka:       [FAIL][16] ([i915#6298]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12624/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_68395v3/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#6257]: https://gitlab.freedesktop.org/drm/intel/issues/6257
  [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#7059]: https://gitlab.freedesktop.org/drm/intel/issues/7059
  [i915#7229]: https://gitlab.freedesktop.org/drm/intel/issues/7229
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913


Build changes
-------------

  * Linux: CI_DRM_12624 -> Patchwork_68395v3

  CI-20190529: 20190529
  CI_DRM_12624: 18fa3d2237f6df82980349f6bef5281096dfc91d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7134: 61b8c0a0c8a9611c47749c0b1a262843892cccd7 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_68395v3: 18fa3d2237f6df82980349f6bef5281096dfc91d @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

b750c3640981 drm/i915: Use uabi engines for the default engine map

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_68395v3/index.html

[-- Attachment #2: Type: text/html, Size: 6722 bytes --]

Re: [Intel-gfx] [PATCH] drm/i915: Use uabi engines for the default engine map

[Cavitt, Jonathan]

-----Original Message-----
From: Cavitt, Jonathan <jonathan.cavitt@intel.com> 
Sent: Monday, January 23, 2023 7:25 AM
To: intel-gfx@lists.freedesktop.org
Cc: Cavitt, Jonathan <jonathan.cavitt@intel.com>; Dutt, Sudeep <sudeep.dutt@intel.com>
Subject: [PATCH] drm/i915: Use uabi engines for the default engine map
> 
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Default engine map is exactly about uabi engines so no excuse not to use
> the appropriate iterator to populate it.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>

Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
-Jonathan Cavitt

> ---
>  drivers/gpu/drm/i915/gem/i915_gem_context.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index 454e73a433c8..42a39e103d7c 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -1096,16 +1096,15 @@ static struct i915_gem_engines *alloc_engines(unsigned int count)
>  static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
>  						struct intel_sseu rcs_sseu)
>  {
> -	const struct intel_gt *gt = to_gt(ctx->i915);
> +	const unsigned int max = I915_NUM_ENGINES;
>  	struct intel_engine_cs *engine;
>  	struct i915_gem_engines *e, *err;
> -	enum intel_engine_id id;
>  
> -	e = alloc_engines(I915_NUM_ENGINES);
> +	e = alloc_engines(max);
>  	if (!e)
>  		return ERR_PTR(-ENOMEM);
>  
> -	for_each_engine(engine, gt, id) {
> +	for_each_uabi_engine(engine, ctx->i915) {
>  		struct intel_context *ce;
>  		struct intel_sseu sseu = {};
>  		int ret;
> @@ -1113,7 +1112,7 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
>  		if (engine->legacy_idx == INVALID_ENGINE)
>  			continue;
>  
> -		GEM_BUG_ON(engine->legacy_idx >= I915_NUM_ENGINES);
> +		GEM_BUG_ON(engine->legacy_idx >= max);
>  		GEM_BUG_ON(e->engines[engine->legacy_idx]);
>  
>  		ce = intel_context_create(engine);
> -- 
> 2.25.1
> 
> 

Re: [Intel-gfx] [PATCH] drm/i915: Use uabi engines for the default engine map

[Tvrtko Ursulin]

On 23/01/2023 18:56, Jonathan Cavitt wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Default engine map is exactly about uabi engines so no excuse not to use
> the appropriate iterator to populate it.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Reveiwed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>

Fixed the r-b spelling and pushed - thanks for the review and sending 
the patch over!

Regards,

Tvrtko

> ---
>   drivers/gpu/drm/i915/gem/i915_gem_context.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index 454e73a433c8..42a39e103d7c 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -1096,16 +1096,15 @@ static struct i915_gem_engines *alloc_engines(unsigned int count)
>   static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
>   						struct intel_sseu rcs_sseu)
>   {
> -	const struct intel_gt *gt = to_gt(ctx->i915);
> +	const unsigned int max = I915_NUM_ENGINES;
>   	struct intel_engine_cs *engine;
>   	struct i915_gem_engines *e, *err;
> -	enum intel_engine_id id;
>   
> -	e = alloc_engines(I915_NUM_ENGINES);
> +	e = alloc_engines(max);
>   	if (!e)
>   		return ERR_PTR(-ENOMEM);
>   
> -	for_each_engine(engine, gt, id) {
> +	for_each_uabi_engine(engine, ctx->i915) {
>   		struct intel_context *ce;
>   		struct intel_sseu sseu = {};
>   		int ret;
> @@ -1113,7 +1112,7 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
>   		if (engine->legacy_idx == INVALID_ENGINE)
>   			continue;
>   
> -		GEM_BUG_ON(engine->legacy_idx >= I915_NUM_ENGINES);
> +		GEM_BUG_ON(engine->legacy_idx >= max);
>   		GEM_BUG_ON(e->engines[engine->legacy_idx]);
>   
>   		ce = intel_context_create(engine);

[Intel-gfx] [PATCH] drm/i915: Use uabi engines for the default engine map

[Jonathan Cavitt]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Default engine map is exactly about uabi engines so no excuse not to use
the appropriate iterator to populate it.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Reveiwed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 454e73a433c8..42a39e103d7c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1096,16 +1096,15 @@ static struct i915_gem_engines *alloc_engines(unsigned int count)
 static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
 						struct intel_sseu rcs_sseu)
 {
-	const struct intel_gt *gt = to_gt(ctx->i915);
+	const unsigned int max = I915_NUM_ENGINES;
 	struct intel_engine_cs *engine;
 	struct i915_gem_engines *e, *err;
-	enum intel_engine_id id;
 
-	e = alloc_engines(I915_NUM_ENGINES);
+	e = alloc_engines(max);
 	if (!e)
 		return ERR_PTR(-ENOMEM);
 
-	for_each_engine(engine, gt, id) {
+	for_each_uabi_engine(engine, ctx->i915) {
 		struct intel_context *ce;
 		struct intel_sseu sseu = {};
 		int ret;
@@ -1113,7 +1112,7 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
 		if (engine->legacy_idx == INVALID_ENGINE)
 			continue;
 
-		GEM_BUG_ON(engine->legacy_idx >= I915_NUM_ENGINES);
+		GEM_BUG_ON(engine->legacy_idx >= max);
 		GEM_BUG_ON(e->engines[engine->legacy_idx]);
 
 		ce = intel_context_create(engine);
-- 
2.25.1

Re: [Intel-gfx] [PATCH] drm/i915: Use uabi engines for the default engine map

[Tvrtko Ursulin]

On 20/01/2023 19:19, Jonathan Cavitt wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Default engine map is exactly about uabi engines so no excuse not to use
> the appropriate iterator to populate it.

Okay I guess this was something I did as part of multi-tile enabling 
refactor some years ago, so even if it looks a bit head scratching on 
it's own it still has a purpose so IMO if someone r-b's it is okay to merge.

Jonathan even you could r-b, but you need to add your s-o-b in any case 
and re-send.

Regards,

Tvrtko

> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_context.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index 454e73a433c8..42a39e103d7c 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -1096,16 +1096,15 @@ static struct i915_gem_engines *alloc_engines(unsigned int count)
>   static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
>   						struct intel_sseu rcs_sseu)
>   {
> -	const struct intel_gt *gt = to_gt(ctx->i915);
> +	const unsigned int max = I915_NUM_ENGINES;
>   	struct intel_engine_cs *engine;
>   	struct i915_gem_engines *e, *err;
> -	enum intel_engine_id id;
>   
> -	e = alloc_engines(I915_NUM_ENGINES);
> +	e = alloc_engines(max);
>   	if (!e)
>   		return ERR_PTR(-ENOMEM);
>   
> -	for_each_engine(engine, gt, id) {
> +	for_each_uabi_engine(engine, ctx->i915) {
>   		struct intel_context *ce;
>   		struct intel_sseu sseu = {};
>   		int ret;
> @@ -1113,7 +1112,7 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
>   		if (engine->legacy_idx == INVALID_ENGINE)
>   			continue;
>   
> -		GEM_BUG_ON(engine->legacy_idx >= I915_NUM_ENGINES);
> +		GEM_BUG_ON(engine->legacy_idx >= max);
>   		GEM_BUG_ON(e->engines[engine->legacy_idx]);
>   
>   		ce = intel_context_create(engine);

[Intel-gfx] [PATCH] drm/i915: Use uabi engines for the default engine map

[Jonathan Cavitt]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Default engine map is exactly about uabi engines so no excuse not to use
the appropriate iterator to populate it.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 454e73a433c8..42a39e103d7c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1096,16 +1096,15 @@ static struct i915_gem_engines *alloc_engines(unsigned int count)
 static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
 						struct intel_sseu rcs_sseu)
 {
-	const struct intel_gt *gt = to_gt(ctx->i915);
+	const unsigned int max = I915_NUM_ENGINES;
 	struct intel_engine_cs *engine;
 	struct i915_gem_engines *e, *err;
-	enum intel_engine_id id;
 
-	e = alloc_engines(I915_NUM_ENGINES);
+	e = alloc_engines(max);
 	if (!e)
 		return ERR_PTR(-ENOMEM);
 
-	for_each_engine(engine, gt, id) {
+	for_each_uabi_engine(engine, ctx->i915) {
 		struct intel_context *ce;
 		struct intel_sseu sseu = {};
 		int ret;
@@ -1113,7 +1112,7 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx,
 		if (engine->legacy_idx == INVALID_ENGINE)
 			continue;
 
-		GEM_BUG_ON(engine->legacy_idx >= I915_NUM_ENGINES);
+		GEM_BUG_ON(engine->legacy_idx >= max);
 		GEM_BUG_ON(e->engines[engine->legacy_idx]);
 
 		ce = intel_context_create(engine);
-- 
2.25.1