Re: [Intel-gfx] [PATCH v2 05/10] drm/i915/gen9: Get rid of redundant watermark values
Op 17-10-16 om 08:05 schreef Daniel Vetter:
> On Thu, Oct 13, 2016 at 05:04:03PM -0300, Paulo Zanoni wrote:
>> Em Qui, 2016-10-13 às 15:39 +0200, Maarten Lankhorst escreveu:
>>> Op 08-10-16 om 02:11 schreef Lyude:
Now that we've make skl_wm_levels make a little more sense, we can
remove all of the redundant wm information. Up until now we'd been
storing two copies of all of the skl watermarks: one being the
skl_pipe_wm structs, the other being the global wm struct in
drm_i915_private containing the raw register values. This is
confusing
and problematic, since it means we're prone to accidentally letting
the
two copies go out of sync. So, get rid of all of the functions
responsible for computing the register values and just use a single
helper, skl_write_wm_level(), to convert and write the new
watermarks on
the fly.
Changes since v1:
- Fixup skl_write_wm_level()
- Fixup skl_wm_level_from_reg_val()
- Don't forget to copy *active to intel_crtc->wm.active.skl
Signed-off-by: Lyude
Reviewed-by: Maarten Lankhorst
Cc: Ville Syrjälä
Cc: Paulo Zanoni
---
drivers/gpu/drm/i915/i915_drv.h | 2 -
drivers/gpu/drm/i915/intel_display.c | 14 ++-
drivers/gpu/drm/i915/intel_drv.h | 6 +-
drivers/gpu/drm/i915/intel_pm.c | 204 ---
drivers/gpu/drm/i915/intel_sprite.c | 8 +-
5 files changed, 90 insertions(+), 144 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 0287c93..76583b2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1644,8 +1644,6 @@ struct skl_ddb_allocation {
struct skl_wm_values {
unsigned dirty_pipes;
struct skl_ddb_allocation ddb;
- uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
- uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
};
struct skl_wm_level {
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index a71d05a..39400a0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3378,6 +3378,8 @@ static void
skylake_update_primary_plane(struct drm_plane *plane,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state-
> base.crtc);
struct drm_framebuffer *fb = plane_state->base.fb;
const struct skl_wm_values *wm = &dev_priv-
> wm.skl_results;
+ const struct skl_plane_wm *p_wm =
+ &crtc_state->wm.skl.optimal.planes[0];
int pipe = intel_crtc->pipe;
u32 plane_ctl;
unsigned int rotation = plane_state->base.rotation;
@@ -3414,7 +3416,7 @@ static void
skylake_update_primary_plane(struct drm_plane *plane,
intel_crtc->adjusted_y = src_y;
if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base))
- skl_write_plane_wm(intel_crtc, wm, 0);
+ skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, 0);
I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x);
@@ -3448,6 +3450,8 @@ static void
skylake_disable_primary_plane(struct drm_plane *primary,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate =
to_intel_crtc_state(crtc->state);
+ const struct skl_plane_wm *p_wm = &cstate-
> wm.skl.optimal.planes[0];
int pipe = intel_crtc->pipe;
/*
@@ -3455,7 +3459,8 @@ static void
skylake_disable_primary_plane(struct drm_plane *primary,
* plane's visiblity isn't actually changing neither is
its watermarks.
*/
if (!crtc->primary->state->visible)
- skl_write_plane_wm(intel_crtc, &dev_priv-
> wm.skl_results, 0);
+ skl_write_plane_wm(intel_crtc, p_wm,
+ &dev_priv->wm.skl_results.ddb,
0);
I915_WRITE(PLANE_CTL(pipe, 0), 0);
I915_WRITE(PLANE_SURF(pipe, 0), 0);
@@ -10819,12 +10824,15 @@ static void i9xx_update_cursor(struct
drm_crtc *crtc, u32 base,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate =
to_intel_crtc_state(crtc->state);
const struct skl_wm_values *wm = &dev_priv-
> wm.skl_results;
+ const struct skl_plane_wm *p_wm =
+ &cstate->wm.skl.optimal.planes[PLANE_CURSOR];
int pipe = intel_crtc->pipe;
uint32_t cntl = 0;
if (INTEL_GEN(dev_priv) >= 9 && wm->dirty_pipes &
drm_crtc_mask(crt
Re: [Intel-gfx] [PATCH v2 05/10] drm/i915/gen9: Get rid of redundant watermark values
On Thu, Oct 13, 2016 at 05:04:03PM -0300, Paulo Zanoni wrote:
> Em Qui, 2016-10-13 às 15:39 +0200, Maarten Lankhorst escreveu:
> > Op 08-10-16 om 02:11 schreef Lyude:
> > >
> > > Now that we've make skl_wm_levels make a little more sense, we can
> > > remove all of the redundant wm information. Up until now we'd been
> > > storing two copies of all of the skl watermarks: one being the
> > > skl_pipe_wm structs, the other being the global wm struct in
> > > drm_i915_private containing the raw register values. This is
> > > confusing
> > > and problematic, since it means we're prone to accidentally letting
> > > the
> > > two copies go out of sync. So, get rid of all of the functions
> > > responsible for computing the register values and just use a single
> > > helper, skl_write_wm_level(), to convert and write the new
> > > watermarks on
> > > the fly.
> > >
> > > Changes since v1:
> > > - Fixup skl_write_wm_level()
> > > - Fixup skl_wm_level_from_reg_val()
> > > - Don't forget to copy *active to intel_crtc->wm.active.skl
> > >
> > > Signed-off-by: Lyude
> > > Reviewed-by: Maarten Lankhorst
> > > Cc: Ville Syrjälä
> > > Cc: Paulo Zanoni
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.h | 2 -
> > > drivers/gpu/drm/i915/intel_display.c | 14 ++-
> > > drivers/gpu/drm/i915/intel_drv.h | 6 +-
> > > drivers/gpu/drm/i915/intel_pm.c | 204 ---
> > >
> > > drivers/gpu/drm/i915/intel_sprite.c | 8 +-
> > > 5 files changed, 90 insertions(+), 144 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 0287c93..76583b2 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1644,8 +1644,6 @@ struct skl_ddb_allocation {
> > > struct skl_wm_values {
> > > unsigned dirty_pipes;
> > > struct skl_ddb_allocation ddb;
> > > - uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
> > > - uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
> > > };
> > >
> > > struct skl_wm_level {
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index a71d05a..39400a0 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3378,6 +3378,8 @@ static void
> > > skylake_update_primary_plane(struct drm_plane *plane,
> > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state-
> > > >base.crtc);
> > > struct drm_framebuffer *fb = plane_state->base.fb;
> > > const struct skl_wm_values *wm = &dev_priv-
> > > >wm.skl_results;
> > > + const struct skl_plane_wm *p_wm =
> > > + &crtc_state->wm.skl.optimal.planes[0];
> > > int pipe = intel_crtc->pipe;
> > > u32 plane_ctl;
> > > unsigned int rotation = plane_state->base.rotation;
> > > @@ -3414,7 +3416,7 @@ static void
> > > skylake_update_primary_plane(struct drm_plane *plane,
> > > intel_crtc->adjusted_y = src_y;
> > >
> > > if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base))
> > > - skl_write_plane_wm(intel_crtc, wm, 0);
> > > + skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, 0);
> > >
> > > I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> > > I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x);
> > > @@ -3448,6 +3450,8 @@ static void
> > > skylake_disable_primary_plane(struct drm_plane *primary,
> > > struct drm_device *dev = crtc->dev;
> > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > > + struct intel_crtc_state *cstate =
> > > to_intel_crtc_state(crtc->state);
> > > + const struct skl_plane_wm *p_wm = &cstate-
> > > >wm.skl.optimal.planes[0];
> > > int pipe = intel_crtc->pipe;
> > >
> > > /*
> > > @@ -3455,7 +3459,8 @@ static void
> > > skylake_disable_primary_plane(struct drm_plane *primary,
> > > * plane's visiblity isn't actually changing neither is
> > > its watermarks.
> > > */
> > > if (!crtc->primary->state->visible)
> > > - skl_write_plane_wm(intel_crtc, &dev_priv-
> > > >wm.skl_results, 0);
> > > + skl_write_plane_wm(intel_crtc, p_wm,
> > > + &dev_priv->wm.skl_results.ddb,
> > > 0);
> > >
> > > I915_WRITE(PLANE_CTL(pipe, 0), 0);
> > > I915_WRITE(PLANE_SURF(pipe, 0), 0);
> > > @@ -10819,12 +10824,15 @@ static void i9xx_update_cursor(struct
> > > drm_crtc *crtc, u32 base,
> > > struct drm_device *dev = crtc->dev;
> > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > > + struct intel_crtc_state *cstate =
> > > to_intel_crtc_state(crtc->state);
> > > const struct skl_wm_values *wm = &dev_priv-
> > > >wm.skl_results;
> > > + const struct skl_plane_wm *p_wm =
> > > + &cstate->wm.skl.optimal.planes[PLANE_CURSOR];
> > > int pipe = intel_crtc->pipe;
> > > uint32_t cntl = 0;
> > >
> > > if (INTEL_GEN(dev_priv) >= 9 && wm->di
Re: [Intel-gfx] [PATCH v2 05/10] drm/i915/gen9: Get rid of redundant watermark values
Your is SAGV related, and when we don't make the SAGV happy people's
machines usually hang. So I'm definitely for your patches getting
merged first
On Thu, 2016-10-13 at 17:07 -0300, Paulo Zanoni wrote:
> Em Qui, 2016-10-13 às 17:04 -0300, Paulo Zanoni escreveu:
> >
> > Em Qui, 2016-10-13 às 15:39 +0200, Maarten Lankhorst escreveu:
> > >
> > >
> > > Op 08-10-16 om 02:11 schreef Lyude:
> > > >
> > > >
> > > >
> > > > Now that we've make skl_wm_levels make a little more sense, we
> > > > can
> > > > remove all of the redundant wm information. Up until now we'd
> > > > been
> > > > storing two copies of all of the skl watermarks: one being the
> > > > skl_pipe_wm structs, the other being the global wm struct in
> > > > drm_i915_private containing the raw register values. This is
> > > > confusing
> > > > and problematic, since it means we're prone to accidentally
> > > > letting
> > > > the
> > > > two copies go out of sync. So, get rid of all of the functions
> > > > responsible for computing the register values and just use a
> > > > single
> > > > helper, skl_write_wm_level(), to convert and write the new
> > > > watermarks on
> > > > the fly.
> > > >
> > > > Changes since v1:
> > > > - Fixup skl_write_wm_level()
> > > > - Fixup skl_wm_level_from_reg_val()
> > > > - Don't forget to copy *active to intel_crtc->wm.active.skl
> > > >
> > > > Signed-off-by: Lyude
> > > > Reviewed-by: Maarten Lankhorst > > > om
> > > > >
> > > > >
> > > > Cc: Ville Syrjälä
> > > > Cc: Paulo Zanoni
> > > > ---
> > > > drivers/gpu/drm/i915/i915_drv.h | 2 -
> > > > drivers/gpu/drm/i915/intel_display.c | 14 ++-
> > > > drivers/gpu/drm/i915/intel_drv.h | 6 +-
> > > > drivers/gpu/drm/i915/intel_pm.c | 204 ---
> > > > --
> > > > --
> > > >
> > > > drivers/gpu/drm/i915/intel_sprite.c | 8 +-
> > > > 5 files changed, 90 insertions(+), 144 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > > b/drivers/gpu/drm/i915/i915_drv.h
> > > > index 0287c93..76583b2 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -1644,8 +1644,6 @@ struct skl_ddb_allocation {
> > > > struct skl_wm_values {
> > > > unsigned dirty_pipes;
> > > > struct skl_ddb_allocation ddb;
> > > > - uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
> > > > - uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
> > > > };
> > > >
> > > > struct skl_wm_level {
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > index a71d05a..39400a0 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -3378,6 +3378,8 @@ static void
> > > > skylake_update_primary_plane(struct drm_plane *plane,
> > > > struct intel_crtc *intel_crtc =
> > > > to_intel_crtc(crtc_state-
> > > > >
> > > > >
> > > > > base.crtc);
> > > > struct drm_framebuffer *fb = plane_state->base.fb;
> > > > const struct skl_wm_values *wm = &dev_priv-
> > > > >
> > > > >
> > > > > wm.skl_results;
> > > > + const struct skl_plane_wm *p_wm =
> > > > + &crtc_state->wm.skl.optimal.planes[0];
> > > > int pipe = intel_crtc->pipe;
> > > > u32 plane_ctl;
> > > > unsigned int rotation = plane_state->base.rotation;
> > > > @@ -3414,7 +3416,7 @@ static void
> > > > skylake_update_primary_plane(struct drm_plane *plane,
> > > > intel_crtc->adjusted_y = src_y;
> > > >
> > > > if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc-
> > > > >base))
> > > > - skl_write_plane_wm(intel_crtc, wm, 0);
> > > > + skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb,
> > > > 0);
> > > >
> > > > I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> > > > I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) |
> > > > src_x);
> > > > @@ -3448,6 +3450,8 @@ static void
> > > > skylake_disable_primary_plane(struct drm_plane *primary,
> > > > struct drm_device *dev = crtc->dev;
> > > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > > > + struct intel_crtc_state *cstate =
> > > > to_intel_crtc_state(crtc->state);
> > > > + const struct skl_plane_wm *p_wm = &cstate-
> > > > >
> > > > >
> > > > > wm.skl.optimal.planes[0];
> > > > int pipe = intel_crtc->pipe;
> > > >
> > > > /*
> > > > @@ -3455,7 +3459,8 @@ static void
> > > > skylake_disable_primary_plane(struct drm_plane *primary,
> > > > * plane's visiblity isn't actually changing neither
> > > > is
> > > > its watermarks.
> > > > */
> > > > if (!crtc->primary->state->visible)
> > > > - skl_write_plane_wm(intel_crtc, &dev_priv-
> > > > >
> > > > >
> > > > > wm.skl_results, 0);
> > > > + skl_write_plane_wm(intel
Re: [Intel-gfx] [PATCH v2 05/10] drm/i915/gen9: Get rid of redundant watermark values
Em Qui, 2016-10-13 às 17:04 -0300, Paulo Zanoni escreveu:
> Em Qui, 2016-10-13 às 15:39 +0200, Maarten Lankhorst escreveu:
> >
> > Op 08-10-16 om 02:11 schreef Lyude:
> > >
> > >
> > > Now that we've make skl_wm_levels make a little more sense, we
> > > can
> > > remove all of the redundant wm information. Up until now we'd
> > > been
> > > storing two copies of all of the skl watermarks: one being the
> > > skl_pipe_wm structs, the other being the global wm struct in
> > > drm_i915_private containing the raw register values. This is
> > > confusing
> > > and problematic, since it means we're prone to accidentally
> > > letting
> > > the
> > > two copies go out of sync. So, get rid of all of the functions
> > > responsible for computing the register values and just use a
> > > single
> > > helper, skl_write_wm_level(), to convert and write the new
> > > watermarks on
> > > the fly.
> > >
> > > Changes since v1:
> > > - Fixup skl_write_wm_level()
> > > - Fixup skl_wm_level_from_reg_val()
> > > - Don't forget to copy *active to intel_crtc->wm.active.skl
> > >
> > > Signed-off-by: Lyude
> > > Reviewed-by: Maarten Lankhorst > > >
> > > Cc: Ville Syrjälä
> > > Cc: Paulo Zanoni
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.h | 2 -
> > > drivers/gpu/drm/i915/intel_display.c | 14 ++-
> > > drivers/gpu/drm/i915/intel_drv.h | 6 +-
> > > drivers/gpu/drm/i915/intel_pm.c | 204 -
> > > --
> > >
> > > drivers/gpu/drm/i915/intel_sprite.c | 8 +-
> > > 5 files changed, 90 insertions(+), 144 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 0287c93..76583b2 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1644,8 +1644,6 @@ struct skl_ddb_allocation {
> > > struct skl_wm_values {
> > > unsigned dirty_pipes;
> > > struct skl_ddb_allocation ddb;
> > > - uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
> > > - uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
> > > };
> > >
> > > struct skl_wm_level {
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index a71d05a..39400a0 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3378,6 +3378,8 @@ static void
> > > skylake_update_primary_plane(struct drm_plane *plane,
> > > struct intel_crtc *intel_crtc =
> > > to_intel_crtc(crtc_state-
> > > >
> > > > base.crtc);
> > > struct drm_framebuffer *fb = plane_state->base.fb;
> > > const struct skl_wm_values *wm = &dev_priv-
> > > >
> > > > wm.skl_results;
> > > + const struct skl_plane_wm *p_wm =
> > > + &crtc_state->wm.skl.optimal.planes[0];
> > > int pipe = intel_crtc->pipe;
> > > u32 plane_ctl;
> > > unsigned int rotation = plane_state->base.rotation;
> > > @@ -3414,7 +3416,7 @@ static void
> > > skylake_update_primary_plane(struct drm_plane *plane,
> > > intel_crtc->adjusted_y = src_y;
> > >
> > > if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base))
> > > - skl_write_plane_wm(intel_crtc, wm, 0);
> > > + skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb,
> > > 0);
> > >
> > > I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> > > I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) |
> > > src_x);
> > > @@ -3448,6 +3450,8 @@ static void
> > > skylake_disable_primary_plane(struct drm_plane *primary,
> > > struct drm_device *dev = crtc->dev;
> > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > > + struct intel_crtc_state *cstate =
> > > to_intel_crtc_state(crtc->state);
> > > + const struct skl_plane_wm *p_wm = &cstate-
> > > >
> > > > wm.skl.optimal.planes[0];
> > > int pipe = intel_crtc->pipe;
> > >
> > > /*
> > > @@ -3455,7 +3459,8 @@ static void
> > > skylake_disable_primary_plane(struct drm_plane *primary,
> > > * plane's visiblity isn't actually changing neither is
> > > its watermarks.
> > > */
> > > if (!crtc->primary->state->visible)
> > > - skl_write_plane_wm(intel_crtc, &dev_priv-
> > > >
> > > > wm.skl_results, 0);
> > > + skl_write_plane_wm(intel_crtc, p_wm,
> > > + &dev_priv-
> > > >wm.skl_results.ddb,
> > > 0);
> > >
> > > I915_WRITE(PLANE_CTL(pipe, 0), 0);
> > > I915_WRITE(PLANE_SURF(pipe, 0), 0);
> > > @@ -10819,12 +10824,15 @@ static void i9xx_update_cursor(struct
> > > drm_crtc *crtc, u32 base,
> > > struct drm_device *dev = crtc->dev;
> > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > > + struct intel_crtc_state *cstate =
> > > to_intel_crtc_state(crtc->state);
> > > const struct skl_wm_values *wm = &dev_priv-
> > > >
> > > > wm.skl_results;
> > > + const struct skl_plane_wm *p_wm =
> > > + &cstate->wm.skl.optimal.planes[PLANE_CURSOR]
Re: [Intel-gfx] [PATCH v2 05/10] drm/i915/gen9: Get rid of redundant watermark values
Em Qui, 2016-10-13 às 15:39 +0200, Maarten Lankhorst escreveu:
> Op 08-10-16 om 02:11 schreef Lyude:
> >
> > Now that we've make skl_wm_levels make a little more sense, we can
> > remove all of the redundant wm information. Up until now we'd been
> > storing two copies of all of the skl watermarks: one being the
> > skl_pipe_wm structs, the other being the global wm struct in
> > drm_i915_private containing the raw register values. This is
> > confusing
> > and problematic, since it means we're prone to accidentally letting
> > the
> > two copies go out of sync. So, get rid of all of the functions
> > responsible for computing the register values and just use a single
> > helper, skl_write_wm_level(), to convert and write the new
> > watermarks on
> > the fly.
> >
> > Changes since v1:
> > - Fixup skl_write_wm_level()
> > - Fixup skl_wm_level_from_reg_val()
> > - Don't forget to copy *active to intel_crtc->wm.active.skl
> >
> > Signed-off-by: Lyude
> > Reviewed-by: Maarten Lankhorst
> > Cc: Ville Syrjälä
> > Cc: Paulo Zanoni
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 2 -
> > drivers/gpu/drm/i915/intel_display.c | 14 ++-
> > drivers/gpu/drm/i915/intel_drv.h | 6 +-
> > drivers/gpu/drm/i915/intel_pm.c | 204 ---
> >
> > drivers/gpu/drm/i915/intel_sprite.c | 8 +-
> > 5 files changed, 90 insertions(+), 144 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 0287c93..76583b2 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1644,8 +1644,6 @@ struct skl_ddb_allocation {
> > struct skl_wm_values {
> > unsigned dirty_pipes;
> > struct skl_ddb_allocation ddb;
> > - uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
> > - uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
> > };
> >
> > struct skl_wm_level {
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index a71d05a..39400a0 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3378,6 +3378,8 @@ static void
> > skylake_update_primary_plane(struct drm_plane *plane,
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state-
> > >base.crtc);
> > struct drm_framebuffer *fb = plane_state->base.fb;
> > const struct skl_wm_values *wm = &dev_priv-
> > >wm.skl_results;
> > + const struct skl_plane_wm *p_wm =
> > + &crtc_state->wm.skl.optimal.planes[0];
> > int pipe = intel_crtc->pipe;
> > u32 plane_ctl;
> > unsigned int rotation = plane_state->base.rotation;
> > @@ -3414,7 +3416,7 @@ static void
> > skylake_update_primary_plane(struct drm_plane *plane,
> > intel_crtc->adjusted_y = src_y;
> >
> > if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base))
> > - skl_write_plane_wm(intel_crtc, wm, 0);
> > + skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, 0);
> >
> > I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> > I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x);
> > @@ -3448,6 +3450,8 @@ static void
> > skylake_disable_primary_plane(struct drm_plane *primary,
> > struct drm_device *dev = crtc->dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > + struct intel_crtc_state *cstate =
> > to_intel_crtc_state(crtc->state);
> > + const struct skl_plane_wm *p_wm = &cstate-
> > >wm.skl.optimal.planes[0];
> > int pipe = intel_crtc->pipe;
> >
> > /*
> > @@ -3455,7 +3459,8 @@ static void
> > skylake_disable_primary_plane(struct drm_plane *primary,
> > * plane's visiblity isn't actually changing neither is
> > its watermarks.
> > */
> > if (!crtc->primary->state->visible)
> > - skl_write_plane_wm(intel_crtc, &dev_priv-
> > >wm.skl_results, 0);
> > + skl_write_plane_wm(intel_crtc, p_wm,
> > + &dev_priv->wm.skl_results.ddb,
> > 0);
> >
> > I915_WRITE(PLANE_CTL(pipe, 0), 0);
> > I915_WRITE(PLANE_SURF(pipe, 0), 0);
> > @@ -10819,12 +10824,15 @@ static void i9xx_update_cursor(struct
> > drm_crtc *crtc, u32 base,
> > struct drm_device *dev = crtc->dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > + struct intel_crtc_state *cstate =
> > to_intel_crtc_state(crtc->state);
> > const struct skl_wm_values *wm = &dev_priv-
> > >wm.skl_results;
> > + const struct skl_plane_wm *p_wm =
> > + &cstate->wm.skl.optimal.planes[PLANE_CURSOR];
> > int pipe = intel_crtc->pipe;
> > uint32_t cntl = 0;
> >
> > if (INTEL_GEN(dev_priv) >= 9 && wm->dirty_pipes &
> > drm_crtc_mask(crtc))
> > - skl_write_cursor_wm(intel_crtc, wm);
> > + skl_write_cursor_wm(intel_crtc, p_wm, &wm->ddb);
> >
> > if (plane_state && plane_state->base.visi
Re: [Intel-gfx] [PATCH v2 05/10] drm/i915/gen9: Get rid of redundant watermark values
Op 08-10-16 om 02:11 schreef Lyude:
> Now that we've make skl_wm_levels make a little more sense, we can
> remove all of the redundant wm information. Up until now we'd been
> storing two copies of all of the skl watermarks: one being the
> skl_pipe_wm structs, the other being the global wm struct in
> drm_i915_private containing the raw register values. This is confusing
> and problematic, since it means we're prone to accidentally letting the
> two copies go out of sync. So, get rid of all of the functions
> responsible for computing the register values and just use a single
> helper, skl_write_wm_level(), to convert and write the new watermarks on
> the fly.
>
> Changes since v1:
> - Fixup skl_write_wm_level()
> - Fixup skl_wm_level_from_reg_val()
> - Don't forget to copy *active to intel_crtc->wm.active.skl
>
> Signed-off-by: Lyude
> Reviewed-by: Maarten Lankhorst
> Cc: Ville Syrjälä
> Cc: Paulo Zanoni
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 -
> drivers/gpu/drm/i915/intel_display.c | 14 ++-
> drivers/gpu/drm/i915/intel_drv.h | 6 +-
> drivers/gpu/drm/i915/intel_pm.c | 204
> ---
> drivers/gpu/drm/i915/intel_sprite.c | 8 +-
> 5 files changed, 90 insertions(+), 144 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 0287c93..76583b2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1644,8 +1644,6 @@ struct skl_ddb_allocation {
> struct skl_wm_values {
> unsigned dirty_pipes;
> struct skl_ddb_allocation ddb;
> - uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
> - uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
> };
>
> struct skl_wm_level {
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index a71d05a..39400a0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3378,6 +3378,8 @@ static void skylake_update_primary_plane(struct
> drm_plane *plane,
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> struct drm_framebuffer *fb = plane_state->base.fb;
> const struct skl_wm_values *wm = &dev_priv->wm.skl_results;
> + const struct skl_plane_wm *p_wm =
> + &crtc_state->wm.skl.optimal.planes[0];
> int pipe = intel_crtc->pipe;
> u32 plane_ctl;
> unsigned int rotation = plane_state->base.rotation;
> @@ -3414,7 +3416,7 @@ static void skylake_update_primary_plane(struct
> drm_plane *plane,
> intel_crtc->adjusted_y = src_y;
>
> if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base))
> - skl_write_plane_wm(intel_crtc, wm, 0);
> + skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, 0);
>
> I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x);
> @@ -3448,6 +3450,8 @@ static void skylake_disable_primary_plane(struct
> drm_plane *primary,
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> + const struct skl_plane_wm *p_wm = &cstate->wm.skl.optimal.planes[0];
> int pipe = intel_crtc->pipe;
>
> /*
> @@ -3455,7 +3459,8 @@ static void skylake_disable_primary_plane(struct
> drm_plane *primary,
>* plane's visiblity isn't actually changing neither is its watermarks.
>*/
> if (!crtc->primary->state->visible)
> - skl_write_plane_wm(intel_crtc, &dev_priv->wm.skl_results, 0);
> + skl_write_plane_wm(intel_crtc, p_wm,
> +&dev_priv->wm.skl_results.ddb, 0);
>
> I915_WRITE(PLANE_CTL(pipe, 0), 0);
> I915_WRITE(PLANE_SURF(pipe, 0), 0);
> @@ -10819,12 +10824,15 @@ static void i9xx_update_cursor(struct drm_crtc
> *crtc, u32 base,
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> const struct skl_wm_values *wm = &dev_priv->wm.skl_results;
> + const struct skl_plane_wm *p_wm =
> + &cstate->wm.skl.optimal.planes[PLANE_CURSOR];
> int pipe = intel_crtc->pipe;
> uint32_t cntl = 0;
>
> if (INTEL_GEN(dev_priv) >= 9 && wm->dirty_pipes & drm_crtc_mask(crtc))
> - skl_write_cursor_wm(intel_crtc, wm);
> + skl_write_cursor_wm(intel_crtc, p_wm, &wm->ddb);
>
> if (plane_state && plane_state->base.visible) {
> cntl = MCURSOR_GAMMA_ENABLE;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index d684f4f..958dc72 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_
