From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matt Roper Subject: Re: [PATCH v4 0/6] Finally fix watermarks Date: Fri, 29 Jul 2016 13:33:24 -0700 Message-ID: <20160729203324.GT32025@intel.com> References: <1469554483-24999-1-git-send-email-cpaul@redhat.com> <20160729000352.GR32025@intel.com> <20160729093905.GU4329@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20160729093905.GU4329@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: David Airlie , intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Daniel Vetter List-Id: dri-devel@lists.freedesktop.org T24gRnJpLCBKdWwgMjksIDIwMTYgYXQgMTI6Mzk6MDVQTSArMDMwMCwgVmlsbGUgU3lyasOkbMOk IHdyb3RlOgo+IE9uIFRodSwgSnVsIDI4LCAyMDE2IGF0IDA1OjAzOjUyUE0gLTA3MDAsIE1hdHQg Um9wZXIgd3JvdGU6Cj4gPiBUaGlzIGlzIGNvbXBsZXRlbHkgdW50ZXN0ZWQgKGFuZCBwcm9iYWJs eSBob3JyaWJseSBicm9rZW4vYnVnZ3kpLCBidXQKPiA+IGhlcmUncyBhIHF1aWNrIG1vY2t1cCBv ZiB0aGUgZ2VuZXJhbCBhcHByb2FjaCBJIHdhcyB0aGlua2luZyBmb3IKPiA+IGVuc3VyaW5nIERE QiAmIFdNJ3MgY2FuIGJlIHVwZGF0ZWQgdG9nZXRoZXIgd2hpbGUgZW5zdXJpbmcgdGhlCj4gPiB0 aHJlZS1zdGVwIHBpcGUgZmx1c2hpbmcgcHJvY2VzcyBpcyBob25vcmVkOgo+ID4gCj4gPiAgICAg ICAgIGh0dHBzOi8vZ2l0aHViLmNvbS9tYXR0cm9wZS9rZXJuZWwvY29tbWl0cy9leHBlcmltZW50 YWwvbHl1ZGVfZGRiCj4gPiAKPiA+IEJhc2ljYWxseSB0aGUgaWRlYSBpcyB0byB0YWtlIG5vdGUg b2Ygd2hhdCdzIGhhcHBlbmluZyB0byB0aGUgcGlwZSdzIEREQgo+ID4gYWxsb2NhdGlvbiAoc2hy aW5raW5nLCBncm93aW5nLCB1bmNoYW5nZWQsIGV0Yy4pIGR1cmluZyB0aGUgYXRvbWljIGNoZWNr Cj4gPiBwaGFzZTsKPiAKPiBEaWRuJ3QgbG9vayB0b28gY2xvc2VseSwgYnV0IEkgdGhpbmsgeW91 IGNhbid0IGFjdHVhbGx5IGRvIHRoYXQgdW5sZXNzCj4geW91IGxvY2sgYWxsIHRoZSBjcnRjcyB3 aGVuZXZlciB0aGUgbnVtYmVyIG9mIGFjdGl2ZSBwaXBlcyBpcyBnb2luZCB0bwo+IGNoYW5nZS4g TWVhbmluZyB3ZSdkIGVzc2VudGlhbGx5IGJlIGJhY2sgdG8gdGhlIG9uZS1iaWctbW9kZXNldC1s b2NrCj4gYXBwb3JhY2gsIHdoaWNoIHdpbGwgY2F1c2UgbWlzc2VkIGZsaXBzIGFuZCB3aGFub3Qg b24gdGhlIG90aGVyIHBpcGVzLgo+IAo+IFRoZSBhbHRlcm5hdGl2ZSBJIHRoaW5rIHdvdWxkIGNv bnNpc3Qgb2Y6Cj4gLSBtYWtlIHN1cmUgbGV2ZWwgMCB3YXRlcm1hcmsgbmV2ZXIgZXhjZWVkcyB0 b3RhbF9kZGJfc2l6ZS9tYXhfcGlwZXMsCj4gICBzbyB0aGF0IGEgbW9kZXNldCBkb2Vzbid0IGhh dmUgdG8gY2FyZSBhYm91dCB0aGUgd21zIGZvciB0aGUgb3RoZXIKPiAgIHBpcGVzIG5vdCBmaXR0 aW5nIGluCgpVbmZvcnR1bmF0ZWx5IHRoaXMgcGFydCBpcyB0aGUgcHJvYmxlbS4gIFlvdSBtaWdo dCBnZXQgYXdheSB3aXRoIGRvaW5nCnRoaXMgb24gU0tML0tCTCB3aGljaCBvbmx5IGhhdmUgdGhy ZWUgcGxhbmVzIG1heCBwZXIgcGlwZSBhbmQgYSBsYXJnZQooODk2IGJsb2NrKSBEREIsIGJ1dCBv biBCWFQgeW91IGhhdmUgdXAgdG8gZm91ciBwbGFuZXMgKHdlIGRvbid0CmFjdHVhbGx5IGVuYWJs ZSB0aGUgdG9wbW9zdCBwbGFuZSBpbiBhIGZ1bGwtZmVhdHVyZWQgbWFubmVyIGp1c3QgeWV0LApi dXQgbmVlZCB0byBzb29uKSwgeWV0IHRoZSB0b3RhbCBEREIgaXMgb25seSA1MTIgYmxvY2tzLiAg U2FkbHksIHRoZQpwbGF0Zm9ybSB3aXRoIG1vcmUgcGxhbmVzIHdhcyBnaXZlbiBhIHNtYWxsZXIg RERCLi4uICAgOi0oCgpXZSdyZSBhbHJlYWR5IGluIHRyb3VibGUgYmVjYXVzZSB1c2VycyB0aGF0 IGFyZSBydW5uaW5nIHNldHVwcyBsaWtlIDN4CjRLIHdpdGggbW9zdC9hbGwgcGxhbmVzIGluIHVz ZSBhdCBsYXJnZSBzaXplcyBjYW4ndCBmaW5kIGxldmVsIDAKd2F0ZXJtYXJrcyB0aGF0IHNhdGlz ZnkgdGhlaXIgbmVlZHMgYW5kIHdlIGhhdmUgdG8gcmVqZWN0IHRoZSB3aG9sZQpjb25maWd1cmF0 aW9uLiAgSWYgd2UgZnVydGhlciBsaW1pdCBlYWNoIHBpcGUncyB1c2FnZSB0byB0b3RhbC9tYXhw aXBlcwooaS5lLiwgMTcwIGJsb2NrcyBwZXIgcGlwZSBvbiBCWFQpLCB0aGVuIHdlJ3JlIGdvaW5n IHRvIGhpdCBzaW1pbGFyCmlzc3VlcyB3aGVuIG9ubHkgZHJpdmluZyBvbmUgb3IgdHdvIGRpc3Bs YXlzIHdpdGggd2l0aCBhbGwgb2YgdGhlIHBsYW5lcwppbiB1c2UsIGV2ZW4gdGhvdWdoIHdlIHNo b3VsZCBoYXZlIGhhZCBtb3JlIEREQiBzcGFjZSB0byB3b3JrIHdpdGguCgpJIGd1ZXNzIHNlcmlv dXMgcGxhbmUgdXNhZ2UgaXNuJ3QgdG9vIGNvbW1vbiBpbiBkZXNrdG9wIHNldHVwcyB0b2RheSwK YnV0IGl0J3MgYSB2ZXJ5IGNyaXRpY2FsIGZlYXR1cmUgaW4gdGhlIGVtYmVkZGVkIHdvcmxkOyB3 ZSBjYW4ndCByZWFsbHkKYWZmb3JkIHRvIGNyaXBwbGUgcGxhbmUgdXNhZ2UgZnVydGhlci4gIFVu Zm9ydHVuYXRlbHksIGFzIHlvdSBwb2ludCBvdXQKYWJvdmUsIHRoaXMgbWVhbnMgdGhhdCB3ZSBo YXZlIHRvIGZvbGxvdyB0aGUgYnNwZWMncyBEREIgYWxsb2NhdGlvbgptZXRob2QsIHdoaWNoIGlu IHR1cm4gbWVhbnMgd2UgbmVlZCB0byBncmFiIF9hbGxfIENSVEMgbG9ja3MgYW55IHRpbWUKX2Fu eV8gQ1JUQyBpcyBiZWluZyB0dXJuZWQgb24gb3IgdHVybmVkIG9mZiB3aGljaCBpcyBhIEJLTC1z dHlsZSB3YXkgb2YKZG9pbmcgdGhpbmdzLgoKCk1hdHQKCj4gLSBsZXZlbCAxKyB3YXRlcm1hcmtz IHdvdWxkIGJlIGNoZWNrZWQgYWdhaW5zdCB0b3RhbF9kZGJfc2l6ZQo+IC0gcHJvdGVjdCB0aGUg cGxhbmUvcGlwZSBjb21taXQgd2l0aCB0aGUgd20gbXV0ZXggd2hlbmV2ZXIgdGhlIHdtcwo+ICAg bmVlZCB0byBiZSByZXByb2dyYW1tZWQKPiAtIGtlZXAgdGhlIGZsdXNoX3dtIHRoaW5nIGFyb3Vu ZCBmb3IgdGhlIGNhc2Ugd2hlbiBkZGIgc2l6ZSBkb2VzIGdldAo+ICAgY2hhbmdlZCwgcHJvdGVj dCBpdCB3aXRoIHRoZSB3bSBsb2NrCj4gLSB3aGVuIHByb2dyYW1taW5nIHdtcywgd2Ugd2lsbCBm aXJzdCBmaWx0ZXIgb3V0IGFueSBsZXZlbCB0aGF0Cj4gICBkb2Vzbid0IGZpdCBpbiB3aXRoIHRo ZSBjdXJyZW50IGRkYiBzaXplLCBhbmQgdGhlbiBwcm9ncmFtIHRoZQo+ICAgcmVzdCBpbgo+IC0g cG90ZW50aWFsbHkgaW50cm9kdWNlIHBlci1waXBlIHdtIGxvY2tzIGlmIHRoZSBvbmUgYmlnIGxv Y2sgbG9va3MKPiAgIGxpa2UgYW4gaXNzdWUsIHdoaWNoIGl0IG1pZ2h0IGlmIHRoZSBmbHVzaF93 bSBob2xkcyBpdCBhbGwgdGhlIHdheQo+ICAgdGhyb3VnaAo+IAo+ID4gdGhlbiBkdXJpbmcgdGhl IGNvbW1pdCBwaGFzZSwgd2UgbG9vcCBvdmVyIHRoZSBDUlRDJ3MgdGhyZWUgdGltZXMKPiA+IGlu c3RlYWQgb2YganVzdCBvbmNlLCBidXQgb25seSBvcGVyYXRlIG9uIGEgc3Vic2V0IG9mIHRoZSBD UlRDJ3MgaW4gZWFjaAo+ID4gbG9vcC4gIFdoaWxlIG9wZXJhdGluZyBvbiBlYWNoIENSVEMsIHRo ZSBwbGFuZSwgV00sIGFuZCBEREIgYWxsIGdldAo+ID4gcHJvZ3JhbW1lZCB0b2dldGhlciBhbmQg aGF2ZSBhIHNpbmdsZSBmbHVzaCBmb3IgYWxsIHRocmVlLgo+ID4KPiA+IAo+ID4gCj4gPiAKPiA+ IE1hdHQKPiA+IAo+ID4gT24gVHVlLCBKdWwgMjYsIDIwMTYgYXQgMDE6MzQ6MzZQTSAtMDQwMCwg THl1ZGUgd3JvdGU6Cj4gPiA+IExhdGVzdCB2ZXJzaW9uIG9mIGh0dHBzOi8vbGttbC5vcmcvbGtt bC8yMDE2LzcvMjYvMjkwIC4gUmVzZW5kaW5nIHRoZSB3aG9sZQo+ID4gPiB0aGluZyB0byBrZWVw IGl0IGluIG9uZSBwbGFjZS4KPiA+ID4gCj4gPiA+IEx5dWRlICg1KToKPiA+ID4gICBkcm0vaTkx NS9za2w6IEFkZCBzdXBwb3J0IGZvciB0aGUgU0FHViwgZml4IHVuZGVycnVuIGhhbmdzCj4gPiA+ ICAgZHJtL2k5MTUvc2tsOiBPbmx5IGZsdXNoIHBpcGVzIHdoZW4gd2UgY2hhbmdlIHRoZSBkZGIg YWxsb2NhdGlvbgo+ID4gPiAgIGRybS9pOTE1L3NrbDogRml4IGV4dHJhIHdoaXRlc3BhY2UgaW4g c2tsX2ZsdXNoX3dtX3ZhbHVlcygpCj4gPiA+ICAgZHJtL2k5MTUvc2tsOiBVcGRhdGUgcGxhbmUg d2F0ZXJtYXJrcyBhdG9taWNhbGx5IGR1cmluZyBwbGFuZSB1cGRhdGVzCj4gPiA+ICAgZHJtL2k5 MTUvc2tsOiBBbHdheXMgd2FpdCBmb3IgcGlwZXMgdG8gdXBkYXRlIGFmdGVyIGEgZmx1c2gKPiA+ ID4gCj4gPiA+IE1hdHQgUm9wZXIgKDEpOgo+ID4gPiAgIGRybS9pOTE1L2dlbjk6IE9ubHkgY29w eSBXTSByZXN1bHRzIGZvciBjaGFuZ2VkIHBpcGVzIHRvIHNrbF9odwo+ID4gPiAKPiA+ID4gIGRy aXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZHJ2LmggICAgICB8ICAgMyArCj4gPiA+ICBkcml2ZXJz L2dwdS9kcm0vaTkxNS9pOTE1X3JlZy5oICAgICAgfCAgIDUgKwo+ID4gPiAgZHJpdmVycy9ncHUv ZHJtL2k5MTUvaW50ZWxfZGlzcGxheS5jIHwgIDI0ICsrKysKPiA+ID4gIGRyaXZlcnMvZ3B1L2Ry bS9pOTE1L2ludGVsX2Rydi5oICAgICB8ICAgNCArCj4gPiA+ICBkcml2ZXJzL2dwdS9kcm0vaTkx NS9pbnRlbF9wbS5jICAgICAgfCAyNDAgKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKy0t LS0KPiA+ID4gIGRyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX3Nwcml0ZS5jICB8ICAgMiArCj4g PiA+ICA2IGZpbGVzIGNoYW5nZWQsIDI1NSBpbnNlcnRpb25zKCspLCAyMyBkZWxldGlvbnMoLSkK PiA+ID4gCj4gPiA+IC0tIAo+ID4gPiAyLjcuNAo+ID4gPiAKPiA+ID4gX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KPiA+ID4gSW50ZWwtZ2Z4IG1haWxpbmcg bGlzdAo+ID4gPiBJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCj4gPiA+IGh0dHBzOi8v bGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cj4gPiAKPiA+ IC0tIAo+ID4gTWF0dCBSb3Blcgo+ID4gR3JhcGhpY3MgU29mdHdhcmUgRW5naW5lZXIKPiA+IElv VEcgUGxhdGZvcm0gRW5hYmxpbmcgJiBEZXZlbG9wbWVudAo+ID4gSW50ZWwgQ29ycG9yYXRpb24K PiA+ICg5MTYpIDM1Ni0yNzk1Cj4gCj4gLS0gCj4gVmlsbGUgU3lyasOkbMOkCj4gSW50ZWwgT1RD CgotLSAKTWF0dCBSb3BlcgpHcmFwaGljcyBTb2Z0d2FyZSBFbmdpbmVlcgpJb1RHIFBsYXRmb3Jt IEVuYWJsaW5nICYgRGV2ZWxvcG1lbnQKSW50ZWwgQ29ycG9yYXRpb24KKDkxNikgMzU2LTI3OTUK X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753760AbcG2Uda (ORCPT ); Fri, 29 Jul 2016 16:33:30 -0400 Received: from mga11.intel.com ([192.55.52.93]:25470 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752772AbcG2UdZ (ORCPT ); Fri, 29 Jul 2016 16:33:25 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,441,1464678000"; d="scan'208";a="856043747" Date: Fri, 29 Jul 2016 13:33:24 -0700 From: Matt Roper To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: Lyude , intel-gfx@lists.freedesktop.org, Maarten Lankhorst , David Airlie , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Daniel Vetter Subject: Re: [Intel-gfx] [PATCH v4 0/6] Finally fix watermarks Message-ID: <20160729203324.GT32025@intel.com> References: <1469554483-24999-1-git-send-email-cpaul@redhat.com> <20160729000352.GR32025@intel.com> <20160729093905.GU4329@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20160729093905.GU4329@intel.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jul 29, 2016 at 12:39:05PM +0300, Ville Syrjälä wrote: > On Thu, Jul 28, 2016 at 05:03:52PM -0700, Matt Roper wrote: > > This is completely untested (and probably horribly broken/buggy), but > > here's a quick mockup of the general approach I was thinking for > > ensuring DDB & WM's can be updated together while ensuring the > > three-step pipe flushing process is honored: > > > > https://github.com/mattrope/kernel/commits/experimental/lyude_ddb > > > > Basically the idea is to take note of what's happening to the pipe's DDB > > allocation (shrinking, growing, unchanged, etc.) during the atomic check > > phase; > > Didn't look too closely, but I think you can't actually do that unless > you lock all the crtcs whenever the number of active pipes is goind to > change. Meaning we'd essentially be back to the one-big-modeset-lock > apporach, which will cause missed flips and whanot on the other pipes. > > The alternative I think would consist of: > - make sure level 0 watermark never exceeds total_ddb_size/max_pipes, > so that a modeset doesn't have to care about the wms for the other > pipes not fitting in Unfortunately this part is the problem. You might get away with doing this on SKL/KBL which only have three planes max per pipe and a large (896 block) DDB, but on BXT you have up to four planes (we don't actually enable the topmost plane in a full-featured manner just yet, but need to soon), yet the total DDB is only 512 blocks. Sadly, the platform with more planes was given a smaller DDB... :-( We're already in trouble because users that are running setups like 3x 4K with most/all planes in use at large sizes can't find level 0 watermarks that satisfy their needs and we have to reject the whole configuration. If we further limit each pipe's usage to total/maxpipes (i.e., 170 blocks per pipe on BXT), then we're going to hit similar issues when only driving one or two displays with with all of the planes in use, even though we should have had more DDB space to work with. I guess serious plane usage isn't too common in desktop setups today, but it's a very critical feature in the embedded world; we can't really afford to cripple plane usage further. Unfortunately, as you point out above, this means that we have to follow the bspec's DDB allocation method, which in turn means we need to grab _all_ CRTC locks any time _any_ CRTC is being turned on or turned off which is a BKL-style way of doing things. Matt > - level 1+ watermarks would be checked against total_ddb_size > - protect the plane/pipe commit with the wm mutex whenever the wms > need to be reprogrammed > - keep the flush_wm thing around for the case when ddb size does get > changed, protect it with the wm lock > - when programming wms, we will first filter out any level that > doesn't fit in with the current ddb size, and then program the > rest in > - potentially introduce per-pipe wm locks if the one big lock looks > like an issue, which it might if the flush_wm holds it all the way > through > > > then during the commit phase, we loop over the CRTC's three times > > instead of just once, but only operate on a subset of the CRTC's in each > > loop. While operating on each CRTC, the plane, WM, and DDB all get > > programmed together and have a single flush for all three. > > > > > > > > > > Matt > > > > On Tue, Jul 26, 2016 at 01:34:36PM -0400, Lyude wrote: > > > Latest version of https://lkml.org/lkml/2016/7/26/290 . Resending the whole > > > thing to keep it in one place. > > > > > > Lyude (5): > > > drm/i915/skl: Add support for the SAGV, fix underrun hangs > > > drm/i915/skl: Only flush pipes when we change the ddb allocation > > > drm/i915/skl: Fix extra whitespace in skl_flush_wm_values() > > > drm/i915/skl: Update plane watermarks atomically during plane updates > > > drm/i915/skl: Always wait for pipes to update after a flush > > > > > > Matt Roper (1): > > > drm/i915/gen9: Only copy WM results for changed pipes to skl_hw > > > > > > drivers/gpu/drm/i915/i915_drv.h | 3 + > > > drivers/gpu/drm/i915/i915_reg.h | 5 + > > > drivers/gpu/drm/i915/intel_display.c | 24 ++++ > > > drivers/gpu/drm/i915/intel_drv.h | 4 + > > > drivers/gpu/drm/i915/intel_pm.c | 240 +++++++++++++++++++++++++++++++---- > > > drivers/gpu/drm/i915/intel_sprite.c | 2 + > > > 6 files changed, 255 insertions(+), 23 deletions(-) > > > > > > -- > > > 2.7.4 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Matt Roper > > Graphics Software Engineer > > IoTG Platform Enabling & Development > > Intel Corporation > > (916) 356-2795 > > -- > Ville Syrjälä > Intel OTC -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795