On 19/08/16 04:57 PM, Emil Velikov wrote: > On 19 August 2016 at 03:09, Michel Dänzer <michel at daenzer.net> wrote: >> On 19/08/16 11:02 AM, Yu, Qiang wrote: >>> >>> Each point of the patch set is not broken. Patches are arranged like >>> this to show how I do it: >>> 1. create a pageflip.c to host common page flip code >>> 2. copy amdgpu DDX DRI2 page flip code to modesetting dri2.c >>> 3. merge common DRI2 and present page flip code to pageflip.c >>> >>> Patch 4 is not a fix for broken code, just a commit from amdgpu DDX >>> to prevent present and DRI2 from flipping at the same time. >> >> This does mean though that if one has only up to patch 3 applied (e.g. >> during a bisection), one is exposed to the issues fixed by patch 4. So >> maybe patch 4 should be squashed into patch 3. >> > Precisely what I meant with "broken" - new code introduces a > functionality which conflicts with existing one. > > And yes, one can see the code flow (and that it's partially copied > from amdgpu/ati) As-is the series brings unnecessary churn, since it > adds new code only to (re)move it. > Something many developers/projects consider a bad way to split patches. It makes some sense to add the code copied from another driver as verbatim as possible first. That makes it easier to compare it with the other driver. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
