2014-02-11 19:54 GMT-02:00 Daniel Vetter <daniel@xxxxxxxx>: > On Tue, Feb 11, 2014 at 6:20 PM, Paulo Zanoni <przanoni@xxxxxxxxx> wrote: >> 2014-02-11 15:09 GMT-02:00 Paulo Zanoni <przanoni@xxxxxxxxx>: >>> 2014-02-11 13:44 GMT-02:00 Daniel Vetter <daniel@xxxxxxxx>: >>>> On Tue, Feb 11, 2014 at 4:23 PM, Paulo Zanoni <przanoni@xxxxxxxxx> wrote: >>>>> >>>>> 2014-02-10 15:23 GMT-02:00 Daniel Vetter <daniel@xxxxxxxx>: >>>>>> On Mon, Feb 10, 2014 at 02:17:03PM +0000, Damien Lespiau wrote: >>>>>>> On Fri, Jan 17, 2014 at 01:51:09PM -0200, Paulo Zanoni wrote: >>>>>>> > From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> >>>>>>> > >>>>>>> > We want to remove those 3 boolean arguments. This is the first step. >>>>>>> > The "pipe" passed as the argument is always intel_crtc->pipe. >>>>>>> > >>>>>>> > Also adjust the function documentation. >>>>>>> > >>>>>>> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> >>>>>>> >>>>>>> Reviewed-by: Damien Lespiau <damien.lespiau@xxxxxxxxx> >>>>>> >>>>>> Ok, I've pulled in the entire series, but a bunch of things changed so had >>>>>> to resolve some (minor) conflicts. Please double-check that I didn't botch >>>>>> things up too badly. >>>>> >>>>> You forgot to apply patch 2, and this is probably the reason why every >>>>> subsequent patch gave you a conflict. >>>> >>>> The conflicts where actually with one of Ville's patches to move the >>>> plane enabling around. I've fixed those up but apparently then missed >>>> the other conflict hidden underneath those. >>>> >>>>> You also applied patch 3 twice: once for Ironlake and once for >>>>> Haswell. You shouldn't change the Ironlake function. >>>>> >>>>> Do you plan to rebase or do I need to submit patches on top? >>>> >>>> I've applied the missing patched and dropped the ironlake patch of the >>>> double-merged one. So if the new tree looks ok no need to resend >>>> anything. >> >> Doesn't look ok yet. > > Oh dear ... I've tried again. Not sure whether I should have though :( Now it looks correct :) > -Daniel > >> So previously I had "[PATCH 2/8] drm/i915: don't wait for vblank after >> enabling pipe on HSW", which removes a wait_for_vblank on HSW. >> >> Then on "[PATCH 7/8] drm/i915: remove wait_for_vblank argument form >> intel_enable_pipe" we just change the function parameters without >> changing the function behavior. >> >> With this, if we bisect something to patch 2 we know the problem is >> that we stopped waiting for a vblank, and if we bisect to patch 7 we >> know the problem is something else. >> >> But since you just skipped patch 2, patch 7 is now more than just a >> coding style change: it actually does what patch 2 was supposed to do. >> So in a way, we can say patch 2 is not really necessary, but it was >> written weeks before patch 7, and patch 7 should be just a result of >> the review comments. >> >> And the version of "drm/i915: don't wait for vblank after enabling >> pipe on HSW" which you just committed as a last patch instead of >> second patch (the one that changes the argument to >> intel_crtc_update_cursor) is just plain wrong. That needs to be >> reverted. So either we add the original patch 2 at the right place, or >> we completely discard it... >> >> I know it's common to change the patch ordering when applying to our >> trees, but it can be quite dangerous... >> >> Thanks, >> Paulo >> >>>> >>>>> IMHO if a series starts getting messy to apply, I think you should >>>>> probably just ask the author to rebase and resend the final stuff. >>>>> Maybe with this we would be able to reduce the amount of bad merges, >>>>> which is becoming a very common problem, at least for my patches. >>>> >>>> The problem is that small conflicts are really common, both because I >>>> want people to submit against drm-intel-nightly (so that I can do the >>>> backmerging and branch shuffling correctly) and because of our >>>> development speed. I don't think me asking for rebases in all these >>>> cases is the better option. I tend to poke people to double-check when >>>> I screw things up, but I guess it's just been bad luck that recently >>>> the conflict fallout has always hit your patches :( >>>> >>>> Cheers, Daniel >>>> -- >>>> Daniel Vetter >>>> Software Engineer, Intel Corporation >>>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch >>> >>> >>> >>> -- >>> Paulo Zanoni >> >> >> >> -- >> Paulo Zanoni > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Paulo Zanoni _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
