RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

>-----Original Message-----
>From: Alex Williamson [mailto:alex.williamson@xxxxxxxxxx]
>Sent: Friday, May 12, 2017 10:58 AM
>To: Chen, Xiaoguang <xiaoguang.chen@xxxxxxxxx>
>Cc: Gerd Hoffmann <kraxel@xxxxxxxxxx>; Tian, Kevin <kevin.tian@xxxxxxxxx>;
>intel-gfx@xxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
>zhenyuw@xxxxxxxxxxxxxxx; Lv, Zhiyuan <zhiyuan.lv@xxxxxxxxx>; intel-gvt-
>dev@xxxxxxxxxxxxxxxxxxxxx; Wang, Zhi A <zhi.a.wang@xxxxxxxxx>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Fri, 12 May 2017 02:12:10 +0000
>"Chen, Xiaoguang" <xiaoguang.chen@xxxxxxxxx> wrote:
>
>> Hi Alex and Gerd,
>>
>> >-----Original Message-----
>> >From: intel-gvt-dev
>> >[mailto:intel-gvt-dev-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of
>> >Alex Williamson
>> >Sent: Thursday, May 11, 2017 11:45 PM
>> >To: Gerd Hoffmann <kraxel@xxxxxxxxxx>
>> >Cc: Tian, Kevin <kevin.tian@xxxxxxxxx>;
>> >intel-gfx@xxxxxxxxxxxxxxxxxxxxx; linux- kernel@xxxxxxxxxxxxxxx;
>> >zhenyuw@xxxxxxxxxxxxxxx; Lv, Zhiyuan <zhiyuan.lv@xxxxxxxxx>; Chen,
>> >Xiaoguang <xiaoguang.chen@xxxxxxxxx>; intel-
>> >gvt-dev@xxxxxxxxxxxxxxxxxxxxx; Wang, Zhi A <zhi.a.wang@xxxxxxxxx>
>> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
>> >dmabuf
>> >
>> >On Thu, 11 May 2017 15:27:53 +0200
>> >Gerd Hoffmann <kraxel@xxxxxxxxxx> wrote:
>> >
>> >>   Hi,
>> >>
>> >> > While read the framebuffer region we have to tell the vendor
>> >> > driver which
>> >framebuffer we want to read? There are two framebuffers now in KVMGT
>> >that is primary and cursor.
>> >> > There are two methods to implement this:
>> >> > 1) write the plane id first and then read the framebuffer.
>> >> > 2) create 2 vfio regions one for primary and one for cursor.
>> >>
>> >> (3) Place information for both planes into one vfio region.
>> >>     Which allows to fetch both with a single read() syscall.
>> >>
>> >> The question is how you'll get the file descriptor then.  If the
>> >> ioctl returns the dma-buf fd only you have a racy interface:
>> >> Things can change between read(vfio-region) and ioctl(need-dmabuf-fd).
>> >>
>> >> ioctl(need-dma-buf) could return both dmabuf fd and plane info to
>> >> fix the race, but then it is easier to go with ioctl only interface
>> >> (simliar to the orginal one from dec last year) I think.
>> >
>> >If the dmabuf fd is provided by a separate mdev vendor driver
>> >specific ioctl, I don't see how vfio regions should be involved.  Selecting which
>framebuffer
>> >should be an ioctl parameter.
>> Based on your last mail. I think the implementation looks like this:
>> 1) user query the framebuffer information by reading the vfio region.
>> 2) if the framebuffer changed(such as framebuffer's graphics address changed,
>size changed etc) we will need to create a new dmabuf fd.
>> 3) create a new dmabuf fd using vfio device specific ioctl.
>>
>> >What sort of information needs to be conveyed
>> >about each plane?
>> Only plane id is needed.
>>
>> >Is it static information or something that needs to be read
>> >repeatedly?
>> It is static information. For our case plane id 1 represent primary plane and 3 for
>cursor plane. 2 means sprite plane which will not be used in our case.
>>
>> >Do we need it before we get the dmabuf fd or can it be an ioctl on
>> >the dmabuf fd?
>> We need it while query the framebuffer. In kernel we need the plane id to
>decide which plane we should decode.
>> Below is my current implementation:
>> 1) user first query the framebuffer(primary or cursor) and kernel decode the
>framebuffer and return the framebuffer information to user and also save a copy
>in kernel.
>> 2) user compared the framebuffer and if the framebuffer changed  creating a
>new dmabuf fd.
>
>If the contents of the framebuffer change or if the parameters of the framebuffer
>change?  
If the parameters of the framebuffer change we need to create new dmabuf.


>I can't image that creating a new dmabuf fd for every visual change
>within the framebuffer would be efficient, but I don't have any concept of what a
>dmabuf actually does.
>
>> 3) kernel create a new dmabuf fd based on saved framebuffer information.
>>
>> So we need plane id in step 1.
>> In step 3 we create a dmabuf fd only using saved framebuffer information(no
>other information is needed).
>
>What changes to the framebuffer require a new dmabuf fd?  Shouldn't the user
>query the parameters of the framebuffer through a dmabuf fd and shouldn't the
>dmabuf fd have some signaling mechanism to the user (eventfd perhaps) to notify
>the user to re-evaluate the parameters?
>Otherwise are you imagining that the user polls the vfio region?  Why can a
>dmabuf fd not persist across changes to the framebuffer?  Can someone explain
>what a dmabuf is and how it works in terms that a non-graphics person can
>understand?  Thanks,

A dmabuf will be associated a gem object. In our case the backing storage of the gem object is from the framebuffer.
We use the GMA(graphics memory address) to traverse the GTT(graphics translation table) to get all the pages belong to the framebuffer and assign these pages to the gem object.
So once the GMA of a framebuffer changed we have to create new dmabuf for it. The other parameters of the framebuffer such as the size of the framebuffer, the tiling mode changed we also have to create a new dmabuf.

We did consider the signal mechanism to notify the user but doing so the kernel need to save a lot of information of the framebuffers so we decide to let the user check whether to create a new dmabuf or not.
So the usage flow is:
1) query the framebuffer information
2) compare with saved dmabuf whether need to create a new dmabuf
3) create a new dmabuf(save the dmabuf and related framebuffer information such as gma, size....)


>
>Alex