[PATCH 1/2] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update

[PATCH 1/2] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update

[Mika Kuoppala]

Register DBUF_CTL_S2 is read and it's value is not used. As
there is no explanation why we should prime the hardware with
read, remove it as spurious.

Fixes: aa9664ffe863 ("drm/i915/icl: Enable 2nd DBuf slice only when needed")
Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index f945db6ea420..770de2632530 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3236,8 +3236,7 @@ static u8 intel_dbuf_max_slices(struct drm_i915_private *dev_priv)
 void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
 			    u8 req_slices)
 {
-	u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
-	u32 val;
+	const u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
 	bool ret;
 
 	if (req_slices > intel_dbuf_max_slices(dev_priv)) {
@@ -3248,7 +3247,6 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
 	if (req_slices == hw_enabled_slices || req_slices == 0)
 		return;
 
-	val = I915_READ(DBUF_CTL_S2);
 	if (req_slices > hw_enabled_slices)
 		ret = intel_dbuf_slice_set(dev_priv, DBUF_CTL_S2, true);
 	else
-- 
2.17.1

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

[PATCH 2/2] drm/i915: Request no slices if no active pipes

[Mika Kuoppala]

Skip the hardware dbuf slice update if we don't have active
pipes. With no active pipes, we don't have powerwell and thus
programming the dbuf slice counts leads to accessing
hardware without runtime pm ref.

Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c    | 32 +++++++++++++++----------
 drivers/gpu/drm/i915/intel_drv.h        |  3 +--
 drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++++--
 3 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 05125c7c2aa1..0514b89611ac 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12644,23 +12644,23 @@ static void skl_update_crtcs(struct drm_atomic_state *state)
 	struct intel_crtc *intel_crtc;
 	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	struct intel_crtc_state *cstate;
-	unsigned int updated = 0;
+	unsigned int updated = 0, active_count = 0;
+	u8 required_slices;
 	bool progress;
 	enum pipe pipe;
 	int i;
-	u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
-	u8 required_slices = intel_state->wm_results.ddb.enabled_slices;
-
 	const struct skl_ddb_entry *entries[I915_MAX_PIPES] = {};
 
-	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i)
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		/* ignore allocations for crtc's that have been turned off. */
-		if (new_crtc_state->active)
+		if (new_crtc_state->active) {
+			active_count++;
 			entries[i] = &to_intel_crtc_state(old_crtc_state)->wm.skl.ddb;
+		}
+	}
 
-	/* If 2nd DBuf slice required, enable it here */
-	if (INTEL_GEN(dev_priv) >= 11 && required_slices > hw_enabled_slices)
-		icl_dbuf_slices_update(dev_priv, required_slices);
+	required_slices = active_count ? intel_state->wm_results.ddb.enabled_slices : 0;
+	intel_dbuf_slices_update(dev_priv, required_slices);
 
 	/*
 	 * Whenever the number of active pipes changes, we need to make sure we
@@ -12670,6 +12670,7 @@ static void skl_update_crtcs(struct drm_atomic_state *state)
 	 */
 	do {
 		progress = false;
+		active_count = 0;
 
 		for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 			bool vbl_wait = false;
@@ -12679,7 +12680,12 @@ static void skl_update_crtcs(struct drm_atomic_state *state)
 			cstate = to_intel_crtc_state(new_crtc_state);
 			pipe = intel_crtc->pipe;
 
-			if (updated & cmask || !cstate->base.active)
+			if (!cstate->base.active)
+				continue;
+
+			active_count++;
+
+			if (updated & cmask)
 				continue;
 
 			if (skl_ddb_allocation_overlaps(dev_priv,
@@ -12713,9 +12719,9 @@ static void skl_update_crtcs(struct drm_atomic_state *state)
 		}
 	} while (progress);
 
-	/* If 2nd DBuf slice is no more required disable it */
-	if (INTEL_GEN(dev_priv) >= 11 && required_slices < hw_enabled_slices)
-		icl_dbuf_slices_update(dev_priv, required_slices);
+
+	required_slices = active_count ? intel_state->wm_results.ddb.enabled_slices : 0;
+	intel_dbuf_slices_update(dev_priv, required_slices);
 }
 
 static void intel_atomic_helper_free_state(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 21819a9bdcae..d643f8877097 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2086,8 +2086,7 @@ bool intel_display_power_get_if_enabled(struct drm_i915_private *dev_priv,
 					enum intel_display_power_domain domain);
 void intel_display_power_put(struct drm_i915_private *dev_priv,
 			     enum intel_display_power_domain domain);
-void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
-			    u8 req_slices);
+void intel_dbuf_slices_update(struct drm_i915_private *dev_priv, u8 req_slices);
 
 static inline void
 assert_rpm_device_not_suspended(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 770de2632530..3a271ac22fec 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3233,8 +3233,8 @@ static u8 intel_dbuf_max_slices(struct drm_i915_private *dev_priv)
 	return 2;
 }
 
