[PATCH] drm/i915: Check idle to active before processing CSQ

[PATCH] drm/i915: Check idle to active before processing CSQ

[Mika Kuoppala]

If idle to active bit is set, the rest of the fields
in CSQ are not valid.

Bail out early if this is the case in order to prevent
rest of the loop inspecting stale values.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 99bba8e..96218bf 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -497,6 +497,9 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
 		status_id = I915_READ(RING_CONTEXT_STATUS_BUF(ring) +
 				(read_pointer % 6) * 8 + 4);
 
+		if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
+			continue;
+
 		if (status & GEN8_CTX_STATUS_PREEMPTED) {
 			if (status & GEN8_CTX_STATUS_LITE_RESTORE) {
 				if (execlists_check_remove_request(ring, status_id))
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Check idle to active before processing CSQ

[Daniel Vetter]

On Thu, Aug 06, 2015 at 05:09:17PM +0300, Mika Kuoppala wrote:
> If idle to active bit is set, the rest of the fields
> in CSQ are not valid.
> 
> Bail out early if this is the case in order to prevent
> rest of the loop inspecting stale values.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

Same questions here too, what's the impact. E.g. if you only found this by
bspec/code inspection then it's for -next, but if it's to fix some known
breakage then it's for -fixes + cc: stable.

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 99bba8e..96218bf 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -497,6 +497,9 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
>  		status_id = I915_READ(RING_CONTEXT_STATUS_BUF(ring) +
>  				(read_pointer % 6) * 8 + 4);
>  
> +		if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
> +			continue;
> +
>  		if (status & GEN8_CTX_STATUS_PREEMPTED) {
>  			if (status & GEN8_CTX_STATUS_LITE_RESTORE) {
>  				if (execlists_check_remove_request(ring, status_id))
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Check idle to active before processing CSQ

[Mika Kuoppala]

Daniel Vetter <daniel@ffwll.ch> writes:

> On Thu, Aug 06, 2015 at 05:09:17PM +0300, Mika Kuoppala wrote:
>> If idle to active bit is set, the rest of the fields
>> in CSQ are not valid.
>> 
>> Bail out early if this is the case in order to prevent
>> rest of the loop inspecting stale values.
>> 
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>
> Same questions here too, what's the impact. E.g. if you only found this by
> bspec/code inspection then it's for -next, but if it's to fix some known
> breakage then it's for -fixes + cc: stable.
>

To this and the masked write one: Both of these were found
when I was trying to find out root cause for skl hangs.

They are both for -next. Both are in the correctness
department vrt bspec and I haven't observed any other
impact.

Point taken on being more verbose.

-Mika

> Thanks, Daniel
>
>> ---
>>  drivers/gpu/drm/i915/intel_lrc.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> index 99bba8e..96218bf 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -497,6 +497,9 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
>>  		status_id = I915_READ(RING_CONTEXT_STATUS_BUF(ring) +
>>  				(read_pointer % 6) * 8 + 4);
>>  
>> +		if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
>> +			continue;
>> +
>>  		if (status & GEN8_CTX_STATUS_PREEMPTED) {
>>  			if (status & GEN8_CTX_STATUS_LITE_RESTORE) {
>>  				if (execlists_check_remove_request(ring, status_id))
>> -- 
>> 2.1.4
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Check idle to active before processing CSQ

[Daniel Vetter]

On Fri, Aug 07, 2015 at 11:15:56AM +0300, Mika Kuoppala wrote:
> Daniel Vetter <daniel@ffwll.ch> writes:
> 
> > On Thu, Aug 06, 2015 at 05:09:17PM +0300, Mika Kuoppala wrote:
> >> If idle to active bit is set, the rest of the fields
> >> in CSQ are not valid.
> >> 
> >> Bail out early if this is the case in order to prevent
> >> rest of the loop inspecting stale values.
> >> 
> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> >
> > Same questions here too, what's the impact. E.g. if you only found this by
> > bspec/code inspection then it's for -next, but if it's to fix some known
> > breakage then it's for -fixes + cc: stable.
> >
> 
> To this and the masked write one: Both of these were found
> when I was trying to find out root cause for skl hangs.
> 
> They are both for -next. Both are in the correctness
> department vrt bspec and I haven't observed any other
> impact.
> 
> Point taken on being more verbose.

Thanks I added a note about this to the first patch and merged it. This
one here still seems to miss an r-b.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Check idle to active before processing CSQ

[Siluvery, Arun]

On 07/08/2015 12:52, Daniel Vetter wrote:
> On Fri, Aug 07, 2015 at 11:15:56AM +0300, Mika Kuoppala wrote:
>> Daniel Vetter <daniel@ffwll.ch> writes:
>>
>>> On Thu, Aug 06, 2015 at 05:09:17PM +0300, Mika Kuoppala wrote:
>>>> If idle to active bit is set, the rest of the fields
>>>> in CSQ are not valid.
>>>>
>>>> Bail out early if this is the case in order to prevent
>>>> rest of the loop inspecting stale values.
>>>>
>>>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>>>

looks good to me, didn't observe any impact with this patch.
Reviewed-by: Arun Siluvery <arun.siluvery@linux.intel.com>

regards
Arun

>>> Same questions here too, what's the impact. E.g. if you only found this by
>>> bspec/code inspection then it's for -next, but if it's to fix some known
>>> breakage then it's for -fixes + cc: stable.
>>>
>>
>> To this and the masked write one: Both of these were found
>> when I was trying to find out root cause for skl hangs.
>>
>> They are both for -next. Both are in the correctness
>> department vrt bspec and I haven't observed any other
>> impact.
>>
>> Point taken on being more verbose.
>
> Thanks I added a note about this to the first patch and merged it. This
> one here still seems to miss an r-b.
> -Daniel
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Check idle to active before processing CSQ

[Daniel Vetter]

On Fri, Aug 07, 2015 at 03:30:12PM +0100, Siluvery, Arun wrote:
> On 07/08/2015 12:52, Daniel Vetter wrote:
> >On Fri, Aug 07, 2015 at 11:15:56AM +0300, Mika Kuoppala wrote:
> >>Daniel Vetter <daniel@ffwll.ch> writes:
> >>
> >>>On Thu, Aug 06, 2015 at 05:09:17PM +0300, Mika Kuoppala wrote:
> >>>>If idle to active bit is set, the rest of the fields
> >>>>in CSQ are not valid.
> >>>>
> >>>>Bail out early if this is the case in order to prevent
> >>>>rest of the loop inspecting stale values.
> >>>>
> >>>>Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> >>>
> 
> looks good to me, didn't observe any impact with this patch.
> Reviewed-by: Arun Siluvery <arun.siluvery@linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel

> 
> regards
> Arun
> 
> >>>Same questions here too, what's the impact. E.g. if you only found this by
> >>>bspec/code inspection then it's for -next, but if it's to fix some known
> >>>breakage then it's for -fixes + cc: stable.
> >>>
> >>
> >>To this and the masked write one: Both of these were found
> >>when I was trying to find out root cause for skl hangs.
> >>
> >>They are both for -next. Both are in the correctness
> >>department vrt bspec and I haven't observed any other
> >>impact.
> >>
> >>Point taken on being more verbose.
> >
> >Thanks I added a note about this to the first patch and merged it. This
> >one here still seems to miss an r-b.
> >-Daniel
> >
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Check idle to active before processing CSQ

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 7109
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                                  302/302              302/302
SNB                                  315/315              315/315
IVB                                  336/336              336/336
BYT                                  283/283              283/283
HSW                                  378/378              378/378
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx