From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pekka Paalanen Subject: Re: [PATCH 2/6] xf86drmMode: separate drmModeAtomicCommit() and drmModeAtomicCleanup() Date: Fri, 21 Aug 2015 13:08:15 +0300 Message-ID: <20150821130815.7ecd85c6@gmail.com> References: <1439945924-22630-1-git-send-email-human.hwang@samsung.com> <1439945924-22630-2-git-send-email-human.hwang@samsung.com> <20150821135449.6e2c5b00@samsung.com> <20150821094226.4c2a23c3@gmail.com> <20150821161859.68b61bc1@samsung.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2111211369==" Return-path: Received: from mail-la0-f41.google.com (mail-la0-f41.google.com [209.85.215.41]) by gabe.freedesktop.org (Postfix) with ESMTPS id 077BD6E9F2 for ; Fri, 21 Aug 2015 03:08:28 -0700 (PDT) Received: by lahi9 with SMTP id i9so38280172lah.2 for ; Fri, 21 Aug 2015 03:08:26 -0700 (PDT) In-Reply-To: <20150821161859.68b61bc1@samsung.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Hyungwon Hwang Cc: Emil Velikov , ML dri-devel List-Id: dri-devel@lists.freedesktop.org --===============2111211369== Content-Type: multipart/signed; micalg=pgp-sha256; boundary="Sig_/wUBijWAG/mg4/BrMz6Q+j9m"; protocol="application/pgp-signature" --Sig_/wUBijWAG/mg4/BrMz6Q+j9m Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Fri, 21 Aug 2015 16:18:59 +0900 Hyungwon Hwang wrote: > Hi Pekka, >=20 > On Fri, 21 Aug 2015 09:42:26 +0300 > Pekka Paalanen wrote: >=20 > > On Fri, 21 Aug 2015 13:54:49 +0900 > > Hyungwon Hwang wrote: > >=20 > > > Hi Emil, > > >=20 > > > On Thu, 20 Aug 2015 17:17:27 +0100 > > > Emil Velikov wrote: > > >=20 > > > > Hi Hyungwon, > > > >=20 > > > > On 19 August 2015 at 01:58, Hyungwon Hwang > > > > wrote: > > > > > This patch seprates the code, which sorts proprty sets and > > > > > eliminates duplicate properties, from drmModeAtomicCommit(). Now > > > > > drmModeAtomicCleanup() has to do the job before calling > > > > > drmModeAtomicCommit(), and drmModeAtomicCommit() just converts > > > > > the cleaned request to IOCTL argument. > > > > > > > > > Afaict the commit message should say why we want this, rather than > > > > rewording what the patch does. > > > >=20 > > > > I'm not sure about the atomic status for wayland and others but > > > > this commit might cause issues there. Additionally, with this > > > > patch we'll send a lot of useless information to the kernel if > > > > one omits drmModeAtomicCleanup(). The kernel will likely discard > > > > it but still this doesn't seem like a good idea imho. > > >=20 > > > Yes. I agree that this change burdens the userspace application to > > > use API correctly. In my case, for modetest, the function of > > > cleaning up the request is needed, so I thought that this > > > separation would be needed. Overall, I agree with you. So I will > > > drop this patch, and find another way which is specific for > > > modetest. > >=20 > > Hi, > >=20 > > why do you need that, exactly? >=20 > To make the buffer for plane, I needed to figure out the width and the > height which the user set which are in the request, but not applied to > the kernel yet. To get the value from the request, I thought cleaning > the request before I try to getting the value from the request was > needed because the user could set the different values for the same > property. Hi, I would say that that is completely out of the scope of the libdrm API. The caller of that API set those values, so it knows them - why should libdrm offer an API to query back the values you just put in the req? The caller is storing those values somewhere else too in any case, exactly because it wants to allocate buffers of the same size. So very likely those values were already stored *before* the atomic request was even created. IMHO, it is the responsibility of the application to track what it itself is doing. If you have to write some data structures to do that, then that's what you should do. Libdrm is not a replacement for that. Thanks, pq --Sig_/wUBijWAG/mg4/BrMz6Q+j9m Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIVAwUBVdb4liNf5bQRqqqnAQh+2w//bRlyc8mtCNx7VmZ1RH4M0L06GZyZmASX SRR772Mi1ika9raYW+11RlIbRgc5duHA0//jECoQ55mKeW5XzkD+U0+wSuLx19ux 3cWIWwrqZBgeRkLKysfNEXhfQSvesxu6HLH3uYSqz/DXYgmRMPjvImf0DGln7wu4 Wqs+GZK5Tverxm1pQPCK7OAGWBwmj3EwfjG06TjE801RZM3R8f1xUVdNoBBkG3pZ kfyZZQEZKt28if6G0jUD5rRNQqPqqewZD7pBug+7WeZTOjLckhc2749TuFlBX66x VAAEhEpkVzZ+8hyGjbwbgqq+/o0LvbcPGUAEHHOYCBBJB7ZN3Iig5iQyXL+XoQ4a +thJNmCzUCyY1lJotiLthoBCPp1UtIcEpVN8AmiCjQwPatoa/1ZT4IFrKRfxYkfr r45YypifnEBzUnvZ+BBh83sV/1nF0hTQu0ZnEYCVLXpufGNMXWH4gdlTE7yAj21H YDhqEiAt/XVUfEKq7l2rnD1HqWgmKT6a2l4Dsfe3XfmlgWlI30BB/an9v57TKzRu Kf2eJTxLdvLosIGkAaoHYoKlg4JLcaAi0kArh5L/LlYTJLmozvFSo4h6slWcKrtD mmFa7RNxZY8OkwMcpukLkP4Iu9gwOxWGcA1T9Be352NyFMNanIFJ8Q1v9rDZbZZx 4SpcJWWNGCs= =3Sq+ -----END PGP SIGNATURE----- --Sig_/wUBijWAG/mg4/BrMz6Q+j9m-- --===============2111211369== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============2111211369==--