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

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

[Jani Nikula]

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@intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Reported-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index b8aee506d595..9cd59141953d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -491,6 +491,12 @@ static void intel_mst_enable_dp(struct intel_encoder *encoder,
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 
+	drm_WARN_ON(&dev_priv->drm, pipe_config->has_pch_encoder);
+
+	intel_enable_pipe(pipe_config);
+
+	intel_crtc_vblank_on(pipe_config);
+
 	DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
 
 	if (intel_de_wait_for_set(dev_priv, intel_dp->regs.dp_tp_status,
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/mst: fix pipe and vblank enable

[Patchwork]

== Series Details ==

Series: drm/i915/mst: fix pipe and vblank enable
URL   : https://patchwork.freedesktop.org/series/73006/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7867 -> Patchwork_16427
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16427/index.html

Known issues
------------

  Here are the changes found in Patchwork_16427 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_blt:
    - fi-bsw-nick:        [PASS][1] -> [INCOMPLETE][2] ([i915#392])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7867/fi-bsw-nick/igt@i915_selftest@live_blt.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16427/fi-bsw-nick/igt@i915_selftest@live_blt.html
    - fi-hsw-4770:        [PASS][3] -> [DMESG-FAIL][4] ([i915#725])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7867/fi-hsw-4770/igt@i915_selftest@live_blt.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16427/fi-hsw-4770/igt@i915_selftest@live_blt.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-byt-n2820:       [PASS][5] -> [DMESG-FAIL][6] ([i915#1052])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7867/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16427/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          [PASS][7] -> [FAIL][8] ([i915#217])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7867/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16427/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html

  
#### Possible fixes ####

  * igt@gem_close_race@basic-threads:
    - fi-byt-j1900:       [INCOMPLETE][9] ([i915#45]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7867/fi-byt-j1900/igt@gem_close_race@basic-threads.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16427/fi-byt-j1900/igt@gem_close_race@basic-threads.html

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770r:       [DMESG-FAIL][11] ([i915#725]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7867/fi-hsw-4770r/igt@i915_selftest@live_blt.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16427/fi-hsw-4770r/igt@i915_selftest@live_blt.html

  
#### Warnings ####

  * igt@gem_exec_parallel@fds:
    - fi-byt-n2820:       [TIMEOUT][13] ([fdo#112271]) -> [FAIL][14] ([i915#694])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7867/fi-byt-n2820/igt@gem_exec_parallel@fds.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16427/fi-byt-n2820/igt@gem_exec_parallel@fds.html

  
  [fdo#112271]: https://bugs.freedesktop.org/show_bug.cgi?id=112271
  [i915#1052]: https://gitlab.freedesktop.org/drm/intel/issues/1052
  [i915#217]: https://gitlab.freedesktop.org/drm/intel/issues/217
  [i915#392]: https://gitlab.freedesktop.org/drm/intel/issues/392
  [i915#45]: https://gitlab.freedesktop.org/drm/intel/issues/45
  [i915#694]: https://gitlab.freedesktop.org/drm/intel/issues/694
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725


Participating hosts (42 -> 39)
------------------------------

  Additional (4): fi-hsw-peppy fi-bdw-5557u fi-kbl-r fi-kbl-7500u 
  Missing    (7): fi-hsw-4200u fi-byt-squawks fi-bwr-2160 fi-ctg-p8600 fi-bsw-kefka fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7867 -> Patchwork_16427

  CI-20190529: 20190529
  CI_DRM_7867: a4c409e48c6281538b1e375545dfb5989fa02063 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5418: 4028bd390b41925f6e26f6f11b31e05054652527 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16427: 96040803c6cafbdec7efb41d208e63340c1e410d @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

96040803c6ca drm/i915/mst: fix pipe and vblank enable

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16427/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Kulkarni, Vandita]

> -----Original Message-----
> From: Jani Nikula <jani.nikula@intel.com>
> Sent: Wednesday, February 5, 2020 2:00 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; Kulkarni, Vandita
> <vandita.kulkarni@intel.com>; Ville Syrjala <ville.syrjala@linux.intel.com>;
> Lisovskiy, Stanislav <stanislav.lisovskiy@intel.com>
> Subject: [PATCH] drm/i915/mst: fix pipe and vblank enable
> 
> 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@intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Reported-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Looks good to me.
Reviewed-by: Vandita Kulkarni <vandita.kulkarni@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index b8aee506d595..9cd59141953d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -491,6 +491,12 @@ static void intel_mst_enable_dp(struct intel_encoder
> *encoder,
>  	struct intel_dp *intel_dp = &intel_dig_port->dp;
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> 
> +	drm_WARN_ON(&dev_priv->drm, pipe_config->has_pch_encoder);
> +
> +	intel_enable_pipe(pipe_config);
> +
> +	intel_crtc_vblank_on(pipe_config);
> +
>  	DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
> 
>  	if (intel_de_wait_for_set(dev_priv, intel_dp->regs.dp_tp_status,
> --
> 2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Lisovskiy, Stanislav]

On Wed, 2020-02-05 at 10:29 +0200, Jani Nikula 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@intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Reported-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Checked, seems to fix my displays, so

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index b8aee506d595..9cd59141953d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -491,6 +491,12 @@ static void intel_mst_enable_dp(struct
> intel_encoder *encoder,
>  	struct intel_dp *intel_dp = &intel_dig_port->dp;
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  
> +	drm_WARN_ON(&dev_priv->drm, pipe_config->has_pch_encoder);
> +
> +	intel_enable_pipe(pipe_config);
> +
> +	intel_crtc_vblank_on(pipe_config);
> +
>  	DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
>  
>  	if (intel_de_wait_for_set(dev_priv, intel_dp-
> >regs.dp_tp_status,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

On Wed, 05 Feb 2020, Jani Nikula <jani.nikula@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@intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Reported-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@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.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index b8aee506d595..9cd59141953d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -491,6 +491,12 @@ static void intel_mst_enable_dp(struct intel_encoder *encoder,
>  	struct intel_dp *intel_dp = &intel_dig_port->dp;
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  
> +	drm_WARN_ON(&dev_priv->drm, pipe_config->has_pch_encoder);
> +
> +	intel_enable_pipe(pipe_config);
> +
> +	intel_crtc_vblank_on(pipe_config);
> +
>  	DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
>  
>  	if (intel_de_wait_for_set(dev_priv, intel_dp->regs.dp_tp_status,

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Lisovskiy, Stanislav]

On Mon, 2020-02-10 at 13:32 +0200, Jani Nikula wrote:
> On Wed, 05 Feb 2020, Jani Nikula <jani.nikula@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@intel.com>
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Reported-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > Signed-off-by: Jani Nikula <jani.nikula@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

> 
> BR,
> Jani.
> 
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index b8aee506d595..9cd59141953d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -491,6 +491,12 @@ static void intel_mst_enable_dp(struct
> > intel_encoder *encoder,
> >  	struct intel_dp *intel_dp = &intel_dig_port->dp;
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  
> > +	drm_WARN_ON(&dev_priv->drm, pipe_config->has_pch_encoder);
> > +
> > +	intel_enable_pipe(pipe_config);
> > +
> > +	intel_crtc_vblank_on(pipe_config);
> > +
> >  	DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
> >  
> >  	if (intel_de_wait_for_set(dev_priv, intel_dp-
> > >regs.dp_tp_status,
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [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@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@intel.com>
> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > Reported-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > > Signed-off-by: Jani Nikula <jani.nikula@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

On Mon, 10 Feb 2020, Arkadiusz Hiler <arkadiusz.hiler@intel.com> wrote:
> 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.

I am *not* trying to say that it's all your fault and you need to
provide all results faster for the ever-increasing firehose of incoming
patches.

I'd like to pose the question, what would all this look like if we made
it a hard requirement that we need a go/no-go decision on every patch
series within 24 hours? I emphasize that I don't mean full results in 24
hours. Given all the other constraints, how could we provide as much
useful information as possible within 24 hours to make a decision?

In another thread I said, we've shifted a bit from review being the
bottle neck to shard runs being the bottle neck. It's still much more
likely that a patch will change due to review feedback instead of shard
run results. Half a dozen rounds of review ping pong directly leads to
half a dozen rounds of mostly unnecessary testing. I would not outright
dismiss only running full igt on reviewed/acked patches.

Additionally, there are smaller optimizations to be made (obviously all
depending on developer bandwidth to implement this stuff), such as
identifying patches that don't change the resulting binary
(comment/documentation/whitespace changes), and only running build
testing on them.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Sarvela, Tomi P]

> From: Jani Nikula <jani.nikula@intel.com>
> 
> On Mon, 10 Feb 2020, Arkadiusz Hiler <arkadiusz.hiler@intel.com> wrote:
> > 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.
> 
> I am *not* trying to say that it's all your fault and you need to
> provide all results faster for the ever-increasing firehose of incoming
> patches.
> 
> I'd like to pose the question, what would all this look like if we made
> it a hard requirement that we need a go/no-go decision on every patch
> series within 24 hours? I emphasize that I don't mean full results in 24
> hours. Given all the other constraints, how could we provide as much
> useful information as possible within 24 hours to make a decision?
> 
> In another thread I said, we've shifted a bit from review being the
> bottle neck to shard runs being the bottle neck. It's still much more
> likely that a patch will change due to review feedback instead of shard
> run results. Half a dozen rounds of review ping pong directly leads to
> half a dozen rounds of mostly unnecessary testing. I would not outright
> dismiss only running full igt on reviewed/acked patches.

This is actually a good idea. In practice, the shards are swamped by the
amount of builds today, and the throughput has been close to 1/h a long
time, even with work ongoing to prune or tighten stupidest IGT tests.

We could make the shard run requirements stricter: in addition to passing
BAT it would need some amount of Acks. Patchwork already collects them.

Another idea has been moving the serialized shard run queue to something
that can handle reordering: trybots can be moved after everything else. This
doesn't affect to the shard queue length though, if we still want to test
everything.

> Additionally, there are smaller optimizations to be made (obviously all
> depending on developer bandwidth to implement this stuff), such as
> identifying patches that don't change the resulting binary
> (comment/documentation/whitespace changes), and only running build
> testing on them.

This idea has been floating around, and would help in 5% changes or so
(which is still noticeable: 1-2 more builds / day tested instead of queued).

Just need a good diff checker that says "text changes only, skip it".

Tomi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

On Fri, 14 Feb 2020, "Sarvela, Tomi P" <tomi.p.sarvela@intel.com> wrote:
>> From: Jani Nikula <jani.nikula@intel.com>
>> 
>> On Mon, 10 Feb 2020, Arkadiusz Hiler <arkadiusz.hiler@intel.com> wrote:
>> > 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.
>> 
>> I am *not* trying to say that it's all your fault and you need to
>> provide all results faster for the ever-increasing firehose of incoming
>> patches.
>> 
>> I'd like to pose the question, what would all this look like if we made
>> it a hard requirement that we need a go/no-go decision on every patch
>> series within 24 hours? I emphasize that I don't mean full results in 24
>> hours. Given all the other constraints, how could we provide as much
>> useful information as possible within 24 hours to make a decision?
>> 
>> In another thread I said, we've shifted a bit from review being the
>> bottle neck to shard runs being the bottle neck. It's still much more
>> likely that a patch will change due to review feedback instead of shard
>> run results. Half a dozen rounds of review ping pong directly leads to
>> half a dozen rounds of mostly unnecessary testing. I would not outright
>> dismiss only running full igt on reviewed/acked patches.
>
> This is actually a good idea. In practice, the shards are swamped by the
> amount of builds today, and the throughput has been close to 1/h a long
> time, even with work ongoing to prune or tighten stupidest IGT tests.
>
> We could make the shard run requirements stricter: in addition to passing
> BAT it would need some amount of Acks. Patchwork already collects them.

Of course, patchwork isn't accurate in picking acks/reviews, but I don't
think it has to be. Err on the side of testing, and provide a way to
start shard runs manually, also because sometimes you do want the
results ASAP on v1. (On that note, would be nice if people could
*remove* their patch series from the shard queueu too.)

> Another idea has been moving the serialized shard run queue to something
> that can handle reordering: trybots can be moved after everything else. This
> doesn't affect to the shard queue length though, if we still want to test
> everything.

Next we'll be figuring out a fair scheduler that does not starve the
trybot queue. ;)

>> Additionally, there are smaller optimizations to be made (obviously all
>> depending on developer bandwidth to implement this stuff), such as
>> identifying patches that don't change the resulting binary
>> (comment/documentation/whitespace changes), and only running build
>> testing on them.
>
> This idea has been floating around, and would help in 5% changes or so
> (which is still noticeable: 1-2 more builds / day tested instead of queued).
>
> Just need a good diff checker that says "text changes only, skip it".

It's probably not as trivial as it initially sounds, but gut feeling
says that it's also not a problem that nobody has tried to solve before.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx