Re: [PATCH] drm/i915: Add a policy note for removing workarounds

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

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