Re: [PATCH v5 4/6] drm/i915/gvt: Deliver vGPU refresh event to userspace

From: Zhenyu Wang
Date: Mon Aug 26 2019 - 04:00:40 EST

Next message: Greg KH: "Re: Linux 5.2.10"
Previous message: Greg Kroah-Hartman: "Re: [PATCH v2 3/3] nvme: fire discovery log page change events to userspace"
Next in thread: Tian, Kevin: "RE: [PATCH v5 4/6] drm/i915/gvt: Deliver vGPU refresh event to userspace"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 2019.08.16 10:35:26 +0800, Tina Zhang wrote:
> Deliver the display refresh events to the user land. Userspace can use
> the irq mask/unmask mechanism to disable or enable the event delivery.
>
> As we know, delivering refresh event at each vblank safely avoids
> tearing and unexpected event overwhelming, but there are still spaces
> to optimize.
>
> For handling the normal case, deliver the page flip refresh
> event at each vblank, in other words, bounded by vblanks. Skipping some
> events bring performance enhancement while not hurting user experience.
>
> For single framebuffer case, deliver the refresh events to userspace at
> all vblanks. This heuristic at each vblank leverages pageflip_count
> incresements to determine if there is no page flip happens after a certain
> period and so that the case is regarded as single framebuffer one.
> Although this heuristic makes incorrect decision sometimes and it depends
> on guest behavior, for example, when no cursor movements happen, the
> user experience does not harm and front buffer is still correctly acquired.
> Meanwhile, in actual single framebuffer case, the user experience is
> enhanced compared with page flip events only.
>
> Addtionally, to mitigate the events delivering footprints, one eventfd and
> 8 byte eventfd counter partition are leveraged.
>
> v2:
> - Support vfio_irq_info_cap_display_plane_events. (Tina)
>
> Signed-off-by: Tina Zhang <tina.zhang@xxxxxxxxx>
> Signed-off-by: Kechen Lu <kechen.lu@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gvt/display.c | 22 ++++
> drivers/gpu/drm/i915/gvt/gvt.h | 2 +
> drivers/gpu/drm/i915/gvt/kvmgt.c | 159 +++++++++++++++++++++++++++--
> 3 files changed, 174 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
> index 1a0a4ae4826e..616285e4a014 100644
> --- a/drivers/gpu/drm/i915/gvt/display.c
> +++ b/drivers/gpu/drm/i915/gvt/display.c
> @@ -34,6 +34,8 @@
>
> #include "i915_drv.h"
> #include "gvt.h"
> +#include <uapi/linux/vfio.h>
> +#include <drm/drm_plane.h>
>
> static int get_edp_pipe(struct intel_vgpu *vgpu)
> {
> @@ -387,6 +389,8 @@ void intel_gvt_check_vblank_emulation(struct intel_gvt *gvt)
> mutex_unlock(&gvt->lock);
> }
>
> +#define PAGEFLIP_DELAY_THR 10
> +
> static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
> {
> struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> @@ -396,7 +400,10 @@ static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
> [PIPE_B] = PIPE_B_VBLANK,
> [PIPE_C] = PIPE_C_VBLANK,
> };
> + int pri_flip_event = SKL_FLIP_EVENT(pipe, PLANE_PRIMARY);
> int event;
> + u64 eventfd_signal_val = 0;
> + static int no_pageflip_count;
>
> if (pipe < PIPE_A || pipe > PIPE_C)
> return;
> @@ -407,11 +414,26 @@ static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
> if (!pipe_is_enabled(vgpu, pipe))
> continue;
>
> + if (event == pri_flip_event)
> + eventfd_signal_val |= DISPLAY_PRI_REFRESH_EVENT_VAL;
> +
> intel_vgpu_trigger_virtual_event(vgpu, event);
> }
>
> + if (eventfd_signal_val)
> + no_pageflip_count = 0;
> + else if (!eventfd_signal_val && no_pageflip_count > PAGEFLIP_DELAY_THR)

extra !eventfd_signal_val

> + eventfd_signal_val |= DISPLAY_PRI_REFRESH_EVENT_VAL;
> + else
> + no_pageflip_count++;

no_pageflip_count should be per-vgpu instead of static.

