On Fri, Jun 27, 2014 at 03:21:20PM -0700, Rodrigo Vivi wrote:
> Reviewed-by: Rodrigo Vivi <[1]rodrigo.vivi@xxxxxxxxx>
>
> On Fri, Jun 27, 2014 at 3:09 PM, Ben Widawsky
> <[2]benjamin.widawsky@xxxxxxxxx> wrote:
>
> There are some cases in the code where we need to know how many rings to
> iterate over, but cannot use for_each_ring(). These are always error
> cases
> which happen either before ring setup, or after ring teardown (or
> reset).
>
> Note, a NUM_RINGS issue exists in semaphores, but this is fixed by the
> remaining semaphore patches which Rodrigo will resubmit shortly. I'd
> rather see those patches for fixing the problem than fix it here.
>
> I found this initially for the BSD2 case where on the same platform we
> can have differing rings. AFAICT however this effects many platforms.
>
> I'd CC stable on this, except I think all the issues have been around
> for multiple releases without bug reports.
>
> Compile tested only for now.
>
> Signed-off-by: Ben Widawsky <[3]ben@xxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 6 +++---
> drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
> drivers/gpu/drm/i915/intel_ringbuffer.h | 2 ++
> 3 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index b9bac25..0c044a9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -403,7 +403,7 @@ void i915_gem_context_reset(struct drm_device *dev)
>
> /* Prevent the hardware from restoring the last context (which
> hung) on
> * the next switch */
> - for (i = 0; i < I915_NUM_RINGS; i++) {
> + for (i = 0; i < I915_ACTIVE_RINGS(dev); i++) {
> struct intel_engine_cs *ring = &dev_priv->ring[i];
> struct intel_context *dctx = ring->default_context;
>
> @@ -456,7 +456,7 @@ int i915_gem_context_init(struct drm_device *dev)
> }
>
> /* NB: RCS will hold a ref for all rings */
> - for (i = 0; i < I915_NUM_RINGS; i++)
> + for (i = 0; i < I915_ACTIVE_RINGS(dev); i++)
> dev_priv->ring[i].default_context = ctx;
>
> DRM_DEBUG_DRIVER("%s context support initialized\n",
> dev_priv->hw_context_size ? "HW" : "fake");
> @@ -493,7 +493,7 @@ void i915_gem_context_fini(struct drm_device *dev)
> i915_gem_object_ggtt_unpin(dctx->obj);
> }
>
> - for (i = 0; i < I915_NUM_RINGS; i++) {
> + for (i = 0; i < I915_ACTIVE_RINGS(dev); i++) {
> struct intel_engine_cs *ring = &dev_priv->ring[i];
>
> if (ring->last_context)
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
> b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 86362de..6e5250d 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -848,7 +848,7 @@ static uint32_t i915_error_generate_code(struct
> drm_i915_private *dev_priv,
> * synchronization commands which almost always appear in the
> case
> * strictly a client bug. Use instdone to differentiate those
> some.
> */
> - for (i = 0; i < I915_NUM_RINGS; i++) {
> + for (i = 0; i < I915_ACTIVE_RINGS(dev_priv->dev); i++) {
> if (error->ring[i].hangcheck_action == HANGCHECK_HUNG) {
> if (ring_id)
> *ring_id = i;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
> b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index e72017b..67e2919 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -90,6 +90,8 @@ struct intel_engine_cs {
> } id;
> #define I915_NUM_RINGS 5
> #define LAST_USER_RING (VECS + 1)
> +#define I915_ACTIVE_RINGS(dev) hweight8(INTEL_INFO(dev)->ring_mask)
What does the popcount of the mask have to do with the validity of the
arrays being iterated over in this patch?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