-void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
-			    u8 req_slices)
+static void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
+				   const u8 req_slices)
 {
 	const u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
 	bool ret;
@@ -3256,6 +3256,14 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
 		dev_priv->wm.skl_hw.ddb.enabled_slices = req_slices;
 }
 
+void intel_dbuf_slices_update(struct drm_i915_private *dev_priv, u8 slices)
+{
+	if (INTEL_GEN(dev_priv) < 11)
+		return;
+
+	icl_dbuf_slices_update(dev_priv, slices);
+}
+
 static void icl_dbuf_enable(struct drm_i915_private *dev_priv)
 {
 	I915_WRITE(DBUF_CTL_S1, I915_READ(DBUF_CTL_S1) | DBUF_POWER_REQUEST);
-- 
2.17.1

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

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update
URL   : https://patchwork.freedesktop.org/series/52299/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
5c3d07354868 drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update
b25fc211da75 drm/i915: Request no slices if no active pipes
-:85: CHECK:LINE_SPACING: Please don't use multiple blank lines
#85: FILE: drivers/gpu/drm/i915/intel_display.c:12722:
 
+

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

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

[PATCH 2/2] drm/i915: Request no slices if no active pipes

[Mika Kuoppala]

Skip the hardware dbuf slice update if we don't have active
pipes. With no active pipes, we don't have powerwell and thus
programming the dbuf slice counts leads to accessing
hardware without runtime pm ref.

v2: bugzilla tag (Imre)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108654
Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c    | 32 +++++++++++++++----------
 drivers/gpu/drm/i915/intel_drv.h        |  3 +--
 drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++++--
 3 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 05125c7c2aa1..0514b89611ac 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12644,23 +12644,23 @@ static void skl_update_crtcs(struct drm_atomic_state *state)
 	struct intel_crtc *intel_crtc;
 	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
 	struct intel_crtc_state *cstate;
-	unsigned int updated = 0;
+	unsigned int updated = 0, active_count = 0;
+	u8 required_slices;
 	bool progress;
 	enum pipe pipe;
 	int i;
-	u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
-	u8 required_slices = intel_state->wm_results.ddb.enabled_slices;
-
 	const struct skl_ddb_entry *entries[I915_MAX_PIPES] = {};
 
-	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i)
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		/* ignore allocations for crtc's that have been turned off. */
-		if (new_crtc_state->active)
+		if (new_crtc_state->active) {
+			active_count++;
 			entries[i] = &to_intel_crtc_state(old_crtc_state)->wm.skl.ddb;
+		}
+	}
 
