[PATCH] drm/i915/gt: Add delay to let engine resumes properly

[Gote, Nitin R]

Hi Rodrigo,

> 
> On Tue, Apr 29, 2025 at 01:16:13PM +0000, Gote, Nitin R wrote:
> > > Hi Nitin,
> > >
> > > [...]
> > >
> > > > > > +		if (wait_for_atomic((!stop_ring(engine) == 0), 20)) {
> > > > > >  			drm_err(&engine->i915->drm,
> > > > > >  				"failed to set %s head to zero "
> > > > > >  				"ctl %08x head %08x tail %08x start
> %08x\n",
> > > > >
> > > > > I am wondering if xcs_resume() calling stop_ring() too would
> > > > > benefit from having this timeout on hand as well. That would
> > > > > require moving
> > > > > wait_for_atomic((!stop_ring(engine) == 0), 20) along with your
> > > > > comment to a separate wrapper function.
> > > > > What do you think?
> > > >
> > > > In xcs_resume(), there is no need for a timeout for stop_ring(),
> > > > as we have not
> > > encountered any issues/errors in xcs_resume().
> > > > So, I think, currently there is no need for a separate wrapper function.
> > > In that case, I do not have any more concerns:
> > > Reviewed-by: Krzysztof Karas <krzysztof.karas at intel.com>
> > >
> > Thanks for the review Krzysztof.
> >
> > > Best Regards,
> > > Krzysztof
> >
> > Hi Rodrigo/Jani,
> > May I ask you to push this change?
> 
> I just pushed the patch, but I'm wondering if a simple
> ENGINE_POSTING_READ(engine, RING_HEAD); ENGINE_POSTING_READ(engine,
> RING_TAIL);
> 
> before the return inside the stop_ring() wouldn't be enough to accomplish the
> same here...
> 

Thank you for pushing the change.
I have already checked using ENGINE_POSTING_READ() inside stop_ring(), but it was not enough.

- Nitin

> >
> > Thanks,
> > Nitin
> >
> >
> >