Only comment issues. Besides these:
Reviewed-by: Tomasz Lis <tomasz.lis@xxxxxxxxx>
On 2019-04-11 10:44, Michal Wajdeczko wrote:
GuC stores some data in there, which might be stale after a reset.
Reinitialize whole ADS in case any part of it was corrupted during
previous GuC run.
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: MichaĹ Winiarski <michal.winiarski@xxxxxxxxx>
Cc: Tomasz Lis <tomasz.lis@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_guc.h | 2 +
drivers/gpu/drm/i915/intel_guc_ads.c | 85 ++++++++++++++++++----------
drivers/gpu/drm/i915/intel_guc_ads.h | 1 +
3 files changed, 57 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 2c59ff8d9f39..4f3cf8eddfe6 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -26,6 +26,7 @@
#define _INTEL_GUC_H_
#include "intel_uncore.h"
+#include "intel_guc_ads.h"
#include "intel_guc_fw.h"
#include "intel_guc_fwif.h"
#include "intel_guc_ct.h"
@@ -177,6 +178,7 @@ u32 intel_guc_reserved_gtt_size(struct intel_guc *guc);
static inline int intel_guc_sanitize(struct intel_guc *guc)
{
intel_uc_fw_sanitize(&guc->fw);
+ intel_guc_ads_reset(guc);
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_guc_ads.c b/drivers/gpu/drm/i915/intel_guc_ads.c
index abab5cb6909a..97926effb944 100644
--- a/drivers/gpu/drm/i915/intel_guc_ads.c
+++ b/drivers/gpu/drm/i915/intel_guc_ads.c
@@ -72,43 +72,28 @@ static void guc_ct_pool_entries_init(struct guc_ct_pool_entry *pool, u32 num)
*/
#define LR_HW_CONTEXT_SIZE (80 * sizeof(u32))
-/**
- * intel_guc_ads_create() - creates GuC ADS
- * @guc: intel_guc struct
- *
- */
-int intel_guc_ads_create(struct intel_guc *guc)
+/* The ads obj includes the struct itself and buffers passed to GuC */
+struct __guc_ads_blob {
+ struct guc_ads ads;
+ struct guc_policies policies;
+ struct guc_mmio_reg_state reg_state;
+ struct guc_gt_system_info system_info;
+ struct guc_clients_info clients_info;
+ struct guc_ct_pool_entry ct_pool[GUC_CT_POOL_SIZE];
+ u8 reg_state_buffer[GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE];
+} __packed;
+
+static int __guc_ads_reinit(struct intel_guc *guc)
{
struct drm_i915_private *dev_priv = guc_to_i915(guc);
- struct i915_vma *vma;
- /* The ads obj includes the struct itself and buffers passed to GuC */
- struct {
- struct guc_ads ads;
- struct guc_policies policies;
- struct guc_mmio_reg_state reg_state;
- struct guc_gt_system_info system_info;
- struct guc_clients_info clients_info;
- struct guc_ct_pool_entry ct_pool[GUC_CT_POOL_SIZE];
- u8 reg_state_buffer[GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE];
- } __packed *blob;
+ struct __guc_ads_blob *blob;
const u32 skipped_size = LRC_PPHWSP_SZ * PAGE_SIZE + LR_HW_CONTEXT_SIZE;
u32 base;
u8 engine_class;
- int ret;
-
- GEM_BUG_ON(guc->ads_vma);
-
- vma = intel_guc_allocate_vma(guc, PAGE_ALIGN(sizeof(*blob)));
- if (IS_ERR(vma))
- return PTR_ERR(vma);
-
- guc->ads_vma = vma;
blob = i915_gem_object_pin_map(guc->ads_vma->obj, I915_MAP_WB);
- if (IS_ERR(blob)) {
- ret = PTR_ERR(blob);
- goto err_vma;
- }
+ if (IS_ERR(blob))
+ return PTR_ERR(blob);
/* GuC scheduling policies */
guc_policies_init(&blob->policies);
@@ -142,7 +127,7 @@ int intel_guc_ads_create(struct intel_guc *guc)
blob->system_info.vebox_enable_mask = VEBOX_MASK(dev_priv);
blob->system_info.vdbox_sfc_support_mask = RUNTIME_INFO(dev_priv)->vdbox_sfc_access;
- base = intel_guc_ggtt_offset(guc, vma);
+ base = intel_guc_ggtt_offset(guc, guc->ads_vma);
/* Clients info */
guc_ct_pool_entries_init(blob->ct_pool, ARRAY_SIZE(blob->ct_pool));
@@ -161,6 +146,32 @@ int intel_guc_ads_create(struct intel_guc *guc)
i915_gem_object_unpin_map(guc->ads_vma->obj);
return 0;
+}
+
+/**
+ * intel_guc_ads_create() - creates GuC ADS
+ * @guc: intel_guc struct
Are we really ok with documentation which just reads names with spaces
instead of underscores?
I believe the description should go deeper, or at least use different
words to describe the thing.
ie. here:
intel_guc_ads_create() - allocates and initializes GuC Additional Data Struct
@guc: instance of intel_guc which will own the ADS
+ *
+ */
+int intel_guc_ads_create(struct intel_guc *guc)
+{
+ const u32 size = PAGE_ALIGN(sizeof(struct __guc_ads_blob));
+ struct i915_vma *vma;
+ int ret;
+
+ GEM_BUG_ON(guc->ads_vma);
+
+ vma = intel_guc_allocate_vma(guc, size);
+ if (IS_ERR(vma))
+ return PTR_ERR(vma);
+
+ guc->ads_vma = vma;
+
+ ret = __guc_ads_reinit(guc);
+ if (ret)
+ goto err_vma;
+
+ return 0;
err_vma:
i915_vma_unpin_and_release(&guc->ads_vma, 0);
@@ -171,3 +182,15 @@ void intel_guc_ads_destroy(struct intel_guc *guc)
{
i915_vma_unpin_and_release(&guc->ads_vma, 0);
}
+
+/**
+ * intel_guc_ads_reset() - resets GuC ADS
Again:
intel_guc_ads_reset() - prepares GuC Additional Data Struct for reuse
-Tomasz
+ * @guc: intel_guc struct
+ *
+ */
+void intel_guc_ads_reset(struct intel_guc *guc)
+{
+ if (!guc->ads_vma)
+ return;
+ __guc_ads_reinit(guc);
+}
diff --git a/drivers/gpu/drm/i915/intel_guc_ads.h b/drivers/gpu/drm/i915/intel_guc_ads.h
index c4735742c564..7f40f9cd5fb9 100644
--- a/drivers/gpu/drm/i915/intel_guc_ads.h
+++ b/drivers/gpu/drm/i915/intel_guc_ads.h
@@ -29,5 +29,6 @@ struct intel_guc;
int intel_guc_ads_create(struct intel_guc *guc);
void intel_guc_ads_destroy(struct intel_guc *guc);
+void intel_guc_ads_reset(struct intel_guc *guc);
#endif
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