-	/* If 2nd DBuf slice required, enable it here */
-	if (INTEL_GEN(dev_priv) >= 11 && required_slices > hw_enabled_slices)
-		icl_dbuf_slices_update(dev_priv, required_slices);
+	required_slices = active_count ? intel_state->wm_results.ddb.enabled_slices : 0;
+	intel_dbuf_slices_update(dev_priv, required_slices);
 
 	/*
 	 * Whenever the number of active pipes changes, we need to make sure we
@@ -12670,6 +12670,7 @@ static void skl_update_crtcs(struct drm_atomic_state *state)
 	 */
 	do {
 		progress = false;
+		active_count = 0;
 
 		for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 			bool vbl_wait = false;
@@ -12679,7 +12680,12 @@ static void skl_update_crtcs(struct drm_atomic_state *state)
 			cstate = to_intel_crtc_state(new_crtc_state);
 			pipe = intel_crtc->pipe;
 
-			if (updated & cmask || !cstate->base.active)
+			if (!cstate->base.active)
+				continue;
+
+			active_count++;
+
+			if (updated & cmask)
 				continue;
 
 			if (skl_ddb_allocation_overlaps(dev_priv,
@@ -12713,9 +12719,9 @@ static void skl_update_crtcs(struct drm_atomic_state *state)
 		}
 	} while (progress);
 
-	/* If 2nd DBuf slice is no more required disable it */
-	if (INTEL_GEN(dev_priv) >= 11 && required_slices < hw_enabled_slices)
-		icl_dbuf_slices_update(dev_priv, required_slices);
+
+	required_slices = active_count ? intel_state->wm_results.ddb.enabled_slices : 0;
+	intel_dbuf_slices_update(dev_priv, required_slices);
 }
 
 static void intel_atomic_helper_free_state(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 21819a9bdcae..d643f8877097 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2086,8 +2086,7 @@ bool intel_display_power_get_if_enabled(struct drm_i915_private *dev_priv,
 					enum intel_display_power_domain domain);
 void intel_display_power_put(struct drm_i915_private *dev_priv,
 			     enum intel_display_power_domain domain);
-void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
-			    u8 req_slices);
+void intel_dbuf_slices_update(struct drm_i915_private *dev_priv, u8 req_slices);
 
 static inline void
 assert_rpm_device_not_suspended(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 770de2632530..3a271ac22fec 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3233,8 +3233,8 @@ static u8 intel_dbuf_max_slices(struct drm_i915_private *dev_priv)
 	return 2;
 }
 
-void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
-			    u8 req_slices)
+static void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
+				   const u8 req_slices)
 {
 	const u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
 	bool ret;
@@ -3256,6 +3256,14 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
 		dev_priv->wm.skl_hw.ddb.enabled_slices = req_slices;
 }
 
