[PATCH 0/5] Misc DG2 enabling patches

[PATCH 0/5] Misc DG2 enabling patches

[Ramalingam C]

Assorted DG2 enabling patches.

Bruce Chang (1):
  drm/i915/dg2: Add Wa_22011100796

Ramalingam C (3):
  drm/i915/dg2: Add Wa_22011450934
  drm/i915: align the plane_vma to min_page_size of stolen mem
  drm/i915: More gt idling time with guc submission

Stuart Summers (1):
  drm/i915/guc: Allow user to override driver load failure without GuC

 .../drm/i915/display/intel_plane_initial.c    |  5 ++--
 drivers/gpu/drm/i915/gt/intel_lrc.c           | 28 +++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_reset.c         |  9 ++++++
 drivers/gpu/drm/i915/gt/uc/intel_uc.c         | 17 +++++++++--
 drivers/gpu/drm/i915/i915_params.h            |  1 +
 drivers/gpu/drm/i915/i915_reg.h               |  4 +++
 .../gpu/drm/i915/selftests/igt_flush_test.c   |  2 +-
 7 files changed, 61 insertions(+), 5 deletions(-)

-- 
2.20.1

[PATCH 1/5] drm/i915/dg2: Add Wa_22011450934

[Ramalingam C]

An indirect ctx wabb is implemented as per Wa_22011450934 to avoid rcs
restore hang during context restore of a preempted context in GPGPU mode

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
cc: Chris Wilson <chris.p.wilson@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 28 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h     |  4 ++++
 2 files changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 89a95a125fc8..8440f4b0f613 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1165,6 +1165,29 @@ gen12_emit_cmd_buf_wa(const struct intel_context *ce, u32 *cs)
 	return cs;
 }
 
+/*
+ * On DG2 during context restore of a preempted context in GPGPU mode,
+ * RCS restore hang is detected. This is extremely timing dependent.
+ * To address this below sw wabb is implemented for DG2 A steppings.
+ */
+static u32 *
+dg2_emit_rcs_hang_wabb(const struct intel_context *ce, u32 *cs)
+{
+	*cs++ = MI_LOAD_REGISTER_IMM(1);
+	*cs++ = i915_mmio_reg_offset(GEN12_STATE_ACK_DEBUG);
+	*cs++ = 0x21;
+
+	*cs++ = MI_LOAD_REGISTER_REG;
+	*cs++ = i915_mmio_reg_offset(RING_NOPID(ce->engine->mmio_base));
+	*cs++ = i915_mmio_reg_offset(GEN12_CULLBIT1);
+
+	*cs++ = MI_LOAD_REGISTER_REG;
+	*cs++ = i915_mmio_reg_offset(RING_NOPID(ce->engine->mmio_base));
+	*cs++ = i915_mmio_reg_offset(GEN12_CULLBIT2);
+
+	return cs;
+}
+
 static u32 *
 gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs)
 {
@@ -1172,6 +1195,11 @@ gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs)
 	cs = gen12_emit_cmd_buf_wa(ce, cs);
 	cs = gen12_emit_restore_scratch(ce, cs);
 
