RE: [RFC PATCH v4 6/6] drm/i915/gvt: Add cursor plane reg update trap emulation handler

Lu, Kechen
2019-07-19T09:33:55Z

Hi,

> -----Original Message-----
> From: Zhenyu Wang [mailto:zhenyuw@linux.intel.com]
> Sent: Friday, July 19, 2019 2:35 PM
> To: Lu, Kechen <kechen.lu@intel.com>
> Cc: intel-gvt-dev@lists.freedesktop.org; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; kraxel@redhat.com; zhenyuw@linux.intel.com;
> Lv, Zhiyuan <zhiyuan.lv@intel.com>; Wang, Zhi A
> <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>; Yuan, Hang
> <hang.yuan@intel.com>; alex.williamson@redhat.com
> Subject: Re: [RFC PATCH v4 6/6] drm/i915/gvt: Add cursor plane reg
> update trap emulation handler
>
> On 2019.07.18 23:56:40 +0800, Kechen Lu wrote:
> > This patch adds the cursor plane CURBASE reg update trap handler in
> > order to :
> >
> > - Deliver the cursor refresh event at each vblank emulation, the
> > flip_done_event bit check is supposed to do here. If cursor plane
> > updates happen, deliver the cursor refresh events.
> >
> > - Support the sync and async cursor plane updates and corresponding
> > cursor plane flip interrupts reporting.
> >
> > Signed-off-by: Kechen Lu <kechen.lu@intel.com>
> > ---
> > drivers/gpu/drm/i915/gvt/display.c | 11 +++++++++++
> > drivers/gpu/drm/i915/gvt/handlers.c | 27
> > ++++++++++++++++++++++++--- drivers/gpu/drm/i915/gvt/interrupt.c |
> > 7 +++++++ drivers/gpu/drm/i915/gvt/interrupt.h | 3 +++
> > 4 files changed, 45 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/display.c
> > b/drivers/gpu/drm/i915/gvt/display.c
> > index df52e4b4c1b0..a0accc51d44f 100644
> > --- a/drivers/gpu/drm/i915/gvt/display.c
> > +++ b/drivers/gpu/drm/i915/gvt/display.c
> > @@ -399,6 +399,7 @@ static void emulate_vblank_on_pipe(struct
> > intel_vgpu
> *vgpu, int pipe)
> > [PIPE_C] = PIPE_C_VBLANK,
> > };
> > int pri_flip_event = SKL_FLIP_EVENT(pipe, PLANE_PRIMARY);
> > + int cur_flip_event = CURSOR_A_FLIP_DONE + pipe;
> > int event;
> > u64 eventfd_signal_val = 0;
> > static int pageflip_count;
> > @@ -417,6 +418,11 @@ static void emulate_vblank_on_pipe(struct
> intel_vgpu *vgpu, int pipe)
> > pageflip_count += PAGEFLIP_INC_COUNT;
> > }
> >
> > + if (event == cur_flip_event) {
> > + eventfd_signal_val +=
> DISPLAY_CUR_REFRESH_EVENT_INC;
> > + pageflip_count += PAGEFLIP_INC_COUNT;
> > + }
> > +
> > intel_vgpu_trigger_virtual_event(vgpu, event);
> > }
> >
> > @@ -430,6 +436,11 @@ static void emulate_vblank_on_pipe(struct
> intel_vgpu *vgpu, int pipe)
> > eventfd_signal_val +=
> DISPLAY_PRI_REFRESH_EVENT_INC;
> > pageflip_count += PAGEFLIP_INC_COUNT;
> > }
> > +
> > + if (event == PLANE_CURSOR) {
> > + eventfd_signal_val +=
> DISPLAY_CUR_REFRESH_EVENT_INC;
> > + pageflip_count += PAGEFLIP_INC_COUNT;
> > + }
> > }
> >
> > if (--pageflip_count < 0) {
> > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> > b/drivers/gpu/drm/i915/gvt/handlers.c
> > index 6ad29c4f08e5..821ff88977d8 100644
> > --- a/drivers/gpu/drm/i915/gvt/handlers.c
> > +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> > @@ -767,6 +767,27 @@ static int pri_surf_mmio_write(struct
> > intel_vgpu
> *vgpu, unsigned int offset,
> > return 0;
> > }
> >
> > +#define CURBASE_TO_PIPE(reg) \
> > + calc_index(offset, _CURABASE, _CURBBASE, 0, CURBASE(PIPE_C))
> > +
> > +static int cur_surf_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
> > + void *p_data, unsigned int bytes) {
> > + struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> > + u32 pipe = CURBASE_TO_PIPE(offset);
> > + int event = CURSOR_A_FLIP_DONE + pipe;
> > +
> > + write_vreg(vgpu, offset, p_data, bytes);
> > +
> > + if (vgpu_vreg_t(vgpu, CURCNTR(pipe)) & PLANE_CTL_ASYNC_FLIP) {
> > + intel_vgpu_trigger_virtual_event(vgpu, event);
> > + set_bit(PLANE_CURSOR, vgpu->display.async_flip_event[pipe]);
> > + } else
> > + set_bit(event, vgpu->irq.flip_done_event[pipe]);
> > +
> > + return 0;
> > +}
> > +
> > #define SPRSURF_TO_PIPE(offset) \
> > calc_index(offset, _SPRA_SURF, _SPRB_SURF, 0, SPRSURF(PIPE_C))
> >
> > @@ -1955,9 +1976,9 @@ static int init_generic_mmio_info(struct
> > intel_gvt
> *gvt)
> > MMIO_D(CURPOS(PIPE_B), D_ALL);
> > MMIO_D(CURPOS(PIPE_C), D_ALL);
> >
> > - MMIO_D(CURBASE(PIPE_A), D_ALL);
> > - MMIO_D(CURBASE(PIPE_B), D_ALL);
> > - MMIO_D(CURBASE(PIPE_C), D_ALL);
> > + MMIO_DH(CURBASE(PIPE_A), D_ALL, NULL, cur_surf_mmio_write);
> > + MMIO_DH(CURBASE(PIPE_B), D_ALL, NULL, cur_surf_mmio_write);
> > + MMIO_DH(CURBASE(PIPE_C), D_ALL, NULL, cur_surf_mmio_write);
>
> I think we should also track cursor pos change right?
>