+void intel_dbuf_slices_update(struct drm_i915_private *dev_priv, u8 slices)
+{
+	if (INTEL_GEN(dev_priv) < 11)
+		return;
+
+	icl_dbuf_slices_update(dev_priv, slices);
+}
+
 static void icl_dbuf_enable(struct drm_i915_private *dev_priv)
 {
 	I915_WRITE(DBUF_CTL_S1, I915_READ(DBUF_CTL_S1) | DBUF_POWER_REQUEST);
-- 
2.17.1

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

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update
URL   : https://patchwork.freedesktop.org/series/52299/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5112 -> Patchwork_10791 =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10791 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10791, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@drv_selftest@live_guc:
      fi-apl-guc:         PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_execlists:
      fi-apl-guc:         PASS -> INCOMPLETE (fdo#103927, fdo#106693)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362)

    
    ==== Possible fixes ====

    igt@kms_frontbuffer_tracking@basic:
      fi-icl-u2:          FAIL (fdo#103167) -> PASS

    
    ==== Warnings ====

    igt@drv_selftest@live_contexts:
      fi-icl-u2:          INCOMPLETE (fdo#108315) -> DMESG-FAIL (fdo#108569)

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#106693 https://bugs.freedesktop.org/show_bug.cgi?id=106693
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#108315 https://bugs.freedesktop.org/show_bug.cgi?id=108315
  fdo#108569 https://bugs.freedesktop.org/show_bug.cgi?id=108569


== Participating hosts (53 -> 46) ==

  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-snb-2520m fi-ctg-p8600 


== Build changes ==

    * Linux: CI_DRM_5112 -> Patchwork_10791

  CI_DRM_5112: 9bb09c1a2b52d761b1dc212fe33641e19c9454cd @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10791: b25fc211da75a483a51a48e5e1309f9ad6e8f42f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

b25fc211da75 drm/i915: Request no slices if no active pipes
5c3d07354868 drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update

== Logs ==

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

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update (rev2)
URL   : https://patchwork.freedesktop.org/series/52299/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
1b65f68a1dad drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update
e4ba00d8d494 drm/i915: Request no slices if no active pipes
-:88: CHECK:LINE_SPACING: Please don't use multiple blank lines
#88: FILE: drivers/gpu/drm/i915/intel_display.c:12722:
 
+

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

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

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update (rev2)
URL   : https://patchwork.freedesktop.org/series/52299/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5113 -> Patchwork_10793 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/52299/revisions/2/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_module_reload@basic-no-display:
      fi-byt-clapper:     PASS -> WARN (fdo#108688)

    igt@gem_ctx_create@basic-files:
      fi-icl-u2:          PASS -> DMESG-WARN (fdo#107724)
      fi-bsw-kefka:       PASS -> FAIL (fdo#108656)

    igt@gem_exec_suspend@basic-s3:
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    igt@kms_frontbuffer_tracking@basic:
      fi-icl-u2:          PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362)

    
    ==== Possible fixes ====

    igt@gem_close_race@basic-threads:
      fi-bsw-kefka:       FAIL (fdo#108656) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
  fdo#108656 https://bugs.freedesktop.org/show_bug.cgi?id=108656
  fdo#108688 https://bugs.freedesktop.org/show_bug.cgi?id=108688


== Participating hosts (53 -> 46) ==

  Additional (1): fi-kbl-7560u 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-snb-2520m fi-ctg-p8600 


== Build changes ==

    * Linux: CI_DRM_5113 -> Patchwork_10793

  CI_DRM_5113: 54a159cbae565f40866b2a54edef1620ef8581f1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10793: e4ba00d8d494f8fd767f36cd496af438c13b6736 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e4ba00d8d494 drm/i915: Request no slices if no active pipes
1b65f68a1dad drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update

== Logs ==

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

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update (rev2)
URL   : https://patchwork.freedesktop.org/series/52299/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5113_full -> Patchwork_10793_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10793_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10793_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@perf_pmu@rc6:
      shard-kbl:          SKIP -> PASS

    igt@pm_rc6_residency@rc6-accuracy:
      shard-kbl:          PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drm_import_export@import-close-race-flink:
      shard-skl:          PASS -> TIMEOUT (fdo#108667)

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-skl:          NOTRUN -> TIMEOUT (fdo#108039)

    igt@gem_pwrite@display:
      shard-snb:          NOTRUN -> DMESG-WARN (fdo#107469) +1

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#107956) +1
      shard-kbl:          NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_color@pipe-c-ctm-blue-to-red:
      shard-skl:          PASS -> FAIL (fdo#107201)

    igt@kms_color@pipe-c-legacy-gamma:
      shard-apl:          PASS -> FAIL (fdo#104782)

    igt@kms_cursor_crc@cursor-128x128-random:
      shard-apl:          PASS -> FAIL (fdo#103232)

    igt@kms_cursor_crc@cursor-256x256-onscreen:
      shard-skl:          PASS -> FAIL (fdo#103232)

    igt@kms_cursor_crc@cursor-256x256-suspend:
      shard-skl:          PASS -> INCOMPLETE (fdo#104108, fdo#107773)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105363, fdo#102887)

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-skl:          PASS -> FAIL (fdo#105363)

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-skl:          NOTRUN -> FAIL (fdo#100368)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
      shard-glk:          PASS -> FAIL (fdo#103167)

    igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
      shard-skl:          NOTRUN -> FAIL (fdo#107815, fdo#108145)

    igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +1

    igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
      shard-skl:          PASS -> FAIL (fdo#107815)

    igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
      shard-glk:          PASS -> FAIL (fdo#103166)

    igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
      shard-apl:          PASS -> FAIL (fdo#103166) +1

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    igt@prime_busy@wait-hang-default:
      shard-snb:          NOTRUN -> INCOMPLETE (fdo#105411)

    
    ==== Possible fixes ====

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-kbl:          INCOMPLETE (fdo#106023, fdo#103665, fdo#106887) -> PASS

    igt@gem_softpin@noreloc-s3:
      shard-skl:          INCOMPLETE (fdo#104108, fdo#107773) -> PASS

    igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
      shard-glk:          DMESG-WARN (fdo#106538, fdo#105763) -> PASS

    igt@kms_flip@absolute-wf_vblank:
      shard-kbl:          DMESG-WARN (fdo#105345, fdo#103313) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
      shard-glk:          FAIL (fdo#103167) -> PASS

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-render:
      shard-glk:          DMESG-FAIL (fdo#106538) -> PASS

    igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
      shard-skl:          INCOMPLETE (fdo#104108) -> PASS

    igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
      shard-glk:          FAIL (fdo#103166) -> PASS

    igt@kms_setmode@basic:
      shard-hsw:          FAIL (fdo#99912) -> PASS

    igt@pm_rpm@dpms-mode-unset-lpsp:
      shard-skl:          INCOMPLETE (fdo#107807) -> PASS

    
    ==== Warnings ====

    igt@kms_vblank@pipe-a-query-busy:
      shard-snb:          INCOMPLETE (fdo#105411) -> DMESG-WARN (fdo#107469)

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
  fdo#105345 https://bugs.freedesktop.org/show_bug.cgi?id=105345
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106887 https://bugs.freedesktop.org/show_bug.cgi?id=106887
  fdo#107201 https://bugs.freedesktop.org/show_bug.cgi?id=107201
  fdo#107469 https://bugs.freedesktop.org/show_bug.cgi?id=107469
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108039 https://bugs.freedesktop.org/show_bug.cgi?id=108039
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108667 https://bugs.freedesktop.org/show_bug.cgi?id=108667
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_5113 -> Patchwork_10793

  CI_DRM_5113: 54a159cbae565f40866b2a54edef1620ef8581f1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10793: e4ba00d8d494f8fd767f36cd496af438c13b6736 @ 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_10793/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update

[Imre Deak]

On Fri, Nov 09, 2018 at 04:09:23PM +0200, Mika Kuoppala wrote:
> Register DBUF_CTL_S2 is read and it's value is not used. As
> there is no explanation why we should prime the hardware with
> read, remove it as spurious.
> 
> Fixes: aa9664ffe863 ("drm/i915/icl: Enable 2nd DBuf slice only when needed")
> Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index f945db6ea420..770de2632530 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -3236,8 +3236,7 @@ static u8 intel_dbuf_max_slices(struct drm_i915_private *dev_priv)
>  void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
>  			    u8 req_slices)
>  {
> -	u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
> -	u32 val;
> +	const u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
>  	bool ret;
>  
>  	if (req_slices > intel_dbuf_max_slices(dev_priv)) {
> @@ -3248,7 +3247,6 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
>  	if (req_slices == hw_enabled_slices || req_slices == 0)
>  		return;
>  
> -	val = I915_READ(DBUF_CTL_S2);
>  	if (req_slices > hw_enabled_slices)
>  		ret = intel_dbuf_slice_set(dev_priv, DBUF_CTL_S2, true);
>  	else
> -- 
> 2.17.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update

[Mika Kuoppala]

Imre Deak <imre.deak@intel.com> writes:

> On Fri, Nov 09, 2018 at 04:09:23PM +0200, Mika Kuoppala wrote:
>> Register DBUF_CTL_S2 is read and it's value is not used. As
>> there is no explanation why we should prime the hardware with
>> read, remove it as spurious.
>> 
>> Fixes: aa9664ffe863 ("drm/i915/icl: Enable 2nd DBuf slice only when needed")
>> Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Pushed, thanks for review.
-Mika

>
>> ---
>>  drivers/gpu/drm/i915/intel_runtime_pm.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> index f945db6ea420..770de2632530 100644
>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> @@ -3236,8 +3236,7 @@ static u8 intel_dbuf_max_slices(struct drm_i915_private *dev_priv)
>>  void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
>>  			    u8 req_slices)
>>  {
>> -	u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
>> -	u32 val;
>> +	const u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
>>  	bool ret;
>>  
>>  	if (req_slices > intel_dbuf_max_slices(dev_priv)) {
>> @@ -3248,7 +3247,6 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
>>  	if (req_slices == hw_enabled_slices || req_slices == 0)
>>  		return;
>>  
>> -	val = I915_READ(DBUF_CTL_S2);
>>  	if (req_slices > hw_enabled_slices)
>>  		ret = intel_dbuf_slice_set(dev_priv, DBUF_CTL_S2, true);
>>  	else
>> -- 
>> 2.17.1
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Request no slices if no active pipes

[Imre Deak]

On Fri, Nov 09, 2018 at 04:44:54PM +0200, Mika Kuoppala wrote:
> Skip the hardware dbuf slice update if we don't have active
> pipes. With no active pipes, we don't have powerwell and thus
> programming the dbuf slice counts leads to accessing
> hardware without runtime pm ref.
> 
> v2: bugzilla tag (Imre)
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108654
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c    | 32 +++++++++++++++----------
>  drivers/gpu/drm/i915/intel_drv.h        |  3 +--
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++++--
>  3 files changed, 30 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 05125c7c2aa1..0514b89611ac 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12644,23 +12644,23 @@ static void skl_update_crtcs(struct drm_atomic_state *state)
>  	struct intel_crtc *intel_crtc;
>  	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
>  	struct intel_crtc_state *cstate;
> -	unsigned int updated = 0;
> +	unsigned int updated = 0, active_count = 0;
> +	u8 required_slices;
>  	bool progress;
>  	enum pipe pipe;
>  	int i;
> -	u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
> -	u8 required_slices = intel_state->wm_results.ddb.enabled_slices;
> -
>  	const struct skl_ddb_entry *entries[I915_MAX_PIPES] = {};
>  
> -	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i)
> +	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
>  		/* ignore allocations for crtc's that have been turned off. */
> -		if (new_crtc_state->active)
> +		if (new_crtc_state->active) {
> +			active_count++;
>  			entries[i] = &to_intel_crtc_state(old_crtc_state)->wm.skl.ddb;
> +		}
> +	}
>  
> -	/* If 2nd DBuf slice required, enable it here */
> -	if (INTEL_GEN(dev_priv) >= 11 && required_slices > hw_enabled_slices)
> -		icl_dbuf_slices_update(dev_priv, required_slices);

Looks like this is the update that triggers the problem in the bugzilla
ticket. Wondering how this can happen since we have all outputs and
runtime suspended. Then by now icl_dbuf_disable() should have zeroed
dev_priv->wm.skl_hw.ddb.enabled_slices and skl_compute_ddb() copied this
0 to intel_state->wm_results.ddb.enabled_slices during the compute phase
of the problematic commit (since no crtcs are active this 0 should've
been kept as-is). Then here both hw_enabled_slices and required_slices
should be 0, but it's obviously not the case.

What could happen I think is that the store in
icl_dbuf_disable() / icl_dbuf_enable() can race with the load in
skl_compute_ddb(). So perhaps skl_compute_ddb() copied from dev_priv
enabled_slices as 1 or 2 (the only values we compute during a commit),
then icl_dbuf_disable() zeroed in a racy way enabled_slices in dev_priv
and we get here required_slices being 1 or 2 and hw_enabled_slices being
0.

So while the change in this patch gets rid of the symptom, I think we
should fix the root cause:

1. Do not update enabled_slices in icl_dbuf_enable() / icl_dbuf_disable()
   We instead depend on the DDB HW readout to set the correct initial value
   after module loading/resume and any following commit to update it if
   needed (based on new resolution etc.).

   After this we could still end up with stale values in
   wm.skl_hw.ddb.enabled_slices, for instance if it's 2 b/c of two pipes
   being enabled we wouldn't set it to 1 atm when disabling both pipes
   in the same atomic commit. So we'd also need the following 2 changes.

2. Force-enable only a single slice in icl_dbuf_enable() (as the spec
   requires) keeping the 2nd slice enabled only if BIOS has enabled it
   (DDB HW readout will set the correct enabled_slices afterwards).

3. Make sure we always compute the proper enabled_slices in the atomic
   state, by moving the calculation from intel_get_ddb_size() to
   earlier, performing it even if all crtcs are disabled.

> +	required_slices = active_count ? intel_state->wm_results.ddb.enabled_slices : 0;
> +	intel_dbuf_slices_update(dev_priv, required_slices);
>  
>  	/*
>  	 * Whenever the number of active pipes changes, we need to make sure we
> @@ -12670,6 +12670,7 @@ static void skl_update_crtcs(struct drm_atomic_state *state)
>  	 */
>  	do {
>  		progress = false;
> +		active_count = 0;
>  
>  		for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
>  			bool vbl_wait = false;
> @@ -12679,7 +12680,12 @@ static void skl_update_crtcs(struct drm_atomic_state *state)
>  			cstate = to_intel_crtc_state(new_crtc_state);
>  			pipe = intel_crtc->pipe;
>  
> -			if (updated & cmask || !cstate->base.active)
> +			if (!cstate->base.active)
> +				continue;
> +
> +			active_count++;
> +
> +			if (updated & cmask)
>  				continue;
>  
>  			if (skl_ddb_allocation_overlaps(dev_priv,
> @@ -12713,9 +12719,9 @@ static void skl_update_crtcs(struct drm_atomic_state *state)
>  		}
>  	} while (progress);
>  
> -	/* If 2nd DBuf slice is no more required disable it */
> -	if (INTEL_GEN(dev_priv) >= 11 && required_slices < hw_enabled_slices)
> -		icl_dbuf_slices_update(dev_priv, required_slices);
> +
> +	required_slices = active_count ? intel_state->wm_results.ddb.enabled_slices : 0;
> +	intel_dbuf_slices_update(dev_priv, required_slices);
>  }
>  
>  static void intel_atomic_helper_free_state(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 21819a9bdcae..d643f8877097 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2086,8 +2086,7 @@ bool intel_display_power_get_if_enabled(struct drm_i915_private *dev_priv,
>  					enum intel_display_power_domain domain);
>  void intel_display_power_put(struct drm_i915_private *dev_priv,
>  			     enum intel_display_power_domain domain);
> -void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
> -			    u8 req_slices);
> +void intel_dbuf_slices_update(struct drm_i915_private *dev_priv, u8 req_slices);
>  
>  static inline void
>  assert_rpm_device_not_suspended(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 770de2632530..3a271ac22fec 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -3233,8 +3233,8 @@ static u8 intel_dbuf_max_slices(struct drm_i915_private *dev_priv)
>  	return 2;
>  }
>  
> -void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
> -			    u8 req_slices)
> +static void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
> +				   const u8 req_slices)
>  {
>  	const u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
>  	bool ret;
> @@ -3256,6 +3256,14 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
>  		dev_priv->wm.skl_hw.ddb.enabled_slices = req_slices;
>  }
>  
> +void intel_dbuf_slices_update(struct drm_i915_private *dev_priv, u8 slices)
> +{
> +	if (INTEL_GEN(dev_priv) < 11)
> +		return;
> +
> +	icl_dbuf_slices_update(dev_priv, slices);
> +}
> +
>  static void icl_dbuf_enable(struct drm_i915_private *dev_priv)
>  {
>  	I915_WRITE(DBUF_CTL_S1, I915_READ(DBUF_CTL_S1) | DBUF_POWER_REQUEST);
> -- 
> 2.17.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx