Re: [PATCH] drm/i915: Partial abandonment of legacy DRM logging macros

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 08/11/2022 12:01, Jani Nikula wrote:
> On Tue, 08 Nov 2022, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Convert some usages of legacy DRM logging macros into versions which tell
>> us on which device have the events occurred.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: John Harrison <John.C.Harrison@Intel.com>
>> ---
>>   drivers/gpu/drm/i915/gem/i915_gem_context.c   |  2 +-
>>   .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 23 ++++++----
>>   .../drm/i915/gt/intel_execlists_submission.c  | 13 +++---
>>   drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c  |  4 +-
>>   drivers/gpu/drm/i915/gt/intel_gt.c            |  4 +-
>>   drivers/gpu/drm/i915/gt/intel_gt_irq.c        |  8 ++--
>>   drivers/gpu/drm/i915/gt/intel_rps.c           |  6 ++-
>>   drivers/gpu/drm/i915/gt/intel_workarounds.c   | 43 +++++++++++--------
>>   .../gpu/drm/i915/gt/intel_workarounds_types.h |  4 ++
>>   .../gpu/drm/i915/gt/selftest_workarounds.c    |  4 +-
>>   drivers/gpu/drm/i915/i915_debugfs.c           |  4 +-
>>   drivers/gpu/drm/i915/i915_gem.c               |  2 +-
>>   drivers/gpu/drm/i915/i915_getparam.c          |  2 +-
>>   drivers/gpu/drm/i915/i915_irq.c               | 12 +++---
>>   drivers/gpu/drm/i915/i915_perf.c              | 14 +++---
>>   drivers/gpu/drm/i915/i915_query.c             | 12 +++---
>>   drivers/gpu/drm/i915/i915_sysfs.c             |  3 +-
>>   drivers/gpu/drm/i915/i915_vma.c               | 16 ++++---
>>   drivers/gpu/drm/i915/intel_uncore.c           | 21 +++++----
>>   19 files changed, 116 insertions(+), 81 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>> index 01402f3c58f6..7f2831efc798 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>> @@ -546,7 +546,7 @@ set_proto_ctx_engines_bond(struct i915_user_extension __user *base, void *data)
>>   	}
>>   
>>   	if (intel_engine_uses_guc(master)) {
>> -		DRM_DEBUG("bonding extension not supported with GuC submission");
>> +		drm_dbg(&i915->drm, "bonding extension not supported with GuC submission");
>>   		return -ENODEV;
>>   	}
>>   
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> index 1160723c9d2d..1eb7b66191b2 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> @@ -2148,7 +2148,8 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
>>   	return err;
>>   }
>>   
>> -static int i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec)
>> +static int i915_gem_check_execbuffer(struct drm_i915_private *i915,
>> +				     struct drm_i915_gem_execbuffer2 *exec)
>>   {
>>   	if (exec->flags & __I915_EXEC_ILLEGAL_FLAGS)
>>   		return -EINVAL;
>> @@ -2161,7 +2162,7 @@ static int i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec)
>>   	}
>>   
>>   	if (exec->DR4 == 0xffffffff) {
>> -		DRM_DEBUG("UXA submitting garbage DR4, fixing up\n");
>> +		drm_dbg(&i915->drm, "UXA submitting garbage DR4, fixing up\n");
>>   		exec->DR4 = 0;
>>   	}
>>   	if (exec->DR1 || exec->DR4)
>> @@ -2744,6 +2745,7 @@ add_timeline_fence_array(struct i915_execbuffer *eb,
>>   			 const struct drm_i915_gem_execbuffer_ext_timeline_fences *timeline_fences)
>>   {
>>   	struct drm_i915_gem_exec_fence __user *user_fences;
>> +	struct drm_device *drm = &eb->i915->drm;
> 
> Elsewhere we've been pretty strict about not adding struct drm_device as
> a local variable, just struct drm_i915_private *i915. We don't want to
> have both, and in general it's more likely i915 is needed than
> drm_device, if not now then in the future. Even if it means having to
> use &i915->drm here.

Yeah it smelled bad while I was typing it.. will change.

Regards,

Tvrtko