Re: [Intel-gfx] [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: [Intel-gfx] [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: [Intel-gfx] [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: [Intel-gfx] [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 v8 6/8] drm/i915/uapi/pxp: Add a GET_PARAM for PXP

2023-04-27 Thread Teres Alexis, Alan Previn
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 > > > - Jordan still wants the extension query > Yes, I do, but so far it doesn't appear that any kernel devs think > it's a reasonable request. >

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: [Intel-gfx] [PATCH 1/5] drm/i915/guc: Don't capture Gen8 regs on Xe devices

2023-04-26 Thread Teres Alexis, Alan Previn
On Wed, 2023-04-26 at 10:23 -0700, Harrison, John C wrote: > On 4/25/2023 10:55, Teres Alexis, Alan Previn wrote: > > On Thu, 2023-04-06 at 15:26 -0700, Harrison, John C wrote: > > > From: John Harrison > > > > > > A pair of pre-Xe registers were being includ

Re: [Intel-gfx] [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: [Intel-gfx] IOCTL feature detection (Was: Re: [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: [Intel-gfx] [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: [Intel-gfx] IOCTL feature detection (Was: Re: [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: [Intel-gfx] [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: [Intel-gfx] [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: [Intel-gfx] [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: [Intel-gfx] [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

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

2023-04-19 Thread Teres Alexis, Alan Previn
On Wed, 2023-03-29 at 09:56 -0700, Ceraolo Spurio, Daniele wrote: > From: Alexander Usyskin > > Add GSC proxy driver. It to allows messaging between GSC component > on Intel on board graphics card and CSE device. alan:nit: isn't "Intel integrated GPU" clearer than "Intel on-board graphics

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

2023-04-18 Thread Teres Alexis, Alan Previn
On Wed, 2023-03-29 at 09:56 -0700, Ceraolo Spurio, Daniele wrote: > From: Alexander Usyskin > > GSC Proxy component is used for communication between the > Intel graphics driver and MEI driver. > diff --git a/include/drm/i915_gsc_proxy_mei_interface.h >

Re: [Intel-gfx] [PATCH v7 6/8] drm/i915/uapi/pxp: Fix UAPI spec comments and add GET_PARAM for PXP

2023-04-18 Thread Teres Alexis, Alan Previn
On Tue, 2023-04-18 at 21:06 +0300, Landwerlin, Lionel G wrote: > On 14/04/2023 18:17, Teres Alexis, Alan Previn wrote: > > Hi Lionel, does this patch work for you? > > > Hi, Sorry for the late answer. > > That looks good : alan: Great - thanks Lionel! :) I'll help

Re: [Intel-gfx] [PATCH v7 5/8] drm/i915/pxp: Add ARB session creation and cleanup

2023-04-17 Thread Teres Alexis, Alan Previn
On Mon, 2023-04-17 at 12:15 -0700, Ceraolo Spurio, Daniele wrote: > On 4/17/2023 11:21 AM, Teres Alexis, Alan Previn wrote: > > On Mon, 2023-04-10 at 10:14 -0700, Ceraolo Spurio, Daniele wrote: > > > > @@ -354,8 +368,14 @@ int intel_pxp_start(

Re: [Intel-gfx] [PATCH v7 5/8] drm/i915/pxp: Add ARB session creation and cleanup

2023-04-17 Thread Teres Alexis, Alan Previn
On Mon, 2023-04-10 at 10:14 -0700, Ceraolo Spurio, Daniele wrote: > > > alan:snip > > @@ -354,8 +368,14 @@ int intel_pxp_start(struct intel_pxp *pxp) > > if (!intel_pxp_is_enabled(pxp)) > > return -ENODEV; > > > > - if (wait_for(pxp_component_bound(pxp), 250)) > > -

Re: [Intel-gfx] [PATCH v7 4/8] drm/i915/pxp: Add GSC-CS backend to send GSC fw messages

2023-04-17 Thread Teres Alexis, Alan Previn
On Mon, 2023-04-10 at 09:28 -0700, Ceraolo Spurio, Daniele wrote: > alan:snip > > #define GSC_OUTFLAG_MSG_PENDING 1 > > +#define GSC_INFLAG_MSG_CLEANUP BIT(1) > > For consistency those should all be BIT() defines alan: will do. > > With the define fixed: > > Reviewed-by: Daniele Ceraolo

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

2023-04-17 Thread Teres Alexis, Alan Previn
On Mon, 2023-04-10 at 09:10 -0700, Ceraolo Spurio, Daniele wrote: > alan:snip > > +int > > +intel_gsc_uc_heci_cmd_submit_nonpriv(struct intel_gsc_uc *gsc, > > +struct intel_context *ce, > > +struct intel_gsc_heci_non_priv_pkt *pkt,

Re: [Intel-gfx] [PATCH v7 6/8] drm/i915/uapi/pxp: Fix UAPI spec comments and add GET_PARAM for PXP

2023-04-14 Thread Teres Alexis, Alan Previn
Hi Lionel, does this patch work for you? On Mon, 2023-04-10 at 10:22 -0700, Ceraolo Spurio, Daniele wrote: > On 4/6/2023 10:44 AM, Alan Previn wrote: alan:snip > > +/* > > + * Query the status of PXP support in i915. > > + * > > + * The query can fail in the following scenarios with the listed

Re: [Intel-gfx] [PATCH v6 6/8] drm/i915/pxp: MTL-KCR interrupt ctrl's are in GT-0

2023-04-05 Thread Teres Alexis, Alan Previn
> alan:snip > >   void intel_pxp_irq_enable(struct intel_pxp *pxp) > >   { > > - struct intel_gt *gt = pxp->ctrl_gt; > > + struct intel_gt *gt = intel_pxp_get_irq_gt(pxp); > > in this function we use the gt for: > > 1 - the lock: see above about this > > 2 - gen11_gt_reset_one_iir(): this

Re: [Intel-gfx] [PATCH v6 7/8] drm/i915/pxp: On MTL, KCR enabling doesn't wait on tee component

2023-04-05 Thread Teres Alexis, Alan Previn
> alan:snip > > @@ -140,10 +141,15 @@ static void pxp_init_full(struct intel_pxp *pxp) > > if (ret) > > return; > > > > - if (HAS_ENGINE(pxp->ctrl_gt, GSC0)) > > + if (HAS_ENGINE(pxp->ctrl_gt, GSC0)) { > > ret = intel_pxp_gsccs_init(pxp); > > - else > > +

Re: [Intel-gfx] [PATCH v6 5/8] drm/i915/pxp: Add ARB session creation and cleanup

2023-03-30 Thread Teres Alexis, Alan Previn
On Thu, 2023-03-30 at 13:25 +0100, Tvrtko Ursulin wrote: > On 30/03/2023 01:10, Teres Alexis, Alan Previn wrote: > > On Wed, 2023-03-29 at 08:43 +0100, Tvrtko Ursulin wrote: > > > On 28/03/2023 18:52, Rodrigo Vivi wrote: > > > > On Tue, Mar 28, 2023 at 05:01:36PM +0

Re: [Intel-gfx] [PATCH v6 5/8] drm/i915/pxp: Add ARB session creation and cleanup

2023-03-29 Thread Teres Alexis, Alan Previn
On Wed, 2023-03-29 at 08:43 +0100, Tvrtko Ursulin wrote: > On 28/03/2023 18:52, Rodrigo Vivi wrote: > > On Tue, Mar 28, 2023 at 05:01:36PM +0000, Teres Alexis, Alan Previn wrote: > > > On Mon, 2023-03-27 at 17:15 +0100, Tvrtko Ursulin wrote: > > > alan:snip > Ho

Re: [Intel-gfx] [PATCH v6 5/8] drm/i915/pxp: Add ARB session creation and cleanup

2023-03-28 Thread Teres Alexis, Alan Previn
On Mon, 2023-03-27 at 17:15 +0100, Tvrtko Ursulin wrote: > These two: > e6177ec586d1 ("drm/i915/huc: stall media submission until HuC is loaded") > b76c14c8fb2a ("drm/i915/huc: better define HuC status getparam possible > return values.") > They do not help here? It is not possible to use or

Re: [Intel-gfx] [PATCH v6 5/8] drm/i915/pxp: Add ARB session creation and cleanup

2023-03-25 Thread Teres Alexis, Alan Previn
alan:snip > @@ -353,8 +367,20 @@ int intel_pxp_start(struct intel_pxp *pxp) alan:snip > > + if (HAS_ENGINE(pxp->ctrl_gt, GSC0)) { > > + /* > > +* GSC-fw loading, GSC-proxy init (requiring an mei component > > driver) and > > +* HuC-fw loading must all occur

Re: [Intel-gfx] [PATCH v6 5/8] drm/i915/pxp: Add ARB session creation and cleanup

2023-03-25 Thread Teres Alexis, Alan Previn
On Fri, 2023-03-03 at 17:34 -0800, Ceraolo Spurio, Daniele wrote: > > On 2/27/2023 6:21 PM, Alan Previn wrote: > > Add MTL's function for ARB session creation using PXP firmware > > version 4.3 ABI structure format. > alan:snip > > + ret = gsccs_send_message_retry_complete(pxp, > > +

Re: [Intel-gfx] [PATCH v5] drm/i915/pxp: limit drm-errors or warning on firmware API failures

2023-03-24 Thread Teres Alexis, Alan Previn
Thanks Daniele, Thanks Jani. ...alan

Re: [Intel-gfx] [PATCH v6 4/8] drm/i915/pxp: Add GSC-CS backend to send GSC fw messages

2023-03-23 Thread Teres Alexis, Alan Previn
Hi Daniele - thanks for reviewing this - i will fix all of code in accordance to the review comments you provided with some exceptions / alternatives: On Fri, 2023-03-03 at 17:07 -0800, Ceraolo Spurio, Daniele wrote: > > On 2/27/2023 6:21 PM, Alan Previn wrote: > > Add GSC engine based method

Re: [Intel-gfx] [PATCH v3] drm/i915/pxp: limit drm-errors or warning on firmware API failures

2023-03-23 Thread Teres Alexis, Alan Previn
On Thu, 2023-03-23 at 08:35 +, Tvrtko Ursulin wrote: On 23/03/2023 00:27, Teres Alexis, Alan Previn wrote: On Fri, 2023-03-17 at 13:37 +0200, Tamminen, Eero T wrote: Hi, On 16.3.2023 10.50, Tvrtko Ursulin wrote: [ 11.674183] i915 :00:02.0: PXP init-arb-session-15 failed due

Re: [Intel-gfx] [PATCH v3] drm/i915/pxp: limit drm-errors or warning on firmware API failures

2023-03-22 Thread Teres Alexis, Alan Previn
On Fri, 2023-03-17 at 13:37 +0200, Tamminen, Eero T wrote: > Hi, > > On 16.3.2023 10.50, Tvrtko Ursulin wrote: > > > [   11.674183] i915 :00:02.0: PXP init-arb-session-15 failed due > > > to BIOS/SOC:0x101a:ERR_PLATFORM_CONFIG > ... > > Alan - is this expected during normal operation on

Re: [Intel-gfx] [PATCH] drm/i915/huc: Cancel HuC delayed load timer on reset.

2023-03-16 Thread Teres Alexis, Alan Previn
Reviewed-by: Alan Previn P.S. side note - while reviewing this, i wish we got rid of those "ops_on/off->intel_uc_funcname" macro obsfucations since i couldnt find intel_uc_sanitize but did find the direct call - so inconsistent. On Mon, 2023-03-13 at 13:55 -0700, Ceraolo Spurio, Daniele

Re: [Intel-gfx] [PATCH v2 3/5] drm/i915/guc: Provide debugfs for log relay sub-buf info

2023-03-14 Thread Teres Alexis, Alan Previn
On Thu, 2023-03-09 at 15:29 -0800, Teres Alexis, Alan Previn wrote: > > > alan:snip > > > +static int guc_log_relay_subbuf_size_get(void *data, u64 *val) > > > +{ > > > + struct intel_guc_log *log = data; > > > + > > > + if (!log->vma

Re: [Intel-gfx] [PATCH v2 4/5] drm/i915/guc: Rename GuC log relay debugfs descriptively

2023-03-09 Thread Teres Alexis, Alan Previn
> > > -static int guc_log_relay_open(struct inode *inode, struct file *file) > > > +static int guc_log_relay_ctl_open(struct inode *inode, struct file *file) > > > > Again not objecting, but what is the purpose/thinking behind adding _ctl_ > > to these function names? The previous names seemed

Re: [Intel-gfx] [PATCH v2 5/5] drm/i915/guc: Move guc_log_relay_chan debugfs path to uc

2023-03-09 Thread Teres Alexis, Alan Previn
On Wed, 2022-12-07 at 09:24 -0800, Dixit, Ashutosh wrote: > On Tue, 06 Dec 2022 01:21:00 -0800, Alan Previn wrote: > > > > alan: snip > > intel_gt_debugfs_register_files(root, files, ARRAY_SIZE(files), uc); > > > > intel_guc_debugfs_register(>guc, root); > > After moving the line

Re: [Intel-gfx] [PATCH v2 4/5] drm/i915/guc: Rename GuC log relay debugfs descriptively

2023-03-09 Thread Teres Alexis, Alan Previn
Again, only now having time to relook at this. Will re-rev and reconnect with Ashutosh offline since I've been absent on this for too long. Thanks again Ashutosh for reviewing all these back in May. On Wed, 2022-12-07 at 08:50 -0800, Dixit, Ashutosh wrote: > On Tue, 06 Dec 2022 01:20:59 -0800,

Re: [Intel-gfx] [PATCH v2 3/5] drm/i915/guc: Provide debugfs for log relay sub-buf info

2023-03-09 Thread Teres Alexis, Alan Previn
Finally got some time to relook at this series. Responses. I'll re-rev this and re-connect with Ashutosh offline considering how long i've been silent on this. On Wed, 2022-12-07 at 08:43 -0800, Dixit, Ashutosh wrote: > On Tue, 06 Dec 2022 01:20:58 -0800, Alan Previn wrote: > > > > diff --git

Re: [Intel-gfx] [PATCH v2 3/3] drm/i915: Include timeline seqno in error capture

2023-03-08 Thread Teres Alexis, Alan Previn
On Wed, 2023-03-08 at 14:02 -0800, Teres Alexis, Alan Previn wrote: > On Thu, 2023-02-16 at 18:24 -0800, john.c.harri...@intel.com wrote: > > From: John Harrison > > > > The seqno value actually written out to memory is no longer in the > > regular HWSP. Instead, i

Re: [Intel-gfx] [PATCH v2 3/3] drm/i915: Include timeline seqno in error capture

2023-03-08 Thread Teres Alexis, Alan Previn
On Thu, 2023-02-16 at 18:24 -0800, john.c.harri...@intel.com wrote: > From: John Harrison > > The seqno value actually written out to memory is no longer in the > regular HWSP. Instead, it is now in its own private timeline buffer. > Thus, it is no longer visible in an error capture. So,

Re: [Intel-gfx] [PATCH 1/2] drm/i915/gsc: flush the GSC worker before wedging on unload

2023-03-07 Thread Teres Alexis, Alan Previn
On Thu, 2023-02-23 at 09:21 -0800, Ceraolo Spurio, Daniele wrote: > If we unload the driver and wedge before the GSC worker is complete, > the worker will hit an error on its submission to the GSC engine and > then exit. This is hard to hit for a user, but it is reproducible > with skipping

Re: [Intel-gfx] [PATCH 2/2] drm/i915/gsc: Fix race between HW init and GSC FW load

2023-03-07 Thread Teres Alexis, Alan Previn
On Thu, 2023-02-23 at 09:21 -0800, Ceraolo Spurio, Daniele wrote: > The GSC FW load is a slow process (up to 250 ms), so we defer it to a > dedicated worker to avoid stalling the init flow for that long. However, > we currently start this worker before the HW init is complete, so there > is a

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

2023-03-02 Thread Teres Alexis, Alan Previn
On Mon, 2023-02-27 at 18:21 -0800, 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. > > al

Re: [Intel-gfx] [PATCH] drm/i915/gsc: Fix the Driver-FLR completion

2023-02-23 Thread Teres Alexis, Alan Previn
Thanks Daniele, you are right about the fixes tag - i totally forgot that MTL is still force-probe. Will respin with the bit definition fix, remove the fixes-tag and leave out the get/put runtime-pm from rev3 (as per your comment on rev3). Rev4 coming right up. ...alan P.S. I had the same

Re: [Intel-gfx] [PATCH v5 5/8] drm/i915/pxp: Add ARB session creation and cleanup

2023-02-23 Thread Teres Alexis, Alan Previn
On Fri, 2023-02-17 at 03:12 +, Teres Alexis, Alan Previn wrote: > On Tue, 2023-02-14 at 13:38 -0800, Teres Alexis, Alan Previn wrote: > > Add MTL's function for ARB session creation using PXP firmware > > version 4.3 ABI structure format. > > alan:snip > > N

Re: [Intel-gfx] [PATCH v5 5/8] drm/i915/pxp: Add ARB session creation and cleanup

2023-02-23 Thread Teres Alexis, Alan Previn
On Tue, 2023-02-14 at 13:38 -0800, Teres Alexis, Alan Previn wrote: > Add MTL's function for ARB session creation using PXP firmware > version 4.3 ABI structure format. > alan:snip > > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c > b/drivers/gpu/drm/i915/pxp/i

Re: [Intel-gfx] [PATCH] drm/i915/gsc: Fix the Driver-FLR completion

2023-02-23 Thread Teres Alexis, Alan Previn
On Wed, 2023-02-22 at 13:01 -0800, Teres Alexis, Alan Previn wrote: > The Driver-FLR flow may inadvertently exit early before the full > completion of the re-init of the internal HW state if we only poll > GU_DEBUG Bit31 (polling for it to toggle from 0 -> 1). Instead > we

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

2023-02-22 Thread Teres Alexis, Alan Previn
On Tue, 2023-02-14 at 13:38 -0800, 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. > > NOTE

Re: [Intel-gfx] [PATCH v5 8/8] drm/i915/pxp: Enable PXP with MTL-GSC-CS

2023-02-18 Thread Teres Alexis, Alan Previn
On Tue, 2023-02-14 at 13:38 -0800, Teres Alexis, Alan Previn wrote: > Enable PXP with MTL-GSC-CS: add the has_pxp into device info > and increase the timeouts for new GSC-CS + firmware specs. > > Signed-off-by: Alan Previn > --- > drivers/gpu/drm/i915/i915_pci.c

Re: [Intel-gfx] [PATCH v5 5/8] drm/i915/pxp: Add ARB session creation and cleanup

2023-02-16 Thread Teres Alexis, Alan Previn
On Tue, 2023-02-14 at 13:38 -0800, Teres Alexis, Alan Previn wrote: > Add MTL's function for ARB session creation using PXP firmware > version 4.3 ABI structure format. > > Also add MTL's function for ARB session invalidation but this > reuses PXP firmware version 4.2 ABI s

Re: [Intel-gfx] [PATCH v5 4/8] drm/i915/pxp: Add GSC-CS backend to send GSC fw messages

2023-02-15 Thread Teres Alexis, Alan Previn
On Tue, 2023-02-14 at 13:38 -0800, Teres Alexis, Alan Previn wrote: alan:snip > +static int gsccs_send_message(struct intel_pxp *pxp, > + void *msg_in, size_t msg_in_size, > + void *msg_out, size_t msg_out

Re: [Intel-gfx] [PATCH v4 5/8] drm/i915/pxp: Add ARB session creation and cleanup

2023-02-14 Thread Teres Alexis, Alan Previn
On Thu, 2023-02-09 at 16:42 -0800, Teres Alexis, Alan Previn wrote: > Add MTL's function for ARB session creation using PXP firmware > version 4.3 ABI structure format. > > Also add MTL's function for ARB session invalidation but this > reuses PXP firmware version 4.2 ABI s

Re: [Intel-gfx] [PATCH v3 4/8] drm/i915/pxp: Add GSC-CS backend to send GSC fw messages

2023-02-09 Thread Teres Alexis, Alan Previn
missed somethings on host-session-handle - for next rev. On Wed, 2023-01-25 at 00:06 -0800, Teres Alexis, Alan Previn wrote: > Add GSC engine based method for sending PXP firmware packets > to the GSC firmware for MTL (and future) products. > > Use the newly added helpers to populat

Re: [Intel-gfx] [PATCH 3/6] drm/i915/guc: More debug print updates - GuC reg capture

2023-02-04 Thread Teres Alexis, Alan Previn
So i do have one request - but its a nit - for the following case, should it be a guc_warn instead of a guc_dbg? (last hunk in this patch) "No register capture node found for 0x%04X / 0x%08X\n", ce->guc_id.id, ce->lrc.lrca);" Otherwise LGTM, Reviewed-by: Alan Previn On Thu,

Re: [Intel-gfx] [PATCH 1/3] drm/i915/guc: Fix missing ecodes

2023-02-03 Thread Teres Alexis, Alan Previn
Reviewed-by: Alan Previn On Thu, 2023-02-02 at 17:10 -0800, Harrison, John C wrote: > From: John Harrison > > Error captures are tagged with an 'ecode'. This is a pseduo-unique magic > number that is meant to distinguish similar seeming bugs with > different underlying signatures. It is a

Re: [Intel-gfx] [PATCH 2/3] drm/i915/guc: Clean up of register capture search

2023-02-03 Thread Teres Alexis, Alan Previn
I see you are inferring that a guc-id of zero can be valid. I am guessing that might have contributed to some lost captures? Thanks for catching this. Reviewed-by: Alan Previn On Thu, 2023-02-02 at 17:10 -0800, john.c.harri...@intel.com wrote: > From: John Harrison > > The comparison in the

Re: [Intel-gfx] [PATCH] drm/i915/pxp: limit drm-errors or warnings on firmware API failures

2023-02-02 Thread Teres Alexis, Alan Previn
On Thu, 2023-02-02 at 08:43 +, Tvrtko Ursulin wrote: > > On 02/02/2023 08:13, Alan Previn wrote: > > MESA driver is creating protected context on every driver handle > > initialization to query caps bit for app. So when running CI tests, > > they are observing hundreds of drm_errors when

Re: [Intel-gfx] [PATCH v9 5/6] drm/i915/mtl: Add function to send command to GSC CS

2023-02-01 Thread Teres Alexis, Alan Previn
Conditional Rb with below fix in intel_hdcp_gsc_free_message: Reviewed-by: Alan Previn On Tue, 2023-01-31 at 12:03 +0530, Kandpal, Suraj wrote: > Add function that takes care of sending command to gsc cs. We start > of with allocation of memory for our command intel_hdcp_gsc_message that >

Re: [Intel-gfx] [PATCH v3 5/8] drm/i915/pxp: Add ARB session creation with new PXP API Ver4.3

2023-01-27 Thread Teres Alexis, Alan Previn
Alexis, Alan Previn wrote: > Add MTL's function for ARB session creation using PXP firmware > version 4.3 ABI structure format. > > Before checking the return status, look at the GSC-CS-Mem-Header's > pending-bit which means the GSC firmware is busy and we should > resubmit. >

Re: [Intel-gfx] [PATCH v7 3/6] mei: clean pending read with vtag on bus

2023-01-27 Thread Teres Alexis, Alan Previn
Hi Greg, appreciate your time on this, Patch #2 adds a device link between i915 and mei (at bind time) specifically for the PXP interface that is subject to the issue being fixed. Change is on i915 but implication is mei suspend-resume aligfnent with i915. Rodrigo has already reviewed it but

Re: [Intel-gfx] [PATCH v8 5/6] drm/i915/mtl: Add function to send command to GSC CS

2023-01-27 Thread Teres Alexis, Alan Previn
On Tue, 2023-01-24 at 15:12 +0530, Kandpal, Suraj wrote: > Add function that takes care of sending command to gsc cs. We start > of with allocation of memory for our command intel_hdcp_gsc_message that > contains gsc cs memory header as directed in specs followed by the > actual payload hdcp

Re: [Intel-gfx] [PATCH] drm/i915/guc: Fix missing ecodes

2023-01-26 Thread Teres Alexis, Alan Previn
Firstly, thanks for catching this miss. Since I only have one trivial nit and one non-blocker ask. and the non-blocker ask will not impact the patch intent as it merely tweaks an existing debug message, I believe we have an rb: Reviewed-by: Alan Previn On Tue, 2023-01-24 at 16:49 -0800,

Re: [Intel-gfx] [PATCH v2 6/9] drm/i915/pxp: Add ARB session creation with new PXP API Ver4.3

2023-01-24 Thread Teres Alexis, Alan Previn
Thanks Daniele - agreed on all counts - will fix accordingly - already done and tested internally - will re-rev shortly. ..alan On Wed, 2023-01-18 at 17:50 -0800, Ceraolo Spurio, Daniele wrote: > > > On 1/11/2023 1:42 PM, Alan Previn wrote: > > Add MTL's function for ARB session creation using

Re: [Intel-gfx] [PATCH v6 2/6] drm/i915/pxp: add device link between i915 and mei_pxp

2023-01-24 Thread Teres Alexis, Alan Previn
Thanks Rodrigo - will do. On Tue, 2023-01-24 at 10:10 -0500, Vivi, Rodrigo wrote: > On Mon, Jan 23, 2023 at 09:31:46PM -0800, Alan Previn wrote: > > From: Alexander Usyskin > > > > Add device link with i915 as consumer and mei_pxp as supplier > > to ensure proper ordering of power flows. > > >

Re: [Intel-gfx] [PATCH v6 5/6] drm/i915/pxp: Trigger the global teardown for before suspending

2023-01-24 Thread Teres Alexis, Alan Previn
On Tue, 2023-01-24 at 13:43 -0500, Vivi, Rodrigo wrote: > On Tue, Jan 24, 2023 at 10:08:52AM -0800, Juston Li wrote: > > On Tue, 2023-01-24 at 10:17 -0500, Rodrigo Vivi wrote: > > > On Mon, Jan 23, 2023 at 09:31:49PM -0800, Alan Previn wrote: > > > > A driver bug was recently discovered where the

Re: [Intel-gfx] [PATCH v5 2/6] drm/i915/pxp: add device link between i915 and mei_pxp

2023-01-23 Thread Teres Alexis, Alan Previn
On Mon, 2023-01-23 at 09:31 -0500, Vivi, Rodrigo wrote: > On Sun, Jan 22, 2023 at 06:57:24AM +, Usyskin, Alexander wrote: > > > > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c > > > b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c > > > > index d50354bfb993..bef6d7f8ac55 100644 > > > > ---

Re: [Intel-gfx] [PATCH v2 8/9] drm/i915/pxp: On MTL, KCR enabling doesn't wait on tee component

2023-01-23 Thread Teres Alexis, Alan Previn
Thanks - will go with your suggestion - ditch all the abstraction / consolidation .. add only the two additional calls with the explicit runtime-takes - that minimizes the changes. And won't forget to the fix the '&' -> '&&' ...alan On Wed, 2023-01-18 at 18:17 -0800, Ceraolo Spurio, Daniele

Re: [Intel-gfx] [PATCH v2 7/9] drm/i915/pxp: MTL-KCR interrupt ctrl's are in GT-0

2023-01-23 Thread Teres Alexis, Alan Previn
On Wed, 2023-01-18 at 18:01 -0800, Ceraolo Spurio, Daniele wrote: > > > On 1/11/2023 1:42 PM, Alan Previn wrote: > > Despite KCR subsystem being in the media-tile (close to the > > GSC-CS), the IRQ controls for it are on GT-0 with other global > > IRQ controls. Thus, add a helper for KCR hw

Re: [Intel-gfx] [PATCH v2 5/9] drm/i915/pxp: Add GSC-CS backend to send GSC fw messages

2023-01-20 Thread Teres Alexis, Alan Previn
On Fri, 2023-01-20 at 23:46 +, Teres Alexis, Alan Previn wrote: > Thanks for the review comment... > Replying here with the summary of our offline chat: So we concluded that for the next rev, lets move the buffer/vma allocations from patch #2 into this patch #5-send-message. But for

Re: [Intel-gfx] [PATCH v2 5/9] drm/i915/pxp: Add GSC-CS backend to send GSC fw messages

2023-01-20 Thread Teres Alexis, Alan Previn
Thanks for the review comment... On Wed, 2023-01-18 at 17:40 -0800, Ceraolo Spurio, Daniele wrote: > > > On 1/11/2023 1:42 PM, Alan Previn wrote: > > Add GSC engine based method for sending PXP firmware packets > > to the GSC firmware for MTL (and future) products. Use the newly > > added

Re: [Intel-gfx] [PATCH v2 4/9] drm/i915/pxp: Add MTL helpers to submit Heci-Cmd-Packet to GSC

2023-01-20 Thread Teres Alexis, Alan Previn
On Wed, 2023-01-18 at 17:28 -0800, Ceraolo Spurio, Daniele wrote: > > > > > > On 1/11/2023 1:42 PM, Alan Previn wrote: > > > > Add helper functions into (new) common heci-packet-submission files > > > > to handle generating the MTL GSC-CS Memory-Header and emitting of > > > > the Heci-Cmd-Packet

Re: [Intel-gfx] [PATCH v2 3/9] drm/i915/pxp: Add MTL hw-plumbing enabling for KCR operation

2023-01-20 Thread Teres Alexis, Alan Previn
Thanks for reviewing. On Wed, 2023-01-18 at 16:09 -0800, Ceraolo Spurio, Daniele wrote: > > > > Alan: [snip] > > > > -/* KCR register definitions */ > > > > -#define KCR_INIT _MMIO(0x320f0) > > > > -/* Setting KCR Init bit is required after system boot */ > > > > -#define

Re: [Intel-gfx] [PATCH v2 2/9] drm/i915/pxp: Add GSC-CS back-end resource init and cleanup

2023-01-20 Thread Teres Alexis, Alan Previn
Thanks for reviewing, Daniele. Responses are inline below. On Wed, 2023-01-18 at 15:55 -0800, Ceraolo Spurio, Daniele wrote: > > > > > > On 1/11/2023 1:42 PM, Alan Previn wrote: > > > > For MTL, PXP transport back-end uses the GSC engine to submit > > > > HECI packets for PXP arb session

Re: [Intel-gfx] [PATCH 1/1] drm/i915/gsc: Fix the Driver-FLR completion

2023-01-20 Thread Teres Alexis, Alan Previn
Thanks for reviewing - sounds good - will fix those comments up as per your recommendation. On Fri, 2023-01-20 at 11:14 +0200, Jani Nikula wrote: > On Thu, 19 Jan 2023, Alan Previn wrote: > > The Driver-FLR flow may inadvertently exit early before the full > > completion of the re-init of the

Re: [Intel-gfx] [PATCH 1/1] drm/i915/gsc: Fix the Driver-FLR completion

2023-01-20 Thread Teres Alexis, Alan Previn
rg; dri-de...@lists.freedesktop.org; Teres Alexis, > > Alan Previn ; Vivi, Rodrigo > > > > Subject: [Intel-gfx] [PATCH 1/1] drm/i915/gsc: Fix the Driver-FLR completion > > > > alan:snip.. > > +   /* Completion Step 1 - poll for 'CNTL-BIT31 = 0' wait for hw

Re: [Intel-gfx] [PATCH v2 1/9] drm/i915/pxp: Add MTL PXP GSC-CS back-end skeleton

2023-01-19 Thread Teres Alexis, Alan Previn
Thanks for reviewing Daniele - will fix these on re-rev. And you're right - we dont need a variable "gsccs" (so HAS_ENGINE should work fine). On Wed, 2023-01-18 at 09:51 -0800, Ceraolo Spurio, Daniele wrote: > > Alan: [snip]

Re: [Intel-gfx] [PATCH v5 2/6] drm/i915/pxp: add device link between i915 and mei_pxp

2023-01-19 Thread Teres Alexis, Alan Previn
Thanks for reviewing the patch. I will fix the code according to your recommendation. I assume we could probably go with -ENOLINK as the error (instead of -ENOMEM). However, i'll wait for Alexander to respond on whether he needs drm_WARN_ON and your question on RPM. ...alan On Thu, 2023-01-19

Re: [Intel-gfx] [PATCH v5 5/6] drm/i915/pxp: Trigger the global teardown for before suspending

2023-01-19 Thread Teres Alexis, Alan Previn
Thanks for reviewing - responses below. On Thu, 2023-01-19 at 14:35 -0500, Vivi, Rodrigo wrote: > On Thu, Jan 12, 2023 at 05:18:49PM -0800, Alan Previn wrote: > > A driver bug was recently discovered where the security firmware was > > receiving internal HW signals indicating that session key

Re: [Intel-gfx] [PATCH v5 4/6] drm/i915/pxp: Invalidate all PXP fw sessions during teardown

2023-01-19 Thread Teres Alexis, Alan Previn
Thanks Rodrigo for the ack. On Thu, 2023-01-19 at 14:28 -0500, Vivi, Rodrigo wrote: > On Thu, Jan 12, 2023 at 05:18:48PM -0800, Alan Previn wrote: > > A gap was recently discovered where if an application did not > > invalidate all of the stream keys (intentionally or not), and the > > driver did

Re: [Intel-gfx] [PATCH 1/1] drm/i915/gsc: Fix the Driver-FLR completion

2023-01-19 Thread Teres Alexis, Alan Previn
Forwarded offline. Let's hold off R-B or merging until I verify that hw spec update is finalized to be exactly as what this patch is (probably a minor delay). On Thu, 2023-01-19 at 14:57 -0500, Vivi, Rodrigo wrote: > On Thu, Jan 19, 2023 at 11:49:55AM -0800, Alan Previn wrote: > > The

Re: [Intel-gfx] [PATCH v7 5/6] drm/i915/mtl: Add function to send command to GSC CS

2023-01-18 Thread Teres Alexis, Alan Previn
On Tue, 2023-01-10 at 16:27 +0530, Suraj Kandpal wrote: > Add function that takes care of sending command to gsc cs. We start > of with allocation of memory for our command intel_hdcp_gsc_message that > contains gsc cs memory header as directed in specs followed by the > actual payload hdcp

Re: [Intel-gfx] [PATCH v4 6/6] drm/i915/pxp: Pxp hw init should be in resume_complete

2023-01-12 Thread Teres Alexis, Alan Previn
Thanks for reviewing. Will fix all and re-rev. On Thu, 2023-01-12 at 09:28 -0800, Ceraolo Spurio, Daniele wrote: > > On 1/11/2023 4:37 PM, Alan Previn wrote: > > During suspend flow, i915 currently achors' on the > > pm_suspend_prepare > > callback as the location where we quiesce the entire GPU

Re: [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/pxp: Add MTL PXP Support

2023-01-11 Thread Teres Alexis, Alan Previn
Forgot to test with .config PXP set to off - will re-rev On Wed, 2023-01-11 at 01:32 +, Patchwork wrote: > == Series Details == > > Series: drm/i915/pxp: Add MTL PXP Support > URL   : https://patchwork.freedesktop.org/series/112647/ > State : failure > > == Summary == > > Error: make

Re: [Intel-gfx] [PATCH v4 6/7] drm/i915/mtl: Add function to send command to GSC CS

2022-12-27 Thread Teres Alexis, Alan Previn
I've send offline email to clear up session cleanup behavior for hdcp (maybe its a PXP-only thing). But still wanna pursue a bit more on below portion: On Fri, 2022-12-23 at 09:07 +, Kandpal, Suraj wrote: > > > > > > > + memset(cmd, 0, obj->base.size); > > > > Alan: question: how often is

Re: [Intel-gfx] [PATCH] drm/i915/display: Check source height is > 0

2022-12-27 Thread Teres Alexis, Alan Previn
Is there a better place for this check higher up the intel specific atomic-check? (so the check won't be skl specific - i notice that intel_adjusted_rate is also called by ilk_foo as well and non-backend-specific functions). Else, perhaps intel_adjusted_rate should add a check + WARN? (if we

Re: [Intel-gfx] [PATCH v4 5/7] drm/i915/hdcp: Fill wired_cmd_in structures at a single place

2022-12-27 Thread Teres Alexis, Alan Previn
Just my 2 cents (im not hdcp expert but understand a little on the GSC vs MEI): IIRC, HW architecture wise, MEI and GSC-CS (command streamer) are both merely transport protocols to a backend-service. The backend-service here being HDCP-subsystem within the security firmware (one of several

Re: [Intel-gfx] [PATCH v4 1/7] drm/i915/gsc: Create GSC request submission mechanism

2022-12-23 Thread Teres Alexis, Alan Previn
LGTM: Reviewed-by: Alan Previn On Thu, 2022-12-22 at 12:13 +0530, Kandpal, Suraj wrote: > HDCP and PXP will require a common function to allow it to > submit commands to the gsc cs. Also adding the gsc mtl header > that needs to be added on to the existing payloads of HDCP > and PXP. > > --v4 >

Re: [Intel-gfx] [PATCH v4 6/7] drm/i915/mtl: Add function to send command to GSC CS

2022-12-23 Thread Teres Alexis, Alan Previn
Most of it looks good - but I have two issues: 1. I believe there are some reuse efficiency gaps depending on the frequency of these hdcp-gsc messages... but it might not be an issue depending on the answers to the questions i have below. 2. Missing the session-cleanup request ...alan On

Re: [Intel-gfx] [PATCH v3 1/7] drm/i915/gsc: Create GSC request submission mechanism

2022-12-21 Thread Teres Alexis, Alan Previn
> > > Alan:[snip] > > > + u8 gsc_address; > > > +#define HECI_MEADDRESS_PXP 17 > > > +#define HECI_MEADDRESS_HDCP 18 > > > + Alan: btw, from the internal specs, if i understand it correctly, at the heci command packet level, this ought to be called "heci_client_id", not "gsc_address".

Re: [Intel-gfx] [PATCH v3 0/7] drm/i915/pxp: Add missing cleanup steps for PXP global-teardown

2022-12-21 Thread Teres Alexis, Alan Previn
Apologies for the noise, please ignore this thread - will resend. Had unnecessary in-reply-to tag causing split in series URLs. On Wed, 2022-12-21 at 14:54 -0800, Alan Previn wrote: > Add missing cleanup steps for PXP global-teardown

Re: [Intel-gfx] [PATCH v3 6/7] drm/i915/mtl: Add function to send command to GSC CS

2022-12-21 Thread Teres Alexis, Alan Previn
On Tue, 2022-12-20 at 07:19 +, Kandpal, Suraj wrote: > > > > On Wed, 2022-12-14 at 14:37 +0530, Suraj Kandpal wrote: > > > > > > Alan: See my review comment on patch #1 - i believe most of this function > > above > > (intel_hdcp_gsc_msg_send) could go into a common > >

<    1   2   3   4   5   >