All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Add a policy note for removing workarounds
@ 2017-11-17 10:26 Chris Wilson
  2017-11-17 10:53 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Chris Wilson @ 2017-11-17 10:26 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Daniel Vetter, Rodrigo Vivi

Rodrigo gave a persuasive argument for keeping workarounds: that they
serve as a good guide for the bring up of the next generation. Not only
do workarounds persist into the early revisions, they show where the
workarounds were previously added to the code flow and sometimes the old
workarounds have an explanation that give insight into their wider
implications.

Based on his suggestion, document the policy that we want to keep the
workarounds from the current generation to guide the next. Older
preproduction workarounds we still want to remove to keep the code
clean.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 57dfaf04d819..fbfa9434c1d1 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -833,6 +833,11 @@ static void i915_workqueues_cleanup(struct drm_i915_private *dev_priv)
  * We don't keep the workarounds for pre-production hardware, so we expect our
  * driver to fail on these machines in one way or another. A little warning on
  * dmesg may help both the user and the bug triagers.
+ *
+ * Our policy for removing pre-production workarounds is to keep the
+ * current gen workarounds as a guide to the bring-up of the next gen
+ * (workarounds have a habit of persisting!). Anything older than that
+ * should be removed along with the complications they introduce.
  */
 static void intel_detect_preproduction_hw(struct drm_i915_private *dev_priv)
 {
-- 
2.15.0

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Add a policy note for removing workarounds
  2017-11-17 10:26 [PATCH] drm/i915: Add a policy note for removing workarounds Chris Wilson
@ 2017-11-17 10:53 ` Patchwork
  2017-11-17 11:11 ` [PATCH] " Jani Nikula
  2017-11-17 12:36 ` ✓ Fi.CI.IGT: success for " Patchwork
  2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-11-17 10:53 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Add a policy note for removing workarounds
URL   : https://patchwork.freedesktop.org/series/33995/
State : success

== Summary ==

Series 33995v1 drm/i915: Add a policy note for removing workarounds
https://patchwork.freedesktop.org/api/1.0/series/33995/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                pass       -> FAIL       (fi-kbl-7500u) fdo#103163
        Subgroup hdmi-hpd-fast:
                skip       -> FAIL       (fi-kbl-7500u) fdo#102672
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                fail       -> PASS       (fi-gdg-551) fdo#102618
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713

fdo#103163 https://bugs.freedesktop.org/show_bug.cgi?id=103163
fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
fdo#102618 https://bugs.freedesktop.org/show_bug.cgi?id=102618
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:443s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:383s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:536s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:277s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:503s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:505s
fi-byt-j1900     total:289  pass:254  dwarn:0   dfail:0   fail:0   skip:35  time:499s
fi-byt-n2820     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:494s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:434s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:264s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:538s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:429s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:442s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:430s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:487s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:463s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:2   skip:23  time:475s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:542s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:482s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:536s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:578s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:453s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:544s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:565s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:515s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:495s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:464s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:561s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:419s
Blacklisted hosts:
fi-cfl-s2        total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:613s

7cb3896259a7b9c0aa5ce0d09398704edb680607 drm-tip: 2017y-11m-17d-10h-07m-29s UTC integration manifest
d91e88d08451 drm/i915: Add a policy note for removing workarounds

== Logs ==

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Add a policy note for removing workarounds
  2017-11-17 10:26 [PATCH] drm/i915: Add a policy note for removing workarounds Chris Wilson
  2017-11-17 10:53 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-11-17 11:11 ` Jani Nikula
  2017-11-17 17:33   ` Rodrigo Vivi
  2017-11-17 12:36 ` ✓ Fi.CI.IGT: success for " Patchwork
  2 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2017-11-17 11:11 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: Daniel Vetter, Rodrigo Vivi

On Fri, 17 Nov 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Rodrigo gave a persuasive argument for keeping workarounds: that they
> serve as a good guide for the bring up of the next generation. Not only
> do workarounds persist into the early revisions, they show where the
> workarounds were previously added to the code flow and sometimes the old
> workarounds have an explanation that give insight into their wider
> implications.
>
> Based on his suggestion, document the policy that we want to keep the
> workarounds from the current generation to guide the next. Older
> preproduction workarounds we still want to remove to keep the code
> clean.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Acked-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 57dfaf04d819..fbfa9434c1d1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -833,6 +833,11 @@ static void i915_workqueues_cleanup(struct drm_i915_private *dev_priv)
>   * We don't keep the workarounds for pre-production hardware, so we expect our
>   * driver to fail on these machines in one way or another. A little warning on
>   * dmesg may help both the user and the bug triagers.
> + *
> + * Our policy for removing pre-production workarounds is to keep the
> + * current gen workarounds as a guide to the bring-up of the next gen
> + * (workarounds have a habit of persisting!). Anything older than that
> + * should be removed along with the complications they introduce.
>   */
>  static void intel_detect_preproduction_hw(struct drm_i915_private *dev_priv)
>  {

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: Add a policy note for removing workarounds
  2017-11-17 10:26 [PATCH] drm/i915: Add a policy note for removing workarounds Chris Wilson
  2017-11-17 10:53 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-11-17 11:11 ` [PATCH] " Jani Nikula
@ 2017-11-17 12:36 ` Patchwork
  2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-11-17 12:36 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Add a policy note for removing workarounds
URL   : https://patchwork.freedesktop.org/series/33995/
State : success

== Summary ==

Test kms_cursor_legacy:
        Subgroup flip-vs-cursor-varying-size:
                fail       -> PASS       (shard-hsw) fdo#102670
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-blt:
                fail       -> PASS       (shard-snb) fdo#101623 +1
        Subgroup fbc-1p-primscrn-pri-shrfb-draw-mmap-gtt:
                skip       -> PASS       (shard-hsw) fdo#103167
Test kms_flip:
        Subgroup vblank-vs-modeset-suspend-interruptible:
                skip       -> PASS       (shard-hsw)
        Subgroup blt-wf_vblank-vs-dpms-interruptible:
                skip       -> PASS       (shard-hsw) fdo#102614
        Subgroup dpms-vs-vblank-race-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#103060
Test drv_module_reload:
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (shard-hsw) fdo#102707 +1
        Subgroup basic-reload:
                pass       -> DMESG-WARN (shard-snb) fdo#102848
Test kms_busy:
        Subgroup extended-modeset-hang-newfb-with-reset-render-a:
                skip       -> PASS       (shard-hsw) fdo#102249
Test kms_atomic_transition:
        Subgroup 1x-modeset-transitions-fencing:
                skip       -> PASS       (shard-hsw)
Test kms_plane:
        Subgroup plane-position-hole-pipe-a-planes:
                skip       -> PASS       (shard-hsw)

fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707
fdo#102848 https://bugs.freedesktop.org/show_bug.cgi?id=102848
fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249

shard-hsw        total:2585 pass:1471 dwarn:3   dfail:1   fail:11  skip:1099 time:9460s
shard-snb        total:2585 pass:1259 dwarn:2   dfail:1   fail:12  skip:1311 time:8036s
Blacklisted hosts:
shard-apl        total:2585 pass:1624 dwarn:1   dfail:2   fail:22  skip:936 time:13400s
shard-kbl        total:2492 pass:1654 dwarn:5   dfail:1   fail:25  skip:806 time:10379s

== Logs ==

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Add a policy note for removing workarounds
  2017-11-17 11:11 ` [PATCH] " Jani Nikula
@ 2017-11-17 17:33   ` Rodrigo Vivi
  2017-11-17 17:51     ` Chris Wilson
  0 siblings, 1 reply; 6+ messages in thread
From: Rodrigo Vivi @ 2017-11-17 17:33 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Daniel Vetter, intel-gfx

On Fri, Nov 17, 2017 at 11:11:28AM +0000, Jani Nikula wrote:
> On Fri, 17 Nov 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Rodrigo gave a persuasive argument for keeping workarounds: that they
> > serve as a good guide for the bring up of the next generation. Not only
> > do workarounds persist into the early revisions, they show where the
> > workarounds were previously added to the code flow and sometimes the old
> > workarounds have an explanation that give insight into their wider
> > implications.

Thanks! :)

> >
> > Based on his suggestion, document the policy that we want to keep the
> > workarounds from the current generation to guide the next. Older
> > preproduction workarounds we still want to remove to keep the code
> > clean.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 57dfaf04d819..fbfa9434c1d1 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -833,6 +833,11 @@ static void i915_workqueues_cleanup(struct drm_i915_private *dev_priv)
> >   * We don't keep the workarounds for pre-production hardware, so we expect our
> >   * driver to fail on these machines in one way or another. A little warning on
> >   * dmesg may help both the user and the bug triagers.
> > + *
> > + * Our policy for removing pre-production workarounds is to keep the
> > + * current gen workarounds as a guide to the bring-up of the next gen
> > + * (workarounds have a habit of persisting!). Anything older than that
> > + * should be removed along with the complications they introduce.
> >   */

Maybe it would be good to mention that they should be at least protected
by the REVID checks if they stay around.

But with or without this change:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



> >  static void intel_detect_preproduction_hw(struct drm_i915_private *dev_priv)
> >  {
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Add a policy note for removing workarounds
  2017-11-17 17:33   ` Rodrigo Vivi
@ 2017-11-17 17:51     ` Chris Wilson
  0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2017-11-17 17:51 UTC (permalink / raw)
  To: Rodrigo Vivi, Jani Nikula; +Cc: Daniel Vetter, intel-gfx

Quoting Rodrigo Vivi (2017-11-17 17:33:49)
> On Fri, Nov 17, 2017 at 11:11:28AM +0000, Jani Nikula wrote:
> > On Fri, 17 Nov 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > Rodrigo gave a persuasive argument for keeping workarounds: that they
> > > serve as a good guide for the bring up of the next generation. Not only
> > > do workarounds persist into the early revisions, they show where the
> > > workarounds were previously added to the code flow and sometimes the old
> > > workarounds have an explanation that give insight into their wider
> > > implications.
> 
> Thanks! :)
> 
> > >
> > > Based on his suggestion, document the policy that we want to keep the
> > > workarounds from the current generation to guide the next. Older
> > > preproduction workarounds we still want to remove to keep the code
> > > clean.
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > Acked-by: Jani Nikula <jani.nikula@intel.com>
> > 
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > index 57dfaf04d819..fbfa9434c1d1 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -833,6 +833,11 @@ static void i915_workqueues_cleanup(struct drm_i915_private *dev_priv)
> > >   * We don't keep the workarounds for pre-production hardware, so we expect our
> > >   * driver to fail on these machines in one way or another. A little warning on
> > >   * dmesg may help both the user and the bug triagers.
> > > + *
> > > + * Our policy for removing pre-production workarounds is to keep the
> > > + * current gen workarounds as a guide to the bring-up of the next gen
> > > + * (workarounds have a habit of persisting!). Anything older than that
> > > + * should be removed along with the complications they introduce.
> > >   */
> 
> Maybe it would be good to mention that they should be at least protected
> by the REVID checks if they stay around.

Not quite sure how we want to word that. Basically it amounts to that
when we have production units and completed alpha-supported, then
sometime later we want to start tainting the pre-production sdp.
Or maybe it should be simply on completion of alpha-support those
pre-production sdp are tainted.

> But with or without this change:
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Taken the simple comment, suggestions welcome.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-11-17 17:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-17 10:26 [PATCH] drm/i915: Add a policy note for removing workarounds Chris Wilson
2017-11-17 10:53 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-11-17 11:11 ` [PATCH] " Jani Nikula
2017-11-17 17:33   ` Rodrigo Vivi
2017-11-17 17:51     ` Chris Wilson
2017-11-17 12:36 ` ✓ Fi.CI.IGT: success for " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.