Re: [PATCH 06/17] drm/xe/oa/uapi: Add/remove OA config perf ops

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

On Tue, 19 Dec 2023 11:10:22 -0800, Umesh Nerlige Ramappa wrote:
>

Hi Umesh,

> > +static bool xe_oa_reg_in_range_table(u32 addr, const struct xe_mmio_range *table)
> > +{
> > +	while (table->start || table->end) {
>
> nit: why not start && end? I would expect both start and end defined for a
> range.

Done (it was carry over from i915).

Note that the plan here is to clean up these checks (remove separate
mux/flex/b_c counter register checks) eventually, as discussed in a
previous review. I am tracking this task but haven't got round to it
yet. We can do this cleanup later since it doesn't affect uapi.

> > +	/* Config id 0 is invalid, id 1 for kernel stored test config */
>
> kernel doesn't store a test config anymore, so the comment can be
> updated. You can start with 1 below though, but that's up to you.
>
> > +	oa_config->id = idr_alloc(&oa->metrics_idr, oa_config, 2, 0, GFP_KERNEL);
> > +	if (oa_config->id < 0) {
> > +		drm_dbg(&oa->xe->drm, "Failed to create sysfs entry for OA config\n");
> > +		err = oa_config->id;
> > +		goto sysfs_err;
> > +	}

Done, dropped comment and starting with 1 now.

> minor nits above, otherwise lgtm,
>
> Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>

Thanks.
--
Ashutosh