From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Anholt Subject: Re: [RFC PATCH] drm/panfrost: Add initial panfrost driver Date: Fri, 08 Mar 2019 08:28:59 -0800 Message-ID: <87h8cdxrck.fsf@anholt.net> References: <20190308002408.32682-1-robh@kernel.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0010285533==" Return-path: Received: from anholt.net (anholt.net [50.246.234.109]) by gabe.freedesktop.org (Postfix) with ESMTP id 7C3E96E3F5 for ; Fri, 8 Mar 2019 16:29:05 +0000 (UTC) In-Reply-To: <20190308002408.32682-1-robh@kernel.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Rob Herring , dri-devel@lists.freedesktop.org Cc: Tomeu Vizoso , Maxime Ripard , David Airlie , "Marty E. Plummer" , Sean Paul , Alyssa Rosenzweig List-Id: dri-devel@lists.freedesktop.org --===============0010285533== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Rob Herring writes: > From: "Marty E. Plummer" > > This adds the initial driver for panfrost which supports Arm Mali > Midgard and Bifrost family of GPUs. Currently, only the T860 Midgard GPU > has been tested. > > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > Cc: Alyssa Rosenzweig > Cc: Lyude Paul > Cc: Eric Anholt > Signed-off-by: Marty E. Plummer > Signed-off-by: Tomeu Vizoso > Signed-off-by: Rob Herring > --- > Sending this out in the spirit of release early, release often. We're=20 > close to parity compared to mesa + the vendor driver. There's a few=20 > issues Tomeu is chasing.=20 > > There's still some pieces of the h/w setup we've just hardcoded. Locking= =20 > in various places is probably missing. Error recovery is non-existent=20 > (other than module unload/load). There's some work to add tracepoints=20 > and perf counters that's not here yet. Bifrost GPUs are definitely not=20 > supported yet other than identifying them. Primarily the MMU setup is=20 > missing. > > How's performance? Great, because I haven't measured it. > > This patch and its dependencies are available here[1]. The mesa support=20 > is here[2]. Both are likely to change (daily). My inclination would be to merge this soon, when basic checkpatch is in shape. The UABI looks good, and that's the important part. > +static void panfrost_unlock_bo_reservations(struct drm_gem_object **bos, > + int bo_count, > + struct ww_acquire_ctx *acquire_ctx) > +{ > + int i; > + > + for (i =3D 0; i < bo_count; i++) { > + ww_mutex_unlock(&bos[i]->resv->lock); > + } > + ww_acquire_fini(acquire_ctx); > +} > + > +/* Takes the reservation lock on all the BOs being referenced, so that > + * at queue submit time we can update the reservations. > + * > + * We don't lock the RCL the tile alloc/state BOs, or overflow memory > + * (all of which are on exec->unref_list). They're entirely private > + * to panfrost, so we don't attach dma-buf fences to them. > + */ > +static int panfrost_lock_bo_reservations(struct drm_gem_object **bos, > + int bo_count, > + struct ww_acquire_ctx *acquire_ctx) > +{ > + int contended_lock =3D -1; > + int i, ret; > + > + ww_acquire_init(acquire_ctx, &reservation_ww_class); > + > +retry: > + if (contended_lock !=3D -1) { > + struct drm_gem_object *bo =3D bos[contended_lock]; > + > + ret =3D ww_mutex_lock_slow_interruptible(&bo->resv->lock, > + acquire_ctx); > + if (ret) { > + ww_acquire_done(acquire_ctx); > + return ret; > + } > + } > + > + for (i =3D 0; i < bo_count; i++) { > + if (i =3D=3D contended_lock) > + continue; > + > + ret =3D ww_mutex_lock_interruptible(&bos[i]->resv->lock, > + acquire_ctx); > + if (ret) { > + int j; > + > + for (j =3D 0; j < i; j++) > + ww_mutex_unlock(&bos[j]->resv->lock); > + > + if (contended_lock !=3D -1 && contended_lock >=3D i) { > + struct drm_gem_object *bo =3D bos[contended_lock]; > + > + ww_mutex_unlock(&bo->resv->lock); > + } > + > + if (ret =3D=3D -EDEADLK) { > + contended_lock =3D i; > + goto retry; > + } > + > + ww_acquire_done(acquire_ctx); > + return ret; > + } > + } > + > + ww_acquire_done(acquire_ctx); > + > + /* Reserve space for our shared (read-only) fence references, > + * before we commit the job to the hardware. > + */ > + for (i =3D 0; i < bo_count; i++) { > + ret =3D reservation_object_reserve_shared(bos[i]->resv, 1); > + if (ret) { > + panfrost_unlock_bo_reservations(bos, bo_count, > + acquire_ctx); > + return ret; > + } > + } > + > + return 0; > +} I just sent out my shared helpers for most of this function, hopefully you can use them. It looks like you've got v3d's silliness with the fences -- we reserve a shared slot, then use excl only anyway. For v3d I'm planning on moving to just excl -- only one of my entrypoints has info on write vs read-only, and I don't know of a usecase where having multiple read-only consumers of a shared buffer simultaneously matters. More importantly, I think you also have my bug of not doing implicit synchronization on buffers, which will break X11 rendering sometimes. X11's GL requirements are that previously-submitted rendering by the client fd will execute before X11's rendering on its fd to the same buffers. If you're running a single client, X11's copies are cheap enough that it'll probably work out most of the time. > --- /dev/null > +++ b/include/uapi/drm/panfrost_drm.h > +#define DRM_IOCTL_PANFROST_SUBMIT DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFR= OST_SUBMIT, struct drm_panfrost_submit) > +#define DRM_IOCTL_PANFROST_WAIT_BO DRM_IOWR(DRM_COMMAND_BASE + DRM_PANF= ROST_WAIT_BO, struct drm_panfrost_wait_bo) > +#define DRM_IOCTL_PANFROST_CREATE_BO DRM_IOWR(DRM_COMMAND_BASE + DRM_PA= NFROST_CREATE_BO, struct drm_panfrost_create_bo) > +#define DRM_IOCTL_PANFROST_MMAP_BO DRM_IOWR(DRM_COMMAND_BASE + DRM_PANF= ROST_MMAP_BO, struct drm_panfrost_mmap_bo) > +#define DRM_IOCTL_PANFROST_GET_PARAM DRM_IOWR(DRM_COMMAND_BASE + DRM_PA= NFROST_GET_PARAM, struct drm_panfrost_get_param) > +#define DRM_IOCTL_PANFROST_GET_BO_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM= _PANFROST_GET_BO_OFFSET, struct drm_panfrost_get_bo_offset) SUBMIT and WAIT_BO might be IOR instead of IOWR --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAlyCmEsACgkQtdYpNtH8 nuhsDg/8DD6no2A3Qt2mwV+Wmy22XVOfAk/VbqFnTTf0A0PKaUon0338DiQLNcTU kRQInFKrznv/KEvrp84AKC3w+aIkSrPZ/bQkvubil3XzpVjxHAi5tSU4mzb5V1mf RPYfXk6NTKNEJUM1FjeVmL0Q9TLu+taoMdNv+DMlUXe8z5C7I459p3+VtZUTr29A 6fzJQuRek3RJgNP8foTijvhqxQcantxuMUzpD98/9h50yrAibqQMfZotazDnKp0b n+KOJUppRuBIRhw/pJ6Y70Xo7AqyqpBVTDwqa+mqUUcxP8FZvJj8eoFkwvx/3c8a NzkGr+Sygh5rEUI8E9f/f4W4EF3k7mvSTITax1TwnIFiyK1Eq5SqTNgnxj1/G4l1 UkVguVQ1WQmzuxUHqlDiBEYspCxNqhI5hGYosVZrZ9yn7/PSkH1mih0RgDXN3mDe /YJryqlSJ2i/fMK+eUnzsIIY16xLPdv4uLAtPE7+NlnopU7fVSyFRkb6ZmRy7wVE cgE9HMSjVG+SSqGjIsLTaEEvtiBWh9Ql9yN2W6YW2d+62Ru5hWSa72E0ctDSj0UY 6glvqgtBOuBEiNJocou2jch/nhh9hQru9JqAUMLrBuEC+VEp0QaTr8iW5HpO9Vir BGYE7BKZeN4CSDq1/dZA8nosR4f+uZWelg3a5rNLIBfbNtt9PVY= =ZUvx -----END PGP SIGNATURE----- --=-=-=-- --===============0010285533== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVs --===============0010285533==--