From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm: atomic: Clarify documentation around drm_atomic_crtc_needs_modeset Date: Mon, 17 Oct 2016 08:07:37 +0200 Message-ID: <20161017060737.GP20761@phenom.ffwll.local> References: <20161012065618.GI20761@phenom.ffwll.local> <1476352028-16701-1-git-send-email-brian.starkey@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-lf0-x241.google.com (mail-lf0-x241.google.com [IPv6:2a00:1450:4010:c07::241]) by gabe.freedesktop.org (Postfix) with ESMTPS id A99F56E073 for ; Mon, 17 Oct 2016 06:07:42 +0000 (UTC) Received: by mail-lf0-x241.google.com with SMTP id l131so21919336lfl.0 for ; Sun, 16 Oct 2016 23:07:42 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Alex Deucher Cc: Maling list - DRI developers , LKML List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCBPY3QgMTMsIDIwMTYgYXQgMTA6NTQ6NTFBTSAtMDQwMCwgQWxleCBEZXVjaGVyIHdy b3RlOgo+IE9uIFRodSwgT2N0IDEzLCAyMDE2IGF0IDU6NDcgQU0sIEJyaWFuIFN0YXJrZXkgPGJy aWFuLnN0YXJrZXlAYXJtLmNvbT4gd3JvdGU6Cj4gPiBBZGQgc29tZSBhZGRpdGlvbmFsIGNvbW1l bnRzIHRvIG1vcmUgZXhwbGljaXRseSBkZXNjcmliZSB0aGUgbWVhbmluZyBhbmQKPiA+IHVzYWdl IG9mIHRoZSB0aHJlZSBDUlRDIG1vZGVzZXQgZGV0ZWN0aW9uIGJvb2xlYW5zOiBtb2RlX2NoYW5n ZWQsCj4gPiBjb25uZWN0b3JzX2NoYW5nZWQgYW5kIGFjdGl2ZV9jaGFuZ2VkLgo+ID4KPiA+IFN1 Z2dlc3RlZC1ieTogRGFuaWVsIFZldHRlciA8ZGFuaWVsLnZldHRlckBmZndsbC5jaD4KPiA+IFNp Z25lZC1vZmYtYnk6IEJyaWFuIFN0YXJrZXkgPGJyaWFuLnN0YXJrZXlAYXJtLmNvbT4KPiA+IC0t LQo+ID4KPiA+IEhpIERhbmllbCwKPiA+Cj4gPiBJIGd1ZXNzIEkgYXNrZWQgZm9yIHRoaXMgb25l IDotKSwgcGxlYXNlIGp1c3QgY2hlY2sgbXkgdW5kZXJzdGFuZGluZwo+ID4gaXMgY29ycmVjdC4K PiA+Cj4gCj4gVGhlIHdob2xlIHRocmVhZCB3YXMgdmVyeSBlbmxpZ2h0ZW5pbmcgZm9yIG1lIHdp dGggcmVzcGVjdCB0byB0aG9zZQo+IGZsYWdzIGFzIHdlbGwuICBUaGUgcGF0Y2ggbG9va3MgZ29v ZCB0byBtZS4KPiBBY2tlZC1ieTogQWxleCBEZXVjaGVyIDxhbGV4YW5kZXIuZGV1Y2hlckBhbWQu Y29tPgoKQXBwbGllZCB0byBkcm0tbWlzYywgdGhhbmtzLgotRGFuaWVsCgo+IAo+ID4gVGhhbmtz LAo+ID4gQnJpYW4KPiA+Cj4gPiAgZHJpdmVycy9ncHUvZHJtL2RybV9hdG9taWNfaGVscGVyLmMg fCAgICA5ICsrKysrLS0tLQo+ID4gIGluY2x1ZGUvZHJtL2RybV9hdG9taWMuaCAgICAgICAgICAg IHwgICAxMSArKysrKysrKysrLQo+ID4gIGluY2x1ZGUvZHJtL2RybV9jcnRjLmggICAgICAgICAg ICAgIHwgICAgNSArKysrKwo+ID4gIDMgZmlsZXMgY2hhbmdlZCwgMjAgaW5zZXJ0aW9ucygrKSwg NSBkZWxldGlvbnMoLSkKPiA+Cj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2RybV9h dG9taWNfaGVscGVyLmMgYi9kcml2ZXJzL2dwdS9kcm0vZHJtX2F0b21pY19oZWxwZXIuYwo+ID4g aW5kZXggNzhlYTczNS4uZmI0MDcxYSAxMDA2NDQKPiA+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9k cm1fYXRvbWljX2hlbHBlci5jCj4gPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vZHJtX2F0b21pY19o ZWxwZXIuYwo+ID4gQEAgLTQ1OCwxMCArNDU4LDExIEBAIG1vZGVfZml4dXAoc3RydWN0IGRybV9h dG9taWNfc3RhdGUgKnN0YXRlKQo+ID4gICAqIHJlbW92ZWQgZnJvbSB0aGUgY3J0Yy4KPiA+ICAg KiBjcnRjX3N0YXRlLT5hY3RpdmVfY2hhbmdlZCBpcyBzZXQgd2hlbiBjcnRjX3N0YXRlLT5hY3Rp dmUgY2hhbmdlcywKPiA+ICAgKiB3aGljaCBpcyB1c2VkIGZvciBkcG1zLgo+ID4gKyAqIFNlZSBh bHNvOiBkcm1fYXRvbWljX2NydGNfbmVlZHNfbW9kZXNldCgpCj4gPiAgICoKPiA+ICAgKiBJTVBP UlRBTlQ6Cj4gPiAgICoKPiA+IC0gKiBEcml2ZXJzIHdoaWNoIHVwZGF0ZSAtPm1vZGVfY2hhbmdl ZCAoZS5nLiBpbiB0aGVpciAtPmF0b21pY19jaGVjayBob29rcyBpZiBhCj4gPiArICogRHJpdmVy cyB3aGljaCBzZXQgLT5tb2RlX2NoYW5nZWQgKGUuZy4gaW4gdGhlaXIgLT5hdG9taWNfY2hlY2sg aG9va3MgaWYgYQo+ID4gICAqIHBsYW5lIHVwZGF0ZSBjYW4ndCBiZSBkb25lIHdpdGhvdXQgYSBm dWxsIG1vZGVzZXQpIF9tdXN0XyBjYWxsIHRoaXMgZnVuY3Rpb24KPiA+ICAgKiBhZnRlcndhcmRz IGFmdGVyIHRoYXQgY2hhbmdlLiBJdCBpcyBwZXJtaXR0ZWQgdG8gY2FsbCB0aGlzIGZ1bmN0aW9u IG11bHRpcGxlCj4gPiAgICogdGltZXMgZm9yIHRoZSBzYW1lIHVwZGF0ZSwgZS5nLiB3aGVuIHRo ZSAtPmF0b21pY19jaGVjayBmdW5jdGlvbnMgZGVwZW5kIHVwb24KPiA+IEBAIC01MTAsOSArNTEx LDkgQEAgZHJtX2F0b21pY19oZWxwZXJfY2hlY2tfbW9kZXNldChzdHJ1Y3QgZHJtX2RldmljZSAq ZGV2LAo+ID4KPiA+ICAgICAgICAgZm9yX2VhY2hfY29ubmVjdG9yX2luX3N0YXRlKHN0YXRlLCBj b25uZWN0b3IsIGNvbm5lY3Rvcl9zdGF0ZSwgaSkgewo+ID4gICAgICAgICAgICAgICAgIC8qCj4g PiAtICAgICAgICAgICAgICAgICogVGhpcyBvbmx5IHNldHMgY3J0Yy0+bW9kZV9jaGFuZ2VkIGZv ciByb3V0aW5nIGNoYW5nZXMsCj4gPiAtICAgICAgICAgICAgICAgICogZHJpdmVycyBtdXN0IHNl dCBjcnRjLT5tb2RlX2NoYW5nZWQgdGhlbXNlbHZlcyB3aGVuIGNvbm5lY3Rvcgo+ID4gLSAgICAg ICAgICAgICAgICAqIHByb3BlcnRpZXMgbmVlZCB0byBiZSB1cGRhdGVkLgo+ID4gKyAgICAgICAg ICAgICAgICAqIFRoaXMgb25seSBzZXRzIGNydGMtPmNvbm5lY3RvcnNfY2hhbmdlZCBmb3Igcm91 dGluZyBjaGFuZ2VzLAo+ID4gKyAgICAgICAgICAgICAgICAqIGRyaXZlcnMgbXVzdCBzZXQgY3J0 Yy0+Y29ubmVjdG9yc19jaGFuZ2VkIHRoZW1zZWx2ZXMgd2hlbgo+ID4gKyAgICAgICAgICAgICAg ICAqIGNvbm5lY3RvciBwcm9wZXJ0aWVzIG5lZWQgdG8gYmUgdXBkYXRlZC4KPiA+ICAgICAgICAg ICAgICAgICAgKi8KPiA+ICAgICAgICAgICAgICAgICByZXQgPSB1cGRhdGVfY29ubmVjdG9yX3Jv dXRpbmcoc3RhdGUsIGNvbm5lY3RvciwKPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgY29ubmVjdG9yX3N0YXRlKTsKPiA+IGRpZmYgLS1naXQgYS9pbmNs dWRlL2RybS9kcm1fYXRvbWljLmggYi9pbmNsdWRlL2RybS9kcm1fYXRvbWljLmgKPiA+IGluZGV4 IGQ5YWZmMDYuLjFjZTI1NWYgMTAwNjQ0Cj4gPiAtLS0gYS9pbmNsdWRlL2RybS9kcm1fYXRvbWlj LmgKPiA+ICsrKyBiL2luY2x1ZGUvZHJtL2RybV9hdG9taWMuaAo+ID4gQEAgLTM2OCw4ICszNjgs MTcgQEAgaW50IF9fbXVzdF9jaGVjayBkcm1fYXRvbWljX25vbmJsb2NraW5nX2NvbW1pdChzdHJ1 Y3QgZHJtX2F0b21pY19zdGF0ZSAqc3RhdGUpOwo+ID4gICAqCj4gPiAgICogVG8gZ2l2ZSBkcml2 ZXJzIGZsZXhpYmlsaXR5IHN0cnVjdCAmZHJtX2NydGNfc3RhdGUgaGFzIDMgYm9vbGVhbnMgdG8g dHJhY2sKPiA+ICAgKiB3aGV0aGVyIHRoZSBzdGF0ZSBDUlRDIGNoYW5nZWQgZW5vdWdoIHRvIG5l ZWQgYSBmdWxsIG1vZGVzZXQgY3ljbGU6Cj4gPiAtICogY29ubmVjdG9yc19jaGFuZ2VkLCBtb2Rl X2NoYW5nZWQgYW5kIGFjdGl2ZV9jaGFuZ2UuIFRoaXMgaGVscGVyIHNpbXBseQo+ID4gKyAqIGNv bm5lY3RvcnNfY2hhbmdlZCwgbW9kZV9jaGFuZ2VkIGFuZCBhY3RpdmVfY2hhbmdlZC4gVGhpcyBo ZWxwZXIgc2ltcGx5Cj4gPiAgICogY29tYmluZXMgdGhlc2UgdGhyZWUgdG8gY29tcHV0ZSB0aGUg b3ZlcmFsbCBuZWVkIGZvciBhIG1vZGVzZXQgZm9yIEBzdGF0ZS4KPiA+ICsgKgo+ID4gKyAqIFRo ZSBhdG9taWMgaGVscGVyIGNvZGUgc2V0cyB0aGVzZSBib29sZWFucywgYnV0IGRyaXZlcnMgY2Fu IGFuZCBzaG91bGQKPiA+ICsgKiBjaGFuZ2UgdGhlbSBhcHByb3ByaWF0ZWx5IHRvIGFjY3VyYXRl bHkgcmVwcmVzZW50IHdoZXRoZXIgYSBtb2Rlc2V0IGlzCj4gPiArICogcmVhbGx5IG5lZWRlZC4g SW4gZ2VuZXJhbCwgZHJpdmVycyBzaG91bGQgYXZvaWQgZnVsbCBtb2Rlc2V0cyB3aGVuZXZlcgo+ ID4gKyAqIHBvc3NpYmxlLgo+ID4gKyAqCj4gPiArICogRm9yIGV4YW1wbGUgaWYgdGhlIENSVEMg bW9kZSBoYXMgY2hhbmdlZCwgYW5kIHRoZSBoYXJkd2FyZSBpcyBhYmxlIHRvIGVuYWN0Cj4gPiAr ICogdGhlIHJlcXVlc3RlZCBtb2RlIGNoYW5nZSB3aXRob3V0IGdvaW5nIHRocm91Z2ggYSBmdWxs IG1vZGVzZXQsIHRoZSBkcml2ZXIKPiA+ICsgKiBzaG91bGQgY2xlYXIgbW9kZV9jaGFuZ2VkIGR1 cmluZyBpdHMgLT5hdG9taWNfY2hlY2suCj4gPiAgICovCj4gPiAgc3RhdGljIGlubGluZSBib29s Cj4gPiAgZHJtX2F0b21pY19jcnRjX25lZWRzX21vZGVzZXQoc3RydWN0IGRybV9jcnRjX3N0YXRl ICpzdGF0ZSkKPiA+IGRpZmYgLS1naXQgYS9pbmNsdWRlL2RybS9kcm1fY3J0Yy5oIGIvaW5jbHVk ZS9kcm0vZHJtX2NydGMuaAo+ID4gaW5kZXggYzRhMzE2NC4uMWYwOTRkMiAxMDA2NDQKPiA+IC0t LSBhL2luY2x1ZGUvZHJtL2RybV9jcnRjLmgKPiA+ICsrKyBiL2luY2x1ZGUvZHJtL2RybV9jcnRj LmgKPiA+IEBAIC0xMTYsNiArMTE2LDExIEBAIHN0cnVjdCBkcm1fcGxhbmVfaGVscGVyX2Z1bmNz Owo+ID4gICAqIG5ldmVyIHJldHVybiBpbiBhIGZhaWx1cmUgZnJvbSB0aGUgLT5hdG9taWNfY2hl Y2sgY2FsbGJhY2suIFVzZXJzcGFjZSBhc3N1bWVzCj4gPiAgICogdGhhdCBhIERQTVMgT24gd2ls bCBhbHdheXMgc3VjY2VlZC4gSW4gb3RoZXIgd29yZHM6IEBlbmFibGUgY29udHJvbHMgcmVzb3Vy Y2UKPiA+ICAgKiBhc3NpZ25tZW50LCBAYWN0aXZlIGNvbnRyb2xzIHRoZSBhY3R1YWwgaGFyZHdh cmUgc3RhdGUuCj4gPiArICoKPiA+ICsgKiBUaGUgdGhyZWUgYm9vbGVhbnMgYWN0aXZlX2NoYW5n ZWQsIGNvbm5lY3RvcnNfY2hhbmdlZCBhbmQgbW9kZV9jaGFuZ2VkIGFyZQo+ID4gKyAqIGludGVu ZGVkIHRvIGluZGljYXRlIHdoZXRoZXIgYSBmdWxsIG1vZGVzZXQgaXMgbmVlZGVkLCByYXRoZXIg dGhhbiBzdHJpY3RseQo+ID4gKyAqIGRlc2NyaWJpbmcgd2hhdCBoYXMgY2hhbmdlZCBpbiBhIGNv bW1pdC4KPiA+ICsgKiBTZWUgYWxzbzogZHJtX2F0b21pY19jcnRjX25lZWRzX21vZGVzZXQoKQo+ ID4gICAqLwo+ID4gIHN0cnVjdCBkcm1fY3J0Y19zdGF0ZSB7Cj4gPiAgICAgICAgIHN0cnVjdCBk cm1fY3J0YyAqY3J0YzsKPiA+IC0tCj4gPiAxLjcuOS41Cj4gPgo+ID4gX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KPiA+IGRyaS1kZXZlbCBtYWlsaW5nIGxp c3QKPiA+IGRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKPiA+IGh0dHBzOi8vbGlzdHMu ZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCgotLSAKRGFuaWVsIFZl dHRlcgpTb2Z0d2FyZSBFbmdpbmVlciwgSW50ZWwgQ29ycG9yYXRpb24KaHR0cDovL2Jsb2cuZmZ3 bGwuY2gKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJp LWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBz Oi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756668AbcJQGYT (ORCPT ); Mon, 17 Oct 2016 02:24:19 -0400 Received: from mail-lf0-f66.google.com ([209.85.215.66]:35160 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756215AbcJQGYL (ORCPT ); Mon, 17 Oct 2016 02:24:11 -0400 Date: Mon, 17 Oct 2016 08:07:37 +0200 From: Daniel Vetter To: Alex Deucher Cc: Brian Starkey , Daniel Vetter , Maling list - DRI developers , LKML Subject: Re: [PATCH] drm: atomic: Clarify documentation around drm_atomic_crtc_needs_modeset Message-ID: <20161017060737.GP20761@phenom.ffwll.local> Mail-Followup-To: Alex Deucher , Brian Starkey , Maling list - DRI developers , LKML References: <20161012065618.GI20761@phenom.ffwll.local> <1476352028-16701-1-git-send-email-brian.starkey@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 Thu, Oct 13, 2016 at 10:54:51AM -0400, Alex Deucher wrote: > On Thu, Oct 13, 2016 at 5:47 AM, Brian Starkey wrote: > > Add some additional comments to more explicitly describe the meaning and > > usage of the three CRTC modeset detection booleans: mode_changed, > > connectors_changed and active_changed. > > > > Suggested-by: Daniel Vetter > > Signed-off-by: Brian Starkey > > --- > > > > Hi Daniel, > > > > I guess I asked for this one :-), please just check my understanding > > is correct. > > > > The whole thread was very enlightening for me with respect to those > flags as well. The patch looks good to me. > Acked-by: Alex Deucher Applied to drm-misc, thanks. -Daniel > > > Thanks, > > Brian > > > > drivers/gpu/drm/drm_atomic_helper.c | 9 +++++---- > > include/drm/drm_atomic.h | 11 ++++++++++- > > include/drm/drm_crtc.h | 5 +++++ > > 3 files changed, 20 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > > index 78ea735..fb4071a 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -458,10 +458,11 @@ mode_fixup(struct drm_atomic_state *state) > > * removed from the crtc. > > * crtc_state->active_changed is set when crtc_state->active changes, > > * which is used for dpms. > > + * See also: drm_atomic_crtc_needs_modeset() > > * > > * IMPORTANT: > > * > > - * Drivers which update ->mode_changed (e.g. in their ->atomic_check hooks if a > > + * Drivers which set ->mode_changed (e.g. in their ->atomic_check hooks if a > > * plane update can't be done without a full modeset) _must_ call this function > > * afterwards after that change. It is permitted to call this function multiple > > * times for the same update, e.g. when the ->atomic_check functions depend upon > > @@ -510,9 +511,9 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, > > > > for_each_connector_in_state(state, connector, connector_state, i) { > > /* > > - * This only sets crtc->mode_changed for routing changes, > > - * drivers must set crtc->mode_changed themselves when connector > > - * properties need to be updated. > > + * This only sets crtc->connectors_changed for routing changes, > > + * drivers must set crtc->connectors_changed themselves when > > + * connector properties need to be updated. > > */ > > ret = update_connector_routing(state, connector, > > connector_state); > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > > index d9aff06..1ce255f 100644 > > --- a/include/drm/drm_atomic.h > > +++ b/include/drm/drm_atomic.h > > @@ -368,8 +368,17 @@ int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state); > > * > > * To give drivers flexibility struct &drm_crtc_state has 3 booleans to track > > * whether the state CRTC changed enough to need a full modeset cycle: > > - * connectors_changed, mode_changed and active_change. This helper simply > > + * connectors_changed, mode_changed and active_changed. This helper simply > > * combines these three to compute the overall need for a modeset for @state. > > + * > > + * The atomic helper code sets these booleans, but drivers can and should > > + * change them appropriately to accurately represent whether a modeset is > > + * really needed. In general, drivers should avoid full modesets whenever > > + * possible. > > + * > > + * For example if the CRTC mode has changed, and the hardware is able to enact > > + * the requested mode change without going through a full modeset, the driver > > + * should clear mode_changed during its ->atomic_check. > > */ > > static inline bool > > drm_atomic_crtc_needs_modeset(struct drm_crtc_state *state) > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > > index c4a3164..1f094d2 100644 > > --- a/include/drm/drm_crtc.h > > +++ b/include/drm/drm_crtc.h > > @@ -116,6 +116,11 @@ struct drm_plane_helper_funcs; > > * never return in a failure from the ->atomic_check callback. Userspace assumes > > * that a DPMS On will always succeed. In other words: @enable controls resource > > * assignment, @active controls the actual hardware state. > > + * > > + * The three booleans active_changed, connectors_changed and mode_changed are > > + * intended to indicate whether a full modeset is needed, rather than strictly > > + * describing what has changed in a commit. > > + * See also: drm_atomic_crtc_needs_modeset() > > */ > > struct drm_crtc_state { > > struct drm_crtc *crtc; > > -- > > 1.7.9.5 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch