[Intel-gfx] [PATCH 05/12] drm/i915: Fix DRAM size reporting for BXT

[Ville Syrjälä]

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 at 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 at 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.

-- 
Ville Syrjälä
Intel