Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <5aa210ec-2252-1709-70b7-f1421ce27440@amd.com>
Date:   Mon, 17 Oct 2022 09:58:41 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.2
Subject: Re: [PATCH] drm/amd/display: add a WARN() to irq service functions
To:     Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc:     Harry Wentland <harry.wentland@amd.com>,
        Leo Li <sunpeng.li@amd.com>,
        Alex Deucher <alexander.deucher@amd.com>,
        =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>,
        "Pan, Xinhui" <Xinhui.Pan@amd.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        Mario Limonciello <mario.limonciello@amd.com>,
        Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>,
        Alex Hung <alex.hung@amd.com>, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org, amd-gfx@lists.freedesktop.org
References: <20221014153129.68674-1-hamza.mahfooz@amd.com>
 <ec75ba21-97e0-9479-8d87-46cc8032d44c@amd.com>
Content-Language: en-US
From:   Hamza Mahfooz <hamza.mahfooz@amd.com>
In-Reply-To: <ec75ba21-97e0-9479-8d87-46cc8032d44c@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dWdWLy9PWEZjOWlHV3ZBeEpUb1pnYURyc3BYbUZmeHZPT29oVjRKc1c4alFy?=
 =?utf-8?B?NUpIZi9nTi93RjJTd1BlcVFkeTVXZlRmT3dHb1Z0WUR4VDlvRFgyd2Y5OWwr?=
 =?utf-8?B?TC84b0VkM3doT1d2U1BXZlZUa1ZBV3hLOXNZSFlRcmp1T29QeGw2NmY4TEVl?=
 =?utf-8?B?Q2hCZVlBQ2UzT1Mvb0tPSlVpT2lzVS9FdDQ0T3lVMlZIb0thTDg0MGdQWmJG?=
 =?utf-8?B?eTJFalJOZW5INDA1N3JlSWhrdTVIRjV2OWE2YzQza3FyQzFYQXpnbmw3N2dD?=
 =?utf-8?B?Vml3OVFZMTd0WDBkV3FYVDZVcy9xNW5jTFo5YjZaSlFOdWVjZ0ZUMjRYYUtV?=
 =?utf-8?B?L0hVMXNPeEN6Z1pqcDZ1YjFpRFZWNE11b2RBMXJzZ2NFUzVFbUdueEQvcklO?=
 =?utf-8?B?ekxQN2hNQkRkNEJRNmtYa1JaV0dHUjdWRWlReXFIRTJ5RncwWXNjdHExYzlX?=
 =?utf-8?B?ZXVxWDlkQ1c4THdPaGk1ZTJ6QlBydUlYNU1RSzZnSlNmVFJ2WE5YM1dBYTIv?=
 =?utf-8?B?aTlNUmllc255ZUYrbWhVakhqRTFGN2E2azJLOU56aW80OUZNRGJFS0hBUFg4?=
 =?utf-8?B?bjFBWFBiRWVteElOaHhZeE04alF2ZDg2SzU0R2lxbzdWWm1Xd1dUWTl0cnh2?=
 =?utf-8?B?S2RqY2p6Z3M5SlVvYXV3YUxzZnlNSG5nbk9wNUFSL3FOWVRvSkpPa1dTcUc2?=
 =?utf-8?B?M0FCL2Q3RE8yQlJKUzlqZWhwNUtIa3ZEem9PMC9kanVPSU1HTG5oNlZNNHdr?=
 =?utf-8?B?S0MrcXlocGtoN01FU3hQakQ2TXd4QzBQN2pYekp4RDJkaEhvOFAwY1Q3VC9y?=
 =?utf-8?B?UHR2RzhTUHhMRURRN0lMa3JqWWo4cWphNEthK3ZqVmtHNk1ybkhwMGZscE1x?=
 =?utf-8?B?bVBaeHRNMmNKK21Qdy9ZSE1lb1VrWndLb1V3TVFQelRybXdZUUJaZWVkYytj?=
 =?utf-8?B?OHBnbnNsekpWblNleUxLQjJ5NjlzaEpQWTZLdWtHZ3Z6U3M5Sk9mTFB6OEl2?=
 =?utf-8?B?K0RsOVdla2g3ZHNmbGRYQ24weFVLVitYRkxkYmxIenBCY3g0ZzdxdzBEdXBr?=
 =?utf-8?B?UEtIQStjZ2VMSFcyOFo0dHY2a2ozbnBMaWV3amZNR2pjbWZWSGo1Rm5Kc09I?=
 =?utf-8?B?ZEJ3djVKeE1zUVIydHJueTJuUnZyai9IVnB3VERjUU04c0tEL3k0UXVYZ1RF?=
 =?utf-8?B?SkFGR1AyL0wzWW50eGFMNjQ3TEFSdmJhak9zL3JPR3EvU3Z1am9XUS8valdJ?=
 =?utf-8?B?eVV3aWhUMUZvS0w1UWRaMXk2QWw4Qkw3NW5DUTJiUmZKSlYwTHcyNDkwNVRD?=
 =?utf-8?B?cDhTQm43WTl5NmJaclRJMkRhRXFXR1hWb25YdzV2Mmd4T05UNzFGNFAwVDBJ?=
 =?utf-8?B?dUhBelpnOE1VOGhCZytvYXZSNzRPaG5KNTVBNjgydDYwS2c2dkVDUG8xWkVT?=
 =?utf-8?B?Z0JVVU5FeVl0TlVjSVY3WFRUTVRmMFpOaFhFWUt3TGgxWkp0RkN2bDZUOUto?=
 =?utf-8?B?VXAzeGY3M3FDU2FtV2FkL1ZLbXQ5M1ROK0o3aGpXMUE5L0hKeHRsd0tZbGdD?=
 =?utf-8?B?ajdNVTJ1bkJxcTlUV3JkT2NSS2lrRUhCaWlMS0YvNyttZ2toeTZjUEZKdzVX?=
 =?utf-8?B?ZGVsOXpTSXNJWEFwVUY2NFZ6dWdSc0ZIQWlOZ3RvbWFycGVVZDVsMVc5Umwz?=
 =?utf-8?B?T1FrYU1xbG4zejFmNStycHBoeHZLTldGSDAvVndZQTdJc2tNSDN6KzhIQ21N?=
 =?utf-8?B?L3pkK0dMazdkZGxETXo1M1dFU2NiRlZKYitMbFg2bUZyMm01ejR1L3M5cGxT?=
 =?utf-8?B?TTZ5RTR0ZnlLUkt0UnJ4ZlhCMnpLVHBkNThmMmFNUnlWSjVxak1KQW1OQlda?=
 =?utf-8?B?WkliZmZtTEl6emozUS8xK0pURlhzQVNqZTVHZjJML2htWFNCcDhRMkpvYkxQ?=
 =?utf-8?B?TUpRZ05NcksvSHFJN1g0QWdDWFVZVnp6RmxpYVp4NVZ5a3BVSkdNT0ZUT29r?=
 =?utf-8?B?YkRiVzUxL3RrV0hvMlpIbEp6UmkrcmxyMWdnSEgwc2w4YWVFeUtXOTNuaE1p?=
 =?utf-8?B?ZDNVWU1tSXlDb1BJc2pMaHJUcG55ZHhhakI2cjVsVW5KYkRSRFlzSFZkZC91?=
 =?utf-8?Q?BTlLkPYZXqTMs2bd14/MC9Olr?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 313bf977-7fce-4f12-3b07-08dab047a2bf
X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB6280.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Oct 2022 13:58:14.7216
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ti4JPD7N352Y8bZgq0p6ohF5b/6hlSU5vPIGUdhoIvoGQupa7/yJFZs7UI/u/d0gtbJCmN9ychWfeJppLAXmgw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR12MB7357
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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@amd.com>
>> ---
>>   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