All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Price <steven.price@arm.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: "Tomeu Vizoso" <tomeu.vizoso@collabora.com>,
	"Daniel Vetter" <daniel.vetter@ffwll.ch>,
	"Intel Graphics Development" <intel-gfx@lists.freedesktop.org>,
	"DRI Development" <dri-devel@lists.freedesktop.org>,
	linaro-mm-sig@lists.linaro.org,
	"Luben Tuikov" <luben.tuikov@amd.com>,
	"Alyssa Rosenzweig" <alyssa.rosenzweig@collabora.com>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Daniel Vetter" <daniel.vetter@intel.com>,
	"Lee Jones" <lee.jones@linaro.org>,
	"Christian König" <christian.koenig@amd.com>,
	linux-media@vger.kernel.org
Subject: Re: [PATCH 03/11] drm/panfrost: Use xarray and helpers for depedency tracking
Date: Thu, 3 Jun 2021 08:48:04 +0100	[thread overview]
Message-ID: <e9081dec-0225-a24d-261c-3ab750b29614@arm.com> (raw)
In-Reply-To: <YLfTGbb52KtbPk7q@phenom.ffwll.local>

On 02/06/2021 19:51, Daniel Vetter wrote:
> On Wed, Jun 02, 2021 at 03:06:50PM +0100, Steven Price wrote:
>> On 21/05/2021 10:09, Daniel Vetter wrote:
[...]
>>> +	if (!xa_empty(&job->deps))
>>> +		return xa_erase(&job->deps, job->last_dep++);
>>
>> Rather than tracking last_dep separately this could be written using
>> xa_find():
>>
>>     if (xa_find(&job->deps, &i, ULONG_MAX, XA_PRESENT))
>> 	return xa_erase(&job->deps, &i);
> 
> I copypasted this from other drivers, imo consistency is better than
> looking pretty. I think eventually we should stuff this as optional
> helpers into drm/scheduler.
> 
> Also yours walks the xa twice.

Agreed this isn't as efficient (I was somewhat disappointed xarray
doesn't expose a "get and remove the first element" API). Moving this
into drm/scheduler seems like the right long term solution - so matching
other drivers first is a good move.

Thanks,

Steve

WARNING: multiple messages have this Message-ID (diff)
From: Steven Price <steven.price@arm.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: "Daniel Vetter" <daniel.vetter@ffwll.ch>,
	"DRI Development" <dri-devel@lists.freedesktop.org>,
	"Intel Graphics Development" <intel-gfx@lists.freedesktop.org>,
	"Christian König" <christian.koenig@amd.com>,
	"Luben Tuikov" <luben.tuikov@amd.com>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Lee Jones" <lee.jones@linaro.org>,
	"Rob Herring" <robh@kernel.org>,
	"Tomeu Vizoso" <tomeu.vizoso@collabora.com>,
	"Alyssa Rosenzweig" <alyssa.rosenzweig@collabora.com>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org,
	"Daniel Vetter" <daniel.vetter@intel.com>
Subject: Re: [PATCH 03/11] drm/panfrost: Use xarray and helpers for depedency tracking
Date: Thu, 3 Jun 2021 08:48:04 +0100	[thread overview]
Message-ID: <e9081dec-0225-a24d-261c-3ab750b29614@arm.com> (raw)
In-Reply-To: <YLfTGbb52KtbPk7q@phenom.ffwll.local>

On 02/06/2021 19:51, Daniel Vetter wrote:
> On Wed, Jun 02, 2021 at 03:06:50PM +0100, Steven Price wrote:
>> On 21/05/2021 10:09, Daniel Vetter wrote:
[...]
>>> +	if (!xa_empty(&job->deps))
>>> +		return xa_erase(&job->deps, job->last_dep++);
>>
>> Rather than tracking last_dep separately this could be written using
>> xa_find():
>>
>>     if (xa_find(&job->deps, &i, ULONG_MAX, XA_PRESENT))
>> 	return xa_erase(&job->deps, &i);
> 
> I copypasted this from other drivers, imo consistency is better than
> looking pretty. I think eventually we should stuff this as optional
> helpers into drm/scheduler.
> 
> Also yours walks the xa twice.

Agreed this isn't as efficient (I was somewhat disappointed xarray
doesn't expose a "get and remove the first element" API). Moving this
into drm/scheduler seems like the right long term solution - so matching
other drivers first is a good move.

Thanks,

Steve

