From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Zimmermann Subject: Re: [PATCH v3 01/19] drm: Add |struct drm_gem_vram_object| and helpers Date: Fri, 3 May 2019 12:14:53 +0200 Message-ID: References: <20190429144341.12615-1-tzimmermann@suse.de> <20190429144341.12615-2-tzimmermann@suse.de> <20190429195855.GA6610@ravnborg.org> <1d14ef87-e1cd-4f4a-3632-bc045a1981c6@suse.de> <20190430092327.GA13757@ravnborg.org> <6e07e6c9-2ce7-c39f-8d55-46e811c61510@amd.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1340691829==" Return-path: Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by gabe.freedesktop.org (Postfix) with ESMTPS id 52C6289951 for ; Fri, 3 May 2019 10:15:00 +0000 (UTC) In-Reply-To: <6e07e6c9-2ce7-c39f-8d55-46e811c61510@amd.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: "Koenig, Christian" , Sam Ravnborg Cc: "airlied@linux.ie" , "puck.chen@hisilicon.com" , "dri-devel@lists.freedesktop.org" , "virtualization@lists.linux-foundation.org" , "z.liuxinliang@hisilicon.com" , "hdegoede@redhat.com" , "kong.kongxinwei@hisilicon.com" , "Huang, Ray" , "kraxel@redhat.com" , "zourongrong@gmail.com" List-Id: dri-devel@lists.freedesktop.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --===============1340691829== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="vinSYn3Ek3VhSjo8J1G9S0Y4NUtZueyta" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --vinSYn3Ek3VhSjo8J1G9S0Y4NUtZueyta Content-Type: multipart/mixed; boundary="FnMk8yDTXVw7b8dZMfI9KgILEZkJNQ4wY"; protected-headers="v1" From: Thomas Zimmermann To: "Koenig, Christian" , Sam Ravnborg Cc: "airlied@linux.ie" , "puck.chen@hisilicon.com" , "dri-devel@lists.freedesktop.org" , "virtualization@lists.linux-foundation.org" , "z.liuxinliang@hisilicon.com" , "hdegoede@redhat.com" , "kong.kongxinwei@hisilicon.com" , "Huang, Ray" , "kraxel@redhat.com" , "zourongrong@gmail.com" Message-ID: Subject: Re: [PATCH v3 01/19] drm: Add |struct drm_gem_vram_object| and helpers References: <20190429144341.12615-1-tzimmermann@suse.de> <20190429144341.12615-2-tzimmermann@suse.de> <20190429195855.GA6610@ravnborg.org> <1d14ef87-e1cd-4f4a-3632-bc045a1981c6@suse.de> <20190430092327.GA13757@ravnborg.org> <6e07e6c9-2ce7-c39f-8d55-46e811c61510@amd.com> In-Reply-To: <6e07e6c9-2ce7-c39f-8d55-46e811c61510@amd.com> --FnMk8yDTXVw7b8dZMfI9KgILEZkJNQ4wY Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable Hi Christian, would you review the whole patch set? Daniel mentioned that he'd prefer to leave the review to memory-mgmt developers. Best regards Thomas Am 30.04.19 um 11:35 schrieb Koenig, Christian: > Am 30.04.19 um 11:23 schrieb Sam Ravnborg: >> [CAUTION: External Email] >> >> Hi Thomas. >> >>>>> + >>>>> +/** >>>>> + * Returns the container of type &struct drm_gem_vram_object >>>>> + * for field bo. >>>>> + * @bo: the VRAM buffer object >>>>> + * Returns: The containing GEM VRAM object >>>>> + */ >>>>> +static inline struct drm_gem_vram_object* drm_gem_vram_of_bo( >>>>> + struct ttm_buffer_object *bo) >>>>> +{ >>>>> + return container_of(bo, struct drm_gem_vram_object, bo); >>>>> +} >>>> Indent funny. USe same indent as used in other parts of file for >>>> function arguments. >>> If I put the argument next to the function's name, it will exceed the= >>> 80-character limit. From the coding-style document, I could not see w= hat >>> to do in this case. One solution would move the return type to a >>> separate line before the function name. I've not seen that anywhere i= n >>> the source code, so moving the argument onto a separate line and >>> indenting by one tab appears to be the next best solution. Please let= me >>> know if there's if there's a preferred style for cases like this one.= >> Readability has IMO higher priority than some limit of 80 chars. >> And it hurts readability (at least my OCD) when style changes >> as you do with indent here. So my personal preference is to fix >> indent and accect longer lines. >=20 > In this case the an often used convention (which is also kind of=20 > readable) is to add a newline after the return values, but before the=20 > function name. E.g. something like this: >=20 > static inline struct drm_gem_vram_object* > drm_gem_vram_of_bo(struct ttm_buffer_object *bo) >=20 > Regards, > Christian. >=20 >> >> But you ask for a preferred style - which I do not think we have in th= is >> case. So it boils down to what you prefer. >> >> Enough bikeshedding, thanks for the quick response. >> >> Sam >=20 --=20 Thomas Zimmermann Graphics Driver Developer SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany GF: Felix Imend=C3=B6rffer, Mary Higgins, Sri Rasiah HRB 21284 (AG N=C3=BCrnberg) --FnMk8yDTXVw7b8dZMfI9KgILEZkJNQ4wY-- --vinSYn3Ek3VhSjo8J1G9S0Y4NUtZueyta Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAEBCAAdFiEEchf7rIzpz2NEoWjlaA3BHVMLeiMFAlzMFJ0ACgkQaA3BHVML eiODzwf+MSQUU/Kb78xC7KULkP8K0t/v2EDctG4y3L95WeQV5Eatc3bLqVkDYSt+ j9UYMT0XrYCsqUWJmJBOwTG3S2/v+tPKNMmcpKLyR3nm7aSyIBF8MWFm/EMeEMmZ LCTp9h7tpchF3zmSPdfvBgZdE7gG4sWLlw7r2zMYYx5c+TK6/yb28mIqG/Im/PQD dHgVpBsc3lJYk3ISc3UTE0Ek9CwP4yePqmP1ybF9i+ZkDRYctrltqWtGn+p7DONk LVEUElBN32Va31pPmQ7rHmmi2JgpfcyHwy0I972n1ByqECvIn22v1sdvMSyHJrnz YbTw0DSE/uqQlmi7z/upykVqFWyaQA== =O1HM -----END PGP SIGNATURE----- --vinSYn3Ek3VhSjo8J1G9S0Y4NUtZueyta-- --===============1340691829== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVs --===============1340691829==--