* [PATCH 1/2] drm/i915: Shut up GuC errors when it's disabled
@ 2015-10-23 8:56 Daniel Vetter
2015-10-23 8:56 ` [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise Daniel Vetter
` (3 more replies)
0 siblings, 4 replies; 23+ messages in thread
From: Daniel Vetter @ 2015-10-23 8:56 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter, Mika Kuoppala
DRM_ERROR an continue without any issues aren't allowed since that
causes noise in the CI system. But we absolutely want to have the
DRM_ERROR when we want to run with GuC.
For simplicity just short-circuit all the loader code when it's not
needed.
v2: Mika&Chris complained that I shouldn't hit send on patches written
before coffee kicks in.
Cc: Alex Dai <yu.dai@intel.com>
Cc: Dave Gordon <david.s.gordon@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 15 +++------------
drivers/gpu/drm/i915/intel_guc_loader.c | 6 ++++++
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 01fef54ecb2d..41c1adb930ef 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4747,18 +4747,9 @@ i915_gem_init_hw(struct drm_device *dev)
if (HAS_GUC_UCODE(dev)) {
ret = intel_guc_ucode_load(dev);
if (ret) {
- /*
- * If we got an error and GuC submission is enabled, map
- * the error to -EIO so the GPU will be declared wedged.
- * OTOH, if we didn't intend to use the GuC anyway, just
- * discard the error and carry on.
- */
- DRM_ERROR("Failed to initialize GuC, error %d%s\n", ret,
- i915.enable_guc_submission ? "" :
- " (ignored)");
- ret = i915.enable_guc_submission ? -EIO : 0;
- if (ret)
- goto out;
+ DRM_ERROR("Failed to initialize GuC, error %d\n", ret,
+ ret = -EIO;
+ goto out;
}
}
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index c0281df52702..bbbf9b3b00eb 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -364,6 +364,9 @@ int intel_guc_ucode_load(struct drm_device *dev)
struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
int err = 0;
+ if (!i915.enable_guc_submission)
+ return 0;
+
DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
@@ -589,6 +592,9 @@ void intel_guc_ucode_init(struct drm_device *dev)
fw_path = ""; /* unknown device */
}
+ if (!i915.enable_guc_submission)
+ return;
+
guc_fw->guc_dev = dev;
guc_fw->guc_fw_path = fw_path;
guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE;
--
2.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise
2015-10-23 8:56 [PATCH 1/2] drm/i915: Shut up GuC errors when it's disabled Daniel Vetter
@ 2015-10-23 8:56 ` Daniel Vetter
2015-10-23 9:03 ` Chris Wilson
2015-10-23 9:00 ` [PATCH] drm/i915: Don't complain about lack of ACPI video bios Daniel Vetter
` (2 subsequent siblings)
3 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2015-10-23 8:56 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter
We get tons of cases where the master interrupt handler apparently set
a bit, with the SDEIIR agreeing. No idea what's going on there, but
it's consistent on gen8+, no one seems to care about it and it's
making CI results flaky.
Shut it up.
No idea what's going on here, but we've had fun with PCH interrupts
before:
commit 44498aea293b37af1d463acd9658cdce1ecdf427
Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
Date: Fri Feb 22 17:05:28 2013 -0300
drm/i915: also disable south interrupts when handling them
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9caf22caed89..a614e89fc934 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2353,9 +2353,13 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
spt_irq_handler(dev, pch_iir);
else
cpt_irq_handler(dev, pch_iir);
- } else
- DRM_ERROR("The master control interrupt lied (SDE)!\n");
-
+ } else {
+ /*
+ * Like on previous PCH there seems to be something
+ * fishy going on with forwarding PCH interrupts.
+ */
+ DRM_DEBUG_DRIVER("The master control interrupt lied (SDE)!\n");
+ }
}
I915_WRITE_FW(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
--
2.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise
2015-10-23 8:56 ` [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise Daniel Vetter
@ 2015-10-23 9:03 ` Chris Wilson
2015-10-23 9:14 ` Daniel Vetter
2015-10-23 9:21 ` Jani Nikula
0 siblings, 2 replies; 23+ messages in thread
From: Chris Wilson @ 2015-10-23 9:03 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development
On Fri, Oct 23, 2015 at 10:56:12AM +0200, Daniel Vetter wrote:
> We get tons of cases where the master interrupt handler apparently set
> a bit, with the SDEIIR agreeing. No idea what's going on there, but
> it's consistent on gen8+, no one seems to care about it and it's
> making CI results flaky.
Just delete the message and delete them all. There isn't anything we can
do and if anybody actually cared (which apparently they didn't in the
first place), they could just trace the mmio.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise
2015-10-23 9:03 ` Chris Wilson
@ 2015-10-23 9:14 ` Daniel Vetter
2015-10-23 14:21 ` Dave Gordon
2015-10-23 9:21 ` Jani Nikula
1 sibling, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2015-10-23 9:14 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter, Intel Graphics Development,
Daniel Vetter
On Fri, Oct 23, 2015 at 10:03:50AM +0100, Chris Wilson wrote:
> On Fri, Oct 23, 2015 at 10:56:12AM +0200, Daniel Vetter wrote:
> > We get tons of cases where the master interrupt handler apparently set
> > a bit, with the SDEIIR agreeing. No idea what's going on there, but
> > it's consistent on gen8+, no one seems to care about it and it's
> > making CI results flaky.
>
> Just delete the message and delete them all. There isn't anything we can
> do and if anybody actually cared (which apparently they didn't in the
> first place), they could just trace the mmio.
The non-SDE ones don't fire and I think are useful. And in case we have
another report from users that gmbus is not reliably working with their
touchpad (this is how we discovered the original pch irq issues) I think
finding some breadcrumbs in dmesg would be useful. The SDE one happens
rarely enough that I don't think it should be a performance issue, ever.
Hence why I decided to keep them.
-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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise
2015-10-23 9:14 ` Daniel Vetter
@ 2015-10-23 14:21 ` Dave Gordon
0 siblings, 0 replies; 23+ messages in thread
From: Dave Gordon @ 2015-10-23 14:21 UTC (permalink / raw)
To: Daniel Vetter, Chris Wilson, Daniel Vetter,
Intel Graphics Development, Daniel Vetter
On 23/10/15 10:14, Daniel Vetter wrote:
> On Fri, Oct 23, 2015 at 10:03:50AM +0100, Chris Wilson wrote:
>> On Fri, Oct 23, 2015 at 10:56:12AM +0200, Daniel Vetter wrote:
>>> We get tons of cases where the master interrupt handler apparently set
>>> a bit, with the SDEIIR agreeing. No idea what's going on there, but
>>> it's consistent on gen8+, no one seems to care about it and it's
>>> making CI results flaky.
>>
>> Just delete the message and delete them all. There isn't anything we can
>> do and if anybody actually cared (which apparently they didn't in the
>> first place), they could just trace the mmio.
>
> The non-SDE ones don't fire and I think are useful. And in case we have
> another report from users that gmbus is not reliably working with their
> touchpad (this is how we discovered the original pch irq issues) I think
> finding some breadcrumbs in dmesg would be useful. The SDE one happens
> rarely enough that I don't think it should be a performance issue, ever.
>
> Hence why I decided to keep them.
> -Daniel
I used to get the "master interrupt lied" message quite frequently.
Since it was not clear whether it really meant that the master had an
extra bit set or that one (any) of the detail registers was missing an
interrupt bit, I tried servicing the interrupt anyway. But it didn't
help with any of the issues I was seeing at the time, and the message no
longer occurs (on SKL) with more recent kernels.
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise
2015-10-23 9:03 ` Chris Wilson
2015-10-23 9:14 ` Daniel Vetter
@ 2015-10-23 9:21 ` Jani Nikula
2015-10-23 9:23 ` Daniel Vetter
1 sibling, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2015-10-23 9:21 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development
On Fri, 23 Oct 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Fri, Oct 23, 2015 at 10:56:12AM +0200, Daniel Vetter wrote:
>> We get tons of cases where the master interrupt handler apparently set
>> a bit, with the SDEIIR agreeing. No idea what's going on there, but
>> it's consistent on gen8+, no one seems to care about it and it's
>> making CI results flaky.
>
> Just delete the message and delete them all. There isn't anything we can
> do and if anybody actually cared (which apparently they didn't in the
> first place), they could just trace the mmio.
Except this one is a regression, introduced by a bisected commit, and
suspiciously the errors pop up during aux transfers.
https://bugs.freedesktop.org/show_bug.cgi?id=92084
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise
2015-10-23 9:21 ` Jani Nikula
@ 2015-10-23 9:23 ` Daniel Vetter
2015-10-23 13:33 ` Ville Syrjälä
0 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2015-10-23 9:23 UTC (permalink / raw)
To: Jani Nikula; +Cc: Daniel Vetter, Daniel Vetter, Intel Graphics Development
On Fri, Oct 23, 2015 at 12:21:37PM +0300, Jani Nikula wrote:
> On Fri, 23 Oct 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Fri, Oct 23, 2015 at 10:56:12AM +0200, Daniel Vetter wrote:
> >> We get tons of cases where the master interrupt handler apparently set
> >> a bit, with the SDEIIR agreeing. No idea what's going on there, but
> >> it's consistent on gen8+, no one seems to care about it and it's
> >> making CI results flaky.
> >
> > Just delete the message and delete them all. There isn't anything we can
> > do and if anybody actually cared (which apparently they didn't in the
> > first place), they could just trace the mmio.
>
> Except this one is a regression, introduced by a bisected commit, and
> suspiciously the errors pop up during aux transfers.
dp aux is a red herring very likely, since it's just the source of a _lot_
of sde interrupts.
> https://bugs.freedesktop.org/show_bug.cgi?id=92084
No one demonstrated any bad side-effects of this, let's shut it up (but
keep the breadcrumb in debug logs in case) and move on to other bugs. We
have enough.
-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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise
2015-10-23 9:23 ` Daniel Vetter
@ 2015-10-23 13:33 ` Ville Syrjälä
2015-10-23 13:40 ` Chris Wilson
2015-10-23 14:42 ` Daniel Vetter
0 siblings, 2 replies; 23+ messages in thread
From: Ville Syrjälä @ 2015-10-23 13:33 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
On Fri, Oct 23, 2015 at 11:23:12AM +0200, Daniel Vetter wrote:
> On Fri, Oct 23, 2015 at 12:21:37PM +0300, Jani Nikula wrote:
> > On Fri, 23 Oct 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > On Fri, Oct 23, 2015 at 10:56:12AM +0200, Daniel Vetter wrote:
> > >> We get tons of cases where the master interrupt handler apparently set
> > >> a bit, with the SDEIIR agreeing. No idea what's going on there, but
> > >> it's consistent on gen8+, no one seems to care about it and it's
> > >> making CI results flaky.
> > >
> > > Just delete the message and delete them all. There isn't anything we can
> > > do and if anybody actually cared (which apparently they didn't in the
> > > first place), they could just trace the mmio.
> >
> > Except this one is a regression, introduced by a bisected commit, and
> > suspiciously the errors pop up during aux transfers.
>
> dp aux is a red herring very likely, since it's just the source of a _lot_
> of sde interrupts.
>
> > https://bugs.freedesktop.org/show_bug.cgi?id=92084
>
> No one demonstrated any bad side-effects of this, let's shut it up (but
> keep the breadcrumb in debug logs in case) and move on to other bugs. We
> have enough.
I was still waiting for an answer to my latest idea how to avoid the
error. Would have been a very simple thing to test for anyone with the
hardware.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise
2015-10-23 13:33 ` Ville Syrjälä
@ 2015-10-23 13:40 ` Chris Wilson
2015-10-23 13:47 ` Ville Syrjälä
2015-10-23 14:42 ` Daniel Vetter
1 sibling, 1 reply; 23+ messages in thread
From: Chris Wilson @ 2015-10-23 13:40 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
On Fri, Oct 23, 2015 at 04:33:52PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 23, 2015 at 11:23:12AM +0200, Daniel Vetter wrote:
> > On Fri, Oct 23, 2015 at 12:21:37PM +0300, Jani Nikula wrote:
> > > On Fri, 23 Oct 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > > On Fri, Oct 23, 2015 at 10:56:12AM +0200, Daniel Vetter wrote:
> > > >> We get tons of cases where the master interrupt handler apparently set
> > > >> a bit, with the SDEIIR agreeing. No idea what's going on there, but
> > > >> it's consistent on gen8+, no one seems to care about it and it's
> > > >> making CI results flaky.
> > > >
> > > > Just delete the message and delete them all. There isn't anything we can
> > > > do and if anybody actually cared (which apparently they didn't in the
> > > > first place), they could just trace the mmio.
> > >
> > > Except this one is a regression, introduced by a bisected commit, and
> > > suspiciously the errors pop up during aux transfers.
> >
> > dp aux is a red herring very likely, since it's just the source of a _lot_
> > of sde interrupts.
> >
> > > https://bugs.freedesktop.org/show_bug.cgi?id=92084
> >
> > No one demonstrated any bad side-effects of this, let's shut it up (but
> > keep the breadcrumb in debug logs in case) and move on to other bugs. We
> > have enough.
>
> I was still waiting for an answer to my latest idea how to avoid the
> error. Would have been a very simple thing to test for anyone with the
> hardware.
Presumably,
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2345,6 +2345,7 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
u32 pch_iir = I915_READ(SDEIIR);
if (pch_iir) {
I915_WRITE(SDEIIR, pch_iir);
+ POSTING_READ(SDEIIR);
ret = IRQ_HANDLED;
?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise
2015-10-23 13:40 ` Chris Wilson
@ 2015-10-23 13:47 ` Ville Syrjälä
0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2015-10-23 13:47 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter, Daniel Vetter,
Intel Graphics Development, Daniel Vetter
On Fri, Oct 23, 2015 at 02:40:31PM +0100, Chris Wilson wrote:
> On Fri, Oct 23, 2015 at 04:33:52PM +0300, Ville Syrjälä wrote:
> > On Fri, Oct 23, 2015 at 11:23:12AM +0200, Daniel Vetter wrote:
> > > On Fri, Oct 23, 2015 at 12:21:37PM +0300, Jani Nikula wrote:
> > > > On Fri, 23 Oct 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > > > On Fri, Oct 23, 2015 at 10:56:12AM +0200, Daniel Vetter wrote:
> > > > >> We get tons of cases where the master interrupt handler apparently set
> > > > >> a bit, with the SDEIIR agreeing. No idea what's going on there, but
> > > > >> it's consistent on gen8+, no one seems to care about it and it's
> > > > >> making CI results flaky.
> > > > >
> > > > > Just delete the message and delete them all. There isn't anything we can
> > > > > do and if anybody actually cared (which apparently they didn't in the
> > > > > first place), they could just trace the mmio.
> > > >
> > > > Except this one is a regression, introduced by a bisected commit, and
> > > > suspiciously the errors pop up during aux transfers.
> > >
> > > dp aux is a red herring very likely, since it's just the source of a _lot_
> > > of sde interrupts.
> > >
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=92084
> > >
> > > No one demonstrated any bad side-effects of this, let's shut it up (but
> > > keep the breadcrumb in debug logs in case) and move on to other bugs. We
> > > have enough.
> >
> > I was still waiting for an answer to my latest idea how to avoid the
> > error. Would have been a very simple thing to test for anyone with the
> > hardware.
>
> Presumably,
>
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2345,6 +2345,7 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
> u32 pch_iir = I915_READ(SDEIIR);
> if (pch_iir) {
> I915_WRITE(SDEIIR, pch_iir);
> + POSTING_READ(SDEIIR);
> ret = IRQ_HANDLED;
>
> ?
No, we already tried that and it didn't help.
What I was thinking is we'd just do a nop write to the
PCH_PORT_HOTPLUG register like so:
dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
if (!hotplug_trigger)
dig_hotplug_reg &= ~(*_HOTPLUG_STATUS_MASK);
I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg
if (!hotplug_trigger)
return;
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise
2015-10-23 13:33 ` Ville Syrjälä
2015-10-23 13:40 ` Chris Wilson
@ 2015-10-23 14:42 ` Daniel Vetter
2015-10-23 15:22 ` Ville Syrjälä
1 sibling, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2015-10-23 14:42 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Daniel Vetter, Intel Graphics Development
On Fri, Oct 23, 2015 at 3:33 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Fri, Oct 23, 2015 at 11:23:12AM +0200, Daniel Vetter wrote:
>> On Fri, Oct 23, 2015 at 12:21:37PM +0300, Jani Nikula wrote:
>> > On Fri, 23 Oct 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > > On Fri, Oct 23, 2015 at 10:56:12AM +0200, Daniel Vetter wrote:
>> > >> We get tons of cases where the master interrupt handler apparently set
>> > >> a bit, with the SDEIIR agreeing. No idea what's going on there, but
>> > >> it's consistent on gen8+, no one seems to care about it and it's
>> > >> making CI results flaky.
>> > >
>> > > Just delete the message and delete them all. There isn't anything we can
>> > > do and if anybody actually cared (which apparently they didn't in the
>> > > first place), they could just trace the mmio.
>> >
>> > Except this one is a regression, introduced by a bisected commit, and
>> > suspiciously the errors pop up during aux transfers.
>>
>> dp aux is a red herring very likely, since it's just the source of a _lot_
>> of sde interrupts.
>>
>> > https://bugs.freedesktop.org/show_bug.cgi?id=92084
>>
>> No one demonstrated any bad side-effects of this, let's shut it up (but
>> keep the breadcrumb in debug logs in case) and move on to other bugs. We
>> have enough.
>
> I was still waiting for an answer to my latest idea how to avoid the
> error. Would have been a very simple thing to test for anyone with the
> hardware.
We can still do that ofc. But this was a regression, trivial to fix
(we didn't have the message before at all), creating noise in our CI
and 16 months old. Our rule is that an interim solution/revert should
be applied latest within 1 week, this was more than overdue.
Yep, QA is back and I'll make sure everyone knows ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise
2015-10-23 14:42 ` Daniel Vetter
@ 2015-10-23 15:22 ` Ville Syrjälä
2015-10-27 2:31 ` Daniel Vetter
0 siblings, 1 reply; 23+ messages in thread
From: Ville Syrjälä @ 2015-10-23 15:22 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development
On Fri, Oct 23, 2015 at 04:42:19PM +0200, Daniel Vetter wrote:
> On Fri, Oct 23, 2015 at 3:33 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Fri, Oct 23, 2015 at 11:23:12AM +0200, Daniel Vetter wrote:
> >> On Fri, Oct 23, 2015 at 12:21:37PM +0300, Jani Nikula wrote:
> >> > On Fri, 23 Oct 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >> > > On Fri, Oct 23, 2015 at 10:56:12AM +0200, Daniel Vetter wrote:
> >> > >> We get tons of cases where the master interrupt handler apparently set
> >> > >> a bit, with the SDEIIR agreeing. No idea what's going on there, but
> >> > >> it's consistent on gen8+, no one seems to care about it and it's
> >> > >> making CI results flaky.
> >> > >
> >> > > Just delete the message and delete them all. There isn't anything we can
> >> > > do and if anybody actually cared (which apparently they didn't in the
> >> > > first place), they could just trace the mmio.
> >> >
> >> > Except this one is a regression, introduced by a bisected commit, and
> >> > suspiciously the errors pop up during aux transfers.
> >>
> >> dp aux is a red herring very likely, since it's just the source of a _lot_
> >> of sde interrupts.
> >>
> >> > https://bugs.freedesktop.org/show_bug.cgi?id=92084
> >>
> >> No one demonstrated any bad side-effects of this, let's shut it up (but
> >> keep the breadcrumb in debug logs in case) and move on to other bugs. We
> >> have enough.
> >
> > I was still waiting for an answer to my latest idea how to avoid the
> > error. Would have been a very simple thing to test for anyone with the
> > hardware.
>
> We can still do that ofc. But this was a regression, trivial to fix
> (we didn't have the message before at all),
The regressing patch change didn't add the message, so there was a clear
change in behaviour, and now it's papered over.
> creating noise in our CI
> and 16 months old. Our rule is that an interim solution/revert should
> be applied latest within 1 week, this was more than overdue.
>
> Yep, QA is back and I'll make sure everyone knows ;-)
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise
2015-10-23 15:22 ` Ville Syrjälä
@ 2015-10-27 2:31 ` Daniel Vetter
2015-10-27 8:10 ` Jani Nikula
0 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2015-10-27 2:31 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Daniel Vetter, Intel Graphics Development
On Fri, Oct 23, 2015 at 8:22 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> The regressing patch change didn't add the message, so there was a clear
> change in behaviour, and now it's papered over.
It did move around the DRM_ERROR for all the others and also added the
SDE one for consistency. At least that's how I read that patch - I
could't find the SDE DRM_ERROR in the old code. Did I miss something?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise
2015-10-27 2:31 ` Daniel Vetter
@ 2015-10-27 8:10 ` Jani Nikula
2015-10-28 0:22 ` Daniel Vetter
0 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2015-10-27 8:10 UTC (permalink / raw)
To: Daniel Vetter, Ville Syrjälä
Cc: Daniel Vetter, Intel Graphics Development
On Tue, 27 Oct 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Oct 23, 2015 at 8:22 AM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
>> The regressing patch change didn't add the message, so there was a clear
>> change in behaviour, and now it's papered over.
>
> It did move around the DRM_ERROR for all the others and also added the
> SDE one for consistency. At least that's how I read that patch - I
> could't find the SDE DRM_ERROR in the old code. Did I miss something?
Yes. We tried and failed to point out that this is a bisected regression
with a bug report. The bad commit is *NOT* when the error message was
added or moved. The first bad commit is
commit aaf5ec2e51ab1d9c5e962b4728a1107ed3ff7a3e
Author: Sonika Jindal <sonika.jindal@intel.com>
Date: Wed Jul 8 17:07:47 2015 +0530
drm/i915: Handle HPD when it has actually occurred
which triggers printing of the error message. This is all mentioned in
the bug, along with a few attempts at remedying the situation.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise
2015-10-27 8:10 ` Jani Nikula
@ 2015-10-28 0:22 ` Daniel Vetter
0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2015-10-28 0:22 UTC (permalink / raw)
To: Jani Nikula; +Cc: Daniel Vetter, Intel Graphics Development
On Tue, Oct 27, 2015 at 1:10 AM, Jani Nikula
<jani.nikula@linux.intel.com> wrote:
> On Tue, 27 Oct 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Fri, Oct 23, 2015 at 8:22 AM, Ville Syrjälä
>> <ville.syrjala@linux.intel.com> wrote:
>>> The regressing patch change didn't add the message, so there was a clear
>>> change in behaviour, and now it's papered over.
>>
>> It did move around the DRM_ERROR for all the others and also added the
>> SDE one for consistency. At least that's how I read that patch - I
>> could't find the SDE DRM_ERROR in the old code. Did I miss something?
>
> Yes. We tried and failed to point out that this is a bisected regression
> with a bug report. The bad commit is *NOT* when the error message was
> added or moved. The first bad commit is
>
> commit aaf5ec2e51ab1d9c5e962b4728a1107ed3ff7a3e
> Author: Sonika Jindal <sonika.jindal@intel.com>
> Date: Wed Jul 8 17:07:47 2015 +0530
>
> drm/i915: Handle HPD when it has actually occurred
>
> which triggers printing of the error message. This is all mentioned in
> the bug, along with a few attempts at remedying the situation.
Oops, I pasted the wrong commit into the commit message :( Sorry for
all the confusion and me not noticing the real bisect result.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] drm/i915: Don't complain about lack of ACPI video bios
2015-10-23 8:56 [PATCH 1/2] drm/i915: Shut up GuC errors when it's disabled Daniel Vetter
2015-10-23 8:56 ` [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise Daniel Vetter
@ 2015-10-23 9:00 ` Daniel Vetter
2015-10-23 11:46 ` Jani Nikula
2015-10-23 9:10 ` [PATCH] drm/i915: Shut up GuC errors when it's disabled Daniel Vetter
2015-10-23 9:21 ` [PATCH 1/2] " kbuild test robot
3 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2015-10-23 9:00 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter
Another CI fail we have for no reason. Totally unjustified since
nothing fails at all.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/i915/intel_opregion.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 6dc13c02c28e..e362a30776fa 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -682,7 +682,7 @@ static void intel_didl_outputs(struct drm_device *dev)
}
if (!acpi_video_bus) {
- DRM_ERROR("No ACPI video bus found\n");
+ DRM_DEBUG_KMS("No ACPI video bus found\n");
return;
}
--
2.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH] drm/i915: Don't complain about lack of ACPI video bios
2015-10-23 9:00 ` [PATCH] drm/i915: Don't complain about lack of ACPI video bios Daniel Vetter
@ 2015-10-23 11:46 ` Jani Nikula
0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2015-10-23 11:46 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter
On Fri, 23 Oct 2015, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Another CI fail we have for no reason. Totally unjustified since
> nothing fails at all.
Acked-by: Jani Nikula <jani.nikula@intel.com>
I guess we could do the blind set here as well, but *shrug*.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/gpu/drm/i915/intel_opregion.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index 6dc13c02c28e..e362a30776fa 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -682,7 +682,7 @@ static void intel_didl_outputs(struct drm_device *dev)
> }
>
> if (!acpi_video_bus) {
> - DRM_ERROR("No ACPI video bus found\n");
> + DRM_DEBUG_KMS("No ACPI video bus found\n");
> return;
> }
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] drm/i915: Shut up GuC errors when it's disabled
2015-10-23 8:56 [PATCH 1/2] drm/i915: Shut up GuC errors when it's disabled Daniel Vetter
2015-10-23 8:56 ` [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise Daniel Vetter
2015-10-23 9:00 ` [PATCH] drm/i915: Don't complain about lack of ACPI video bios Daniel Vetter
@ 2015-10-23 9:10 ` Daniel Vetter
2015-10-23 10:56 ` Mika Kuoppala
2015-10-23 9:21 ` [PATCH 1/2] " kbuild test robot
3 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2015-10-23 9:10 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter, Mika Kuoppala
DRM_ERROR an continue without any issues aren't allowed since that
causes noise in the CI system. But we absolutely want to have the
DRM_ERROR when we want to run with GuC.
For simplicity just short-circuit all the loader code when it's not
needed.
v2: Mika&Chris complained that I shouldn't hit send on patches written
before coffee kicks in.
v3: Make it compile at least ...
Cc: Alex Dai <yu.dai@intel.com>
Cc: Dave Gordon <david.s.gordon@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 15 +++------------
drivers/gpu/drm/i915/intel_guc_loader.c | 6 ++++++
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c067706ce53c..9552647a925d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4752,18 +4752,9 @@ i915_gem_init_hw(struct drm_device *dev)
if (HAS_GUC_UCODE(dev)) {
ret = intel_guc_ucode_load(dev);
if (ret) {
- /*
- * If we got an error and GuC submission is enabled, map
- * the error to -EIO so the GPU will be declared wedged.
- * OTOH, if we didn't intend to use the GuC anyway, just
- * discard the error and carry on.
- */
- DRM_ERROR("Failed to initialize GuC, error %d%s\n", ret,
- i915.enable_guc_submission ? "" :
- " (ignored)");
- ret = i915.enable_guc_submission ? -EIO : 0;
- if (ret)
- goto out;
+ DRM_ERROR("Failed to initialize GuC, error %d\n", ret);
+ ret = -EIO;
+ goto out;
}
}
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index c0281df52702..bbbf9b3b00eb 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -364,6 +364,9 @@ int intel_guc_ucode_load(struct drm_device *dev)
struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
int err = 0;
+ if (!i915.enable_guc_submission)
+ return 0;
+
DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
@@ -589,6 +592,9 @@ void intel_guc_ucode_init(struct drm_device *dev)
fw_path = ""; /* unknown device */
}
+ if (!i915.enable_guc_submission)
+ return;
+
guc_fw->guc_dev = dev;
guc_fw->guc_fw_path = fw_path;
guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE;
--
2.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH] drm/i915: Shut up GuC errors when it's disabled
2015-10-23 9:10 ` [PATCH] drm/i915: Shut up GuC errors when it's disabled Daniel Vetter
@ 2015-10-23 10:56 ` Mika Kuoppala
2015-10-23 11:32 ` Daniel Vetter
0 siblings, 1 reply; 23+ messages in thread
From: Mika Kuoppala @ 2015-10-23 10:56 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter
Daniel Vetter <daniel.vetter@ffwll.ch> writes:
> DRM_ERROR an continue without any issues aren't allowed since that
> causes noise in the CI system. But we absolutely want to have the
> DRM_ERROR when we want to run with GuC.
>
> For simplicity just short-circuit all the loader code when it's not
> needed.
>
> v2: Mika&Chris complained that I shouldn't hit send on patches written
> before coffee kicks in.
>
> v3: Make it compile at least ...
>
> Cc: Alex Dai <yu.dai@intel.com>
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
I did test only the case where we have guc disabled,
and the patch does what it advertizes.
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 15 +++------------
> drivers/gpu/drm/i915/intel_guc_loader.c | 6 ++++++
> 2 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index c067706ce53c..9552647a925d 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4752,18 +4752,9 @@ i915_gem_init_hw(struct drm_device *dev)
> if (HAS_GUC_UCODE(dev)) {
> ret = intel_guc_ucode_load(dev);
> if (ret) {
> - /*
> - * If we got an error and GuC submission is enabled, map
> - * the error to -EIO so the GPU will be declared wedged.
> - * OTOH, if we didn't intend to use the GuC anyway, just
> - * discard the error and carry on.
> - */
> - DRM_ERROR("Failed to initialize GuC, error %d%s\n", ret,
> - i915.enable_guc_submission ? "" :
> - " (ignored)");
> - ret = i915.enable_guc_submission ? -EIO : 0;
> - if (ret)
> - goto out;
> + DRM_ERROR("Failed to initialize GuC, error %d\n", ret);
> + ret = -EIO;
> + goto out;
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index c0281df52702..bbbf9b3b00eb 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -364,6 +364,9 @@ int intel_guc_ucode_load(struct drm_device *dev)
> struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> int err = 0;
>
> + if (!i915.enable_guc_submission)
> + return 0;
> +
> DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
> intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
> intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
> @@ -589,6 +592,9 @@ void intel_guc_ucode_init(struct drm_device *dev)
> fw_path = ""; /* unknown device */
> }
>
> + if (!i915.enable_guc_submission)
> + return;
> +
> guc_fw->guc_dev = dev;
> guc_fw->guc_fw_path = fw_path;
> guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE;
> --
> 2.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH] drm/i915: Shut up GuC errors when it's disabled
2015-10-23 10:56 ` Mika Kuoppala
@ 2015-10-23 11:32 ` Daniel Vetter
0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2015-10-23 11:32 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter
On Fri, Oct 23, 2015 at 01:56:40PM +0300, Mika Kuoppala wrote:
> Daniel Vetter <daniel.vetter@ffwll.ch> writes:
>
> > DRM_ERROR an continue without any issues aren't allowed since that
> > causes noise in the CI system. But we absolutely want to have the
> > DRM_ERROR when we want to run with GuC.
> >
> > For simplicity just short-circuit all the loader code when it's not
> > needed.
> >
> > v2: Mika&Chris complained that I shouldn't hit send on patches written
> > before coffee kicks in.
> >
> > v3: Make it compile at least ...
> >
> > Cc: Alex Dai <yu.dai@intel.com>
> > Cc: Dave Gordon <david.s.gordon@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>
> I did test only the case where we have guc disabled,
> and the patch does what it advertizes.
>
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Applied, thanks.
-Daniel
>
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 15 +++------------
> > drivers/gpu/drm/i915/intel_guc_loader.c | 6 ++++++
> > 2 files changed, 9 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index c067706ce53c..9552647a925d 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4752,18 +4752,9 @@ i915_gem_init_hw(struct drm_device *dev)
> > if (HAS_GUC_UCODE(dev)) {
> > ret = intel_guc_ucode_load(dev);
> > if (ret) {
> > - /*
> > - * If we got an error and GuC submission is enabled, map
> > - * the error to -EIO so the GPU will be declared wedged.
> > - * OTOH, if we didn't intend to use the GuC anyway, just
> > - * discard the error and carry on.
> > - */
> > - DRM_ERROR("Failed to initialize GuC, error %d%s\n", ret,
> > - i915.enable_guc_submission ? "" :
> > - " (ignored)");
> > - ret = i915.enable_guc_submission ? -EIO : 0;
> > - if (ret)
> > - goto out;
> > + DRM_ERROR("Failed to initialize GuC, error %d\n", ret);
> > + ret = -EIO;
> > + goto out;
> > }
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> > index c0281df52702..bbbf9b3b00eb 100644
> > --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> > @@ -364,6 +364,9 @@ int intel_guc_ucode_load(struct drm_device *dev)
> > struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> > int err = 0;
> >
> > + if (!i915.enable_guc_submission)
> > + return 0;
> > +
> > DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
> > intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
> > intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
> > @@ -589,6 +592,9 @@ void intel_guc_ucode_init(struct drm_device *dev)
> > fw_path = ""; /* unknown device */
> > }
> >
> > + if (!i915.enable_guc_submission)
> > + return;
> > +
> > guc_fw->guc_dev = dev;
> > guc_fw->guc_fw_path = fw_path;
> > guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE;
> > --
> > 2.5.1
--
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/2] drm/i915: Shut up GuC errors when it's disabled
2015-10-23 8:56 [PATCH 1/2] drm/i915: Shut up GuC errors when it's disabled Daniel Vetter
` (2 preceding siblings ...)
2015-10-23 9:10 ` [PATCH] drm/i915: Shut up GuC errors when it's disabled Daniel Vetter
@ 2015-10-23 9:21 ` kbuild test robot
3 siblings, 0 replies; 23+ messages in thread
From: kbuild test robot @ 2015-10-23 9:21 UTC (permalink / raw)
Cc: Daniel Vetter, Intel Graphics Development, Mika Kuoppala,
kbuild-all, Daniel Vetter
[-- Attachment #1: Type: text/plain, Size: 3097 bytes --]
Hi Daniel,
[auto build test ERROR on drm-intel/for-linux-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-i915-Shut-up-GuC-errors-when-it-s-disabled/20151023-165910
config: x86_64-randconfig-x010-10211707 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
drivers/gpu/drm/i915/i915_gem.c: In function 'i915_gem_init_hw':
>> drivers/gpu/drm/i915/i915_gem.c:5202:0: error: unterminated argument list invoking macro "DRM_ERROR"
}
^
>> drivers/gpu/drm/i915/i915_gem.c:4753:4: error: 'DRM_ERROR' undeclared (first use in this function)
DRM_ERROR("Failed to initialize GuC, error %d\n", ret,
^
drivers/gpu/drm/i915/i915_gem.c:4753:4: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/gpu/drm/i915/i915_gem.c:4753:4: error: expected ';' at end of input
>> drivers/gpu/drm/i915/i915_gem.c:4753:4: error: expected declaration or statement at end of input
>> drivers/gpu/drm/i915/i915_gem.c:4753:4: error: expected declaration or statement at end of input
>> drivers/gpu/drm/i915/i915_gem.c:4753:4: error: expected declaration or statement at end of input
>> drivers/gpu/drm/i915/i915_gem.c:4746:4: error: label 'out' used but not defined
goto out;
^
>> drivers/gpu/drm/i915/i915_gem.c:4697:14: warning: unused variable 'j' [-Wunused-variable]
int ret, i, j;
^
drivers/gpu/drm/i915/i915_gem.c: At top level:
drivers/gpu/drm/i915/i915_gem.c:2895:1: warning: 'i915_gem_retire_work_handler' defined but not used [-Wunused-function]
i915_gem_retire_work_handler(struct work_struct *work)
^
drivers/gpu/drm/i915/i915_gem.c:2914:1: warning: 'i915_gem_idle_work_handler' defined but not used [-Wunused-function]
i915_gem_idle_work_handler(struct work_struct *work)
^
drivers/gpu/drm/i915/i915_gem.c: In function 'i915_gem_init_hw':
>> drivers/gpu/drm/i915/i915_gem.c:4753:4: warning: control reaches end of non-void function [-Wreturn-type]
DRM_ERROR("Failed to initialize GuC, error %d\n", ret,
^
vim +/DRM_ERROR +5202 drivers/gpu/drm/i915/i915_gem.c
ea70299d Dave Gordon 2015-07-09 5196
ea70299d Dave Gordon 2015-07-09 5197 return obj;
ea70299d Dave Gordon 2015-07-09 5198
ea70299d Dave Gordon 2015-07-09 5199 fail:
ea70299d Dave Gordon 2015-07-09 5200 drm_gem_object_unreference(&obj->base);
ea70299d Dave Gordon 2015-07-09 5201 return ERR_PTR(ret);
ea70299d Dave Gordon 2015-07-09 @5202 }
:::::: The code at line 5202 was first introduced by commit
:::::: ea70299d6e6961dd6adce2cbdf64e6e8a7ea97c0 drm/i915: Add i915_gem_object_create_from_data()
:::::: TO: Dave Gordon <david.s.gordon@intel.com>
:::::: CC: Daniel Vetter <daniel.vetter@ffwll.ch>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 21159 bytes --]
[-- Attachment #3: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] drm/i915: Shut up GuC errors when it's disabled
@ 2015-10-23 8:33 Daniel Vetter
2015-10-23 8:48 ` Chris Wilson
0 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2015-10-23 8:33 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter
DRM_ERROR an continue without any issues aren't allowed since that
causes noise in the CI system. But we absolutely want to have the
DRM_ERROR when we want to run with GuC.
For simplicity just short-circuit all the loader code when it's not
needed.
Cc: Alex Dai <yu.dai@intel.com>
Cc: Dave Gordon <david.s.gordon@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 10 +---------
drivers/gpu/drm/i915/intel_guc_loader.c | 6 ++++++
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 01fef54ecb2d..0604d15bde72 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4747,18 +4747,10 @@ i915_gem_init_hw(struct drm_device *dev)
if (HAS_GUC_UCODE(dev)) {
ret = intel_guc_ucode_load(dev);
if (ret) {
- /*
- * If we got an error and GuC submission is enabled, map
- * the error to -EIO so the GPU will be declared wedged.
- * OTOH, if we didn't intend to use the GuC anyway, just
- * discard the error and carry on.
- */
DRM_ERROR("Failed to initialize GuC, error %d%s\n", ret,
i915.enable_guc_submission ? "" :
" (ignored)");
- ret = i915.enable_guc_submission ? -EIO : 0;
- if (ret)
- goto out;
+ return -EIO;
}
}
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index c0281df52702..bbbf9b3b00eb 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -364,6 +364,9 @@ int intel_guc_ucode_load(struct drm_device *dev)
struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
int err = 0;
+ if (!i915.enable_guc_submission)
+ return 0;
+
DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
@@ -589,6 +592,9 @@ void intel_guc_ucode_init(struct drm_device *dev)
fw_path = ""; /* unknown device */
}
+ if (!i915.enable_guc_submission)
+ return;
+
guc_fw->guc_dev = dev;
guc_fw->guc_fw_path = fw_path;
guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE;
--
2.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH] drm/i915: Shut up GuC errors when it's disabled
2015-10-23 8:33 [PATCH] " Daniel Vetter
@ 2015-10-23 8:48 ` Chris Wilson
0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2015-10-23 8:48 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development
On Fri, Oct 23, 2015 at 10:33:56AM +0200, Daniel Vetter wrote:
> DRM_ERROR an continue without any issues aren't allowed since that
> causes noise in the CI system. But we absolutely want to have the
> DRM_ERROR when we want to run with GuC.
>
> For simplicity just short-circuit all the loader code when it's not
> needed.
Or even simpler would be only to DRM_ERROR if enabled.
> Cc: Alex Dai <yu.dai@intel.com>
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 10 +---------
> drivers/gpu/drm/i915/intel_guc_loader.c | 6 ++++++
> 2 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 01fef54ecb2d..0604d15bde72 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4747,18 +4747,10 @@ i915_gem_init_hw(struct drm_device *dev)
> if (HAS_GUC_UCODE(dev)) {
> ret = intel_guc_ucode_load(dev);
> if (ret) {
> - /*
> - * If we got an error and GuC submission is enabled, map
> - * the error to -EIO so the GPU will be declared wedged.
> - * OTOH, if we didn't intend to use the GuC anyway, just
> - * discard the error and carry on.
> - */
> DRM_ERROR("Failed to initialize GuC, error %d%s\n", ret,
> i915.enable_guc_submission ? "" :
> " (ignored)");
The error message is now confused.
> - ret = i915.enable_guc_submission ? -EIO : 0;
> - if (ret)
> - goto out;
> + return -EIO;
You still need to jump to out to undo the rpm.
ret = -EIO;
goto out;
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index c0281df52702..bbbf9b3b00eb 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -364,6 +364,9 @@ int intel_guc_ucode_load(struct drm_device *dev)
> struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> int err = 0;
>
> + if (!i915.enable_guc_submission)
> + return 0;
Further simplification inside intel_guc_ucode_load() now possible, or
the question is why is the firmware being loaded currently if not used?
> DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
> intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
> intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
> @@ -589,6 +592,9 @@ void intel_guc_ucode_init(struct drm_device *dev)
> fw_path = ""; /* unknown device */
> }
>
> + if (!i915.enable_guc_submission)
> + return;
Can be moved ahead of this block. Later we can then use
dev_priv->guc.guc_fw->guc_dev (the tautology!) to see if we have setup
the GUC paths.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2015-10-28 0:22 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-23 8:56 [PATCH 1/2] drm/i915: Shut up GuC errors when it's disabled Daniel Vetter
2015-10-23 8:56 ` [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise Daniel Vetter
2015-10-23 9:03 ` Chris Wilson
2015-10-23 9:14 ` Daniel Vetter
2015-10-23 14:21 ` Dave Gordon
2015-10-23 9:21 ` Jani Nikula
2015-10-23 9:23 ` Daniel Vetter
2015-10-23 13:33 ` Ville Syrjälä
2015-10-23 13:40 ` Chris Wilson
2015-10-23 13:47 ` Ville Syrjälä
2015-10-23 14:42 ` Daniel Vetter
2015-10-23 15:22 ` Ville Syrjälä
2015-10-27 2:31 ` Daniel Vetter
2015-10-27 8:10 ` Jani Nikula
2015-10-28 0:22 ` Daniel Vetter
2015-10-23 9:00 ` [PATCH] drm/i915: Don't complain about lack of ACPI video bios Daniel Vetter
2015-10-23 11:46 ` Jani Nikula
2015-10-23 9:10 ` [PATCH] drm/i915: Shut up GuC errors when it's disabled Daniel Vetter
2015-10-23 10:56 ` Mika Kuoppala
2015-10-23 11:32 ` Daniel Vetter
2015-10-23 9:21 ` [PATCH 1/2] " kbuild test robot
-- strict thread matches above, loose matches on Subject: below --
2015-10-23 8:33 [PATCH] " Daniel Vetter
2015-10-23 8:48 ` Chris Wilson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox