From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 16/16] drm/i915: Handle SKL+ WM/DDB registers next to all other plane registers
Date: Wed, 15 May 2024 14:17:09 +0300 [thread overview]
Message-ID: <ZkSZtcjf24OnAiEH@intel.com> (raw)
In-Reply-To: <871q65e8p3.fsf@intel.com>
On Mon, May 13, 2024 at 11:52:08PM +0300, Jani Nikula wrote:
> On Fri, 10 May 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > 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ä <ville.syrjala@linux.intel.com>
>
> [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.
Yeah, I was mulling over whether to just define the register bits
separately for the cursor registers as well and have its own versions
of these. Might be what I'll end up doing.
I think there are also still some other PSR related plane registers
that are defined in a non-standard way, so those might need similar
treatment as well.
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Thanks. Pushed the lot.
>
> > +
> > #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
--
Ville Syrjälä
Intel
next prev parent reply other threads:[~2024-05-15 11:17 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-10 15:23 [PATCH 00/16] drm/i915: skl+ plane register stuff Ville Syrjala
2024-05-10 15:23 ` [PATCH 01/16] drm/i915: Nuke _MMIO_PLANE_GAMC() Ville Syrjala
2024-05-13 9:50 ` Jani Nikula
2024-05-10 15:23 ` [PATCH 02/16] drm/i915: Extract skl_universal_plane_regs.h Ville Syrjala
2024-05-13 10:07 ` Jani Nikula
2024-05-10 15:23 ` [PATCH 03/16] drm/i915: Extract intel_cursor_regs.h Ville Syrjala
2024-05-13 10:10 ` Jani Nikula
2024-05-10 15:23 ` [PATCH 04/16] drm/i915: Move skl+ wm/ddb registers to proper headers Ville Syrjala
2024-05-13 10:13 ` Jani Nikula
2024-05-10 15:23 ` [PATCH 05/16] drm/i915/gvt: Use the proper PLANE_AUX_DIST() define Ville Syrjala
2024-05-13 10:21 ` Jani Nikula
2024-05-10 15:23 ` [PATCH 06/16] drm/i915/gvt: Use the proper PLANE_AUX_OFFSET() define Ville Syrjala
2024-05-13 10:23 ` Jani Nikula
2024-05-10 15:23 ` [PATCH 07/16] drm/i915/gvt: Use the full PLANE_KEY*() defines Ville Syrjala
2024-05-13 10:25 ` Jani Nikula
2024-05-10 15:23 ` [PATCH 08/16] drm/i915/gvt: Use PLANE_CTL and PLANE_SURF defines Ville Syrjala
2024-05-13 10:30 ` Jani Nikula
2024-05-10 15:23 ` [PATCH 09/16] drm/i915: Drop useless PLANE_FOO_3 register defines Ville Syrjala
2024-05-13 10:32 ` Jani Nikula
2024-05-13 16:58 ` [PATCH v2 " Ville Syrjala
2024-05-10 15:23 ` [PATCH 10/16] drm/i915: Shuffle the skl+ plane register definitions Ville Syrjala
2024-05-13 11:28 ` Jani Nikula
2024-05-13 16:13 ` Ville Syrjälä
2024-05-13 16:59 ` [PATCH v2 " Ville Syrjala
2024-05-13 20:30 ` Jani Nikula
2024-05-10 15:23 ` [PATCH 11/16] drm/i915: Use REG_BIT for PLANE_WM bits Ville Syrjala
2024-05-13 10:38 ` Jani Nikula
2024-05-13 16:59 ` [PATCH v2 " Ville Syrjala
2024-05-10 15:23 ` [PATCH 12/16] drm/i915: Drop a few unwanted tabs from skl+ plane reg defines Ville Syrjala
2024-05-13 10:40 ` Jani Nikula
2024-05-13 17:00 ` [PATCH v2 " Ville Syrjala
2024-05-10 15:23 ` [PATCH 13/16] drm/i915: Refactor skl+ plane register offset calculations Ville Syrjala
2024-05-13 17:00 ` [PATCH v2 " Ville Syrjala
2024-05-13 20:41 ` Jani Nikula
2024-05-13 20:43 ` Jani Nikula
2024-05-10 15:23 ` [PATCH 14/16] drm/i915: Extract skl_plane_{wm,ddb}_reg_val() Ville Syrjala
2024-05-13 20:43 ` Jani Nikula
2024-05-10 15:23 ` [PATCH 15/16] drm/i915: Nuke skl_write_wm_level() and skl_ddb_entry_write() Ville Syrjala
2024-05-13 20:46 ` Jani Nikula
2024-05-10 15:23 ` [PATCH 16/16] drm/i915: Handle SKL+ WM/DDB registers next to all other plane registers Ville Syrjala
2024-05-13 20:52 ` Jani Nikula
2024-05-15 11:17 ` Ville Syrjälä [this message]
2024-05-10 16:28 ` ✓ Fi.CI.BAT: success for drm/i915: skl+ plane register stuff Patchwork
2024-05-11 20:02 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-05-13 18:39 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: skl+ plane register stuff (rev6) Patchwork
2024-05-13 18:39 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-05-13 18:55 ` ✓ Fi.CI.BAT: success " Patchwork
2024-05-14 2:01 ` ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZkSZtcjf24OnAiEH@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).