[Intel-gfx] [PATCH 4/5] drm/i915/kbl+: Enable IPC only for symmetric memory configurations
Kumar, Mahesh
mahesh1.kumar at intel.com
Wed Aug 22 13:02:33 UTC 2018
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 at intel.com>
>>>>> ---
>>>>> 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 at lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list