Re: [PATCH v5] drm/amdgpu: handle IH ring1 overflow

[philip yang <yangp@xxxxxxx>]

On 2021-11-25 2:03 a.m., Christian König wrote:

> Am
>       24.11.21 um 21:20 schrieb Felix Kuehling:
>       
> > Am 2021-11-24 um 10:23 a.m. schrieb philip
> >         yang:
> >         
> > > > >      #define
> > > > >               amdgpu_ih_get_wptr(adev, ih)
> > > > >               
> > > > >               (adev)->irq.ih_funcs->get_wptr((adev), (ih))
> > > > >               
> > > > >                  #define amdgpu_ih_decode_iv(adev, iv) \
> > > > >               
> > > > >                      (adev)->irq.ih_funcs->decode_iv((adev), (ih),
> > > > >               (iv))
> > > > >               
> > > > >               +#define amdgpu_ih_decode_iv_ts(adev, ih, rptr, offset) \
> > > > >               
> > > > >               +   
> > > > >               (WARN_ON_ONCE(!(adev)->irq.ih_funcs->decode_iv_ts) ?
> > > > >               0 : \
> > > > >               
> > > > >             
> > > >             Please drop that WARN_ON_ONCE here.
> > > >             
> > > >             
> > > >           
> > >           Agree, will drop it.
> > >           
> > >           
> > >         
> >         I suggested this. We're assuming that this function will never
> >         be called
> >         
> >         on hardware that doesn't support time stamps, and that all
> >         hardware with
> >         
> >         time stamps will implement the decode_iv_ts function. But it's
> >         good to
> >         
> >         get a log message if that assumption is ever broken, rather than
> >         just
> >         
> >         silently getting wrong results.
> >         
> >       
>       
>       Well exactly that's the point, you won't get wrong results but a
>       NULL pointer exception instead.
>       
>       
>       So we already have a backtrace in the logs.
>       
>

The check will avoid NULL pointer access backtrace, so WARNING backtrace is obvious, useful for future chips to implement decode_iv_ts interface or use the helper function.

Regards,

Philip

>       
>       Regards,
>       
>       Christian.
>       
>       
> >         
> >         Regards,
> >         
> >            Felix
> >         
> >         
> >         
> > > Regards,
> > >           
> > >           
> > >           Philip
> > >           
> > >           
> > >         
> >       
>       
>