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.
> 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
