[PATCH] drm/i915/icl: do a posting read after irq install

[PATCH] drm/i915/icl: do a posting read after irq install

[Daniele Ceraolo Spurio]

When reading GEN11_GT_INTR_DWx closely after enabling the interrupts
in gen11_irq_postinstall, the returned value is garbage. This can
cause other parts of the setup code (e.g. gen11_reset_one_iir) to
think that there are interrupts to be cleared when there are none.

The garbage value is only seen on the first read done after the enable,
so this looks like a posting issue. Adding a posting read after enabling
the interrupts does indeed fix the problem.

Note that the posting read has been purposely added outside of
gen11_master_intr_enable since the issue has only been observed when the
full interrupt setup is performed.

Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5fd5080c4ccb..7056ae2d1e0e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4089,6 +4089,7 @@ static int gen11_irq_postinstall(struct drm_device *dev)
 	I915_WRITE(GEN11_DISPLAY_INT_CTL, GEN11_DISPLAY_IRQ_ENABLE);
 
 	gen11_master_intr_enable(dev_priv->regs);
+	POSTING_READ(GEN11_GFX_MSTR_IRQ);
 
 	return 0;
 }
-- 
2.20.1

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

Re: [PATCH] drm/i915/icl: do a posting read after irq install

[Daniele Ceraolo Spurio]

On 1/22/2019 6:32 PM, Daniele Ceraolo Spurio wrote:
> When reading GEN11_GT_INTR_DWx closely after enabling the interrupts
> in gen11_irq_postinstall, the returned value is garbage. This can

To clarify, this only happens (or at least I've only seen it) during 
runtime_resume.

Daniele

> cause other parts of the setup code (e.g. gen11_reset_one_iir) to
> think that there are interrupts to be cleared when there are none.
>
> The garbage value is only seen on the first read done after the enable,
> so this looks like a posting issue. Adding a posting read after enabling
> the interrupts does indeed fix the problem.
>
> Note that the posting read has been purposely added outside of
> gen11_master_intr_enable since the issue has only been observed when the
> full interrupt setup is performed.
>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_irq.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 5fd5080c4ccb..7056ae2d1e0e 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4089,6 +4089,7 @@ static int gen11_irq_postinstall(struct drm_device *dev)
>   	I915_WRITE(GEN11_DISPLAY_INT_CTL, GEN11_DISPLAY_IRQ_ENABLE);
>   
>   	gen11_master_intr_enable(dev_priv->regs);
> +	POSTING_READ(GEN11_GFX_MSTR_IRQ);
>   
>   	return 0;
>   }

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

✓ Fi.CI.BAT: success for drm/i915/icl: do a posting read after irq install

[Patchwork]

== Series Details ==

