From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Clinton Taylor <Clinton.A.Taylor@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/3] drm/i915: Include "ignore lines" in skl+ wm state
Date: Wed, 13 Feb 2019 23:28:54 +0200 [thread overview]
Message-ID: <20190213212854.GJ20097@intel.com> (raw)
In-Reply-To: <afd414a3-8c87-441f-b59b-5a02e5cbf031@intel.com>
On Wed, Feb 13, 2019 at 11:44:44AM -0800, Clinton Taylor wrote:
>
> On 2/13/19 8:54 AM, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We'll need to poke at the "ignore lines" bit in the skl+
> > watermark registers for a w/a. Include that bit in the wm
> > state.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > drivers/gpu/drm/i915/intel_pm.c | 44 +++++++++++++++++++++------------
> > 3 files changed, 30 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 17fe942eaafa..5c8d0489a1cd 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1126,6 +1126,7 @@ struct skl_wm_level {
> > u16 plane_res_b;
> > u8 plane_res_l;
> > bool plane_en;
> > + bool ignore_lines;
> > };
> >
> > /* Stores plane specific WM parameters */
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index f3f0b53c9e0e..7b3c41e9771f 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6032,6 +6032,7 @@ enum {
> > #define _CUR_WM_TRANS_A_0 0x70168
> > #define _CUR_WM_TRANS_B_0 0x71168
> > #define PLANE_WM_EN (1 << 31)
> > +#define PLANE_WM_IGNORE_LINES (1 << 30)
> > #define PLANE_WM_LINES_SHIFT 14
> > #define PLANE_WM_LINES_MASK 0x1f
> > #define PLANE_WM_BLOCKS_MASK 0x7ff /* skl+: 10 bits, icl+ 11 bits */
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index ea492fff6bf8..7dd2ab0ca21b 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5056,11 +5056,12 @@ static void skl_write_wm_level(struct drm_i915_private *dev_priv,
> > {
> > u32 val = 0;
> >
> > - if (level->plane_en) {
> > + if (level->plane_en)
> > val |= PLANE_WM_EN;
> > - val |= level->plane_res_b;
> > - val |= level->plane_res_l << PLANE_WM_LINES_SHIFT;
> > - }
> > + if (level->ignore_lines)
> > + val |= PLANE_WM_IGNORE_LINES;
> > + val |= level->plane_res_b;
> > + val |= level->plane_res_l << PLANE_WM_LINES_SHIFT;
>
> Is there a reason to program IGNORE_LINES, plane_res_b, and plane_res_l
> even when PLANE_WM_EN is not set? This is a change to functionality not
> described in the commit message.
Everything will be zero when the wm is disabled, except in
this special case for wm1. Just programming everything always
makes the code less messy.
>
> Since the WM is not enabled anyway:
>
> Reviewed-by: Clint Taylor <Clinton.A.Taylor@intel.com>
>
> -Clint
>
>
> >
> > I915_WRITE_FW(reg, val);
> > }
> > @@ -5126,6 +5127,7 @@ bool skl_wm_level_equals(const struct skl_wm_level *l1,
> > const struct skl_wm_level *l2)
> > {
> > return l1->plane_en == l2->plane_en &&
> > + l1->ignore_lines == l2->ignore_lines &&
> > l1->plane_res_l == l2->plane_res_l &&
> > l1->plane_res_b == l2->plane_res_b;
> > }
> > @@ -5334,19 +5336,28 @@ skl_print_wm_changes(struct intel_atomic_state *state)
> > enast(new_wm->wm[6].plane_en), enast(new_wm->wm[7].plane_en),
> > enast(new_wm->trans_wm.plane_en));
> >
> > - DRM_DEBUG_KMS("[PLANE:%d:%s] lines %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d"
> > - " -> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d\n",
> > + DRM_DEBUG_KMS("[PLANE:%d:%s] lines %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d"
> > + " -> %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d\n",
> > plane->base.base.id, plane->base.name,
> > - old_wm->wm[0].plane_res_l, old_wm->wm[1].plane_res_l,
> > - old_wm->wm[2].plane_res_l, old_wm->wm[3].plane_res_l,
> > - old_wm->wm[4].plane_res_l, old_wm->wm[5].plane_res_l,
> > - old_wm->wm[6].plane_res_l, old_wm->wm[7].plane_res_l,
> > - old_wm->trans_wm.plane_res_l,
> > - new_wm->wm[0].plane_res_l, new_wm->wm[1].plane_res_l,
> > - new_wm->wm[2].plane_res_l, new_wm->wm[3].plane_res_l,
> > - new_wm->wm[4].plane_res_l, new_wm->wm[5].plane_res_l,
> > - new_wm->wm[6].plane_res_l, new_wm->wm[7].plane_res_l,
> > - new_wm->trans_wm.plane_res_l);
> > + enast(old_wm->wm[0].ignore_lines), old_wm->wm[0].plane_res_l,
> > + enast(old_wm->wm[1].ignore_lines), old_wm->wm[1].plane_res_l,
> > + enast(old_wm->wm[2].ignore_lines), old_wm->wm[2].plane_res_l,
> > + enast(old_wm->wm[3].ignore_lines), old_wm->wm[3].plane_res_l,
> > + enast(old_wm->wm[4].ignore_lines), old_wm->wm[4].plane_res_l,
> > + enast(old_wm->wm[5].ignore_lines), old_wm->wm[5].plane_res_l,
> > + enast(old_wm->wm[6].ignore_lines), old_wm->wm[6].plane_res_l,
> > + enast(old_wm->wm[7].ignore_lines), old_wm->wm[7].plane_res_l,
> > + enast(old_wm->trans_wm.ignore_lines), old_wm->trans_wm.plane_res_l,
> > +
> > + enast(new_wm->wm[0].ignore_lines), new_wm->wm[0].plane_res_l,
> > + enast(new_wm->wm[1].ignore_lines), new_wm->wm[1].plane_res_l,
> > + enast(new_wm->wm[2].ignore_lines), new_wm->wm[2].plane_res_l,
> > + enast(new_wm->wm[3].ignore_lines), new_wm->wm[3].plane_res_l,
> > + enast(new_wm->wm[4].ignore_lines), new_wm->wm[4].plane_res_l,
> > + enast(new_wm->wm[5].ignore_lines), new_wm->wm[5].plane_res_l,
> > + enast(new_wm->wm[6].ignore_lines), new_wm->wm[6].plane_res_l,
> > + enast(new_wm->wm[7].ignore_lines), new_wm->wm[7].plane_res_l,
> > + enast(new_wm->trans_wm.ignore_lines), new_wm->trans_wm.plane_res_l);
> >
> > DRM_DEBUG_KMS("[PLANE:%d:%s] blocks %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d"
> > " -> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d\n",
> > @@ -5689,6 +5700,7 @@ static inline void skl_wm_level_from_reg_val(u32 val,
> > struct skl_wm_level *level)
> > {
> > level->plane_en = val & PLANE_WM_EN;
> > + level->ignore_lines = val & PLANE_WM_IGNORE_LINES;
> > level->plane_res_b = val & PLANE_WM_BLOCKS_MASK;
> > level->plane_res_l = (val >> PLANE_WM_LINES_SHIFT) &
> > PLANE_WM_LINES_MASK;
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-02-13 21:28 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-13 16:54 [PATCH 1/3] Revert "drm/i915: W/A for underruns with WM1+ disabled on icl" Ville Syrjala
2019-02-13 16:54 ` [PATCH 2/3] drm/i915: Include "ignore lines" in skl+ wm state Ville Syrjala
2019-02-13 19:44 ` Clinton Taylor
2019-02-13 21:28 ` Ville Syrjälä [this message]
2019-02-13 16:54 ` [PATCH 3/3] drm/i915: Implement new w/a for underruns with wm1+ disabled Ville Syrjala
2019-02-13 20:04 ` Clinton Taylor
2019-02-13 20:36 ` Clinton Taylor
2019-02-13 19:40 ` [PATCH 1/3] Revert "drm/i915: W/A for underruns with WM1+ disabled on icl" Clinton Taylor
2019-02-14 12:47 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] " Patchwork
2019-02-14 12:49 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-02-14 13:07 ` ✓ Fi.CI.BAT: success " Patchwork
2019-02-14 16:47 ` ✓ Fi.CI.IGT: " 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=20190213212854.GJ20097@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=Clinton.A.Taylor@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/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