[igt-dev] [PATCH i-g-t v4] lib/igt_draw: Change MOCS settings for MTL

[B, Jeevan]

> -----Original Message-----
> From: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> Sent: Tuesday, January 3, 2023 11:43 PM
> To: igt-dev at lists.freedesktop.org
> Cc: B, Jeevan <jeevan.b at intel.com>; De Marchi, Lucas
> <lucas.demarchi at intel.com>; Kempczynski, Zbigniew
> <zbigniew.kempczynski at intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t v4] lib/igt_draw: Change MOCS settings for
> MTL
> 
> Hi,
> 
> On 2022-12-26 at 23:24:29 +0530, Jeevan B wrote:
> > On MTL, we want the table entry labelled "UC (GO:Mem)"
> > which has index 5.  This means that the MOCS value is 10.
> >
> > v2: Add define for MOCS settings. (Lucas)
> > v3: Extending get_mocs_index and using get_mocs_index
> >     to get mocs val. (Zbigniew)
> >
> > Signed-off-by: Jeevan B <jeevan.b at intel.com>
> > ---
> >  lib/i915/intel_mocs.c | 12 ++++++++----
> >  lib/igt_draw.c        |  4 ++--
> >  2 files changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/i915/intel_mocs.c b/lib/i915/intel_mocs.c index
> > df541ab0..f757bfbb 100644
> > --- a/lib/i915/intel_mocs.c
> > +++ b/lib/i915/intel_mocs.c
> > @@ -11,6 +11,8 @@
> >  #define DG1_MOCS_WB_IDX				5
> >  #define DG2_MOCS_UC_IDX				1
> >  #define DG2_MOCS_WB_IDX				3
> > +#define MTL_MOCS_UC_IDX				5
> > +#define MTL_MOCS_WB_IDX				10
> >  #define GEN12_MOCS_UC_IDX			3
> >  #define GEN12_MOCS_WB_IDX			2
> >  #define XY_BLOCK_COPY_BLT_MOCS_SHIFT		21
> > @@ -32,13 +34,15 @@ static void get_mocs_index(int fd, struct
> drm_i915_mocs_index *mocs)
> >  	 * This helper function is providing current UC as well
> >  	 * as WB MOCS index based on platform.
> >  	 */
> > -	if (IS_DG1(devid)) {
> > -		mocs->uc_index = DG1_MOCS_UC_IDX;
> > -		mocs->wb_index = DG1_MOCS_WB_IDX;
> 
> Please keep old code, DG1 first.

Based on last revision comments I fixed the order. (MTL,DG2,DG1,Gen12) 
Is it ok ?? 
> 
> > +	if (IS_METEORLAKE(devid)) {
> > +                mocs->uc_index = MTL_MOCS_UC_IDX;
> > +                mocs->wb_index = MTL_MOCS_WB_IDX;
> >  	} else if (IS_DG2(devid)) {
> >  		mocs->uc_index = DG2_MOCS_UC_IDX;
> >  		mocs->wb_index = DG2_MOCS_WB_IDX;
> > -
> > +	} else if (IS_DG1(devid)) {
> 
> Here put new one for MTL (instead of moving DG1 around).
> 
> > +		mocs->uc_index = DG1_MOCS_UC_IDX;
> > +		mocs->wb_index = DG1_MOCS_WB_IDX;
> >  	} else if (IS_GEN12(devid)) {
> >  		mocs->uc_index = GEN12_MOCS_UC_IDX;
> >  		mocs->wb_index = GEN12_MOCS_WB_IDX; diff --git
> a/lib/igt_draw.c
> > b/lib/igt_draw.c index 975d65cd..58ce0539 100644
> > --- a/lib/igt_draw.c
> > +++ b/lib/igt_draw.c
> > @@ -36,6 +36,7 @@
> >  #include "i830_reg.h"
> >  #include "i915/gem_create.h"
> >  #include "i915/gem_mman.h"
> > +#include "i915/intel_mocs.h"
> >
> >  #ifndef PAGE_ALIGN
> >  #ifndef PAGE_SIZE
> > @@ -702,8 +703,7 @@ static void draw_rect_blt(int fd, struct cmd_data
> *cmd_data,
> >  		pitch = tiling ? buf->stride / 4 : buf->stride;
> >
> >  		intel_bb_out(ibb, XY_FAST_COLOR_BLT | blt_cmd_depth);
> > -		/* DG2 MOCS entry 2 is "UC - Non-Coherent; GO:Memory" */
> > -		intel_bb_out(ibb, blt_cmd_tiling | 2 << 21 | (pitch-1));
> 
> If you look at DG2 definitions you will see there is 1 and 3 so this change now
> looks like a fix for DG2 ?
> Maybe this one should go first as a fix, then adding MTL defs ?
> Or at least mention this fix in commit description.
> 
Sure, I will split this into 2 patches. 

Thanks 
Jeevan B 
 
> Regards,
> Kamil
> 
> > +		intel_bb_out(ibb, blt_cmd_tiling | intel_get_uc_mocs(fd) << 21
> |
> > +(pitch-1));
> >  		intel_bb_out(ibb, (rect->y << 16) | rect->x);
> >  		intel_bb_out(ibb, ((rect->y + rect->h) << 16) | (rect->x + rect-
> >w));
> >  		intel_bb_emit_reloc_fenced(ibb, dst->handle, 0,
> > --
> > 2.36.0
> >