[i-g-t] tests/intel/i915_suspend: Fix WARNs of invalid fd

[Kamil Konieczny]

Hi Gupta,,
On 2024-01-18 at 04:44:59 +0000, Gupta, Anshuman wrote:
> 
> 
> > -----Original Message-----
> > From: Modem, Bhanuprakash <bhanuprakash.modem at intel.com>
> > Sent: Wednesday, January 17, 2024 8:52 PM
> > To: Kamil Konieczny <kamil.konieczny at linux.intel.com>; igt-
> > dev at lists.freedesktop.org; Gupta, Anshuman <anshuman.gupta at intel.com>
> > Subject: Re: [i-g-t] tests/intel/i915_suspend: Fix WARNs of invalid fd
> > 
> > Hi Kamil,
> > 
> > On 17-01-2024 04:42 pm, Kamil Konieczny wrote:
> > > Hi Bhanuprakash,
> > > On 2024-01-09 at 18:48:02 +0530, Bhanuprakash Modem wrote:
> > >> Call drm_close_driver() only if fd >=0, otherwise it'll will throw WARN:
> > >>
> > >> (i915_suspend:7563) drmtest-WARNING: Don't attempt to close
> > >> standard/invalid file descriptor: -1
> > >>
> > >> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
> > >> ---
> > >>   tests/intel/i915_suspend.c | 4 +++-
> > >>   1 file changed, 3 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/tests/intel/i915_suspend.c b/tests/intel/i915_suspend.c
> > >> index c25805584..82cabfa41 100644
> > >> --- a/tests/intel/i915_suspend.c
> > >> +++ b/tests/intel/i915_suspend.c
> > >> @@ -300,7 +300,9 @@ test_suspend_without_i915(int state)
> > >>   		igt_pm_get_d3cold_allowed(card.pci_slot_name,
> > &d3cold_allowed);
> > >>   		igt_pm_set_d3cold_allowed(card.pci_slot_name, 0);
> > >>   	}
> > >> -	drm_close_driver(fd);
> > >
> > > imho proper fix would be remove this and
> > > fd=__drm_open_driver(DRIVER_INTEL)
> > > from this function, as function is named:
> > > test_suspend_without_i915
> > 
> > I don't have much expertise in D states. After reading the comments in the
> > subtest "When module is unloaded and s2idle is triggered, PCI core leaves the
> > endpoint in D0 and the bridge in D3 state causing PCIE spec violation and
> > config space is read as 0xFF. Keep the bridge in D0 before module unload to
> > prevent this issue"
> This is not a real time use case, either a system boots with i915 driver or not.
> Removing i915 module and trigger suspend is not a valid use case but we want to test that
> suspend coverage without i915. So we need to disallow d3cold(which will bring the pcie bridge to D0).
> igt_pm_set_d3cold_allowed(card.pci_slot_name, 0);
> ***above is applicable tp  discrete card.

So this code should look like:

if (state == SUSPEND_STATE_FREEZE) {
    igt_devices_scan(false);
    if (igt_device_find_first_i915_discrete_card(&card)) {
            fd = __drm_open_driver(DRIVER_INTEL); /* write comment why we need this */
            igt_pm_get_d3cold_allowed(card.pci_slot_name, &d3cold_allowed);
            igt_pm_set_d3cold_allowed(card.pci_slot_name, 0);
            drm_close_driver(fd);
    }
}

But this will fix problem on iGPU but still doesn't answer why
__drm_open_driver fails... as in dmesg from fails I see i915
was loaded/unloaded successfully.

Regards,
Kamil

>  
> Thanks,
> Anshuman Gupta  
> > 
> > IMHO, still this patch is a valid fix. Perhaps, we can drop
> > drm_close_driver() and use close().
> > 
> > + Anshuman to comment
> > 
> > - Bhanu
> > 
> > >
> > > we are expecting i915 driver to be _unloaded_.
> > >
> > > Regards,
> > > Kamil
> > >
> > >> +
> > >> +	if (fd >= 0)
> > >> +		drm_close_driver(fd);
> > >>
> > >>   	igt_kmsg(KMSG_INFO "Unloading i915\n");
> > >>   	igt_assert_eq(igt_i915_driver_unload(),0);
> > >> --
> > >> 2.40.0
> > >>