From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A8848C5DF62 for ; Tue, 5 Nov 2019 16:23:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7227921882 for ; Tue, 5 Nov 2019 16:23:42 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ADZ8m3Wu" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390293AbfKEQXl (ORCPT ); Tue, 5 Nov 2019 11:23:41 -0500 Received: from mail-ed1-f65.google.com ([209.85.208.65]:35569 "EHLO mail-ed1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390163AbfKEQXk (ORCPT ); Tue, 5 Nov 2019 11:23:40 -0500 Received: by mail-ed1-f65.google.com with SMTP id r16so1879944edq.2; Tue, 05 Nov 2019 08:23:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=iBa+KNl3xfEoQ3x091yW97I1pFeckDAp+HE7IN9AsKI=; b=ADZ8m3Wuu05byt5FiXJhBT03hcjcIPcfov3vj/h4AVSisEIHZv9YfyIkUT2yDdd4dx B0WixpljI53ZSMmyqljoOzIpkP2LuXY8q1vsJVhIMk/imCrzuPLrJzMyLbBzQhTHJcQ3 R4bwlQ771m5PoPm1m9/UDGcBbh5qNQ6wl/QR2DsanVV1tJzcm7ppsmlvpQh4HjfNbG5t 63hV9BAt6JFSGsTOVQemjsGCjUeCkNqKjkqY/D7rG5gPA76/kjtkk8ORD20jUVmfhzho 8FiJ2Q40mtGYzMsduyUFwURe02dZkTSI3PLMEjzjKAQyod30gBQVCzquG/kQuAr4bEA1 HtTw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=iBa+KNl3xfEoQ3x091yW97I1pFeckDAp+HE7IN9AsKI=; b=bSc93u1iKmt8RKJHZwplrhOL22GmfXy24kCg8iY6N0og23zD9vXR9J3GP+go5OObNO 90x3mt3818uarDPDCvOwck1zWnhJpAhz+VrjXWGYNuPn9OAD+axfT3C48NA26/RHy6cD 3ti5pGlEHuRYLIrP9hoQ/uauB7yDw4sLbIPBSZZvPAd1OnLVFit+DRGmbXW1iNjsMNCD fVRmr5D6Ld5451js6bLcPi5rjPv4rJ5mXriwO0Lp4sN5g4OIrVumabs4gqQaJTLSVJ3P YV4wIGQgtJF1PnPG1glFCBY3njqS+pK74sY6wNxKbGRaCftFb2GV3XNlLqlfkkFVr/JA Vy7g== X-Gm-Message-State: APjAAAWRItUOyWv2p0AEO7yzlzoorGZ/rB0kwlXTyHgQsjgLxAAooukH mvKMJ3ELgGMzC8myFCCsWj6zPgNUqRdWuU6he2s= X-Google-Smtp-Source: APXvYqw6DHiWy5Jq5QER0vWdT0e+O38XHNiZfNfrlZo7CtFTenpQYQGBiQkm25QU55awefud20PSxaXykIphCpiisVU= X-Received: by 2002:a17:906:73d5:: with SMTP id n21mr30279787ejl.228.1572971018138; Tue, 05 Nov 2019 08:23:38 -0800 (PST) MIME-Version: 1.0 References: <20191105000129.GA6536@onstation.org> <20191105100804.GA9492@onstation.org> In-Reply-To: <20191105100804.GA9492@onstation.org> From: Rob Clark Date: Tue, 5 Nov 2019 08:23:27 -0800 Message-ID: Subject: Re: [Freedreno] drm/msm: 'pp done time out' errors after async commit changes To: Brian Masney Cc: Rob Clark , freedreno , Sean Paul , Linux Kernel Mailing List , dri-devel , linux-arm-msm Content-Type: text/plain; charset="UTF-8" Sender: linux-arm-msm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On Tue, Nov 5, 2019 at 2:08 AM Brian Masney wrote: > > On Mon, Nov 04, 2019 at 04:19:07PM -0800, Rob Clark wrote: > > On Mon, Nov 4, 2019 at 4:01 PM Brian Masney wrote: > > > > > > Hey Rob, > > > > > > Since commit 2d99ced787e3 ("drm/msm: async commit support"), the frame > > > buffer console on my Nexus 5 began throwing these errors: > > > > > > msm fd900000.mdss: pp done time out, lm=0 > > > > > > The display still works. > > > > > > I see that mdp5_flush_commit() was introduced in commit 9f6b65642bd2 > > > ("drm/msm: add kms->flush_commit()") with a TODO comment and the commit > > > description mentions flushing registers. I assume that this is the > > > proper fix. If so, can you point me to where these registers are > > > defined and I can work on the mdp5 implementation. > > > > See mdp5_ctl_commit(), which writes the CTL_FLUSH registers.. the idea > > would be to defer writing CTL_FLUSH[ctl_id] = flush_mask until > > kms->flush() (which happens from a timer shortly before vblank). > > > > But I think the async flush case should not come up with fbcon? It > > was really added to cope with hwcursor updates (and userspace that > > assumes it can do an unlimited # of cursor updates per frame).. the > > intention was that nothing should change in the sequence for mdp5 (but > > I guess that was not the case). > > The 'pp done time out' errors go away if I revert the following three > commits: > > cd6d923167b1 ("drm/msm/dpu: async commit support") > d934a712c5e6 ("drm/msm: add atomic traces") > 2d99ced787e3 ("drm/msm: async commit support") > > I reverted the first one to fix a compiler error, and the second one so > that the last patch can be reverted without any merge conflicts. > > I see that crtc_flush() calls mdp5_ctl_commit(). I tried to use > crtc_flush_all() in mdp5_flush_commit() and the contents of the frame > buffer dance around the screen like its out of sync. I renamed > crtc_flush_all() to mdp5_crtc_flush_all() and removed the static > declaration. Here's the relevant part of what I tried: > > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > @@ -171,7 +171,15 @@ static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state *st > > static void mdp5_flush_commit(struct msm_kms *kms, unsigned crtc_mask) > { > - /* TODO */ > + struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms)); > + struct drm_crtc *crtc; > + > + for_each_crtc_mask(mdp5_kms->dev, crtc, crtc_mask) { > + if (!crtc->state->active) > + continue; > + > + mdp5_crtc_flush_all(crtc); > + } > } > > Any tips would be appreciated. I think this is along the lines of what we need to enable async commit for mdp5 (but also removing the flush from the atomic-commit path).. the principle behind the async commit is to do all the atomic state commit normally, but defer writing the flush bits. This way, if you get another async update before the next vblank, you just apply it immediately instead of waiting for vblank. But I guess you are on a command mode panel, if I remember? Which is a case I didn't have a way to test. And I'm not entirely about how kms_funcs->vsync_time() should be implemented for cmd mode panels. That all said, I think we should first fix what is broken, before worrying about extending async commit support to mdp5.. which shouldn't hit the async==true path, due to not implementing kms_funcs->vsync_time(). What I think is going on is that, in the cmd mode case, mdp5_wait_flush() (indirectly) calls mdp5_crtc_wait_for_pp_done(), which waits for a pp-done irq regardless of whether there is a flush in progress. Since there is no flush pending, the irq never comes. But the expectation is that kms_funcs->wait_flush() returns immediately if there is nothing to wait for. BR, -R From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rob Clark Subject: Re: [Freedreno] drm/msm: 'pp done time out' errors after async commit changes Date: Tue, 5 Nov 2019 08:23:27 -0800 Message-ID: References: <20191105000129.GA6536@onstation.org> <20191105100804.GA9492@onstation.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20191105100804.GA9492@onstation.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Brian Masney Cc: Rob Clark , freedreno , Linux Kernel Mailing List , dri-devel , linux-arm-msm , Sean Paul List-Id: dri-devel@lists.freedesktop.org T24gVHVlLCBOb3YgNSwgMjAxOSBhdCAyOjA4IEFNIEJyaWFuIE1hc25leSA8bWFzbmV5YkBvbnN0 YXRpb24ub3JnPiB3cm90ZToKPgo+IE9uIE1vbiwgTm92IDA0LCAyMDE5IGF0IDA0OjE5OjA3UE0g LTA4MDAsIFJvYiBDbGFyayB3cm90ZToKPiA+IE9uIE1vbiwgTm92IDQsIDIwMTkgYXQgNDowMSBQ TSBCcmlhbiBNYXNuZXkgPG1hc25leWJAb25zdGF0aW9uLm9yZz4gd3JvdGU6Cj4gPiA+Cj4gPiA+ IEhleSBSb2IsCj4gPiA+Cj4gPiA+IFNpbmNlIGNvbW1pdCAyZDk5Y2VkNzg3ZTMgKCJkcm0vbXNt OiBhc3luYyBjb21taXQgc3VwcG9ydCIpLCB0aGUgZnJhbWUKPiA+ID4gYnVmZmVyIGNvbnNvbGUg b24gbXkgTmV4dXMgNSBiZWdhbiB0aHJvd2luZyB0aGVzZSBlcnJvcnM6Cj4gPiA+Cj4gPiA+IG1z bSBmZDkwMDAwMC5tZHNzOiBwcCBkb25lIHRpbWUgb3V0LCBsbT0wCj4gPiA+Cj4gPiA+IFRoZSBk aXNwbGF5IHN0aWxsIHdvcmtzLgo+ID4gPgo+ID4gPiBJIHNlZSB0aGF0IG1kcDVfZmx1c2hfY29t bWl0KCkgd2FzIGludHJvZHVjZWQgaW4gY29tbWl0IDlmNmI2NTY0MmJkMgo+ID4gPiAoImRybS9t c206IGFkZCBrbXMtPmZsdXNoX2NvbW1pdCgpIikgd2l0aCBhIFRPRE8gY29tbWVudCBhbmQgdGhl IGNvbW1pdAo+ID4gPiBkZXNjcmlwdGlvbiBtZW50aW9ucyBmbHVzaGluZyByZWdpc3RlcnMuIEkg YXNzdW1lIHRoYXQgdGhpcyBpcyB0aGUKPiA+ID4gcHJvcGVyIGZpeC4gSWYgc28sIGNhbiB5b3Ug cG9pbnQgbWUgdG8gd2hlcmUgdGhlc2UgcmVnaXN0ZXJzIGFyZQo+ID4gPiBkZWZpbmVkIGFuZCBJ IGNhbiB3b3JrIG9uIHRoZSBtZHA1IGltcGxlbWVudGF0aW9uLgo+ID4KPiA+IFNlZSBtZHA1X2N0 bF9jb21taXQoKSwgd2hpY2ggd3JpdGVzIHRoZSBDVExfRkxVU0ggcmVnaXN0ZXJzLi4gdGhlIGlk ZWEKPiA+IHdvdWxkIGJlIHRvIGRlZmVyIHdyaXRpbmcgQ1RMX0ZMVVNIW2N0bF9pZF0gPSBmbHVz aF9tYXNrIHVudGlsCj4gPiBrbXMtPmZsdXNoKCkgKHdoaWNoIGhhcHBlbnMgZnJvbSBhIHRpbWVy IHNob3J0bHkgYmVmb3JlIHZibGFuaykuCj4gPgo+ID4gQnV0IEkgdGhpbmsgdGhlIGFzeW5jIGZs dXNoIGNhc2Ugc2hvdWxkIG5vdCBjb21lIHVwIHdpdGggZmJjb24/ICBJdAo+ID4gd2FzIHJlYWxs eSBhZGRlZCB0byBjb3BlIHdpdGggaHdjdXJzb3IgdXBkYXRlcyAoYW5kIHVzZXJzcGFjZSB0aGF0 Cj4gPiBhc3N1bWVzIGl0IGNhbiBkbyBhbiB1bmxpbWl0ZWQgIyBvZiBjdXJzb3IgdXBkYXRlcyBw ZXIgZnJhbWUpLi4gdGhlCj4gPiBpbnRlbnRpb24gd2FzIHRoYXQgbm90aGluZyBzaG91bGQgY2hh bmdlIGluIHRoZSBzZXF1ZW5jZSBmb3IgbWRwNSAoYnV0Cj4gPiBJIGd1ZXNzIHRoYXQgd2FzIG5v dCB0aGUgY2FzZSkuCj4KPiBUaGUgJ3BwIGRvbmUgdGltZSBvdXQnIGVycm9ycyBnbyBhd2F5IGlm IEkgcmV2ZXJ0IHRoZSBmb2xsb3dpbmcgdGhyZWUKPiBjb21taXRzOgo+Cj4gY2Q2ZDkyMzE2N2Ix ICgiZHJtL21zbS9kcHU6IGFzeW5jIGNvbW1pdCBzdXBwb3J0IikKPiBkOTM0YTcxMmM1ZTYgKCJk cm0vbXNtOiBhZGQgYXRvbWljIHRyYWNlcyIpCj4gMmQ5OWNlZDc4N2UzICgiZHJtL21zbTogYXN5 bmMgY29tbWl0IHN1cHBvcnQiKQo+Cj4gSSByZXZlcnRlZCB0aGUgZmlyc3Qgb25lIHRvIGZpeCBh IGNvbXBpbGVyIGVycm9yLCBhbmQgdGhlIHNlY29uZCBvbmUgc28KPiB0aGF0IHRoZSBsYXN0IHBh dGNoIGNhbiBiZSByZXZlcnRlZCB3aXRob3V0IGFueSBtZXJnZSBjb25mbGljdHMuCj4KPiBJIHNl ZSB0aGF0IGNydGNfZmx1c2goKSBjYWxscyBtZHA1X2N0bF9jb21taXQoKS4gSSB0cmllZCB0byB1 c2UKPiBjcnRjX2ZsdXNoX2FsbCgpIGluIG1kcDVfZmx1c2hfY29tbWl0KCkgYW5kIHRoZSBjb250 ZW50cyBvZiB0aGUgZnJhbWUKPiBidWZmZXIgZGFuY2UgYXJvdW5kIHRoZSBzY3JlZW4gbGlrZSBp dHMgb3V0IG9mIHN5bmMuIEkgcmVuYW1lZAo+IGNydGNfZmx1c2hfYWxsKCkgdG8gbWRwNV9jcnRj X2ZsdXNoX2FsbCgpIGFuZCByZW1vdmVkIHRoZSBzdGF0aWMKPiBkZWNsYXJhdGlvbi4gSGVyZSdz IHRoZSByZWxldmFudCBwYXJ0IG9mIHdoYXQgSSB0cmllZDoKPgo+IC0tLSBhL2RyaXZlcnMvZ3B1 L2RybS9tc20vZGlzcC9tZHA1L21kcDVfa21zLmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vbXNt L2Rpc3AvbWRwNS9tZHA1X2ttcy5jCj4gQEAgLTE3MSw3ICsxNzEsMTUgQEAgc3RhdGljIHZvaWQg bWRwNV9wcmVwYXJlX2NvbW1pdChzdHJ1Y3QgbXNtX2ttcyAqa21zLCBzdHJ1Y3QgZHJtX2F0b21p Y19zdGF0ZSAqc3QKPgo+ICBzdGF0aWMgdm9pZCBtZHA1X2ZsdXNoX2NvbW1pdChzdHJ1Y3QgbXNt X2ttcyAqa21zLCB1bnNpZ25lZCBjcnRjX21hc2spCj4gIHsKPiAtICAgICAgIC8qIFRPRE8gKi8K PiArICAgICAgIHN0cnVjdCBtZHA1X2ttcyAqbWRwNV9rbXMgPSB0b19tZHA1X2ttcyh0b19tZHBf a21zKGttcykpOwo+ICsgICAgICAgc3RydWN0IGRybV9jcnRjICpjcnRjOwo+ICsKPiArICAgICAg IGZvcl9lYWNoX2NydGNfbWFzayhtZHA1X2ttcy0+ZGV2LCBjcnRjLCBjcnRjX21hc2spIHsKPiAr ICAgICAgICAgICAgICAgaWYgKCFjcnRjLT5zdGF0ZS0+YWN0aXZlKQo+ICsgICAgICAgICAgICAg ICAgICAgICAgIGNvbnRpbnVlOwo+ICsKPiArICAgICAgICAgICAgICAgbWRwNV9jcnRjX2ZsdXNo X2FsbChjcnRjKTsKPiArICAgICAgIH0KPiAgfQo+Cj4gQW55IHRpcHMgd291bGQgYmUgYXBwcmVj aWF0ZWQuCgoKSSB0aGluayB0aGlzIGlzIGFsb25nIHRoZSBsaW5lcyBvZiB3aGF0IHdlIG5lZWQg dG8gZW5hYmxlIGFzeW5jIGNvbW1pdApmb3IgbWRwNSAoYnV0IGFsc28gcmVtb3ZpbmcgdGhlIGZs dXNoIGZyb20gdGhlIGF0b21pYy1jb21taXQgcGF0aCkuLgp0aGUgcHJpbmNpcGxlIGJlaGluZCB0 aGUgYXN5bmMgY29tbWl0IGlzIHRvIGRvIGFsbCB0aGUgYXRvbWljIHN0YXRlCmNvbW1pdCBub3Jt YWxseSwgYnV0IGRlZmVyIHdyaXRpbmcgdGhlIGZsdXNoIGJpdHMuICBUaGlzIHdheSwgaWYgeW91 CmdldCBhbm90aGVyIGFzeW5jIHVwZGF0ZSBiZWZvcmUgdGhlIG5leHQgdmJsYW5rLCB5b3UganVz dCBhcHBseSBpdAppbW1lZGlhdGVseSBpbnN0ZWFkIG9mIHdhaXRpbmcgZm9yIHZibGFuay4KCkJ1 dCBJIGd1ZXNzIHlvdSBhcmUgb24gYSBjb21tYW5kIG1vZGUgcGFuZWwsIGlmIEkgcmVtZW1iZXI/ ICBXaGljaCBpcwphIGNhc2UgSSBkaWRuJ3QgaGF2ZSBhIHdheSB0byB0ZXN0LiAgQW5kIEknbSBu b3QgZW50aXJlbHkgYWJvdXQgaG93Cmttc19mdW5jcy0+dnN5bmNfdGltZSgpIHNob3VsZCBiZSBp bXBsZW1lbnRlZCBmb3IgY21kIG1vZGUgcGFuZWxzLgoKVGhhdCBhbGwgc2FpZCwgSSB0aGluayB3 ZSBzaG91bGQgZmlyc3QgZml4IHdoYXQgaXMgYnJva2VuLCBiZWZvcmUKd29ycnlpbmcgYWJvdXQg ZXh0ZW5kaW5nIGFzeW5jIGNvbW1pdCBzdXBwb3J0IHRvIG1kcDUuLiB3aGljaApzaG91bGRuJ3Qg aGl0IHRoZSBhc3luYz09dHJ1ZSBwYXRoLCBkdWUgdG8gbm90IGltcGxlbWVudGluZwprbXNfZnVu Y3MtPnZzeW5jX3RpbWUoKS4KCldoYXQgSSB0aGluayBpcyBnb2luZyBvbiBpcyB0aGF0LCBpbiB0 aGUgY21kIG1vZGUgY2FzZSwKbWRwNV93YWl0X2ZsdXNoKCkgKGluZGlyZWN0bHkpIGNhbGxzIG1k cDVfY3J0Y193YWl0X2Zvcl9wcF9kb25lKCksCndoaWNoIHdhaXRzIGZvciBhIHBwLWRvbmUgaXJx IHJlZ2FyZGxlc3Mgb2Ygd2hldGhlciB0aGVyZSBpcyBhIGZsdXNoCmluIHByb2dyZXNzLiAgU2lu Y2UgdGhlcmUgaXMgbm8gZmx1c2ggcGVuZGluZywgdGhlIGlycSBuZXZlciBjb21lcy4KQnV0IHRo ZSBleHBlY3RhdGlvbiBpcyB0aGF0IGttc19mdW5jcy0+d2FpdF9mbHVzaCgpIHJldHVybnMKaW1t ZWRpYXRlbHkgaWYgdGhlcmUgaXMgbm90aGluZyB0byB3YWl0IGZvci4KCkJSLAotUgpfX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGlu ZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVl ZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWw=