intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: John Harrison <john.c.harrison@intel.com>
To: Martin Peres <martin.peres@free.fr>,
	Matthew Brost <matthew.brost@intel.com>,
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 47/47] drm/i915/guc: Unblock GuC submission on Gen11+
Date: Wed, 30 Jun 2021 11:58:25 -0700	[thread overview]
Message-ID: <05e1d462-57ae-888a-888c-3ad486150821@intel.com> (raw)
In-Reply-To: <88cbe963-7188-f4ae-5acf-01a80bd2fe25@free.fr>

On 6/30/2021 01:22, Martin Peres wrote:
> On 24/06/2021 10:05, Matthew Brost wrote:
>> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>
>> Unblock GuC submission on Gen11+ platforms.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/uc/intel_guc.h            |  1 +
>>   drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c |  8 ++++++++
>>   drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h |  3 +--
>>   drivers/gpu/drm/i915/gt/uc/intel_uc.c             | 14 +++++++++-----
>>   4 files changed, 19 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>> index fae01dc8e1b9..77981788204f 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>> @@ -54,6 +54,7 @@ struct intel_guc {
>>       struct ida guc_ids;
>>       struct list_head guc_id_list;
>>   +    bool submission_supported;
>>       bool submission_selected;
>>         struct i915_vma *ads_vma;
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> index a427336ce916..405339202280 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> @@ -2042,6 +2042,13 @@ void intel_guc_submission_disable(struct 
>> intel_guc *guc)
>>       /* Note: By the time we're here, GuC may have already been 
>> reset */
>>   }
>>   +static bool __guc_submission_supported(struct intel_guc *guc)
>> +{
>> +    /* GuC submission is unavailable for pre-Gen11 */
>> +    return intel_guc_is_supported(guc) &&
>> +           INTEL_GEN(guc_to_gt(guc)->i915) >= 11;
>> +}
>> +
>>   static bool __guc_submission_selected(struct intel_guc *guc)
>>   {
>>       struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
>> @@ -2054,6 +2061,7 @@ static bool __guc_submission_selected(struct 
>> intel_guc *guc)
>>     void intel_guc_submission_init_early(struct intel_guc *guc)
>>   {
>> +    guc->submission_supported = __guc_submission_supported(guc);
>>       guc->submission_selected = __guc_submission_selected(guc);
>>   }
>>   diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h 
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
>> index a2a3fad72be1..be767eb6ff71 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
>> @@ -37,8 +37,7 @@ int intel_guc_wait_for_pending_msg(struct intel_guc 
>> *guc,
>>     static inline bool intel_guc_submission_is_supported(struct 
>> intel_guc *guc)
>>   {
>> -    /* XXX: GuC submission is unavailable for now */
>> -    return false;
>> +    return guc->submission_supported;
>>   }
>>     static inline bool intel_guc_submission_is_wanted(struct 
>> intel_guc *guc)
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c 
>> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> index 7a69c3c027e9..61be0aa81492 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> @@ -34,8 +34,15 @@ static void uc_expand_default_options(struct 
>> intel_uc *uc)
>>           return;
>>       }
>>   -    /* Default: enable HuC authentication only */
>> -    i915->params.enable_guc = ENABLE_GUC_LOAD_HUC;
>> +    /* Intermediate platforms are HuC authentication only */
>> +    if (IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
>> +        drm_dbg(&i915->drm, "Disabling GuC only due to old 
>> platform\n");
>
> This comment does not seem accurate, given that DG1 is barely out, and 
> ADL is not out yet. How about:
>
> "Disabling GuC on untested platforms"?
>
Just because something is not in the shops yet does not mean it is new. 
Technology is always obsolete by the time it goes on sale.

And the issue is not a lack of testing, it is a question of whether we 
are allowed to change the default on something that has already started 
being used by customers or not (including pre-release beta customers). 
I.e. it is basically a political decision not an engineering decision.


>> +        i915->params.enable_guc = ENABLE_GUC_LOAD_HUC;
>> +        return;
>> +    }
>> +
>> +    /* Default: enable HuC authentication and GuC submission */
>> +    i915->params.enable_guc = ENABLE_GUC_LOAD_HUC | 
>> ENABLE_GUC_SUBMISSION;
>
> This seems to be in contradiction with the GuC submission plan which 
> states:
>
> "Not enabled by default on any current platforms but can be enabled 
> via modparam enable_guc".
All current platforms have already been explicitly tested for above. 
This is setting the default on newer platforms - ADL-P and later. For 
which the official expectation is to have GuC enabled.

>
> When you rework the patch, could you please add a warning when the 
> user force-enables the GuC Command Submission? 
There already is one. If you set the module parameter then the kernel is 
tainted. That means 'here be dragons' - you have done something 
officially not supported to your kernel so all bets are off, if it blows 
up it is your own problem.

> Something like:
>
> "WARNING: The user force-enabled the experimental GuC command 
> submission backend using i915.enable_guc. Please disable it if 
> experiencing stability issues. No bug reports will be accepted on this 
> backend".
>
> This should allow you to work on the backend, while communicating 
> clearly to users that it is not ready just yet. Once it has matured, 
> the warning can be removed.
The fact that ADL-P is not on the shelves in your local retail store 
should be sufficient to ensure that users are aware that ADL-P support 
is not entirely mature yet. And in many ways, not just GuC based submission.

John.


>
> Cheers,
> Martin
>
>>   }
>>     /* Reset GuC providing us with fresh state for both GuC and HuC.
>> @@ -313,9 +320,6 @@ static int __uc_init(struct intel_uc *uc)
>>       if (i915_inject_probe_failure(uc_to_gt(uc)->i915))
>>           return -ENOMEM;
>>   -    /* XXX: GuC submission is unavailable for now */
>> -    GEM_BUG_ON(intel_uc_uses_guc_submission(uc));
>> -
>>       ret = intel_guc_init(guc);
>>       if (ret)
>>           return ret;
>>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2021-06-30 18:58 UTC|newest]

Thread overview: 170+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-24  7:04 [Intel-gfx] [PATCH 00/47] GuC submission support Matthew Brost
2021-06-24  7:04 ` [Intel-gfx] [PATCH 01/47] drm/i915/guc: Relax CTB response timeout Matthew Brost
2021-06-24 17:23   ` Michal Wajdeczko
2021-06-24  7:04 ` [Intel-gfx] [PATCH 02/47] drm/i915/guc: Improve error message for unsolicited CT response Matthew Brost
2021-06-25 11:58   ` Michal Wajdeczko
2021-06-24  7:04 ` [Intel-gfx] [PATCH 03/47] drm/i915/guc: Increase size of CTB buffers Matthew Brost
2021-06-24 13:49   ` Michal Wajdeczko
2021-06-24 15:41     ` Matthew Brost
2021-06-25 12:03       ` Michal Wajdeczko
2021-06-24  7:04 ` [Intel-gfx] [PATCH 04/47] drm/i915/guc: Add non blocking CTB send function Matthew Brost
2021-06-24 14:48   ` Michal Wajdeczko
2021-06-24 15:49     ` Matthew Brost
2021-06-24 17:02       ` Michal Wajdeczko
2021-06-24 22:41         ` Matthew Brost
2021-06-25 11:50           ` Michal Wajdeczko
2021-06-25 17:53             ` Matthew Brost
2021-06-24 22:47         ` Matthew Brost
2021-06-24  7:04 ` [Intel-gfx] [PATCH 05/47] drm/i915/guc: Add stall timer to " Matthew Brost
2021-06-24 17:37   ` Michal Wajdeczko
2021-06-24 23:01     ` Matthew Brost
2021-06-24  7:04 ` [Intel-gfx] [PATCH 06/47] drm/i915/guc: Optimize CTB writes and reads Matthew Brost
2021-06-25 13:09   ` Michal Wajdeczko
2021-06-25 18:26     ` Matthew Brost
2021-06-25 20:28     ` Matthew Brost
2021-06-24  7:04 ` [Intel-gfx] [PATCH 07/47] drm/i915/guc: Module load failure test for CT buffer creation Matthew Brost
2021-06-24  7:04 ` [Intel-gfx] [PATCH 08/47] drm/i915/guc: Add new GuC interface defines and structures Matthew Brost
2021-06-29 21:11   ` John Harrison
2021-06-30  0:30     ` Matthew Brost
2021-06-24  7:04 ` [Intel-gfx] [PATCH 09/47] drm/i915/guc: Remove GuC stage descriptor, add lrc descriptor Matthew Brost
2021-06-25 19:44   ` John Harrison
2021-06-24  7:04 ` [Intel-gfx] [PATCH 10/47] drm/i915/guc: Add lrc descriptor context lookup array Matthew Brost
2021-06-25 13:17   ` Michal Wajdeczko
2021-06-25 17:26     ` Matthew Brost
2021-06-29 21:20       ` John Harrison
2021-06-24  7:04 ` [Intel-gfx] [PATCH 11/47] drm/i915/guc: Implement GuC submission tasklet Matthew Brost
2021-06-29 22:04   ` John Harrison
2021-06-30  0:41     ` Matthew Brost
2021-06-24  7:04 ` [Intel-gfx] [PATCH 12/47] drm/i915/guc: Add bypass tasklet submission path to GuC Matthew Brost
2021-06-29 22:09   ` John Harrison
2021-06-24  7:04 ` [Intel-gfx] [PATCH 13/47] drm/i915/guc: Implement GuC context operations for new inteface Matthew Brost
2021-06-25 13:25   ` Michal Wajdeczko
2021-06-25 17:46     ` Matthew Brost
2021-06-24  7:04 ` [Intel-gfx] [PATCH 14/47] drm/i915/guc: Insert fence on context when deregistering Matthew Brost
2021-07-09 22:39   ` John Harrison
2021-06-24  7:04 ` [Intel-gfx] [PATCH 15/47] drm/i915/guc: Defer context unpin until scheduling is disabled Matthew Brost
2021-07-09 22:48   ` John Harrison
2021-06-24  7:04 ` [Intel-gfx] [PATCH 16/47] drm/i915/guc: Disable engine barriers with GuC during unpin Matthew Brost
2021-07-09 22:53   ` John Harrison
2021-07-10  3:00     ` Matthew Brost
2021-07-12 17:57       ` John Harrison
2021-07-12 18:11         ` Daniel Vetter
2021-06-24  7:04 ` [Intel-gfx] [PATCH 17/47] drm/i915/guc: Extend deregistration fence to schedule disable Matthew Brost
2021-07-09 22:59   ` John Harrison
2021-07-10  3:36     ` Matthew Brost
2021-07-12 17:54       ` John Harrison
2021-06-24  7:04 ` [Intel-gfx] [PATCH 18/47] drm/i915: Disable preempt busywait when using GuC scheduling Matthew Brost
2021-07-09 23:03   ` John Harrison
2021-06-24  7:04 ` [Intel-gfx] [PATCH 19/47] drm/i915/guc: Ensure request ordering via completion fences Matthew Brost
2021-07-15  1:51   ` Daniele Ceraolo Spurio
2021-06-24  7:04 ` [Intel-gfx] [PATCH 20/47] drm/i915/guc: Disable semaphores when using GuC scheduling Matthew Brost
2021-07-09 23:53   ` John Harrison
2021-07-15  0:07     ` Matthew Brost
2021-06-24  7:04 ` [Intel-gfx] [PATCH 21/47] drm/i915/guc: Ensure G2H response has space in buffer Matthew Brost
2021-07-13 18:36   ` John Harrison
2021-07-15  0:06     ` Matthew Brost
2021-07-15  0:12       ` John Harrison
2021-06-24  7:04 ` [Intel-gfx] [PATCH 22/47] drm/i915/guc: Update intel_gt_wait_for_idle to work with GuC Matthew Brost
2021-07-10  0:16   ` John Harrison
2021-07-10  3:55     ` Matthew Brost
2021-07-17  4:09       ` Matthew Brost
2021-06-24  7:04 ` [Intel-gfx] [PATCH 23/47] drm/i915/guc: Update GuC debugfs to support new GuC Matthew Brost
2021-07-12 18:05   ` John Harrison
2021-07-12 20:59     ` Matthew Brost
2021-07-12 21:37       ` John Harrison
2021-07-13  8:51   ` Michal Wajdeczko
2021-07-14 23:56     ` Matthew Brost
2021-06-24  7:04 ` [Intel-gfx] [PATCH 24/47] drm/i915/guc: Add several request trace points Matthew Brost
2021-07-12 18:08   ` John Harrison
2021-07-13  9:06   ` Tvrtko Ursulin
2021-07-20  1:59     ` Matthew Brost
2021-07-22 13:55       ` Tvrtko Ursulin
2021-06-24  7:04 ` [Intel-gfx] [PATCH 25/47] drm/i915: Add intel_context tracing Matthew Brost
2021-07-12 18:10   ` John Harrison
2021-07-12 21:47     ` Matthew Brost
2021-07-12 21:51       ` John Harrison
2021-06-24  7:04 ` [Intel-gfx] [PATCH 26/47] drm/i915/guc: GuC virtual engines Matthew Brost
2021-07-15  1:21   ` Daniele Ceraolo Spurio
2021-06-24  7:04 ` [Intel-gfx] [PATCH 27/47] drm/i915: Track 'serial' counts for " Matthew Brost
2021-07-12 18:11   ` John Harrison
2021-07-12 20:06     ` Matthew Brost
2021-06-24  7:04 ` [Intel-gfx] [PATCH 28/47] drm/i915: Hold reference to intel_context over life of i915_request Matthew Brost
2021-07-12 18:23   ` John Harrison
2021-07-12 20:05     ` Matthew Brost
2021-07-12 21:36       ` Matthew Brost
2021-07-12 21:48         ` John Harrison
2021-06-24  7:04 ` [Intel-gfx] [PATCH 29/47] drm/i915/guc: Disable bonding extension with GuC submission Matthew Brost
2021-07-12 18:23   ` John Harrison
2021-06-24  7:04 ` [Intel-gfx] [PATCH 30/47] drm/i915/guc: Direct all breadcrumbs for a class to single breadcrumbs Matthew Brost
2021-07-12 19:19   ` John Harrison
2021-06-24  7:05 ` [Intel-gfx] [PATCH 31/47] drm/i915/guc: Reset implementation for new GuC interface Matthew Brost
2021-07-12 19:58   ` John Harrison
2021-07-15  0:53     ` Matthew Brost
2021-07-15  9:36   ` Tvrtko Ursulin
2021-07-26 22:48     ` Matthew Brost
2021-07-27  8:56       ` Tvrtko Ursulin
2021-07-27 18:30         ` Matthew Brost
2021-06-24  7:05 ` [Intel-gfx] [PATCH 32/47] drm/i915: Reset GPU immediately if submission is disabled Matthew Brost
2021-07-12 20:01   ` John Harrison
2021-06-24  7:05 ` [Intel-gfx] [PATCH 33/47] drm/i915/guc: Add disable interrupts to guc sanitize Matthew Brost
2021-07-12 20:11   ` John Harrison
2021-06-24  7:05 ` [Intel-gfx] [PATCH 34/47] drm/i915/guc: Suspend/resume implementation for new interface Matthew Brost
2021-07-12 22:56   ` John Harrison
2021-06-24  7:05 ` [Intel-gfx] [PATCH 35/47] drm/i915/guc: Handle context reset notification Matthew Brost
2021-07-12 22:58   ` John Harrison
2021-07-15  0:32     ` Matthew Brost
2021-06-24  7:05 ` [Intel-gfx] [PATCH 36/47] drm/i915/guc: Handle engine reset failure notification Matthew Brost
2021-07-12 22:59   ` John Harrison
2021-06-24  7:05 ` [Intel-gfx] [PATCH 37/47] drm/i915/guc: Enable the timer expired interrupt for GuC Matthew Brost
2021-07-12 23:00   ` John Harrison
2021-06-24  7:05 ` [Intel-gfx] [PATCH 38/47] drm/i915/guc: Provide mmio list to be saved/restored on engine reset Matthew Brost
2021-06-24  7:05 ` [Intel-gfx] [PATCH 39/47] drm/i915/guc: Don't complain about reset races Matthew Brost
2021-06-24 15:55   ` Matthew Brost
2021-06-24  7:05 ` [Intel-gfx] [PATCH 40/47] drm/i915/guc: Enable GuC engine reset Matthew Brost
2021-06-24 16:19   ` Matthew Brost
2021-06-24  7:05 ` [Intel-gfx] [PATCH 41/47] drm/i915/guc: Capture error state on context reset Matthew Brost
2021-07-12 23:05   ` John Harrison
2021-06-24  7:05 ` [Intel-gfx] [PATCH 42/47] drm/i915/guc: Fix for error capture after full GPU reset with GuC Matthew Brost
2021-07-15  0:43   ` Matthew Brost
2021-06-24  7:05 ` [Intel-gfx] [PATCH 43/47] drm/i915/guc: Hook GuC scheduling policies up Matthew Brost
2021-06-25  0:59   ` Matthew Brost
2021-06-25 19:10     ` John Harrison
2021-07-10 18:56       ` Matthew Brost
2021-06-24  7:05 ` [Intel-gfx] [PATCH 44/47] drm/i915/guc: Connect reset modparam updates to GuC policy flags Matthew Brost
2021-06-25  1:10   ` Matthew Brost
2021-06-24  7:05 ` [Intel-gfx] [PATCH 45/47] drm/i915/guc: Include scheduling policies in the debugfs state dump Matthew Brost
2021-06-24 16:34   ` Matthew Brost
2021-06-24  7:05 ` [Intel-gfx] [PATCH 46/47] drm/i915/guc: Add golden context to GuC ADS Matthew Brost
2021-06-24  7:05 ` [Intel-gfx] [PATCH 47/47] drm/i915/guc: Unblock GuC submission on Gen11+ Matthew Brost
2021-06-30  8:22   ` Martin Peres
2021-06-30 18:00     ` Matthew Brost
2021-07-01 18:24       ` Martin Peres
2021-07-02  8:13         ` Martin Peres
2021-07-02 13:06           ` Michal Wajdeczko
2021-07-02 13:12             ` Martin Peres
2021-07-02 14:08               ` Michal Wajdeczko
2021-06-30 18:58     ` John Harrison [this message]
2021-07-01  8:14       ` Pekka Paalanen
2021-07-01 18:27         ` Martin Peres
2021-07-01 19:28           ` Daniel Vetter
2021-07-02  7:29             ` Pekka Paalanen
2021-07-02  8:09               ` Martin Peres
2021-07-02 15:07                 ` Michal Wajdeczko
2021-07-03  8:21                   ` Martin Peres
2021-07-07  0:57                     ` John Harrison
2021-07-07  7:47                       ` Pekka Paalanen
2021-07-07 10:11                       ` Michal Wajdeczko
2021-07-15  0:49   ` Matthew Brost
2021-06-24  7:17 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for GuC submission support Patchwork
2021-06-24  7:19 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-06-24  7:47 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2021-07-12 19:23 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for GuC submission support (rev2) Patchwork
2021-10-22  9:35 ` [Intel-gfx] [PATCH 00/47] GuC submission support Joonas Lahtinen
2021-10-22 16:42   ` Matthew Brost
2021-10-25  9:37     ` Joonas Lahtinen
2021-10-25 15:15       ` Matthew Brost
2021-10-26  8:59         ` Joonas Lahtinen
2021-10-26 15:43           ` Matthew Brost
2021-10-26 15:51           ` Matthew Brost
2021-10-27  9:21             ` Joonas Lahtinen
2021-10-25 17:06       ` John Harrison

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=05e1d462-57ae-888a-888c-3ad486150821@intel.com \
    --to=john.c.harrison@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=martin.peres@free.fr \
    --cc=matthew.brost@intel.com \
    /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).