Re: [PATCH v6 2/6] drm/xe/guc: Don't store capture nodes in xe_devcoredump_snapshot

2025-02-12 Thread Teres Alexis, Alan Previn
alan: I will respin this rev with the changes mentiond below - thanks Zhanjun for the time in reviewing this. :) On Mon, 2025-02-10 at 18:41 -0500, Dong, Zhanjun wrote: > See my comments inline below. > > Regards, > Zhanjun Dong > > On 2025-01-28 1:36 p.m., Alan Previn wrote: > > GuC-Err-Captur

Re: [PATCH v7 6/6] drm/xe/guc: Update comments on GuC-Err-Capture flows

2025-02-12 Thread Teres Alexis, Alan Previn
On Tue, 2025-02-11 at 18:09 -0500, Dong, Zhanjun wrote: > > > alan:snip > >    * > > - * User Sysfs / Debugfs > > - * > > - *  --> xe_devcoredump_read-> > > + * User Devcoredump Sysfs > > + * -- > > + *  --> xe_devcoredump_read-> (user cats devcore

Re: [PATCH v7 3/6] drm/xe/guc: Split engine state print between xe_hw_engine vs xe_guc_capture

2025-02-12 Thread Teres Alexis, Alan Previn
On Tue, 2025-02-11 at 17:48 -0500, Dong, Zhanjun wrote: > > > On 2025-02-10 6:32 p.m., Alan Previn wrote: > > Relocate the xe_engine_snapshot_print function from xe_guc_capture.c > > into xe_hw_engine.c but split out the GuC-Err-Capture register printing > > portion out into a separate helper ins

Re: [PATCH V9] drm/xe/uapi: Use hint for guc to set GT frequency

2025-02-12 Thread Teres Alexis, Alan Previn
On Wed, 2025-02-12 at 17:48 +0530, Tejas Upadhyay wrote: > Allow user to provide a low latency hint. When set, KMD sends a hint > to GuC which results in special handling for that process. SLPC will > ramp the GT frequency aggressively every time it switches to this > process. > > We need to enabl

Re: [PATCH v7 5/6] drm/xe/xe_hw_engine: Update xe_hw_engine capture for debugfs/gt_reset

2025-02-11 Thread Teres Alexis, Alan Previn
On Mon, 2025-02-10 at 15:32 -0800, Teres Alexis, Alan Previn wrote: > xe_hw_engine_print is called by debugfs to do an immediate raw > --- a/drivers/gpu/drm/xe/xe_guc_capture_snapshot_types.h > +++ b/drivers/gpu/drm/xe/xe_guc_capture_snapshot_types.h > @@ -12,7 +12,11 @@ >  stru

Re: [PATCH v6 3/6] drm/xe/guc: Split engine state print between xe_hw_engine vs xe_guc_capture

2025-02-10 Thread Teres Alexis, Alan Previn
On Fri, 2025-01-31 at 10:55 -0800, Teres Alexis, Alan Previn wrote: > On Thu, 2025-01-30 at 17:42 -0500, Vivi, Rodrigo wrote: > > On Tue, Jan 28, 2025 at 10:36:49AM -0800, Alan Previn wrote: > > > > > alan:snip > > > -   if (!snapshot->matched_no

Re: [PATCH v6 3/6] drm/xe/guc: Split engine state print between xe_hw_engine vs xe_guc_capture

2025-01-31 Thread Teres Alexis, Alan Previn
On Thu, 2025-01-30 at 17:42 -0500, Vivi, Rodrigo wrote: > On Tue, Jan 28, 2025 at 10:36:49AM -0800, Alan Previn wrote: > > Relocate the xe_engine_snapshot_print function from xe_guc_capture.c > > into xe_hw_engine.c but split out the GuC-Err-Capture register printing > > portion out into a separate

Re: [PATCH v6 4/6] drm/xe/guc: Move xe_hw_engine_snapshot creation back to xe_hw_engine.c

2025-01-31 Thread Teres Alexis, Alan Previn
On Thu, 2025-01-30 at 17:43 -0500, Vivi, Rodrigo wrote: > On Tue, Jan 28, 2025 at 10:36:50AM -0800, Alan Previn wrote: alan:snip > > @@ -55,8 +55,7 @@ void xe_hw_engine_handle_irq(struct xe_hw_engine *hwe, > > u16 intr_vec); > >  void xe_hw_engine_enable_ring(struct xe_hw_engine *hwe); > >  u32 xe

Re: [PATCH v6 1/6] drm/xe/guc: Rename __guc_capture_parsed_output

2025-01-31 Thread Teres Alexis, Alan Previn
> > +++ b/drivers/gpu/drm/xe/xe_guc_capture_snapshot_types.h > > @@ -0,0 +1,53 @@ > > +/* SPDX-License-Identifier: MIT */ > > +/* > > + * Copyright © 2021-2024 Intel Corporation > > 2025 > > then > > Reviewed-by: Rodrigo Vivi > will do - thanks

Re: [PATCH v6 2/6] drm/xe/guc: Don't store capture nodes in xe_devcoredump_snapshot

2025-01-30 Thread Teres Alexis, Alan Previn
On Tue, 2025-01-28 at 10:36 -0800, Teres Alexis, Alan Previn wrote: > GuC-Err-Capture should not be storing register snapshot > nodes directly inside of the top level xe_devcoredump_snapshot > structure that it doesn't control. Furthermore, that is > is not right from a driver s

Re: [PATCH v4 0/1] Maintenence of devcoredump <-> GuC-Err-Capture plumbing

2025-01-28 Thread Teres Alexis, Alan Previn
Update: new URL with next rev that includes the split -> https://patchwork.freedesktop.org/series/144050/ basically the same squash but i decided to drop some trivial things like comments and name of function/variables. ...alan On Thu, 2025-01-23 at 18:01 +0000, Teres Alexis, Alan Pre

Re: [PATCH v4 0/1] Maintenence of devcoredump <-> GuC-Err-Capture plumbing

2025-01-23 Thread Teres Alexis, Alan Previn
On Tue, 2025-01-21 at 18:15 -0500, Vivi, Rodrigo wrote: > On Tue, Jan 21, 2025 at 11:09:34AM -0800, Alan Previn wrote: > > > A 'series' of 1 patch is not a series. Cover letter is not needed. > > However, this patch is the size of a series and it should be > split. I'm really surprised that some

Re: [PATCH 1/1] drm/xe/guc/capture: Maintenence of devcoredump <-> GuC-Err-Capture plumbing

2024-12-02 Thread Teres Alexis, Alan Previn
On Tue, 2024-11-26 at 12:09 -0500, Dong, Zhanjun wrote: > See my comments inline below: > > Regards, > Zhanjun > > On 2024-11-17 1:44 p.m., Alan Previn wrote: > > The order of the devcoredump event flow is: > > drm-scheduler -> guc-submission-execq-timed-out-job -> > > guc-submission-kill-job ->

Re: [PATCH v1] drm/i915/guc: Flush ct receive tasklet during reset preparation

2024-11-04 Thread Teres Alexis, Alan Previn
Just some minor nits on header. Otherwise, LGTM: Reviewed-by: Alan Previn On Wed, 2024-10-30 at 15:38 -0700, Zhanjun Dong wrote: > GuC to host communication is interrupt driven, the handling has 3 > parts: interrupt context, tasklet and request queue worker. > During GuC reset prepare, interrupt

Re: [PATCH v2] drm/i915/guc: Enable PXP GuC autoteardown flow

2024-09-06 Thread Teres Alexis, Alan Previn
LGTM: Reviewed-by: Alan Previn On Fri, 2024-09-06 at 10:40 -0700, john.c.harri...@intel.com wrote: > From: Juston Li > > This feature flag enables GuC autoteardown which allows for a grace > period before session teardown. > > Also add a HAS_PXP() helper to share with the other place that wan

Re: [PATCH] drm/i915/guc: Correct capture of EIR register on hang

2024-02-27 Thread Teres Alexis, Alan Previn
On Fri, 2024-02-23 at 12:32 -0800, john.c.harri...@intel.com wrote: > From: John Harrison alan:snip > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c > @@ -51,6 +51,7 @@ > { RING_ESR(0),  0,  0, "ESR" }, \ >  

Re: [PATCH v9 0/2] Resolve suspend-resume racing with GuC destroy-context-worker

2024-01-02 Thread Teres Alexis, Alan Previn
On Wed, 2023-12-27 at 20:55 -0800, Teres Alexis, Alan Previn wrote: > This series is the result of debugging issues root caused to > races between the GuC's destroyed_worker_func being triggered > vs repeating suspend-resume cycles with concurrent delayed > fence signals f

Re: [PATCH v8 2/2] drm/i915/guc: Close deregister-context race against CT-loss

2023-12-27 Thread Teres Alexis, Alan Previn
On Tue, 2023-12-26 at 10:11 -0500, Vivi, Rodrigo wrote: > On Wed, Dec 20, 2023 at 11:08:59PM +0000, Teres Alexis, Alan Previn wrote: > > On Wed, 2023-12-13 at 16:23 -0500, Vivi, Rodrigo wrote: alan:snip > > > > > > alan: Thanks Rodrigo for the RB last week, just quick

Re: [PATCH v8 2/2] drm/i915/guc: Close deregister-context race against CT-loss

2023-12-20 Thread Teres Alexis, Alan Previn
On Wed, 2023-12-13 at 16:23 -0500, Vivi, Rodrigo wrote: > On Tue, Dec 12, 2023 at 08:57:16AM -0800, Alan Previn wrote: > > If we are at the end of suspend or very early in resume > > its possible an async fence signal (via rcu_call) is triggered > > to free_engines which could lead us to the execut

Re: [PATCH v7 2/2] drm/i915/guc: Close deregister-context race against CT-loss

2023-11-30 Thread Teres Alexis, Alan Previn
> As far as i can tell, its only if we started resetting / wedging right after > this > queued worker got started. alan: hope Daniele can proof read my tracing and confirm if got it right.

Re: [PATCH v7 2/2] drm/i915/guc: Close deregister-context race against CT-loss

2023-11-30 Thread Teres Alexis, Alan Previn
On Thu, 2023-11-30 at 16:18 -0500, Vivi, Rodrigo wrote: > On Wed, Nov 29, 2023 at 04:20:13PM -0800, Alan Previn wrote: alan:snip > > + > > if (unlikely(disabled)) { > > release_guc_id(guc, ce); > > __guc_context_destroy(ce); > > - return; > > + return

Re: [PATCH v2 1/1] drm/i915/pxp: Add missing tag for Wa_14019159160

2023-11-29 Thread Teres Alexis, Alan Previn
On Wed, 2023-11-29 at 13:13 -0800, Teres Alexis, Alan Previn wrote: > On Mon, 2023-11-27 at 15:24 -0500, Vivi, Rodrigo wrote: > > On Wed, Nov 22, 2023 at 12:30:03PM -0800, Alan Previn wrote: > alan:snip > alan: thanks for reviewing and apologize for replyi

Re: [PATCH v2 1/1] drm/i915/pxp: Add missing tag for Wa_14019159160

2023-11-29 Thread Teres Alexis, Alan Previn
On Mon, 2023-11-27 at 15:24 -0500, Vivi, Rodrigo wrote: > On Wed, Nov 22, 2023 at 12:30:03PM -0800, Alan Previn wrote: alan:snip alan: thanks for reviewing and apologize for replying to this late. > > /* > > -* On MTL and newer platforms, protected contexts require setting > > -* the L

Re: [Intel-gfx] [PATCH v5] drm/i915/pxp: Add drm_dbgs for critical PXP events.

2023-11-29 Thread Teres Alexis, Alan Previn
On Fri, 2023-11-24 at 08:30 +, Tvrtko Ursulin wrote: > On 22/11/2023 19:15, Alan Previn wrote: alan:snip alan: thanks for reviewing. > > if (iir & GEN12_DISPLAY_STATE_RESET_COMPLETE_INTERRUPT) > > - pxp->session_events |= PXP_TERMINATION_COMPLETE; > > + pxp->session_even

Re: [PATCH v6 2/2] drm/i915/guc: Close deregister-context race against CT-loss

2023-11-29 Thread Teres Alexis, Alan Previn
On Mon, 2023-11-27 at 16:51 -0500, Vivi, Rodrigo wrote: alan: Firstly, thanks for taking the time to review this, knowing you have a lot on your plate right now. > alan:snip > > @@ -3301,19 +3315,38 @@ static inline void guc_lrc_desc_unpin(struct > > intel_context *ce) > > /* Seal race with

Re: [Intel-gfx] [PATCH v3 1/1] drm/i915/pxp: Add missing tag for Wa_14019159160

2023-11-29 Thread Teres Alexis, Alan Previn
On Tue, 2023-11-28 at 10:03 -0800, Roper, Matthew D wrote: > On Mon, Nov 27, 2023 at 12:11:50PM -0800, Alan Previn wrote: > > Add missing tag for "Wa_14019159160 - Case 2" (for existing > > PXP code that ensures run alone mode bit is set to allow > > PxP-decryption. alan:snip. alan: thanks for revi

Re: [PATCH v1 1/1] drm/i915/pxp: Bail early in pxp tee backend on first teardown error

2023-11-16 Thread Teres Alexis, Alan Previn
On Thu, 2023-11-16 at 15:20 -0800, Teres Alexis, Alan Previn wrote: > For Gen12 when using mei-pxp tee backend tranport, if we are coming > up from a cold boot or from a resume (not runtime resume), we can > optionally quicken the very first session cleanup that would occur > as part

Re: [PATCH v1 1/1] drm/i915/gt: Dont wait forever when idling in suspend

2023-11-14 Thread Teres Alexis, Alan Previn
On Tue, 2023-11-14 at 08:22 -0800, Teres Alexis, Alan Previn wrote: > When suspending, add a timeout when calling > intel_gt_pm_wait_for_idle else if we have a leaked > wakeref (which would be indicative of a bug elsewhere > in the driver), driver will at exit the suspend-resume > c

Re: [Intel-gfx] [PATCH v4 3/3] drm/i915/gt: Timeout when waiting for idle in suspending

2023-11-14 Thread Teres Alexis, Alan Previn
On Tue, 2023-11-14 at 17:52 +, Tvrtko Ursulin wrote: > On 14/11/2023 17:37, Teres Alexis, Alan Previn wrote: > > On Tue, 2023-11-14 at 17:27 +, Tvrtko Ursulin wrote: > > > On 13/11/2023 17:57, Teres Alexis, Alan Previn wrote: > > > > On Wed, 2023-10-25 at 13:

Re: [Intel-gfx] [PATCH v4 3/3] drm/i915/gt: Timeout when waiting for idle in suspending

2023-11-14 Thread Teres Alexis, Alan Previn
On Tue, 2023-11-14 at 12:36 -0500, Vivi, Rodrigo wrote: > On Tue, Nov 14, 2023 at 05:27:18PM +, Tvrtko Ursulin wrote: > > > > On 13/11/2023 17:57, Teres Alexis, Alan Previn wrote: > > > On Wed, 2023-10-25 at 13:58 +0100, Tvrtko Ursulin wrote: > > > > On

Re: [Intel-gfx] [PATCH v4 3/3] drm/i915/gt: Timeout when waiting for idle in suspending

2023-11-14 Thread Teres Alexis, Alan Previn
On Tue, 2023-11-14 at 17:27 +, Tvrtko Ursulin wrote: > On 13/11/2023 17:57, Teres Alexis, Alan Previn wrote: > > On Wed, 2023-10-25 at 13:58 +0100, Tvrtko Ursulin wrote: > > > On 04/10/2023 18:59, Teres Alexis, Alan Previn wrote: > > > > On Thu, 2023-09-28 at 13:

Re: [Intel-gfx] [PATCH v3] drm/i915: Skip pxp init if gt is wedged

2023-11-13 Thread Teres Alexis, Alan Previn
On Mon, 2023-11-13 at 14:49 -0800, Zhanjun Dong wrote: > The gt wedged could be triggered by missing guc firmware file, HW not > working, etc. Once triggered, it means all gt usage is dead, therefore we > can't enable pxp under this fatal error condition. > > alan:skip alan: this looks good (as p

Re: [PATCH] drm/i915: Initialize residency registers earlier

2023-11-13 Thread Teres Alexis, Alan Previn
On Mon, 2023-10-30 at 16:45 -0700, Belgaumkar, Vinay wrote: alan:skip > +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c > @@ -608,11 +608,13 @@ void intel_rc6_init(struct intel_rc6 *rc6) > /* Disable runtime-pm until we can save the GPU state with rc6 pctx */ > rpm_get(rc6); > > - if (!

Re: [Intel-gfx] [PATCH v4 3/3] drm/i915/gt: Timeout when waiting for idle in suspending

2023-11-13 Thread Teres Alexis, Alan Previn
On Wed, 2023-10-25 at 13:58 +0100, Tvrtko Ursulin wrote: > On 04/10/2023 18:59, Teres Alexis, Alan Previn wrote: > > On Thu, 2023-09-28 at 13:46 +0100, Tvrtko Ursulin wrote: > > > On 27/09/2023 17:36, Teres Alexis, Alan Previn wrote: alan:snip > > > It is not possi

Re: [Intel-gfx] [PATCH] drm/i915: Skip pxp init if gt is wedged

2023-10-31 Thread Teres Alexis, Alan Previn
On Fri, 2023-10-27 at 10:13 +0300, Jani Nikula wrote: > On Thu, 26 Oct 2023, Zhanjun Dong wrote: > alan:snip > I'll note that nobody checks intel_pxp_init() return status, so this > silently skips PXP. > > BR, > Jani. alan:snip > > + if (intel_gt_is_wedged(gt)) > > + return -ENODEV;

Re: [Intel-gfx] [PATCH v4 3/3] drm/i915/gt: Timeout when waiting for idle in suspending

2023-10-04 Thread Teres Alexis, Alan Previn
On Thu, 2023-09-28 at 13:46 +0100, Tvrtko Ursulin wrote: > On 27/09/2023 17:36, Teres Alexis, Alan Previn wrote: > > Thanks for taking the time to review this Tvrtko, replies inline below. alan:snip > > > > > > Main concern is that we need to be sure there are no possi

Re: [PATCH v4 2/3] drm/i915/guc: Close deregister-context race against CT-loss

2023-10-04 Thread Teres Alexis, Alan Previn
On Wed, 2023-10-04 at 06:34 +, Gupta, Anshuman wrote: > > > -Original Message- > > From: Teres Alexis, Alan Previn > @@ -289,6 +289,13 @@ int intel_gt_resume(struct intel_gt *gt) > > > > static void wait_for_suspend(struct intel_gt *gt) { > >

Re: [Intel-gfx] [PATCH v4 3/3] drm/i915/gt: Timeout when waiting for idle in suspending

2023-09-27 Thread Teres Alexis, Alan Previn
Thanks for taking the time to review this Tvrtko, replies inline below. On Wed, 2023-09-27 at 10:02 +0100, Tvrtko Ursulin wrote: > On 26/09/2023 20:05, Alan Previn wrote: > > When suspending, add a timeout when calling > > intel_gt_pm_wait_for_idle else if we have a lost > > G2H event that holds a

Re: [PATCH v6 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout

2023-09-17 Thread Teres Alexis, Alan Previn
On Sat, 2023-09-16 at 10:25 +0800, lkp wrote: > Hi Alan, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on cf1e91e884bb1113c653e654e9de1754fc1d4488] > > aAll errors (new ones prefixed by >>): > > alan:snip alan: missed building with PXP config after that l

Re: [PATCH v3] drm/i915/pxp: Add drm_dbgs for critical PXP events.

2023-09-15 Thread Teres Alexis, Alan Previn
On Fri, 2023-09-15 at 13:15 -0700, Teres Alexis, Alan Previn wrote: > Debugging PXP issues can't even begin without understanding precedding > sequence of important events. Add drm_dbg into the most important PXP events. > > v3 : - move gt_dbg to after mutex

Re: [PATCH v5 3/3] drm/i915/lrc: User PXP contexts requires runalone bit in lrc

2023-09-15 Thread Teres Alexis, Alan Previn
On Sat, 2023-09-09 at 15:38 -0700, Teres Alexis, Alan Previn wrote: > On Meteorlake onwards, HW specs require that all user contexts that > run on render or compute engines and require PXP must enforce > run-alone bit in lrc. Add this enforcement for protected contexts. alan:snip >

Re: [PATCH v5 2/3] drm/i915/pxp/mtl: Update pxp-firmware packet size

2023-09-15 Thread Teres Alexis, Alan Previn
On Sat, 2023-09-09 at 15:38 -0700, Teres Alexis, Alan Previn wrote: > Update the GSC-fw input/output HECI packet size to match > updated internal fw specs. > > Signed-off-by: Alan Previn > --- > drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h | 4 ++-- > 1 file change

Re: [PATCH v5 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout

2023-09-15 Thread Teres Alexis, Alan Previn
On Sat, 2023-09-09 at 15:38 -0700, Teres Alexis, Alan Previn wrote: > Update the max GSC-fw response time to match updated internal > fw specs. Because this response time is an SLA on the firmware, > not inclusive of i915->GuC->HW handoff latency, when submitting > requests

Re: [PATCH v5 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout

2023-09-15 Thread Teres Alexis, Alan Previn
On Sat, 2023-09-09 at 15:38 -0700, Teres Alexis, Alan Previn wrote: > Update the max GSC-fw response time to match updated internal > fw specs. Because this response time is an SLA on the firmware, > not inclusive of i915->GuC->HW handoff latency, when submitting > requests

Re: [PATCH v5 2/3] drm/i915/pxp/mtl: Update pxp-firmware packet size

2023-09-15 Thread Teres Alexis, Alan Previn
On Sat, 2023-09-09 at 15:38 -0700, Teres Alexis, Alan Previn wrote: > Update the GSC-fw input/output HECI packet size to match > updated internal fw specs. > > Signed-off-by: Alan Previn > alan:snip > -/* PXP-Packet sizes for MTL's GSCCS-HECI instruction */ > -#define

Re: [PATCH v1 1/1] drm/i915/pxp: Add drm_dbgs for critical PXP events.

2023-09-13 Thread Teres Alexis, Alan Previn
On Mon, 2023-09-11 at 12:26 +0300, Jani Nikula wrote: > On Wed, 06 Sep 2023, Alan Previn wrote: > > Debugging PXP issues can't even begin without understanding precedding > > sequence of events. Add drm_dbg into the most important PXP events. > > > > Signed-off-by: Alan Previn alan:snip > > >

Re: [PATCH v4 2/3] drm/i915/pxp/mtl: Update pxp-firmware packet size

2023-09-06 Thread Teres Alexis, Alan Previn
On Wed, 2023-09-06 at 17:15 -0700, Teres Alexis, Alan Previn wrote: > Update the GSC-fw input/output HECI packet size to match > updated internal fw specs. alan:snip > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h > @@ -14,8 +14,8 @@ > > +/* PXP-Packet sizes fo

Re: [PATCH v2 2/3] drm/i915/guc: Close deregister-context race against CT-loss

2023-08-28 Thread Teres Alexis, Alan Previn
the worker when !intel_guc_is_ready (ct-is-disabled). ...alan On Fri, 2023-08-25 at 11:54 -0700, Teres Alexis, Alan Previn wrote: > just a follow up note-to-self: > > On Tue, 2023-08-15 at 12:08 -0700, Teres Alexis, Alan Previn wrote: > > On Tue, 2023-08-15 at 09:56 -0400, Vivi, Ro

Re: [PATCH v2 2/3] drm/i915/guc: Close deregister-context race against CT-loss

2023-08-25 Thread Teres Alexis, Alan Previn
just a follow up note-to-self: On Tue, 2023-08-15 at 12:08 -0700, Teres Alexis, Alan Previn wrote: > On Tue, 2023-08-15 at 09:56 -0400, Vivi, Rodrigo wrote: > > On Mon, Aug 14, 2023 at 06:12:09PM -0700, Alan Previn wrote: > > > [snip] in guc_submission_send_busy_loop, we ar

Re: [PATCH v2 1/3] drm/i915/guc: Flush context destruction worker at suspend

2023-08-25 Thread Teres Alexis, Alan Previn
Thanks again Rodrigo for reviewing and apologies for my tardy replies. We are stil testing on shipping platforms and these latest patches seemed to have reduced the frequency and solved the "system hangs" while suspending but its still causing issues so we continue to debug. (issue is that its runn

Re: [PATCH v3 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout

2023-08-15 Thread Teres Alexis, Alan Previn
On Tue, 2023-08-15 at 13:29 -0700, Teres Alexis, Alan Previn wrote: > Update the max GSC-fw response time to match updated internal > fw specs. Because this response time is an SLA on the firmware, > not inclusive of i915->GuC->HW handoff latency, when submitting > requests

Re: [PATCH v2 2/3] drm/i915/guc: Close deregister-context race against CT-loss

2023-08-15 Thread Teres Alexis, Alan Previn
On Tue, 2023-08-15 at 09:56 -0400, Vivi, Rodrigo wrote: > On Mon, Aug 14, 2023 at 06:12:09PM -0700, Alan Previn wrote: > > If we are at the end of suspend or very early in resume > > its possible an async fence signal could lead us to the > > execution of the context destruction worker (after the >

Re: [PATCH v2 3/3] drm/i915/gt: Timeout when waiting for idle in suspending

2023-08-15 Thread Teres Alexis, Alan Previn
Thanks Rodrigo - agreed on everything below - will re-rev. On Tue, 2023-08-15 at 09:51 -0400, Vivi, Rodrigo wrote: > On Mon, Aug 14, 2023 at 06:12:10PM -0700, Alan Previn wrote: > > When suspending, add a timeout when calling > > intel_gt_pm_wait_for_idle else if we have a lost > > G2H event that

Re: [PATCH v2 0/3] Resolve suspend-resume racing with GuC destroy-context-worker

2023-08-14 Thread Teres Alexis, Alan Previn
On Mon, 2023-08-14 at 18:12 -0700, Teres Alexis, Alan Previn wrote: > This series is the result of debugging issues root caused to > races between the GuC's destroyed_worker_func being triggered > vs repeating suspend-resume cycles with concurrent delayed > fence signals for eng

Re: [Intel-gfx] [PATCH v1 1/3] drm/i915/guc: Flush context destruction worker at suspend

2023-08-14 Thread Teres Alexis, Alan Previn
> > > Rodrigo: And why here and not some upper layer? like in prepare > alan: wait_for_suspend does both the checking for idle as well as the > potential > wedging if guc or hw has hung at this late state. if i call from the upper > layer before this wait_for_suspend, it might be too early b

Re: [PATCH v1] drm/i915/pxp/mtl: Update gsc-heci cmd size and timeout

2023-08-10 Thread Teres Alexis, Alan Previn
On Fri, 2023-07-07 at 11:34 -0700, Teres Alexis, Alan Previn wrote: > Update the max GSC-HECI packet size and the max firmware > response timeout to match internal fw specs. > > Signed-off-by: Alan Previn I'm going to re-rev this and change the subject slightly to &quo

Re: [PATCH v1 2/3] drm/i915/guc: Close deregister-context race against CT-loss

2023-08-09 Thread Teres Alexis, Alan Previn
On Wed, 2023-08-02 at 16:35 -0700, Teres Alexis, Alan Previn wrote: > If we are at the end of suspend or very early in resume > its possible an async fence signal could lead us to the > execution of the context destruction worker (after the > prior worker flush). > alan:snip >

Re: [PATCH v1 1/3] drm/i915/guc: Flush context destruction worker at suspend

2023-08-09 Thread Teres Alexis, Alan Previn
Thanks Rodrigo for reviewing this. On Mon, 2023-08-07 at 13:52 -0400, Vivi, Rodrigo wrote: > On Wed, Aug 02, 2023 at 04:34:59PM -0700, Alan Previn wrote: > > Suspend is not like reset, it can unroll, so we have to properly > > flush pending context-guc-id deregistrations to complete before > > we

Re: [Intel-gfx] [PATCH v1 3/3] drm/i915/gt: Timeout when waiting for idle in suspending

2023-08-09 Thread Teres Alexis, Alan Previn
Thanks Rodrigo for reviewing this. On Mon, 2023-08-07 at 13:56 -0400, Vivi, Rodrigo wrote: > On Wed, Aug 02, 2023 at 04:35:01PM -0700, Alan Previn wrote: > > When suspending, add a timeout when calling > > intel_gt_pm_wait_for_idle else if we have a lost > > G2H event that holds a wakeref (which w

Re: [PATCH v4 1/1] drm/i915/pxp: Optimize GET_PARAM:PXP_STATUS

2023-08-03 Thread Teres Alexis, Alan Previn
On Wed, 2023-08-02 at 11:25 -0700, Teres Alexis, Alan Previn wrote: > After recent discussions with Mesa folks, it was requested > that we optimize i915's GET_PARAM for the PXP_STATUS without > changing the UAPI spec. > > Add these additional optimizations: >- If any

Re: [PATCH v1 0/3] Resolve suspend-resume racing with GuC destroy-context-worker

2023-08-02 Thread Teres Alexis, Alan Previn
On Wed, 2023-08-02 at 16:34 -0700, Teres Alexis, Alan Previn wrote: > This series is the result of debugging issues root caused to > races between the GuC's destroyed_worker_func being triggered vs > repeating suspend-resume cycles with concurrent delayed > fence signals f

Re: [PATCH] drm/i915/pxp/mtl: intel_pxp_init_hw needs runtime-pm inside pm-complete

2023-08-02 Thread Teres Alexis, Alan Previn
> > > > alan:snip Thanks Vinay and Daniele - i'll respin with below fix. > > @@ -48,7 +50,8 @@ void intel_pxp_resume_complete() > > if (!HAS_ENGINE(pxp->ctrl_gt, GSC0) && !pxp->pxp_component) > > return; > > > > - intel_pxp_init_hw(pxp); > > + with_intel_runtime_pm(&pxp-

Re: [PATCH v6] drm/i915/selftest/gsc: Ensure GSC Proxy init completes before selftests

2023-07-20 Thread Teres Alexis, Alan Previn
On Thu, 2023-07-20 at 14:52 -0700, Ceraolo Spurio, Daniele wrote: > > On 7/20/2023 2:40 PM, Alan Previn wrote: > > On MTL, if the GSC Proxy init flows haven't completed, submissions to the > > GSC engine will fail. Those init flows are dependent on the mei's > > gsc_proxy component that is loaded

Re: [Intel-gfx] [PATCH v4] drm/i915/selftest/gsc: Ensure GSC Proxy init completes before selftests

2023-07-12 Thread Teres Alexis, Alan Previn
On Wed, 2023-07-12 at 10:19 +0100, Tvrtko Ursulin wrote: > On 11/07/2023 23:02, Alan Previn wrote: > > On MTL, if the GSC Proxy init flows haven't completed, submissions to the > > GSC engine will fail. Those init flows are dependent on the mei's > > gsc_proxy component that is loaded in parallel w

Re: [Intel-gfx] [PATCH v3] drm/i915/selftest/gsc: Ensure GSC Proxy init completes before selftests

2023-07-11 Thread Teres Alexis, Alan Previn
On Tue, 2023-07-11 at 11:49 -0700, Ceraolo Spurio, Daniele wrote: > > > > > @@ -134,6 +193,8 @@ static int __run_selftests(const char *name, > > > >{ > > > > int err = 0; > > > > > > > > + __wait_on_all_system_dependencies(data); > > > Why does this need to be top level selft

Re: [Intel-gfx] [PATCH v3] drm/i915/selftest/gsc: Ensure GSC Proxy init completes before selftests

2023-07-11 Thread Teres Alexis, Alan Previn
Thanks fore reviewing Tvrtko, below are my responses. I'll rerev without generalized func ptr and only for the subtests that need it. ...alan On Thu, 2023-06-29 at 22:44 +0100, Tvrtko Ursulin wrote: > On 29/06/2023 21:42, Alan Previn wrote: > > On MTL, if the GSC Proxy init flows haven't completed

Re: [Intel-gfx] [PATCH] drm/i915/pxp: Optimize GET_PARAM:PXP_STATUS

2023-06-29 Thread Teres Alexis, Alan Previn
On Tue, 2023-06-20 at 09:30 -0500, Balasubrawmanian, Vivaik wrote: > On 6/1/2023 12:45 PM, Alan Previn wrote: > > After recent discussions with Mesa folks, it was requested > > that we optimize i915's GET_PARAM for the PXP_STATUS without > > changing the UAPI spec. > > > > This patch adds this add

Re: [PATCH v3] drm/xe/guc: Fix h2g_write usage of GUC_CTB_MSG_MAX_LEN

2023-06-29 Thread Teres Alexis, Alan Previn
On Wed, 2023-06-28 at 21:44 +, Brost, Matthew wrote: > On Wed, Jun 28, 2023 at 11:17:18AM -0700, Alan Previn wrote: > > In the ABI header, GUC_CTB_MSG_MIN_LEN is '1' because > > GUC_CTB_HDR_LEN is 1. This aligns with H2G/G2H CTB specification > > where all command formats are defined in units o

Re: [PATCH v2 3/5] drm/i915/mtl/gsc: query the GSC FW for its compatibility version

2023-06-08 Thread Teres Alexis, Alan Previn
On Mon, 2023-06-05 at 19:24 -0700, Ceraolo Spurio, Daniele wrote: > The compatibility version is queried via an MKHI command. Right now, the > only existing interface is 1.0 > This is basically the interface version for the GSC FW, so the plan is > to use it as the main tracked version, including f

Re: [PATCH v1] drm/i915/gsc: take a wakeref for the proxy-init-completion check

2023-06-08 Thread Teres Alexis, Alan Previn
On Thu, 2023-06-08 at 11:19 -0700, Ceraolo Spurio, Daniele wrote: > On 6/8/2023 11:04 AM, Alan Previn wrote: > > Ensure intel_gsc_uc_fw_init_done and intel_gsc_uc_fw_proxy_init > > takes a wakeref before reading GSC Shim registers. alan:snip > > > bool intel_gsc_uc_fw_proxy_init_done(struct inte

Re: [Intel-xe] [v1] drm/xe/guc: Fix h2g_write usage of GUC_CTB_MSG_MAX_LEN

2023-06-08 Thread Teres Alexis, Alan Previn
On Thu, 2023-06-08 at 00:12 +, Teres Alexis, Alan Previn wrote: > On Fri, 2023-06-02 at 11:52 -0700, Alan Previn wrote: > > alan: good point - i will go back and find if we have this value internally > spec'd before we continue. alan: actually, even if the spec allowed ve

Re: [PATCH v2 2/5] drm/i915/mtl/gsc: extract release and security versions from the gsc binary

2023-06-08 Thread Teres Alexis, Alan Previn
Everything looks good to me, so Reviewed-by: Alan Previn On Mon, 2023-06-05 at 19:23 -0700, Ceraolo Spurio, Daniele wrote: > The release and security versions of the GSC binary are not used at > runtime to decide interface compatibility (there is a separate version > for that), but they're still

Re: [v2] drm/i915/selftest/gsc: Ensure GSC Proxy init completes before selftests

2023-06-08 Thread Teres Alexis, Alan Previn
On Thu, 2023-06-08 at 18:14 +, Dong, Zhanjun wrote: > See my comments below. > > > -Original Message- > > From: Alan Previn alan:snip > > +static int > > +__wait_gsc_proxy_completed(struct drm_i915_private *i915, > > + unsigned long timeout_ms) > > +{ > > + boo

Re: [v1] drm/xe/guc: Fix h2g_write usage of GUC_CTB_MSG_MAX_LEN

2023-06-07 Thread Teres Alexis, Alan Previn
On Fri, 2023-06-02 at 11:52 -0700, Alan Previn wrote: > In the ABI header, GUC_CTB_MSG_MIN_LEN is '1' because > GUC_CTB_HDR_LEN is 1. This aligns with H2G/G2H CTB specification > where all command formats are defined in units of dwords so that '1' > is a dword. Accordingly, GUC_CTB_MSG_MAX_LEN is 2

Re: [PATCH v3] drm/i915/mtl/gsc: Add a gsc_info debugfs

2023-06-07 Thread Teres Alexis, Alan Previn
On Mon, 2023-06-05 at 21:32 -0700, Ceraolo Spurio, Daniele wrote: > Add a new debugfs to dump information about the GSC. This includes: > > - the FW path and SW tracking status; > - the release, security and compatibility versions; > - the HECI1 status registers. > > Note that those are the same

Re: [PATCH v2 1/5] drm/i915/gsc: fixes and updates for GSC memory allocation

2023-06-07 Thread Teres Alexis, Alan Previn
On Mon, 2023-06-05 at 19:23 -0700, Ceraolo Spurio, Daniele wrote: > A few fixes/updates are required around the GSC memory allocation and it > is easier to do them all at the same time. The changes are as follows: > > 1 - Switch the memory allocation to stolen memory. We need to avoid > accesses f

Re: [PATCH 2/6] drm/i915/uc/gsc: fixes and updates for GSC memory allocation

2023-06-05 Thread Teres Alexis, Alan Previn
On Tue, 2023-05-23 at 08:21 -0700, Ceraolo Spurio, Daniele wrote: > > > > > > +static int gsc_allocate_and_map_vma(struct intel_gsc_uc *gsc, u32 size) > > alan:snip > > > + obj = i915_gem_object_create_stolen(gt->i915, s0ize); > > > + if (IS_ERR(obj)) > > > + return PTR_ERR(obj); > > > +

Re: [PATCH 3/6] drm/i915/uc/gsc: extract release and security versions from the gsc binary

2023-06-05 Thread Teres Alexis, Alan Previn
On Fri, 2023-05-26 at 18:27 -0700, Ceraolo Spurio, Daniele wrote: > > > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_meu_headers.h > > > b/drivers/gpu/drm/i915/gt/uc/intel_gsc_meu_headers.h > > > index d55a66202576..8bce2b8aed84 100644 > > > --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_meu_h

Re: [PATCH 6/6] drm/i915/uc/gsc: Add a gsc_info debugfs

2023-06-05 Thread Teres Alexis, Alan Previn
On Wed, 2023-05-31 at 17:25 -0700, Ceraolo Spurio, Daniele wrote: > > On 5/26/2023 3:57 PM, Teres Alexis, Alan Previn wrote: > > On Fri, 2023-05-05 at 09:04 -0700, Ceraolo Spurio, Daniele wrote: > > > Add a new debugfs to dump information about the GSC. This includes: >

Re: [Intel-gfx] [PATCH] drm/i915/pxp: Optimize GET_PARAM:PXP_STATUS

2023-06-02 Thread Teres Alexis, Alan Previn
Thanks Jani - will rev this up and fix these. On Fri, 2023-06-02 at 16:03 +0300, Jani Nikula wrote: > On Thu, 01 Jun 2023, Alan Previn wrote: > > After recent discussions with Mesa folks, it was requested > > that we optimize i915's GET_PARAM for the PXP_STATUS without > > changing the UAPI spec.

Re: [PATCH v5 5/7] drm/i915/mtl/huc: auth HuC via GSC

2023-06-01 Thread Teres Alexis, Alan Previn
On Wed, 2023-05-31 at 16:54 -0700, Ceraolo Spurio, Daniele wrote: > The full authentication via the GSC requires an heci packet submission > to the GSC FW via the GSC CS. The GSC has new PXP command for this > (literally called NEW_HUC_AUTH). > The intel_huc_auth function is also updated to handle

Re: [Intel-gfx] [PATCH] drm/i915/pxp: use correct format string for size_t

2023-06-01 Thread Teres Alexis, Alan Previn
On Thu, 2023-06-01 at 23:36 +0200, Arnd Bergmann wrote: > From: Arnd Bergmann > > While 'unsigned long' needs the %ld format string, size_t needs the %z > modifier: alan:snip > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c > @@ -143,7 +143,7 @@ gsccs_send_message(struct intel_pxp *pxp, >

Re: [PATCH v3 5/7] drm/i915/mtl/huc: auth HuC via GSC

2023-05-30 Thread Teres Alexis, Alan Previn
On Fri, 2023-05-26 at 17:52 -0700, Ceraolo Spurio, Daniele wrote: > The full authentication via the GSC requires an heci packet submission > to the GSC FW via the GSC CS. The GSC has new PXP command for this > (literally called NEW_HUC_AUTH). > The intel_huc_auth fuction is also updated to handle b

Re: [PATCH 6/6] drm/i915/uc/gsc: Add a gsc_info debugfs

2023-05-26 Thread Teres Alexis, Alan Previn
On Fri, 2023-05-05 at 09:04 -0700, Ceraolo Spurio, Daniele wrote: > Add a new debugfs to dump information about the GSC. This includes: alan:snip Actually everything looks good except for a couple of questions + asks - hope we can close on this patch in next rev. > > - the FW path and SW trackin

Re: [PATCH 5/6] drm/i915/uc/gsc: define gsc fw

2023-05-25 Thread Teres Alexis, Alan Previn
Considering the only request i have below is touching up of existing comments (as far as this patch is concerned), and since the rest of the code looks good, here is my R-b - but i hope you can anwser my newbie question at the bottom: Reviewed-by: Alan Previn On Fri, 2023-05-05 at 09:04 -0700,

Re: [PATCH 4/6] drm/i915/uc/gsc: query the GSC FW for its compatibility version

2023-05-25 Thread Teres Alexis, Alan Previn
On Fri, 2023-05-05 at 09:04 -0700, Ceraolo Spurio, Daniele wrote: > The compatibility version is queried via an MKHI command. Right now, the > only existing interface is 1.0 > This is basically the interface version for the GSC FW, so the plan is > to use it as the main tracked version, including f

Re: [PATCH 3/6] drm/i915/uc/gsc: extract release and security versions from the gsc binary

2023-05-25 Thread Teres Alexis, Alan Previn
On Thu, 2023-05-25 at 09:56 -0700, Ceraolo Spurio, Daniele wrote: > On 5/24/2023 10:14 PM, Teres Alexis, Alan Previn wrote: > > On Fri, 2023-05-05 at 09:04 -0700, Ceraolo Spurio, Daniele wrote: alan:snip > > > --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.h > > > +++ b

Re: [PATCH 3/6] drm/i915/uc/gsc: extract release and security versions from the gsc binary

2023-05-24 Thread Teres Alexis, Alan Previn
On Fri, 2023-05-05 at 09:04 -0700, Ceraolo Spurio, Daniele wrote: alan: snip > +int intel_gsc_fw_get_binary_info(struct intel_uc_fw *gsc_fw, const void > *data, size_t size) > +{ alan:snip > + /* > + * The GSC binary starts with the pointer layout, which contains the > + * locati

Re: [PATCH 2/6] drm/i915/uc/gsc: fixes and updates for GSC memory allocation

2023-05-22 Thread Teres Alexis, Alan Previn
On Fri, 2023-05-05 at 09:04 -0700, Ceraolo Spurio, Daniele wrote: > A few fixes/updates are required around the GSC memory allocation and it > is easier to do them all at the same time. The changes are as follows: alan:snip > @@ -109,38 +110,21 @@ static int gsc_fw_load_prepare(struct intel_gsc_

Re: [PATCH v2 5/8] drm/i915/huc: differentiate the 2 steps of the MTL HuC auth flow

2023-05-12 Thread Teres Alexis, Alan Previn
On Fri, 2023-04-28 at 11:58 -0700, Ceraolo Spurio, Daniele wrote: > Before we add the second step of the MTL HuC auth (via GSC), we need to > have the ability to differentiate between them. To do so, the huc > authentication check is duplicated for GuC and GSC auth, with meu > binaries being consid

Re: [PATCH v2] drm/i915/huc: Parse the GSC-enabled HuC binary

2023-05-12 Thread Teres Alexis, Alan Previn
On Tue, 2023-05-02 at 08:27 -0700, Ceraolo Spurio, Daniele wrote: > The new binaries that support the 2-step authentication have contain the > legacy-style binary, which we can use for loading the HuC via DMA. To > find out where this is located in the image, we need to parse the meu > manifest of

Re: [Intel-gfx] [PATCH] drm/i915/guc: Fix confused register capture list creation

2023-05-11 Thread Teres Alexis, Alan Previn
On Thu, 2023-05-11 at 18:35 -0700, john.c.harri...@intel.com wrote: > From: John Harrison > > The GuC has a completely separate engine class enum when referring to > register capture lists, which combines render and compute. The driver > was using the 'normal' GuC specific engine class enum inste

Re: [Intel-gfx] [PATCH v9 6/8] drm/i915/uapi/pxp: Add a GET_PARAM for PXP

2023-05-10 Thread Teres Alexis, Alan Previn
alan:snip > This is why I asked if it was it was "basically certain that in a > production environment, then it will eventually return 1 meaning it's > ready". Alan's response was a little ambiguous on this point. alan: if we get a '2' and never transition to '1' - thats a kernel bug or firmware

Re: [PATCH v9 6/8] drm/i915/uapi/pxp: Add a GET_PARAM for PXP

2023-05-04 Thread Teres Alexis, Alan Previn
On Thu, 2023-04-27 at 16:48 -0700, Teres Alexis, Alan Previn wrote: > Because of the additional firmware, component-driver and > initialization depedencies required on MTL platform before a > PXP context can be created, UMD calling for PXP creation as a > way to get-caps can take a l

Re: [PATCH v3 4/4] drm/i915/gsc: add support for GSC proxy interrupt

2023-05-03 Thread Teres Alexis, Alan Previn
On Tue, 2023-05-02 at 09:38 -0700, Ceraolo Spurio, Daniele wrote: > The GSC notifies us of a proxy request via the HECI2 interrupt. The > interrupt must be enabled both in the HECI layer and in our usual gt irq > programming; for the latter, the interrupt is enabled via the same enable > register a

Re: [PATCH v3 3/4] drm/i915/gsc: add initial support for GSC proxy

2023-05-03 Thread Teres Alexis, Alan Previn
On Tue, 2023-05-02 at 09:38 -0700, Ceraolo Spurio, Daniele wrote: > The GSC uC needs to communicate with the CSME to perform certain > operations. Since the GSC can't perform this communication directly > on platforms where it is integrated in GT, i915 needs to transfer the > messages from GSC to

Re: [PATCH v3 2/4] mei: gsc_proxy: add gsc proxy driver

2023-05-03 Thread Teres Alexis, Alan Previn
We only had nits before and all sorted now, so.. Reviewed-by: Alan Previn On Tue, 2023-05-02 at 09:38 -0700, Ceraolo Spurio, Daniele wrote: > From: Alexander Usyskin > > Add GSC proxy driver. It to allows messaging between GSC component > on Intel graphics card and CSE device. > > Cc: Alan Pre

Re: [PATCH v3 1/4] drm/i915/mtl: Define GSC Proxy component interface

2023-05-03 Thread Teres Alexis, Alan Previn
LGTM Reviewed-by: Alan Previn On Tue, 2023-05-02 at 09:38 -0700, Ceraolo Spurio, Daniele wrote: > From: Alexander Usyskin > > GSC Proxy component is used for communication between the > Intel graphics driver and MEI driver. > > Cc: Alan Previn > Signed-off-by: Alexander Usyskin > Signed-off-

Re: [Intel-gfx] [PATCH v2 3/4] drm/i915/guc: Capture list naming clean up

2023-05-03 Thread Teres Alexis, Alan Previn
LGTM: Reviewed-by: Alan Previn On Fri, 2023-04-28 at 11:56 -0700, john.c.harri...@intel.com wrote: > From: John Harrison > > Don't use 'xe_lp*' prefixes for register lists that are common with > Gen8. > > Don't add Xe only GSC registers to pre-Xe devices that don't > even have a GSC engine. >

Re: [PATCH v9 3/8] drm/i915/pxp: Add MTL helpers to submit Heci-Cmd-Packet to GSC

2023-04-27 Thread Teres Alexis, Alan Previn
On Thu, 2023-04-27 at 16:48 -0700, Teres Alexis, Alan Previn wrote: > Add helper functions into a new file for heci-packet-submission. > The helpers will handle generating the MTL GSC-CS Memory-Header > and submission of the Heci-Cmd-Packet instructions to the engine. > > alan:

  1   2   3   >