Re: [Intel-gfx] [PATCH 6/6] drm/i915/gen10: Calculate and enable transition WM

2017-06-13 Thread Mahesh Kumar 

Hi,


On Tuesday 13 June 2017 02:59 PM, Lankhorst, Maarten wrote:

Hey,

Mahesh Kumar schreef op di 13-06-2017 om 11:34 [+0530]:

GEN > 9 require transition WM to be programmed if IPC is enabled.
This patch calculates & enable transition WM for supported platforms.
If transition WM is enabled, Plane read requests are sent at high
priority until filling above the transition watermark, then the
requests are sent at lower priority until dropping below the level-0
WM.
The lower priority requests allow other memory clients to have better
memory access.

transition minimum is the minimum amount needed for trans_wm to work
to
ensure  the demote does not happen before enough data has been read
to
meet the level 0 watermark requirements.

transition amount is configurable value. Higher values will
tend to cause longer periods of high priority reads followed by
longer
periods of lower priority reads. Tuning to lower values will tend to
cause shorter periods of high and lower priority reads.

Keeping transition amount to 0 in this patch.
Signed-off-by: Mahesh Kumar 
---
  drivers/gpu/drm/i915/intel_pm.c | 51
++---
  1 file changed, 48 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 10ec2660acd7..6b951aa14840 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4163,6 +4163,15 @@ skl_enable_plane_wm_levels(const struct
drm_i915_private *dev_priv,
level_wm->plane_en = true;
}
}
+
+   /*
+* Unsupported GEN will have plane_res_b = 0 & transition WM
for
+* them will get disabled here.
+*/
+   if (wm->trans_wm.plane_res_b && wm->trans_wm.plane_res_b <
plane_ddb)
+   wm->trans_wm.plane_en = true;
+   else
+   wm->trans_wm.plane_en = false;
  }
  
  static int

@@ -4639,13 +4648,48 @@ skl_compute_linetime_wm(struct
intel_crtc_state *cstate)
  }
  
  static void skl_compute_transition_wm(struct intel_crtc_state

*cstate,
+ struct skl_wm_params *wp,
+ struct skl_wm_level *wm_l0,
  struct skl_wm_level *trans_wm
/* out */)
  {
+   struct drm_device *dev = cstate->base.crtc->dev;
+   const struct drm_i915_private *dev_priv = to_i915(dev);
+   uint16_t trans_min, trans_y_tile_min;
+   uint16_t trans_amount = 0; /* This is configurable amount */
+   uint16_t trans_offset_b, res_blocks;
+
if (!cstate->base.active)
return;
-   /* Until we know more, just disable transition WMs */
-   trans_wm->plane_en = false;
+   /* Transition WM are not recommended by HW team for GEN9 */
+   if (INTEL_GEN(dev_priv) <= 9)
+   return;
+
+   /* Transition WM don't have any impact if ipc is disabled */
+   if (!dev_priv->ipc_enabled)
+   return;

ipc_enabled is never set, so this patch on its own doesn't do much. :)
If I enable IPC, I observed many fifo underrun in only fi-glk, that's 
the reason I didn't push IPC patch.

I'll debug further for under-run in glk with IPC & post that patch later.

thanks for review :)

-Mahesh

Otherwise series looks good, I didn't check the math on this patch,
but the series doesn't regress KBL.

For patch 3 and 4:
Reviewed-by: Maarten Lankhorst 

For patch 5 and 6:
Acked-by: Maarten Lankhorst 


+   if (INTEL_GEN(dev_priv) >= 10)
+   trans_min = 4;
+
+   trans_offset_b = trans_min + trans_amount;
+   trans_y_tile_min = (uint16_t) mul_round_up_u32_fixed16(2,
+   wp-

y_tile_minimum);

+
+   if (wp->y_tiled) {
+   res_blocks = max(wm_l0->plane_res_b,
trans_y_tile_min) +
+   trans_offset_b;
+   } else {
+   res_blocks = wm_l0->plane_res_b + trans_offset_b;
+   }
+
+   res_blocks += 1;
+
+   /* WA BUG:1938466 add one block for non y-tile planes */
+   if (!wp->y_tiled && IS_CNL_REVID(dev_priv, CNL_REVID_A0,
CNL_REVID_A0))
+   res_blocks += 1;
+
+   trans_wm->plane_res_b = res_blocks;
  }
  
  static int skl_build_pipe_wm(struct intel_crtc_state *cstate,

@@ -4684,7 +4728,8 @@ static int skl_build_pipe_wm(struct
intel_crtc_state *cstate,
_params, wm);
if (ret)
return ret;
-   skl_compute_transition_wm(cstate, >trans_wm);
+   skl_compute_transition_wm(cstate, _params, 

wm[0],

+ >trans_wm);
}
pipe_wm->linetime = skl_compute_linetime_wm(cstate);
  


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org

