Re: [Intel-gfx] [PATCH 1/4] drm/i915: Separate GuC/HuC specific functionality from intel_uc
On Fri, Sep 01, 2017 at 11:02:09AM +0530, Sagar Arun Kamble wrote:
> Removed unnecessary intel_uc.h includes as it is present in i915_drv.h.
> Created intel_guc.c and intel_guc.h for placing GuC specific code.
> Created intel_huc.h to refer to HuC specific functions.
>
> v2: Prepared intel_uc_common.h. huc_auth code declaration adjusted.
> Moved enable/disable_communication to intel_uc.c (Michal)
>
> Cc: Chris Wilson
> Cc: Michal Wajdeczko
> Cc: Daniele Ceraolo Spurio
> Signed-off-by: Sagar Arun Kamble
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/i915_drv.c| 1 -
> drivers/gpu/drm/i915/i915_guc_submission.c | 1 -
> drivers/gpu/drm/i915/intel_guc.c | 193 ++
> drivers/gpu/drm/i915/intel_guc.h | 200 +++
> drivers/gpu/drm/i915/intel_guc_loader.c| 1 -
> drivers/gpu/drm/i915/intel_huc.c | 50 +-
> drivers/gpu/drm/i915/intel_huc.h | 38 +
> drivers/gpu/drm/i915/intel_uc.c| 128 +--
> drivers/gpu/drm/i915/intel_uc.h| 254
> +
> drivers/gpu/drm/i915/intel_uc_common.h | 101
> 11 files changed, 545 insertions(+), 423 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/intel_guc.c
> create mode 100644 drivers/gpu/drm/i915/intel_guc.h
> create mode 100644 drivers/gpu/drm/i915/intel_huc.h
> create mode 100644 drivers/gpu/drm/i915/intel_uc_common.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 892f52b..efc5b30 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -59,6 +59,7 @@ i915-y += i915_cmd_parser.o \
>
> # general-purpose microcontroller (GuC) support
> i915-y += intel_uc.o \
> + intel_guc.o \
> intel_guc_ct.o \
> intel_guc_log.o \
> intel_guc_loader.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index f10a078..2ae730c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -50,7 +50,6 @@
> #include "i915_trace.h"
> #include "i915_vgpu.h"
> #include "intel_drv.h"
> -#include "intel_uc.h"
>
> static struct drm_driver driver;
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
> b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 48a1e93..602ae8a 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -23,7 +23,6 @@
> */
> #include
> #include "i915_drv.h"
> -#include "intel_uc.h"
>
> #include
>
> diff --git a/drivers/gpu/drm/i915/intel_guc.c
> b/drivers/gpu/drm/i915/intel_guc.c
> new file mode 100644
> index 000..978a0e3
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -0,0 +1,193 @@
> +/*
> + * Copyright © 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"
> +
> +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
> +{
> + WARN(1, "Unexpected send: action=%#x\n", *action);
> + return -ENODEV;
> +}
> +
> +static void gen8_guc_raise_irq(struct intel_guc *guc)
> +{
> + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> +
> + I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
> +}
> +
> +void intel_guc_init_early(struct intel_guc *guc)
> +{
> + intel_guc_ct_init_early(&guc->ct);
> +
> + mutex_init(&guc->send_mutex);
> + guc->send = intel_guc_send_nop;
> + guc->notify = gen8_guc_raise_irq;
> +}
> +
> +static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
> +{
> + GEM_BUG_ON(!guc->send_regs.base);
> + GEM_BUG_ON(!guc->send_regs.count);
> + GEM_BUG_ON(i >= guc->send_regs.count);
> +
> + return _MMIO(guc->send_regs.base + 4 * i);
> +}
> +
> +vo
Re: [Intel-gfx] [PATCH 1/4] drm/i915: Separate GuC/HuC specific functionality from intel_uc
On 9/7/2017 5:54 PM, Michał Winiarski wrote: On Fri, Sep 01, 2017 at 11:02:09AM +0530, Sagar Arun Kamble wrote: Removed unnecessary intel_uc.h includes as it is present in i915_drv.h. Created intel_guc.c and intel_guc.h for placing GuC specific code. Created intel_huc.h to refer to HuC specific functions. v2: Prepared intel_uc_common.h. huc_auth code declaration adjusted. Moved enable/disable_communication to intel_uc.c (Michal) In v2 you also renamed things, moved things around (and addressed all of the other review comments from Michał). Yes. Sorry. Will update in the next revision. Cc: Chris Wilson Cc: Michal Wajdeczko Cc: Daniele Ceraolo Spurio Signed-off-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.c| 1 - drivers/gpu/drm/i915/i915_guc_submission.c | 1 - drivers/gpu/drm/i915/intel_guc.c | 193 ++ drivers/gpu/drm/i915/intel_guc.h | 200 +++ drivers/gpu/drm/i915/intel_guc_loader.c| 1 - drivers/gpu/drm/i915/intel_huc.c | 50 +- drivers/gpu/drm/i915/intel_huc.h | 38 + drivers/gpu/drm/i915/intel_uc.c| 128 +-- drivers/gpu/drm/i915/intel_uc.h| 254 + drivers/gpu/drm/i915/intel_uc_common.h | 101 11 files changed, 545 insertions(+), 423 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_guc.c create mode 100644 drivers/gpu/drm/i915/intel_guc.h create mode 100644 drivers/gpu/drm/i915/intel_huc.h create mode 100644 drivers/gpu/drm/i915/intel_uc_common.h [SNIP] diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h index 22ae52b..c87a2b4 100644 --- a/drivers/gpu/drm/i915/intel_uc.h +++ b/drivers/gpu/drm/i915/intel_uc.h @@ -24,256 +24,8 @@ #ifndef _INTEL_UC_H_ #define _INTEL_UC_H_ [SNIP] -/* intel_huc.c */ -void intel_huc_select_fw(struct intel_huc *huc); -void intel_huc_init_hw(struct intel_huc *huc); -void intel_guc_auth_huc(struct drm_i915_private *dev_priv); +#include +#include +#include ^^^ Will this build? (well... it passed BAT, but it doesn't compile on my box). drivers/gpu/drm/i915 is not -I, so we should use quote marks, not angle brackets. Separate header, why? Can't we merge intel_uc_common.h with intel_uc.h? -Michał Will change these to quote marks. I added this as separate header to not declare struct intel_uc_fw in same header as struct intel_guc and struct intel_huc. We can merge but then it will take #include for intel_guc.h and intel_huc.h to the end of file. If we want to keep intel_guc.h and intel_huc.h at the top, they should reference pointer to struct intel_uc_fw. For readability I feel having intel_uc_common.h also seems intuitive. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915: Separate GuC/HuC specific functionality from intel_uc
On Fri, Sep 01, 2017 at 11:02:09AM +0530, Sagar Arun Kamble wrote: > Removed unnecessary intel_uc.h includes as it is present in i915_drv.h. > Created intel_guc.c and intel_guc.h for placing GuC specific code. > Created intel_huc.h to refer to HuC specific functions. > > v2: Prepared intel_uc_common.h. huc_auth code declaration adjusted. > Moved enable/disable_communication to intel_uc.c (Michal) In v2 you also renamed things, moved things around (and addressed all of the other review comments from Michał). > > Cc: Chris Wilson > Cc: Michal Wajdeczko > Cc: Daniele Ceraolo Spurio > Signed-off-by: Sagar Arun Kamble > --- > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/i915_drv.c| 1 - > drivers/gpu/drm/i915/i915_guc_submission.c | 1 - > drivers/gpu/drm/i915/intel_guc.c | 193 ++ > drivers/gpu/drm/i915/intel_guc.h | 200 +++ > drivers/gpu/drm/i915/intel_guc_loader.c| 1 - > drivers/gpu/drm/i915/intel_huc.c | 50 +- > drivers/gpu/drm/i915/intel_huc.h | 38 + > drivers/gpu/drm/i915/intel_uc.c| 128 +-- > drivers/gpu/drm/i915/intel_uc.h| 254 > + > drivers/gpu/drm/i915/intel_uc_common.h | 101 > 11 files changed, 545 insertions(+), 423 deletions(-) > create mode 100644 drivers/gpu/drm/i915/intel_guc.c > create mode 100644 drivers/gpu/drm/i915/intel_guc.h > create mode 100644 drivers/gpu/drm/i915/intel_huc.h > create mode 100644 drivers/gpu/drm/i915/intel_uc_common.h [SNIP] > diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h > index 22ae52b..c87a2b4 100644 > --- a/drivers/gpu/drm/i915/intel_uc.h > +++ b/drivers/gpu/drm/i915/intel_uc.h > @@ -24,256 +24,8 @@ > #ifndef _INTEL_UC_H_ > #define _INTEL_UC_H_ > [SNIP] > -/* intel_huc.c */ > -void intel_huc_select_fw(struct intel_huc *huc); > -void intel_huc_init_hw(struct intel_huc *huc); > -void intel_guc_auth_huc(struct drm_i915_private *dev_priv); > +#include > +#include > +#include ^^^ Will this build? (well... it passed BAT, but it doesn't compile on my box). drivers/gpu/drm/i915 is not -I, so we should use quote marks, not angle brackets. Separate header, why? Can't we merge intel_uc_common.h with intel_uc.h? -Michał ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915: Separate GuC/HuC specific functionality from intel_uc
Removed unnecessary intel_uc.h includes as it is present in i915_drv.h.
Created intel_guc.c and intel_guc.h for placing GuC specific code.
Created intel_huc.h to refer to HuC specific functions.
v2: Prepared intel_uc_common.h. huc_auth code declaration adjusted.
Moved enable/disable_communication to intel_uc.c (Michal)
Cc: Chris Wilson
Cc: Michal Wajdeczko
Cc: Daniele Ceraolo Spurio
Signed-off-by: Sagar Arun Kamble
---
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_drv.c| 1 -
drivers/gpu/drm/i915/i915_guc_submission.c | 1 -
drivers/gpu/drm/i915/intel_guc.c | 193 ++
drivers/gpu/drm/i915/intel_guc.h | 200 +++
drivers/gpu/drm/i915/intel_guc_loader.c| 1 -
drivers/gpu/drm/i915/intel_huc.c | 50 +-
drivers/gpu/drm/i915/intel_huc.h | 38 +
drivers/gpu/drm/i915/intel_uc.c| 128 +--
drivers/gpu/drm/i915/intel_uc.h| 254 +
drivers/gpu/drm/i915/intel_uc_common.h | 101
11 files changed, 545 insertions(+), 423 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_guc.c
create mode 100644 drivers/gpu/drm/i915/intel_guc.h
create mode 100644 drivers/gpu/drm/i915/intel_huc.h
create mode 100644 drivers/gpu/drm/i915/intel_uc_common.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 892f52b..efc5b30 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -59,6 +59,7 @@ i915-y += i915_cmd_parser.o \
# general-purpose microcontroller (GuC) support
i915-y += intel_uc.o \
+ intel_guc.o \
intel_guc_ct.o \
intel_guc_log.o \
intel_guc_loader.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f10a078..2ae730c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -50,7 +50,6 @@
#include "i915_trace.h"
#include "i915_vgpu.h"
#include "intel_drv.h"
-#include "intel_uc.h"
static struct drm_driver driver;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
b/drivers/gpu/drm/i915/i915_guc_submission.c
index 48a1e93..602ae8a 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -23,7 +23,6 @@
*/
#include
#include "i915_drv.h"
-#include "intel_uc.h"
#include
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
new file mode 100644
index 000..978a0e3
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -0,0 +1,193 @@
+/*
+ * Copyright © 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"
+
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
+{
+ WARN(1, "Unexpected send: action=%#x\n", *action);
+ return -ENODEV;
+}
+
+static void gen8_guc_raise_irq(struct intel_guc *guc)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+ I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
+}
+
+void intel_guc_init_early(struct intel_guc *guc)
+{
+ intel_guc_ct_init_early(&guc->ct);
+
+ mutex_init(&guc->send_mutex);
+ guc->send = intel_guc_send_nop;
+ guc->notify = gen8_guc_raise_irq;
+}
+
+static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
+{
+ GEM_BUG_ON(!guc->send_regs.base);
+ GEM_BUG_ON(!guc->send_regs.count);
+ GEM_BUG_ON(i >= guc->send_regs.count);
+
+ return _MMIO(guc->send_regs.base + 4 * i);
+}
+
+void intel_guc_init_send_regs(struct intel_guc *guc)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ enum forcewake_domains fw_domains = 0;
+ unsigned int i;
+
+ guc->send_regs.base = i915_mmio_reg_offset(SOFT_SCRATCH(0));
+ guc->send_regs.count = SOFT_SCRATCH_CO
