> -----Original Message----- > From: Zhang, Tina > Sent: Monday, December 3, 2018 4:53 PM > To: 'Zhenyu Wang' <zhenyuw@xxxxxxxxxxxxxxx> > 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 > > > > > -----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. The "has" here was actually "hasn't". Sorry for this typo. BR, Tina > > 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