All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Gordon <david.s.gordon@intel.com>
To: Daniel Vetter <daniel@ffwll.ch>,
	"Gong, Zhipeng" <zhipeng.gong@intel.com>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>,
	"Vivi, Rodrigo" <rodrigo.vivi@intel.com>
Subject: Re: [PATCH 1/7] drm/i915: Specify bsd rings through exec flag
Date: Wed, 10 Dec 2014 15:55:18 +0000	[thread overview]
Message-ID: <54886CE6.1060401@intel.com> (raw)
In-Reply-To: <20141210091143.GZ27182@phenom.ffwll.local>

On 10/12/14 09:11, Daniel Vetter wrote:
> On Wed, Dec 10, 2014 at 02:18:15AM +0000, Gong, Zhipeng wrote:
>> On Tue, 2014-12-09 at 10:46 +0100, Daniel Vetter wrote:
>>> On Mon, Dec 08, 2014 at 01:55:56PM -0800, Rodrigo Vivi wrote:

[snip]

>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>>> index e1ed85a..d9081ec 100644
>>>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>>> @@ -1273,8 +1273,23 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>>>>       else if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_BSD) {
>>>>               if (HAS_BSD2(dev)) {
>>>>                       int ring_id;
>>>> -                     ring_id = gen8_dispatch_bsd_ring(dev, file);
>>>> -                     ring = &dev_priv->ring[ring_id];
>>>> +
>>>> +                     switch (args->flags & I915_EXEC_BSD_MASK) {
>>>> +                     case I915_EXEC_BSD_DEFAULT:
>>>> +                             ring_id = gen8_dispatch_bsd_ring(dev, file);
>>>> +                             ring = &dev_priv->ring[ring_id];
>>>> +                             break;
>>>> +                     case I915_EXEC_BSD_RING1:
>>>> +                             ring = &dev_priv->ring[VCS];
>>>
>>> Do we have any use-case for selecting ring1 specifically? I've thought
>>> it's only ring2 that is special?
>> The HEVC GPU commands should be dispatched to BSD RING 1 instead of BSD
>> RING2 as the two rings are asymmetrical. 
>> For the H264 decoding/encoding either ring is OK.
> 
> Well then same arguments applies with ring2 since only ring1 is special?
> It's just to minimize abi and reduce the amount of rope we hand to
> userspace.

Anyone who knows to use any of these flags is taking responsibility for
doing explicit engine allocation, so why not give them all the options
-- if for no other reason, more symmetry is good.

As an examle, there could be a case where userspace knows better than
the kernel how long each batch will take, and can predict an optimal
allocation pattern rather than just flip-flopping. So even when a batch
*can* run on either engine, there might be a reason to pick a specific one.

e.g.	short-1 -> ring 1
	short-2 -> ring 1
	long-1  -> ring 2
	short-3 -> ring 1
	long-2  -> ring 1

because the program knows that the three short batches together will
take less time than the one first long one.

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

  reply	other threads:[~2014-12-10 15:55 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-24 16:29 [PATCH 0/7] drm-intel-collector - update Rodrigo Vivi
2014-11-24 16:29 ` [PATCH 1/7] drm/i915: Specify bsd rings through exec flag Rodrigo Vivi
2014-11-25 13:04   ` Daniel Vetter
2014-12-08 21:55     ` Rodrigo Vivi
2014-12-09  9:46       ` Daniel Vetter
2014-12-10  2:18         ` Gong, Zhipeng
2014-12-10  9:11           ` Daniel Vetter
2014-12-10 15:55             ` Dave Gordon [this message]
2014-12-11  3:50               ` Zhao, Yakui
2014-12-24 21:32                 ` Rodrigo Vivi
2014-11-24 16:29 ` [PATCH 2/7] drm/i915: add I915_PARAM_HAS_BSD2 to i915_getparam Rodrigo Vivi
2014-11-24 16:29 ` [PATCH 3/7] drm/i915: Move the ban period onto the context Rodrigo Vivi
2014-11-24 16:29 ` [PATCH 4/7] drm/i915: Add ioctl to set per-context parameters Rodrigo Vivi
2014-11-24 16:29 ` [PATCH 5/7] drm/i915: Put logical pipe_control emission into a helper Rodrigo Vivi
2014-11-24 16:29 ` [PATCH 6/7] drm/i915: Add WaCsStallBeforeStateCacheInvalidate:bdw, chv to logical ring Rodrigo Vivi
2014-11-24 16:29 ` [PATCH 7/7] drm/i915: Broaden application of set-domain(GTT) Rodrigo Vivi
2014-11-25 12:43   ` [PATCH 7/7] drm/i915: Broaden application of shuang.he
2014-11-25 13:05   ` [PATCH 7/7] drm/i915: Broaden application of set-domain(GTT) Daniel Vetter

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=54886CE6.1060401@intel.com \
    --to=david.s.gordon@intel.com \
    --cc=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=rodrigo.vivi@intel.com \
    --cc=zhipeng.gong@intel.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.