From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lyude Paul Subject: Re: [PATCH] drm/i915/gen9: only add the planes actually affected by ddb changes Date: Thu, 29 Sep 2016 14:45:05 -0400 Message-ID: <1475174705.16365.14.camel@redhat.com> References: <1475174392-27427-1-git-send-email-paulo.r.zanoni@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1F6D86E8EA for ; Thu, 29 Sep 2016 18:45:07 +0000 (UTC) In-Reply-To: <1475174392-27427-1-git-send-email-paulo.r.zanoni@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Paulo Zanoni , intel-gfx@lists.freedesktop.org Cc: stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24gVGh1LCAyMDE2LTA5LTI5IGF0IDE1OjM5IC0wMzAwLCBQYXVsbyBaYW5vbmkgd3JvdGU6Cj4g V2Ugd2VyZSBwcmV2aW91c2x5IGFkZGluZyBhbGwgdGhlIHBsYW5lcyBvd25lZCBieSB0aGUgQ1JU QyBldmVuIHdoZW4KPiB0aGUgZGRiIHBhcnRpdGlvbmluZyBkaWRuJ3QgY2hhbmdlIGZvciB0aGVt LiBBcyBhIGNvbnNlcXVlbmNlLCBhIGxvdAo+IG9mIGZ1bmN0aW9ucyB3ZXJlIGJlaW5nIGNhbGxl ZCB3aGVuIHdlIHdlcmUganVzdCBtb3ZpbmcgdGhlIGN1cnNvcgo+IGFyb3VuZCB0aGUgc2NyZWVu LCBzdWNoIGFzIHNreWxha2VfdXBkYXRlX3ByaW1hcnlfcGxhbmUoKS4KPiAKPiBUaGlzIHdhcyBj YXVzaW5nIGZsaWNrZXJpbmcgb24gdGhlIHByaW1hcnkgcGxhbmUgd2hlbiBtb3ZpbmcgdGhlCj4g Y3Vyc29yLiBJJ20gbm90IDEwMCUgc3VyZSB3aGljaCBvcGVyYXRpb24gY2F1c2VkIHRoZSBmbGlj a2VyaW5nLCBidXQKPiB3ZSB3ZXJlIHdyaXRpbmcgdG8gYSBsb3Qgb2YgcmVnaXN0ZXJzLCBzbyBp dCBjb3VsZCBiZSBhbnkgb2YgdGhlc2UKPiB3cml0ZXMuIFdpdGggdGhpcyBwYXRjaCwganVzdCBt b3ZpbmcgdGhlIG1vdXNlIHdvbid0IGFkZCB0aGUgcHJpbWFyeQo+IHBsYW5lIHRvIHRoZSBjb21t aXQgc2luY2UgaXQgd29uJ3QgdHJpZ2dlciBhIGNoYW5nZSBpbiBEREIKPiBwYXJ0aXRpb25pbmcu Cj4gCj4gRml4ZXM6IDA1YTc2ZDNkNmFkMSAoImRybS9pOTE1L3NrbDogRW5zdXJlIHBpcGVzIHdp dGggY2hhbmdlZCB3bXMgZ2V0Cj4gYWRkZWQgdG8gdGhlIHN0YXRlIikKPiAKPiBCdWd6aWxsYTog aHR0cHM6Ly9idWdzLmZyZWVkZXNrdG9wLm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTc4ODgKPiBDYzog THl1ZGUgPGNwYXVsQHJlZGhhdC5jb20+Cj4gQ2M6IE1pa2UgTG90aGlhbiA8bWlrZUBmaXJlYnVy bi5jby51az4KPiBDYzogc3RhYmxlQHZnZXIua2VybmVsLm9yZwo+IFJlcG9ydGVkLWFuZC1iaXNl Y3RlZC1ieTogTWlrZSBMb3RoaWFuIDxtaWtlQGZpcmVidXJuLmNvLnVrPgo+IFNpZ25lZC1vZmYt Ynk6IFBhdWxvIFphbm9uaSA8cGF1bG8uci56YW5vbmlAaW50ZWwuY29tPgo+IC0tLQo+IMKgZHJp dmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfcG0uYyB8IDQxCj4gKysrKysrKysrKysrKysrKysrKysr KysrKysrKysrKysrKysrKysrKy0KPiDCoDEgZmlsZSBjaGFuZ2VkLCA0MCBpbnNlcnRpb25zKCsp LCAxIGRlbGV0aW9uKC0pCj4gCj4gCj4gSSBjYW4gY29uZmlybSB0aGlzIGZpeGVzIHRoZSBmbGlj a2VyaW5nIEkgd2FzIHNlZWluZy4KPiAKPiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJt L2k5MTUvaW50ZWxfcG0uYwo+IGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfcG0uYwo+IGlu ZGV4IDVkMzlhZDIuLjFjZjM0YTMgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2k5MTUv aW50ZWxfcG0uYwo+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX3BtLmMKPiBAQCAt Mzk2Niw2ICszOTY2LDQ1IEBAIHBpcGVzX21vZGlmaWVkKHN0cnVjdCBkcm1fYXRvbWljX3N0YXRl ICpzdGF0ZSkKPiDCoAlyZXR1cm4gcmV0Owo+IMKgfQo+IMKgCj4gK2ludAo+ICtza2xfZGRiX2Fk ZF9hZmZlY3RlZF9wbGFuZXMoc3RydWN0IGludGVsX2NydGNfc3RhdGUgKmNzdGF0ZSkKPiArewo+ ICsJc3RydWN0IGRybV9hdG9taWNfc3RhdGUgKnN0YXRlID0gY3N0YXRlLT5iYXNlLnN0YXRlOwo+ ICsJc3RydWN0IGRybV9kZXZpY2UgKmRldiA9IHN0YXRlLT5kZXY7Cj4gKwlzdHJ1Y3QgZHJtX2Ny dGMgKmNydGMgPSBjc3RhdGUtPmJhc2UuY3J0YzsKPiArCXN0cnVjdCBpbnRlbF9jcnRjICppbnRl bF9jcnRjID0gdG9faW50ZWxfY3J0YyhjcnRjKTsKPiArCXN0cnVjdCBkcm1faTkxNV9wcml2YXRl ICpkZXZfcHJpdiA9IHRvX2k5MTUoZGV2KTsKPiArCXN0cnVjdCBpbnRlbF9hdG9taWNfc3RhdGUg KmludGVsX3N0YXRlID0KPiB0b19pbnRlbF9hdG9taWNfc3RhdGUoc3RhdGUpOwo+ICsJc3RydWN0 IHNrbF9kZGJfYWxsb2NhdGlvbiAqbmV3X2RkYiA9ICZpbnRlbF9zdGF0ZS0KPiA+d21fcmVzdWx0 cy5kZGI7Cj4gKwlzdHJ1Y3Qgc2tsX2RkYl9hbGxvY2F0aW9uICpjdXJfZGRiID0gJmRldl9wcml2 LQo+ID53bS5za2xfaHcuZGRiOwo+ICsJc3RydWN0IGRybV9wbGFuZV9zdGF0ZSAqcGxhbmVfc3Rh dGU7Cj4gKwlzdHJ1Y3QgZHJtX3BsYW5lICpwbGFuZTsKPiArCWVudW0gcGlwZSBwaXBlID0gaW50 ZWxfY3J0Yy0+cGlwZTsKPiArCWludCBpZDsKPiArCj4gKwlXQVJOX09OKCFkcm1fYXRvbWljX2dl dF9leGlzdGluZ19jcnRjX3N0YXRlKHN0YXRlLCBjcnRjKSk7Cj4gKwo+ICsJZHJtX2Zvcl9lYWNo X3BsYW5lX21hc2socGxhbmUsIGRldiwgY3J0Yy0+c3RhdGUtPnBsYW5lX21hc2spIAo+IHsKPiAr CQlpZCA9IHNrbF93bV9wbGFuZV9pZCh0b19pbnRlbF9wbGFuZShwbGFuZSkpOwo+ICsKPiArCQlp ZiAoY3VyX2RkYi0+cGxhbmVbcGlwZV1baWRdLnN0YXJ0ID09Cj4gKwkJwqDCoMKgwqBuZXdfZGRi LT5wbGFuZVtwaXBlXVtpZF0uc3RhcnQgJiYKPiArCQnCoMKgwqDCoGN1cl9kZGItPnBsYW5lW3Bp cGVdW2lkXS5lbmQgPT0KPiArCQnCoMKgwqDCoG5ld19kZGItPnBsYW5lW3BpcGVdW2lkXS5lbmQg JiYKPiArCQnCoMKgwqDCoGN1cl9kZGItPnlfcGxhbmVbcGlwZV1baWRdLnN0YXJ0ID09Cj4gKwkJ wqDCoMKgwqBuZXdfZGRiLT55X3BsYW5lW3BpcGVdW2lkXS5zdGFydCAmJgo+ICsJCcKgwqDCoMKg Y3VyX2RkYi0+eV9wbGFuZVtwaXBlXVtpZF0uZW5kID09Cj4gKwkJwqDCoMKgwqBuZXdfZGRiLT55 X3BsYW5lW3BpcGVdW2lkXS5lbmQpCj4gKwkJCWNvbnRpbnVlOwoKSnVzdCB1c2Ugc2tsX2RkYl9l bnRyeV9lcXVhbHMoKSBoZXJlLgpXaXRoIHRoYXQgZml4ZWQ6CgpSZXZpZXdlZC1ieTogTHl1ZGUg PGNwYXVsQHJlZGhhdC5jb20+Cgo+ICsKPiArCQlwbGFuZV9zdGF0ZSA9IGRybV9hdG9taWNfZ2V0 X3BsYW5lX3N0YXRlKHN0YXRlLAo+IHBsYW5lKTsKPiArCQlpZiAoSVNfRVJSKHBsYW5lX3N0YXRl KSkKPiArCQkJcmV0dXJuIFBUUl9FUlIocGxhbmVfc3RhdGUpOwo+ICsJfQo+ICsKPiArCXJldHVy biAwOwo+ICt9Cj4gKwo+IMKgc3RhdGljIGludAo+IMKgc2tsX2NvbXB1dGVfZGRiKHN0cnVjdCBk cm1fYXRvbWljX3N0YXRlICpzdGF0ZSkKPiDCoHsKPiBAQCAtNDAzMCw3ICs0MDY5LDcgQEAgc2ts X2NvbXB1dGVfZGRiKHN0cnVjdCBkcm1fYXRvbWljX3N0YXRlICpzdGF0ZSkKPiDCoAkJaWYgKHJl dCkKPiDCoAkJCXJldHVybiByZXQ7Cj4gwqAKPiAtCQlyZXQgPSBkcm1fYXRvbWljX2FkZF9hZmZl Y3RlZF9wbGFuZXMoc3RhdGUsCj4gJmludGVsX2NydGMtPmJhc2UpOwo+ICsJCXJldCA9IHNrbF9k ZGJfYWRkX2FmZmVjdGVkX3BsYW5lcyhjc3RhdGUpOwo+IMKgCQlpZiAocmV0KQo+IMKgCQkJcmV0 dXJuIHJldDsKPiDCoAl9Cl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fCkludGVsLWdmeCBtYWlsaW5nIGxpc3QKSW50ZWwtZ2Z4QGxpc3RzLmZyZWVkZXNrdG9w Lm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2ludGVs LWdmeAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:54870 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932431AbcI2SpH (ORCPT ); Thu, 29 Sep 2016 14:45:07 -0400 Message-ID: <1475174705.16365.14.camel@redhat.com> Subject: Re: [PATCH] drm/i915/gen9: only add the planes actually affected by ddb changes From: Lyude Paul To: Paulo Zanoni , intel-gfx@lists.freedesktop.org Cc: Mike Lothian , stable@vger.kernel.org Date: Thu, 29 Sep 2016 14:45:05 -0400 In-Reply-To: <1475174392-27427-1-git-send-email-paulo.r.zanoni@intel.com> References: <1475174392-27427-1-git-send-email-paulo.r.zanoni@intel.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: On Thu, 2016-09-29 at 15:39 -0300, Paulo Zanoni wrote: > We were previously adding all the planes owned by the CRTC even when > the ddb partitioning didn't change for them. As a consequence, a lot > of functions were being called when we were just moving the cursor > around the screen, such as skylake_update_primary_plane(). > > This was causing flickering on the primary plane when moving the > cursor. I'm not 100% sure which operation caused the flickering, but > we were writing to a lot of registers, so it could be any of these > writes. With this patch, just moving the mouse won't add the primary > plane to the commit since it won't trigger a change in DDB > partitioning. > > Fixes: 05a76d3d6ad1 ("drm/i915/skl: Ensure pipes with changed wms get > added to the state") > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97888 > Cc: Lyude > Cc: Mike Lothian > Cc: stable@vger.kernel.org > Reported-and-bisected-by: Mike Lothian > Signed-off-by: Paulo Zanoni > --- >  drivers/gpu/drm/i915/intel_pm.c | 41 > ++++++++++++++++++++++++++++++++++++++++- >  1 file changed, 40 insertions(+), 1 deletion(-) > > > I can confirm this fixes the flickering I was seeing. > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c > index 5d39ad2..1cf34a3 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3966,6 +3966,45 @@ pipes_modified(struct drm_atomic_state *state) >   return ret; >  } >   > +int > +skl_ddb_add_affected_planes(struct intel_crtc_state *cstate) > +{ > + struct drm_atomic_state *state = cstate->base.state; > + struct drm_device *dev = state->dev; > + struct drm_crtc *crtc = cstate->base.crtc; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct drm_i915_private *dev_priv = to_i915(dev); > + struct intel_atomic_state *intel_state = > to_intel_atomic_state(state); > + struct skl_ddb_allocation *new_ddb = &intel_state- > >wm_results.ddb; > + struct skl_ddb_allocation *cur_ddb = &dev_priv- > >wm.skl_hw.ddb; > + struct drm_plane_state *plane_state; > + struct drm_plane *plane; > + enum pipe pipe = intel_crtc->pipe; > + int id; > + > + WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc)); > + > + drm_for_each_plane_mask(plane, dev, crtc->state->plane_mask) > { > + id = skl_wm_plane_id(to_intel_plane(plane)); > + > + if (cur_ddb->plane[pipe][id].start == > +     new_ddb->plane[pipe][id].start && > +     cur_ddb->plane[pipe][id].end == > +     new_ddb->plane[pipe][id].end && > +     cur_ddb->y_plane[pipe][id].start == > +     new_ddb->y_plane[pipe][id].start && > +     cur_ddb->y_plane[pipe][id].end == > +     new_ddb->y_plane[pipe][id].end) > + continue; Just use skl_ddb_entry_equals() here. With that fixed: Reviewed-by: Lyude > + > + plane_state = drm_atomic_get_plane_state(state, > plane); > + if (IS_ERR(plane_state)) > + return PTR_ERR(plane_state); > + } > + > + return 0; > +} > + >  static int >  skl_compute_ddb(struct drm_atomic_state *state) >  { > @@ -4030,7 +4069,7 @@ skl_compute_ddb(struct drm_atomic_state *state) >   if (ret) >   return ret; >   > - ret = drm_atomic_add_affected_planes(state, > &intel_crtc->base); > + ret = skl_ddb_add_affected_planes(cstate); >   if (ret) >   return ret; >   }