From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [PATCH v4 01/14] drm: Centralize format information Date: Fri, 16 Sep 2016 12:44:31 +0300 Message-ID: References: <1473345868-25453-1-git-send-email-laurent.pinchart@ideasonboard.com> <1ef50c0f-dc06-0a62-8728-82ee601eec34@ti.com> <20160915161212.GU3075@imgtec.com> <6266324.K2me61iYYT@avalon> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0849600879==" Return-path: Received: from arroyo.ext.ti.com (arroyo.ext.ti.com [198.47.19.12]) by gabe.freedesktop.org (Postfix) with ESMTPS id C73656E9DA for ; Fri, 16 Sep 2016 09:44:43 +0000 (UTC) In-Reply-To: <6266324.K2me61iYYT@avalon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Laurent Pinchart , Eric Engestrom Cc: Daniel Vetter , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============0849600879== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="GFd1pl2RjDe4HkUcXptVJV1aPRjiRM0KA" --GFd1pl2RjDe4HkUcXptVJV1aPRjiRM0KA Content-Type: multipart/mixed; boundary="JuNSesjhrWIaaeuNpfp7fJXRI0bVijS99"; protected-headers="v1" From: Tomi Valkeinen To: Laurent Pinchart , Eric Engestrom Cc: Daniel Vetter , dri-devel@lists.freedesktop.org Message-ID: Subject: Re: [PATCH v4 01/14] drm: Centralize format information References: <1473345868-25453-1-git-send-email-laurent.pinchart@ideasonboard.com> <1ef50c0f-dc06-0a62-8728-82ee601eec34@ti.com> <20160915161212.GU3075@imgtec.com> <6266324.K2me61iYYT@avalon> In-Reply-To: <6266324.K2me61iYYT@avalon> --JuNSesjhrWIaaeuNpfp7fJXRI0bVijS99 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 16/09/16 02:30, Laurent Pinchart wrote: > I've checked the existing code that this patch series is replacing, and= the=20 > [ARGB]{4}8888 formats are currently reported as having a depth of 32. I= 'm not=20 > sure why that's the case, but I'd rather not touch it in this patch. If= this=20 > is a bug we should fix it in a separate patch. I agree, I don't want this series to be held up. But this depth is clearly broken, in some way or another. Even more reason to move it to fb code =3D). >>> I'm not sure if it's worth the hassle, but if the depth is only for >>> fbdev compat code, maybe a separate format->depth table in fbdev code= >>> (for the formats fbdev supports) would make this cleaner and avoid an= y >>> future mistakes with new drm drivers. >> >> I agree actually, having it here will encourage anyone to use it. If y= ou >> don't want to split it out, at least add a comment along the lines of >> your reply: >> >>>> This is used to implement the fbdev compatibility code, as fbdev (un= like >>>> kms) makes use of that information. >=20 > I've double-checked the existing usage of the depth value, and it turns= out=20 > that quite a few drivers still use it for various purpose, through stru= ct=20 > drm_framebuffer.depth. I thus wonder whether splitting the depth value = from=20 > the format information table would really help, as drivers would have a= way to=20 > access it anyway. Ok. Btw, are you sure alpha is not counted into depth? With a quick glance, also drm_mode_legacy_fb_format() seems to expect depth to include alpha. I'm just thinking here that if "depth" is not clearly defined anywhere, and the most common formats, 24bit RGB formats, are defined with depth including alpha, well, maybe then that's how depth should be defined. Then again, I had a quick glance at the fbdev code, and fb_get_color_depth() suggests that alpha is not counted in... Tomi --JuNSesjhrWIaaeuNpfp7fJXRI0bVijS99-- --GFd1pl2RjDe4HkUcXptVJV1aPRjiRM0KA Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJX277/AAoJEPo9qoy8lh71f38QAJIYRX4D6haJFRiyyG9R3O+u 3rM4W7rPLSeIFYDC39870FaiJxHl/LtLaAKI4mAzgX0WelDf2gZCvGO1Xgeq/ZIq 3IcDKRkIuK3nNkLPMGa/6UplOxixB5m5ctfG+dZ2rPsX6JjdAaXjIzwVrbI0jTcn dS+gjXAoMf6+qmdH/U7XnqECo9s99Vq6Z11jd0ASdeU2KQtWWihPTNQZDesMVF9p FvzgCXDkW/MbGx74E00xPcE0QVBgOFURsBpyh50HeRONBaV973z7eK9i0o9Xx5ap aYMr7A7e4LaWUx5agQ2HB7S/RhJLiVoLfkmKcXqgTi33tpPSt/sTm4FtarDkEPO7 VYz5ASsa7QWSPbyYGwa+v6uD45OARiZ5mnRTFMuRBNJaU6L2zsh/nb6j7sG8XOdu pWllPmBJj0NaTho4XfNKGjKgGCWzf4MgrRW6h65ikxJ6NHOPqQah2wJ7/H7kYG/0 4kUPJGlS/CuNL0Pe9KCIdiQOLcju6ZH52dFm3Uy2DsfbjT+ab0XUr5utuJOlB9N0 gJc0Eb/DUx+lNMxIgSQMDzyw4qoBv+M3sIITIFLCZBVavLra7l3XcPDecjS+jJWt 3+QplyDvXq1f1Nph81j7ujzCF1pfQ1ifw61ZsZToNqZ/JVhda+YoeC/b81PSpMg5 VmEYHDruWkIQLIJ8Ci4h =Kf0Q -----END PGP SIGNATURE----- --GFd1pl2RjDe4HkUcXptVJV1aPRjiRM0KA-- --===============0849600879== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0849600879==--