On 7/3/2021 01:21, Martin Peres wrote:
On 02/07/2021 18:07, Michal Wajdeczko wrote:
On 02.07.2021 10:09, Martin Peres wrote:
On 02/07/2021 10:29, Pekka Paalanen wrote:
On Thu, 1 Jul 2021 21:28:06 +0200
Daniel Vetter <daniel@xxxxxxxx> wrote:
On Thu, Jul 1, 2021 at 8:27 PM Martin Peres <martin.peres@xxxxxxx>
wrote:
On 01/07/2021 11:14, Pekka Paalanen wrote:
On Wed, 30 Jun 2021 11:58:25 -0700
John Harrison <john.c.harrison@xxxxxxxxx> wrote:
On 6/30/2021 01:22, Martin Peres wrote:
On 24/06/2021 10:05, Matthew Brost wrote:
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Unblock GuC submission on Gen11+ platforms.
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Signed-off-by: Daniele Ceraolo Spurio
<daniele.ceraolospurio@xxxxxxxxx>
Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx>
---
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_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.
That is a very good reason to not use terminology like "new",
"old",
"current", "modern" etc. at all.
End users like me definitely do not share your interpretation of
"old".
Yep, old and new is relative. In the end, what matters is the
validation
effort, which is why I was proposing "untested platforms".
Also, remember that you are not writing these messages for Intel
engineers, but instead are writing for Linux *users*.
It's drm_dbg. Users don't read this stuff, at least not users with no
clue what the driver does and stuff like that.
If I had a problem, I would read it, and I have no clue what anything
of that is.
Exactly.
I don't see how replacing 'old' for 'untested' helps anybody to
understand anything. Untested just implies we can't be bothered to test
stuff before publishing it. And as previously stated, this is purely a
political decision not a technical one. Sure, change the message to be
'Disabling GuC submission but enabling HuC loading via GuC on platform
XXX' if that makes it clearer what is going on. Or just drop the message
completely. It's simply explaining what the default option is for the
current platform which you can also get by reading the code. However, I
disagree that 'untested' is the correct message. Quite a lot of testing
has been happening on TGL+ with GuC submission enabled.
This level of defense for what is clearly a bad *debug* message (at the
very least, the grammar) makes no sense at all!
I don't want to hear arguments like "Not my patch" from a developer
literally sending the patch to the ML and who added his SoB to the
patch, playing with words, or minimizing the problem of having such a
message.
Agree that 'not my patch' is never a good excuse, but equally we can't
blame original patch author as patch was updated few times since then.
I never wanted to blame the author here, I was only speaking about the
handling of feedback on the patch.
Maybe to avoid confusions and simplify reviews, we could split this
patch into two smaller: first one that really unblocks GuC submission on
all Gen11+ (see __guc_submission_supported) and second one that updates
defaults for Gen12+ (see uc_expand_default_options), as original patch
(from ~2019) evolved more than what title/commit message says.
Both work for me, as long as it is a collaborative effort.
I'm not seeing how splitting the patch up fixes the complaints about the
debug message.
And to be clear, no-one is actually arguing for a code change as such?
The issue is just about the text of the debug message? Or did I miss
something somewhere?
John.
Cheers,
Martin
Then we can fix all messaging and make sure it's clear and understood.
Thanks,
Michal
All of the above are just clear signals for the community to get off
your playground, which is frankly unacceptable. Your email address does
not matter.
In the spirit of collaboration, your response should have been "Good
catch, how about XXXX or YYYY?". This would not have wasted everyone's
time in an attempt to just have it your way.
My level of confidence in this GuC transition was already low, but you
guys are working hard to shoot yourself in the foot. Trust should be
earned!
Martin
Thanks,
pq
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
