On Fri, May 23, 2014 at 12:51:29AM +0200, Daniel Vetter wrote: > On Thu, May 22, 2014 at 03:48:16PM -0700, Matt Roper wrote: > > 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@xxxxxxxxx> 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. > > Ok, if you think this won't create an unnecessary fuss in the code then > I'm ok with this, too. After all most hw has limited offset fields, so > drivers must clamp anyway. So dunno really how much sense that limit check > makes in set_plane. But we have it, so doesn't hurt too much really. > > A different consideration though is that with the igt_kms library the plan > is to just switch to atomic modeset eventually, which also means using > universal planes for cursors and primary planes. Which means as soon as we > do that we'll likely hit that restriction again. > > Not sure what to do now here. > -Daniel When we switch to atomic modeset in i-g-t, everything will be coming in via the propertyset interface, right? Since the range limiting check is in the setplane ioctl code now, I'd expect us to bypass it completely. Matt -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
