[Intel-gfx] [PATCH 05/15] drm/i915/skl: Simplify wm structures slightly (v2)
Daniel Vetter
daniel at ffwll.ch
Wed Sep 30 08:13:48 PDT 2015
On Thu, Sep 24, 2015 at 03:53:10PM -0700, Matt Roper wrote:
> A bunch of SKL watermark-related structures have the cursor plane as a
> separate entry from the rest of the planes. Since a previous patch
> updated I915_MAX_PLANES such that those plane arrays now have a slot for
> the cursor, update the code to use the new slot in the existing plane
> arrays and kill off the cursor-specific structures.
>
> There shouldn't be any functional change here; this is just shuffling
> around how the data is stored in some of the data structures. The whole
> patch is generated with Coccinelle via the following semantic patch:
>
> @@ struct skl_pipe_wm_parameters WMP; @@
> - WMP.cursor
> + WMP.plane[PLANE_CURSOR]
>
> @@ struct skl_pipe_wm_parameters *WMP; @@
> - WMP->cursor
> + WMP->plane[PLANE_CURSOR]
>
> @@ @@
> struct skl_pipe_wm_parameters {
> ...
> - struct intel_plane_wm_parameters cursor;
> ...
> };
>
> @@
> struct skl_ddb_allocation DDB;
> expression E;
> @@
> - DDB.cursor[E]
> + DDB.plane[E][PLANE_CURSOR]
>
> @@
> struct skl_ddb_allocation *DDB;
> expression E;
> @@
> - DDB->cursor[E]
> + DDB->plane[E][PLANE_CURSOR]
>
> @@ @@
> struct skl_ddb_allocation {
> ...
> - struct skl_ddb_entry cursor[I915_MAX_PIPES];
> ...
> };
>
> @@
> struct skl_wm_values WMV;
> expression E1, E2;
> @@
> (
> - WMV.cursor[E1][E2]
> + WMV.plane[E1][PLANE_CURSOR][E2]
> |
> - WMV.cursor_trans[E1]
> + WMV.plane_trans[E1][PLANE_CURSOR]
> )
>
> @@
> struct skl_wm_values *WMV;
> expression E1, E2;
> @@
> (
> - WMV->cursor[E1][E2]
> + WMV->plane[E1][PLANE_CURSOR][E2]
> |
> - WMV->cursor_trans[E1]
> + WMV->plane_trans[E1][PLANE_CURSOR]
> )
>
> @@ @@
> struct skl_wm_values {
> ...
> - uint32_t cursor[I915_MAX_PIPES][8];
> ...
> - uint32_t cursor_trans[I915_MAX_PIPES];
> ...
> };
>
> @@ struct skl_wm_level WML; @@
> (
> - WML.cursor_en
> + WML.plane_en[PLANE_CURSOR]
> |
> - WML.cursor_res_b
> + WML.plane_res_b[PLANE_CURSOR]
> |
> - WML.cursor_res_l
> + WML.plane_res_l[PLANE_CURSOR]
> )
>
> @@ struct skl_wm_level *WML; @@
> (
> - WML->cursor_en
> + WML->plane_en[PLANE_CURSOR]
> |
> - WML->cursor_res_b
> + WML->plane_res_b[PLANE_CURSOR]
> |
> - WML->cursor_res_l
> + WML->plane_res_l[PLANE_CURSOR]
> )
>
> @@ @@
> struct skl_wm_level {
> ...
> - bool cursor_en;
> ...
> - uint16_t cursor_res_b;
> - uint8_t cursor_res_l;
> ...
> };
>
> v2: Use a PLANE_CURSOR enum entry rather than making the code reference
> I915_MAX_PLANES or I915_MAX_PLANES+1, which was confusing. (Ander)
>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
Merged up to this one here.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.h | 8 +---
> drivers/gpu/drm/i915/intel_display.c | 4 +-
> drivers/gpu/drm/i915/intel_pm.c | 93 +++++++++++++++++++-----------------
> 4 files changed, 52 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 5615d3d..03c196e 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3144,7 +3144,7 @@ static int i915_ddb_info(struct seq_file *m, void *unused)
> skl_ddb_entry_size(entry));
> }
>
> - entry = &ddb->cursor[pipe];
> + entry = &ddb->plane[pipe][PLANE_CURSOR];
> seq_printf(m, " %-13s%8u%8u%8u\n", "Cursor", entry->start,
> entry->end, skl_ddb_entry_size(entry));
> }
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index eac9414..bd542cb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1588,8 +1588,7 @@ 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]; /* y-plane */
> - struct skl_ddb_entry cursor[I915_MAX_PIPES];
> + struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
> };
>
> struct skl_wm_values {
> @@ -1597,18 +1596,13 @@ struct skl_wm_values {
> struct skl_ddb_allocation ddb;
> uint32_t wm_linetime[I915_MAX_PIPES];
> uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
> - uint32_t cursor[I915_MAX_PIPES][8];
> uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
> - uint32_t cursor_trans[I915_MAX_PIPES];
> };
>
> struct skl_wm_level {
> bool plane_en[I915_MAX_PLANES];
> - bool cursor_en;
> uint16_t plane_res_b[I915_MAX_PLANES];
> uint8_t plane_res_l[I915_MAX_PLANES];
> - uint16_t cursor_res_b;
> - uint8_t cursor_res_l;
> };
>
> /*
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dea1f23..a3e62bc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12564,8 +12564,8 @@ static void check_wm_state(struct drm_device *dev)
> }
>
> /* cursor */
> - hw_entry = &hw_ddb.cursor[pipe];
> - sw_entry = &sw_ddb->cursor[pipe];
> + hw_entry = &hw_ddb.plane[pipe][PLANE_CURSOR];
> + sw_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
>
> if (skl_ddb_entry_equal(hw_entry, sw_entry))
> continue;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 9e6e9c2..8829047 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1792,7 +1792,6 @@ struct skl_pipe_wm_parameters {
> uint32_t pipe_htotal;
> uint32_t pixel_rate; /* in KHz */
> struct intel_plane_wm_parameters plane[I915_MAX_PLANES];
> - struct intel_plane_wm_parameters cursor;
> };
>
> struct ilk_wm_maximums {
> @@ -2906,7 +2905,8 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
> }
>
> val = I915_READ(CUR_BUF_CFG(pipe));
> - skl_ddb_entry_init_from_hw(&ddb->cursor[pipe], val);
> + skl_ddb_entry_init_from_hw(&ddb->plane[pipe][PLANE_CURSOR],
> + val);
> }
> }
>
> @@ -2975,13 +2975,14 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
> alloc_size = skl_ddb_entry_size(alloc);
> if (alloc_size == 0) {
> memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
> - memset(&ddb->cursor[pipe], 0, sizeof(ddb->cursor[pipe]));
> + memset(&ddb->plane[pipe][PLANE_CURSOR], 0,
> + sizeof(ddb->plane[pipe][PLANE_CURSOR]));
> return;
> }
>
> cursor_blocks = skl_cursor_allocation(config);
> - ddb->cursor[pipe].start = alloc->end - cursor_blocks;
> - ddb->cursor[pipe].end = alloc->end;
> + ddb->plane[pipe][PLANE_CURSOR].start = alloc->end - cursor_blocks;
> + ddb->plane[pipe][PLANE_CURSOR].end = alloc->end;
>
> alloc_size -= cursor_blocks;
> alloc->end -= cursor_blocks;
> @@ -3120,8 +3121,8 @@ static bool skl_ddb_allocation_changed(const struct skl_ddb_allocation *new_ddb,
> sizeof(new_ddb->plane[pipe])))
> return true;
>
> - if (memcmp(&new_ddb->cursor[pipe], &cur_ddb->cursor[pipe],
> - sizeof(new_ddb->cursor[pipe])))
> + if (memcmp(&new_ddb->plane[pipe][PLANE_CURSOR], &cur_ddb->plane[pipe][PLANE_CURSOR],
> + sizeof(new_ddb->plane[pipe][PLANE_CURSOR])))
> return true;
>
> return false;
> @@ -3181,17 +3182,17 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
> p->plane[0].rotation = crtc->primary->state->rotation;
>
> fb = crtc->cursor->state->fb;
> - p->cursor.y_bytes_per_pixel = 0;
> + p->plane[PLANE_CURSOR].y_bytes_per_pixel = 0;
> if (fb) {
> - p->cursor.enabled = true;
> - p->cursor.bytes_per_pixel = fb->bits_per_pixel / 8;
> - p->cursor.horiz_pixels = crtc->cursor->state->crtc_w;
> - p->cursor.vert_pixels = crtc->cursor->state->crtc_h;
> + p->plane[PLANE_CURSOR].enabled = true;
> + p->plane[PLANE_CURSOR].bytes_per_pixel = fb->bits_per_pixel / 8;
> + p->plane[PLANE_CURSOR].horiz_pixels = crtc->cursor->state->crtc_w;
> + p->plane[PLANE_CURSOR].vert_pixels = crtc->cursor->state->crtc_h;
> } else {
> - p->cursor.enabled = false;
> - p->cursor.bytes_per_pixel = 0;
> - p->cursor.horiz_pixels = 64;
> - p->cursor.vert_pixels = 64;
> + p->plane[PLANE_CURSOR].enabled = false;
> + p->plane[PLANE_CURSOR].bytes_per_pixel = 0;
> + p->plane[PLANE_CURSOR].horiz_pixels = 64;
> + p->plane[PLANE_CURSOR].vert_pixels = 64;
> }
> }
>
> @@ -3305,11 +3306,12 @@ static void skl_compute_wm_level(const struct drm_i915_private *dev_priv,
> &result->plane_res_l[i]);
> }
>
> - ddb_blocks = skl_ddb_entry_size(&ddb->cursor[pipe]);
> - result->cursor_en = skl_compute_plane_wm(dev_priv, p, &p->cursor,
> + ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][PLANE_CURSOR]);
> + result->plane_en[PLANE_CURSOR] = skl_compute_plane_wm(dev_priv, p,
> + &p->plane[PLANE_CURSOR],
> ddb_blocks, level,
> - &result->cursor_res_b,
> - &result->cursor_res_l);
> + &result->plane_res_b[PLANE_CURSOR],
> + &result->plane_res_l[PLANE_CURSOR]);
> }
>
> static uint32_t
> @@ -3337,7 +3339,7 @@ static void skl_compute_transition_wm(struct drm_crtc *crtc,
> /* Until we know more, just disable transition WMs */
> for (i = 0; i < intel_num_planes(intel_crtc); i++)
> trans_wm->plane_en[i] = false;
> - trans_wm->cursor_en = false;
> + trans_wm->plane_en[PLANE_CURSOR] = false;
> }
>
> static void skl_compute_pipe_wm(struct drm_crtc *crtc,
> @@ -3386,13 +3388,13 @@ static void skl_compute_wm_results(struct drm_device *dev,
>
> temp = 0;
>
> - temp |= p_wm->wm[level].cursor_res_l << PLANE_WM_LINES_SHIFT;
> - temp |= p_wm->wm[level].cursor_res_b;
> + 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].cursor_en)
> + if (p_wm->wm[level].plane_en[PLANE_CURSOR])
> temp |= PLANE_WM_EN;
>
> - r->cursor[pipe][level] = temp;
> + r->plane[pipe][PLANE_CURSOR][level] = temp;
>
> }
>
> @@ -3408,12 +3410,12 @@ static void skl_compute_wm_results(struct drm_device *dev,
> }
>
> temp = 0;
> - temp |= p_wm->trans_wm.cursor_res_l << PLANE_WM_LINES_SHIFT;
> - temp |= p_wm->trans_wm.cursor_res_b;
> - if (p_wm->trans_wm.cursor_en)
> + 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_EN;
>
> - r->cursor_trans[pipe] = temp;
> + r->plane_trans[pipe][PLANE_CURSOR] = temp;
>
> r->wm_linetime[pipe] = p_wm->linetime;
> }
> @@ -3447,12 +3449,13 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv,
> I915_WRITE(PLANE_WM(pipe, i, level),
> new->plane[pipe][i][level]);
> I915_WRITE(CUR_WM(pipe, level),
> - new->cursor[pipe][level]);
> + new->plane[pipe][PLANE_CURSOR][level]);
> }
> for (i = 0; i < intel_num_planes(crtc); i++)
> I915_WRITE(PLANE_WM_TRANS(pipe, i),
> new->plane_trans[pipe][i]);
> - I915_WRITE(CUR_WM_TRANS(pipe), new->cursor_trans[pipe]);
> + I915_WRITE(CUR_WM_TRANS(pipe),
> + new->plane_trans[pipe][PLANE_CURSOR]);
>
> for (i = 0; i < intel_num_planes(crtc); i++) {
> skl_ddb_entry_write(dev_priv,
> @@ -3464,7 +3467,7 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv,
> }
>
> skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe),
> - &new->ddb.cursor[pipe]);
> + &new->ddb.plane[pipe][PLANE_CURSOR]);
> }
> }
>
> @@ -3677,10 +3680,9 @@ static void skl_clear_wm(struct skl_wm_values *watermarks, enum pipe pipe)
> watermarks->wm_linetime[pipe] = 0;
> memset(watermarks->plane[pipe], 0,
> sizeof(uint32_t) * 8 * I915_MAX_PLANES);
> - memset(watermarks->cursor[pipe], 0, sizeof(uint32_t) * 8);
> memset(watermarks->plane_trans[pipe],
> 0, sizeof(uint32_t) * I915_MAX_PLANES);
> - watermarks->cursor_trans[pipe] = 0;
> + watermarks->plane_trans[pipe][PLANE_CURSOR] = 0;
>
> /* Clear ddb entries for pipe */
> memset(&watermarks->ddb.pipe[pipe], 0, sizeof(struct skl_ddb_entry));
> @@ -3688,7 +3690,8 @@ static void skl_clear_wm(struct skl_wm_values *watermarks, enum pipe pipe)
> sizeof(struct skl_ddb_entry) * I915_MAX_PLANES);
> memset(&watermarks->ddb.y_plane[pipe], 0,
> sizeof(struct skl_ddb_entry) * I915_MAX_PLANES);
> - memset(&watermarks->ddb.cursor[pipe], 0, sizeof(struct skl_ddb_entry));
> + memset(&watermarks->ddb.plane[pipe][PLANE_CURSOR], 0,
> + sizeof(struct skl_ddb_entry));
>
> }
>
> @@ -3844,10 +3847,10 @@ static void skl_pipe_wm_active_state(uint32_t val,
> (val >> PLANE_WM_LINES_SHIFT) &
> PLANE_WM_LINES_MASK;
> } else {
> - active->wm[level].cursor_en = is_enabled;
> - active->wm[level].cursor_res_b =
> + 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].cursor_res_l =
> + active->wm[level].plane_res_l[PLANE_CURSOR] =
> (val >> PLANE_WM_LINES_SHIFT) &
> PLANE_WM_LINES_MASK;
> }
> @@ -3860,10 +3863,10 @@ static void skl_pipe_wm_active_state(uint32_t val,
> (val >> PLANE_WM_LINES_SHIFT) &
> PLANE_WM_LINES_MASK;
> } else {
> - active->trans_wm.cursor_en = is_enabled;
> - active->trans_wm.cursor_res_b =
> + 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.cursor_res_l =
> + active->trans_wm.plane_res_l[PLANE_CURSOR] =
> (val >> PLANE_WM_LINES_SHIFT) &
> PLANE_WM_LINES_MASK;
> }
> @@ -3889,12 +3892,12 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> for (i = 0; i < intel_num_planes(intel_crtc); i++)
> hw->plane[pipe][i][level] =
> I915_READ(PLANE_WM(pipe, i, level));
> - hw->cursor[pipe][level] = I915_READ(CUR_WM(pipe, 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->cursor_trans[pipe] = I915_READ(CUR_WM_TRANS(pipe));
> + hw->plane_trans[pipe][PLANE_CURSOR] = I915_READ(CUR_WM_TRANS(pipe));
>
> if (!intel_crtc->active)
> return;
> @@ -3909,7 +3912,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> skl_pipe_wm_active_state(temp, active, false,
> false, i, level);
> }
> - temp = hw->cursor[pipe][level];
> + temp = hw->plane[pipe][PLANE_CURSOR][level];
> skl_pipe_wm_active_state(temp, active, false, true, i, level);
> }
>
> @@ -3918,7 +3921,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> skl_pipe_wm_active_state(temp, active, true, false, i, 0);
> }
>
> - temp = hw->cursor_trans[pipe];
> + temp = hw->plane_trans[pipe][PLANE_CURSOR];
> skl_pipe_wm_active_state(temp, active, true, true, i, 0);
> }
>
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list