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

From: Harry Wentland
Date: Mon Oct 17 2022 - 14:49:13 EST


On 2022-10-17 12:13, 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>

Reviewed-by: Harry Wentland <harry.wentland@xxxxxxx>

Harry

> ---
> v2: detail specific instance that I'm interested in and use ASSERT()
> instead of WARN().
>
> v3: move ASSERT()s inside the new if blocks.
> ---
> .../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..d100edaedbbb 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(0);
> + }
> +
> 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(0);
> + }
> +
> return info->funcs->ack(irq_service, info);
> + }
>
> dal_irq_service_ack_generic(irq_service, info);
>