[Intel-gfx] [PATCH v2 01/10] drm/i915/debugfs: Pass guc_log struct to i915_guc_log_info

[Daniele Ceraolo Spurio]

On 2/4/20 11:58 PM, Jani Nikula wrote:
> On Tue, 04 Feb 2020, "Michal Wajdeczko" <michal.wajdeczko at intel.com> wrote:
>> On Tue, 04 Feb 2020 00:28:29 +0100, Daniele Ceraolo Spurio
>> <daniele.ceraolospurio at intel.com> wrote:
>>
>>> The log struct is the only thing the function needs (apart from
>>> the seq_file), so we can pass just that instead of the whole dev_priv.
>>>
>>> v2: Split this change to its own patch (Michal)
>>>
>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>>> Cc: John Harrison <John.C.Harrison at Intel.com>
>>> Cc: Matthew Brost <matthew.brost at intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_debugfs.c | 6 ++----
>>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>>> b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index e75e8212f03b..7264c0ff766c 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -1753,10 +1753,8 @@ stringify_guc_log_type(enum guc_log_buffer_type
>>> type)
>>>   	return "";
>>>   }
>>> -static void i915_guc_log_info(struct seq_file *m,
>>> -			      struct drm_i915_private *dev_priv)
>>> +static void i915_guc_log_info(struct seq_file *m, struct intel_guc_log
>>> *log)
>>
>> maybe to avoid polluting i915_debugfs.c we should move this function to
>> gt/uc/intel_guc_log.c as universal printer:
> 
> On that note, I'm going to split display bits from i915_debugfs.c to a
> file of its own under display/ [1]. I think there's enough guc/huc stuff
> to warrant moving them to a separate file maybe under gt/uc/.

Andi is already looking at moving all the GT bits under gt/, so the uc 
functions will move as part of that.

Daniele

> 
> 
> BR,
> Jani.
> 
> [1] http://patchwork.freedesktop.org/patch/msgid/20200204151810.8189-1-jani.nikula@intel.com
> 
> 
>>
>> void intel_guc_log_info(struct intel_guc_log *log, struct drm_printer *p)
>>
>>>   {
>>> -	struct intel_guc_log *log = &dev_priv->gt.uc.guc.log;
>>>   	enum guc_log_buffer_type type;
>>> 	if (!intel_guc_log_relay_created(log)) {
>>> @@ -1784,7 +1782,7 @@ static int i915_guc_info(struct seq_file *m, void
>>> *data)
>>>   	if (!USES_GUC(dev_priv))
>>>   		return -ENODEV;
>>> -	i915_guc_log_info(m, dev_priv);
>>> +	i915_guc_log_info(m, &dev_priv->gt.uc.guc.log);
>>
>> too many dots ... this is "guc" info function, maybe we should have:
>>
>> 	struct intel_guc *guc = &dev_priv->gt.uc.guc;
>> or
>> 	struct intel_gt *gt = &dev_priv->gt;
>> 	struct intel_uc *uc = &gt->uc;
>> 	struct intel_guc *guc = &uc->guc;
>>
>> as that "guc" is likely to be reused in "more" below
>>
>>> 	/* Add more as required ... */
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>