Re: [PATCH 03/11] drm/i915/pvc: Define MOCS table for PVC

From: Lucas De Marchi <lucas.demarchi@intel.com>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
	Ayaz A Siddiqui <ayaz.siddiqui@intel.com>,
	Fei Yang <fei.yang@intel.com>
Subject: Re: [PATCH 03/11] drm/i915/pvc: Define MOCS table for PVC
Date: Mon, 2 May 2022 23:22:29 -0700	[thread overview]
Message-ID: <20220503062229.5q75lqaxftr64dcf@ldmartin-desk2> (raw)
In-Reply-To: <YnBJmselMqaM3zNE@mdroper-desk1.amr.corp.intel.com>

On Mon, May 02, 2022 at 02:14:02PM -0700, Matt Roper wrote:
>On Mon, May 02, 2022 at 02:03:28PM -0700, Lucas De Marchi wrote:
>> On Mon, May 02, 2022 at 09:34:09AM -0700, Matt Roper wrote:
>> > From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>...
>> > @@ -2002,11 +2002,18 @@ engine_fake_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
>> > 	 * Streamers on Gen12 onward platforms.
>> > 	 */
>> > 	if (GRAPHICS_VER(engine->i915) >= 12) {
>> > -		mocs = engine->gt->mocs.uc_index;
>> > +		if (HAS_L3_CCS_READ(engine->i915) &&
>> > +		    engine->class == COMPUTE_CLASS)
>> > +			mocs_r = engine->gt->mocs.wb_index;
>> > +		else
>> > +			mocs_r = engine->gt->mocs.uc_index;
>>
>> shouldn't we add a warning in get_mocs_settings() if HAS_L3_CCS_READ(engine->i915)
>> and mocs.wb_index is 0 (since index 0 shouldn't really be used in latest
>> platforms)?
>
>We should be careful about that assumption...index 0 is valid on DG2
>today, although HAS_L3_CCS_READ() doesn't apply there.  And a couple
>platforms in the future we're also going to have index 0 being valid on
>a platform where HAS_L3_CCS_READ() is true (bspec 71582).  Index 0 would
>still be the wrong entry to pick for WB behavior there, but it is a
>legitimate entry in general.

ok, but comment is more about "forgetting to initialize it in
get_mocs_settings() and then using it here". Using 0 as "it was not
initialized" may be an easy way to do that.

Lucas De Marchi