Re: [Intel-gfx] [PATCH 6/6] drm/i915/gen10: Calculate and enable transition WM

2017-06-13 Thread Lankhorst, Maarten 
Hey,

Mahesh Kumar schreef op di 13-06-2017 om 11:34 [+0530]:
> GEN > 9 require transition WM to be programmed if IPC is enabled.
> This patch calculates & enable transition WM for supported platforms.
> If transition WM is enabled, Plane read requests are sent at high
> priority until filling above the transition watermark, then the
> requests are sent at lower priority until dropping below the level-0
> WM.
> The lower priority requests allow other memory clients to have better
> memory access.
> 
> transition minimum is the minimum amount needed for trans_wm to work
> to
> ensure  the demote does not happen before enough data has been read
> to
> meet the level 0 watermark requirements.
> 
> transition amount is configurable value. Higher values will
> tend to cause longer periods of high priority reads followed by
> longer
> periods of lower priority reads. Tuning to lower values will tend to
> cause shorter periods of high and lower priority reads.
> 
> Keeping transition amount to 0 in this patch.

> Signed-off-by: Mahesh Kumar 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 51
> ++---
>  1 file changed, 48 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 10ec2660acd7..6b951aa14840 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4163,6 +4163,15 @@ skl_enable_plane_wm_levels(const struct
> drm_i915_private *dev_priv,
>   level_wm->plane_en = true;
>   }
>   }
> +
> + /*
> +  * Unsupported GEN will have plane_res_b = 0 & transition WM
> for
> +  * them will get disabled here.
> +  */
> + if (wm->trans_wm.plane_res_b && wm->trans_wm.plane_res_b <
> plane_ddb)
> + wm->trans_wm.plane_en = true;
> + else
> + wm->trans_wm.plane_en = false;
>  }
>  
>  static int
> @@ -4639,13 +4648,48 @@ skl_compute_linetime_wm(struct
> intel_crtc_state *cstate)
>  }
>  
>  static void skl_compute_transition_wm(struct intel_crtc_state
> *cstate,
> +   struct skl_wm_params *wp,
> +   struct skl_wm_level *wm_l0,
>     struct skl_wm_level *trans_wm
> /* out */)
>  {
> + struct drm_device *dev = cstate->base.crtc->dev;
> + const struct drm_i915_private *dev_priv = to_i915(dev);
> + uint16_t trans_min, trans_y_tile_min;
> + uint16_t trans_amount = 0; /* This is configurable amount */
> + uint16_t trans_offset_b, res_blocks;
> +
>   if (!cstate->base.active)
>   return;
> - /* Until we know more, just disable transition WMs */
> - trans_wm->plane_en = false;
> + /* Transition WM are not recommended by HW team for GEN9 */
> + if (INTEL_GEN(dev_priv) <= 9)
> + return;
> +
> + /* Transition WM don't have any impact if ipc is disabled */
> + if (!dev_priv->ipc_enabled)
> + return;
ipc_enabled is never set, so this patch on its own doesn't do much. :)

Otherwise series looks good, I didn't check the math on this patch,
but the series doesn't regress KBL.

For patch 3 and 4:
Reviewed-by: Maarten Lankhorst 

For patch 5 and 6:
Acked-by: Maarten Lankhorst 

> + if (INTEL_GEN(dev_priv) >= 10)
> + trans_min = 4;
> +
> + trans_offset_b = trans_min + trans_amount;
> + trans_y_tile_min = (uint16_t) mul_round_up_u32_fixed16(2,
> + wp-
> >y_tile_minimum);
> +
> + if (wp->y_tiled) {
> + res_blocks = max(wm_l0->plane_res_b,
> trans_y_tile_min) +
> + trans_offset_b;
> + } else {
> + res_blocks = wm_l0->plane_res_b + trans_offset_b;
> + }
> +
> + res_blocks += 1;
> +
> + /* WA BUG:1938466 add one block for non y-tile planes */
> + if (!wp->y_tiled && IS_CNL_REVID(dev_priv, CNL_REVID_A0,
> CNL_REVID_A0))
> + res_blocks += 1;
> +
> + trans_wm->plane_res_b = res_blocks;
>  }
>  
>  static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
> @@ -4684,7 +4728,8 @@ static int skl_build_pipe_wm(struct
> intel_crtc_state *cstate,
>   _params, wm);
>   if (ret)
>   return ret;
> - skl_compute_transition_wm(cstate, >trans_wm);
> + skl_compute_transition_wm(cstate, _params, 
> >wm[0],
> +   >trans_wm);
>   }
>   pipe_wm->linetime = skl_compute_linetime_wm(cstate);
>  

