On Mon, 25 Feb 2019, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> We'll need information about the memory configuration on cnl+ too.
> Extend the code to parse the slightly changed register layout.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 69 ++++++++++++++++++++++++---------
> drivers/gpu/drm/i915/i915_reg.h | 17 +++++++-
> 2 files changed, 66 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e3aafe2bf3b7..95361814b531 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1095,17 +1095,43 @@ static int skl_get_dimm_ranks(u16 val)
> if (skl_get_dimm_size(val) == 0)
> return 0;
>
> - switch (val & SKL_DRAM_RANK_MASK) {
> - case SKL_DRAM_RANK_SINGLE:
> - case SKL_DRAM_RANK_DUAL:
> - val = (val & SKL_DRAM_RANK_MASK) >> SKL_DRAM_RANK_SHIFT;
> - return val + 1;
> + val = (val & SKL_DRAM_RANK_MASK) >> SKL_DRAM_RANK_SHIFT;
> +
> + return val + 1;
This part is a bit out of place. Rebase fail?
> +}
> +
> +static int cnl_get_dimm_size(u16 val)
> +{
> + return (val & CNL_DRAM_SIZE_MASK) / 2;
Multiples of 0.5 GB... what an odd unit. What if there's an odd value?
> +}
> +
> +static int cnl_get_dimm_width(u16 val)
> +{
> + if (cnl_get_dimm_size(val) == 0)
> + return 0;
> +
> + switch (val & CNL_DRAM_WIDTH_MASK) {
> + case CNL_DRAM_WIDTH_X8:
> + case CNL_DRAM_WIDTH_X16:
> + case CNL_DRAM_WIDTH_X32:
> + val = (val & CNL_DRAM_WIDTH_MASK) >> CNL_DRAM_WIDTH_SHIFT;
> + return 8 << val;
> default:
> MISSING_CASE(val);
> return 0;
> }
> }
>
> +static int cnl_get_dimm_ranks(u16 val)
> +{
> + if (cnl_get_dimm_size(val) == 0)
> + return 0;
> +
> + val = (val & CNL_DRAM_RANK_MASK) >> CNL_DRAM_RANK_SHIFT;
> +
> + return val + 1;
> +}
> +
> static bool
> skl_is_16gb_dimm(const struct dram_dimm_info *dimm)
> {
> @@ -1113,12 +1139,19 @@ skl_is_16gb_dimm(const struct dram_dimm_info *dimm)
> }
>
> static void
> -skl_dram_get_dimm_info(struct dram_dimm_info *dimm,
> +skl_dram_get_dimm_info(struct drm_i915_private *dev_priv,
> + struct dram_dimm_info *dimm,
> int channel, char dimm_name, u16 val)
> {
> - dimm->size = skl_get_dimm_size(val);
> - dimm->width = skl_get_dimm_width(val);
> - dimm->ranks = skl_get_dimm_ranks(val);
> + if (INTEL_GEN(dev_priv) >= 10) {
> + dimm->size = cnl_get_dimm_size(val);
> + dimm->width = cnl_get_dimm_width(val);
> + dimm->ranks = cnl_get_dimm_ranks(val);
> + } else {
> + dimm->size = skl_get_dimm_size(val);
> + dimm->width = skl_get_dimm_width(val);
> + dimm->ranks = skl_get_dimm_ranks(val);
> + }
>
> DRM_DEBUG_KMS("CH%d DIMM %c size: %d GB, width: X%d, ranks: %d, 16Gb DIMMs: %s\n",
> channel, dimm_name, dimm->size, dimm->width, dimm->ranks,
> @@ -1126,11 +1159,14 @@ skl_dram_get_dimm_info(struct dram_dimm_info *dimm,
> }
>
> static int
> -skl_dram_get_channel_info(struct dram_channel_info *ch,
> +skl_dram_get_channel_info(struct drm_i915_private *dev_priv,
> + struct dram_channel_info *ch,
> int channel, u32 val)
> {
> - skl_dram_get_dimm_info(&ch->dimm_l, channel, 'L', val & 0xffff);
> - skl_dram_get_dimm_info(&ch->dimm_s, channel, 'S', val >> 16);
> + skl_dram_get_dimm_info(dev_priv, &ch->dimm_l,
> + channel, 'L', val & 0xffff);
> + skl_dram_get_dimm_info(dev_priv, &ch->dimm_s,
> + channel, 'S', val >> 16);
>
> if (ch->dimm_l.size == 0 && ch->dimm_s.size == 0) {
> DRM_DEBUG_KMS("CH%d not populated\n", channel);
> @@ -1172,12 +1208,12 @@ skl_dram_get_channels_info(struct drm_i915_private *dev_priv)
> int ret;
>
> val = I915_READ(SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
> - ret = skl_dram_get_channel_info(&ch0, 0, val);
> + ret = skl_dram_get_channel_info(dev_priv, &ch0, 0, val);
> 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, 1, val);
> + ret = skl_dram_get_channel_info(dev_priv, &ch1, 1, val);
> if (ret == 0)
> dram_info->num_channels++;
>
> @@ -1369,13 +1405,10 @@ intel_get_dram_info(struct drm_i915_private *dev_priv)
> if (INTEL_GEN(dev_priv) < 9)
> return;
>
> - /* Need to calculate bandwidth only for Gen9 */
> if (IS_GEN9_LP(dev_priv))
> ret = bxt_get_dram_info(dev_priv);
> - else if (IS_GEN(dev_priv, 9))
> - ret = skl_get_dram_info(dev_priv);
> else
> - ret = skl_dram_get_channels_info(dev_priv);
> + ret = skl_get_dram_info(dev_priv);
The part that's hidden here is the use of the common parts in
skl_get_dram_info() and in particular
SKL_MEMORY_FREQ_MULTIPLIER_HZ. Spec says "The value is given in units of
133.33 MHz". But it says the same for SKL too. What am I missing?
Tentative Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> if (ret)
> return;
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 730bb1917fd1..b35b0220764f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9875,8 +9875,21 @@ enum skl_power_gate {
> #define SKL_DRAM_WIDTH_X32 (0x2 << 8)
> #define SKL_DRAM_RANK_MASK (0x1 << 10)
> #define SKL_DRAM_RANK_SHIFT 10
> -#define SKL_DRAM_RANK_SINGLE (0x0 << 10)
> -#define SKL_DRAM_RANK_DUAL (0x1 << 10)
> +#define SKL_DRAM_RANK_1 (0x0 << 10)
> +#define SKL_DRAM_RANK_2 (0x1 << 10)
> +#define SKL_DRAM_RANK_MASK (0x1 << 10)
> +#define CNL_DRAM_SIZE_MASK 0x7F
> +#define CNL_DRAM_WIDTH_MASK (0x3 << 7)
> +#define CNL_DRAM_WIDTH_SHIFT 7
> +#define CNL_DRAM_WIDTH_X8 (0x0 << 7)
> +#define CNL_DRAM_WIDTH_X16 (0x1 << 7)
> +#define CNL_DRAM_WIDTH_X32 (0x2 << 7)
> +#define CNL_DRAM_RANK_MASK (0x3 << 9)
> +#define CNL_DRAM_RANK_SHIFT 9
> +#define CNL_DRAM_RANK_1 (0x0 << 9)
> +#define CNL_DRAM_RANK_2 (0x1 << 9)
> +#define CNL_DRAM_RANK_3 (0x2 << 9)
> +#define CNL_DRAM_RANK_4 (0x3 << 9)
>
> /* Please see hsw_read_dcomp() and hsw_write_dcomp() before using this register,
> * since on HSW we can't write to it using I915_WRITE. */
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
