Re: [Intel-gfx] [PATCH] drm/i915/selftests: Fix selftest_mocs for DGFX
On 2/14/20 10:29 AM, Chris Wilson wrote:
Quoting Daniele Ceraolo Spurio (2020-02-14 17:56:58)
On 2/12/20 4:49 PM, Brian Welty wrote:
On 2/12/2020 4:34 PM, Chris Wilson wrote:
Quoting Brian Welty (2020-02-13 00:14:18)
For DGFX devices, the MOCS control value is not initialized or used.
Then why is the table populated?
-Chris
The format has changed (been reduced?) for DGFX.
drm_i915_mocs_entry.l3cc_value is what is still initialized/used.
Probably first needed is the patch that defines the table entries for DGFX.
Ugh, I didn't notice this wasn't applied yet. Let me ask about this.
We do have:
commit e6e2ac07118b15f25683fcbd59ea1be73ec9465d
Author: Lucas De Marchi
Date: Thu Oct 24 12:51:21 2019 -0700
drm/i915: do not set MOCS control values on dgfx
So I see no reason not to add this change to the test side to match
that. Maybe we can add an additional check in the test to validate that
all the control_entries are set to 0 in the table on DGFX?
My expectation was that as we were not setting mocs values, we would not
have defined a table for it. However, the table is combined for mocs and
l3cc. l3cc is still used, right?
yes, l3cc is still used. The diff below looks ok to me to keep the
table-driven approach.
Daniele
My ideal would be that our tables did remain the truth value we could
use directly -- that would require splitting the tables though.
If we did something like
diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c
b/drivers/gpu/drm/i915/gt/selftest_mocs.c
index de1f83100fb6..2c636257f12c 100644
--- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
@@ -12,7 +12,8 @@
#include "selftests/igt_spinner.h"
struct live_mocs {
- struct drm_i915_mocs_table table;
+ struct drm_i915_mocs_table mocs;
+ struct drm_i915_mocs_table l3cc;
struct i915_vma *scratch;
void *vaddr;
};
@@ -68,13 +69,32 @@ static struct i915_vma *create_scratch(struct intel_gt *gt)
return vma;
}
+static bool has_l3cc(struct drm_i915_private *i915)
+{
+ return true;
+}
+
+static bool has_mocs(struct drm_i915_private *i915)
+{
+ return !IS_DGFX(i915);
+}
+
static int live_mocs_init(struct live_mocs *arg, struct intel_gt *gt)
{
+ struct drm_i915_mocs_table table;
int err;
- if (!get_mocs_settings(gt->i915, >table))
+ memset(arg, 0, sizeof(*arg));
+
+ if (!get_mocs_settings(gt->i915, ))
return -EINVAL;
+ if (has_l3cc(gt->i915))
+ arg->l3cc = table;
+
+ if (has_mocs(gt->i915))
+ arg->mocs = table;
+
arg->scratch = create_scratch(gt);
if (IS_ERR(arg->scratch))
return PTR_ERR(arg->scratch);
@@ -223,9 +243,9 @@ static int check_mocs_engine(struct live_mocs *arg,
/* Read the mocs tables back using SRM */
offset = i915_ggtt_offset(vma);
if (!err)
- err = read_mocs_table(rq, >table, );
+ err = read_mocs_table(rq, >mocs, );
if (!err && ce->engine->class == RENDER_CLASS)
- err = read_l3cc_table(rq, >table, );
+ err = read_l3cc_table(rq, >l3cc, );
offset -= i915_ggtt_offset(vma);
GEM_BUG_ON(offset > PAGE_SIZE);
@@ -236,9 +256,9 @@ static int check_mocs_engine(struct live_mocs *arg,
/* Compare the results against the expected tables */
vaddr = arg->vaddr;
if (!err)
- err = check_mocs_table(ce->engine, >table, );
+ err = check_mocs_table(ce->engine, >mocs, );
if (!err && ce->engine->class == RENDER_CLASS)
- err = check_l3cc_table(ce->engine, >table, );
+ err = check_l3cc_table(ce->engine, >l3cc, );
if (err)
return err;
we could retain the table driven approach?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/selftests: Fix selftest_mocs for DGFX
Quoting Chris Wilson (2020-02-14 18:29:31)
> +static bool has_l3cc(struct drm_i915_private *i915)
> +{
> + return true;
> +}
> +
> +static bool has_mocs(struct drm_i915_private *i915)
> +{
> + return !IS_DGFX(i915);
> +}
> +
> static int live_mocs_init(struct live_mocs *arg, struct intel_gt *gt)
> {
> + struct drm_i915_mocs_table table;
> int err;
>
> - if (!get_mocs_settings(gt->i915, >table))
> + memset(arg, 0, sizeof(*arg));
> +
> + if (!get_mocs_settings(gt->i915, ))
> return -EINVAL;
On top of that, if we did something like return a mask from
get_mocs_settings() for HAS_L3CC | HAS_MOCS (or put it into table) we
can share the init with the main code and reduce potential drift.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/selftests: Fix selftest_mocs for DGFX
Quoting Daniele Ceraolo Spurio (2020-02-14 17:56:58)
>
>
> On 2/12/20 4:49 PM, Brian Welty wrote:
> >
> > On 2/12/2020 4:34 PM, Chris Wilson wrote:
> >> Quoting Brian Welty (2020-02-13 00:14:18)
> >>> For DGFX devices, the MOCS control value is not initialized or used.
> >>
> >> Then why is the table populated?
> >> -Chris
> >>
> >
> > The format has changed (been reduced?) for DGFX.
> > drm_i915_mocs_entry.l3cc_value is what is still initialized/used.
> > Probably first needed is the patch that defines the table entries for DGFX.
> > Ugh, I didn't notice this wasn't applied yet. Let me ask about this.
> >
>
> We do have:
>
> commit e6e2ac07118b15f25683fcbd59ea1be73ec9465d
> Author: Lucas De Marchi
> Date: Thu Oct 24 12:51:21 2019 -0700
>
> drm/i915: do not set MOCS control values on dgfx
>
> So I see no reason not to add this change to the test side to match
> that. Maybe we can add an additional check in the test to validate that
> all the control_entries are set to 0 in the table on DGFX?
My expectation was that as we were not setting mocs values, we would not
have defined a table for it. However, the table is combined for mocs and
l3cc. l3cc is still used, right?
My ideal would be that our tables did remain the truth value we could
use directly -- that would require splitting the tables though.
If we did something like
diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c
b/drivers/gpu/drm/i915/gt/selftest_mocs.c
index de1f83100fb6..2c636257f12c 100644
--- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
@@ -12,7 +12,8 @@
#include "selftests/igt_spinner.h"
struct live_mocs {
- struct drm_i915_mocs_table table;
+ struct drm_i915_mocs_table mocs;
+ struct drm_i915_mocs_table l3cc;
struct i915_vma *scratch;
void *vaddr;
};
@@ -68,13 +69,32 @@ static struct i915_vma *create_scratch(struct intel_gt *gt)
return vma;
}
+static bool has_l3cc(struct drm_i915_private *i915)
+{
+ return true;
+}
+
+static bool has_mocs(struct drm_i915_private *i915)
+{
+ return !IS_DGFX(i915);
+}
+
static int live_mocs_init(struct live_mocs *arg, struct intel_gt *gt)
{
+ struct drm_i915_mocs_table table;
int err;
- if (!get_mocs_settings(gt->i915, >table))
+ memset(arg, 0, sizeof(*arg));
+
+ if (!get_mocs_settings(gt->i915, ))
return -EINVAL;
+ if (has_l3cc(gt->i915))
+ arg->l3cc = table;
+
+ if (has_mocs(gt->i915))
+ arg->mocs = table;
+
arg->scratch = create_scratch(gt);
if (IS_ERR(arg->scratch))
return PTR_ERR(arg->scratch);
@@ -223,9 +243,9 @@ static int check_mocs_engine(struct live_mocs *arg,
/* Read the mocs tables back using SRM */
offset = i915_ggtt_offset(vma);
if (!err)
- err = read_mocs_table(rq, >table, );
+ err = read_mocs_table(rq, >mocs, );
if (!err && ce->engine->class == RENDER_CLASS)
- err = read_l3cc_table(rq, >table, );
+ err = read_l3cc_table(rq, >l3cc, );
offset -= i915_ggtt_offset(vma);
GEM_BUG_ON(offset > PAGE_SIZE);
@@ -236,9 +256,9 @@ static int check_mocs_engine(struct live_mocs *arg,
/* Compare the results against the expected tables */
vaddr = arg->vaddr;
if (!err)
- err = check_mocs_table(ce->engine, >table, );
+ err = check_mocs_table(ce->engine, >mocs, );
if (!err && ce->engine->class == RENDER_CLASS)
- err = check_l3cc_table(ce->engine, >table, );
+ err = check_l3cc_table(ce->engine, >l3cc, );
if (err)
return err;
we could retain the table driven approach?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/selftests: Fix selftest_mocs for DGFX
On 2/12/20 4:49 PM, Brian Welty wrote: On 2/12/2020 4:34 PM, Chris Wilson wrote: Quoting Brian Welty (2020-02-13 00:14:18) For DGFX devices, the MOCS control value is not initialized or used. Then why is the table populated? -Chris The format has changed (been reduced?) for DGFX. drm_i915_mocs_entry.l3cc_value is what is still initialized/used. Probably first needed is the patch that defines the table entries for DGFX. Ugh, I didn't notice this wasn't applied yet. Let me ask about this. We do have: commit e6e2ac07118b15f25683fcbd59ea1be73ec9465d Author: Lucas De Marchi Date: Thu Oct 24 12:51:21 2019 -0700 drm/i915: do not set MOCS control values on dgfx So I see no reason not to add this change to the test side to match that. Maybe we can add an additional check in the test to validate that all the control_entries are set to 0 in the table on DGFX? Daniele -Brian ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/selftests: Fix selftest_mocs for DGFX
On 2/12/2020 4:34 PM, Chris Wilson wrote: > Quoting Brian Welty (2020-02-13 00:14:18) >> For DGFX devices, the MOCS control value is not initialized or used. > > Then why is the table populated? > -Chris > The format has changed (been reduced?) for DGFX. drm_i915_mocs_entry.l3cc_value is what is still initialized/used. Probably first needed is the patch that defines the table entries for DGFX. Ugh, I didn't notice this wasn't applied yet. Let me ask about this. -Brian ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/selftests: Fix selftest_mocs for DGFX
Quoting Brian Welty (2020-02-13 00:14:18) > For DGFX devices, the MOCS control value is not initialized or used. Then why is the table populated? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/selftests: Fix selftest_mocs for DGFX
For DGFX devices, the MOCS control value is not initialized or used.
Update the selftest to skip reading and checking control values
for these devices.
References: e6e2ac07118b ("drm/i915: do not set MOCS control values on dgfx")
Fixes: 3fb33cd32ffd ("drm/i915/selftests: Add coverage of mocs registers")
Signed-off-by: Brian Welty
---
drivers/gpu/drm/i915/gt/selftest_mocs.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c
b/drivers/gpu/drm/i915/gt/selftest_mocs.c
index de1f83100fb6..8a94a546d580 100644
--- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
@@ -199,7 +199,7 @@ static int check_l3cc_table(struct intel_engine_cs *engine,
return 0;
}
-static int check_mocs_engine(struct live_mocs *arg,
+static int check_mocs_engine(struct intel_gt *gt, struct live_mocs *arg,
struct intel_context *ce)
{
struct i915_vma *vma = arg->scratch;
@@ -222,7 +222,7 @@ static int check_mocs_engine(struct live_mocs *arg,
/* Read the mocs tables back using SRM */
offset = i915_ggtt_offset(vma);
- if (!err)
+ if (!err && !IS_DGFX(gt->i915))
err = read_mocs_table(rq, >table, );
if (!err && ce->engine->class == RENDER_CLASS)
err = read_l3cc_table(rq, >table, );
@@ -235,7 +235,7 @@ static int check_mocs_engine(struct live_mocs *arg,
/* Compare the results against the expected tables */
vaddr = arg->vaddr;
- if (!err)
+ if (!err && !IS_DGFX(gt->i915))
err = check_mocs_table(ce->engine, >table, );
if (!err && ce->engine->class == RENDER_CLASS)
err = check_l3cc_table(ce->engine, >table, );
@@ -262,7 +262,7 @@ static int live_mocs_kernel(void *arg)
for_each_engine(engine, gt, id) {
intel_engine_pm_get(engine);
- err = check_mocs_engine(, engine->kernel_context);
+ err = check_mocs_engine(gt, , engine->kernel_context);
intel_engine_pm_put(engine);
if (err)
break;
@@ -295,7 +295,7 @@ static int live_mocs_clean(void *arg)
break;
}
- err = check_mocs_engine(, ce);
+ err = check_mocs_engine(gt, , ce);
intel_context_put(ce);
if (err)
break;
@@ -332,7 +332,7 @@ static int active_engine_reset(struct intel_context *ce,
return err;
}
-static int __live_mocs_reset(struct live_mocs *mocs,
+static int __live_mocs_reset(struct intel_gt *gt, struct live_mocs *mocs,
struct intel_context *ce)
{
int err;
@@ -341,7 +341,7 @@ static int __live_mocs_reset(struct live_mocs *mocs,
if (err)
return err;
- err = check_mocs_engine(mocs, ce);
+ err = check_mocs_engine(gt, mocs, ce);
if (err)
return err;
@@ -349,13 +349,13 @@ static int __live_mocs_reset(struct live_mocs *mocs,
if (err)
return err;
- err = check_mocs_engine(mocs, ce);
+ err = check_mocs_engine(gt, mocs, ce);
if (err)
return err;
- intel_gt_reset(ce->engine->gt, ce->engine->mask, "mocs");
+ intel_gt_reset(gt, ce->engine->mask, "mocs");
- err = check_mocs_engine(mocs, ce);
+ err = check_mocs_engine(gt, mocs, ce);
if (err)
return err;
@@ -390,7 +390,7 @@ static int live_mocs_reset(void *arg)
}
intel_engine_pm_get(engine);
- err = __live_mocs_reset(, ce);
+ err = __live_mocs_reset(gt, , ce);
intel_engine_pm_put(engine);
intel_context_put(ce);
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
