From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965135AbcDLTaA (ORCPT ); Tue, 12 Apr 2016 15:30:00 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:35977 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756401AbcDLT36 (ORCPT ); Tue, 12 Apr 2016 15:29:58 -0400 Date: Tue, 12 Apr 2016 21:30:14 +0200 From: Daniel Vetter To: Liviu Dudau Cc: Dave Airlie , Daniel Stone , David Brown , Brian Starkey , devicetree@vger.kernel.org, LKML , DRI devel Subject: Re: [RFC][PATCH 2/2] drm/arm: Add support for Mali Display Processors Message-ID: <20160412193014.GN2510@phenom.ffwll.local> Mail-Followup-To: Liviu Dudau , Dave Airlie , Daniel Stone , David Brown , Brian Starkey , devicetree@vger.kernel.org, LKML , DRI devel References: <1459527712-9488-1-git-send-email-Liviu.Dudau@arm.com> <1459527712-9488-3-git-send-email-Liviu.Dudau@arm.com> <20160412154757.GG2510@phenom.ffwll.local> <20160412171349.GK16063@e106497-lin.cambridge.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160412171349.GK16063@e106497-lin.cambridge.arm.com> X-Operating-System: Linux phenom 4.4.0-1-amd64 User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Apr 12, 2016 at 06:13:49PM +0100, Liviu Dudau wrote: > On Tue, Apr 12, 2016 at 05:47:57PM +0200, Daniel Vetter wrote: > > On Fri, Apr 01, 2016 at 05:21:52PM +0100, Liviu Dudau wrote: > > > +static int malidp_enable_vblank(struct drm_device *drm, unsigned int crtc) > > > +{ > > > + return 0; > > > +} > > > + > > > +static void malidp_disable_vblank(struct drm_device *drm, unsigned int pipe) > > > +{ > > > +} > > > > Might be worth it to create a patch for drm_irq.c to make > > enable/disable_vblank functions optional. Otoh does your chip really keep > > on generating vblank irqs all the time, with no way to shut it up? That > > would be terrible for power consumption ... Especially since you have no > > hw counter either. > > Initially I had code here that was turning off the vblank irq, then I've read > the comment in drmP.h that the routine should be a no-op when hardware counters > are missing, hence this version. As for the display processor: it will generate > an interrupt for every finished scanout cycle, but it has support for variable > vsync. Interrupt can be disabled, but I've read in the drmP.h that it is required > for timestamping support when one doesn't have hw counters. > > I'm OK with fixing drm_irq.c to not require enable/disable_vblank but then the > comments in drmP.h will also have to change? Nah, you bring up a good point actually - you really can't disable vblank if there's no hw counter. At least not right now. I think dummy functions in drm_irq.c like drm_vblank_get/set_no_hw_counter to make this clear would be nice. Or maybe just a comment here. The other option would be to finally fake this using high-precision timestamps, since a lot of mobile hw seems to have forgotten to add a proper vblank counter. But that has issues (it can drift), and probably better done separately. > > > +static void malidp_de_plane_disable(struct drm_plane *plane, > > > + struct drm_plane_state *state) > > > +{ > > > + struct malidp_plane *mp = to_malidp_plane(plane); > > > + > > > + /* ToDo: figure out the attached framebuffer lifecycle */ > > > > You don't need to figure this out, atomic helpers will take care of the fb > > for you. > > It is more in line with un/pinning the framebuffer and making sure that the > framebuffer has been scanned out before unref-ing it. That should be taken care of by the vblank wait the helpers do for you. Again happans all automatically (except you need to keep that in mind for the async work). > Thanks again for finding time to review the code. No problem. Cheers, 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 2/2] drm/arm: Add support for Mali Display Processors Date: Tue, 12 Apr 2016 21:30:14 +0200 Message-ID: <20160412193014.GN2510@phenom.ffwll.local> References: <1459527712-9488-1-git-send-email-Liviu.Dudau@arm.com> <1459527712-9488-3-git-send-email-Liviu.Dudau@arm.com> <20160412154757.GG2510@phenom.ffwll.local> <20160412171349.GK16063@e106497-lin.cambridge.arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20160412171349.GK16063@e106497-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: Liviu Dudau Cc: devicetree@vger.kernel.org, LKML , DRI devel List-Id: devicetree@vger.kernel.org T24gVHVlLCBBcHIgMTIsIDIwMTYgYXQgMDY6MTM6NDlQTSArMDEwMCwgTGl2aXUgRHVkYXUgd3Jv dGU6Cj4gT24gVHVlLCBBcHIgMTIsIDIwMTYgYXQgMDU6NDc6NTdQTSArMDIwMCwgRGFuaWVsIFZl dHRlciB3cm90ZToKPiA+IE9uIEZyaSwgQXByIDAxLCAyMDE2IGF0IDA1OjIxOjUyUE0gKzAxMDAs IExpdml1IER1ZGF1IHdyb3RlOgo+ID4gPiArc3RhdGljIGludCBtYWxpZHBfZW5hYmxlX3ZibGFu ayhzdHJ1Y3QgZHJtX2RldmljZSAqZHJtLCB1bnNpZ25lZCBpbnQgY3J0YykKPiA+ID4gK3sKPiA+ ID4gKwlyZXR1cm4gMDsKPiA+ID4gK30KPiA+ID4gKwo+ID4gPiArc3RhdGljIHZvaWQgbWFsaWRw X2Rpc2FibGVfdmJsYW5rKHN0cnVjdCBkcm1fZGV2aWNlICpkcm0sIHVuc2lnbmVkIGludCBwaXBl KQo+ID4gPiArewo+ID4gPiArfQo+ID4gCj4gPiBNaWdodCBiZSB3b3J0aCBpdCB0byBjcmVhdGUg YSBwYXRjaCBmb3IgZHJtX2lycS5jIHRvIG1ha2UKPiA+IGVuYWJsZS9kaXNhYmxlX3ZibGFuayBm dW5jdGlvbnMgb3B0aW9uYWwuIE90b2ggZG9lcyB5b3VyIGNoaXAgcmVhbGx5IGtlZXAKPiA+IG9u IGdlbmVyYXRpbmcgdmJsYW5rIGlycXMgYWxsIHRoZSB0aW1lLCB3aXRoIG5vIHdheSB0byBzaHV0 IGl0IHVwPyBUaGF0Cj4gPiB3b3VsZCBiZSB0ZXJyaWJsZSBmb3IgcG93ZXIgY29uc3VtcHRpb24g Li4uIEVzcGVjaWFsbHkgc2luY2UgeW91IGhhdmUgbm8KPiA+IGh3IGNvdW50ZXIgZWl0aGVyLgo+ IAo+IEluaXRpYWxseSBJIGhhZCBjb2RlIGhlcmUgdGhhdCB3YXMgdHVybmluZyBvZmYgdGhlIHZi bGFuayBpcnEsIHRoZW4gSSd2ZSByZWFkCj4gdGhlIGNvbW1lbnQgaW4gZHJtUC5oIHRoYXQgdGhl IHJvdXRpbmUgc2hvdWxkIGJlIGEgbm8tb3Agd2hlbiBoYXJkd2FyZSBjb3VudGVycwo+IGFyZSBt aXNzaW5nLCBoZW5jZSB0aGlzIHZlcnNpb24uIEFzIGZvciB0aGUgZGlzcGxheSBwcm9jZXNzb3I6 IGl0IHdpbGwgZ2VuZXJhdGUKPiBhbiBpbnRlcnJ1cHQgZm9yIGV2ZXJ5IGZpbmlzaGVkIHNjYW5v dXQgY3ljbGUsIGJ1dCBpdCBoYXMgc3VwcG9ydCBmb3IgdmFyaWFibGUKPiB2c3luYy4gSW50ZXJy dXB0IGNhbiBiZSBkaXNhYmxlZCwgYnV0IEkndmUgcmVhZCBpbiB0aGUgZHJtUC5oIHRoYXQgaXQg aXMgcmVxdWlyZWQKPiBmb3IgdGltZXN0YW1waW5nIHN1cHBvcnQgd2hlbiBvbmUgZG9lc24ndCBo YXZlIGh3IGNvdW50ZXJzLgo+IAo+IEknbSBPSyB3aXRoIGZpeGluZyBkcm1faXJxLmMgdG8gbm90 IHJlcXVpcmUgZW5hYmxlL2Rpc2FibGVfdmJsYW5rIGJ1dCB0aGVuIHRoZQo+IGNvbW1lbnRzIGlu IGRybVAuaCB3aWxsIGFsc28gaGF2ZSB0byBjaGFuZ2U/CgpOYWgsIHlvdSBicmluZyB1cCBhIGdv b2QgcG9pbnQgYWN0dWFsbHkgLSB5b3UgcmVhbGx5IGNhbid0IGRpc2FibGUgdmJsYW5rCmlmIHRo ZXJlJ3Mgbm8gaHcgY291bnRlci4gQXQgbGVhc3Qgbm90IHJpZ2h0IG5vdy4gSSB0aGluayBkdW1t eSBmdW5jdGlvbnMKaW4gZHJtX2lycS5jIGxpa2UgZHJtX3ZibGFua19nZXQvc2V0X25vX2h3X2Nv dW50ZXIgdG8gbWFrZSB0aGlzIGNsZWFyCndvdWxkIGJlIG5pY2UuIE9yIG1heWJlIGp1c3QgYSBj b21tZW50IGhlcmUuCgpUaGUgb3RoZXIgb3B0aW9uIHdvdWxkIGJlIHRvIGZpbmFsbHkgZmFrZSB0 aGlzIHVzaW5nIGhpZ2gtcHJlY2lzaW9uCnRpbWVzdGFtcHMsIHNpbmNlIGEgbG90IG9mIG1vYmls ZSBodyBzZWVtcyB0byBoYXZlIGZvcmdvdHRlbiB0byBhZGQgYQpwcm9wZXIgdmJsYW5rIGNvdW50 ZXIuIEJ1dCB0aGF0IGhhcyBpc3N1ZXMgKGl0IGNhbiBkcmlmdCksIGFuZCBwcm9iYWJseQpiZXR0 ZXIgZG9uZSBzZXBhcmF0ZWx5LgoKPiA+ID4gK3N0YXRpYyB2b2lkIG1hbGlkcF9kZV9wbGFuZV9k aXNhYmxlKHN0cnVjdCBkcm1fcGxhbmUgKnBsYW5lLAo+ID4gPiArCQkJCSAgICBzdHJ1Y3QgZHJt X3BsYW5lX3N0YXRlICpzdGF0ZSkKPiA+ID4gK3sKPiA+ID4gKwlzdHJ1Y3QgbWFsaWRwX3BsYW5l ICptcCA9IHRvX21hbGlkcF9wbGFuZShwbGFuZSk7Cj4gPiA+ICsKPiA+ID4gKwkvKiBUb0RvOiBm aWd1cmUgb3V0IHRoZSBhdHRhY2hlZCBmcmFtZWJ1ZmZlciBsaWZlY3ljbGUgKi8KPiA+IAo+ID4g WW91IGRvbid0IG5lZWQgdG8gZmlndXJlIHRoaXMgb3V0LCBhdG9taWMgaGVscGVycyB3aWxsIHRh a2UgY2FyZSBvZiB0aGUgZmIKPiA+IGZvciB5b3UuCj4gCj4gSXQgaXMgbW9yZSBpbiBsaW5lIHdp dGggdW4vcGlubmluZyB0aGUgZnJhbWVidWZmZXIgYW5kIG1ha2luZyBzdXJlIHRoYXQgdGhlCj4g ZnJhbWVidWZmZXIgaGFzIGJlZW4gc2Nhbm5lZCBvdXQgYmVmb3JlIHVucmVmLWluZyBpdC4KClRo YXQgc2hvdWxkIGJlIHRha2VuIGNhcmUgb2YgYnkgdGhlIHZibGFuayB3YWl0IHRoZSBoZWxwZXJz IGRvIGZvciB5b3UuCkFnYWluIGhhcHBhbnMgYWxsIGF1dG9tYXRpY2FsbHkgKGV4Y2VwdCB5b3Ug bmVlZCB0byBrZWVwIHRoYXQgaW4gbWluZCBmb3IKdGhlIGFzeW5jIHdvcmspLgoKPiBUaGFua3Mg YWdhaW4gZm9yIGZpbmRpbmcgdGltZSB0byByZXZpZXcgdGhlIGNvZGUuCgpObyBwcm9ibGVtLgoK Q2hlZXJzLCBEYW5pZWwKLS0gCkRhbmllbCBWZXR0ZXIKU29mdHdhcmUgRW5naW5lZXIsIEludGVs IENvcnBvcmF0aW9uCmh0dHA6Ly9ibG9nLmZmd2xsLmNoCl9fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVs QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWls bWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo=