From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
Brian Welty <brian.welty@intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/selftests: Fix selftest_mocs for DGFX
Date: Fri, 14 Feb 2020 11:36:05 -0800 [thread overview]
Message-ID: <b77b1bcf-4e69-219f-92a7-dfd94e49ed9b@intel.com> (raw)
In-Reply-To: <158170497173.15393.11944816323451861470@skylake-alporthouse-com>
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 <lucas.demarchi@intel.com>
>> 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, &arg->table))
> + memset(arg, 0, sizeof(*arg));
> +
> + if (!get_mocs_settings(gt->i915, &table))
> 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, &arg->table, &offset);
> + err = read_mocs_table(rq, &arg->mocs, &offset);
> if (!err && ce->engine->class == RENDER_CLASS)
> - err = read_l3cc_table(rq, &arg->table, &offset);
> + err = read_l3cc_table(rq, &arg->l3cc, &offset);
> 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, &arg->table, &vaddr);
> + err = check_mocs_table(ce->engine, &arg->mocs, &vaddr);
> if (!err && ce->engine->class == RENDER_CLASS)
> - err = check_l3cc_table(ce->engine, &arg->table, &vaddr);
> + err = check_l3cc_table(ce->engine, &arg->l3cc, &vaddr);
> 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
next prev parent reply other threads:[~2020-02-14 19:36 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-13 0:14 [Intel-gfx] [PATCH] drm/i915/selftests: Fix selftest_mocs for DGFX Brian Welty
2020-02-13 0:34 ` Chris Wilson
2020-02-13 0:49 ` Brian Welty
2020-02-14 17:56 ` Daniele Ceraolo Spurio
2020-02-14 18:29 ` Chris Wilson
2020-02-14 18:37 ` Chris Wilson
2020-02-14 19:36 ` Daniele Ceraolo Spurio [this message]
2020-02-13 7:33 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2020-02-13 7:54 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-02-14 21:28 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/selftests: Fix selftest_mocs for DGFX (rev2) Patchwork
2020-02-14 21:52 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-02-16 15:56 ` [Intel-gfx] [PATCH] drm/i915/gt: Refactor l3cc/mocs availability Chris Wilson
2020-02-16 16:50 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/selftests: Fix selftest_mocs for DGFX (rev3) Patchwork
2020-02-18 20:46 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b77b1bcf-4e69-219f-92a7-dfd94e49ed9b@intel.com \
--to=daniele.ceraolospurio@intel.com \
--cc=brian.welty@intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).