Hi,
On 8/22/2018 12:26 AM, Rodrigo Vivi wrote:
On Tue, Aug 21, 2018 at 09:30:21PM +0530, Kumar, Mahesh wrote:
Hi,
On 8/21/2018 8:27 PM, Kumar, Mahesh wrote:
Hi,
On 8/17/2018 11:50 PM, Rodrigo Vivi wrote:
On Thu, Jul 26, 2018 at 07:44:09PM +0530, Mahesh Kumar wrote:
IPC may cause underflows if not used with dual channel symmetric
memory configuration. Disable IPC for non symmetric configurations in
affected platforms.
Display WA #1141
Signed-off-by: Mahesh Kumar <mahesh1.kumar@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.c | 43
++++++++++++++++++++++++++++++++++++-----
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 2 +-
3 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index 86bc2e685522..2273664166bc 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1141,21 +1141,47 @@ skl_dram_get_channel_info(struct
dram_channel_info *ch, u32 val)
return 0;
}
+static bool
+intel_is_dram_ipc_capable(struct drm_i915_private *dev_priv,
+ u32 val_ch0, u32 val_ch1,
+ struct dram_channel_info *ch0)
what about
intel_is_dram_symmetric() ?
sounds good.
+{
+ /* Display WA #1141: SKL:all KBL:all CNL:A CNL:B */
move this to the wa call, not the the function check...
+ if (INTEL_GEN(dev_priv) > 9 &&
+ !IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0))
please don't add CNL pre-prod wa
ok sure
+ return true;
+
+ if (!IS_KABYLAKE(dev_priv) && !IS_SKYLAKE(dev_priv))
+ return true;
actually remove all platforms checks here...
Agree will remove these checks, function will just return if memory is
symmetric.
+
+ if (val_ch0 != val_ch1)
+ return false;
+
+ if (ch0->s_info.size == 0)
+ return true;
+ if (ch0->l_info.size == ch0->s_info.size &&
+ ch0->l_info.width == ch0->s_info.width &&
+ ch0->l_info.rank == ch0->s_info.rank)
+ return true;
+
+ return false;
return (val_ch0 == val_ch1 && (ch0->s_info.size == 0 ||
(ch0->l_info.size == ch0->s_info.size &&
ch0->l_info.width == ch0->s_info.width &&
ch0->l_info.rank == ch0->s_info.rank)))
+}
+
static int
skl_dram_get_channels_info(struct drm_i915_private *dev_priv)
{
struct dram_info *dram_info = &dev_priv->dram_info;
struct dram_channel_info ch0, ch1;
- u32 val;
+ u32 val_ch0, val_ch1;
int ret;
- val = I915_READ(SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
- ret = skl_dram_get_channel_info(&ch0, val);
+ val_ch0 = I915_READ(SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
+ ret = skl_dram_get_channel_info(&ch0, val_ch0);
if (ret == 0)
dram_info->num_channels++;
- val = I915_READ(SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN);
- ret = skl_dram_get_channel_info(&ch1, val);
+ val_ch1 = I915_READ(SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN);
+ ret = skl_dram_get_channel_info(&ch1, val_ch1);
if (ret == 0)
dram_info->num_channels++;
@@ -1185,6 +1211,13 @@ skl_dram_get_channels_info(struct
drm_i915_private *dev_priv)
if (ch0.is_16gb_dimm || ch1.is_16gb_dimm)
dram_info->is_16gb_dimm = true;
+ if (intel_is_dram_ipc_capable(dev_priv, val_ch0, val_ch1, &ch0))
+ dev_priv->ipc_capable_mem = true;
+ else
+ dev_priv->ipc_capable_mem = false;
+
+ DRM_DEBUG_KMS("memory configuration is %sIPC capable\n",
+ dev_priv->ipc_capable_mem ? "" : "not ");
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 854f3c828e01..036d6554c017 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2112,6 +2112,7 @@ struct drm_i915_private {
bool chv_phy_assert[2];
bool ipc_enabled;
+ bool ipc_capable_mem;
I don't think we need to stage this...
With above changes storing this info is not needed, But we need to keep
a flag in dram_info to check if "memory is symmetric", Otherwise we need
to compute all the memory related info again.
-Mahesh
/* Used to save the pipe-to-encoder mapping for audio */
struct intel_encoder *av_enc_map[I915_MAX_PIPES];
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 2446f53adf21..39e400d5f555 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6097,7 +6097,7 @@ void intel_enable_ipc(struct
drm_i915_private *dev_priv)
u32 val;
/* Display WA #0477 WaDisableIPC: skl */
- if (IS_SKYLAKE(dev_priv)) {
+ if (IS_SKYLAKE(dev_priv) || !dev_priv->ipc_capable_mem) {
dev_priv->ipc_enabled = false;
This is not the WA 1141 for other platforms than SKL. Please only
keep skl here.
For the other WA add 4us across all watermark levels
/* Display WA #1141: skl,kbl,cfl */
if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) &&
intel_is_dram_symmetric())
levels += 4;
Adding 4us in latency is already taken care in skl_compute_plane_wm.
Will handle here, enabling of IPC only if memory is symmetric.
But WA1141 describe that we only need to avoid IPC on SKL...
not on every gen9... Everywhere else the 4s should be enough.
so if the 4us is already in place maybe the patch that we need
is to only apply the 4us if memory is symmetric and not skl.
WA also says enable IPC only if memory is symmetric,
<KBL WA: When IPC is enabled, watermark latency values must be increased
by 4us across all levels.
IPC must be enabled only with dual channel symmetric memory configurations.>
-Mahesh
-Mahesh
return;
}
--
2.16.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
