From: Zhi Wang <zhi.a.wang@intel.com>
To: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
intel-gfx@lists.freedesktop.org, igvt-g@lists.01.org
Cc: daniel.vetter@ffwll.ch, david.j.cowperthwaite@intel.com,
zhiyuan.lv@intel.com
Subject: Re: [RFCv2 03/14] drm/i915: Introduce host graphics memory/fence partition for GVT-g
Date: Tue, 23 Feb 2016 21:23:05 +0800 [thread overview]
Message-ID: <56CC5D39.7090408@intel.com> (raw)
In-Reply-To: <1456233390.7789.80.camel@linux.intel.com>
On 02/23/16 21:16, Joonas Lahtinen wrote:
> On to, 2016-02-18 at 19:42 +0800, Zhi Wang wrote:
>> From: Bing Niu <bing.niu@intel.com>
>>
>> This patch introduces host graphics memory/fence partition when GVT-g
>> is enabled.
>>
>> Under GVT-g, i915 host driver only owned limited graphics resources,
>> others are managed by GVT-g resource allocator and kept for other vGPUs.
>>
>> v2:
>> - Address all coding-style comments from Joonas previously.
>> - Fix errors and warnning reported by checkpatch.pl. (Joonas)
>> - Move the graphs into the header files. (Daniel)
>>
>> Signed-off-by: Bing Niu <bing.niu@intel.com>
>> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
>> ---
>> drivers/gpu/drm/i915/gvt/gvt.c | 4 ++++
>> drivers/gpu/drm/i915/gvt/params.c | 12 ++++++++++++
>> drivers/gpu/drm/i915/gvt/params.h | 3 +++
>> drivers/gpu/drm/i915/i915_drv.h | 35 +++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/i915_gem.c | 4 +++-
>> drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
>> drivers/gpu/drm/i915/i915_vgpu.c | 21 +++++++++++++++++----
>> 7 files changed, 76 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
>> index 52cfa32..2099b7e 100644
>> --- a/drivers/gpu/drm/i915/gvt/gvt.c
>> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
>> @@ -348,6 +348,10 @@ void *gvt_create_pgt_device(struct drm_i915_private *dev_priv)
>> goto err;
>> }
>>
>> + dev_priv->gvt.host_fence_sz = gvt.host_fence_sz;
>> + dev_priv->gvt.host_low_gm_sz_in_mb = gvt.host_low_gm_sz;
>> + dev_priv->gvt.host_high_gm_sz_in_mb = gvt.host_high_gm_sz;
>
> I'm thinking, could we expose the pgt_device struct (at least
> partially, and then have a PIMPL pattern), to avoid this kind of
> duplication of declarations and unnecessary copies between i915 and
> i915_gvt modules?
>
> It's little rough that the gvt driver writes to i915_private struct.
> I'd rather see that gvt.host_fence_sz and other variables get sanitized
> and then written to pgt_device (maybe the public part would be
> i915_pgt_device) and used by gvt and i915 code.
>
> Was this ever considered?
>
The previous version do something similar like that, both i915 and gvt
reads GVT module kernel parameter but considered that GVT modules could
be configured as "n" in kernel configuration, probably we should let
i915 to store this information and GVT to configure it if GVT is active?
>> +
>> gvt_dbg_core("pgt device creation done, id %d", pdev->id);
>>
>> return pdev;
>> diff --git a/drivers/gpu/drm/i915/gvt/params.c b/drivers/gpu/drm/i915/gvt/params.c
>> index d381d17..75195fd 100644
>> --- a/drivers/gpu/drm/i915/gvt/params.c
>> +++ b/drivers/gpu/drm/i915/gvt/params.c
>> @@ -26,7 +26,19 @@
>> struct gvt_kernel_params gvt = {
>> .enable = false,
>> .debug = 0,
>> + .host_low_gm_sz = 96, /* in MB */
>> + .host_high_gm_sz = 384, /* in MB */
>> + .host_fence_sz = 4,
>> };
>>
>> module_param_named(gvt_enable, gvt.enable, bool, 0600);
>> MODULE_PARM_DESC(gvt_enable, "Enable Intel GVT-g host support");
>> +
>> +module_param_named(gvt_host_low_gm_sz, gvt.host_low_gm_sz, int, 0600);
>> +MODULE_PARM_DESC(gvt_host_low_gm_sz, "Amount of aperture size of host (in MB)");
>
> As i915_gvt will be the host module, "gvt_host" can be removed so the
> module parameters become;
>
> i915_gvt.low_gm_sz instead of i915_gvt.gvt_host_low_gm_sz
>
> Also should these be _unsafe parameters because I bet they can make the
> machine go crazy if wrong?
>
Yes, the params need to be checked, indeed.
>> +
>> +module_param_named(gvt_host_high_gm_sz, gvt.host_high_gm_sz, int, 0600);
>> +MODULE_PARM_DESC(gvt_host_high_gm_sz, "Amount of high memory size of host (in MB)");
>> +
>> +module_param_named(gvt_host_fence_sz, gvt.host_fence_sz, int, 0600);
>> +MODULE_PARM_DESC(gvt_host_fence_sz, "Amount of fence size of host (in MB)");
>> diff --git a/drivers/gpu/drm/i915/gvt/params.h b/drivers/gpu/drm/i915/gvt/params.h
>> index d2955b9..f4e9356 100644
>> --- a/drivers/gpu/drm/i915/gvt/params.h
>> +++ b/drivers/gpu/drm/i915/gvt/params.h
>> @@ -27,6 +27,9 @@
>> struct gvt_kernel_params {
>> bool enable;
>> int debug;
>> + int host_low_gm_sz;
>> + int host_high_gm_sz;
>> + int host_fence_sz;
>> };
>>
>> extern struct gvt_kernel_params gvt;
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 2f897c3..1fd5575 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1705,8 +1705,43 @@ struct i915_workarounds {
>> u32 hw_whitelist_count[I915_NUM_RINGS];
>> };
>>
>> +/*
>> + * Under GVT-g, i915 host driver only owned limited graphics resources,
>> + * others are managed by GVT-g resource allocator and kept for other vGPUs.
>> + *
>> + * For graphics memory space partition, a typical layout looks like:
>> + *
>> + * +-------+-----------------------+------+-----------------------+
>> + * |* Host | *GVT-g Resource |* Host| *GVT-g Resource |
>> + * | Owned | Allocator Managed | Owned| Allocator Managed |
>> + * | | | | |
>> + * +---------------+-------+----------------------+-------+-------+
>> + * | | | | | | | | |
>> + * | i915 | vm 1 | vm 2 | vm 3 | i915 | vm 1 | vm 2 | vm 3 |
>> + * | | | | | | | | |
>> + * +-------+-------+-------+--------------+-------+-------+-------+
>> + * | Aperture | Hidden |
>> + * +-------------------------------+------------------------------+
>> + * | GGTT memory space |
>> + * +--------------------------------------------------------------+
>> + *
>> + * Similar with fence registers partition:
>> + *
>> + * +------ +-----------------------+
>> + * | * Host| GVT-g Resource |
>> + * | Owned | Allocator Managed +
>> + * 0 | 31
>> + * +---------------+-------+-------+
>> + * | | | | |
>> + * | i915 | vm 1 | vm 2 | vm 3 |
>> + * | | | | |
>> + * +-------+-------+-------+-------+
>> + */
>> struct i915_gvt {
>> void *pgt_device;
>> + u32 host_low_gm_sz_in_mb;
>> + u32 host_high_gm_sz_in_mb;
>> + int host_fence_sz;
>> };
>>
>> struct i915_virtual_gpu {
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index f68f346..1c0006a 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -5078,7 +5078,9 @@ i915_gem_load_init(struct drm_device *dev)
>> else
>> dev_priv->num_fence_regs = 8;
>>
>> - if (intel_vgpu_active(dev))
>> + if (intel_gvt_active(dev))
>> + dev_priv->num_fence_regs = dev_priv->gvt.host_fence_sz;
>> + else if (intel_vgpu_active(dev))
>> dev_priv->num_fence_regs =
>> I915_READ(vgtif_reg(avail_rs.fence_num));
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 9127f8f..de09dd4 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -2734,7 +2734,7 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
>> i915_address_space_init(ggtt_vm, dev_priv);
>> ggtt_vm->total += PAGE_SIZE;
>>
>> - if (intel_vgpu_active(dev)) {
>> + if (intel_vgpu_active(dev) || intel_gvt_active(dev)) {
>> ret = intel_vgt_balloon(dev);
>> if (ret)
>> return ret;
>> @@ -2833,7 +2833,7 @@ void i915_global_gtt_cleanup(struct drm_device *dev)
>> i915_gem_cleanup_stolen(dev);
>>
>> if (drm_mm_initialized(&vm->mm)) {
>> - if (intel_vgpu_active(dev))
>> + if (intel_vgpu_active(dev) || intel_gvt_active(dev))
>> intel_vgt_deballoon();
>>
>> drm_mm_takedown(&vm->mm);
>> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
>> index dea7429..7be1435 100644
>> --- a/drivers/gpu/drm/i915/i915_vgpu.c
>> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
>> @@ -188,10 +188,23 @@ int intel_vgt_balloon(struct drm_device *dev)
>> unsigned long unmappable_base, unmappable_size, unmappable_end;
>> int ret;
>>
>> - mappable_base = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.base));
>> - mappable_size = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.size));
>> - unmappable_base = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.base));
>> - unmappable_size = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.size));
>> + if (intel_gvt_active(dev)) {
>> + mappable_base = 0;
>> + mappable_size = dev_priv->gvt.host_low_gm_sz_in_mb << 20;
>> + unmappable_base = dev_priv->gtt.mappable_end;
>> + unmappable_size = dev_priv->gvt.host_high_gm_sz_in_mb << 20;
>> + } else if (intel_vgpu_active(dev)) {
>> + mappable_base = I915_READ(
>> + vgtif_reg(avail_rs.mappable_gmadr.base));
>> + mappable_size = I915_READ(
>> + vgtif_reg(avail_rs.mappable_gmadr.size));
>> + unmappable_base = I915_READ(
>> + vgtif_reg(avail_rs.nonmappable_gmadr.base));
>> + unmappable_size = I915_READ(
>> + vgtif_reg(avail_rs.nonmappable_gmadr.size));
>> + } else {
>> + return -ENODEV;
>> + }
>>
>> mappable_end = mappable_base + mappable_size;
>> unmappable_end = unmappable_base + unmappable_size;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-02-23 13:25 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-18 11:42 [RFCv2 PATCH 00/14] gvt: Hacking i915 for GVT context requirement Zhi Wang
2016-02-18 11:42 ` [RFCv2 01/14] drm/i915: factor out i915_pvinfo.h Zhi Wang
2016-02-22 13:23 ` Joonas Lahtinen
2016-02-23 2:40 ` Zhi Wang
2016-02-18 11:42 ` [RFCv2 02/14] drm/i915/gvt: Introduce the basic architecture of GVT-g Zhi Wang
2016-02-23 12:42 ` Joonas Lahtinen
2016-02-24 7:45 ` Tian, Kevin
2016-02-25 11:24 ` Joonas Lahtinen
2016-02-26 5:58 ` Zhi Wang
2016-02-23 12:53 ` Joonas Lahtinen
2016-02-24 7:50 ` Tian, Kevin
2016-02-24 8:08 ` Tian, Kevin
2016-02-26 5:38 ` Zhi Wang
2016-02-18 11:42 ` [RFCv2 03/14] drm/i915: Introduce host graphics memory/fence partition for GVT-g Zhi Wang
2016-02-23 13:16 ` Joonas Lahtinen
2016-02-23 13:23 ` Zhi Wang [this message]
2016-02-24 7:42 ` Tian, Kevin
2016-02-25 13:13 ` Joonas Lahtinen
2016-02-26 5:21 ` Zhi Wang
2016-02-26 13:54 ` Joonas Lahtinen
2016-02-23 13:26 ` Joonas Lahtinen
2016-02-24 8:22 ` Tian, Kevin
2016-02-26 5:29 ` Zhi Wang
2016-02-18 11:42 ` [RFCv2 04/14] drm/i915: factor out alloc_context_idr() and __i915_gem_create_context() Zhi Wang
2016-02-24 8:27 ` Tian, Kevin
2016-02-18 11:42 ` [RFCv2 05/14] drm/i915: factor out __create_legacy_hw_context() Zhi Wang
2016-02-18 11:42 ` [RFCv2 06/14] drm/i915: let __i915_gem_context_create() takes context creation params Zhi Wang
2016-02-24 8:35 ` Tian, Kevin
2016-02-18 11:42 ` [RFCv2 07/14] drm/i915: factor out __intel_lr_context_deferred_alloc() Zhi Wang
2016-02-24 8:37 ` Tian, Kevin
2016-02-18 11:42 ` [RFCv2 08/14] drm/i915: Support per-PPGTT address space mode Zhi Wang
2016-02-24 8:47 ` Tian, Kevin
2016-02-18 11:42 ` [RFCv2 09/14] drm/i915: generate address mode bit from PPGTT instance Zhi Wang
2016-02-18 11:42 ` [RFCv2 10/14] drm/i915: update PDPs by condition when submit the LRC context Zhi Wang
2016-02-24 8:49 ` Tian, Kevin
2016-02-25 15:02 ` Wang, Zhi A
2016-02-26 13:49 ` Joonas Lahtinen
2016-02-18 11:42 ` [RFCv2 11/14] drm/i915: Introduce execlist context status change notification Zhi Wang
2016-02-18 11:42 ` [RFCv2 12/14] drm/i915: factor out execlists_i915_pick_requests() Zhi Wang
2016-02-18 11:42 ` [RFCv2 13/14] drm/i915: Support context single submission when GVT is active Zhi Wang
2016-02-24 8:52 ` Tian, Kevin
2016-02-18 11:42 ` [RFCv2 14/14] drm/i915: Introduce GVT context creation API Zhi Wang
2016-02-18 12:02 ` ✗ Fi.CI.BAT: failure for gvt: Hacking i915 for GVT context requirement Patchwork
2016-02-24 8:55 ` [RFCv2 PATCH 00/14] " Tian, Kevin
2016-02-24 9:18 ` Wang, Zhi A
2016-02-24 9:38 ` Tian, Kevin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56CC5D39.7090408@intel.com \
--to=zhi.a.wang@intel.com \
--cc=daniel.vetter@ffwll.ch \
--cc=david.j.cowperthwaite@intel.com \
--cc=igvt-g@lists.01.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=joonas.lahtinen@linux.intel.com \
--cc=zhiyuan.lv@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).