| Message ID | 1438870157-18032-1-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <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
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
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 >
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 > > >
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 '*'
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))
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(+)