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

["Teres Alexis, Alan Previn" <alan.previn.teres.alexis@intel.com>]

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 LRC run-alone bit or else the encryption will not happen.
> > +	 * Wa_14019159160 - Case 2: mtl
> > +	 * On some platforms, protected contexts require setting
> > +	 * the LRC run-alone bit or else the encryption/decryption will not happen.
> > +	 * NOTE: Case 2 only applies to PXP use-case of said workaround.
> >  	 */
> 
> hmm, interesting enough, on the wa description I read that it is incomplete for MTL/ARL
> and something about a fuse bit. We should probably chase for some clarification in the
> HSD?!
alan: yes, i went through the HSD description with the architect and we both had agreed that
that from the KMD's perspective. At that time, the checking of the fuse from KMD's
could be optimized out for Case-2-PXP: if the fuses was set a certain way, KMD can skip setting
the bit in lrc because hw will enforce runalone in pxp mode irrespective of what KMD does but
if fuse was was not set that way KMD needed to set the runalone in lrc. So for case2, its simpler
to just turn it on always when the context is protected. However, that said, i realized the
wording of the HSDs have changed since the original patch was implemented so i will reclarify
offline and reply back. NOTE: i believe John Harrison is working on case-3 through a
separate series.

> 
> > -	if (GRAPHICS_VER_FULL(ce->engine->i915) >= IP_VER(12, 70) &&
> > -	    (ce->engine->class == COMPUTE_CLASS || ce->engine->class == RENDER_CLASS)) {
> > +	if (IS_METEORLAKE(ce->engine->i915) && (ce->engine->class == COMPUTE_CLASS ||
> > +						ce->engine->class == RENDER_CLASS)) {
> 
> This check now excludes the ARL with the same IP, no?!
alan: yeah - re-reved this part just now.
> 
> >  		rcu_read_lock();
> >  		gem_ctx = rcu_dereference(ce->gem_context);
> >  		if (gem_ctx)
> > 
> > base-commit: 5429d55de723544dfc0630cf39d96392052b27a1
> > -- 
> > 2.39.0
> >