> @@ -880,17 +881,22 @@ int svm_migrate_init(struct amdgpu_device *adev)
>        * should remove reserved size
>        */
>       size = ALIGN(adev->gmc.real_vram_size, 2ULL << 20);
> -     res = devm_request_free_mem_region(adev->dev, &iomem_resource, size);
> +     if (xgmi_connected_to_cpu)
> +             res = lookup_resource(&iomem_resource, adev->gmc.aper_base);
> +     else
> +             res = devm_request_free_mem_region(adev->dev, &iomem_resource, 
> size);
> +

Can you explain what the point of the lookup_resource is here? res->start
is obviously identical to the start value you pass in.  So this is used
as a way to query the length, but I'm pretty sure the driver must
already know that as it inserted the resource itself, right?

On a slightly higher level comment svm_migrate_init is a bit of a mess
with all the if/else already, and with the above addressed will become
a bit more.  I think splitting it into a device private and device
generic case would probably help people finding it to understand the code
much better later on.  Even more so with a useful comment.

Reply via email to