From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maxime Ripard Subject: Re: [PATCH v3 1/8] drm/blend: Add a generic alpha property Date: Wed, 21 Feb 2018 14:07:57 +0100 Message-ID: <20180221130757.al7i57kiptrqh4j2@flea.lan> References: <9403570d73b6eb63ab87488d5e8eb767ba6f1ac7.1518802627.git-series.maxime.ripard@bootlin.com> <20180216182041.GU5453@intel.com> <1841868.3gpkeNesVL@avalon> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0301907458==" Return-path: Received: from mail.free-electrons.com (mail.free-electrons.com [62.4.15.54]) by gabe.freedesktop.org (Postfix) with ESMTP id 094E26E62E for ; Wed, 21 Feb 2018 13:08:08 +0000 (UTC) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: dri-devel , Boris Brezillon , Chen-Yu Tsai , Laurent Pinchart , Daniel Vetter , Linux ARM List-Id: dri-devel@lists.freedesktop.org --===============0301907458== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="fa7adbnk54nurehu" Content-Disposition: inline --fa7adbnk54nurehu Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Feb 19, 2018 at 10:58:40PM +0100, Daniel Vetter wrote: > On Mon, Feb 19, 2018 at 9:19 PM, Laurent Pinchart > wrote: > > Hi Ville, > > > > On Friday, 16 February 2018 20:20:41 EET Ville Syrj=C3=A4l=C3=A4 wrote: > >> On Fri, Feb 16, 2018 at 06:39:29PM +0100, Maxime Ripard wrote: > >> > Some drivers duplicate the logic to create a property to store a per= -plane > >> > alpha. > >> > > >> > This is especially useful if we ever want to support extra protocols= for > >> > Wayland like: > >> > https://lists.freedesktop.org/archives/wayland-devel/2017-August/034= 741.ht > >> > ml > >> > > >> > Let's create a helper in order to move that to the core. > >> > > >> > Cc: Laurent Pinchart > >> > Reviewed-by: Boris Brezillon > >> > Signed-off-by: Maxime Ripard > >> > --- > >> > > >> > Documentation/gpu/kms-properties.csv | 2 +- > >> > drivers/gpu/drm/drm_atomic.c | 4 ++++- > >> > drivers/gpu/drm/drm_atomic_helper.c | 4 ++++- > >> > drivers/gpu/drm/drm_blend.c | 32 +++++++++++++++++++++++++= ++++- > >> > include/drm/drm_blend.h | 1 +- > >> > include/drm/drm_plane.h | 6 +++++- > >> > 6 files changed, 48 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/Documentation/gpu/kms-properties.csv > >> > b/Documentation/gpu/kms-properties.csv index 927b65e14219..25ad3503d= 663 > >> > 100644 > >> > --- a/Documentation/gpu/kms-properties.csv > >> > +++ b/Documentation/gpu/kms-properties.csv > >> > @@ -99,5 +99,5 @@ radeon,DVI-I,=E2=80=9Ccoherent=E2=80=9D,RANGE,"Min= =3D0, > >> > Max=3D1",Connector,TBD> > >> > ,,"""underscan vborder""",RANGE,"Min=3D0, Max=3D128",Connector,TBD > >> > ,Audio,=E2=80=9Caudio=E2=80=9D,ENUM,"{ ""off"", ""on"", ""auto"" }"= ,Connector,TBD > >> > ,FMT Dithering,=E2=80=9Cdither=E2=80=9D,ENUM,"{ ""off"", ""on"" }",= Connector,TBD > >> > > >> > -rcar-du,Generic,"""alpha""",RANGE,"Min=3D0, Max=3D255",Plane,TBD > >> > +,,"""alpha""",RANGE,"Min=3D0, Max=3DDriver dependant",Plane,Opacity= of the > >> > plane from transparent (0) to fully opaque (MAX). If this property i= s set > >> > to a value different than max, and that the pixel will define an alp= ha > >> > component, the property will have precendance and the pixel value wi= ll be > >> > ignored. >=20 > Please don't document new properties in that csv file, it's an > unreadable mess. Instead follow how we document standardized > properties nowadays in full-blown sections. For plane blending we > have: >=20 > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#plane-composition-p= roperties Ack > >> > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h > >> > index 8185e3468a23..5a6f29524f12 100644 > >> > --- a/include/drm/drm_plane.h > >> > +++ b/include/drm/drm_plane.h > >> > @@ -42,6 +42,7 @@ struct drm_modeset_acquire_ctx; > >> > * plane (in 16.16) > >> > * @src_w: width of visible portion of plane (in 16.16) > >> > * @src_h: height of visible portion of plane (in 16.16) > >> > + * @alpha: opacity of the plane > >> > * @rotation: rotation of the plane > >> > * @zpos: priority of the given plane on crtc (optional) > >> > * Note that multiple active planes on the same crtc can have an > >> > identical > >> > @@ -105,6 +106,9 @@ struct drm_plane_state { > >> > uint32_t src_x, src_y; > >> > uint32_t src_h, src_w; > >> > > >> > + /* Plane opacity */ > >> > + u8 alpha; > >> > >> We may want to make that u16. The general we expect 16bpc for most col= or > >> related things, but since this is a range prop I suppose we should just > >> expose the actual hardware range. But making it u16 might avoid some h= ead > >> scratching for the first person to have hardware with higher precision. > >> Either that or we should make the prop creation fail if the driver asks > >> for more bits than we have in the state. > > > > I'm tempted to go one step further and always make the alpha property 1= 6-bits > > wide for new users (we can't do so for existing users as it could break > > userspace), and let drivers convert that internally to the range they n= eed. > > There could however be drawbacks I don't foresee. >=20 > I think scaling the range to match the hw is the most sensible (yes > I'm flip-flopping around here). And once someone needs more than u8, > we can extend the internal representation easily. The external > representation in the property is an u64, that /should/ be enough for > the next few years :-) Just to make sure we're on the same page, you want to keep the u8, and if the hardware uses say an u16, the driver for that hardware will do the upscaling? Thanks! Maxime --=20 Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com --fa7adbnk54nurehu Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEE0VqZU19dR2zEVaqr0rTAlCFNr3QFAlqNbywACgkQ0rTAlCFN r3Td8RAAgdj3Gv0FeQmtfD4qjQdrWQSWM2nJLMgIUQAx3GRiAPdsFX8ahhBcj0fF hIQ/xvdj2072g7u+eU+30bG3QfDpK8OvsZcZGjz2K28jxkNyRQa9pyR6t32FFDeH nbuOdlZ/U1Xo/oE+aK57mVobsDUzIighnDwT1Mq9GFeGU/CsfgAcNat4EJGq/GKd 3zKG+CodsxbAmwl9ggfQ/6AgdBM5iRXJBI3WQ6LXCut3k5Kv+1mx9MXsIKmo0akM auxIJx3AmCjn+vSy5jrWSYRJFGPGa48fOjiLT6dvCJ7USIiE4zpGTiQ+H3FOoQz8 Jf192B7LWLtDCi5zYpr8lBvRCQiea7cPoz0+BDwKPON2YPvMCknsZ9kTGiEjwWK3 WLHB81OmrvwMgllCxNMszeGbhbZmsAj8JyUo8QhyRJjFksWhpCcztuVjsIvS7itw ZYGPd/0yinS8uY/s/vjy4WlLxZsOEjX8Gin+2LKWLk4L4RrnzMePP0jtLOXYVud1 RpVNlmFP46V18zfcdZUwahAwKIh48bv2f0ZBD/Gbk/fT2o+aDAGpkqqOzm8gCdyw H+WxXd+b8/9LKFTtfpUQ10FFoQsHhcSXna1U0z2sjilxe1ENv52BD/ofSjEOmE+D 4l4gEMCwNs8nbfqpUGEvLasnxo8Jk0hrWDsJaU2RVbxhPrxX37A= =giRs -----END PGP SIGNATURE----- --fa7adbnk54nurehu-- --===============0301907458== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0301907458==--