* [PATCH] drm/i915: Drop ilk_wm_max_level()
@ 2015-12-21 19:32 Matt Roper
2015-12-22 8:20 ` ✗ warning: Fi.CI.BAT Patchwork
2016-01-18 14:36 ` [PATCH] drm/i915: Drop ilk_wm_max_level() Ville Syrjälä
0 siblings, 2 replies; 3+ messages in thread
From: Matt Roper @ 2015-12-21 19:32 UTC (permalink / raw)
To: intel-gfx
Storing the max_level in dev_priv as VLV/CHV already do is a bit simpler
than calling this standalone function, especially since some of the
callsites need to special-case the call to check whether they're running
on VLV/CHV.
This function was further confusing since it wasn't actually specific to
ILK-style platforms as its name implied (it was also used on Skylake and
would even be called on pre-ILK platforms via debugfs). Aside from this
function, our watermark programming across various platform-types
doesn't really have any overlap.
Note that we do change debugfs behavior slightly with this patch;
i915_*_wm_latency files will all now appear empty on pre-ILK platforms
rather than showing two watermark levels of 0.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 18 ++-------------
drivers/gpu/drm/i915/intel_pm.c | 44 +++++++++++++++++--------------------
2 files changed, 22 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 0fc38bb..ae36656 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4445,14 +4445,7 @@ static void wm_latency_show(struct seq_file *m, const uint16_t wm[8])
{
struct drm_device *dev = m->private;
int level;
- int num_levels;
-
- if (IS_CHERRYVIEW(dev))
- num_levels = 3;
- else if (IS_VALLEYVIEW(dev))
- num_levels = 1;
- else
- num_levels = ilk_wm_max_level(dev) + 1;
+ int num_levels = to_i915(dev)->wm.max_level + 1;
drm_modeset_lock_all(dev);
@@ -4560,18 +4553,11 @@ static ssize_t wm_latency_write(struct file *file, const char __user *ubuf,
struct seq_file *m = file->private_data;
struct drm_device *dev = m->private;
uint16_t new[8] = { 0 };
- int num_levels;
+ int num_levels = to_i915(dev)->wm.max_level + 1;
int level;
int ret;
char tmp[32];
- if (IS_CHERRYVIEW(dev))
- num_levels = 3;
- else if (IS_VALLEYVIEW(dev))
- num_levels = 1;
- else
- num_levels = ilk_wm_max_level(dev) + 1;
-
if (len >= sizeof(tmp))
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b076580..71d814c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2027,7 +2027,7 @@ static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[8])
if (IS_GEN9(dev)) {
uint32_t val;
int ret, i;
- int level, max_level = ilk_wm_max_level(dev);
+ int level, max_level = dev_priv->wm.max_level;
/* read the first set of memory latencies[0:3] */
val = 0; /* data0 to be programmed to 0 for first set */
@@ -2142,24 +2142,11 @@ static void intel_fixup_cur_wm_latency(struct drm_device *dev, uint16_t wm[5])
wm[3] *= 2;
}
-int ilk_wm_max_level(const struct drm_device *dev)
-{
- /* how many WM levels are we expecting */
- if (INTEL_INFO(dev)->gen >= 9)
- return 7;
- else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
- return 4;
- else if (INTEL_INFO(dev)->gen >= 6)
- return 3;
- else
- return 2;
-}
-
static void intel_print_wm_latency(struct drm_device *dev,
const char *name,
const uint16_t wm[8])
{
- int level, max_level = ilk_wm_max_level(dev);
+ int level, max_level = to_i915(dev)->wm.max_level;
for (level = 0; level <= max_level; level++) {
unsigned int latency = wm[level];
@@ -2188,7 +2175,7 @@ static void intel_print_wm_latency(struct drm_device *dev,
static bool ilk_increase_wm_latency(struct drm_i915_private *dev_priv,
uint16_t wm[5], uint16_t min)
{
- int level, max_level = ilk_wm_max_level(dev_priv->dev);
+ int level, max_level = dev_priv->wm.max_level;
if (wm[0] >= min)
return false;
@@ -2242,6 +2229,13 @@ static void ilk_setup_wm_latency(struct drm_device *dev)
if (IS_GEN6(dev))
snb_wm_latency_quirk(dev);
+
+ if (IS_HASWELL(dev) || IS_BROADWELL(dev))
+ dev_priv->wm.max_level = 4;
+ else if (INTEL_INFO(dev)->gen >= 6)
+ dev_priv->wm.max_level = 3;
+ else
+ dev_priv->wm.max_level = 2;
}
static void skl_setup_wm_latency(struct drm_device *dev)
@@ -2250,6 +2244,8 @@ static void skl_setup_wm_latency(struct drm_device *dev)
intel_read_wm_latency(dev, dev_priv->wm.skl_latency);
intel_print_wm_latency(dev, "Gen9 Plane", dev_priv->wm.skl_latency);
+
+ dev_priv->wm.max_level = 7;
}
/* Compute new watermarks for the pipe */
@@ -2265,7 +2261,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
struct intel_plane_state *pristate = NULL;
struct intel_plane_state *sprstate = NULL;
struct intel_plane_state *curstate = NULL;
- int level, max_level = ilk_wm_max_level(dev);
+ int level, max_level = dev_priv->wm.max_level;
/* LP0 watermark maximums depend on this pipe alone */
struct intel_wm_config config = {
.num_pipes_active = 1,
@@ -2389,7 +2385,7 @@ static void ilk_wm_merge(struct drm_device *dev,
struct intel_pipe_wm *merged)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- int level, max_level = ilk_wm_max_level(dev);
+ int level, max_level = dev_priv->wm.max_level;
int last_enabled_level = max_level;
/* ILK/SNB/IVB: LP1+ watermarks only w/ single pipe */
@@ -2530,7 +2526,7 @@ static struct intel_pipe_wm *ilk_find_best_result(struct drm_device *dev,
struct intel_pipe_wm *r1,
struct intel_pipe_wm *r2)
{
- int level, max_level = ilk_wm_max_level(dev);
+ int level, max_level = to_i915(dev)->wm.max_level;
int level1 = 0, level2 = 0;
for (level = 1; level <= max_level; level++) {
@@ -3232,7 +3228,7 @@ static void skl_compute_pipe_wm(struct intel_crtc_state *cstate,
{
struct drm_device *dev = cstate->base.crtc->dev;
const struct drm_i915_private *dev_priv = dev->dev_private;
- int level, max_level = ilk_wm_max_level(dev);
+ int level, max_level = dev_priv->wm.max_level;
for (level = 0; level <= max_level; level++) {
skl_compute_wm_level(dev_priv, ddb, cstate,
@@ -3248,7 +3244,7 @@ static void skl_compute_wm_results(struct drm_device *dev,
struct skl_wm_values *r,
struct intel_crtc *intel_crtc)
{
- int level, max_level = ilk_wm_max_level(dev);
+ int level, max_level = to_i915(dev)->wm.max_level;
enum pipe pipe = intel_crtc->pipe;
uint32_t temp;
int i;
@@ -3317,7 +3313,7 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv,
struct intel_crtc *crtc;
for_each_intel_crtc(dev, crtc) {
- int i, level, max_level = ilk_wm_max_level(dev);
+ int i, level, max_level = dev_priv->wm.max_level;
enum pipe pipe = crtc->pipe;
if (!new->dirty[pipe])
@@ -3711,7 +3707,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
int level, i, max_level;
uint32_t temp;
- max_level = ilk_wm_max_level(dev);
+ max_level = dev_priv->wm.max_level;
hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
@@ -3801,7 +3797,7 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
active->wm[0].cur_val = tmp & WM0_PIPE_CURSOR_MASK;
active->linetime = hw->wm_linetime[pipe];
} else {
- int level, max_level = ilk_wm_max_level(dev);
+ int level, max_level = dev_priv->wm.max_level;
/*
* For inactive pipes, all watermark levels
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 3+ messages in thread
* ✗ warning: Fi.CI.BAT
2015-12-21 19:32 [PATCH] drm/i915: Drop ilk_wm_max_level() Matt Roper
@ 2015-12-22 8:20 ` Patchwork
2016-01-18 14:36 ` [PATCH] drm/i915: Drop ilk_wm_max_level() Ville Syrjälä
1 sibling, 0 replies; 3+ messages in thread
From: Patchwork @ 2015-12-22 8:20 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
== Summary ==
Built on 78deeec98b10627fe2050ce8ebfa2ea2d5b9e6c7 drm-intel-nightly: 2015y-12m-21d-16h-03m-57s UTC integration manifest
Test gem_mmap_gtt:
Subgroup basic-small-bo:
dmesg-warn -> PASS (bdw-nuci7)
Test gem_storedw_loop:
Subgroup basic-render:
pass -> DMESG-WARN (skl-i5k-2)
dmesg-warn -> PASS (skl-i7k-2)
Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass -> DMESG-WARN (skl-i5k-2)
pass -> DMESG-WARN (hsw-xps12)
skip -> PASS (bdw-nuci7)
Subgroup basic-plain-flip:
skip -> PASS (bdw-nuci7)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-a:
skip -> PASS (bdw-nuci7)
dmesg-warn -> PASS (byt-nuc)
Subgroup read-crc-pipe-a-frame-sequence:
skip -> PASS (bdw-nuci7)
Subgroup read-crc-pipe-b:
dmesg-warn -> PASS (skl-i5k-2)
dmesg-warn -> PASS (hsw-xps12)
pass -> DMESG-WARN (snb-dellxps)
skip -> PASS (bdw-nuci7)
Subgroup read-crc-pipe-b-frame-sequence:
skip -> PASS (bdw-nuci7)
Subgroup read-crc-pipe-c:
skip -> PASS (bdw-nuci7)
pass -> DMESG-WARN (skl-i7k-2)
Subgroup suspend-read-crc-pipe-b:
pass -> DMESG-WARN (snb-x220t)
Test kms_setmode:
Subgroup basic-clone-single-crtc:
dmesg-warn -> PASS (snb-dellxps)
Test pm_rpm:
Subgroup basic-pci-d3-state:
skip -> PASS (bdw-nuci7)
pass -> DMESG-WARN (bdw-ultra)
Subgroup basic-rte:
skip -> PASS (bdw-nuci7)
dmesg-warn -> PASS (bdw-ultra)
bdw-nuci7 total:132 pass:122 dwarn:1 dfail:0 fail:0 skip:9
bdw-ultra total:132 pass:124 dwarn:2 dfail:0 fail:0 skip:6
bsw-nuc-2 total:135 pass:114 dwarn:1 dfail:0 fail:0 skip:20
byt-nuc total:135 pass:120 dwarn:2 dfail:0 fail:0 skip:13
hsw-brixbox total:135 pass:126 dwarn:2 dfail:0 fail:0 skip:7
hsw-gt2 total:135 pass:130 dwarn:1 dfail:0 fail:0 skip:4
hsw-xps12 total:132 pass:125 dwarn:3 dfail:0 fail:0 skip:4
ilk-hp8440p total:135 pass:100 dwarn:0 dfail:0 fail:0 skip:35
ivb-t430s total:135 pass:127 dwarn:2 dfail:0 fail:0 skip:6
skl-i5k-2 total:135 pass:121 dwarn:6 dfail:0 fail:0 skip:8
skl-i7k-2 total:135 pass:122 dwarn:5 dfail:0 fail:0 skip:8
snb-dellxps total:135 pass:121 dwarn:2 dfail:0 fail:0 skip:12
snb-x220t total:135 pass:121 dwarn:2 dfail:0 fail:1 skip:11
Results at /archive/results/CI_IGT_test/Patchwork_785/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: Drop ilk_wm_max_level()
2015-12-21 19:32 [PATCH] drm/i915: Drop ilk_wm_max_level() Matt Roper
2015-12-22 8:20 ` ✗ warning: Fi.CI.BAT Patchwork
@ 2016-01-18 14:36 ` Ville Syrjälä
1 sibling, 0 replies; 3+ messages in thread
From: Ville Syrjälä @ 2016-01-18 14:36 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Mon, Dec 21, 2015 at 11:32:50AM -0800, Matt Roper wrote:
> Storing the max_level in dev_priv as VLV/CHV already do is a bit simpler
> than calling this standalone function, especially since some of the
> callsites need to special-case the call to check whether they're running
> on VLV/CHV.
>
> This function was further confusing since it wasn't actually specific to
> ILK-style platforms as its name implied (it was also used on Skylake and
> would even be called on pre-ILK platforms via debugfs). Aside from this
> function, our watermark programming across various platform-types
> doesn't really have any overlap.
>
> Note that we do change debugfs behavior slightly with this patch;
> i915_*_wm_latency files will all now appear empty on pre-ILK platforms
> rather than showing two watermark levels of 0.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 18 ++-------------
> drivers/gpu/drm/i915/intel_pm.c | 44 +++++++++++++++++--------------------
> 2 files changed, 22 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 0fc38bb..ae36656 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4445,14 +4445,7 @@ static void wm_latency_show(struct seq_file *m, const uint16_t wm[8])
> {
> struct drm_device *dev = m->private;
> int level;
> - int num_levels;
> -
> - if (IS_CHERRYVIEW(dev))
> - num_levels = 3;
> - else if (IS_VALLEYVIEW(dev))
> - num_levels = 1;
> - else
> - num_levels = ilk_wm_max_level(dev) + 1;
> + int num_levels = to_i915(dev)->wm.max_level + 1;
>
> drm_modeset_lock_all(dev);
>
> @@ -4560,18 +4553,11 @@ static ssize_t wm_latency_write(struct file *file, const char __user *ubuf,
> struct seq_file *m = file->private_data;
> struct drm_device *dev = m->private;
> uint16_t new[8] = { 0 };
> - int num_levels;
> + int num_levels = to_i915(dev)->wm.max_level + 1;
> int level;
> int ret;
> char tmp[32];
>
> - if (IS_CHERRYVIEW(dev))
> - num_levels = 3;
> - else if (IS_VALLEYVIEW(dev))
> - num_levels = 1;
> - else
> - num_levels = ilk_wm_max_level(dev) + 1;
> -
One idea I had at some point would be adjust dev_priv->wm.max_level based
on what the user writes to this file. Ie. it would allow the user to
enable/disable specific wm levels in a fairly nice way. In theory you
can do that already, but a bunch of stuff in the wm code might
scream at/not like eg. a latency value of 0. If we would adjust max_levels
appropritely we should be able to avoid those problems.
To do that we should probably keep the ilk_wm_max_level() (or something
like it) around and it would return the hardware limit, whereas dev_priv
contains the sw limit.
> if (len >= sizeof(tmp))
> return -EINVAL;
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b076580..71d814c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2027,7 +2027,7 @@ static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[8])
> if (IS_GEN9(dev)) {
> uint32_t val;
> int ret, i;
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = dev_priv->wm.max_level;
>
> /* read the first set of memory latencies[0:3] */
> val = 0; /* data0 to be programmed to 0 for first set */
> @@ -2142,24 +2142,11 @@ static void intel_fixup_cur_wm_latency(struct drm_device *dev, uint16_t wm[5])
> wm[3] *= 2;
> }
>
> -int ilk_wm_max_level(const struct drm_device *dev)
> -{
> - /* how many WM levels are we expecting */
> - if (INTEL_INFO(dev)->gen >= 9)
> - return 7;
> - else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> - return 4;
> - else if (INTEL_INFO(dev)->gen >= 6)
> - return 3;
> - else
> - return 2;
> -}
> -
> static void intel_print_wm_latency(struct drm_device *dev,
> const char *name,
> const uint16_t wm[8])
> {
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = to_i915(dev)->wm.max_level;
>
> for (level = 0; level <= max_level; level++) {
> unsigned int latency = wm[level];
> @@ -2188,7 +2175,7 @@ static void intel_print_wm_latency(struct drm_device *dev,
> static bool ilk_increase_wm_latency(struct drm_i915_private *dev_priv,
> uint16_t wm[5], uint16_t min)
> {
> - int level, max_level = ilk_wm_max_level(dev_priv->dev);
> + int level, max_level = dev_priv->wm.max_level;
>
> if (wm[0] >= min)
> return false;
> @@ -2242,6 +2229,13 @@ static void ilk_setup_wm_latency(struct drm_device *dev)
>
> if (IS_GEN6(dev))
> snb_wm_latency_quirk(dev);
> +
> + if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> + dev_priv->wm.max_level = 4;
> + else if (INTEL_INFO(dev)->gen >= 6)
> + dev_priv->wm.max_level = 3;
> + else
> + dev_priv->wm.max_level = 2;
Maybe we could just compute this based on how many non-zero wm latency
values we managed to read out?
> }
>
> static void skl_setup_wm_latency(struct drm_device *dev)
> @@ -2250,6 +2244,8 @@ static void skl_setup_wm_latency(struct drm_device *dev)
>
> intel_read_wm_latency(dev, dev_priv->wm.skl_latency);
> intel_print_wm_latency(dev, "Gen9 Plane", dev_priv->wm.skl_latency);
> +
> + dev_priv->wm.max_level = 7;
> }
>
> /* Compute new watermarks for the pipe */
> @@ -2265,7 +2261,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
> struct intel_plane_state *pristate = NULL;
> struct intel_plane_state *sprstate = NULL;
> struct intel_plane_state *curstate = NULL;
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = dev_priv->wm.max_level;
> /* LP0 watermark maximums depend on this pipe alone */
> struct intel_wm_config config = {
> .num_pipes_active = 1,
> @@ -2389,7 +2385,7 @@ static void ilk_wm_merge(struct drm_device *dev,
> struct intel_pipe_wm *merged)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = dev_priv->wm.max_level;
> int last_enabled_level = max_level;
>
> /* ILK/SNB/IVB: LP1+ watermarks only w/ single pipe */
> @@ -2530,7 +2526,7 @@ static struct intel_pipe_wm *ilk_find_best_result(struct drm_device *dev,
> struct intel_pipe_wm *r1,
> struct intel_pipe_wm *r2)
> {
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = to_i915(dev)->wm.max_level;
> int level1 = 0, level2 = 0;
>
> for (level = 1; level <= max_level; level++) {
> @@ -3232,7 +3228,7 @@ static void skl_compute_pipe_wm(struct intel_crtc_state *cstate,
> {
> struct drm_device *dev = cstate->base.crtc->dev;
> const struct drm_i915_private *dev_priv = dev->dev_private;
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = dev_priv->wm.max_level;
>
> for (level = 0; level <= max_level; level++) {
> skl_compute_wm_level(dev_priv, ddb, cstate,
> @@ -3248,7 +3244,7 @@ static void skl_compute_wm_results(struct drm_device *dev,
> struct skl_wm_values *r,
> struct intel_crtc *intel_crtc)
> {
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = to_i915(dev)->wm.max_level;
> enum pipe pipe = intel_crtc->pipe;
> uint32_t temp;
> int i;
> @@ -3317,7 +3313,7 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv,
> struct intel_crtc *crtc;
>
> for_each_intel_crtc(dev, crtc) {
> - int i, level, max_level = ilk_wm_max_level(dev);
> + int i, level, max_level = dev_priv->wm.max_level;
> enum pipe pipe = crtc->pipe;
>
> if (!new->dirty[pipe])
> @@ -3711,7 +3707,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> int level, i, max_level;
> uint32_t temp;
>
> - max_level = ilk_wm_max_level(dev);
> + max_level = dev_priv->wm.max_level;
>
> hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
>
> @@ -3801,7 +3797,7 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> active->wm[0].cur_val = tmp & WM0_PIPE_CURSOR_MASK;
> active->linetime = hw->wm_linetime[pipe];
> } else {
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = dev_priv->wm.max_level;
>
> /*
> * For inactive pipes, all watermark levels
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-01-18 14:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-21 19:32 [PATCH] drm/i915: Drop ilk_wm_max_level() Matt Roper
2015-12-22 8:20 ` ✗ warning: Fi.CI.BAT Patchwork
2016-01-18 14:36 ` [PATCH] drm/i915: Drop ilk_wm_max_level() Ville Syrjälä
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.