Re: [PATCH 5/7] drm/xe/lrc: Remove leftover TODO

["Summers, Stuart" <stuart.summers@intel.com>]

On Wed, 2025-07-09 at 16:24 -0500, Lucas De Marchi wrote:
> On Wed, Jul 09, 2025 at 07:37:02PM +0000, Stuart Summers wrote:
> > On Thu, 2025-07-03 at 15:41 -0700, Lucas De Marchi wrote:
> > > There isn't anything to set for CTX_TIMESTAMP handling in the
> > > empty
> > > LRC, that is set on every LRC init since it should always start
> > > from
> > > 0
> > > rather than the value saved in the image after first submission.
> > > 
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_lrc.c | 2 --
> > >  1 file changed, 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_lrc.c
> > > b/drivers/gpu/drm/xe/xe_lrc.c
> > > index 75f4678cb090a..2c735b3679f86 100644
> > > --- a/drivers/gpu/drm/xe/xe_lrc.c
> > > +++ b/drivers/gpu/drm/xe/xe_lrc.c
> > > @@ -603,8 +603,6 @@ static void set_context_control(u32 *regs,
> > > struct
> > > xe_hw_engine *hwe)
> > >         if (xe_gt_has_indirect_ring_state(hwe->gt))
> > >                 regs[CTX_CONTEXT_CONTROL] |=
> > >                         _MASKED_BIT_ENABLE(CTX_CTRL_INDIRECT_RING
> > > _STA
> > > TE_ENABLE);
> > > -
> > > -       /* TODO: Timestamp */
> > 
> > Seems interesting to add your commit comment here instead of the
> > TODO
> > unless we're already documenting that somewhere else.
> 
> It seems counter productive to add a comment in a random place about
> things that are done elsewhere. If this was "the normal place to do
> it,
> but handled elsewhere", ok. But this is not and it's still here
> because
> it went unnoticed when the timestamp handling was added.

I'm just thinking about implicit zeroing, but what you said here makes
sense too. I agree the documentation would be better elsewhere and not
really tied to the TODO:
Reviewed-by: Stuart Summers <stuart.summers@intel.com>

> 
> Lucas De Marchi