[PATCH v2] drm/amd/amdgpu: cleanup coding style a bit

Ruhl, Michael J michael.j.ruhl at intel.com
Fri May 8 15:45:07 UTC 2020


>-----Original Message-----
>From: dri-devel <dri-devel-bounces at lists.freedesktop.org> On Behalf Of
>Bernard Zhao
>Sent: Thursday, May 7, 2020 5:13 AM
>To: Alex Deucher <alexander.deucher at amd.com>; Christian König
><christian.koenig at amd.com>; David (ChunMing) Zhou
><David1.Zhou at amd.com>; David Airlie <airlied at linux.ie>; Daniel Vetter
><daniel at ffwll.ch>; Tom St Denis <tom.stdenis at amd.com>; Sam Ravnborg
><sam at ravnborg.org>; Ori Messinger <Ori.Messinger at amd.com>; Bernard
>Zhao <bernard at vivo.com>; amd-gfx at lists.freedesktop.org; dri-
>devel at lists.freedesktop.org; linux-kernel at vger.kernel.org
>Cc: opensource.kernel at vivo.com
>Subject: [PATCH v2] drm/amd/amdgpu: cleanup coding style a bit
>
>There is DEVICE_ATTR mechanism in separate attribute define.
>So this change is to use attr array, also use
>sysfs_create_files in init function & sysfs_remove_files in
>fini function.
>This maybe make the code a bit readable.
>
>Signed-off-by: Bernard Zhao <bernard at vivo.com>
>
>Changes since V1:
>*Use DEVICE_ATTR mechanism
>
>Link for V1:
>*https://lore.kernel.org/patchwork/patch/1228076/
>---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 43 ++++++-------------
>-
> 1 file changed, 13 insertions(+), 30 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>index 82a3299e53c0..57bbc70662ff 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>@@ -148,6 +148,15 @@ static DEVICE_ATTR(mem_info_vis_vram_used,
>S_IRUGO,
> static DEVICE_ATTR(mem_info_vram_vendor, S_IRUGO,
> 		   amdgpu_mem_info_vram_vendor, NULL);
>
>+static struct attribute *amdgpu_vram_mgr_attributes[] = {
>+	&dev_attr_mem_info_vram_total.attr,
>+	&dev_attr_mem_info_vis_vram_total.attr,
>+	&dev_attr_mem_info_vram_used.attr,
>+	&dev_attr_mem_info_vis_vram_used.attr,
>+	&dev_attr_mem_info_vram_vendor.attr,
>+	NULL
>+};
>+
> /**
>  * amdgpu_vram_mgr_init - init VRAM manager and DRM MM
>  *
>@@ -172,31 +181,9 @@ static int amdgpu_vram_mgr_init(struct
>ttm_mem_type_manager *man,
> 	man->priv = mgr;
>
> 	/* Add the two VRAM-related sysfs files */
>-	ret = device_create_file(adev->dev,
>&dev_attr_mem_info_vram_total);
>-	if (ret) {
>-		DRM_ERROR("Failed to create device file
>mem_info_vram_total\n");
>-		return ret;
>-	}
>-	ret = device_create_file(adev->dev,
>&dev_attr_mem_info_vis_vram_total);
>-	if (ret) {
>-		DRM_ERROR("Failed to create device file
>mem_info_vis_vram_total\n");
>-		return ret;
>-	}
>-	ret = device_create_file(adev->dev,
>&dev_attr_mem_info_vram_used);
>-	if (ret) {
>-		DRM_ERROR("Failed to create device file
>mem_info_vram_used\n");
>-		return ret;
>-	}
>-	ret = device_create_file(adev->dev,
>&dev_attr_mem_info_vis_vram_used);
>-	if (ret) {
>-		DRM_ERROR("Failed to create device file
>mem_info_vis_vram_used\n");
>-		return ret;
>-	}
>-	ret = device_create_file(adev->dev,
>&dev_attr_mem_info_vram_vendor);
>-	if (ret) {
>-		DRM_ERROR("Failed to create device file
>mem_info_vram_vendor\n");
>-		return ret;
>-	}
>+	ret = sysfs_create_files(&adev->dev->kobj,
>amdgpu_vram_mgr_attributes);
>+	if (ret)
>+		DRM_ERROR("Failed to register sysfs\n");

This looks good to me.

I think that there is a new error macro (drm_err?) that you might
want to use instead of DRM_ERROR().

Otherwise:

Acked-by: Michael J. Ruhl <michael.j.ruhl at intel.com>

m

>
> 	return 0;
> }
>@@ -219,11 +206,7 @@ static int amdgpu_vram_mgr_fini(struct
>ttm_mem_type_manager *man)
> 	spin_unlock(&mgr->lock);
> 	kfree(mgr);
> 	man->priv = NULL;
>-	device_remove_file(adev->dev, &dev_attr_mem_info_vram_total);
>-	device_remove_file(adev->dev,
>&dev_attr_mem_info_vis_vram_total);
>-	device_remove_file(adev->dev, &dev_attr_mem_info_vram_used);
>-	device_remove_file(adev->dev,
>&dev_attr_mem_info_vis_vram_used);
>-	device_remove_file(adev->dev,
>&dev_attr_mem_info_vram_vendor);
>+	sysfs_remove_files(&adev->dev->kobj,
>amdgpu_vram_mgr_attributes);
> 	return 0;
> }
>
>--
>2.26.2
>
>_______________________________________________
>dri-devel mailing list
>dri-devel at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel


More information about the dri-devel mailing list