diff mbox

[02/12] drm/i915/gen9: Add WaOCLCoherentLineFlush

Message ID 1443188026-1222-3-git-send-email-arun.siluvery@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

arun.siluvery@linux.intel.com Sept. 25, 2015, 1:33 p.m. UTC 
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Ville Syrjälä Sept. 25, 2015, 5:09 p.m. UTC | #1 
On Fri, Sep 25, 2015 at 02:33:36PM +0100, Arun Siluvery wrote:
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ab5ac5e..093a5e4 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -63,6 +63,10 @@ static void gen9_init_clock_gating(struct drm_device *dev)
>  	/* WaDisableKillLogic:bxt,skl */
>  	I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) |
>  		   ECOCHK_DIS_TLB);
> +
> +	/* WaOCLCoherentLineFlush:skl,bxt */
> +	I915_WRITE(GEN8_L3SQCREG4, (I915_READ(GEN8_L3SQCREG4) |
> +				    GEN8_LQSC_FLUSH_COHERENT_LINES));

According to Bspec + w/a db this should be done for BDW too (actually
BSpec shows it for BDW only?). If that's the case, then we should be
able to kill gen8_emit_flush_coherentl3_wa(), no? Well, as long as
someone goes and adds the DC flush to the normal post batch flush.

>  }
>  
>  static void skl_init_clock_gating(struct drm_device *dev)
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
arun.siluvery@linux.intel.com Sept. 28, 2015, 3:51 p.m. UTC | #2 
On 25/09/2015 18:09, Ville Syrjälä wrote:
> On Fri, Sep 25, 2015 at 02:33:36PM +0100, Arun Siluvery wrote:
>> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_pm.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index ab5ac5e..093a5e4 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -63,6 +63,10 @@ static void gen9_init_clock_gating(struct drm_device *dev)
>>   	/* WaDisableKillLogic:bxt,skl */
>>   	I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) |
>>   		   ECOCHK_DIS_TLB);
>> +
>> +	/* WaOCLCoherentLineFlush:skl,bxt */
>> +	I915_WRITE(GEN8_L3SQCREG4, (I915_READ(GEN8_L3SQCREG4) |
>> +				    GEN8_LQSC_FLUSH_COHERENT_LINES));
>
> According to Bspec + w/a db this should be done for BDW too (actually
> BSpec shows it for BDW only?). If that's the case, then we should be
> able to kill gen8_emit_flush_coherentl3_wa(), no? Well, as long as
> someone goes and adds the DC flush to the normal post batch flush.
>
Yes this is applicable for BDW also but I wanted to keep only Gen9 
patches in this series. I will send separate patch for BDW.
We would still need gen8_emit_flush_coherentl3_wa() because WA requires 
that the flush need to happen from the WA batch itself during context 
switch.

regards
Arun

>>   }
>>
>>   static void skl_init_clock_gating(struct drm_device *dev)
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Ville Syrjälä Sept. 28, 2015, 4:02 p.m. UTC | #3 
On Mon, Sep 28, 2015 at 04:51:52PM +0100, Arun Siluvery wrote:
> On 25/09/2015 18:09, Ville Syrjälä wrote:
> > On Fri, Sep 25, 2015 at 02:33:36PM +0100, Arun Siluvery wrote:
> >> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_pm.c | 4 ++++
> >>   1 file changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >> index ab5ac5e..093a5e4 100644
> >> --- a/drivers/gpu/drm/i915/intel_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >> @@ -63,6 +63,10 @@ static void gen9_init_clock_gating(struct drm_device *dev)
> >>   	/* WaDisableKillLogic:bxt,skl */
> >>   	I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) |
> >>   		   ECOCHK_DIS_TLB);
> >> +
> >> +	/* WaOCLCoherentLineFlush:skl,bxt */
> >> +	I915_WRITE(GEN8_L3SQCREG4, (I915_READ(GEN8_L3SQCREG4) |
> >> +				    GEN8_LQSC_FLUSH_COHERENT_LINES));
> >
> > According to Bspec + w/a db this should be done for BDW too (actually
> > BSpec shows it for BDW only?). If that's the case, then we should be
> > able to kill gen8_emit_flush_coherentl3_wa(), no? Well, as long as
> > someone goes and adds the DC flush to the normal post batch flush.
> >
> Yes this is applicable for BDW also but I wanted to keep only Gen9 
> patches in this series. I will send separate patch for BDW.
> We would still need gen8_emit_flush_coherentl3_wa() because WA requires 
> that the flush need to happen from the WA batch itself during context 
> switch.

If we already flush all coherent lines out from DC after each batch,
how can there still be coherent lines in the DC on a context switch?

But spelling that out made me think that mid-batch preemption would
still need it I suppose. But w/o preemption I see no reason why it
would be needed.
arun.siluvery@linux.intel.com Sept. 28, 2015, 4:35 p.m. UTC | #4 
On 28/09/2015 17:02, Ville Syrjälä wrote:
> On Mon, Sep 28, 2015 at 04:51:52PM +0100, Arun Siluvery wrote:
>> On 25/09/2015 18:09, Ville Syrjälä wrote:
>>> On Fri, Sep 25, 2015 at 02:33:36PM +0100, Arun Siluvery wrote:
>>>> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/intel_pm.c | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>>> index ab5ac5e..093a5e4 100644
>>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>>> @@ -63,6 +63,10 @@ static void gen9_init_clock_gating(struct drm_device *dev)
>>>>    	/* WaDisableKillLogic:bxt,skl */
>>>>    	I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) |
>>>>    		   ECOCHK_DIS_TLB);
>>>> +
>>>> +	/* WaOCLCoherentLineFlush:skl,bxt */
>>>> +	I915_WRITE(GEN8_L3SQCREG4, (I915_READ(GEN8_L3SQCREG4) |
>>>> +				    GEN8_LQSC_FLUSH_COHERENT_LINES));
>>>
>>> According to Bspec + w/a db this should be done for BDW too (actually
>>> BSpec shows it for BDW only?). If that's the case, then we should be
>>> able to kill gen8_emit_flush_coherentl3_wa(), no? Well, as long as
>>> someone goes and adds the DC flush to the normal post batch flush.
>>>
>> Yes this is applicable for BDW also but I wanted to keep only Gen9
>> patches in this series. I will send separate patch for BDW.
>> We would still need gen8_emit_flush_coherentl3_wa() because WA requires
>> that the flush need to happen from the WA batch itself during context
>> switch.
>
> If we already flush all coherent lines out from DC after each batch,
> how can there still be coherent lines in the DC on a context switch?
>
> But spelling that out made me think that mid-batch preemption would
> still need it I suppose. But w/o preemption I see no reason why it
> would be needed.
>
Yes, this is to account preemption case (sorry I should've mentioned 
earlier). Most of the workarounds in WA batch are to account for 
preemption related issues.

regards
Arun
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ab5ac5e..093a5e4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -63,6 +63,10 @@  static void gen9_init_clock_gating(struct drm_device *dev)
 	/* WaDisableKillLogic:bxt,skl */
 	I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) |
 		   ECOCHK_DIS_TLB);
+
+	/* WaOCLCoherentLineFlush:skl,bxt */
+	I915_WRITE(GEN8_L3SQCREG4, (I915_READ(GEN8_L3SQCREG4) |
+				    GEN8_LQSC_FLUSH_COHERENT_LINES));
 }
 
 static void skl_init_clock_gating(struct drm_device *dev)