[Intel-gfx] [PATCH 0/5] drm/i915: Add the support of dual BSD rings on BDW GT3

[Zhao Yakui]

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 at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>