From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932475AbcJLG4l (ORCPT ); Wed, 12 Oct 2016 02:56:41 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:34369 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932434AbcJLG4X (ORCPT ); Wed, 12 Oct 2016 02:56:23 -0400 Date: Wed, 12 Oct 2016 08:56:18 +0200 From: Daniel Vetter To: Brian Starkey Cc: Daniel Vetter , dri-devel , Linux Kernel Mailing List , "linux-media@vger.kernel.org" , Liviu Dudau , "Clark, Rob" , Hans Verkuil , Eric Anholt , "Syrjala, Ville" Subject: Re: [RFC PATCH 00/11] Introduce writeback connectors Message-ID: <20161012065618.GI20761@phenom.ffwll.local> Mail-Followup-To: Brian Starkey , dri-devel , Linux Kernel Mailing List , "linux-media@vger.kernel.org" , Liviu Dudau , "Clark, Rob" , Hans Verkuil , Eric Anholt , "Syrjala, Ville" References: <1476197648-24918-1-git-send-email-brian.starkey@arm.com> <20161011154359.GD20761@phenom.ffwll.local> <20161011164305.GA14337@e106950-lin.cambridge.arm.com> <20161011194422.GC14337@e106950-lin.cambridge.arm.com> <20161011212423.GA10077@e106950-lin.cambridge.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161011212423.GA10077@e106950-lin.cambridge.arm.com> X-Operating-System: Linux phenom 4.6.0-1-amd64 User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 11, 2016 at 10:24:23PM +0100, Brian Starkey wrote: > On Tue, Oct 11, 2016 at 10:02:43PM +0200, Daniel Vetter wrote: > > The problem with just that is that there's lots of different things > > that can feed into the overall needs_modeset variable. That's why we > > split it up into multiple booleans. > > > > So yes you're supposed to clear connectors_changed if there is some > > change that you can handle without a full modeset. If you want, think > > of connectors_changed as > > needs_modeset_due_to_change_in_connnector_state, but that's cumbersome > > to type and too long ;-) > > > > All right, got it :-). This intention wasn't clear to me from the > comments in the code. A patch to update the kernel-doc to make it clearer (there's mode_changed, connectors_changed and active_changed, plus drm_crtc_needs_modeset) would be awesome. I'm trying to write useful docs, but since I designed this all I sometimes forget to make the non-obvious assumptions clear enough. Volunteered? > > > > tbh I don't like that, I think it'd be better to make this truly > > > > one-shot. Otherwise we'll have real fun problems with hw where the > > > > writeback can take longer than a vblank (it happens ...). So one-shot, > > > > with auto-clearing to NULL/0 is imo the right approach. > > > > > > That's an interesting point about hardware which won't finish within > > > one frame; but I don't see how "true one-shot" helps. > > > > > > What's the expected behaviour if userspace makes a new atomic commit > > > with a writeback framebuffer whilst a previous writeback is ongoing? > > > > > > In both cases, you either need to block or fail the commit - whether > > > the framebuffer gets removed when it's done is immaterial. > > > > See Eric's question. We need to define that, and I think the simplest > > approach is a completion fence/sync_file. It's destaged now in 4.9, we > > can use them. I think the simplest uabi would be a pointer property > > (u64) where we write the fd of the fence we'll signal when write-out > > completes. > > > > That tells userspace that the previous writeback is finished, I agree that's > needed. It doesn't define any behaviour in case userspace asks for another > writeback before that fence fires though. Hm, good point. I guess we could just state that if userspace does a writeback, and issues a new writeback before both a) the atomic flip and b) the write back complete fence signalled will lead to undefined behaviour. Undefined as in: data corruption, rejected atomic commit or anything else than a kernel crash is allowed. This is similar to doing a page flip and starting to render to the old buffers before the flip event signalled completion: Userspace gets the mess it asked for ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [RFC PATCH 00/11] Introduce writeback connectors Date: Wed, 12 Oct 2016 08:56:18 +0200 Message-ID: <20161012065618.GI20761@phenom.ffwll.local> References: <1476197648-24918-1-git-send-email-brian.starkey@arm.com> <20161011154359.GD20761@phenom.ffwll.local> <20161011164305.GA14337@e106950-lin.cambridge.arm.com> <20161011194422.GC14337@e106950-lin.cambridge.arm.com> <20161011212423.GA10077@e106950-lin.cambridge.arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wm0-x241.google.com (mail-wm0-x241.google.com [IPv6:2a00:1450:400c:c09::241]) by gabe.freedesktop.org (Postfix) with ESMTPS id 31E636E7C3 for ; Wed, 12 Oct 2016 06:56:23 +0000 (UTC) Received: by mail-wm0-x241.google.com with SMTP id b80so896603wme.3 for ; Tue, 11 Oct 2016 23:56:23 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20161011212423.GA10077@e106950-lin.cambridge.arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Brian Starkey Cc: Liviu Dudau , Linux Kernel Mailing List , dri-devel , Hans Verkuil , "linux-media@vger.kernel.org" List-Id: dri-devel@lists.freedesktop.org T24gVHVlLCBPY3QgMTEsIDIwMTYgYXQgMTA6MjQ6MjNQTSArMDEwMCwgQnJpYW4gU3RhcmtleSB3 cm90ZToKPiBPbiBUdWUsIE9jdCAxMSwgMjAxNiBhdCAxMDowMjo0M1BNICswMjAwLCBEYW5pZWwg VmV0dGVyIHdyb3RlOgo+ID4gVGhlIHByb2JsZW0gd2l0aCBqdXN0IHRoYXQgaXMgdGhhdCB0aGVy ZSdzIGxvdHMgb2YgZGlmZmVyZW50IHRoaW5ncwo+ID4gdGhhdCBjYW4gZmVlZCBpbnRvIHRoZSBv dmVyYWxsIG5lZWRzX21vZGVzZXQgdmFyaWFibGUuIFRoYXQncyB3aHkgd2UKPiA+IHNwbGl0IGl0 IHVwIGludG8gbXVsdGlwbGUgYm9vbGVhbnMuCj4gPiAKPiA+IFNvIHllcyB5b3UncmUgc3VwcG9z ZWQgdG8gY2xlYXIgY29ubmVjdG9yc19jaGFuZ2VkIGlmIHRoZXJlIGlzIHNvbWUKPiA+IGNoYW5n ZSB0aGF0IHlvdSBjYW4gaGFuZGxlIHdpdGhvdXQgYSBmdWxsIG1vZGVzZXQuIElmIHlvdSB3YW50 LCB0aGluawo+ID4gb2YgY29ubmVjdG9yc19jaGFuZ2VkIGFzCj4gPiBuZWVkc19tb2Rlc2V0X2R1 ZV90b19jaGFuZ2VfaW5fY29ubm5lY3Rvcl9zdGF0ZSwgYnV0IHRoYXQncyBjdW1iZXJzb21lCj4g PiB0byB0eXBlIGFuZCB0b28gbG9uZyA7LSkKPiA+IAo+IAo+IEFsbCByaWdodCwgZ290IGl0IDot KS4gVGhpcyBpbnRlbnRpb24gd2Fzbid0IGNsZWFyIHRvIG1lIGZyb20gdGhlCj4gY29tbWVudHMg aW4gdGhlIGNvZGUuCgpBIHBhdGNoIHRvIHVwZGF0ZSB0aGUga2VybmVsLWRvYyB0byBtYWtlIGl0 IGNsZWFyZXIgKHRoZXJlJ3MgbW9kZV9jaGFuZ2VkLApjb25uZWN0b3JzX2NoYW5nZWQgYW5kIGFj dGl2ZV9jaGFuZ2VkLCBwbHVzIGRybV9jcnRjX25lZWRzX21vZGVzZXQpIHdvdWxkCmJlIGF3ZXNv bWUuIEknbSB0cnlpbmcgdG8gd3JpdGUgdXNlZnVsIGRvY3MsIGJ1dCBzaW5jZSBJIGRlc2lnbmVk IHRoaXMgYWxsCkkgc29tZXRpbWVzIGZvcmdldCB0byBtYWtlIHRoZSBub24tb2J2aW91cyBhc3N1 bXB0aW9ucyBjbGVhciBlbm91Z2guCgpWb2x1bnRlZXJlZD8KCj4gPiA+ID4gdGJoIEkgZG9uJ3Qg bGlrZSB0aGF0LCBJIHRoaW5rIGl0J2QgYmUgYmV0dGVyIHRvIG1ha2UgdGhpcyB0cnVseQo+ID4g PiA+IG9uZS1zaG90LiBPdGhlcndpc2Ugd2UnbGwgaGF2ZSByZWFsIGZ1biBwcm9ibGVtcyB3aXRo IGh3IHdoZXJlIHRoZQo+ID4gPiA+IHdyaXRlYmFjayBjYW4gdGFrZSBsb25nZXIgdGhhbiBhIHZi bGFuayAoaXQgaGFwcGVucyAuLi4pLiBTbyBvbmUtc2hvdCwKPiA+ID4gPiB3aXRoIGF1dG8tY2xl YXJpbmcgdG8gTlVMTC8wIGlzIGltbyB0aGUgcmlnaHQgYXBwcm9hY2guCj4gPiA+IAo+ID4gPiBU aGF0J3MgYW4gaW50ZXJlc3RpbmcgcG9pbnQgYWJvdXQgaGFyZHdhcmUgd2hpY2ggd29uJ3QgZmlu aXNoIHdpdGhpbgo+ID4gPiBvbmUgZnJhbWU7IGJ1dCBJIGRvbid0IHNlZSBob3cgInRydWUgb25l LXNob3QiIGhlbHBzLgo+ID4gPiAKPiA+ID4gV2hhdCdzIHRoZSBleHBlY3RlZCBiZWhhdmlvdXIg aWYgdXNlcnNwYWNlIG1ha2VzIGEgbmV3IGF0b21pYyBjb21taXQKPiA+ID4gd2l0aCBhIHdyaXRl YmFjayBmcmFtZWJ1ZmZlciB3aGlsc3QgYSBwcmV2aW91cyB3cml0ZWJhY2sgaXMgb25nb2luZz8K PiA+ID4gCj4gPiA+IEluIGJvdGggY2FzZXMsIHlvdSBlaXRoZXIgbmVlZCB0byBibG9jayBvciBm YWlsIHRoZSBjb21taXQgLSB3aGV0aGVyCj4gPiA+IHRoZSBmcmFtZWJ1ZmZlciBnZXRzIHJlbW92 ZWQgd2hlbiBpdCdzIGRvbmUgaXMgaW1tYXRlcmlhbC4KPiA+IAo+ID4gU2VlIEVyaWMncyBxdWVz dGlvbi4gV2UgbmVlZCB0byBkZWZpbmUgdGhhdCwgYW5kIEkgdGhpbmsgdGhlIHNpbXBsZXN0Cj4g PiBhcHByb2FjaCBpcyBhIGNvbXBsZXRpb24gZmVuY2Uvc3luY19maWxlLiBJdCdzIGRlc3RhZ2Vk IG5vdyBpbiA0LjksIHdlCj4gPiBjYW4gdXNlIHRoZW0uIEkgdGhpbmsgdGhlIHNpbXBsZXN0IHVh Ymkgd291bGQgYmUgYSBwb2ludGVyIHByb3BlcnR5Cj4gPiAodTY0KSB3aGVyZSB3ZSB3cml0ZSB0 aGUgZmQgb2YgdGhlIGZlbmNlIHdlJ2xsIHNpZ25hbCB3aGVuIHdyaXRlLW91dAo+ID4gY29tcGxl dGVzLgo+ID4gCj4gCj4gVGhhdCB0ZWxscyB1c2Vyc3BhY2UgdGhhdCB0aGUgcHJldmlvdXMgd3Jp dGViYWNrIGlzIGZpbmlzaGVkLCBJIGFncmVlIHRoYXQncwo+IG5lZWRlZC4gSXQgZG9lc24ndCBk ZWZpbmUgYW55IGJlaGF2aW91ciBpbiBjYXNlIHVzZXJzcGFjZSBhc2tzIGZvciBhbm90aGVyCj4g d3JpdGViYWNrIGJlZm9yZSB0aGF0IGZlbmNlIGZpcmVzIHRob3VnaC4KCkhtLCBnb29kIHBvaW50 LiBJIGd1ZXNzIHdlIGNvdWxkIGp1c3Qgc3RhdGUgdGhhdCBpZiB1c2Vyc3BhY2UgZG9lcyBhCndy aXRlYmFjaywgYW5kIGlzc3VlcyBhIG5ldyB3cml0ZWJhY2sgYmVmb3JlIGJvdGggYSkgdGhlIGF0 b21pYyBmbGlwIGFuZApiKSB0aGUgd3JpdGUgYmFjayBjb21wbGV0ZSBmZW5jZSBzaWduYWxsZWQg d2lsbCBsZWFkIHRvIHVuZGVmaW5lZApiZWhhdmlvdXIuIFVuZGVmaW5lZCBhcyBpbjogZGF0YSBj b3JydXB0aW9uLCByZWplY3RlZCBhdG9taWMgY29tbWl0IG9yCmFueXRoaW5nIGVsc2UgdGhhbiBh IGtlcm5lbCBjcmFzaCBpcyBhbGxvd2VkLiBUaGlzIGlzIHNpbWlsYXIgdG8gZG9pbmcgYQpwYWdl IGZsaXAgYW5kIHN0YXJ0aW5nIHRvIHJlbmRlciB0byB0aGUgb2xkIGJ1ZmZlcnMgYmVmb3JlIHRo ZSBmbGlwIGV2ZW50CnNpZ25hbGxlZCBjb21wbGV0aW9uOiBVc2Vyc3BhY2UgZ2V0cyB0aGUgbWVz cyBpdCBhc2tlZCBmb3IgOy0pCi1EYW5pZWwKLS0gCkRhbmllbCBWZXR0ZXIKU29mdHdhcmUgRW5n aW5lZXIsIEludGVsIENvcnBvcmF0aW9uCmh0dHA6Ly9ibG9nLmZmd2xsLmNoCl9fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxp c3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNr dG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo=