From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matt Roper Subject: Re: [PATCH 2/2] tests/pm_rpm: add planes subtests Date: Tue, 5 Aug 2014 14:51:23 -0700 Message-ID: <20140805215122.GC20744@intel.com> References: <1406572636-1809-1-git-send-email-przanoni@gmail.com> <1406572636-1809-5-git-send-email-przanoni@gmail.com> <20140728234744.GO16114@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id B1F2E6E527 for ; Tue, 5 Aug 2014 14:51:04 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Paulo Zanoni Cc: Intel Graphics Development , Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Tue, Aug 05, 2014 at 06:34:38PM -0300, Paulo Zanoni wrote: > 2014-07-28 20:47 GMT-03:00 Matt Roper : > > On Mon, Jul 28, 2014 at 03:37:15PM -0300, Paulo Zanoni wrote: > >> From: Paulo Zanoni > >> > >> Just like the cursor subtests, these also trigger WARNs on the current > >> Kernel. > >> > >> Signed-off-by: Paulo Zanoni > > > > I feel like a lot of the setup you have to do here is duplicating logic > > we have in the igt_kms library. Was there functionality lacking from > > that library that prevented you from using it to write this test? If > > so, I can look into adding it. > > Every single ioctl we call can result in runtime PM get/put calls > inside the driver, so for pm_rpm.c I would like to keep using the low > level interfaces, to make sure the suspends and resumes are > controlled. > > Anyway, I never really looked at the library before. It seems the > biggest functionality missing from it is documentation. I just took a > look at the .c file and couldn't find anything that looked like would > reduce my diffstat, since the libdrm functions we call on pm_rpm.c are > very simple. Any suggestions? The main areas where I thought it might be possible to slim down a bit by using igt_kms were all the setup code --- grabbing plane resources, counting/looping over planes, grabbing properties to check plane types, etc. igt_kms will build up the plane list internally and hide a lot of that long, boring code from your tests. But since you've already written the test without it, I don't feel there's any major need to rewrite it with igt_kms; I was just curious if there was anything specific you thought was lacking from the library so that we could address it in the future. I did add some kerneldoc comments when I added the new interfaces in preparation for universal planes & atomic modeset, but you're right that there's still a lot that could be better documented going forward. Matt -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795