Re: [PATCH 2/6] drm: Refactor setplane to allow internal use (v3)

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

On Fri, May 23, 2014 at 12:37:52AM +0200, Daniel Vetter wrote:
> On Thu, May 22, 2014 at 6:33 PM, Matt Roper <matthew.d.roper@intel.com> wrote:
> > v3:
> >  - Move integer overflow checking from setplane_internal to setplane
> >    ioctl.  The upcoming legacy cursor support via universal planes needs
> >    to maintain current cursor ioctl semantics and not return error for
> >    these extreme values (found via intel-gpu-tools kms_cursor_crc test).
> 
> Imo that's just the test being silly. I think it's valid to reject
> such nonsense with -EINVAL and limit the random-walk range of our
> testcase a bit. The igt testcases are just guidelines for what our ABI
> is, but not the hard rules. And clarifying the ABI in such cases is
> imo the right approach.
> 
> That random-walk testcase just was meant to test correctly clamping.
> E.g. on hardware with just 12bit for the cursor placement we'd wrap
> around and show a cursor again that should be off. So as long as you
> still test that you can restrict the range of the test a bit.
> -Daniel

I was a bit torn on this.  Since we accept offscreen plane positions,
I'm not sure it makes sense to really treat "offscreen" differently than
"really, really offscreen."  The legacy cursor ioctls have never cared
about this in the past so I didn't want to add in artificial range
limiting just because the setplane ioctl has had it for a while.

There's actually an i-g-t test that specifically tries to program
specifically at INT_MAX (not just the random walk test that might get
lucky and go out of bounds), so I figured the best course was just to
preserve the current behavior; I can't really see any downsides to not
range-limiting the cursors.


Matt

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795