[Intel-gfx] [PATCH] drm/i915: disable sampler indirect state in bindless heap

[Lionel Landwerlin]

On 29/03/2023 01:49, Matt Atwood wrote:
> On Tue, Mar 28, 2023 at 04:14:33PM +0530, Kalvala, Haridhar wrote:
>> On 3/9/2023 8:56 PM, Lionel Landwerlin wrote:
>>> By default the indirect state sampler data (border colors) are stored
>>> in the same heap as the SAMPLER_STATE structure. For userspace drivers
>>> that can be 2 different heaps (dynamic state heap & bindless sampler
>>> state heap). This means that border colors have to copied in 2
>>> different places so that the same SAMPLER_STATE structure find the
>>> right data.
>>>
>>> This change is forcing the indirect state sampler data to only be in
>>> the dynamic state pool (more convinient for userspace drivers, they
>>> only have to have one copy of the border colors). This is reproducing
>>> the behavior of the Windows drivers.
>>>
> Bspec:46052


Sorry, missed your answer.


Should I just add the Bspec number to the commit message ?


Thanks,


-Lionel


>>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>>> Cc: stable at vger.kernel.org
>>> ---
>>>    drivers/gpu/drm/i915/gt/intel_gt_regs.h     |  1 +
>>>    drivers/gpu/drm/i915/gt/intel_workarounds.c | 17 +++++++++++++++++
>>>    2 files changed, 18 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> index 08d76aa06974c..1aaa471d08c56 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> @@ -1141,6 +1141,7 @@
>>>    #define   ENABLE_SMALLPL			REG_BIT(15)
>>>    #define   SC_DISABLE_POWER_OPTIMIZATION_EBB	REG_BIT(9)
>>>    #define   GEN11_SAMPLER_ENABLE_HEADLESS_MSG	REG_BIT(5)
>>> +#define   GEN11_INDIRECT_STATE_BASE_ADDR_OVERRIDE	REG_BIT(0)
>>>    #define GEN9_HALF_SLICE_CHICKEN7		MCR_REG(0xe194)
>>>    #define   DG2_DISABLE_ROUND_ENABLE_ALLOW_FOR_SSLA	REG_BIT(15)
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>>> index 32aa1647721ae..734b64e714647 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>>> @@ -2542,6 +2542,23 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
>>>    				 ENABLE_SMALLPL);
>>>    	}
>>> +	if (GRAPHICS_VER(i915) >= 11) {
>> Hi Lionel,
>>
>> Not sure should this implementation be part of "rcs_engine_wa_init" or
>> "general_render_compute_wa_init".
>>
>>> +		/* This is not a Wa (although referred to as
>>> +		 * WaSetInidrectStateOverride in places), this allows
>>> +		 * applications that reference sampler states through
>>> +		 * the BindlessSamplerStateBaseAddress to have their
>>> +		 * border color relative to DynamicStateBaseAddress
>>> +		 * rather than BindlessSamplerStateBaseAddress.
>>> +		 *
>>> +		 * Otherwise SAMPLER_STATE border colors have to be
>>> +		 * copied in multiple heaps (DynamicStateBaseAddress &
>>> +		 * BindlessSamplerStateBaseAddress)
>>> +		 */
>>> +		wa_mcr_masked_en(wal,
>>> +				 GEN10_SAMPLER_MODE,
>>   since we checking the condition for GEN11 or above, can this register be
>> defined as GEN11_SAMPLER_MODE
> We use the name of the first time the register was introduced, gen 10 is
> fine here.
>>> +				 GEN11_INDIRECT_STATE_BASE_ADDR_OVERRIDE);
>>> +	}
>>> +
>>>    	if (GRAPHICS_VER(i915) == 11) {
>>>    		/* This is not an Wa. Enable for better image quality */
>>>    		wa_masked_en(wal,
>> -- 
>> Regards,
>> Haridhar Kalvala
>>
> Regards,
> MattA