[igt-dev] [PATCH i-g-t] tests/kms_flip: Keep signal helpers enabled after a failed interruptible subtest

[Shankar, Uma]

> -----Original Message-----
> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Anshuman
> Gupta
> Sent: Thursday, May 21, 2020 6:33 PM
> To: Deak, Imre <imre.deak at intel.com>
> Cc: igt-dev at lists.freedesktop.org; Gupta, Nidhi1 <nidhi1.gupta at intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_flip: Keep signal helpers enabled
> after a failed interruptible subtest
> 
> On 2020-05-19 at 18:29:56 +0530, Imre Deak wrote:
> > For the duration of all the interruptible subtests the signal helper
> > is assumed to stay running after each subtest, both in case the
> > subtest passes or aborts with a failure. So make sure we don't leave
> > the signal helper suspended in case igt_assert() aborts.
> >
> > References: https://gitlab.freedesktop.org/drm/intel/issues/1883
> > Fixes: 60e8be7ccc72 ("tests/kms_flip: Retry test in case of a DP/HDMI
> > link reset")
> > Cc: Uma Shankar <uma.shankar at intel.com>
> > Cc: Nidhi Gupta <nidhi1.gupta at intel.com>
> > Signed-off-by: Imre Deak <imre.deak at intel.com>
> Looks good to me,
> But i have some doubts regarding the reported failure in issue 1883.
> > ---
> >  tests/kms_flip.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/kms_flip.c b/tests/kms_flip.c index
> > 46bfc5858..7bf6958af 100755
> > --- a/tests/kms_flip.c
> > +++ b/tests/kms_flip.c
> > @@ -1325,10 +1325,15 @@ retry:
> >  	 * reset event, which the driver signals with a hotplug event.
> >  	 */
> >  	if (!state_ok) {
> > +		bool hotplug_detected;
> > +
> >  		igt_suspend_signal_helper();
> > -		igt_assert(!retried && igt_hotplug_detected(mon, 3));
> AFAIU correctly the test didn't received any HOTPLUG uevent due to any
> wait_event_interruptible() waitqueue in driver blocking it till 3 seconds, if this
> waitqueue in kernel get interrupted by SIGCONT signal, which is primarly signal
> helper in this igt, then igt_hotplug_detected() will detect a hotplug. Hope my
> understading here is correct here.

Here since we have suspended the signal handler which will ignore or mask any interrupts,
we need to ensure to resume it before we assert.

> Thanks,
> Anshuman Gupta.
> > +		if (!retried)
> > +			hotplug_detected = igt_hotplug_detected(mon, 3);
> >  		igt_resume_signal_helper();
> >
> > +		igt_assert(!retried && hotplug_detected);
> > +
> >  		igt_debug("Retrying after a hotplug event\n");
> >  		retried = true;
> >  		memset(&o->vblank_state, 0, sizeof(o->vblank_state));
> > --
> > 2.23.1
> >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev