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 11:33 PM, Daniel Vetter wrote:
> On Wed, Oct 22, 2014 at 08:27:50PM +0800, Yu, Zhang wrote:
>>
>>
>> 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?
>
> Yup. Plus the version for assigning READ vfuncs (on many platforms they
> don't match up). Probably best if you do this conversion as a prep patch
> before the vgt series so that I can merge it right away.
> -Daniel
Sure, thanks.
I 'll send this patch later, before our v3 vgpu patch series. :)

Yu
>