From: Jani Nikula <jani.nikula@linux.intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 01/12] drm/i915: Store DIMM rank information as a number
Date: Mon, 04 Mar 2019 18:17:50 +0200 [thread overview]
Message-ID: <87d0n6mx5d.fsf@intel.com> (raw)
In-Reply-To: <20190225202907.731-2-ville.syrjala@linux.intel.com>
On Mon, 25 Feb 2019, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Life will be easier later if we have the ranks stored
> as a bare number.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 92 +++++++++++++++------------------
> drivers/gpu/drm/i915/i915_drv.h | 11 ++--
> 2 files changed, 45 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index c6354f6cdbdb..48c6bc44072d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1068,28 +1068,28 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
> intel_gvt_sanitize_options(dev_priv);
> }
>
> -static enum dram_rank skl_get_dimm_rank(u8 size, u32 rank)
> +static int skl_get_dimm_ranks(u8 size, u32 rank)
> {
> if (size == 0)
> - return I915_DRAM_RANK_INVALID;
> + return 0;
> if (rank == SKL_DRAM_RANK_SINGLE)
> - return I915_DRAM_RANK_SINGLE;
> + return 1;
> else if (rank == SKL_DRAM_RANK_DUAL)
> - return I915_DRAM_RANK_DUAL;
> + return 2;
>
> - return I915_DRAM_RANK_INVALID;
> + return 0;
> }
>
> static bool
> -skl_is_16gb_dimm(enum dram_rank rank, u8 size, u8 width)
> +skl_is_16gb_dimm(u8 ranks, u8 size, u8 width)
> {
> - if (rank == I915_DRAM_RANK_SINGLE && width == 8 && size == 16)
> + if (ranks == 1 && width == 8 && size == 16)
> return true;
> - else if (rank == I915_DRAM_RANK_DUAL && width == 8 && size == 32)
> + else if (ranks == 2 && width == 8 && size == 32)
> return true;
> - else if (rank == SKL_DRAM_RANK_SINGLE && width == 16 && size == 8)
> + else if (ranks == 1 && width == 16 && size == 8)
> return true;
> - else if (rank == SKL_DRAM_RANK_DUAL && width == 16 && size == 16)
> + else if (ranks == 2 && width == 16 && size == 16)
> return true;
>
> return false;
> @@ -1120,28 +1120,24 @@ skl_dram_get_channel_info(struct dram_channel_info *ch, u32 val)
>
> tmp_l = val & SKL_DRAM_RANK_MASK;
> tmp_s = s_val & SKL_DRAM_RANK_MASK;
> - ch->l_info.rank = skl_get_dimm_rank(ch->l_info.size, tmp_l);
> - ch->s_info.rank = skl_get_dimm_rank(ch->s_info.size, tmp_s);
> -
> - if (ch->l_info.rank == I915_DRAM_RANK_DUAL ||
> - ch->s_info.rank == I915_DRAM_RANK_DUAL)
> - ch->rank = I915_DRAM_RANK_DUAL;
> - else if (ch->l_info.rank == I915_DRAM_RANK_SINGLE &&
> - ch->s_info.rank == I915_DRAM_RANK_SINGLE)
> - ch->rank = I915_DRAM_RANK_DUAL;
> + ch->l_info.ranks = skl_get_dimm_ranks(ch->l_info.size, tmp_l);
> + ch->s_info.ranks = skl_get_dimm_ranks(ch->s_info.size, tmp_s);
> +
> + if (ch->l_info.ranks == 2 || ch->s_info.ranks == 2)
> + ch->ranks = 2;
> + else if (ch->l_info.ranks == 1 && ch->s_info.ranks == 1)
> + ch->ranks = 2;
> else
> - ch->rank = I915_DRAM_RANK_SINGLE;
> + ch->ranks = 1;
>
> - ch->is_16gb_dimm = skl_is_16gb_dimm(ch->l_info.rank, ch->l_info.size,
> + ch->is_16gb_dimm = skl_is_16gb_dimm(ch->l_info.ranks, ch->l_info.size,
> ch->l_info.width) ||
> - skl_is_16gb_dimm(ch->s_info.rank, ch->s_info.size,
> + skl_is_16gb_dimm(ch->s_info.ranks, ch->s_info.size,
> ch->s_info.width);
>
> - DRM_DEBUG_KMS("(size:width:rank) L(%dGB:X%d:%s) S(%dGB:X%d:%s)\n",
> - ch->l_info.size, ch->l_info.width,
> - ch->l_info.rank ? "dual" : "single",
> - ch->s_info.size, ch->s_info.width,
> - ch->s_info.rank ? "dual" : "single");
I don't understand how the above ternary operators could ever have
produced the right results before.
> + DRM_DEBUG_KMS("(size:width:ranks) L(%dGB:X%d:%d) S(%dGB:X%d:%d)\n",
> + ch->l_info.size, ch->l_info.width, ch->l_info.ranks,
> + ch->s_info.size, ch->s_info.width, ch->s_info.ranks);
%u instead of %d? Ditto for all debug prints.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
> return 0;
> }
> @@ -1154,7 +1150,7 @@ intel_is_dram_symmetric(u32 val_ch0, u32 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)));
> + ch0->l_info.ranks == ch0->s_info.ranks)));
> }
>
> static int
> @@ -1185,13 +1181,12 @@ skl_dram_get_channels_info(struct drm_i915_private *dev_priv)
> * will be same as if single rank memory, so consider single rank
> * memory.
> */
> - if (ch0.rank == I915_DRAM_RANK_SINGLE ||
> - ch1.rank == I915_DRAM_RANK_SINGLE)
> - dram_info->rank = I915_DRAM_RANK_SINGLE;
> + if (ch0.ranks == 1 || ch1.ranks == 1)
> + dram_info->ranks = 1;
> else
> - dram_info->rank = max(ch0.rank, ch1.rank);
> + dram_info->ranks = max(ch0.ranks, ch1.ranks);
>
> - if (dram_info->rank == I915_DRAM_RANK_INVALID) {
> + if (dram_info->ranks == 0) {
> DRM_INFO("couldn't get memory rank information\n");
> return -EINVAL;
> }
> @@ -1262,8 +1257,7 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv)
> * Now read each DUNIT8/9/10/11 to check the rank of each dimms.
> */
> for (i = BXT_D_CR_DRP0_DUNIT_START; i <= BXT_D_CR_DRP0_DUNIT_END; i++) {
> - u8 size, width;
> - enum dram_rank rank;
> + u8 size, width, ranks;
> u32 tmp;
>
> val = I915_READ(BXT_D_CR_DRP0_DUNIT(i));
> @@ -1274,11 +1268,11 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv)
> tmp = val & BXT_DRAM_RANK_MASK;
>
> if (tmp == BXT_DRAM_RANK_SINGLE)
> - rank = I915_DRAM_RANK_SINGLE;
> + ranks = 1;
> else if (tmp == BXT_DRAM_RANK_DUAL)
> - rank = I915_DRAM_RANK_DUAL;
> + ranks = 2;
> else
> - rank = I915_DRAM_RANK_INVALID;
> + ranks = 0;
>
> tmp = val & BXT_DRAM_SIZE_MASK;
> if (tmp == BXT_DRAM_SIZE_4GB)
> @@ -1296,22 +1290,21 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv)
>
> tmp = (val & BXT_DRAM_WIDTH_MASK) >> BXT_DRAM_WIDTH_SHIFT;
> width = (1 << tmp) * 8;
> - DRM_DEBUG_KMS("dram size:%dGB width:X%d rank:%s\n", size,
> - width, rank == I915_DRAM_RANK_SINGLE ? "single" :
> - rank == I915_DRAM_RANK_DUAL ? "dual" : "unknown");
> + DRM_DEBUG_KMS("dram size:%dGB width:X%d ranks:%d\n",
> + size, width, ranks);
>
> /*
> * If any of the channel is single rank channel,
> * worst case output will be same as if single rank
> * memory, so consider single rank memory.
> */
> - if (dram_info->rank == I915_DRAM_RANK_INVALID)
> - dram_info->rank = rank;
> - else if (rank == I915_DRAM_RANK_SINGLE)
> - dram_info->rank = I915_DRAM_RANK_SINGLE;
> + if (dram_info->ranks == 0)
> + dram_info->ranks = ranks;
> + else if (ranks == 1)
> + dram_info->ranks = 1;
> }
>
> - if (dram_info->rank == I915_DRAM_RANK_INVALID) {
> + if (dram_info->ranks == 0) {
> DRM_INFO("couldn't get memory rank information\n");
> return -EINVAL;
> }
> @@ -1328,7 +1321,7 @@ intel_get_dram_info(struct drm_i915_private *dev_priv)
> int ret;
>
> dram_info->valid = false;
> - dram_info->rank = I915_DRAM_RANK_INVALID;
> + dram_info->ranks = 0;
> dram_info->bandwidth_kbps = 0;
> dram_info->num_channels = 0;
>
> @@ -1358,9 +1351,8 @@ intel_get_dram_info(struct drm_i915_private *dev_priv)
> sprintf(bandwidth_str, "unknown");
> DRM_DEBUG_KMS("DRAM bandwidth:%s, total-channels: %u\n",
> bandwidth_str, dram_info->num_channels);
> - DRM_DEBUG_KMS("DRAM rank: %s rank 16GB-dimm:%s\n",
> - (dram_info->rank == I915_DRAM_RANK_DUAL) ?
> - "dual" : "single", yesno(dram_info->is_16gb_dimm));
> + DRM_DEBUG_KMS("DRAM ranks: %d, 16GB-dimm:%s\n",
> + dram_info->ranks, yesno(dram_info->is_16gb_dimm));
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cc09caf3870e..c9cb13a6edaf 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1833,11 +1833,7 @@ struct drm_i915_private {
> bool valid;
> bool is_16gb_dimm;
> u8 num_channels;
> - enum dram_rank {
> - I915_DRAM_RANK_INVALID = 0,
> - I915_DRAM_RANK_SINGLE,
> - I915_DRAM_RANK_DUAL
> - } rank;
> + u8 ranks;
> u32 bandwidth_kbps;
> bool symmetric_memory;
> } dram_info;
> @@ -2071,10 +2067,9 @@ struct drm_i915_private {
>
> struct dram_channel_info {
> struct info {
> - u8 size, width;
> - enum dram_rank rank;
> + u8 size, width, ranks;
> } l_info, s_info;
> - enum dram_rank rank;
> + u8 ranks;
> bool is_16gb_dimm;
> };
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-03-04 16:15 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-25 20:28 [PATCH 00/12] Polish DRAM information readout code Ville Syrjala
2019-02-25 20:28 ` [PATCH 01/12] drm/i915: Store DIMM rank information as a number Ville Syrjala
2019-03-04 16:17 ` Jani Nikula [this message]
2019-03-04 16:32 ` Ville Syrjälä
2019-02-25 20:28 ` [PATCH 02/12] drm/i915: Extract functions to derive SKL+ DIMM info Ville Syrjala
2019-03-04 16:32 ` Jani Nikula
2019-03-04 16:44 ` Ville Syrjälä
2019-02-25 20:28 ` [PATCH 03/12] drm/i915: Polish skl_is_16gb_dimm() Ville Syrjala
2019-02-26 15:26 ` [PATCH v2 " Ville Syrjala
2019-03-04 18:19 ` Jani Nikula
2019-02-25 20:28 ` [PATCH 04/12] drm/i915: Extract BXT DIMM helpers Ville Syrjala
2019-02-26 15:27 ` [PATCH v2 " Ville Syrjala
2019-03-04 18:26 ` Jani Nikula
2019-02-25 20:29 ` [PATCH 05/12] drm/i915: Fix DRAM size reporting for BXT Ville Syrjala
2019-02-25 20:35 ` Chris Wilson
2019-02-25 20:48 ` Ville Syrjälä
2019-02-25 20:57 ` Chris Wilson
2019-02-25 21:06 ` Ville Syrjälä
2019-02-26 15:27 ` [PATCH v2 " Ville Syrjala
2019-03-04 18:56 ` Jani Nikula
2019-02-25 20:29 ` [PATCH 06/12] drm/i915: Extract DIMM info on GLK too Ville Syrjala
2019-03-05 17:00 ` Jani Nikula
2019-02-25 20:29 ` [PATCH 07/12] drm/i915: Use dram_dimm_info more Ville Syrjala
2019-03-04 19:13 ` Jani Nikula
2019-02-25 20:29 ` [PATCH 08/12] drm/i915: Generalize intel_is_dram_symmetric() Ville Syrjala
2019-03-04 19:57 ` Jani Nikula
2019-02-25 20:29 ` [PATCH 09/12] drm/i914: s/l_info/dimm_l/ etc Ville Syrjala
2019-03-04 19:58 ` Jani Nikula
2019-02-25 20:29 ` [PATCH 10/12] drm/i915: Clean up intel_get_dram_info() a bit Ville Syrjala
2019-03-04 20:01 ` Jani Nikula
2019-02-25 20:29 ` [PATCH 11/12] drm/i915: Extract DIMM info on cnl+ Ville Syrjala
2019-03-05 16:16 ` Jani Nikula
2019-03-06 19:25 ` Ville Syrjälä
2019-03-06 19:48 ` Jani Nikula
2019-02-25 20:29 ` [PATCH 12/12] drm/i915: Read out memory type Ville Syrjala
2019-02-26 17:57 ` [PATCH v2 " Ville Syrjala
2019-03-05 16:35 ` Jani Nikula
2019-02-25 21:00 ` ✗ Fi.CI.CHECKPATCH: warning for Polish DRAM information readout code Patchwork
2019-02-25 21:04 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-02-25 21:21 ` ✓ Fi.CI.BAT: success " Patchwork
2019-02-26 5:52 ` ✓ Fi.CI.IGT: " Patchwork
2019-02-26 15:37 ` ✗ Fi.CI.BAT: failure for Polish DRAM information readout code (rev4) Patchwork
2019-02-26 19:13 ` ✗ Fi.CI.CHECKPATCH: warning for Polish DRAM information readout code (rev5) Patchwork
2019-02-26 19:18 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-02-26 19:42 ` ✓ Fi.CI.BAT: success " Patchwork
2019-02-26 23:33 ` ✓ Fi.CI.IGT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87d0n6mx5d.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.