Re: [Intel-gfx] [PATCH v4 04/16] drm/i915/perf: Determine gen12 oa ctx offset at runtime

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Wed, Oct 12, 2022 at 04:46:07PM -0700, Dixit, Ashutosh wrote:
>On Wed, 12 Oct 2022 15:27:27 -0700, Umesh Nerlige Ramappa wrote:
>>
>> +static u32 oa_context_image_offset(struct intel_context *ce, u32 reg)
>> +{
>> +	u32 offset, len = (ce->engine->context_size - PAGE_SIZE) / 4;
>> +	u32 *state = ce->lrc_reg_state;
>> +
>> +	for (offset = 0; offset < len; ) {
>> +		if (IS_MI_LRI_CMD(state[offset])) {
>> +			/*
>> +			 * We expect reg-value pairs in MI_LRI command, so
>> +			 * MI_LRI_LEN() should be even, if not, issue a warning.
>> +			 */
>> +			drm_WARN_ON(&ce->engine->i915->drm,
>> +				    MI_LRI_LEN(state[offset]) & 0x1);
>> +
>> +			if (oa_find_reg_in_lri(state, reg, &offset, len))
>> +				break;
>> +		} else {
>> +			offset++;
>> +		}
>> +	}
>> +
>> +	/*
>> +	 * Note that the the reg value is written to 'offset + 1' location, so
>> +	 * ensure that the offset is less than (len - 1).
>> +	 */
>> +	return offset < len ? offset : U32_MAX;
>
>Should this then be 'offset < len - 1'? It is actually equivalent since
>length is even and we do idx += 2 in oa_find_reg_in_lri so 'offset < len'
>is the same as 'offset < len - 1' but since we mention (len - 1) in the
>comment maybe clearer to use 'offset < len - 1'?

I was experimenting with len - 1, but removed it later. Forgot to remove 
the comment. I will drop the comment.

Thanks,
Umesh