Re: [PATCH 04/12] drm/i915/gen9: Merge HALF_SLICE_CHICKEN3 WA

From: Jani Nikula <jani.nikula@linux.intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Arun Siluvery" <arun.siluvery@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, Mika Kuoppala <mika.kuoppala@intel.com>
Subject: Re: [PATCH 04/12] drm/i915/gen9: Merge HALF_SLICE_CHICKEN3 WA
Date: Mon, 28 Sep 2015 11:47:06 +0300	[thread overview]
Message-ID: <87k2rbkk1h.fsf@intel.com> (raw)
In-Reply-To: <20150925171218.GY26517@intel.com>

On Fri, 25 Sep 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Sep 25, 2015 at 02:33:38PM +0100, Arun Siluvery wrote:
>> Merge WaDisableSamplerPowerBypassForSOPingPong and another WA which has no name
>> as they are part of same register. This will save an entry in WA array.
>> 
>> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++++++++----------
>>  1 file changed, 8 insertions(+), 10 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> index ad16ef4..963b3ca 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> @@ -914,9 +914,14 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring)
>>  	WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN,
>>  			  PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE);
>>  
>> -	/* Syncing dependencies between camera and graphics:skl,bxt */
>> -	WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3,
>> -			  GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC);
>> +	/* WA: Syncing dependencies between camera and graphics:skl,bxt */
>> +	/* WaDisableSamplerPowerBypassForSOPingPong:skl,bxt */
>> +	tmp = GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC;
>> +	if (IS_SKYLAKE(dev) ||
>> +	    (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_B0)) {
>> +		tmp |= GEN8_SAMPLER_POWER_BYPASS_DIS;
>> +	}
>> +	WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3, tmp);
>
> I really dislike these platform+stepping checks in the shared codepath.
> If there's any difference between the platforms, IMO the w/a should go into
> the per-platform function.

Agreed, also on a more generic level than these specific functions.

BR,
Jani.

>
>>  
>>  	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) == SKL_REVID_A0 ||
>>  	    INTEL_REVID(dev) == SKL_REVID_B0)) ||
>> @@ -967,13 +972,6 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring)
>>  		tmp |= HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE;
>>  	WA_SET_BIT_MASKED(HDC_CHICKEN0, tmp);
>>  
>> -	/* WaDisableSamplerPowerBypassForSOPingPong:skl,bxt */
>> -	if (IS_SKYLAKE(dev) ||
>> -	    (IS_BROXTON(dev) && INTEL_REVID(dev) <= BXT_REVID_B0)) {
>> -		WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3,
>> -				  GEN8_SAMPLER_POWER_BYPASS_DIS);
>> -	}
>> -
>>  	/* WaDisableSTUnitPowerOptimization:skl,bxt */
>>  	WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN2, GEN8_ST_PO_DISABLE);
>>  
>> -- 
>> 1.9.1
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx