diff mbox series

[v2,2/4] drm/i915/gt: Ensure memory quiesced before invalidation

Message ID 20230627094327.134775-3-andi.shyti@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series Update AUX invalidation sequence | expand

Commit Message

Andi Shyti June 27, 2023, 9:43 a.m. UTC
From: Jonathan Cavitt <jonathan.cavitt@intel.com>

All memory traffic must be quiesced before requesting
an aux invalidation on platforms that use Aux CCS.

Fixes: 972282c4cf24 ("drm/i915/gen12: Add aux table invalidate for all engines")
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Cc: <stable@vger.kernel.org> # v5.8+
---
 drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Nirmoy Das July 12, 2023, 2:17 p.m. UTC | #1
Hi Andi and Jonathan,

On 6/27/2023 11:43 AM, Andi Shyti wrote:
> From: Jonathan Cavitt <jonathan.cavitt@intel.com>
>
> All memory traffic must be quiesced before requesting
> an aux invalidation on platforms that use Aux CCS.
>
> Fixes: 972282c4cf24 ("drm/i915/gen12: Add aux table invalidate for all engines")
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: <stable@vger.kernel.org> # v5.8+
> ---
>   drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> index 563efee055602..e10e1ad0e841f 100644
> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> @@ -202,6 +202,13 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
>   {
>   	struct intel_engine_cs *engine = rq->engine;
>   
> +	/*
> +	 * Aux invalidations on Aux CCS platforms require
> +	 * memory traffic is quiesced prior.

I see that we are doing aux inval on EMIT_INVALIDATE so it make sense to

  do if ((mode & EMIT_INVALIDATE) && !HAS_FLAT_CCS(engine->i915) )

> +	 */
> +	if (!HAS_FLAT_CCS(engine->i915))
> +		mode |= EMIT_FLUSH;

I think this generic EMIT_FLUSH is not enough. I seeing some missing 
flags for PIPE_CONTROL

As per https://gfxspecs.intel.com/Predator/Home/Index/43904. It makes 
sense to move this to a

new function given the complexity of PIPE_CONTROL flags requires for this.


Regards,

Nirmoy


> +
>   	if (mode & EMIT_FLUSH) {
>   		u32 flags = 0;
>   		int err;
Cavitt, Jonathan July 12, 2023, 3:39 p.m. UTC | #2
-----Original Message-----
From: Nirmoy Das <nirmoy.das@linux.intel.com> 
Sent: Wednesday, July 12, 2023 7:18 AM
To: Andi Shyti <andi.shyti@linux.intel.com>; Cavitt, Jonathan <jonathan.cavitt@intel.com>
Cc: Intel GFX <intel-gfx@lists.freedesktop.org>; Roper, Matthew D <matthew.d.roper@intel.com>; Chris Wilson <chris@chris-wilson.co.uk>; Mika Kuoppala <mika.kuoppala@linux.intel.com>
Subject: Re: [Intel-gfx] [PATCH v2 2/4] drm/i915/gt: Ensure memory quiesced before invalidation
>
>Hi Andi and Jonathan,
>
>On 6/27/2023 11:43 AM, Andi Shyti wrote:
>> From: Jonathan Cavitt <jonathan.cavitt@intel.com>
>>
>> All memory traffic must be quiesced before requesting
>> an aux invalidation on platforms that use Aux CCS.
>>
>> Fixes: 972282c4cf24 ("drm/i915/gen12: Add aux table invalidate for all engines")
>> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
>> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
>> Cc: <stable@vger.kernel.org> # v5.8+
>> ---
>>   drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
>> index 563efee055602..e10e1ad0e841f 100644
>> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
>> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
>> @@ -202,6 +202,13 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
>>   {
>>   	struct intel_engine_cs *engine = rq->engine;
>>   
>> +	/*
>> +	 * Aux invalidations on Aux CCS platforms require
>> +	 * memory traffic is quiesced prior.
>
>I see that we are doing aux inval on EMIT_INVALIDATE so it make sense to
>
>  do if ((mode & EMIT_INVALIDATE) && !HAS_FLAT_CCS(engine->i915) )
>

This is agreeable, though I don't think there's any instances of us calling gen12_emit_flush_rcs with a blank mode,
since that wouldn't accomplish anything.  So I don't think the additional check/safety net is necessary, but it doesn't
hurt to have.

>> +	 */
>> +	if (!HAS_FLAT_CCS(engine->i915))
>> +		mode |= EMIT_FLUSH;
>
>I think this generic EMIT_FLUSH is not enough. I seeing some missing 
>flags for PIPE_CONTROL
>
>As per https://gfxspecs.intel.com/Predator/Home/Index/43904. It makes 
>sense to move this to a
>
>new function given the complexity of PIPE_CONTROL flags requires for this.
>

I'm assuming when you're talking about the missing flags for PIPE_CONTROL, you're
referring to CCS Flush, correct?  Because every other flag is already covered in the
EMIT_FLUSH path.

I feel like I had this conversation with Matt while the internal version was
developed back in February, and the consensus was that the CCS Flush
requirement was already covered.  On the other hand, it looks like the CCS Flush
requirement was only recently added back in May, so it might be worth
double-checking at the very least.

Although... if CCS Flush is a missing flag, I wonder how we're supposed to set it,
as there doesn’t appear to be a definition for such a flag in intel_gpu_commands.h...

-Jonathan Cavitt

>
>Regards,
>
>Nirmoy
>
>
>> +
>>   	if (mode & EMIT_FLUSH) {
>>   		u32 flags = 0;
>>   		int err;
>
Nirmoy Das July 13, 2023, 9:31 a.m. UTC | #3
On 7/12/2023 5:39 PM, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Nirmoy Das <nirmoy.das@linux.intel.com>
> Sent: Wednesday, July 12, 2023 7:18 AM
> To: Andi Shyti <andi.shyti@linux.intel.com>; Cavitt, Jonathan <jonathan.cavitt@intel.com>
> Cc: Intel GFX <intel-gfx@lists.freedesktop.org>; Roper, Matthew D <matthew.d.roper@intel.com>; Chris Wilson <chris@chris-wilson.co.uk>; Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Subject: Re: [Intel-gfx] [PATCH v2 2/4] drm/i915/gt: Ensure memory quiesced before invalidation
>> Hi Andi and Jonathan,
>>
>> On 6/27/2023 11:43 AM, Andi Shyti wrote:
>>> From: Jonathan Cavitt <jonathan.cavitt@intel.com>
>>>
>>> All memory traffic must be quiesced before requesting
>>> an aux invalidation on platforms that use Aux CCS.
>>>
>>> Fixes: 972282c4cf24 ("drm/i915/gen12: Add aux table invalidate for all engines")
>>> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
>>> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
>>> Cc: <stable@vger.kernel.org> # v5.8+
>>> ---
>>>    drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 7 +++++++
>>>    1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
>>> index 563efee055602..e10e1ad0e841f 100644
>>> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
>>> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
>>> @@ -202,6 +202,13 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
>>>    {
>>>    	struct intel_engine_cs *engine = rq->engine;
>>>    
>>> +	/*
>>> +	 * Aux invalidations on Aux CCS platforms require
>>> +	 * memory traffic is quiesced prior.
>> I see that we are doing aux inval on EMIT_INVALIDATE so it make sense to
>>
>>   do if ((mode & EMIT_INVALIDATE) && !HAS_FLAT_CCS(engine->i915) )
>>
> This is agreeable, though I don't think there's any instances of us calling gen12_emit_flush_rcs with a blank mode,
> since that wouldn't accomplish anything.  So I don't think the additional check/safety net is necessary, but it doesn't
> hurt to have.
>
>>> +	 */
>>> +	if (!HAS_FLAT_CCS(engine->i915))
>>> +		mode |= EMIT_FLUSH;
>> I think this generic EMIT_FLUSH is not enough. I seeing some missing
>> flags for PIPE_CONTROL
>>
>> As per https://gfxspecs.intel.com/Predator/Home/Index/43904. It makes
>> sense to move this to a
>>
>> new function given the complexity of PIPE_CONTROL flags requires for this.
>>
> I'm assuming when you're talking about the missing flags for PIPE_CONTROL, you're
> referring to CCS Flush, correct?  Because every other flag is already covered in the
> EMIT_FLUSH path.

Yes, CCS Flush and I don't see a L3 fabric flush as well.


>
> I feel like I had this conversation with Matt while the internal version was
> developed back in February, and the consensus was that the CCS Flush
> requirement was already covered.

Wasn't aware of this, would be nice to have a confirmation and a comment 
so we

don't get confused in future.

>    On the other hand, it looks like the CCS Flush
> requirement was only recently added back in May, so it might be worth
> double-checking at the very least.
>
> Although... if CCS Flush is a missing flag, I wonder how we're supposed to set it,
> as there doesn’t appear to be a definition for such a flag in intel_gpu_commands.h...


Yes, not yet but we should add a flag for that.


Thanks,

Nirmoy

> -Jonathan Cavitt
>
>> Regards,
>>
>> Nirmoy
>>
>>
>>> +
>>>    	if (mode & EMIT_FLUSH) {
>>>    		u32 flags = 0;
>>>    		int err;
Andi Shyti July 13, 2023, 12:31 p.m. UTC | #4
Hi Nirmoy and Jonathan,

> > > > @@ -202,6 +202,13 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
> > > >    {
> > > >    	struct intel_engine_cs *engine = rq->engine;
> > > > +	/*
> > > > +	 * Aux invalidations on Aux CCS platforms require
> > > > +	 * memory traffic is quiesced prior.
> > > I see that we are doing aux inval on EMIT_INVALIDATE so it make sense to
> > > 
> > >   do if ((mode & EMIT_INVALIDATE) && !HAS_FLAT_CCS(engine->i915) )
> > > 
> > This is agreeable, though I don't think there's any instances of us calling gen12_emit_flush_rcs with a blank mode,
> > since that wouldn't accomplish anything.  So I don't think the additional check/safety net is necessary, but it doesn't
> > hurt to have.

so... do we agree here that we don't add anything? I don't really
mind...

Or, I can queue up a patch 5 adding this "pedantic" check and we
can discuss it separately.

> > > > +	 */
> > > > +	if (!HAS_FLAT_CCS(engine->i915))
> > > > +		mode |= EMIT_FLUSH;
> > > I think this generic EMIT_FLUSH is not enough. I seeing some missing
> > > flags for PIPE_CONTROL
> > > 
> > > As per https://gfxspecs.intel.com/Predator/Home/Index/43904. It makes
> > > sense to move this to a
> > > 
> > > new function given the complexity of PIPE_CONTROL flags requires for this.
> > > 
> > I'm assuming when you're talking about the missing flags for PIPE_CONTROL, you're
> > referring to CCS Flush, correct?  Because every other flag is already covered in the
> > EMIT_FLUSH path.
> 
> Yes, CCS Flush and I don't see a L3 fabric flush as well.
> 
> 
> > 
> > I feel like I had this conversation with Matt while the internal version was
> > developed back in February, and the consensus was that the CCS Flush
> > requirement was already covered.
> 
> Wasn't aware of this, would be nice to have a confirmation and a comment so
> we
> 
> don't get confused in future.
> 
> >    On the other hand, it looks like the CCS Flush
> > requirement was only recently added back in May, so it might be worth
> > double-checking at the very least.
> > 
> > Although... if CCS Flush is a missing flag, I wonder how we're supposed to set it,
> > as there doesn’t appear to be a definition for such a flag in intel_gpu_commands.h...
> 
> 
> Yes, not yet but we should add a flag for that.

Is it OK if I add in the comment that EMIT_FLUSH covers the CCS
flushing?

Andi
Nirmoy Das July 13, 2023, 2:12 p.m. UTC | #5
Hi Andi,

On 7/13/2023 2:31 PM, Andi Shyti wrote:
> Hi Nirmoy and Jonathan,
>
>>>>> @@ -202,6 +202,13 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
>>>>>     {
>>>>>     	struct intel_engine_cs *engine = rq->engine;
>>>>> +	/*
>>>>> +	 * Aux invalidations on Aux CCS platforms require
>>>>> +	 * memory traffic is quiesced prior.
>>>> I see that we are doing aux inval on EMIT_INVALIDATE so it make sense to
>>>>
>>>>    do if ((mode & EMIT_INVALIDATE) && !HAS_FLAT_CCS(engine->i915) )
>>>>
>>> This is agreeable, though I don't think there's any instances of us calling gen12_emit_flush_rcs with a blank mode,
>>> since that wouldn't accomplish anything.  So I don't think the additional check/safety net is necessary, but it doesn't
>>> hurt to have.
> so... do we agree here that we don't add anything? I don't really
> mind...

Not a blocking objection but if you are sending another revision of this 
then why not.


> Or, I can queue up a patch 5 adding this "pedantic" check and we
> can discuss it separately.
>
>>>>> +	 */
>>>>> +	if (!HAS_FLAT_CCS(engine->i915))
>>>>> +		mode |= EMIT_FLUSH;
>>>> I think this generic EMIT_FLUSH is not enough. I seeing some missing
>>>> flags for PIPE_CONTROL
>>>>
>>>> As per https://gfxspecs.intel.com/Predator/Home/Index/43904. It makes
>>>> sense to move this to a
>>>>
>>>> new function given the complexity of PIPE_CONTROL flags requires for this.
>>>>
>>> I'm assuming when you're talking about the missing flags for PIPE_CONTROL, you're
>>> referring to CCS Flush, correct?  Because every other flag is already covered in the
>>> EMIT_FLUSH path.
>> Yes, CCS Flush and I don't see a L3 fabric flush as well.
>>
>>
>>> I feel like I had this conversation with Matt while the internal version was
>>> developed back in February, and the consensus was that the CCS Flush
>>> requirement was already covered.
>> Wasn't aware of this, would be nice to have a confirmation and a comment so
>> we
>>
>> don't get confused in future.
>>
>>>     On the other hand, it looks like the CCS Flush
>>> requirement was only recently added back in May, so it might be worth
>>> double-checking at the very least.
>>>
>>> Although... if CCS Flush is a missing flag, I wonder how we're supposed to set it,
>>> as there doesn’t appear to be a definition for such a flag in intel_gpu_commands.h...
>>
>> Yes, not yet but we should add a flag for that.
> Is it OK if I add in the comment that EMIT_FLUSH covers the CCS
> flushing?


is it though ? I don't see that in the bspec, may be I am missing 
something ?


Regards,

Nirmoy

>
> Andi
Cavitt, Jonathan July 13, 2023, 2:23 p.m. UTC | #6
-----Original Message-----
From: Nirmoy Das <nirmoy.das@linux.intel.com> 
Sent: Thursday, July 13, 2023 7:12 AM
To: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Cavitt, Jonathan <jonathan.cavitt@intel.com>; Intel GFX <intel-gfx@lists.freedesktop.org>; Roper, Matthew D <matthew.d.roper@intel.com>; Chris Wilson <chris@chris-wilson.co.uk>; Mika Kuoppala <mika.kuoppala@linux.intel.com>
Subject: Re: [Intel-gfx] [PATCH v2 2/4] drm/i915/gt: Ensure memory quiesced before invalidation
>
>Hi Andi,
>
>On 7/13/2023 2:31 PM, Andi Shyti wrote:
>> Hi Nirmoy and Jonathan,
>>
>>>>>> @@ -202,6 +202,13 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
>>>>>>     {
>>>>>>     	struct intel_engine_cs *engine = rq->engine;
>>>>>> +	/*
>>>>>> +	 * Aux invalidations on Aux CCS platforms require
>>>>>> +	 * memory traffic is quiesced prior.
>>>>> I see that we are doing aux inval on EMIT_INVALIDATE so it make sense to
>>>>>
>>>>>    do if ((mode & EMIT_INVALIDATE) && !HAS_FLAT_CCS(engine->i915) )
>>>>>
>>>> This is agreeable, though I don't think there's any instances of us calling gen12_emit_flush_rcs with a blank mode,
>>>> since that wouldn't accomplish anything.  So I don't think the additional check/safety net is necessary, but it doesn't
>>>> hurt to have.
>> so... do we agree here that we don't add anything? I don't really
>> mind...
>
>Not a blocking objection but if you are sending another revision of this 
>then why not.


That's about my stance on it as well.  I'd defer the decision to Nirmoy here.


>
>
>> Or, I can queue up a patch 5 adding this "pedantic" check and we
>> can discuss it separately.


I wouldn't offer much in terms of the discussion, unfortunately, so I'm fairly certain the
only thing that would come from that is a series of questions asking why the patch
wasn't squashed with this one to begin with.


>>
>>>>>> +	 */
>>>>>> +	if (!HAS_FLAT_CCS(engine->i915))
>>>>>> +		mode |= EMIT_FLUSH;
>>>>> I think this generic EMIT_FLUSH is not enough. I seeing some missing
>>>>> flags for PIPE_CONTROL
>>>>>
>>>>> As per https://gfxspecs.intel.com/Predator/Home/Index/43904. It makes
>>>>> sense to move this to a
>>>>>
>>>>> new function given the complexity of PIPE_CONTROL flags requires for this.
>>>>>
>>>> I'm assuming when you're talking about the missing flags for PIPE_CONTROL, you're
>>>> referring to CCS Flush, correct?  Because every other flag is already covered in the
>>>> EMIT_FLUSH path.
>>> Yes, CCS Flush and I don't see a L3 fabric flush as well.


Does PIPE_CONTROL_FLUSH_L3 not do this?


>>>
>>>
>>>> I feel like I had this conversation with Matt while the internal version was
>>>> developed back in February, and the consensus was that the CCS Flush
>>>> requirement was already covered.
>>> Wasn't aware of this, would be nice to have a confirmation and a comment so
>>> we
>>>
>>> don't get confused in future.
>>>
>>>>     On the other hand, it looks like the CCS Flush
>>>> requirement was only recently added back in May, so it might be worth
>>>> double-checking at the very least.
>>>>
>>>> Although... if CCS Flush is a missing flag, I wonder how we're supposed to set it,
>>>> as there doesn't appear to be a definition for such a flag in intel_gpu_commands.h...
>>>
>>> Yes, not yet but we should add a flag for that.
>> Is it OK if I add in the comment that EMIT_FLUSH covers the CCS
>> flushing?
>
>
>is it though ? I don't see that in the bspec, may be I am missing 
>something ?


That would certainly explain why there's no flag for it, if performing the CCS Flush
is a part of the EMIT_FLUSH pipeline by default.
-Jonathan Cavitt


>
>
>Regards,
>
>Nirmoy
>
>>
>> Andi
>
Nirmoy Das July 14, 2023, 10:24 a.m. UTC | #7
On 7/13/2023 4:23 PM, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Nirmoy Das <nirmoy.das@linux.intel.com>
> Sent: Thursday, July 13, 2023 7:12 AM
> To: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Cavitt, Jonathan <jonathan.cavitt@intel.com>; Intel GFX <intel-gfx@lists.freedesktop.org>; Roper, Matthew D <matthew.d.roper@intel.com>; Chris Wilson <chris@chris-wilson.co.uk>; Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Subject: Re: [Intel-gfx] [PATCH v2 2/4] drm/i915/gt: Ensure memory quiesced before invalidation
>> Hi Andi,
>>
>> On 7/13/2023 2:31 PM, Andi Shyti wrote:
>>> Hi Nirmoy and Jonathan,
>>>
>>>>>>> @@ -202,6 +202,13 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
>>>>>>>      {
>>>>>>>      	struct intel_engine_cs *engine = rq->engine;
>>>>>>> +	/*
>>>>>>> +	 * Aux invalidations on Aux CCS platforms require
>>>>>>> +	 * memory traffic is quiesced prior.
>>>>>> I see that we are doing aux inval on EMIT_INVALIDATE so it make sense to
>>>>>>
>>>>>>     do if ((mode & EMIT_INVALIDATE) && !HAS_FLAT_CCS(engine->i915) )
>>>>>>
>>>>> This is agreeable, though I don't think there's any instances of us calling gen12_emit_flush_rcs with a blank mode,
>>>>> since that wouldn't accomplish anything.  So I don't think the additional check/safety net is necessary, but it doesn't
>>>>> hurt to have.
>>> so... do we agree here that we don't add anything? I don't really
>>> mind...
>> Not a blocking objection but if you are sending another revision of this
>> then why not.
>
> That's about my stance on it as well.  I'd defer the decision to Nirmoy here.
>
>
>>
>>> Or, I can queue up a patch 5 adding this "pedantic" check and we
>>> can discuss it separately.
>
> I wouldn't offer much in terms of the discussion, unfortunately, so I'm fairly certain the
> only thing that would come from that is a series of questions asking why the patch
> wasn't squashed with this one to begin with.
>
>
>>>>>>> +	 */
>>>>>>> +	if (!HAS_FLAT_CCS(engine->i915))
>>>>>>> +		mode |= EMIT_FLUSH;
>>>>>> I think this generic EMIT_FLUSH is not enough. I seeing some missing
>>>>>> flags for PIPE_CONTROL
>>>>>>
>>>>>> As per https://gfxspecs.intel.com/Predator/Home/Index/43904. It makes
>>>>>> sense to move this to a
>>>>>>
>>>>>> new function given the complexity of PIPE_CONTROL flags requires for this.
>>>>>>
>>>>> I'm assuming when you're talking about the missing flags for PIPE_CONTROL, you're
>>>>> referring to CCS Flush, correct?  Because every other flag is already covered in the
>>>>> EMIT_FLUSH path.
>>>> Yes, CCS Flush and I don't see a L3 fabric flush as well.
>
> Does PIPE_CONTROL_FLUSH_L3 not do this?

It does actually, was not very clear from 1st look.


>
>
>>>>
>>>>> I feel like I had this conversation with Matt while the internal version was
>>>>> developed back in February, and the consensus was that the CCS Flush
>>>>> requirement was already covered.
>>>> Wasn't aware of this, would be nice to have a confirmation and a comment so
>>>> we
>>>>
>>>> don't get confused in future.
>>>>
>>>>>      On the other hand, it looks like the CCS Flush
>>>>> requirement was only recently added back in May, so it might be worth
>>>>> double-checking at the very least.
>>>>>
>>>>> Although... if CCS Flush is a missing flag, I wonder how we're supposed to set it,
>>>>> as there doesn't appear to be a definition for such a flag in intel_gpu_commands.h...
>>>> Yes, not yet but we should add a flag for that.
>>> Is it OK if I add in the comment that EMIT_FLUSH covers the CCS
>>> flushing?
>>
>> is it though ? I don't see that in the bspec, may be I am missing
>> something ?
>
> That would certainly explain why there's no flag for it, if performing the CCS Flush
> is a part of the EMIT_FLUSH pipeline by default.

CCS Flush is new and I see a new bit for that in the gfxspec.

With that added, the patch looks good to me.


Thanks for your patience, Jonathan!


Nirmoy


Regards,

Nirmoy

> -Jonathan Cavitt
>
>
>>
>> Regards,
>>
>> Nirmoy
>>
>>> Andi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
index 563efee055602..e10e1ad0e841f 100644
--- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
@@ -202,6 +202,13 @@  int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
 {
 	struct intel_engine_cs *engine = rq->engine;
 
+	/*
+	 * Aux invalidations on Aux CCS platforms require
+	 * memory traffic is quiesced prior.
+	 */
+	if (!HAS_FLAT_CCS(engine->i915))
+		mode |= EMIT_FLUSH;
+
 	if (mode & EMIT_FLUSH) {
 		u32 flags = 0;
 		int err;