All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: Move VBIOS version to sysfs
@ 2017-08-25 13:43 Kent Russell
       [not found] ` <1503668585-27962-1-git-send-email-kent.russell-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Kent Russell @ 2017-08-25 13:43 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Kent Russell

sysfs is more stable, and doesn't require root to access

Signed-off-by: Kent Russell <kent.russell@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 54 +++++++++++++-----------------
 1 file changed, 23 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index c041496..77929e4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -67,7 +67,6 @@ static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev);
 static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev);
 static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev);
 static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev);
-static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device *adev);
 
 static const char *amdgpu_asic_name[] = {
 	"TAHITI",
@@ -890,6 +889,20 @@ static uint32_t cail_ioreg_read(struct card_info *info, uint32_t reg)
 	return r;
 }
 
+static ssize_t amdgpu_atombios_get_vbios_version(struct device *dev,
+						 struct device_attribute *attr,
+						 char *buf)
+{
+	struct drm_device *ddev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = ddev->dev_private;
+	struct atom_context *ctx = adev->mode_info.atom_context;
+
+	return snprintf(buf, PAGE_SIZE, "%s\n", ctx->vbios_version);
+}
+
+static DEVICE_ATTR(vbios_version, 0444, amdgpu_atombios_get_vbios_version,
+		   NULL);
+
 /**
  * amdgpu_atombios_fini - free the driver info and callbacks for atombios
  *
@@ -909,6 +922,7 @@ static void amdgpu_atombios_fini(struct amdgpu_device *adev)
 	adev->mode_info.atom_context = NULL;
 	kfree(adev->mode_info.atom_card_info);
 	adev->mode_info.atom_card_info = NULL;
+	device_remove_file(adev->dev, &dev_attr_vbios_version);
 }
 
 /**
@@ -925,6 +939,7 @@ static int amdgpu_atombios_init(struct amdgpu_device *adev)
 {
 	struct card_info *atom_card_info =
 	    kzalloc(sizeof(struct card_info), GFP_KERNEL);
+	int ret;
 
 	if (!atom_card_info)
 		return -ENOMEM;
@@ -961,6 +976,13 @@ static int amdgpu_atombios_init(struct amdgpu_device *adev)
 		amdgpu_atombios_scratch_regs_init(adev);
 		amdgpu_atombios_allocate_fb_scratch(adev);
 	}
+
+	ret = device_create_file(adev->dev, &dev_attr_vbios_version);
+	if (ret) {
+		DRM_ERROR("Failed to create device file for VBIOS version\n");
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -2256,10 +2278,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	if (r)
 		DRM_ERROR("Creating vbios dump debugfs failed (%d).\n", r);
 
-	r = amdgpu_debugfs_vbios_version_init(adev);
-	if (r)
-		DRM_ERROR("Creating vbios version debugfs failed (%d).\n", r);
-
 	if ((amdgpu_testing & 1)) {
 		if (adev->accel_working)
 			amdgpu_test_moves(adev);
@@ -3851,39 +3869,17 @@ static int amdgpu_debugfs_get_vbios_dump(struct seq_file *m, void *data)
 	return 0;
 }
 
-static int amdgpu_debugfs_get_vbios_version(struct seq_file *m, void *data)
-{
-	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct drm_device *dev = node->minor->dev;
-	struct amdgpu_device *adev = dev->dev_private;
-	struct atom_context *ctx = adev->mode_info.atom_context;
-
-	seq_printf(m, "%s\n", ctx->vbios_version);
-	return 0;
-}
-
 static const struct drm_info_list amdgpu_vbios_dump_list[] = {
 		{"amdgpu_vbios",
 		 amdgpu_debugfs_get_vbios_dump,
 		 0, NULL},
 };
 
-static const struct drm_info_list amdgpu_vbios_version_list[] = {
-		{"amdgpu_vbios_version",
-		 amdgpu_debugfs_get_vbios_version,
-		 0, NULL},
-};
-
 static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)
 {
 	return amdgpu_debugfs_add_files(adev,
 					amdgpu_vbios_dump_list, 1);
 }
-static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device *adev)
-{
-	return amdgpu_debugfs_add_files(adev,
-					amdgpu_vbios_version_list, 1);
-}
 #else
 static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev)
 {
@@ -3897,9 +3893,5 @@ static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)
 {
 	return 0;
 }
-static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device *adev)
-{
-	return 0;
-}
 static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev) { }
 #endif
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 11+ messages in thread
* Re: [PATCH] drm/amdgpu: Move VBIOS version to sysfs
       [not found] ` <1503668585-27962-1-git-send-email-kent.russell-5C7GfCeVMHo@public.gmane.org>
