[Intel-gfx] [PATCH 47/47] drm/i915/guc: Unblock GuC submission on Gen11+

[Pekka Paalanen]

On Tue, 6 Jul 2021 17:57:35 -0700
John Harrison <john.c.harrison at intel.com> wrote:

> 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 at ffwll.ch> wrote:
> >>>>  
> >>>>> On Thu, Jul 1, 2021 at 8:27 PM Martin Peres <martin.peres at free.fr>
> >>>>> wrote:  
> >>>>>>
> >>>>>> On 01/07/2021 11:14, Pekka Paalanen wrote:  
> >>>>>>> On Wed, 30 Jun 2021 11:58:25 -0700
> >>>>>>> John Harrison <john.c.harrison at intel.com> 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 at intel.com>
> >>>>>>>>>>
> >>>>>>>>>> Unblock GuC submission on Gen11+ platforms.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> >>>>>>>>>> Signed-off-by: Daniele Ceraolo Spurio
> >>>>>>>>>> <daniele.ceraolospurio at intel.com>
> >>>>>>>>>> Signed-off-by: Matthew Brost <matthew.brost at 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_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.

Hi,

it seems to me that "untested" was just a wrong guess, nothing more. It
was presented with "how about?", not as an exact demand.

You don't have to attack that word, just use another phrasing that is
both correct and not misleading to the majority of tech savvy people.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20210707/aac0591d/attachment.sig>