[PATCH 1/2] drm/i915: Shut up GuC errors when it's disabled

[PATCH 1/2] drm/i915: Shut up GuC errors when it's disabled

[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.

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

[PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise

[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

[PATCH] drm/i915: Don't complain about lack of ACPI video bios

[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

Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise

[Chris Wilson]

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

[PATCH] drm/i915: Shut up GuC errors when it's disabled

[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.

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

Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise

[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

Re: [PATCH 1/2] drm/i915: Shut up GuC errors when it's disabled

[kbuild test robot]

[-- 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

Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise

[Jani Nikula]

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

Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise

[Daniel Vetter]

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

Re: [PATCH] drm/i915: Shut up GuC errors when it's disabled

[Mika Kuoppala]

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

Re: [PATCH] drm/i915: Shut up GuC errors when it's disabled

[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

Re: [PATCH] drm/i915: Don't complain about lack of ACPI video bios

[Jani Nikula]

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

Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise

[Ville Syrjälä]

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

Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise

[Chris Wilson]

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

Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise

[Ville Syrjälä]

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

Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise

[Dave Gordon]

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

Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise

[Daniel Vetter]

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

Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise

[Ville Syrjälä]

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

Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise

[Daniel Vetter]

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

Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise

[Jani Nikula]

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

Re: [PATCH 2/2] drm/i915: shut up gen8+ SDE irq dmesg noise

[Daniel Vetter]

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