* [PATCH] drm/i915: Simplify has_sagv
@ 2018-10-19 17:20 Rodrigo Vivi
2018-10-22 6:28 ` Jani Nikula
` (10 more replies)
0 siblings, 11 replies; 23+ messages in thread
From: Rodrigo Vivi @ 2018-10-19 17:20 UTC (permalink / raw)
To: intel-gfx; +Cc: Jani Nikula, Lucas De Marchi, Rodrigo Vivi
Let's add a platform has_sagv instead of having a full
function that handle platform by platform.
The specially case for SKL for not controlled sagv
is already taken care inside intel_enable_sagv, so there's
no need to duplicate the check here.
v2: Go one step further and remove skl special case. (Jani)
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_pci.c | 1 +
drivers/gpu/drm/i915/intel_device_info.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 20 +++-----------------
4 files changed, 7 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3017ef037fed..8eab6bdff8a4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2650,6 +2650,8 @@ intel_info(const struct drm_i915_private *dev_priv)
#define HAS_DDI(dev_priv) ((dev_priv)->info.has_ddi)
#define HAS_FPGA_DBG_UNCLAIMED(dev_priv) ((dev_priv)->info.has_fpga_dbg)
#define HAS_PSR(dev_priv) ((dev_priv)->info.has_psr)
+#define HAS_SAGV(dev_priv) ((dev_priv)->info.has_sagv && \
+ dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
#define HAS_RC6(dev_priv) ((dev_priv)->info.has_rc6)
#define HAS_RC6p(dev_priv) ((dev_priv)->info.has_rc6p)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 0a05cc7ace14..21ca9917b86e 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -465,6 +465,7 @@ static const struct intel_device_info intel_cherryview_info = {
.has_csr = 1, \
.has_guc = 1, \
.has_ipc = 1, \
+ .has_sagv = 1, \
.ddb_size = 896
#define SKL_PLATFORM \
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index af7002640cdf..e77c8b62783f 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -117,6 +117,7 @@ enum intel_ppgtt {
func(hws_needs_physical); \
func(overlay_needs_physical); \
func(supports_tv); \
+ func(has_sagv); \
func(has_ipc);
#define GEN_MAX_SLICES (6) /* CNL upper bound */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 67a4d0735291..09c21f6151fd 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3609,20 +3609,6 @@ static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
return false;
}
-static bool
-intel_has_sagv(struct drm_i915_private *dev_priv)
-{
- if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
- IS_CANNONLAKE(dev_priv) || IS_ICELAKE(dev_priv))
- return true;
-
- if (IS_SKYLAKE(dev_priv) &&
- dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
- return true;
-
- return false;
-}
-
/*
* SAGV dynamically adjusts the system agent voltage and clock frequencies
* depending on power and performance requirements. The display engine access
@@ -3639,7 +3625,7 @@ intel_enable_sagv(struct drm_i915_private *dev_priv)
{
int ret;
- if (!intel_has_sagv(dev_priv))
+ if (!HAS_SAGV(dev_priv))
return 0;
if (dev_priv->sagv_status == I915_SAGV_ENABLED)
@@ -3676,7 +3662,7 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
{
int ret;
- if (!intel_has_sagv(dev_priv))
+ if (!HAS_SAGV(dev_priv))
return 0;
if (dev_priv->sagv_status == I915_SAGV_DISABLED)
@@ -3721,7 +3707,7 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
int level, latency;
int sagv_block_time_us;
- if (!intel_has_sagv(dev_priv))
+ if (!HAS_SAGV(dev_priv))
return false;
if (IS_GEN9(dev_priv))
--
2.19.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Simplify has_sagv
2018-10-19 17:20 [PATCH] drm/i915: Simplify has_sagv Rodrigo Vivi
@ 2018-10-22 6:28 ` Jani Nikula
2018-10-22 16:57 ` Rodrigo Vivi
2018-10-22 13:13 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Simplify has_sagv Patchwork
` (9 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2018-10-22 6:28 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, Rodrigo Vivi
On Fri, 19 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> Let's add a platform has_sagv instead of having a full
> function that handle platform by platform.
>
> The specially case for SKL for not controlled sagv
> is already taken care inside intel_enable_sagv, so there's
> no need to duplicate the check here.
>
> v2: Go one step further and remove skl special case. (Jani)
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/i915_pci.c | 1 +
> drivers/gpu/drm/i915/intel_device_info.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 20 +++-----------------
> 4 files changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3017ef037fed..8eab6bdff8a4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2650,6 +2650,8 @@ intel_info(const struct drm_i915_private *dev_priv)
> #define HAS_DDI(dev_priv) ((dev_priv)->info.has_ddi)
> #define HAS_FPGA_DBG_UNCLAIMED(dev_priv) ((dev_priv)->info.has_fpga_dbg)
> #define HAS_PSR(dev_priv) ((dev_priv)->info.has_psr)
> +#define HAS_SAGV(dev_priv) ((dev_priv)->info.has_sagv && \
> + dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
Grumble. Somehow it feels wrong to tie the HAS_* macros to some runtime
checks.
Anyway, dev_priv should be wrapped in parens.
BR,
Jani.
>
> #define HAS_RC6(dev_priv) ((dev_priv)->info.has_rc6)
> #define HAS_RC6p(dev_priv) ((dev_priv)->info.has_rc6p)
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 0a05cc7ace14..21ca9917b86e 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -465,6 +465,7 @@ static const struct intel_device_info intel_cherryview_info = {
> .has_csr = 1, \
> .has_guc = 1, \
> .has_ipc = 1, \
> + .has_sagv = 1, \
> .ddb_size = 896
>
> #define SKL_PLATFORM \
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index af7002640cdf..e77c8b62783f 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -117,6 +117,7 @@ enum intel_ppgtt {
> func(hws_needs_physical); \
> func(overlay_needs_physical); \
> func(supports_tv); \
> + func(has_sagv); \
> func(has_ipc);
>
> #define GEN_MAX_SLICES (6) /* CNL upper bound */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 67a4d0735291..09c21f6151fd 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3609,20 +3609,6 @@ static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
> return false;
> }
>
> -static bool
> -intel_has_sagv(struct drm_i915_private *dev_priv)
> -{
> - if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> - IS_CANNONLAKE(dev_priv) || IS_ICELAKE(dev_priv))
> - return true;
> -
> - if (IS_SKYLAKE(dev_priv) &&
> - dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
> - return true;
> -
> - return false;
> -}
> -
> /*
> * SAGV dynamically adjusts the system agent voltage and clock frequencies
> * depending on power and performance requirements. The display engine access
> @@ -3639,7 +3625,7 @@ intel_enable_sagv(struct drm_i915_private *dev_priv)
> {
> int ret;
>
> - if (!intel_has_sagv(dev_priv))
> + if (!HAS_SAGV(dev_priv))
> return 0;
>
> if (dev_priv->sagv_status == I915_SAGV_ENABLED)
> @@ -3676,7 +3662,7 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
> {
> int ret;
>
> - if (!intel_has_sagv(dev_priv))
> + if (!HAS_SAGV(dev_priv))
> return 0;
>
> if (dev_priv->sagv_status == I915_SAGV_DISABLED)
> @@ -3721,7 +3707,7 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
> int level, latency;
> int sagv_block_time_us;
>
> - if (!intel_has_sagv(dev_priv))
> + if (!HAS_SAGV(dev_priv))
> return false;
>
> if (IS_GEN9(dev_priv))
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Simplify has_sagv
2018-10-19 17:20 [PATCH] drm/i915: Simplify has_sagv Rodrigo Vivi
2018-10-22 6:28 ` Jani Nikula
@ 2018-10-22 13:13 ` Patchwork
2018-10-22 13:14 ` ✗ Fi.CI.SPARSE: " Patchwork
` (8 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-10-22 13:13 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Simplify has_sagv
URL : https://patchwork.freedesktop.org/series/51266/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
8a60019e49e8 drm/i915: Simplify has_sagv
-:27: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'dev_priv' - possible side-effects?
#27: FILE: drivers/gpu/drm/i915/i915_drv.h:2653:
+#define HAS_SAGV(dev_priv) ((dev_priv)->info.has_sagv && \
+ dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
total: 0 errors, 0 warnings, 1 checks, 66 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* ✗ Fi.CI.SPARSE: warning for drm/i915: Simplify has_sagv
2018-10-19 17:20 [PATCH] drm/i915: Simplify has_sagv Rodrigo Vivi
2018-10-22 6:28 ` Jani Nikula
2018-10-22 13:13 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Simplify has_sagv Patchwork
@ 2018-10-22 13:14 ` Patchwork
2018-10-22 13:37 ` ✓ Fi.CI.BAT: success " Patchwork
` (7 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-10-22 13:14 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Simplify has_sagv
URL : https://patchwork.freedesktop.org/series/51266/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Simplify has_sagv
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3725:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3727:16: warning: expression using sizeof(void)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Simplify has_sagv
2018-10-19 17:20 [PATCH] drm/i915: Simplify has_sagv Rodrigo Vivi
` (2 preceding siblings ...)
2018-10-22 13:14 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2018-10-22 13:37 ` Patchwork
2018-10-22 16:46 ` ✓ Fi.CI.IGT: " Patchwork
` (6 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-10-22 13:37 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Simplify has_sagv
URL : https://patchwork.freedesktop.org/series/51266/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5017 -> Patchwork_10517 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/51266/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_10517 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_exec_suspend@basic:
fi-icl-u: NOTRUN -> DMESG-WARN (fdo#107724) +25
igt@gem_exec_suspend@basic-s3:
fi-kbl-soraka: NOTRUN -> INCOMPLETE (fdo#107556, fdo#107774, fdo#107859)
fi-icl-u: NOTRUN -> DMESG-WARN (fdo#108512)
igt@kms_flip@basic-flip-vs-dpms:
fi-hsw-4770r: PASS -> DMESG-WARN (fdo#105602)
==== Possible fixes ====
igt@gem_exec_suspend@basic-s4-devices:
fi-kbl-7500u: DMESG-WARN (fdo#107139, fdo#105128) -> PASS
igt@kms_frontbuffer_tracking@basic:
fi-byt-clapper: FAIL (fdo#103167) -> PASS
igt@prime_vgem@basic-fence-flip:
fi-cfl-8700k: FAIL (fdo#104008) -> PASS
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
fdo#107774 https://bugs.freedesktop.org/show_bug.cgi?id=107774
fdo#107859 https://bugs.freedesktop.org/show_bug.cgi?id=107859
fdo#108512 https://bugs.freedesktop.org/show_bug.cgi?id=108512
== Participating hosts (51 -> 47) ==
Additional (2): fi-kbl-soraka fi-icl-u
Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-icl-u2 fi-bsw-cyan fi-ctg-p8600
== Build changes ==
* Linux: CI_DRM_5017 -> Patchwork_10517
CI_DRM_5017: 9510f8e44127260f92b5b6c3127aafa22b15f741 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4685: 78619fde4008424c472906041edb1d204e014f7c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10517: 8a60019e49e8cc1b8a0ec2a249f44bb41590c6cb @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
8a60019e49e8 drm/i915: Simplify has_sagv
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10517/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: Simplify has_sagv
2018-10-19 17:20 [PATCH] drm/i915: Simplify has_sagv Rodrigo Vivi
` (3 preceding siblings ...)
2018-10-22 13:37 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-10-22 16:46 ` Patchwork
2018-10-22 17:20 ` ✗ Fi.CI.SPARSE: warning for drm/i915: Simplify has_sagv (rev2) Patchwork
` (5 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-10-22 16:46 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Simplify has_sagv
URL : https://patchwork.freedesktop.org/series/51266/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5017_full -> Patchwork_10517_full =
== Summary - SUCCESS ==
No regressions found.
== Known issues ==
Here are the changes found in Patchwork_10517_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_ppgtt@blt-vs-render-ctx0:
shard-kbl: PASS -> INCOMPLETE (fdo#103665, fdo#106023)
igt@gem_softpin@noreloc-s3:
shard-kbl: PASS -> DMESG-WARN (fdo#103313)
igt@gem_userptr_blits@readonly-unsync:
shard-skl: NOTRUN -> INCOMPLETE (fdo#108074)
igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
shard-skl: NOTRUN -> DMESG-WARN (fdo#107956)
igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
shard-glk: PASS -> FAIL (fdo#108145) +1
igt@kms_cursor_crc@cursor-128x128-suspend:
shard-skl: PASS -> INCOMPLETE (fdo#104108) +1
igt@kms_cursor_crc@cursor-64x21-onscreen:
shard-apl: PASS -> FAIL (fdo#103232)
igt@kms_cursor_crc@cursor-64x64-suspend:
shard-glk: PASS -> FAIL (fdo#103232)
igt@kms_flip@plain-flip-ts-check-interruptible:
shard-skl: PASS -> FAIL (fdo#100368)
igt@kms_flip_tiling@flip-yf-tiled:
shard-skl: NOTRUN -> FAIL (fdo#108145) +1
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
shard-apl: PASS -> FAIL (fdo#103167) +1
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-pwrite:
shard-apl: SKIP -> INCOMPLETE (fdo#103927)
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
shard-glk: PASS -> FAIL (fdo#103167) +2
igt@kms_plane@plane-position-covered-pipe-a-planes:
shard-glk: PASS -> FAIL (fdo#103166)
igt@kms_plane_alpha_blend@pipe-c-alpha-basic:
shard-skl: NOTRUN -> FAIL (fdo#107815, fdo#108145)
igt@pm_rpm@dpms-lpsp:
shard-skl: PASS -> INCOMPLETE (fdo#107807)
igt@pm_rpm@system-suspend:
shard-skl: PASS -> INCOMPLETE (fdo#104108, fdo#107807)
==== Possible fixes ====
igt@kms_color@pipe-a-legacy-gamma:
shard-skl: FAIL (fdo#108145, fdo#104782) -> PASS
igt@kms_cursor_crc@cursor-64x21-onscreen:
shard-glk: FAIL (fdo#103232) -> PASS
igt@kms_draw_crc@draw-method-rgb565-pwrite-xtiled:
shard-skl: FAIL (fdo#103184) -> PASS
igt@kms_flip@flip-vs-expired-vblank:
shard-skl: FAIL (fdo#105363) -> PASS
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-apl: FAIL (fdo#102887, fdo#105363) -> PASS
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
shard-apl: FAIL (fdo#103167) -> PASS
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
shard-glk: FAIL (fdo#103167) -> PASS +2
igt@kms_plane@plane-position-covered-pipe-a-planes:
shard-apl: FAIL (fdo#103166) -> PASS +2
igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
shard-glk: FAIL (fdo#108145) -> PASS
igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
shard-glk: FAIL (fdo#103166) -> PASS
igt@kms_vblank@pipe-b-ts-continuation-dpms-rpm:
shard-glk: DMESG-WARN (fdo#106538, fdo#105763) -> PASS +1
igt@pm_rps@reset:
shard-apl: FAIL (fdo#102250) -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102250 https://bugs.freedesktop.org/show_bug.cgi?id=102250
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#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
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#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
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#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#108074 https://bugs.freedesktop.org/show_bug.cgi?id=108074
fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
== Participating hosts (6 -> 6) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_5017 -> Patchwork_10517
CI_DRM_5017: 9510f8e44127260f92b5b6c3127aafa22b15f741 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4685: 78619fde4008424c472906041edb1d204e014f7c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10517: 8a60019e49e8cc1b8a0ec2a249f44bb41590c6cb @ 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_10517/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] drm/i915: Simplify has_sagv
2018-10-22 6:28 ` Jani Nikula
@ 2018-10-22 16:57 ` Rodrigo Vivi
2018-10-22 23:48 ` Paulo Zanoni
2018-10-23 7:23 ` Jani Nikula
0 siblings, 2 replies; 23+ messages in thread
From: Rodrigo Vivi @ 2018-10-22 16:57 UTC (permalink / raw)
To: intel-gfx; +Cc: Jani Nikula, Lucas De Marchi, Rodrigo Vivi
Let's add a platform has_sagv instead of having a full
function that handle platform by platform.
The specially case for SKL for not controlled sagv
is already taken care inside intel_enable_sagv, so there's
no need to duplicate the check here.
v2: Go one step further and remove skl special case. (Jani)
v3: Separate runtime status handle from has_sagv flag. (Jani)
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_pci.c | 1 +
drivers/gpu/drm/i915/intel_device_info.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 29 ++++++++----------------
4 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3017ef037fed..9b98ceb2d029 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2650,6 +2650,7 @@ intel_info(const struct drm_i915_private *dev_priv)
#define HAS_DDI(dev_priv) ((dev_priv)->info.has_ddi)
#define HAS_FPGA_DBG_UNCLAIMED(dev_priv) ((dev_priv)->info.has_fpga_dbg)
#define HAS_PSR(dev_priv) ((dev_priv)->info.has_psr)
+#define HAS_SAGV(dev_priv) ((dev_priv)->info.has_sagv)
#define HAS_RC6(dev_priv) ((dev_priv)->info.has_rc6)
#define HAS_RC6p(dev_priv) ((dev_priv)->info.has_rc6p)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 44e745921ac1..0b09155eab62 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -465,6 +465,7 @@ static const struct intel_device_info intel_cherryview_info = {
.has_csr = 1, \
.has_guc = 1, \
.has_ipc = 1, \
+ .has_sagv = 1, \
.ddb_size = 896
#define SKL_PLATFORM \
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index af7002640cdf..e77c8b62783f 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -117,6 +117,7 @@ enum intel_ppgtt {
func(hws_needs_physical); \
func(overlay_needs_physical); \
func(supports_tv); \
+ func(has_sagv); \
func(has_ipc);
#define GEN_MAX_SLICES (6) /* CNL upper bound */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 67a4d0735291..7e38ed8421c7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3609,20 +3609,6 @@ static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
return false;
}
-static bool
-intel_has_sagv(struct drm_i915_private *dev_priv)
-{
- if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
- IS_CANNONLAKE(dev_priv) || IS_ICELAKE(dev_priv))
- return true;
-
- if (IS_SKYLAKE(dev_priv) &&
- dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
- return true;
-
- return false;
-}
-
/*
* SAGV dynamically adjusts the system agent voltage and clock frequencies
* depending on power and performance requirements. The display engine access
@@ -3639,10 +3625,11 @@ intel_enable_sagv(struct drm_i915_private *dev_priv)
{
int ret;
- if (!intel_has_sagv(dev_priv))
+ if (!HAS_SAGV(dev_priv))
return 0;
- if (dev_priv->sagv_status == I915_SAGV_ENABLED)
+ if (dev_priv->sagv_status == I915_SAGV_ENABLED ||
+ dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
return 0;
DRM_DEBUG_KMS("Enabling the SAGV\n");
@@ -3676,10 +3663,11 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
{
int ret;
- if (!intel_has_sagv(dev_priv))
+ if (!HAS_SAGV(dev_priv))
return 0;
- if (dev_priv->sagv_status == I915_SAGV_DISABLED)
+ if (dev_priv->sagv_status == I915_SAGV_DISABLED ||
+ dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
return 0;
DRM_DEBUG_KMS("Disabling the SAGV\n");
@@ -3721,7 +3709,10 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
int level, latency;
int sagv_block_time_us;
- if (!intel_has_sagv(dev_priv))
+ if (!HAS_SAGV(dev_priv))
+ return false;
+
+ if (dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
return false;
if (IS_GEN9(dev_priv))
--
2.19.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* ✗ Fi.CI.SPARSE: warning for drm/i915: Simplify has_sagv (rev2)
2018-10-19 17:20 [PATCH] drm/i915: Simplify has_sagv Rodrigo Vivi
` (4 preceding siblings ...)
2018-10-22 16:46 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-10-22 17:20 ` Patchwork
2018-10-22 17:37 ` ✓ Fi.CI.BAT: success " Patchwork
` (4 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-10-22 17:20 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Simplify has_sagv (rev2)
URL : https://patchwork.freedesktop.org/series/51266/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Simplify has_sagv
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3725:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3726:16: warning: expression using sizeof(void)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Simplify has_sagv (rev2)
2018-10-19 17:20 [PATCH] drm/i915: Simplify has_sagv Rodrigo Vivi
` (5 preceding siblings ...)
2018-10-22 17:20 ` ✗ Fi.CI.SPARSE: warning for drm/i915: Simplify has_sagv (rev2) Patchwork
@ 2018-10-22 17:37 ` Patchwork
2018-10-22 20:42 ` ✗ Fi.CI.IGT: failure " Patchwork
` (3 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-10-22 17:37 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Simplify has_sagv (rev2)
URL : https://patchwork.freedesktop.org/series/51266/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5017 -> Patchwork_10528 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/51266/revisions/2/mbox/
== Known issues ==
Here are the changes found in Patchwork_10528 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_flip@basic-flip-vs-modeset:
fi-skl-6700hq: PASS -> DMESG-WARN (fdo#105998)
igt@kms_pipe_crc_basic@read-crc-pipe-b:
fi-byt-clapper: PASS -> FAIL (fdo#107362)
==== Possible fixes ====
igt@kms_frontbuffer_tracking@basic:
fi-byt-clapper: FAIL (fdo#103167) -> PASS
igt@prime_vgem@basic-fence-flip:
fi-cfl-8700k: FAIL (fdo#104008) -> PASS
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
== Participating hosts (51 -> 45) ==
Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-icl-u2 fi-bsw-cyan fi-ctg-p8600
== Build changes ==
* Linux: CI_DRM_5017 -> Patchwork_10528
CI_DRM_5017: 9510f8e44127260f92b5b6c3127aafa22b15f741 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4685: 78619fde4008424c472906041edb1d204e014f7c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10528: 68f538bbb8610161409ce0ccd51a85fb064ed4ae @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
68f538bbb861 drm/i915: Simplify has_sagv
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10528/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* ✗ Fi.CI.IGT: failure for drm/i915: Simplify has_sagv (rev2)
2018-10-19 17:20 [PATCH] drm/i915: Simplify has_sagv Rodrigo Vivi
` (6 preceding siblings ...)
2018-10-22 17:37 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-10-22 20:42 ` Patchwork
2018-10-26 21:21 ` ✗ Fi.CI.BAT: failure for drm/i915: Simplify has_sagv (rev3) Patchwork
` (2 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-10-22 20:42 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Simplify has_sagv (rev2)
URL : https://patchwork.freedesktop.org/series/51266/
State : failure
== Summary ==
= CI Bug Log - changes from CI_DRM_5017_full -> Patchwork_10528_full =
== Summary - FAILURE ==
Serious unknown changes coming with Patchwork_10528_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10528_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_10528_full:
=== IGT changes ===
==== Possible regressions ====
igt@pm_rps@reset:
shard-skl: PASS -> FAIL
== Known issues ==
Here are the changes found in Patchwork_10528_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_ppgtt@blt-vs-render-ctx0:
shard-kbl: PASS -> INCOMPLETE (fdo#103665, fdo#106023)
igt@gem_userptr_blits@readonly-unsync:
shard-skl: NOTRUN -> INCOMPLETE (fdo#108074)
igt@gem_workarounds@suspend-resume:
shard-kbl: PASS -> INCOMPLETE (fdo#103665)
igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
shard-skl: NOTRUN -> DMESG-WARN (fdo#107956)
igt@kms_cursor_crc@cursor-128x128-onscreen:
shard-apl: PASS -> FAIL (fdo#103232) +2
igt@kms_cursor_crc@cursor-256x256-sliding:
shard-glk: PASS -> FAIL (fdo#103232) +2
igt@kms_draw_crc@draw-method-xrgb2101010-render-untiled:
shard-skl: PASS -> FAIL (fdo#103184)
igt@kms_flip_tiling@flip-yf-tiled:
shard-skl: NOTRUN -> FAIL (fdo#108145) +1
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
shard-apl: PASS -> FAIL (fdo#103167) +1
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
shard-glk: PASS -> FAIL (fdo#103167) +1
igt@kms_plane@plane-position-covered-pipe-b-planes:
shard-apl: PASS -> FAIL (fdo#103166)
igt@kms_plane_alpha_blend@pipe-b-alpha-7efc:
shard-skl: NOTRUN -> FAIL (fdo#108146)
igt@kms_plane_alpha_blend@pipe-c-alpha-basic:
shard-skl: NOTRUN -> FAIL (fdo#108145, fdo#107815)
igt@pm_rpm@pm-caching:
shard-skl: PASS -> INCOMPLETE (fdo#107807)
igt@syncobj_wait@wait-for-submit-complex:
shard-skl: NOTRUN -> INCOMPLETE (fdo#108490)
==== Possible fixes ====
igt@gem_cpu_reloc@full:
shard-skl: INCOMPLETE (fdo#108073) -> PASS
igt@gem_ctx_isolation@bcs0-s3:
shard-kbl: INCOMPLETE (fdo#103665) -> PASS
igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
shard-kbl: DMESG-WARN (fdo#107956) -> PASS
igt@kms_color@pipe-a-legacy-gamma:
shard-skl: FAIL (fdo#104782, fdo#108145) -> PASS
igt@kms_cursor_crc@cursor-64x21-onscreen:
shard-glk: FAIL (fdo#103232) -> PASS
igt@kms_draw_crc@draw-method-rgb565-pwrite-xtiled:
shard-skl: FAIL (fdo#103184) -> PASS
igt@kms_flip@flip-vs-expired-vblank:
shard-skl: FAIL (fdo#105363) -> PASS
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-apl: FAIL (fdo#102887, fdo#105363) -> PASS
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-gtt:
shard-skl: FAIL (fdo#105682) -> PASS
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
shard-apl: FAIL (fdo#103167) -> PASS +1
igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-render:
shard-skl: FAIL (fdo#103167) -> PASS +2
igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
shard-skl: FAIL (fdo#103166) -> PASS
igt@kms_plane@plane-position-covered-pipe-a-planes:
shard-apl: FAIL (fdo#103166) -> PASS +2
igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
shard-skl: FAIL (fdo#108145, fdo#107815) -> PASS
igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
shard-glk: FAIL (fdo#108145) -> PASS
igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
shard-glk: FAIL (fdo#103166) -> PASS
igt@kms_setmode@basic:
shard-apl: FAIL (fdo#99912) -> PASS
shard-kbl: FAIL (fdo#99912) -> PASS
igt@kms_vblank@pipe-b-ts-continuation-dpms-rpm:
shard-glk: DMESG-WARN (fdo#106538, fdo#105763) -> PASS +1
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#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
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#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#108073 https://bugs.freedesktop.org/show_bug.cgi?id=108073
fdo#108074 https://bugs.freedesktop.org/show_bug.cgi?id=108074
fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
fdo#108146 https://bugs.freedesktop.org/show_bug.cgi?id=108146
fdo#108490 https://bugs.freedesktop.org/show_bug.cgi?id=108490
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_5017 -> Patchwork_10528
CI_DRM_5017: 9510f8e44127260f92b5b6c3127aafa22b15f741 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4685: 78619fde4008424c472906041edb1d204e014f7c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10528: 68f538bbb8610161409ce0ccd51a85fb064ed4ae @ 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_10528/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Simplify has_sagv
2018-10-22 16:57 ` Rodrigo Vivi
@ 2018-10-22 23:48 ` Paulo Zanoni
2018-10-23 0:06 ` Rodrigo Vivi
2018-10-23 7:23 ` Jani Nikula
1 sibling, 1 reply; 23+ messages in thread
From: Paulo Zanoni @ 2018-10-22 23:48 UTC (permalink / raw)
To: Rodrigo Vivi, intel-gfx; +Cc: Jani Nikula, Lucas De Marchi
Em Seg, 2018-10-22 às 09:57 -0700, Rodrigo Vivi escreveu:
> Let's add a platform has_sagv instead of having a full
> function that handle platform by platform.
>
> The specially case for SKL for not controlled sagv
> is already taken care inside intel_enable_sagv, so there's
> no need to duplicate the check here.
>
> v2: Go one step further and remove skl special case. (Jani)
>
> v3: Separate runtime status handle from has_sagv flag. (Jani)
I know this has probably been discussed in the past and I missed it
(since I know this was done a few times for other things), but how/why
is this approach better than the current one? Can you please write it
in the commit message?
Quickly checking the implementation of intel_has_sagv() seems much
easier than swimming through the nested macros of i915_pci.c. And the
special-caseness of the NOT_CONTROLLED case being restricted to gen9 in
the current (non-patched) code also at first appears to be better than
this proposal. But I'm totally willing to read your arguments and
reevaluate my decisions based on them, so please present them.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_pci.c | 1 +
> drivers/gpu/drm/i915/intel_device_info.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 29 ++++++++------------
> ----
> 4 files changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 3017ef037fed..9b98ceb2d029 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2650,6 +2650,7 @@ intel_info(const struct drm_i915_private
> *dev_priv)
> #define HAS_DDI(dev_priv) ((dev_priv)->info.has_ddi)
> #define HAS_FPGA_DBG_UNCLAIMED(dev_priv) ((dev_priv)-
> >info.has_fpga_dbg)
> #define HAS_PSR(dev_priv) ((dev_priv)->info.has_psr)
> +#define HAS_SAGV(dev_priv) ((dev_priv)-
> >info.has_sagv)
>
> #define HAS_RC6(dev_priv) ((dev_priv)->info.has_rc6)
> #define HAS_RC6p(dev_priv) ((dev_priv)-
> >info.has_rc6p)
> diff --git a/drivers/gpu/drm/i915/i915_pci.c
> b/drivers/gpu/drm/i915/i915_pci.c
> index 44e745921ac1..0b09155eab62 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -465,6 +465,7 @@ static const struct intel_device_info
> intel_cherryview_info = {
> .has_csr = 1, \
> .has_guc = 1, \
> .has_ipc = 1, \
> + .has_sagv = 1, \
> .ddb_size = 896
>
> #define SKL_PLATFORM \
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h
> b/drivers/gpu/drm/i915/intel_device_info.h
> index af7002640cdf..e77c8b62783f 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -117,6 +117,7 @@ enum intel_ppgtt {
> func(hws_needs_physical); \
> func(overlay_needs_physical); \
> func(supports_tv); \
> + func(has_sagv); \
> func(has_ipc);
>
> #define GEN_MAX_SLICES (6) /* CNL upper bound */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 67a4d0735291..7e38ed8421c7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3609,20 +3609,6 @@ static bool skl_needs_memory_bw_wa(struct
> intel_atomic_state *state)
> return false;
> }
>
> -static bool
> -intel_has_sagv(struct drm_i915_private *dev_priv)
> -{
> - if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> - IS_CANNONLAKE(dev_priv) || IS_ICELAKE(dev_priv))
> - return true;
> -
> - if (IS_SKYLAKE(dev_priv) &&
> - dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
> - return true;
> -
> - return false;
> -}
> -
> /*
> * SAGV dynamically adjusts the system agent voltage and clock
> frequencies
> * depending on power and performance requirements. The display
> engine access
> @@ -3639,10 +3625,11 @@ intel_enable_sagv(struct drm_i915_private
> *dev_priv)
> {
> int ret;
>
> - if (!intel_has_sagv(dev_priv))
> + if (!HAS_SAGV(dev_priv))
> return 0;
>
> - if (dev_priv->sagv_status == I915_SAGV_ENABLED)
> + if (dev_priv->sagv_status == I915_SAGV_ENABLED ||
> + dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
> return 0;
>
> DRM_DEBUG_KMS("Enabling the SAGV\n");
> @@ -3676,10 +3663,11 @@ intel_disable_sagv(struct drm_i915_private
> *dev_priv)
> {
> int ret;
>
> - if (!intel_has_sagv(dev_priv))
> + if (!HAS_SAGV(dev_priv))
> return 0;
>
> - if (dev_priv->sagv_status == I915_SAGV_DISABLED)
> + if (dev_priv->sagv_status == I915_SAGV_DISABLED ||
> + dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
> return 0;
>
> DRM_DEBUG_KMS("Disabling the SAGV\n");
> @@ -3721,7 +3709,10 @@ bool intel_can_enable_sagv(struct
> drm_atomic_state *state)
> int level, latency;
> int sagv_block_time_us;
>
> - if (!intel_has_sagv(dev_priv))
> + if (!HAS_SAGV(dev_priv))
> + return false;
> +
> + if (dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
> return false;
>
> if (IS_GEN9(dev_priv))
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Simplify has_sagv
2018-10-22 23:48 ` Paulo Zanoni
@ 2018-10-23 0:06 ` Rodrigo Vivi
2018-10-23 0:32 ` Paulo Zanoni
0 siblings, 1 reply; 23+ messages in thread
From: Rodrigo Vivi @ 2018-10-23 0:06 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Jani Nikula, intel-gfx, Lucas De Marchi
On Mon, Oct 22, 2018 at 04:48:50PM -0700, Paulo Zanoni wrote:
> Em Seg, 2018-10-22 às 09:57 -0700, Rodrigo Vivi escreveu:
> > Let's add a platform has_sagv instead of having a full
> > function that handle platform by platform.
> >
> > The specially case for SKL for not controlled sagv
> > is already taken care inside intel_enable_sagv, so there's
> > no need to duplicate the check here.
> >
> > v2: Go one step further and remove skl special case. (Jani)
> >
> > v3: Separate runtime status handle from has_sagv flag. (Jani)
>
> I know this has probably been discussed in the past and I missed it
> (since I know this was done a few times for other things), but how/why
> is this approach better than the current one? Can you please write it
> in the commit message?
my bad... after stripping this out of my RFC series this got
indeed out of context.
I'm trying to get rid of many platform codename checks spread
around the code.
I believe we should do most of the decisions based on display_gen
or has_feature. And use the platform codename as the last resource
only when gen or feature cannot describe things better.
> Quickly checking the implementation of intel_has_sagv() seems much
> easier than swimming through the nested macros of i915_pci.c. And the
> special-caseness of the NOT_CONTROLLED case being restricted to gen9 in
> the current (non-patched) code also at first appears to be better than
> this proposal. But I'm totally willing to read your arguments and
> reevaluate my decisions based on them, so please present them.
When you are looking only to a single feature this might be true.
When you are looking to the platform itself consolidating the features
definitions in a single place is easier to check legacy features and
add new platforms.
Specially I'd like to add a new platform gen line gen++ without
need to use any codename. Or without mixing things up.
INTEL_GEN >= 11 || IS_CANNONLAKE :/
So adding gen_n+1 should be as trivial
as gen_n + 1 legacy...
>
> >
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/i915_pci.c | 1 +
> > drivers/gpu/drm/i915/intel_device_info.h | 1 +
> > drivers/gpu/drm/i915/intel_pm.c | 29 ++++++++------------
> > ----
> > 4 files changed, 13 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 3017ef037fed..9b98ceb2d029 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2650,6 +2650,7 @@ intel_info(const struct drm_i915_private
> > *dev_priv)
> > #define HAS_DDI(dev_priv) ((dev_priv)->info.has_ddi)
> > #define HAS_FPGA_DBG_UNCLAIMED(dev_priv) ((dev_priv)-
> > >info.has_fpga_dbg)
> > #define HAS_PSR(dev_priv) ((dev_priv)->info.has_psr)
> > +#define HAS_SAGV(dev_priv) ((dev_priv)-
> > >info.has_sagv)
> >
> > #define HAS_RC6(dev_priv) ((dev_priv)->info.has_rc6)
> > #define HAS_RC6p(dev_priv) ((dev_priv)-
> > >info.has_rc6p)
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > b/drivers/gpu/drm/i915/i915_pci.c
> > index 44e745921ac1..0b09155eab62 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -465,6 +465,7 @@ static const struct intel_device_info
> > intel_cherryview_info = {
> > .has_csr = 1, \
> > .has_guc = 1, \
> > .has_ipc = 1, \
> > + .has_sagv = 1, \
> > .ddb_size = 896
> >
> > #define SKL_PLATFORM \
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.h
> > b/drivers/gpu/drm/i915/intel_device_info.h
> > index af7002640cdf..e77c8b62783f 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > @@ -117,6 +117,7 @@ enum intel_ppgtt {
> > func(hws_needs_physical); \
> > func(overlay_needs_physical); \
> > func(supports_tv); \
> > + func(has_sagv); \
> > func(has_ipc);
> >
> > #define GEN_MAX_SLICES (6) /* CNL upper bound */
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 67a4d0735291..7e38ed8421c7 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3609,20 +3609,6 @@ static bool skl_needs_memory_bw_wa(struct
> > intel_atomic_state *state)
> > return false;
> > }
> >
> > -static bool
> > -intel_has_sagv(struct drm_i915_private *dev_priv)
> > -{
> > - if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> > - IS_CANNONLAKE(dev_priv) || IS_ICELAKE(dev_priv))
> > - return true;
> > -
> > - if (IS_SKYLAKE(dev_priv) &&
> > - dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
> > - return true;
> > -
> > - return false;
> > -}
> > -
> > /*
> > * SAGV dynamically adjusts the system agent voltage and clock
> > frequencies
> > * depending on power and performance requirements. The display
> > engine access
> > @@ -3639,10 +3625,11 @@ intel_enable_sagv(struct drm_i915_private
> > *dev_priv)
> > {
> > int ret;
> >
> > - if (!intel_has_sagv(dev_priv))
> > + if (!HAS_SAGV(dev_priv))
> > return 0;
> >
> > - if (dev_priv->sagv_status == I915_SAGV_ENABLED)
> > + if (dev_priv->sagv_status == I915_SAGV_ENABLED ||
> > + dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
> > return 0;
> >
> > DRM_DEBUG_KMS("Enabling the SAGV\n");
> > @@ -3676,10 +3663,11 @@ intel_disable_sagv(struct drm_i915_private
> > *dev_priv)
> > {
> > int ret;
> >
> > - if (!intel_has_sagv(dev_priv))
> > + if (!HAS_SAGV(dev_priv))
> > return 0;
> >
> > - if (dev_priv->sagv_status == I915_SAGV_DISABLED)
> > + if (dev_priv->sagv_status == I915_SAGV_DISABLED ||
> > + dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
> > return 0;
> >
> > DRM_DEBUG_KMS("Disabling the SAGV\n");
> > @@ -3721,7 +3709,10 @@ bool intel_can_enable_sagv(struct
> > drm_atomic_state *state)
> > int level, latency;
> > int sagv_block_time_us;
> >
> > - if (!intel_has_sagv(dev_priv))
> > + if (!HAS_SAGV(dev_priv))
> > + return false;
> > +
> > + if (dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
> > return false;
> >
> > if (IS_GEN9(dev_priv))
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Simplify has_sagv
2018-10-23 0:06 ` Rodrigo Vivi
@ 2018-10-23 0:32 ` Paulo Zanoni
0 siblings, 0 replies; 23+ messages in thread
From: Paulo Zanoni @ 2018-10-23 0:32 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: Jani Nikula, intel-gfx, Lucas De Marchi
Em Seg, 2018-10-22 às 17:06 -0700, Rodrigo Vivi escreveu:
> On Mon, Oct 22, 2018 at 04:48:50PM -0700, Paulo Zanoni wrote:
> > Em Seg, 2018-10-22 às 09:57 -0700, Rodrigo Vivi escreveu:
> > > Let's add a platform has_sagv instead of having a full
> > > function that handle platform by platform.
> > >
> > > The specially case for SKL for not controlled sagv
> > > is already taken care inside intel_enable_sagv, so there's
> > > no need to duplicate the check here.
> > >
> > > v2: Go one step further and remove skl special case. (Jani)
> > >
> > > v3: Separate runtime status handle from has_sagv flag. (Jani)
> >
> > I know this has probably been discussed in the past and I missed it
> > (since I know this was done a few times for other things), but
> > how/why
> > is this approach better than the current one? Can you please write
> > it
> > in the commit message?
>
> my bad... after stripping this out of my RFC series this got
> indeed out of context.
>
> I'm trying to get rid of many platform codename checks spread
> around the code.
>
> I believe we should do most of the decisions based on display_gen
> or has_feature. And use the platform codename as the last resource
> only when gen or feature cannot describe things better.
In terms of "describing things better", I don't see a lot of difference
between HAS_SOMETHING() and intel_has_something(). The
intel_has_something() version even already gives us a signal that maybe
the decision is made based on runtime information instead of being
simply per-platform, which is this case.
>
> > Quickly checking the implementation of intel_has_sagv() seems much
> > easier than swimming through the nested macros of i915_pci.c. And
> > the
> > special-caseness of the NOT_CONTROLLED case being restricted to
> > gen9 in
> > the current (non-patched) code also at first appears to be better
> > than
> > this proposal. But I'm totally willing to read your arguments and
> > reevaluate my decisions based on them, so please present them.
>
> When you are looking only to a single feature this might be true.
> When you are looking to the platform itself consolidating the
> features
> definitions in a single place is easier to check legacy features and
> add new platforms.
So please make sure this is in the commit message. I may or may not
agree this is the best way to proceed, but at least now I can know why
it is the way it is when I git-blame/git-log.
>
> Specially I'd like to add a new platform gen line gen++ without
> need to use any codename. Or without mixing things up.
> INTEL_GEN >= 11 || IS_CANNONLAKE :/
You can achieve this by redefining intel_has_sagv() instead of killing
it. Currently, our code assumes new platforms won't support sagv, but
that seems unlikely considering the recent history. Just invert it:
has_sagv() {
if (is_gen9_lp || intel_gen < 9)
return false;
if (!is_skl)
return true;
return (status != controlled)
}
Note: I'm not blocking this patch.
>
> So adding gen_n+1 should be as trivial
> as gen_n + 1 legacy...
>
> >
> > >
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > > drivers/gpu/drm/i915/i915_pci.c | 1 +
> > > drivers/gpu/drm/i915/intel_device_info.h | 1 +
> > > drivers/gpu/drm/i915/intel_pm.c | 29 ++++++++--------
> > > ----
> > > ----
> > > 4 files changed, 13 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 3017ef037fed..9b98ceb2d029 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2650,6 +2650,7 @@ intel_info(const struct drm_i915_private
> > > *dev_priv)
> > > #define HAS_DDI(dev_priv) ((dev_priv)-
> > > >info.has_ddi)
> > > #define HAS_FPGA_DBG_UNCLAIMED(dev_priv) ((dev_priv)-
> > > > info.has_fpga_dbg)
> > >
> > > #define HAS_PSR(dev_priv) ((dev_priv)-
> > > >info.has_psr)
> > > +#define HAS_SAGV(dev_priv) ((dev_priv)-
> > > > info.has_sagv)
> > >
> > >
> > > #define HAS_RC6(dev_priv) ((dev_priv)-
> > > >info.has_rc6)
> > > #define HAS_RC6p(dev_priv) ((dev_priv)-
> > > > info.has_rc6p)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > > b/drivers/gpu/drm/i915/i915_pci.c
> > > index 44e745921ac1..0b09155eab62 100644
> > > --- a/drivers/gpu/drm/i915/i915_pci.c
> > > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > > @@ -465,6 +465,7 @@ static const struct intel_device_info
> > > intel_cherryview_info = {
> > > .has_csr = 1, \
> > > .has_guc = 1, \
> > > .has_ipc = 1, \
> > > + .has_sagv = 1, \
> > > .ddb_size = 896
> > >
> > > #define SKL_PLATFORM \
> > > diff --git a/drivers/gpu/drm/i915/intel_device_info.h
> > > b/drivers/gpu/drm/i915/intel_device_info.h
> > > index af7002640cdf..e77c8b62783f 100644
> > > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > > @@ -117,6 +117,7 @@ enum intel_ppgtt {
> > > func(hws_needs_physical); \
> > > func(overlay_needs_physical); \
> > > func(supports_tv); \
> > > + func(has_sagv); \
> > > func(has_ipc);
> > >
> > > #define GEN_MAX_SLICES (6) /* CNL upper bound */
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > > b/drivers/gpu/drm/i915/intel_pm.c
> > > index 67a4d0735291..7e38ed8421c7 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -3609,20 +3609,6 @@ static bool skl_needs_memory_bw_wa(struct
> > > intel_atomic_state *state)
> > > return false;
> > > }
> > >
> > > -static bool
> > > -intel_has_sagv(struct drm_i915_private *dev_priv)
> > > -{
> > > - if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> > > - IS_CANNONLAKE(dev_priv) || IS_ICELAKE(dev_priv))
> > > - return true;
> > > -
> > > - if (IS_SKYLAKE(dev_priv) &&
> > > - dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
> > > - return true;
> > > -
> > > - return false;
> > > -}
> > > -
> > > /*
> > > * SAGV dynamically adjusts the system agent voltage and clock
> > > frequencies
> > > * depending on power and performance requirements. The display
> > > engine access
> > > @@ -3639,10 +3625,11 @@ intel_enable_sagv(struct drm_i915_private
> > > *dev_priv)
> > > {
> > > int ret;
> > >
> > > - if (!intel_has_sagv(dev_priv))
> > > + if (!HAS_SAGV(dev_priv))
> > > return 0;
> > >
> > > - if (dev_priv->sagv_status == I915_SAGV_ENABLED)
> > > + if (dev_priv->sagv_status == I915_SAGV_ENABLED ||
> > > + dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
> > > return 0;
> > >
> > > DRM_DEBUG_KMS("Enabling the SAGV\n");
> > > @@ -3676,10 +3663,11 @@ intel_disable_sagv(struct
> > > drm_i915_private
> > > *dev_priv)
> > > {
> > > int ret;
> > >
> > > - if (!intel_has_sagv(dev_priv))
> > > + if (!HAS_SAGV(dev_priv))
> > > return 0;
> > >
> > > - if (dev_priv->sagv_status == I915_SAGV_DISABLED)
> > > + if (dev_priv->sagv_status == I915_SAGV_DISABLED ||
> > > + dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
> > > return 0;
> > >
> > > DRM_DEBUG_KMS("Disabling the SAGV\n");
> > > @@ -3721,7 +3709,10 @@ bool intel_can_enable_sagv(struct
> > > drm_atomic_state *state)
> > > int level, latency;
> > > int sagv_block_time_us;
> > >
> > > - if (!intel_has_sagv(dev_priv))
> > > + if (!HAS_SAGV(dev_priv))
> > > + return false;
> > > +
> > > + if (dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
> > > return false;
> > >
> > > if (IS_GEN9(dev_priv))
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Simplify has_sagv
2018-10-22 16:57 ` Rodrigo Vivi
2018-10-22 23:48 ` Paulo Zanoni
@ 2018-10-23 7:23 ` Jani Nikula
2018-10-23 19:05 ` Rodrigo Vivi
2018-10-26 20:03 ` [PATCH] drm/i915: Simplify has_sagv function Rodrigo Vivi
1 sibling, 2 replies; 23+ messages in thread
From: Jani Nikula @ 2018-10-23 7:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, Rodrigo Vivi
On Mon, 22 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> Let's add a platform has_sagv instead of having a full
> function that handle platform by platform.
>
> The specially case for SKL for not controlled sagv
> is already taken care inside intel_enable_sagv, so there's
> no need to duplicate the check here.
>
> v2: Go one step further and remove skl special case. (Jani)
>
> v3: Separate runtime status handle from has_sagv flag. (Jani)
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_pci.c | 1 +
> drivers/gpu/drm/i915/intel_device_info.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 29 ++++++++----------------
> 4 files changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3017ef037fed..9b98ceb2d029 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2650,6 +2650,7 @@ intel_info(const struct drm_i915_private *dev_priv)
> #define HAS_DDI(dev_priv) ((dev_priv)->info.has_ddi)
> #define HAS_FPGA_DBG_UNCLAIMED(dev_priv) ((dev_priv)->info.has_fpga_dbg)
> #define HAS_PSR(dev_priv) ((dev_priv)->info.has_psr)
> +#define HAS_SAGV(dev_priv) ((dev_priv)->info.has_sagv)
>
> #define HAS_RC6(dev_priv) ((dev_priv)->info.has_rc6)
> #define HAS_RC6p(dev_priv) ((dev_priv)->info.has_rc6p)
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 44e745921ac1..0b09155eab62 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -465,6 +465,7 @@ static const struct intel_device_info intel_cherryview_info = {
> .has_csr = 1, \
> .has_guc = 1, \
> .has_ipc = 1, \
> + .has_sagv = 1, \
> .ddb_size = 896
>
> #define SKL_PLATFORM \
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index af7002640cdf..e77c8b62783f 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -117,6 +117,7 @@ enum intel_ppgtt {
> func(hws_needs_physical); \
> func(overlay_needs_physical); \
> func(supports_tv); \
> + func(has_sagv); \
> func(has_ipc);
>
> #define GEN_MAX_SLICES (6) /* CNL upper bound */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 67a4d0735291..7e38ed8421c7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3609,20 +3609,6 @@ static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
> return false;
> }
>
> -static bool
> -intel_has_sagv(struct drm_i915_private *dev_priv)
> -{
> - if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> - IS_CANNONLAKE(dev_priv) || IS_ICELAKE(dev_priv))
> - return true;
> -
> - if (IS_SKYLAKE(dev_priv) &&
> - dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
> - return true;
> -
> - return false;
> -}
Argh. All I ever wanted was a version of [1] that removed the Skylake
special casing for I915_SAGV_NOT_CONTROLLED. I even wrote what the
function could be in its entirety:
return (IS_GEN9_BC(dev_priv) || INTEL_GEN(dev_priv) >= 10) &&
dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED;
IMO all iterations since have been worse.
BR,
Jani.
[1] http://patchwork.freedesktop.org/patch/msgid/20181018233447.5187-9-rodrigo.vivi@intel.com
> -
> /*
> * SAGV dynamically adjusts the system agent voltage and clock frequencies
> * depending on power and performance requirements. The display engine access
> @@ -3639,10 +3625,11 @@ intel_enable_sagv(struct drm_i915_private *dev_priv)
> {
> int ret;
>
> - if (!intel_has_sagv(dev_priv))
> + if (!HAS_SAGV(dev_priv))
> return 0;
>
> - if (dev_priv->sagv_status == I915_SAGV_ENABLED)
> + if (dev_priv->sagv_status == I915_SAGV_ENABLED ||
> + dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
> return 0;
>
> DRM_DEBUG_KMS("Enabling the SAGV\n");
> @@ -3676,10 +3663,11 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
> {
> int ret;
>
> - if (!intel_has_sagv(dev_priv))
> + if (!HAS_SAGV(dev_priv))
> return 0;
>
> - if (dev_priv->sagv_status == I915_SAGV_DISABLED)
> + if (dev_priv->sagv_status == I915_SAGV_DISABLED ||
> + dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
> return 0;
>
> DRM_DEBUG_KMS("Disabling the SAGV\n");
> @@ -3721,7 +3709,10 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
> int level, latency;
> int sagv_block_time_us;
>
> - if (!intel_has_sagv(dev_priv))
> + if (!HAS_SAGV(dev_priv))
> + return false;
> +
> + if (dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
> return false;
>
> if (IS_GEN9(dev_priv))
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Simplify has_sagv
2018-10-23 7:23 ` Jani Nikula
@ 2018-10-23 19:05 ` Rodrigo Vivi
2018-10-24 10:13 ` Jani Nikula
2018-10-26 20:03 ` [PATCH] drm/i915: Simplify has_sagv function Rodrigo Vivi
1 sibling, 1 reply; 23+ messages in thread
From: Rodrigo Vivi @ 2018-10-23 19:05 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, Lucas De Marchi
On Tue, Oct 23, 2018 at 10:23:39AM +0300, Jani Nikula wrote:
> On Mon, 22 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > Let's add a platform has_sagv instead of having a full
> > function that handle platform by platform.
> >
> > The specially case for SKL for not controlled sagv
> > is already taken care inside intel_enable_sagv, so there's
> > no need to duplicate the check here.
> >
> > v2: Go one step further and remove skl special case. (Jani)
> >
> > v3: Separate runtime status handle from has_sagv flag. (Jani)
> >
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/i915_pci.c | 1 +
> > drivers/gpu/drm/i915/intel_device_info.h | 1 +
> > drivers/gpu/drm/i915/intel_pm.c | 29 ++++++++----------------
> > 4 files changed, 13 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 3017ef037fed..9b98ceb2d029 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2650,6 +2650,7 @@ intel_info(const struct drm_i915_private *dev_priv)
> > #define HAS_DDI(dev_priv) ((dev_priv)->info.has_ddi)
> > #define HAS_FPGA_DBG_UNCLAIMED(dev_priv) ((dev_priv)->info.has_fpga_dbg)
> > #define HAS_PSR(dev_priv) ((dev_priv)->info.has_psr)
> > +#define HAS_SAGV(dev_priv) ((dev_priv)->info.has_sagv)
> >
> > #define HAS_RC6(dev_priv) ((dev_priv)->info.has_rc6)
> > #define HAS_RC6p(dev_priv) ((dev_priv)->info.has_rc6p)
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> > index 44e745921ac1..0b09155eab62 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -465,6 +465,7 @@ static const struct intel_device_info intel_cherryview_info = {
> > .has_csr = 1, \
> > .has_guc = 1, \
> > .has_ipc = 1, \
> > + .has_sagv = 1, \
> > .ddb_size = 896
> >
> > #define SKL_PLATFORM \
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> > index af7002640cdf..e77c8b62783f 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > @@ -117,6 +117,7 @@ enum intel_ppgtt {
> > func(hws_needs_physical); \
> > func(overlay_needs_physical); \
> > func(supports_tv); \
> > + func(has_sagv); \
> > func(has_ipc);
> >
> > #define GEN_MAX_SLICES (6) /* CNL upper bound */
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 67a4d0735291..7e38ed8421c7 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3609,20 +3609,6 @@ static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
> > return false;
> > }
> >
> > -static bool
> > -intel_has_sagv(struct drm_i915_private *dev_priv)
> > -{
> > - if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> > - IS_CANNONLAKE(dev_priv) || IS_ICELAKE(dev_priv))
> > - return true;
> > -
> > - if (IS_SKYLAKE(dev_priv) &&
> > - dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
> > - return true;
> > -
> > - return false;
> > -}
>
> Argh. All I ever wanted was a version of [1] that removed the Skylake
> special casing for I915_SAGV_NOT_CONTROLLED. I even wrote what the
> function could be in its entirety:
>
> return (IS_GEN9_BC(dev_priv) || INTEL_GEN(dev_priv) >= 10) &&
> dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED;
Well, that works for me as well.
>
> IMO all iterations since have been worse.
But I thought while doing this I could consolidade it along with all
the other has_feature cases.
I believe we should either have everything as info.has_feature or everything
as has_feature().
for instance if we end up ever having 2 platforms of same gen where
one has_sagv and the other doesn't we would have that in the platform
definition while making us to define another codename and add it here
or even worse if we don't have a codename available like CNL_WITH_PORT_F :/
But yeap, for now let's move with your suggestion.
Thanks,
Rodrigo.
>
> BR,
> Jani.
>
>
> [1] http://patchwork.freedesktop.org/patch/msgid/20181018233447.5187-9-rodrigo.vivi@intel.com
>
>
>
> > -
> > /*
> > * SAGV dynamically adjusts the system agent voltage and clock frequencies
> > * depending on power and performance requirements. The display engine access
> > @@ -3639,10 +3625,11 @@ intel_enable_sagv(struct drm_i915_private *dev_priv)
> > {
> > int ret;
> >
> > - if (!intel_has_sagv(dev_priv))
> > + if (!HAS_SAGV(dev_priv))
> > return 0;
> >
> > - if (dev_priv->sagv_status == I915_SAGV_ENABLED)
> > + if (dev_priv->sagv_status == I915_SAGV_ENABLED ||
> > + dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
> > return 0;
> >
> > DRM_DEBUG_KMS("Enabling the SAGV\n");
> > @@ -3676,10 +3663,11 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
> > {
> > int ret;
> >
> > - if (!intel_has_sagv(dev_priv))
> > + if (!HAS_SAGV(dev_priv))
> > return 0;
> >
> > - if (dev_priv->sagv_status == I915_SAGV_DISABLED)
> > + if (dev_priv->sagv_status == I915_SAGV_DISABLED ||
> > + dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
> > return 0;
> >
> > DRM_DEBUG_KMS("Disabling the SAGV\n");
> > @@ -3721,7 +3709,10 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
> > int level, latency;
> > int sagv_block_time_us;
> >
> > - if (!intel_has_sagv(dev_priv))
> > + if (!HAS_SAGV(dev_priv))
> > + return false;
> > +
> > + if (dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED)
> > return false;
> >
> > if (IS_GEN9(dev_priv))
>
> --
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Simplify has_sagv
2018-10-23 19:05 ` Rodrigo Vivi
@ 2018-10-24 10:13 ` Jani Nikula
2018-10-24 17:23 ` Rodrigo Vivi
0 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2018-10-24 10:13 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, Lucas De Marchi
On Tue, 23 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> But I thought while doing this I could consolidade it along with all
> the other has_feature cases.
>
> I believe we should either have everything as info.has_feature or everything
> as has_feature().
>
> for instance if we end up ever having 2 platforms of same gen where
> one has_sagv and the other doesn't we would have that in the platform
> definition while making us to define another codename and add it here
> or even worse if we don't have a codename available like CNL_WITH_PORT_F :/
If that is to be in device info, it doesn't require an extra codename,
it requires an extra device info with the flag.
This ties to the goal of making dev_priv->info a pointer to the static
const data in i915_pci.c i.e. making ->info truly const. There's three
categories of info:
1) immutable device properties
2) properties set once during probe, immutable afterwards
3) runtime
Currently we more or less happily conflate these, along with some module
parameters too. The mkwrite_device_info() use has profilerated much
wider than it was ever intended; we need to nuke that.
We also have HAS_FOO() and IS_FOO() macros that do checks on pci id or
gen or platform or a combination of them. It's a mess, and it's not
getting better without conscious effort.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Simplify has_sagv
2018-10-24 10:13 ` Jani Nikula
@ 2018-10-24 17:23 ` Rodrigo Vivi
0 siblings, 0 replies; 23+ messages in thread
From: Rodrigo Vivi @ 2018-10-24 17:23 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, Lucas De Marchi
On Wed, Oct 24, 2018 at 01:13:51PM +0300, Jani Nikula wrote:
> On Tue, 23 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > But I thought while doing this I could consolidade it along with all
> > the other has_feature cases.
> >
> > I believe we should either have everything as info.has_feature or everything
> > as has_feature().
> >
> > for instance if we end up ever having 2 platforms of same gen where
> > one has_sagv and the other doesn't we would have that in the platform
> > definition while making us to define another codename and add it here
> > or even worse if we don't have a codename available like CNL_WITH_PORT_F :/
>
> If that is to be in device info, it doesn't require an extra codename,
> it requires an extra device info with the flag.
>
> This ties to the goal of making dev_priv->info a pointer to the static
> const data in i915_pci.c i.e. making ->info truly const. There's three
> categories of info:
>
> 1) immutable device properties
> 2) properties set once during probe, immutable afterwards
> 3) runtime
>
> Currently we more or less happily conflate these, along with some module
> parameters too. The mkwrite_device_info() use has profilerated much
> wider than it was ever intended; we need to nuke that.
>
> We also have HAS_FOO() and IS_FOO() macros that do checks on pci id or
> gen or platform or a combination of them. It's a mess, and it's not
> getting better without conscious effort.
I agree with all that you said.
We probably just disagree on how to make HAS_FOO(dev_priv) IS_FOO(dev_priv)
better.
In my opinion we should move towards adding all immutable device
properties that describe and differentiate the platforms inside
device info. With that consolidated there:
- code gets uniform
- minimize gen and platform checks spread all over the code
- it gets easier to add platforms
But nevermind, I won't insist on this path ;)
>
> BR,
> Jani.
>
>
> --
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] drm/i915: Simplify has_sagv function
2018-10-23 7:23 ` Jani Nikula
2018-10-23 19:05 ` Rodrigo Vivi
@ 2018-10-26 20:03 ` Rodrigo Vivi
2018-10-29 10:08 ` Jani Nikula
1 sibling, 1 reply; 23+ messages in thread
From: Rodrigo Vivi @ 2018-10-26 20:03 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
The specially case for SKL for not controlled sagv
is already taken care inside intel_enable_sagv, so there's
no need to duplicate the check here.
v2: Go one step further and remove skl special case. (Jani)
v3: Separate runtime status handle from has_sagv flag.
v4: Go back and accept simple Jani proposed solution.
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index bc70f6bb86ae..82c82e233154 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3611,15 +3611,8 @@ static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
static bool
intel_has_sagv(struct drm_i915_private *dev_priv)
{
- if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
- IS_CANNONLAKE(dev_priv) || IS_ICELAKE(dev_priv))
- return true;
-
- if (IS_SKYLAKE(dev_priv) &&
- dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
- return true;
-
- return false;
+ return (IS_GEN9_BC(dev_priv) || INTEL_GEN(dev_priv) >= 10) &&
+ dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED;
}
/*
--
2.19.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Simplify has_sagv (rev3)
2018-10-19 17:20 [PATCH] drm/i915: Simplify has_sagv Rodrigo Vivi
` (7 preceding siblings ...)
2018-10-22 20:42 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2018-10-26 21:21 ` Patchwork
2018-10-29 15:59 ` ✓ Fi.CI.BAT: success " Patchwork
2018-10-29 18:10 ` ✓ Fi.CI.IGT: " Patchwork
10 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-10-26 21:21 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Simplify has_sagv (rev3)
URL : https://patchwork.freedesktop.org/series/51266/
State : failure
== Summary ==
= CI Bug Log - changes from CI_DRM_5044 -> Patchwork_10615 =
== Summary - FAILURE ==
Serious unknown changes coming with Patchwork_10615 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10615, 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/51266/revisions/3/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10615:
=== IGT changes ===
==== Possible regressions ====
igt@drv_selftest@live_gtt:
fi-elk-e7500: PASS -> DMESG-FAIL
== Known issues ==
Here are the changes found in Patchwork_10615 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@drv_selftest@live_contexts:
fi-icl-u: PASS -> DMESG-FAIL (fdo#108569)
igt@drv_selftest@live_hangcheck:
fi-icl-u: PASS -> INCOMPLETE (fdo#108315)
igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
fi-glk-j4005: PASS -> FAIL (fdo#106765)
igt@kms_flip@basic-flip-vs-wf_vblank:
fi-glk-j4005: PASS -> FAIL (fdo#100368)
igt@pm_rpm@module-reload:
fi-glk-j4005: PASS -> DMESG-WARN (fdo#107726)
==== Possible fixes ====
igt@gem_exec_suspend@basic-s3:
fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS
igt@gem_sync@basic-each:
fi-glk-j4005: DMESG-WARN (fdo#105719) -> PASS
igt@kms_flip@basic-flip-vs-modeset:
fi-glk-j4005: DMESG-WARN (fdo#106000) -> PASS
igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
fi-byt-clapper: FAIL (fdo#103191, fdo#107362) -> PASS
igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence:
fi-glk-j4005: DMESG-FAIL (fdo#106000) -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#105719 https://bugs.freedesktop.org/show_bug.cgi?id=105719
fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
fdo#106765 https://bugs.freedesktop.org/show_bug.cgi?id=106765
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
fdo#107726 https://bugs.freedesktop.org/show_bug.cgi?id=107726
fdo#108315 https://bugs.freedesktop.org/show_bug.cgi?id=108315
fdo#108569 https://bugs.freedesktop.org/show_bug.cgi?id=108569
== Participating hosts (47 -> 44) ==
Additional (1): fi-gdg-551
Missing (4): fi-ctg-p8600 fi-ilk-m540 fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_5044 -> Patchwork_10615
CI_DRM_5044: c4487dca27970879bf67f331614142c749984d65 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4698: af57164fcb16950187ad402ed31f565e88c42a78 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10615: 468abc2400d183c94072669a5a472a8c46c0fd65 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
468abc2400d1 drm/i915: Simplify has_sagv function
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10615/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Simplify has_sagv function
2018-10-26 20:03 ` [PATCH] drm/i915: Simplify has_sagv function Rodrigo Vivi
@ 2018-10-29 10:08 ` Jani Nikula
2018-10-29 18:58 ` Rodrigo Vivi
0 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2018-10-29 10:08 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
On Fri, 26 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> The specially case for SKL for not controlled sagv
> is already taken care inside intel_enable_sagv, so there's
> no need to duplicate the check here.
>
> v2: Go one step further and remove skl special case. (Jani)
> v3: Separate runtime status handle from has_sagv flag.
> v4: Go back and accept simple Jani proposed solution.
Thanks.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index bc70f6bb86ae..82c82e233154 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3611,15 +3611,8 @@ static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
> static bool
> intel_has_sagv(struct drm_i915_private *dev_priv)
> {
> - if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> - IS_CANNONLAKE(dev_priv) || IS_ICELAKE(dev_priv))
> - return true;
> -
> - if (IS_SKYLAKE(dev_priv) &&
> - dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
> - return true;
> -
> - return false;
> + return (IS_GEN9_BC(dev_priv) || INTEL_GEN(dev_priv) >= 10) &&
> + dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED;
> }
>
> /*
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Simplify has_sagv (rev3)
2018-10-19 17:20 [PATCH] drm/i915: Simplify has_sagv Rodrigo Vivi
` (8 preceding siblings ...)
2018-10-26 21:21 ` ✗ Fi.CI.BAT: failure for drm/i915: Simplify has_sagv (rev3) Patchwork
@ 2018-10-29 15:59 ` Patchwork
2018-10-29 18:10 ` ✓ Fi.CI.IGT: " Patchwork
10 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-10-29 15:59 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Simplify has_sagv (rev3)
URL : https://patchwork.freedesktop.org/series/51266/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5045 -> Patchwork_10627 =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_10627 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10627, 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/51266/revisions/3/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10627:
=== IGT changes ===
==== Warnings ====
igt@drv_selftest@live_guc:
fi-icl-u: SKIP -> PASS +2
== Known issues ==
Here are the changes found in Patchwork_10627 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@drv_selftest@live_hangcheck:
fi-kbl-7560u: PASS -> INCOMPLETE (fdo#108044)
igt@gem_exec_suspend@basic-s3:
fi-kbl-soraka: NOTRUN -> INCOMPLETE (fdo#107859, fdo#107556, fdo#107774)
fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718)
igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
fi-icl-u2: PASS -> DMESG-WARN (fdo#106107)
==== Possible fixes ====
igt@drv_selftest@live_contexts:
fi-icl-u2: DMESG-FAIL (fdo#108569) -> PASS
igt@drv_selftest@live_hangcheck:
fi-skl-guc: DMESG-FAIL (fdo#108593) -> PASS
==== Warnings ====
igt@drv_selftest@live_contexts:
fi-icl-u: INCOMPLETE (fdo#108535) -> DMESG-FAIL (fdo#108569)
fdo#106107 https://bugs.freedesktop.org/show_bug.cgi?id=106107
fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
fdo#107774 https://bugs.freedesktop.org/show_bug.cgi?id=107774
fdo#107859 https://bugs.freedesktop.org/show_bug.cgi?id=107859
fdo#108044 https://bugs.freedesktop.org/show_bug.cgi?id=108044
fdo#108535 https://bugs.freedesktop.org/show_bug.cgi?id=108535
fdo#108569 https://bugs.freedesktop.org/show_bug.cgi?id=108569
fdo#108593 https://bugs.freedesktop.org/show_bug.cgi?id=108593
== Participating hosts (46 -> 44) ==
Additional (1): fi-kbl-soraka
Missing (3): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan
== Build changes ==
* Linux: CI_DRM_5045 -> Patchwork_10627
CI_DRM_5045: b5ec1a938aea1befeb92e6a51313f4d637265fdf @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4699: 1270ec553741ac20c45178d2b26f9a9562ea565f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10627: c6c8bb0232d64d455a5c1acff83ec787ae2cb5f8 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
c6c8bb0232d6 drm/i915: Simplify has_sagv function
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10627/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: Simplify has_sagv (rev3)
2018-10-19 17:20 [PATCH] drm/i915: Simplify has_sagv Rodrigo Vivi
` (9 preceding siblings ...)
2018-10-29 15:59 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-10-29 18:10 ` Patchwork
10 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2018-10-29 18:10 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Simplify has_sagv (rev3)
URL : https://patchwork.freedesktop.org/series/51266/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5045_full -> Patchwork_10627_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_10627_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10627_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_10627_full:
=== IGT changes ===
==== Warnings ====
igt@kms_plane_lowres@pipe-a-tiling-none:
shard-snb: PASS -> SKIP +2
== Known issues ==
Here are the changes found in Patchwork_10627_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_ppgtt@blt-vs-render-ctxn:
shard-kbl: PASS -> INCOMPLETE (fdo#106023, fdo#106887, fdo#103665)
shard-skl: NOTRUN -> TIMEOUT (fdo#108039)
igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
shard-skl: NOTRUN -> DMESG-WARN (fdo#107956)
igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
shard-glk: PASS -> FAIL (fdo#108145)
igt@kms_chv_cursor_fail@pipe-b-256x256-bottom-edge:
shard-skl: NOTRUN -> FAIL (fdo#104671)
igt@kms_cursor_crc@cursor-128x42-sliding:
shard-apl: PASS -> FAIL (fdo#103232) +2
igt@kms_cursor_crc@cursor-256x256-random:
shard-glk: PASS -> FAIL (fdo#103232)
igt@kms_cursor_crc@cursor-64x21-random:
shard-skl: NOTRUN -> FAIL (fdo#103232)
igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-ytiled:
shard-apl: PASS -> INCOMPLETE (fdo#103927)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
shard-skl: NOTRUN -> FAIL (fdo#103167) +3
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
shard-apl: PASS -> FAIL (fdo#103167) +1
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
shard-glk: PASS -> FAIL (fdo#103167) +1
igt@kms_plane@pixel-format-pipe-a-planes:
shard-skl: NOTRUN -> DMESG-FAIL (fdo#103166, fdo#106885)
igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
shard-skl: NOTRUN -> FAIL (fdo#108145) +4
igt@kms_plane_alpha_blend@pipe-b-alpha-7efc:
shard-skl: NOTRUN -> FAIL (fdo#108145, fdo#107815) +2
igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
shard-glk: PASS -> FAIL (fdo#103166) +2
igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
shard-apl: PASS -> FAIL (fdo#103166) +1
igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
shard-skl: NOTRUN -> FAIL (fdo#103166, fdo#107815)
igt@kms_setmode@basic:
shard-kbl: PASS -> FAIL (fdo#99912)
igt@pm_backlight@fade_with_suspend:
shard-skl: NOTRUN -> FAIL (fdo#107847)
==== Possible fixes ====
igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
shard-glk: DMESG-WARN (fdo#107956) -> PASS
igt@kms_cursor_crc@cursor-128x128-suspend:
shard-skl: INCOMPLETE (fdo#104108) -> PASS
igt@kms_cursor_crc@cursor-256x85-offscreen:
shard-skl: FAIL (fdo#103232) -> PASS
igt@kms_cursor_crc@cursor-64x21-sliding:
shard-apl: FAIL (fdo#103232) -> PASS +2
igt@kms_cursor_legacy@pipe-a-torture-move:
shard-skl: INCOMPLETE -> PASS
igt@kms_draw_crc@draw-method-rgb565-mmap-wc-ytiled:
shard-glk: FAIL (fdo#103184) -> PASS
igt@kms_flip@flip-vs-expired-vblank:
shard-skl: FAIL (fdo#105363) -> PASS
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff:
shard-apl: FAIL (fdo#103167) -> PASS +1
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-fullscreen:
shard-glk: FAIL (fdo#103167) -> PASS +1
igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
shard-skl: FAIL (fdo#107815) -> PASS
igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
shard-apl: FAIL (fdo#103166) -> PASS +1
igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
shard-glk: FAIL (fdo#103166) -> PASS +1
igt@kms_setmode@basic:
shard-apl: FAIL (fdo#99912) -> PASS
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
fdo#106885 https://bugs.freedesktop.org/show_bug.cgi?id=106885
fdo#106887 https://bugs.freedesktop.org/show_bug.cgi?id=106887
fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
fdo#107847 https://bugs.freedesktop.org/show_bug.cgi?id=107847
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#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (6 -> 6) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_5045 -> Patchwork_10627
CI_DRM_5045: b5ec1a938aea1befeb92e6a51313f4d637265fdf @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4699: 1270ec553741ac20c45178d2b26f9a9562ea565f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10627: c6c8bb0232d64d455a5c1acff83ec787ae2cb5f8 @ 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_10627/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] drm/i915: Simplify has_sagv function
2018-10-29 10:08 ` Jani Nikula
@ 2018-10-29 18:58 ` Rodrigo Vivi
0 siblings, 0 replies; 23+ messages in thread
From: Rodrigo Vivi @ 2018-10-29 18:58 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Mon, Oct 29, 2018 at 12:08:38PM +0200, Jani Nikula wrote:
> On Fri, 26 Oct 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > The specially case for SKL for not controlled sagv
> > is already taken care inside intel_enable_sagv, so there's
> > no need to duplicate the check here.
> >
> > v2: Go one step further and remove skl special case. (Jani)
> > v3: Separate runtime status handle from has_sagv flag.
> > v4: Go back and accept simple Jani proposed solution.
>
> Thanks.
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
pushed to dinq. Thanks for review and idea!
>
>
>
> >
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 11 ++---------
> > 1 file changed, 2 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index bc70f6bb86ae..82c82e233154 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3611,15 +3611,8 @@ static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
> > static bool
> > intel_has_sagv(struct drm_i915_private *dev_priv)
> > {
> > - if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
> > - IS_CANNONLAKE(dev_priv) || IS_ICELAKE(dev_priv))
> > - return true;
> > -
> > - if (IS_SKYLAKE(dev_priv) &&
> > - dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED)
> > - return true;
> > -
> > - return false;
> > + return (IS_GEN9_BC(dev_priv) || INTEL_GEN(dev_priv) >= 10) &&
> > + dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED;
> > }
> >
> > /*
>
> --
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2018-10-29 18:58 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-19 17:20 [PATCH] drm/i915: Simplify has_sagv Rodrigo Vivi
2018-10-22 6:28 ` Jani Nikula
2018-10-22 16:57 ` Rodrigo Vivi
2018-10-22 23:48 ` Paulo Zanoni
2018-10-23 0:06 ` Rodrigo Vivi
2018-10-23 0:32 ` Paulo Zanoni
2018-10-23 7:23 ` Jani Nikula
2018-10-23 19:05 ` Rodrigo Vivi
2018-10-24 10:13 ` Jani Nikula
2018-10-24 17:23 ` Rodrigo Vivi
2018-10-26 20:03 ` [PATCH] drm/i915: Simplify has_sagv function Rodrigo Vivi
2018-10-29 10:08 ` Jani Nikula
2018-10-29 18:58 ` Rodrigo Vivi
2018-10-22 13:13 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Simplify has_sagv Patchwork
2018-10-22 13:14 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-10-22 13:37 ` ✓ Fi.CI.BAT: success " Patchwork
2018-10-22 16:46 ` ✓ Fi.CI.IGT: " Patchwork
2018-10-22 17:20 ` ✗ Fi.CI.SPARSE: warning for drm/i915: Simplify has_sagv (rev2) Patchwork
2018-10-22 17:37 ` ✓ Fi.CI.BAT: success " Patchwork
2018-10-22 20:42 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-10-26 21:21 ` ✗ Fi.CI.BAT: failure for drm/i915: Simplify has_sagv (rev3) Patchwork
2018-10-29 15:59 ` ✓ Fi.CI.BAT: success " Patchwork
2018-10-29 18:10 ` ✓ Fi.CI.IGT: " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.