Re: [RFC 2/7] drm/i915/gvt: Introduce GEN8 private PAT ops

[Zhi Wang <zhi.a.wang@intel.com>]

It's OK. I can split it. :)

On 08/16/17 13:12, Zhenyu Wang wrote:
> On 2017.08.16 05:31:10 +0800, Zhi Wang wrote:
>> This patch introduces private PAT operations for GEN8.
>>
>> The "get_pat_value" operation offers an API to get a PPAT entry from the
>> virtual or physical PPAT MMIO registers. Similar with the "get_pat_value",
>> the "set_pat_value" operation offers the API to write a PPAT entry back to
>> PPAT MMIO registers. The caller doesn't need to care about the PPAT MMIO
>> layout, since the layout is different between different GENs.
>>
>> The "match_pat_value" will try to compare two PAT entries and give a score
>> to indicate how perfect they match with each other. The most important
>> attribute is "cache attribute", which affects the correctness and has to
>> be matched. Other attributes gain different scores by the their
>> importance in a partial match.
>>
>> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gvt/gtt.c | 81 ++++++++++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/gvt/gtt.h |  9 +++++
>>   2 files changed, 90 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
>> index e6dfc33..16bfca9 100644
>> --- a/drivers/gpu/drm/i915/gvt/gtt.c
>> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
>> @@ -402,6 +402,80 @@ DEFINE_PPGTT_GMA_TO_INDEX(gen8, l3_pdp, (gma >> 30 & 0x3));
>>   DEFINE_PPGTT_GMA_TO_INDEX(gen8, l4_pdp, (gma >> 30 & 0x1ff));
>>   DEFINE_PPGTT_GMA_TO_INDEX(gen8, pml4, (gma >> 39 & 0x1ff));
>>   
>> +static unsigned int gen8_pat_get_value(u32 *mem, unsigned int index,
>> +				       struct intel_gvt *gvt)
>> +{
> 
> Really dislike to provide many kinds of usage interfaces for one function,
> it's easy to cause confusion and mostly unnecessary, although we have lot
> in gtt.c...and better to put driver struct as first parameter to align with
> other function conversion.
> 
>> +	struct drm_i915_private *dev_priv = gvt->dev_priv;
>> +	int reg_index = index / 4;
>> +	u32 buf[2];
>> +
>> +	if (WARN_ON(reg_index >= 2))
>> +		return 0;
>> +
>> +	if (!mem) {
>> +		mem = buf;
>> +		mem[reg_index] = reg_index ? I915_READ(GEN8_PRIVATE_PAT_HI) :
>> +				 I915_READ(GEN8_PRIVATE_PAT_LO);
>> +	}
>> +
>> +	index -= reg_index * 4;
>> +	return (mem[reg_index] >> (index * 8)) & 0x3f;
>> +}
>> +
>> +static void gen8_pat_set_value(u32 *mem, unsigned int index, unsigned int value,
>> +			       struct intel_gvt *gvt)
>> +{
>> +	struct drm_i915_private *dev_priv = gvt->dev_priv;
>> +	int reg_index = index / 4;
>> +	u32 buf[2];
>> +	bool writeback = false;
>> +
>> +	if (WARN_ON(reg_index >= 2))
>> +		return;
>> +
>> +	if (!mem) {
>> +		mem = buf;
>> +		writeback = true;
>> +		mem[reg_index] = reg_index ? I915_READ(GEN8_PRIVATE_PAT_HI) :
>> +				 I915_READ(GEN8_PRIVATE_PAT_LO);
>> +	}
>> +
>> +	index -= reg_index * 4;
>> +
>> +	mem[reg_index] &= ~(0xff << (index * 8));
>> +	mem[reg_index] |= ((value & 0x3f) << (index * 8));
>> +
>> +	if (writeback) {
>> +		if (reg_index)
>> +			I915_WRITE(GEN8_PRIVATE_PAT_LO, mem[1]);
>> +		else
>> +			I915_WRITE(GEN8_PRIVATE_PAT_LO, mem[0]);
>> +	}
>> +}
>> +
>> +#define gen8_pat_ca(v) ((v) & 0x3)
>> +#define gen8_pat_tc(v) ((v) >> 2 & 0x3)
>> +#define gen8_pat_age(v) ((v) >> 4 & 0x3)
>> +
>> +static unsigned int gen8_pat_match_value(unsigned int src, unsigned int dst)
>> +{
>> +	unsigned int score = 0;
>> +
>> +	if (gen8_pat_ca(src) != gen8_pat_ca(dst))
>> +		return 0;
>> +
>> +	if (gen8_pat_age(src) == gen8_pat_age(dst))
>> +		score += 1;
>> +
>> +	if (gen8_pat_tc(src) == gen8_pat_tc(dst))
>> +		score += 2;
>> +
>> +	if (score == 3)
>> +		return ~0;
>> +
>> +	return score;
>> +}
>> +
>>   static struct intel_gvt_gtt_pte_ops gen8_gtt_pte_ops = {
>>   	.get_entry = gtt_get_entry64,
>>   	.set_entry = gtt_set_entry64,
>> @@ -421,6 +495,12 @@ static struct intel_gvt_gtt_gma_ops gen8_gtt_gma_ops = {
>>   	.gma_to_pml4_index = gen8_gma_to_pml4_index,
>>   };
>>   
>> +static struct intel_gvt_gtt_pat_ops gen8_pat_ops = {
>> +	.get_pat_value = gen8_pat_get_value,
>> +	.set_pat_value = gen8_pat_set_value,
>> +	.match_pat_value = gen8_pat_match_value,
>> +};
>> +
>>   static int gtt_entry_p2m(struct intel_vgpu *vgpu, struct intel_gvt_gtt_entry *p,
>>   		struct intel_gvt_gtt_entry *m)
>>   {
>> @@ -2267,6 +2347,7 @@ int intel_gvt_init_gtt(struct intel_gvt *gvt)
>>   		|| IS_KABYLAKE(gvt->dev_priv)) {
>>   		gvt->gtt.pte_ops = &gen8_gtt_pte_ops;
>>   		gvt->gtt.gma_ops = &gen8_gtt_gma_ops;
>> +		gvt->gtt.pat_ops = &gen8_pat_ops;
>>   		gvt->gtt.mm_alloc_page_table = gen8_mm_alloc_page_table;
>>   		gvt->gtt.mm_free_page_table = gen8_mm_free_page_table;
>>   	} else {
>> diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
>> index 30a4c8d..7a9eb05 100644
>> --- a/drivers/gpu/drm/i915/gvt/gtt.h
>> +++ b/drivers/gpu/drm/i915/gvt/gtt.h
>> @@ -77,9 +77,18 @@ struct intel_gvt_gtt_gma_ops {
>>   	unsigned long (*gma_to_pml4_index)(unsigned long gma);
>>   };
>>   
>> +struct intel_gvt_gtt_pat_ops {
>> +	unsigned int (*get_pat_value)(u32 *mem, unsigned int index,
>> +				      struct intel_gvt *gvt);
>> +	void (*set_pat_value)(u32 *mem, unsigned int index, unsigned int value,
>> +			      struct intel_gvt *gvt);
>> +	unsigned int (*match_pat_value)(unsigned int src, unsigned int dst);
>> +};
>> +
>>   struct intel_gvt_gtt {
>>   	struct intel_gvt_gtt_pte_ops *pte_ops;
>>   	struct intel_gvt_gtt_gma_ops *gma_ops;
>> +	struct intel_gvt_gtt_pat_ops *pat_ops;
>>   	int (*mm_alloc_page_table)(struct intel_vgpu_mm *mm);
>>   	void (*mm_free_page_table)(struct intel_vgpu_mm *mm);
>>   	struct list_head oos_page_use_list_head;
>> -- 
>> 2.7.4
>>
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx