Re: [PATCH] drm/xe/oa: Allow reading after disabling OA stream

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

On Thu, 12 Mar 2026 05:25:58 -0700, Umesh Nerlige Ramappa wrote:
>
> On Wed, Mar 11, 2026 at 11:52:38PM -0700, Dixit, Ashutosh wrote:
> > On Wed, 11 Mar 2026 21:24:14 -0700, Umesh Nerlige Ramappa wrote:
> >>
> >
> > Hi Umesh,
> >
> >> On Wed, Mar 11, 2026 at 02:01:07PM -0700, Ashutosh Dixit wrote:
> >> > Some OA data might be present in the OA buffer when OA stream is
> >> > disabled. Allow UMD's to retrieve this data, so that all data till the
> >> > point when OA stream is disabled can be retrieved.
> >> >
> >> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> >> > ---
> >> > drivers/gpu/drm/xe/xe_oa.c | 3 +--
> >> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c
> >> > index 72fc4424017bf..f840827960eab 100644
> >> > --- a/drivers/gpu/drm/xe/xe_oa.c
> >> > +++ b/drivers/gpu/drm/xe/xe_oa.c
> >> > @@ -543,8 +543,7 @@ static ssize_t xe_oa_read(struct file *file, char __user *buf,
> >> >	size_t offset = 0;
> >> >	int ret;
> >> >
> >> > -	/* Can't read from disabled streams */
> >> > -	if (!stream->enabled || !stream->sample)
> >> > +	if (!stream->sample)
> >> >		return -EINVAL;
> >>
> >> What if there is no data to be read when the stream was disabled? In polled
> >> mode, we are probably fine, but in blocking mode, we may just wait forever
> >> here.
> >
> > Yes, correct. There is no problem in non-blocking read() (it will return
> > -EAGAIN if no data). But poll() or blocking read() can block.
> >
> > However, as far as I see, the responsibility of unblocking the blocked
> > thread (e.g. by means of a signal) rests with UMD.
> >
> > Note: blocking read() cannot return 0 (it can, only on EOF, see 'man 2
> > read' but I don't want to do it because "what is EOF"). Similarly poll()
> > cannot return if there's no data, UMD will need to specify a timeout in
> > poll().
> >
> > Note, KMD release() method will not get called if UMD thread is blocked in
> > read(). So stream cannot be closed till the blocked thread is unblocked.
> >
> > I would imagine UMD will use non-blocking read()'s to retrieve data after
> > stream is disabled, till non-blocking read() returns -EAGAIN. If they don't
> > use non-blocking read(), they will have to handle the blocked thread using
> > one of the methods above. There's nothing KMD can do.
> >
> > Unless you or Robert (Cc'd) can prove otherwise :-)
>
> I guess you can argue both ways. I would think if the stream is disabled,
> there is really no meaning in supporting the blocked mode, but then blocked
> mode itself means that caller is blocked until data is available, so I
> guess it's okay to leave it as is.

Yes, today we never return 0 from read(). We either return a +ve number
(num bytes read) or a -ve error. Or we block. So I strongly prefer not
changing this behavior. Some UMD might be checking for >= 0, some only > 0,
so if we change the 0 behavior we might also start breaking UMD/tests.

> I noticed another thing - When you disable a stream, you also end up
> disabling the hrtimer. Then who updates the software tail such that all the
> remaining data is read? Note that the poll only waits on the poll_wq. It
> does not check if new reports are available.

Yeah, thanks for catching this, I think we need to do something here. Let
me look some more into this and update the patch in the next couple of
days.

> >
> >> >
> >> >	if (!(file->f_flags & O_NONBLOCK)) {
> >> > --
> >> > 2.48.1
> >> >