Re: [Intel-gfx] [PATCH 2/2] drm/i915/i9xx_wm: Prefer intel_de functions over intel_uncore.

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

On Wed, Mar 08, 2023 at 07:50:27PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 08, 2023 at 11:58:59AM -0500, Rodrigo Vivi wrote:
> >  	} else if (IS_I915GM(dev_priv)) {
> >  		/*
> >  		 * FIXME can't find a bit like this for 915G, and
> >  		 * yet it does have the related watermark in
> >  		 * FW_BLC_SELF. What's going on?
> >  		 */
> > -		was_enabled = intel_uncore_read(&dev_priv->uncore, INSTPM) & INSTPM_SELF_EN;
> > +		was_enabled = intel_de_read(dev_priv, INSTPM) & INSTPM_SELF_EN;
> >  		val = enable ? _MASKED_BIT_ENABLE(INSTPM_SELF_EN) :
> >  			       _MASKED_BIT_DISABLE(INSTPM_SELF_EN);
> > -		intel_uncore_write(&dev_priv->uncore, INSTPM, val);
> > -		intel_uncore_posting_read(&dev_priv->uncore, INSTPM);
> > +		intel_de_write(dev_priv, INSTPM, val);
> > +		intel_de_posting_read(dev_priv, INSTPM);
> 
> I'm still not really convinced that we want to
> use intel_de_*() for non-display registers.

hmmm... I see...
so should we create a new component out of i915/display and move
these calls there?

but in the end of the day it is the same uncore functions that
are getting calling underneath anyway, right?!

I believe i915/display should only call intel_de for mmio, so it
gets easier on the code reuse on Xe.

> 
> -- 
> Ville Syrjälä
> Intel