Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Brian Starkey <Brian.Starkey@arm.com>
To:     Paul Kocialkowski <paul.kocialkowski@bootlin.com>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        Maxime Ripard <maxime.ripard@bootlin.com>,
        "linux-sunxi@googlegroups.com" <linux-sunxi@googlegroups.com>,
        David Airlie <airlied@linux.ie>, Chen-Yu Tsai <wens@csie.org>,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
        Sean Paul <sean@poorly.run>, nd <nd@arm.com>
Subject: Re: [PATCH v2 31/43] drm/sun4i: Add a dedicated ioctl call for
 allocating tiled buffers
Thread-Topic: [PATCH v2 31/43] drm/sun4i: Add a dedicated ioctl call for
 allocating tiled buffers
Thread-Index: AQHUgx/q489EEFisaUSe0aohlFTijA==
Date:   Fri, 23 Nov 2018 11:30:20 +0000
Message-ID: <20181123113019.43vrzvj42l35el7z@DESKTOP-E1NTVVP.localdomain>
References: <20181123092515.2511-1-paul.kocialkowski@bootlin.com>
 <20181123092515.2511-32-paul.kocialkowski@bootlin.com>
In-Reply-To: <20181123092515.2511-32-paul.kocialkowski@bootlin.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
user-agent: NeoMutt/20180716-849-147d51
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-publictraffictype: Email
x-microsoft-exchange-diagnostics: 1;AM0PR08MB4099;6:Np1u0MqhcVW/t60jtXrxWg2bAFmVXfXSnu4tE5Wt8UBARd+WBKKJhEq0xnHSZcvIdZuZkPlXQ73jVjob7n14pxc6yZKWMfjgR0nrnClMKGRcFa+dAO/kfHaPRGKqIRvAo+HPrs7THd/tAZ6KbMuzbK08nWYgH48p9/Z3TiY+xYvU2aw1gibCSUkx0iuSeVEghQ3wIZCVQLSVQ9Y0chV6DBc5INVfoMkkbQwDVd8c0ZMs9nCVEFs73BboG72CKc1hlr0OnxYgJ35U22W4pvPXRqjyfLOo0nmwydeWpoa+mWtqH5ANr8ErgzE5/IT94XsSq70R+EYun2Zo1PlAfW8XhrP7EyRg74rNIAzbJuArSFKfbvHG+oBIKsXMqOAdWbyJdoig+Zsh8Rcjet7kjG05CkF51PhajrfP00LiknYqNx6eSHUl3ZYFrfKH3ni2kNfUeHkqyp7LuELkvhiPPHOmDQ==;5:boJYrO8KywcUR+aE9rK8c33pJ1Bm6hAWgoEZ9/uVXKXvZ+A7A693DFsSBb4l/R8w8ruRqNToM5GCSRA3tIdVRBrJj9X6hO0NK5xkdXfr2ayUMuT1jUYaRTfHq2QELP1QFebfIJnWaH7v0faLEIIwTnanYijVJA8r7E/WreOCMzk=;7:fO64V9+NRxYFq3TR9tIxVQsShDhCIIJiV/Yd4H/laOgsLJYwQE4i56p8D9JwLxvjcRICfGCxwJtXk8CbWTbBiYn+jy9TiFWy1ScoYYAT/kRGH2AMcXa+Uwe7YgucyJ1GYQ/iFoZzZCvpKOIcReEAKQ==
x-ms-office365-filtering-correlation-id: 6c9b9609-c860-4ef0-667f-08d651370cca
x-ms-office365-filtering-ht: Tenant
x-microsoft-antispam: BCL:0;PCL:0;RULEID:(2390098)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600074)(711020)(4618075)(2017052603328)(7153060)(7193020);SRVR:AM0PR08MB4099;
x-ms-traffictypediagnostic: AM0PR08MB4099:
nodisclaimer: True
x-microsoft-antispam-prvs: <AM0PR08MB4099E85A959E594F52B5C6EEF0D40@AM0PR08MB4099.eurprd08.prod.outlook.com>
x-ms-exchange-senderadcheck: 1
x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(3002001)(10201501046)(3231442)(944501410)(52105112)(93006095)(93001095)(6055026)(148016)(149066)(150057)(6041310)(20161123558120)(20161123562045)(20161123564045)(20161123560045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(201708071742011)(7699051)(76991095);SRVR:AM0PR08MB4099;BCL:0;PCL:0;RULEID:;SRVR:AM0PR08MB4099;
x-forefront-prvs: 086597191B
x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(366004)(39860400002)(376002)(346002)(396003)(136003)(199004)(189003)(44832011)(229853002)(8936002)(105586002)(8676002)(6306002)(66066001)(6486002)(106356001)(81166006)(256004)(6436002)(81156014)(14444005)(33896004)(476003)(4326008)(11346002)(575784001)(86362001)(52116002)(99286004)(386003)(186003)(102836004)(486006)(7416002)(76176011)(6506007)(26005)(6246003)(54906003)(53936002)(478600001)(5660300001)(68736007)(14454004)(6916009)(1076002)(72206003)(3846002)(6116002)(71190400001)(71200400001)(97736004)(25786009)(316002)(9686003)(446003)(2906002)(7736002)(2900100001)(58126008)(6512007)(305945005)(2004002);DIR:OUT;SFP:1101;SCL:1;SRVR:AM0PR08MB4099;H:AM0PR08MB3025.eurprd08.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;A:1;MX:1;
received-spf: None (protection.outlook.com: arm.com does not designate
 permitted sender hosts)
x-microsoft-antispam-message-info: A8T7ncx16N5O9g629SSd3ufZSl26TzTZxDeCNCFDdLJP26/hH08jp+I/Es4qk4qtr8GhEGseAIquHegEkKsXWtFqK1JBytku5Oas44zq1yRcxTjaChMfR+VCPjO5ztjFVhT0msE63WC56gByqYLqgEo3yt9T9RylYZ1Buyg6GWL25a+8d9eGWrcY+vKmst0+Qw4Z75j7XDMUdP2Tqn2Wy8Q/wZKYcxR83qfFgE2RX6YdmrXjv5FbsMnCSJiSn0p0+q3LfcLpAv5NkZDzItD0BKnkIZzRiK5Jr48WzHDcgS8nMN+6LvEWyEn2Hld67p26GzDZ5rAsOvbGIdCprvP9wgfrpzJ6hKTFGlmUBVkRJ4w=
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3EC585BC2A7BAB48B698E197F41D16FD@eurprd08.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: arm.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6c9b9609-c860-4ef0-667f-08d651370cca
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Nov 2018 11:30:20.2348
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR08MB4099
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Paul,

On Fri, Nov 23, 2018 at 10:25:03AM +0100, Paul Kocialkowski wrote:
>This introduces a dedicated ioctl for allocating buffers for the VPU
>tiling mode. It allows setting up buffers that comply to the hardware
>alignment requirements, by aligning the stride and height to 32 bytes.
>
>Only YUV semiplanar and planar formats are allowed by the ioctl, as the
>hardware does not support the tiling mode for other formats.

What's the general feeling about a more generic version of this ioctl?
There doesn't seem to be anything Allwinner-specific in the ioctl
arguments.

It effectively boils down to:

	size =3D driver->get_fb_size_with_modifier(...);
 	cma_obj =3D drm_gem_cma_create(drm, size);

It would look exactly the same for Mali-DP, and probably others too.
Is it better to try and define something we can share instead of Arm
adding another nearly identical ioctl() later?

I think the minimal viable thing would be to just add modifiers to
your structure (I put them first because padding):

  struct drm_gem_create_with_modifiers {
  	__u64 modifiers[4];
  	__u32 height;
  	__u32 width;
  	__u32 format;
  	/* handle, offsets, pitches, size will be returned */
  	__u32 handle;
  	__u32 pitches[4];
  	__u32 offsets[4];
  	__u64 size;
  };

Thanks,
-Brian

>
>Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>---
> drivers/gpu/drm/sun4i/sun4i_drv.c | 89 +++++++++++++++++++++++++++++++
> include/uapi/drm/sun4i_drm.h      | 42 +++++++++++++++
> 2 files changed, 131 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/sun=
4i_drv.c
>index ccdeae6299eb..5ae32973cf34 100644
>--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
>+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
>@@ -21,6 +21,7 @@
> #include <drm/drm_gem_cma_helper.h>
> #include <drm/drm_fb_helper.h>
> #include <drm/drm_of.h>
>+#include <drm/sun4i_drm.h>
>
> #include "sun4i_drv.h"
> #include "sun4i_frontend.h"
>@@ -28,6 +29,92 @@
> #include "sun4i_tcon.h"
> #include "sun8i_tcon_top.h"
>
>+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;
>+	uint32_t chroma_width;
>+	const struct drm_format_info *info;
>+	int ret;
>+
>+	info =3D drm_format_info(args->format);
>+	if (!info)
>+		return -EINVAL;
>+
>+	/* The tiled output format only applies to non-packed YUV formats. */
>+	if (!info->is_yuv || info->num_planes =3D=3D 1)
>+		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 our tiled format. */
>+	luma_stride =3D ALIGN(args->width, 32);
>+	luma_height =3D ALIGN(args->height, 32);
>+
>+	chroma_width =3D args->width;
>+
>+	/* Semiplanar formats have both U and V data in their chroma plane. */
>+	if (drm_format_info_is_yuv_semiplanar(info))
>+		chroma_width *=3D 2;
>+
>+	chroma_stride =3D ALIGN(DIV_ROUND_UP(chroma_width, info->hsub), 32);
>+	chroma_height =3D ALIGN(DIV_ROUND_UP(args->height, info->vsub), 32);
>+
>+	if (drm_format_info_is_yuv_semiplanar(info)) {
>+		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 (drm_format_info_is_yuv_planar(info)) {
>+		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 other 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 const struct drm_ioctl_desc sun4i_drv_ioctls[] =3D {
>+	DRM_IOCTL_DEF_DRV(SUN4I_GEM_CREATE_TILED, drm_sun4i_gem_create_tiled,
>+			  DRM_UNLOCKED),
>+};
>+
> static int drm_sun4i_gem_dumb_create(struct drm_file *file_priv,
> 				     struct drm_device *drm,
> 				     struct drm_mode_create_dumb *args)
>@@ -44,6 +131,8 @@ static struct drm_driver sun4i_drv_driver =3D {
> 	.driver_features	=3D DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER=
_ATOMIC,
>
> 	/* Generic Operations */
>+	.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",
>diff --git a/include/uapi/drm/sun4i_drm.h b/include/uapi/drm/sun4i_drm.h
>new file mode 100644
>index 000000000000..2c77584b057b
>--- /dev/null
>+++ b/include/uapi/drm/sun4i_drm.h
>@@ -0,0 +1,42 @@
>+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
>+/* sun4i_drm.h
>+ *
>+ * Copyright (C) 2018 Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>+ *
>+ * This program is free software; you can redistribute  it and/or modify =
it
>+ * under  the terms of  the GNU General  Public License as published by t=
he
>+ * Free Software Foundation;  either version 2 of the  License, or (at yo=
ur
>+ * option) any later version.
>+ */
>+
>+#ifndef _UAPI_SUN4I_DRM_H_
>+#define _UAPI_SUN4I_DRM_H_
>+
>+#include "drm.h"
>+
>+#if defined(__cplusplus)
>+extern "C" {
>+#endif
>+
>+struct drm_sun4i_gem_create_tiled {
>+	__u32 height;
>+	__u32 width;
>+	__u32 format;
>+	/* handle, offsets, pitches, size will be returned */
>+	__u32 handle;
>+	__u32 pitches[4];
>+	__u32 offsets[4];
>+	__u64 size;
>+};
>+
>+#define DRM_SUN4I_GEM_CREATE_TILED	0x00
>+
>+#define DRM_IOCTL_SUN4I_GEM_CREATE_TILED \
>+	DRM_IOWR(DRM_COMMAND_BASE + DRM_SUN4I_GEM_CREATE_TILED, \
>+		 struct drm_sun4i_gem_create_tiled)
>+
>+#if defined(__cplusplus)
>+}
>+#endif
>+
>+#endif /* _UAPI_SUN4I_DRM_H_ */
>--=20
>2.19.1
>
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel