> -----Original Message----- > From: Zhenyu Wang [mailto:zhenyuw@xxxxxxxxxxxxxxx] > Sent: Monday, December 3, 2018 4:21 PM > To: Zhang, Tina <tina.zhang@xxxxxxxxx> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Kondapally, Kalyan > <kalyan.kondapally@xxxxxxxxx>; intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx; Wang, Zhi > A <zhi.a.wang@xxxxxxxxx> > Subject: Re: [RFC PATCH 2/7] drm/i915/gvt: Use meta fbs to stand for vGPU's > planes > > On 2018.12.03 15:35:17 +0800, Tina Zhang wrote: > > Create and initialize vGPU meta framebuffers during vGPU's creation. > > Each meta framebuffer is an intel_framebuffer. Userspace can get the > > userspace visible identifier of that meta framebuffer by accessing > > plane_id_index attribute. > > > > For example: > > In "/sys/bus/pci/devices/0000\:00\:02.0/$vGPU_id/intel_vgpu/" > > directory, > > > > /* plane_id_index: pipe#(bit16:8) and plane#(bit7:0)*/ > > echo "0x10" > plane_index_id //Set the index to the plane 0 of pipe > > 1 > > > > /* > > * Dump userspace visible identifier of the meta framebuffer > > * standing for the primary plane of the vGPU's pipe one > > */ > > cat plane_index_id //dump the id for plane 0 of pipe 1 > > > > Then userspace can use this id with the exsting KMS IOCTL, e.g. > > drmModeSetPlane, to assign a physical plane to this virtual plane. > > How does user know which plane/pipe is active for vGPU? Looks from current > code it has no way to know that. I think for guest display assignment, we need > to know the display state of vGPU and it looks there's no state tracking, e.g what > if guest driver switches to another plane for display? How could user know that > then adjust for assignment? So far, GVT-g has supported multi-heads. In another word, there is only one priramy plane for each vGPU. GVT-g only provides the EDID for one fixed display port and in the guest point of view, there is only one pipe having a connected monitor, a.k.a only the planes on one pipe can be used by guest OS. > > And really don't like sysfs to r/w file for id, even if doesn't use vfio gfx dmabuf > interface for guest display object presentation, a vendor specific vfio device ioctl > if possible is better. Yeah, we could consider about that. Thanks. BR, Tina > > > > > Signed-off-by: Tina Zhang <tina.zhang@xxxxxxxxx> > > Cc: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx> > > Cc: Zhi Wang <zhi.a.wang@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/gvt/display.c | 150 > +++++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/i915/gvt/gvt.h | 16 ++++ > > drivers/gpu/drm/i915/gvt/kvmgt.c | 46 ++++++++++++ > > 3 files changed, 212 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/gvt/display.c > > b/drivers/gpu/drm/i915/gvt/display.c > > index df1e141..a9176a1 100644 > > --- a/drivers/gpu/drm/i915/gvt/display.c > > +++ b/drivers/gpu/drm/i915/gvt/display.c > > @@ -442,6 +442,152 @@ void intel_gvt_emulate_vblank(struct intel_gvt > *gvt) > > mutex_unlock(&gvt->lock); > > } > > > > +struct intel_vgpu_fb_meta_data { > > + u32 vgpu_plane_id; /* > vgpu_id(23:16):vgpu_pipe(15:8):vgpu_plane(7:0)*/ > > + struct intel_vgpu *vgpu; // the vGPU this meta_fb belongs to }; > > + > > +static void meta_fb_destroy(struct drm_framebuffer *fb) { > > + struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); > > + > > + if (intel_fb->meta_fb.type_id != INTEL_META_FB_VGPU) > > + return; > > + > > + kfree(intel_fb->meta_fb.private); > > + intel_fb->meta_fb.private = NULL; > > + > > + drm_framebuffer_cleanup(fb); > > + kfree(intel_fb); > > +} > > + > > +static void clean_meta_fb(struct intel_vgpu *vgpu) { > > + enum pipe pipe; > > + enum plane_id plane_id; > > + struct intel_framebuffer *intel_fb; > > + > > + for (pipe = 0; pipe < I915_MAX_PIPES; pipe++) { > > + for (plane_id = 0; plane_id < I915_MAX_PLANES; plane_id++) { > > + intel_fb = vgpu- > >display.meta_fbs.meta_fb[pipe][plane_id]; > > + if (!intel_fb) > > + drm_framebuffer_put(&intel_fb->base); > > + > > + intel_fb = NULL; > > + } > > + } > > +} > > + > > +static int meta_fb_create_handle(struct drm_framebuffer *fb, > > + struct drm_file *file, > > + unsigned int *handle) > > +{ > > + return -ENODEV; > > +} > > + > > +static int meta_fb_dirty(struct drm_framebuffer *fb, > > + struct drm_file *file, > > + unsigned int flags, > > + unsigned int color, > > + struct drm_clip_rect *clips, > > + unsigned int num_clips) > > +{ > > + return 0; > > +} > > + > > +static const struct drm_framebuffer_funcs meta_fb_funcs = { > > + .destroy = meta_fb_destroy, > > + .create_handle = meta_fb_create_handle, > > + .dirty = meta_fb_dirty, > > +}; > > + > > +static void meta_fb_update(struct intel_framebuffer *intel_fb, > > + enum pipe pipe, enum plane_id plane_id) { > > + struct intel_vgpu_fb_meta_data *meta_data; > > + struct intel_gvt *gvt; > > + > > + if (!intel_fb || intel_fb->meta_fb.type_id != INTEL_META_FB_VGPU) > > + return; > > + > > + meta_data = intel_fb->meta_fb.private; > > + gvt = meta_data->vgpu->gvt; > > + > > + if (gvt->assigned_plane[pipe][plane_id].vgpu_plane_id != > > + meta_data->vgpu_plane_id) { > > + gvt->assigned_plane[pipe][plane_id].vgpu_plane_id = > > + meta_data->vgpu_plane_id; > > + gvt->assigned_plane[pipe][plane_id].framebuffer_id = > > + intel_fb->base.base.id; > > + intel_fb->meta_fb.ggtt_offset = 0; > > + intel_fb->meta_fb.should_be_offscreen = true; > > + } else if (!intel_fb->meta_fb.ggtt_offset) { > > + intel_fb->meta_fb.should_be_offscreen = true; > > + } else { > > + intel_fb->meta_fb.should_be_offscreen = false; > > + } > > +} > > + > > +static int init_meta_fb(struct intel_vgpu *vgpu) { > > + struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv; > > + struct intel_vgpu_fb_meta_data *meta_data; > > + struct drm_mode_fb_cmd2 mode_cmd; > > + struct intel_framebuffer *intel_fb; > > + enum pipe pipe; > > + enum plane_id plane_id; > > + int ret = 0; > > + > > + for (pipe = 0; pipe < I915_MAX_PIPES; pipe++) { > > + for (plane_id = 0; plane_id < I915_MAX_PLANES; plane_id++) { > > + intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL); > > + if (!intel_fb) > > + return -ENOMEM; > > + > > + /* > > + * Create a drm_framebuffer with defaults. > > + */ > > + mode_cmd.pixel_format = DRM_FORMAT_XRGB8888; > > + mode_cmd.width = dev_priv- > >drm.mode_config.max_width; > > + mode_cmd.height = dev_priv- > >drm.mode_config.max_height; > > + mode_cmd.flags = DRM_MODE_FB_MODIFIERS; > > + mode_cmd.handles[0] = 0; > > + mode_cmd.pitches[0] = mode_cmd.width * 4; > > + mode_cmd.offsets[0] = 0; > > + mode_cmd.modifier[0] = > DRM_FORMAT_MOD_LINEAR; > > + > > + drm_helper_mode_fill_fb_struct(&dev_priv->drm, > > + &intel_fb->base, > &mode_cmd); > > + > > + ret = drm_framebuffer_init(&dev_priv->drm, > > + &intel_fb->base, > &meta_fb_funcs); > > + if (ret) { > > + DRM_ERROR("%s: framebuffer init > failed %d\n", > > + __func__, ret); > > + kfree(intel_fb); > > + return ret; > > + } > > + > > + meta_data = kmalloc(sizeof(struct > intel_vgpu_fb_meta_data), > > + GFP_KERNEL); > > + if (unlikely(!meta_data)) { > > + return -ENOMEM; > > + } > > + > > + meta_data->vgpu_plane_id = (vgpu->id << 16) | > > + (pipe << 8) | plane_id; > > + meta_data->vgpu = vgpu; > > + > > + intel_fb->meta_fb.private = meta_data; > > + intel_fb->meta_fb.update = meta_fb_update; > > + intel_fb->meta_fb.type_id = INTEL_META_FB_VGPU; > > + > > + vgpu->display.meta_fbs.meta_fb[pipe][plane_id] = > intel_fb; > > + } > > + } > > + return 0; > > +} > > + > > /** > > * intel_vgpu_clean_display - clean vGPU virtual display emulation > > * @vgpu: a vGPU > > @@ -457,6 +603,8 @@ void intel_vgpu_clean_display(struct intel_vgpu > *vgpu) > > clean_virtual_dp_monitor(vgpu, PORT_D); > > else > > clean_virtual_dp_monitor(vgpu, PORT_B); > > + > > + clean_meta_fb(vgpu); > > } > > > > /** > > @@ -476,6 +624,8 @@ int intel_vgpu_init_display(struct intel_vgpu > > *vgpu, u64 resolution) > > > > intel_vgpu_init_i2c_edid(vgpu); > > > > + init_meta_fb(vgpu); > > + > > if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) > > return setup_virtual_dp_monitor(vgpu, PORT_D, GVT_DP_D, > > resolution); > > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h > > b/drivers/gpu/drm/i915/gvt/gvt.h index 31f6cdb..0ab10b0 100644 > > --- a/drivers/gpu/drm/i915/gvt/gvt.h > > +++ b/drivers/gpu/drm/i915/gvt/gvt.h > > @@ -131,10 +131,16 @@ struct intel_vgpu_opregion { > > > > #define vgpu_opregion(vgpu) (&(vgpu->opregion)) > > > > +struct intel_vgpu_meta_fbs { > > + struct intel_framebuffer > *meta_fb[I915_MAX_PIPES][I915_MAX_PLANES]; > > + u32 plane_id_index; > > +}; > > + > > struct intel_vgpu_display { > > struct intel_vgpu_i2c_edid i2c_edid; > > struct intel_vgpu_port ports[I915_MAX_PORTS]; > > struct intel_vgpu_sbi sbi; > > + struct intel_vgpu_meta_fbs meta_fbs; > > }; > > > > struct vgpu_sched_ctl { > > @@ -301,6 +307,13 @@ struct intel_vgpu_type { > > enum intel_vgpu_edid resolution; > > }; > > > > +struct assigned_plane { > > + u32 vgpu_plane_id; > > + > > + /* userspace visible identifier */ > > + int framebuffer_id; > > +}; > > + > > struct intel_gvt { > > /* GVT scope lock, protect GVT itself, and all resource currently > > * not yet protected by special locks(vgpu and scheduler lock). > > @@ -340,6 +353,9 @@ struct intel_gvt { > > } engine_mmio_list; > > > > struct dentry *debugfs_root; > > + > > + /* vGPU plane assignment */ > > + struct assigned_plane > > +assigned_plane[I915_MAX_PIPES][I915_MAX_PLANES]; > > }; > > > > static inline struct intel_gvt *to_gvt(struct drm_i915_private *i915) > > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c > > b/drivers/gpu/drm/i915/gvt/kvmgt.c > > index c107214..7f4704d 100644 > > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c > > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c > > @@ -1420,12 +1420,58 @@ hw_id_show(struct device *dev, struct > device_attribute *attr, > > return sprintf(buf, "\n"); > > } > > > > +static ssize_t > > +plane_id_index_show(struct device *dev, struct device_attribute *attr, > > + char *buf) > > +{ > > + struct mdev_device *mdev = mdev_from_dev(dev); > > + > > + if (mdev) { > > + struct intel_vgpu *vgpu = (struct intel_vgpu *) > > + mdev_get_drvdata(mdev); > > + enum pipe pipe = vgpu->display.meta_fbs.plane_id_index & > > + 0x000000F0; > > + enum plane_id plane_id = vgpu- > >display.meta_fbs.plane_id_index & > > + 0x0000000F; > > + > > + if ((pipe < I915_MAX_PIPES || plane_id < I915_MAX_PLANES) > && > > + vgpu->display.meta_fbs.meta_fb[pipe][plane_id]) { > > + return sprintf(buf, "%u\n", > > + vgpu->display.meta_fbs.meta_fb[pipe][plane_id]- > >base.base.id); > > + } > > + } > > + return sprintf(buf, "\n"); > > +} > > + > > +static ssize_t > > +plane_id_index_store(struct device *dev, struct device_attribute *attr, > > + const char *buf, size_t n) > > +{ > > + struct mdev_device *mdev = mdev_from_dev(dev); > > + ssize_t ret; > > + u32 val; > > + > > + ret = kstrtou32(buf, 0, &val); > > + if (ret) > > + return ret; > > + > > + if (mdev) { > > + struct intel_vgpu *vgpu = (struct intel_vgpu *) > > + mdev_get_drvdata(mdev); > > + vgpu->display.meta_fbs.plane_id_index = val; > > + } > > + > > + return n; > > +} > > + > > static DEVICE_ATTR_RO(vgpu_id); > > static DEVICE_ATTR_RO(hw_id); > > +static DEVICE_ATTR_RW(plane_id_index); > > > > static struct attribute *intel_vgpu_attrs[] = { > > &dev_attr_vgpu_id.attr, > > &dev_attr_hw_id.attr, > > + &dev_attr_plane_id_index.attr, > > NULL > > }; > > > > -- > > 2.7.4 > > > > _______________________________________________ > > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx