Re: [PATCH 1/3] drm/xe/xe_oa: Clear status only if relevant bits are set

On Thu, 06 Aug 2026 15:47:04 -0700, Umesh Nerlige Ramappa wrote:
>
> OA monitors only a few bits in the status field. The current
> read-modify-write will write to the MMIO even if none of the relevant
> bits are set. Clear the status only if relevant bits are set.
>
> Signed-off-by: Umesh Nerlige Ramappa <[email protected]>
> ---
>  drivers/gpu/drm/xe/xe_oa.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c
> index cb526de0ef87..183956170757 100644
> --- a/drivers/gpu/drm/xe/xe_oa.c
> +++ b/drivers/gpu/drm/xe/xe_oa.c
> @@ -543,14 +543,16 @@ static int __xe_oa_read(struct xe_oa_stream *stream, char __user *buf,
>			size_t count, size_t *offset)
>  {
>	/* Only clear our bits to avoid side-effects */
> -	stream->oa_status = xe_mmio_rmw32(&stream->gt->mmio, __oa_regs(stream)->oa_status,
> -					  OASTATUS_RELEVANT_BITS, 0);
> +	stream->oa_status = xe_mmio_read32(&stream->gt->mmio, __oa_regs(stream)->oa_status);
>	/*
>	 * Signal to userspace that there is non-zero OA status to read via
>	 * @DRM_XE_OBSERVATION_IOCTL_STATUS observation stream fd ioctl
>	 */
> -	if (stream->oa_status & OASTATUS_RELEVANT_BITS)
> +	if (stream->oa_status & OASTATUS_RELEVANT_BITS) {
> +		xe_mmio_write32(&stream->gt->mmio, __oa_regs(stream)->oa_status,
> +				stream->oa_status & ~OASTATUS_RELEVANT_BITS);

Hmm, because pcie reads are expensive, and pcie writes are posted writes,
how much difference is this really going to make?

Anyway, if you still want to merge this, move the comment /* Only clear our
bits to avoid side-effects */ above this line. With that, this is:

Reviewed-by: Ashutosh Dixit <[email protected]>

>		return -EIO;
> +	}
>
>	return xe_oa_append_reports(stream, buf, count, offset);
>  }
> --
> 2.51.0
>