WARNING: multiple messages have this Message-ID (diff)
From: Steven Price <steven.price@arm.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: "Rob Herring" <robh@kernel.org>,
	"Tomeu Vizoso" <tomeu.vizoso@collabora.com>,
	"Daniel Vetter" <daniel.vetter@ffwll.ch>,
	"Intel Graphics Development" <intel-gfx@lists.freedesktop.org>,
	"DRI Development" <dri-devel@lists.freedesktop.org>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	linaro-mm-sig@lists.linaro.org,
	"Luben Tuikov" <luben.tuikov@amd.com>,
	"Alyssa Rosenzweig" <alyssa.rosenzweig@collabora.com>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Daniel Vetter" <daniel.vetter@intel.com>,
	"Lee Jones" <lee.jones@linaro.org>,
	"Christian König" <christian.koenig@amd.com>,
	linux-media@vger.kernel.org
Subject: Re: [Intel-gfx] [PATCH 03/11] drm/panfrost: Use xarray and helpers for depedency tracking
Date: Thu, 3 Jun 2021 08:48:04 +0100	[thread overview]
Message-ID: <e9081dec-0225-a24d-261c-3ab750b29614@arm.com> (raw)
In-Reply-To: <YLfTGbb52KtbPk7q@phenom.ffwll.local>

On 02/06/2021 19:51, Daniel Vetter wrote:
> On Wed, Jun 02, 2021 at 03:06:50PM +0100, Steven Price wrote:
>> On 21/05/2021 10:09, Daniel Vetter wrote:
[...]
>>> +	if (!xa_empty(&job->deps))
>>> +		return xa_erase(&job->deps, job->last_dep++);
>>
>> Rather than tracking last_dep separately this could be written using
>> xa_find():
>>
>>     if (xa_find(&job->deps, &i, ULONG_MAX, XA_PRESENT))
>> 	return xa_erase(&job->deps, &i);
> 
> I copypasted this from other drivers, imo consistency is better than
> looking pretty. I think eventually we should stuff this as optional
> helpers into drm/scheduler.
> 
> Also yours walks the xa twice.

Agreed this isn't as efficient (I was somewhat disappointed xarray
doesn't expose a "get and remove the first element" API). Moving this
into drm/scheduler seems like the right long term solution - so matching
other drivers first is a good move.

Thanks,

Steve
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2021-06-03  7:48 UTC|newest]

