[igt-dev] [PATCH i-g-t] tests/kms_fbcon_fbt: When restoring fbcon always set mode to text mode

[Shankar, Uma]

> -----Original Message-----
> From: Souza, Jose <jose.souza at intel.com>
> Sent: Friday, May 29, 2020 10:21 PM
> To: Shankar, Uma <uma.shankar at intel.com>; igt-dev at lists.freedesktop.org
> Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_fbcon_fbt: When restoring fbcon
> always set mode to text mode
> 
> On Fri, 2020-05-29 at 21:19 +0530, Shankar, Uma wrote:
> > > -----Original Message-----
> > > From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of
> > > José Roberto de Souza
> > > Sent: Friday, May 29, 2020 6:22 AM
> > > To: igt-dev at lists.freedesktop.org
> > > Subject: [igt-dev] [PATCH i-g-t] tests/kms_fbcon_fbt: When restoring
> > > fbcon always set mode to text mode
> > >
> > > If by some reason or tests, this tests is executed and VT mode is
> > > already in KD_GRAPHICS the call to kmstest_set_vt_graphics_mode()
> > > will set orig_vt_mode as KD_GRAPHICS and when it was calling
> > > kmstest_restore_vt_mode() it would set KD_GRAPHICS again not
> > > returning to fbcon and causing the test to fail.
> > >
> > > As it can be seen here:
> > > (kms_fbcon_fbt:11004) igt_kms-DEBUG: VT: graphics mode set (mode was
> 0x1)"
> > > https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_499/fi-ehl-
> > > 1/igt at kms_fbcon_fbt@fbc.html
> > >
> > > So here adding a new function to alaways set mode the KD_TEXT when
> > > we want to restore to fbcon.
> > >
> > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > ---
> > >  lib/igt_kms.c         | 11 +++++++++++
> > >  lib/igt_kms.h         |  1 +
> > >  tests/kms_fbcon_fbt.c |  2 +-
> > >  3 files changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c index
> > > d4cbc1c53..afef59396 100644
> > > --- a/lib/igt_kms.c
> > > +++ b/lib/igt_kms.c
> > > @@ -968,6 +968,17 @@ void kmstest_set_vt_graphics_mode(void)
> > >  	igt_debug("VT: graphics mode set (mode was 0x%lx)\n", ret);  }
> > >
> > > +/**
> > > + * kmstest_set_vt_text_mode:
> > > + *
> > > + * Sets the controlling VT (if available) into text mode.
> > > + * Unlikely kmstest_set_vt_graphics_mode() it do not install an igt
> > > +exit
> > > + * handler to set the VT back to the previous mode.
> > > + */
> > > +void kmstest_set_vt_text_mode(void) {
> > > +	igt_assert(set_vt_mode(KD_TEXT) >= 0); }
> > >
> > >  static void reset_connectors_at_exit(int sig)  { diff --git
> > > a/lib/igt_kms.h b/lib/igt_kms.h index adca59ac6..32a0e4cc6 100644
> > > --- a/lib/igt_kms.h
> > > +++ b/lib/igt_kms.h
> > > @@ -93,6 +93,7 @@ void kmstest_dump_mode(drmModeModeInfo *mode);
> > > int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id);  void
> > > kmstest_set_vt_graphics_mode(void);
> > >  void kmstest_restore_vt_mode(void);
> > > +void kmstest_set_vt_text_mode(void);
> > >
> > >  enum igt_atomic_crtc_properties {
> > >         IGT_CRTC_BACKGROUND = 0,
> > > diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c index
> > > e6dd4353b..e99a1f2f2 100644
> > > --- a/tests/kms_fbcon_fbt.c
> > > +++ b/tests/kms_fbcon_fbt.c
> > > @@ -283,7 +283,7 @@ static void restore_fbcon(struct drm_info *drm)
> > >  	kmstest_unset_all_crtcs(drm->fd, drm->res);
> > >  	igt_remove_fb(drm->fd, &drm->fb);
> > >  	igt_device_drop_master(drm->fd);
> > > -	kmstest_restore_vt_mode();
> > > +	kmstest_set_vt_text_mode();
> >
> > Can we level set the state to KD_TEXT in beginning only before we begin our
> test.
> > This way test will start with the expected state, and we can then set
> > KD_GRAPHICS, restore will ensure previous state is set again as
> > KD_TEXT, and we can just continue with the existing restore logic. Will that
> cause any issue ?
> 
> We could be overwriting the previous state, KD_TEXT is the state we expect in
> our CI but it do not means that is the state that other users have prior of
> executing this test, that is why kmstest_restore_vt_mode() don't just set to
> KD_TEXT.

But will this not put the mode as KD_TEXT for other user as well who would be expecting it to be
KD_GRAPHICS. How we ensure that we exit with original state of  KD_GRAPHICS with which
we started.

> >
> > Regards,
> > Uma Shankar
> >
> > >  }
> > >
> > >  static void subtest(struct drm_info *drm, struct feature *feature,
> > > bool suspend)
> > > --
> > > 2.26.2
> > >
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev