Re: [PATCH 2/3] drm/i915: Decode system memory bandwidth

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em Qua, 2017-02-15 às 20:18 +0530, Mahesh Kumar escreveu:
> This patch adds support to decode system memory bandwidth
> which will be used for arbitrated display memory percentage
> calculation in GEN9 based system.
> 
> Changes from v1:
>  - Address comments from Paulo
>  - implement decode function for SKL/KBL also
> Changes from v2:
>  - Rewrite the code as per HW team inputs
>  - Addresses review comments
> Changes from v3:
>  - Fix compilation warning
> Changes from v4:
>  - Address review comments from Paulo
>  - Round-off the frequency & bandwidth results (Paulo)
> 
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> ---
> 

<snip>

> +
> +static void
> +intel_get_memdev_info(struct drm_i915_private *dev_priv)
> +{
> +	struct memdev_info *memdev_info = &dev_priv->memdev_info;
> +	int ret;
> +
> +	memdev_info->valid = false;
> +	memdev_info->rank = I915_DRAM_RANK_INVALID;
> +	memdev_info->num_channels = 0;
> +
> +	if (!IS_GEN9(dev_priv))
> +		return;
> +
> +	if (IS_BROXTON(dev_priv))
> +		ret = bxt_get_memdev_info(dev_priv);
> +	else
> +		ret = skl_get_memdev_info(dev_priv);
> +	if (ret)
> +		return;

Quick question: we now have GLK and it's going to call
skl_get_memdev_info(). Is this intentional?

> +
> +	DRM_DEBUG_DRIVER("DRAM bandwidth: %u KBps, total-channels:
> %u\n",
> +				memdev_info->bandwidth_kbps,
> +				memdev_info->num_channels);
> +	DRM_DEBUG_DRIVER("DRAM rank: %s rank\n",
> +				(memdev_info->rank ==
> I915_DRAM_RANK_DUAL) ?
> +						"dual" : "single");
> +}
> +

Besides, I see BAT pointed a bunch of WARNs for this series, so I'll
wait until BAT gives use the approval stamp before I continue this
review, since it's likely that I'll end up having to re-review once the
WARNs get fixed.

Thanks,
Paulo

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