Yes, this may be the case, according to the bspec, the CURPOS regs take into effects at vblanks. But in Linux i915 driver, the CURPOS regs programming companies with CURBASE regs. I'm not sure the Windows driver logic, but I can do some experiments. If that is the case, yes, we also need to trap the cursor pos change.
Thanks.

Best Regards,
Kechen

> >
> > MMIO_D(CUR_FBC_CTL(PIPE_A), D_ALL);
> > MMIO_D(CUR_FBC_CTL(PIPE_B), D_ALL); diff --git
> > a/drivers/gpu/drm/i915/gvt/interrupt.c
> > b/drivers/gpu/drm/i915/gvt/interrupt.c
> > index 951681813230..9c2b9d2e1529 100644
> > --- a/drivers/gpu/drm/i915/gvt/interrupt.c
> > +++ b/drivers/gpu/drm/i915/gvt/interrupt.c
> > @@ -113,6 +113,9 @@ static const char * const
> irq_name[INTEL_GVT_EVENT_MAX] = {
> > [SPRITE_A_FLIP_DONE] = "Sprite Plane A flip done",
> > [SPRITE_B_FLIP_DONE] = "Sprite Plane B flip done",
> > [SPRITE_C_FLIP_DONE] = "Sprite Plane C flip done",
> > + [CURSOR_A_FLIP_DONE] = "Cursor Plane A flip done",
> > + [CURSOR_B_FLIP_DONE] = "Cursor Plane B flip done",
> > + [CURSOR_C_FLIP_DONE] = "Cursor Plane C flip done",
> >
> > [PCU_THERMAL] = "PCU Thermal Event",
> > [PCU_PCODE2DRIVER_MAILBOX] = "PCU pcode2driver mailbox event",
> @@
> > -593,6 +596,10 @@ static void gen8_init_irq(
> > SET_BIT_INFO(irq, 4, SPRITE_A_FLIP_DONE,
> INTEL_GVT_IRQ_INFO_DE_PIPE_A);
> > SET_BIT_INFO(irq, 4, SPRITE_B_FLIP_DONE,
> INTEL_GVT_IRQ_INFO_DE_PIPE_B);
> > SET_BIT_INFO(irq, 4, SPRITE_C_FLIP_DONE,
> > INTEL_GVT_IRQ_INFO_DE_PIPE_C);
> > +
> > + SET_BIT_INFO(irq, 6, CURSOR_A_FLIP_DONE,
> INTEL_GVT_IRQ_INFO_DE_PIPE_A);
> > + SET_BIT_INFO(irq, 6, CURSOR_B_FLIP_DONE,
> INTEL_GVT_IRQ_INFO_DE_PIPE_B);
> > + SET_BIT_INFO(irq, 6, CURSOR_C_FLIP_DONE,
> > +INTEL_GVT_IRQ_INFO_DE_PIPE_C);
> > }
> >
> > /* GEN8 interrupt PCU events */
> > diff --git a/drivers/gpu/drm/i915/gvt/interrupt.h
> > b/drivers/gpu/drm/i915/gvt/interrupt.h
> > index 5313fb1b33e1..158f1c7a23f2 100644
> > --- a/drivers/gpu/drm/i915/gvt/interrupt.h
> > +++ b/drivers/gpu/drm/i915/gvt/interrupt.h
> > @@ -92,6 +92,9 @@ enum intel_gvt_event_type {
> > SPRITE_A_FLIP_DONE,
> > SPRITE_B_FLIP_DONE,
> > SPRITE_C_FLIP_DONE,
> > + CURSOR_A_FLIP_DONE,
> > + CURSOR_B_FLIP_DONE,
> > + CURSOR_C_FLIP_DONE,
> >
> > PCU_THERMAL,
> > PCU_PCODE2DRIVER_MAILBOX,
> > --
> > 2.17.1
> >
>
> --
> Open Source Technology Center, Intel ltd.
>
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827