From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zhao Yakui Subject: Re: [PATCH 0/5] drm/i915: Add the support of dual BSD rings on BDW GT3 Date: Thu, 10 Apr 2014 11:28:46 +0800 Message-ID: <1397100526.2039.137.camel@genxdev-ykzhao.sh.intel.com> References: <1397008796-3595-1-git-send-email-yakui.zhao@intel.com> <20140409144558.GE9262@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 9CCC76E11D for ; Wed, 9 Apr 2014 20:30:45 -0700 (PDT) In-Reply-To: <20140409144558.GE9262@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org On Wed, 2014-04-09 at 08:45 -0600, Daniel Vetter wrote: > On Wed, Apr 09, 2014 at 09:59:51AM +0800, Zhao Yakui wrote: > > > > This is the patch set that tries to add the support of dual BSD rings on BDW > > GT3. Based on hardware spec, the BDW GT3 has two independent BSD rings, which > > can be used to process the video commands. To be simpler, it is transparent > > to user-space driver/middleware. In such case the kernel driver will decide > > which ring is to dispatch the BSD video command. > > > > As every BSD ring is powerful, it is enough to dispatch the BSD video command > > based on the drm fd. In such case the different BSD ring is used for video playing > > back and encoding. At the same time the coarse dispatch mechanism can help to avoid > > the object synchronization between the BSD rings. > > Ok, I've quickly read through it all and commented on a few things. Imo > the last patch should be massively simplified, at least for the first > round. Other things look small. > Hi, Daniel Thanks for your review. > What's still missing are testcases, and I have two things in mind here: > - Exercise the 2nd ring dispatch and sync a bit. Since the 2nd bsd ring is > hidden within the kernel I think the right approach would be to open a > few drm fds (10 or so) and then randomly use them with a dummy reloc. We > have two testcases which can be used as blueprints that need > adjustement: > > - gem_ring_sync_loop: Probably easiest to copy it to a new file as > gem_multi_bsd_sync_loop. This test exercises semaphores. > - gem_dummy_reloc_loop, subtest mixed: Almost the same as the above, but > the sync is done _inside_ the loop and hence this exercises gpu/cpu > sync. We need both tests adjusted, for for this we need a new > multi-bsd test. Agree with your concerns. I will try to add the gem_multi_bsd_sync_loop/dummy_reloc_loop test case so that it can test the sync with multi-BSD. BTW: How about if I directly add multiple fds in gem_ring_sync_loop test case and then test the sync among the different rings? In such case the user-application doesn't need to know the existence of multi-BSD rings. > > - New testcase to fully test main execbuffer flags. This is simply > something that's we don't yet have. The next guy to touch execbuf code > needs to add it, and it looks like that's you ;-) I've done a JIRA task > for the resource streamer work, but I think the resource streamer wont > be merged anytime soon. So I'll reassign to you. Jira task is VIZ-3129. For the new testcase of execbuffer flag: Do you have any idea about which kind of exec flag needs to be checked? Do you have any idea about the expected failure/successful behavour for the flags? For example: I915_EXEC_PINNED : If one object is not pinned and submitted, what behavour is expected? Fail or wrong? Thanks. Yakui > > Thanks, Daniel > > > > > > > Zhao Yakui (5): > > drm/i915: Split the BDW device definition to prepare for dual BSD > > rings on BDW GT3 > > drm/i915: Initialize the second BSD ring on BDW GT3 machine > > drm/i915: Handle the irq interrupt for the second BSD ring > > drm/i915: Add the VCS2 switch in Intel_ring_setup_status_page for Gen7 to > > remove the switch check warning > > drm/i915: Use the coarse mechanism based on drm fd to dispatch the BSD command > > on BDW GT3 > > > > drivers/gpu/drm/i915/i915_dma.c | 14 ++++++ > > drivers/gpu/drm/i915/i915_drv.c | 24 ++++++++- > > drivers/gpu/drm/i915/i915_drv.h | 5 ++ > > drivers/gpu/drm/i915/i915_gem.c | 9 +++- > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 73 +++++++++++++++++++++++++++- > > drivers/gpu/drm/i915/i915_gpu_error.c | 1 + > > drivers/gpu/drm/i915/i915_irq.c | 5 +- > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > drivers/gpu/drm/i915/intel_ringbuffer.c | 57 ++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_ringbuffer.h | 6 ++- > > include/drm/i915_pciids.h | 10 ++-- > > 11 files changed, 197 insertions(+), 8 deletions(-) > > > > -- > > 1.7.10.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >