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

Kamil Konieczny kamil.konieczny at linux.intel.com
Wed Jan 17 18:51:36 UTC 2024


Hi Bhanuprakash,
On 2024-01-17 at 20:52:17 +0530, Modem, Bhanuprakash wrote:
> 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"
> 
> IMHO, still this patch is a valid fix. Perhaps, we can drop
> drm_close_driver() and use close().
> 
> + Anshuman to comment
> 
> - Bhanu
> 

Function was opened with __drm_open_driver so maybe we should use here
__drm_close_driver(fd) but this looks like an error, e.g. if we require
here a driver to be loaded then fd == -1 looks like an error and should
be checked after __drm_open_driver() above.

Regards,
Kamil

> > 
> > 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
> > > 


More information about the igt-dev mailing list