Re: [Intel-gfx] [PATCH v10 1/2] drm/i915: Refactor intel_can_enable_sagv

2019-11-12 Thread Lisovskiy, Stanislav
On Mon, 2019-11-11 at 16:15 -0800, Matt Roper wrote:
> On Thu, Nov 07, 2019 at 05:30:36PM +0200, Stanislav Lisovskiy wrote:
> > Currently intel_can_enable_sagv function contains
> > a mix of workarounds for different platforms
> > some of them are not valid for gens >= 11 already,
> > so lets split it into separate functions.
> > 
> > v2:
> > - Rework watermark calculation algorithm to
> >   attempt to calculate Level 0 watermark
> >   with added sagv block time latency and
> >   check if it fits in DBuf in order to
> >   determine if SAGV can be enabled already
> >   at this stage, just as BSpec 49325 states.
> >   if that fails rollback to usual Level 0
> >   latency and disable SAGV.
> > - Remove unneeded tabs(James Ausmus)
> > 
> > v3: Rebased the patch
> > 
> > v4: - Added back interlaced check for Gen12 and
> >   added separate function for TGL SAGV check
> >   (thanks to James Ausmus for spotting)
> > - Removed unneeded gen check
> > - Extracted Gen12 SAGV decision making code
> >   to a separate function from skl_compute_wm
> > 
> > v5: - Added SAGV global state to dev_priv, because
> >   we need to track all pipes, not only those
> >   in atomic state. Each pipe has now correspondent
> >   bit mask reflecting, whether it can tolerate
> >   SAGV or not(thanks to Ville Syrjala for suggestions).
> > - Now using active flag instead of enable in crc
> >   usage check.
> > 
> > v6: - Fixed rebase conflicts
> > 
> > Signed-off-by: Stanislav Lisovskiy 
> > Cc: Ville Syrjälä 
> > Cc: James Ausmus 
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c  |   4 +
> >  .../drm/i915/display/intel_display_types.h|   9 +
> >  drivers/gpu/drm/i915/i915_drv.h   |   6 +
> >  drivers/gpu/drm/i915/intel_pm.c   | 296
> > +-
> >  4 files changed, 303 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 876fc25968bf..7ea1e7518ab6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -14855,6 +14855,10 @@ static void
> > intel_atomic_commit_tail(struct intel_atomic_state *state)
> > if (dev_priv->display.optimize_watermarks)
> > dev_priv->display.optimize_watermarks(state,
> >   new_crtc_
> > state);
> > +   if (state->crtc_sagv_mask & BIT(crtc->pipe))
> > +   dev_priv->crtc_sagv_mask |= BIT(crtc->pipe);
> > +   else
> > +   dev_priv->crtc_sagv_mask &= ~BIT(crtc->pipe);
> > }
> >  
> > for_each_oldnew_intel_crtc_in_state(state, crtc,
> > old_crtc_state, new_crtc_state, i) {
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index fadd9853f966..fb274538af23 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -490,6 +490,14 @@ struct intel_atomic_state {
> >  */
> > u8 active_pipe_changes;
> >  
> > +   /*
> > +* Contains a mask which reflects whether correspondent pipe
> > +* can tolerate SAGV or not, so that we can make a decision
> > +* at atomic_commit_tail stage, whether we enable it or not
> > +* based on global state in dev_priv.
> > +*/
> > +   u32 crtc_sagv_mask;
> > +
> > u8 active_pipes;
> > /* minimum acceptable cdclk for each pipe */
> > int min_cdclk[I915_MAX_PIPES];
> > @@ -670,6 +678,7 @@ struct skl_plane_wm {
> > struct skl_wm_level wm[8];
> > struct skl_wm_level uv_wm[8];
> > struct skl_wm_level trans_wm;
> > +   struct skl_wm_level sagv_wm0;
> > bool is_planar;
> >  };
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 7e0f67babe20..4f4e2e839513 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1176,6 +1176,12 @@ struct drm_i915_private {
> >  
> > u32 sagv_block_time_us;
> >  
> > +   /*
> > +* Contains a bit mask, whether correspondent
> > +* pipe allows SAGV or not.
> > +*/
> > +   u32 crtc_sagv_mask;
> > +
> > struct {
> > /*
> >  * Raw watermark latency values:
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 2d389e437e87..c792dd168742 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3740,7 +3740,7 @@ intel_disable_sagv(struct drm_i915_private
> > *dev_priv)
> > return 0;
> >  }
> >  
> > -bool intel_can_enable_sagv(struct intel_atomic_state *state)
> > +static void skl_set_sagv_mask(struct intel_atomic_state *state)
> >  {
> > struct drm_device *dev = state->base.dev;
> > struct drm_i915_private 

Re: [Intel-gfx] [PATCH v10 1/2] drm/i915: Refactor intel_can_enable_sagv

2019-11-11 Thread Matt Roper
On Thu, Nov 07, 2019 at 05:30:36PM +0200, Stanislav Lisovskiy wrote:
> Currently intel_can_enable_sagv function contains
> a mix of workarounds for different platforms
> some of them are not valid for gens >= 11 already,
> so lets split it into separate functions.
> 
> v2:
> - Rework watermark calculation algorithm to
>   attempt to calculate Level 0 watermark
>   with added sagv block time latency and
>   check if it fits in DBuf in order to
>   determine if SAGV can be enabled already
>   at this stage, just as BSpec 49325 states.
>   if that fails rollback to usual Level 0
>   latency and disable SAGV.
> - Remove unneeded tabs(James Ausmus)
> 
> v3: Rebased the patch
> 
> v4: - Added back interlaced check for Gen12 and
>   added separate function for TGL SAGV check
>   (thanks to James Ausmus for spotting)
> - Removed unneeded gen check
> - Extracted Gen12 SAGV decision making code
>   to a separate function from skl_compute_wm
> 
> v5: - Added SAGV global state to dev_priv, because
>   we need to track all pipes, not only those
>   in atomic state. Each pipe has now correspondent
>   bit mask reflecting, whether it can tolerate
>   SAGV or not(thanks to Ville Syrjala for suggestions).
> - Now using active flag instead of enable in crc
>   usage check.
> 
> v6: - Fixed rebase conflicts
> 
> Signed-off-by: Stanislav Lisovskiy 
> Cc: Ville Syrjälä 
> Cc: James Ausmus 
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |   4 +
>  .../drm/i915/display/intel_display_types.h|   9 +
>  drivers/gpu/drm/i915/i915_drv.h   |   6 +
>  drivers/gpu/drm/i915/intel_pm.c   | 296 +-
>  4 files changed, 303 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 876fc25968bf..7ea1e7518ab6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14855,6 +14855,10 @@ static void intel_atomic_commit_tail(struct 
> intel_atomic_state *state)
>   if (dev_priv->display.optimize_watermarks)
>   dev_priv->display.optimize_watermarks(state,
> new_crtc_state);
> + if (state->crtc_sagv_mask & BIT(crtc->pipe))
> + dev_priv->crtc_sagv_mask |= BIT(crtc->pipe);
> + else
> + dev_priv->crtc_sagv_mask &= ~BIT(crtc->pipe);
>   }
>  
>   for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, 
> new_crtc_state, i) {
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index fadd9853f966..fb274538af23 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -490,6 +490,14 @@ struct intel_atomic_state {
>*/
>   u8 active_pipe_changes;
>  
> + /*
> +  * Contains a mask which reflects whether correspondent pipe
> +  * can tolerate SAGV or not, so that we can make a decision
> +  * at atomic_commit_tail stage, whether we enable it or not
> +  * based on global state in dev_priv.
> +  */
> + u32 crtc_sagv_mask;
> +
>   u8 active_pipes;
>   /* minimum acceptable cdclk for each pipe */
>   int min_cdclk[I915_MAX_PIPES];
> @@ -670,6 +678,7 @@ struct skl_plane_wm {
>   struct skl_wm_level wm[8];
>   struct skl_wm_level uv_wm[8];
>   struct skl_wm_level trans_wm;
> + struct skl_wm_level sagv_wm0;
>   bool is_planar;
>  };
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7e0f67babe20..4f4e2e839513 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1176,6 +1176,12 @@ struct drm_i915_private {
>  
>   u32 sagv_block_time_us;
>  
> + /*
> +  * Contains a bit mask, whether correspondent
> +  * pipe allows SAGV or not.
> +  */
> + u32 crtc_sagv_mask;
> +
>   struct {
>   /*
>* Raw watermark latency values:
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2d389e437e87..c792dd168742 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3740,7 +3740,7 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
>   return 0;
>  }
>  
> -bool intel_can_enable_sagv(struct intel_atomic_state *state)
> +static void skl_set_sagv_mask(struct intel_atomic_state *state)
>  {
>   struct drm_device *dev = state->base.dev;
>   struct drm_i915_private *dev_priv = to_i915(dev);
> @@ -3750,21 +3750,23 @@ bool intel_can_enable_sagv(struct intel_atomic_state 
> *state)
>   enum pipe pipe;
>   int level, latency;
>  
> + state->crtc_sagv_mask = 0;
> +
>   if 

[Intel-gfx] [PATCH v10 1/2] drm/i915: Refactor intel_can_enable_sagv

2019-11-07 Thread Stanislav Lisovskiy
Currently intel_can_enable_sagv function contains
a mix of workarounds for different platforms
some of them are not valid for gens >= 11 already,
so lets split it into separate functions.

v2:
- Rework watermark calculation algorithm to
  attempt to calculate Level 0 watermark
  with added sagv block time latency and
  check if it fits in DBuf in order to
  determine if SAGV can be enabled already
  at this stage, just as BSpec 49325 states.
  if that fails rollback to usual Level 0
  latency and disable SAGV.
- Remove unneeded tabs(James Ausmus)

v3: Rebased the patch

v4: - Added back interlaced check for Gen12 and
  added separate function for TGL SAGV check
  (thanks to James Ausmus for spotting)
- Removed unneeded gen check
- Extracted Gen12 SAGV decision making code
  to a separate function from skl_compute_wm

v5: - Added SAGV global state to dev_priv, because
  we need to track all pipes, not only those
  in atomic state. Each pipe has now correspondent
  bit mask reflecting, whether it can tolerate
  SAGV or not(thanks to Ville Syrjala for suggestions).
- Now using active flag instead of enable in crc
  usage check.

v6: - Fixed rebase conflicts

Signed-off-by: Stanislav Lisovskiy 
Cc: Ville Syrjälä 
Cc: James Ausmus 
---
 drivers/gpu/drm/i915/display/intel_display.c  |   4 +
 .../drm/i915/display/intel_display_types.h|   9 +
 drivers/gpu/drm/i915/i915_drv.h   |   6 +
 drivers/gpu/drm/i915/intel_pm.c   | 296 +-
 4 files changed, 303 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 876fc25968bf..7ea1e7518ab6 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14855,6 +14855,10 @@ static void intel_atomic_commit_tail(struct 
intel_atomic_state *state)
if (dev_priv->display.optimize_watermarks)
dev_priv->display.optimize_watermarks(state,
  new_crtc_state);
+   if (state->crtc_sagv_mask & BIT(crtc->pipe))
+   dev_priv->crtc_sagv_mask |= BIT(crtc->pipe);
+   else
+   dev_priv->crtc_sagv_mask &= ~BIT(crtc->pipe);
}
 
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state, i) {
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
b/drivers/gpu/drm/i915/display/intel_display_types.h
index fadd9853f966..fb274538af23 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -490,6 +490,14 @@ struct intel_atomic_state {
 */
u8 active_pipe_changes;
 
+   /*
+* Contains a mask which reflects whether correspondent pipe
+* can tolerate SAGV or not, so that we can make a decision
+* at atomic_commit_tail stage, whether we enable it or not
+* based on global state in dev_priv.
+*/
+   u32 crtc_sagv_mask;
+
u8 active_pipes;
/* minimum acceptable cdclk for each pipe */
int min_cdclk[I915_MAX_PIPES];
@@ -670,6 +678,7 @@ struct skl_plane_wm {
struct skl_wm_level wm[8];
struct skl_wm_level uv_wm[8];
struct skl_wm_level trans_wm;
+   struct skl_wm_level sagv_wm0;
bool is_planar;
 };
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7e0f67babe20..4f4e2e839513 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1176,6 +1176,12 @@ struct drm_i915_private {
 
u32 sagv_block_time_us;
 
+   /*
+* Contains a bit mask, whether correspondent
+* pipe allows SAGV or not.
+*/
+   u32 crtc_sagv_mask;
+
struct {
/*
 * Raw watermark latency values:
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2d389e437e87..c792dd168742 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3740,7 +3740,7 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
return 0;
 }
 
-bool intel_can_enable_sagv(struct intel_atomic_state *state)
+static void skl_set_sagv_mask(struct intel_atomic_state *state)
 {
struct drm_device *dev = state->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
@@ -3750,21 +3750,23 @@ bool intel_can_enable_sagv(struct intel_atomic_state 
*state)
enum pipe pipe;
int level, latency;
 
+   state->crtc_sagv_mask = 0;
+
if (!intel_has_sagv(dev_priv))
-   return false;
+   return;
 
/*
 * If there are no active CRTCs, no additional checks need be performed
 */
if (hweight8(state->active_pipes) == 0)
-   return 

[Intel-gfx] [PATCH v10 1/2] drm/i915: Refactor intel_can_enable_sagv

2019-11-07 Thread Stanislav Lisovskiy
Currently intel_can_enable_sagv function contains
a mix of workarounds for different platforms
some of them are not valid for gens >= 11 already,
so lets split it into separate functions.

v2:
- Rework watermark calculation algorithm to
  attempt to calculate Level 0 watermark
  with added sagv block time latency and
  check if it fits in DBuf in order to
  determine if SAGV can be enabled already
  at this stage, just as BSpec 49325 states.
  if that fails rollback to usual Level 0
  latency and disable SAGV.
- Remove unneeded tabs(James Ausmus)

v3: Rebased the patch

v4: - Added back interlaced check for Gen12 and
  added separate function for TGL SAGV check
  (thanks to James Ausmus for spotting)
- Removed unneeded gen check
- Extracted Gen12 SAGV decision making code
  to a separate function from skl_compute_wm

v5: - Added SAGV global state to dev_priv, because
  we need to track all pipes, not only those
  in atomic state. Each pipe has now correspondent
  bit mask reflecting, whether it can tolerate
  SAGV or not(thanks to Ville Syrjala for suggestions).
- Now using active flag instead of enable in crc
  usage check.

v6: - Fixed rebase conflicts

Signed-off-by: Stanislav Lisovskiy 
Cc: Ville Syrjälä 
Cc: James Ausmus 
---
 drivers/gpu/drm/i915/display/intel_display.c  |   4 +
 .../drm/i915/display/intel_display_types.h|   9 +
 drivers/gpu/drm/i915/i915_drv.h   |   6 +
 drivers/gpu/drm/i915/intel_pm.c   | 296 +-
 4 files changed, 303 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 876fc25968bf..7ea1e7518ab6 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14855,6 +14855,10 @@ static void intel_atomic_commit_tail(struct 
intel_atomic_state *state)
if (dev_priv->display.optimize_watermarks)
dev_priv->display.optimize_watermarks(state,
  new_crtc_state);
+   if (state->crtc_sagv_mask & BIT(crtc->pipe))
+   dev_priv->crtc_sagv_mask |= BIT(crtc->pipe);
+   else
+   dev_priv->crtc_sagv_mask &= ~BIT(crtc->pipe);
}
 
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state, i) {
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
b/drivers/gpu/drm/i915/display/intel_display_types.h
index fadd9853f966..fb274538af23 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -490,6 +490,14 @@ struct intel_atomic_state {
 */
u8 active_pipe_changes;
 
+   /*
+* Contains a mask which reflects whether correspondent pipe
+* can tolerate SAGV or not, so that we can make a decision
+* at atomic_commit_tail stage, whether we enable it or not
+* based on global state in dev_priv.
+*/
+   u32 crtc_sagv_mask;
+
u8 active_pipes;
/* minimum acceptable cdclk for each pipe */
int min_cdclk[I915_MAX_PIPES];
@@ -670,6 +678,7 @@ struct skl_plane_wm {
struct skl_wm_level wm[8];
struct skl_wm_level uv_wm[8];
struct skl_wm_level trans_wm;
+   struct skl_wm_level sagv_wm0;
bool is_planar;
 };
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7e0f67babe20..4f4e2e839513 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1176,6 +1176,12 @@ struct drm_i915_private {
 
u32 sagv_block_time_us;
 
+   /*
+* Contains a bit mask, whether correspondent
+* pipe allows SAGV or not.
+*/
+   u32 crtc_sagv_mask;
+
struct {
/*
 * Raw watermark latency values:
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2d389e437e87..c792dd168742 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3740,7 +3740,7 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
return 0;
 }
 
-bool intel_can_enable_sagv(struct intel_atomic_state *state)
+static void skl_set_sagv_mask(struct intel_atomic_state *state)
 {
struct drm_device *dev = state->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
@@ -3750,21 +3750,23 @@ bool intel_can_enable_sagv(struct intel_atomic_state 
*state)
enum pipe pipe;
int level, latency;
 
+   state->crtc_sagv_mask = 0;
+
if (!intel_has_sagv(dev_priv))
-   return false;
+   return;
 
/*
 * If there are no active CRTCs, no additional checks need be performed
 */
if (hweight8(state->active_pipes) == 0)
-   return 

[Intel-gfx] [PATCH v10 1/2] drm/i915: Refactor intel_can_enable_sagv

2019-11-05 Thread Stanislav Lisovskiy
Currently intel_can_enable_sagv function contains
a mix of workarounds for different platforms
some of them are not valid for gens >= 11 already,
so lets split it into separate functions.

v2:
- Rework watermark calculation algorithm to
  attempt to calculate Level 0 watermark
  with added sagv block time latency and
  check if it fits in DBuf in order to
  determine if SAGV can be enabled already
  at this stage, just as BSpec 49325 states.
  if that fails rollback to usual Level 0
  latency and disable SAGV.
- Remove unneeded tabs(James Ausmus)

v3: Rebased the patch

v4: - Added back interlaced check for Gen12 and
  added separate function for TGL SAGV check
  (thanks to James Ausmus for spotting)
- Removed unneeded gen check
- Extracted Gen12 SAGV decision making code
  to a separate function from skl_compute_wm

v5: - Added SAGV global state to dev_priv, because
  we need to track all pipes, not only those
  in atomic state. Each pipe has now correspondent
  bit mask reflecting, whether it can tolerate
  SAGV or not(thanks to Ville Syrjala for suggestions).
- Now using active flag instead of enable in crc
  usage check.

v6: - Fixed rebase conflicts

Signed-off-by: Stanislav Lisovskiy 
Cc: Ville Syrjälä 
Cc: James Ausmus 
---
 drivers/gpu/drm/i915/display/intel_display.c  |   4 +
 .../drm/i915/display/intel_display_types.h|   9 +
 drivers/gpu/drm/i915/i915_drv.h   |   6 +
 drivers/gpu/drm/i915/intel_pm.c   | 296 +-
 4 files changed, 303 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 876fc25968bf..7ea1e7518ab6 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14855,6 +14855,10 @@ static void intel_atomic_commit_tail(struct 
intel_atomic_state *state)
if (dev_priv->display.optimize_watermarks)
dev_priv->display.optimize_watermarks(state,
  new_crtc_state);
+   if (state->crtc_sagv_mask & BIT(crtc->pipe))
+   dev_priv->crtc_sagv_mask |= BIT(crtc->pipe);
+   else
+   dev_priv->crtc_sagv_mask &= ~BIT(crtc->pipe);
}
 
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state, i) {
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
b/drivers/gpu/drm/i915/display/intel_display_types.h
index fadd9853f966..fb274538af23 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -490,6 +490,14 @@ struct intel_atomic_state {
 */
u8 active_pipe_changes;
 
+   /*
+* Contains a mask which reflects whether correspondent pipe
+* can tolerate SAGV or not, so that we can make a decision
+* at atomic_commit_tail stage, whether we enable it or not
+* based on global state in dev_priv.
+*/
+   u32 crtc_sagv_mask;
+
u8 active_pipes;
/* minimum acceptable cdclk for each pipe */
int min_cdclk[I915_MAX_PIPES];
@@ -670,6 +678,7 @@ struct skl_plane_wm {
struct skl_wm_level wm[8];
struct skl_wm_level uv_wm[8];
struct skl_wm_level trans_wm;
+   struct skl_wm_level sagv_wm0;
bool is_planar;
 };
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7e0f67babe20..4f4e2e839513 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1176,6 +1176,12 @@ struct drm_i915_private {
 
u32 sagv_block_time_us;
 
+   /*
+* Contains a bit mask, whether correspondent
+* pipe allows SAGV or not.
+*/
+   u32 crtc_sagv_mask;
+
struct {
/*
 * Raw watermark latency values:
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2d389e437e87..c792dd168742 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3740,7 +3740,7 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
return 0;
 }
 
-bool intel_can_enable_sagv(struct intel_atomic_state *state)
+static void skl_set_sagv_mask(struct intel_atomic_state *state)
 {
struct drm_device *dev = state->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
@@ -3750,21 +3750,23 @@ bool intel_can_enable_sagv(struct intel_atomic_state 
*state)
enum pipe pipe;
int level, latency;
 
+   state->crtc_sagv_mask = 0;
+
if (!intel_has_sagv(dev_priv))
-   return false;
+   return;
 
/*
 * If there are no active CRTCs, no additional checks need be performed
 */
if (hweight8(state->active_pipes) == 0)
-   return 

[Intel-gfx] [PATCH v10 1/2] drm/i915: Refactor intel_can_enable_sagv

2019-11-01 Thread Stanislav Lisovskiy
Currently intel_can_enable_sagv function contains
a mix of workarounds for different platforms
some of them are not valid for gens >= 11 already,
so lets split it into separate functions.

v2:
- Rework watermark calculation algorithm to
  attempt to calculate Level 0 watermark
  with added sagv block time latency and
  check if it fits in DBuf in order to
  determine if SAGV can be enabled already
  at this stage, just as BSpec 49325 states.
  if that fails rollback to usual Level 0
  latency and disable SAGV.
- Remove unneeded tabs(James Ausmus)

v3: Rebased the patch

v4: - Added back interlaced check for Gen12 and
  added separate function for TGL SAGV check
  (thanks to James Ausmus for spotting)
- Removed unneeded gen check
- Extracted Gen12 SAGV decision making code
  to a separate function from skl_compute_wm

v5: - Added SAGV global state to dev_priv, because
  we need to track all pipes, not only those
  in atomic state. Each pipe has now correspondent
  bit mask reflecting, whether it can tolerate
  SAGV or not(thanks to Ville Syrjala for suggestions).
- Now using active flag instead of enable in crc
  usage check.

Signed-off-by: Stanislav Lisovskiy 
Cc: Ville Syrjälä 
Cc: James Ausmus 
---
 drivers/gpu/drm/i915/display/intel_display.c  |   4 +
 .../drm/i915/display/intel_display_types.h|   9 +
 drivers/gpu/drm/i915/i915_drv.h   |   6 +
 drivers/gpu/drm/i915/intel_pm.c   | 299 +-
 4 files changed, 304 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 348ce0456696..8d1881de0bab 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14729,6 +14729,10 @@ static void intel_atomic_commit_tail(struct 
intel_atomic_state *state)
if (dev_priv->display.optimize_watermarks)
dev_priv->display.optimize_watermarks(state,
  new_crtc_state);
+   if (state->crtc_sagv_mask & BIT(crtc->pipe))
+   dev_priv->crtc_sagv_mask |= BIT(crtc->pipe);
+   else
+   dev_priv->crtc_sagv_mask &= ~BIT(crtc->pipe);
}
 
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state, i) {
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 4341bd66a418..38cbabc79b2f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -490,6 +490,14 @@ struct intel_atomic_state {
 */
u8 active_pipe_changes;
 
+   /*
+* Contains a mask which reflects whether correspondent pipe
+* can tolerate SAGV or not, so that we can make a decision
+* at atomic_commit_tail stage, whether we enable it or not
+* based on global state in dev_priv.
+*/
+   u32 crtc_sagv_mask;
+
u8 active_pipes;
/* minimum acceptable cdclk for each pipe */
int min_cdclk[I915_MAX_PIPES];
@@ -650,6 +658,7 @@ struct skl_plane_wm {
struct skl_wm_level wm[8];
struct skl_wm_level uv_wm[8];
struct skl_wm_level trans_wm;
+   struct skl_wm_level sagv_wm0;
bool is_planar;
 };
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4bfd842e2914..60f1c4cc3eeb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1172,6 +1172,12 @@ struct drm_i915_private {
 
u32 sagv_block_time_us;
 
+   /*
+* Contains a bit mask, whether correspondent
+* pipe allows SAGV or not.
+*/
+   u32 crtc_sagv_mask;
+
struct {
/*
 * Raw watermark latency values:
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5d2b460d3ee5..45a966ade1f7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3741,7 +3741,7 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
return 0;
 }
 
-bool intel_can_enable_sagv(struct intel_atomic_state *state)
+static void skl_set_sagv_mask(struct intel_atomic_state *state)
 {
struct drm_device *dev = state->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
@@ -3751,21 +3751,23 @@ bool intel_can_enable_sagv(struct intel_atomic_state 
*state)
enum pipe pipe;
int level, latency;
 
+   state->crtc_sagv_mask = 0;
+
if (!intel_has_sagv(dev_priv))
-   return false;
+   return;
 
/*
 * If there are no active CRTCs, no additional checks need be performed
 */
if (hweight8(state->active_pipes) == 0)
-   return true;
+   return;