Thread overview: 175+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-21  9:09 [Intel-gfx] [PATCH 01/11] drm/amdgpu: Comply with implicit fencing rules Daniel Vetter
2021-05-21  9:09 ` Daniel Vetter
2021-05-21  9:09 ` [Intel-gfx] [PATCH 02/11] drm/panfrost: Remove sched_lock Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-21  9:32   ` Lucas Stach
2021-05-21  9:32     ` [Intel-gfx] " Lucas Stach
2021-05-21 14:49     ` Daniel Vetter
2021-05-21 14:49       ` Daniel Vetter
2021-05-21  9:09 ` [PATCH 03/11] drm/panfrost: Use xarray and helpers for depedency tracking Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-21  9:09   ` [Intel-gfx] " Daniel Vetter
2021-06-02 14:06   ` Steven Price
2021-06-02 14:06     ` Steven Price
2021-06-02 14:06     ` Steven Price
2021-06-02 18:51     ` Daniel Vetter
2021-06-02 18:51       ` Daniel Vetter
2021-06-02 18:51       ` [Intel-gfx] " Daniel Vetter
2021-06-03  7:48       ` Steven Price [this message]
2021-06-03  7:48         ` Steven Price
2021-06-03  7:48         ` Steven Price
2021-05-21  9:09 ` [PATCH 04/11] drm/panfrost: Fix implicit sync Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-21  9:09   ` [Intel-gfx] " Daniel Vetter
2021-05-21 12:22   ` Daniel Stone
2021-05-21 12:22     ` Daniel Stone
2021-05-21 12:22     ` [Intel-gfx] " Daniel Stone
2021-05-21 12:28     ` [Intel-gfx] [Linaro-mm-sig] " Christian König
2021-05-21 12:28       ` Christian König
2021-05-21 12:54       ` [Intel-gfx] " Daniel Stone
2021-05-21 12:54         ` Daniel Stone
2021-05-21 12:54         ` Daniel Stone
2021-05-21 13:09         ` Christian König
2021-05-21 13:09           ` Christian König
2021-05-21 13:09           ` [Intel-gfx] " Christian König
2021-05-21 13:23           ` Daniel Stone
2021-05-21 13:23             ` Daniel Stone
2021-05-21 13:23             ` Daniel Stone
2021-05-21  9:09 ` [Intel-gfx] [PATCH 05/11] drm/atomic-helper: make drm_gem_plane_helper_prepare_fb the default Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-21  9:09 ` [Intel-gfx] [PATCH 06/11] drm/<driver>: drm_gem_plane_helper_prepare_fb is now " Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-21  9:38   ` Lucas Stach
2021-05-21  9:38     ` Lucas Stach
2021-05-21  9:38     ` Lucas Stach
2021-05-21  9:38     ` Lucas Stach
2021-05-21  9:38     ` Lucas Stach
2021-05-21  9:38     ` Lucas Stach
2021-05-21  9:38     ` [Intel-gfx] " Lucas Stach
2021-05-21 12:20   ` Heiko Stübner
2021-05-21 12:20     ` Heiko Stübner
2021-05-21 12:20     ` [Intel-gfx] " Heiko Stübner
2021-05-21 12:20     ` Heiko Stübner
2021-05-21 12:20     ` Heiko Stübner
2021-05-21 12:20     ` Heiko Stübner
2021-05-21 12:20     ` Heiko Stübner
2021-05-21 12:22   ` Paul Cercueil
2021-05-21 12:22     ` [Intel-gfx] " Paul Cercueil
2021-05-21 12:22     ` Paul Cercueil
2021-05-21 12:22     ` Paul Cercueil
2021-05-21 12:22     ` Paul Cercueil
2021-05-21 12:22     ` Paul Cercueil
2021-05-21 12:22     ` Paul Cercueil
2021-05-21 15:53   ` Jernej Škrabec
2021-05-21 15:53     ` Jernej Škrabec
2021-05-21 15:53     ` [Intel-gfx] " Jernej Škrabec
2021-05-21 15:53     ` Jernej Škrabec
2021-05-21 15:53     ` Jernej Škrabec
2021-05-21 15:53     ` Jernej Škrabec
2021-05-21 15:53     ` Jernej Škrabec
2021-05-21 23:18   ` Chun-Kuang Hu
2021-05-21 23:18     ` Chun-Kuang Hu
2021-05-21 23:18     ` Chun-Kuang Hu
2021-05-21 23:18     ` Chun-Kuang Hu
2021-05-21 23:18     ` Chun-Kuang Hu
2021-05-21 23:18     ` [Intel-gfx] " Chun-Kuang Hu
2021-05-21 23:18     ` Chun-Kuang Hu
2021-05-23 12:17   ` Martin Blumenstingl
2021-05-23 12:17     ` [Intel-gfx] " Martin Blumenstingl
2021-05-23 12:17     ` Martin Blumenstingl
2021-05-23 12:17     ` Martin Blumenstingl
2021-05-23 12:17     ` Martin Blumenstingl
2021-05-23 12:17     ` Martin Blumenstingl
2021-05-23 12:17     ` Martin Blumenstingl
2021-05-24  7:54   ` [Intel-gfx] " Tomi Valkeinen
2021-05-24  7:54     ` Tomi Valkeinen
2021-05-24  7:54     ` Tomi Valkeinen
2021-05-24  7:54     ` Tomi Valkeinen
2021-05-24  7:54     ` Tomi Valkeinen
2021-05-24  7:54     ` Tomi Valkeinen
2021-05-24  7:54     ` Tomi Valkeinen
2021-05-28  9:55   ` [Intel-gfx] " Philippe CORNU
2021-05-28  9:55     ` Philippe CORNU
2021-05-28  9:55     ` Philippe CORNU
2021-05-28  9:55     ` Philippe CORNU
2021-05-28  9:55     ` Philippe CORNU
2021-05-28  9:55     ` Philippe CORNU
2021-05-28  9:55     ` Philippe CORNU
2021-05-21  9:09 ` [Intel-gfx] [PATCH 07/11] drm/armada: Remove prepare/cleanup_fb hooks Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-21  9:09 ` [Intel-gfx] [PATCH 08/11] drm/vram-helpers: Create DRM_GEM_VRAM_PLANE_HELPER_FUNCS Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-21  9:33   ` tiantao (H)
2021-05-21  9:33     ` [Intel-gfx] " tiantao (H)
2021-05-21  9:09 ` [Intel-gfx] [PATCH 09/11] drm/omap: Follow implicit fencing in prepare_fb Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-24  7:53   ` [Intel-gfx] " Tomi Valkeinen
2021-05-24  7:53     ` Tomi Valkeinen
2021-05-21  9:09 ` [Intel-gfx] [PATCH 10/11] drm/simple-helper: drm_gem_simple_display_pipe_prepare_fb as default Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-25 17:48   ` Noralf Trønnes
2021-05-25 17:48     ` [Intel-gfx] " Noralf Trønnes
2021-05-25 17:53     ` Daniel Vetter
2021-05-25 17:53       ` Daniel Vetter
2021-05-21  9:09 ` [Intel-gfx] [PATCH 11/11] drm/tiny: drm_gem_simple_display_pipe_prepare_fb is the default Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-21  9:09   ` Daniel Vetter
2021-05-21 13:41   ` David Lechner
2021-05-21 13:41     ` David Lechner
2021-05-21 13:41     ` [Intel-gfx] " David Lechner
2021-05-21 13:41     ` David Lechner
2021-05-21 14:09   ` Noralf Trønnes
2021-05-21 14:09     ` [Intel-gfx] " Noralf Trønnes
2021-05-21 14:09     ` Noralf Trønnes
2021-05-21 14:09     ` Noralf Trønnes
2021-05-25 16:05     ` Daniel Vetter
2021-05-25 16:05       ` Daniel Vetter
2021-05-25 16:05       ` Daniel Vetter
2021-05-25 16:05       ` [Intel-gfx] " Daniel Vetter
2021-05-21 14:13   ` Oleksandr Andrushchenko
2021-05-21 14:13     ` [Intel-gfx] " Oleksandr Andrushchenko
2021-05-21 14:13     ` Oleksandr Andrushchenko
2021-05-21 14:13     ` Oleksandr Andrushchenko
2021-05-28  0:38   ` Linus Walleij
2021-05-28  0:38     ` Linus Walleij
2021-05-28  0:38     ` Linus Walleij
2021-05-28  0:38     ` [Intel-gfx] " Linus Walleij
2021-05-21  9:18 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/11] drm/amdgpu: Comply with implicit fencing rules Patchwork
2021-05-21  9:20 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-05-21  9:45 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-05-21  9:46 ` [Intel-gfx] [PATCH 01/11] " Bas Nieuwenhuizen
2021-05-21  9:46   ` Bas Nieuwenhuizen
2021-05-21 14:37   ` [Intel-gfx] " Daniel Vetter
2021-05-21 14:37     ` Daniel Vetter
2021-05-21 15:00     ` [Intel-gfx] " Bas Nieuwenhuizen
2021-05-21 15:00       ` Bas Nieuwenhuizen
2021-05-21 15:16       ` [Intel-gfx] " Daniel Vetter
2021-05-21 15:16         ` Daniel Vetter
2021-05-21 18:08         ` [Mesa-dev] " Christian König
2021-05-21 18:08           ` [Intel-gfx] " Christian König
2021-05-21 18:31           ` Daniel Vetter
2021-05-21 18:31             ` Daniel Vetter
2021-05-22  8:30             ` [Intel-gfx] " Christian König
2021-05-22  8:30               ` Christian König
2021-05-25 13:05               ` [Intel-gfx] " Daniel Vetter
2021-05-25 13:05                 ` Daniel Vetter
2021-05-25 15:05                 ` Christian König
2021-05-25 15:23                   ` [Intel-gfx] " Daniel Vetter
2021-05-25 15:23                     ` Daniel Vetter
2021-05-26 13:32                     ` Christian König
2021-05-26 13:32                       ` [Intel-gfx] " Christian König
2021-05-26 13:51                       ` Daniel Vetter
2021-05-26 13:51                         ` Daniel Vetter
2021-05-21 11:22 ` [Intel-gfx] " Christian König
2021-05-21 11:22   ` Christian König
2021-05-21 14:58 ` [Intel-gfx] [Mesa-dev] " Rob Clark
2021-05-21 14:58   ` Rob Clark
2021-05-21 14:58   ` [Intel-gfx] " Daniel Vetter
2021-05-21 14:58     ` Daniel Vetter
2021-05-23  5:00 ` [Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [01/11] " Patchwork

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=e9081dec-0225-a24d-261c-3ab750b29614@arm.com \
    --to=steven.price@arm.com \
    --cc=alexander.deucher@amd.com \
    --cc=alyssa.rosenzweig@collabora.com \
    --cc=christian.koenig@amd.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=lee.jones@linaro.org \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-media@vger.kernel.org \
    --cc=luben.tuikov@amd.com \
    --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.