Subject: Re: [PATCH v9 5/7] vfio: Define vfio based dma-buf operations
To: Gerd Hoffmann <kraxel@redhat.com>,
        Alex Williamson <alex.williamson@redhat.com>
CC: Xiaoguang Chen <xiaoguang.chen@intel.com>, <chris@chris-wilson.co.uk>,
        <intel-gfx@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
        <zhenyuw@linux.intel.com>, <zhiyuan.lv@intel.com>,
        <intel-gvt-dev@lists.freedesktop.org>, <zhi.a.wang@intel.com>,
        <kevin.tian@intel.com>
References: <1497513611-2814-1-git-send-email-xiaoguang.chen@intel.com>
 <1497513611-2814-6-git-send-email-xiaoguang.chen@intel.com>
 <f19da207-a7f5-d3d3-4577-b071e7d75cf7@nvidia.com>
 <1497542438.29252.1.camel@redhat.com> <20170615143833.7526351b@w520.home>
 <24c4880b-24f5-ea07-834c-c77d3e895c78@nvidia.com>
 <20170616103959.3b6f1681@t450s.home> <1497854053.4207.2.camel@redhat.com>
 <20170619085409.26f5c14c@w520.home> <1497947703.16795.4.camel@redhat.com>
From: Kirti Wankhede <kwankhede@nvidia.com>
Message-ID: <791b0316-1c83-622c-9cef-d54dae1209f9@nvidia.com>
Date: Tue, 20 Jun 2017 19:25:31 +0530
MIME-Version: 1.0
In-Reply-To: <1497947703.16795.4.camel@redhat.com>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2805
Lines: 82


On 6/20/2017 2:05 PM, Gerd Hoffmann wrote:
>   Hi,
> 
>>> Hmm, plane isn't really an ID, it is a type, with type being either
>>> DRM_PLANE_TYPE_PRIMARY or DRM_PLANE_TYPE_CURSOR, so I don't think
>>> the
>>> flage above make sense.
>>
>> The intention was that ..._REGION_ID and ...PLANE_ID are describing
>> what the vfio_device_query_gfx_plane.id field represents, either a
>> region index or a plane identifier.  The type of plane would be
>> represented within the vfio_device_gfx_plane_info struct.
> 
> The planes don't really have an id, we should rename that to
> plane_type, or maybe drm_plane_type (simliar to the drm_format_*
> fields), to avoid that confusion.
> 
> plane_type is set by userspace to specify what kind of plane it asks
> for.
> 

Ok. so there should be two fields:
- plane type : DRM_PLANE_TYPE_PRIMARY or DRM_PLANE_TYPE_CURSOR
- id : fd for dmabuf or region index for region type

Adding reply to Gerd's question from earlier mail:
> What are the nvidia plane for cursor support btw?

We don't support cursor for console vnc. Ideally console vnc should be
used by admin for configuration or during maintenance, which refresh
primary surface at low refresh rate, 10 fps. We recommend to use
remoting solution for actual use.

>>> Also I think it would be useful to have some way to figure the
>>> device
>>> capabilities as the userspace workflow will look quite different
>>> for
>>> the two cases.
>>
>> In the region case, VFIO_DEVICE_GET_REGION_INFO would include a
>> device
>> specific region with a hopefully common identifier to identify it as
>> a
>> graphics framebuffer.
> 
> Ok, that should work to figure whenever the mdev supports a plane
> region or not.
> 
>> In the dmabuf case,VFIO_DEVICE_QUERY_GFX_PLANE would indicate the
>> plane as a "plane ID" and some sort of
>> VFIO_DEVICE_GET_GFX_PLANE(VFIO_GFX_TYPE_DMABUF) ioctl would be
>> necessary to get a file descriptor to that plane.
>>
>> What else are you thinking we need?  Thanks,
> 
> I need to know whenever the mdev supports dmabufs or not, at device
> initialization time (because dmabufs require opengl support), when
> VFIO_DEVICE_QUERY_GFX_PLANE doesn't work due to the guest not having
> the device initialized yet.
> 
> Maybe we should have a error field in the ioctl struct, or we need to
> clearly define error codes so the kernel doesn't just throw EINVAL in
> all cases.
> 
> Or just a VFIO_DEVICE_GFX_CAPS ioctl which returns NONE, REGION or
> DMABUF.
>

Right we need to know this at device initialization time for both cases
to initialize VGACommonState structure for that device and also need
NONE to decide whether to init console vnc or not. We have a mechanism
to disable console vnc path and we recommend to disable it for better
performance.

Thanks,
Kirti

> cheers,
>   Gerd
>