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

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Jul 12, 2019 at 04:12:04PM +0300, Imre Deak wrote:
> 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).

I think that's ok as a limitation. Not pretty (since it means the DE
really has to automatically handle hot(un)plug), but good enough.
-Daniel

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev