On Wed, May 18, 2022 at 02:44:30PM +0300, Jani Nikula wrote:
> On Wed, 18 May 2022, Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx> wrote:
> > Otherwise we seem to get FIFO underruns. It is being disabled
> > anyway, so kind of logical to write those as zeroes, even if
> > disabling is temporary.
> >
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
> > ---
> > .../drm/i915/display/skl_universal_plane.c | 2 +-
> > drivers/gpu/drm/i915/intel_pm.c | 46 +++++++++++++++++++
> > drivers/gpu/drm/i915/intel_pm.h | 2 +
> > 3 files changed, 49 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index caa03324a733..c0251189c042 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -633,7 +633,7 @@ icl_plane_disable_arm(struct intel_plane *plane,
> > if (icl_is_hdr_plane(dev_priv, plane_id))
> > intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0);
> >
> > - skl_write_plane_wm(plane, crtc_state);
> > + skl_write_zero_plane_wm(plane, crtc_state);
> >
> > intel_psr2_disable_plane_sel_fetch(plane, crtc_state);
> > intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index ee0047fdc95d..2470c037bfae 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5885,6 +5885,52 @@ void skl_write_plane_wm(struct intel_plane *plane,
> > PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y);
> > }
> >
> > +void skl_write_zero_plane_wm(struct intel_plane *plane,
> > + const struct intel_crtc_state *crtc_state)
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > + int level, max_level = ilk_wm_max_level(dev_priv);
> > + enum plane_id plane_id = plane->id;
> > + enum pipe pipe = plane->pipe;
> > + struct skl_pipe_wm pipe_wm;
> > + 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];
> > +
> > + for (level = 0; level <= max_level; level++) {
>
> Not your doing here, but why have we adopted this error prone inclusive
> max that always makes you take a double look in the for loops?!
>
> BR,
> Jani.
No clue, really. It seems to be used same way all over the place in
intel_pm.c. I was suggesting there is some smart reasoning behind this,
so simply follow the common approach.
For me it is more confusing that apparently it is called "ilk_*" which
is IronLake probably, however we use it everywhere.
I would call it simple i915_wm_max_level and make it determine itself based on
dev_priv which platform it is..
Stan
>
> > + pipe_wm.planes[plane_id].wm[level].blocks = 0;
> > + pipe_wm.planes[plane_id].wm[level].lines = 0;
> > + }
> > +
> > + pipe_wm.planes[plane_id].trans_wm.blocks = 0;
> > + pipe_wm.planes[plane_id].trans_wm.lines = 0;
> > +
> > + for (level = 0; level <= max_level; level++)
> > + skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane_id, level),
> > + skl_plane_wm_level(&pipe_wm, plane_id, level));
> > +
> > + skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id),
> > + skl_plane_trans_wm(&pipe_wm, plane_id));
> > +
> > + if (HAS_HW_SAGV_WM(dev_priv)) {
> > + const struct skl_plane_wm *wm = &pipe_wm.planes[plane_id];
> > +
> > + skl_write_wm_level(dev_priv, PLANE_WM_SAGV(pipe, plane_id),
> > + &wm->sagv.wm0);
> > + skl_write_wm_level(dev_priv, PLANE_WM_SAGV_TRANS(pipe, plane_id),
> > + &wm->sagv.trans_wm);
> > + }
> > +
> > + skl_ddb_entry_write(dev_priv,
> > + PLANE_BUF_CFG(pipe, plane_id), ddb);
> > +
> > + if (DISPLAY_VER(dev_priv) < 11)
> > + skl_ddb_entry_write(dev_priv,
> > + PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_y);
> > +}
> > +
> > +
> > void skl_write_cursor_wm(struct intel_plane *plane,
> > const struct intel_crtc_state *crtc_state)
> > {
> > diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> > index 50604cf7398c..fb0ac4f143ab 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.h
> > +++ b/drivers/gpu/drm/i915/intel_pm.h
> > @@ -61,6 +61,8 @@ bool skl_wm_level_equals(const struct skl_wm_level *l1,
> > bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb,
> > const struct skl_ddb_entry *entries,
> > int num_entries, int ignore_idx);
> > +void skl_write_zero_plane_wm(struct intel_plane *plane,
> > + const struct intel_crtc_state *crtc_state);
> > 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,
>
> --
> Jani Nikula, Intel Open Source Graphics Center
