[Intel-gfx] [PATCH 2/8] drm/i915: introduce intel_has_sagv()

[Zanoni, Paulo R]

Em Sex, 2016-09-09 às 11:06 +0300, Jani Nikula escreveu:
> On Thu, 08 Sep 2016, Lyude Paul <cpaul at redhat.com> wrote:
> > 
> > On Thu, 2016-09-08 at 11:59 +0300, Jani Nikula wrote:
> > > 
> > > On Wed, 07 Sep 2016, Lyude <cpaul at redhat.com> wrote:
> > > > 
> > > > 
> > > > On Tue, 2016-09-06 at 21:52 -0300, Paulo Zanoni wrote:
> > > > > 
> > > > > +static bool
> > > > > +intel_has_sagv(struct drm_i915_private *dev_priv)
> > > > > +{
> > > > > +	return IS_SKYLAKE(dev_priv);
> > > > > +}
> > > > > +
> > > > 
> > > > Not sure I agree on this one. Even if a system is skylake or
> > > > kabylake,
> > > > there's a couple of very early skylake machines that don't
> > > > actually
> > > > have an SAGV on them. Hence the I915_SAGV_NOT_CONTROLLED value
> > > > we set
> > > > if we get mailbox errors.
> > > 
> > > If by "very early" you mean pre-production, we don't care.

Ok, so I'd like some clarification regarding this from the maintainers.
I always thought we didn't really care, but do this:

$ git grep _REVID_

If we don't care, why do we have this? Newer platforms also have this.
And many of these REVID checks are only pre-prod.

> > 
> > The problem is if we don't handle that case though then a couple of
> > the machines in CI start failing tests since all of the SAGV
> > mailbox
> > commands don't end up working :(
> 
> Regardless of whose CI you refer to, no pre-production machines
> should
> be used for CI. Which machines are these?

I suppose he's talking about our CI.

> 
> Can we be sure all production machines have SAGV?

Our specs don't mention anything regarding this. I'll have to ask for
clarification, but I don't think it will be a good idea to remove the
code if CI starts complaining.

> 
> BR,
> Jani.
> 
>