Re: [PATCH 1/5] drm/i915: Bail out early on gen6_signal if no semaphores

Ville Syrjälä
2014-05-15T18:39:12Z

On Thu, May 15, 2014 at 08:58:07PM +0300, Mika Kuoppala wrote:
> If we dont have semaphores enabled, we allocate 4
> dwords for signalling. But end up emitting more regardless.
>
> Fix this by bailing out early if semaphores are not enabled.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78274
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78283
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 3974e82..93b4062 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -684,6 +684,8 @@ static int gen6_signal(struct intel_ring_buffer *signaller,
> #define MBOX_UPDATE_DWORDS 4
> if (i915_semaphore_is_enabled(dev))
> num_dwords += ((I915_NUM_RINGS-1) * MBOX_UPDATE_DWORDS);
> + else
> + return intel_ring_begin(signaller, num_dwords);

I guess that's the minimal band aid we can do. Or just add another
i915_semaphore_is_enabled() check with return after the
intel_ring_begin() we already have.

Ben's gen8 semaphore series makes the .signal function pointer optional,
so the check will be in the caller. But extracting just that minimal
part from that patch would be more work.

I don't see any real problem in going with this minimal fix for now, so:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>
> ret = intel_ring_begin(signaller, num_dwords);
> if (ret)
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrjälä
Intel OTC