* [PATCH v3 00/10] Start of skl watermark cleanup
@ 2016-10-14 21:31 ` Lyude
0 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx; +Cc: linux-kernel, dri-devel, Daniel Vetter, Lyude
While it (mostly) works, the code for handling watermarks on Skylake has been
kind of ugly for a while. As well a lot of it isn't that friendly to atomic
transactions, Lots of copy paste, redundant wm values, etc. While this isn't a
full cleanup, it's a good start. As well, we add a couple of features for
making debugging watermarks a little easier.
Rebased for latest nightly, new r-bs added + some changes
Lyude (10):
drm/i915/skl: Move per-pipe ddb allocations into crtc states
drm/i915/skl: Remove linetime from skl_wm_values
drm/i915/gen9: Make skl_wm_level per-plane
drm/i915/gen9: Cleanup skl_pipe_wm_active_state
drm/i915/gen9: Get rid of redundant watermark values
drm/i915/gen9: Add ddb changes to atomic debug output
drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable
drm/i915/gen9: Add skl_wm_level_equals()
drm/i915/gen9: Actually verify WM levels in verify_wm_state()
drm/i915/gen9: Don't wrap strings in verify_wm_state()
drivers/gpu/drm/i915/i915_drv.h | 10 +-
drivers/gpu/drm/i915/intel_display.c | 138 ++++++++---
drivers/gpu/drm/i915/intel_drv.h | 24 +-
drivers/gpu/drm/i915/intel_pm.c | 466 +++++++++++++++++------------------
drivers/gpu/drm/i915/intel_sprite.c | 8 +-
5 files changed, 355 insertions(+), 291 deletions(-)
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 01/10] drm/i915/skl: Move per-pipe ddb allocations into crtc states
2016-10-14 21:31 ` Lyude
@ 2016-10-14 21:31 ` Lyude
-1 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx
Cc: Lyude, Ville Syrjälä,
Matt Roper, Paulo Zanoni, Daniel Vetter, Jani Nikula,
David Airlie, dri-devel, linux-kernel
First part of cleaning up all of the skl watermark code. This moves the
structures for storing the ddb allocations of each pipe into
intel_crtc_state, along with moving the structures for storing the
current ddb allocations active on hardware into intel_crtc.
Changes since v1:
- Don't replace alloc->start = alloc->end = 0;
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/intel_display.c | 16 ++++++++-------
drivers/gpu/drm/i915/intel_drv.h | 8 +++++---
drivers/gpu/drm/i915/intel_pm.c | 40 +++++++++++++++---------------------
4 files changed, 30 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fe875b2..ac4287f99 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1641,7 +1641,6 @@ static inline bool skl_ddb_entry_equal(const struct skl_ddb_entry *e1,
}
struct skl_ddb_allocation {
- struct skl_ddb_entry pipe[I915_MAX_PIPES];
struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES]; /* packed/uv */
struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
};
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 60c699e..7a68cc3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14246,12 +14246,11 @@ static void skl_update_crtcs(struct drm_atomic_state *state,
unsigned int *crtc_vblank_mask)
{
struct drm_device *dev = state->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
struct drm_crtc *crtc;
+ struct intel_crtc *intel_crtc;
struct drm_crtc_state *old_crtc_state;
- struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
- struct skl_ddb_allocation *cur_ddb = &dev_priv->wm.skl_hw.ddb;
+ struct intel_crtc_state *cstate;
unsigned int updated = 0;
bool progress;
enum pipe pipe;
@@ -14269,12 +14268,14 @@ static void skl_update_crtcs(struct drm_atomic_state *state,
for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
bool vbl_wait = false;
unsigned int cmask = drm_crtc_mask(crtc);
- pipe = to_intel_crtc(crtc)->pipe;
+
+ intel_crtc = to_intel_crtc(crtc);
+ cstate = to_intel_crtc_state(crtc->state);
+ pipe = intel_crtc->pipe;
if (updated & cmask || !crtc->state->active)
continue;
- if (skl_ddb_allocation_overlaps(state, cur_ddb, new_ddb,
- pipe))
+ if (skl_ddb_allocation_overlaps(state, intel_crtc))
continue;
updated |= cmask;
@@ -14285,7 +14286,8 @@ static void skl_update_crtcs(struct drm_atomic_state *state,
* then we need to wait for a vblank to pass for the
* new ddb allocation to take effect.
*/
- if (!skl_ddb_allocation_equals(cur_ddb, new_ddb, pipe) &&
+ if (!skl_ddb_entry_equal(&cstate->wm.skl.ddb,
+ &intel_crtc->hw_ddb) &&
!crtc->state->active_changed &&
intel_state->wm_results.dirty_pipes != updated)
vbl_wait = true;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5bc1154..10a0cf2 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -496,6 +496,7 @@ struct intel_crtc_wm_state {
struct {
/* gen9+ only needs 1-step wm programming */
struct skl_pipe_wm optimal;
+ struct skl_ddb_entry ddb;
/* cached plane data rate */
unsigned plane_data_rate[I915_MAX_PLANES];
@@ -733,6 +734,9 @@ struct intel_crtc {
bool cxsr_allowed;
} wm;
+ /* gen9+: ddb allocation currently being used */
+ struct skl_ddb_entry hw_ddb;
+
int scanline_offset;
struct {
@@ -1755,9 +1759,7 @@ bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
const struct skl_ddb_allocation *new,
enum pipe pipe);
bool skl_ddb_allocation_overlaps(struct drm_atomic_state *state,
- const struct skl_ddb_allocation *old,
- const struct skl_ddb_allocation *new,
- enum pipe pipe);
+ struct intel_crtc *intel_crtc);
void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
const struct skl_wm_values *wm);
void skl_write_plane_wm(struct intel_crtc *intel_crtc,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 48b3904..66586af 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3062,7 +3062,6 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
struct drm_crtc *for_crtc = cstate->base.crtc;
unsigned int pipe_size, ddb_size;
int nth_active_pipe;
- int pipe = to_intel_crtc(for_crtc)->pipe;
if (WARN_ON(!state) || !cstate->base.active) {
alloc->start = 0;
@@ -3090,7 +3089,7 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
* we currently hold.
*/
if (!intel_state->active_pipe_changes) {
- *alloc = dev_priv->wm.skl_hw.ddb.pipe[pipe];
+ *alloc = to_intel_crtc(for_crtc)->hw_ddb;
return;
}
@@ -3358,7 +3357,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
struct drm_plane *plane;
struct drm_plane_state *pstate;
enum pipe pipe = intel_crtc->pipe;
- struct skl_ddb_entry *alloc = &ddb->pipe[pipe];
+ struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb;
uint16_t alloc_size, start, cursor_blocks;
uint16_t *minimum = cstate->wm.skl.minimum_blocks;
uint16_t *y_minimum = cstate->wm.skl.minimum_y_blocks;
@@ -3374,7 +3373,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
return 0;
if (!cstate->base.active) {
- ddb->pipe[pipe].start = ddb->pipe[pipe].end = 0;
+ alloc->start = alloc->end = 0;
return 0;
}
@@ -3901,14 +3900,6 @@ void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
&wm->ddb.plane[pipe][PLANE_CURSOR]);
}
-bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
- const struct skl_ddb_allocation *new,
- enum pipe pipe)
-{
- return new->pipe[pipe].start == old->pipe[pipe].start &&
- new->pipe[pipe].end == old->pipe[pipe].end;
-}
-
static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
const struct skl_ddb_entry *b)
{
@@ -3916,22 +3907,22 @@ static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
}
bool skl_ddb_allocation_overlaps(struct drm_atomic_state *state,
- const struct skl_ddb_allocation *old,
- const struct skl_ddb_allocation *new,
- enum pipe pipe)
+ struct intel_crtc *intel_crtc)
{
- struct drm_device *dev = state->dev;
- struct intel_crtc *intel_crtc;
- enum pipe otherp;
+ struct drm_crtc *other_crtc;
+ struct drm_crtc_state *other_cstate;
+ struct intel_crtc *other_intel_crtc;
+ const struct skl_ddb_entry *ddb =
+ &to_intel_crtc_state(intel_crtc->base.state)->wm.skl.ddb;
+ int i;
- for_each_intel_crtc(dev, intel_crtc) {
- otherp = intel_crtc->pipe;
+ for_each_crtc_in_state(state, other_crtc, other_cstate, i) {
+ other_intel_crtc = to_intel_crtc(other_crtc);
- if (otherp == pipe)
+ if (other_intel_crtc == intel_crtc)
continue;
- if (skl_ddb_entries_overlap(&new->pipe[pipe],
- &old->pipe[otherp]))
+ if (skl_ddb_entries_overlap(ddb, &other_intel_crtc->hw_ddb))
return true;
}
@@ -4096,7 +4087,6 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
memcpy(dst->plane_trans[pipe], src->plane_trans[pipe],
sizeof(dst->plane_trans[pipe]));
- dst->ddb.pipe[pipe] = src->ddb.pipe[pipe];
memcpy(dst->ddb.y_plane[pipe], src->ddb.y_plane[pipe],
sizeof(dst->ddb.y_plane[pipe]));
memcpy(dst->ddb.plane[pipe], src->ddb.plane[pipe],
@@ -4204,6 +4194,8 @@ static void skl_update_wm(struct drm_crtc *crtc)
skl_copy_wm_for_pipe(hw_vals, results, pipe);
+ intel_crtc->hw_ddb = cstate->wm.skl.ddb;
+
mutex_unlock(&dev_priv->wm.wm_mutex);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 01/10] drm/i915/skl: Move per-pipe ddb allocations into crtc states
@ 2016-10-14 21:31 ` Lyude
0 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, linux-kernel, dri-devel, Daniel Vetter, Lyude
First part of cleaning up all of the skl watermark code. This moves the
structures for storing the ddb allocations of each pipe into
intel_crtc_state, along with moving the structures for storing the
current ddb allocations active on hardware into intel_crtc.
Changes since v1:
- Don't replace alloc->start = alloc->end = 0;
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/intel_display.c | 16 ++++++++-------
drivers/gpu/drm/i915/intel_drv.h | 8 +++++---
drivers/gpu/drm/i915/intel_pm.c | 40 +++++++++++++++---------------------
4 files changed, 30 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fe875b2..ac4287f99 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1641,7 +1641,6 @@ static inline bool skl_ddb_entry_equal(const struct skl_ddb_entry *e1,
}
struct skl_ddb_allocation {
- struct skl_ddb_entry pipe[I915_MAX_PIPES];
struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES]; /* packed/uv */
struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
};
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 60c699e..7a68cc3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14246,12 +14246,11 @@ static void skl_update_crtcs(struct drm_atomic_state *state,
unsigned int *crtc_vblank_mask)
{
struct drm_device *dev = state->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
struct drm_crtc *crtc;
+ struct intel_crtc *intel_crtc;
struct drm_crtc_state *old_crtc_state;
- struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
- struct skl_ddb_allocation *cur_ddb = &dev_priv->wm.skl_hw.ddb;
+ struct intel_crtc_state *cstate;
unsigned int updated = 0;
bool progress;
enum pipe pipe;
@@ -14269,12 +14268,14 @@ static void skl_update_crtcs(struct drm_atomic_state *state,
for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
bool vbl_wait = false;
unsigned int cmask = drm_crtc_mask(crtc);
- pipe = to_intel_crtc(crtc)->pipe;
+
+ intel_crtc = to_intel_crtc(crtc);
+ cstate = to_intel_crtc_state(crtc->state);
+ pipe = intel_crtc->pipe;
if (updated & cmask || !crtc->state->active)
continue;
- if (skl_ddb_allocation_overlaps(state, cur_ddb, new_ddb,
- pipe))
+ if (skl_ddb_allocation_overlaps(state, intel_crtc))
continue;
updated |= cmask;
@@ -14285,7 +14286,8 @@ static void skl_update_crtcs(struct drm_atomic_state *state,
* then we need to wait for a vblank to pass for the
* new ddb allocation to take effect.
*/
- if (!skl_ddb_allocation_equals(cur_ddb, new_ddb, pipe) &&
+ if (!skl_ddb_entry_equal(&cstate->wm.skl.ddb,
+ &intel_crtc->hw_ddb) &&
!crtc->state->active_changed &&
intel_state->wm_results.dirty_pipes != updated)
vbl_wait = true;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5bc1154..10a0cf2 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -496,6 +496,7 @@ struct intel_crtc_wm_state {
struct {
/* gen9+ only needs 1-step wm programming */
struct skl_pipe_wm optimal;
+ struct skl_ddb_entry ddb;
/* cached plane data rate */
unsigned plane_data_rate[I915_MAX_PLANES];
@@ -733,6 +734,9 @@ struct intel_crtc {
bool cxsr_allowed;
} wm;
+ /* gen9+: ddb allocation currently being used */
+ struct skl_ddb_entry hw_ddb;
+
int scanline_offset;
struct {
@@ -1755,9 +1759,7 @@ bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
const struct skl_ddb_allocation *new,
enum pipe pipe);
bool skl_ddb_allocation_overlaps(struct drm_atomic_state *state,
- const struct skl_ddb_allocation *old,
- const struct skl_ddb_allocation *new,
- enum pipe pipe);
+ struct intel_crtc *intel_crtc);
void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
const struct skl_wm_values *wm);
void skl_write_plane_wm(struct intel_crtc *intel_crtc,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 48b3904..66586af 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3062,7 +3062,6 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
struct drm_crtc *for_crtc = cstate->base.crtc;
unsigned int pipe_size, ddb_size;
int nth_active_pipe;
- int pipe = to_intel_crtc(for_crtc)->pipe;
if (WARN_ON(!state) || !cstate->base.active) {
alloc->start = 0;
@@ -3090,7 +3089,7 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
* we currently hold.
*/
if (!intel_state->active_pipe_changes) {
- *alloc = dev_priv->wm.skl_hw.ddb.pipe[pipe];
+ *alloc = to_intel_crtc(for_crtc)->hw_ddb;
return;
}
@@ -3358,7 +3357,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
struct drm_plane *plane;
struct drm_plane_state *pstate;
enum pipe pipe = intel_crtc->pipe;
- struct skl_ddb_entry *alloc = &ddb->pipe[pipe];
+ struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb;
uint16_t alloc_size, start, cursor_blocks;
uint16_t *minimum = cstate->wm.skl.minimum_blocks;
uint16_t *y_minimum = cstate->wm.skl.minimum_y_blocks;
@@ -3374,7 +3373,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
return 0;
if (!cstate->base.active) {
- ddb->pipe[pipe].start = ddb->pipe[pipe].end = 0;
+ alloc->start = alloc->end = 0;
return 0;
}
@@ -3901,14 +3900,6 @@ void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
&wm->ddb.plane[pipe][PLANE_CURSOR]);
}
-bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
- const struct skl_ddb_allocation *new,
- enum pipe pipe)
-{
- return new->pipe[pipe].start == old->pipe[pipe].start &&
- new->pipe[pipe].end == old->pipe[pipe].end;
-}
-
static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
const struct skl_ddb_entry *b)
{
@@ -3916,22 +3907,22 @@ static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
}
bool skl_ddb_allocation_overlaps(struct drm_atomic_state *state,
- const struct skl_ddb_allocation *old,
- const struct skl_ddb_allocation *new,
- enum pipe pipe)
+ struct intel_crtc *intel_crtc)
{
- struct drm_device *dev = state->dev;
- struct intel_crtc *intel_crtc;
- enum pipe otherp;
+ struct drm_crtc *other_crtc;
+ struct drm_crtc_state *other_cstate;
+ struct intel_crtc *other_intel_crtc;
+ const struct skl_ddb_entry *ddb =
+ &to_intel_crtc_state(intel_crtc->base.state)->wm.skl.ddb;
+ int i;
- for_each_intel_crtc(dev, intel_crtc) {
- otherp = intel_crtc->pipe;
+ for_each_crtc_in_state(state, other_crtc, other_cstate, i) {
+ other_intel_crtc = to_intel_crtc(other_crtc);
- if (otherp == pipe)
+ if (other_intel_crtc == intel_crtc)
continue;
- if (skl_ddb_entries_overlap(&new->pipe[pipe],
- &old->pipe[otherp]))
+ if (skl_ddb_entries_overlap(ddb, &other_intel_crtc->hw_ddb))
return true;
}
@@ -4096,7 +4087,6 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
memcpy(dst->plane_trans[pipe], src->plane_trans[pipe],
sizeof(dst->plane_trans[pipe]));
- dst->ddb.pipe[pipe] = src->ddb.pipe[pipe];
memcpy(dst->ddb.y_plane[pipe], src->ddb.y_plane[pipe],
sizeof(dst->ddb.y_plane[pipe]));
memcpy(dst->ddb.plane[pipe], src->ddb.plane[pipe],
@@ -4204,6 +4194,8 @@ static void skl_update_wm(struct drm_crtc *crtc)
skl_copy_wm_for_pipe(hw_vals, results, pipe);
+ intel_crtc->hw_ddb = cstate->wm.skl.ddb;
+
mutex_unlock(&dev_priv->wm.wm_mutex);
}
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 02/10] drm/i915/skl: Remove linetime from skl_wm_values
2016-10-14 21:31 ` Lyude
@ 2016-10-14 21:31 ` Lyude
-1 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx
Cc: Lyude, Ville Syrjälä,
Matt Roper, Paulo Zanoni, Daniel Vetter, Jani Nikula,
David Airlie, dri-devel, linux-kernel
Next part of cleaning up the watermark code for skl. This is easy, since
it seems that we never actually needed to keep track of the linetime in
the skl_wm_values struct anyway.
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/intel_display.c | 6 ++++--
drivers/gpu/drm/i915/intel_pm.c | 7 +------
3 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ac4287f99..e56d4a4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1648,7 +1648,6 @@ struct skl_ddb_allocation {
struct skl_wm_values {
unsigned dirty_pipes;
struct skl_ddb_allocation ddb;
- uint32_t wm_linetime[I915_MAX_PIPES];
uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
};
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7a68cc3..ced70ad 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14844,6 +14844,8 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *intel_cstate =
+ to_intel_crtc_state(crtc->state);
struct intel_crtc_state *old_intel_state =
to_intel_crtc_state(old_crtc_state);
bool modeset = needs_modeset(crtc->state);
@@ -14860,13 +14862,13 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
intel_color_load_luts(crtc->state);
}
- if (to_intel_crtc_state(crtc->state)->update_pipe)
+ if (intel_cstate->update_pipe)
intel_update_pipe_config(intel_crtc, old_intel_state);
else if (INTEL_GEN(dev_priv) >= 9) {
skl_detach_scalers(intel_crtc);
I915_WRITE(PIPE_WM_LINETIME(pipe),
- dev_priv->wm.skl_hw.wm_linetime[pipe]);
+ intel_cstate->wm.skl.optimal.linetime);
}
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 66586af..a7d5721 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3845,8 +3845,6 @@ static void skl_compute_wm_results(struct drm_device *dev,
temp |= PLANE_WM_EN;
r->plane_trans[pipe][PLANE_CURSOR] = temp;
-
- r->wm_linetime[pipe] = p_wm->linetime;
}
static void skl_ddb_entry_write(struct drm_i915_private *dev_priv,
@@ -4081,7 +4079,6 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
struct skl_wm_values *src,
enum pipe pipe)
{
- dst->wm_linetime[pipe] = src->wm_linetime[pipe];
memcpy(dst->plane[pipe], src->plane[pipe],
sizeof(dst->plane[pipe]));
memcpy(dst->plane_trans[pipe], src->plane_trans[pipe],
@@ -4332,8 +4329,6 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
max_level = ilk_wm_max_level(dev_priv);
- hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
-
for (level = 0; level <= max_level; level++) {
for (i = 0; i < intel_num_planes(intel_crtc); i++)
hw->plane[pipe][i][level] =
@@ -4350,7 +4345,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
hw->dirty_pipes |= drm_crtc_mask(crtc);
- active->linetime = hw->wm_linetime[pipe];
+ active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
for (level = 0; level <= max_level; level++) {
for (i = 0; i < intel_num_planes(intel_crtc); i++) {
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 02/10] drm/i915/skl: Remove linetime from skl_wm_values
@ 2016-10-14 21:31 ` Lyude
0 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx
Cc: Paulo Zanoni, David Airlie, linux-kernel, dri-devel, Daniel Vetter
Next part of cleaning up the watermark code for skl. This is easy, since
it seems that we never actually needed to keep track of the linetime in
the skl_wm_values struct anyway.
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/intel_display.c | 6 ++++--
drivers/gpu/drm/i915/intel_pm.c | 7 +------
3 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ac4287f99..e56d4a4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1648,7 +1648,6 @@ struct skl_ddb_allocation {
struct skl_wm_values {
unsigned dirty_pipes;
struct skl_ddb_allocation ddb;
- uint32_t wm_linetime[I915_MAX_PIPES];
uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
};
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7a68cc3..ced70ad 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14844,6 +14844,8 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *intel_cstate =
+ to_intel_crtc_state(crtc->state);
struct intel_crtc_state *old_intel_state =
to_intel_crtc_state(old_crtc_state);
bool modeset = needs_modeset(crtc->state);
@@ -14860,13 +14862,13 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
intel_color_load_luts(crtc->state);
}
- if (to_intel_crtc_state(crtc->state)->update_pipe)
+ if (intel_cstate->update_pipe)
intel_update_pipe_config(intel_crtc, old_intel_state);
else if (INTEL_GEN(dev_priv) >= 9) {
skl_detach_scalers(intel_crtc);
I915_WRITE(PIPE_WM_LINETIME(pipe),
- dev_priv->wm.skl_hw.wm_linetime[pipe]);
+ intel_cstate->wm.skl.optimal.linetime);
}
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 66586af..a7d5721 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3845,8 +3845,6 @@ static void skl_compute_wm_results(struct drm_device *dev,
temp |= PLANE_WM_EN;
r->plane_trans[pipe][PLANE_CURSOR] = temp;
-
- r->wm_linetime[pipe] = p_wm->linetime;
}
static void skl_ddb_entry_write(struct drm_i915_private *dev_priv,
@@ -4081,7 +4079,6 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
struct skl_wm_values *src,
enum pipe pipe)
{
- dst->wm_linetime[pipe] = src->wm_linetime[pipe];
memcpy(dst->plane[pipe], src->plane[pipe],
sizeof(dst->plane[pipe]));
memcpy(dst->plane_trans[pipe], src->plane_trans[pipe],
@@ -4332,8 +4329,6 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
max_level = ilk_wm_max_level(dev_priv);
- hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
-
for (level = 0; level <= max_level; level++) {
for (i = 0; i < intel_num_planes(intel_crtc); i++)
hw->plane[pipe][i][level] =
@@ -4350,7 +4345,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
hw->dirty_pipes |= drm_crtc_mask(crtc);
- active->linetime = hw->wm_linetime[pipe];
+ active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
for (level = 0; level <= max_level; level++) {
for (i = 0; i < intel_num_planes(intel_crtc); i++) {
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 03/10] drm/i915/gen9: Make skl_wm_level per-plane
2016-10-14 21:31 ` Lyude
@ 2016-10-14 21:31 ` Lyude
-1 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx
Cc: Lyude, Ville Syrjälä,
Matt Roper, Paulo Zanoni, Daniel Vetter, Jani Nikula,
David Airlie, dri-devel, linux-kernel
Having skl_wm_level contain all of the watermarks for each plane is
annoying since it prevents us from having any sort of object to
represent a single watermark level, something we take advantage of in
the next commit to cut down on all of the copy paste code in here.
Changes since v1:
- Style nitpicks
- Fix accidental usage of i vs. PLANE_CURSOR
- Split out skl_pipe_wm_active_state simplification into seperate patch
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 6 +-
drivers/gpu/drm/i915/intel_drv.h | 6 +-
drivers/gpu/drm/i915/intel_pm.c | 207 +++++++++++++++++++--------------------
3 files changed, 111 insertions(+), 108 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e56d4a4..4d1133f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1653,9 +1653,9 @@ struct skl_wm_values {
};
struct skl_wm_level {
- bool plane_en[I915_MAX_PLANES];
- uint16_t plane_res_b[I915_MAX_PLANES];
- uint8_t plane_res_l[I915_MAX_PLANES];
+ bool plane_en;
+ uint16_t plane_res_b;
+ uint8_t plane_res_l;
};
/*
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 10a0cf2..84301d3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -468,9 +468,13 @@ struct intel_pipe_wm {
bool sprites_scaled;
};
-struct skl_pipe_wm {
+struct skl_plane_wm {
struct skl_wm_level wm[8];
struct skl_wm_level trans_wm;
+};
+
+struct skl_pipe_wm {
+ struct skl_plane_wm planes[I915_MAX_PLANES];
uint32_t linetime;
};
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a7d5721..1bdccc9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3674,67 +3674,52 @@ static int
skl_compute_wm_level(const struct drm_i915_private *dev_priv,
struct skl_ddb_allocation *ddb,
struct intel_crtc_state *cstate,
+ struct intel_plane *intel_plane,
int level,
struct skl_wm_level *result)
{
struct drm_atomic_state *state = cstate->base.state;
struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
- struct drm_plane *plane;
- struct intel_plane *intel_plane;
- struct intel_plane_state *intel_pstate;
+ struct drm_plane *plane = &intel_plane->base;
+ struct intel_plane_state *intel_pstate = NULL;
uint16_t ddb_blocks;
enum pipe pipe = intel_crtc->pipe;
int ret;
+ int i = skl_wm_plane_id(intel_plane);
+
+ if (state)
+ intel_pstate =
+ intel_atomic_get_existing_plane_state(state,
+ intel_plane);
/*
- * We'll only calculate watermarks for planes that are actually
- * enabled, so make sure all other planes are set as disabled.
+ * Note: If we start supporting multiple pending atomic commits against
+ * the same planes/CRTC's in the future, plane->state will no longer be
+ * the correct pre-state to use for the calculations here and we'll
+ * need to change where we get the 'unchanged' plane data from.
+ *
+ * For now this is fine because we only allow one queued commit against
+ * a CRTC. Even if the plane isn't modified by this transaction and we
+ * don't have a plane lock, we still have the CRTC's lock, so we know
+ * that no other transactions are racing with us to update it.
*/
- memset(result, 0, sizeof(*result));
+ if (!intel_pstate)
+ intel_pstate = to_intel_plane_state(plane->state);
- for_each_intel_plane_mask(&dev_priv->drm,
- intel_plane,
- cstate->base.plane_mask) {
- int i = skl_wm_plane_id(intel_plane);
-
- plane = &intel_plane->base;
- intel_pstate = NULL;
- if (state)
- intel_pstate =
- intel_atomic_get_existing_plane_state(state,
- intel_plane);
+ WARN_ON(!intel_pstate->base.fb);
- /*
- * Note: If we start supporting multiple pending atomic commits
- * against the same planes/CRTC's in the future, plane->state
- * will no longer be the correct pre-state to use for the
- * calculations here and we'll need to change where we get the
- * 'unchanged' plane data from.
- *
- * For now this is fine because we only allow one queued commit
- * against a CRTC. Even if the plane isn't modified by this
- * transaction and we don't have a plane lock, we still have
- * the CRTC's lock, so we know that no other transactions are
- * racing with us to update it.
- */
- if (!intel_pstate)
- intel_pstate = to_intel_plane_state(plane->state);
+ ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][i]);
- WARN_ON(!intel_pstate->base.fb);
-
- ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][i]);
-
- ret = skl_compute_plane_wm(dev_priv,
- cstate,
- intel_pstate,
- ddb_blocks,
- level,
- &result->plane_res_b[i],
- &result->plane_res_l[i],
- &result->plane_en[i]);
- if (ret)
- return ret;
- }
+ ret = skl_compute_plane_wm(dev_priv,
+ cstate,
+ intel_pstate,
+ ddb_blocks,
+ level,
+ &result->plane_res_b,
+ &result->plane_res_l,
+ &result->plane_en);
+ if (ret)
+ return ret;
return 0;
}
@@ -3755,19 +3740,11 @@ skl_compute_linetime_wm(struct intel_crtc_state *cstate)
static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
struct skl_wm_level *trans_wm /* out */)
{
- struct drm_crtc *crtc = cstate->base.crtc;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_plane *intel_plane;
-
if (!cstate->base.active)
return;
/* Until we know more, just disable transition WMs */
- for_each_intel_plane_on_crtc(crtc->dev, intel_crtc, intel_plane) {
- int i = skl_wm_plane_id(intel_plane);
-
- trans_wm->plane_en[i] = false;
- }
+ trans_wm->plane_en = false;
}
static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
@@ -3776,19 +3753,33 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
{
struct drm_device *dev = cstate->base.crtc->dev;
const struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_plane *intel_plane;
+ struct skl_plane_wm *wm;
int level, max_level = ilk_wm_max_level(dev_priv);
int ret;
- for (level = 0; level <= max_level; level++) {
- ret = skl_compute_wm_level(dev_priv, ddb, cstate,
- level, &pipe_wm->wm[level]);
- if (ret)
- return ret;
+ /*
+ * We'll only calculate watermarks for planes that are actually
+ * enabled, so make sure all other planes are set as disabled.
+ */
+ memset(pipe_wm->planes, 0, sizeof(pipe_wm->planes));
+
+ for_each_intel_plane_mask(&dev_priv->drm,
+ intel_plane,
+ cstate->base.plane_mask) {
+ wm = &pipe_wm->planes[skl_wm_plane_id(intel_plane)];
+
+ for (level = 0; level <= max_level; level++) {
+ ret = skl_compute_wm_level(dev_priv, ddb, cstate,
+ intel_plane, level,
+ &wm->wm[level]);
+ if (ret)
+ return ret;
+ }
+ skl_compute_transition_wm(cstate, &wm->trans_wm);
}
pipe_wm->linetime = skl_compute_linetime_wm(cstate);
- skl_compute_transition_wm(cstate, &pipe_wm->trans_wm);
-
return 0;
}
@@ -3798,50 +3789,56 @@ static void skl_compute_wm_results(struct drm_device *dev,
struct intel_crtc *intel_crtc)
{
int level, max_level = ilk_wm_max_level(to_i915(dev));
+ struct skl_plane_wm *plane_wm;
enum pipe pipe = intel_crtc->pipe;
uint32_t temp;
int i;
- for (level = 0; level <= max_level; level++) {
- for (i = 0; i < intel_num_planes(intel_crtc); i++) {
+ for (i = 0; i < intel_num_planes(intel_crtc); i++) {
+ plane_wm = &p_wm->planes[i];
+
+ for (level = 0; level <= max_level; level++) {
temp = 0;
- temp |= p_wm->wm[level].plane_res_l[i] <<
+ temp |= plane_wm->wm[level].plane_res_l <<
PLANE_WM_LINES_SHIFT;
- temp |= p_wm->wm[level].plane_res_b[i];
- if (p_wm->wm[level].plane_en[i])
+ temp |= plane_wm->wm[level].plane_res_b;
+ if (plane_wm->wm[level].plane_en)
temp |= PLANE_WM_EN;
r->plane[pipe][i][level] = temp;
}
- temp = 0;
-
- temp |= p_wm->wm[level].plane_res_l[PLANE_CURSOR] << PLANE_WM_LINES_SHIFT;
- temp |= p_wm->wm[level].plane_res_b[PLANE_CURSOR];
+ }
- if (p_wm->wm[level].plane_en[PLANE_CURSOR])
+ for (level = 0; level <= max_level; level++) {
+ plane_wm = &p_wm->planes[PLANE_CURSOR];
+ temp = 0;
+ temp |= plane_wm->wm[level].plane_res_l << PLANE_WM_LINES_SHIFT;
+ temp |= plane_wm->wm[level].plane_res_b;
+ if (plane_wm->wm[level].plane_en)
temp |= PLANE_WM_EN;
r->plane[pipe][PLANE_CURSOR][level] = temp;
-
}
/* transition WMs */
for (i = 0; i < intel_num_planes(intel_crtc); i++) {
+ plane_wm = &p_wm->planes[i];
temp = 0;
- temp |= p_wm->trans_wm.plane_res_l[i] << PLANE_WM_LINES_SHIFT;
- temp |= p_wm->trans_wm.plane_res_b[i];
- if (p_wm->trans_wm.plane_en[i])
+ temp |= plane_wm->trans_wm.plane_res_l << PLANE_WM_LINES_SHIFT;
+ temp |= plane_wm->trans_wm.plane_res_b;
+ if (plane_wm->trans_wm.plane_en)
temp |= PLANE_WM_EN;
r->plane_trans[pipe][i] = temp;
}
+ plane_wm = &p_wm->planes[PLANE_CURSOR];
temp = 0;
- temp |= p_wm->trans_wm.plane_res_l[PLANE_CURSOR] << PLANE_WM_LINES_SHIFT;
- temp |= p_wm->trans_wm.plane_res_b[PLANE_CURSOR];
- if (p_wm->trans_wm.plane_en[PLANE_CURSOR])
+ temp |= plane_wm->trans_wm.plane_res_l << PLANE_WM_LINES_SHIFT;
+ temp |= plane_wm->trans_wm.plane_res_b;
+ if (plane_wm->trans_wm.plane_en)
temp |= PLANE_WM_EN;
r->plane_trans[pipe][PLANE_CURSOR] = temp;
@@ -4282,35 +4279,37 @@ static void skl_pipe_wm_active_state(uint32_t val,
if (!is_transwm) {
if (!is_cursor) {
- active->wm[level].plane_en[i] = is_enabled;
- active->wm[level].plane_res_b[i] =
- val & PLANE_WM_BLOCKS_MASK;
- active->wm[level].plane_res_l[i] =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
+ active->planes[i].wm[level].plane_en = is_enabled;
+ active->planes[i].wm[level].plane_res_b =
+ val & PLANE_WM_BLOCKS_MASK;
+ active->planes[i].wm[level].plane_res_l =
+ (val >> PLANE_WM_LINES_SHIFT) &
+ PLANE_WM_LINES_MASK;
} else {
- active->wm[level].plane_en[PLANE_CURSOR] = is_enabled;
- active->wm[level].plane_res_b[PLANE_CURSOR] =
- val & PLANE_WM_BLOCKS_MASK;
- active->wm[level].plane_res_l[PLANE_CURSOR] =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
+ active->planes[PLANE_CURSOR].wm[level].plane_en =
+ is_enabled;
+ active->planes[PLANE_CURSOR].wm[level].plane_res_b =
+ val & PLANE_WM_BLOCKS_MASK;
+ active->planes[PLANE_CURSOR].wm[level].plane_res_l =
+ (val >> PLANE_WM_LINES_SHIFT) &
+ PLANE_WM_LINES_MASK;
}
} else {
if (!is_cursor) {
- active->trans_wm.plane_en[i] = is_enabled;
- active->trans_wm.plane_res_b[i] =
- val & PLANE_WM_BLOCKS_MASK;
- active->trans_wm.plane_res_l[i] =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
+ active->planes[i].trans_wm.plane_en = is_enabled;
+ active->planes[i].trans_wm.plane_res_b =
+ val & PLANE_WM_BLOCKS_MASK;
+ active->planes[i].trans_wm.plane_res_l =
+ (val >> PLANE_WM_LINES_SHIFT) &
+ PLANE_WM_LINES_MASK;
} else {
- active->trans_wm.plane_en[PLANE_CURSOR] = is_enabled;
- active->trans_wm.plane_res_b[PLANE_CURSOR] =
- val & PLANE_WM_BLOCKS_MASK;
- active->trans_wm.plane_res_l[PLANE_CURSOR] =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
+ active->planes[PLANE_CURSOR].trans_wm.plane_en =
+ is_enabled;
+ active->planes[PLANE_CURSOR].trans_wm.plane_res_b =
+ val & PLANE_WM_BLOCKS_MASK;
+ active->planes[PLANE_CURSOR].trans_wm.plane_res_l =
+ (val >> PLANE_WM_LINES_SHIFT) &
+ PLANE_WM_LINES_MASK;
}
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 03/10] drm/i915/gen9: Make skl_wm_level per-plane
@ 2016-10-14 21:31 ` Lyude
0 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx
Cc: Paulo Zanoni, David Airlie, linux-kernel, dri-devel, Daniel Vetter
Having skl_wm_level contain all of the watermarks for each plane is
annoying since it prevents us from having any sort of object to
represent a single watermark level, something we take advantage of in
the next commit to cut down on all of the copy paste code in here.
Changes since v1:
- Style nitpicks
- Fix accidental usage of i vs. PLANE_CURSOR
- Split out skl_pipe_wm_active_state simplification into seperate patch
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 6 +-
drivers/gpu/drm/i915/intel_drv.h | 6 +-
drivers/gpu/drm/i915/intel_pm.c | 207 +++++++++++++++++++--------------------
3 files changed, 111 insertions(+), 108 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e56d4a4..4d1133f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1653,9 +1653,9 @@ struct skl_wm_values {
};
struct skl_wm_level {
- bool plane_en[I915_MAX_PLANES];
- uint16_t plane_res_b[I915_MAX_PLANES];
- uint8_t plane_res_l[I915_MAX_PLANES];
+ bool plane_en;
+ uint16_t plane_res_b;
+ uint8_t plane_res_l;
};
/*
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 10a0cf2..84301d3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -468,9 +468,13 @@ struct intel_pipe_wm {
bool sprites_scaled;
};
-struct skl_pipe_wm {
+struct skl_plane_wm {
struct skl_wm_level wm[8];
struct skl_wm_level trans_wm;
+};
+
+struct skl_pipe_wm {
+ struct skl_plane_wm planes[I915_MAX_PLANES];
uint32_t linetime;
};
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a7d5721..1bdccc9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3674,67 +3674,52 @@ static int
skl_compute_wm_level(const struct drm_i915_private *dev_priv,
struct skl_ddb_allocation *ddb,
struct intel_crtc_state *cstate,
+ struct intel_plane *intel_plane,
int level,
struct skl_wm_level *result)
{
struct drm_atomic_state *state = cstate->base.state;
struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
- struct drm_plane *plane;
- struct intel_plane *intel_plane;
- struct intel_plane_state *intel_pstate;
+ struct drm_plane *plane = &intel_plane->base;
+ struct intel_plane_state *intel_pstate = NULL;
uint16_t ddb_blocks;
enum pipe pipe = intel_crtc->pipe;
int ret;
+ int i = skl_wm_plane_id(intel_plane);
+
+ if (state)
+ intel_pstate =
+ intel_atomic_get_existing_plane_state(state,
+ intel_plane);
/*
- * We'll only calculate watermarks for planes that are actually
- * enabled, so make sure all other planes are set as disabled.
+ * Note: If we start supporting multiple pending atomic commits against
+ * the same planes/CRTC's in the future, plane->state will no longer be
+ * the correct pre-state to use for the calculations here and we'll
+ * need to change where we get the 'unchanged' plane data from.
+ *
+ * For now this is fine because we only allow one queued commit against
+ * a CRTC. Even if the plane isn't modified by this transaction and we
+ * don't have a plane lock, we still have the CRTC's lock, so we know
+ * that no other transactions are racing with us to update it.
*/
- memset(result, 0, sizeof(*result));
+ if (!intel_pstate)
+ intel_pstate = to_intel_plane_state(plane->state);
- for_each_intel_plane_mask(&dev_priv->drm,
- intel_plane,
- cstate->base.plane_mask) {
- int i = skl_wm_plane_id(intel_plane);
-
- plane = &intel_plane->base;
- intel_pstate = NULL;
- if (state)
- intel_pstate =
- intel_atomic_get_existing_plane_state(state,
- intel_plane);
+ WARN_ON(!intel_pstate->base.fb);
- /*
- * Note: If we start supporting multiple pending atomic commits
- * against the same planes/CRTC's in the future, plane->state
- * will no longer be the correct pre-state to use for the
- * calculations here and we'll need to change where we get the
- * 'unchanged' plane data from.
- *
- * For now this is fine because we only allow one queued commit
- * against a CRTC. Even if the plane isn't modified by this
- * transaction and we don't have a plane lock, we still have
- * the CRTC's lock, so we know that no other transactions are
- * racing with us to update it.
- */
- if (!intel_pstate)
- intel_pstate = to_intel_plane_state(plane->state);
+ ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][i]);
- WARN_ON(!intel_pstate->base.fb);
-
- ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][i]);
-
- ret = skl_compute_plane_wm(dev_priv,
- cstate,
- intel_pstate,
- ddb_blocks,
- level,
- &result->plane_res_b[i],
- &result->plane_res_l[i],
- &result->plane_en[i]);
- if (ret)
- return ret;
- }
+ ret = skl_compute_plane_wm(dev_priv,
+ cstate,
+ intel_pstate,
+ ddb_blocks,
+ level,
+ &result->plane_res_b,
+ &result->plane_res_l,
+ &result->plane_en);
+ if (ret)
+ return ret;
return 0;
}
@@ -3755,19 +3740,11 @@ skl_compute_linetime_wm(struct intel_crtc_state *cstate)
static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
struct skl_wm_level *trans_wm /* out */)
{
- struct drm_crtc *crtc = cstate->base.crtc;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_plane *intel_plane;
-
if (!cstate->base.active)
return;
/* Until we know more, just disable transition WMs */
- for_each_intel_plane_on_crtc(crtc->dev, intel_crtc, intel_plane) {
- int i = skl_wm_plane_id(intel_plane);
-
- trans_wm->plane_en[i] = false;
- }
+ trans_wm->plane_en = false;
}
static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
@@ -3776,19 +3753,33 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
{
struct drm_device *dev = cstate->base.crtc->dev;
const struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_plane *intel_plane;
+ struct skl_plane_wm *wm;
int level, max_level = ilk_wm_max_level(dev_priv);
int ret;
- for (level = 0; level <= max_level; level++) {
- ret = skl_compute_wm_level(dev_priv, ddb, cstate,
- level, &pipe_wm->wm[level]);
- if (ret)
- return ret;
+ /*
+ * We'll only calculate watermarks for planes that are actually
+ * enabled, so make sure all other planes are set as disabled.
+ */
+ memset(pipe_wm->planes, 0, sizeof(pipe_wm->planes));
+
+ for_each_intel_plane_mask(&dev_priv->drm,
+ intel_plane,
+ cstate->base.plane_mask) {
+ wm = &pipe_wm->planes[skl_wm_plane_id(intel_plane)];
+
+ for (level = 0; level <= max_level; level++) {
+ ret = skl_compute_wm_level(dev_priv, ddb, cstate,
+ intel_plane, level,
+ &wm->wm[level]);
+ if (ret)
+ return ret;
+ }
+ skl_compute_transition_wm(cstate, &wm->trans_wm);
}
pipe_wm->linetime = skl_compute_linetime_wm(cstate);
- skl_compute_transition_wm(cstate, &pipe_wm->trans_wm);
-
return 0;
}
@@ -3798,50 +3789,56 @@ static void skl_compute_wm_results(struct drm_device *dev,
struct intel_crtc *intel_crtc)
{
int level, max_level = ilk_wm_max_level(to_i915(dev));
+ struct skl_plane_wm *plane_wm;
enum pipe pipe = intel_crtc->pipe;
uint32_t temp;
int i;
- for (level = 0; level <= max_level; level++) {
- for (i = 0; i < intel_num_planes(intel_crtc); i++) {
+ for (i = 0; i < intel_num_planes(intel_crtc); i++) {
+ plane_wm = &p_wm->planes[i];
+
+ for (level = 0; level <= max_level; level++) {
temp = 0;
- temp |= p_wm->wm[level].plane_res_l[i] <<
+ temp |= plane_wm->wm[level].plane_res_l <<
PLANE_WM_LINES_SHIFT;
- temp |= p_wm->wm[level].plane_res_b[i];
- if (p_wm->wm[level].plane_en[i])
+ temp |= plane_wm->wm[level].plane_res_b;
+ if (plane_wm->wm[level].plane_en)
temp |= PLANE_WM_EN;
r->plane[pipe][i][level] = temp;
}
- temp = 0;
-
- temp |= p_wm->wm[level].plane_res_l[PLANE_CURSOR] << PLANE_WM_LINES_SHIFT;
- temp |= p_wm->wm[level].plane_res_b[PLANE_CURSOR];
+ }
- if (p_wm->wm[level].plane_en[PLANE_CURSOR])
+ for (level = 0; level <= max_level; level++) {
+ plane_wm = &p_wm->planes[PLANE_CURSOR];
+ temp = 0;
+ temp |= plane_wm->wm[level].plane_res_l << PLANE_WM_LINES_SHIFT;
+ temp |= plane_wm->wm[level].plane_res_b;
+ if (plane_wm->wm[level].plane_en)
temp |= PLANE_WM_EN;
r->plane[pipe][PLANE_CURSOR][level] = temp;
-
}
/* transition WMs */
for (i = 0; i < intel_num_planes(intel_crtc); i++) {
+ plane_wm = &p_wm->planes[i];
temp = 0;
- temp |= p_wm->trans_wm.plane_res_l[i] << PLANE_WM_LINES_SHIFT;
- temp |= p_wm->trans_wm.plane_res_b[i];
- if (p_wm->trans_wm.plane_en[i])
+ temp |= plane_wm->trans_wm.plane_res_l << PLANE_WM_LINES_SHIFT;
+ temp |= plane_wm->trans_wm.plane_res_b;
+ if (plane_wm->trans_wm.plane_en)
temp |= PLANE_WM_EN;
r->plane_trans[pipe][i] = temp;
}
+ plane_wm = &p_wm->planes[PLANE_CURSOR];
temp = 0;
- temp |= p_wm->trans_wm.plane_res_l[PLANE_CURSOR] << PLANE_WM_LINES_SHIFT;
- temp |= p_wm->trans_wm.plane_res_b[PLANE_CURSOR];
- if (p_wm->trans_wm.plane_en[PLANE_CURSOR])
+ temp |= plane_wm->trans_wm.plane_res_l << PLANE_WM_LINES_SHIFT;
+ temp |= plane_wm->trans_wm.plane_res_b;
+ if (plane_wm->trans_wm.plane_en)
temp |= PLANE_WM_EN;
r->plane_trans[pipe][PLANE_CURSOR] = temp;
@@ -4282,35 +4279,37 @@ static void skl_pipe_wm_active_state(uint32_t val,
if (!is_transwm) {
if (!is_cursor) {
- active->wm[level].plane_en[i] = is_enabled;
- active->wm[level].plane_res_b[i] =
- val & PLANE_WM_BLOCKS_MASK;
- active->wm[level].plane_res_l[i] =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
+ active->planes[i].wm[level].plane_en = is_enabled;
+ active->planes[i].wm[level].plane_res_b =
+ val & PLANE_WM_BLOCKS_MASK;
+ active->planes[i].wm[level].plane_res_l =
+ (val >> PLANE_WM_LINES_SHIFT) &
+ PLANE_WM_LINES_MASK;
} else {
- active->wm[level].plane_en[PLANE_CURSOR] = is_enabled;
- active->wm[level].plane_res_b[PLANE_CURSOR] =
- val & PLANE_WM_BLOCKS_MASK;
- active->wm[level].plane_res_l[PLANE_CURSOR] =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
+ active->planes[PLANE_CURSOR].wm[level].plane_en =
+ is_enabled;
+ active->planes[PLANE_CURSOR].wm[level].plane_res_b =
+ val & PLANE_WM_BLOCKS_MASK;
+ active->planes[PLANE_CURSOR].wm[level].plane_res_l =
+ (val >> PLANE_WM_LINES_SHIFT) &
+ PLANE_WM_LINES_MASK;
}
} else {
if (!is_cursor) {
- active->trans_wm.plane_en[i] = is_enabled;
- active->trans_wm.plane_res_b[i] =
- val & PLANE_WM_BLOCKS_MASK;
- active->trans_wm.plane_res_l[i] =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
+ active->planes[i].trans_wm.plane_en = is_enabled;
+ active->planes[i].trans_wm.plane_res_b =
+ val & PLANE_WM_BLOCKS_MASK;
+ active->planes[i].trans_wm.plane_res_l =
+ (val >> PLANE_WM_LINES_SHIFT) &
+ PLANE_WM_LINES_MASK;
} else {
- active->trans_wm.plane_en[PLANE_CURSOR] = is_enabled;
- active->trans_wm.plane_res_b[PLANE_CURSOR] =
- val & PLANE_WM_BLOCKS_MASK;
- active->trans_wm.plane_res_l[PLANE_CURSOR] =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
+ active->planes[PLANE_CURSOR].trans_wm.plane_en =
+ is_enabled;
+ active->planes[PLANE_CURSOR].trans_wm.plane_res_b =
+ val & PLANE_WM_BLOCKS_MASK;
+ active->planes[PLANE_CURSOR].trans_wm.plane_res_l =
+ (val >> PLANE_WM_LINES_SHIFT) &
+ PLANE_WM_LINES_MASK;
}
}
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 04/10] drm/i915/gen9: Cleanup skl_pipe_wm_active_state
2016-10-14 21:31 ` Lyude
@ 2016-10-14 21:31 ` Lyude
-1 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx
Cc: Lyude, Ville Syrjälä,
Matt Roper, Paulo Zanoni, Daniel Vetter, Jani Nikula,
David Airlie, dri-devel, linux-kernel
This function is a wreck, let's help it get its life back together and
cleanup all of the copy pasta here.
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 52 +++++++++++------------------------------
1 file changed, 14 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1bdccc9..2d63a37 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4271,46 +4271,22 @@ static void ilk_optimize_watermarks(struct intel_crtc_state *cstate)
static void skl_pipe_wm_active_state(uint32_t val,
struct skl_pipe_wm *active,
bool is_transwm,
- bool is_cursor,
int i,
int level)
{
+ struct skl_plane_wm *plane_wm = &active->planes[i];
bool is_enabled = (val & PLANE_WM_EN) != 0;
if (!is_transwm) {
- if (!is_cursor) {
- active->planes[i].wm[level].plane_en = is_enabled;
- active->planes[i].wm[level].plane_res_b =
- val & PLANE_WM_BLOCKS_MASK;
- active->planes[i].wm[level].plane_res_l =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
- } else {
- active->planes[PLANE_CURSOR].wm[level].plane_en =
- is_enabled;
- active->planes[PLANE_CURSOR].wm[level].plane_res_b =
- val & PLANE_WM_BLOCKS_MASK;
- active->planes[PLANE_CURSOR].wm[level].plane_res_l =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
- }
+ plane_wm->wm[level].plane_en = is_enabled;
+ plane_wm->wm[level].plane_res_b = val & PLANE_WM_BLOCKS_MASK;
+ plane_wm->wm[level].plane_res_l =
+ (val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
} else {
- if (!is_cursor) {
- active->planes[i].trans_wm.plane_en = is_enabled;
- active->planes[i].trans_wm.plane_res_b =
- val & PLANE_WM_BLOCKS_MASK;
- active->planes[i].trans_wm.plane_res_l =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
- } else {
- active->planes[PLANE_CURSOR].trans_wm.plane_en =
- is_enabled;
- active->planes[PLANE_CURSOR].trans_wm.plane_res_b =
- val & PLANE_WM_BLOCKS_MASK;
- active->planes[PLANE_CURSOR].trans_wm.plane_res_l =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
- }
+ plane_wm->trans_wm.plane_en = is_enabled;
+ plane_wm->trans_wm.plane_res_b = val & PLANE_WM_BLOCKS_MASK;
+ plane_wm->trans_wm.plane_res_l =
+ (val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
}
}
@@ -4349,20 +4325,20 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
for (level = 0; level <= max_level; level++) {
for (i = 0; i < intel_num_planes(intel_crtc); i++) {
temp = hw->plane[pipe][i][level];
- skl_pipe_wm_active_state(temp, active, false,
- false, i, level);
+ skl_pipe_wm_active_state(temp, active, false, i, level);
}
temp = hw->plane[pipe][PLANE_CURSOR][level];
- skl_pipe_wm_active_state(temp, active, false, true, i, level);
+ skl_pipe_wm_active_state(temp, active, false, PLANE_CURSOR,
+ level);
}
for (i = 0; i < intel_num_planes(intel_crtc); i++) {
temp = hw->plane_trans[pipe][i];
- skl_pipe_wm_active_state(temp, active, true, false, i, 0);
+ skl_pipe_wm_active_state(temp, active, true, i, 0);
}
temp = hw->plane_trans[pipe][PLANE_CURSOR];
- skl_pipe_wm_active_state(temp, active, true, true, i, 0);
+ skl_pipe_wm_active_state(temp, active, true, PLANE_CURSOR, 0);
intel_crtc->wm.active.skl = *active;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 04/10] drm/i915/gen9: Cleanup skl_pipe_wm_active_state
@ 2016-10-14 21:31 ` Lyude
0 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, linux-kernel, dri-devel, Daniel Vetter, Lyude
This function is a wreck, let's help it get its life back together and
cleanup all of the copy pasta here.
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 52 +++++++++++------------------------------
1 file changed, 14 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1bdccc9..2d63a37 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4271,46 +4271,22 @@ static void ilk_optimize_watermarks(struct intel_crtc_state *cstate)
static void skl_pipe_wm_active_state(uint32_t val,
struct skl_pipe_wm *active,
bool is_transwm,
- bool is_cursor,
int i,
int level)
{
+ struct skl_plane_wm *plane_wm = &active->planes[i];
bool is_enabled = (val & PLANE_WM_EN) != 0;
if (!is_transwm) {
- if (!is_cursor) {
- active->planes[i].wm[level].plane_en = is_enabled;
- active->planes[i].wm[level].plane_res_b =
- val & PLANE_WM_BLOCKS_MASK;
- active->planes[i].wm[level].plane_res_l =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
- } else {
- active->planes[PLANE_CURSOR].wm[level].plane_en =
- is_enabled;
- active->planes[PLANE_CURSOR].wm[level].plane_res_b =
- val & PLANE_WM_BLOCKS_MASK;
- active->planes[PLANE_CURSOR].wm[level].plane_res_l =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
- }
+ plane_wm->wm[level].plane_en = is_enabled;
+ plane_wm->wm[level].plane_res_b = val & PLANE_WM_BLOCKS_MASK;
+ plane_wm->wm[level].plane_res_l =
+ (val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
} else {
- if (!is_cursor) {
- active->planes[i].trans_wm.plane_en = is_enabled;
- active->planes[i].trans_wm.plane_res_b =
- val & PLANE_WM_BLOCKS_MASK;
- active->planes[i].trans_wm.plane_res_l =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
- } else {
- active->planes[PLANE_CURSOR].trans_wm.plane_en =
- is_enabled;
- active->planes[PLANE_CURSOR].trans_wm.plane_res_b =
- val & PLANE_WM_BLOCKS_MASK;
- active->planes[PLANE_CURSOR].trans_wm.plane_res_l =
- (val >> PLANE_WM_LINES_SHIFT) &
- PLANE_WM_LINES_MASK;
- }
+ plane_wm->trans_wm.plane_en = is_enabled;
+ plane_wm->trans_wm.plane_res_b = val & PLANE_WM_BLOCKS_MASK;
+ plane_wm->trans_wm.plane_res_l =
+ (val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
}
}
@@ -4349,20 +4325,20 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
for (level = 0; level <= max_level; level++) {
for (i = 0; i < intel_num_planes(intel_crtc); i++) {
temp = hw->plane[pipe][i][level];
- skl_pipe_wm_active_state(temp, active, false,
- false, i, level);
+ skl_pipe_wm_active_state(temp, active, false, i, level);
}
temp = hw->plane[pipe][PLANE_CURSOR][level];
- skl_pipe_wm_active_state(temp, active, false, true, i, level);
+ skl_pipe_wm_active_state(temp, active, false, PLANE_CURSOR,
+ level);
}
for (i = 0; i < intel_num_planes(intel_crtc); i++) {
temp = hw->plane_trans[pipe][i];
- skl_pipe_wm_active_state(temp, active, true, false, i, 0);
+ skl_pipe_wm_active_state(temp, active, true, i, 0);
}
temp = hw->plane_trans[pipe][PLANE_CURSOR];
- skl_pipe_wm_active_state(temp, active, true, true, i, 0);
+ skl_pipe_wm_active_state(temp, active, true, PLANE_CURSOR, 0);
intel_crtc->wm.active.skl = *active;
}
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 05/10] drm/i915/gen9: Get rid of redundant watermark values
2016-10-14 21:31 ` Lyude
@ 2016-10-14 21:31 ` Lyude
-1 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx
Cc: Lyude, Ville Syrjälä,
Matt Roper, Paulo Zanoni, Daniel Vetter, Jani Nikula,
David Airlie, dri-devel, linux-kernel
Now that we've make skl_wm_levels make a little more sense, we can
remove all of the redundant wm information. Up until now we'd been
storing two copies of all of the skl watermarks: one being the
skl_pipe_wm structs, the other being the global wm struct in
drm_i915_private containing the raw register values. This is confusing
and problematic, since it means we're prone to accidentally letting the
two copies go out of sync. So, get rid of all of the functions
responsible for computing the register values and just use a single
helper, skl_write_wm_level(), to convert and write the new watermarks on
the fly.
Changes since v1:
- Fixup skl_write_wm_level()
- Fixup skl_wm_level_from_reg_val()
- Don't forget to copy *active to intel_crtc->wm.active.skl
Changes since v2:
- Fix usage of wrong cstate
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 -
drivers/gpu/drm/i915/intel_display.c | 14 ++-
drivers/gpu/drm/i915/intel_drv.h | 6 +-
drivers/gpu/drm/i915/intel_pm.c | 204 ++++++++++++-----------------------
drivers/gpu/drm/i915/intel_sprite.c | 8 +-
5 files changed, 91 insertions(+), 143 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4d1133f..929b643 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1648,8 +1648,6 @@ struct skl_ddb_allocation {
struct skl_wm_values {
unsigned dirty_pipes;
struct skl_ddb_allocation ddb;
- uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
- uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
};
struct skl_wm_level {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ced70ad..18e82b4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3378,6 +3378,8 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_framebuffer *fb = plane_state->base.fb;
const struct skl_wm_values *wm = &dev_priv->wm.skl_results;
+ const struct skl_plane_wm *p_wm =
+ &crtc_state->wm.skl.optimal.planes[0];
int pipe = intel_crtc->pipe;
u32 plane_ctl;
unsigned int rotation = plane_state->base.rotation;
@@ -3414,7 +3416,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
intel_crtc->adjusted_y = src_y;
if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base))
- skl_write_plane_wm(intel_crtc, wm, 0);
+ skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, 0);
I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x);
@@ -3448,6 +3450,8 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+ const struct skl_plane_wm *p_wm = &cstate->wm.skl.optimal.planes[0];
int pipe = intel_crtc->pipe;
/*
@@ -3455,7 +3459,8 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
* plane's visiblity isn't actually changing neither is its watermarks.
*/
if (!crtc->primary->state->visible)
- skl_write_plane_wm(intel_crtc, &dev_priv->wm.skl_results, 0);
+ skl_write_plane_wm(intel_crtc, p_wm,
+ &dev_priv->wm.skl_results.ddb, 0);
I915_WRITE(PLANE_CTL(pipe, 0), 0);
I915_WRITE(PLANE_SURF(pipe, 0), 0);
@@ -10826,12 +10831,15 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
const struct skl_wm_values *wm = &dev_priv->wm.skl_results;
+ const struct skl_plane_wm *p_wm =
+ &cstate->wm.skl.optimal.planes[PLANE_CURSOR];
int pipe = intel_crtc->pipe;
uint32_t cntl = 0;
if (INTEL_GEN(dev_priv) >= 9 && wm->dirty_pipes & drm_crtc_mask(crtc))
- skl_write_cursor_wm(intel_crtc, wm);
+ skl_write_cursor_wm(intel_crtc, p_wm, &wm->ddb);
if (plane_state && plane_state->base.visible) {
cntl = MCURSOR_GAMMA_ENABLE;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 84301d3..a85ce2c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1765,9 +1765,11 @@ bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
bool skl_ddb_allocation_overlaps(struct drm_atomic_state *state,
struct intel_crtc *intel_crtc);
void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
- const struct skl_wm_values *wm);
+ const struct skl_plane_wm *wm,
+ const struct skl_ddb_allocation *ddb);
void skl_write_plane_wm(struct intel_crtc *intel_crtc,
- const struct skl_wm_values *wm,
+ const struct skl_plane_wm *wm,
+ const struct skl_ddb_allocation *ddb,
int plane);
uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
bool ilk_disable_lp_wm(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2d63a37..5df5cea 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3004,6 +3004,8 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
struct drm_crtc *crtc;
+ struct intel_crtc_state *cstate;
+ struct skl_plane_wm *wm;
enum pipe pipe;
int level, plane;
@@ -3024,19 +3026,22 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
/* Since we're now guaranteed to only have one active CRTC... */
pipe = ffs(intel_state->active_crtcs) - 1;
crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+ cstate = to_intel_crtc_state(crtc->state);
if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE)
return false;
for_each_plane(dev_priv, pipe, plane) {
+ wm = &cstate->wm.skl.optimal.planes[plane];
+
/* Skip this plane if it's not enabled */
- if (intel_state->wm_results.plane[pipe][plane][0] == 0)
+ if (!wm->wm[0].plane_en)
continue;
/* Find the highest enabled wm level for this plane */
for (level = ilk_wm_max_level(dev_priv);
- intel_state->wm_results.plane[pipe][plane][level] == 0; --level)
- { }
+ !wm->wm[level].plane_en;
+ --level) { }
/*
* If any of the planes on this pipe don't enable wm levels
@@ -3783,67 +3788,6 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
return 0;
}
-static void skl_compute_wm_results(struct drm_device *dev,
- struct skl_pipe_wm *p_wm,
- struct skl_wm_values *r,
- struct intel_crtc *intel_crtc)
-{
- int level, max_level = ilk_wm_max_level(to_i915(dev));
- struct skl_plane_wm *plane_wm;
- enum pipe pipe = intel_crtc->pipe;
- uint32_t temp;
- int i;
-
- for (i = 0; i < intel_num_planes(intel_crtc); i++) {
- plane_wm = &p_wm->planes[i];
-
- for (level = 0; level <= max_level; level++) {
- temp = 0;
-
- temp |= plane_wm->wm[level].plane_res_l <<
- PLANE_WM_LINES_SHIFT;
- temp |= plane_wm->wm[level].plane_res_b;
- if (plane_wm->wm[level].plane_en)
- temp |= PLANE_WM_EN;
-
- r->plane[pipe][i][level] = temp;
- }
-
- }
-
- for (level = 0; level <= max_level; level++) {
- plane_wm = &p_wm->planes[PLANE_CURSOR];
- temp = 0;
- temp |= plane_wm->wm[level].plane_res_l << PLANE_WM_LINES_SHIFT;
- temp |= plane_wm->wm[level].plane_res_b;
- if (plane_wm->wm[level].plane_en)
- temp |= PLANE_WM_EN;
-
- r->plane[pipe][PLANE_CURSOR][level] = temp;
- }
-
- /* transition WMs */
- for (i = 0; i < intel_num_planes(intel_crtc); i++) {
- plane_wm = &p_wm->planes[i];
- temp = 0;
- temp |= plane_wm->trans_wm.plane_res_l << PLANE_WM_LINES_SHIFT;
- temp |= plane_wm->trans_wm.plane_res_b;
- if (plane_wm->trans_wm.plane_en)
- temp |= PLANE_WM_EN;
-
- r->plane_trans[pipe][i] = temp;
- }
-
- plane_wm = &p_wm->planes[PLANE_CURSOR];
- temp = 0;
- temp |= plane_wm->trans_wm.plane_res_l << PLANE_WM_LINES_SHIFT;
- temp |= plane_wm->trans_wm.plane_res_b;
- if (plane_wm->trans_wm.plane_en)
- temp |= PLANE_WM_EN;
-
- r->plane_trans[pipe][PLANE_CURSOR] = temp;
-}
-
static void skl_ddb_entry_write(struct drm_i915_private *dev_priv,
i915_reg_t reg,
const struct skl_ddb_entry *entry)
@@ -3854,8 +3798,24 @@ static void skl_ddb_entry_write(struct drm_i915_private *dev_priv,
I915_WRITE(reg, 0);
}
+static void skl_write_wm_level(struct drm_i915_private *dev_priv,
+ i915_reg_t reg,
+ const struct skl_wm_level *level)
+{
+ uint32_t val = 0;
+
+ if (level->plane_en) {
+ val |= PLANE_WM_EN;
+ val |= level->plane_res_b;
+ val |= level->plane_res_l << PLANE_WM_LINES_SHIFT;
+ }
+
+ I915_WRITE(reg, val);
+}
+
void skl_write_plane_wm(struct intel_crtc *intel_crtc,
- const struct skl_wm_values *wm,
+ const struct skl_plane_wm *wm,
+ const struct skl_ddb_allocation *ddb,
int plane)
{
struct drm_crtc *crtc = &intel_crtc->base;
@@ -3865,19 +3825,21 @@ void skl_write_plane_wm(struct intel_crtc *intel_crtc,
enum pipe pipe = intel_crtc->pipe;
for (level = 0; level <= max_level; level++) {
- I915_WRITE(PLANE_WM(pipe, plane, level),
- wm->plane[pipe][plane][level]);
+ skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane, level),
+ &wm->wm[level]);
}
- I915_WRITE(PLANE_WM_TRANS(pipe, plane), wm->plane_trans[pipe][plane]);
+ skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane),
+ &wm->trans_wm);
skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane),
- &wm->ddb.plane[pipe][plane]);
+ &ddb->plane[pipe][plane]);
skl_ddb_entry_write(dev_priv, PLANE_NV12_BUF_CFG(pipe, plane),
- &wm->ddb.y_plane[pipe][plane]);
+ &ddb->y_plane[pipe][plane]);
}
void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
- const struct skl_wm_values *wm)
+ const struct skl_plane_wm *wm,
+ const struct skl_ddb_allocation *ddb)
{
struct drm_crtc *crtc = &intel_crtc->base;
struct drm_device *dev = crtc->dev;
@@ -3886,13 +3848,13 @@ void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
enum pipe pipe = intel_crtc->pipe;
for (level = 0; level <= max_level; level++) {
- I915_WRITE(CUR_WM(pipe, level),
- wm->plane[pipe][PLANE_CURSOR][level]);
+ skl_write_wm_level(dev_priv, CUR_WM(pipe, level),
+ &wm->wm[level]);
}
- I915_WRITE(CUR_WM_TRANS(pipe), wm->plane_trans[pipe][PLANE_CURSOR]);
+ skl_write_wm_level(dev_priv, CUR_WM_TRANS(pipe), &wm->trans_wm);
skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe),
- &wm->ddb.plane[pipe][PLANE_CURSOR]);
+ &ddb->plane[pipe][PLANE_CURSOR]);
}
static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
@@ -4076,11 +4038,6 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
struct skl_wm_values *src,
enum pipe pipe)
{
- memcpy(dst->plane[pipe], src->plane[pipe],
- sizeof(dst->plane[pipe]));
- memcpy(dst->plane_trans[pipe], src->plane_trans[pipe],
- sizeof(dst->plane_trans[pipe]));
-
memcpy(dst->ddb.y_plane[pipe], src->ddb.y_plane[pipe],
sizeof(dst->ddb.y_plane[pipe]));
memcpy(dst->ddb.plane[pipe], src->ddb.plane[pipe],
@@ -4129,7 +4086,6 @@ skl_compute_wm(struct drm_atomic_state *state)
* no suitable watermark values can be found.
*/
for_each_crtc_in_state(state, crtc, cstate, i) {
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_crtc_state *intel_cstate =
to_intel_crtc_state(cstate);
@@ -4147,7 +4103,6 @@ skl_compute_wm(struct drm_atomic_state *state)
continue;
intel_cstate->update_wm_pre = true;
- skl_compute_wm_results(crtc->dev, pipe_wm, results, intel_crtc);
}
return 0;
@@ -4181,9 +4136,11 @@ static void skl_update_wm(struct drm_crtc *crtc)
int plane;
for (plane = 0; plane < intel_num_planes(intel_crtc); plane++)
- skl_write_plane_wm(intel_crtc, results, plane);
+ skl_write_plane_wm(intel_crtc, &pipe_wm->planes[plane],
+ &results->ddb, plane);
- skl_write_cursor_wm(intel_crtc, results);
+ skl_write_cursor_wm(intel_crtc, &pipe_wm->planes[PLANE_CURSOR],
+ &results->ddb);
}
skl_copy_wm_for_pipe(hw_vals, results, pipe);
@@ -4268,26 +4225,13 @@ static void ilk_optimize_watermarks(struct intel_crtc_state *cstate)
mutex_unlock(&dev_priv->wm.wm_mutex);
}
-static void skl_pipe_wm_active_state(uint32_t val,
- struct skl_pipe_wm *active,
- bool is_transwm,
- int i,
- int level)
+static inline void skl_wm_level_from_reg_val(uint32_t val,
+ struct skl_wm_level *level)
{
- struct skl_plane_wm *plane_wm = &active->planes[i];
- bool is_enabled = (val & PLANE_WM_EN) != 0;
-
- if (!is_transwm) {
- plane_wm->wm[level].plane_en = is_enabled;
- plane_wm->wm[level].plane_res_b = val & PLANE_WM_BLOCKS_MASK;
- plane_wm->wm[level].plane_res_l =
- (val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
- } else {
- plane_wm->trans_wm.plane_en = is_enabled;
- plane_wm->trans_wm.plane_res_b = val & PLANE_WM_BLOCKS_MASK;
- plane_wm->trans_wm.plane_res_l =
- (val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
- }
+ level->plane_en = val & PLANE_WM_EN;
+ level->plane_res_b = val & PLANE_WM_BLOCKS_MASK;
+ level->plane_res_l = (val >> PLANE_WM_LINES_SHIFT) &
+ PLANE_WM_LINES_MASK;
}
static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
@@ -4297,49 +4241,41 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+ struct intel_plane *intel_plane;
struct skl_pipe_wm *active = &cstate->wm.skl.optimal;
+ struct skl_plane_wm *wm;
enum pipe pipe = intel_crtc->pipe;
- int level, i, max_level;
- uint32_t temp;
+ int level, id, max_level;
+ uint32_t val;
max_level = ilk_wm_max_level(dev_priv);
- for (level = 0; level <= max_level; level++) {
- for (i = 0; i < intel_num_planes(intel_crtc); i++)
- hw->plane[pipe][i][level] =
- I915_READ(PLANE_WM(pipe, i, level));
- hw->plane[pipe][PLANE_CURSOR][level] = I915_READ(CUR_WM(pipe, level));
- }
-
- for (i = 0; i < intel_num_planes(intel_crtc); i++)
- hw->plane_trans[pipe][i] = I915_READ(PLANE_WM_TRANS(pipe, i));
- hw->plane_trans[pipe][PLANE_CURSOR] = I915_READ(CUR_WM_TRANS(pipe));
-
- if (!intel_crtc->active)
- return;
-
- hw->dirty_pipes |= drm_crtc_mask(crtc);
+ for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
+ id = skl_wm_plane_id(intel_plane);
+ wm = &cstate->wm.skl.optimal.planes[id];
- active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
+ for (level = 0; level <= max_level; level++) {
+ if (id != PLANE_CURSOR)
+ val = I915_READ(PLANE_WM(pipe, id, level));
+ else
+ val = I915_READ(CUR_WM(pipe, level));
- for (level = 0; level <= max_level; level++) {
- for (i = 0; i < intel_num_planes(intel_crtc); i++) {
- temp = hw->plane[pipe][i][level];
- skl_pipe_wm_active_state(temp, active, false, i, level);
+ skl_wm_level_from_reg_val(val, &wm->wm[level]);
}
- temp = hw->plane[pipe][PLANE_CURSOR][level];
- skl_pipe_wm_active_state(temp, active, false, PLANE_CURSOR,
- level);
- }
- for (i = 0; i < intel_num_planes(intel_crtc); i++) {
- temp = hw->plane_trans[pipe][i];
- skl_pipe_wm_active_state(temp, active, true, i, 0);
+ if (id != PLANE_CURSOR)
+ val = I915_READ(PLANE_WM_TRANS(pipe, id));
+ else
+ val = I915_READ(CUR_WM_TRANS(pipe));
+
+ skl_wm_level_from_reg_val(val, &wm->trans_wm);
}
- temp = hw->plane_trans[pipe][PLANE_CURSOR];
- skl_pipe_wm_active_state(temp, active, true, PLANE_CURSOR, 0);
+ if (!intel_crtc->active)
+ return;
+ hw->dirty_pipes |= drm_crtc_mask(crtc);
+ active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
intel_crtc->wm.active.skl = *active;
}
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index c7d9a20..e7d2cff 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -208,6 +208,8 @@ skl_update_plane(struct drm_plane *drm_plane,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
const int pipe = intel_plane->pipe;
const int plane = intel_plane->plane + 1;
+ const struct skl_plane_wm *p_wm =
+ &crtc_state->wm.skl.optimal.planes[plane];
u32 plane_ctl;
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
u32 surf_addr = plane_state->main.offset;
@@ -232,7 +234,7 @@ skl_update_plane(struct drm_plane *drm_plane,
plane_ctl |= skl_plane_ctl_rotation(rotation);
if (wm->dirty_pipes & drm_crtc_mask(crtc))
- skl_write_plane_wm(intel_crtc, wm, plane);
+ skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, plane);
if (key->flags) {
I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value);
@@ -289,6 +291,7 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
struct drm_device *dev = dplane->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_plane *intel_plane = to_intel_plane(dplane);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
const int pipe = intel_plane->pipe;
const int plane = intel_plane->plane + 1;
@@ -298,7 +301,8 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
*/
if (!dplane->state->visible)
skl_write_plane_wm(to_intel_crtc(crtc),
- &dev_priv->wm.skl_results, plane);
+ &cstate->wm.skl.optimal.planes[plane],
+ &dev_priv->wm.skl_results.ddb, plane);
I915_WRITE(PLANE_CTL(pipe, plane), 0);
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 05/10] drm/i915/gen9: Get rid of redundant watermark values
@ 2016-10-14 21:31 ` Lyude
0 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, linux-kernel, dri-devel, Daniel Vetter, Lyude
Now that we've make skl_wm_levels make a little more sense, we can
remove all of the redundant wm information. Up until now we'd been
storing two copies of all of the skl watermarks: one being the
skl_pipe_wm structs, the other being the global wm struct in
drm_i915_private containing the raw register values. This is confusing
and problematic, since it means we're prone to accidentally letting the
two copies go out of sync. So, get rid of all of the functions
responsible for computing the register values and just use a single
helper, skl_write_wm_level(), to convert and write the new watermarks on
the fly.
Changes since v1:
- Fixup skl_write_wm_level()
- Fixup skl_wm_level_from_reg_val()
- Don't forget to copy *active to intel_crtc->wm.active.skl
Changes since v2:
- Fix usage of wrong cstate
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 -
drivers/gpu/drm/i915/intel_display.c | 14 ++-
drivers/gpu/drm/i915/intel_drv.h | 6 +-
drivers/gpu/drm/i915/intel_pm.c | 204 ++++++++++++-----------------------
drivers/gpu/drm/i915/intel_sprite.c | 8 +-
5 files changed, 91 insertions(+), 143 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4d1133f..929b643 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1648,8 +1648,6 @@ struct skl_ddb_allocation {
struct skl_wm_values {
unsigned dirty_pipes;
struct skl_ddb_allocation ddb;
- uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
- uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
};
struct skl_wm_level {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ced70ad..18e82b4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3378,6 +3378,8 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_framebuffer *fb = plane_state->base.fb;
const struct skl_wm_values *wm = &dev_priv->wm.skl_results;
+ const struct skl_plane_wm *p_wm =
+ &crtc_state->wm.skl.optimal.planes[0];
int pipe = intel_crtc->pipe;
u32 plane_ctl;
unsigned int rotation = plane_state->base.rotation;
@@ -3414,7 +3416,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
intel_crtc->adjusted_y = src_y;
if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base))
- skl_write_plane_wm(intel_crtc, wm, 0);
+ skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, 0);
I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x);
@@ -3448,6 +3450,8 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+ const struct skl_plane_wm *p_wm = &cstate->wm.skl.optimal.planes[0];
int pipe = intel_crtc->pipe;
/*
@@ -3455,7 +3459,8 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
* plane's visiblity isn't actually changing neither is its watermarks.
*/
if (!crtc->primary->state->visible)
- skl_write_plane_wm(intel_crtc, &dev_priv->wm.skl_results, 0);
+ skl_write_plane_wm(intel_crtc, p_wm,
+ &dev_priv->wm.skl_results.ddb, 0);
I915_WRITE(PLANE_CTL(pipe, 0), 0);
I915_WRITE(PLANE_SURF(pipe, 0), 0);
@@ -10826,12 +10831,15 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
const struct skl_wm_values *wm = &dev_priv->wm.skl_results;
+ const struct skl_plane_wm *p_wm =
+ &cstate->wm.skl.optimal.planes[PLANE_CURSOR];
int pipe = intel_crtc->pipe;
uint32_t cntl = 0;
if (INTEL_GEN(dev_priv) >= 9 && wm->dirty_pipes & drm_crtc_mask(crtc))
- skl_write_cursor_wm(intel_crtc, wm);
+ skl_write_cursor_wm(intel_crtc, p_wm, &wm->ddb);
if (plane_state && plane_state->base.visible) {
cntl = MCURSOR_GAMMA_ENABLE;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 84301d3..a85ce2c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1765,9 +1765,11 @@ bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
bool skl_ddb_allocation_overlaps(struct drm_atomic_state *state,
struct intel_crtc *intel_crtc);
void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
- const struct skl_wm_values *wm);
+ const struct skl_plane_wm *wm,
+ const struct skl_ddb_allocation *ddb);
void skl_write_plane_wm(struct intel_crtc *intel_crtc,
- const struct skl_wm_values *wm,
+ const struct skl_plane_wm *wm,
+ const struct skl_ddb_allocation *ddb,
int plane);
uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
bool ilk_disable_lp_wm(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2d63a37..5df5cea 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3004,6 +3004,8 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
struct drm_crtc *crtc;
+ struct intel_crtc_state *cstate;
+ struct skl_plane_wm *wm;
enum pipe pipe;
int level, plane;
@@ -3024,19 +3026,22 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
/* Since we're now guaranteed to only have one active CRTC... */
pipe = ffs(intel_state->active_crtcs) - 1;
crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+ cstate = to_intel_crtc_state(crtc->state);
if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE)
return false;
for_each_plane(dev_priv, pipe, plane) {
+ wm = &cstate->wm.skl.optimal.planes[plane];
+
/* Skip this plane if it's not enabled */
- if (intel_state->wm_results.plane[pipe][plane][0] == 0)
+ if (!wm->wm[0].plane_en)
continue;
/* Find the highest enabled wm level for this plane */
for (level = ilk_wm_max_level(dev_priv);
- intel_state->wm_results.plane[pipe][plane][level] == 0; --level)
- { }
+ !wm->wm[level].plane_en;
+ --level) { }
/*
* If any of the planes on this pipe don't enable wm levels
@@ -3783,67 +3788,6 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
return 0;
}
-static void skl_compute_wm_results(struct drm_device *dev,
- struct skl_pipe_wm *p_wm,
- struct skl_wm_values *r,
- struct intel_crtc *intel_crtc)
-{
- int level, max_level = ilk_wm_max_level(to_i915(dev));
- struct skl_plane_wm *plane_wm;
- enum pipe pipe = intel_crtc->pipe;
- uint32_t temp;
- int i;
-
- for (i = 0; i < intel_num_planes(intel_crtc); i++) {
- plane_wm = &p_wm->planes[i];
-
- for (level = 0; level <= max_level; level++) {
- temp = 0;
-
- temp |= plane_wm->wm[level].plane_res_l <<
- PLANE_WM_LINES_SHIFT;
- temp |= plane_wm->wm[level].plane_res_b;
- if (plane_wm->wm[level].plane_en)
- temp |= PLANE_WM_EN;
-
- r->plane[pipe][i][level] = temp;
- }
-
- }
-
- for (level = 0; level <= max_level; level++) {
- plane_wm = &p_wm->planes[PLANE_CURSOR];
- temp = 0;
- temp |= plane_wm->wm[level].plane_res_l << PLANE_WM_LINES_SHIFT;
- temp |= plane_wm->wm[level].plane_res_b;
- if (plane_wm->wm[level].plane_en)
- temp |= PLANE_WM_EN;
-
- r->plane[pipe][PLANE_CURSOR][level] = temp;
- }
-
- /* transition WMs */
- for (i = 0; i < intel_num_planes(intel_crtc); i++) {
- plane_wm = &p_wm->planes[i];
- temp = 0;
- temp |= plane_wm->trans_wm.plane_res_l << PLANE_WM_LINES_SHIFT;
- temp |= plane_wm->trans_wm.plane_res_b;
- if (plane_wm->trans_wm.plane_en)
- temp |= PLANE_WM_EN;
-
- r->plane_trans[pipe][i] = temp;
- }
-
- plane_wm = &p_wm->planes[PLANE_CURSOR];
- temp = 0;
- temp |= plane_wm->trans_wm.plane_res_l << PLANE_WM_LINES_SHIFT;
- temp |= plane_wm->trans_wm.plane_res_b;
- if (plane_wm->trans_wm.plane_en)
- temp |= PLANE_WM_EN;
-
- r->plane_trans[pipe][PLANE_CURSOR] = temp;
-}
-
static void skl_ddb_entry_write(struct drm_i915_private *dev_priv,
i915_reg_t reg,
const struct skl_ddb_entry *entry)
@@ -3854,8 +3798,24 @@ static void skl_ddb_entry_write(struct drm_i915_private *dev_priv,
I915_WRITE(reg, 0);
}
+static void skl_write_wm_level(struct drm_i915_private *dev_priv,
+ i915_reg_t reg,
+ const struct skl_wm_level *level)
+{
+ uint32_t val = 0;
+
+ if (level->plane_en) {
+ val |= PLANE_WM_EN;
+ val |= level->plane_res_b;
+ val |= level->plane_res_l << PLANE_WM_LINES_SHIFT;
+ }
+
+ I915_WRITE(reg, val);
+}
+
void skl_write_plane_wm(struct intel_crtc *intel_crtc,
- const struct skl_wm_values *wm,
+ const struct skl_plane_wm *wm,
+ const struct skl_ddb_allocation *ddb,
int plane)
{
struct drm_crtc *crtc = &intel_crtc->base;
@@ -3865,19 +3825,21 @@ void skl_write_plane_wm(struct intel_crtc *intel_crtc,
enum pipe pipe = intel_crtc->pipe;
for (level = 0; level <= max_level; level++) {
- I915_WRITE(PLANE_WM(pipe, plane, level),
- wm->plane[pipe][plane][level]);
+ skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane, level),
+ &wm->wm[level]);
}
- I915_WRITE(PLANE_WM_TRANS(pipe, plane), wm->plane_trans[pipe][plane]);
+ skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane),
+ &wm->trans_wm);
skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane),
- &wm->ddb.plane[pipe][plane]);
+ &ddb->plane[pipe][plane]);
skl_ddb_entry_write(dev_priv, PLANE_NV12_BUF_CFG(pipe, plane),
- &wm->ddb.y_plane[pipe][plane]);
+ &ddb->y_plane[pipe][plane]);
}
void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
- const struct skl_wm_values *wm)
+ const struct skl_plane_wm *wm,
+ const struct skl_ddb_allocation *ddb)
{
struct drm_crtc *crtc = &intel_crtc->base;
struct drm_device *dev = crtc->dev;
@@ -3886,13 +3848,13 @@ void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
enum pipe pipe = intel_crtc->pipe;
for (level = 0; level <= max_level; level++) {
- I915_WRITE(CUR_WM(pipe, level),
- wm->plane[pipe][PLANE_CURSOR][level]);
+ skl_write_wm_level(dev_priv, CUR_WM(pipe, level),
+ &wm->wm[level]);
}
- I915_WRITE(CUR_WM_TRANS(pipe), wm->plane_trans[pipe][PLANE_CURSOR]);
+ skl_write_wm_level(dev_priv, CUR_WM_TRANS(pipe), &wm->trans_wm);
skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe),
- &wm->ddb.plane[pipe][PLANE_CURSOR]);
+ &ddb->plane[pipe][PLANE_CURSOR]);
}
static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
@@ -4076,11 +4038,6 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
struct skl_wm_values *src,
enum pipe pipe)
{
- memcpy(dst->plane[pipe], src->plane[pipe],
- sizeof(dst->plane[pipe]));
- memcpy(dst->plane_trans[pipe], src->plane_trans[pipe],
- sizeof(dst->plane_trans[pipe]));
-
memcpy(dst->ddb.y_plane[pipe], src->ddb.y_plane[pipe],
sizeof(dst->ddb.y_plane[pipe]));
memcpy(dst->ddb.plane[pipe], src->ddb.plane[pipe],
@@ -4129,7 +4086,6 @@ skl_compute_wm(struct drm_atomic_state *state)
* no suitable watermark values can be found.
*/
for_each_crtc_in_state(state, crtc, cstate, i) {
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_crtc_state *intel_cstate =
to_intel_crtc_state(cstate);
@@ -4147,7 +4103,6 @@ skl_compute_wm(struct drm_atomic_state *state)
continue;
intel_cstate->update_wm_pre = true;
- skl_compute_wm_results(crtc->dev, pipe_wm, results, intel_crtc);
}
return 0;
@@ -4181,9 +4136,11 @@ static void skl_update_wm(struct drm_crtc *crtc)
int plane;
for (plane = 0; plane < intel_num_planes(intel_crtc); plane++)
- skl_write_plane_wm(intel_crtc, results, plane);
+ skl_write_plane_wm(intel_crtc, &pipe_wm->planes[plane],
+ &results->ddb, plane);
- skl_write_cursor_wm(intel_crtc, results);
+ skl_write_cursor_wm(intel_crtc, &pipe_wm->planes[PLANE_CURSOR],
+ &results->ddb);
}
skl_copy_wm_for_pipe(hw_vals, results, pipe);
@@ -4268,26 +4225,13 @@ static void ilk_optimize_watermarks(struct intel_crtc_state *cstate)
mutex_unlock(&dev_priv->wm.wm_mutex);
}
-static void skl_pipe_wm_active_state(uint32_t val,
- struct skl_pipe_wm *active,
- bool is_transwm,
- int i,
- int level)
+static inline void skl_wm_level_from_reg_val(uint32_t val,
+ struct skl_wm_level *level)
{
- struct skl_plane_wm *plane_wm = &active->planes[i];
- bool is_enabled = (val & PLANE_WM_EN) != 0;
-
- if (!is_transwm) {
- plane_wm->wm[level].plane_en = is_enabled;
- plane_wm->wm[level].plane_res_b = val & PLANE_WM_BLOCKS_MASK;
- plane_wm->wm[level].plane_res_l =
- (val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
- } else {
- plane_wm->trans_wm.plane_en = is_enabled;
- plane_wm->trans_wm.plane_res_b = val & PLANE_WM_BLOCKS_MASK;
- plane_wm->trans_wm.plane_res_l =
- (val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
- }
+ level->plane_en = val & PLANE_WM_EN;
+ level->plane_res_b = val & PLANE_WM_BLOCKS_MASK;
+ level->plane_res_l = (val >> PLANE_WM_LINES_SHIFT) &
+ PLANE_WM_LINES_MASK;
}
static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
@@ -4297,49 +4241,41 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+ struct intel_plane *intel_plane;
struct skl_pipe_wm *active = &cstate->wm.skl.optimal;
+ struct skl_plane_wm *wm;
enum pipe pipe = intel_crtc->pipe;
- int level, i, max_level;
- uint32_t temp;
+ int level, id, max_level;
+ uint32_t val;
max_level = ilk_wm_max_level(dev_priv);
- for (level = 0; level <= max_level; level++) {
- for (i = 0; i < intel_num_planes(intel_crtc); i++)
- hw->plane[pipe][i][level] =
- I915_READ(PLANE_WM(pipe, i, level));
- hw->plane[pipe][PLANE_CURSOR][level] = I915_READ(CUR_WM(pipe, level));
- }
-
- for (i = 0; i < intel_num_planes(intel_crtc); i++)
- hw->plane_trans[pipe][i] = I915_READ(PLANE_WM_TRANS(pipe, i));
- hw->plane_trans[pipe][PLANE_CURSOR] = I915_READ(CUR_WM_TRANS(pipe));
-
- if (!intel_crtc->active)
- return;
-
- hw->dirty_pipes |= drm_crtc_mask(crtc);
+ for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
+ id = skl_wm_plane_id(intel_plane);
+ wm = &cstate->wm.skl.optimal.planes[id];
- active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
+ for (level = 0; level <= max_level; level++) {
+ if (id != PLANE_CURSOR)
+ val = I915_READ(PLANE_WM(pipe, id, level));
+ else
+ val = I915_READ(CUR_WM(pipe, level));
- for (level = 0; level <= max_level; level++) {
- for (i = 0; i < intel_num_planes(intel_crtc); i++) {
- temp = hw->plane[pipe][i][level];
- skl_pipe_wm_active_state(temp, active, false, i, level);
+ skl_wm_level_from_reg_val(val, &wm->wm[level]);
}
- temp = hw->plane[pipe][PLANE_CURSOR][level];
- skl_pipe_wm_active_state(temp, active, false, PLANE_CURSOR,
- level);
- }
- for (i = 0; i < intel_num_planes(intel_crtc); i++) {
- temp = hw->plane_trans[pipe][i];
- skl_pipe_wm_active_state(temp, active, true, i, 0);
+ if (id != PLANE_CURSOR)
+ val = I915_READ(PLANE_WM_TRANS(pipe, id));
+ else
+ val = I915_READ(CUR_WM_TRANS(pipe));
+
+ skl_wm_level_from_reg_val(val, &wm->trans_wm);
}
- temp = hw->plane_trans[pipe][PLANE_CURSOR];
- skl_pipe_wm_active_state(temp, active, true, PLANE_CURSOR, 0);
+ if (!intel_crtc->active)
+ return;
+ hw->dirty_pipes |= drm_crtc_mask(crtc);
+ active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
intel_crtc->wm.active.skl = *active;
}
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index c7d9a20..e7d2cff 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -208,6 +208,8 @@ skl_update_plane(struct drm_plane *drm_plane,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
const int pipe = intel_plane->pipe;
const int plane = intel_plane->plane + 1;
+ const struct skl_plane_wm *p_wm =
+ &crtc_state->wm.skl.optimal.planes[plane];
u32 plane_ctl;
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
u32 surf_addr = plane_state->main.offset;
@@ -232,7 +234,7 @@ skl_update_plane(struct drm_plane *drm_plane,
plane_ctl |= skl_plane_ctl_rotation(rotation);
if (wm->dirty_pipes & drm_crtc_mask(crtc))
- skl_write_plane_wm(intel_crtc, wm, plane);
+ skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, plane);
if (key->flags) {
I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value);
@@ -289,6 +291,7 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
struct drm_device *dev = dplane->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_plane *intel_plane = to_intel_plane(dplane);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
const int pipe = intel_plane->pipe;
const int plane = intel_plane->plane + 1;
@@ -298,7 +301,8 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
*/
if (!dplane->state->visible)
skl_write_plane_wm(to_intel_crtc(crtc),
- &dev_priv->wm.skl_results, plane);
+ &cstate->wm.skl.optimal.planes[plane],
+ &dev_priv->wm.skl_results.ddb, plane);
I915_WRITE(PLANE_CTL(pipe, plane), 0);
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v4 05/10] drm/i915/gen9: Get rid of redundant watermark values
2016-10-14 21:31 ` Lyude
(?)
@ 2016-10-18 18:09 ` Paulo Zanoni
-1 siblings, 0 replies; 28+ messages in thread
From: Paulo Zanoni @ 2016-10-18 18:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: "cpaul@redhat.com" <cpaul@redhat.com>
Now that we've make skl_wm_levels make a little more sense, we can
remove all of the redundant wm information. Up until now we'd been
storing two copies of all of the skl watermarks: one being the
skl_pipe_wm structs, the other being the global wm struct in
drm_i915_private containing the raw register values. This is confusing
and problematic, since it means we're prone to accidentally letting the
two copies go out of sync. So, get rid of all of the functions
responsible for computing the register values and just use a single
helper, skl_write_wm_level(), to convert and write the new watermarks on
the fly.
Changes since v1:
- Fixup skl_write_wm_level()
- Fixup skl_wm_level_from_reg_val()
- Don't forget to copy *active to intel_crtc->wm.active.skl
Changes since v2:
- Fix usage of wrong cstate
Changes since v3 (by Paulo):
- Rebase
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (v2)
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Lyude <cpaul@redhat.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 -
drivers/gpu/drm/i915/intel_display.c | 14 ++-
drivers/gpu/drm/i915/intel_drv.h | 6 +-
drivers/gpu/drm/i915/intel_pm.c | 203 ++++++++++++-----------------------
drivers/gpu/drm/i915/intel_sprite.c | 8 +-
5 files changed, 90 insertions(+), 143 deletions(-)
I just merged my fixes, and that caused conflicts on the part of this patch that
touches intel_can_enable_sagv(). So I decided to just resolve the conflicts
since I created them.
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 33c44c6..456bab9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1648,8 +1648,6 @@ struct skl_ddb_allocation {
struct skl_wm_values {
unsigned dirty_pipes;
struct skl_ddb_allocation ddb;
- uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
- uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
};
struct skl_wm_level {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9395878..0e18cf47 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3385,6 +3385,8 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_framebuffer *fb = plane_state->base.fb;
const struct skl_wm_values *wm = &dev_priv->wm.skl_results;
+ const struct skl_plane_wm *p_wm =
+ &crtc_state->wm.skl.optimal.planes[0];
int pipe = intel_crtc->pipe;
u32 plane_ctl;
unsigned int rotation = plane_state->base.rotation;
@@ -3421,7 +3423,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
intel_crtc->adjusted_y = src_y;
if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base))
- skl_write_plane_wm(intel_crtc, wm, 0);
+ skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, 0);
I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x);
@@ -3455,6 +3457,8 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+ const struct skl_plane_wm *p_wm = &cstate->wm.skl.optimal.planes[0];
int pipe = intel_crtc->pipe;
/*
@@ -3462,7 +3466,8 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
* plane's visiblity isn't actually changing neither is its watermarks.
*/
if (!crtc->primary->state->visible)
- skl_write_plane_wm(intel_crtc, &dev_priv->wm.skl_results, 0);
+ skl_write_plane_wm(intel_crtc, p_wm,
+ &dev_priv->wm.skl_results.ddb, 0);
I915_WRITE(PLANE_CTL(pipe, 0), 0);
I915_WRITE(PLANE_SURF(pipe, 0), 0);
@@ -10833,12 +10838,15 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
const struct skl_wm_values *wm = &dev_priv->wm.skl_results;
+ const struct skl_plane_wm *p_wm =
+ &cstate->wm.skl.optimal.planes[PLANE_CURSOR];
int pipe = intel_crtc->pipe;
uint32_t cntl = 0;
if (INTEL_GEN(dev_priv) >= 9 && wm->dirty_pipes & drm_crtc_mask(crtc))
- skl_write_cursor_wm(intel_crtc, wm);
+ skl_write_cursor_wm(intel_crtc, p_wm, &wm->ddb);
if (plane_state && plane_state->base.visible) {
cntl = MCURSOR_GAMMA_ENABLE;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5760420..a158d7e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1773,9 +1773,11 @@ bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
bool skl_ddb_allocation_overlaps(struct drm_atomic_state *state,
struct intel_crtc *intel_crtc);
void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
- const struct skl_wm_values *wm);
+ const struct skl_plane_wm *wm,
+ const struct skl_ddb_allocation *ddb);
void skl_write_plane_wm(struct intel_crtc *intel_crtc,
- const struct skl_wm_values *wm,
+ const struct skl_plane_wm *wm,
+ const struct skl_ddb_allocation *ddb,
int plane);
uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
bool ilk_disable_lp_wm(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 48d2b71..84e3272 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3020,8 +3020,10 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
struct intel_crtc *crtc;
struct intel_plane *plane;
+ struct intel_crtc_state *cstate;
+ struct skl_plane_wm *wm;
enum pipe pipe;
- int level, id, latency;
+ int level, latency;
if (!intel_has_sagv(dev_priv))
return false;
@@ -3040,20 +3042,21 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
/* Since we're now guaranteed to only have one active CRTC... */
pipe = ffs(intel_state->active_crtcs) - 1;
crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+ cstate = to_intel_crtc_state(crtc->base.state);
if (crtc->base.state->adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
return false;
for_each_intel_plane_on_crtc(dev, crtc, plane) {
- id = skl_wm_plane_id(plane);
+ wm = &cstate->wm.skl.optimal.planes[skl_wm_plane_id(plane)];
/* Skip this plane if it's not enabled */
- if (intel_state->wm_results.plane[pipe][id][0] == 0)
+ if (!wm->wm[0].plane_en)
continue;
/* Find the highest enabled wm level for this plane */
for (level = ilk_wm_max_level(dev_priv);
- intel_state->wm_results.plane[pipe][id][level] == 0; --level)
+ !wm->wm[level].plane_en; --level)
{ }
latency = dev_priv->wm.skl_latency[level];
@@ -3814,66 +3817,6 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
return 0;
}
-static void skl_compute_wm_results(struct drm_device *dev,
- struct skl_pipe_wm *p_wm,
- struct skl_wm_values *r,
- struct intel_crtc *intel_crtc)
-{
- int level, max_level = ilk_wm_max_level(to_i915(dev));
- struct skl_plane_wm *plane_wm;
- enum pipe pipe = intel_crtc->pipe;
- uint32_t temp;
- int i;
-
- for (i = 0; i < intel_num_planes(intel_crtc); i++) {
- plane_wm = &p_wm->planes[i];
-
- for (level = 0; level <= max_level; level++) {
- temp = 0;
-
- temp |= plane_wm->wm[level].plane_res_l <<
- PLANE_WM_LINES_SHIFT;
- temp |= plane_wm->wm[level].plane_res_b;
- if (plane_wm->wm[level].plane_en)
- temp |= PLANE_WM_EN;
-
- r->plane[pipe][i][level] = temp;
- }
- }
-
- for (level = 0; level <= max_level; level++) {
- plane_wm = &p_wm->planes[PLANE_CURSOR];
- temp = 0;
- temp |= plane_wm->wm[level].plane_res_l << PLANE_WM_LINES_SHIFT;
- temp |= plane_wm->wm[level].plane_res_b;
- if (plane_wm->wm[level].plane_en)
- temp |= PLANE_WM_EN;
-
- r->plane[pipe][PLANE_CURSOR][level] = temp;
- }
-
- /* transition WMs */
- for (i = 0; i < intel_num_planes(intel_crtc); i++) {
- plane_wm = &p_wm->planes[i];
- temp = 0;
- temp |= plane_wm->trans_wm.plane_res_l << PLANE_WM_LINES_SHIFT;
- temp |= plane_wm->trans_wm.plane_res_b;
- if (plane_wm->trans_wm.plane_en)
- temp |= PLANE_WM_EN;
-
- r->plane_trans[pipe][i] = temp;
- }
-
- plane_wm = &p_wm->planes[PLANE_CURSOR];
- temp = 0;
- temp |= plane_wm->trans_wm.plane_res_l << PLANE_WM_LINES_SHIFT;
- temp |= plane_wm->trans_wm.plane_res_b;
- if (plane_wm->trans_wm.plane_en)
- temp |= PLANE_WM_EN;
-
- r->plane_trans[pipe][PLANE_CURSOR] = temp;
-}
-
static void skl_ddb_entry_write(struct drm_i915_private *dev_priv,
i915_reg_t reg,
const struct skl_ddb_entry *entry)
@@ -3884,8 +3827,24 @@ static void skl_ddb_entry_write(struct drm_i915_private *dev_priv,
I915_WRITE(reg, 0);
}
+static void skl_write_wm_level(struct drm_i915_private *dev_priv,
+ i915_reg_t reg,
+ const struct skl_wm_level *level)
+{
+ uint32_t val = 0;
+
+ if (level->plane_en) {
+ val |= PLANE_WM_EN;
+ val |= level->plane_res_b;
+ val |= level->plane_res_l << PLANE_WM_LINES_SHIFT;
+ }
+
+ I915_WRITE(reg, val);
+}
+
void skl_write_plane_wm(struct intel_crtc *intel_crtc,
- const struct skl_wm_values *wm,
+ const struct skl_plane_wm *wm,
+ const struct skl_ddb_allocation *ddb,
int plane)
{
struct drm_crtc *crtc = &intel_crtc->base;
@@ -3895,19 +3854,21 @@ void skl_write_plane_wm(struct intel_crtc *intel_crtc,
enum pipe pipe = intel_crtc->pipe;
for (level = 0; level <= max_level; level++) {
- I915_WRITE(PLANE_WM(pipe, plane, level),
- wm->plane[pipe][plane][level]);
+ skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane, level),
+ &wm->wm[level]);
}
- I915_WRITE(PLANE_WM_TRANS(pipe, plane), wm->plane_trans[pipe][plane]);
+ skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane),
+ &wm->trans_wm);
skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane),
- &wm->ddb.plane[pipe][plane]);
+ &ddb->plane[pipe][plane]);
skl_ddb_entry_write(dev_priv, PLANE_NV12_BUF_CFG(pipe, plane),
- &wm->ddb.y_plane[pipe][plane]);
+ &ddb->y_plane[pipe][plane]);
}
void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
- const struct skl_wm_values *wm)
+ const struct skl_plane_wm *wm,
+ const struct skl_ddb_allocation *ddb)
{
struct drm_crtc *crtc = &intel_crtc->base;
struct drm_device *dev = crtc->dev;
@@ -3916,13 +3877,13 @@ void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
enum pipe pipe = intel_crtc->pipe;
for (level = 0; level <= max_level; level++) {
- I915_WRITE(CUR_WM(pipe, level),
- wm->plane[pipe][PLANE_CURSOR][level]);
+ skl_write_wm_level(dev_priv, CUR_WM(pipe, level),
+ &wm->wm[level]);
}
- I915_WRITE(CUR_WM_TRANS(pipe), wm->plane_trans[pipe][PLANE_CURSOR]);
+ skl_write_wm_level(dev_priv, CUR_WM_TRANS(pipe), &wm->trans_wm);
skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe),
- &wm->ddb.plane[pipe][PLANE_CURSOR]);
+ &ddb->plane[pipe][PLANE_CURSOR]);
}
static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
@@ -4106,11 +4067,6 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
struct skl_wm_values *src,
enum pipe pipe)
{
- memcpy(dst->plane[pipe], src->plane[pipe],
- sizeof(dst->plane[pipe]));
- memcpy(dst->plane_trans[pipe], src->plane_trans[pipe],
- sizeof(dst->plane_trans[pipe]));
-
memcpy(dst->ddb.y_plane[pipe], src->ddb.y_plane[pipe],
sizeof(dst->ddb.y_plane[pipe]));
memcpy(dst->ddb.plane[pipe], src->ddb.plane[pipe],
@@ -4159,7 +4115,6 @@ skl_compute_wm(struct drm_atomic_state *state)
* no suitable watermark values can be found.
*/
for_each_crtc_in_state(state, crtc, cstate, i) {
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_crtc_state *intel_cstate =
to_intel_crtc_state(cstate);
@@ -4177,7 +4132,6 @@ skl_compute_wm(struct drm_atomic_state *state)
continue;
intel_cstate->update_wm_pre = true;
- skl_compute_wm_results(crtc->dev, pipe_wm, results, intel_crtc);
}
return 0;
@@ -4211,9 +4165,11 @@ static void skl_update_wm(struct drm_crtc *crtc)
int plane;
for (plane = 0; plane < intel_num_planes(intel_crtc); plane++)
- skl_write_plane_wm(intel_crtc, results, plane);
+ skl_write_plane_wm(intel_crtc, &pipe_wm->planes[plane],
+ &results->ddb, plane);
- skl_write_cursor_wm(intel_crtc, results);
+ skl_write_cursor_wm(intel_crtc, &pipe_wm->planes[PLANE_CURSOR],
+ &results->ddb);
}
skl_copy_wm_for_pipe(hw_vals, results, pipe);
@@ -4298,26 +4254,13 @@ static void ilk_optimize_watermarks(struct intel_crtc_state *cstate)
mutex_unlock(&dev_priv->wm.wm_mutex);
}
-static void skl_pipe_wm_active_state(uint32_t val,
- struct skl_pipe_wm *active,
- bool is_transwm,
- int i,
- int level)
+static inline void skl_wm_level_from_reg_val(uint32_t val,
+ struct skl_wm_level *level)
{
- struct skl_plane_wm *plane_wm = &active->planes[i];
- bool is_enabled = (val & PLANE_WM_EN) != 0;
-
- if (!is_transwm) {
- plane_wm->wm[level].plane_en = is_enabled;
- plane_wm->wm[level].plane_res_b = val & PLANE_WM_BLOCKS_MASK;
- plane_wm->wm[level].plane_res_l =
- (val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
- } else {
- plane_wm->trans_wm.plane_en = is_enabled;
- plane_wm->trans_wm.plane_res_b = val & PLANE_WM_BLOCKS_MASK;
- plane_wm->trans_wm.plane_res_l =
- (val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
- }
+ level->plane_en = val & PLANE_WM_EN;
+ level->plane_res_b = val & PLANE_WM_BLOCKS_MASK;
+ level->plane_res_l = (val >> PLANE_WM_LINES_SHIFT) &
+ PLANE_WM_LINES_MASK;
}
static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
@@ -4327,49 +4270,41 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+ struct intel_plane *intel_plane;
struct skl_pipe_wm *active = &cstate->wm.skl.optimal;
+ struct skl_plane_wm *wm;
enum pipe pipe = intel_crtc->pipe;
- int level, i, max_level;
- uint32_t temp;
+ int level, id, max_level;
+ uint32_t val;
max_level = ilk_wm_max_level(dev_priv);
- for (level = 0; level <= max_level; level++) {
- for (i = 0; i < intel_num_planes(intel_crtc); i++)
- hw->plane[pipe][i][level] =
- I915_READ(PLANE_WM(pipe, i, level));
- hw->plane[pipe][PLANE_CURSOR][level] = I915_READ(CUR_WM(pipe, level));
- }
-
- for (i = 0; i < intel_num_planes(intel_crtc); i++)
- hw->plane_trans[pipe][i] = I915_READ(PLANE_WM_TRANS(pipe, i));
- hw->plane_trans[pipe][PLANE_CURSOR] = I915_READ(CUR_WM_TRANS(pipe));
-
- if (!intel_crtc->active)
- return;
-
- hw->dirty_pipes |= drm_crtc_mask(crtc);
+ for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
+ id = skl_wm_plane_id(intel_plane);
+ wm = &cstate->wm.skl.optimal.planes[id];
- active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
+ for (level = 0; level <= max_level; level++) {
+ if (id != PLANE_CURSOR)
+ val = I915_READ(PLANE_WM(pipe, id, level));
+ else
+ val = I915_READ(CUR_WM(pipe, level));
- for (level = 0; level <= max_level; level++) {
- for (i = 0; i < intel_num_planes(intel_crtc); i++) {
- temp = hw->plane[pipe][i][level];
- skl_pipe_wm_active_state(temp, active, false, i, level);
+ skl_wm_level_from_reg_val(val, &wm->wm[level]);
}
- temp = hw->plane[pipe][PLANE_CURSOR][level];
- skl_pipe_wm_active_state(temp, active, false, PLANE_CURSOR,
- level);
- }
- for (i = 0; i < intel_num_planes(intel_crtc); i++) {
- temp = hw->plane_trans[pipe][i];
- skl_pipe_wm_active_state(temp, active, true, i, 0);
+ if (id != PLANE_CURSOR)
+ val = I915_READ(PLANE_WM_TRANS(pipe, id));
+ else
+ val = I915_READ(CUR_WM_TRANS(pipe));
+
+ skl_wm_level_from_reg_val(val, &wm->trans_wm);
}
- temp = hw->plane_trans[pipe][PLANE_CURSOR];
- skl_pipe_wm_active_state(temp, active, true, PLANE_CURSOR, 0);
+ if (!intel_crtc->active)
+ return;
+ hw->dirty_pipes |= drm_crtc_mask(crtc);
+ active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
intel_crtc->wm.active.skl = *active;
}
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index c7d9a20..e7d2cff 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -208,6 +208,8 @@ skl_update_plane(struct drm_plane *drm_plane,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
const int pipe = intel_plane->pipe;
const int plane = intel_plane->plane + 1;
+ const struct skl_plane_wm *p_wm =
+ &crtc_state->wm.skl.optimal.planes[plane];
u32 plane_ctl;
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
u32 surf_addr = plane_state->main.offset;
@@ -232,7 +234,7 @@ skl_update_plane(struct drm_plane *drm_plane,
plane_ctl |= skl_plane_ctl_rotation(rotation);
if (wm->dirty_pipes & drm_crtc_mask(crtc))
- skl_write_plane_wm(intel_crtc, wm, plane);
+ skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, plane);
if (key->flags) {
I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value);
@@ -289,6 +291,7 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
struct drm_device *dev = dplane->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_plane *intel_plane = to_intel_plane(dplane);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
const int pipe = intel_plane->pipe;
const int plane = intel_plane->plane + 1;
@@ -298,7 +301,8 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
*/
if (!dplane->state->visible)
skl_write_plane_wm(to_intel_crtc(crtc),
- &dev_priv->wm.skl_results, plane);
+ &cstate->wm.skl.optimal.planes[plane],
+ &dev_priv->wm.skl_results.ddb, plane);
I915_WRITE(PLANE_CTL(pipe, plane), 0);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 06/10] drm/i915/gen9: Add ddb changes to atomic debug output
2016-10-14 21:31 ` Lyude
@ 2016-10-14 21:31 ` Lyude
-1 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx
Cc: Lyude, Ville Syrjälä,
Matt Roper, Paulo Zanoni, Daniel Vetter, Jani Nikula,
David Airlie, dri-devel, linux-kernel
Finally, add some debugging output for ddb changes in the atomic debug
output. This makes it a lot easier to spot bugs from incorrect ddb
allocations.
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 54 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5df5cea..2fe851e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4044,6 +4044,58 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
sizeof(dst->ddb.plane[pipe]));
}
+static void
+skl_print_wm_changes(const struct drm_atomic_state *state)
+{
+ const struct drm_device *dev = state->dev;
+ const struct drm_i915_private *dev_priv = to_i915(dev);
+ const struct intel_atomic_state *intel_state =
+ to_intel_atomic_state(state);
+ const struct drm_crtc *crtc;
+ const struct drm_crtc_state *cstate;
+ const struct drm_plane *plane;
+ const struct intel_plane *intel_plane;
+ const struct drm_plane_state *pstate;
+ const struct skl_ddb_allocation *old_ddb = &dev_priv->wm.skl_hw.ddb;
+ const struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
+ enum pipe pipe;
+ int id;
+ int i, j;
+
+ for_each_crtc_in_state(state, crtc, cstate, i) {
+ pipe = to_intel_crtc(crtc)->pipe;
+
+ for_each_plane_in_state(state, plane, pstate, j) {
+ const struct skl_ddb_entry *old, *new;
+
+ intel_plane = to_intel_plane(plane);
+ id = skl_wm_plane_id(intel_plane);
+ old = &old_ddb->plane[pipe][id];
+ new = &new_ddb->plane[pipe][id];
+
+ if (intel_plane->pipe != pipe)
+ continue;
+
+ if (skl_ddb_entry_equal(old, new))
+ continue;
+
+ if (id != PLANE_CURSOR) {
+ DRM_DEBUG_ATOMIC("[PLANE:%d:plane %d%c] ddb (%d - %d) -> (%d - %d)\n",
+ plane->base.id, id + 1,
+ pipe_name(pipe),
+ old->start, old->end,
+ new->start, new->end);
+ } else {
+ DRM_DEBUG_ATOMIC("[PLANE:%d:cursor %c] ddb (%d - %d) -> (%d - %d)\n",
+ plane->base.id,
+ pipe_name(pipe),
+ old->start, old->end,
+ new->start, new->end);
+ }
+ }
+ }
+}
+
static int
skl_compute_wm(struct drm_atomic_state *state)
{
@@ -4105,6 +4157,8 @@ skl_compute_wm(struct drm_atomic_state *state)
intel_cstate->update_wm_pre = true;
}
+ skl_print_wm_changes(state);
+
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 06/10] drm/i915/gen9: Add ddb changes to atomic debug output
@ 2016-10-14 21:31 ` Lyude
0 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx
Cc: Paulo Zanoni, David Airlie, linux-kernel, dri-devel, Daniel Vetter
Finally, add some debugging output for ddb changes in the atomic debug
output. This makes it a lot easier to spot bugs from incorrect ddb
allocations.
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 54 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5df5cea..2fe851e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4044,6 +4044,58 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
sizeof(dst->ddb.plane[pipe]));
}
+static void
+skl_print_wm_changes(const struct drm_atomic_state *state)
+{
+ const struct drm_device *dev = state->dev;
+ const struct drm_i915_private *dev_priv = to_i915(dev);
+ const struct intel_atomic_state *intel_state =
+ to_intel_atomic_state(state);
+ const struct drm_crtc *crtc;
+ const struct drm_crtc_state *cstate;
+ const struct drm_plane *plane;
+ const struct intel_plane *intel_plane;
+ const struct drm_plane_state *pstate;
+ const struct skl_ddb_allocation *old_ddb = &dev_priv->wm.skl_hw.ddb;
+ const struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
+ enum pipe pipe;
+ int id;
+ int i, j;
+
+ for_each_crtc_in_state(state, crtc, cstate, i) {
+ pipe = to_intel_crtc(crtc)->pipe;
+
+ for_each_plane_in_state(state, plane, pstate, j) {
+ const struct skl_ddb_entry *old, *new;
+
+ intel_plane = to_intel_plane(plane);
+ id = skl_wm_plane_id(intel_plane);
+ old = &old_ddb->plane[pipe][id];
+ new = &new_ddb->plane[pipe][id];
+
+ if (intel_plane->pipe != pipe)
+ continue;
+
+ if (skl_ddb_entry_equal(old, new))
+ continue;
+
+ if (id != PLANE_CURSOR) {
+ DRM_DEBUG_ATOMIC("[PLANE:%d:plane %d%c] ddb (%d - %d) -> (%d - %d)\n",
+ plane->base.id, id + 1,
+ pipe_name(pipe),
+ old->start, old->end,
+ new->start, new->end);
+ } else {
+ DRM_DEBUG_ATOMIC("[PLANE:%d:cursor %c] ddb (%d - %d) -> (%d - %d)\n",
+ plane->base.id,
+ pipe_name(pipe),
+ old->start, old->end,
+ new->start, new->end);
+ }
+ }
+ }
+}
+
static int
skl_compute_wm(struct drm_atomic_state *state)
{
@@ -4105,6 +4157,8 @@ skl_compute_wm(struct drm_atomic_state *state)
intel_cstate->update_wm_pre = true;
}
+ skl_print_wm_changes(state);
+
return 0;
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 07/10] drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable
2016-10-14 21:31 ` Lyude
` (6 preceding siblings ...)
(?)
@ 2016-10-14 21:31 ` Lyude
2016-10-18 18:12 ` Paulo Zanoni
-1 siblings, 1 reply; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx
Cc: Lyude, Maarten Lankhorst, Ville Syrjälä,
Matt Roper, Paulo Zanoni, Daniel Vetter, Jani Nikula,
David Airlie, dri-devel, linux-kernel
There's not much of a reason this should have the locations to read out
the hardware state hardcoded, so allow the caller to specify the
location and add this function to intel_drv.h. As well, we're going to
need this function to be reusable for the next patch.
Changes since v1:
- Fix accidental behavior change in the code that Paulo pointed out
Signed-off-by: Lyude <cpaul@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_pm.c | 28 ++++++++++++++++++----------
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a85ce2c..7036310 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1756,6 +1756,8 @@ void ilk_wm_get_hw_state(struct drm_device *dev);
void skl_wm_get_hw_state(struct drm_device *dev);
void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
struct skl_ddb_allocation *ddb /* out */);
+void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
+ struct skl_pipe_wm *out);
bool intel_can_enable_sagv(struct drm_atomic_state *state);
int intel_enable_sagv(struct drm_i915_private *dev_priv);
int intel_disable_sagv(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2fe851e..6eaeb87 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4288,15 +4288,13 @@ static inline void skl_wm_level_from_reg_val(uint32_t val,
PLANE_WM_LINES_MASK;
}
-static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
+void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
+ struct skl_pipe_wm *out)
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
struct intel_plane *intel_plane;
- struct skl_pipe_wm *active = &cstate->wm.skl.optimal;
struct skl_plane_wm *wm;
enum pipe pipe = intel_crtc->pipe;
int level, id, max_level;
@@ -4306,7 +4304,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
id = skl_wm_plane_id(intel_plane);
- wm = &cstate->wm.skl.optimal.planes[id];
+ wm = &out->planes[id];
for (level = 0; level <= max_level; level++) {
if (id != PLANE_CURSOR)
@@ -4328,20 +4326,30 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
if (!intel_crtc->active)
return;
- hw->dirty_pipes |= drm_crtc_mask(crtc);
- active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
- intel_crtc->wm.active.skl = *active;
+ out->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
}
void skl_wm_get_hw_state(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
struct skl_ddb_allocation *ddb = &dev_priv->wm.skl_hw.ddb;
struct drm_crtc *crtc;
+ struct intel_crtc *intel_crtc;
+ struct intel_crtc_state *cstate;
skl_ddb_get_hw_state(dev_priv, ddb);
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
- skl_pipe_wm_get_hw_state(crtc);
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+ intel_crtc = to_intel_crtc(crtc);
+ cstate = to_intel_crtc_state(crtc->state);
+
+ skl_pipe_wm_get_hw_state(crtc, &cstate->wm.skl.optimal);
+
+ if (intel_crtc->active) {
+ hw->dirty_pipes |= drm_crtc_mask(crtc);
+ intel_crtc->wm.active.skl = cstate->wm.skl.optimal;
+ }
+ }
if (dev_priv->active_crtcs) {
/* Fully recompute DDB on first atomic commit */
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 07/10] drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable
2016-10-14 21:31 ` [PATCH v2 07/10] drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable Lyude
@ 2016-10-18 18:12 ` Paulo Zanoni
0 siblings, 0 replies; 28+ messages in thread
From: Paulo Zanoni @ 2016-10-18 18:12 UTC (permalink / raw)
To: Lyude, intel-gfx
Cc: Maarten Lankhorst, Ville Syrjälä,
Matt Roper, Daniel Vetter, Jani Nikula, David Airlie, dri-devel,
linux-kernel
Em Sex, 2016-10-14 às 17:31 -0400, Lyude escreveu:
> There's not much of a reason this should have the locations to read
> out
> the hardware state hardcoded, so allow the caller to specify the
> location and add this function to intel_drv.h. As well, we're going
> to
> need this function to be reusable for the next patch.
>
> Changes since v1:
> - Fix accidental behavior change in the code that Paulo pointed out
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
I just submitted v4 of patch 5 solving the conflicts I created. With
that + this review, we can merge this series. If you give me an ack on
patch 5 I can just go and merge these, so we can move to Maarten's
series and then later to Mahesh's series.
>
> Signed-off-by: Lyude <cpaul@redhat.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_pm.c | 28 ++++++++++++++++++----------
> 2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index a85ce2c..7036310 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1756,6 +1756,8 @@ void ilk_wm_get_hw_state(struct drm_device
> *dev);
> void skl_wm_get_hw_state(struct drm_device *dev);
> void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
> struct skl_ddb_allocation *ddb /* out */);
> +void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
> + struct skl_pipe_wm *out);
> bool intel_can_enable_sagv(struct drm_atomic_state *state);
> int intel_enable_sagv(struct drm_i915_private *dev_priv);
> int intel_disable_sagv(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 2fe851e..6eaeb87 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4288,15 +4288,13 @@ static inline void
> skl_wm_level_from_reg_val(uint32_t val,
> PLANE_WM_LINES_MASK;
> }
>
> -static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> +void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
> + struct skl_pipe_wm *out)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_crtc_state *cstate = to_intel_crtc_state(crtc-
> >state);
> struct intel_plane *intel_plane;
> - struct skl_pipe_wm *active = &cstate->wm.skl.optimal;
> struct skl_plane_wm *wm;
> enum pipe pipe = intel_crtc->pipe;
> int level, id, max_level;
> @@ -4306,7 +4304,7 @@ static void skl_pipe_wm_get_hw_state(struct
> drm_crtc *crtc)
>
> for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> id = skl_wm_plane_id(intel_plane);
> - wm = &cstate->wm.skl.optimal.planes[id];
> + wm = &out->planes[id];
>
> for (level = 0; level <= max_level; level++) {
> if (id != PLANE_CURSOR)
> @@ -4328,20 +4326,30 @@ static void skl_pipe_wm_get_hw_state(struct
> drm_crtc *crtc)
> if (!intel_crtc->active)
> return;
>
> - hw->dirty_pipes |= drm_crtc_mask(crtc);
> - active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
> - intel_crtc->wm.active.skl = *active;
> + out->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
> }
>
> void skl_wm_get_hw_state(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
> struct skl_ddb_allocation *ddb = &dev_priv->wm.skl_hw.ddb;
> struct drm_crtc *crtc;
> + struct intel_crtc *intel_crtc;
> + struct intel_crtc_state *cstate;
>
> skl_ddb_get_hw_state(dev_priv, ddb);
> - list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
> - skl_pipe_wm_get_hw_state(crtc);
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
> {
> + intel_crtc = to_intel_crtc(crtc);
> + cstate = to_intel_crtc_state(crtc->state);
> +
> + skl_pipe_wm_get_hw_state(crtc, &cstate-
> >wm.skl.optimal);
> +
> + if (intel_crtc->active) {
> + hw->dirty_pipes |= drm_crtc_mask(crtc);
> + intel_crtc->wm.active.skl = cstate-
> >wm.skl.optimal;
> + }
> + }
>
> if (dev_priv->active_crtcs) {
> /* Fully recompute DDB on first atomic commit */
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 07/10] drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable
@ 2016-10-18 18:12 ` Paulo Zanoni
0 siblings, 0 replies; 28+ messages in thread
From: Paulo Zanoni @ 2016-10-18 18:12 UTC (permalink / raw)
To: Lyude, intel-gfx; +Cc: David Airlie, linux-kernel, dri-devel, Daniel Vetter
Em Sex, 2016-10-14 às 17:31 -0400, Lyude escreveu:
> There's not much of a reason this should have the locations to read
> out
> the hardware state hardcoded, so allow the caller to specify the
> location and add this function to intel_drv.h. As well, we're going
> to
> need this function to be reusable for the next patch.
>
> Changes since v1:
> - Fix accidental behavior change in the code that Paulo pointed out
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
I just submitted v4 of patch 5 solving the conflicts I created. With
that + this review, we can merge this series. If you give me an ack on
patch 5 I can just go and merge these, so we can move to Maarten's
series and then later to Mahesh's series.
>
> Signed-off-by: Lyude <cpaul@redhat.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_pm.c | 28 ++++++++++++++++++----------
> 2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index a85ce2c..7036310 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1756,6 +1756,8 @@ void ilk_wm_get_hw_state(struct drm_device
> *dev);
> void skl_wm_get_hw_state(struct drm_device *dev);
> void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
> struct skl_ddb_allocation *ddb /* out */);
> +void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
> + struct skl_pipe_wm *out);
> bool intel_can_enable_sagv(struct drm_atomic_state *state);
> int intel_enable_sagv(struct drm_i915_private *dev_priv);
> int intel_disable_sagv(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 2fe851e..6eaeb87 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4288,15 +4288,13 @@ static inline void
> skl_wm_level_from_reg_val(uint32_t val,
> PLANE_WM_LINES_MASK;
> }
>
> -static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> +void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
> + struct skl_pipe_wm *out)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_crtc_state *cstate = to_intel_crtc_state(crtc-
> >state);
> struct intel_plane *intel_plane;
> - struct skl_pipe_wm *active = &cstate->wm.skl.optimal;
> struct skl_plane_wm *wm;
> enum pipe pipe = intel_crtc->pipe;
> int level, id, max_level;
> @@ -4306,7 +4304,7 @@ static void skl_pipe_wm_get_hw_state(struct
> drm_crtc *crtc)
>
> for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> id = skl_wm_plane_id(intel_plane);
> - wm = &cstate->wm.skl.optimal.planes[id];
> + wm = &out->planes[id];
>
> for (level = 0; level <= max_level; level++) {
> if (id != PLANE_CURSOR)
> @@ -4328,20 +4326,30 @@ static void skl_pipe_wm_get_hw_state(struct
> drm_crtc *crtc)
> if (!intel_crtc->active)
> return;
>
> - hw->dirty_pipes |= drm_crtc_mask(crtc);
> - active->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
> - intel_crtc->wm.active.skl = *active;
> + out->linetime = I915_READ(PIPE_WM_LINETIME(pipe));
> }
>
> void skl_wm_get_hw_state(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
> struct skl_ddb_allocation *ddb = &dev_priv->wm.skl_hw.ddb;
> struct drm_crtc *crtc;
> + struct intel_crtc *intel_crtc;
> + struct intel_crtc_state *cstate;
>
> skl_ddb_get_hw_state(dev_priv, ddb);
> - list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
> - skl_pipe_wm_get_hw_state(crtc);
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
> {
> + intel_crtc = to_intel_crtc(crtc);
> + cstate = to_intel_crtc_state(crtc->state);
> +
> + skl_pipe_wm_get_hw_state(crtc, &cstate-
> >wm.skl.optimal);
> +
> + if (intel_crtc->active) {
> + hw->dirty_pipes |= drm_crtc_mask(crtc);
> + intel_crtc->wm.active.skl = cstate-
> >wm.skl.optimal;
> + }
> + }
>
> if (dev_priv->active_crtcs) {
> /* Fully recompute DDB on first atomic commit */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 08/10] drm/i915/gen9: Add skl_wm_level_equals()
2016-10-14 21:31 ` Lyude
@ 2016-10-14 21:31 ` Lyude
-1 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx
Cc: Lyude, Maarten Lankhorst, Ville Syrjälä,
Matt Roper, Paulo Zanoni, Daniel Vetter, Jani Nikula,
David Airlie, dri-devel, linux-kernel
Helper we're going to be using for implementing verification of the wm
levels in skl_verify_wm_level().
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_pm.c | 14 ++++++++++++++
2 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7036310..96963ea 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1761,6 +1761,8 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
bool intel_can_enable_sagv(struct drm_atomic_state *state);
int intel_enable_sagv(struct drm_i915_private *dev_priv);
int intel_disable_sagv(struct drm_i915_private *dev_priv);
+bool skl_wm_level_equals(const struct skl_wm_level *l1,
+ const struct skl_wm_level *l2);
bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
const struct skl_ddb_allocation *new,
enum pipe pipe);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6eaeb87..fae3ce4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3857,6 +3857,20 @@ void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
&ddb->plane[pipe][PLANE_CURSOR]);
}
+bool skl_wm_level_equals(const struct skl_wm_level *l1,
+ const struct skl_wm_level *l2)
+{
+ if (l1->plane_en != l2->plane_en)
+ return false;
+
+ /* If both planes aren't enabled, the rest shouldn't matter */
+ if (!l1->plane_en)
+ return true;
+
+ return (l1->plane_res_l == l2->plane_res_l &&
+ l1->plane_res_b == l2->plane_res_b);
+}
+
static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
const struct skl_ddb_entry *b)
{
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 08/10] drm/i915/gen9: Add skl_wm_level_equals()
@ 2016-10-14 21:31 ` Lyude
0 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, linux-kernel, dri-devel, Daniel Vetter, Lyude
Helper we're going to be using for implementing verification of the wm
levels in skl_verify_wm_level().
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_pm.c | 14 ++++++++++++++
2 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7036310..96963ea 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1761,6 +1761,8 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
bool intel_can_enable_sagv(struct drm_atomic_state *state);
int intel_enable_sagv(struct drm_i915_private *dev_priv);
int intel_disable_sagv(struct drm_i915_private *dev_priv);
+bool skl_wm_level_equals(const struct skl_wm_level *l1,
+ const struct skl_wm_level *l2);
bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
const struct skl_ddb_allocation *new,
enum pipe pipe);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6eaeb87..fae3ce4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3857,6 +3857,20 @@ void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
&ddb->plane[pipe][PLANE_CURSOR]);
}
+bool skl_wm_level_equals(const struct skl_wm_level *l1,
+ const struct skl_wm_level *l2)
+{
+ if (l1->plane_en != l2->plane_en)
+ return false;
+
+ /* If both planes aren't enabled, the rest shouldn't matter */
+ if (!l1->plane_en)
+ return true;
+
+ return (l1->plane_res_l == l2->plane_res_l &&
+ l1->plane_res_b == l2->plane_res_b);
+}
+
static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
const struct skl_ddb_entry *b)
{
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 09/10] drm/i915/gen9: Actually verify WM levels in verify_wm_state()
2016-10-14 21:31 ` Lyude
@ 2016-10-14 21:31 ` Lyude
-1 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx
Cc: Lyude, Maarten Lankhorst, Ville Syrjälä,
Matt Roper, Paulo Zanoni, Daniel Vetter, Jani Nikula,
David Airlie, dri-devel, linux-kernel
Thanks to Paulo Zanoni for indirectly pointing this out.
Looks like we never actually added any code for checking whether or not
we actually wrote watermark levels properly. Let's fix that.
Changes since v1:
- Use %u instead of %d when printing WM state mismatches
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 100 +++++++++++++++++++++++++++++------
1 file changed, 84 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 18e82b4..ca12f0e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13455,30 +13455,66 @@ static void verify_wm_state(struct drm_crtc *crtc,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct skl_ddb_allocation hw_ddb, *sw_ddb;
- struct skl_ddb_entry *hw_entry, *sw_entry;
+ struct skl_pipe_wm hw_wm, *sw_wm;
+ struct skl_plane_wm *hw_plane_wm, *sw_plane_wm;
+ struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
const enum pipe pipe = intel_crtc->pipe;
- int plane;
+ int plane, level, max_level = ilk_wm_max_level(dev_priv);
if (INTEL_INFO(dev)->gen < 9 || !new_state->active)
return;
+ skl_pipe_wm_get_hw_state(crtc, &hw_wm);
+ sw_wm = &intel_crtc->wm.active.skl;
+
skl_ddb_get_hw_state(dev_priv, &hw_ddb);
sw_ddb = &dev_priv->wm.skl_hw.ddb;
/* planes */
for_each_plane(dev_priv, pipe, plane) {
- hw_entry = &hw_ddb.plane[pipe][plane];
- sw_entry = &sw_ddb->plane[pipe][plane];
+ hw_plane_wm = &hw_wm.planes[plane];
+ sw_plane_wm = &sw_wm->planes[plane];
- if (skl_ddb_entry_equal(hw_entry, sw_entry))
- continue;
+ /* Watermarks */
+ for (level = 0; level <= max_level; level++) {
+ if (skl_wm_level_equals(&hw_plane_wm->wm[level],
+ &sw_plane_wm->wm[level]))
+ continue;
+
+ DRM_ERROR("mismatch in WM pipe %c plane %d level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+ pipe_name(pipe), plane + 1, level,
+ sw_plane_wm->wm[level].plane_en,
+ sw_plane_wm->wm[level].plane_res_b,
+ sw_plane_wm->wm[level].plane_res_l,
+ hw_plane_wm->wm[level].plane_en,
+ hw_plane_wm->wm[level].plane_res_b,
+ hw_plane_wm->wm[level].plane_res_l);
+ }
- DRM_ERROR("mismatch in DDB state pipe %c plane %d "
- "(expected (%u,%u), found (%u,%u))\n",
- pipe_name(pipe), plane + 1,
- sw_entry->start, sw_entry->end,
- hw_entry->start, hw_entry->end);
+ if (!skl_wm_level_equals(&hw_plane_wm->trans_wm,
+ &sw_plane_wm->trans_wm)) {
+ DRM_ERROR("mismatch in trans WM pipe %c plane %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+ pipe_name(pipe), plane + 1,
+ sw_plane_wm->trans_wm.plane_en,
+ sw_plane_wm->trans_wm.plane_res_b,
+ sw_plane_wm->trans_wm.plane_res_l,
+ hw_plane_wm->trans_wm.plane_en,
+ hw_plane_wm->trans_wm.plane_res_b,
+ hw_plane_wm->trans_wm.plane_res_l);
+ }
+
+ /* DDB */
+ hw_ddb_entry = &hw_ddb.plane[pipe][plane];
+ sw_ddb_entry = &sw_ddb->plane[pipe][plane];
+
+ if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
+ DRM_ERROR("mismatch in DDB state pipe %c plane %d "
+ "(expected (%u,%u), found (%u,%u))\n",
+ pipe_name(pipe), plane + 1,
+ sw_ddb_entry->start, sw_ddb_entry->end,
+ hw_ddb_entry->start, hw_ddb_entry->end);
+ }
}
/*
@@ -13488,15 +13524,47 @@ static void verify_wm_state(struct drm_crtc *crtc,
* once the plane becomes visible, we can skip this check
*/
if (intel_crtc->cursor_addr) {
- hw_entry = &hw_ddb.plane[pipe][PLANE_CURSOR];
- sw_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
+ hw_plane_wm = &hw_wm.planes[PLANE_CURSOR];
+ sw_plane_wm = &sw_wm->planes[PLANE_CURSOR];
+
+ /* Watermarks */
+ for (level = 0; level <= max_level; level++) {
+ if (skl_wm_level_equals(&hw_plane_wm->wm[level],
+ &sw_plane_wm->wm[level]))
+ continue;
+
+ DRM_ERROR("mismatch in WM pipe %c cursor level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+ pipe_name(pipe), level,
+ sw_plane_wm->wm[level].plane_en,
+ sw_plane_wm->wm[level].plane_res_b,
+ sw_plane_wm->wm[level].plane_res_l,
+ hw_plane_wm->wm[level].plane_en,
+ hw_plane_wm->wm[level].plane_res_b,
+ hw_plane_wm->wm[level].plane_res_l);
+ }
+
+ if (!skl_wm_level_equals(&hw_plane_wm->trans_wm,
+ &sw_plane_wm->trans_wm)) {
+ DRM_ERROR("mismatch in trans WM pipe %c cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+ pipe_name(pipe),
+ sw_plane_wm->trans_wm.plane_en,
+ sw_plane_wm->trans_wm.plane_res_b,
+ sw_plane_wm->trans_wm.plane_res_l,
+ hw_plane_wm->trans_wm.plane_en,
+ hw_plane_wm->trans_wm.plane_res_b,
+ hw_plane_wm->trans_wm.plane_res_l);
+ }
+
+ /* DDB */
+ hw_ddb_entry = &hw_ddb.plane[pipe][PLANE_CURSOR];
+ sw_ddb_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
- if (!skl_ddb_entry_equal(hw_entry, sw_entry)) {
+ if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
DRM_ERROR("mismatch in DDB state pipe %c cursor "
"(expected (%u,%u), found (%u,%u))\n",
pipe_name(pipe),
- sw_entry->start, sw_entry->end,
- hw_entry->start, hw_entry->end);
+ sw_ddb_entry->start, sw_ddb_entry->end,
+ hw_ddb_entry->start, hw_ddb_entry->end);
}
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 09/10] drm/i915/gen9: Actually verify WM levels in verify_wm_state()
@ 2016-10-14 21:31 ` Lyude
0 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, linux-kernel, dri-devel, Daniel Vetter, Lyude
Thanks to Paulo Zanoni for indirectly pointing this out.
Looks like we never actually added any code for checking whether or not
we actually wrote watermark levels properly. Let's fix that.
Changes since v1:
- Use %u instead of %d when printing WM state mismatches
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 100 +++++++++++++++++++++++++++++------
1 file changed, 84 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 18e82b4..ca12f0e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13455,30 +13455,66 @@ static void verify_wm_state(struct drm_crtc *crtc,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct skl_ddb_allocation hw_ddb, *sw_ddb;
- struct skl_ddb_entry *hw_entry, *sw_entry;
+ struct skl_pipe_wm hw_wm, *sw_wm;
+ struct skl_plane_wm *hw_plane_wm, *sw_plane_wm;
+ struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
const enum pipe pipe = intel_crtc->pipe;
- int plane;
+ int plane, level, max_level = ilk_wm_max_level(dev_priv);
if (INTEL_INFO(dev)->gen < 9 || !new_state->active)
return;
+ skl_pipe_wm_get_hw_state(crtc, &hw_wm);
+ sw_wm = &intel_crtc->wm.active.skl;
+
skl_ddb_get_hw_state(dev_priv, &hw_ddb);
sw_ddb = &dev_priv->wm.skl_hw.ddb;
/* planes */
for_each_plane(dev_priv, pipe, plane) {
- hw_entry = &hw_ddb.plane[pipe][plane];
- sw_entry = &sw_ddb->plane[pipe][plane];
+ hw_plane_wm = &hw_wm.planes[plane];
+ sw_plane_wm = &sw_wm->planes[plane];
- if (skl_ddb_entry_equal(hw_entry, sw_entry))
- continue;
+ /* Watermarks */
+ for (level = 0; level <= max_level; level++) {
+ if (skl_wm_level_equals(&hw_plane_wm->wm[level],
+ &sw_plane_wm->wm[level]))
+ continue;
+
+ DRM_ERROR("mismatch in WM pipe %c plane %d level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+ pipe_name(pipe), plane + 1, level,
+ sw_plane_wm->wm[level].plane_en,
+ sw_plane_wm->wm[level].plane_res_b,
+ sw_plane_wm->wm[level].plane_res_l,
+ hw_plane_wm->wm[level].plane_en,
+ hw_plane_wm->wm[level].plane_res_b,
+ hw_plane_wm->wm[level].plane_res_l);
+ }
- DRM_ERROR("mismatch in DDB state pipe %c plane %d "
- "(expected (%u,%u), found (%u,%u))\n",
- pipe_name(pipe), plane + 1,
- sw_entry->start, sw_entry->end,
- hw_entry->start, hw_entry->end);
+ if (!skl_wm_level_equals(&hw_plane_wm->trans_wm,
+ &sw_plane_wm->trans_wm)) {
+ DRM_ERROR("mismatch in trans WM pipe %c plane %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+ pipe_name(pipe), plane + 1,
+ sw_plane_wm->trans_wm.plane_en,
+ sw_plane_wm->trans_wm.plane_res_b,
+ sw_plane_wm->trans_wm.plane_res_l,
+ hw_plane_wm->trans_wm.plane_en,
+ hw_plane_wm->trans_wm.plane_res_b,
+ hw_plane_wm->trans_wm.plane_res_l);
+ }
+
+ /* DDB */
+ hw_ddb_entry = &hw_ddb.plane[pipe][plane];
+ sw_ddb_entry = &sw_ddb->plane[pipe][plane];
+
+ if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
+ DRM_ERROR("mismatch in DDB state pipe %c plane %d "
+ "(expected (%u,%u), found (%u,%u))\n",
+ pipe_name(pipe), plane + 1,
+ sw_ddb_entry->start, sw_ddb_entry->end,
+ hw_ddb_entry->start, hw_ddb_entry->end);
+ }
}
/*
@@ -13488,15 +13524,47 @@ static void verify_wm_state(struct drm_crtc *crtc,
* once the plane becomes visible, we can skip this check
*/
if (intel_crtc->cursor_addr) {
- hw_entry = &hw_ddb.plane[pipe][PLANE_CURSOR];
- sw_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
+ hw_plane_wm = &hw_wm.planes[PLANE_CURSOR];
+ sw_plane_wm = &sw_wm->planes[PLANE_CURSOR];
+
+ /* Watermarks */
+ for (level = 0; level <= max_level; level++) {
+ if (skl_wm_level_equals(&hw_plane_wm->wm[level],
+ &sw_plane_wm->wm[level]))
+ continue;
+
+ DRM_ERROR("mismatch in WM pipe %c cursor level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+ pipe_name(pipe), level,
+ sw_plane_wm->wm[level].plane_en,
+ sw_plane_wm->wm[level].plane_res_b,
+ sw_plane_wm->wm[level].plane_res_l,
+ hw_plane_wm->wm[level].plane_en,
+ hw_plane_wm->wm[level].plane_res_b,
+ hw_plane_wm->wm[level].plane_res_l);
+ }
+
+ if (!skl_wm_level_equals(&hw_plane_wm->trans_wm,
+ &sw_plane_wm->trans_wm)) {
+ DRM_ERROR("mismatch in trans WM pipe %c cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
+ pipe_name(pipe),
+ sw_plane_wm->trans_wm.plane_en,
+ sw_plane_wm->trans_wm.plane_res_b,
+ sw_plane_wm->trans_wm.plane_res_l,
+ hw_plane_wm->trans_wm.plane_en,
+ hw_plane_wm->trans_wm.plane_res_b,
+ hw_plane_wm->trans_wm.plane_res_l);
+ }
+
+ /* DDB */
+ hw_ddb_entry = &hw_ddb.plane[pipe][PLANE_CURSOR];
+ sw_ddb_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
- if (!skl_ddb_entry_equal(hw_entry, sw_entry)) {
+ if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
DRM_ERROR("mismatch in DDB state pipe %c cursor "
"(expected (%u,%u), found (%u,%u))\n",
pipe_name(pipe),
- sw_entry->start, sw_entry->end,
- hw_entry->start, hw_entry->end);
+ sw_ddb_entry->start, sw_ddb_entry->end,
+ hw_ddb_entry->start, hw_ddb_entry->end);
}
}
}
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 10/10] drm/i915/gen9: Don't wrap strings in verify_wm_state()
2016-10-14 21:31 ` Lyude
@ 2016-10-14 21:31 ` Lyude
-1 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx
Cc: Lyude, Maarten Lankhorst, Ville Syrjälä,
Matt Roper, Paulo Zanoni, Daniel Vetter, Jani Nikula,
David Airlie, dri-devel, linux-kernel
Wrapping strings is against the guidelines in Documentation/CodingStyle,
chapter 2.
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ca12f0e..eaec9f3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13509,8 +13509,7 @@ static void verify_wm_state(struct drm_crtc *crtc,
sw_ddb_entry = &sw_ddb->plane[pipe][plane];
if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
- DRM_ERROR("mismatch in DDB state pipe %c plane %d "
- "(expected (%u,%u), found (%u,%u))\n",
+ DRM_ERROR("mismatch in DDB state pipe %c plane %d (expected (%u,%u), found (%u,%u))\n",
pipe_name(pipe), plane + 1,
sw_ddb_entry->start, sw_ddb_entry->end,
hw_ddb_entry->start, hw_ddb_entry->end);
@@ -13560,8 +13559,7 @@ static void verify_wm_state(struct drm_crtc *crtc,
sw_ddb_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
- DRM_ERROR("mismatch in DDB state pipe %c cursor "
- "(expected (%u,%u), found (%u,%u))\n",
+ DRM_ERROR("mismatch in DDB state pipe %c cursor (expected (%u,%u), found (%u,%u))\n",
pipe_name(pipe),
sw_ddb_entry->start, sw_ddb_entry->end,
hw_ddb_entry->start, hw_ddb_entry->end);
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 10/10] drm/i915/gen9: Don't wrap strings in verify_wm_state()
@ 2016-10-14 21:31 ` Lyude
0 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-14 21:31 UTC (permalink / raw)
To: intel-gfx
Cc: Paulo Zanoni, David Airlie, linux-kernel, dri-devel, Daniel Vetter
Wrapping strings is against the guidelines in Documentation/CodingStyle,
chapter 2.
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ca12f0e..eaec9f3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13509,8 +13509,7 @@ static void verify_wm_state(struct drm_crtc *crtc,
sw_ddb_entry = &sw_ddb->plane[pipe][plane];
if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
- DRM_ERROR("mismatch in DDB state pipe %c plane %d "
- "(expected (%u,%u), found (%u,%u))\n",
+ DRM_ERROR("mismatch in DDB state pipe %c plane %d (expected (%u,%u), found (%u,%u))\n",
pipe_name(pipe), plane + 1,
sw_ddb_entry->start, sw_ddb_entry->end,
hw_ddb_entry->start, hw_ddb_entry->end);
@@ -13560,8 +13559,7 @@ static void verify_wm_state(struct drm_crtc *crtc,
sw_ddb_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
- DRM_ERROR("mismatch in DDB state pipe %c cursor "
- "(expected (%u,%u), found (%u,%u))\n",
+ DRM_ERROR("mismatch in DDB state pipe %c cursor (expected (%u,%u), found (%u,%u))\n",
pipe_name(pipe),
sw_ddb_entry->start, sw_ddb_entry->end,
hw_ddb_entry->start, hw_ddb_entry->end);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* ✗ Fi.CI.BAT: warning for Start of skl watermark cleanup (rev3)
2016-10-14 21:31 ` Lyude
` (10 preceding siblings ...)
(?)
@ 2016-10-14 22:21 ` Patchwork
2016-10-17 6:00 ` Saarinen, Jani
-1 siblings, 1 reply; 28+ messages in thread
From: Patchwork @ 2016-10-14 22:21 UTC (permalink / raw)
To: cpaul; +Cc: intel-gfx
== Series Details ==
Series: Start of skl watermark cleanup (rev3)
URL : https://patchwork.freedesktop.org/series/13333/
State : warning
== Summary ==
Series 13333v3 Start of skl watermark cleanup
https://patchwork.freedesktop.org/api/1.0/series/13333/revisions/3/mbox/
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
dmesg-warn -> PASS (fi-skl-6700k)
dmesg-warn -> PASS (fi-skl-6770hq)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-a-frame-sequence:
dmesg-warn -> PASS (fi-ilk-650)
Subgroup read-crc-pipe-b:
dmesg-warn -> PASS (fi-ilk-650)
Subgroup suspend-read-crc-pipe-c:
incomplete -> PASS (fi-skl-6700k)
Test vgem_basic:
Subgroup unload:
pass -> SKIP (fi-hsw-4770)
pass -> SKIP (fi-bdw-5557u)
skip -> PASS (fi-skl-6770hq)
fi-bdw-5557u total:246 pass:230 dwarn:0 dfail:0 fail:0 skip:16
fi-bsw-n3050 total:246 pass:204 dwarn:0 dfail:0 fail:0 skip:42
fi-bxt-t5700 total:246 pass:216 dwarn:0 dfail:0 fail:0 skip:30
fi-byt-n2820 total:246 pass:210 dwarn:0 dfail:0 fail:1 skip:35
fi-hsw-4770 total:246 pass:223 dwarn:0 dfail:0 fail:0 skip:23
fi-hsw-4770r total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22
fi-ilk-650 total:246 pass:184 dwarn:0 dfail:0 fail:2 skip:60
fi-ivb-3520m total:246 pass:221 dwarn:0 dfail:0 fail:0 skip:25
fi-ivb-3770 total:246 pass:221 dwarn:0 dfail:0 fail:0 skip:25
fi-kbl-7200u total:246 pass:222 dwarn:0 dfail:0 fail:0 skip:24
fi-skl-6260u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14
fi-skl-6700hq total:246 pass:223 dwarn:0 dfail:0 fail:0 skip:23
fi-skl-6700k total:246 pass:221 dwarn:1 dfail:0 fail:0 skip:24
fi-skl-6770hq total:246 pass:230 dwarn:1 dfail:0 fail:1 skip:14
fi-snb-2520m total:246 pass:210 dwarn:0 dfail:0 fail:0 skip:36
fi-snb-2600 total:246 pass:209 dwarn:0 dfail:0 fail:0 skip:37
fi-byt-j1900 failed to connect after reboot
Results at /archive/results/CI_IGT_test/Patchwork_2729/
38ecbe9082e477953fe165265c76e6c6061aeaf6 drm-intel-nightly: 2016y-10m-14d-16h-23m-48s UTC integration manifest
e6ec3db drm/i915/gen9: Don't wrap strings in verify_wm_state()
be9c54e drm/i915/gen9: Actually verify WM levels in verify_wm_state()
158bb87 drm/i915/gen9: Add skl_wm_level_equals()
95570fd drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable
1f4ad42 drm/i915/gen9: Add ddb changes to atomic debug output
9fd875d drm/i915/gen9: Get rid of redundant watermark values
aaf254a drm/i915/gen9: Cleanup skl_pipe_wm_active_state
6e1087f drm/i915/gen9: Make skl_wm_level per-plane
b1399ab drm/i915/skl: Remove linetime from skl_wm_values
599c303 drm/i915/skl: Move per-pipe ddb allocations into crtc states
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: ✗ Fi.CI.BAT: warning for Start of skl watermark cleanup (rev3)
2016-10-14 22:21 ` ✗ Fi.CI.BAT: warning for Start of skl watermark cleanup (rev3) Patchwork
@ 2016-10-17 6:00 ` Saarinen, Jani
0 siblings, 0 replies; 28+ messages in thread
From: Saarinen, Jani @ 2016-10-17 6:00 UTC (permalink / raw)
To: intel-gfx, cpaul
> == Series Details ==
>
> Series: Start of skl watermark cleanup (rev3)
> URL : https://patchwork.freedesktop.org/series/13333/
> State : warning
>
> == Summary ==
>
> Series 13333v3 Start of skl watermark cleanup
> https://patchwork.freedesktop.org/api/1.0/series/13333/revisions/3/mbox/
>
> Test kms_flip:
> Subgroup basic-flip-vs-wf_vblank:
> dmesg-warn -> PASS (fi-skl-6700k)
> dmesg-warn -> PASS (fi-skl-6770hq)
> Test kms_pipe_crc_basic:
> Subgroup read-crc-pipe-a-frame-sequence:
> dmesg-warn -> PASS (fi-ilk-650)
> Subgroup read-crc-pipe-b:
> dmesg-warn -> PASS (fi-ilk-650)
> Subgroup suspend-read-crc-pipe-c:
> incomplete -> PASS (fi-skl-6700k)
> Test vgem_basic:
> Subgroup unload:
> pass -> SKIP (fi-hsw-4770)
> pass -> SKIP (fi-bdw-5557u)
> skip -> PASS (fi-skl-6770hq)
Known un-stable. Will be worked on still.
>
> fi-bdw-5557u total:246 pass:230 dwarn:0 dfail:0 fail:0 skip:16
> fi-bsw-n3050 total:246 pass:204 dwarn:0 dfail:0 fail:0 skip:42
> fi-bxt-t5700 total:246 pass:216 dwarn:0 dfail:0 fail:0 skip:30
> fi-byt-n2820 total:246 pass:210 dwarn:0 dfail:0 fail:1 skip:35
> fi-hsw-4770 total:246 pass:223 dwarn:0 dfail:0 fail:0 skip:23
> fi-hsw-4770r total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22
> fi-ilk-650 total:246 pass:184 dwarn:0 dfail:0 fail:2 skip:60
> fi-ivb-3520m total:246 pass:221 dwarn:0 dfail:0 fail:0 skip:25
> fi-ivb-3770 total:246 pass:221 dwarn:0 dfail:0 fail:0 skip:25
> fi-kbl-7200u total:246 pass:222 dwarn:0 dfail:0 fail:0 skip:24
> fi-skl-6260u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14
> fi-skl-6700hq total:246 pass:223 dwarn:0 dfail:0 fail:0 skip:23
> fi-skl-6700k total:246 pass:221 dwarn:1 dfail:0 fail:0 skip:24
> fi-skl-6770hq total:246 pass:230 dwarn:1 dfail:0 fail:1 skip:14
> fi-snb-2520m total:246 pass:210 dwarn:0 dfail:0 fail:0 skip:36
> fi-snb-2600 total:246 pass:209 dwarn:0 dfail:0 fail:0 skip:37
> fi-byt-j1900 failed to connect after reboot
>
> Results at /archive/results/CI_IGT_test/Patchwork_2729/
>
> 38ecbe9082e477953fe165265c76e6c6061aeaf6 drm-intel-nightly: 2016y-10m-
> 14d-16h-23m-48s UTC integration manifest e6ec3db drm/i915/gen9: Don't
> wrap strings in verify_wm_state() be9c54e drm/i915/gen9: Actually verify
> WM levels in verify_wm_state()
> 158bb87 drm/i915/gen9: Add skl_wm_level_equals() 95570fd
> drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable
> 1f4ad42 drm/i915/gen9: Add ddb changes to atomic debug output 9fd875d
> drm/i915/gen9: Get rid of redundant watermark values aaf254a
> drm/i915/gen9: Cleanup skl_pipe_wm_active_state 6e1087f drm/i915/gen9:
> Make skl_wm_level per-plane b1399ab drm/i915/skl: Remove linetime from
> skl_wm_values
> 599c303 drm/i915/skl: Move per-pipe ddb allocations into crtc states
>
Jani Saarinen
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 00/10] Start of skl watermark cleanup
2016-10-14 21:31 ` Lyude
@ 2016-10-16 23:21 ` Lyude
-1 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-16 23:21 UTC (permalink / raw)
To: intel-gfx
Cc: Daniel Vetter, Jani Nikula, David Airlie, dri-devel, linux-kernel
Pushed patches 1-4 in this series (with some very small style changes
to make checkpatch happy), drm-intel-nightly also rebuilt.
On Fri, 2016-10-14 at 17:31 -0400, Lyude wrote:
> While it (mostly) works, the code for handling watermarks on Skylake
> has been
> kind of ugly for a while. As well a lot of it isn't that friendly to
> atomic
> transactions, Lots of copy paste, redundant wm values, etc. While
> this isn't a
> full cleanup, it's a good start. As well, we add a couple of features
> for
> making debugging watermarks a little easier.
>
> Rebased for latest nightly, new r-bs added + some changes
>
> Lyude (10):
> drm/i915/skl: Move per-pipe ddb allocations into crtc states
> drm/i915/skl: Remove linetime from skl_wm_values
> drm/i915/gen9: Make skl_wm_level per-plane
> drm/i915/gen9: Cleanup skl_pipe_wm_active_state
> drm/i915/gen9: Get rid of redundant watermark values
> drm/i915/gen9: Add ddb changes to atomic debug output
> drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable
> drm/i915/gen9: Add skl_wm_level_equals()
> drm/i915/gen9: Actually verify WM levels in verify_wm_state()
> drm/i915/gen9: Don't wrap strings in verify_wm_state()
>
> drivers/gpu/drm/i915/i915_drv.h | 10 +-
> drivers/gpu/drm/i915/intel_display.c | 138 ++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 24 +-
> drivers/gpu/drm/i915/intel_pm.c | 466 +++++++++++++++++------
> ------------
> drivers/gpu/drm/i915/intel_sprite.c | 8 +-
> 5 files changed, 355 insertions(+), 291 deletions(-)
>
--
Cheers,
Lyude
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 00/10] Start of skl watermark cleanup
@ 2016-10-16 23:21 ` Lyude
0 siblings, 0 replies; 28+ messages in thread
From: Lyude @ 2016-10-16 23:21 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter, dri-devel, linux-kernel
Pushed patches 1-4 in this series (with some very small style changes
to make checkpatch happy), drm-intel-nightly also rebuilt.
On Fri, 2016-10-14 at 17:31 -0400, Lyude wrote:
> While it (mostly) works, the code for handling watermarks on Skylake
> has been
> kind of ugly for a while. As well a lot of it isn't that friendly to
> atomic
> transactions, Lots of copy paste, redundant wm values, etc. While
> this isn't a
> full cleanup, it's a good start. As well, we add a couple of features
> for
> making debugging watermarks a little easier.
>
> Rebased for latest nightly, new r-bs added + some changes
>
> Lyude (10):
> drm/i915/skl: Move per-pipe ddb allocations into crtc states
> drm/i915/skl: Remove linetime from skl_wm_values
> drm/i915/gen9: Make skl_wm_level per-plane
> drm/i915/gen9: Cleanup skl_pipe_wm_active_state
> drm/i915/gen9: Get rid of redundant watermark values
> drm/i915/gen9: Add ddb changes to atomic debug output
> drm/i915/gen9: Make skl_pipe_wm_get_hw_state() reusable
> drm/i915/gen9: Add skl_wm_level_equals()
> drm/i915/gen9: Actually verify WM levels in verify_wm_state()
> drm/i915/gen9: Don't wrap strings in verify_wm_state()
>
> drivers/gpu/drm/i915/i915_drv.h | 10 +-
> drivers/gpu/drm/i915/intel_display.c | 138 ++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 24 +-
> drivers/gpu/drm/i915/intel_pm.c | 466 +++++++++++++++++------
> ------------
> drivers/gpu/drm/i915/intel_sprite.c | 8 +-
> 5 files changed, 355 insertions(+), 291 deletions(-)
>
--
Cheers,
Lyude
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 28+ messages in thread