Re: [PATCH i-g-t 3/3] tests/intel/xe_survivability: Reload audio driver on exit

["Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>]

On 10/28/2026 1:43 PM, Dnyaneshwar Bhadane wrote:
> 
> 
> On 27-Oct-26 4:25 PM, Borah, Chaitanya Kumar wrote:
>>
>>
>> On 10/23/2026 10:58 AM, Bhadane, Dnyaneshwar wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Bhadane, Dnyaneshwar
>>>> Sent: Friday, October 23, 2026 10:32 AM
>>>> To: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>; igt-
>>>> dev@lists.freedesktop.org
>>>> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; 
>>>> Anirban, Sk
>>>> <sk.anirban@intel.com>; Grzelak, Michal <michal.grzelak@intel.com>; 
>>>> B, Jeevan
>>>> <Jeevan.B@intel.com>; Sharma, Swati2 <swati2.sharma@intel.com>
>>>> Subject: RE: [PATCH i-g-t 3/3] tests/intel/xe_survivability: Reload 
>>>> audio driver on
>>>> exit
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of
>>>>> Chaitanya Kumar Borah
>>>>> Sent: Thursday, October 15, 2026 7:31 PM
>>>>> To: igt-dev@lists.freedesktop.org
>>>>> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; Anirban,
>>>>> Sk <sk.anirban@intel.com>; Grzelak, Michal <michal.grzelak@intel.com>;
>>>>> B, Jeevan <jeevan.b@intel.com>; Sharma, Swati2
>>>>> <swati2.sharma@intel.com>
>>>>> Subject: [PATCH i-g-t 3/3] tests/intel/xe_survivability: Reload audio
>>>>> driver on exit
>>>>>
>>>>> Commit 117e9ecbdd1d ("xe/igt_kmod_unbind: release audio before
>>>>> unbinding
>>>>> xe") made igt_audio_driver_unload() be called before unbinding xe to
>>>>> avoid the audio-component bind warnings/errors that happen if xe is
>>>>> unbound while snd_hda_intel is still attached. Both call sites in this
>>>>> file passed NULL for the unloaded driver name, so it was never
>>>>> tracked, and nothing ever reloaded it afterwards.
>>>>>
>>>>> As a result snd_hda_intel/snd_hdmi_lpe_audio is left unloaded for the
>>>>> rest of the boot even when every subtest passes, since xe always ends
>>>>> up rebound by
>>>>> restore() but audio never does.
>>>>>
>>>>> Capture the unloaded driver name and reload it wherever xe gets bound
>>>>> again in restore().
>>>>>
>>>>> Cc: Sk Anirban <sk.anirban@intel.com>
>>>>> Cc: Michał Grzelak <michal.grzelak@intel.com>
>>>>> Cc: Jeevan B <jeevan.b@intel.com>
>>>>> Cc: Swati Sharma <swati2.sharma@intel.com>
>>>>> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>>>>> ---
>>>>>   tests/intel/xe_survivability.c | 11 +++++++++--
>>>>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/tests/intel/xe_survivability.c
>>>>> b/tests/intel/xe_survivability.c index
>>>>> bae9eca27..2be8e9ec0 100644
>>>>> --- a/tests/intel/xe_survivability.c
>>>>> +++ b/tests/intel/xe_survivability.c
>>>>> @@ -39,6 +39,7 @@
>>>>>    */
>>>>>
>>>>>   static char bus_addr[NAME_MAX];
>>>>> +static char *snd_driver;
>>>>>
>>>>>   static void ignore_wedged_in_dmesg(void)  { @@ -158,7 +159,7 @@
>>>>> static void restore(int sig)  {
>>>>>       int configfs_fd;
>>>>>
>>>>> -    igt_audio_driver_unload(NULL);
>>>>> +    igt_audio_driver_unload(&snd_driver);
>>>>>       igt_kmod_unbind("xe", bus_addr);
>>>>>
>>>>>       configfs_fd = igt_configfs_open("xe"); @@ -167,11 +168,17 @@ 
>>>>> static
>>>>> void restore(int sig)
>>>>>       close(configfs_fd);
>>>>>
>>>>>       igt_kmod_bind("xe", bus_addr);
>>>>> +
>>>>> +    if (snd_driver) {
>>>>> +        igt_kmod_load(snd_driver, NULL);
>>>> Hello Chaitanya,
>>>>
>>>> Would it be possible to have a helper like igt_audio_driver_load(), 
>>>> so we don't
>>>> have to maintain the driver's name in multiple files ?
>>> Something like default to "snd_hda_intel" if passed with NULL.
>>> int igt_audio_driver_load(const char *who)
>>> {
>>>     const char *drv = who ? who : "snd_hda_intel";
>>>     int ret;
>>>
>>>     if (igt_kmod_is_loaded(drv))
>>>         return 0;
>>>
>>>     ret = igt_kmod_load(drv, NULL);
>>>     if (ret)
>>>         igt_debug("Could not load audio driver %s (%s)\n",
>>>               drv, strerror(-ret));
>>>
>>>     return ret;
>>> }
>>
>> I would prefer keeping the exposure of the change to per file/test. I 
>> think "load back what I unloaded" is a good semantics to have instead 
>> of maintaining the driver name globally somewhere.
>>
>> But I would agree that there is a scope for a helper which can help 
>> get rid of the repeated if (snd_driver) {...} code block.
>>
>> I am thinking of something like.
>>
>> int igt_audio_driver_load(char **who)
>> {
>>      int ret;
>>
>>      if (!who || !*who)
>>          return 0;
>>
>>      ret = igt_kmod_load(*who, NULL);
>>      if (ret)
>>          igt_warn("Could not load audio driver %s (%s)\n", *who, 
>> strerror(-ret));
>>
>>      free(*who);
> Hi,
> Unwanted behavior if the order is igt_audio_driver_load(snd_driver) -> 
> igt_audio_driver_unload(snd_driver).  Freeing this will loose the audio 
> driver name.  Let testcase decide when they want to free this 
> 'snd_driver' mostly on testcase cleanup on exist.
> 
> 

Good catch. The memory handling of the string should be left to the 
caller. Will fix it in the next revision.

> 
> Dnyaneshwar,
> 
>  >      *who = NULL;
>>
>>      return ret;
>> }
>>
>> ==
>> Chaitanya
>>
>>
>>>
>>> Br,
>>> Dnyaneshwar,
>>>>
>>>> Br,
>>>> Dnyaneshwar
>>>>> +        free(snd_driver);
>>>>> +        snd_driver = NULL;
>>>>> +    }
>>>>>   }
>>>>>
>>>>>   static void set_survivability_mode(int configfs_device_fd, bool 
>>>>> value)  {
>>>>> -    igt_audio_driver_unload(NULL);
>>>>> +    igt_audio_driver_unload(&snd_driver);
>>>>>       igt_kmod_unbind("xe", bus_addr);
>>>>>       igt_sysfs_set_boolean(configfs_device_fd, 
>>>>> "survivability_mode", value);
>>>>>       igt_kmod_bind("xe", bus_addr);
>>>>> -- 
>>>>> 2.50.1
>>>
>>
>