Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752159AbdFOWLb (ORCPT ); Thu, 15 Jun 2017 18:11:31 -0400 Received: from anholt.net ([50.246.234.109]:57832 "EHLO anholt.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750830AbdFOWLa (ORCPT ); Thu, 15 Jun 2017 18:11:30 -0400 From: Eric Anholt To: Boris Brezillon Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] drm/vc4: Add get/set tiling ioctls. In-Reply-To: <20170613094645.625031a5@bbrezillon> References: <20170608001336.12842-1-eric@anholt.net> <20170608001336.12842-2-eric@anholt.net> <20170613094645.625031a5@bbrezillon> User-Agent: Notmuch/0.22.2+1~gb0bcfaa (http://notmuchmail.org) Emacs/24.5.1 (x86_64-pc-linux-gnu) Date: Thu, 15 Jun 2017 15:11:26 -0700 Message-ID: <87d1a452b5.fsf@eliezer.anholt.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6485 Lines: 194 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Boris Brezillon writes: > Hi Eric, > > On Wed, 7 Jun 2017 17:13:36 -0700 > Eric Anholt wrote: > >> This allows mesa to set the tiling format for a BO and have that >> tiling format be respected by mesa on the other side of an >> import/export (and by vc4 scanout in the kernel), without defining a >> protocol to pass the tiling through userspace. >>=20 >> Signed-off-by: Eric Anholt >> --- >>=20 >> igt tests (which caught two edge cases) submitted to intel-gfx. >>=20 >> Mesa code: https://github.com/anholt/mesa/commits/drm-vc4-tiling >>=20 >> drivers/gpu/drm/vc4/vc4_bo.c | 83 ++++++++++++++++++++++++++++++++++++= +++++++ >> drivers/gpu/drm/vc4/vc4_drv.c | 2 ++ >> drivers/gpu/drm/vc4/vc4_drv.h | 6 ++++ >> drivers/gpu/drm/vc4/vc4_kms.c | 41 ++++++++++++++++++++- >> include/uapi/drm/vc4_drm.h | 16 +++++++++ >> 5 files changed, 147 insertions(+), 1 deletion(-) >>=20 >> diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c >> index 80b2f9e55c5c..21649109fd4f 100644 >> --- a/drivers/gpu/drm/vc4/vc4_bo.c >> +++ b/drivers/gpu/drm/vc4/vc4_bo.c >> @@ -347,6 +347,7 @@ void vc4_free_object(struct drm_gem_object *gem_bo) >> bo->validated_shader =3D NULL; >> } >>=20=20 >> + bo->t_format =3D false; >> bo->free_time =3D jiffies; >> list_add(&bo->size_head, cache_list); >> list_add(&bo->unref_head, &vc4->bo_cache.time_list); >> @@ -572,6 +573,88 @@ vc4_create_shader_bo_ioctl(struct drm_device *dev, = void *data, >> return ret; >> } >>=20=20 >> +/** >> + * vc4_set_tiling_ioctl() - Sets the tiling modifier for a BO. >> + * @dev: DRM device >> + * @data: ioctl argument >> + * @file_priv: DRM file for this fd >> + * >> + * The tiling state of the BO decides the default modifier of an fb if >> + * no specific modifier was set by userspace, and the return value of >> + * vc4_get_tiling_ioctl() (so that userspace can treat a BO it >> + * received from dmabuf as the same tiling format as the producer >> + * used). >> + */ >> +int vc4_set_tiling_ioctl(struct drm_device *dev, void *data, >> + struct drm_file *file_priv) >> +{ >> + struct drm_vc4_set_tiling *args =3D data; >> + struct drm_gem_object *gem_obj; >> + struct vc4_bo *bo; >> + bool t_format; >> + >> + if (args->flags !=3D 0) >> + return -EINVAL; >> + >> + switch (args->modifier) { >> + case DRM_FORMAT_MOD_NONE: >> + t_format =3D false; >> + break; >> + case DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED: >> + t_format =3D true; >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + gem_obj =3D drm_gem_object_lookup(file_priv, args->handle); >> + if (!gem_obj) { >> + DRM_ERROR("Failed to look up GEM BO %d\n", args->handle); >> + return -ENOENT; >> + } >> + bo =3D to_vc4_bo(gem_obj); >> + bo->t_format =3D t_format; >> + >> + drm_gem_object_unreference_unlocked(gem_obj); >> + >> + return 0; >> +} >> + >> +/** >> + * vc4_get_tiling_ioctl() - Gets the tiling modifier for a BO. >> + * @dev: DRM device >> + * @data: ioctl argument >> + * @file_priv: DRM file for this fd >> + * >> + * Returns the tiling modifier for a BO as set by vc4_set_tiling_ioctl(= ). >> + */ >> +int vc4_get_tiling_ioctl(struct drm_device *dev, void *data, >> + struct drm_file *file_priv) >> +{ >> + struct drm_vc4_get_tiling *args =3D data; >> + struct drm_gem_object *gem_obj; >> + struct vc4_bo *bo; >> + >> + if (args->flags !=3D 0 || args->modifier !=3D 0) >> + return -EINVAL; >> + >> + gem_obj =3D drm_gem_object_lookup(file_priv, args->handle); >> + if (!gem_obj) { >> + DRM_ERROR("Failed to look up GEM BO %d\n", args->handle); >> + return -ENOENT; >> + } >> + bo =3D to_vc4_bo(gem_obj); >> + >> + if (bo->t_format) >> + args->modifier =3D DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED; >> + else >> + args->modifier =3D DRM_FORMAT_MOD_NONE; >> + >> + drm_gem_object_unreference_unlocked(gem_obj); >> + >> + return 0; >> +} >> + >> void vc4_bo_cache_init(struct drm_device *dev) >> { >> struct vc4_dev *vc4 =3D to_vc4_dev(dev); >> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv= .c >> index 136bb4213dc0..c6b487c3d2b7 100644 >> --- a/drivers/gpu/drm/vc4/vc4_drv.c >> +++ b/drivers/gpu/drm/vc4/vc4_drv.c >> @@ -138,6 +138,8 @@ static const struct drm_ioctl_desc vc4_drm_ioctls[] = =3D { >> DRM_IOCTL_DEF_DRV(VC4_GET_HANG_STATE, vc4_get_hang_state_ioctl, >> DRM_ROOT_ONLY), >> DRM_IOCTL_DEF_DRV(VC4_GET_PARAM, vc4_get_param_ioctl, DRM_RENDER_ALLOW= ), >> + DRM_IOCTL_DEF_DRV(VC4_SET_TILING, vc4_set_tiling_ioctl, DRM_RENDER_ALL= OW), >> + DRM_IOCTL_DEF_DRV(VC4_GET_TILING, vc4_get_tiling_ioctl, DRM_RENDER_ALL= OW), >> }; >>=20=20 >> static struct drm_driver vc4_drm_driver =3D { >> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv= .h >> index a5bf2e5e0b57..df22698d62ee 100644 >> --- a/drivers/gpu/drm/vc4/vc4_drv.h >> +++ b/drivers/gpu/drm/vc4/vc4_drv.h >> @@ -148,6 +148,8 @@ struct vc4_bo { >> */ >> uint64_t write_seqno; >>=20=20 >> + bool t_format; >> + > > Will we need the DRM_VC4_SET/GET_TILING ioctls when importing a BO > that is using H264 tile mode? If this is the case, we should probably > store the modifier directly. I'm not sure. Whoever is getting buffers from the ISP is going to be doing the prime import to vc4 for displaying it on a plane, so it seems about equal complexity ot me to do it either way. If we're using some existing dma-buf based media stack, it might support plane modifiers already, though. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAllDBg4ACgkQtdYpNtH8 nuh7yA/9Fs1LgQT5xECijDkUnVEgZJn8hsh2NjZmahiFReV5ShncDAK1DkAk/yOt 2l1CdhJ+rdNEbR+0r3TEZBSgTR7zauz53AO9i0YfgtiS9ePIjjTHBXriirDJ4xVO xhvMUZuvRrEtdpcd+oOALxKuE6ZNF5VkrHhmF0q1wV9kW/KhPTdRd7j2dllgFhq3 iG5TM7adejBMk0iqQ4Q5Zk4SlsWX5zVzsjdse05H1oGiRDam3UkP8YgYqqpY70+v NVNJCEkyjgMSHP37IBoJelVrTVygO1nKnbY8kCZp8JQwaPUQ1bdpn9Hy7d9uNFjy cmAYWylhnczW2ewsgY6KV76SIMVa2jpZ/2Qyq4u7XcCWOCW4GqgVJznTLbLy/bb7 uYg/1g0zyB00q4FbY1zGcnxMiuhwt8GzwEfPeXchsr8v1Uaxq8kAWGerY8F1z95x xC92sVuEpbnPiU2trY6FjknAng9qEMzHaSIXwGdQa2TK+AGnBnwRNJ0MWruy5lyN +fxqKsAUzDTYfL15TDubD74pb3i3Nr1YtQe/cw8gW+ivFjt/lJg4aDlVV8sftYOS ni2sq5/7/JmuBj2TaG/t91QyEVFEfKYf5amf8vCKit6EN01Sj/70PBd3fwT5DT9c ac7PlOj8RqoeHGaVyLuEQyW7KFalhld3IuCNQI34OhBnsmx6j20= =yB0b -----END PGP SIGNATURE----- --=-=-=--