[Intel-gfx] [PATCH] drm/i915: Add a policy note for removing workarounds

[Chris Wilson]

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 at 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 at chris-wilson.co.uk>
> > > Cc: Jani Nikula <jani.nikula at intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> > 
> > Acked-by: Jani Nikula <jani.nikula at 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 at intel.com>

Taken the simple comment, suggestions welcome.
-Chris