Re: [Intel-gfx] [PATCH 08/12] drm/i915: Generalize intel_is_dram_symmetric()

2019-03-04 Thread Jani Nikula
On Mon, 25 Feb 2019, Ville Syrjala  wrote:
> From: Ville Syrjälä 
>
> Decouple intel_is_dram_symmetric() from the raw register values
> by comparing just the dram_channel_info structs.

The idea is sound.

>
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 28 
>  1 file changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 3d6a08e907e3..9261bd0dccd6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1155,14 +1155,12 @@ skl_dram_get_channel_info(struct dram_channel_info 
> *ch,
>  }
>  
>  static bool
> -intel_is_dram_symmetric(u32 val_ch0, u32 val_ch1,
> - struct dram_channel_info *ch0)
> +intel_is_dram_symmetric(const struct dram_channel_info *ch0,
> + const struct dram_channel_info *ch1)
>  {
> - return (val_ch0 == val_ch1 &&
> + return !memcmp(ch0, ch1, sizeof(*ch0)) &&
>   (ch0->s_info.size == 0 ||
> -  (ch0->l_info.size == ch0->s_info.size &&
> -   ch0->l_info.width == ch0->s_info.width &&
> -   ch0->l_info.ranks == ch0->s_info.ranks)));
> +  !memcmp(>l_info, >s_info, sizeof(ch0->l_info)));

However using memcmp like this gives me the creeps. It's probably going
to work just fine. It's just the knowledge that it's not guaranteed to
work that bugs me.

Oh well.

Reviewed-by: Jani Nikula 

>  }
>  
>  static int
> @@ -1170,16 +1168,16 @@ skl_dram_get_channels_info(struct drm_i915_private 
> *dev_priv)
>  {
>   struct dram_info *dram_info = _priv->dram_info;
>   struct dram_channel_info ch0 = {}, ch1 = {};
> - u32 val_ch0, val_ch1;
> + u32 val;
>   int ret;
>  
> - val_ch0 = I915_READ(SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
> - ret = skl_dram_get_channel_info(, 0, val_ch0);
> + val = I915_READ(SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
> + ret = skl_dram_get_channel_info(, 0, val);
>   if (ret == 0)
>   dram_info->num_channels++;
>  
> - val_ch1 = I915_READ(SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN);
> - ret = skl_dram_get_channel_info(, 1, val_ch1);
> + val = I915_READ(SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN);
> + ret = skl_dram_get_channel_info(, 1, val);
>   if (ret == 0)
>   dram_info->num_channels++;
>  
> @@ -1205,12 +1203,10 @@ skl_dram_get_channels_info(struct drm_i915_private 
> *dev_priv)
>  
>   dram_info->is_16gb_dimm = ch0.is_16gb_dimm || ch1.is_16gb_dimm;
>  
> - dev_priv->dram_info.symmetric_memory = intel_is_dram_symmetric(val_ch0,
> -val_ch1,
> -);
> + dram_info->symmetric_memory = intel_is_dram_symmetric(, );
>  
> - DRM_DEBUG_KMS("memory configuration is %sSymmetric memory\n",
> -   dev_priv->dram_info.symmetric_memory ? "" : "not ");
> + DRM_DEBUG_KMS("Memory configuration is symmetric? %s\n",
> +   yesno(dram_info->symmetric_memory));
>   return 0;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 08/12] drm/i915: Generalize intel_is_dram_symmetric()

2019-02-25 Thread Ville Syrjala
From: Ville Syrjälä 

Decouple intel_is_dram_symmetric() from the raw register values
by comparing just the dram_channel_info structs.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_drv.c | 28 
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3d6a08e907e3..9261bd0dccd6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1155,14 +1155,12 @@ skl_dram_get_channel_info(struct dram_channel_info *ch,
 }
 
 static bool
-intel_is_dram_symmetric(u32 val_ch0, u32 val_ch1,
-   struct dram_channel_info *ch0)
+intel_is_dram_symmetric(const struct dram_channel_info *ch0,
+   const struct dram_channel_info *ch1)
 {
-   return (val_ch0 == val_ch1 &&
+   return !memcmp(ch0, ch1, sizeof(*ch0)) &&
(ch0->s_info.size == 0 ||
-(ch0->l_info.size == ch0->s_info.size &&
- ch0->l_info.width == ch0->s_info.width &&
- ch0->l_info.ranks == ch0->s_info.ranks)));
+!memcmp(>l_info, >s_info, sizeof(ch0->l_info)));
 }
 
 static int
@@ -1170,16 +1168,16 @@ skl_dram_get_channels_info(struct drm_i915_private 
*dev_priv)
 {
struct dram_info *dram_info = _priv->dram_info;
struct dram_channel_info ch0 = {}, ch1 = {};
-   u32 val_ch0, val_ch1;
+   u32 val;
int ret;
 
-   val_ch0 = I915_READ(SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
-   ret = skl_dram_get_channel_info(, 0, val_ch0);
+   val = I915_READ(SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
+   ret = skl_dram_get_channel_info(, 0, val);
if (ret == 0)
dram_info->num_channels++;
 
-   val_ch1 = I915_READ(SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN);
-   ret = skl_dram_get_channel_info(, 1, val_ch1);
+   val = I915_READ(SKL_MAD_DIMM_CH1_0_0_0_MCHBAR_MCMAIN);
+   ret = skl_dram_get_channel_info(, 1, val);
if (ret == 0)
dram_info->num_channels++;
 
@@ -1205,12 +1203,10 @@ skl_dram_get_channels_info(struct drm_i915_private 
*dev_priv)
 
dram_info->is_16gb_dimm = ch0.is_16gb_dimm || ch1.is_16gb_dimm;
 
-   dev_priv->dram_info.symmetric_memory = intel_is_dram_symmetric(val_ch0,
-  val_ch1,
-  );
+   dram_info->symmetric_memory = intel_is_dram_symmetric(, );
 
-   DRM_DEBUG_KMS("memory configuration is %sSymmetric memory\n",
- dev_priv->dram_info.symmetric_memory ? "" : "not ");
+   DRM_DEBUG_KMS("Memory configuration is symmetric? %s\n",
+ yesno(dram_info->symmetric_memory));
return 0;
 }
 
-- 
2.19.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx