[Intel-gfx] [PATCH v13 2/5] drm/i915: Introduce private PAT management

[Wang, Zhi A]

Thanks! :) I see.

-----Original Message-----
From: Joonas Lahtinen [mailto:joonas.lahtinen at linux.intel.com] 
Sent: Tuesday, September 12, 2017 4:34 PM
To: Wang, Zhi A <zhi.a.wang at intel.com>; intel-gfx at lists.freedesktop.org; intel-gvt-dev at lists.freedesktop.org
Cc: Vivi, Rodrigo <rodrigo.vivi at intel.com>; Widawsky, Benjamin <benjamin.widawsky at intel.com>; zhenyuw at linux.intel.com; chris at chris-wilson.co.uk
Subject: Re: [PATCH v13 2/5] drm/i915: Introduce private PAT management

On Tue, 2017-09-12 at 15:20 +0800, Zhi Wang wrote:
> On 09/11/17 16:59, Joonas Lahtinen wrote:
> > On Mon, 2017-09-11 at 12:26 +0800, Zhi Wang wrote:
> > > The private PAT management is to support PPAT entry manipulation. 
> > > Two

<SNIP>

> > > +static unsigned int bdw_private_pat_match(u8 src, u8 dst) {
> > > +	unsigned int score = 0;
> > > +
> > > +	/* Cache attribute has to be matched. */
> > > +	if (GEN8_PPAT_GET_CA(src) != GEN8_PPAT_GET_CA(dst))
> > > +		return 0;
> > 
> > We're not giving any points for when only cache attribute matches? 
> > Does not this result in ENOSPC when we would have an entry with 
> > matching "cache attribute", but no other matching entries while PPAT is full.
> > 
> > 	so maybe score += 4 here?
> > 
> 
> Aiha. cache attribute of src == cache attribute of dst is mandatory 
> since the mismatch of other attribute only causes performance drop, 
> but mismatch of cache attribute causes problem of correctness.

Yes, that's why I suggested;

if (CA(src) != CA(dst))
	return 0;

score += 4;

if (...)
	score += ...

Because currently, if only the cache attribute matches (which is enough for correctness), the score is returned as zero which is then rejected just like if cache attribute did not match. If only cache attribute matches, we should return non-zero, like my example above.

Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation