Re: [igt-dev] [PATCH RFC i-g-t] tests/kms_chamelium: disable output when resetting state

[Imre Deak <imre.deak@intel.com>]

On Thu, Jul 11, 2019 at 03:14:48PM +0200, Daniel Vetter wrote:
> On Thu, Jul 11, 2019 at 2:47 PM Ser, Simon <simon.ser@intel.com> wrote:
> >
> > On Thu, 2019-07-11 at 14:40 +0200, Daniel Vetter wrote:
> > > On Thu, Jul 11, 2019 at 1:11 PM Simon Ser <simon.ser@intel.com> wrote:
> > > > Quoting Imre from the bug comments:
> > > >
> > > > > If unplugging a DP-alt display while there is an active modeset on it, and
> > > > > then replugging the DP-alt display, the HW/FW/driver won't signal the
> > > > > corresponding plug-in hotplug event until the mode is disabled.
> > > > >
> > > > > The failing tests do this:
> > > > >
> > > > > - <have the output enabled>
> > > > > - unplug the display, keeping the output enabled
> > > > > - wait/poll for the disconnect status -> success
> > > > > - replug the display, while the output is still enabled
> > > > > - wait/poll for the connect status -> fail
> > > > >
> > > > > This sequence is not supported with DP-alt sinks (like the USB-C to HDMI
> > > > > dongle), as I described above.
> > > >
> > > > This patch changes reset_state to disable the output before unplugging the
> > > > connector (via chamelium_reset).
> > > >
> > > > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111045
> > > > ---
> > > >
> > > > This is just a RFC to check that the root issue is what we think it is.
> > >
> > > Yeah we can't merge this, this would completely break the hotunplug
> > > usecase, which is:
> > > 1. user yanks cable
> > > 2. desktop reacts to hotunplug and shuts down the screen
> >
> > Would disabling the screen after the drmModeConnector says it's
> > disconnected be better?
> 
> I guess depends what you want to test. If it's the hotunplug event
> itself, then that should arrive without userspace doing anything at
> all. If you don't care for the uevent, not sure why you'd poll
> connector status ...
> 
> > > If the hw can't do a hotunplug while there's an active mode, then we
> > > have a chicken&egg situation, and we need to resort to some form of
> > > polling (hooray). Hotunplug better work if the display is on.

Hot unplug with an active mode does work and the corresponding uevent
will be sent by the driver, it's just the replug uevent that won't be
delivered until the active mode gets disabled (which should happen
anyway as part of the normal unplug uevent processing by user space).

> >
> > TBH I'm not sure why it's so important to have all tests keep the mode
> > active during an unplug-replug sequence.
> 
> That's another use-case, at least for !DP-MST: We re-train on replug
> to make sure a quick cable yank doesn't result in disappointment and
> black screens. MST is a bit different in this regard, but iirc Lyude
> has been thinking about making that happen even for mst. So also
> something that would be nice to test.
> -Daniel
> 
> >
> > > Historical aside: Polling is disabled on may old VGA outputs because
> > > the polling can result in screen flickering/corruption. But hw got
> > > better, so even VGA managed to handle hotunplug correctly. No DE (or
> > > user fwiw) assumes nowadays that hotunplug is broken.
> > > -Daniel
> > >
> > > >  tests/kms_chamelium.c | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > > > index b749b5598c1d..f538ad394480 100644
> > > > --- a/tests/kms_chamelium.c
> > > > +++ b/tests/kms_chamelium.c
> > > > @@ -216,6 +216,13 @@ static void
> > > >  reset_state(data_t *data, struct chamelium_port *port)
> > > >  {
> > > >         int p;
> > > > +       drmModeConnector *connector;
> > > > +       igt_output_t *output;
> > > > +
> > > > +       connector = chamelium_port_get_connector(data->chamelium, port, false);
> > > > +       output = igt_output_from_connector(&data->display, connector);
> > > > +       igt_output_set_pipe(output, PIPE_NONE);
> > > > +       igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > > >
> > > >         chamelium_reset(data->chamelium);
> > > >
> > > > --
> > > > 2.22.0
> > > >
> > > > _______________________________________________
> > > > igt-dev mailing list
> > > > igt-dev@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > >
> > >
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev