| Message ID | 20190225202907.731-6-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Polish DRAM information readout code | expand |
Quoting Ville Syrjala (2019-02-25 20:29:00) > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > The BXT DUNIT register tells us the size of each DRAM device > in Gb. We want to report the size of the whole DIMM in GB, so > that it matches how we report it for non-LP platforms. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 1f4a966a9727..c40a738dabd3 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1322,7 +1322,14 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv) > width = bxt_get_dimm_width(val); > ranks = bxt_get_dimm_ranks(val); > > - DRM_DEBUG_KMS("CH%d DIMM size: % dGB, width: X%d, ranks:%d\n", > + /* > + * Size in register is Gb per DRAM device. > + * Convert to total GB to match the way > + * we report this for non-LP platforms. > + */ > + size = size * ranks * 8 / (width ?: 1); Should it be /8 for Gbits to GBytes? -Chris
On Mon, Feb 25, 2019 at 08:35:08PM +0000, Chris Wilson wrote: > Quoting Ville Syrjala (2019-02-25 20:29:00) > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > The BXT DUNIT register tells us the size of each DRAM device > > in Gb. We want to report the size of the whole DIMM in GB, so > > that it matches how we report it for non-LP platforms. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index 1f4a966a9727..c40a738dabd3 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -1322,7 +1322,14 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv) > > width = bxt_get_dimm_width(val); > > ranks = bxt_get_dimm_ranks(val); > > > > - DRM_DEBUG_KMS("CH%d DIMM size: % dGB, width: X%d, ranks:%d\n", > > + /* > > + * Size in register is Gb per DRAM device. > > + * Convert to total GB to match the way > > + * we report this for non-LP platforms. > > + */ > > + size = size * ranks * 8 / (width ?: 1); > > Should it be /8 for Gbits to GBytes? It's a hand optimized version of size*ranks*64 / width / 8
Quoting Ville Syrjälä (2019-02-25 20:48:10) > On Mon, Feb 25, 2019 at 08:35:08PM +0000, Chris Wilson wrote: > > Quoting Ville Syrjala (2019-02-25 20:29:00) > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > The BXT DUNIT register tells us the size of each DRAM device > > > in Gb. We want to report the size of the whole DIMM in GB, so > > > that it matches how we report it for non-LP platforms. > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_drv.c | 9 ++++++++- > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > > index 1f4a966a9727..c40a738dabd3 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -1322,7 +1322,14 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv) > > > width = bxt_get_dimm_width(val); > > > ranks = bxt_get_dimm_ranks(val); > > > > > > - DRM_DEBUG_KMS("CH%d DIMM size: % dGB, width: X%d, ranks:%d\n", > > > + /* > > > + * Size in register is Gb per DRAM device. > > > + * Convert to total GB to match the way > > > + * we report this for non-LP platforms. > > > + */ > > > + size = size * ranks * 8 / (width ?: 1); > > > > Should it be /8 for Gbits to GBytes? > > It's a hand optimized version of > > size*ranks*64 / width / 8 Maybe let the compiler handle the constants, otherwise every time I see this I'll think it's backwards ;) Maybe be even size *= 64 * ranks / (width ?: 1); /* * Size in register is Gb per DRAM device. * Convert to total GB to match the way * we report this for non-LP platforms. */ size /= 8; -Chris
On Mon, Feb 25, 2019 at 08:57:45PM +0000, Chris Wilson wrote: > Quoting Ville Syrjälä (2019-02-25 20:48:10) > > On Mon, Feb 25, 2019 at 08:35:08PM +0000, Chris Wilson wrote: > > > Quoting Ville Syrjala (2019-02-25 20:29:00) > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > The BXT DUNIT register tells us the size of each DRAM device > > > > in Gb. We want to report the size of the whole DIMM in GB, so > > > > that it matches how we report it for non-LP platforms. > > > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > --- > > > > drivers/gpu/drm/i915/i915_drv.c | 9 ++++++++- > > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > > > index 1f4a966a9727..c40a738dabd3 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > @@ -1322,7 +1322,14 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv) > > > > width = bxt_get_dimm_width(val); > > > > ranks = bxt_get_dimm_ranks(val); > > > > > > > > - DRM_DEBUG_KMS("CH%d DIMM size: % dGB, width: X%d, ranks:%d\n", > > > > + /* > > > > + * Size in register is Gb per DRAM device. > > > > + * Convert to total GB to match the way > > > > + * we report this for non-LP platforms. > > > > + */ > > > > + size = size * ranks * 8 / (width ?: 1); > > > > > > Should it be /8 for Gbits to GBytes? > > > > It's a hand optimized version of > > > > size*ranks*64 / width / 8 > > Maybe let the compiler handle the constants, otherwise every time I see > this I'll think it's backwards ;) > > Maybe be even > size *= 64 * ranks / (width ?: 1); > > /* > * Size in register is Gb per DRAM device. > * Convert to total GB to match the way > * we report this for non-LP platforms. > */ > size /= 8; I suppose clarity is better here. I'll have to double check the types though. There's the opposite calculation in is_16gb_dimm(). I suppose that should get the same treatment. Hmm. Maybe I should even extract something like: intel_dimm_num_devices() { return 64 * ranks / width; } and use that everywhere.
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 1f4a966a9727..c40a738dabd3 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1322,7 +1322,14 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv) width = bxt_get_dimm_width(val); ranks = bxt_get_dimm_ranks(val); - DRM_DEBUG_KMS("CH%d DIMM size: % dGB, width: X%d, ranks:%d\n", + /* + * Size in register is Gb per DRAM device. + * Convert to total GB to match the way + * we report this for non-LP platforms. + */ + size = size * ranks * 8 / (width ?: 1); + + DRM_DEBUG_KMS("CH%d DIMM size: %d GB, width: X%d, ranks: %d\n", i - BXT_D_CR_DRP0_DUNIT_START, size, width, ranks);