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 for en

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

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; > > +

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

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; > > +

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

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

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 &g

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

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 po

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

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

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

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 block

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 PXP4

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, w

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

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 engine-f

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

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 "Upd

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

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 PXP i

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 for en

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); > > +

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

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

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

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

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

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

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

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 ver

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) > > +{ > > +

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

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

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 > > > ---

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

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

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

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

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

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 > + *

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

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

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

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

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

2023-05-04 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

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

2023-05-04 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

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 >

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:

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

2023-04-27 Thread Teres Alexis, Alan Previn
(fixed email addresses again - why is my Evolution client deteorating??) On Thu, 2023-04-27 at 17:18 +, Teres Alexis, Alan Previn wrote: > On Wed, 2023-04-26 at 15:35 -0700, Justen, Jordan L wrote: > > On 2023-04-26 11:17:16, Teres Alexis, Alan Previn wrote: > alan:snip >

Re: [Intel-gfx] [PATCH] drm/i915/guc: Fix error capture for virtual engines

2023-04-27 Thread Teres Alexis, Alan Previn
On Fri, 2023-04-14 at 17:27 -0700, john.c.harri...@intel.com wrote: > From: John Harrison > > GuC based register dumps in error capture logs were basically broken > for virtual engines. This can be seen in igt@gem_exec_balancer@hang: > [IGT] gem_exec_balancer: starting subtest hang > [drm]

Re: [Intel-gfx] [PATCH 6/6] drm/i915/guc: Capture list clean up - 5

2023-04-26 Thread Teres Alexis, Alan Previn
On Wed, 2023-04-26 at 11:24 -0700, john.c.harri...@intel.com wrote: > From: John Harrison > > Rename the 'default_' register list prefix to 'gen8_' as that is the > more accurate name. > > Signed-off-by: John Harrison Reviewed-by: Alan Previn

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

2023-04-26 Thread Teres Alexis, Alan Previn
On Thu, 2023-04-20 at 22:34 -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

Re: IOCTL feature detection (Was: Re: [Intel-gfx] [PATCH 8/8] drm/i915: Allow user to set cache at BO creation)

2023-04-26 Thread Teres Alexis, Alan Previn
On Wed, 2023-04-26 at 13:52 +0200, Daniel Vetter wrote: > On Tue, Apr 25, 2023 at 04:41:54PM +0300, Joonas Lahtinen wrote: > > (+ Faith and Daniel as they have been involved in previous discussions) > > Quoting Jordan Justen (2023-04-24 20:13:00) > > > On 2023-04-24 02:08:43, Tvrtko Ursulin wrote:

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

2023-04-25 Thread Teres Alexis, Alan Previn
On Thu, 2023-04-06 at 15:26 -0700, john.c.harri...@intel.com wrote: > From: John Harrison > > Don't use GEN9 as a prefix for register lists that contain all GEN8 > registers. alan:snip alan: This patch as a stand-along looks good, so I'll provide the RB but take note of the comment below that

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

2023-04-25 Thread Teres Alexis, Alan Previn
On Thu, 2023-04-06 at 15:26 -0700, john.c.harri...@intel.com wrote: > From: John Harrison > > Fix Xe_LP name. > > Signed-off-by: John Harrison alan:snip > -/* GEN9/XE_LPD - Render / Compute Per-Engine-Instance */ > +/* GEN8+ Render / Compute Per-Engine-Instance */ alan: two comments on this:

Re: [Intel-gfx] [PATCH 3/5] drm/i915/guc: Capture list clean up - 2

2023-04-25 Thread Teres Alexis, Alan Previn
On Thu, 2023-04-06 at 15:26 -0700, john.c.harri...@intel.com wrote: > From: John Harrison > > Don't use 'xe_lp*' prefixes for register lists that are common with > Gen8. alan:snip > @@ -177,32 +177,32 @@ static const struct __guc_mmio_reg_descr > empty_regs_list[] = { > static const struct

Re: [Intel-gfx] [PATCH 2/5] drm/i915/guc: Capture list clean up - 1

2023-04-25 Thread Teres Alexis, Alan Previn
On Thu, 2023-04-06 at 15:26 -0700, john.c.harri...@intel.com wrote: > From: John Harrison > > Remove 99% duplicated steered register list code. Also, include the > pre-Xe steered registers in the pre-Xe list generation. > > Signed-off-by: John Harrison alan: Nice work - good cleanup. Thanks

Re: [PATCH 1/5] drm/i915/guc: Don't capture Gen8 regs on Xe devices

2023-04-25 Thread Teres Alexis, Alan Previn
On Thu, 2023-04-06 at 15:26 -0700, Harrison, John C wrote: > From: John Harrison > > A pair of pre-Xe registers were being included in the Xe capture list. > GuC was rejecting those as being invalid and logging errors about > them. So, stop doing it. > alan:snip > #define

Re: IOCTL feature detection (Was: Re: [Intel-gfx] [PATCH 8/8] drm/i915: Allow user to set cache at BO creation)

2023-04-25 Thread Teres Alexis, Alan Previn
On Tue, 2023-04-25 at 16:41 +0300, Joonas Lahtinen wrote: > (+ Faith and Daniel as they have been involved in previous discussions) An orthogonal (but losely related) question: Is PXP the only subsystem that has the unique problem of: Uses a delayed worker to complete all dependencies for init..

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

2023-04-20 Thread Teres Alexis, Alan Previn
I think we are also bottom-ing on the opens fo this patch too: On Thu, 2023-04-20 at 13:21 -0700, Ceraolo Spurio, Daniele wrote: > On 4/20/2023 11:49 AM, Teres Alexis, Alan Previn wrote: > > On Wed, 2023-03-29 at 09:56 -0700, Ceraolo Spurio, Daniele wrote: > > > The GSC noti

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

2023-04-20 Thread Teres Alexis, Alan Previn
i guess we are settled with this patch... On Thu, 2023-04-20 at 15:04 -0700, Ceraolo Spurio, Daniele wrote: > On 4/18/2023 11:57 PM, Teres Alexis, Alan Previn wrote: > > On Wed, 2023-03-29 at 09:56 -0700, Ceraolo Spurio, Daniele wrote: > > > From: Alexander Usyskin > >

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

2023-04-20 Thread Teres Alexis, Alan Previn
On Wed, 2023-03-29 at 09:56 -0700, Ceraolo Spurio, Daniele wrote: > The GSC notifies us of a proxy request via the HECI2 interrupt. The alan:snip > @@ -256,6 +262,7 @@ void gen11_gt_irq_postinstall(struct intel_gt *gt) > u32 irqs = GT_RENDER_USER_INTERRUPT; > u32 guc_mask =

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

2023-04-19 Thread Teres Alexis, Alan Previn
I have a number of comments but most are personal preferences and so i labelled them nits. I did catch a few minor coding styling issues and am assuming those need to be enforced as per i915/kernel rules? That said, since they are so minor (or maybe they are not strict), I'm providing a

  1   2   3   >