All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Summers, Stuart" <stuart.summers@intel.com>
To: "intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>,
	"Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>
Cc: "Brost, Matthew" <matthew.brost@intel.com>,
	"Upadhyay, Tejas" <tejas.upadhyay@intel.com>,
	"Yadav, Arvind" <arvind.yadav@intel.com>,
	"thomas.hellstrom@linux.intel.com"
	<thomas.hellstrom@linux.intel.com>
Subject: Re: [RFC 13/15] drm/xe/lrc: Pass exec_queue to xe_lrc_create for access counter params
Date: Wed, 22 Apr 2026 20:50:45 +0000	[thread overview]
Message-ID: <80d9d30ec9a3554498d8a76c894ceb782dffa970.camel@intel.com> (raw)
In-Reply-To: <20260318074456.2839499-14-himal.prasad.ghimiray@intel.com>

On Wed, 2026-03-18 at 13:14 +0530, Himal Prasad Ghimiray wrote:
> Update xe_lrc_create() and xe_lrc_init() to receive exec_queue
> pointer,
> enabling access to q->acc.{trigger, notify, granularity} during LRC
> initialization. Program CTX_ACC_CTR_THOLD and CTX_ASID registers with
> access counter values.
> 
> Bspec: 59264, 59265
> Signed-off-by: Himal Prasad Ghimiray
> <himal.prasad.ghimiray@intel.com>
> ---
>  drivers/gpu/drm/xe/regs/xe_lrc_layout.h | 10 +++++++
>  drivers/gpu/drm/xe/xe_exec_queue.c      |  2 +-
>  drivers/gpu/drm/xe/xe_execlist.c        |  2 +-
>  drivers/gpu/drm/xe/xe_lrc.c             | 35 ++++++++++++++++++-----
> --
>  drivers/gpu/drm/xe/xe_lrc.h             |  5 ++--
>  5 files changed, 41 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/regs/xe_lrc_layout.h
> b/drivers/gpu/drm/xe/regs/xe_lrc_layout.h
> index b5eff383902c..35ee070059dd 100644
> --- a/drivers/gpu/drm/xe/regs/xe_lrc_layout.h
> +++ b/drivers/gpu/drm/xe/regs/xe_lrc_layout.h
> @@ -40,4 +40,14 @@
>  #define INDIRECT_CTX_RING_START_UDW    (0x08 + 1)
>  #define INDIRECT_CTX_RING_CTL          (0x0a + 1)
>  
> +/* Fields for CTX_ACC_CTR_THOLD */
> +#define ACC_TRIGGER_MASK               REG_GENMASK(15, 0)
> +#define
> ACC_TRIGGER_VAL(x)             REG_FIELD_PREP(ACC_TRIGGER_MASK, x)
> +#define ACC_NOTIFY_MASK                        REG_GENMASK(31, 16)
> +#define
> ACC_NOTIFY_VAL(x)              REG_FIELD_PREP(ACC_NOTIFY_MASK, x)
> +
> +/* Fields for CTX_ASID */
> +#define ACC_GRANULARITY_MASK            REG_GENMASK(22, 20)
> +#define ACC_GRANULARITY_VAL(x)         
> REG_FIELD_PREP(ACC_GRANULARITY_MASK, x)
> +
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c
> b/drivers/gpu/drm/xe/xe_exec_queue.c
> index 815e82011c6d..05c808b330b4 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> @@ -379,7 +379,7 @@ static int __xe_exec_queue_init(struct
> xe_exec_queue *q, u32 exec_queue_flags)
>  
>                         marker = xe_gt_sriov_vf_wait_valid_ggtt(q-
> >gt);
>  
> -                       lrc = xe_lrc_create(q->hwe, q->vm, q-
> >replay_state,
> +                       lrc = xe_lrc_create(q, q->hwe, q->vm,

It looks like the majority of cases here are passing hwe in from the q
directly and not something separate - the one case I see where that
isn't the case is in the execlist code...

Can we drop the q->hwe parameter and just get that from the q itself in
xe_lrc_create() now?

> q->replay_state,
>                                             xe_lrc_ring_size(), q-
> >msix_vec, flags);
>                         if (IS_ERR(lrc)) {
>                                 err = PTR_ERR(lrc);
> diff --git a/drivers/gpu/drm/xe/xe_execlist.c
> b/drivers/gpu/drm/xe/xe_execlist.c
> index 7e8a3a7db741..05de991ae90a 100644
> --- a/drivers/gpu/drm/xe/xe_execlist.c
> +++ b/drivers/gpu/drm/xe/xe_execlist.c
> @@ -268,7 +268,7 @@ struct xe_execlist_port
> *xe_execlist_port_create(struct xe_device *xe,
>  
>         port->hwe = hwe;
>  
> -       port->lrc = xe_lrc_create(hwe, NULL, NULL, SZ_16K,
> XE_IRQ_DEFAULT_MSIX, 0);
> +       port->lrc = xe_lrc_create(NULL, hwe, NULL, NULL, SZ_16K,
> XE_IRQ_DEFAULT_MSIX, 0);
>         if (IS_ERR(port->lrc)) {
>                 err = PTR_ERR(port->lrc);
>                 goto err;
> diff --git a/drivers/gpu/drm/xe/xe_lrc.c
> b/drivers/gpu/drm/xe/xe_lrc.c
> index 35b365ac55e5..9cb91ce63d59 100644
> --- a/drivers/gpu/drm/xe/xe_lrc.c
> +++ b/drivers/gpu/drm/xe/xe_lrc.c
> @@ -1439,8 +1439,10 @@ void xe_lrc_set_multi_queue_priority(struct
> xe_lrc *lrc, enum xe_multi_queue_pri
>         lrc->desc |= FIELD_PREP(LRC_PRIORITY,
> xe_multi_queue_prio_to_lrc(lrc, priority));
>  }
>  
> -static int xe_lrc_ctx_init(struct xe_lrc *lrc, struct xe_hw_engine
> *hwe, struct xe_vm *vm,
> -                          void *replay_state, u16 msix_vec, u32
> init_flags)
> +static int xe_lrc_ctx_init(struct xe_lrc *lrc, struct xe_exec_queue
> *q,
> +                          struct xe_hw_engine *hwe, struct xe_vm
> *vm,
> +                          void *replay_state,
> +                          u16 msix_vec, u32 init_flags)
>  {
>         struct xe_gt *gt = hwe->gt;
>         struct xe_tile *tile = gt_to_tile(gt);
> @@ -1527,8 +1529,20 @@ static int xe_lrc_ctx_init(struct xe_lrc *lrc,
> struct xe_hw_engine *hwe, struct
>         if (lrc_to_xe(lrc)->info.has_64bit_timestamp)
>                 xe_lrc_write_ctx_reg(lrc, CTX_TIMESTAMP_UDW, 0);
>  
> -       if (xe->info.has_asid && vm)
> -               xe_lrc_write_ctx_reg(lrc, CTX_ASID, vm->usm.asid);
> +       if (xe->info.has_asid && vm) {
> +               u32 asid;
> +
> +               if (q)
> +                       asid = vm->usm.asid | ACC_GRANULARITY_VAL(q-
> >acc.granularity);
> +               else
> +                       asid = vm->usm.asid;

Seems like something we could inline here like:
asid = vm->usm.asid | q ? ACC_GRANULARITY_VAL(q->acc.granularity) : 0;

But maybe it's just as clear the way you have it...

Thanks,
Stuart

> +               xe_lrc_write_ctx_reg(lrc, CTX_ASID, asid);
> +       }
> +
> +       if (q && xe->info.has_access_counter && vm)
> +               xe_lrc_write_ctx_reg(lrc, CTX_ACC_CTR_THOLD,
> +                                    ACC_NOTIFY_VAL(q->acc.notify) |
> +                                    ACC_TRIGGER_VAL(q-
> >acc.trigger));
>  
>         lrc->desc = LRC_VALID;
>         lrc->desc |= FIELD_PREP(LRC_ADDRESSING_MODE,
> LRC_LEGACY_64B_CONTEXT);
> @@ -1570,7 +1584,8 @@ static int xe_lrc_ctx_init(struct xe_lrc *lrc,
> struct xe_hw_engine *hwe, struct
>         return err;
>  }
>  
> -static int xe_lrc_init(struct xe_lrc *lrc, struct xe_hw_engine *hwe,
> struct xe_vm *vm,
> +static int xe_lrc_init(struct xe_lrc *lrc, struct xe_exec_queue *q,
> +                      struct xe_hw_engine *hwe, struct xe_vm *vm,
>                        void *replay_state, u32 ring_size, u16
> msix_vec, u32 init_flags)
>  {
>         struct xe_gt *gt = hwe->gt;
> @@ -1626,7 +1641,7 @@ static int xe_lrc_init(struct xe_lrc *lrc,
> struct xe_hw_engine *hwe, struct xe_v
>         xe_hw_fence_ctx_init(&lrc->fence_ctx, hwe->gt,
>                              hwe->fence_irq, hwe->name);
>  
> -       err = xe_lrc_ctx_init(lrc, hwe, vm, replay_state, msix_vec,
> init_flags);
> +       err = xe_lrc_ctx_init(lrc, q, hwe, vm, replay_state,
> msix_vec, init_flags);
>         if (err)
>                 goto err_lrc_finish;
>  
> @@ -1642,6 +1657,7 @@ static int xe_lrc_init(struct xe_lrc *lrc,
> struct xe_hw_engine *hwe, struct xe_v
>  
>  /**
>   * xe_lrc_create - Create a LRC
> + * @q: Exec queue (can be NULL for kernel queues)
>   * @hwe: Hardware Engine
>   * @vm: The VM (address space)
>   * @replay_state: GPU hang replay state
> @@ -1654,8 +1670,9 @@ static int xe_lrc_init(struct xe_lrc *lrc,
> struct xe_hw_engine *hwe, struct xe_v
>   * Return pointer to created LRC upon success and an error pointer
>   * upon failure.
>   */
> -struct xe_lrc *xe_lrc_create(struct xe_hw_engine *hwe, struct xe_vm
> *vm,
> -                            void *replay_state, u32 ring_size, u16
> msix_vec, u32 flags)
> +struct xe_lrc *xe_lrc_create(struct xe_exec_queue *q, struct
> xe_hw_engine *hwe,
> +                            struct xe_vm *vm, void *replay_state,
> u32 ring_size,
> +                            u16 msix_vec, u32 flags)
>  {
>         struct xe_lrc *lrc;
>         int err;
> @@ -1664,7 +1681,7 @@ struct xe_lrc *xe_lrc_create(struct
> xe_hw_engine *hwe, struct xe_vm *vm,
>         if (!lrc)
>                 return ERR_PTR(-ENOMEM);
>  
> -       err = xe_lrc_init(lrc, hwe, vm, replay_state, ring_size,
> msix_vec, flags);
> +       err = xe_lrc_init(lrc, q, hwe, vm, replay_state, ring_size,
> msix_vec, flags);
>         if (err) {
>                 kfree(lrc);
>                 return ERR_PTR(err);
> diff --git a/drivers/gpu/drm/xe/xe_lrc.h
> b/drivers/gpu/drm/xe/xe_lrc.h
> index e7c975f9e2d9..b101aabe3d0d 100644
> --- a/drivers/gpu/drm/xe/xe_lrc.h
> +++ b/drivers/gpu/drm/xe/xe_lrc.h
> @@ -51,8 +51,9 @@ struct xe_lrc_snapshot {
>  #define XE_LRC_CREATE_USER_CTX         BIT(2)
>  #define XE_LRC_DISABLE_STATE_CACHE_PERF_FIX    BIT(3)
>  
> -struct xe_lrc *xe_lrc_create(struct xe_hw_engine *hwe, struct xe_vm
> *vm,
> -                            void *replay_state, u32 ring_size, u16
> msix_vec, u32 flags);
> +struct xe_lrc *xe_lrc_create(struct xe_exec_queue *q, struct
> xe_hw_engine *hwe,
> +                            struct xe_vm *vm, void *replay_state,
> +                            u32 ring_size, u16 msix_vec, u32 flags);
>  void xe_lrc_destroy(struct kref *ref);
>  
>  /**


  reply	other threads:[~2026-04-22 20:50 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-18  7:44 [RFC 00/15] drm/xe: Access counter consumer layer Himal Prasad Ghimiray
2026-03-18  7:34 ` ✗ CI.checkpatch: warning for " Patchwork
2026-03-18  7:35 ` ✗ CI.KUnit: failure " Patchwork
2026-03-18  7:44 ` [RFC 01/15] drm/xe: Add xe_usm_queue generic USM circular buffer Himal Prasad Ghimiray
2026-04-01 21:28   ` Matthew Brost
2026-04-06  4:46     ` Ghimiray, Himal Prasad
2026-03-18  7:44 ` [RFC 02/15] drm/xe/pagefault: Use xe_usm_queue helpers Himal Prasad Ghimiray
2026-03-18  7:44 ` [RFC 03/15] drm/xe: Stub out new access_counter layer Himal Prasad Ghimiray
2026-04-02 21:46   ` Matthew Brost
2026-04-06  5:28     ` Ghimiray, Himal Prasad
2026-04-14 20:06   ` Summers, Stuart
2026-03-18  7:44 ` [RFC 04/15] drm/xe: Implement xe_access_counter_init Himal Prasad Ghimiray
2026-03-18  7:44 ` [RFC 05/15] drm/xe: Implement xe_access_counter_handler Himal Prasad Ghimiray
2026-04-03  2:06   ` Matthew Brost
2026-03-18  7:44 ` [RFC 06/15] drm/xe: Extract xe_vma_lock_and_validate helper Himal Prasad Ghimiray
2026-04-01 22:03   ` Matthew Brost
2026-03-18  7:44 ` [RFC 07/15] drm/xe: Move ASID to FAULT VM lookup to xe_device Himal Prasad Ghimiray
2026-04-02 21:50   ` Matthew Brost
2026-04-07  6:41     ` Ghimiray, Himal Prasad
2026-03-18  7:44 ` [RFC 08/15] drm/xe: Implement xe_access_counter_queue_work Himal Prasad Ghimiray
2026-04-01 21:10   ` Matthew Brost
2026-04-01 22:01     ` Matthew Brost
2026-04-01 22:11   ` Matthew Brost
2026-04-02 22:06   ` Matthew Brost
2026-04-22 20:35   ` Summers, Stuart
2026-03-18  7:44 ` [RFC 09/15] drm/xe/trace: Add xe_vma_acc trace event for access counter notifications Himal Prasad Ghimiray
2026-04-03  1:01   ` Matthew Brost
2026-03-18  7:44 ` [RFC 10/15] drm/xe/svm: Handle svm ranges on access ctr trigger Himal Prasad Ghimiray
2026-04-03  0:25   ` Matthew Brost
2026-03-18  7:44 ` [RFC 11/15] drm/xe: Add xe_guc_access_counter layer Himal Prasad Ghimiray
2026-04-02 21:27   ` Matthew Brost
2026-04-14 21:24   ` Summers, Stuart
2026-04-22 20:38   ` Summers, Stuart
2026-03-18  7:44 ` [RFC 12/15] drm/xe/uapi: Add access counter parameter extension for exec queue Himal Prasad Ghimiray
2026-03-24 14:25   ` Francois Dugast
2026-04-01 14:46     ` Matthew Brost
2026-04-01 16:36       ` Ghimiray, Himal Prasad
2026-03-18  7:44 ` [RFC 13/15] drm/xe/lrc: Pass exec_queue to xe_lrc_create for access counter params Himal Prasad Ghimiray
2026-04-22 20:50   ` Summers, Stuart [this message]
2026-03-18  7:44 ` [RFC 14/15] drm/xe/vm: Add xe_vma_supports_access_ctr() helper Himal Prasad Ghimiray
2026-04-22 20:54   ` Summers, Stuart
2026-03-18  7:44 ` [RFC 15/15] drm/xe/pt: Set NC PTE bit for VMAs ineligible for access counting Himal Prasad Ghimiray
2026-04-03  0:09   ` Matthew Brost

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=80d9d30ec9a3554498d8a76c894ceb782dffa970.camel@intel.com \
    --to=stuart.summers@intel.com \
    --cc=arvind.yadav@intel.com \
    --cc=himal.prasad.ghimiray@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.brost@intel.com \
    --cc=tejas.upadhyay@intel.com \
    --cc=thomas.hellstrom@linux.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.