[01/12] drm/i915: Store DIMM rank information as a number
diff mbox series

Message ID 20190225202907.731-2-ville.syrjala@linux.intel.com
State New
Headers show
Series
  • Polish DRAM information readout code
Related show

Commit Message

Ville Syrjälä Feb. 25, 2019, 8:28 p.m. UTC
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(-)

Comments

Jani Nikula March 4, 2019, 4:17 p.m. UTC | #1
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;
>  };
Ville Syrjälä March 4, 2019, 4:32 p.m. UTC | #2
On Mon, Mar 04, 2019 at 06:17:50PM +0200, Jani Nikula wrote:
> 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.

Good point. I didn't even notice :)

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

Doesn't really matter for these small values. But no harm in %u either
so might as well I suppose.

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

Patch
diff mbox series

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");
+	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);
 
 	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;
 };