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=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 D3D42C43381 for ; Fri, 1 Mar 2019 14:47:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A964920675 for ; Fri, 1 Mar 2019 14:47:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387907AbfCAOrG (ORCPT ); Fri, 1 Mar 2019 09:47:06 -0500 Received: from mga01.intel.com ([192.55.52.88]:60692 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728395AbfCAOrG (ORCPT ); Fri, 1 Mar 2019 09:47:06 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Mar 2019 06:47:05 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,427,1544515200"; d="scan'208";a="278941164" Received: from elsaidmo-mobl2.ger.corp.intel.com (HELO [10.252.61.31]) ([10.252.61.31]) by orsmga004.jf.intel.com with ESMTP; 01 Mar 2019 06:47:03 -0800 Subject: Re: [PATCH 12/17] drm/rcar-du: Convert to using __drm_atomic_helper_crtc_reset() for reset. To: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, Kieran Bingham , linux-renesas-soc@vger.kernel.org References: <20190301125627.7285-1-maarten.lankhorst@linux.intel.com> <20190301125627.7285-13-maarten.lankhorst@linux.intel.com> <20190301131358.GC32244@pendragon.ideasonboard.com> <93a242fe-41fc-bbe5-0ccd-f26eb729fe85@linux.intel.com> <20190301143640.GD32244@pendragon.ideasonboard.com> From: Maarten Lankhorst Message-ID: Date: Fri, 1 Mar 2019 15:47:02 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <20190301143640.GD32244@pendragon.ideasonboard.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-renesas-soc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-renesas-soc@vger.kernel.org Op 01-03-2019 om 15:36 schreef Laurent Pinchart: > Hi Marteen, > > On Fri, Mar 01, 2019 at 03:08:20PM +0100, Maarten Lankhorst wrote: >> Op 01-03-2019 om 14:13 schreef Laurent Pinchart: >>> On Fri, Mar 01, 2019 at 01:56:22PM +0100, Maarten Lankhorst wrote: >>>> Convert rcar-du to using __drm_atomic_helper_crtc_reset(), instead of >>>> writing its own version. Instead of open coding destroy_state(), call >>>> it directly for freeing the old state. >>> I don't think the second sentence applies to this patch. >>> >>>> Signed-off-by: Maarten Lankhorst >>>> Cc: Laurent Pinchart >>>> Cc: Kieran Bingham >>>> Cc: linux-renesas-soc@vger.kernel.org >>>> --- >>>> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 11 +++-------- >>>> 1 file changed, 3 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c >>>> index 4cdea14d552f..7766551e67fc 100644 >>>> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c >>>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c >>>> @@ -891,22 +891,17 @@ static void rcar_du_crtc_cleanup(struct drm_crtc *crtc) >>>> >>>> static void rcar_du_crtc_reset(struct drm_crtc *crtc) >>>> { >>>> - struct rcar_du_crtc_state *state; >>>> + struct rcar_du_crtc_state *state = kzalloc(sizeof(*state), GFP_KERNEL); >>>> >>>> - if (crtc->state) { >>>> + if (crtc->state) >>>> rcar_du_crtc_atomic_destroy_state(crtc, crtc->state); >>>> - crtc->state = NULL; >>>> - } >>>> >>>> - state = kzalloc(sizeof(*state), GFP_KERNEL); >>>> + __drm_atomic_helper_crtc_reset(crtc, &state->state); >>> state may be NULL here if the above kzalloc() failed. Let's keep the >>> original order of the function, and simply call >>> __drm_atomic_helper_crtc_reset() after the NULL check below. >> There were 10 different ways crtc was implemented, I felt it was good to settle on one. >> >> We don't handle during reset at all, would need to start propagating this first before we should handle errors, imho. > That's not the point. As state can be NULL, you could end up > dereferencing a NULL pointer. The fact that the base state is the first > field in the rcar_du_crtc_state structure is just luck, and shouldn't be > relied on. Would it be ok if I changed it to state ? &state->state : NULL and let the compiler deal with it? Will probably fix up all other patches as well before committing. >> Looking more closely, it's the same way that errors in >> rcar_du_plane_reset() are handled. :) > It's not, the return value of kzalloc() is checked explicitly in > rcar_du_plane_reset() before calling __drm_atomic_helper_plane_reset(). > Please copy the code flow of rcar_du_plane_reset() to implement > rcar_du_crtc_reset(). > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maarten Lankhorst Subject: Re: [PATCH 12/17] drm/rcar-du: Convert to using __drm_atomic_helper_crtc_reset() for reset. Date: Fri, 1 Mar 2019 15:47:02 +0100 Message-ID: References: <20190301125627.7285-1-maarten.lankhorst@linux.intel.com> <20190301125627.7285-13-maarten.lankhorst@linux.intel.com> <20190301131358.GC32244@pendragon.ideasonboard.com> <93a242fe-41fc-bbe5-0ccd-f26eb729fe85@linux.intel.com> <20190301143640.GD32244@pendragon.ideasonboard.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20190301143640.GD32244@pendragon.ideasonboard.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Laurent Pinchart Cc: linux-renesas-soc@vger.kernel.org, intel-gfx@lists.freedesktop.org, Kieran Bingham , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org T3AgMDEtMDMtMjAxOSBvbSAxNTozNiBzY2hyZWVmIExhdXJlbnQgUGluY2hhcnQ6Cj4gSGkgTWFy dGVlbiwKPgo+IE9uIEZyaSwgTWFyIDAxLCAyMDE5IGF0IDAzOjA4OjIwUE0gKzAxMDAsIE1hYXJ0 ZW4gTGFua2hvcnN0IHdyb3RlOgo+PiBPcCAwMS0wMy0yMDE5IG9tIDE0OjEzIHNjaHJlZWYgTGF1 cmVudCBQaW5jaGFydDoKPj4+IE9uIEZyaSwgTWFyIDAxLCAyMDE5IGF0IDAxOjU2OjIyUE0gKzAx MDAsIE1hYXJ0ZW4gTGFua2hvcnN0IHdyb3RlOgo+Pj4+IENvbnZlcnQgcmNhci1kdSB0byB1c2lu ZyBfX2RybV9hdG9taWNfaGVscGVyX2NydGNfcmVzZXQoKSwgaW5zdGVhZCBvZgo+Pj4+IHdyaXRp bmcgaXRzIG93biB2ZXJzaW9uLiBJbnN0ZWFkIG9mIG9wZW4gY29kaW5nIGRlc3Ryb3lfc3RhdGUo KSwgY2FsbAo+Pj4+IGl0IGRpcmVjdGx5IGZvciBmcmVlaW5nIHRoZSBvbGQgc3RhdGUuCj4+PiBJ IGRvbid0IHRoaW5rIHRoZSBzZWNvbmQgc2VudGVuY2UgYXBwbGllcyB0byB0aGlzIHBhdGNoLgo+ Pj4KPj4+PiBTaWduZWQtb2ZmLWJ5OiBNYWFydGVuIExhbmtob3JzdCA8bWFhcnRlbi5sYW5raG9y c3RAbGludXguaW50ZWwuY29tPgo+Pj4+IENjOiBMYXVyZW50IFBpbmNoYXJ0IDxsYXVyZW50LnBp bmNoYXJ0QGlkZWFzb25ib2FyZC5jb20+Cj4+Pj4gQ2M6IEtpZXJhbiBCaW5naGFtIDxraWVyYW4u YmluZ2hhbStyZW5lc2FzQGlkZWFzb25ib2FyZC5jb20+Cj4+Pj4gQ2M6IGxpbnV4LXJlbmVzYXMt c29jQHZnZXIua2VybmVsLm9yZwo+Pj4+IC0tLQo+Pj4+ICBkcml2ZXJzL2dwdS9kcm0vcmNhci1k dS9yY2FyX2R1X2NydGMuYyB8IDExICsrKy0tLS0tLS0tCj4+Pj4gIDEgZmlsZSBjaGFuZ2VkLCAz IGluc2VydGlvbnMoKyksIDggZGVsZXRpb25zKC0pCj4+Pj4KPj4+PiBkaWZmIC0tZ2l0IGEvZHJp dmVycy9ncHUvZHJtL3JjYXItZHUvcmNhcl9kdV9jcnRjLmMgYi9kcml2ZXJzL2dwdS9kcm0vcmNh ci1kdS9yY2FyX2R1X2NydGMuYwo+Pj4+IGluZGV4IDRjZGVhMTRkNTUyZi4uNzc2NjU1MWU2N2Zj IDEwMDY0NAo+Pj4+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9yY2FyLWR1L3JjYXJfZHVfY3J0Yy5j Cj4+Pj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL3JjYXItZHUvcmNhcl9kdV9jcnRjLmMKPj4+PiBA QCAtODkxLDIyICs4OTEsMTcgQEAgc3RhdGljIHZvaWQgcmNhcl9kdV9jcnRjX2NsZWFudXAoc3Ry dWN0IGRybV9jcnRjICpjcnRjKQo+Pj4+ICAKPj4+PiAgc3RhdGljIHZvaWQgcmNhcl9kdV9jcnRj X3Jlc2V0KHN0cnVjdCBkcm1fY3J0YyAqY3J0YykKPj4+PiAgewo+Pj4+IC0Jc3RydWN0IHJjYXJf ZHVfY3J0Y19zdGF0ZSAqc3RhdGU7Cj4+Pj4gKwlzdHJ1Y3QgcmNhcl9kdV9jcnRjX3N0YXRlICpz dGF0ZSA9IGt6YWxsb2Moc2l6ZW9mKCpzdGF0ZSksIEdGUF9LRVJORUwpOwo+Pj4+ICAKPj4+PiAt CWlmIChjcnRjLT5zdGF0ZSkgewo+Pj4+ICsJaWYgKGNydGMtPnN0YXRlKQo+Pj4+ICAJCXJjYXJf ZHVfY3J0Y19hdG9taWNfZGVzdHJveV9zdGF0ZShjcnRjLCBjcnRjLT5zdGF0ZSk7Cj4+Pj4gLQkJ Y3J0Yy0+c3RhdGUgPSBOVUxMOwo+Pj4+IC0JfQo+Pj4+ICAKPj4+PiAtCXN0YXRlID0ga3phbGxv YyhzaXplb2YoKnN0YXRlKSwgR0ZQX0tFUk5FTCk7Cj4+Pj4gKwlfX2RybV9hdG9taWNfaGVscGVy X2NydGNfcmVzZXQoY3J0YywgJnN0YXRlLT5zdGF0ZSk7Cj4+PiBzdGF0ZSBtYXkgYmUgTlVMTCBo ZXJlIGlmIHRoZSBhYm92ZSBremFsbG9jKCkgZmFpbGVkLiBMZXQncyBrZWVwIHRoZQo+Pj4gb3Jp Z2luYWwgb3JkZXIgb2YgdGhlIGZ1bmN0aW9uLCBhbmQgc2ltcGx5IGNhbGwKPj4+IF9fZHJtX2F0 b21pY19oZWxwZXJfY3J0Y19yZXNldCgpIGFmdGVyIHRoZSBOVUxMIGNoZWNrIGJlbG93Lgo+PiBU aGVyZSB3ZXJlIDEwIGRpZmZlcmVudCB3YXlzIGNydGMgd2FzIGltcGxlbWVudGVkLCBJIGZlbHQg aXQgd2FzIGdvb2QgdG8gc2V0dGxlIG9uIG9uZS4KPj4KPj4gV2UgZG9uJ3QgaGFuZGxlIGR1cmlu ZyByZXNldCBhdCBhbGwsIHdvdWxkIG5lZWQgdG8gc3RhcnQgcHJvcGFnYXRpbmcgdGhpcyBmaXJz dCBiZWZvcmUgd2Ugc2hvdWxkIGhhbmRsZSBlcnJvcnMsIGltaG8uCj4gVGhhdCdzIG5vdCB0aGUg cG9pbnQuIEFzIHN0YXRlIGNhbiBiZSBOVUxMLCB5b3UgY291bGQgZW5kIHVwCj4gZGVyZWZlcmVu Y2luZyBhIE5VTEwgcG9pbnRlci4gVGhlIGZhY3QgdGhhdCB0aGUgYmFzZSBzdGF0ZSBpcyB0aGUg Zmlyc3QKPiBmaWVsZCBpbiB0aGUgcmNhcl9kdV9jcnRjX3N0YXRlIHN0cnVjdHVyZSBpcyBqdXN0 IGx1Y2ssIGFuZCBzaG91bGRuJ3QgYmUKPiByZWxpZWQgb24uCgpXb3VsZCBpdCBiZSBvayBpZiBJ IGNoYW5nZWQgaXQgdG8gc3RhdGUgPyAmc3RhdGUtPnN0YXRlIDogTlVMTCBhbmQgbGV0IHRoZSBj b21waWxlciBkZWFsIHdpdGggaXQ/CgpXaWxsIHByb2JhYmx5IGZpeCB1cCBhbGwgb3RoZXIgcGF0 Y2hlcyBhcyB3ZWxsIGJlZm9yZSBjb21taXR0aW5nLgoKPj4gTG9va2luZyBtb3JlIGNsb3NlbHks IGl0J3MgdGhlIHNhbWUgd2F5IHRoYXQgZXJyb3JzIGluCj4+IHJjYXJfZHVfcGxhbmVfcmVzZXQo KSBhcmUgaGFuZGxlZC4gOikKPiBJdCdzIG5vdCwgdGhlIHJldHVybiB2YWx1ZSBvZiBremFsbG9j KCkgaXMgY2hlY2tlZCBleHBsaWNpdGx5IGluCj4gcmNhcl9kdV9wbGFuZV9yZXNldCgpIGJlZm9y ZSBjYWxsaW5nIF9fZHJtX2F0b21pY19oZWxwZXJfcGxhbmVfcmVzZXQoKS4KPiBQbGVhc2UgY29w eSB0aGUgY29kZSBmbG93IG9mIHJjYXJfZHVfcGxhbmVfcmVzZXQoKSB0byBpbXBsZW1lbnQKPiBy Y2FyX2R1X2NydGNfcmVzZXQoKS4KPgoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJl ZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGlu Zm8vaW50ZWwtZ2Z4