Re: [PATCH v2] drm/amd/display: add an ASSERT() to irq service functions

From: Harry Wentland
Date: Mon Oct 17 2022 - 11:57:59 EST




On 2022-10-17 11:38, 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, for instance we
> have had reports of this condition being triggered when removing a
> secondary display that is setup in mirror mode and is connected using
> usb-c. So, to have access to more detailed debugging information, add an
> ASSERT() to dal_irq_service_ack() and dal_irq_service_set() that only
> triggers when we encounter an unimplemented function.
>
> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@xxxxxxx>
> ---
> v2: detail specific instance that I'm interested in and use ASSERT()
> instead of WARN().
> ---
> .../gpu/drm/amd/display/dc/irq/irq_service.c | 17 +++++++++++++++--
> 1 file changed, 15 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..3d6ab4fd25cb 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,15 @@ bool dal_irq_service_set(
>
> dal_irq_service_ack(irq_service, source);
>
> - if (info->funcs && info->funcs->set)
> + if (info->funcs && info->funcs->set) {
> + if (info->funcs->set == dal_irq_service_dummy_set)
> + DC_LOG_WARNING("%s: src: %d, st: %d\n", __func__,
> + source, enable);
> +
> + ASSERT(info->funcs->set != dal_irq_service_dummy_set);

I think you'll want the ASSERT inside the if block.

Harry

> +
> return info->funcs->set(irq_service, info, enable);
> + }
>
> dal_irq_service_set_generic(irq_service, info, enable);
>
> @@ -146,8 +153,14 @@ bool dal_irq_service_ack(
> return false;
> }
>
> - if (info->funcs && info->funcs->ack)
> + if (info->funcs && info->funcs->ack) {
> + if (info->funcs->ack == dal_irq_service_dummy_ack)
> + DC_LOG_WARNING("%s: src: %d\n", __func__, source);
> +
> + ASSERT(info->funcs->ack != dal_irq_service_dummy_ack);
> +
> return info->funcs->ack(irq_service, info);
> + }
>
> dal_irq_service_ack_generic(irq_service, info);
>