@ 2017-08-25 13:59   ` Tom St Denis
       [not found]     ` <60b4865c-9989-8a20-b200-050e6ff8a441-5C7GfCeVMHo@public.gmane.org>
  2017-08-25 14:41   ` Alex Deucher
  1 sibling, 1 reply; 11+ messages in thread
From: Tom St Denis @ 2017-08-25 13:59 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Acked-by: Tom St Denis <tom.stdenis@amd.com>

Seems to be the easiest way to find this is via

/sys/bus/pci/devices/${devname}/vbios_version

Tom

On 25/08/17 09:43 AM, Kent Russell wrote:
> sysfs is more stable, and doesn't require root to access
> 
> Signed-off-by: Kent Russell <kent.russell@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 54 +++++++++++++-----------------
>   1 file changed, 23 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index c041496..77929e4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -67,7 +67,6 @@ static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev);
>   static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev);
>   static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev);
>   static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev);
> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device *adev);
>   
>   static const char *amdgpu_asic_name[] = {
>   	"TAHITI",
> @@ -890,6 +889,20 @@ static uint32_t cail_ioreg_read(struct card_info *info, uint32_t reg)
>   	return r;
>   }
>   
> +static ssize_t amdgpu_atombios_get_vbios_version(struct device *dev,
> +						 struct device_attribute *attr,
> +						 char *buf)
> +{
> +	struct drm_device *ddev = dev_get_drvdata(dev);
> +	struct amdgpu_device *adev = ddev->dev_private;
> +	struct atom_context *ctx = adev->mode_info.atom_context;
> +
> +	return snprintf(buf, PAGE_SIZE, "%s\n", ctx->vbios_version);
> +}
> +
> +static DEVICE_ATTR(vbios_version, 0444, amdgpu_atombios_get_vbios_version,
> +		   NULL);
> +
>   /**
>    * amdgpu_atombios_fini - free the driver info and callbacks for atombios
>    *
> @@ -909,6 +922,7 @@ static void amdgpu_atombios_fini(struct amdgpu_device *adev)
>   	adev->mode_info.atom_context = NULL;
>   	kfree(adev->mode_info.atom_card_info);
>   	adev->mode_info.atom_card_info = NULL;
> +	device_remove_file(adev->dev, &dev_attr_vbios_version);
>   }
>   
>   /**
> @@ -925,6 +939,7 @@ static int amdgpu_atombios_init(struct amdgpu_device *adev)
>   {
>   	struct card_info *atom_card_info =
>   	    kzalloc(sizeof(struct card_info), GFP_KERNEL);
> +	int ret;
>   
>   	if (!atom_card_info)
>   		return -ENOMEM;
> @@ -961,6 +976,13 @@ static int amdgpu_atombios_init(struct amdgpu_device *adev)
>   		amdgpu_atombios_scratch_regs_init(adev);
>   		amdgpu_atombios_allocate_fb_scratch(adev);
>   	}
> +
> +	ret = device_create_file(adev->dev, &dev_attr_vbios_version);
> +	if (ret) {
> +		DRM_ERROR("Failed to create device file for VBIOS version\n");
> +		return ret;
> +	}
> +
>   	return 0;
>   }
>   
> @@ -2256,10 +2278,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>   	if (r)
>   		DRM_ERROR("Creating vbios dump debugfs failed (%d).\n", r);
>   
> -	r = amdgpu_debugfs_vbios_version_init(adev);
> -	if (r)
> -		DRM_ERROR("Creating vbios version debugfs failed (%d).\n", r);
> -
>   	if ((amdgpu_testing & 1)) {
>   		if (adev->accel_working)
>   			amdgpu_test_moves(adev);
> @@ -3851,39 +3869,17 @@ static int amdgpu_debugfs_get_vbios_dump(struct seq_file *m, void *data)
>   	return 0;
>   }
>   
> -static int amdgpu_debugfs_get_vbios_version(struct seq_file *m, void *data)
> -{
> -	struct drm_info_node *node = (struct drm_info_node *) m->private;
> -	struct drm_device *dev = node->minor->dev;
> -	struct amdgpu_device *adev = dev->dev_private;
> -	struct atom_context *ctx = adev->mode_info.atom_context;
> -
> -	seq_printf(m, "%s\n", ctx->vbios_version);
> -	return 0;
> -}
> -
>   static const struct drm_info_list amdgpu_vbios_dump_list[] = {
>   		{"amdgpu_vbios",
>   		 amdgpu_debugfs_get_vbios_dump,
>   		 0, NULL},
>   };
>   
> -static const struct drm_info_list amdgpu_vbios_version_list[] = {
> -		{"amdgpu_vbios_version",
> -		 amdgpu_debugfs_get_vbios_version,
> -		 0, NULL},
> -};
> -
>   static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)
>   {
>   	return amdgpu_debugfs_add_files(adev,
>   					amdgpu_vbios_dump_list, 1);
>   }
> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device *adev)
> -{
> -	return amdgpu_debugfs_add_files(adev,
> -					amdgpu_vbios_version_list, 1);
> -}
>   #else
>   static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev)
>   {
> @@ -3897,9 +3893,5 @@ static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)
>   {
>   	return 0;
>   }
> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device *adev)
> -{
> -	return 0;
> -}
>   static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev) { }
>   #endif
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH] drm/amdgpu: Move VBIOS version to sysfs
       [not found] ` <1503668585-27962-1-git-send-email-kent.russell-5C7GfCeVMHo@public.gmane.org>
  2017-08-25 13:59   ` Tom St Denis
@ 2017-08-25 14:41   ` Alex Deucher
       [not found]     ` <CADnq5_MYCTT73r0X_=JCtTVkYb-2_oK1LQQb85-dQk3V-wud0Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  1 sibling, 1 reply; 11+ messages in thread
From: Alex Deucher @ 2017-08-25 14:41 UTC (permalink / raw)
  To: Kent Russell; +Cc: amd-gfx list

On Fri, Aug 25, 2017 at 9:43 AM, Kent Russell <kent.russell@amd.com> wrote:
> sysfs is more stable, and doesn't require root to access
>
> Signed-off-by: Kent Russell <kent.russell@amd.com>

Might as well move the vbios binary itself to sysfs as well.  It
should be a stable interface :)

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 54 +++++++++++++-----------------
>  1 file changed, 23 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index c041496..77929e4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -67,7 +67,6 @@ static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev);
>  static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev);
>  static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev);
>  static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev);
> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device *adev);
>
>  static const char *amdgpu_asic_name[] = {
>         "TAHITI",
> @@ -890,6 +889,20 @@ static uint32_t cail_ioreg_read(struct card_info *info, uint32_t reg)
>         return r;
>  }
>
> +static ssize_t amdgpu_atombios_get_vbios_version(struct device *dev,
> +                                                struct device_attribute *attr,
> +                                                char *buf)
> +{
> +       struct drm_device *ddev = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = ddev->dev_private;
> +       struct atom_context *ctx = adev->mode_info.atom_context;
> +
> +       return snprintf(buf, PAGE_SIZE, "%s\n", ctx->vbios_version);
> +}
> +
> +static DEVICE_ATTR(vbios_version, 0444, amdgpu_atombios_get_vbios_version,
> +                  NULL);
> +
>  /**
>   * amdgpu_atombios_fini - free the driver info and callbacks for atombios
>   *
> @@ -909,6 +922,7 @@ static void amdgpu_atombios_fini(struct amdgpu_device *adev)
>         adev->mode_info.atom_context = NULL;
>         kfree(adev->mode_info.atom_card_info);
>         adev->mode_info.atom_card_info = NULL;
> +       device_remove_file(adev->dev, &dev_attr_vbios_version);
>  }
>
>  /**
> @@ -925,6 +939,7 @@ static int amdgpu_atombios_init(struct amdgpu_device *adev)
>  {
>         struct card_info *atom_card_info =
>             kzalloc(sizeof(struct card_info), GFP_KERNEL);
> +       int ret;
>
>         if (!atom_card_info)
>                 return -ENOMEM;
> @@ -961,6 +976,13 @@ static int amdgpu_atombios_init(struct amdgpu_device *adev)
>                 amdgpu_atombios_scratch_regs_init(adev);
>                 amdgpu_atombios_allocate_fb_scratch(adev);
>         }
> +
> +       ret = device_create_file(adev->dev, &dev_attr_vbios_version);
> +       if (ret) {
> +               DRM_ERROR("Failed to create device file for VBIOS version\n");
> +               return ret;
> +       }
> +
>         return 0;
>  }
>
> @@ -2256,10 +2278,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>         if (r)
>                 DRM_ERROR("Creating vbios dump debugfs failed (%d).\n", r);
>
> -       r = amdgpu_debugfs_vbios_version_init(adev);
> -       if (r)
> -               DRM_ERROR("Creating vbios version debugfs failed (%d).\n", r);
> -
>         if ((amdgpu_testing & 1)) {
>                 if (adev->accel_working)
>                         amdgpu_test_moves(adev);
> @@ -3851,39 +3869,17 @@ static int amdgpu_debugfs_get_vbios_dump(struct seq_file *m, void *data)
>         return 0;
>  }
>
> -static int amdgpu_debugfs_get_vbios_version(struct seq_file *m, void *data)
> -{
> -       struct drm_info_node *node = (struct drm_info_node *) m->private;
> -       struct drm_device *dev = node->minor->dev;
> -       struct amdgpu_device *adev = dev->dev_private;
> -       struct atom_context *ctx = adev->mode_info.atom_context;
> -
> -       seq_printf(m, "%s\n", ctx->vbios_version);
> -       return 0;
> -}
> -
>  static const struct drm_info_list amdgpu_vbios_dump_list[] = {
>                 {"amdgpu_vbios",
>                  amdgpu_debugfs_get_vbios_dump,
>                  0, NULL},
>  };
>
> -static const struct drm_info_list amdgpu_vbios_version_list[] = {
> -               {"amdgpu_vbios_version",
> -                amdgpu_debugfs_get_vbios_version,
> -                0, NULL},
> -};
> -
>  static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)
>  {
>         return amdgpu_debugfs_add_files(adev,
>                                         amdgpu_vbios_dump_list, 1);
>  }
> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device *adev)
> -{
> -       return amdgpu_debugfs_add_files(adev,
> -                                       amdgpu_vbios_version_list, 1);
> -}
>  #else
>  static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev)
>  {
> @@ -3897,9 +3893,5 @@ static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)
>  {
>         return 0;
>  }
> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device *adev)
> -{
> -       return 0;
> -}
>  static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev) { }
>  #endif
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread
* RE: [PATCH] drm/amdgpu: Move VBIOS version to sysfs
       [not found]     ` <60b4865c-9989-8a20-b200-050e6ff8a441-5C7GfCeVMHo@public.gmane.org>
