intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
To: Matthew Brost <matthew.brost@intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 06/51] drm/i915/guc: Implement GuC context operations for new inteface
Date: Wed, 21 Jul 2021 16:51:38 -0700	[thread overview]
Message-ID: <dd809d70-1ed2-7013-7580-734ce752e87a@intel.com> (raw)
In-Reply-To: <20210720040400.GA23029@sdutt-i7>



On 7/19/2021 9:04 PM, Matthew Brost wrote:
> On Mon, Jul 19, 2021 at 05:51:46PM -0700, Daniele Ceraolo Spurio wrote:
>>
>> On 7/16/2021 1:16 PM, Matthew Brost wrote:
>>> Implement GuC context operations which includes GuC specific operations
>>> alloc, pin, unpin, and destroy.
>>>
>>> v2:
>>>    (Daniel Vetter)
>>>     - Use msleep_interruptible rather than cond_resched in busy loop
>>>    (Michal)
>>>     - Remove C++ style comment
>>>
>>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>>> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/gt/intel_context.c       |   5 +
>>>    drivers/gpu/drm/i915/gt/intel_context_types.h |  22 +-
>>>    drivers/gpu/drm/i915/gt/intel_lrc_reg.h       |   1 -
>>>    drivers/gpu/drm/i915/gt/uc/intel_guc.h        |  40 ++
>>>    drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c     |   4 +
>>>    .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 666 ++++++++++++++++--
>>>    drivers/gpu/drm/i915/i915_reg.h               |   1 +
>>>    drivers/gpu/drm/i915/i915_request.c           |   1 +
>>>    8 files changed, 685 insertions(+), 55 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
>>> index bd63813c8a80..32fd6647154b 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_context.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
>>> @@ -384,6 +384,11 @@ intel_context_init(struct intel_context *ce, struct intel_engine_cs *engine)
>>>    	mutex_init(&ce->pin_mutex);
>>> +	spin_lock_init(&ce->guc_state.lock);
>>> +
>>> +	ce->guc_id = GUC_INVALID_LRC_ID;
>>> +	INIT_LIST_HEAD(&ce->guc_id_link);
>>> +
>>>    	i915_active_init(&ce->active,
>>>    			 __intel_context_active, __intel_context_retire, 0);
>>>    }
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
>>> index 6d99631d19b9..606c480aec26 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
>>> @@ -96,6 +96,7 @@ struct intel_context {
>>>    #define CONTEXT_BANNED			6
>>>    #define CONTEXT_FORCE_SINGLE_SUBMISSION	7
>>>    #define CONTEXT_NOPREEMPT		8
>>> +#define CONTEXT_LRCA_DIRTY		9
>>>    	struct {
>>>    		u64 timeout_us;
>>> @@ -138,14 +139,29 @@ struct intel_context {
>>>    	u8 wa_bb_page; /* if set, page num reserved for context workarounds */
>>> +	struct {
>>> +		/** lock: protects everything in guc_state */
>>> +		spinlock_t lock;
>>> +		/**
>>> +		 * sched_state: scheduling state of this context using GuC
>>> +		 * submission
>>> +		 */
>>> +		u8 sched_state;
>>> +	} guc_state;
>>> +
>>>    	/* GuC scheduling state flags that do not require a lock. */
>>>    	atomic_t guc_sched_state_no_lock;
>>> +	/* GuC LRC descriptor ID */
>>> +	u16 guc_id;
>>> +
>>> +	/* GuC LRC descriptor reference count */
>>> +	atomic_t guc_id_ref;
>>> +
>>>    	/*
>>> -	 * GuC LRC descriptor ID - Not assigned in this patch but future patches
>>> -	 * in the series will.
>>> +	 * GuC ID link - in list when unpinned but guc_id still valid in GuC
>>>    	 */
>>> -	u16 guc_id;
>>> +	struct list_head guc_id_link;
>>>    };
>>>    #endif /* __INTEL_CONTEXT_TYPES__ */
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc_reg.h b/drivers/gpu/drm/i915/gt/intel_lrc_reg.h
>>> index 41e5350a7a05..49d4857ad9b7 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_lrc_reg.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_lrc_reg.h
>>> @@ -87,7 +87,6 @@
>>>    #define GEN11_CSB_WRITE_PTR_MASK	(GEN11_CSB_PTR_MASK << 0)
>>>    #define MAX_CONTEXT_HW_ID	(1 << 21) /* exclusive */
>>> -#define MAX_GUC_CONTEXT_HW_ID	(1 << 20) /* exclusive */
>>>    #define GEN11_MAX_CONTEXT_HW_ID	(1 << 11) /* exclusive */
>>>    /* in Gen12 ID 0x7FF is reserved to indicate idle */
>>>    #define GEN12_MAX_CONTEXT_HW_ID	(GEN11_MAX_CONTEXT_HW_ID - 1)
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>>> index 8c7b92f699f1..30773cd699f5 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>>> @@ -7,6 +7,7 @@
>>>    #define _INTEL_GUC_H_
>>>    #include <linux/xarray.h>
>>> +#include <linux/delay.h>
>>>    #include "intel_uncore.h"
>>>    #include "intel_guc_fw.h"
>>> @@ -44,6 +45,14 @@ struct intel_guc {
>>>    		void (*disable)(struct intel_guc *guc);
>>>    	} interrupts;
>>> +	/*
>>> +	 * contexts_lock protects the pool of free guc ids and a linked list of
>>> +	 * guc ids available to be stolen
>>> +	 */
>>> +	spinlock_t contexts_lock;
>>> +	struct ida guc_ids;
>>> +	struct list_head guc_id_list;
>>> +
>>>    	bool submission_selected;
>>>    	struct i915_vma *ads_vma;
>>> @@ -101,6 +110,34 @@ intel_guc_send_and_receive(struct intel_guc *guc, const u32 *action, u32 len,
>>>    				 response_buf, response_buf_size, 0);
>>>    }
>>> +static inline int intel_guc_send_busy_loop(struct intel_guc* guc,
>>> +					   const u32 *action,
>>> +					   u32 len,
>>> +					   bool loop)
>>> +{
>>> +	int err;
>>> +	unsigned int sleep_period_ms = 1;
>>> +	bool not_atomic = !in_atomic() && !irqs_disabled();
>>> +
>>> +	/* No sleeping with spin locks, just busy loop */
>>> +	might_sleep_if(loop && not_atomic);
>>> +
>>> +retry:
>>> +	err = intel_guc_send_nb(guc, action, len);
>>> +	if (unlikely(err == -EBUSY && loop)) {
>>> +		if (likely(not_atomic)) {
>>> +			if (msleep_interruptible(sleep_period_ms))
>>> +				return -EINTR;
>>> +			sleep_period_ms = sleep_period_ms << 1;
>>> +		} else {
>>> +			cpu_relax();
>> Potentially something we can change later, but if we're in atomic context we
>> can't keep looping without a timeout while we get -EBUSY, we need to bail
>> out early.
>>
> Eventually intel_guc_send_nb will pop with a different return code after
> 1 second I think if the GuC is hung.

I know, the point is that 1 second is too long in atomic context. This 
has to either complete quick in atomic or be forked off to a worker. Can 
fix as a follow up.

Daniele

>   
>>> +		}
>>> +		goto retry;
>>> +	}
>>> +
>>> +	return err;
>>> +}
>>> +
>>>    static inline void intel_guc_to_host_event_handler(struct intel_guc *guc)
>>>    {
>>>    	intel_guc_ct_event_handler(&guc->ct);
>>> @@ -202,6 +239,9 @@ static inline void intel_guc_disable_msg(struct intel_guc *guc, u32 mask)
>>>    int intel_guc_reset_engine(struct intel_guc *guc,
>>>    			   struct intel_engine_cs *engine);
>>> +int intel_guc_deregister_done_process_msg(struct intel_guc *guc,
>>> +					  const u32 *msg, u32 len);
>>> +
>>>    void intel_guc_load_status(struct intel_guc *guc, struct drm_printer *p);
>>>    #endif
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>>> index 83ec60ea3f89..28ff82c5be45 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>>> @@ -928,6 +928,10 @@ static int ct_process_request(struct intel_guc_ct *ct, struct ct_incoming_msg *r
>>>    	case INTEL_GUC_ACTION_DEFAULT:
>>>    		ret = intel_guc_to_host_process_recv_msg(guc, payload, len);
>>>    		break;
>>> +	case INTEL_GUC_ACTION_DEREGISTER_CONTEXT_DONE:
>>> +		ret = intel_guc_deregister_done_process_msg(guc, payload,
>>> +							    len);
>>> +		break;
>>>    	default:
>>>    		ret = -EOPNOTSUPP;
>>>    		break;
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>>> index 53b4a5eb4a85..a47b3813b4d0 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>>> @@ -13,7 +13,9 @@
>>>    #include "gt/intel_gt.h"
>>>    #include "gt/intel_gt_irq.h"
>>>    #include "gt/intel_gt_pm.h"
>>> +#include "gt/intel_gt_requests.h"
>>>    #include "gt/intel_lrc.h"
>>> +#include "gt/intel_lrc_reg.h"
>>>    #include "gt/intel_mocs.h"
>>>    #include "gt/intel_ring.h"
>>> @@ -85,6 +87,73 @@ static inline void clr_context_enabled(struct intel_context *ce)
>>>    		   &ce->guc_sched_state_no_lock);
>>>    }
>>> +/*
>>> + * Below is a set of functions which control the GuC scheduling state which
>>> + * require a lock, aside from the special case where the functions are called
>>> + * from guc_lrc_desc_pin(). In that case it isn't possible for any other code
>>> + * path to be executing on the context.
>>> + */
>> Is there a reason to avoid taking the lock in guc_lrc_desc_pin, even if it
>> isn't strictly needed? I'd prefer to avoid the asymmetry in the locking
> If this code is called from releasing a fence a circular dependency is
> created. Once we move the DRM scheduler the locking becomes way easier
> and we clean this up then. We already have task for locking clean up.
>
>> scheme if possible, as that might case trouble if thing change in the
>> future.
> Again this is temporary code, so I think we can live with this until the
> DRM scheduler lands.
>
>>> +#define SCHED_STATE_WAIT_FOR_DEREGISTER_TO_REGISTER	BIT(0)
>>> +#define SCHED_STATE_DESTROYED				BIT(1)
>>> +static inline void init_sched_state(struct intel_context *ce)
>>> +{
>>> +	/* Only should be called from guc_lrc_desc_pin() */
>>> +	atomic_set(&ce->guc_sched_state_no_lock, 0);
>>> +	ce->guc_state.sched_state = 0;
>>> +}
>>> +
>>> +static inline bool
>>> +context_wait_for_deregister_to_register(struct intel_context *ce)
>>> +{
>>> +	return (ce->guc_state.sched_state &
>>> +		SCHED_STATE_WAIT_FOR_DEREGISTER_TO_REGISTER);
>> No need for (). Below as well.
>>
> Yep.
>
>>> +}
>>> +
>>> +static inline void
>>> +set_context_wait_for_deregister_to_register(struct intel_context *ce)
>>> +{
>>> +	/* Only should be called from guc_lrc_desc_pin() */
>>> +	ce->guc_state.sched_state |=
>>> +		SCHED_STATE_WAIT_FOR_DEREGISTER_TO_REGISTER;
>>> +}
>>> +
>>> +static inline void
>>> +clr_context_wait_for_deregister_to_register(struct intel_context *ce)
>>> +{
>>> +	lockdep_assert_held(&ce->guc_state.lock);
>>> +	ce->guc_state.sched_state =
>>> +		(ce->guc_state.sched_state &
>>> +		 ~SCHED_STATE_WAIT_FOR_DEREGISTER_TO_REGISTER);
>> nit: can also use
>>
>> ce->guc_state.sched_state &=
>> 	~SCHED_STATE_WAIT_FOR_DEREGISTER_TO_REGISTER
>>
> Yep.
>   
>>> +}
>>> +
>>> +static inline bool
>>> +context_destroyed(struct intel_context *ce)
>>> +{
>>> +	return (ce->guc_state.sched_state & SCHED_STATE_DESTROYED);
>>> +}
>>> +
>>> +static inline void
>>> +set_context_destroyed(struct intel_context *ce)
>>> +{
>>> +	lockdep_assert_held(&ce->guc_state.lock);
>>> +	ce->guc_state.sched_state |= SCHED_STATE_DESTROYED;
>>> +}
>>> +
>>> +static inline bool context_guc_id_invalid(struct intel_context *ce)
>>> +{
>>> +	return (ce->guc_id == GUC_INVALID_LRC_ID);
>>> +}
>>> +
>>> +static inline void set_context_guc_id_invalid(struct intel_context *ce)
>>> +{
>>> +	ce->guc_id = GUC_INVALID_LRC_ID;
>>> +}
>>> +
>>> +static inline struct intel_guc *ce_to_guc(struct intel_context *ce)
>>> +{
>>> +	return &ce->engine->gt->uc.guc;
>>> +}
>>> +
>>>    static inline struct i915_priolist *to_priolist(struct rb_node *rb)
>>>    {
>>>    	return rb_entry(rb, struct i915_priolist, node);
>>> @@ -155,6 +224,9 @@ static int guc_add_request(struct intel_guc *guc, struct i915_request *rq)
>>>    	int len = 0;
>>>    	bool enabled = context_enabled(ce);
>>> +	GEM_BUG_ON(!atomic_read(&ce->guc_id_ref));
>>> +	GEM_BUG_ON(context_guc_id_invalid(ce));
>>> +
>>>    	if (!enabled) {
>>>    		action[len++] = INTEL_GUC_ACTION_SCHED_CONTEXT_MODE_SET;
>>>    		action[len++] = ce->guc_id;
>>> @@ -417,6 +489,10 @@ int intel_guc_submission_init(struct intel_guc *guc)
>>>    	xa_init_flags(&guc->context_lookup, XA_FLAGS_LOCK_IRQ);
>>> +	spin_lock_init(&guc->contexts_lock);
>>> +	INIT_LIST_HEAD(&guc->guc_id_list);
>>> +	ida_init(&guc->guc_ids);
>>> +
>>>    	return 0;
>>>    }
>>> @@ -429,9 +505,303 @@ void intel_guc_submission_fini(struct intel_guc *guc)
>>>    	i915_sched_engine_put(guc->sched_engine);
>>>    }
>>> -static int guc_context_alloc(struct intel_context *ce)
>>> +static inline void queue_request(struct i915_sched_engine *sched_engine,
>>> +				 struct i915_request *rq,
>>> +				 int prio)
>>>    {
>>> -	return lrc_alloc(ce, ce->engine);
>>> +	GEM_BUG_ON(!list_empty(&rq->sched.link));
>>> +	list_add_tail(&rq->sched.link,
>>> +		      i915_sched_lookup_priolist(sched_engine, prio));
>>> +	set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
>>> +}
>>> +
>>> +static int guc_bypass_tasklet_submit(struct intel_guc *guc,
>>> +				     struct i915_request *rq)
>>> +{
>>> +	int ret;
>>> +
>>> +	__i915_request_submit(rq);
>>> +
>>> +	trace_i915_request_in(rq, 0);
>>> +
>>> +	guc_set_lrc_tail(rq);
>>> +	ret = guc_add_request(guc, rq);
>>> +	if (ret == -EBUSY)
>>> +		guc->stalled_request = rq;
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static void guc_submit_request(struct i915_request *rq)
>>> +{
>>> +	struct i915_sched_engine *sched_engine = rq->engine->sched_engine;
>>> +	struct intel_guc *guc = &rq->engine->gt->uc.guc;
>>> +	unsigned long flags;
>>> +
>>> +	/* Will be called from irq-context when using foreign fences. */
>>> +	spin_lock_irqsave(&sched_engine->lock, flags);
>>> +
>>> +	if (guc->stalled_request || !i915_sched_engine_is_empty(sched_engine))
>>> +		queue_request(sched_engine, rq, rq_prio(rq));
>>> +	else if (guc_bypass_tasklet_submit(guc, rq) == -EBUSY)
>>> +		tasklet_hi_schedule(&sched_engine->tasklet);
>>> +
>>> +	spin_unlock_irqrestore(&sched_engine->lock, flags);
>>> +}
>>> +
>>> +#define GUC_ID_START	64	/* First 64 guc_ids reserved */
>>> +static int new_guc_id(struct intel_guc *guc)
>>> +{
>>> +	return ida_simple_get(&guc->guc_ids, GUC_ID_START,
>>> +			      GUC_MAX_LRC_DESCRIPTORS, GFP_KERNEL |
>>> +			      __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
>>> +}
>>> +
>>> +static void __release_guc_id(struct intel_guc *guc, struct intel_context *ce)
>>> +{
>>> +	if (!context_guc_id_invalid(ce)) {
>>> +		ida_simple_remove(&guc->guc_ids, ce->guc_id);
>>> +		reset_lrc_desc(guc, ce->guc_id);
>>> +		set_context_guc_id_invalid(ce);
>>> +	}
>>> +	if (!list_empty(&ce->guc_id_link))
>>> +		list_del_init(&ce->guc_id_link);
>>> +}
>>> +
>>> +static void release_guc_id(struct intel_guc *guc, struct intel_context *ce)
>>> +{
>>> +	unsigned long flags;
>>> +
>>> +	spin_lock_irqsave(&guc->contexts_lock, flags);
>>> +	__release_guc_id(guc, ce);
>>> +	spin_unlock_irqrestore(&guc->contexts_lock, flags);
>>> +}
>>> +
>>> +static int steal_guc_id(struct intel_guc *guc)
>>> +{
>>> +	struct intel_context *ce;
>>> +	int guc_id;
>>> +
>>> +	if (!list_empty(&guc->guc_id_list)) {
>>> +		ce = list_first_entry(&guc->guc_id_list,
>>> +				      struct intel_context,
>>> +				      guc_id_link);
>>> +
>>> +		GEM_BUG_ON(atomic_read(&ce->guc_id_ref));
>>> +		GEM_BUG_ON(context_guc_id_invalid(ce));
>>> +
>>> +		list_del_init(&ce->guc_id_link);
>>> +		guc_id = ce->guc_id;
>>> +		set_context_guc_id_invalid(ce);
>>> +		return guc_id;
>>> +	} else {
>>> +		return -EAGAIN;
>>> +	}
>>> +}
>>> +
>>> +static int assign_guc_id(struct intel_guc *guc, u16 *out)
>>> +{
>>> +	int ret;
>>> +
>>> +	ret = new_guc_id(guc);
>>> +	if (unlikely(ret < 0)) {
>>> +		ret = steal_guc_id(guc);
>>> +		if (ret < 0)
>>> +			return ret;
>>> +	}
>>> +
>>> +	*out = ret;
>>> +	return 0;
>> Is it worth adding spinlock_held asserts for guc->contexts_lock in these ID
>> functions? Doubles up as a documentation of what locking we expect.
>>
> Never a bad idea to have asserts in the code. Will add.
>
>>> +}
>>> +
>>> +#define PIN_GUC_ID_TRIES	4
>>> +static int pin_guc_id(struct intel_guc *guc, struct intel_context *ce)
>>> +{
>>> +	int ret = 0;
>>> +	unsigned long flags, tries = PIN_GUC_ID_TRIES;
>>> +
>>> +	GEM_BUG_ON(atomic_read(&ce->guc_id_ref));
>>> +
>>> +try_again:
>>> +	spin_lock_irqsave(&guc->contexts_lock, flags);
>>> +
>>> +	if (context_guc_id_invalid(ce)) {
>>> +		ret = assign_guc_id(guc, &ce->guc_id);
>>> +		if (ret)
>>> +			goto out_unlock;
>>> +		ret = 1;	/* Indidcates newly assigned guc_id */
>>> +	}
>>> +	if (!list_empty(&ce->guc_id_link))
>>> +		list_del_init(&ce->guc_id_link);
>>> +	atomic_inc(&ce->guc_id_ref);
>>> +
>>> +out_unlock:
>>> +	spin_unlock_irqrestore(&guc->contexts_lock, flags);
>>> +
>>> +	/*
>>> +	 * -EAGAIN indicates no guc_ids are available, let's retire any
>>> +	 * outstanding requests to see if that frees up a guc_id. If the first
>>> +	 * retire didn't help, insert a sleep with the timeslice duration before
>>> +	 * attempting to retire more requests. Double the sleep period each
>>> +	 * subsequent pass before finally giving up. The sleep period has max of
>>> +	 * 100ms and minimum of 1ms.
>>> +	 */
>>> +	if (ret == -EAGAIN && --tries) {
>>> +		if (PIN_GUC_ID_TRIES - tries > 1) {
>>> +			unsigned int timeslice_shifted =
>>> +				ce->engine->props.timeslice_duration_ms <<
>>> +				(PIN_GUC_ID_TRIES - tries - 2);
>>> +			unsigned int max = min_t(unsigned int, 100,
>>> +						 timeslice_shifted);
>>> +
>>> +			msleep(max_t(unsigned int, max, 1));
>>> +		}
>>> +		intel_gt_retire_requests(guc_to_gt(guc));
>>> +		goto try_again;
>>> +	}
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static void unpin_guc_id(struct intel_guc *guc, struct intel_context *ce)
>>> +{
>>> +	unsigned long flags;
>>> +
>>> +	GEM_BUG_ON(atomic_read(&ce->guc_id_ref) < 0);
>>> +
>>> +	spin_lock_irqsave(&guc->contexts_lock, flags);
>>> +	if (!context_guc_id_invalid(ce) && list_empty(&ce->guc_id_link) &&
>>> +	    !atomic_read(&ce->guc_id_ref))
>>> +		list_add_tail(&ce->guc_id_link, &guc->guc_id_list);
>>> +	spin_unlock_irqrestore(&guc->contexts_lock, flags);
>>> +}
>>> +
>>> +static int __guc_action_register_context(struct intel_guc *guc,
>>> +					 u32 guc_id,
>>> +					 u32 offset)
>>> +{
>>> +	u32 action[] = {
>>> +		INTEL_GUC_ACTION_REGISTER_CONTEXT,
>>> +		guc_id,
>>> +		offset,
>>> +	};
>>> +
>>> +	return intel_guc_send_busy_loop(guc, action, ARRAY_SIZE(action), true);
>>> +}
>>> +
>>> +static int register_context(struct intel_context *ce)
>>> +{
>>> +	struct intel_guc *guc = ce_to_guc(ce);
>>> +	u32 offset = intel_guc_ggtt_offset(guc, guc->lrc_desc_pool) +
>>> +		ce->guc_id * sizeof(struct guc_lrc_desc);
>>> +
>>> +	return __guc_action_register_context(guc, ce->guc_id, offset);
>>> +}
>>> +
>>> +static int __guc_action_deregister_context(struct intel_guc *guc,
>>> +					   u32 guc_id)
>>> +{
>>> +	u32 action[] = {
>>> +		INTEL_GUC_ACTION_DEREGISTER_CONTEXT,
>>> +		guc_id,
>>> +	};
>>> +
>>> +	return intel_guc_send_busy_loop(guc, action, ARRAY_SIZE(action), true);
>>> +}
>>> +
>>> +static int deregister_context(struct intel_context *ce, u32 guc_id)
>>> +{
>>> +	struct intel_guc *guc = ce_to_guc(ce);
>>> +
>>> +	return __guc_action_deregister_context(guc, guc_id);
>>> +}
>>> +
>>> +static intel_engine_mask_t adjust_engine_mask(u8 class, intel_engine_mask_t mask)
>>> +{
>>> +	switch (class) {
>>> +	case RENDER_CLASS:
>>> +		return mask >> RCS0;
>>> +	case VIDEO_ENHANCEMENT_CLASS:
>>> +		return mask >> VECS0;
>>> +	case VIDEO_DECODE_CLASS:
>>> +		return mask >> VCS0;
>>> +	case COPY_ENGINE_CLASS:
>>> +		return mask >> BCS0;
>>> +	default:
>>> +		GEM_BUG_ON("Invalid Class");
>> we usually use MISSING_CASE for this type of errors.
> As soon as start talking in logical space (series immediatly after this
> gets merged) this function gets deleted but will fix.
>
>>> +		return 0;
>>> +	}
>>> +}
>>> +
>>> +static void guc_context_policy_init(struct intel_engine_cs *engine,
>>> +				    struct guc_lrc_desc *desc)
>>> +{
>>> +	desc->policy_flags = 0;
>>> +
>>> +	desc->execution_quantum = CONTEXT_POLICY_DEFAULT_EXECUTION_QUANTUM_US;
>>> +	desc->preemption_timeout = CONTEXT_POLICY_DEFAULT_PREEMPTION_TIME_US;
>>> +}
>>> +
>>> +static int guc_lrc_desc_pin(struct intel_context *ce)
>>> +{
>>> +	struct intel_runtime_pm *runtime_pm =
>>> +		&ce->engine->gt->i915->runtime_pm;
>> If you move this after the engine var below you can skip the ce->engine
>> jump. Also, you can shorten the pointer chasing by using
>> engine->uncore->rpm.
>>
> Sure.
>
>>> +	struct intel_engine_cs *engine = ce->engine;
>>> +	struct intel_guc *guc = &engine->gt->uc.guc;
>>> +	u32 desc_idx = ce->guc_id;
>>> +	struct guc_lrc_desc *desc;
>>> +	bool context_registered;
>>> +	intel_wakeref_t wakeref;
>>> +	int ret = 0;
>>> +
>>> +	GEM_BUG_ON(!engine->mask);
>>> +
>>> +	/*
>>> +	 * Ensure LRC + CT vmas are is same region as write barrier is done
>>> +	 * based on CT vma region.
>>> +	 */
>>> +	GEM_BUG_ON(i915_gem_object_is_lmem(guc->ct.vma->obj) !=
>>> +		   i915_gem_object_is_lmem(ce->ring->vma->obj));
>>> +
>>> +	context_registered = lrc_desc_registered(guc, desc_idx);
>>> +
>>> +	reset_lrc_desc(guc, desc_idx);
>>> +	set_lrc_desc_registered(guc, desc_idx, ce);
>>> +
>>> +	desc = __get_lrc_desc(guc, desc_idx);
>>> +	desc->engine_class = engine_class_to_guc_class(engine->class);
>>> +	desc->engine_submit_mask = adjust_engine_mask(engine->class,
>>> +						      engine->mask);
>>> +	desc->hw_context_desc = ce->lrc.lrca;
>>> +	desc->priority = GUC_CLIENT_PRIORITY_KMD_NORMAL;
>>> +	desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD;
>>> +	guc_context_policy_init(engine, desc);
>>> +	init_sched_state(ce);
>>> +
>>> +	/*
>>> +	 * The context_lookup xarray is used to determine if the hardware
>>> +	 * context is currently registered. There are two cases in which it
>>> +	 * could be regisgered either the guc_id has been stole from from
>> typo regisgered
>>
> John got this one. Fixed.
>   
>>> +	 * another context or the lrc descriptor address of this context has
>>> +	 * changed. In either case the context needs to be deregistered with the
>>> +	 * GuC before registering this context.
>>> +	 */
>>> +	if (context_registered) {
>>> +		set_context_wait_for_deregister_to_register(ce);
>>> +		intel_context_get(ce);
>>> +
>>> +		/*
>>> +		 * If stealing the guc_id, this ce has the same guc_id as the
>>> +		 * context whos guc_id was stole.
>>> +		 */
>>> +		with_intel_runtime_pm(runtime_pm, wakeref)
>>> +			ret = deregister_context(ce, ce->guc_id);
>>> +	} else {
>>> +		with_intel_runtime_pm(runtime_pm, wakeref)
>>> +			ret = register_context(ce);
>>> +	}
>>> +
>>> +	return ret;
>>>    }
>>>    static int guc_context_pre_pin(struct intel_context *ce,
>>> @@ -443,36 +813,139 @@ static int guc_context_pre_pin(struct intel_context *ce,
>>>    static int guc_context_pin(struct intel_context *ce, void *vaddr)
>>>    {
>>> +	if (i915_ggtt_offset(ce->state) !=
>>> +	    (ce->lrc.lrca & CTX_GTT_ADDRESS_MASK))
>>> +		set_bit(CONTEXT_LRCA_DIRTY, &ce->flags);
>> Shouldn't this be set after lrc_pin()? I can see ce->lrc.lrca being
>> re-assigned in there; it looks like it can only change if the context is
>> new, but for future-proofing IMO better to re-order here.
>>
> No this is right. ce->state is assigned in lr_pre_pin and ce->lrc.lrca is
> assigned in lrc_pin. To catch the change you have to check between
> those two functions.
>
>>> +
>>>    	return lrc_pin(ce, ce->engine, vaddr);
>> Could use a comment to say that the GuC context pinning call is delayed
>> until request alloc and to look at the comment in the latter function for
>> details (or move the explanation here and refer here from request alloc).
>>
> Yes.
>   
>>>    }
>>> +static void guc_context_unpin(struct intel_context *ce)
>>> +{
>>> +	struct intel_guc *guc = ce_to_guc(ce);
>>> +
>>> +	unpin_guc_id(guc, ce);
>>> +	lrc_unpin(ce);
>>> +}
>>> +
>>> +static void guc_context_post_unpin(struct intel_context *ce)
>>> +{
>>> +	lrc_post_unpin(ce);
>> why do we need this function? we can just pass lrc_post_unpin to the ops
>> (like we already do).
>>
> We don't before style reasons I think having a GuC specific wrapper is
> correct.
>
>>> +}
>>> +
>>> +static inline void guc_lrc_desc_unpin(struct intel_context *ce)
>>> +{
>>> +	struct intel_engine_cs *engine = ce->engine;
>>> +	struct intel_guc *guc = &engine->gt->uc.guc;
>>> +	unsigned long flags;
>>> +
>>> +	GEM_BUG_ON(!lrc_desc_registered(guc, ce->guc_id));
>>> +	GEM_BUG_ON(ce != __get_context(guc, ce->guc_id));
>>> +
>>> +	spin_lock_irqsave(&ce->guc_state.lock, flags);
>>> +	set_context_destroyed(ce);
>>> +	spin_unlock_irqrestore(&ce->guc_state.lock, flags);
>>> +
>>> +	deregister_context(ce, ce->guc_id);
>>> +}
>>> +
>>> +static void guc_context_destroy(struct kref *kref)
>>> +{
>>> +	struct intel_context *ce = container_of(kref, typeof(*ce), ref);
>>> +	struct intel_runtime_pm *runtime_pm = &ce->engine->gt->i915->runtime_pm;
>> same as above, going through engine->uncore->rpm is shorter
>>
> Sure.
>   
>>> +	struct intel_guc *guc = &ce->engine->gt->uc.guc;
>>> +	intel_wakeref_t wakeref;
>>> +	unsigned long flags;
>>> +
>>> +	/*
>>> +	 * If the guc_id is invalid this context has been stolen and we can free
>>> +	 * it immediately. Also can be freed immediately if the context is not
>>> +	 * registered with the GuC.
>>> +	 */
>>> +	if (context_guc_id_invalid(ce) ||
>>> +	    !lrc_desc_registered(guc, ce->guc_id)) {
>>> +		release_guc_id(guc, ce);
>> it feels a bit weird that we call release_guc_id in the case where the id is
>> invalid. The code handles it fine, but still the flow doesn't feel clean.
>> Not a blocker.
>>
> I understand. Let's see if this can get cleaned up at some point.
>
>>> +		lrc_destroy(kref);
>>> +		return;
>>> +	}
>>> +
>>> +	/*
>>> +	 * We have to acquire the context spinlock and check guc_id again, if it
>>> +	 * is valid it hasn't been stolen and needs to be deregistered. We
>>> +	 * delete this context from the list of unpinned guc_ids available to
>>> +	 * stole to seal a race with guc_lrc_desc_pin(). When the G2H CTB
>>> +	 * returns indicating this context has been deregistered the guc_id is
>>> +	 * returned to the pool of available guc_ids.
>>> +	 */
>>> +	spin_lock_irqsave(&guc->contexts_lock, flags);
>>> +	if (context_guc_id_invalid(ce)) {
>>> +		__release_guc_id(guc, ce);
>> But here the call to __release_guc_id is unneded, right? the ce doesn't own
>> the ID anymore and both the steal and the release function already clean
>> ce->guc_id_link, so there should be nothing left to clean.
>>
> This is dead code. Will delete.
>
>>> +		spin_unlock_irqrestore(&guc->contexts_lock, flags);
>>> +		lrc_destroy(kref);
>>> +		return;
>>> +	}
>>> +
>>> +	if (!list_empty(&ce->guc_id_link))
>>> +		list_del_init(&ce->guc_id_link);
>>> +	spin_unlock_irqrestore(&guc->contexts_lock, flags);
>>> +
>>> +	/*
>>> +	 * We defer GuC context deregistration until the context is destroyed
>>> +	 * in order to save on CTBs. With this optimization ideally we only need
>>> +	 * 1 CTB to register the context during the first pin and 1 CTB to
>>> +	 * deregister the context when the context is destroyed. Without this
>>> +	 * optimization, a CTB would be needed every pin & unpin.
>>> +	 *
>>> +	 * XXX: Need to acqiure the runtime wakeref as this can be triggered
>>> +	 * from context_free_worker when not runtime wakeref is held.
>>> +	 * guc_lrc_desc_unpin requires the runtime as a GuC register is written
>>> +	 * in H2G CTB to deregister the context. A future patch may defer this
>>> +	 * H2G CTB if the runtime wakeref is zero.
>>> +	 */
>>> +	with_intel_runtime_pm(runtime_pm, wakeref)
>>> +		guc_lrc_desc_unpin(ce);
>>> +}
>>> +
>>> +static int guc_context_alloc(struct intel_context *ce)
>>> +{
>>> +	return lrc_alloc(ce, ce->engine);
>>> +}
>>> +
>>>    static const struct intel_context_ops guc_context_ops = {
>>>    	.alloc = guc_context_alloc,
>>>    	.pre_pin = guc_context_pre_pin,
>>>    	.pin = guc_context_pin,
>>> -	.unpin = lrc_unpin,
>>> -	.post_unpin = lrc_post_unpin,
>>> +	.unpin = guc_context_unpin,
>>> +	.post_unpin = guc_context_post_unpin,
>>>    	.enter = intel_context_enter_engine,
>>>    	.exit = intel_context_exit_engine,
>>>    	.reset = lrc_reset,
>>> -	.destroy = lrc_destroy,
>>> +	.destroy = guc_context_destroy,
>>>    };
>>> -static int guc_request_alloc(struct i915_request *request)
>>> +static bool context_needs_register(struct intel_context *ce, bool new_guc_id)
>>>    {
>>> +	return new_guc_id || test_bit(CONTEXT_LRCA_DIRTY, &ce->flags) ||
>>> +		!lrc_desc_registered(ce_to_guc(ce), ce->guc_id);
>>> +}
>>> +
>>> +static int guc_request_alloc(struct i915_request *rq)
>>> +{
>>> +	struct intel_context *ce = rq->context;
>>> +	struct intel_guc *guc = ce_to_guc(ce);
>>>    	int ret;
>>> -	GEM_BUG_ON(!intel_context_is_pinned(request->context));
>>> +	GEM_BUG_ON(!intel_context_is_pinned(rq->context));
>>>    	/*
>>>    	 * Flush enough space to reduce the likelihood of waiting after
>>>    	 * we start building the request - in which case we will just
>>>    	 * have to repeat work.
>>>    	 */
>>> -	request->reserved_space += GUC_REQUEST_SIZE;
>>> +	rq->reserved_space += GUC_REQUEST_SIZE;
>>>    	/*
>>>    	 * Note that after this point, we have committed to using
>>> @@ -483,56 +956,47 @@ static int guc_request_alloc(struct i915_request *request)
>>>    	 */
>>>    	/* Unconditionally invalidate GPU caches and TLBs. */
>>> -	ret = request->engine->emit_flush(request, EMIT_INVALIDATE);
>>> +	ret = rq->engine->emit_flush(rq, EMIT_INVALIDATE);
>>>    	if (ret)
>>>    		return ret;
>>> -	request->reserved_space -= GUC_REQUEST_SIZE;
>>> -	return 0;
>>> -}
>>> +	rq->reserved_space -= GUC_REQUEST_SIZE;
>>> -static inline void queue_request(struct i915_sched_engine *sched_engine,
>>> -				 struct i915_request *rq,
>>> -				 int prio)
>>> -{
>>> -	GEM_BUG_ON(!list_empty(&rq->sched.link));
>>> -	list_add_tail(&rq->sched.link,
>>> -		      i915_sched_lookup_priolist(sched_engine, prio));
>>> -	set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
>>> -}
>>> -
>>> -static int guc_bypass_tasklet_submit(struct intel_guc *guc,
>>> -				     struct i915_request *rq)
>>> -{
>>> -	int ret;
>>> -
>>> -	__i915_request_submit(rq);
>>> -
>>> -	trace_i915_request_in(rq, 0);
>>> -
>>> -	guc_set_lrc_tail(rq);
>>> -	ret = guc_add_request(guc, rq);
>>> -	if (ret == -EBUSY)
>>> -		guc->stalled_request = rq;
>>> -
>>> -	return ret;
>>> -}
>>> -
>>> -static void guc_submit_request(struct i915_request *rq)
>>> -{
>>> -	struct i915_sched_engine *sched_engine = rq->engine->sched_engine;
>>> -	struct intel_guc *guc = &rq->engine->gt->uc.guc;
>>> -	unsigned long flags;
>>> +	/*
>>> +	 * Call pin_guc_id here rather than in the pinning step as with
>>> +	 * dma_resv, contexts can be repeatedly pinned / unpinned trashing the
>>> +	 * guc_ids and creating horrible race conditions. This is especially bad
>>> +	 * when guc_ids are being stolen due to over subscription. By the time
>>> +	 * this function is reached, it is guaranteed that the guc_id will be
>>> +	 * persistent until the generated request is retired. Thus, sealing these
>>> +	 * race conditions. It is still safe to fail here if guc_ids are
>>> +	 * exhausted and return -EAGAIN to the user indicating that they can try
>>> +	 * again in the future.
>>> +	 *
>>> +	 * There is no need for a lock here as the timeline mutex ensures at
>>> +	 * most one context can be executing this code path at once. The
>>> +	 * guc_id_ref is incremented once for every request in flight and
>>> +	 * decremented on each retire. When it is zero, a lock around the
>>> +	 * increment (in pin_guc_id) is needed to seal a race with unpin_guc_id.
>>> +	 */
>>> +	if (atomic_add_unless(&ce->guc_id_ref, 1, 0))
>>> +		return 0;
>>> -	/* Will be called from irq-context when using foreign fences. */
>>> -	spin_lock_irqsave(&sched_engine->lock, flags);
>>> +	ret = pin_guc_id(guc, ce);	/* returns 1 if new guc_id assigned */
>>> +	if (unlikely(ret < 0))
>>> +		return ret;;
>> typo ";;"
>>
> Yep.
>
>>> +	if (context_needs_register(ce, !!ret)) {
>>> +		ret = guc_lrc_desc_pin(ce);
>>> +		if (unlikely(ret)) {	/* unwind */
>>> +			atomic_dec(&ce->guc_id_ref);
>>> +			unpin_guc_id(guc, ce);
>>> +			return ret;
>>> +		}
>>> +	}
>>> -	if (guc->stalled_request || !i915_sched_engine_is_empty(sched_engine))
>>> -		queue_request(sched_engine, rq, rq_prio(rq));
>>> -	else if (guc_bypass_tasklet_submit(guc, rq) == -EBUSY)
>>> -		tasklet_hi_schedule(&sched_engine->tasklet);
>>> +	clear_bit(CONTEXT_LRCA_DIRTY, &ce->flags);
>>
>> Might be worth moving the pinning to a separate function to keep the
>> request_alloc focused on the request. Can be done as a follow up.
>>
> Sure. Let me see how that looks in a follow up.
>   
>>> -	spin_unlock_irqrestore(&sched_engine->lock, flags);
>>> +	return 0;
>>>    }
>>>    static void sanitize_hwsp(struct intel_engine_cs *engine)
>>> @@ -606,6 +1070,46 @@ static void guc_set_default_submission(struct intel_engine_cs *engine)
>>>    	engine->submit_request = guc_submit_request;
>>>    }
>>> +static inline void guc_kernel_context_pin(struct intel_guc *guc,
>>> +					  struct intel_context *ce)
>>> +{
>>> +	if (context_guc_id_invalid(ce))
>>> +		pin_guc_id(guc, ce);
>>> +	guc_lrc_desc_pin(ce);
>>> +}
>>> +
>>> +static inline void guc_init_lrc_mapping(struct intel_guc *guc)
>>> +{
>>> +	struct intel_gt *gt = guc_to_gt(guc);
>>> +	struct intel_engine_cs *engine;
>>> +	enum intel_engine_id id;
>>> +
>>> +	/* make sure all descriptors are clean... */
>>> +	xa_destroy(&guc->context_lookup);
>>> +
>>> +	/*
>>> +	 * Some contexts might have been pinned before we enabled GuC
>>> +	 * submission, so we need to add them to the GuC bookeeping.
>>> +	 * Also, after a reset the GuC we want to make sure that the information
>>> +	 * shared with GuC is properly reset. The kernel lrcs are not attached
>>> +	 * to the gem_context, so they need to be added separately.
>>> +	 *
>>> +	 * Note: we purposely do not check the error return of
>>> +	 * guc_lrc_desc_pin, because that function can only fail in two cases.
>>> +	 * One, if there aren't enough free IDs, but we're guaranteed to have
>>> +	 * enough here (we're either only pinning a handful of lrc on first boot
>>> +	 * or we're re-pinning lrcs that were already pinned before the reset).
>>> +	 * Two, if the GuC has died and CTBs can't make forward progress.
>>> +	 * Presumably, the GuC should be alive as this function is called on
>>> +	 * driver load or after a reset. Even if it is dead, another full GPU
>>> +	 * reset will be triggered and this function would be called again.
>>> +	 */
>>> +
>>> +	for_each_engine(engine, gt, id)
>>> +		if (engine->kernel_context)
>>> +			guc_kernel_context_pin(guc, engine->kernel_context);
>>> +}
>>> +
>>>    static void guc_release(struct intel_engine_cs *engine)
>>>    {
>>>    	engine->sanitize = NULL; /* no longer in control, nothing to sanitize */
>>> @@ -718,6 +1222,7 @@ int intel_guc_submission_setup(struct intel_engine_cs *engine)
>>>    void intel_guc_submission_enable(struct intel_guc *guc)
>>>    {
>>> +	guc_init_lrc_mapping(guc);
>>>    }
>>>    void intel_guc_submission_disable(struct intel_guc *guc)
>>> @@ -743,3 +1248,62 @@ void intel_guc_submission_init_early(struct intel_guc *guc)
>>>    {
>>>    	guc->submission_selected = __guc_submission_selected(guc);
>>>    }
>>> +
>>> +static inline struct intel_context *
>>> +g2h_context_lookup(struct intel_guc *guc, u32 desc_idx)
>>> +{
>>> +	struct intel_context *ce;
>>> +
>>> +	if (unlikely(desc_idx >= GUC_MAX_LRC_DESCRIPTORS)) {
>>> +		drm_dbg(&guc_to_gt(guc)->i915->drm,
>>> +			"Invalid desc_idx %u", desc_idx);
>>> +		return NULL;
>>> +	}
>>> +
>>> +	ce = __get_context(guc, desc_idx);
>>> +	if (unlikely(!ce)) {
>>> +		drm_dbg(&guc_to_gt(guc)->i915->drm,
>>> +			"Context is NULL, desc_idx %u", desc_idx);
>>> +		return NULL;
>>> +	}
>>> +
>>> +	return ce;
>>> +}
>>> +
>>> +int intel_guc_deregister_done_process_msg(struct intel_guc *guc,
>>> +					  const u32 *msg,
>>> +					  u32 len)
>>> +{
>>> +	struct intel_context *ce;
>>> +	u32 desc_idx = msg[0];
>>> +
>>> +	if (unlikely(len < 1)) {
>>> +		drm_dbg(&guc_to_gt(guc)->i915->drm, "Invalid length %u", len);
>>> +		return -EPROTO;
>>> +	}
>>> +
>>> +	ce = g2h_context_lookup(guc, desc_idx);
>>> +	if (unlikely(!ce))
>>> +		return -EPROTO;
>>> +
>>> +	if (context_wait_for_deregister_to_register(ce)) {
>>> +		struct intel_runtime_pm *runtime_pm =
>>> +			&ce->engine->gt->i915->runtime_pm;
>>> +		intel_wakeref_t wakeref;
>>> +
>>> +		/*
>>> +		 * Previous owner of this guc_id has been deregistered, now safe
>>> +		 * register this context.
>>> +		 */
>>> +		with_intel_runtime_pm(runtime_pm, wakeref)
>>> +			register_context(ce);
>>> +		clr_context_wait_for_deregister_to_register(ce);
>>> +		intel_context_put(ce);
>>> +	} else if (context_destroyed(ce)) {
>>> +		/* Context has been destroyed */
>>> +		release_guc_id(guc, ce);
>>> +		lrc_destroy(&ce->ref);
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index 943fe485c662..204c95c39353 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -4142,6 +4142,7 @@ enum {
>>>    	FAULT_AND_CONTINUE /* Unsupported */
>>>    };
>>> +#define CTX_GTT_ADDRESS_MASK GENMASK(31, 12)
>>>    #define GEN8_CTX_VALID (1 << 0)
>>>    #define GEN8_CTX_FORCE_PD_RESTORE (1 << 1)
>>>    #define GEN8_CTX_FORCE_RESTORE (1 << 2)
>>> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
>>> index 86b4c9f2613d..b48c4905d3fc 100644
>>> --- a/drivers/gpu/drm/i915/i915_request.c
>>> +++ b/drivers/gpu/drm/i915/i915_request.c
>>> @@ -407,6 +407,7 @@ bool i915_request_retire(struct i915_request *rq)
>>>    	 */
>>>    	if (!list_empty(&rq->sched.link))
>>>    		remove_from_engine(rq);
>>> +	atomic_dec(&rq->context->guc_id_ref);
>> Does this work/make sense  if GuC is disabled?
>>
> It doesn't matter if the GuC is disabled as this var isn't used if it
> is. A following patch adds a vfunc here and pushes this in the GuC
> backend.
>
> Matt
>
>> Daniele
>>
>>>    	GEM_BUG_ON(!llist_empty(&rq->execute_cb));
>>>    	__list_del_entry(&rq->link); /* poison neither prev/next (RCU walks) */

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2021-07-21 23:51 UTC|newest]

Thread overview: 111+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-16 20:16 [Intel-gfx] [PATCH 00/51] GuC submission support Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 01/51] drm/i915/guc: Add new GuC interface defines and structures Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 02/51] drm/i915/guc: Remove GuC stage descriptor, add LRC descriptor Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 03/51] drm/i915/guc: Add LRC descriptor context lookup array Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 04/51] drm/i915/guc: Implement GuC submission tasklet Matthew Brost
2021-07-19 23:01   ` John Harrison
2021-07-19 22:55     ` Matthew Brost
2021-07-20  0:26       ` John Harrison
2021-07-16 20:16 ` [Intel-gfx] [PATCH 05/51] drm/i915/guc: Add bypass tasklet submission path to GuC Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 06/51] drm/i915/guc: Implement GuC context operations for new inteface Matthew Brost
2021-07-20  0:23   ` John Harrison
2021-07-20  2:45     ` Matthew Brost
2021-07-20  0:51   ` Daniele Ceraolo Spurio
2021-07-20  4:04     ` Matthew Brost
2021-07-21 23:51       ` Daniele Ceraolo Spurio [this message]
2021-07-22  7:57         ` Michal Wajdeczko
2021-07-22 15:48           ` Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 07/51] drm/i915/guc: Insert fence on context when deregistering Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 08/51] drm/i915/guc: Defer context unpin until scheduling is disabled Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 09/51] drm/i915/guc: Disable engine barriers with GuC during unpin Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 10/51] drm/i915/guc: Extend deregistration fence to schedule disable Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 11/51] drm/i915: Disable preempt busywait when using GuC scheduling Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 12/51] drm/i915/guc: Ensure request ordering via completion fences Matthew Brost
2021-07-19 23:46   ` Daniele Ceraolo Spurio
2021-07-20  2:48     ` Matthew Brost
2021-07-20  2:50       ` Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 13/51] drm/i915/guc: Disable semaphores when using GuC scheduling Matthew Brost
2021-07-20  0:33   ` John Harrison
2021-07-16 20:16 ` [Intel-gfx] [PATCH 14/51] drm/i915/guc: Ensure G2H response has space in buffer Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 15/51] drm/i915/guc: Update intel_gt_wait_for_idle to work with GuC Matthew Brost
2021-07-20  1:03   ` John Harrison
2021-07-20  1:53     ` Matthew Brost
2021-07-20 19:49       ` John Harrison
2021-07-16 20:16 ` [Intel-gfx] [PATCH 16/51] drm/i915/guc: Update GuC debugfs to support new GuC Matthew Brost
2021-07-20  1:13   ` John Harrison
2021-07-16 20:16 ` [Intel-gfx] [PATCH 17/51] drm/i915/guc: Add several request trace points Matthew Brost
2021-07-20  1:27   ` John Harrison
2021-07-20  2:10     ` Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 18/51] drm/i915: Add intel_context tracing Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 19/51] drm/i915/guc: GuC virtual engines Matthew Brost
2021-07-19 23:33   ` Daniele Ceraolo Spurio
2021-07-19 23:27     ` Matthew Brost
2021-07-19 23:42       ` Daniele Ceraolo Spurio
2021-07-19 23:32         ` Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 20/51] drm/i915: Track 'serial' counts for " Matthew Brost
2021-07-20  1:28   ` John Harrison
2021-07-20  1:54     ` Matthew Brost
2021-07-20 16:47       ` Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 21/51] drm/i915: Hold reference to intel_context over life of i915_request Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 22/51] drm/i915/guc: Disable bonding extension with GuC submission Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 23/51] drm/i915/guc: Direct all breadcrumbs for a class to single breadcrumbs Matthew Brost
2021-07-20 19:45   ` John Harrison
2021-07-22 12:46   ` Tvrtko Ursulin
2021-07-26 22:25     ` Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 24/51] drm/i915: Add i915_sched_engine destroy vfunc Matthew Brost
2021-07-20 19:55   ` John Harrison
2021-07-20 19:53     ` Matthew Brost
2021-07-16 20:16 ` [Intel-gfx] [PATCH 25/51] drm/i915: Move active request tracking to a vfunc Matthew Brost
2021-07-20 20:05   ` John Harrison
2021-07-16 20:16 ` [Intel-gfx] [PATCH 26/51] drm/i915/guc: Reset implementation for new GuC interface Matthew Brost
2021-07-20 20:19   ` John Harrison
2021-07-20 20:59     ` Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 27/51] drm/i915: Reset GPU immediately if submission is disabled Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 28/51] drm/i915/guc: Add disable interrupts to guc sanitize Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 29/51] drm/i915/guc: Suspend/resume implementation for new interface Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 30/51] drm/i915/guc: Handle context reset notification Matthew Brost
2021-07-20 20:29   ` John Harrison
2021-07-20 20:38     ` Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 31/51] drm/i915/guc: Handle engine reset failure notification Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 32/51] drm/i915/guc: Enable the timer expired interrupt for GuC Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 33/51] drm/i915/guc: Provide mmio list to be saved/restored on engine reset Matthew Brost
2021-07-22  4:47   ` Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 34/51] drm/i915/guc: Don't complain about reset races Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 35/51] drm/i915/guc: Enable GuC engine reset Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 36/51] drm/i915/guc: Capture error state on context reset Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 37/51] drm/i915/guc: Fix for error capture after full GPU reset with GuC Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 38/51] drm/i915/guc: Hook GuC scheduling policies up Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 39/51] drm/i915/guc: Connect reset modparam updates to GuC policy flags Matthew Brost
2021-07-16 20:04   ` Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 40/51] drm/i915/guc: Include scheduling policies in the debugfs state dump Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 41/51] drm/i915/guc: Add golden context to GuC ADS Matthew Brost
2021-07-19 17:24   ` Matthew Brost
2021-07-19 18:25     ` John Harrison
2021-07-19 18:30       ` Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 42/51] drm/i915/guc: Implement banned contexts for GuC submission Matthew Brost
2021-07-20 21:41   ` John Harrison
2021-07-16 20:17 ` [Intel-gfx] [PATCH 43/51] drm/i915/guc: Support request cancellation Matthew Brost
2021-07-22 19:56   ` Daniele Ceraolo Spurio
2021-07-22 20:13     ` Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 44/51] drm/i915/selftest: Better error reporting from hangcheck selftest Matthew Brost
2021-07-16 20:13   ` Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 45/51] drm/i915/selftest: Fix workarounds selftest for GuC submission Matthew Brost
2021-07-20 17:14   ` Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 46/51] drm/i915/selftest: Fix MOCS " Matthew Brost
2021-07-16 23:57   ` Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 47/51] drm/i915/selftest: Increase some timeouts in live_requests Matthew Brost
2021-07-20 21:46   ` John Harrison
2021-07-22  8:13   ` Tvrtko Ursulin
2021-07-16 20:17 ` [Intel-gfx] [PATCH 48/51] drm/i915/selftest: Fix hangcheck self test for GuC submission Matthew Brost
2021-07-16 23:43   ` Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 49/51] drm/i915/selftest: Bump selftest timeouts for hangcheck Matthew Brost
2021-07-16 22:23   ` Matthew Brost
2021-07-22  8:17   ` Tvrtko Ursulin
2021-07-16 20:17 ` [Intel-gfx] [PATCH 50/51] drm/i915/guc: Implement GuC priority management Matthew Brost
2021-07-22 20:26   ` Daniele Ceraolo Spurio
2021-07-22 21:38     ` Matthew Brost
2021-07-22 21:50       ` Daniele Ceraolo Spurio
2021-07-22 21:55         ` Matthew Brost
2021-07-16 20:17 ` [Intel-gfx] [PATCH 51/51] drm/i915/guc: Unblock GuC submission on Gen11+ Matthew Brost
2021-07-17  1:10 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for GuC submission support (rev3) Patchwork
2021-07-19  9:06 ` [Intel-gfx] [PATCH 00/51] GuC submission support Tvrtko Ursulin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=dd809d70-1ed2-7013-7580-734ce752e87a@intel.com \
    --to=daniele.ceraolospurio@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=matthew.brost@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).