Re: [PATCH v13 2/5] drm/i915: Introduce private PAT management

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx