Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752049AbeAPURi (ORCPT + 1 other); Tue, 16 Jan 2018 15:17:38 -0500 Received: from mail.free-electrons.com ([62.4.15.54]:34858 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750925AbeAPURh (ORCPT ); Tue, 16 Jan 2018 15:17:37 -0500 Date: Tue, 16 Jan 2018 21:17:24 +0100 From: Maxime Ripard To: Ayan Halder Cc: Laurent Pinchart , Thomas Petazzoni , Boris Brezillon , Seung-Woo Kim , linux-kernel@vger.kernel.org, Kyungmin Park , Chen-Yu Tsai , dri-devel@lists.freedesktop.org, Daniel Vetter , thomas@vitsch.nl, linux-arm-kernel@lists.infradead.org, Mark Yao Subject: Re: [PATCH 01/19] drm/fourcc: Add a function to tell if the format embeds alpha Message-ID: <20180116201724.lyhnsxfik532lzcq@flea.lan> References: <1796930.sWoKI78acY@avalon> <20180109132833.petfgnh3fvbbvosv@flea> <20180115154744.GA28269@arm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="7fpsyklgkot64nkr" Content-Disposition: inline In-Reply-To: <20180115154744.GA28269@arm.com> User-Agent: NeoMutt/20171215 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: --7fpsyklgkot64nkr Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Ayan, On Mon, Jan 15, 2018 at 03:47:44PM +0000, Ayan Halder wrote: > On Tue, Jan 09, 2018 at 02:28:33PM +0100, Maxime Ripard wrote: > > 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. > > > > > > > > Let's create a helper to avoid duplicating that logic. > > > > > > > > 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(+) > > > > > > > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fou= rcc.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 c= omponent > > > > + * @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); > > > > > > How about adding the information to struct drm_format_info instead ? > > > drm_format_has_alpha() could then be implemented as > > > > > > bool drm_format_has_alpha(uint32_t format) > > > { > > > const struct drm_format_info *info; > > > > > > 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. > > Are you going to send an updated patch with the change mentioned here. > Or should I update my patch (https://patchwork.kernel.org/patch/10161023/) > and change the type of '.alpha' to boolean to denote if the color > format has an alpha channel or not. Yes, I already had those patches ready. I'm just waiting for the discussion on the alpha property to settle before sending a v2. Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com --7fpsyklgkot64nkr Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEE0VqZU19dR2zEVaqr0rTAlCFNr3QFAlpeXc0ACgkQ0rTAlCFN r3SgGw/9Fe5EzagMNgjCfOkvelk/Vzk6Ky7sMhURFhk0BtvGRVD8bZQPdOT9Nng8 nweu0llrgCZQ6fhxjNSd75C/nwLlKM9+3We4HPfOOQB5o0pGGSSvnbYxQM42IVOQ QFH4kYz3Bqyu6ItDh5kYHuNvxrr8xD9+8s6fN5zH2SYYM7hwCW3Y1sPE1IhBYHtn TYMok4HlfWqem8uFI0TotnqBT5YLWvOO/sNIvsDs6fYV/XeLL5db6r5CyrQiUIPD DR0XlIBdyt9XzCgI0UAjqQlreW2wyFIgRNeHa8E71NPGe9/6j1HrH05pmlu4ONwh HGmmYT5JsLJtdS2mqPrtsCcT+8+Pg1bra0/WZkX5Nf6zb5GHYs1hzr83BV02ibO3 39b3VPVzvz30sLDCSsPIe3OkxhhM47XuDDvwrBHKhCIuPPoy25a1kYoc/3W9iX7Q O/4m/+112ruezfVlSvQLrYLP1xBfo5UHaHDzQzUTLqVp1mIdhzfM1ajFmre3CTqs GafeCYSROEovb4eHz3CTPvNLT0eoRT2oeMidGaA1Apu5Nxjyr6R42gSS9BnedYo2 zw3RQQH3i4kocrMN049ChFlST8Uc1a/m5kGRhzbQyzT7hZupKO6DovY3VD/14XfP JzlucR8Pn1YKn9YobpXSaHrRftQ8mMnjoAf8Y26r0b9rGWyABqw= =RkfK -----END PGP SIGNATURE----- --7fpsyklgkot64nkr--