On Wed, 2014-07-02 at 07:52 +0800, Zhao, Yakui wrote:
> On Tue, 2014-07-01 at 09:26 -0600, Vivi, Rodrigo wrote:
> > It seems the flexibility on rings is more wanted and needed than I imagined.
> > Please ignore this patch here...
> >
> > I liked both execution flag or debugfs, but exec flag would cover this
> > case of different applications using different command streamers.
> >
> > With flags Would it be something like:
> > Execution without flag = ping-pong
> > Flag BSD1 use only VCS1
> > Flag BSD2 use only VCS2
> >
>
> IMO the execution flag looks reasonable. It can cover the flexibility of
> different applications. In such case it can determine which ring is used
> to dispatch command at runtime.
>
I prefer the execution flag too.
Thanks
Haihao
> > Haihao, what do you think?
> >
> > With debugfs would be something like i195_dual_bsd_ring file with 3
> > options: all bsd1 bsd2
> >
> > Thanks,
> > Rodrigo.
> >
> > -----Original Message-----
> > From: Zhao, Yakui
> > Sent: Monday, June 30, 2014 6:37 PM
> > To: Vivi, Rodrigo
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915: Introduce dual_bsd_ring
> > parameter.
> >
> > On Mon, 2014-06-30 at 10:51 -0600, Rodrigo Vivi wrote:
> > > On Broadwell GT3 we have 2 Video Command Streamers (VCS), but
> > > userspace has no control when using VCS1 or VCS2. So we cannot test,
> > > validate or debug specific changes or workaround that might affect
> > > only one or another ring. So this patch introduces a mechanism to
> > > avoid the ping-pong selection and use one specific ring given at boot
> > > time.
> >
> > If it is mainly used for the test/validation, can we add one override
> > flag so that the user-space app can explicitly declare which BSD ring is
> > used to dispatch the corresponding BSD commands? In such case it will force
> > to dispatch the corresponding commands on the ring passed by
> > user-application.
> >
> > At the same time this patch is not helpful under the following scenario.
> > For example: One application hopes to use the BSD Ring 0 while another
> > application hopes to use the BSD ring 1.
> >
> > >
> > > Signed-off-by: Rodrigo Vivi <rodrigo.v...@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 34
> > > ++++++++++++++++++------------
> > > drivers/gpu/drm/i915/i915_params.c | 6 ++++++
> > > 3 files changed, 27 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h index 8cea596..7b6614f 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2069,6 +2069,7 @@ struct i915_params {
> > > int panel_ignore_lid;
> > > unsigned int powersave;
> > > int semaphores;
> > > + int dual_bsd_ring;
> > > unsigned int lvds_downclock;
> > > int lvds_channel_mode;
> > > int panel_use_ssc;
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > index d815ef5..09f350e 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > @@ -1035,26 +1035,32 @@ static int gen8_dispatch_bsd_ring(struct
> > > drm_device *dev, {
> > > struct drm_i915_private *dev_priv = dev->dev_private;
> > > struct drm_i915_file_private *file_priv = file->driver_priv;
> > > + int ring_id;
> > > + int dual = i915.dual_bsd_ring;
> > >
> > > /* Check whether the file_priv is using one ring */
> > > if (file_priv->bsd_ring)
> > > return file_priv->bsd_ring->id;
> > > - else {
> > > - /* If no, use the ping-pong mechanism to select one ring */
> > > - int ring_id;
> > >
> > > - mutex_lock(&dev->struct_mutex);
> > > - if (dev_priv->mm.bsd_ring_dispatch_index == 0) {
> > > - ring_id = VCS;
> > > - dev_priv->mm.bsd_ring_dispatch_index = 1;
> > > - } else {
> > > - ring_id = VCS2;
> > > - dev_priv->mm.bsd_ring_dispatch_index = 0;
> > > - }
> > > - file_priv->bsd_ring = &dev_priv->ring[ring_id];
> > > - mutex_unlock(&dev->struct_mutex);
> > > - return ring_id;
> > > + /* If no, use the parameter defined or ping-pong mechanism
> > > + * to select one ring */
> > > + mutex_lock(&dev->struct_mutex);
> > > +
> > > + if (dual == 1 || (dual != 2 &&
> > > + dev_priv->mm.bsd_ring_dispatch_index == 0)) {
> > > + ring_id = VCS;
> > > + dev_priv->mm.bsd_ring_dispatch_index = 1;
> > > + } else {
> > > + ring_id = VCS2;
> > > + dev_priv->mm.bsd_ring_dispatch_index = 0;
> > > }
> > > +
> > > + file_priv->bsd_ring = &dev_priv->ring[ring_id];
> > > + mutex_unlock(&dev->struct_mutex);
> > > +
> > > + WARN(dual, "Forcibly trying to use only one bsd ring. Using: %s\n",
> > > + file_priv->bsd_ring->name);
> > > + return ring_id;
> > > }
> > >
> > > static struct drm_i915_gem_object *
> > > diff --git a/drivers/gpu/drm/i915/i915_params.c
> > > b/drivers/gpu/drm/i915/i915_params.c
> > > index 8145729..d4871c8 100644
> > > --- a/drivers/gpu/drm/i915/i915_params.c
> > > +++ b/drivers/gpu/drm/i915/i915_params.c
> > > @@ -29,6 +29,7 @@ struct i915_params i915 __read_mostly = {
> > > .panel_ignore_lid = 1,
> > > .powersave = 1,
> > > .semaphores = -1,
> > > + .dual_bsd_ring = 0,
> > > .lvds_downclock = 0,
> > > .lvds_channel_mode = 0,
> > > .panel_use_ssc = -1,
> > > @@ -70,6 +71,11 @@ MODULE_PARM_DESC(semaphores,
> > > "Use semaphores for inter-ring sync "
> > > "(default: -1 (use per-chip defaults))");
> > >
> > > +module_param_named(dual_bsd_ring, i915.dual_bsd_ring, int, 0600);
> > > +MODULE_PARM_DESC(dual_bsd_ring,
> > > + "Specify bds rings for VCS when there are multiple VCSs available."
> > > + "(0=All available bsd rings [default], 1=only VCS1, 2=only VCS2)");
> > > +
> > > module_param_named(enable_rc6, i915.enable_rc6, int, 0400);
> > > MODULE_PARM_DESC(enable_rc6,
> > > "Enable power-saving render C-state 6. "
> >
> >
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
