[PATCH 1/2] drm/i915: Combine cleanup_status_page()

[PATCH 1/2] drm/i915: Combine cleanup_status_page()

[Chris Wilson]

Pull the physical status page cleanup into a common
cleanup_status_page() for caller simplicity.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_engine_cs.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 1a34e8ff82d5..292eae19fce2 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -530,19 +530,14 @@ void intel_engine_cleanup_scratch(struct intel_engine_cs *engine)
 	i915_vma_unpin_and_release(&engine->scratch, 0);
 }
 
-static void cleanup_phys_status_page(struct intel_engine_cs *engine)
+static void cleanup_status_page(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = engine->i915;
+	struct drm_dma_handle *dmah;
 
-	if (!dev_priv->status_page_dmah)
-		return;
+	dmah = fetch_and_zero(&engine->i915->status_page_dmah);
+	if (dmah)
+		drm_pci_free(&engine->i915->drm, dmah);
 
-	drm_pci_free(&dev_priv->drm, dev_priv->status_page_dmah);
-	engine->status_page.page_addr = NULL;
-}
-
-static void cleanup_status_page(struct intel_engine_cs *engine)
-{
 	i915_vma_unpin_and_release(&engine->status_page.vma,
 				   I915_VMA_RELEASE_MAP);
 }
@@ -710,10 +705,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
 
 	intel_engine_cleanup_scratch(engine);
 
-	if (HWS_NEEDS_PHYSICAL(engine->i915))
-		cleanup_phys_status_page(engine);
-	else
-		cleanup_status_page(engine);
+	cleanup_status_page(engine);
 
 	intel_engine_fini_breadcrumbs(engine);
 	intel_engine_cleanup_cmd_parser(engine);
-- 
2.19.0.rc1

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

[PATCH 2/2] drm/i915: Use a cached mapping for the physical HWS

[Chris Wilson]

Older gen use a physical address for the hardware status page, for which
we use cache-coherent writes. As the writes are into the cpu cache, we use
a normal WB mapped page to read the HWS, used for our seqno tracking.

Anecdotally, I observed lost breadcrumbs writes into the HWS on i965gm,
which so far have not reoccurred with this patch. How reliable that
evidence is remains to be seen.

v2: Explicitly pass the expected physical address to the hw
v3: Also remember the wild writes we once had for HWS above 4G.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h         |  1 -
 drivers/gpu/drm/i915/intel_engine_cs.c  | 25 +++++++++++++------------
 drivers/gpu/drm/i915/intel_ringbuffer.c |  7 +++++--
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9771f39d99b3..5a4da5b723fd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1666,7 +1666,6 @@ struct drm_i915_private {
 	struct intel_engine_cs *engine_class[MAX_ENGINE_CLASS + 1]
 					    [MAX_ENGINE_INSTANCE + 1];
 
-	struct drm_dma_handle *status_page_dmah;
 	struct resource mch_res;
 
 	/* protects the irq masks */
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 292eae19fce2..10cd051ba29e 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -532,11 +532,11 @@ void intel_engine_cleanup_scratch(struct intel_engine_cs *engine)
 
 static void cleanup_status_page(struct intel_engine_cs *engine)
 {
-	struct drm_dma_handle *dmah;
+	if (HWS_NEEDS_PHYSICAL(engine->i915)) {
+		void *addr = fetch_and_zero(&engine->status_page.page_addr);
 
-	dmah = fetch_and_zero(&engine->i915->status_page_dmah);
-	if (dmah)
-		drm_pci_free(&engine->i915->drm, dmah);
+		__free_page(virt_to_page(addr));
+	}
 
 	i915_vma_unpin_and_release(&engine->status_page.vma,
 				   I915_VMA_RELEASE_MAP);
@@ -605,17 +605,18 @@ static int init_status_page(struct intel_engine_cs *engine)
 
 static int init_phys_status_page(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = engine->i915;
-
-	GEM_BUG_ON(engine->id != RCS);
+	struct page *page;
 
-	dev_priv->status_page_dmah =
-		drm_pci_alloc(&dev_priv->drm, PAGE_SIZE, PAGE_SIZE);
-	if (!dev_priv->status_page_dmah)
+	/*
+	 * Though the HWS register does support 36bit addresses, historically
+	 * we have had hangs and corruption reported due to wild writes if
+	 * the HWS is placed above 4G.
+	 */
+	page = alloc_page(GFP_KERNEL | __GFP_DMA32 | __GFP_ZERO);
+	if (!page)
 		return -ENOMEM;
 
-	engine->status_page.page_addr = dev_priv->status_page_dmah->vaddr;
-	memset(engine->status_page.page_addr, 0, PAGE_SIZE);
+	engine->status_page.page_addr = page_address(page);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 44432677160c..86604dd1c5a5 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -344,11 +344,14 @@ gen7_render_ring_flush(struct i915_request *rq, u32 mode)
 static void ring_setup_phys_status_page(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
+	struct page *page = virt_to_page(engine->status_page.page_addr);
+	phys_addr_t phys = PFN_PHYS(page_to_pfn(page));
 	u32 addr;
 
-	addr = dev_priv->status_page_dmah->busaddr;
+	addr = lower_32_bits(phys);
 	if (INTEL_GEN(dev_priv) >= 4)
-		addr |= (dev_priv->status_page_dmah->busaddr >> 28) & 0xf0;
+		addr |= (phys >> 28) & 0xf0;
+
 	I915_WRITE(HWS_PGA, addr);
 }
 
-- 
2.19.0.rc1

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

✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915: Combine cleanup_status_page()

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Combine cleanup_status_page()
URL   : https://patchwork.freedesktop.org/series/49085/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Combine cleanup_status_page()
Okay!

Commit: drm/i915: Use a cached mapping for the physical HWS
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3687:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3686:16: warning: expression using sizeof(void)

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

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Combine cleanup_status_page()

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Combine cleanup_status_page()
URL   : https://patchwork.freedesktop.org/series/49085/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4757 -> Patchwork_10074 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-byt-clapper:     PASS -> INCOMPLETE (fdo#102657)

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

    
    ==== Warnings ====

    igt@pm_rpm@module-reload:
      fi-bsw-n3050:       DMESG-WARN (fdo#107704) -> DMESG-FAIL (fdo#107704)

    
  fdo#102657 https://bugs.freedesktop.org/show_bug.cgi?id=102657
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#107704 https://bugs.freedesktop.org/show_bug.cgi?id=107704


== Participating hosts (53 -> 48) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4757 -> Patchwork_10074

  CI_DRM_4757: 1465de895e2b5d9e74e9a85189c9075155efa30d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4621: 125eee6e981eac0a004aeb4f327439a132ceac5c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10074: 23b2d4ef0d61b349b77010f58f76d6d2a987b8fd @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

23b2d4ef0d61 drm/i915: Use a cached mapping for the physical HWS
06a72134dc5f drm/i915: Combine cleanup_status_page()

== Logs ==

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

Re: [PATCH 1/2] drm/i915: Combine cleanup_status_page()

[Matthew Auld]

On Mon, 3 Sep 2018 at 16:25, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Pull the physical status page cleanup into a common
> cleanup_status_page() for caller simplicity.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Combine cleanup_status_page()

[Chris Wilson]

Quoting Matthew Auld (2018-09-03 17:39:09)
> On Mon, 3 Sep 2018 at 16:25, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > Pull the physical status page cleanup into a common
> > cleanup_status_page() for caller simplicity.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>

And applied just on the off-chance it helps with a strange one-off
occurrence of a missed breadcrumb on gdg. Thanks,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Combine cleanup_status_page()

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Combine cleanup_status_page()
URL   : https://patchwork.freedesktop.org/series/49085/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4757_full -> Patchwork_10074_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-glk:          PASS -> FAIL (fdo#106886)

    igt@gem_exec_basic@basic-bsd1:
      shard-snb:          NOTRUN -> INCOMPLETE (fdo#105411)

    igt@gem_exec_big:
      shard-hsw:          PASS -> INCOMPLETE (fdo#103540)

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

    igt@kms_vblank@pipe-a-ts-continuation-suspend:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)

    
    ==== Possible fixes ====

    igt@gem_exec_await@wide-contexts:
      shard-glk:          FAIL (fdo#105900) -> PASS

    igt@gem_wait@await-bsd2:
      shard-snb:          INCOMPLETE (fdo#105411) -> SKIP

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
      shard-apl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS +2

    igt@kms_vblank@pipe-b-ts-continuation-idle-hang:
      shard-apl:          DMESG-WARN -> PASS

    igt@pm_rc6_residency@rc6-accuracy:
      shard-glk:          INCOMPLETE (fdo#103359, k.org#198133) -> PASS

    
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


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

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4757 -> Patchwork_10074

  CI_DRM_4757: 1465de895e2b5d9e74e9a85189c9075155efa30d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4621: 125eee6e981eac0a004aeb4f327439a132ceac5c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10074: 23b2d4ef0d61b349b77010f58f76d6d2a987b8fd @ 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_10074/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx