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:   Eric Anholt <eric@anholt.net>
To:     Rob Herring <robh@kernel.org>, dri-devel@lists.freedesktop.org
Cc:     linux-kernel@vger.kernel.org,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <maxime.ripard@bootlin.com>,
        Sean Paul <sean@poorly.run>, David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>, Lyude Paul <lyude@redhat.com>,
        "Marty E . Plummer" <hanetzer@startmail.com>,
        Tomeu Vizoso <tomeu.vizoso@collabora.com>,
        Neil Armstrong <narmstrong@baylibre.com>,
        Alyssa Rosenzweig <alyssa@rosenzweig.io>
Subject: Re: [PATCH v3 3/3] drm/panfrost: Add initial panfrost driver
In-Reply-To: <20190409205427.6943-4-robh@kernel.org>
References: <20190409205427.6943-1-robh@kernel.org> <20190409205427.6943-4-robh@kernel.org>
User-Agent: Notmuch/0.22.2+1~gb0bcfaa (http://notmuchmail.org) Emacs/26.1 (x86_64-pc-linux-gnu)
Date:   Tue, 09 Apr 2019 14:25:53 -0700
Message-ID: <87v9zmdg66.fsf@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
Precedence: bulk

--=-=-=
Content-Type: text/plain

Rob Herring <robh@kernel.org> writes:

> This adds the initial driver for panfrost which supports Arm Mali
> Midgard and Bifrost family of GPUs. Currently, only the T860 and
> T760 Midgard GPUs have been tested.

> +static int panfrost_ioctl_get_bo_offset(struct drm_device *dev, void *data,
> +			    struct drm_file *file_priv)
> +{
> +	struct drm_panfrost_get_bo_offset *args = data;
> +	struct drm_gem_object *gem_obj;
> +	struct panfrost_gem_object *bo;
> +

Missing check for pad == 0.  With that fixed,

Reviewed-by: Eric Anholt <eric@anholt.net>

> +	gem_obj = drm_gem_object_lookup(file_priv, args->handle);
> +	if (!gem_obj) {
> +		DRM_DEBUG("Failed to look up GEM BO %d\n", args->handle);
> +		return -ENOENT;
> +	}
> +	bo = to_panfrost_bo(gem_obj);
> +
> +	args->offset = bo->node.start << PAGE_SHIFT;
> +
> +	drm_gem_object_put_unlocked(gem_obj);
> +	return 0;
> +}


> +static void panfrost_job_timedout(struct drm_sched_job *sched_job)
> +{
> +	struct panfrost_job *job = to_panfrost_job(sched_job);
> +	struct panfrost_device *pfdev = job->pfdev;
> +	int js = panfrost_job_get_slot(job);
> +	int i;
> +
> +	/*
> +	 * If the GPU managed to complete this jobs fence, the timeout is
> +	 * spurious. Bail out.
> +	 */
> +	if (dma_fence_is_signaled(job->done_fence))
> +		return;

Note: The scheduler calls cancel_delayed_work_sync() in
s_job->finish_work, so this is just reducing the race for the job
successfully completing near the timeout but finish_work being in the
workqueue across the timeout boundary.  I dropped it from v3d.

--=-=-=
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAlytDeEACgkQtdYpNtH8
nuh3JRAAtq3pl35Q1d7+I6lbFCjyLdVHXmNShOpudBRqlPvTkKycYdhWyeUHIyRj
GVDmnaFUdQURCYWr64mqcsqu6U/N/pIAyttRVqxb4qp67Z403BZOC+BlJ1EAJgNv
vzJHUZfSZtyRrF4pez1uyY44pKbAY9ERxvYORhN4QnHfnG9h973fsHaeAD0Yjcv7
nm8yGthWiH3Tn7rqIwRmMVWJf0nhs24QLpjHZSTe7Tsbpb6jcN/nnzkPEgd5XxAm
aP75lVGfMnzISrvC1fJV4hwalMlJaXQFEP2cknDdB7mLcwh7ImrDwxCaMQwh/eB3
dpMBpbr73CM5lvgbacG30PLrr3SNlVkS5XerqqmSxWbFnvQnBbeWiLUaCPzRHerw
SMAMZq2ML2nfn2eGpipQ7EIRmhZHM6lTJsViEPHIiGrdKUUeR4L+guKMlqGjaJ0F
n3Vsw+neyW62lIbD82L5vblZB0KMB50In7Mje0wrF9qncCT6qnYNvGa7NPeJcOJ9
fhLsa2rbX9EoeGK4tLHQrdzcfNVz/FzmNhhGom1ifw+fa4llhdLAxUstn2mpP6sl
PSCwgdmahlQAa1PciacTQHrVWw2lQDnPzcvn9+MApG2mbIqPZF9khCrbbRgmIXI5
8dnMFLTWue7UBLXMEVerxahEVe5Srnj3I34lk5eSa9M0ohcccvA=
=7SIf
-----END PGP SIGNATURE-----
--=-=-=--