Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757426AbeAIN2q (ORCPT + 1 other); Tue, 9 Jan 2018 08:28:46 -0500 Received: from mail.free-electrons.com ([62.4.15.54]:56569 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753369AbeAIN2p (ORCPT ); Tue, 9 Jan 2018 08:28:45 -0500 Date: Tue, 9 Jan 2018 14:28:33 +0100 From: Maxime Ripard To: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org, Chen-Yu Tsai , Daniel Vetter , Jani Nikula , Sean Paul , Thomas Petazzoni , Boris Brezillon , Seung-Woo Kim , linux-kernel@vger.kernel.org, Kyungmin Park , Mark Yao , linux-arm-kernel@lists.infradead.org, thomas@vitsch.nl Subject: Re: [PATCH 01/19] drm/fourcc: Add a function to tell if the format embeds alpha Message-ID: <20180109132833.petfgnh3fvbbvosv@flea> References: <1796930.sWoKI78acY@avalon> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="75uacyorp64dmk7o" Content-Disposition: inline In-Reply-To: <1796930.sWoKI78acY@avalon> User-Agent: NeoMutt/20171215 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: --75uacyorp64dmk7o Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Laurent, On Tue, Jan 09, 2018 at 02:29:58PM +0200, Laurent Pinchart wrote: > On Tuesday, 9 January 2018 12:56:20 EET Maxime Ripard wrote: > > There's a bunch of drivers that duplicate the same function to know if a > > particular format embeds an alpha component or not. > >=20 > > Let's create a helper to avoid duplicating that logic. > >=20 > > Cc: Boris Brezillon > > Cc: Eric Anholt > > Cc: Inki Dae > > Cc: Joonyoung Shim > > Cc: Kyungmin Park > > Cc: Laurent Pinchart > > Cc: Mark Yao > > Cc: Seung-Woo Kim > > Signed-off-by: Maxime Ripard > > --- > > drivers/gpu/drm/drm_fourcc.c | 43 ++++++++++++++++++++++++++++++++++++= +- > > include/drm/drm_fourcc.h | 1 +- > > 2 files changed, 44 insertions(+) > >=20 > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c > > index 9c0152df45ad..6e6227d6a46b 100644 > > --- a/drivers/gpu/drm/drm_fourcc.c > > +++ b/drivers/gpu/drm/drm_fourcc.c > > @@ -348,3 +348,46 @@ int drm_format_plane_height(int height, uint32_t > > format, int plane) return height / info->vsub; > > } > > EXPORT_SYMBOL(drm_format_plane_height); > > + > > +/** > > + * drm_format_has_alpha - get whether the format embeds an alpha compo= nent > > + * @format: pixel format (DRM_FORMAT_*) > > + * > > + * Returns: > > + * true if the format embeds an alpha component, false otherwise. > > + */ > > +bool drm_format_has_alpha(uint32_t format) > > +{ > > + switch (format) { > > + case DRM_FORMAT_ARGB4444: > > + case DRM_FORMAT_ABGR4444: > > + case DRM_FORMAT_RGBA4444: > > + case DRM_FORMAT_BGRA4444: > > + case DRM_FORMAT_ARGB1555: > > + case DRM_FORMAT_ABGR1555: > > + case DRM_FORMAT_RGBA5551: > > + case DRM_FORMAT_BGRA5551: > > + case DRM_FORMAT_ARGB8888: > > + case DRM_FORMAT_ABGR8888: > > + case DRM_FORMAT_RGBA8888: > > + case DRM_FORMAT_BGRA8888: > > + case DRM_FORMAT_ARGB2101010: > > + case DRM_FORMAT_ABGR2101010: > > + case DRM_FORMAT_RGBA1010102: > > + case DRM_FORMAT_BGRA1010102: > > + case DRM_FORMAT_AYUV: > > + case DRM_FORMAT_XRGB8888_A8: > > + case DRM_FORMAT_XBGR8888_A8: > > + case DRM_FORMAT_RGBX8888_A8: > > + case DRM_FORMAT_BGRX8888_A8: > > + case DRM_FORMAT_RGB888_A8: > > + case DRM_FORMAT_BGR888_A8: > > + case DRM_FORMAT_RGB565_A8: > > + case DRM_FORMAT_BGR565_A8: > > + return true; > > + > > + default: > > + return false; > > + } > > +} > > +EXPORT_SYMBOL(drm_format_has_alpha); >=20 > How about adding the information to struct drm_format_info instead ?=20 > drm_format_has_alpha() could then be implemented as >=20 > bool drm_format_has_alpha(uint32_t format) > { > const struct drm_format_info *info; >=20 > info =3D drm_format_info(format); > return info ? info->has_alpha : false; > } I considered it, and wasn't too sure about if adding more fields to drm_format_info was ok. I can definitely do it that way. > although drivers should really use the drm_framebuffer::format field dire= ctly=20 > in most cases, so the helper might not be needed at all. The drivers converted in my serie shouldn't be too hard to convert to use drm_format_info directly, so that can be removed as well. Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com --75uacyorp64dmk7o Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEE0VqZU19dR2zEVaqr0rTAlCFNr3QFAlpUw4AACgkQ0rTAlCFN r3T02w//foDSIWGaUMWZFoxAN09BYnWx7bqjPZ7tfmfn4nyrNkk0FgeW2VNpCc0I kMchYE77fJ/2fSDHS5oEj64iWZ0lO5dt5Ct7LV8vdQdCmVBtnvubP6rMgzN8O+qc H2pTjQfQ9rLiS7TMWpW3+UDK3LIrAWnUHr3Hsrxyl1VTqJbni2txD+4nQnZ4uPgV cfjGHFutKVZrU3mNhobNcbAQqygnk3ftuOAYsF+IT0Fpokkauij2RYF5SLe3kZi2 V8l46TJyL1ajg+SyOSnyKO+2uha2edUYkSNkIobC/JXYHAtccTGIV65Ib+sge0PV 6AgddOJpli4t0nL22mwDyTtM/UWewiSS/uG5uj1a70q1ppW2Z3mxSJlTZf4yYXMS foi4EGsoMXvSxwXU/tYkYEngurdYuHWHKTCV8BYIkqzKSZHJxRqIt5AIZPiaAmJ+ vbU4GOQie93wsR6Ew0LZAZkmmzdL2dYlrI1jC/65220ookk0dr37dBUBgEej8/+v larCEjMi8yuDoaK9fJkv3/iq8ZHORpgmH4uMss6IowrTV1mdgqjJdX0BOS+GMMbl nyI/Qy7wWur2AuTGSB1xwg/3+7coj59ZhxJJbnc4Jg0mur54FCW5c7u7WKHhtAsW HEKhGpglMpbAwj6nZr0OT/xzDwrHk5sTwUdaiAlDBVrCiak62hI= =WcXz -----END PGP SIGNATURE----- --75uacyorp64dmk7o--