@ 2017-08-25 15:36       ` Russell, Kent
  0 siblings, 0 replies; 11+ messages in thread
From: Russell, Kent @ 2017-08-25 15:36 UTC (permalink / raw)
  To: StDenis, Tom, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

That's right, it'll be in there, or the sym-linked /sys/class/drm/card0/device , depending on how you like to find the amdgpu device sysfs files.

 Kent

-----Original Message-----
From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of Tom St Denis
Sent: Friday, August 25, 2017 9:59 AM
To: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: Move VBIOS version to sysfs

Acked-by: Tom St Denis <tom.stdenis@amd.com>

Seems to be the easiest way to find this is via

/sys/bus/pci/devices/${devname}/vbios_version

Tom

On 25/08/17 09:43 AM, Kent Russell wrote:
> sysfs is more stable, and doesn't require root to access
> 
> Signed-off-by: Kent Russell <kent.russell@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 54 +++++++++++++-----------------
>   1 file changed, 23 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index c041496..77929e4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -67,7 +67,6 @@ static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev);
>   static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev);
>   static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev);
>   static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device 
> *adev); -static int amdgpu_debugfs_vbios_version_init(struct 
> amdgpu_device *adev);
>   
>   static const char *amdgpu_asic_name[] = {
>   	"TAHITI",
> @@ -890,6 +889,20 @@ static uint32_t cail_ioreg_read(struct card_info *info, uint32_t reg)
>   	return r;
>   }
>   
> +static ssize_t amdgpu_atombios_get_vbios_version(struct device *dev,
> +						 struct device_attribute *attr,
> +						 char *buf)
> +{
> +	struct drm_device *ddev = dev_get_drvdata(dev);
> +	struct amdgpu_device *adev = ddev->dev_private;
> +	struct atom_context *ctx = adev->mode_info.atom_context;
> +
> +	return snprintf(buf, PAGE_SIZE, "%s\n", ctx->vbios_version); }
> +
> +static DEVICE_ATTR(vbios_version, 0444, amdgpu_atombios_get_vbios_version,
> +		   NULL);
> +
>   /**
>    * amdgpu_atombios_fini - free the driver info and callbacks for atombios
>    *
> @@ -909,6 +922,7 @@ static void amdgpu_atombios_fini(struct amdgpu_device *adev)
>   	adev->mode_info.atom_context = NULL;
>   	kfree(adev->mode_info.atom_card_info);
>   	adev->mode_info.atom_card_info = NULL;
> +	device_remove_file(adev->dev, &dev_attr_vbios_version);
>   }
>   
>   /**
> @@ -925,6 +939,7 @@ static int amdgpu_atombios_init(struct amdgpu_device *adev)
>   {
>   	struct card_info *atom_card_info =
>   	    kzalloc(sizeof(struct card_info), GFP_KERNEL);
> +	int ret;
>   
>   	if (!atom_card_info)
>   		return -ENOMEM;
> @@ -961,6 +976,13 @@ static int amdgpu_atombios_init(struct amdgpu_device *adev)
>   		amdgpu_atombios_scratch_regs_init(adev);
>   		amdgpu_atombios_allocate_fb_scratch(adev);
>   	}
> +
> +	ret = device_create_file(adev->dev, &dev_attr_vbios_version);
> +	if (ret) {
> +		DRM_ERROR("Failed to create device file for VBIOS version\n");
> +		return ret;
> +	}
> +
>   	return 0;
>   }
>   
> @@ -2256,10 +2278,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>   	if (r)
>   		DRM_ERROR("Creating vbios dump debugfs failed (%d).\n", r);
>   
> -	r = amdgpu_debugfs_vbios_version_init(adev);
> -	if (r)
> -		DRM_ERROR("Creating vbios version debugfs failed (%d).\n", r);
> -
>   	if ((amdgpu_testing & 1)) {
>   		if (adev->accel_working)
>   			amdgpu_test_moves(adev);
> @@ -3851,39 +3869,17 @@ static int amdgpu_debugfs_get_vbios_dump(struct seq_file *m, void *data)
>   	return 0;
>   }
>   
> -static int amdgpu_debugfs_get_vbios_version(struct seq_file *m, void 
> *data) -{
> -	struct drm_info_node *node = (struct drm_info_node *) m->private;
> -	struct drm_device *dev = node->minor->dev;
> -	struct amdgpu_device *adev = dev->dev_private;
> -	struct atom_context *ctx = adev->mode_info.atom_context;
> -
> -	seq_printf(m, "%s\n", ctx->vbios_version);
> -	return 0;
> -}
> -
>   static const struct drm_info_list amdgpu_vbios_dump_list[] = {
>   		{"amdgpu_vbios",
>   		 amdgpu_debugfs_get_vbios_dump,
>   		 0, NULL},
>   };
>   
> -static const struct drm_info_list amdgpu_vbios_version_list[] = {
> -		{"amdgpu_vbios_version",
> -		 amdgpu_debugfs_get_vbios_version,
> -		 0, NULL},
> -};
> -
>   static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)
>   {
>   	return amdgpu_debugfs_add_files(adev,
>   					amdgpu_vbios_dump_list, 1);
>   }
> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device 
> *adev) -{
> -	return amdgpu_debugfs_add_files(adev,
> -					amdgpu_vbios_version_list, 1);
> -}
>   #else
>   static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev)
>   {
> @@ -3897,9 +3893,5 @@ static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)
>   {
>   	return 0;
>   }
> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device 
> *adev) -{
> -	return 0;
> -}
>   static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev) { }
>   #endif
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread
* RE: [PATCH] drm/amdgpu: Move VBIOS version to sysfs
       [not found]     ` <CADnq5_MYCTT73r0X_=JCtTVkYb-2_oK1LQQb85-dQk3V-wud0Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-08-25 15:37       ` Russell, Kent
       [not found]         ` <BN6PR1201MB0180C5182F56FF596C602553859B0-6iU6OBHu2P/H0AMcJMwsYmrFom/aUZj6nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
  2017-08-25 19:13       ` Christian König
  1 sibling, 1 reply; 11+ messages in thread
From: Russell, Kent @ 2017-08-25 15:37 UTC (permalink / raw)
  To: Alex Deucher; +Cc: amd-gfx list

Christian was saying that the VBIOS binary should remain in debugfs (on the other thread). I'll let you two discuss, since I have zero vested interest in the location of the VBIOS binary :)

 Kent

-----Original Message-----
From: Alex Deucher [mailto:alexdeucher@gmail.com] 
Sent: Friday, August 25, 2017 10:41 AM
To: Russell, Kent
Cc: amd-gfx list
Subject: Re: [PATCH] drm/amdgpu: Move VBIOS version to sysfs

On Fri, Aug 25, 2017 at 9:43 AM, Kent Russell <kent.russell@amd.com> wrote:
> sysfs is more stable, and doesn't require root to access
>
> Signed-off-by: Kent Russell <kent.russell@amd.com>

Might as well move the vbios binary itself to sysfs as well.  It should be a stable interface :)

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 54 
> +++++++++++++-----------------
>  1 file changed, 23 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index c041496..77929e4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -67,7 +67,6 @@ static int amdgpu_debugfs_regs_init(struct 
> amdgpu_device *adev);  static void amdgpu_debugfs_regs_cleanup(struct 
> amdgpu_device *adev);  static int 
> amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev);  static 
> int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev); 
> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device 
> *adev);
>
>  static const char *amdgpu_asic_name[] = {
>         "TAHITI",
> @@ -890,6 +889,20 @@ static uint32_t cail_ioreg_read(struct card_info *info, uint32_t reg)
>         return r;
>  }
>
> +static ssize_t amdgpu_atombios_get_vbios_version(struct device *dev,
> +                                                struct device_attribute *attr,
> +                                                char *buf) {
> +       struct drm_device *ddev = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = ddev->dev_private;
> +       struct atom_context *ctx = adev->mode_info.atom_context;
> +
> +       return snprintf(buf, PAGE_SIZE, "%s\n", ctx->vbios_version); }
> +
> +static DEVICE_ATTR(vbios_version, 0444, amdgpu_atombios_get_vbios_version,
> +                  NULL);
> +
>  /**
>   * amdgpu_atombios_fini - free the driver info and callbacks for atombios
>   *
> @@ -909,6 +922,7 @@ static void amdgpu_atombios_fini(struct amdgpu_device *adev)
>         adev->mode_info.atom_context = NULL;
>         kfree(adev->mode_info.atom_card_info);
>         adev->mode_info.atom_card_info = NULL;
> +       device_remove_file(adev->dev, &dev_attr_vbios_version);
>  }
>
>  /**
> @@ -925,6 +939,7 @@ static int amdgpu_atombios_init(struct 
> amdgpu_device *adev)  {
>         struct card_info *atom_card_info =
>             kzalloc(sizeof(struct card_info), GFP_KERNEL);
> +       int ret;
>
>         if (!atom_card_info)
>                 return -ENOMEM;
> @@ -961,6 +976,13 @@ static int amdgpu_atombios_init(struct amdgpu_device *adev)
>                 amdgpu_atombios_scratch_regs_init(adev);
>                 amdgpu_atombios_allocate_fb_scratch(adev);
>         }
> +
> +       ret = device_create_file(adev->dev, &dev_attr_vbios_version);
> +       if (ret) {
> +               DRM_ERROR("Failed to create device file for VBIOS version\n");
> +               return ret;
> +       }
> +
>         return 0;
>  }
>
> @@ -2256,10 +2278,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>         if (r)
>                 DRM_ERROR("Creating vbios dump debugfs failed 
> (%d).\n", r);
>
> -       r = amdgpu_debugfs_vbios_version_init(adev);
> -       if (r)
> -               DRM_ERROR("Creating vbios version debugfs failed (%d).\n", r);
> -
>         if ((amdgpu_testing & 1)) {
>                 if (adev->accel_working)
>                         amdgpu_test_moves(adev); @@ -3851,39 +3869,17 
> @@ static int amdgpu_debugfs_get_vbios_dump(struct seq_file *m, void *data)
>         return 0;
>  }
>
> -static int amdgpu_debugfs_get_vbios_version(struct seq_file *m, void 
> *data) -{
> -       struct drm_info_node *node = (struct drm_info_node *) m->private;
> -       struct drm_device *dev = node->minor->dev;
> -       struct amdgpu_device *adev = dev->dev_private;
> -       struct atom_context *ctx = adev->mode_info.atom_context;
> -
> -       seq_printf(m, "%s\n", ctx->vbios_version);
> -       return 0;
> -}
> -
>  static const struct drm_info_list amdgpu_vbios_dump_list[] = {
>                 {"amdgpu_vbios",
>                  amdgpu_debugfs_get_vbios_dump,
>                  0, NULL},
>  };
>
> -static const struct drm_info_list amdgpu_vbios_version_list[] = {
> -               {"amdgpu_vbios_version",
> -                amdgpu_debugfs_get_vbios_version,
> -                0, NULL},
> -};
> -
>  static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)  
> {
>         return amdgpu_debugfs_add_files(adev,
>                                         amdgpu_vbios_dump_list, 1);  } 
> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device 
> *adev) -{
> -       return amdgpu_debugfs_add_files(adev,
> -                                       amdgpu_vbios_version_list, 1);
> -}
>  #else
>  static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device 
> *adev)  { @@ -3897,9 +3893,5 @@ static int 
> amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)  {
>         return 0;
>  }
> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device 
> *adev) -{
> -       return 0;
> -}
>  static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev) { 
> }  #endif
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread
* RE: [PATCH] drm/amdgpu: Move VBIOS version to sysfs
       [not found]         ` <BN6PR1201MB0180C5182F56FF596C602553859B0-6iU6OBHu2P/H0AMcJMwsYmrFom/aUZj6nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
