On 03/01/2018 04:56 AM, Michal Wajdeczko wrote:
On Thu, 01 Mar 2018 02:01:36 +0100, Jackie Li <yaodong.li@xxxxxxxxx>
wrote:
+ if (guc_fw_size >= wopcm->size) {
+ DRM_ERROR("GuC FW (%uKiB) is too big to fit in WOPCM.",
+ guc_fw_size / 1024);
+ return -E2BIG;
+ }
+
+ if (huc_fw_size >= wopcm->size) {
+ DRM_ERROR("HuC FW (%uKiB) is too big to fit in WOPCM.",
+ huc_fw_size / 1024);
+ return -E2BIG;
+ }
+
+ /* Hardware requires GuC WOPCM base to be 16K aligned. */
+ guc_wopcm_base = ALIGN(huc_fw_size + WOPCM_RESERVED_SIZE,
+ GUC_WOPCM_OFFSET_ALIGNMENT);
+ if ((guc_wopcm_base + ctx_rsvd) >= wopcm->size) {
+ DRM_ERROR("GuC WOPCM base(%uKiB) is too big.\n",
+ guc_wopcm_base / 1024);
+ return -E2BIG;
+ }
hmm, all above checks are very unlikely, and are also covered by the
next check below, so maybe we can drop them?
These checks make sure these values are in a known range. so that we
don't need to worry about wrap around issues. e.g.
guc_wopcm_size = wopcm->size - guc_wopcm_base - ctx_rsvd may
wrap around if we don't have (guc_wopcm_base + ctx_rsvd) >= wopcm->size.
+
+ guc_wopcm_size = wopcm->size - guc_wopcm_base - ctx_rsvd;
+ /*
+ * GuC WOPCM size must be multiple of 4K pages. We've got the
maximum
+ * WOPCM size available for GuC. Trim the size value to 4K
boundary.
+ */
+ guc_wopcm_size &= GUC_WOPCM_SIZE_MASK;
+
+ DRM_DEBUG_DRIVER("Calculated GuC WOPCM Region: [%uKiB, %uKiB)\n",
+ guc_wopcm_base / 1024, guc_wopcm_size / 1024);
+
bikeshed: [n, m) notation suggests range n..m, so maybe better to use
DRM_DEBUG_DRIVER("Calculated GuC WOPCM: base %uKiB size %uKiB\n",
I'd like to keep it as [n, m) to suggest it's a region and to align with
other comments
in the code.
+ /*
+ * GuC WOPCM size needs to be big enough to include all GuC
firmware,
+ * extra 8KiB stack for GuC firmware and GUC_WOPCM_RESERVED.
+ */
+ guc_wopcm_rsvd = GUC_WOPCM_RESERVED + GUC_WOPCM_STACK_RESERVED;
+ if ((guc_fw_size + guc_wopcm_rsvd) > guc_wopcm_size) {
+ DRM_ERROR("Need %uKiB WOPCM for GuC, %uKiB available.\n",
+ (guc_fw_size + guc_wopcm_rsvd) / 1024,
+ guc_wopcm_size / 1024);
hmm, maybe to simplify calculations (and match them directly with
diagram)
we should introduce guc_wopcm_size_min:
guc_wopcm_size_min = GUC_WOPCM_RESERVED + GUC_WOPCM_STACK_RESERVED +
guc_fw_size;
if (guc_wopcm_size_min > guc_wopcm_size) {
DRM_ERROR("Need %uKiB WOPCM for GuC, %uKiB available.\n",
guc_wopcm_size_min/1024, guc_wopcm_size/1024);
As we discussed, we need find the maximum size value to meet the HW
requirement. So maybe
I need pass this comment as well.:-)
+ return -E2BIG;
+ }
+
+ err = check_hw_restriction(i915, guc_wopcm_base, guc_wopcm_size);
+ if (err) {
+ DRM_ERROR("Failed to meet HW restriction.\n");
+ return err;
+ }
+
+ wopcm->guc.base = guc_wopcm_base;
+ wopcm->guc.size = guc_wopcm_size;
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/i915/intel_wopcm.h
b/drivers/gpu/drm/i915/intel_wopcm.h
new file mode 100644
index 0000000..39d4847
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_wopcm.h
@@ -0,0 +1,34 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2017-2018 Intel Corporation
+ */
+
+#ifndef _INTEL_WOPCM_H_
+#define _INTEL_WOPCM_H_
+
+#include <linux/types.h>
+
+/**
+ * struct intel_wopcm - overall WOPCM info and WOPCM regions.
+ * @size: size of overall WOPCM.
bikeshed: maybe better to place this doc would be inside struct
to do not mix documentation style ?
Prefer to keep as it is now:-)
+ * @guc: GuC WOPCM Region info.
+ */
+struct intel_wopcm {
+ u32 size;
+ struct {
+ /**
+ * @base: GuC WOPCM base which is offset from WOPCM base.
+ */
+ u32 base;
+ /**
+ * @size: size of the GuC WOPCM region.
+ */
+ u32 size;
+ } guc;
+};
+
+void intel_wopcm_init_early(struct intel_wopcm *wopcm);
+int intel_wopcm_init(struct intel_wopcm *wopcm);
+
+#endif
only few bikesheds plus one suggestion, with that:
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Thanks for the review.
-Jackie
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
