[Intel-gfx] [RFCv2 02/14] drm/i915/gvt: Introduce the basic architecture of GVT-g
Joonas Lahtinen
joonas.lahtinen at linux.intel.com
Tue Feb 23 12:42:12 UTC 2016
Hi,
Code is looking a lot better.
A general question still; why you seem to be preferring multi-stage
alloc and destroy?
Are there going to be scenarios when things will be allocated but not
initialized? I don't see a such use scenario.
I wouldn't split the init functions down as much as you currently do
because that'll require a lot of boilerplate code to propagate the
errors up, which is currently not done. The boilerplate for propagation
becomes necessary when the teardown function is complex, but currently
the teardown itself is less lines of code than the function
boilerplate.
So just squash those into gvt_device_create() and gvt_device_destroy()
where _create() will propagate any lower level errors up and tear down
a partially initialized struct. _destroy() can then expect to just tear
the whole struct down with no ifs.
Regards, Joonas
On to, 2016-02-18 at 19:42 +0800, Zhi Wang wrote:
> This patch introduces the very basic framework of GVT-g device model,
> includes basic prototypes, definitions, initialization.
>
> v2:
> - Introduce i915_gvt.c.
> It's necessary to introduce the stubs between i915 driver and GVT-g host,
> as GVT-g components is configurable in kernel config. When disabled, the
> stubs here do nothing.
>
> Take Joonas's comments:
> - Replace boolean return value with int.
> - Replace customized info/warn/debug macros with DRM macros.
> - Document all non-static functions like i915.
> - Remove empty and unused functions.
> - Replace magic number with marcos.
> - Set GVT-g in kernel config to "n" by default.
>
> Signed-off-by: Zhi Wang <zhi.a.wang at intel.com>
> ---
> drivers/gpu/drm/i915/Kconfig | 15 ++
> drivers/gpu/drm/i915/Makefile | 2 +
> drivers/gpu/drm/i915/gvt/Makefile | 5 +
> drivers/gpu/drm/i915/gvt/debug.h | 57 +++++
> drivers/gpu/drm/i915/gvt/gvt.c | 393 +++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/gvt/gvt.h | 100 +++++++++
> drivers/gpu/drm/i915/gvt/hypercall.h | 30 +++
> drivers/gpu/drm/i915/gvt/mpt.h | 34 +++
> drivers/gpu/drm/i915/gvt/params.c | 32 +++
> drivers/gpu/drm/i915/gvt/params.h | 34 +++
> drivers/gpu/drm/i915/gvt/reg.h | 34 +++
> drivers/gpu/drm/i915/i915_dma.c | 14 ++
> drivers/gpu/drm/i915/i915_drv.h | 12 ++
> drivers/gpu/drm/i915/i915_gvt.c | 93 +++++++++
> drivers/gpu/drm/i915/i915_gvt.h | 49 +++++
> 15 files changed, 904 insertions(+)
> create mode 100644 drivers/gpu/drm/i915/gvt/Makefile
> create mode 100644 drivers/gpu/drm/i915/gvt/debug.h
> create mode 100644 drivers/gpu/drm/i915/gvt/gvt.c
> create mode 100644 drivers/gpu/drm/i915/gvt/gvt.h
> create mode 100644 drivers/gpu/drm/i915/gvt/hypercall.h
> create mode 100644 drivers/gpu/drm/i915/gvt/mpt.h
> create mode 100644 drivers/gpu/drm/i915/gvt/params.c
> create mode 100644 drivers/gpu/drm/i915/gvt/params.h
> create mode 100644 drivers/gpu/drm/i915/gvt/reg.h
> create mode 100644 drivers/gpu/drm/i915/i915_gvt.c
> create mode 100644 drivers/gpu/drm/i915/i915_gvt.h
>
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 4c59793..082e77d 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -45,3 +45,18 @@ config DRM_I915_PRELIMINARY_HW_SUPPORT
> option changes the default for that module option.
>
> If in doubt, say "N".
> +
> +config DRM_I915_GVT
> + tristate "GVT-g host driver"
> + depends on DRM_I915
> + default n
> + help
> + Enabling GVT-g mediated graphics passthrough technique for Intel i915
> + based integrated graphics card. With GVT-g, it's possible to have one
> + integrated i915 device shared by multiple VMs. Performance critical
> + opterations such as apperture accesses and ring buffer operations
> + are pass-throughed to VM, with a minimal set of conflicting resources
> + (e.g. display settings) mediated by vGT driver. The benefit of vGT
> + is on both the performance, given that each VM could directly operate
> + its aperture space and submit commands like running on native, and
> + the feature completeness, given that a true GEN hardware is exposed.
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 0851de07..c65026c 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -91,6 +91,8 @@ i915-y += dvo_ch7017.o \
> intel_sdvo.o \
> intel_tv.o
>
> +obj-$(CONFIG_DRM_I915_GVT) += i915_gvt.o gvt/
> +
> # virtual gpu code
> i915-y += i915_vgpu.o
>
> diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
> new file mode 100644
> index 0000000..959305f
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/Makefile
> @@ -0,0 +1,5 @@
> +GVT_SOURCE := gvt.o params.o
> +
> +ccflags-y += -I$(src) -I$(src)/.. -Wall -Werror -Wno-unused-function
> +i915_gvt-y := $(GVT_SOURCE)
> +obj-$(CONFIG_DRM_I915_GVT) += i915_gvt.o
> diff --git a/drivers/gpu/drm/i915/gvt/debug.h b/drivers/gpu/drm/i915/gvt/debug.h
> new file mode 100644
> index 0000000..0747f28
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/debug.h
> @@ -0,0 +1,57 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef __GVT_DEBUG_H__
> +#define __GVT_DEBUG_H__
> +
> +#define gvt_info(fmt, args...) \
> + DRM_INFO("gvt: "fmt"\n", ##args)
> +
Just;
DRM_INFO("gvt: " fmt, ##args)
Do not automatically add newlines, it will confuse developers. Applies
to all printing.
> +#define gvt_err(fmt, args...) \
> + DRM_ERROR("gvt: "fmt"\n", ##args)
> +
Same here.
> +#define gvt_warn(condition, fmt, args...) \
> + WARN((condition), "gvt: "fmt"\n", ##args)
> +
> +#define gvt_warn_once(condition, fmt, args...) \
> + WARN_ONCE((condition), "gvt: "fmt"\n", ##args)
WARN and WARN_ONCE will include backtrace so prefixing is unnecessary.
I would not prefix them at all. Just use what i915 kernel module
already uses. If needed, split them to their own file first,
i915_debug.h.
> +
> +#define gvt_dbg(level, fmt, args...) \
> + DRM_DEBUG_DRIVER("gvt: "fmt"\n", ##args)
> +
> +enum {
> + GVT_DBG_CORE = (1 << 0),
> + GVT_DBG_MM = (1 << 1),
> + GVT_DBG_IRQ = (1 << 2),
> +};
This enum is not of use.
> +
> +#define gvt_dbg_core(fmt, args...) \
> + gvt_dbg(GVT_DBG_CORE, fmt, ##args)
> +
Rahter like this (not to lose the topic)?
gvt_dbg("core: " fmt, ##args)
> +#define gvt_dbg_mm(fmt, args...) \
> + gvt_dbg(GVT_DBG_MM, fmt, ##args)
> +
> +#define gvt_dbg_irq(fmt, args...) \
> + gvt_dbg(GVT_DBG_IRQ, fmt, ##args)
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> new file mode 100644
> index 0000000..52cfa32
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -0,0 +1,393 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include
> +#include
> +#include
> +
> +#include "gvt.h"
> +
> +struct gvt_host gvt_host;
> +
> +static const char * const supported_hypervisors[] = {
> + [GVT_HYPERVISOR_TYPE_XEN] = "Xen Hypervisor",
> + [GVT_HYPERVISOR_TYPE_KVM] = "KVM",
> +};
> +
> +static int gvt_init_host(void)
> +{
> + struct gvt_host *host = &gvt_host;
> +
Is it really that much more to write gvt_host.initialized? Counting the
"->" vs "." it's three letters...
> + if (!gvt.enable) {
> + gvt_dbg_core("GVT-g has been disabled by kernel parameter");
> + return -EINVAL;
> + }
> +
> + if (host->initialized) {
> + gvt_err("GVT-g has already been initialized!");
> + return -EINVAL;
> + }
> +
> + /* Xen DOM U */
> + if (xen_domain() && !xen_initial_domain())
> + return -ENODEV;
> +
> + if (xen_initial_domain()) {
> + /* Xen Dom0 */
> + host->kdm = try_then_request_module(
> + symbol_get(xengt_kdm), "xengt");
> + host->hypervisor_type = GVT_HYPERVISOR_TYPE_XEN;
> + } else {
> + /* not in Xen. Try KVMGT */
> + host->kdm = try_then_request_module(
> + symbol_get(kvmgt_kdm), "kvm");
> + host->hypervisor_type = GVT_HYPERVISOR_TYPE_KVM;
> + }
> +
> + if (!host->kdm)
> + return -EINVAL;
I think this error should be reported, to aid detecting problems in
module loading.
> +
> + if (!hypervisor_detect_host())
> + return -ENODEV;
> +
This func should be prefixed gvt_, as it is not local to this file.
> + gvt_info("Running with hypervisor %s in host mode",
> + supported_hypervisors[host->hypervisor_type]);
> +
> + idr_init(&host->device_idr);
> + mutex_init(&host->device_idr_lock);
> +
> + host->initialized = true;
> + return 0;
> +}
> +
> +static int init_device_info(struct pgt_device *pdev)
> +{
> + struct gvt_device_info *info = &pdev->device_info;
> +
> + if (!IS_BROADWELL(pdev->dev_priv)) {
> + DRM_DEBUG_DRIVER("Unsupported GEN device");
> + return -ENODEV;
> + }
This could be "else" clause on the next if and will allow easier adding
of future platforms.
> +
> + if (IS_BROADWELL(pdev->dev_priv)) {
> + info->max_gtt_gm_sz = (1ULL << 32); /* 4GB */
> + /*
> + * The layout of BAR0 in BDW:
> + * |< - MMIO 2MB ->|<- Reserved 6MB ->|<- MAX GTT 8MB->|
> + *
> + * GTT offset in BAR0 starts from 8MB to 16MB, and
> + * Whatever GTT size is configured in BIOS,
> + * the size of BAR0 is always 16MB. The actual configured
> + * GTT size can be found in GMCH_CTRL.
> + */
> + info->gtt_start_offset = (1UL << 23); /* 8MB */
> + info->max_gtt_size = (1UL << 23); /* 8MB */
> + info->gtt_entry_size = 8;
> + info->gtt_entry_size_shift = 3;
> + info->gmadr_bytes_in_cmd = 8;
> + info->mmio_size = 2 * 1024 * 1024; /* 2MB */
> + }
> +
> + return 0;
> +}
> +
> +static void init_initial_cfg_space_state(struct pgt_device *pdev)
> +{
> + struct pci_dev *pci_dev = pdev->dev_priv->dev->pdev;
> + int i;
> +
> + gvt_dbg_core("init initial cfg space, id %d", pdev->id);
> +
> + for (i = 0; i < GVT_CFG_SPACE_SZ; i += 4)
> + pci_read_config_dword(pci_dev, i,
> + (u32 *)&pdev->initial_cfg_space[i]);
> +
> + for (i = 0; i < GVT_BAR_NUM; i++) {
> + pdev->bar_size[i] = pci_resource_len(pci_dev, i * 2);
> + gvt_dbg_core("bar %d size: %llx", i, pdev->bar_size[i]);
> + }
> +}
> +
> +static void clean_initial_mmio_state(struct pgt_device *pdev)
> +{
> + if (pdev->gttmmio_va) {
> + iounmap(pdev->gttmmio_va);
> + pdev->gttmmio_va = NULL;
> + }
> +
> + if (pdev->gmadr_va) {
> + iounmap(pdev->gmadr_va);
> + pdev->gmadr_va = NULL;
> + }
> +}
> +
> +static int init_initial_mmio_state(struct pgt_device *pdev)
> +{
> + struct gvt_device_info *info = &pdev->device_info;
> +
> + u64 bar0, bar1;
> + int rc;
> +
> + gvt_dbg_core("init initial mmio state, id %d", pdev->id);
> +
> + bar0 = *(u64 *)&pdev->initial_cfg_space[GVT_REG_CFG_SPACE_BAR0];
> + bar1 = *(u64 *)&pdev->initial_cfg_space[GVT_REG_CFG_SPACE_BAR1];
> +
> + pdev->gttmmio_base = bar0 & ~0xf;
> + pdev->reg_num = info->mmio_size / 4;
> + pdev->gmadr_base = bar1 & ~0xf;
Magic numbers still.
> +
> + pdev->gttmmio_va = ioremap(pdev->gttmmio_base, pdev->bar_size[0]);
> + if (!pdev->gttmmio_va) {
> + gvt_err("fail to map GTTMMIO BAR.");
> + return -EFAULT;
> + }
> +
> + pdev->gmadr_va = ioremap(pdev->gmadr_base, pdev->bar_size[2]);
> + if (!pdev->gmadr_va) {
> + gvt_err("fail to map GMADR BAR.");
> + rc = -EFAULT;
> + goto err;
> + }
> +
> + gvt_dbg_core("bar0: 0x%llx, bar1: 0x%llx", bar0, bar1);
> + gvt_dbg_core("mmio size: %x", pdev->mmio_size);
> + gvt_dbg_core("gttmmio: 0x%llx, gmadr: 0x%llx", pdev->gttmmio_base,
> + pdev->gmadr_base);
> + gvt_dbg_core("gttmmio_va: %p", pdev->gttmmio_va);
> + gvt_dbg_core("gmadr_va: %p", pdev->gmadr_va);
> +
> + return 0;
> +err:
> + clean_initial_mmio_state(pdev);
> + return rc;
> +}
> +
> +static int gvt_service_thread(void *data)
> +{
> + struct pgt_device *pdev = (struct pgt_device *)data;
> + int r;
> +
> + gvt_dbg_core("service thread start, pgt %d", pdev->id);
> +
> + while (!kthread_should_stop()) {
> + r = wait_event_interruptible(pdev->service_thread_wq,
> + kthread_should_stop() || pdev->service_request);
> +
> + if (kthread_should_stop())
> + break;
> +
> + if (gvt_warn_once(r,
> + "service thread is waken up by unexpected signal."))
> + continue;
> + }
> +
> + return 0;
> +}
> +
> +static void clean_service_thread(struct pgt_device *pdev)
> +{
> + if (pdev->service_thread) {
> + kthread_stop(pdev->service_thread);
> + pdev->service_thread = NULL;
> + }
> +}
> +
> +static int init_service_thread(struct pgt_device *pdev)
> +{
> + init_waitqueue_head(&pdev->service_thread_wq);
> +
> + pdev->service_thread = kthread_run(gvt_service_thread,
> + pdev, "gvt_service_thread%d", pdev->id);
> +
> + if (!pdev->service_thread) {
> + gvt_err("fail to start service thread.");
> + return -ENOSPC;
> + }
> +
> + return 0;
> +}
> +
> +static void clean_pgt_device(struct pgt_device *pdev)
> +{
> + clean_service_thread(pdev);
> + clean_initial_mmio_state(pdev);
> +}
> +
> +static int init_pgt_device(struct pgt_device *pdev,
> + struct drm_i915_private *dev_priv)
> +{
> + int rc;
int ret;
> +
> + rc = init_device_info(pdev);
> + if (rc)
> + return rc;
> +
> + init_initial_cfg_space_state(pdev);
> +
> + rc = init_initial_mmio_state(pdev);
> + if (rc)
> + goto err;
> +
> + rc = init_service_thread(pdev);
> + if (rc)
> + goto err;
> +
> + return 0;
> +err:
> + clean_pgt_device(pdev);
Add teardown path, see below.
> + return rc;
> +}
> +
> +static int post_init_pgt_device(struct pgt_device *pdev)
> +{
> + return 0;
> +}
> +
> +static void free_pgt_device(struct pgt_device *pdev)
> +{
> + struct gvt_host *host = &gvt_host;
> +
> + mutex_lock(&host->device_idr_lock);
> + idr_remove(&host->device_idr, pdev->id);
> + mutex_unlock(&host->device_idr_lock);
> +
> + vfree(pdev);
> +}
> +
> +static struct pgt_device *alloc_pgt_device(struct drm_i915_private *dev_priv)
> +{
> + struct gvt_host *host = &gvt_host;
> + struct pgt_device *pdev = NULL;
> +
> + pdev = vzalloc(sizeof(*pdev));
> + if (!pdev)
> + return NULL;
This is a memory error, would like to see this propagated up as
return ERR_PTR(-ENOMEM);
> +
> + mutex_lock(&host->device_idr_lock);
> + pdev->id = idr_alloc(&host->device_idr, pdev, 0, 0, GFP_KERNEL);
> + mutex_unlock(&host->device_idr_lock);
> +
> + if (pdev->id < 0) {
> + gvt_err("fail to allocate pgt device id.");
> + goto err;
> + }
> +
Same here, propagate the error.
> + mutex_init(&pdev->lock);
> + pdev->dev_priv = dev_priv;
> + idr_init(&pdev->instance_idr);
> +
> + return pdev;
> +err:
> + free_pgt_device(pdev);
I'd like to see a teardown path here. Then free_pgt_device() (or rather
destroy_pgt_device() can expect everything to be initialized and when
it it is called, it doesn't need ifs. This makes the driver code more
robust.
Or are we expecting only partially initialized structs for some reason?
> + return NULL;
> +}
> +
> +void gvt_destroy_pgt_device(void *private_data)
> +{
> + struct pgt_device *pdev = (struct pgt_device *)private_data;
> +
> + clean_pgt_device(pdev);
> + free_pgt_device(pdev);
Why multiple calls to destroy a device, there's only one alloc still?
> +}
> +
> +/**
> + * gvt_create_pgt_device - create a GVT physical device
> + * @dev: drm device
> + *
> + * This function is called at the initialization stage, to create a physical
> + * GVT device and initialize necessary GVT components for it.
> + *
> + * Returns:
> + * pointer to the pgt_device structure, NULL if failed.
> + */
> +void *gvt_create_pgt_device(struct drm_i915_private *dev_priv)
> +{
> + struct pgt_device *pdev = NULL;
> + struct gvt_host *host = &gvt_host;
> + int rc;
> +
> + if (!host->initialized && !gvt_init_host()) {
> + gvt_err("gvt_init_host fail");
> + return NULL;
> + }
> +
> + gvt_dbg_core("create new pgt device, i915 dev_priv: %p", dev_priv);
> +
> + pdev = alloc_pgt_device(dev_priv);
> + if (!pdev) {
> + gvt_err("fail to allocate memory for pgt device.");
> + goto err;
> + }
> +
> + gvt_dbg_core("init pgt device, id %d", pdev->id);
> +
> + rc = init_pgt_device(pdev, dev_priv);
> + if (rc) {
Just call the return value variable ret like everywhere.
> + gvt_err("fail to init physical device state.");
> + goto err;
> + }
> +
> + gvt_dbg_core("pgt device creation done, id %d", pdev->id);
> +
> + return pdev;
> +err:
> + if (pdev) {
> + gvt_destroy_pgt_device(pdev);
> + pdev = NULL;
> + }
Proper goto label based teardown path should be used.
err_destroy_pgt:
gvt_destroy_pgt_device(pdev);
err:
> + return NULL;
> +}
> +
> +/**
> + * gvt_post_init_pgt_device - post init a GVT physical device
> + * @dev: drm device
Double check the kerneldocs to be correct per arguments of function.
> + *
> + * This function is called at the end of the initialization stage, to
> + * post-initialize a physical GVT device and initialize necessary
> + * GVT components rely on i915 components.
> + *
> + * Returns:
> + * zero on success, non-zero if failed.
> + */
> +int gvt_post_init_pgt_device(void *private_data)
> +{
> + struct pgt_device *pdev = (struct pgt_device *)private_data;
> + struct gvt_host *host = &gvt_host;
> + int rc;
> +
> + if (!host->initialized) {
> + gvt_err("gvt_host haven't been initialized.");
> + return -ENODEV;
> + }
> +
> + gvt_dbg_core("post init pgt device %d", pdev->id);
> +
> + rc = post_init_pgt_device(pdev);
> + if (rc) {
> + gvt_err("fail to post init physical device state.");
> + return rc;
> + }
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> new file mode 100644
> index 0000000..d450198
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -0,0 +1,100 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef _GVT_H_
> +#define _GVT_H_
> +
> +#include "i915_drv.h"
> +#include "i915_vgpu.h"
> +
> +#include "debug.h"
> +#include "params.h"
> +#include "reg.h"
> +#include "hypercall.h"
> +#include "mpt.h"
> +
> +#define GVT_MAX_VGPU 8
> +
> +enum {
> + GVT_HYPERVISOR_TYPE_XEN = 0,
> + GVT_HYPERVISOR_TYPE_KVM,
> +};
> +
> +struct gvt_host {
> + bool initialized;
> + int hypervisor_type;
> + struct mutex device_idr_lock;
> + struct idr device_idr;
> + struct gvt_kernel_dm *kdm;
> +};
> +
> +extern struct gvt_host gvt_host;
-->
> +extern struct gvt_kernel_dm xengt_kdm;
> +extern struct gvt_kernel_dm kvmgt_kdm;
<-- These should likely be declared somewhere in include/ rather than
here.
> +
> +/* Describe the limitation of HW.*/
> +struct gvt_device_info {
> + u64 max_gtt_gm_sz;
> + u32 gtt_start_offset;
> + u32 gtt_end_offset;
> + u32 max_gtt_size;
> + u32 gtt_entry_size;
> + u32 gtt_entry_size_shift;
> + u32 gmadr_bytes_in_cmd;
> + u32 mmio_size;
> +};
> +
> +struct vgt_device {
> + int id;
> + int vm_id;
> + struct pgt_device *pdev;
> + bool warn_untrack;
> +};
> +
> +struct pgt_device {
Comments to this and other structs about what the memebers are.
> + struct mutex lock;
> + int id;
> +
> + struct drm_i915_private *dev_priv;
> + struct idr instance_idr;
> +
> + struct gvt_device_info device_info;
> +
> + u8 initial_cfg_space[GVT_CFG_SPACE_SZ];
> + u64 bar_size[GVT_BAR_NUM];
> +
> + u64 gttmmio_base;
> + void *gttmmio_va;
> +
> + u64 gmadr_base;
> + void *gmadr_va;
> +
> + u32 mmio_size;
> + u32 reg_num;
> +
> + wait_queue_head_t service_thread_wq;
> + struct task_struct *service_thread;
> + unsigned long service_request;
> +};
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
> new file mode 100644
> index 0000000..0a41874
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/hypercall.h
> @@ -0,0 +1,30 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef _GVT_HYPERCALL_H_
> +#define _GVT_HYPERCALL_H_
> +
> +struct gvt_kernel_dm {
> +};
I would prefer more code introduced here in the initial patch, or this
can be introduced later in the series as whole.
It unnecessarily complicates the review if some files and calls are
introduced with no documentation and implementation and only later
their implementation is added.
I can't really review if using a structure is a good idea if I can't
see the context or implementation of their use.
> +
> +#endif /* _GVT_HYPERCALL_H_ */
> diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
> new file mode 100644
> index 0000000..e594bb8
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/mpt.h
> @@ -0,0 +1,34 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef _GVT_MPT_H_
> +#define _GVT_MPT_H_
> +
> +struct vgt_device;
> +
> +static inline bool hypervisor_detect_host(void)
> +{
> + return false;
> +}
This is also not very reviewable and there's an unnecessary forward
declaration.
> +
> +#endif /* _GVT_MPT_H_ */
> diff --git a/drivers/gpu/drm/i915/gvt/params.c b/drivers/gpu/drm/i915/gvt/params.c
> new file mode 100644
> index 0000000..d381d17
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/params.c
> @@ -0,0 +1,32 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include "gvt.h"
> +
> +struct gvt_kernel_params gvt = {
> + .enable = false,
> + .debug = 0,
> +};
> +
> +module_param_named(gvt_enable, gvt.enable, bool, 0600);
This should just be
module_param_named(enable, gvt.enable, bool, ...)
otherwise parameter has to be passed at boot time like this:
gvt.gvt_enable=1
Where we want
gvt.enable=1
Right?
> +MODULE_PARM_DESC(gvt_enable, "Enable Intel GVT-g host support");
> diff --git a/drivers/gpu/drm/i915/gvt/params.h b/drivers/gpu/drm/i915/gvt/params.h
> new file mode 100644
> index 0000000..d2955b9
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/params.h
> @@ -0,0 +1,34 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef _GVT_PARAMS_H_
> +#define _GVT_PARAMS_H_
> +
> +struct gvt_kernel_params {
> + bool enable;
> + int debug;
This member is unused currently, can be dropped.
> +};
> +
> +extern struct gvt_kernel_params gvt;
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/gvt/reg.h b/drivers/gpu/drm/i915/gvt/reg.h
> new file mode 100644
> index 0000000..d363b74
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/reg.h
> @@ -0,0 +1,34 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef _GVT_REG_H
> +#define _GVT_REG_H
> +
> +#define GVT_CFG_SPACE_SZ 256
> +#define GVT_BAR_NUM 4
> +
> +#define GVT_REG_CFG_SPACE_BAR0 0x10
> +#define GVT_REG_CFG_SPACE_BAR1 0x18
> +#define GVT_REG_CFG_SPACE_BAR2 0x20
Some documentation here would be great.
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 1c6d227..f3bed37 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -35,6 +35,7 @@
> #include "intel_drv.h"
> #include
> #include "i915_drv.h"
> +#include "i915_gvt.h"
> #include "i915_vgpu.h"
> #include "i915_trace.h"
> #include
> @@ -1045,6 +1046,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>
> intel_uncore_init(dev);
>
> + ret = intel_gvt_init(dev);
> + if (ret)
> + goto out_uncore_fini;
> +
> ret = i915_gem_gtt_init(dev);
> if (ret)
> goto out_uncore_fini;
This needs to become "goto out_gvt_cleanup;"
> @@ -1135,6 +1140,12 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> goto out_power_well;
> }
>
> + ret = intel_gvt_post_init(dev);
> + if (ret) {
> + DRM_ERROR("failed to post init pgt device\n");
> + goto out_power_well;
> + }
> +
> /*
> * Notify a valid surface after modesetting,
> * when running inside a VM.
> @@ -1177,6 +1188,7 @@ out_gem_unload:
> out_gtt:
> i915_global_gtt_cleanup(dev);
out_gvt_cleanup:
> out_uncore_fini:
>
> + intel_gvt_cleanup(dev);
This needs to be lifted up to its own label ensure proper teardown if
i915_gem_gtt_init() fails.
> intel_uncore_fini(dev);
> i915_mmio_cleanup(dev);
> put_bridge:
> @@ -1223,6 +1235,8 @@ int i915_driver_unload(struct drm_device *dev)
>
> intel_modeset_cleanup(dev);
>
> + intel_gvt_cleanup(dev);
> +
> /*
> * free the memory space allocated for the child device
> * config parsed from VBT
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 20d9dbd..2f897c3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1705,6 +1705,10 @@ struct i915_workarounds {
> u32 hw_whitelist_count[I915_NUM_RINGS];
> };
>
> +struct i915_gvt {
> + void *pgt_device;
> +};
> +
> struct i915_virtual_gpu {
> bool active;
> };
> @@ -1744,6 +1748,8 @@ struct drm_i915_private {
>
> struct i915_virtual_gpu vgpu;
>
> + struct i915_gvt gvt;
> +
> struct intel_guc guc;
>
> struct intel_csr csr;
> @@ -2780,6 +2786,12 @@ void intel_uncore_forcewake_get__locked(struct drm_i915_private *dev_priv,
> void intel_uncore_forcewake_put__locked(struct drm_i915_private *dev_priv,
> enum forcewake_domains domains);
> void assert_forcewakes_inactive(struct drm_i915_private *dev_priv);
> +
> +static inline bool intel_gvt_active(struct drm_device *dev)
> +{
> + return to_i915(dev)->gvt.pgt_device ? true : false;
> +}
> +
> static inline bool intel_vgpu_active(struct drm_device *dev)
> {
> return to_i915(dev)->vgpu.active;
> diff --git a/drivers/gpu/drm/i915/i915_gvt.c b/drivers/gpu/drm/i915/i915_gvt.c
> new file mode 100644
> index 0000000..3ca7232
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_gvt.c
> @@ -0,0 +1,93 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include "i915_drv.h"
> +#include "i915_gvt.h"
> +
> +/**
> + * DOC: Intel GVT-g host support
> + *
> + * Intel GVT-g is a graphics virtualization technology which shares the
> + * GPU among multiple virtual machines on a time-sharing basis. Each
> + * virtual machine is presented a virtual GPU (vGPU), which has equivalent
> + * features as the underlying physical GPU (pGPU), so i915 driver can run
> + * seamlessly in a virtual machine. This file provides the englightments
> + * of GVT and the necessary components used by GVT in i915 driver.
> + */
> +
> +/**
> + * intel_gvt_init - initialize GVT components at the beginning of i915
> + * driver loading.
> + * @dev: drm device *
> + *
> + * This function is called at the beginning of the initialization stage,
> + * to initialize the GVT components that have to be initialized
> + * before HW gets touched by other i915 components.
> + */
> +int intel_gvt_init(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + dev_priv->gvt.pgt_device = gvt_create_pgt_device(dev_priv);
> + if (intel_gvt_active(dev))
> + DRM_DEBUG_DRIVER("GVT-g is running in host mode\n");
> +
> + return 0;
> +}
> +
> +/**
> + * intel_gvt_post_init - initialize GVT components at the end of i915
> + * driver loading.
> + * @dev: drm device *
> + *
> + * This function is called at the end of the initialization stage,
> + * to initialize the GVT components that have to be initialized after
> + * other i915 components are ready.
> + */
> +int intel_gvt_post_init(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + if (!intel_gvt_active(dev))
> + return 0;
> +
> + return gvt_post_init_pgt_device(dev_priv->gvt.pgt_device);
> +}
> +
> +/**
> + * intel_gvt_cleanup - cleanup GVT components when i915 driver is unloading
> + * @dev: drm device *
> + *
> + * This function is called at the i915 driver unloading stage, to shutdown
> + * GVT components and release the related resources.
> + */
> +void intel_gvt_cleanup(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + if (!intel_gvt_active(dev))
> + return;
> +
> + gvt_destroy_pgt_device(dev_priv->gvt.pgt_device);
> + dev_priv->gvt.pgt_device = NULL;
> +}
> diff --git a/drivers/gpu/drm/i915/i915_gvt.h b/drivers/gpu/drm/i915/i915_gvt.h
> new file mode 100644
> index 0000000..30cc207
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_gvt.h
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef _I915_GVT_H_
> +#define _I915_GVT_H_
I think this file could be intel_gvt.h (all remaining functions are
intel_gvt), and have respective intel_gvt.c file.
> +
> +#ifdef CONFIG_DRM_I915_GVT
Starting here --->
> +extern void *gvt_create_pgt_device(struct drm_i915_private *dev_priv);
> +extern int gvt_post_init_pgt_device(void *data);
> +extern void gvt_destroy_pgt_device(void *data);
> +
<-- to here, these should go to their own include file at
include/drm/i915_gvt.h For an example, see include/drm/i915_drm.h
The respective export symbols then need to be exported, For an example
see;
EXPORT_SYMBOL_GPL(i915_gpu_raise);
> +extern int intel_gvt_init(struct drm_device *dev);
> +extern int intel_gvt_post_init(struct drm_device *dev);
> +extern void intel_gvt_cleanup(struct drm_device *dev);
> +#else
> +extern int intel_gvt_init(struct drm_device *dev)
These should, by convention, rather be static inline;
static inline int intel_gvt_init(...)
> +{
> + return 0;
> +}
> +extern int intel_gvt_post_init(struct drm_device *dev)
> +{
> + return 0;
> +}
> +extern void intel_gvt_cleanup(struct drm_device *dev)
> +{
> +}
> +#endif
> +
> +#endif /* _I915_GVT_H_ */
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
More information about the Intel-gfx
mailing list