@ 2017-08-25 16:14           ` Deucher, Alexander
  0 siblings, 0 replies; 11+ messages in thread
From: Deucher, Alexander @ 2017-08-25 16:14 UTC (permalink / raw)
  To: Russell, Kent, Alex Deucher; +Cc: amd-gfx list

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Russell, Kent
> Sent: Friday, August 25, 2017 11:38 AM
> To: Alex Deucher
> Cc: amd-gfx list
> Subject: RE: [PATCH] drm/amdgpu: Move VBIOS version to sysfs
> 
> Christian was saying that the VBIOS binary should remain in debugfs (on the
> other thread). I'll let you two discuss, since I have zero vested interest in the
> location of the VBIOS binary :)

I don't have a strong preference either way.

Alex

> 
>  Kent
> 
> -----Original Message-----
> From: Alex Deucher [mailto:alexdeucher@gmail.com]
> Sent: Friday, August 25, 2017 10:41 AM
> To: Russell, Kent
> Cc: amd-gfx list
> Subject: Re: [PATCH] drm/amdgpu: Move VBIOS version to sysfs
> 
> On Fri, Aug 25, 2017 at 9:43 AM, Kent Russell <kent.russell@amd.com>
> wrote:
> > sysfs is more stable, and doesn't require root to access
> >
> > Signed-off-by: Kent Russell <kent.russell@amd.com>
> 
> Might as well move the vbios binary itself to sysfs as well.  It should be a
> stable interface :)
> 
> Alex
> 
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 54
> > +++++++++++++-----------------
> >  1 file changed, 23 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index c041496..77929e4 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -67,7 +67,6 @@ static int amdgpu_debugfs_regs_init(struct
> > amdgpu_device *adev);  static void amdgpu_debugfs_regs_cleanup(struct
> > amdgpu_device *adev);  static int
> > amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev);  static
> > int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev);
> > -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device
> > *adev);
> >
> >  static const char *amdgpu_asic_name[] = {
> >         "TAHITI",
> > @@ -890,6 +889,20 @@ static uint32_t cail_ioreg_read(struct card_info
> *info, uint32_t reg)
> >         return r;
> >  }
> >
> > +static ssize_t amdgpu_atombios_get_vbios_version(struct device *dev,
> > +                                                struct device_attribute *attr,
> > +                                                char *buf) {
> > +       struct drm_device *ddev = dev_get_drvdata(dev);
> > +       struct amdgpu_device *adev = ddev->dev_private;
> > +       struct atom_context *ctx = adev->mode_info.atom_context;
> > +
> > +       return snprintf(buf, PAGE_SIZE, "%s\n", ctx->vbios_version); }
> > +
> > +static DEVICE_ATTR(vbios_version, 0444,
> amdgpu_atombios_get_vbios_version,
> > +                  NULL);
> > +
> >  /**
> >   * amdgpu_atombios_fini - free the driver info and callbacks for atombios
> >   *
> > @@ -909,6 +922,7 @@ static void amdgpu_atombios_fini(struct
> amdgpu_device *adev)
> >         adev->mode_info.atom_context = NULL;
> >         kfree(adev->mode_info.atom_card_info);
> >         adev->mode_info.atom_card_info = NULL;
> > +       device_remove_file(adev->dev, &dev_attr_vbios_version);
> >  }
> >
> >  /**
> > @@ -925,6 +939,7 @@ static int amdgpu_atombios_init(struct
> > amdgpu_device *adev)  {
> >         struct card_info *atom_card_info =
> >             kzalloc(sizeof(struct card_info), GFP_KERNEL);
> > +       int ret;
> >
> >         if (!atom_card_info)
> >                 return -ENOMEM;
> > @@ -961,6 +976,13 @@ static int amdgpu_atombios_init(struct
> amdgpu_device *adev)
> >                 amdgpu_atombios_scratch_regs_init(adev);
> >                 amdgpu_atombios_allocate_fb_scratch(adev);
> >         }
> > +
> > +       ret = device_create_file(adev->dev, &dev_attr_vbios_version);
> > +       if (ret) {
> > +               DRM_ERROR("Failed to create device file for VBIOS version\n");
> > +               return ret;
> > +       }
> > +
> >         return 0;
> >  }
> >
> > @@ -2256,10 +2278,6 @@ int amdgpu_device_init(struct amdgpu_device
> *adev,
> >         if (r)
> >                 DRM_ERROR("Creating vbios dump debugfs failed
> > (%d).\n", r);
> >
> > -       r = amdgpu_debugfs_vbios_version_init(adev);
> > -       if (r)
> > -               DRM_ERROR("Creating vbios version debugfs failed (%d).\n", r);
> > -
> >         if ((amdgpu_testing & 1)) {
> >                 if (adev->accel_working)
> >                         amdgpu_test_moves(adev); @@ -3851,39 +3869,17
> > @@ static int amdgpu_debugfs_get_vbios_dump(struct seq_file *m, void
> *data)
> >         return 0;
> >  }
> >
> > -static int amdgpu_debugfs_get_vbios_version(struct seq_file *m, void
> > *data) -{
> > -       struct drm_info_node *node = (struct drm_info_node *) m->private;
> > -       struct drm_device *dev = node->minor->dev;
> > -       struct amdgpu_device *adev = dev->dev_private;
> > -       struct atom_context *ctx = adev->mode_info.atom_context;
> > -
> > -       seq_printf(m, "%s\n", ctx->vbios_version);
> > -       return 0;
> > -}
> > -
> >  static const struct drm_info_list amdgpu_vbios_dump_list[] = {
> >                 {"amdgpu_vbios",
> >                  amdgpu_debugfs_get_vbios_dump,
> >                  0, NULL},
> >  };
> >
> > -static const struct drm_info_list amdgpu_vbios_version_list[] = {
> > -               {"amdgpu_vbios_version",
> > -                amdgpu_debugfs_get_vbios_version,
> > -                0, NULL},
> > -};
> > -
> >  static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device
> *adev)
> > {
> >         return amdgpu_debugfs_add_files(adev,
> >                                         amdgpu_vbios_dump_list, 1);  }
> > -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device
> > *adev) -{
> > -       return amdgpu_debugfs_add_files(adev,
> > -                                       amdgpu_vbios_version_list, 1);
> > -}
> >  #else
> >  static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device
> > *adev)  { @@ -3897,9 +3893,5 @@ static int
> > amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)  {
> >         return 0;
> >  }
> > -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device
> > *adev) -{
> > -       return 0;
> > -}
> >  static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev)
> {
> > }  #endif
> > --
> > 2.7.4
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH] drm/amdgpu: Move VBIOS version to sysfs
       [not found]     ` <CADnq5_MYCTT73r0X_=JCtTVkYb-2_oK1LQQb85-dQk3V-wud0Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2017-08-25 15:37       ` Russell, Kent
@ 2017-08-25 19:13       ` Christian König
       [not found]         ` <cf9249dc-5c0b-22ae-ec53-dbc4b9845d4b-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  1 sibling, 1 reply; 11+ messages in thread
From: Christian König @ 2017-08-25 19:13 UTC (permalink / raw)
  To: Alex Deucher, Kent Russell; +Cc: amd-gfx list

Am 25.08.2017 um 16:41 schrieb Alex Deucher:
> On Fri, Aug 25, 2017 at 9:43 AM, Kent Russell <kent.russell@amd.com> wrote:
>> sysfs is more stable, and doesn't require root to access
>>
>> Signed-off-by: Kent Russell <kent.russell@amd.com>
> Might as well move the vbios binary itself to sysfs as well.  It
> should be a stable interface :)

Yeah, but that is nothing a "normal" client application should be 
interested in.

Additional to that I'm still not sure if a large binary BIOS qualifies 
as single value per file.

Christian.

>
> Alex
>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 54 +++++++++++++-----------------
>>   1 file changed, 23 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index c041496..77929e4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -67,7 +67,6 @@ static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev);
>>   static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev);
>>   static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev);
>>   static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev);
>> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device *adev);
>>
>>   static const char *amdgpu_asic_name[] = {
>>          "TAHITI",
>> @@ -890,6 +889,20 @@ static uint32_t cail_ioreg_read(struct card_info *info, uint32_t reg)
>>          return r;
>>   }
>>
>> +static ssize_t amdgpu_atombios_get_vbios_version(struct device *dev,
>> +                                                struct device_attribute *attr,
>> +                                                char *buf)
>> +{
>> +       struct drm_device *ddev = dev_get_drvdata(dev);
>> +       struct amdgpu_device *adev = ddev->dev_private;
>> +       struct atom_context *ctx = adev->mode_info.atom_context;
>> +
>> +       return snprintf(buf, PAGE_SIZE, "%s\n", ctx->vbios_version);
>> +}
>> +
>> +static DEVICE_ATTR(vbios_version, 0444, amdgpu_atombios_get_vbios_version,
>> +                  NULL);
>> +
>>   /**
>>    * amdgpu_atombios_fini - free the driver info and callbacks for atombios
>>    *
>> @@ -909,6 +922,7 @@ static void amdgpu_atombios_fini(struct amdgpu_device *adev)
>>          adev->mode_info.atom_context = NULL;
>>          kfree(adev->mode_info.atom_card_info);
>>          adev->mode_info.atom_card_info = NULL;
>> +       device_remove_file(adev->dev, &dev_attr_vbios_version);
>>   }
>>
>>   /**
>> @@ -925,6 +939,7 @@ static int amdgpu_atombios_init(struct amdgpu_device *adev)
>>   {
>>          struct card_info *atom_card_info =
>>              kzalloc(sizeof(struct card_info), GFP_KERNEL);
>> +       int ret;
>>
>>          if (!atom_card_info)
>>                  return -ENOMEM;
>> @@ -961,6 +976,13 @@ static int amdgpu_atombios_init(struct amdgpu_device *adev)
>>                  amdgpu_atombios_scratch_regs_init(adev);
>>                  amdgpu_atombios_allocate_fb_scratch(adev);
>>          }
>> +
>> +       ret = device_create_file(adev->dev, &dev_attr_vbios_version);
>> +       if (ret) {
>> +               DRM_ERROR("Failed to create device file for VBIOS version\n");
>> +               return ret;
>> +       }
>> +
>>          return 0;
>>   }
>>
>> @@ -2256,10 +2278,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>>          if (r)
>>                  DRM_ERROR("Creating vbios dump debugfs failed (%d).\n", r);
>>
>> -       r = amdgpu_debugfs_vbios_version_init(adev);
>> -       if (r)
>> -               DRM_ERROR("Creating vbios version debugfs failed (%d).\n", r);
>> -
>>          if ((amdgpu_testing & 1)) {
>>                  if (adev->accel_working)
>>                          amdgpu_test_moves(adev);
>> @@ -3851,39 +3869,17 @@ static int amdgpu_debugfs_get_vbios_dump(struct seq_file *m, void *data)
>>          return 0;
>>   }
>>
>> -static int amdgpu_debugfs_get_vbios_version(struct seq_file *m, void *data)
>> -{
>> -       struct drm_info_node *node = (struct drm_info_node *) m->private;
>> -       struct drm_device *dev = node->minor->dev;
>> -       struct amdgpu_device *adev = dev->dev_private;
>> -       struct atom_context *ctx = adev->mode_info.atom_context;
>> -
>> -       seq_printf(m, "%s\n", ctx->vbios_version);
>> -       return 0;
>> -}
>> -
>>   static const struct drm_info_list amdgpu_vbios_dump_list[] = {
>>                  {"amdgpu_vbios",
>>                   amdgpu_debugfs_get_vbios_dump,
>>                   0, NULL},
>>   };
>>
>> -static const struct drm_info_list amdgpu_vbios_version_list[] = {
>> -               {"amdgpu_vbios_version",
>> -                amdgpu_debugfs_get_vbios_version,
>> -                0, NULL},
>> -};
>> -
>>   static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)
>>   {
>>          return amdgpu_debugfs_add_files(adev,
>>                                          amdgpu_vbios_dump_list, 1);
>>   }
>> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device *adev)
>> -{
>> -       return amdgpu_debugfs_add_files(adev,
>> -                                       amdgpu_vbios_version_list, 1);
>> -}
>>   #else
>>   static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev)
>>   {
>> @@ -3897,9 +3893,5 @@ static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)
>>   {
>>          return 0;
>>   }
>> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device *adev)
>> -{
>> -       return 0;
>> -}
>>   static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev) { }
>>   #endif
>> --
>> 2.7.4
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH] drm/amdgpu: Move VBIOS version to sysfs
       [not found]         ` <cf9249dc-5c0b-22ae-ec53-dbc4b9845d4b-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
