All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Anholt <eric@anholt.net>
To: Rob Herring <robh@kernel.org>, dri-devel@lists.freedesktop.org
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	linux-kernel@vger.kernel.org, David Airlie <airlied@linux.ie>,
	"Marty E . Plummer" <hanetzer@startmail.com>,
	Sean Paul <sean@poorly.run>,
	Alyssa Rosenzweig <alyssa@rosenzweig.io>
Subject: Re: [PATCH v3 3/3] drm/panfrost: Add initial panfrost driver
Date: Tue, 09 Apr 2019 14:25:53 -0700	[thread overview]
Message-ID: <87v9zmdg66.fsf@anholt.net> (raw)
In-Reply-To: <20190409205427.6943-4-robh@kernel.org>


[-- Attachment #1.1: Type: text/plain, Size: 1548 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
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
Date: Tue, 09 Apr 2019 14:25:53 -0700	[thread overview]
Message-ID: <87v9zmdg66.fsf@anholt.net> (raw)
In-Reply-To: <20190409205427.6943-4-robh@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 1548 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

  reply	other threads:[~2019-04-09 21:25 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-09 20:54 [PATCH v3 0/3] Initial Panfrost driver Rob Herring
2019-04-09 20:54 ` Rob Herring
2019-04-09 20:54 ` Rob Herring
     [not found] ` <20190409205427.6943-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2019-04-09 20:54   ` [PATCH v3 1/3] iommu: io-pgtable: Add ARM Mali midgard MMU page table format Rob Herring
2019-04-09 20:54     ` Rob Herring
2019-04-09 20:54     ` Rob Herring
2019-04-09 20:54     ` Rob Herring
2019-04-09 20:54 ` [PATCH v3 2/3] drm: Add a drm_gem_objects_lookup helper Rob Herring
2019-04-09 20:54   ` Rob Herring
2019-04-12 14:52   ` Tomeu Vizoso
2019-04-09 20:54 ` [PATCH v3 3/3] drm/panfrost: Add initial panfrost driver Rob Herring
2019-04-09 21:25   ` Eric Anholt [this message]
2019-04-09 21:25     ` Eric Anholt
2019-04-09 21:27     ` Eric Anholt
2019-04-11 14:12   ` Steven Price

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87v9zmdg66.fsf@anholt.net \
    --to=eric@anholt.net \
    --cc=airlied@linux.ie \
    --cc=alyssa@rosenzweig.io \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hanetzer@startmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxime.ripard@bootlin.com \
    --cc=narmstrong@baylibre.com \
    --cc=robh@kernel.org \
    --cc=sean@poorly.run \
    --cc=tomeu.vizoso@collabora.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.