[Intel-gfx] [PATCH 3/8] drm/i915/pcode: Extend pcode functions for multiple gt's

[Tvrtko Ursulin]

On 11/05/2022 06:26, Dixit, Ashutosh wrote:
> On Tue, 10 May 2022 00:43:29 -0700, Jani Nikula wrote:
>> On Tue, 10 May 2022, Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com> wrote:
>>> On 29/04/2022 20:56, Ashutosh Dixit wrote:
>>>> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>>>> index 90b0ce5051af..bc49eff38c6a 100644
>>>> --- a/drivers/gpu/drm/i915/i915_driver.c
>>>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>>>> @@ -520,6 +520,22 @@ static int i915_set_dma_info(struct drm_i915_private *i915)
>>>> 	return ret;
>>>>    }
>>>>
>>>> +static int i915_pcode_init(struct drm_i915_private *i915)
>>>> +{
>>>> +	struct intel_gt *gt;
>>>> +	int id, ret;
>>>> +
>>>> +	for_each_gt(gt, i915, id) {
>>>> +		ret = intel_pcode_init(gt->uncore);
>>>> +		if (ret) {
>>>> +			drm_err(&gt->i915->drm, "gt %d: intel_pcode_init failed %d\n", id, ret);
>>>
>>> A few nits..
>>>
>>> 1) All other/current logs use "gt%d" (no space).
>>>
>>> 2) intel_pcode_init also logs a drm_err - do we need two? I suggest
>>> leaving this one only since it has more information.
>>>
>>> 3) It would have been nicer to have refactoring of intel_pcode_ to work
>>> on uncore separate from adding for_each_gt.
>>
>> Yeah.
>>
>> Also the obvious first patch would've been to convert intel_pcode.c
>> functions from struct drm_i915_private * to intel_uncore *.
> 
> Will fix up the first 2 points but about this last point, to not break
> incremental compile all callers of the pcode functions also need to be
> converted to i915->uncore or gt->uncore (so it's not possible to convert
> just intel_pcode.c functions without also converting all callers, if that
> was the intent of this comment, unless I am missing something).

Yes the implication is to convert the callers when doing such 
conversion, we never do broken commits.

> But yes the i915_pcode_init() above can be separated out to a separate
> patch so I can do that.

AFAIR that will achieve what is suggested, thanks!

Regards,

Tvrtko