Re: [Intel-gfx] [PATCH 3/9] drm/i915/perf: Validate OA sseu config outside switch

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Tue, 14 Feb 2023 16:54:13 -0800, Umesh Nerlige Ramappa wrote:
>
> Validate the OA sseu config after all params are parsed.

Commit messages for all patches need to answer the "why" or the reason for
the patch. In this case maybe an overkill but probably something like:

Validate the OA sseu config after all params are parsed since the engine
can be passed in as part of perf properties.

>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_perf.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index a879ae4bf8d7..0b2097ad000e 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -3956,6 +3956,8 @@ static int read_properties_unlocked(struct i915_perf *perf,
>	u64 __user *uprop = uprops;
>	u32 i;
>	int ret;
> +	bool config_sseu = false;
> +	struct drm_i915_gem_context_param_sseu user_sseu;

nit: longer lines above shorter lines

>
>	memset(props, 0, sizeof(struct perf_open_properties));
>	props->poll_oa_period = DEFAULT_POLL_PERIOD_NS;
> @@ -4082,8 +4084,6 @@ static int read_properties_unlocked(struct i915_perf *perf,
>			props->hold_preemption = !!value;
>			break;
>		case DRM_I915_PERF_PROP_GLOBAL_SSEU: {
> -			struct drm_i915_gem_context_param_sseu user_sseu;
> -
>			if (GRAPHICS_VER_FULL(perf->i915) >= IP_VER(12, 50)) {
>				drm_dbg(&perf->i915->drm,
>					"SSEU config not supported on gfx %x\n",
> @@ -4098,14 +4098,7 @@ static int read_properties_unlocked(struct i915_perf *perf,
>					"Unable to copy global sseu parameter\n");
>				return -EFAULT;
>			}
> -
> -			ret = get_sseu_config(&props->sseu, props->engine, &user_sseu);
> -			if (ret) {
> -				drm_dbg(&perf->i915->drm,
> -					"Invalid SSEU configuration\n");
> -				return ret;
> -			}
> -			props->has_sseu = true;
> +			config_sseu = true;
>			break;
>		}
>		case DRM_I915_PERF_PROP_POLL_OA_PERIOD:
> @@ -4125,6 +4118,15 @@ static int read_properties_unlocked(struct i915_perf *perf,
>		uprop += 2;
>	}
>
> +	if (config_sseu) {
> +		ret = get_sseu_config(&props->sseu, props->engine, &user_sseu);
> +		if (ret) {
> +			DRM_DEBUG("Invalid SSEU configuration\n");

drm_dbg? DRM_DEBUG is deprecated?

> +			return ret;
> +		}
> +		props->has_sseu = true;
> +	}
> +
>	return 0;
>  }


After addressing the above comments:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>