Re: [Intel-gfx] [PATCH v2 4/6] drm/i915/debugfs: move uC printers and update debugfs file names

From: John Harrison <John.C.Harrison@Intel.com>
To: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 4/6] drm/i915/debugfs: move uC printers and update debugfs file names
Date: Wed, 25 Mar 2020 10:35:43 -0700	[thread overview]
Message-ID: <c90f5adb-f527-728c-49f8-75ad54a7d48e@Intel.com> (raw)
In-Reply-To: <83af4518-a3cb-5066-7eff-cf1f2dacf3ab@intel.com>

On 3/25/2020 10:14, Daniele Ceraolo Spurio wrote:
> On 3/25/20 10:05 AM, John Harrison wrote:
>> On 3/11/2020 18:16, Daniele Ceraolo Spurio wrote:
>>> Move the printers to the respective files for clarity. The
>>> guc_load_status debugfs has been squashed in the guc_info one, has
>>> having separate ones wasn't very useful. The HuC debugfs has been
>>> renamed huc_info to match.
>>>
>>> v2: keep printing HUC_STATUS2 (Tony), avoid const->non-const
>>>      container_of (Jani)
>>>
>>> Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Cc: John Harrison <John.C.Harrison@Intel.com>
>>> Cc: Matthew Brost <matthew.brost@intel.com>
>>> Cc: Tony Ye <tony.ye@intel.com>
>>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>>> ---
>>> <snip>
>>>
>>> +static int i915_huc_info(struct seq_file *m, void *data)
>>>   {
>>>       struct drm_i915_private *dev_priv = node_to_i915(m->private);
>>> -    intel_wakeref_t wakeref;
>>> -    struct drm_printer p;
>>> -
>>> -    if (!HAS_GT_UC(dev_priv))
>>> -        return -ENODEV;
>>> -
>>> -    p = drm_seq_file_printer(m);
>>> -    intel_uc_fw_dump(&dev_priv->gt.uc.huc.fw, &p);
>>> -
>>> -    with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref)
>>> -        seq_printf(m, "\nHuC status 0x%08x:\n", 
>>> I915_READ(HUC_STATUS2));
>>> -
>>> -    return 0;
>>> -}
>>> -
>>> -static int i915_guc_load_status_info(struct seq_file *m, void *data)
>>> -{
>>> -    struct drm_i915_private *dev_priv = node_to_i915(m->private);
>>> -    intel_wakeref_t wakeref;
>>> -    struct drm_printer p;
>>> +    struct intel_huc *huc = &dev_priv->gt.uc.huc;
>>> +    struct drm_printer p = drm_seq_file_printer(m);
>>> -    if (!HAS_GT_UC(dev_priv))
>>> +    if (!intel_huc_is_supported(huc))
>>>           return -ENODEV;
>> Isn't this test duplicated inside intel_huc_load_status() with a 
>> print of 'HuC not supported'? So no need to fail the call here?
>>
>
> intel_huc_load_status is now a generic printer which can be called 
> from other places, so it needs to print useful messages in all cases. 
> From the debugfs POV, I didn't want to change the legacy behavior of 
> returning -ENODEV on platforms that don't support the blob, which IMO 
> is a clear eough indication of the lack of support. Note that in the 
> next patch the code is changed so that the debgufs files are not even 
> created if there is no uC support on the platforms (this is in line 
> with what we do for other GT features).
>
> Daniele
>
Okay. That makes sense.

Reviewed-by: John Harrison <John.C.Harrison@Intel.com>

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