@ 2017-08-25 19:17           ` Alex Deucher
       [not found]             ` <CADnq5_PBqKUsR2aT5UPZ5JPBHKEHqytN=7rzD5UONe9KKf0V-Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Alex Deucher @ 2017-08-25 19:17 UTC (permalink / raw)
  To: Christian König; +Cc: Kent Russell, amd-gfx list

On Fri, Aug 25, 2017 at 3:13 PM, Christian König
<deathsimple@vodafone.de> wrote:
> Am 25.08.2017 um 16:41 schrieb Alex Deucher:
>>
>> On Fri, Aug 25, 2017 at 9:43 AM, Kent Russell <kent.russell@amd.com>
>> wrote:
>>>
>>> sysfs is more stable, and doesn't require root to access
>>>
>>> Signed-off-by: Kent Russell <kent.russell@amd.com>
>>
>> Might as well move the vbios binary itself to sysfs as well.  It
>> should be a stable interface :)
>
>
> Yeah, but that is nothing a "normal" client application should be interested
> in.
>
> Additional to that I'm still not sure if a large binary BIOS qualifies as
> single value per file.

Well, you can read pci roms out of sysfs, so there is precedent.  For
our purposes, debugfs is fine.

Alex

>
> Christian.
>
>
>>
>> Alex
>>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 54
>>> +++++++++++++-----------------
>>>   1 file changed, 23 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index c041496..77929e4 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -67,7 +67,6 @@ static int amdgpu_debugfs_regs_init(struct
>>> amdgpu_device *adev);
>>>   static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev);
>>>   static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device
>>> *adev);
>>>   static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev);
>>> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device
>>> *adev);
>>>
>>>   static const char *amdgpu_asic_name[] = {
>>>          "TAHITI",
>>> @@ -890,6 +889,20 @@ static uint32_t cail_ioreg_read(struct card_info
>>> *info, uint32_t reg)
>>>          return r;
>>>   }
>>>
>>> +static ssize_t amdgpu_atombios_get_vbios_version(struct device *dev,
>>> +                                                struct device_attribute
>>> *attr,
>>> +                                                char *buf)
>>> +{
>>> +       struct drm_device *ddev = dev_get_drvdata(dev);
>>> +       struct amdgpu_device *adev = ddev->dev_private;
>>> +       struct atom_context *ctx = adev->mode_info.atom_context;
>>> +
>>> +       return snprintf(buf, PAGE_SIZE, "%s\n", ctx->vbios_version);
>>> +}
>>> +
>>> +static DEVICE_ATTR(vbios_version, 0444,
>>> amdgpu_atombios_get_vbios_version,
>>> +                  NULL);
>>> +
>>>   /**
>>>    * amdgpu_atombios_fini - free the driver info and callbacks for
>>> atombios
>>>    *
>>> @@ -909,6 +922,7 @@ static void amdgpu_atombios_fini(struct amdgpu_device
>>> *adev)
>>>          adev->mode_info.atom_context = NULL;
>>>          kfree(adev->mode_info.atom_card_info);
>>>          adev->mode_info.atom_card_info = NULL;
>>> +       device_remove_file(adev->dev, &dev_attr_vbios_version);
>>>   }
>>>
>>>   /**
>>> @@ -925,6 +939,7 @@ static int amdgpu_atombios_init(struct amdgpu_device
>>> *adev)
>>>   {
>>>          struct card_info *atom_card_info =
>>>              kzalloc(sizeof(struct card_info), GFP_KERNEL);
>>> +       int ret;
>>>
>>>          if (!atom_card_info)
>>>                  return -ENOMEM;
>>> @@ -961,6 +976,13 @@ static int amdgpu_atombios_init(struct amdgpu_device
>>> *adev)
>>>                  amdgpu_atombios_scratch_regs_init(adev);
>>>                  amdgpu_atombios_allocate_fb_scratch(adev);
>>>          }
>>> +
>>> +       ret = device_create_file(adev->dev, &dev_attr_vbios_version);
>>> +       if (ret) {
>>> +               DRM_ERROR("Failed to create device file for VBIOS
>>> version\n");
>>> +               return ret;
>>> +       }
>>> +
>>>          return 0;
>>>   }
>>>
>>> @@ -2256,10 +2278,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>>>          if (r)
>>>                  DRM_ERROR("Creating vbios dump debugfs failed (%d).\n",
>>> r);
>>>
>>> -       r = amdgpu_debugfs_vbios_version_init(adev);
>>> -       if (r)
>>> -               DRM_ERROR("Creating vbios version debugfs failed
>>> (%d).\n", r);
>>> -
>>>          if ((amdgpu_testing & 1)) {
>>>                  if (adev->accel_working)
>>>                          amdgpu_test_moves(adev);
>>> @@ -3851,39 +3869,17 @@ static int amdgpu_debugfs_get_vbios_dump(struct
>>> seq_file *m, void *data)
>>>          return 0;
>>>   }
>>>
>>> -static int amdgpu_debugfs_get_vbios_version(struct seq_file *m, void
>>> *data)
>>> -{
>>> -       struct drm_info_node *node = (struct drm_info_node *) m->private;
>>> -       struct drm_device *dev = node->minor->dev;
>>> -       struct amdgpu_device *adev = dev->dev_private;
>>> -       struct atom_context *ctx = adev->mode_info.atom_context;
>>> -
>>> -       seq_printf(m, "%s\n", ctx->vbios_version);
>>> -       return 0;
>>> -}
>>> -
>>>   static const struct drm_info_list amdgpu_vbios_dump_list[] = {
>>>                  {"amdgpu_vbios",
>>>                   amdgpu_debugfs_get_vbios_dump,
>>>                   0, NULL},
>>>   };
>>>
>>> -static const struct drm_info_list amdgpu_vbios_version_list[] = {
>>> -               {"amdgpu_vbios_version",
>>> -                amdgpu_debugfs_get_vbios_version,
>>> -                0, NULL},
>>> -};
>>> -
>>>   static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)
>>>   {
>>>          return amdgpu_debugfs_add_files(adev,
>>>                                          amdgpu_vbios_dump_list, 1);
>>>   }
>>> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device *adev)
>>> -{
>>> -       return amdgpu_debugfs_add_files(adev,
>>> -                                       amdgpu_vbios_version_list, 1);
>>> -}
>>>   #else
>>>   static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev)
>>>   {
>>> @@ -3897,9 +3893,5 @@ static int amdgpu_debugfs_vbios_dump_init(struct
>>> amdgpu_device *adev)
>>>   {
>>>          return 0;
>>>   }
>>> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device *adev)
>>> -{
>>> -       return 0;
>>> -}
>>>   static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev) { }
>>>   #endif
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread
* RE: [PATCH] drm/amdgpu: Move VBIOS version to sysfs
       [not found]             ` <CADnq5_PBqKUsR2aT5UPZ5JPBHKEHqytN=7rzD5UONe9KKf0V-Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-08-28 12:39               ` Russell, Kent
       [not found]                 ` <BN6PR1201MB0180A46EEBD258AFD3CE1CEB859E0-6iU6OBHu2P/H0AMcJMwsYmrFom/aUZj6nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Russell, Kent @ 2017-08-28 12:39 UTC (permalink / raw)
  To: Alex Deucher, Christian König; +Cc: amd-gfx list

Would it be possible to get a Reviewed-By since we'll leave the VBIOS in debugfs for now? Thanks!

 Kent

-----Original Message-----
From: Alex Deucher [mailto:alexdeucher@gmail.com] 
Sent: Friday, August 25, 2017 3:17 PM
To: Christian König
Cc: Russell, Kent; amd-gfx list
Subject: Re: [PATCH] drm/amdgpu: Move VBIOS version to sysfs

On Fri, Aug 25, 2017 at 3:13 PM, Christian König <deathsimple@vodafone.de> wrote:
> Am 25.08.2017 um 16:41 schrieb Alex Deucher:
>>
>> On Fri, Aug 25, 2017 at 9:43 AM, Kent Russell <kent.russell@amd.com>
>> wrote:
>>>
>>> sysfs is more stable, and doesn't require root to access
>>>
>>> Signed-off-by: Kent Russell <kent.russell@amd.com>
>>
>> Might as well move the vbios binary itself to sysfs as well.  It 
>> should be a stable interface :)
>
>
> Yeah, but that is nothing a "normal" client application should be 
> interested in.
>
> Additional to that I'm still not sure if a large binary BIOS qualifies 
> as single value per file.

Well, you can read pci roms out of sysfs, so there is precedent.  For our purposes, debugfs is fine.

Alex

>
> Christian.
>
>
>>
>> Alex
>>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 54
>>> +++++++++++++-----------------
>>>   1 file changed, 23 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index c041496..77929e4 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -67,7 +67,6 @@ static int amdgpu_debugfs_regs_init(struct 
>>> amdgpu_device *adev);
>>>   static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev);
>>>   static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device 
>>> *adev);
>>>   static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device 
>>> *adev); -static int amdgpu_debugfs_vbios_version_init(struct 
>>> amdgpu_device *adev);
>>>
>>>   static const char *amdgpu_asic_name[] = {
>>>          "TAHITI",
>>> @@ -890,6 +889,20 @@ static uint32_t cail_ioreg_read(struct 
>>> card_info *info, uint32_t reg)
>>>          return r;
>>>   }
>>>
>>> +static ssize_t amdgpu_atombios_get_vbios_version(struct device *dev,
>>> +                                                struct 
>>> +device_attribute
>>> *attr,
>>> +                                                char *buf) {
>>> +       struct drm_device *ddev = dev_get_drvdata(dev);
>>> +       struct amdgpu_device *adev = ddev->dev_private;
>>> +       struct atom_context *ctx = adev->mode_info.atom_context;
>>> +
>>> +       return snprintf(buf, PAGE_SIZE, "%s\n", ctx->vbios_version); 
>>> +}
>>> +
>>> +static DEVICE_ATTR(vbios_version, 0444,
>>> amdgpu_atombios_get_vbios_version,
>>> +                  NULL);
>>> +
>>>   /**
>>>    * amdgpu_atombios_fini - free the driver info and callbacks for 
>>> atombios
>>>    *
>>> @@ -909,6 +922,7 @@ static void amdgpu_atombios_fini(struct 
>>> amdgpu_device
>>> *adev)
>>>          adev->mode_info.atom_context = NULL;
>>>          kfree(adev->mode_info.atom_card_info);
>>>          adev->mode_info.atom_card_info = NULL;
>>> +       device_remove_file(adev->dev, &dev_attr_vbios_version);
>>>   }
>>>
>>>   /**
>>> @@ -925,6 +939,7 @@ static int amdgpu_atombios_init(struct 
>>> amdgpu_device
>>> *adev)
>>>   {
>>>          struct card_info *atom_card_info =
>>>              kzalloc(sizeof(struct card_info), GFP_KERNEL);
>>> +       int ret;
>>>
>>>          if (!atom_card_info)
>>>                  return -ENOMEM;
>>> @@ -961,6 +976,13 @@ static int amdgpu_atombios_init(struct 
>>> amdgpu_device
>>> *adev)
>>>                  amdgpu_atombios_scratch_regs_init(adev);
>>>                  amdgpu_atombios_allocate_fb_scratch(adev);
>>>          }
>>> +
>>> +       ret = device_create_file(adev->dev, &dev_attr_vbios_version);
>>> +       if (ret) {
>>> +               DRM_ERROR("Failed to create device file for VBIOS
>>> version\n");
>>> +               return ret;
>>> +       }
>>> +
>>>          return 0;
>>>   }
>>>
>>> @@ -2256,10 +2278,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>>>          if (r)
>>>                  DRM_ERROR("Creating vbios dump debugfs failed 
>>> (%d).\n", r);
>>>
>>> -       r = amdgpu_debugfs_vbios_version_init(adev);
>>> -       if (r)
>>> -               DRM_ERROR("Creating vbios version debugfs failed
>>> (%d).\n", r);
>>> -
>>>          if ((amdgpu_testing & 1)) {
>>>                  if (adev->accel_working)
>>>                          amdgpu_test_moves(adev); @@ -3851,39 
>>> +3869,17 @@ static int amdgpu_debugfs_get_vbios_dump(struct
>>> seq_file *m, void *data)
>>>          return 0;
>>>   }
>>>
>>> -static int amdgpu_debugfs_get_vbios_version(struct seq_file *m, 
>>> void
>>> *data)
>>> -{
>>> -       struct drm_info_node *node = (struct drm_info_node *) m->private;
>>> -       struct drm_device *dev = node->minor->dev;
>>> -       struct amdgpu_device *adev = dev->dev_private;
>>> -       struct atom_context *ctx = adev->mode_info.atom_context;
>>> -
>>> -       seq_printf(m, "%s\n", ctx->vbios_version);
>>> -       return 0;
>>> -}
>>> -
>>>   static const struct drm_info_list amdgpu_vbios_dump_list[] = {
>>>                  {"amdgpu_vbios",
>>>                   amdgpu_debugfs_get_vbios_dump,
>>>                   0, NULL},
>>>   };
>>>
>>> -static const struct drm_info_list amdgpu_vbios_version_list[] = {
>>> -               {"amdgpu_vbios_version",
>>> -                amdgpu_debugfs_get_vbios_version,
>>> -                0, NULL},
>>> -};
>>> -
>>>   static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)
>>>   {
>>>          return amdgpu_debugfs_add_files(adev,
>>>                                          amdgpu_vbios_dump_list, 1);
>>>   }
>>> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device 
>>> *adev) -{
>>> -       return amdgpu_debugfs_add_files(adev,
>>> -                                       amdgpu_vbios_version_list, 1);
>>> -}
>>>   #else
>>>   static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev)
>>>   {
>>> @@ -3897,9 +3893,5 @@ static int 
>>> amdgpu_debugfs_vbios_dump_init(struct
>>> amdgpu_device *adev)
>>>   {
>>>          return 0;
>>>   }
>>> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device 
>>> *adev) -{
>>> -       return 0;
>>> -}
>>>   static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev) { }
>>>   #endif
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH] drm/amdgpu: Move VBIOS version to sysfs
       [not found]                 ` <BN6PR1201MB0180A46EEBD258AFD3CE1CEB859E0-6iU6OBHu2P/H0AMcJMwsYmrFom/aUZj6nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
@ 2017-08-28 12:40                   ` Christian König
  2017-08-28 15:37                   ` Deucher, Alexander
  1 sibling, 0 replies; 11+ messages in thread
From: Christian König @ 2017-08-28 12:40 UTC (permalink / raw)
  To: Russell, Kent, Alex Deucher; +Cc: amd-gfx list

Reviewed-by: Christian König <christian.koenig@amd.com>

Am 28.08.2017 um 14:39 schrieb Russell, Kent:
> Would it be possible to get a Reviewed-By since we'll leave the VBIOS in debugfs for now? Thanks!
>
>   Kent
>
> -----Original Message-----
> From: Alex Deucher [mailto:alexdeucher@gmail.com]
> Sent: Friday, August 25, 2017 3:17 PM
> To: Christian König
> Cc: Russell, Kent; amd-gfx list
> Subject: Re: [PATCH] drm/amdgpu: Move VBIOS version to sysfs
>
> On Fri, Aug 25, 2017 at 3:13 PM, Christian König <deathsimple@vodafone.de> wrote:
>> Am 25.08.2017 um 16:41 schrieb Alex Deucher:
>>> On Fri, Aug 25, 2017 at 9:43 AM, Kent Russell <kent.russell@amd.com>
>>> wrote:
>>>> sysfs is more stable, and doesn't require root to access
>>>>
>>>> Signed-off-by: Kent Russell <kent.russell@amd.com>
>>> Might as well move the vbios binary itself to sysfs as well.  It
>>> should be a stable interface :)
>>
>> Yeah, but that is nothing a "normal" client application should be
>> interested in.
>>
>> Additional to that I'm still not sure if a large binary BIOS qualifies
>> as single value per file.
> Well, you can read pci roms out of sysfs, so there is precedent.  For our purposes, debugfs is fine.
>
> Alex
>
>> Christian.
>>
>>
>>> Alex
>>>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 54
>>>> +++++++++++++-----------------
>>>>    1 file changed, 23 insertions(+), 31 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> index c041496..77929e4 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> @@ -67,7 +67,6 @@ static int amdgpu_debugfs_regs_init(struct
>>>> amdgpu_device *adev);
>>>>    static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev);
>>>>    static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device
>>>> *adev);
>>>>    static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device
>>>> *adev); -static int amdgpu_debugfs_vbios_version_init(struct
>>>> amdgpu_device *adev);
>>>>
>>>>    static const char *amdgpu_asic_name[] = {
>>>>           "TAHITI",
>>>> @@ -890,6 +889,20 @@ static uint32_t cail_ioreg_read(struct
>>>> card_info *info, uint32_t reg)
>>>>           return r;
>>>>    }
>>>>
>>>> +static ssize_t amdgpu_atombios_get_vbios_version(struct device *dev,
>>>> +                                                struct
>>>> +device_attribute
>>>> *attr,
>>>> +                                                char *buf) {
>>>> +       struct drm_device *ddev = dev_get_drvdata(dev);
>>>> +       struct amdgpu_device *adev = ddev->dev_private;
>>>> +       struct atom_context *ctx = adev->mode_info.atom_context;
>>>> +
>>>> +       return snprintf(buf, PAGE_SIZE, "%s\n", ctx->vbios_version);
>>>> +}
>>>> +
>>>> +static DEVICE_ATTR(vbios_version, 0444,
>>>> amdgpu_atombios_get_vbios_version,
>>>> +                  NULL);
>>>> +
>>>>    /**
>>>>     * amdgpu_atombios_fini - free the driver info and callbacks for
>>>> atombios
>>>>     *
>>>> @@ -909,6 +922,7 @@ static void amdgpu_atombios_fini(struct
>>>> amdgpu_device
>>>> *adev)
>>>>           adev->mode_info.atom_context = NULL;
>>>>           kfree(adev->mode_info.atom_card_info);
>>>>           adev->mode_info.atom_card_info = NULL;
>>>> +       device_remove_file(adev->dev, &dev_attr_vbios_version);
>>>>    }
>>>>
>>>>    /**
>>>> @@ -925,6 +939,7 @@ static int amdgpu_atombios_init(struct
>>>> amdgpu_device
>>>> *adev)
>>>>    {
>>>>           struct card_info *atom_card_info =
>>>>               kzalloc(sizeof(struct card_info), GFP_KERNEL);
>>>> +       int ret;
>>>>
>>>>           if (!atom_card_info)
>>>>                   return -ENOMEM;
>>>> @@ -961,6 +976,13 @@ static int amdgpu_atombios_init(struct
>>>> amdgpu_device
>>>> *adev)
>>>>                   amdgpu_atombios_scratch_regs_init(adev);
>>>>                   amdgpu_atombios_allocate_fb_scratch(adev);
>>>>           }
>>>> +
>>>> +       ret = device_create_file(adev->dev, &dev_attr_vbios_version);
>>>> +       if (ret) {
>>>> +               DRM_ERROR("Failed to create device file for VBIOS
>>>> version\n");
>>>> +               return ret;
>>>> +       }
>>>> +
>>>>           return 0;
>>>>    }
>>>>
>>>> @@ -2256,10 +2278,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>>>>           if (r)
>>>>                   DRM_ERROR("Creating vbios dump debugfs failed
>>>> (%d).\n", r);
>>>>
>>>> -       r = amdgpu_debugfs_vbios_version_init(adev);
>>>> -       if (r)
>>>> -               DRM_ERROR("Creating vbios version debugfs failed
>>>> (%d).\n", r);
>>>> -
>>>>           if ((amdgpu_testing & 1)) {
>>>>                   if (adev->accel_working)
>>>>                           amdgpu_test_moves(adev); @@ -3851,39
>>>> +3869,17 @@ static int amdgpu_debugfs_get_vbios_dump(struct
>>>> seq_file *m, void *data)
>>>>           return 0;
>>>>    }
>>>>
>>>> -static int amdgpu_debugfs_get_vbios_version(struct seq_file *m,
>>>> void
>>>> *data)
>>>> -{
>>>> -       struct drm_info_node *node = (struct drm_info_node *) m->private;
>>>> -       struct drm_device *dev = node->minor->dev;
>>>> -       struct amdgpu_device *adev = dev->dev_private;
>>>> -       struct atom_context *ctx = adev->mode_info.atom_context;
>>>> -
>>>> -       seq_printf(m, "%s\n", ctx->vbios_version);
>>>> -       return 0;
>>>> -}
>>>> -
>>>>    static const struct drm_info_list amdgpu_vbios_dump_list[] = {
>>>>                   {"amdgpu_vbios",
>>>>                    amdgpu_debugfs_get_vbios_dump,
>>>>                    0, NULL},
>>>>    };
>>>>
>>>> -static const struct drm_info_list amdgpu_vbios_version_list[] = {
>>>> -               {"amdgpu_vbios_version",
>>>> -                amdgpu_debugfs_get_vbios_version,
>>>> -                0, NULL},
>>>> -};
>>>> -
>>>>    static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev)
>>>>    {
>>>>           return amdgpu_debugfs_add_files(adev,
>>>>                                           amdgpu_vbios_dump_list, 1);
>>>>    }
>>>> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device
>>>> *adev) -{
>>>> -       return amdgpu_debugfs_add_files(adev,
>>>> -                                       amdgpu_vbios_version_list, 1);
>>>> -}
>>>>    #else
>>>>    static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev)
>>>>    {
>>>> @@ -3897,9 +3893,5 @@ static int
>>>> amdgpu_debugfs_vbios_dump_init(struct
>>>> amdgpu_device *adev)
>>>>    {
>>>>           return 0;
>>>>    }
>>>> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device
>>>> *adev) -{
>>>> -       return 0;
>>>> -}
>>>>    static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev) { }
>>>>    #endif
>>>> --
>>>> 2.7.4
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread
* RE: [PATCH] drm/amdgpu: Move VBIOS version to sysfs
       [not found]                 ` <BN6PR1201MB0180A46EEBD258AFD3CE1CEB859E0-6iU6OBHu2P/H0AMcJMwsYmrFom/aUZj6nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
  2017-08-28 12:40                   ` Christian König
@ 2017-08-28 15:37                   ` Deucher, Alexander
  1 sibling, 0 replies; 11+ messages in thread
From: Deucher, Alexander @ 2017-08-28 15:37 UTC (permalink / raw)
  To: Russell, Kent, Alex Deucher, Christian König; +Cc: amd-gfx list

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Russell, Kent
> Sent: Monday, August 28, 2017 8:39 AM
> To: Alex Deucher; Christian König
> Cc: amd-gfx list
> Subject: RE: [PATCH] drm/amdgpu: Move VBIOS version to sysfs
> 
> Would it be possible to get a Reviewed-By since we'll leave the VBIOS in
> debugfs for now? Thanks!

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> 
>  Kent
> 
> -----Original Message-----
> From: Alex Deucher [mailto:alexdeucher@gmail.com]
> Sent: Friday, August 25, 2017 3:17 PM
> To: Christian König
> Cc: Russell, Kent; amd-gfx list
> Subject: Re: [PATCH] drm/amdgpu: Move VBIOS version to sysfs
> 
> On Fri, Aug 25, 2017 at 3:13 PM, Christian König <deathsimple@vodafone.de>
> wrote:
> > Am 25.08.2017 um 16:41 schrieb Alex Deucher:
> >>
> >> On Fri, Aug 25, 2017 at 9:43 AM, Kent Russell <kent.russell@amd.com>
> >> wrote:
> >>>
> >>> sysfs is more stable, and doesn't require root to access
> >>>
> >>> Signed-off-by: Kent Russell <kent.russell@amd.com>
> >>
> >> Might as well move the vbios binary itself to sysfs as well.  It
> >> should be a stable interface :)
> >
> >
> > Yeah, but that is nothing a "normal" client application should be
> > interested in.
> >
> > Additional to that I'm still not sure if a large binary BIOS qualifies
> > as single value per file.
> 
> Well, you can read pci roms out of sysfs, so there is precedent.  For our
> purposes, debugfs is fine.
> 
> Alex
> 
> >
> > Christian.
> >
> >
> >>
> >> Alex
> >>
> >>> ---
> >>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 54
> >>> +++++++++++++-----------------
> >>>   1 file changed, 23 insertions(+), 31 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >>> index c041496..77929e4 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >>> @@ -67,7 +67,6 @@ static int amdgpu_debugfs_regs_init(struct
> >>> amdgpu_device *adev);
> >>>   static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device
> *adev);
> >>>   static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device
> >>> *adev);
> >>>   static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device
> >>> *adev); -static int amdgpu_debugfs_vbios_version_init(struct
> >>> amdgpu_device *adev);
> >>>
> >>>   static const char *amdgpu_asic_name[] = {
> >>>          "TAHITI",
> >>> @@ -890,6 +889,20 @@ static uint32_t cail_ioreg_read(struct
> >>> card_info *info, uint32_t reg)
> >>>          return r;
> >>>   }
> >>>
> >>> +static ssize_t amdgpu_atombios_get_vbios_version(struct device
> *dev,
> >>> +                                                struct
> >>> +device_attribute
> >>> *attr,
> >>> +                                                char *buf) {
> >>> +       struct drm_device *ddev = dev_get_drvdata(dev);
> >>> +       struct amdgpu_device *adev = ddev->dev_private;
> >>> +       struct atom_context *ctx = adev->mode_info.atom_context;
> >>> +
> >>> +       return snprintf(buf, PAGE_SIZE, "%s\n", ctx->vbios_version);
> >>> +}
> >>> +
> >>> +static DEVICE_ATTR(vbios_version, 0444,
> >>> amdgpu_atombios_get_vbios_version,
> >>> +                  NULL);
> >>> +
> >>>   /**
> >>>    * amdgpu_atombios_fini - free the driver info and callbacks for
> >>> atombios
> >>>    *
> >>> @@ -909,6 +922,7 @@ static void amdgpu_atombios_fini(struct
> >>> amdgpu_device
> >>> *adev)
> >>>          adev->mode_info.atom_context = NULL;
> >>>          kfree(adev->mode_info.atom_card_info);
> >>>          adev->mode_info.atom_card_info = NULL;
> >>> +       device_remove_file(adev->dev, &dev_attr_vbios_version);
> >>>   }
> >>>
> >>>   /**
> >>> @@ -925,6 +939,7 @@ static int amdgpu_atombios_init(struct
> >>> amdgpu_device
> >>> *adev)
> >>>   {
> >>>          struct card_info *atom_card_info =
> >>>              kzalloc(sizeof(struct card_info), GFP_KERNEL);
> >>> +       int ret;
> >>>
> >>>          if (!atom_card_info)
> >>>                  return -ENOMEM;
> >>> @@ -961,6 +976,13 @@ static int amdgpu_atombios_init(struct
> >>> amdgpu_device
> >>> *adev)
> >>>                  amdgpu_atombios_scratch_regs_init(adev);
> >>>                  amdgpu_atombios_allocate_fb_scratch(adev);
> >>>          }
> >>> +
> >>> +       ret = device_create_file(adev->dev, &dev_attr_vbios_version);
> >>> +       if (ret) {
> >>> +               DRM_ERROR("Failed to create device file for VBIOS
> >>> version\n");
> >>> +               return ret;
> >>> +       }
> >>> +
> >>>          return 0;
> >>>   }
> >>>
> >>> @@ -2256,10 +2278,6 @@ int amdgpu_device_init(struct
> amdgpu_device *adev,
> >>>          if (r)
> >>>                  DRM_ERROR("Creating vbios dump debugfs failed
> >>> (%d).\n", r);
> >>>
> >>> -       r = amdgpu_debugfs_vbios_version_init(adev);
> >>> -       if (r)
> >>> -               DRM_ERROR("Creating vbios version debugfs failed
> >>> (%d).\n", r);
> >>> -
> >>>          if ((amdgpu_testing & 1)) {
> >>>                  if (adev->accel_working)
> >>>                          amdgpu_test_moves(adev); @@ -3851,39
> >>> +3869,17 @@ static int amdgpu_debugfs_get_vbios_dump(struct
> >>> seq_file *m, void *data)
> >>>          return 0;
> >>>   }
> >>>
> >>> -static int amdgpu_debugfs_get_vbios_version(struct seq_file *m,
> >>> void
> >>> *data)
> >>> -{
> >>> -       struct drm_info_node *node = (struct drm_info_node *) m-
> >private;
> >>> -       struct drm_device *dev = node->minor->dev;
> >>> -       struct amdgpu_device *adev = dev->dev_private;
> >>> -       struct atom_context *ctx = adev->mode_info.atom_context;
> >>> -
> >>> -       seq_printf(m, "%s\n", ctx->vbios_version);
> >>> -       return 0;
> >>> -}
> >>> -
> >>>   static const struct drm_info_list amdgpu_vbios_dump_list[] = {
> >>>                  {"amdgpu_vbios",
> >>>                   amdgpu_debugfs_get_vbios_dump,
> >>>                   0, NULL},
> >>>   };
> >>>
> >>> -static const struct drm_info_list amdgpu_vbios_version_list[] = {
> >>> -               {"amdgpu_vbios_version",
> >>> -                amdgpu_debugfs_get_vbios_version,
> >>> -                0, NULL},
> >>> -};
> >>> -
> >>>   static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device
> *adev)
> >>>   {
> >>>          return amdgpu_debugfs_add_files(adev,
> >>>                                          amdgpu_vbios_dump_list, 1);
> >>>   }
> >>> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device
> >>> *adev) -{
> >>> -       return amdgpu_debugfs_add_files(adev,
> >>> -                                       amdgpu_vbios_version_list, 1);
> >>> -}
> >>>   #else
> >>>   static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device
> *adev)
> >>>   {
> >>> @@ -3897,9 +3893,5 @@ static int
> >>> amdgpu_debugfs_vbios_dump_init(struct
> >>> amdgpu_device *adev)
> >>>   {
> >>>          return 0;
> >>>   }
> >>> -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device
> >>> *adev) -{
> >>> -       return 0;
> >>> -}
> >>>   static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device
> *adev) { }
> >>>   #endif
> >>> --
> >>> 2.7.4
> >>>
> >>> _______________________________________________
> >>> amd-gfx mailing list
> >>> amd-gfx@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >>
> >> _______________________________________________
> >> amd-gfx mailing list
> >> amd-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >
> >
> >
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-08-28 15:37 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-25 13:43 [PATCH] drm/amdgpu: Move VBIOS version to sysfs Kent Russell
     [not found] ` <1503668585-27962-1-git-send-email-kent.russell-5C7GfCeVMHo@public.gmane.org>
2017-08-25 13:59   ` Tom St Denis
     [not found]     ` <60b4865c-9989-8a20-b200-050e6ff8a441-5C7GfCeVMHo@public.gmane.org>
2017-08-25 15:36       ` Russell, Kent
2017-08-25 14:41   ` Alex Deucher
     [not found]     ` <CADnq5_MYCTT73r0X_=JCtTVkYb-2_oK1LQQb85-dQk3V-wud0Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-08-25 15:37       ` Russell, Kent
     [not found]         ` <BN6PR1201MB0180C5182F56FF596C602553859B0-6iU6OBHu2P/H0AMcJMwsYmrFom/aUZj6nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
2017-08-25 16:14           ` Deucher, Alexander
2017-08-25 19:13       ` Christian König
     [not found]         ` <cf9249dc-5c0b-22ae-ec53-dbc4b9845d4b-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-08-25 19:17           ` Alex Deucher
     [not found]             ` <CADnq5_PBqKUsR2aT5UPZ5JPBHKEHqytN=7rzD5UONe9KKf0V-Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-08-28 12:39               ` Russell, Kent
     [not found]                 ` <BN6PR1201MB0180A46EEBD258AFD3CE1CEB859E0-6iU6OBHu2P/H0AMcJMwsYmrFom/aUZj6nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
2017-08-28 12:40                   ` Christian König
2017-08-28 15:37                   ` Deucher, Alexander

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.