Re: [RFC 12/44] drm/i915: Disable hardware semaphores when GPU scheduler is enabled

Jesse Barnes
2014-07-02T18:15:48Z

On Thu, 26 Jun 2014 18:24:03 +0100
John.C.Harrison@Intel.com wrote:

> From: John Harrison <John.C.Harrison@Intel.com>
>
> Hardware sempahores require seqno values to be continuously incrementing.
> However, the scheduler's reordering of batch buffers means that the seqno values
> going through the hardware could be out of order. Thus semaphores can not be
> used.
>
> On the other hand, the scheduler superceeds the need for hardware semaphores
> anyway. Having one ring stall waiting for something to complete on another ring
> is inefficient if that ring could be working on some other, independent task.
> This is what the scheduler is meant to do - keep the hardware as busy as
> possible by reordering batch buffers to avoid dependency stalls.
> ---
> drivers/gpu/drm/i915/i915_drv.c | 9 +++++++++
> drivers/gpu/drm/i915/i915_scheduler.c | 9 +++++++++
> drivers/gpu/drm/i915/i915_scheduler.h | 1 +
> drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++++
> 4 files changed, 23 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e2bfdda..748b13a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -33,6 +33,7 @@
> #include "i915_drv.h"
> #include "i915_trace.h"
> #include "intel_drv.h"
> +#include "i915_scheduler.h"
>
> #include <linux/console.h>
> #include <linux/module.h>
> @@ -468,6 +469,14 @@ void intel_detect_pch(struct drm_device *dev)
>
> bool i915_semaphore_is_enabled(struct drm_device *dev)
> {
> + /* Hardware semaphores are not compatible with the scheduler due to the
> + * seqno values being potentially out of order. However, semaphores are
> + * also not required as the scheduler will handle interring dependencies
> + * and try do so in a way that does not cause dead time on the hardware.
> + */
> + if (i915_scheduler_is_enabled(dev))
> + return 0;
> +
> if (INTEL_INFO(dev)->gen < 6)
> return false;
>
> diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
> index e9aa566..d9c1879 100644
> --- a/drivers/gpu/drm/i915/i915_scheduler.c
> +++ b/drivers/gpu/drm/i915/i915_scheduler.c
> @@ -26,6 +26,15 @@
> #include "intel_drv.h"
> #include "i915_scheduler.h"
>
> +bool i915_scheduler_is_enabled(struct drm_device *dev)
> +{
> +#ifdef CONFIG_DRM_I915_SCHEDULER
> + return true;
> +#else
> + return false;
> +#endif
> +}

I think this should be:
if (dev_priv->scheduler)
return true;
return false;

instead?

Otherwise looks fine.

--
Jesse Barnes, Intel Open Source Technology Center