Re: [Intel-gfx] [CI v2 2/2] drm/i915/guc: Introduce buffer based cmd transport

2017-05-23 Thread Chris Wilson 
On Tue, May 23, 2017 at 11:59:46AM -0700, Daniele Ceraolo Spurio wrote:
> On 22/05/17 04:30, Michal Wajdeczko wrote:
> >+static int ctch_init(struct intel_guc *guc,
> >+ struct intel_guc_ct_channel *ctch)
> >+{
> >+struct i915_vma *vma;
> >+void *blob;
> >+int err;
> >+int i;
> >+
> >+GEM_BUG_ON(ctch->vma);
> >+
> >+#if INTEL_GUC_CT_MAX_CHANNELS > 1
> 
> Bikeshed: after reviewing the GuC design intent for CT buffers I
> think we can remove the ida logic completely, even if
> INTEL_GUC_CT_MAX_CHANNELS > 1. Currently we don't expect more than 1
> pair, but, if my understanding is correct, in case we ever need more
> than 1 channel the number should be statically determined and not a
> dynamic thing. I would therefore prefer a static define for it.
> e.g.:
> 
> #define GUC_KMD_CTCH 0
> 
> and if we ever have more:
> 
> #define GUC_xxx_CTCH 1
> #define GUC_yyy_CTCH 2
> 
> We can then pass in the owner when we open the channel:
> 
> ctch_open(guc, ctch, GUC_KMD_CTCH);

If we do forsee that we don't need an ida for the at least the near
future, can we kill it? I'm still dubious about having
INTEL_GUC_CT_MAX_CHANNELS around not tied to any hw/fw concepts or
limits. At the least if you do split the ida into a separate patch, you
can apply it when you need it later.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [CI v2 2/2] drm/i915/guc: Introduce buffer based cmd transport

2017-05-23 Thread Daniele Ceraolo Spurio 



On 22/05/17 04:30, Michal Wajdeczko wrote:

Buffer based command transport can replace MMIO based mechanism.
It may be used to perform host-2-guc and guc-to-host communication.

Portions of this patch are based on work by:
 Michel Thierry 
 Robert Beckett 
 Daniele Ceraolo Spurio 

v2: use gem_object_pin_map (Chris)
don't use DEBUG_RATELIMITED (Chris)
don't track action stats (Chris)
simplify next fence (Chris)
use READ_ONCE (Chris)
move blob allocation to new function (Chris)

Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Oscar Mateo 
Cc: Joonas Lahtinen 
Cc: Chris Wilson 
---
 drivers/gpu/drm/i915/Makefile |   1 +
 drivers/gpu/drm/i915/i915_drv.c   |   2 +
 drivers/gpu/drm/i915/i915_drv.h   |   2 +
 drivers/gpu/drm/i915/intel_guc_ct.c   | 468 ++
 drivers/gpu/drm/i915/intel_guc_ct.h   |  97 +++
 drivers/gpu/drm/i915/intel_guc_fwif.h |  44 
 drivers/gpu/drm/i915/intel_uc.c   |  25 +-
 drivers/gpu/drm/i915/intel_uc.h   |   4 +-
 8 files changed, 641 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_guc_ct.c
 create mode 100644 drivers/gpu/drm/i915/intel_guc_ct.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 7b05fb8..16dccf5 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -58,6 +58,7 @@ i915-y += i915_cmd_parser.o \

 # general-purpose microcontroller (GuC) support
 i915-y += intel_uc.o \
+ intel_guc_ct.o \
  intel_guc_log.o \
  intel_guc_loader.o \
  intel_huc.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d703897..6c78469 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -869,6 +869,7 @@ static int i915_driver_init_early(struct drm_i915_private 
*dev_priv,
i915_workqueues_cleanup(dev_priv);
 err_engines:
i915_engines_cleanup(dev_priv);
+   intel_uc_cleanup(dev_priv);
return ret;
 }

@@ -883,6 +884,7 @@ static void i915_driver_cleanup_early(struct 
drm_i915_private *dev_priv)
intel_irq_fini(dev_priv);
i915_workqueues_cleanup(dev_priv);
i915_engines_cleanup(dev_priv);
+   intel_uc_cleanup(dev_priv);
 }

 static int i915_mmio_setup(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 17883a8..453eea5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -760,6 +760,7 @@ struct intel_csr {
func(has_gmbus_irq); \
func(has_gmch_display); \
func(has_guc); \
+   func(has_guc_ct); \
func(has_hotplug); \
func(has_l3_dpf); \
func(has_llc); \
@@ -2947,6 +2948,7 @@ intel_info(const struct drm_i915_private *dev_priv)
  * properties, so we have separate macros to test them.
  */
 #define HAS_GUC(dev_priv)  ((dev_priv)->info.has_guc)
+#define HAS_GUC_CT(dev_priv)   ((dev_priv)->info.has_guc_ct)
 #define HAS_GUC_UCODE(dev_priv)(HAS_GUC(dev_priv))
 #define HAS_GUC_SCHED(dev_priv)(HAS_GUC(dev_priv))
 #define HAS_HUC_UCODE(dev_priv)(HAS_GUC(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
new file mode 100644
index 000..869a7ad
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -0,0 +1,468 @@
+/*
+ * Copyright © 2016-2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "i915_drv.h"
+#include "intel_guc_ct.h"
+
+enum { CTB_SEND = 0, CTB_RECV = 1 };
+
+static inline const char *guc_ct_buffer_type_to_str(u32

[Intel-gfx] [CI v2 2/2] drm/i915/guc: Introduce buffer based cmd transport

2017-05-22 Thread Michal Wajdeczko 
Buffer based command transport can replace MMIO based mechanism.
It may be used to perform host-2-guc and guc-to-host communication.

Portions of this patch are based on work by:
 Michel Thierry 
 Robert Beckett 
 Daniele Ceraolo Spurio 

v2: use gem_object_pin_map (Chris)
don't use DEBUG_RATELIMITED (Chris)
don't track action stats (Chris)
simplify next fence (Chris)
use READ_ONCE (Chris)
move blob allocation to new function (Chris)

Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Oscar Mateo 
Cc: Joonas Lahtinen 
Cc: Chris Wilson 
---
 drivers/gpu/drm/i915/Makefile |   1 +
 drivers/gpu/drm/i915/i915_drv.c   |   2 +
 drivers/gpu/drm/i915/i915_drv.h   |   2 +
 drivers/gpu/drm/i915/intel_guc_ct.c   | 468 ++
 drivers/gpu/drm/i915/intel_guc_ct.h   |  97 +++
 drivers/gpu/drm/i915/intel_guc_fwif.h |  44 
 drivers/gpu/drm/i915/intel_uc.c   |  25 +-
 drivers/gpu/drm/i915/intel_uc.h   |   4 +-
 8 files changed, 641 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_guc_ct.c
 create mode 100644 drivers/gpu/drm/i915/intel_guc_ct.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 7b05fb8..16dccf5 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -58,6 +58,7 @@ i915-y += i915_cmd_parser.o \
 
 # general-purpose microcontroller (GuC) support
 i915-y += intel_uc.o \
+ intel_guc_ct.o \
  intel_guc_log.o \
  intel_guc_loader.o \
  intel_huc.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d703897..6c78469 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -869,6 +869,7 @@ static int i915_driver_init_early(struct drm_i915_private 
*dev_priv,
i915_workqueues_cleanup(dev_priv);
 err_engines:
i915_engines_cleanup(dev_priv);
+   intel_uc_cleanup(dev_priv);
return ret;
 }
 
@@ -883,6 +884,7 @@ static void i915_driver_cleanup_early(struct 
drm_i915_private *dev_priv)
intel_irq_fini(dev_priv);
i915_workqueues_cleanup(dev_priv);
i915_engines_cleanup(dev_priv);
+   intel_uc_cleanup(dev_priv);
 }
 
 static int i915_mmio_setup(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 17883a8..453eea5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -760,6 +760,7 @@ struct intel_csr {
func(has_gmbus_irq); \
func(has_gmch_display); \
func(has_guc); \
+   func(has_guc_ct); \
func(has_hotplug); \
func(has_l3_dpf); \
func(has_llc); \
@@ -2947,6 +2948,7 @@ intel_info(const struct drm_i915_private *dev_priv)
  * properties, so we have separate macros to test them.
  */
 #define HAS_GUC(dev_priv)  ((dev_priv)->info.has_guc)
+#define HAS_GUC_CT(dev_priv)   ((dev_priv)->info.has_guc_ct)
 #define HAS_GUC_UCODE(dev_priv)(HAS_GUC(dev_priv))
 #define HAS_GUC_SCHED(dev_priv)(HAS_GUC(dev_priv))
 #define HAS_HUC_UCODE(dev_priv)(HAS_GUC(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
new file mode 100644
index 000..869a7ad
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -0,0 +1,468 @@
+/*
+ * Copyright © 2016-2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "i915_drv.h"
+#include "intel_guc_ct.h"
+
+enum { CTB_SEND = 0, CTB_RECV = 1 };
+
+static inline const char *guc_ct_buffer_type_to_str(u32 type)
+{
+   switch (type) {
+   case