From mboxrd@z Thu Jan 1 00:00:00 1970 From: paul.kocialkowski@bootlin.com (Paul Kocialkowski) Date: Tue, 27 Mar 2018 10:41:38 +0200 Subject: [PATCH 09/10] drm/sun4i: Add a dedicated ioctl call for allocating tiled buffers In-Reply-To: <20180323104856.qo7w376xr3gcznmm@flea> References: <20180321152904.22411-1-paul.kocialkowski@bootlin.com> <20180321152904.22411-10-paul.kocialkowski@bootlin.com> <20180323104856.qo7w376xr3gcznmm@flea> Message-ID: <1522140098.1110.40.camel@bootlin.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, 2018-03-23 at 11:48 +0100, Maxime Ripard wrote: > Hi, > > On Wed, Mar 21, 2018 at 04:29:03PM +0100, Paul Kocialkowski wrote: > > This introduces a dedicated ioctl for allocating tiled buffers in > > the > > Allwinner MB32 format, that comes with a handful of specific > > constraints. In particular, the stride of the buffers is expected to > > be > > aligned to 32 bytes. > > You should have more details here. What are those constraints, what is > the expected user? Can you use it only for the scanout, like the dumb > buffers, or also for the other devices in the system? Agreed. > > Signed-off-by: Paul Kocialkowski > > --- > > drivers/gpu/drm/sun4i/sun4i_drv.c | 96 > > +++++++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/sun4i/sun4i_drv.h | 2 + > > include/uapi/drm/sun4i_drm.h | 42 +++++++++++++++++ > > 3 files changed, 140 insertions(+) > > create mode 100644 include/uapi/drm/sun4i_drm.h > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c > > b/drivers/gpu/drm/sun4i/sun4i_drv.c > > index d374bb61c565..e9cb03d34b44 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_drv.c > > +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c > > @@ -21,11 +21,18 @@ > > #include > > #include > > #include > > +#include > > > > #include "sun4i_drv.h" > > #include "sun4i_frontend.h" > > #include "sun4i_framebuffer.h" > > #include "sun4i_tcon.h" > > +#include "sun4i_format.h" > > + > > +static const struct drm_ioctl_desc sun4i_drv_ioctls[] = { > > + DRM_IOCTL_DEF_DRV(SUN4I_GEM_CREATE_TILED, > > drm_sun4i_gem_create_tiled, > > + DRM_AUTH | DRM_RENDER_ALLOW), > > Why do you need DRM_RENDER_ALLOW? as far as I know, this is only > useful for render-nodes. It's probably undeeded indeed. > > +}; > > > > DEFINE_DRM_GEM_CMA_FOPS(sun4i_drv_fops); > > > > @@ -34,6 +41,8 @@ static struct drm_driver sun4i_drv_driver = { > > > > /* Generic Operations */ > > .lastclose = drm_fb_helper_lastclose, > > + .ioctls = sun4i_drv_ioctls, > > + .num_ioctls = ARRAY_SIZE(sun4i_drv_ioctls), > > .fops = &sun4i_drv_fops, > > .name = "sun4i-drm", > > .desc = "Allwinner sun4i Display > > Engine", > > @@ -69,6 +78,93 @@ int drm_sun4i_gem_dumb_create(struct drm_file > > *file_priv, > > return drm_gem_cma_dumb_create_internal(file_priv, drm, > > args); > > } > > > > +int drm_sun4i_gem_create_tiled(struct drm_device *drm, void *data, > > + struct drm_file *file_priv) > > +{ > > + struct drm_sun4i_gem_create_tiled *args = data; > > + struct drm_gem_cma_object *cma_obj; > > + struct drm_gem_object *gem_obj; > > + uint32_t luma_stride, chroma_stride; > > + uint32_t luma_height, chroma_height; > > + int ret; > > + > > + if (!sun4i_format_supports_tiling(args->format)) > > + return -EINVAL; > > + > > + memset(args->pitches, 0, sizeof(args->pitches)); > > + memset(args->offsets, 0, sizeof(args->offsets)); > > + > > + /* Stride and height are aligned to 32 bytes for MB32 tiled > > format. */ > > + luma_stride = ALIGN(args->width, 32); > > + luma_height = ALIGN(args->height, 32); > > + > > + if (sun4i_format_is_semiplanar(args->format)) { > > + chroma_stride = luma_stride; > > + > > + if (sun4i_format_is_yuv420(args->format)) > > + chroma_height = ALIGN(DIV_ROUND_UP(args- > > >height, 2), 32); > > + else if (sun4i_format_is_yuv422(args->format)) > > + chroma_height = luma_height; > > + else > > + return -EINVAL; > > + > > + args->pitches[0] = luma_stride; > > + args->pitches[1] = chroma_stride; > > + > > + args->offsets[0] = 0; > > + args->offsets[1] = luma_stride * luma_height; > > + > > + args->size = luma_stride * luma_height + > > + chroma_stride * chroma_height; > > + } else if (sun4i_format_is_planar(args->format)) { > > + if (sun4i_format_is_yuv411(args->format)) { > > + chroma_stride = ALIGN(DIV_ROUND_UP(args- > > >width, 4), 32); > > + chroma_height = luma_height; > > + } if (sun4i_format_is_yuv420(args->format)) { > > + chroma_stride = ALIGN(DIV_ROUND_UP(args- > > >width, 2), 32); > > + chroma_height = ALIGN(DIV_ROUND_UP(args- > > >height, 2), 32); > > + } else if (sun4i_format_is_yuv422(args->format)) { > > + chroma_stride = ALIGN(DIV_ROUND_UP(args- > > >width, 2), 32); > > + chroma_height = luma_height; > > + } else { > > + return -EINVAL; > > + } > > + > > + args->pitches[0] = luma_stride; > > + args->pitches[1] = chroma_stride; > > + args->pitches[2] = chroma_stride; > > + > > + args->offsets[0] = 0; > > + args->offsets[1] = luma_stride * luma_height; > > + args->offsets[2] = luma_stride * luma_height + > > + chroma_stride * chroma_height; > > + > > + args->size = luma_stride * luma_height + > > + chroma_stride * chroma_height * 2; > > + } else { > > + /* No support for packed formats in tiled mode. */ > > + return -EINVAL; > > + } > > + > > + cma_obj = drm_gem_cma_create(drm, args->size); > > + if (IS_ERR(cma_obj)) > > + return PTR_ERR(cma_obj); > > + > > + gem_obj = &cma_obj->base; > > + > > + /* > > + * allocate a id of idr table where the obj is registered > > + * and handle has the id what user can see. > > + */ > > + ret = drm_gem_handle_create(file_priv, gem_obj, &args- > > >handle); > > + /* drop reference from allocate - handle holds it now. */ > > + drm_gem_object_put_unlocked(gem_obj); > > + if (ret) > > + return ret; > > + > > + return PTR_ERR_OR_ZERO(cma_obj); > > +} > > + > > static void sun4i_remove_framebuffers(void) > > { > > struct apertures_struct *ap; > > diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.h > > b/drivers/gpu/drm/sun4i/sun4i_drv.h > > index 47969711a889..308ff4bfcdd5 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_drv.h > > +++ b/drivers/gpu/drm/sun4i/sun4i_drv.h > > @@ -26,5 +26,7 @@ struct sun4i_drv { > > int drm_sun4i_gem_dumb_create(struct drm_file *file_priv, > > struct drm_device *drm, > > struct drm_mode_create_dumb *args); > > +int drm_sun4i_gem_create_tiled(struct drm_device *dev, void *data, > > + struct drm_file *file_priv); > > Do you need it to be non-static, and part of the header as well? Here as well, I just find that it looks more readable that way, below the drm driver structure definition instead of above it. > I'll let the DRM-misc maintainers comment on the validity of the new > ioctl. Cheers, -- Paul Kocialkowski, Bootlin (formerly Free Electrons) Embedded Linux and kernel engineering https://bootlin.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 488 bytes Desc: This is a digitally signed message part URL: From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Kocialkowski Subject: Re: [PATCH 09/10] drm/sun4i: Add a dedicated ioctl call for allocating tiled buffers Date: Tue, 27 Mar 2018 10:41:38 +0200 Message-ID: <1522140098.1110.40.camel@bootlin.com> References: <20180321152904.22411-1-paul.kocialkowski@bootlin.com> <20180321152904.22411-10-paul.kocialkowski@bootlin.com> <20180323104856.qo7w376xr3gcznmm@flea> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1458319519==" Return-path: Received: from mail.bootlin.com (mail.bootlin.com [62.4.15.54]) by gabe.freedesktop.org (Postfix) with ESMTP id 8387A89B3B for ; Tue, 27 Mar 2018 08:42:43 +0000 (UTC) In-Reply-To: <20180323104856.qo7w376xr3gcznmm@flea> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Maxime Ripard Cc: David Airlie , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Chen-Yu Tsai , Daniel Vetter , linux-arm-kernel@lists.infradead.org List-Id: dri-devel@lists.freedesktop.org --===============1458319519== Content-Type: multipart/signed; micalg="pgp-sha256"; protocol="application/pgp-signature"; boundary="=-3OjRB9vRt7hXtHXTu7nI" --=-3OjRB9vRt7hXtHXTu7nI Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2018-03-23 at 11:48 +0100, Maxime Ripard wrote: > Hi, >=20 > On Wed, Mar 21, 2018 at 04:29:03PM +0100, Paul Kocialkowski wrote: > > This introduces a dedicated ioctl for allocating tiled buffers in > > the > > Allwinner MB32 format, that comes with a handful of specific > > constraints. In particular, the stride of the buffers is expected to > > be > > aligned to 32 bytes. >=20 > You should have more details here. What are those constraints, what is > the expected user? Can you use it only for the scanout, like the dumb > buffers, or also for the other devices in the system? Agreed. > > Signed-off-by: Paul Kocialkowski > > --- > > drivers/gpu/drm/sun4i/sun4i_drv.c | 96 > > +++++++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/sun4i/sun4i_drv.h | 2 + > > include/uapi/drm/sun4i_drm.h | 42 +++++++++++++++++ > > 3 files changed, 140 insertions(+) > > create mode 100644 include/uapi/drm/sun4i_drm.h > >=20 > > diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c > > b/drivers/gpu/drm/sun4i/sun4i_drv.c > > index d374bb61c565..e9cb03d34b44 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_drv.c > > +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c > > @@ -21,11 +21,18 @@ > > #include > > #include > > #include > > +#include > > =20 > > #include "sun4i_drv.h" > > #include "sun4i_frontend.h" > > #include "sun4i_framebuffer.h" > > #include "sun4i_tcon.h" > > +#include "sun4i_format.h" > > + > > +static const struct drm_ioctl_desc sun4i_drv_ioctls[] =3D { > > + DRM_IOCTL_DEF_DRV(SUN4I_GEM_CREATE_TILED, > > drm_sun4i_gem_create_tiled, > > + DRM_AUTH | DRM_RENDER_ALLOW), >=20 > Why do you need DRM_RENDER_ALLOW? as far as I know, this is only > useful for render-nodes. It's probably undeeded indeed. > > +}; > > =20 > > DEFINE_DRM_GEM_CMA_FOPS(sun4i_drv_fops); > > =20 > > @@ -34,6 +41,8 @@ static struct drm_driver sun4i_drv_driver =3D { > > =20 > > /* Generic Operations */ > > .lastclose =3D drm_fb_helper_lastclose, > > + .ioctls =3D sun4i_drv_ioctls, > > + .num_ioctls =3D ARRAY_SIZE(sun4i_drv_ioctls), > > .fops =3D &sun4i_drv_fops, > > .name =3D "sun4i-drm", > > .desc =3D "Allwinner sun4i Display > > Engine", > > @@ -69,6 +78,93 @@ int drm_sun4i_gem_dumb_create(struct drm_file > > *file_priv, > > return drm_gem_cma_dumb_create_internal(file_priv, drm, > > args); > > } > > =20 > > +int drm_sun4i_gem_create_tiled(struct drm_device *drm, void *data, > > + struct drm_file *file_priv) > > +{ > > + struct drm_sun4i_gem_create_tiled *args =3D data; > > + struct drm_gem_cma_object *cma_obj; > > + struct drm_gem_object *gem_obj; > > + uint32_t luma_stride, chroma_stride; > > + uint32_t luma_height, chroma_height; > > + int ret; > > + > > + if (!sun4i_format_supports_tiling(args->format)) > > + return -EINVAL; > > + > > + memset(args->pitches, 0, sizeof(args->pitches)); > > + memset(args->offsets, 0, sizeof(args->offsets)); > > + > > + /* Stride and height are aligned to 32 bytes for MB32 tiled > > format. */ > > + luma_stride =3D ALIGN(args->width, 32); > > + luma_height =3D ALIGN(args->height, 32); > > + > > + if (sun4i_format_is_semiplanar(args->format)) { > > + chroma_stride =3D luma_stride; > > + > > + if (sun4i_format_is_yuv420(args->format)) > > + chroma_height =3D ALIGN(DIV_ROUND_UP(args- > > >height, 2), 32); > > + else if (sun4i_format_is_yuv422(args->format)) > > + chroma_height =3D luma_height; > > + else > > + return -EINVAL; > > + > > + args->pitches[0] =3D luma_stride; > > + args->pitches[1] =3D chroma_stride; > > + > > + args->offsets[0] =3D 0; > > + args->offsets[1] =3D luma_stride * luma_height; > > + > > + args->size =3D luma_stride * luma_height + > > + chroma_stride * chroma_height; > > + } else if (sun4i_format_is_planar(args->format)) { > > + if (sun4i_format_is_yuv411(args->format)) { > > + chroma_stride =3D ALIGN(DIV_ROUND_UP(args- > > >width, 4), 32); > > + chroma_height =3D luma_height; > > + } if (sun4i_format_is_yuv420(args->format)) { > > + chroma_stride =3D ALIGN(DIV_ROUND_UP(args- > > >width, 2), 32); > > + chroma_height =3D ALIGN(DIV_ROUND_UP(args- > > >height, 2), 32); > > + } else if (sun4i_format_is_yuv422(args->format)) { > > + chroma_stride =3D ALIGN(DIV_ROUND_UP(args- > > >width, 2), 32); > > + chroma_height =3D luma_height; > > + } else { > > + return -EINVAL; > > + } > > + > > + args->pitches[0] =3D luma_stride; > > + args->pitches[1] =3D chroma_stride; > > + args->pitches[2] =3D chroma_stride; > > + > > + args->offsets[0] =3D 0; > > + args->offsets[1] =3D luma_stride * luma_height; > > + args->offsets[2] =3D luma_stride * luma_height + > > + chroma_stride * chroma_height; > > + > > + args->size =3D luma_stride * luma_height + > > + chroma_stride * chroma_height * 2; > > + } else { > > + /* No support for packed formats in tiled mode. */ > > + return -EINVAL; > > + } > > + > > + cma_obj =3D drm_gem_cma_create(drm, args->size); > > + if (IS_ERR(cma_obj)) > > + return PTR_ERR(cma_obj); > > + > > + gem_obj =3D &cma_obj->base; > > + > > + /* > > + * allocate a id of idr table where the obj is registered > > + * and handle has the id what user can see. > > + */ > > + ret =3D drm_gem_handle_create(file_priv, gem_obj, &args- > > >handle); > > + /* drop reference from allocate - handle holds it now. */ > > + drm_gem_object_put_unlocked(gem_obj); > > + if (ret) > > + return ret; > > + > > + return PTR_ERR_OR_ZERO(cma_obj); > > +} > > + > > static void sun4i_remove_framebuffers(void) > > { > > struct apertures_struct *ap; > > diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.h > > b/drivers/gpu/drm/sun4i/sun4i_drv.h > > index 47969711a889..308ff4bfcdd5 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_drv.h > > +++ b/drivers/gpu/drm/sun4i/sun4i_drv.h > > @@ -26,5 +26,7 @@ struct sun4i_drv { > > int drm_sun4i_gem_dumb_create(struct drm_file *file_priv, > > struct drm_device *drm, > > struct drm_mode_create_dumb *args); > > +int drm_sun4i_gem_create_tiled(struct drm_device *dev, void *data, > > + struct drm_file *file_priv); >=20 > Do you need it to be non-static, and part of the header as well? Here as well, I just find that it looks more readable that way, below the drm driver structure definition instead of above it. > I'll let the DRM-misc maintainers comment on the validity of the new > ioctl. Cheers, --=20 Paul Kocialkowski, Bootlin (formerly Free Electrons) Embedded Linux and kernel engineering https://bootlin.com --=-3OjRB9vRt7hXtHXTu7nI Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEJZpWjZeIetVBefti3cLmz3+fv9EFAlq6A8IACgkQ3cLmz3+f v9HcCAf/Zft0yw0Qib9nvZwlqFJTq8dZLlE8bBt3YsA0bn0CVp3A81iCc9zrZcl3 xPDwkFyX/layT3FtKbQVydOmUBwbkYiEwS+dBFPSSGhRPZRI/x0zYYrSUxrIkSJs /f18g/4NuXgFvFXihp/LpPBQoCau/4RCG7ADNJiBa689qrwhu0zktzxe84Umz3qY ijsPoefFuy+gfHIGhDSpUeK3nmXlQqP1eNSLvutenhLlcDXvxHT4n7cknUWlZqXQ wmvb6h8oYwIlNzqMokAHYSDJSGaTIYxqKU/TZpQLRzc/4b4HJGcxMQMtJbasGrRm 5p4DM5rxWmekXBOX1AImL5ngLy0FfA== =utnW -----END PGP SIGNATURE----- --=-3OjRB9vRt7hXtHXTu7nI-- --===============1458319519== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1458319519==-- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752171AbeC0In6 (ORCPT ); Tue, 27 Mar 2018 04:43:58 -0400 Received: from mail.bootlin.com ([62.4.15.54]:37798 "EHLO mail.bootlin.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752357AbeC0Imo (ORCPT ); Tue, 27 Mar 2018 04:42:44 -0400 Message-ID: <1522140098.1110.40.camel@bootlin.com> Subject: Re: [PATCH 09/10] drm/sun4i: Add a dedicated ioctl call for allocating tiled buffers From: Paul Kocialkowski To: Maxime Ripard Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, David Airlie , Chen-Yu Tsai , Daniel Vetter , Gustavo Padovan , Sean Paul Date: Tue, 27 Mar 2018 10:41:38 +0200 In-Reply-To: <20180323104856.qo7w376xr3gcznmm@flea> References: <20180321152904.22411-1-paul.kocialkowski@bootlin.com> <20180321152904.22411-10-paul.kocialkowski@bootlin.com> <20180323104856.qo7w376xr3gcznmm@flea> Organization: Bootlin Content-Type: multipart/signed; micalg="pgp-sha256"; protocol="application/pgp-signature"; boundary="=-3OjRB9vRt7hXtHXTu7nI" X-Mailer: Evolution 3.26.5 Mime-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-3OjRB9vRt7hXtHXTu7nI Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2018-03-23 at 11:48 +0100, Maxime Ripard wrote: > Hi, >=20 > On Wed, Mar 21, 2018 at 04:29:03PM +0100, Paul Kocialkowski wrote: > > This introduces a dedicated ioctl for allocating tiled buffers in > > the > > Allwinner MB32 format, that comes with a handful of specific > > constraints. In particular, the stride of the buffers is expected to > > be > > aligned to 32 bytes. >=20 > You should have more details here. What are those constraints, what is > the expected user? Can you use it only for the scanout, like the dumb > buffers, or also for the other devices in the system? Agreed. > > Signed-off-by: Paul Kocialkowski > > --- > > drivers/gpu/drm/sun4i/sun4i_drv.c | 96 > > +++++++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/sun4i/sun4i_drv.h | 2 + > > include/uapi/drm/sun4i_drm.h | 42 +++++++++++++++++ > > 3 files changed, 140 insertions(+) > > create mode 100644 include/uapi/drm/sun4i_drm.h > >=20 > > diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c > > b/drivers/gpu/drm/sun4i/sun4i_drv.c > > index d374bb61c565..e9cb03d34b44 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_drv.c > > +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c > > @@ -21,11 +21,18 @@ > > #include > > #include > > #include > > +#include > > =20 > > #include "sun4i_drv.h" > > #include "sun4i_frontend.h" > > #include "sun4i_framebuffer.h" > > #include "sun4i_tcon.h" > > +#include "sun4i_format.h" > > + > > +static const struct drm_ioctl_desc sun4i_drv_ioctls[] =3D { > > + DRM_IOCTL_DEF_DRV(SUN4I_GEM_CREATE_TILED, > > drm_sun4i_gem_create_tiled, > > + DRM_AUTH | DRM_RENDER_ALLOW), >=20 > Why do you need DRM_RENDER_ALLOW? as far as I know, this is only > useful for render-nodes. It's probably undeeded indeed. > > +}; > > =20 > > DEFINE_DRM_GEM_CMA_FOPS(sun4i_drv_fops); > > =20 > > @@ -34,6 +41,8 @@ static struct drm_driver sun4i_drv_driver =3D { > > =20 > > /* Generic Operations */ > > .lastclose =3D drm_fb_helper_lastclose, > > + .ioctls =3D sun4i_drv_ioctls, > > + .num_ioctls =3D ARRAY_SIZE(sun4i_drv_ioctls), > > .fops =3D &sun4i_drv_fops, > > .name =3D "sun4i-drm", > > .desc =3D "Allwinner sun4i Display > > Engine", > > @@ -69,6 +78,93 @@ int drm_sun4i_gem_dumb_create(struct drm_file > > *file_priv, > > return drm_gem_cma_dumb_create_internal(file_priv, drm, > > args); > > } > > =20 > > +int drm_sun4i_gem_create_tiled(struct drm_device *drm, void *data, > > + struct drm_file *file_priv) > > +{ > > + struct drm_sun4i_gem_create_tiled *args =3D data; > > + struct drm_gem_cma_object *cma_obj; > > + struct drm_gem_object *gem_obj; > > + uint32_t luma_stride, chroma_stride; > > + uint32_t luma_height, chroma_height; > > + int ret; > > + > > + if (!sun4i_format_supports_tiling(args->format)) > > + return -EINVAL; > > + > > + memset(args->pitches, 0, sizeof(args->pitches)); > > + memset(args->offsets, 0, sizeof(args->offsets)); > > + > > + /* Stride and height are aligned to 32 bytes for MB32 tiled > > format. */ > > + luma_stride =3D ALIGN(args->width, 32); > > + luma_height =3D ALIGN(args->height, 32); > > + > > + if (sun4i_format_is_semiplanar(args->format)) { > > + chroma_stride =3D luma_stride; > > + > > + if (sun4i_format_is_yuv420(args->format)) > > + chroma_height =3D ALIGN(DIV_ROUND_UP(args- > > >height, 2), 32); > > + else if (sun4i_format_is_yuv422(args->format)) > > + chroma_height =3D luma_height; > > + else > > + return -EINVAL; > > + > > + args->pitches[0] =3D luma_stride; > > + args->pitches[1] =3D chroma_stride; > > + > > + args->offsets[0] =3D 0; > > + args->offsets[1] =3D luma_stride * luma_height; > > + > > + args->size =3D luma_stride * luma_height + > > + chroma_stride * chroma_height; > > + } else if (sun4i_format_is_planar(args->format)) { > > + if (sun4i_format_is_yuv411(args->format)) { > > + chroma_stride =3D ALIGN(DIV_ROUND_UP(args- > > >width, 4), 32); > > + chroma_height =3D luma_height; > > + } if (sun4i_format_is_yuv420(args->format)) { > > + chroma_stride =3D ALIGN(DIV_ROUND_UP(args- > > >width, 2), 32); > > + chroma_height =3D ALIGN(DIV_ROUND_UP(args- > > >height, 2), 32); > > + } else if (sun4i_format_is_yuv422(args->format)) { > > + chroma_stride =3D ALIGN(DIV_ROUND_UP(args- > > >width, 2), 32); > > + chroma_height =3D luma_height; > > + } else { > > + return -EINVAL; > > + } > > + > > + args->pitches[0] =3D luma_stride; > > + args->pitches[1] =3D chroma_stride; > > + args->pitches[2] =3D chroma_stride; > > + > > + args->offsets[0] =3D 0; > > + args->offsets[1] =3D luma_stride * luma_height; > > + args->offsets[2] =3D luma_stride * luma_height + > > + chroma_stride * chroma_height; > > + > > + args->size =3D luma_stride * luma_height + > > + chroma_stride * chroma_height * 2; > > + } else { > > + /* No support for packed formats in tiled mode. */ > > + return -EINVAL; > > + } > > + > > + cma_obj =3D drm_gem_cma_create(drm, args->size); > > + if (IS_ERR(cma_obj)) > > + return PTR_ERR(cma_obj); > > + > > + gem_obj =3D &cma_obj->base; > > + > > + /* > > + * allocate a id of idr table where the obj is registered > > + * and handle has the id what user can see. > > + */ > > + ret =3D drm_gem_handle_create(file_priv, gem_obj, &args- > > >handle); > > + /* drop reference from allocate - handle holds it now. */ > > + drm_gem_object_put_unlocked(gem_obj); > > + if (ret) > > + return ret; > > + > > + return PTR_ERR_OR_ZERO(cma_obj); > > +} > > + > > static void sun4i_remove_framebuffers(void) > > { > > struct apertures_struct *ap; > > diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.h > > b/drivers/gpu/drm/sun4i/sun4i_drv.h > > index 47969711a889..308ff4bfcdd5 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_drv.h > > +++ b/drivers/gpu/drm/sun4i/sun4i_drv.h > > @@ -26,5 +26,7 @@ struct sun4i_drv { > > int drm_sun4i_gem_dumb_create(struct drm_file *file_priv, > > struct drm_device *drm, > > struct drm_mode_create_dumb *args); > > +int drm_sun4i_gem_create_tiled(struct drm_device *dev, void *data, > > + struct drm_file *file_priv); >=20 > Do you need it to be non-static, and part of the header as well? Here as well, I just find that it looks more readable that way, below the drm driver structure definition instead of above it. > I'll let the DRM-misc maintainers comment on the validity of the new > ioctl. Cheers, --=20 Paul Kocialkowski, Bootlin (formerly Free Electrons) Embedded Linux and kernel engineering https://bootlin.com --=-3OjRB9vRt7hXtHXTu7nI Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEJZpWjZeIetVBefti3cLmz3+fv9EFAlq6A8IACgkQ3cLmz3+f v9HcCAf/Zft0yw0Qib9nvZwlqFJTq8dZLlE8bBt3YsA0bn0CVp3A81iCc9zrZcl3 xPDwkFyX/layT3FtKbQVydOmUBwbkYiEwS+dBFPSSGhRPZRI/x0zYYrSUxrIkSJs /f18g/4NuXgFvFXihp/LpPBQoCau/4RCG7ADNJiBa689qrwhu0zktzxe84Umz3qY ijsPoefFuy+gfHIGhDSpUeK3nmXlQqP1eNSLvutenhLlcDXvxHT4n7cknUWlZqXQ wmvb6h8oYwIlNzqMokAHYSDJSGaTIYxqKU/TZpQLRzc/4b4HJGcxMQMtJbasGrRm 5p4DM5rxWmekXBOX1AImL5ngLy0FfA== =utnW -----END PGP SIGNATURE----- --=-3OjRB9vRt7hXtHXTu7nI--