Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751853AbdGLMpX (ORCPT ); Wed, 12 Jul 2017 08:45:23 -0400 Received: from hqemgate14.nvidia.com ([216.228.121.143]:1683 "EHLO hqemgate14.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751688AbdGLMpV (ORCPT ); Wed, 12 Jul 2017 08:45:21 -0400 X-PGP-Universal: processed; by hqpgpgate102.nvidia.com on Wed, 12 Jul 2017 05:45:19 -0700 Subject: Re: [PATCH v10] vfio: ABI for mdev display dma-buf operation To: "Zhang, Tina" , Gerd Hoffmann , "Tian, Kevin" , "linux-kernel@vger.kernel.org" , "intel-gfx@lists.freedesktop.org" , "alex.williamson@redhat.com" , "zhenyuw@linux.intel.com" , "chris@chris-wilson.co.uk" , "Lv, Zhiyuan" , "intel-gvt-dev@lists.freedesktop.org" , "Wang, Zhi A" References: <1499293795-6265-1-git-send-email-tina.zhang@intel.com> <980a5c09-fa8a-255d-19ad-acf4bb29d271@nvidia.com> <1499753648.8257.3.camel@redhat.com> <20170711091236.run4zirxmr34kazb@phenom.ffwll.local> <237F54289DF84E4997F34151298ABEBC7C57BF86@SHSMSX101.ccr.corp.intel.com> <20170712074038.izr6ureby4vwtq27@phenom.ffwll.local> X-Nvconfidentiality: public From: Kirti Wankhede Message-ID: Date: Wed, 12 Jul 2017 18:15:09 +0530 MIME-Version: 1.0 In-Reply-To: <20170712074038.izr6ureby4vwtq27@phenom.ffwll.local> X-Originating-IP: [10.24.205.12] X-ClientProxiedBy: BGMAIL104.nvidia.com (10.25.59.13) To bgmail102.nvidia.com (10.25.59.11) Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2679 Lines: 67 On 7/12/2017 1:10 PM, Daniel Vetter wrote: > On Wed, Jul 12, 2017 at 02:31:40AM +0000, Zhang, Tina wrote: >> >> >>> -----Original Message----- >>> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On >>> Behalf Of Daniel Vetter >>> Sent: Tuesday, July 11, 2017 5:13 PM >>> To: Gerd Hoffmann >>> Cc: Tian, Kevin ; linux-kernel@vger.kernel.org; intel- >>> gfx@lists.freedesktop.org; alex.williamson@redhat.com; >>> zhenyuw@linux.intel.com; chris@chris-wilson.co.uk; Kirti Wankhede >>> ; Lv, Zhiyuan ; >>> daniel@ffwll.ch; Zhang, Tina ; intel-gvt- >>> dev@lists.freedesktop.org; Wang, Zhi A >>> Subject: Re: [PATCH v10] vfio: ABI for mdev display dma-buf operation >>> >>> On Tue, Jul 11, 2017 at 08:14:08AM +0200, Gerd Hoffmann wrote: >>>> Hi, >>>> >>>>>> +struct vfio_device_query_gfx_plane { >>>>>> + __u32 argsz; >>>>>> + __u32 flags; >>>>>> + struct vfio_device_gfx_plane_info plane_info; >>>>>> + __u32 plane_type; >>>>>> + __s32 fd; /* dma-buf fd */ >>>>>> + __u32 plane_id; >>>>>> +}; >>>>>> + >>>>> >>>>> It would be better to have comment here about what are expected >>>>> values for plane_type and plane_id. >>>> >>>> plane_type is DRM_PLANE_TYPE_*. >>>> >>>> yes, a comment saying so would be good, same for drm_format which is >>>> DRM_FORMAT_*. While looking at these two: renaming plane_type to >>>> drm_plane_type (for consistency) is probably a good idea too. >>>> >>>> plane_id needs a specification. >>> >>> Why do you need plane_type? With universal planes the plane_id along is >>> sufficient to identify a plane on a given drm device instance. I'd just remove it. >>> -Daniel >> The plane_type here, is to ask the mdev vendor driver to return the information according to the value in field plane_type. So, it's a input field. >> The values in plane_type field is the same of drm_plane_type. And yes, it's better to use drm_plane_type instead of plane_id. > > I have no idea what you mean here, I guess that just shows that discussing > an ioctl struct without solid definitions of what field does what and why > is not all that useful. What exactly it plane_id for then? > plane type could be DRM_PLANE_TYPE_PRIMARY or DRM_PLANE_TYPE_CURSOR. In case when VFIO region is used to provide surface to QEMU, plane_id would be region index, for example region 10 could be used for primary surface and region 11 could be used for cursor surface. So in that case, mdev vendor driver should return plane_type and its corresponding plane_id. Thanks, Kirti > This just confused me more ... > -Daniel >