Series: drm/i915/icl: do a posting read after irq install
URL   : https://patchwork.freedesktop.org/series/55598/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5468 -> Patchwork_12010
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55598/revisions/1/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12010 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  * igt@kms_pipe_crc_basic@read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#107362]

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       FAIL [fdo#108767] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS +1

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278


Participating hosts (46 -> 39)
------------------------------

  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ivb-3770 fi-pnv-d510 fi-icl-y 


Build changes
-------------

    * Linux: CI_DRM_5468 -> Patchwork_12010

  CI_DRM_5468: fc4e30d30d90ed5d5bd467de0439e9522d34cdf0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4784: 1c5a4432293369f85859c748c08155e79d92c4ce @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12010: dcb852034e635befa89d31d17a50ec69aad8d1ef @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

dcb852034e63 drm/i915/icl: do a posting read after irq install

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12010/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915/icl: do a posting read after irq install

[Patchwork]

== Series Details ==

Series: drm/i915/icl: do a posting read after irq install
URL   : https://patchwork.freedesktop.org/series/55598/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5468_full -> Patchwork_12010_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12010_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-apl:          PASS -> FAIL [fdo#106510] / [fdo#108145]
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_cursor_crc@cursor-128x128-random:
    - shard-apl:          PASS -> FAIL [fdo#103232] +3

  * igt@kms_cursor_crc@cursor-256x85-onscreen:
    - shard-glk:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-apl:          PASS -> FAIL [fdo#108948]

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@kms_universal_plane@universal-plane-pipe-a-functional:
    - shard-apl:          PASS -> FAIL [fdo#103166] +1

  
#### Possible fixes ####

  * igt@kms_cursor_crc@cursor-128x128-random:
    - shard-glk:          FAIL [fdo#103232] -> PASS

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-glk:          FAIL [fdo#105363] -> PASS +1

  * igt@kms_flip@dpms-vs-vblank-race:
    - shard-kbl:          FAIL [fdo#103060] -> PASS
    - shard-glk:          FAIL [fdo#103060] -> PASS

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-none:
    - shard-glk:          FAIL [fdo#103166] -> PASS +1

  
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#106510]: https://bugs.freedesktop.org/show_bug.cgi?id=106510
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948


Participating hosts (7 -> 4)
------------------------------

  Missing    (3): shard-snb shard-skl shard-iclb 


Build changes
-------------

    * Linux: CI_DRM_5468 -> Patchwork_12010

  CI_DRM_5468: fc4e30d30d90ed5d5bd467de0439e9522d34cdf0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4784: 1c5a4432293369f85859c748c08155e79d92c4ce @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12010: dcb852034e635befa89d31d17a50ec69aad8d1ef @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12010/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/icl: do a posting read after irq install

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2019-01-23 02:32:27)
> When reading GEN11_GT_INTR_DWx closely after enabling the interrupts
> in gen11_irq_postinstall, the returned value is garbage. This can
> cause other parts of the setup code (e.g. gen11_reset_one_iir) to
> think that there are interrupts to be cleared when there are none.
> 
> The garbage value is only seen on the first read done after the enable,
> so this looks like a posting issue. Adding a posting read after enabling
> the interrupts does indeed fix the problem.
> 
> Note that the posting read has been purposely added outside of
> gen11_master_intr_enable since the issue has only been observed when the
> full interrupt setup is performed.
> 
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

irq regs and flushing stale results (side effect or double buffering? or
maybe latching?), seem to go hand in hand.

Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/icl: do a posting read after irq install

[Mika Kuoppala]

Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> writes:

> On 1/22/2019 6:32 PM, Daniele Ceraolo Spurio wrote:
>> When reading GEN11_GT_INTR_DWx closely after enabling the interrupts
>> in gen11_irq_postinstall, the returned value is garbage. This can
>
> To clarify, this only happens (or at least I've only seen it) during 
> runtime_resume.
>

How did you notice?

> Daniele
>
>> cause other parts of the setup code (e.g. gen11_reset_one_iir) to
>> think that there are interrupts to be cleared when there are none.
>>
>> The garbage value is only seen on the first read done after the enable,
>> so this looks like a posting issue. Adding a posting read after enabling
>> the interrupts does indeed fix the problem.
>>
>> Note that the posting read has been purposely added outside of
>> gen11_master_intr_enable since the issue has only been observed when the
>> full interrupt setup is performed.

Scary enough that maybe it would have been warranted inside it. But
well we know where to escalate if it shows up elsewhere.

Acked-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

>>
>> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_irq.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 5fd5080c4ccb..7056ae2d1e0e 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -4089,6 +4089,7 @@ static int gen11_irq_postinstall(struct drm_device *dev)
>>   	I915_WRITE(GEN11_DISPLAY_INT_CTL, GEN11_DISPLAY_IRQ_ENABLE);
>>   
>>   	gen11_master_intr_enable(dev_priv->regs);
>> +	POSTING_READ(GEN11_GFX_MSTR_IRQ);
>>   
>>   	return 0;
>>   }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/icl: do a posting read after irq install

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Daniele Ceraolo Spurio (2019-01-23 02:32:27)
>> When reading GEN11_GT_INTR_DWx closely after enabling the interrupts
>> in gen11_irq_postinstall, the returned value is garbage. This can
>> cause other parts of the setup code (e.g. gen11_reset_one_iir) to
>> think that there are interrupts to be cleared when there are none.
>> 
>> The garbage value is only seen on the first read done after the enable,
>> so this looks like a posting issue. Adding a posting read after enabling
>> the interrupts does indeed fix the problem.
>> 
>> Note that the posting read has been purposely added outside of
>> gen11_master_intr_enable since the issue has only been observed when the
>> full interrupt setup is performed.
>> 
>> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>
> irq regs and flushing stale results (side effect or double buffering? or
> maybe latching?), seem to go hand in hand.
>
> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>

Pushed. Thank you for patch and ack.
-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/icl: do a posting read after irq install

[Daniele Ceraolo Spurio]

On 01/23/2019 03:40 AM, Mika Kuoppala wrote:
> Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> writes:
> 
>> On 1/22/2019 6:32 PM, Daniele Ceraolo Spurio wrote:
>>> When reading GEN11_GT_INTR_DWx closely after enabling the interrupts
>>> in gen11_irq_postinstall, the returned value is garbage. This can
>>
>> To clarify, this only happens (or at least I've only seen it) during
>> runtime_resume.
>>
> 
> How did you notice?
> 

The gen11 guc patches add

	WARN_ON_ONCE(gen11_reset_one_iir(dev_priv, 0, GEN11_GUC));

in the resume path and we saw the warning fire off in testing. A bit of 
extra logging showed that the whole register was in an invalid state 
after interrupts were re-enabled, not just the GuC bit. 
pm_rpm@basic-pci-d3-state hits this consistently.

Daniele

>> Daniele
>>
>>> cause other parts of the setup code (e.g. gen11_reset_one_iir) to
>>> think that there are interrupts to be cleared when there are none.
>>>
>>> The garbage value is only seen on the first read done after the enable,
>>> so this looks like a posting issue. Adding a posting read after enabling
>>> the interrupts does indeed fix the problem.
>>>
>>> Note that the posting read has been purposely added outside of
>>> gen11_master_intr_enable since the issue has only been observed when the
>>> full interrupt setup is performed.
> 
> Scary enough that maybe it would have been warranted inside it. But
> well we know where to escalate if it shows up elsewhere.
> 
> Acked-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> 
>>>
>>> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/i915_irq.c | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>>> index 5fd5080c4ccb..7056ae2d1e0e 100644
>>> --- a/drivers/gpu/drm/i915/i915_irq.c
>>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>>> @@ -4089,6 +4089,7 @@ static int gen11_irq_postinstall(struct drm_device *dev)
>>>    	I915_WRITE(GEN11_DISPLAY_INT_CTL, GEN11_DISPLAY_IRQ_ENABLE);
>>>    
>>>    	gen11_master_intr_enable(dev_priv->regs);
>>> +	POSTING_READ(GEN11_GFX_MSTR_IRQ);
>>>    
>>>    	return 0;
>>>    }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx