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

[Jesse Barnes]

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

> From: John Harrison <John.C.Harrison at 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