Re: [PATCH 2/2] tests/pm_rpm: add planes subtests

[Matt Roper <matthew.d.roper@intel.com>]

On Tue, Aug 05, 2014 at 06:34:38PM -0300, Paulo Zanoni wrote:
> 2014-07-28 20:47 GMT-03:00 Matt Roper <matthew.d.roper@intel.com>:
> > On Mon, Jul 28, 2014 at 03:37:15PM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> Just like the cursor subtests, these also trigger WARNs on the current
> >> Kernel.
> >>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > 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