[Intel-gfx] [PATCH] drm/i915: initialize uncore->lock in uncore_init

[Daniele Ceraolo Spurio]

On 4/1/19 1:24 PM, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2019-04-01 21:14:11)
>> Now that all the uncore management is hidden under the uncore struct,
>> move the lock initialization under the uncore_init as well for better
>> encapsulation.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.c     | 1 -
>>   drivers/gpu/drm/i915/intel_uncore.c | 2 ++
>>   2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 0ca57dc5da5c..667177b7d821 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -873,7 +873,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
>>          spin_lock_init(&dev_priv->irq_lock);
>>          spin_lock_init(&dev_priv->gpu_error.lock);
>>          mutex_init(&dev_priv->backlight_lock);
>> -       spin_lock_init(&dev_priv->uncore.lock);
>>   
>>          mutex_init(&dev_priv->sb_lock);
>>          mutex_init(&dev_priv->av_mutex);
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index 106df24f20a5..250496f792e5 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -1530,6 +1530,8 @@ int intel_uncore_init(struct intel_uncore *uncore)
>>          struct drm_i915_private *i915 = uncore_to_i915(uncore);
>>          int ret;
>>   
>> +       spin_lock_init(&uncore->lock);
> 
> Be consistent and make an init_early(). And while you are here this
> should be called intel_uncore_init_mmio().

you mean to rename uncore_mmio_setup to intel_uncore_init_mmio, or are 
you suggesting to rename intel_uncore_init itself? I was planning to 
split out uncore_mmio_setup in the future to allow multiple uncore 
structures to be initialized separately from the mapping of the bar and 
to pull out unrelated init calls from uncore_init. Something like:

static int i915_driver_init_mmio(...)
{
	[... bridge dev init ...]

	ret = intel_uncore_mmio_setup(&dev_priv->uncore);
	if (ret < 0)
		goto err_bridge;

	i915_check_vgpu(i915);

	intel_uncore_init_mmio_access(&dev_priv->uncore);

	[... other stuff ...]
}

I was planning on sending this later once I had more code for the 
display uncore ready, but I can send it by itself to clean up the ordering.

Thanks,
Daniele

> 
> Eventually we will have the phases annotated.
> -Chris
>