[PATCH v2 0/3] restore WaEnableFloatBlendOptimization

[PATCH v2 0/3] restore WaEnableFloatBlendOptimization

[Talha Nassar]

This is the v2 of my patch after taking the feed from Chris. I have also included the HSDES per Mika's suggestion.
I am attaching the two-patch series from Tvrtko as there is a dependency.
Also to note that git couldn't apply Tvrtko's first patch due to a patch by Daniele that touched the same file and was merged. I had to manually apply the patch and no code changes were necessary.

v1: enabled the wa and added the register as wo in igt test.
v2: also modified the wa mask

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>

Talha Nassar (1):
  drm/i915/icl: restore WaEnableFloatBlendOptimization

Tvrtko Ursulin (2):
  drm/i915: Move workaround infrastructure code up
  drm/i915: Save some lines of source code in workarounds

 drivers/gpu/drm/i915/i915_reg.h          |  3 ++
 drivers/gpu/drm/i915/intel_workarounds.c | 70 +++++++++++++-------------------
 2 files changed, 31 insertions(+), 42 deletions(-)

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 1/3] drm/i915: Move workaround infrastructure code up

[Talha Nassar]

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

Top comment in intel_workarounds.c says common code should come first so
lets respect that. Also, by moving the common code together opportunities
to reduce duplication will become more obvious.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_workarounds.c | 74 ++++++++++++++++----------------
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index 3210ad4..584c4a5 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -153,6 +153,43 @@ __wa_add(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, u32 val)
 	_wa_add(wal, &wa);
 }
 
+static void
+wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
+{
+	struct i915_wa wa = {
+		.reg = reg,
+		.mask = val,
+		.val = _MASKED_BIT_ENABLE(val)
+	};
+
+	_wa_add(wal, &wa);
+}
+
+static void
+wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask,
+		   u32 val)
+{
+	struct i915_wa wa = {
+		.reg = reg,
+		.mask = mask,
+		.val = val
+	};
+
+	_wa_add(wal, &wa);
+}
+
+static void
+wa_write(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
+{
+	wa_write_masked_or(wal, reg, ~0, val);
+}
+
+static void
+wa_write_or(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
+{
+	wa_write_masked_or(wal, reg, val, val);
+}
+
 #define WA_REG(addr, mask, val) __wa_add(wal, (addr), (mask), (val))
 
 #define WA_SET_BIT_MASKED(addr, mask) \
@@ -603,43 +640,6 @@ int intel_engine_emit_ctx_wa(struct i915_request *rq)
 }
 
 static void
-wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
-{
-	struct i915_wa wa = {
-		.reg = reg,
-		.mask = val,
-		.val = _MASKED_BIT_ENABLE(val)
-	};
-
-	_wa_add(wal, &wa);
-}
-
-static void
-wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask,
-		   u32 val)
-{
-	struct i915_wa wa = {
-		.reg = reg,
-		.mask = mask,
-		.val = val
-	};
-
-	_wa_add(wal, &wa);
-}
-
-static void
-wa_write(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
-{
-	wa_write_masked_or(wal, reg, ~0, val);
-}
-
-static void
-wa_write_or(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
-{
-	wa_write_masked_or(wal, reg, val, val);
-}
-
-static void
 gen9_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal)
 {
 	/* WaDisableKillLogic:bxt,skl,kbl */
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/3] drm/i915: Save some lines of source code in workarounds

[Talha Nassar]

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

No functional or code size change - just notice we can compact the source
by re-using a single helper for adding workarounds.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_workarounds.c | 32 ++++++--------------------------
 1 file changed, 6 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index 584c4a5..5c01055 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -142,7 +142,8 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa)
 }
 
 static void
-__wa_add(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, u32 val)
+wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask,
+		   u32 val)
 {
 	struct i915_wa wa = {
 		.reg = reg,
@@ -156,26 +157,7 @@ __wa_add(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, u32 val)
 static void
 wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
 {
-	struct i915_wa wa = {
-		.reg = reg,
-		.mask = val,
-		.val = _MASKED_BIT_ENABLE(val)
-	};
-
-	_wa_add(wal, &wa);
-}
-
-static void
-wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask,
-		   u32 val)
-{
-	struct i915_wa wa = {
-		.reg = reg,
-		.mask = mask,
-		.val = val
-	};
-
-	_wa_add(wal, &wa);
+	wa_write_masked_or(wal, reg, val, _MASKED_BIT_ENABLE(val));
 }
 
 static void
@@ -190,16 +172,14 @@ wa_write_or(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
 	wa_write_masked_or(wal, reg, val, val);
 }
 
-#define WA_REG(addr, mask, val) __wa_add(wal, (addr), (mask), (val))
-
 #define WA_SET_BIT_MASKED(addr, mask) \
-	WA_REG(addr, (mask), _MASKED_BIT_ENABLE(mask))
+	wa_write_masked_or(wal, (addr), (mask), _MASKED_BIT_ENABLE(mask))
 
 #define WA_CLR_BIT_MASKED(addr, mask) \
-	WA_REG(addr, (mask), _MASKED_BIT_DISABLE(mask))
+	wa_write_masked_or(wal, (addr), (mask), _MASKED_BIT_DISABLE(mask))
 
 #define WA_SET_FIELD_MASKED(addr, mask, value) \
-	WA_REG(addr, (mask), _MASKED_FIELD(mask, value))
+	wa_write_masked_or(wal, (addr), (mask), _MASKED_FIELD((mask), (value)))
 
 static void gen8_ctx_workarounds_init(struct intel_engine_cs *engine)
 {
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 3/3] drm/i915/icl: restore WaEnableFloatBlendOptimization

[Talha Nassar]

Enables blend optimization for floating point RTs

This restores the workaround that was reverted in c358514ba8da
("Revert "drm/i915/icl: WaEnableFloatBlendOptimization"").

The revert was due to the register write seemingly not sticking,
but the HW team has confirmed that this is because the
register is WO and that the workaround is indeed required.

Here the wa is added with a mask of 0 since the register is WO.

References: https://hsdes.intel.com/resource/1408134172
References: https://bugs.freedesktop.org/show_bug.cgi?id=107338
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Signed-off-by: Talha Nassar <talha.nassar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h          | 3 +++
 drivers/gpu/drm/i915/intel_workarounds.c | 6 ++++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 03adcf3..6b96477 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2801,6 +2801,9 @@ enum i915_power_well_id {
 #define GEN6_RCS_PWR_FSM _MMIO(0x22ac)
 #define GEN9_RCS_FE_FSM2 _MMIO(0x22a4)
 
+#define GEN10_CACHE_MODE_SS			_MMIO(0xe420)
+#define   FLOAT_BLEND_OPTIMIZATION_ENABLE	(1 << 4)
+
 /* Fuse readout registers for GT */
 #define HSW_PAVP_FUSE1			_MMIO(0x911C)
 #define   HSW_F1_EU_DIS_SHIFT		16
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index 5c01055..15f4a6d 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -549,6 +549,12 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine)
 	if (IS_ICL_REVID(i915, ICL_REVID_A0, ICL_REVID_A0))
 		WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3,
 				  GEN11_BLEND_EMB_FIX_DISABLE_IN_RCC);
+
+	/* WaEnableFloatBlendOptimization:icl */
+	wa_write_masked_or(wal,
+			   GEN10_CACHE_MODE_SS,
+			   0, /* write-only, so skip validation */
+			   _MASKED_BIT_ENABLE(FLOAT_BLEND_OPTIMIZATION_ENABLE));
 }
 
 void intel_engine_init_ctx_wa(struct intel_engine_cs *engine)
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/3] drm/i915: Move workaround infrastructure code up

[Chris Wilson]

Quoting Talha Nassar (2019-02-01 01:08:42)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Top comment in intel_workarounds.c says common code should come first so
> lets respect that. Also, by moving the common code together opportunities
> to reduce duplication will become more obvious.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for restore WaEnableFloatBlendOptimization

[Patchwork]

== Series Details ==

Series: restore WaEnableFloatBlendOptimization
URL   : https://patchwork.freedesktop.org/series/56071/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
5ff28df401d6 drm/i915: Move workaround infrastructure code up
3724f3bfa9a7 drm/i915: Save some lines of source code in workarounds
b3ca9b199190 drm/i915/icl: restore WaEnableFloatBlendOptimization
-:8: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit c358514ba8da ("Revert "drm/i915/icl: WaEnableFloatBlendOptimization"")'
#8: 
This restores the workaround that was reverted in c358514ba8da

total: 1 errors, 0 warnings, 0 checks, 21 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/3] drm/i915/icl: restore WaEnableFloatBlendOptimization

[Chris Wilson]

Quoting Talha Nassar (2019-02-01 01:08:44)
> Enables blend optimization for floating point RTs
> 
> This restores the workaround that was reverted in c358514ba8da
> ("Revert "drm/i915/icl: WaEnableFloatBlendOptimization"").
> 
> The revert was due to the register write seemingly not sticking,
> but the HW team has confirmed that this is because the
> register is WO and that the workaround is indeed required.
> 
> Here the wa is added with a mask of 0 since the register is WO.
> 
> References: https://hsdes.intel.com/resource/1408134172
> References: https://bugs.freedesktop.org/show_bug.cgi?id=107338
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> 
> Signed-off-by: Talha Nassar <talha.nassar@intel.com>

Vouching for the code, not the hw,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for restore WaEnableFloatBlendOptimization

[Patchwork]

== Series Details ==

Series: restore WaEnableFloatBlendOptimization
URL   : https://patchwork.freedesktop.org/series/56071/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5521 -> Patchwork_12113
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/56071/revisions/1/mbox/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@read_all_entries:
    - fi-ilk-650:         PASS -> DMESG-WARN [fdo#106387]

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         PASS -> INCOMPLETE [fdo#103927]

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  
#### Possible fixes ####

  * igt@i915_selftest@live_hangcheck:
    - fi-skl-iommu:       INCOMPLETE [fdo#108602] / [fdo#108744] -> PASS
    - {fi-icl-y}:         INCOMPLETE -> PASS

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

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

  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744


Participating hosts (49 -> 44)
------------------------------

  Missing    (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


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

    * Linux: CI_DRM_5521 -> Patchwork_12113

  CI_DRM_5521: dbd2e19079beb3b7f4077706179fba66d321e49f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4802: 4049adf01014af077df2174def4fadf7cecb066e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12113: b3ca9b19919023e1bda80d7e66a3f8f3009dbc83 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

b3ca9b199190 drm/i915/icl: restore WaEnableFloatBlendOptimization
3724f3bfa9a7 drm/i915: Save some lines of source code in workarounds
5ff28df401d6 drm/i915: Move workaround infrastructure code up

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12113/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for restore WaEnableFloatBlendOptimization

[Patchwork]

== Series Details ==

Series: restore WaEnableFloatBlendOptimization
URL   : https://patchwork.freedesktop.org/series/56071/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5521_full -> Patchwork_12113_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_12113_full:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@runner@aborted}:
    - shard-snb:          NOTRUN -> FAIL

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_cursor_crc@cursor-256x256-onscreen:
    - shard-glk:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-256x85-random:
    - shard-apl:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          PASS -> FAIL [fdo#104873]

  * igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
    - shard-glk:          PASS -> FAIL [fdo#106509] / [fdo#107409]

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          PASS -> FAIL [fdo#105363]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-none:
    - shard-apl:          PASS -> FAIL [fdo#103166] +2

  * igt@kms_setmode@basic:
    - shard-hsw:          PASS -> FAIL [fdo#99912]

  
#### Possible fixes ####

  * igt@kms_color@pipe-c-ctm-max:
    - shard-apl:          FAIL [fdo#108147] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-dpms:
    - shard-apl:          FAIL [fdo#103232] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          FAIL [fdo#105363] -> PASS

  * igt@kms_plane@plane-position-covered-pipe-a-planes:
    - shard-glk:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
    - shard-apl:          FAIL [fdo#103166] -> PASS +1

  
#### Warnings ####

  * igt@gem_exec_params@rel-constants-invalid-rel-gen5:
    - shard-snb:          INCOMPLETE [fdo#105411] / [fdo#107469] -> DMESG-WARN [fdo#107469]

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

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509
  [fdo#107409]: https://bugs.freedesktop.org/show_bug.cgi?id=107409
  [fdo#107469]: https://bugs.freedesktop.org/show_bug.cgi?id=107469
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (6 -> 4)
------------------------------

  Missing    (2): shard-skl shard-iclb 


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

    * Linux: CI_DRM_5521 -> Patchwork_12113

  CI_DRM_5521: dbd2e19079beb3b7f4077706179fba66d321e49f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4802: 4049adf01014af077df2174def4fadf7cecb066e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12113: b3ca9b19919023e1bda80d7e66a3f8f3009dbc83 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12113/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.IGT: success for restore WaEnableFloatBlendOptimization

[Chris Wilson]

Quoting Patchwork (2019-02-01 05:05:57)
> == Series Details ==
> 
> Series: restore WaEnableFloatBlendOptimization
> URL   : https://patchwork.freedesktop.org/series/56071/
> State : success
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_5521_full -> Patchwork_12113_full
> ====================================================
> 
> Summary
> -------
> 
>   **SUCCESS**
> 
>   No regressions found.

Which means either it failed completely (and had no effect) or it worked
and successfully hid itself from gem_workarounds.

Thanks for the patch, pushed!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx