Re: [PATCH v8 08/11] drm/xe/hw_engine_group: Ensure safe transition between execution modes

[Matthew Brost <matthew.brost@intel.com>]

On Thu, Aug 08, 2024 at 08:40:26PM +0200, Francois Dugast wrote:
> Provide a way to safely transition execution modes of the hw engine group
> ahead of the actual execution. When necessary, either wait for running
> jobs to complete or preempt them, thus ensuring mutual exclusion between
> execution modes.
> 
> Unlike a mutex, the rw_semaphore used in this context allows multiple
> submissions in the same mode.
> 
> v2: Use lockdep_assert_held_write, add annotations (Matt Brost)
> 
> v3: Fix kernel doc, remove redundant code (Matt Brost)
> 
> v4: Now that xe_hw_engine_group_suspend_faulting_lr_jobs can fail,
>     propagate the error to the caller (Matt Brost)
> 
> Signed-off-by: Francois Dugast <francois.dugast@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_hw_engine_group.c | 75 +++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_hw_engine_group.h |  5 ++
>  2 files changed, 80 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine_group.c b/drivers/gpu/drm/xe/xe_hw_engine_group.c
> index 8d3ddfc4ee82..e6c235119351 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine_group.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine_group.c
> @@ -235,3 +235,78 @@ static int xe_hw_engine_group_wait_for_dma_fence_jobs(struct xe_hw_engine_group
>  
>  	return 0;
>  }
> +
> +static int switch_mode(struct xe_hw_engine_group *group)
> +{
> +	int err = 0;
> +	enum xe_hw_engine_group_execution_mode new_mode;
> +
> +	lockdep_assert_held_write(&group->mode_sem);
> +
> +	switch (group->cur_mode) {
> +	case EXEC_MODE_LR:
> +		new_mode = EXEC_MODE_DMA_FENCE;
> +		err = xe_hw_engine_group_suspend_faulting_lr_jobs(group);
> +		break;
> +	case EXEC_MODE_DMA_FENCE:
> +		new_mode = EXEC_MODE_LR;
> +		err = xe_hw_engine_group_wait_for_dma_fence_jobs(group);
> +		break;
> +	}
> +
> +	if (err)
> +		return err;
> +
> +	group->cur_mode = new_mode;
> +
> +	return 0;
> +}
> +
> +/**
> + * xe_hw_engine_group_get_mode() - Get the group to execute in the new mode
> + * @group: The hw engine group
> + * @new_mode: The new execution mode
> + * @previous_mode: Pointer to the previous mode provided for use by caller
> + *
> + * Return: 0 if successful, -EINTR if locking failed.
> + */
> +int xe_hw_engine_group_get_mode(struct xe_hw_engine_group *group,
> +				enum xe_hw_engine_group_execution_mode new_mode,
> +				enum xe_hw_engine_group_execution_mode *previous_mode)
> +__acquires(&group->mode_sem)
> +{
> +	int err = down_read_interruptible(&group->mode_sem);
> +
> +	if (err)
> +		return err;
> +
> +	*previous_mode = group->cur_mode;
> +
> +	if (new_mode != group->cur_mode) {
> +		up_read(&group->mode_sem);
> +		err = down_write_killable(&group->mode_sem);
> +		if (err)
> +			return err;
> +
> +		if (new_mode != group->cur_mode) {
> +			err = switch_mode(group);
> +			if (err) {
> +				up_write(&group->mode_sem);
> +				return err;
> +			}
> +		}
> +		downgrade_write(&group->mode_sem);
> +	}
> +
> +	return err;
> +}
> +
> +/**
> + * xe_hw_engine_group_put() - Put the group
> + * @group: The hw engine group
> + */
> +void xe_hw_engine_group_put(struct xe_hw_engine_group *group)
> +__releases(&group->mode_sem)
> +{
> +	up_read(&group->mode_sem);
> +}
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine_group.h b/drivers/gpu/drm/xe/xe_hw_engine_group.h
> index 857a83787504..e0deb7c7bb5b 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine_group.h
> +++ b/drivers/gpu/drm/xe/xe_hw_engine_group.h
> @@ -17,4 +17,9 @@ int xe_hw_engine_setup_groups(struct xe_gt *gt);
>  int xe_hw_engine_group_add_exec_queue(struct xe_hw_engine_group *group, struct xe_exec_queue *q);
>  void xe_hw_engine_group_del_exec_queue(struct xe_hw_engine_group *group, struct xe_exec_queue *q);
>  
> +int xe_hw_engine_group_get_mode(struct xe_hw_engine_group *group,
> +				enum xe_hw_engine_group_execution_mode new_mode,
> +				enum xe_hw_engine_group_execution_mode *previous_mode);
> +void xe_hw_engine_group_put(struct xe_hw_engine_group *group);
> +
>  #endif
> -- 
> 2.43.0
>