Re: [PATCH] drm/amd/display: add a WARN() to irq service functions

[Hamza Mahfooz]

On 2022-10-17 09:06, Rodrigo Siqueira wrote:
> Hi Hamza,
>
> On 10/14/22 11:31, Hamza Mahfooz wrote:
> > Currently, if we encounter unimplemented functions, it is difficult to
> > tell what caused them just by looking at dmesg and that is compounded by
> > the fact that it is often hard to reproduce said issues. So, to have
> > access to more detailed debugging information, add a WARN() to
> > dal_irq_service_ack() and dal_irq_service_set() that only triggers when
> > we encounter an unimplemented function.
>
> Do you know the specific issue that triggered this unimplemented 
> function? It might be useful to describe the situation in the commit 
> message where you see this problem.

Ya, I'll do that in v2.

> > Signed-off-by: Hamza Mahfooz <hamza.mahfooz@xxxxxxx>
> > ---
> >   drivers/gpu/drm/amd/display/dc/irq/irq_service.c | 10 ++++++++--
> >   1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/irq/irq_service.c 
> > b/drivers/gpu/drm/amd/display/dc/irq/irq_service.c
> > index 7bad39bba86b..b895bdd8dc55 100644
> > --- a/drivers/gpu/drm/amd/display/dc/irq/irq_service.c
> > +++ b/drivers/gpu/drm/amd/display/dc/irq/irq_service.c
> > @@ -112,8 +112,11 @@ bool dal_irq_service_set(
> >       dal_irq_service_ack(irq_service, source);
> > -    if (info->funcs && info->funcs->set)
> > +    if (info->funcs && info->funcs->set) {
> > +        WARN(info->funcs->set == dal_irq_service_dummy_set,
> > +             "%s: src: %d, st: %d\n", __func__, source, enable);
> >           return info->funcs->set(irq_service, info, enable);
>
> Do you know if we may hit this condition multiple times?

Yes, it is possible that it will be hit multiple times from different 
callers.

> > +    }
> >       dal_irq_service_set_generic(irq_service, info, enable);
> > @@ -146,8 +149,11 @@ bool dal_irq_service_ack(
> >           return false;
> >       }
> > -    if (info->funcs && info->funcs->ack)
> > +    if (info->funcs && info->funcs->ack) {
> > +        WARN(info->funcs->ack == dal_irq_service_dummy_ack,
> > +             "%s: src: %d\n", __func__, source);
> >           return info->funcs->ack(irq_service, info);
> > +    }
> >       dal_irq_service_ack_generic(irq_service, info);
>
> Just for curiosity, did you run some IGT tests?

No, this was encountered during manual testing.

> Thanks
> Siqueira

--
Hamza