Thanks! :) I see. -----Original Message----- From: Joonas Lahtinen [mailto:joonas.lahtinen@xxxxxxxxxxxxxxx] Sent: Tuesday, September 12, 2017 4:34 PM To: Wang, Zhi A <zhi.a.wang@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx Cc: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>; Widawsky, Benjamin <benjamin.widawsky@xxxxxxxxx>; zhenyuw@xxxxxxxxxxxxxxx; chris@xxxxxxxxxxxxxxxxxx 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