> +
> + if (vgpu->vdev.vblank_trigger && !vgpu->vdev.display_event_mask &&
> + eventfd_signal_val)
> + eventfd_signal(vgpu->vdev.vblank_trigger, eventfd_signal_val);
> +
> if (pipe_is_enabled(vgpu, pipe)) {
> vgpu_vreg_t(vgpu, PIPE_FRMCOUNT_G4X(pipe))++;
> +

extra line

> intel_vgpu_trigger_virtual_event(vgpu, vblank_event[pipe]);
> }
> }
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index cd29ea28d7ed..6c8ed030c30b 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -205,6 +205,8 @@ struct intel_vgpu {
> int num_irqs;
> struct eventfd_ctx *intx_trigger;
> struct eventfd_ctx *msi_trigger;
> + struct eventfd_ctx *vblank_trigger;
> + u32 display_event_mask;
>
> /*
> * Two caches are used to avoid mapping duplicated pages (eg.
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index fd1633342e53..9ace1f4ff9eb 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1250,6 +1250,8 @@ static int intel_vgpu_get_irq_count(struct intel_vgpu *vgpu, int type)
> {
> if (type == VFIO_PCI_INTX_IRQ_INDEX || type == VFIO_PCI_MSI_IRQ_INDEX)
> return 1;
> + else if (type < VFIO_PCI_NUM_IRQS + vgpu->vdev.num_irqs)
> + return vgpu->vdev.irq[type - VFIO_PCI_NUM_IRQS].count;
>
> return 0;
> }
> @@ -1297,7 +1299,60 @@ static int intel_vgpu_set_msi_trigger(struct intel_vgpu *vgpu,
> return 0;
> }
>
> -static int intel_vgpu_set_irqs(struct intel_vgpu *vgpu, u32 flags,
> +static int intel_vgu_set_display_irq_mask(struct intel_vgpu *vgpu,
> + unsigned int index, unsigned int start, unsigned int count,
> + u32 flags, void *data)
> +{
> + if (start != 0 || count > 2)
> + return -EINVAL;
> +
> + if (flags & VFIO_IRQ_SET_DATA_NONE)
> + vgpu->vdev.display_event_mask |= 1;

see below..

> +
> + return 0;
> +}
> +
> +static int intel_vgu_set_display_irq_unmask(struct intel_vgpu *vgpu,
> + unsigned int index, unsigned int start, unsigned int count,
> + u32 flags, void *data)
> +{
> + if (start != 0 || count > 2)
> + return -EINVAL;
> +
> + if (flags & VFIO_IRQ_SET_DATA_NONE)
> + vgpu->vdev.display_event_mask &= 0;

looks display_event_mask is used as flag for enable/disable, just write 1 or 0?

> +
> + return 0;
> +}
> +
> +static int intel_vgpu_set_display_event_trigger(struct intel_vgpu *vgpu,
> + unsigned int index, unsigned int start, unsigned int count,
> + u32 flags, void *data)
> +{
> + struct eventfd_ctx *trigger;
> +
> + if (flags & VFIO_IRQ_SET_DATA_EVENTFD) {
> + int fd = *(int *)data;
> +
> + trigger = eventfd_ctx_fdget(fd);
> + if (IS_ERR(trigger)) {
> + gvt_vgpu_err("eventfd_ctx_fdget failed\n");
> + return PTR_ERR(trigger);
> + }
> + vgpu->vdev.vblank_trigger = trigger;
> + vgpu->vdev.display_event_mask = 0;
> + } else if ((flags & VFIO_IRQ_SET_DATA_NONE) && !count) {
> + trigger = vgpu->vdev.vblank_trigger;
> + if (trigger) {
> + eventfd_ctx_put(trigger);
> + vgpu->vdev.vblank_trigger = NULL;
> + }
> + }
> +
> + return 0;
> +}
> +
> +int intel_vgpu_set_irqs(struct intel_vgpu *vgpu, u32 flags,
> unsigned int index, unsigned int start, unsigned int count,
> void *data)
> {
> @@ -1330,6 +1385,35 @@ static int intel_vgpu_set_irqs(struct intel_vgpu *vgpu, u32 flags,
> break;
> }
> break;
> + default:
> + {
> + int i;
> +
> + if (index >= VFIO_PCI_NUM_IRQS +
> + vgpu->vdev.num_irqs)
> + return -EINVAL;
> + index =
> + array_index_nospec(index,
> + VFIO_PCI_NUM_IRQS +
> + vgpu->vdev.num_irqs);
> +
> + i = index - VFIO_PCI_NUM_IRQS;
> + if (vgpu->vdev.irq[i].type == VFIO_IRQ_TYPE_GFX &&
> + vgpu->vdev.irq[i].subtype ==
> + VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ) {
> + switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) {
> + case VFIO_IRQ_SET_ACTION_MASK:
> + func = intel_vgu_set_display_irq_mask;
> + break;
> + case VFIO_IRQ_SET_ACTION_UNMASK:
> + func = intel_vgu_set_display_irq_unmask;
> + break;
> + case VFIO_IRQ_SET_ACTION_TRIGGER:
> + func = intel_vgpu_set_display_event_trigger;
> + break;
> + }
> + }
> + }
> }
>
> if (!func)
> @@ -1361,7 +1445,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
> info.flags |= VFIO_DEVICE_FLAGS_RESET;
> info.num_regions = VFIO_PCI_NUM_REGIONS +
> vgpu->vdev.num_regions;
> - info.num_irqs = VFIO_PCI_NUM_IRQS;
> + info.num_irqs = VFIO_PCI_NUM_IRQS + vgpu->vdev.num_irqs;
>
> return copy_to_user((void __user *)arg, &info, minsz) ?
> -EFAULT : 0;
> @@ -1521,32 +1605,88 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
> -EFAULT : 0;
> } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) {
> struct vfio_irq_info info;
> + struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
> + unsigned int i;
> + int ret;
>
> minsz = offsetofend(struct vfio_irq_info, count);
>
> if (copy_from_user(&info, (void __user *)arg, minsz))
> return -EFAULT;
>
> - if (info.argsz < minsz || info.index >= VFIO_PCI_NUM_IRQS)
> + if (info.argsz < minsz)
> return -EINVAL;
>
> switch (info.index) {
> case VFIO_PCI_INTX_IRQ_INDEX:
> case VFIO_PCI_MSI_IRQ_INDEX:
> + info.flags = VFIO_IRQ_INFO_EVENTFD;
> break;
> - default:
> + case VFIO_PCI_MSIX_IRQ_INDEX:
> + case VFIO_PCI_ERR_IRQ_INDEX:
> + case VFIO_PCI_REQ_IRQ_INDEX:
> return -EINVAL;
> - }
> + default:
> + {
> + struct vfio_irq_info_cap_type cap_type = {
> + .header.id = VFIO_IRQ_INFO_CAP_TYPE,
> + .header.version = 1 };
>
> - info.flags = VFIO_IRQ_INFO_EVENTFD;
> + if (info.index >= VFIO_PCI_NUM_IRQS +
> + vgpu->vdev.num_irqs)
> + return -EINVAL;
> + info.index =
> + array_index_nospec(info.index,
> + VFIO_PCI_NUM_IRQS +
> + vgpu->vdev.num_irqs);
> +
> + i = info.index - VFIO_PCI_NUM_IRQS;
> +
> + info.flags = vgpu->vdev.irq[i].flags;
> + cap_type.type = vgpu->vdev.irq[i].type;
> + cap_type.subtype = vgpu->vdev.irq[i].subtype;
> +
> + ret = vfio_info_add_capability(&caps,
> + &cap_type.header,
> + sizeof(cap_type));
> + if (ret)
> + return ret;
> +
> + if (vgpu->vdev.irq[i].ops->add_capability) {
> + ret = vgpu->vdev.irq[i].ops->add_capability(vgpu,
> + &caps);
> + if (ret)
> + return ret;
> + }
> + }
> + }
>
> info.count = intel_vgpu_get_irq_count(vgpu, info.index);
>
> if (info.index == VFIO_PCI_INTX_IRQ_INDEX)
> info.flags |= (VFIO_IRQ_INFO_MASKABLE |
> VFIO_IRQ_INFO_AUTOMASKED);
> - else
> - info.flags |= VFIO_IRQ_INFO_NORESIZE;
> +
> + if (caps.size) {
> + info.flags |= VFIO_IRQ_INFO_FLAG_CAPS;
> + if (info.argsz < sizeof(info) + caps.size) {
> + info.argsz = sizeof(info) + caps.size;
> + info.cap_offset = 0;
> + } else {
> + vfio_info_cap_shift(&caps, sizeof(info));
> + if (copy_to_user((void __user *)arg +
> + sizeof(info), caps.buf,
> + caps.size)) {
> + kfree(caps.buf);
> + return -EFAULT;
> + }
> + info.cap_offset = sizeof(info);
> + if (offsetofend(struct vfio_irq_info, cap_offset) > minsz)
> + minsz = offsetofend(struct vfio_irq_info, cap_offset);
> + }
> +
> + kfree(caps.buf);
> + }
>
> return copy_to_user((void __user *)arg, &info, minsz) ?
> -EFAULT : 0;
> @@ -1565,7 +1705,8 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
> int max = intel_vgpu_get_irq_count(vgpu, hdr.index);
>
> ret = vfio_set_irqs_validate_and_prepare(&hdr, max,
> - VFIO_PCI_NUM_IRQS, &data_size);
> + VFIO_PCI_NUM_IRQS + vgpu->vdev.num_irqs,
> + &data_size);
> if (ret) {
> gvt_vgpu_err("intel:vfio_set_irqs_validate_and_prepare failed\n");
> return -EINVAL;
> --
> 2.17.1
>
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

--
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

Attachment: signature.asc
Description: PGP signature

Next message: Greg KH: "Re: Linux 5.2.10"
Previous message: Greg Kroah-Hartman: "Re: [PATCH v2 3/3] nvme: fire discovery log page change events to userspace"
Next in thread: Tian, Kevin: "RE: [PATCH v5 4/6] drm/i915/gvt: Deliver vGPU refresh event to userspace"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]