smime.p7s
Description: S/MIME cryptographic signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 6/6] drm/i915/gen10: Calculate and enable transition WM

2017-06-13 Thread Mahesh Kumar 
GEN > 9 require transition WM to be programmed if IPC is enabled.
This patch calculates & enable transition WM for supported platforms.
If transition WM is enabled, Plane read requests are sent at high
priority until filling above the transition watermark, then the
requests are sent at lower priority until dropping below the level-0 WM.
The lower priority requests allow other memory clients to have better
memory access.

transition minimum is the minimum amount needed for trans_wm to work to
ensure  the demote does not happen before enough data has been read to
meet the level 0 watermark requirements.

transition amount is configurable value. Higher values will
tend to cause longer periods of high priority reads followed by longer
periods of lower priority reads. Tuning to lower values will tend to
cause shorter periods of high and lower priority reads.

Keeping transition amount to 0 in this patch.

Signed-off-by: Mahesh Kumar 
---
 drivers/gpu/drm/i915/intel_pm.c | 51 ++---
 1 file changed, 48 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 10ec2660acd7..6b951aa14840 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4163,6 +4163,15 @@ skl_enable_plane_wm_levels(const struct drm_i915_private 
*dev_priv,
level_wm->plane_en = true;
}
}
+
+   /*
+* Unsupported GEN will have plane_res_b = 0 & transition WM for
+* them will get disabled here.
+*/
+   if (wm->trans_wm.plane_res_b && wm->trans_wm.plane_res_b < plane_ddb)
+   wm->trans_wm.plane_en = true;
+   else
+   wm->trans_wm.plane_en = false;
 }
 
 static int
@@ -4639,13 +4648,48 @@ skl_compute_linetime_wm(struct intel_crtc_state *cstate)
 }
 
 static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
+ struct skl_wm_params *wp,
+ struct skl_wm_level *wm_l0,
  struct skl_wm_level *trans_wm /* out */)
 {
+   struct drm_device *dev = cstate->base.crtc->dev;
+   const struct drm_i915_private *dev_priv = to_i915(dev);
+   uint16_t trans_min, trans_y_tile_min;
+   uint16_t trans_amount = 0; /* This is configurable amount */
+   uint16_t trans_offset_b, res_blocks;
+
if (!cstate->base.active)
return;
 
-   /* Until we know more, just disable transition WMs */
-   trans_wm->plane_en = false;
+   /* Transition WM are not recommended by HW team for GEN9 */
+   if (INTEL_GEN(dev_priv) <= 9)
+   return;
+
+   /* Transition WM don't have any impact if ipc is disabled */
+   if (!dev_priv->ipc_enabled)
+   return;
+
+   if (INTEL_GEN(dev_priv) >= 10)
+   trans_min = 4;
+
+   trans_offset_b = trans_min + trans_amount;
+   trans_y_tile_min = (uint16_t) mul_round_up_u32_fixed16(2,
+   wp->y_tile_minimum);
+
+   if (wp->y_tiled) {
+   res_blocks = max(wm_l0->plane_res_b, trans_y_tile_min) +
+   trans_offset_b;
+   } else {
+   res_blocks = wm_l0->plane_res_b + trans_offset_b;
+   }
+
+   res_blocks += 1;
+
+   /* WA BUG:1938466 add one block for non y-tile planes */
+   if (!wp->y_tiled && IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_A0))
+   res_blocks += 1;
+
+   trans_wm->plane_res_b = res_blocks;
 }
 
 static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
@@ -4684,7 +4728,8 @@ static int skl_build_pipe_wm(struct intel_crtc_state 
*cstate,
_params, wm);
if (ret)
return ret;
-   skl_compute_transition_wm(cstate, >trans_wm);
+   skl_compute_transition_wm(cstate, _params, >wm[0],
+ >trans_wm);
}
pipe_wm->linetime = skl_compute_linetime_wm(cstate);
 
-- 
2.13.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx