Re: [PATCH 2/6] drm/i915/debugfs: reuse max slice/subslices already stored in sseu

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 18/12/2017 15:35, Lionel Landwerlin wrote:
> Now that we have that information in topology fields, let's just reused it.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c | 26 ++++++++++----------------
>   1 file changed, 10 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 0c7890b695c5..6ec7543e698f 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4308,11 +4308,11 @@ static void gen10_sseu_device_status(struct drm_i915_private *dev_priv,
>   				     struct sseu_dev_info *sseu)
>   {
>   	const struct intel_device_info *info = INTEL_INFO(dev_priv);
> -	int s_max = 6, ss_max = 4;
>   	int s, ss;
> -	u32 s_reg[s_max], eu_reg[2 * s_max], eu_mask[2];
> +	u32 s_reg[info->sseu.max_slices],
> +		eu_reg[2 * info->sseu.max_subslices], eu_mask[2];

This is a bit unusual style, perhaps split into separate declarations.

>   
> -	for (s = 0; s < s_max; s++) {
> +	for (s = 0; s < info->sseu.max_slices; s++) {
>   		/*
>   		 * FIXME: Valid SS Mask respects the spec and read
>   		 * only valid bits for those registers, excluding reserverd
> @@ -4334,7 +4334,7 @@ static void gen10_sseu_device_status(struct drm_i915_private *dev_priv,
>   		     GEN9_PGCTL_SSB_EU210_ACK |
>   		     GEN9_PGCTL_SSB_EU311_ACK;
>   
> -	for (s = 0; s < s_max; s++) {
> +	for (s = 0; s < info->sseu.max_slices; s++) {
>   		if ((s_reg[s] & GEN9_PGCTL_SLICE_ACK) == 0)
>   			/* skip disabled slice */
>   			continue;
> @@ -4342,7 +4342,7 @@ static void gen10_sseu_device_status(struct drm_i915_private *dev_priv,
>   		sseu->slice_mask |= BIT(s);
>   		sseu->subslices_mask[s] = info->sseu.subslices_mask[s];
>   
> -		for (ss = 0; ss < ss_max; ss++) {
> +		for (ss = 0; ss < info->sseu.max_subslices; ss++) {
>   			unsigned int eu_cnt;
>   
>   			if (!(s_reg[s] & (GEN9_PGCTL_SS_ACK(ss))))
> @@ -4362,17 +4362,11 @@ static void gen10_sseu_device_status(struct drm_i915_private *dev_priv,
>   static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
>   				    struct sseu_dev_info *sseu)
>   {
> -	int s_max = 3, ss_max = 4;
> +	const struct intel_device_info *info = INTEL_INFO(dev_priv);
>   	int s, ss;
> -	u32 s_reg[s_max], eu_reg[2*s_max], eu_mask[2];
> -
> -	/* BXT has a single slice and at most 3 subslices. */
> -	if (IS_GEN9_LP(dev_priv)) {
> -		s_max = 1;
> -		ss_max = 3;
> -	}
> +	u32 s_reg[info->sseu.max_slices], eu_reg[2*info->sseu.max_subslices], eu_mask[2];

Spaces around operators are preferred.

>   
> -	for (s = 0; s < s_max; s++) {
> +	for (s = 0; s < info->sseu.max_slices; s++) {
>   		s_reg[s] = I915_READ(GEN9_SLICE_PGCTL_ACK(s));
>   		eu_reg[2*s] = I915_READ(GEN9_SS01_EU_PGCTL_ACK(s));
>   		eu_reg[2*s + 1] = I915_READ(GEN9_SS23_EU_PGCTL_ACK(s));
> @@ -4387,7 +4381,7 @@ static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
>   		     GEN9_PGCTL_SSB_EU210_ACK |
>   		     GEN9_PGCTL_SSB_EU311_ACK;
>   
> -	for (s = 0; s < s_max; s++) {
> +	for (s = 0; s < info->sseu.max_slices; s++) {
>   		if ((s_reg[s] & GEN9_PGCTL_SLICE_ACK) == 0)
>   			/* skip disabled slice */
>   			continue;
> @@ -4398,7 +4392,7 @@ static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
>   			sseu->subslices_mask[s] =
>   				INTEL_INFO(dev_priv)->sseu.subslices_mask[s];
>   
> -		for (ss = 0; ss < ss_max; ss++) {
> +		for (ss = 0; ss < info->sseu.max_subslices; ss++) {
>   			unsigned int eu_cnt;
>   
>   			if (IS_GEN9_LP(dev_priv)) {
> 

With the formatting tweaks,

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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