Re: [PATCH v2 7/8] drm/i915: Create vgpu specific write MMIO to reduce traps

["Yu, Zhang" <yu.c.zhang@linux.intel.com>]

On 10/22/2014 12:40 AM, Daniel Vetter wrote:
> On Thu, Oct 16, 2014 at 02:24:27PM +0800, Yu Zhang wrote:
>> In the virtualized environment, forcewake operations are not
>> necessory for the driver, because mmio accesses will be trapped
>> and emulated by the host side, and real forcewake operations are
>> also done in the host. New mmio write handlers are added to directly
>> call the __raw_i915_write, therefore will reduce many traps and
>> increase the overall performance for drivers runing in the VM
>> with Intel GVT-g enhancement.
>>
>> Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
>> Signed-off-by: Jike Song <jike.song@intel.com>
>> Signed-off-by: Kevin Tian <kevin.tian@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_uncore.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index d5f39f3..ec6d5ce 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -719,6 +719,14 @@ hsw_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace)
>>   	REG_WRITE_FOOTER; \
>>   }
>>
>> +#define __vgpu_write(x) \
>> +static void \
>> +vgpu_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
>> +	REG_WRITE_HEADER; \
>> +	__raw_i915_write##x(dev_priv, reg, val); \
>> +	REG_WRITE_FOOTER; \
>> +}
>> +
>>   static const u32 gen8_shadowed_regs[] = {
>>   	FORCEWAKE_MT,
>>   	GEN6_RPNSWREQ,
>> @@ -813,6 +821,10 @@ __gen4_write(8)
>>   __gen4_write(16)
>>   __gen4_write(32)
>>   __gen4_write(64)
>> +__vgpu_write(8)
>> +__vgpu_write(16)
>> +__vgpu_write(32)
>> +__vgpu_write(64)
>>
>>   #undef __chv_write
>>   #undef __gen8_write
>> @@ -820,6 +832,7 @@ __gen4_write(64)
>>   #undef __gen6_write
>>   #undef __gen5_write
>>   #undef __gen4_write
>> +#undef __vgpu_write
>>   #undef REG_WRITE_FOOTER
>>   #undef REG_WRITE_HEADER
>>
>> @@ -950,6 +963,13 @@ void intel_uncore_init(struct drm_device *dev)
>>   		dev_priv->uncore.funcs.mmio_readq  = gen4_read64;
>>   		break;
>>   	}
>> +
>> +	if (intel_vgpu_active(dev)) {
>> +		dev_priv->uncore.funcs.mmio_writeb = vgpu_write8;
>> +		dev_priv->uncore.funcs.mmio_writew = vgpu_write16;
>> +		dev_priv->uncore.funcs.mmio_writel = vgpu_write32;
>> +		dev_priv->uncore.funcs.mmio_writeq = vgpu_write64;
>
> Someone should write a cool macro which uses prepocessor string
> concatenation so that we can compress this all to
>
> 	ASSIGN_WRITE_MMIO_VFUNCS(vgpu)
>
> Then throw in an ASSIGN_READ_MMIO_VFUNC which looks similarly and this
> might actually be pretty. Just an idea for some follow-up cleanup.
> -Daniel
>
Thanks Daniel.
Do you mean something like this:
#define ASSIGN_WRITE_MMIO_VFUNCS(x) \
do {	\
	dev_priv->uncore.funcs.mmio_writeb  = x##_write8;	\
	dev_priv->uncore.funcs.mmio_writew  = x##_write16;	\
	dev_priv->uncore.funcs.mmio_writel  = x##_write32;	\
	dev_priv->uncore.funcs.mmio_writeq  = x##_write64;	\
} while (0)

and then we can use ASSIGN_WRITE_MMIO_VFUNCS(hsw) for hsw and 
ASSIGN_WRITE_MMIO_VFUNCS(vgpu) for vgpu, etc?

>> +	}
>>   }
>>
>>   void intel_uncore_fini(struct drm_device *dev)
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>