[Intel-gfx] [PATCH] drm/i915/mst: fix pipe and vblank enable

[Arkadiusz Hiler]

On Mon, Feb 10, 2020 at 01:43:47PM +0200, Lisovskiy, Stanislav wrote:
> On Mon, 2020-02-10 at 13:32 +0200, Jani Nikula wrote:
> > On Wed, 05 Feb 2020, Jani Nikula <jani.nikula at intel.com> wrote:
> > > Commit 21fd23ac222f ("drm/i915: move pipe, pch and vblank enable to
> > > encoders on DDI platforms") pushed pipe and vblank enable to
> > > encoders on
> > > DDI platforms, however it missed the DP MST encoder. Fix it.
> > > 
> > > Fixes: 21fd23ac222f ("drm/i915: move pipe, pch and vblank enable to
> > > encoders on DDI platforms")
> > > Cc: Vandita Kulkarni <vandita.kulkarni at intel.com>
> > > Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
> > > Reported-by: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> > > Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> > 
> > Thanks for the reviews and testing, pushed to dinq.
> > 
> > I don't usually cut corners, but I've made an exception and pushed
> > this without full IGT results.
> > 
> > It's been 5 days since the patch was posted, the sharded run has
> > fallen between the cracks, and the queue is currently about three
> > days. IMHO it's intolerable for any patch, but especially so for a
> > regression fix that was posted within hours of the bug report.
> 
> Absolutely agree, since we already had a regression, it's pointless
> now to wait longer with such a trivial fix. We are anyway in a bad
> situation now, checking also some other MST issues and having to apply
> this patch manually first in order to get at least this issue ruled
> out.
> 
> Stan

As of why it was silently dropped:

We poke patchwork to check whether there is a newer version of a given
series. If there is we won't waste time on running the older one through
shards.

This bit looks more or less like this:

  RES="$(curl -q https://patchwork.freedesktop.org/api/1.0/series/$SER/revisions/$(( $REV + 1 ))/ 2>/dev/null)"
  [[ "$RES" = "" || "$RES" = *"ot found"* ]] || exit 1

If there is a network issue and curl exits with non-zero exit status
this aborts the shards because of `set -e`, which is what has happened:

  +++ curl -q https://patchwork.freedesktop.org/api/1.0/series/73006/revisions/2/
  ++ RES=
  Build step 'Execute shell' marked build as failure

So a network issue + not robust enough bash script is the cause.

I have fixed the logic there to account for this and in case of network
errors we just go ahead with testing. Thanks for rising this up.


As of the 3 days worth of queued shards:

I agree that this is unacceptable, but we can do only so much from the
CI/infra side. The time has been creeping up steadily over the last year
or so and the machines are not getting any faster.

We are currently sitting on ~58min for a run and Tomi has already done a
lot of in terms of optimization. The overhead is as minimal as it can be
and there is some logic tracking the test execution times and doing
random but balanced test distribution.

We are also considering introducing hard limits on subtest execution
times and hunting down the tests that are exceeding this.

On IGT side there was a recent introduction of dynamic subtest which
should help with time wasted on some of the skips, and I am working on a
more reliable skips for multiple mode testing (currently one subtest =
one execv) but without optimizing the test cases we won't shave off much
time.

-- 
Cheers,
Arek