+	/* Wa_22011450934:dg2 */
+	if (IS_DG2_GRAPHICS_STEP(ce->engine->i915, G10, STEP_A0, STEP_B0) ||
+	    IS_DG2_GRAPHICS_STEP(ce->engine->i915, G11, STEP_A0, STEP_B0))
+		cs = dg2_emit_rcs_hang_wabb(ce, cs);
+
 	/* Wa_16013000631:dg2 */
 	if (IS_DG2_GRAPHICS_STEP(ce->engine->i915, G10, STEP_B0, STEP_C0) ||
 	    IS_DG2_G11(ce->engine->i915))
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ec48406eb37a..3f94f4b5ef6c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -12026,4 +12026,8 @@ enum skl_power_gate {
 #define SLICE_COMMON_ECO_CHICKEN1		_MMIO(0x731C)
 #define   MSC_MSAA_REODER_BUF_BYPASS_DISABLE	REG_BIT(14)
 
+#define GEN12_CULLBIT1			_MMIO(0x6100)
+#define GEN12_CULLBIT2			_MMIO(0x7030)
+#define GEN12_STATE_ACK_DEBUG		_MMIO(0x20BC)
+
 #endif /* _I915_REG_H_ */
-- 
2.20.1

Re: [Intel-gfx] [PATCH 1/5] drm/i915/dg2: Add Wa_22011450934

[Matthew Auld]

On Fri, 28 Jan 2022 at 18:52, Ramalingam C <ramalingam.c@intel.com> wrote:
>
> An indirect ctx wabb is implemented as per Wa_22011450934 to avoid rcs
> restore hang during context restore of a preempted context in GPGPU mode
>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> cc: Chris Wilson <chris.p.wilson@intel.com>
Acked-by: Matthew Auld <matthew.auld@intel.com>

Re: [Intel-gfx] [PATCH 1/5] drm/i915/dg2: Add Wa_22011450934

[Matthew Auld]

On 07/02/2022 11:48, Matthew Auld wrote:
> On Fri, 28 Jan 2022 at 18:52, Ramalingam C <ramalingam.c@intel.com> wrote:
>>
>> An indirect ctx wabb is implemented as per Wa_22011450934 to avoid rcs
>> restore hang during context restore of a preempted context in GPGPU mode
>>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> cc: Chris Wilson <chris.p.wilson@intel.com>
> Acked-by: Matthew Auld <matthew.auld@intel.com>

Also, feel free to upgrade to r-b for this and patches 2-4.

Re: [Intel-gfx] [PATCH 1/5] drm/i915/dg2: Add Wa_22011450934

[Ramalingam C]

On 2022-02-07 at 11:52:48 +0000, Matthew Auld wrote:
> On 07/02/2022 11:48, Matthew Auld wrote:
> > On Fri, 28 Jan 2022 at 18:52, Ramalingam C <ramalingam.c@intel.com> wrote:
> > > 
> > > An indirect ctx wabb is implemented as per Wa_22011450934 to avoid rcs
> > > restore hang during context restore of a preempted context in GPGPU mode
> > > 
> > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > > cc: Chris Wilson <chris.p.wilson@intel.com>
> > Acked-by: Matthew Auld <matthew.auld@intel.com>
> 
> Also, feel free to upgrade to r-b for this and patches 2-4.

Thank you Matt. I have pushed these changes to the tree.

Ram.

[PATCH 2/5] drm/i915: align the plane_vma to min_page_size of stolen mem

[Ramalingam C]

Align the plane vma size to the stolem memory regions' min_page_size.

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
cc: Matthew Auld <matthew.auld@intel.com>
cc: Chris P Wilson <chris.p.wilson@intel.com>
---
 drivers/gpu/drm/i915/display/intel_plane_initial.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
index e4186a0b8edb..543877998078 100644
--- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
@@ -46,17 +46,18 @@ static struct i915_vma *
 initial_plane_vma(struct drm_i915_private *i915,
 		  struct intel_initial_plane_config *plane_config)
 {
+	struct intel_memory_region *mem = i915->mm.stolen_region;
 	struct drm_i915_gem_object *obj;
 	struct i915_vma *vma;
 	u32 base, size;
 
-	if (plane_config->size == 0)
+	if (!mem || plane_config->size == 0)
 		return NULL;
 
 	base = round_down(plane_config->base,
 			  I915_GTT_MIN_ALIGNMENT);
 	size = round_up(plane_config->base + plane_config->size,
-			I915_GTT_MIN_ALIGNMENT);
+			mem->min_page_size);
 	size -= base;
 
 	/*
-- 
2.20.1

Re: [PATCH 2/5] drm/i915: align the plane_vma to min_page_size of stolen mem

[Matthew Auld]

On 28/01/2022 18:52, Ramalingam C wrote:
> Align the plane vma size to the stolem memory regions' min_page_size.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> cc: Matthew Auld <matthew.auld@intel.com>
> cc: Chris P Wilson <chris.p.wilson@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>

Re: [Intel-gfx] [PATCH 2/5] drm/i915: align the plane_vma to min_page_size of stolen mem

[Matthew Auld]

On Mon, 31 Jan 2022 at 10:18, Matthew Auld <matthew.auld@intel.com> wrote:
>
> On 28/01/2022 18:52, Ramalingam C wrote:
> > Align the plane vma size to the stolem memory regions' min_page_size.
> >
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > cc: Matthew Auld <matthew.auld@intel.com>
> > cc: Chris P Wilson <chris.p.wilson@intel.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>

Do you know for sure that the initial fb is allocated in stolen-lmem on DG2 btw?

[PATCH 3/5] drm/i915: More gt idling time with guc submission

[Ramalingam C]

On i915_selftest@live@gt_timelines, we create many contexts in loop and
create and submit request and then destoy contexts. Destroying the context
needs to disable scheduling, wait for G2H, deregister context and wait
for G2H to destroy each context. Idling of the gt has to wait for all
this to complete which is taking ~3sec for this test.

Hence we are increasing the igt_flush_test's timeout for gt idling to
3Sec.

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
cc: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/i915/selftests/igt_flush_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/selftests/igt_flush_test.c b/drivers/gpu/drm/i915/selftests/igt_flush_test.c
index b84594601d30..b484e12df417 100644
--- a/drivers/gpu/drm/i915/selftests/igt_flush_test.c
+++ b/drivers/gpu/drm/i915/selftests/igt_flush_test.c
@@ -19,7 +19,7 @@ int igt_flush_test(struct drm_i915_private *i915)
 
 	cond_resched();
 
-	if (intel_gt_wait_for_idle(gt, HZ) == -ETIME) {
+	if (intel_gt_wait_for_idle(gt, HZ * 3) == -ETIME) {
 		pr_err("%pS timed out, cancelling all further testing.\n",
 		       __builtin_return_address(0));
 
-- 
2.20.1

Re: [PATCH 3/5] drm/i915: More gt idling time with guc submission

[Matthew Auld]

On 28/01/2022 18:52, Ramalingam C wrote:
> On i915_selftest@live@gt_timelines, we create many contexts in loop and
> create and submit request and then destoy contexts. Destroying the context
> needs to disable scheduling, wait for G2H, deregister context and wait
> for G2H to destroy each context. Idling of the gt has to wait for all
> this to complete which is taking ~3sec for this test.
> 
> Hence we are increasing the igt_flush_test's timeout for gt idling to
> 3Sec.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> cc: Matthew Brost <matthew.brost@intel.com>
Acked-by: Matthew Auld <matthew.auld@intel.com>

[PATCH 4/5] drm/i915/dg2: Add Wa_22011100796

[Ramalingam C]

From: Bruce Chang <yu.bruce.chang@intel.com>

Whenever Full soft reset is required, reset all individual engines
first, and then do a full soft reset.

Signed-off-by: Bruce Chang <yu.bruce.chang@intel.com>
cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_reset.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 6f2821cca409..5fae56b89319 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -600,6 +600,15 @@ static int gen8_reset_engines(struct intel_gt *gt,
 		 */
 	}
 
+	/*
+	 * Wa_22011100796:dg2, whenever Full soft reset is required,
+	 * reset all individual engines firstly, and then do a full soft reset.
+	 *
+	 * This is best effort, so ignore any error from the initial reset.
+	 */
+	if (IS_DG2(gt->i915) && engine_mask == ALL_ENGINES)
+		gen11_reset_engines(gt, gt->info.engine_mask, 0);
+
 	if (GRAPHICS_VER(gt->i915) >= 11)
 		ret = gen11_reset_engines(gt, engine_mask, retry);
 	else
-- 
2.20.1

Re: [Intel-gfx] [PATCH 4/5] drm/i915/dg2: Add Wa_22011100796

[Matthew Auld]

On Fri, 28 Jan 2022 at 18:52, Ramalingam C <ramalingam.c@intel.com> wrote:
>
> From: Bruce Chang <yu.bruce.chang@intel.com>
>
> Whenever Full soft reset is required, reset all individual engines
> first, and then do a full soft reset.
>
> Signed-off-by: Bruce Chang <yu.bruce.chang@intel.com>
> cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Acked-by: Matthew Auld <matthew.auld@intel.com>

[PATCH 5/5] drm/i915/guc: Allow user to override driver load failure without GuC

[Ramalingam C]

From: Stuart Summers <stuart.summers@intel.com>

The driver is set currently to fail modprobe when GuC is disabled
(enable_guc=0) after GuC has been loaded on a previous modprobe.
For GuC deprivilege, the BIOS is setting the locked bit, so the
driver always considers the GuC to have been loaded and thus does
not support enable_guc=0 on these platforms.

There are some debug scenarios where loading without GuC can be
interesting. Add a new feature flag for GuC deprivilege and a mode
(enable_guc=0x80) which can be exclusively set to skip the locked
bit check.

cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Signed-off-by: Stuart Summers <stuart.summers@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 17 +++++++++++++++--
 drivers/gpu/drm/i915/i915_params.h    |  1 +
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index da199aa6989f..a1376dbd04fe 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -108,7 +108,7 @@ static void __confirm_options(struct intel_uc *uc)
 			 "Incompatible option enable_guc=%d - %s\n",
 			 i915->params.enable_guc, "GuC submission is N/A");
 
-	if (i915->params.enable_guc & ~ENABLE_GUC_MASK)
+	if (i915->params.enable_guc & ~(ENABLE_GUC_MASK | ENABLE_GUC_DO_NOT_LOAD_GUC))
 		drm_info(&i915->drm,
 			 "Incompatible option enable_guc=%d - %s\n",
 			 i915->params.enable_guc, "undocumented flag");
@@ -416,8 +416,21 @@ static bool uc_is_wopcm_locked(struct intel_uc *uc)
 	       (intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET) & GUC_WOPCM_OFFSET_VALID);
 }
 
+static inline bool skip_lock_check(struct drm_i915_private *i915)
+{
+	/*
+	 * For platforms with GuC deprivilege, if a user *really* wants
+	 * to run without GuC, let that happen by setting enable_guc=0x80.
+	 */
+	return (HAS_GUC_DEPRIVILEGE(i915) &&
+		(i915->params.enable_guc & ENABLE_GUC_DO_NOT_LOAD_GUC) &&
+		!(i915->params.enable_guc & ~ENABLE_GUC_DO_NOT_LOAD_GUC));
+}
+
 static int __uc_check_hw(struct intel_uc *uc)
 {
+	struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
+
 	if (!intel_uc_supports_guc(uc))
 		return 0;
 
@@ -426,7 +439,7 @@ static int __uc_check_hw(struct intel_uc *uc)
 	 * before on this system after reboot, otherwise we risk GPU hangs.
 	 * To check if GuC was loaded before we look at WOPCM registers.
 	 */
-	if (uc_is_wopcm_locked(uc))
+	if (uc_is_wopcm_locked(uc) && likely(!skip_lock_check(i915)))
 		return -EIO;
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index c9d53ff910a0..8996ba2cc3a8 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -32,6 +32,7 @@ struct drm_printer;
 
 #define ENABLE_GUC_SUBMISSION		BIT(0)
 #define ENABLE_GUC_LOAD_HUC		BIT(1)
+#define ENABLE_GUC_DO_NOT_LOAD_GUC	BIT(7)
 #define ENABLE_GUC_MASK			GENMASK(1, 0)
 
 /*
-- 
2.20.1

Re: [Intel-gfx] [PATCH 5/5] drm/i915/guc: Allow user to override driver load failure without GuC

[Daniele Ceraolo Spurio]

On 1/28/2022 10:52 AM, Ramalingam C wrote:
> From: Stuart Summers <stuart.summers@intel.com>
>
> The driver is set currently to fail modprobe when GuC is disabled
> (enable_guc=0) after GuC has been loaded on a previous modprobe.
> For GuC deprivilege, the BIOS is setting the locked bit, so the
> driver always considers the GuC to have been loaded and thus does
> not support enable_guc=0 on these platforms.
>
> There are some debug scenarios where loading without GuC can be
> interesting. Add a new feature flag for GuC deprivilege and a mode
> (enable_guc=0x80) which can be exclusively set to skip the locked
> bit check.

This is a debug-only patch, so IMO it should definitely not be merged 
as-is, because we don't want normal users having access to this option 
as it can lead to an hard gpu hangs if misused. I'm honestly not 
convinced we want this in the tree at all, because you can still run 
without GuC submission by setting enable_guc=2; the only thing this 
patch adds is the ability to skip the GuC/HuC load entirely. If you 
think there is still value in having this ability for debug, then the 
patch should be updated to only allow the new option when one of the 
debug flags is set; I'd go with DEBUG_GEM as we have that enabled by 
default in our CI builds.

Daniele

> cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/uc/intel_uc.c | 17 +++++++++++++++--
>   drivers/gpu/drm/i915/i915_params.h    |  1 +
>   2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index da199aa6989f..a1376dbd04fe 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -108,7 +108,7 @@ static void __confirm_options(struct intel_uc *uc)
>   			 "Incompatible option enable_guc=%d - %s\n",
>   			 i915->params.enable_guc, "GuC submission is N/A");
>   
> -	if (i915->params.enable_guc & ~ENABLE_GUC_MASK)
> +	if (i915->params.enable_guc & ~(ENABLE_GUC_MASK | ENABLE_GUC_DO_NOT_LOAD_GUC))
>   		drm_info(&i915->drm,
>   			 "Incompatible option enable_guc=%d - %s\n",
>   			 i915->params.enable_guc, "undocumented flag");
> @@ -416,8 +416,21 @@ static bool uc_is_wopcm_locked(struct intel_uc *uc)
>   	       (intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET) & GUC_WOPCM_OFFSET_VALID);
>   }
>   
> +static inline bool skip_lock_check(struct drm_i915_private *i915)
> +{
> +	/*
> +	 * For platforms with GuC deprivilege, if a user *really* wants
> +	 * to run without GuC, let that happen by setting enable_guc=0x80.
> +	 */
> +	return (HAS_GUC_DEPRIVILEGE(i915) &&
> +		(i915->params.enable_guc & ENABLE_GUC_DO_NOT_LOAD_GUC) &&
> +		!(i915->params.enable_guc & ~ENABLE_GUC_DO_NOT_LOAD_GUC));
> +}
> +
>   static int __uc_check_hw(struct intel_uc *uc)
>   {
> +	struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
> +
>   	if (!intel_uc_supports_guc(uc))
>   		return 0;
>   
> @@ -426,7 +439,7 @@ static int __uc_check_hw(struct intel_uc *uc)
>   	 * before on this system after reboot, otherwise we risk GPU hangs.
>   	 * To check if GuC was loaded before we look at WOPCM registers.
>   	 */
> -	if (uc_is_wopcm_locked(uc))
> +	if (uc_is_wopcm_locked(uc) && likely(!skip_lock_check(i915)))
>   		return -EIO;
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index c9d53ff910a0..8996ba2cc3a8 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -32,6 +32,7 @@ struct drm_printer;
>   
>   #define ENABLE_GUC_SUBMISSION		BIT(0)
>   #define ENABLE_GUC_LOAD_HUC		BIT(1)
> +#define ENABLE_GUC_DO_NOT_LOAD_GUC	BIT(7)
>   #define ENABLE_GUC_MASK			GENMASK(1, 0)
>   
>   /*

Re: [Intel-gfx] [PATCH 5/5] drm/i915/guc: Allow user to override driver load failure without GuC

[Ramalingam C]

On 2022-02-07 at 08:55:20 -0800, Daniele Ceraolo Spurio wrote:
> 
> 
> On 1/28/2022 10:52 AM, Ramalingam C wrote:
> > From: Stuart Summers <stuart.summers@intel.com>
> > 
> > The driver is set currently to fail modprobe when GuC is disabled
> > (enable_guc=0) after GuC has been loaded on a previous modprobe.
> > For GuC deprivilege, the BIOS is setting the locked bit, so the
> > driver always considers the GuC to have been loaded and thus does
> > not support enable_guc=0 on these platforms.
> > 
> > There are some debug scenarios where loading without GuC can be
> > interesting. Add a new feature flag for GuC deprivilege and a mode
> > (enable_guc=0x80) which can be exclusively set to skip the locked
> > bit check.
> 
> This is a debug-only patch, so IMO it should definitely not be merged as-is,
> because we don't want normal users having access to this option as it can
> lead to an hard gpu hangs if misused. I'm honestly not convinced we want
> this in the tree at all, because you can still run without GuC submission by
> setting enable_guc=2; the only thing this patch adds is the ability to skip
> the GuC/HuC load entirely. If you think there is still value in having this
> ability for debug, then the patch should be updated to only allow the new
> option when one of the debug flags is set; I'd go with DEBUG_GEM as we have
> that enabled by default in our CI builds.

Thank you daniele. Dropping this patch as i dont see any usecase as of
now.

Ram.
> 
> Daniele
> 
> > cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > ---
> >   drivers/gpu/drm/i915/gt/uc/intel_uc.c | 17 +++++++++++++++--
> >   drivers/gpu/drm/i915/i915_params.h    |  1 +
> >   2 files changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > index da199aa6989f..a1376dbd04fe 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > @@ -108,7 +108,7 @@ static void __confirm_options(struct intel_uc *uc)
> >   			 "Incompatible option enable_guc=%d - %s\n",
> >   			 i915->params.enable_guc, "GuC submission is N/A");
> > -	if (i915->params.enable_guc & ~ENABLE_GUC_MASK)
> > +	if (i915->params.enable_guc & ~(ENABLE_GUC_MASK | ENABLE_GUC_DO_NOT_LOAD_GUC))
> >   		drm_info(&i915->drm,
> >   			 "Incompatible option enable_guc=%d - %s\n",
> >   			 i915->params.enable_guc, "undocumented flag");
> > @@ -416,8 +416,21 @@ static bool uc_is_wopcm_locked(struct intel_uc *uc)
> >   	       (intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET) & GUC_WOPCM_OFFSET_VALID);
> >   }
> > +static inline bool skip_lock_check(struct drm_i915_private *i915)
> > +{
> > +	/*
> > +	 * For platforms with GuC deprivilege, if a user *really* wants
> > +	 * to run without GuC, let that happen by setting enable_guc=0x80.
> > +	 */
> > +	return (HAS_GUC_DEPRIVILEGE(i915) &&
> > +		(i915->params.enable_guc & ENABLE_GUC_DO_NOT_LOAD_GUC) &&
> > +		!(i915->params.enable_guc & ~ENABLE_GUC_DO_NOT_LOAD_GUC));
> > +}
> > +
> >   static int __uc_check_hw(struct intel_uc *uc)
> >   {
> > +	struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
> > +
> >   	if (!intel_uc_supports_guc(uc))
> >   		return 0;
> > @@ -426,7 +439,7 @@ static int __uc_check_hw(struct intel_uc *uc)
> >   	 * before on this system after reboot, otherwise we risk GPU hangs.
> >   	 * To check if GuC was loaded before we look at WOPCM registers.
> >   	 */
> > -	if (uc_is_wopcm_locked(uc))
> > +	if (uc_is_wopcm_locked(uc) && likely(!skip_lock_check(i915)))
> >   		return -EIO;
> >   	return 0;
> > diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> > index c9d53ff910a0..8996ba2cc3a8 100644
> > --- a/drivers/gpu/drm/i915/i915_params.h
> > +++ b/drivers/gpu/drm/i915/i915_params.h
> > @@ -32,6 +32,7 @@ struct drm_printer;
> >   #define ENABLE_GUC_SUBMISSION		BIT(0)
> >   #define ENABLE_GUC_LOAD_HUC		BIT(1)
> > +#define ENABLE_GUC_DO_NOT_LOAD_GUC	BIT(7)
> >   #define ENABLE_GUC_MASK			GENMASK(1, 0)
> >   /*
>