On Wed, Mar 23, 2016 at 03:30:48PM +0200, Joonas Lahtinen wrote: > On ke, 2016-03-23 at 09:58 +0100, Daniel Vetter wrote: > > On Tue, Mar 22, 2016 at 02:14:38PM +0000, Matthew Auld wrote: > > > > > > Hi Daniel, > > > > > > > > > > > > > > > > > I thought we do normalize this somewhere. > > > I did write an i-g-t test which submits such a rotation value and it > > > is not rejected. > > Normalize = the drm core makes sure drivers don't see all the > > combinations, but only canonical values. Userspace can still submit values > > with too many bits set. I'm not sure we want (or can, it's ABI) change > > that. > > > > > > > > > > > > > > > > > > > Your patch lacks motivation > > > As in I haven't properly conveyed the motivation behind the patch in > > > the commit message? > > > > > > > > > > > > > > > > > Yes I can usually guess when > > > it's due to static analyzer checks, but you need to explain that. And you > > > need to explain what exactly the analyzer is complaining about. > > > > > > erm, no static analyser, for this patch or any prior, promise, but duly noted ;) > > > > > > Joonas actually suggested this patch, and some of the preceding ones > > > as beginner tasks for me. > > Oh surprising, spotting all these random things all over tends to be stuff > > only static analyzers manage ;-) Patch still needs some motivation, since > > if your igt passes and the driver behaves correctly it's all fine. > > I'm happy to mention that the motivation this was on my backlog is that > it was *YOU* who asked me to implement it along with the IGT tests :P > > But I guess, now if it's implemented in DRM layer already, matter of > making sure the kms_rotation_crc tests the current expected behaviour. > > And by what you described (drivers won't see bad values, ABI accepts > them), it would mean to just attempt to send invalid combinations, they > should be happily accepted but resulting rotation will be undefined. I > myself would reject invalid bit combinations, but if the ABI has > already grown that way, not much to be done at this point. Well so I unlazied and actually read the code. We do have the helper function in drm_rotation_simplify, but it's not called. So still work to do. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
