[PATCH] drm/i915/oa: Check that OA is disabled before unpinning

[PATCH] drm/i915/oa: Check that OA is disabled before unpinning

[Chris Wilson]

Before we unpin the buffer used for OA reports and return it to the
system, we need to be sure that the HW has finished writing into it.
For lack of a better idea, poll OACONTROL to check it is switched off.

References: https://bugs.freedesktop.org/show_bug.cgi?id=106379
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index d9341415df40..019bd2d073ad 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1960,11 +1960,19 @@ static void i915_oa_stream_enable(struct i915_perf_stream *stream)
 static void gen7_oa_disable(struct drm_i915_private *dev_priv)
 {
 	I915_WRITE(GEN7_OACONTROL, 0);
+	if (intel_wait_for_register(dev_priv,
+				    GEN7_OACONTROL, GEN7_OACONTROL_ENABLE, 0,
+				    50))
+		DRM_ERROR("wait for OA to be disabled timed out\n");
 }
 
 static void gen8_oa_disable(struct drm_i915_private *dev_priv)
 {
 	I915_WRITE(GEN8_OACONTROL, 0);
+	if (intel_wait_for_register(dev_priv,
+				    GEN8_OACONTROL, GEN8_OA_COUNTER_ENABLE, 0,
+				    50))
+		DRM_ERROR("wait for OA to be disabled timed out\n");
 }
 
 /**
-- 
2.17.0

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

Re: [PATCH] drm/i915/oa: Check that OA is disabled before unpinning

[Lionel Landwerlin]

On 11/05/18 14:52, Chris Wilson wrote:
> Before we unpin the buffer used for OA reports and return it to the
> system, we need to be sure that the HW has finished writing into it.
> For lack of a better idea, poll OACONTROL to check it is switched off.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=106379
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>

Sounds fair :

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

> ---
>   drivers/gpu/drm/i915/i915_perf.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index d9341415df40..019bd2d073ad 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1960,11 +1960,19 @@ static void i915_oa_stream_enable(struct i915_perf_stream *stream)
>   static void gen7_oa_disable(struct drm_i915_private *dev_priv)
>   {
>   	I915_WRITE(GEN7_OACONTROL, 0);
> +	if (intel_wait_for_register(dev_priv,
> +				    GEN7_OACONTROL, GEN7_OACONTROL_ENABLE, 0,
> +				    50))
> +		DRM_ERROR("wait for OA to be disabled timed out\n");
>   }
>   
>   static void gen8_oa_disable(struct drm_i915_private *dev_priv)
>   {
>   	I915_WRITE(GEN8_OACONTROL, 0);
> +	if (intel_wait_for_register(dev_priv,
> +				    GEN8_OACONTROL, GEN8_OA_COUNTER_ENABLE, 0,
> +				    50))
> +		DRM_ERROR("wait for OA to be disabled timed out\n");
>   }
>   
>   /**


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

✓ Fi.CI.BAT: success for drm/i915/oa: Check that OA is disabled before unpinning

[Patchwork]

== Series Details ==

Series: drm/i915/oa: Check that OA is disabled before unpinning
URL   : https://patchwork.freedesktop.org/series/43055/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4168 -> Patchwork_8981 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
      fi-skl-6770hq:      PASS -> FAIL (fdo#103481)

    igt@pm_rpm@basic-pci-d3-state:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106097)

    
    ==== Possible fixes ====

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         FAIL (fdo#102575) -> PASS

    
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097


== Participating hosts (41 -> 36) ==

  Missing    (5): fi-ctg-p8600 fi-byt-squawks fi-ilk-m540 fi-bxt-dsi fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4168 -> Patchwork_8981

  CI_DRM_4168: 8718b981fec6cff68f2a94ef0aaeaecc2cd6a659 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4475: 35f08c12aa216d5b62a5b9984b575cee6905098f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8981: 26b0d174222de04fcd5ca2d750009e3aca31bfdd @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4475: 3ba0657bff4216d1ec7179935590261855f1651e @ git://anongit.freedesktop.org/piglit


== Linux commits ==

26b0d174222d drm/i915/oa: Check that OA is disabled before unpinning

== Logs ==

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

✓ Fi.CI.IGT: success for drm/i915/oa: Check that OA is disabled before unpinning

[Patchwork]

== Series Details ==

Series: drm/i915/oa: Check that OA is disabled before unpinning
URL   : https://patchwork.freedesktop.org/series/43055/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4168_full -> Patchwork_8981_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8981_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8981_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

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

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8981_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd1:
      shard-kbl:          PASS -> SKIP +1

    igt@gem_mocs_settings@mocs-rc6-dirty-render:
      shard-kbl:          SKIP -> PASS

    igt@gem_pwrite@big-cpu-random:
      shard-glk:          PASS -> SKIP

    igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence:
      shard-apl:          SKIP -> PASS +3

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          PASS -> INCOMPLETE (fdo#106023, fdo#103665)

    igt@kms_flip@2x-flip-vs-absolute-wf_vblank-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#100368)

    igt@kms_flip@modeset-vs-vblank-race-interruptible:
      shard-glk:          PASS -> FAIL (fdo#103060)

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@gem_eio@in-flight-10ms:
      shard-glk:          FAIL (fdo#105957) -> PASS

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-kbl:          INCOMPLETE (fdo#106023, fdo#103665) -> PASS

    igt@kms_flip@absolute-wf_vblank-interruptible:
      shard-glk:          FAIL (fdo#106087) -> PASS

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#102887) -> PASS

    igt@kms_flip@wf_vblank-ts-check-interruptible:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-msflip-blt:
      shard-apl:          FAIL (fdo#103167, fdo#104724) -> PASS

    igt@kms_sysfs_edid_timing:
      shard-apl:          WARN (fdo#100047) -> PASS

    
  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105957 https://bugs.freedesktop.org/show_bug.cgi?id=105957
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106087 https://bugs.freedesktop.org/show_bug.cgi?id=106087
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4168 -> Patchwork_8981

  CI_DRM_4168: 8718b981fec6cff68f2a94ef0aaeaecc2cd6a659 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4475: 35f08c12aa216d5b62a5b9984b575cee6905098f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8981: 26b0d174222de04fcd5ca2d750009e3aca31bfdd @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4475: 3ba0657bff4216d1ec7179935590261855f1651e @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH] drm/i915/oa: Check that OA is disabled before unpinning

[Lionel Landwerlin]

On 11/05/18 15:11, Lionel Landwerlin wrote:
> On 11/05/18 14:52, Chris Wilson wrote:
>> Before we unpin the buffer used for OA reports and return it to the
>> system, we need to be sure that the HW has finished writing into it.
>> For lack of a better idea, poll OACONTROL to check it is switched off.
>>
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=106379
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> Cc: Matthew Auld <matthew.auld@intel.com>
>
> Sounds fair :
>
> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

Tested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

>
>> ---
>>   drivers/gpu/drm/i915/i915_perf.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c 
>> b/drivers/gpu/drm/i915/i915_perf.c
>> index d9341415df40..019bd2d073ad 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -1960,11 +1960,19 @@ static void i915_oa_stream_enable(struct 
>> i915_perf_stream *stream)
>>   static void gen7_oa_disable(struct drm_i915_private *dev_priv)
>>   {
>>       I915_WRITE(GEN7_OACONTROL, 0);
>> +    if (intel_wait_for_register(dev_priv,
>> +                    GEN7_OACONTROL, GEN7_OACONTROL_ENABLE, 0,
>> +                    50))
>> +        DRM_ERROR("wait for OA to be disabled timed out\n");
>>   }
>>     static void gen8_oa_disable(struct drm_i915_private *dev_priv)
>>   {
>>       I915_WRITE(GEN8_OACONTROL, 0);
>> +    if (intel_wait_for_register(dev_priv,
>> +                    GEN8_OACONTROL, GEN8_OA_COUNTER_ENABLE, 0,
>> +                    50))
>> +        DRM_ERROR("wait for OA to be disabled timed out\n");
>>   }
>>     /**
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

Re: [PATCH] drm/i915/oa: Check that OA is disabled before unpinning

[Chris Wilson]

Quoting Lionel Landwerlin (2018-05-11 17:10:49)
> On 11/05/18 15:11, Lionel Landwerlin wrote:
> > On 11/05/18 14:52, Chris Wilson wrote:
> >> Before we unpin the buffer used for OA reports and return it to the
> >> system, we need to be sure that the HW has finished writing into it.
> >> For lack of a better idea, poll OACONTROL to check it is switched off.
> >>
> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=106379
> >> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> >> Cc: Matthew Auld <matthew.auld@intel.com>
> >
> > Sounds fair :
> >
> > Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> 
> Tested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

Thanks for the review and testing, with luck we will get a CI + KASAN
run over the w/e which will put the matter to rest. Pushed,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/oa: Check that OA is disabled before unpinning

[Lionel Landwerlin]

This should be sent to stable right?

-
Lionel

On 11/05/18 14:52, Chris Wilson wrote:
> Before we unpin the buffer used for OA reports and return it to the
> system, we need to be sure that the HW has finished writing into it.
> For lack of a better idea, poll OACONTROL to check it is switched off.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=106379
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_perf.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index d9341415df40..019bd2d073ad 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1960,11 +1960,19 @@ static void i915_oa_stream_enable(struct i915_perf_stream *stream)
>   static void gen7_oa_disable(struct drm_i915_private *dev_priv)
>   {
>   	I915_WRITE(GEN7_OACONTROL, 0);
> +	if (intel_wait_for_register(dev_priv,
> +				    GEN7_OACONTROL, GEN7_OACONTROL_ENABLE, 0,
> +				    50))
> +		DRM_ERROR("wait for OA to be disabled timed out\n");
>   }
>   
>   static void gen8_oa_disable(struct drm_i915_private *dev_priv)
>   {
>   	I915_WRITE(GEN8_OACONTROL, 0);
> +	if (intel_wait_for_register(dev_priv,
> +				    GEN8_OACONTROL, GEN8_OA_COUNTER_ENABLE, 0,
> +				    50))
> +		DRM_ERROR("wait for OA to be disabled timed out\n");
>   }
>   
>   /**


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

Re: [PATCH] drm/i915/oa: Check that OA is disabled before unpinning

[Chris Wilson]

Quoting Lionel Landwerlin (2018-05-17 11:18:16)
> This should be sent to stable right?

Yeah, my bad for not digging out the relevant Fixes:

+cc Joonas for the next batch.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/oa: Check that OA is disabled before unpinning

[Lionel Landwerlin]

On 17/05/18 11:21, Chris Wilson wrote:
> Quoting Lionel Landwerlin (2018-05-17 11:18:16)
>> This should be sent to stable right?
> Yeah, my bad for not digging out the relevant Fixes: +cc Joonas for 
> the next batch. -Chris

I should have looked at it too. Was just in shock ;)

For Haswell:
Fixes: d79651522e89c4 ("drm/i915: Enable i915 perf stream for Haswell OA 
unit")

For Gen8+:
Fixes: 19f81df2859eb1 ("drm/i915/perf: Add OA unit support for Gen 8+")

-
Lionel


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

Re: [PATCH] drm/i915/oa: Check that OA is disabled before unpinning

[Joonas Lahtinen]

Quoting Lionel Landwerlin (2018-05-17 13:29:56)
> On 17/05/18 11:21, Chris Wilson wrote:
> > Quoting Lionel Landwerlin (2018-05-17 11:18:16)
> >> This should be sent to stable right?
> > Yeah, my bad for not digging out the relevant Fixes: +cc Joonas for 
> > the next batch. -Chris
> 
> I should have looked at it too. Was just in shock ;)
> 
> For Haswell:
> Fixes: d79651522e89c4 ("drm/i915: Enable i915 perf stream for Haswell OA 
> unit")
> 
> For Gen8+:
> Fixes: 19f81df2859eb1 ("drm/i915/perf: Add OA unit support for Gen 8+")

There's no PRs for stable, the stuff is supposed to be marked Cc: stable
based on the Fixes: tags...

So just amend with the information and send to stable mailing list.

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