On Fri, 10 May 2024, Ville Syrjala <[email protected]> wrote: > From: Ville Syrjälä <[email protected]> > > Having the plane WM/DDB regitster write functions in skl_watermarks.c > is rather annoying when trying to implement DSB based plane updates. > Move them into the respective files that handle all other plane > register writes. Less places where I need to worry about the DSB > vs. MMIO decisions. > > The downside is that we spread the wm struct details a bit further > afield. But if that becomes too annoying we can probably abstract > things a bit more with a few extra functions. > > Signed-off-by: Ville Syrjälä <[email protected]>
[snip] > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.h > b/drivers/gpu/drm/i915/display/skl_universal_plane.h > index e92e00c01b29..8eb4521ee851 100644 > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.h > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.h > @@ -12,6 +12,8 @@ struct drm_i915_private; > struct intel_crtc; > struct intel_initial_plane_config; > struct intel_plane_state; > +struct skl_ddb_entry; > +struct skl_wm_level; > > enum pipe; > enum plane_id; > @@ -35,4 +37,7 @@ bool icl_is_nv12_y_plane(struct drm_i915_private *dev_priv, > u8 icl_hdr_plane_mask(void); > bool icl_is_hdr_plane(struct drm_i915_private *dev_priv, enum plane_id > plane_id); > > +u32 skl_plane_ddb_reg_val(const struct skl_ddb_entry *entry); > +u32 skl_plane_wm_reg_val(const struct skl_wm_level *level); Yeah, I don't much like interfaces that return register values for registers that aren't even known... but let's see how this pans out. It does what it says on the box. Reviewed-by: Jani Nikula <[email protected]> > + > #endif > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c > b/drivers/gpu/drm/i915/display/skl_watermark.c > index 1daceb8ef9de..2064f72da675 100644 > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > @@ -1396,7 +1396,7 @@ skl_total_relative_data_rate(const struct > intel_crtc_state *crtc_state) > return data_rate; > } > > -static const struct skl_wm_level * > +const struct skl_wm_level * > skl_plane_wm_level(const struct skl_pipe_wm *pipe_wm, > enum plane_id plane_id, > int level) > @@ -1409,7 +1409,7 @@ skl_plane_wm_level(const struct skl_pipe_wm *pipe_wm, > return &wm->wm[level]; > } > > -static const struct skl_wm_level * > +const struct skl_wm_level * > skl_plane_trans_wm(const struct skl_pipe_wm *pipe_wm, > enum plane_id plane_id) > { > @@ -2365,97 +2365,6 @@ static int skl_build_pipe_wm(struct intel_atomic_state > *state, > return skl_wm_check_vblank(crtc_state); > } > > -static u32 skl_plane_ddb_reg_val(const struct skl_ddb_entry *entry) > -{ > - if (!entry->end) > - return 0; > - > - return PLANE_BUF_END(entry->end - 1) | > - PLANE_BUF_START(entry->start); > -} > - > -static u32 skl_plane_wm_reg_val(const struct skl_wm_level *level) > -{ > - u32 val = 0; > - > - if (level->enable) > - val |= PLANE_WM_EN; > - if (level->ignore_lines) > - val |= PLANE_WM_IGNORE_LINES; > - val |= REG_FIELD_PREP(PLANE_WM_BLOCKS_MASK, level->blocks); > - val |= REG_FIELD_PREP(PLANE_WM_LINES_MASK, level->lines); > - > - return val; > -} > - > -void skl_write_plane_wm(struct intel_plane *plane, > - const struct intel_crtc_state *crtc_state) > -{ > - struct drm_i915_private *i915 = to_i915(plane->base.dev); > - enum plane_id plane_id = plane->id; > - enum pipe pipe = plane->pipe; > - const struct skl_pipe_wm *pipe_wm = &crtc_state->wm.skl.optimal; > - const struct skl_ddb_entry *ddb = > - &crtc_state->wm.skl.plane_ddb[plane_id]; > - const struct skl_ddb_entry *ddb_y = > - &crtc_state->wm.skl.plane_ddb_y[plane_id]; > - int level; > - > - for (level = 0; level < i915->display.wm.num_levels; level++) > - intel_de_write_fw(i915, PLANE_WM(pipe, plane_id, level), > - > skl_plane_wm_reg_val(skl_plane_wm_level(pipe_wm, plane_id, level))); > - > - intel_de_write_fw(i915, PLANE_WM_TRANS(pipe, plane_id), > - skl_plane_wm_reg_val(skl_plane_trans_wm(pipe_wm, > plane_id))); > - > - if (HAS_HW_SAGV_WM(i915)) { > - const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id]; > - > - intel_de_write_fw(i915, PLANE_WM_SAGV(pipe, plane_id), > - skl_plane_wm_reg_val(&wm->sagv.wm0)); > - intel_de_write_fw(i915, PLANE_WM_SAGV_TRANS(pipe, plane_id), > - skl_plane_wm_reg_val(&wm->sagv.trans_wm)); > - } > - > - intel_de_write_fw(i915, PLANE_BUF_CFG(pipe, plane_id), > - skl_plane_ddb_reg_val(ddb)); > - > - if (DISPLAY_VER(i915) < 11) > - intel_de_write_fw(i915, PLANE_NV12_BUF_CFG(pipe, plane_id), > - skl_plane_ddb_reg_val(ddb_y)); > -} > - > -void skl_write_cursor_wm(struct intel_plane *plane, > - const struct intel_crtc_state *crtc_state) > -{ > - struct drm_i915_private *i915 = to_i915(plane->base.dev); > - enum plane_id plane_id = plane->id; > - enum pipe pipe = plane->pipe; > - const struct skl_pipe_wm *pipe_wm = &crtc_state->wm.skl.optimal; > - const struct skl_ddb_entry *ddb = > - &crtc_state->wm.skl.plane_ddb[plane_id]; > - int level; > - > - for (level = 0; level < i915->display.wm.num_levels; level++) > - intel_de_write_fw(i915, CUR_WM(pipe, level), > - > skl_plane_wm_reg_val(skl_plane_wm_level(pipe_wm, plane_id, level))); > - > - intel_de_write_fw(i915, CUR_WM_TRANS(pipe), > - skl_plane_wm_reg_val(skl_plane_trans_wm(pipe_wm, > plane_id))); > - > - if (HAS_HW_SAGV_WM(i915)) { > - const struct skl_plane_wm *wm = &pipe_wm->planes[plane_id]; > - > - intel_de_write_fw(i915, CUR_WM_SAGV(pipe), > - skl_plane_wm_reg_val(&wm->sagv.wm0)); > - intel_de_write_fw(i915, CUR_WM_SAGV_TRANS(pipe), > - skl_plane_wm_reg_val(&wm->sagv.trans_wm)); > - } > - > - intel_de_write_fw(i915, CUR_BUF_CFG(pipe), > - skl_plane_ddb_reg_val(ddb)); > -} > - > static bool skl_wm_level_equals(const struct skl_wm_level *l1, > const struct skl_wm_level *l2) > { > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h > b/drivers/gpu/drm/i915/display/skl_watermark.h > index 91f92c0e706e..78b121941237 100644 > --- a/drivers/gpu/drm/i915/display/skl_watermark.h > +++ b/drivers/gpu/drm/i915/display/skl_watermark.h > @@ -18,6 +18,8 @@ struct intel_bw_state; > struct intel_crtc; > struct intel_crtc_state; > struct intel_plane; > +struct skl_pipe_wm; > +struct skl_wm_level; > > u8 intel_enabled_dbuf_slices_mask(struct drm_i915_private *i915); > > @@ -30,11 +32,6 @@ bool intel_has_sagv(struct drm_i915_private *i915); > u32 skl_ddb_dbuf_slice_mask(struct drm_i915_private *i915, > const struct skl_ddb_entry *entry); > > -void skl_write_plane_wm(struct intel_plane *plane, > - const struct intel_crtc_state *crtc_state); > -void skl_write_cursor_wm(struct intel_plane *plane, > - const struct intel_crtc_state *crtc_state); > - > bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb, > const struct skl_ddb_entry *entries, > int num_entries, int ignore_idx); > @@ -51,6 +48,12 @@ unsigned int skl_watermark_max_latency(struct > drm_i915_private *i915, > int initial_wm_level); > void skl_wm_init(struct drm_i915_private *i915); > > +const struct skl_wm_level *skl_plane_wm_level(const struct skl_pipe_wm > *pipe_wm, > + enum plane_id plane_id, > + int level); > +const struct skl_wm_level *skl_plane_trans_wm(const struct skl_pipe_wm > *pipe_wm, > + enum plane_id plane_id); > + > struct intel_dbuf_state { > struct intel_global_state base; -- Jani Nikula, Intel
