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
