[1/1] drm/amd/amdgpu: get maximum and used UVD handles (v3)

Submitted by Nath, Arindam on Dec. 12, 2016, 6:49 p.m.

Details

Message ID 1481568592-3509-1-git-send-email-arindam.nath@amd.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Nath, Arindam Dec. 12, 2016, 6:49 p.m.
From: Arindam Nath <arindam.nath@amd.com>

Change History
--------------

v3: changes suggested by Christian
- Add a check for UVD IP block using AMDGPU_HW_IP_UVD
  query type.
- Add a check for asic_type to be less than
  CHIP_POLARIS10 since starting Polaris, we support
  unlimited UVD instances.
- Add kerneldoc style comment for
  amdgpu_uvd_used_handles().

v2: as suggested by Christian
- Add a new query AMDGPU_INFO_NUM_HANDLES
- Create a helper function to return the number
  of currently used UVD handles.
- Modify the logic to count the number of used
  UVD handles since handles can be freed in
  non-linear fashion.

v1:
- User might want to query the maximum number of UVD
  instances supported by firmware. In addition to that,
  if there are multiple applications using UVD handles
  at the same time, he might also want to query the
  currently used number of handles.

  For this we add two variables max_handles and
  used_handles inside drm_amdgpu_info_hw_ip. So now
  an application (or libdrm) can use AMDGPU_INFO IOCTL
  with AMDGPU_INFO_HW_IP_INFO query type to get these
  values.

Signed-off-by: Arindam Nath <arindam.nath@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 21 +++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 25 +++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  1 +
 include/uapi/drm/amdgpu_drm.h           |  9 +++++++++
 4 files changed, 56 insertions(+)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 174eb59..3273d8c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -570,6 +570,27 @@  static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
 			return -EINVAL;
 		}
 	}
+	case AMDGPU_INFO_NUM_HANDLES: {
+		struct drm_amdgpu_info_num_handles handle;
+
+		switch (info->query_hw_ip.type) {
+		case AMDGPU_HW_IP_UVD:
+			/* Starting Polaris, we support unlimited UVD handles */
+			if (adev->asic_type < CHIP_POLARIS10) {
+				handle.uvd_max_handles = adev->uvd.max_handles;
+				handle.uvd_used_handles = amdgpu_uvd_used_handles(adev);
+
+				return copy_to_user(out, &handle,
+					min((size_t)size, sizeof(handle))) ? -EFAULT : 0;
+			} else {
+				return -EINVAL;
+			}
+
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
 	default:
 		DRM_DEBUG_KMS("Invalid request %d\n", info->query);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index a8816ba..02187fd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -1173,3 +1173,28 @@  int amdgpu_uvd_ring_test_ib(struct amdgpu_ring *ring, long timeout)
 error:
 	return r;
 }
+
+/**
+ * amdgpu_uvd_used_handles - returns used UVD handles
+ *
+ * @adev: amdgpu_device pointer
+ *
+ * Returns the number of UVD handles in use
+ */
+uint32_t amdgpu_uvd_used_handles(struct amdgpu_device *adev)
+{
+	unsigned i;
+	uint32_t used_handles = 0;
+
+	for (i = 0; i < adev->uvd.max_handles; ++i) {
+		/*
+		 * Handles can be freed in any order, and not
+		 * necessarily linear. So we need to count
+		 * all non-zero handles.
+		 */
+		if (atomic_read(&adev->uvd.handles[i]))
+			used_handles++;
+	}
+
+	return used_handles;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
index c850009..0fee861 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
@@ -38,5 +38,6 @@  int amdgpu_uvd_ring_parse_cs(struct amdgpu_cs_parser *parser, uint32_t ib_idx);
 void amdgpu_uvd_ring_begin_use(struct amdgpu_ring *ring);
 void amdgpu_uvd_ring_end_use(struct amdgpu_ring *ring);
 int amdgpu_uvd_ring_test_ib(struct amdgpu_ring *ring, long timeout);
+uint32_t amdgpu_uvd_used_handles(struct amdgpu_device *adev);
 
 #endif
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index bea9875..2cf8df8 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -530,6 +530,8 @@  struct drm_amdgpu_cs_chunk_data {
 	#define AMDGPU_INFO_VBIOS_SIZE		0x1
 	/* Subquery id: Query vbios image */
 	#define AMDGPU_INFO_VBIOS_IMAGE		0x2
+/* Query UVD handles */
+#define AMDGPU_INFO_NUM_HANDLES			0x1C
 
 #define AMDGPU_INFO_MMR_SE_INDEX_SHIFT	0
 #define AMDGPU_INFO_MMR_SE_INDEX_MASK	0xff
@@ -721,6 +723,13 @@  struct drm_amdgpu_info_hw_ip {
 	__u32  _pad;
 };
 
+struct drm_amdgpu_info_num_handles {
+	/** Max handles as supported by firmware for UVD */
+	__u32  uvd_max_handles;
+	/** Handles currently in use for UVD */
+	__u32  uvd_used_handles;
+};
+
 #define AMDGPU_VCE_CLOCK_TABLE_ENTRIES		6
 
 struct drm_amdgpu_info_vce_clock_table_entry {

Comments

On 12 December 2016 at 18:49,  <arindam.nath@amd.com> wrote:
> From: Arindam Nath <arindam.nath@amd.com>
>
> Change History
> --------------
>
> v3: changes suggested by Christian
> - Add a check for UVD IP block using AMDGPU_HW_IP_UVD
>   query type.
> - Add a check for asic_type to be less than
>   CHIP_POLARIS10 since starting Polaris, we support
>   unlimited UVD instances.
> - Add kerneldoc style comment for
>   amdgpu_uvd_used_handles().
>
> v2: as suggested by Christian
> - Add a new query AMDGPU_INFO_NUM_HANDLES
> - Create a helper function to return the number
>   of currently used UVD handles.
> - Modify the logic to count the number of used
>   UVD handles since handles can be freed in
>   non-linear fashion.
>
> v1:
> - User might want to query the maximum number of UVD
>   instances supported by firmware. In addition to that,
>   if there are multiple applications using UVD handles
>   at the same time, he might also want to query the
>   currently used number of handles.
>
>   For this we add two variables max_handles and
>   used_handles inside drm_amdgpu_info_hw_ip. So now
>   an application (or libdrm) can use AMDGPU_INFO IOCTL
>   with AMDGPU_INFO_HW_IP_INFO query type to get these
>   values.
>
> Signed-off-by: Arindam Nath <arindam.nath@amd.com>
> Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 21 +++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 25 +++++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  1 +
>  include/uapi/drm/amdgpu_drm.h           |  9 +++++++++
>  4 files changed, 56 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 174eb59..3273d8c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -570,6 +570,27 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>                         return -EINVAL;
>                 }
>         }
> +       case AMDGPU_INFO_NUM_HANDLES: {
> +               struct drm_amdgpu_info_num_handles handle;
> +
> +               switch (info->query_hw_ip.type) {
> +               case AMDGPU_HW_IP_UVD:
> +                       /* Starting Polaris, we support unlimited UVD handles */
> +                       if (adev->asic_type < CHIP_POLARIS10) {
> +                               handle.uvd_max_handles = adev->uvd.max_handles;
> +                               handle.uvd_used_handles = amdgpu_uvd_used_handles(adev);
> +
> +                               return copy_to_user(out, &handle,
> +                                       min((size_t)size, sizeof(handle))) ? -EFAULT : 0;
> +                       } else {
> +                               return -EINVAL;
Using EINVAL doesn't seem right here. As per man 3 ioctl

      EINVAL The request or arg argument is not valid for this device.

A bit further down you can see the one you want.

      ENODEV The fildes argument refers to a valid STREAMS device, but
the corresponding device driver does not support the ioctl() function.

Worth checking through the existing code and correcting similar thinkos ?

Thanks
Emil
>-----Original Message-----

>From: Emil Velikov [mailto:emil.l.velikov@gmail.com]

>Sent: Wednesday, December 14, 2016 9:26 PM

>To: Nath, Arindam

>Cc: David Airlie; Deucher, Alexander; amd-gfx mailing list; ML dri-devel;

>Koenig, Christian

>Subject: Re: [PATCH 1/1] drm/amd/amdgpu: get maximum and used UVD

>handles (v3)

>

>On 12 December 2016 at 18:49,  <arindam.nath@amd.com> wrote:

>> From: Arindam Nath <arindam.nath@amd.com>

>>

>> Change History

>> --------------

>>

>> v3: changes suggested by Christian

>> - Add a check for UVD IP block using AMDGPU_HW_IP_UVD

>>   query type.

>> - Add a check for asic_type to be less than

>>   CHIP_POLARIS10 since starting Polaris, we support

>>   unlimited UVD instances.

>> - Add kerneldoc style comment for

>>   amdgpu_uvd_used_handles().

>>

>> v2: as suggested by Christian

>> - Add a new query AMDGPU_INFO_NUM_HANDLES

>> - Create a helper function to return the number

>>   of currently used UVD handles.

>> - Modify the logic to count the number of used

>>   UVD handles since handles can be freed in

>>   non-linear fashion.

>>

>> v1:

>> - User might want to query the maximum number of UVD

>>   instances supported by firmware. In addition to that,

>>   if there are multiple applications using UVD handles

>>   at the same time, he might also want to query the

>>   currently used number of handles.

>>

>>   For this we add two variables max_handles and

>>   used_handles inside drm_amdgpu_info_hw_ip. So now

>>   an application (or libdrm) can use AMDGPU_INFO IOCTL

>>   with AMDGPU_INFO_HW_IP_INFO query type to get these

>>   values.

>>

>> Signed-off-by: Arindam Nath <arindam.nath@amd.com>

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

>> ---

>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 21

>+++++++++++++++++++++

>>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 25

>+++++++++++++++++++++++++

>>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  1 +

>>  include/uapi/drm/amdgpu_drm.h           |  9 +++++++++

>>  4 files changed, 56 insertions(+)

>>

>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

>b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

>> index 174eb59..3273d8c 100644

>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

>> @@ -570,6 +570,27 @@ static int amdgpu_info_ioctl(struct drm_device

>*dev, void *data, struct drm_file

>>                         return -EINVAL;

>>                 }

>>         }

>> +       case AMDGPU_INFO_NUM_HANDLES: {

>> +               struct drm_amdgpu_info_num_handles handle;

>> +

>> +               switch (info->query_hw_ip.type) {

>> +               case AMDGPU_HW_IP_UVD:

>> +                       /* Starting Polaris, we support unlimited UVD handles */

>> +                       if (adev->asic_type < CHIP_POLARIS10) {

>> +                               handle.uvd_max_handles = adev->uvd.max_handles;

>> +                               handle.uvd_used_handles =

>amdgpu_uvd_used_handles(adev);

>> +

>> +                               return copy_to_user(out, &handle,

>> +                                       min((size_t)size, sizeof(handle))) ? -EFAULT : 0;

>> +                       } else {

>> +                               return -EINVAL;

>Using EINVAL doesn't seem right here. As per man 3 ioctl

>

>      EINVAL The request or arg argument is not valid for this device.

>

>A bit further down you can see the one you want.

>

>      ENODEV The fildes argument refers to a valid STREAMS device, but

>the corresponding device driver does not support the ioctl() function.


Emil, ENODEV would mean the driver does not support ioctl() itself. But in our case ioctl() is supported.

Since we extract the query type from arg passed to ioctl(), and it is this query AMDGPU_INFO_NUM_HANDLES which is not supported by driver (for > Polaris), doesn’t returning EINVAL make more sense here?

Thanks,
Arindam
>

>Worth checking through the existing code and correcting similar thinkos ?

>

>Thanks

>Emil
On 15 December 2016 at 07:30, Nath, Arindam <Arindam.Nath@amd.com> wrote:
>>-----Original Message-----
>>From: Emil Velikov [mailto:emil.l.velikov@gmail.com]
>>Sent: Wednesday, December 14, 2016 9:26 PM
>>To: Nath, Arindam
>>Cc: David Airlie; Deucher, Alexander; amd-gfx mailing list; ML dri-devel;
>>Koenig, Christian
>>Subject: Re: [PATCH 1/1] drm/amd/amdgpu: get maximum and used UVD
>>handles (v3)
>>
>>On 12 December 2016 at 18:49,  <arindam.nath@amd.com> wrote:
>>> From: Arindam Nath <arindam.nath@amd.com>
>>>
>>> Change History
>>> --------------
>>>
>>> v3: changes suggested by Christian
>>> - Add a check for UVD IP block using AMDGPU_HW_IP_UVD
>>>   query type.
>>> - Add a check for asic_type to be less than
>>>   CHIP_POLARIS10 since starting Polaris, we support
>>>   unlimited UVD instances.
>>> - Add kerneldoc style comment for
>>>   amdgpu_uvd_used_handles().
>>>
>>> v2: as suggested by Christian
>>> - Add a new query AMDGPU_INFO_NUM_HANDLES
>>> - Create a helper function to return the number
>>>   of currently used UVD handles.
>>> - Modify the logic to count the number of used
>>>   UVD handles since handles can be freed in
>>>   non-linear fashion.
>>>
>>> v1:
>>> - User might want to query the maximum number of UVD
>>>   instances supported by firmware. In addition to that,
>>>   if there are multiple applications using UVD handles
>>>   at the same time, he might also want to query the
>>>   currently used number of handles.
>>>
>>>   For this we add two variables max_handles and
>>>   used_handles inside drm_amdgpu_info_hw_ip. So now
>>>   an application (or libdrm) can use AMDGPU_INFO IOCTL
>>>   with AMDGPU_INFO_HW_IP_INFO query type to get these
>>>   values.
>>>
>>> Signed-off-by: Arindam Nath <arindam.nath@amd.com>
>>> Reviewed-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 21
>>+++++++++++++++++++++
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 25
>>+++++++++++++++++++++++++
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  1 +
>>>  include/uapi/drm/amdgpu_drm.h           |  9 +++++++++
>>>  4 files changed, 56 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> index 174eb59..3273d8c 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> @@ -570,6 +570,27 @@ static int amdgpu_info_ioctl(struct drm_device
>>*dev, void *data, struct drm_file
>>>                         return -EINVAL;
>>>                 }
>>>         }
>>> +       case AMDGPU_INFO_NUM_HANDLES: {
>>> +               struct drm_amdgpu_info_num_handles handle;
>>> +
>>> +               switch (info->query_hw_ip.type) {
>>> +               case AMDGPU_HW_IP_UVD:
>>> +                       /* Starting Polaris, we support unlimited UVD handles */
>>> +                       if (adev->asic_type < CHIP_POLARIS10) {
>>> +                               handle.uvd_max_handles = adev->uvd.max_handles;
>>> +                               handle.uvd_used_handles =
>>amdgpu_uvd_used_handles(adev);
>>> +
>>> +                               return copy_to_user(out, &handle,
>>> +                                       min((size_t)size, sizeof(handle))) ? -EFAULT : 0;
>>> +                       } else {
>>> +                               return -EINVAL;
>>Using EINVAL doesn't seem right here. As per man 3 ioctl
>>
>>      EINVAL The request or arg argument is not valid for this device.
>>
>>A bit further down you can see the one you want.
>>
>>      ENODEV The fildes argument refers to a valid STREAMS device, but
>>the corresponding device driver does not support the ioctl() function.
>
> Emil, ENODEV would mean the driver does not support ioctl() itself. But in our case ioctl() is supported.
>
> Since we extract the query type from arg passed to ioctl(), and it is this query AMDGPU_INFO_NUM_HANDLES which is not supported by driver (for > Polaris), doesn’t returning EINVAL make more sense here?
>
Unless I'm reading the code incorrectly - CHIP_POLARIS10 and older do
not have support the query. Thus ENODEV is the one you want to use.
Furthermore EINVAL is (mostly) used to indicate incorrect input
(failed input validation) which userspace uses to check if kernel is
too old/does not support X.

If in doubt I recommend checking how other drivers are handling
things. From memory at least i915 uses the above.

Thanks
Emil
>-----Original Message-----

>From: Emil Velikov [mailto:emil.l.velikov@gmail.com]

>Sent: Thursday, December 15, 2016 5:01 PM

>To: Nath, Arindam

>Cc: David Airlie; Deucher, Alexander; amd-gfx mailing list; ML dri-devel;

>Koenig, Christian

>Subject: Re: [PATCH 1/1] drm/amd/amdgpu: get maximum and used UVD

>handles (v3)

>

>On 15 December 2016 at 07:30, Nath, Arindam <Arindam.Nath@amd.com>

>wrote:

>>>-----Original Message-----

>>>From: Emil Velikov [mailto:emil.l.velikov@gmail.com]

>>>Sent: Wednesday, December 14, 2016 9:26 PM

>>>To: Nath, Arindam

>>>Cc: David Airlie; Deucher, Alexander; amd-gfx mailing list; ML dri-devel;

>>>Koenig, Christian

>>>Subject: Re: [PATCH 1/1] drm/amd/amdgpu: get maximum and used UVD

>>>handles (v3)

>>>

>>>On 12 December 2016 at 18:49,  <arindam.nath@amd.com> wrote:

>>>> From: Arindam Nath <arindam.nath@amd.com>

>>>>

>>>> Change History

>>>> --------------

>>>>

>>>> v3: changes suggested by Christian

>>>> - Add a check for UVD IP block using AMDGPU_HW_IP_UVD

>>>>   query type.

>>>> - Add a check for asic_type to be less than

>>>>   CHIP_POLARIS10 since starting Polaris, we support

>>>>   unlimited UVD instances.

>>>> - Add kerneldoc style comment for

>>>>   amdgpu_uvd_used_handles().

>>>>

>>>> v2: as suggested by Christian

>>>> - Add a new query AMDGPU_INFO_NUM_HANDLES

>>>> - Create a helper function to return the number

>>>>   of currently used UVD handles.

>>>> - Modify the logic to count the number of used

>>>>   UVD handles since handles can be freed in

>>>>   non-linear fashion.

>>>>

>>>> v1:

>>>> - User might want to query the maximum number of UVD

>>>>   instances supported by firmware. In addition to that,

>>>>   if there are multiple applications using UVD handles

>>>>   at the same time, he might also want to query the

>>>>   currently used number of handles.

>>>>

>>>>   For this we add two variables max_handles and

>>>>   used_handles inside drm_amdgpu_info_hw_ip. So now

>>>>   an application (or libdrm) can use AMDGPU_INFO IOCTL

>>>>   with AMDGPU_INFO_HW_IP_INFO query type to get these

>>>>   values.

>>>>

>>>> Signed-off-by: Arindam Nath <arindam.nath@amd.com>

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

>>>> ---

>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 21

>>>+++++++++++++++++++++

>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 25

>>>+++++++++++++++++++++++++

>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  1 +

>>>>  include/uapi/drm/amdgpu_drm.h           |  9 +++++++++

>>>>  4 files changed, 56 insertions(+)

>>>>

>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

>>>b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

>>>> index 174eb59..3273d8c 100644

>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

>>>> @@ -570,6 +570,27 @@ static int amdgpu_info_ioctl(struct drm_device

>>>*dev, void *data, struct drm_file

>>>>                         return -EINVAL;

>>>>                 }

>>>>         }

>>>> +       case AMDGPU_INFO_NUM_HANDLES: {

>>>> +               struct drm_amdgpu_info_num_handles handle;

>>>> +

>>>> +               switch (info->query_hw_ip.type) {

>>>> +               case AMDGPU_HW_IP_UVD:

>>>> +                       /* Starting Polaris, we support unlimited UVD handles */

>>>> +                       if (adev->asic_type < CHIP_POLARIS10) {

>>>> +                               handle.uvd_max_handles = adev->uvd.max_handles;

>>>> +                               handle.uvd_used_handles =

>>>amdgpu_uvd_used_handles(adev);

>>>> +

>>>> +                               return copy_to_user(out, &handle,

>>>> +                                       min((size_t)size, sizeof(handle))) ? -EFAULT : 0;

>>>> +                       } else {

>>>> +                               return -EINVAL;

>>>Using EINVAL doesn't seem right here. As per man 3 ioctl

>>>

>>>      EINVAL The request or arg argument is not valid for this device.

>>>

>>>A bit further down you can see the one you want.

>>>

>>>      ENODEV The fildes argument refers to a valid STREAMS device, but

>>>the corresponding device driver does not support the ioctl() function.

>>

>> Emil, ENODEV would mean the driver does not support ioctl() itself. But in

>our case ioctl() is supported.

>>

>> Since we extract the query type from arg passed to ioctl(), and it is this

>query AMDGPU_INFO_NUM_HANDLES which is not supported by driver (for

>> Polaris), doesn’t returning EINVAL make more sense here?

>>

>Unless I'm reading the code incorrectly - CHIP_POLARIS10 and older do

>not have support the query. Thus ENODEV is the one you want to use.

>Furthermore EINVAL is (mostly) used to indicate incorrect input

>(failed input validation) which userspace uses to check if kernel is

>too old/does not support X.


Actually, the code says only asics older than CHIP_POLARIS10 support the query, beyond it doesn’t.
>

>If in doubt I recommend checking how other drivers are handling

>things. From memory at least i915 uses the above.


I will check how other drivers handle this.

Thanks,
Arindam
>

>Thanks

>Emil
On Thu, Dec 15, 2016 at 1:47 PM, Nath, Arindam <Arindam.Nath@amd.com> wrote:
>>-----Original Message-----
>>From: Emil Velikov [mailto:emil.l.velikov@gmail.com]
>>Sent: Thursday, December 15, 2016 5:01 PM
>>To: Nath, Arindam
>>Cc: David Airlie; Deucher, Alexander; amd-gfx mailing list; ML dri-devel;
>>Koenig, Christian
>>Subject: Re: [PATCH 1/1] drm/amd/amdgpu: get maximum and used UVD
>>handles (v3)
>>
>>On 15 December 2016 at 07:30, Nath, Arindam <Arindam.Nath@amd.com>
>>wrote:
>>>>-----Original Message-----
>>>>From: Emil Velikov [mailto:emil.l.velikov@gmail.com]
>>>>Sent: Wednesday, December 14, 2016 9:26 PM
>>>>To: Nath, Arindam
>>>>Cc: David Airlie; Deucher, Alexander; amd-gfx mailing list; ML dri-devel;
>>>>Koenig, Christian
>>>>Subject: Re: [PATCH 1/1] drm/amd/amdgpu: get maximum and used UVD
>>>>handles (v3)
>>>>
>>>>On 12 December 2016 at 18:49,  <arindam.nath@amd.com> wrote:
>>>>> From: Arindam Nath <arindam.nath@amd.com>
>>>>>
>>>>> Change History
>>>>> --------------
>>>>>
>>>>> v3: changes suggested by Christian
>>>>> - Add a check for UVD IP block using AMDGPU_HW_IP_UVD
>>>>>   query type.
>>>>> - Add a check for asic_type to be less than
>>>>>   CHIP_POLARIS10 since starting Polaris, we support
>>>>>   unlimited UVD instances.
>>>>> - Add kerneldoc style comment for
>>>>>   amdgpu_uvd_used_handles().
>>>>>
>>>>> v2: as suggested by Christian
>>>>> - Add a new query AMDGPU_INFO_NUM_HANDLES
>>>>> - Create a helper function to return the number
>>>>>   of currently used UVD handles.
>>>>> - Modify the logic to count the number of used
>>>>>   UVD handles since handles can be freed in
>>>>>   non-linear fashion.
>>>>>
>>>>> v1:
>>>>> - User might want to query the maximum number of UVD
>>>>>   instances supported by firmware. In addition to that,
>>>>>   if there are multiple applications using UVD handles
>>>>>   at the same time, he might also want to query the
>>>>>   currently used number of handles.
>>>>>
>>>>>   For this we add two variables max_handles and
>>>>>   used_handles inside drm_amdgpu_info_hw_ip. So now
>>>>>   an application (or libdrm) can use AMDGPU_INFO IOCTL
>>>>>   with AMDGPU_INFO_HW_IP_INFO query type to get these
>>>>>   values.
>>>>>
>>>>> Signed-off-by: Arindam Nath <arindam.nath@amd.com>
>>>>> Reviewed-by: Christian König <christian.koenig@amd.com>
>>>>> ---
>>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 21
>>>>+++++++++++++++++++++
>>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 25
>>>>+++++++++++++++++++++++++
>>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  1 +
>>>>>  include/uapi/drm/amdgpu_drm.h           |  9 +++++++++
>>>>>  4 files changed, 56 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>>b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>>> index 174eb59..3273d8c 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>>> @@ -570,6 +570,27 @@ static int amdgpu_info_ioctl(struct drm_device
>>>>*dev, void *data, struct drm_file
>>>>>                         return -EINVAL;
>>>>>                 }
>>>>>         }
>>>>> +       case AMDGPU_INFO_NUM_HANDLES: {
>>>>> +               struct drm_amdgpu_info_num_handles handle;
>>>>> +
>>>>> +               switch (info->query_hw_ip.type) {
>>>>> +               case AMDGPU_HW_IP_UVD:
>>>>> +                       /* Starting Polaris, we support unlimited UVD handles */
>>>>> +                       if (adev->asic_type < CHIP_POLARIS10) {
>>>>> +                               handle.uvd_max_handles = adev->uvd.max_handles;
>>>>> +                               handle.uvd_used_handles =
>>>>amdgpu_uvd_used_handles(adev);
>>>>> +
>>>>> +                               return copy_to_user(out, &handle,
>>>>> +                                       min((size_t)size, sizeof(handle))) ? -EFAULT : 0;
>>>>> +                       } else {
>>>>> +                               return -EINVAL;
>>>>Using EINVAL doesn't seem right here. As per man 3 ioctl
>>>>
>>>>      EINVAL The request or arg argument is not valid for this device.
>>>>
>>>>A bit further down you can see the one you want.
>>>>
>>>>      ENODEV The fildes argument refers to a valid STREAMS device, but
>>>>the corresponding device driver does not support the ioctl() function.
>>>
>>> Emil, ENODEV would mean the driver does not support ioctl() itself. But in
>>our case ioctl() is supported.
>>>
>>> Since we extract the query type from arg passed to ioctl(), and it is this
>>query AMDGPU_INFO_NUM_HANDLES which is not supported by driver (for
>>> Polaris), doesn’t returning EINVAL make more sense here?
>>>
>>Unless I'm reading the code incorrectly - CHIP_POLARIS10 and older do
>>not have support the query. Thus ENODEV is the one you want to use.
>>Furthermore EINVAL is (mostly) used to indicate incorrect input
>>(failed input validation) which userspace uses to check if kernel is
>>too old/does not support X.
>
> Actually, the code says only asics older than CHIP_POLARIS10 support the query, beyond it doesn’t.

Wouldn't it be cleaner to return INT_MAX or something like that?
Right now the interface is inconsistent, it's failing the same way as
if userspace passed something invalid, while (IMHO) it should just say
"unlimited".

Gražvydas
>-----Original Message-----

>From: Grazvydas Ignotas [mailto:notasas@gmail.com]

>Sent: Thursday, December 15, 2016 5:52 PM

>To: Nath, Arindam

>Cc: Emil Velikov; David Airlie; Deucher, Alexander; ML dri-devel; amd-gfx

>mailing list; Koenig, Christian

>Subject: Re: [PATCH 1/1] drm/amd/amdgpu: get maximum and used UVD

>handles (v3)

>

>On Thu, Dec 15, 2016 at 1:47 PM, Nath, Arindam <Arindam.Nath@amd.com>

>wrote:

>>>-----Original Message-----

>>>From: Emil Velikov [mailto:emil.l.velikov@gmail.com]

>>>Sent: Thursday, December 15, 2016 5:01 PM

>>>To: Nath, Arindam

>>>Cc: David Airlie; Deucher, Alexander; amd-gfx mailing list; ML dri-devel;

>>>Koenig, Christian

>>>Subject: Re: [PATCH 1/1] drm/amd/amdgpu: get maximum and used UVD

>>>handles (v3)

>>>

>>>On 15 December 2016 at 07:30, Nath, Arindam <Arindam.Nath@amd.com>

>>>wrote:

>>>>>-----Original Message-----

>>>>>From: Emil Velikov [mailto:emil.l.velikov@gmail.com]

>>>>>Sent: Wednesday, December 14, 2016 9:26 PM

>>>>>To: Nath, Arindam

>>>>>Cc: David Airlie; Deucher, Alexander; amd-gfx mailing list; ML dri-devel;

>>>>>Koenig, Christian

>>>>>Subject: Re: [PATCH 1/1] drm/amd/amdgpu: get maximum and used

>UVD

>>>>>handles (v3)

>>>>>

>>>>>On 12 December 2016 at 18:49,  <arindam.nath@amd.com> wrote:

>>>>>> From: Arindam Nath <arindam.nath@amd.com>

>>>>>>

>>>>>> Change History

>>>>>> --------------

>>>>>>

>>>>>> v3: changes suggested by Christian

>>>>>> - Add a check for UVD IP block using AMDGPU_HW_IP_UVD

>>>>>>   query type.

>>>>>> - Add a check for asic_type to be less than

>>>>>>   CHIP_POLARIS10 since starting Polaris, we support

>>>>>>   unlimited UVD instances.

>>>>>> - Add kerneldoc style comment for

>>>>>>   amdgpu_uvd_used_handles().

>>>>>>

>>>>>> v2: as suggested by Christian

>>>>>> - Add a new query AMDGPU_INFO_NUM_HANDLES

>>>>>> - Create a helper function to return the number

>>>>>>   of currently used UVD handles.

>>>>>> - Modify the logic to count the number of used

>>>>>>   UVD handles since handles can be freed in

>>>>>>   non-linear fashion.

>>>>>>

>>>>>> v1:

>>>>>> - User might want to query the maximum number of UVD

>>>>>>   instances supported by firmware. In addition to that,

>>>>>>   if there are multiple applications using UVD handles

>>>>>>   at the same time, he might also want to query the

>>>>>>   currently used number of handles.

>>>>>>

>>>>>>   For this we add two variables max_handles and

>>>>>>   used_handles inside drm_amdgpu_info_hw_ip. So now

>>>>>>   an application (or libdrm) can use AMDGPU_INFO IOCTL

>>>>>>   with AMDGPU_INFO_HW_IP_INFO query type to get these

>>>>>>   values.

>>>>>>

>>>>>> Signed-off-by: Arindam Nath <arindam.nath@amd.com>

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

>>>>>> ---

>>>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 21

>>>>>+++++++++++++++++++++

>>>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 25

>>>>>+++++++++++++++++++++++++

>>>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  1 +

>>>>>>  include/uapi/drm/amdgpu_drm.h           |  9 +++++++++

>>>>>>  4 files changed, 56 insertions(+)

>>>>>>

>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

>>>>>b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

>>>>>> index 174eb59..3273d8c 100644

>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

>>>>>> @@ -570,6 +570,27 @@ static int amdgpu_info_ioctl(struct drm_device

>>>>>*dev, void *data, struct drm_file

>>>>>>                         return -EINVAL;

>>>>>>                 }

>>>>>>         }

>>>>>> +       case AMDGPU_INFO_NUM_HANDLES: {

>>>>>> +               struct drm_amdgpu_info_num_handles handle;

>>>>>> +

>>>>>> +               switch (info->query_hw_ip.type) {

>>>>>> +               case AMDGPU_HW_IP_UVD:

>>>>>> +                       /* Starting Polaris, we support unlimited UVD handles */

>>>>>> +                       if (adev->asic_type < CHIP_POLARIS10) {

>>>>>> +                               handle.uvd_max_handles = adev->uvd.max_handles;

>>>>>> +                               handle.uvd_used_handles =

>>>>>amdgpu_uvd_used_handles(adev);

>>>>>> +

>>>>>> +                               return copy_to_user(out, &handle,

>>>>>> +                                       min((size_t)size, sizeof(handle))) ? -EFAULT : 0;

>>>>>> +                       } else {

>>>>>> +                               return -EINVAL;

>>>>>Using EINVAL doesn't seem right here. As per man 3 ioctl

>>>>>

>>>>>      EINVAL The request or arg argument is not valid for this device.

>>>>>

>>>>>A bit further down you can see the one you want.

>>>>>

>>>>>      ENODEV The fildes argument refers to a valid STREAMS device, but

>>>>>the corresponding device driver does not support the ioctl() function.

>>>>

>>>> Emil, ENODEV would mean the driver does not support ioctl() itself. But in

>>>our case ioctl() is supported.

>>>>

>>>> Since we extract the query type from arg passed to ioctl(), and it is this

>>>query AMDGPU_INFO_NUM_HANDLES which is not supported by driver

>(for

>>>> Polaris), doesn’t returning EINVAL make more sense here?

>>>>

>>>Unless I'm reading the code incorrectly - CHIP_POLARIS10 and older do

>>>not have support the query. Thus ENODEV is the one you want to use.

>>>Furthermore EINVAL is (mostly) used to indicate incorrect input

>>>(failed input validation) which userspace uses to check if kernel is

>>>too old/does not support X.

>>

>> Actually, the code says only asics older than CHIP_POLARIS10 support the

>query, beyond it doesn’t.

>

>Wouldn't it be cleaner to return INT_MAX or something like that?

>Right now the interface is inconsistent, it's failing the same way as

>if userspace passed something invalid, while (IMHO) it should just say

>"unlimited".


We need to return an error no. consistent with what ioctl() expects to return. This will enable userspace to take corrective action (or no action at all) depending on the error code.

Thanks,
Arindam
>

>Gražvydas
Am 15.12.2016 um 13:33 schrieb Nath, Arindam:
>> -----Original Message-----
>> From: Grazvydas Ignotas [mailto:notasas@gmail.com]
>> Sent: Thursday, December 15, 2016 5:52 PM
>> To: Nath, Arindam
>> Cc: Emil Velikov; David Airlie; Deucher, Alexander; ML dri-devel; amd-gfx
>> mailing list; Koenig, Christian
>> Subject: Re: [PATCH 1/1] drm/amd/amdgpu: get maximum and used UVD
>> handles (v3)
>>
>> On Thu, Dec 15, 2016 at 1:47 PM, Nath, Arindam <Arindam.Nath@amd.com>
>> wrote:
>>>> -----Original Message-----
>>>> From: Emil Velikov [mailto:emil.l.velikov@gmail.com]
>>>> Sent: Thursday, December 15, 2016 5:01 PM
>>>> To: Nath, Arindam
>>>> Cc: David Airlie; Deucher, Alexander; amd-gfx mailing list; ML dri-devel;
>>>> Koenig, Christian
>>>> Subject: Re: [PATCH 1/1] drm/amd/amdgpu: get maximum and used UVD
>>>> handles (v3)
>>>>
>>>> On 15 December 2016 at 07:30, Nath, Arindam <Arindam.Nath@amd.com>
>>>> wrote:
>>>>>> -----Original Message-----
>>>>>> From: Emil Velikov [mailto:emil.l.velikov@gmail.com]
>>>>>> Sent: Wednesday, December 14, 2016 9:26 PM
>>>>>> To: Nath, Arindam
>>>>>> Cc: David Airlie; Deucher, Alexander; amd-gfx mailing list; ML dri-devel;
>>>>>> Koenig, Christian
>>>>>> Subject: Re: [PATCH 1/1] drm/amd/amdgpu: get maximum and used
>> UVD
>>>>>> handles (v3)
>>>>>>
>>>>>> On 12 December 2016 at 18:49,  <arindam.nath@amd.com> wrote:
>>>>>>> From: Arindam Nath <arindam.nath@amd.com>
>>>>>>>
>>>>>>> Change History
>>>>>>> --------------
>>>>>>>
>>>>>>> v3: changes suggested by Christian
>>>>>>> - Add a check for UVD IP block using AMDGPU_HW_IP_UVD
>>>>>>>    query type.
>>>>>>> - Add a check for asic_type to be less than
>>>>>>>    CHIP_POLARIS10 since starting Polaris, we support
>>>>>>>    unlimited UVD instances.
>>>>>>> - Add kerneldoc style comment for
>>>>>>>    amdgpu_uvd_used_handles().
>>>>>>>
>>>>>>> v2: as suggested by Christian
>>>>>>> - Add a new query AMDGPU_INFO_NUM_HANDLES
>>>>>>> - Create a helper function to return the number
>>>>>>>    of currently used UVD handles.
>>>>>>> - Modify the logic to count the number of used
>>>>>>>    UVD handles since handles can be freed in
>>>>>>>    non-linear fashion.
>>>>>>>
>>>>>>> v1:
>>>>>>> - User might want to query the maximum number of UVD
>>>>>>>    instances supported by firmware. In addition to that,
>>>>>>>    if there are multiple applications using UVD handles
>>>>>>>    at the same time, he might also want to query the
>>>>>>>    currently used number of handles.
>>>>>>>
>>>>>>>    For this we add two variables max_handles and
>>>>>>>    used_handles inside drm_amdgpu_info_hw_ip. So now
>>>>>>>    an application (or libdrm) can use AMDGPU_INFO IOCTL
>>>>>>>    with AMDGPU_INFO_HW_IP_INFO query type to get these
>>>>>>>    values.
>>>>>>>
>>>>>>> Signed-off-by: Arindam Nath <arindam.nath@amd.com>
>>>>>>> Reviewed-by: Christian König <christian.koenig@amd.com>
>>>>>>> ---
>>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 21
>>>>>> +++++++++++++++++++++
>>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 25
>>>>>> +++++++++++++++++++++++++
>>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  1 +
>>>>>>>   include/uapi/drm/amdgpu_drm.h           |  9 +++++++++
>>>>>>>   4 files changed, 56 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>>>>> index 174eb59..3273d8c 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>>>>> @@ -570,6 +570,27 @@ static int amdgpu_info_ioctl(struct drm_device
>>>>>> *dev, void *data, struct drm_file
>>>>>>>                          return -EINVAL;
>>>>>>>                  }
>>>>>>>          }
>>>>>>> +       case AMDGPU_INFO_NUM_HANDLES: {
>>>>>>> +               struct drm_amdgpu_info_num_handles handle;
>>>>>>> +
>>>>>>> +               switch (info->query_hw_ip.type) {
>>>>>>> +               case AMDGPU_HW_IP_UVD:
>>>>>>> +                       /* Starting Polaris, we support unlimited UVD handles */
>>>>>>> +                       if (adev->asic_type < CHIP_POLARIS10) {
>>>>>>> +                               handle.uvd_max_handles = adev->uvd.max_handles;
>>>>>>> +                               handle.uvd_used_handles =
>>>>>> amdgpu_uvd_used_handles(adev);
>>>>>>> +
>>>>>>> +                               return copy_to_user(out, &handle,
>>>>>>> +                                       min((size_t)size, sizeof(handle))) ? -EFAULT : 0;
>>>>>>> +                       } else {
>>>>>>> +                               return -EINVAL;
>>>>>> Using EINVAL doesn't seem right here. As per man 3 ioctl
>>>>>>
>>>>>>       EINVAL The request or arg argument is not valid for this device.
>>>>>>
>>>>>> A bit further down you can see the one you want.
>>>>>>
>>>>>>       ENODEV The fildes argument refers to a valid STREAMS device, but
>>>>>> the corresponding device driver does not support the ioctl() function.
>>>>> Emil, ENODEV would mean the driver does not support ioctl() itself. But in
>>>> our case ioctl() is supported.
>>>>> Since we extract the query type from arg passed to ioctl(), and it is this
>>>> query AMDGPU_INFO_NUM_HANDLES which is not supported by driver
>> (for
>>>>> Polaris), doesn’t returning EINVAL make more sense here?
>>>>>
>>>> Unless I'm reading the code incorrectly - CHIP_POLARIS10 and older do
>>>> not have support the query. Thus ENODEV is the one you want to use.
>>>> Furthermore EINVAL is (mostly) used to indicate incorrect input
>>>> (failed input validation) which userspace uses to check if kernel is
>>>> too old/does not support X.
>>> Actually, the code says only asics older than CHIP_POLARIS10 support the
>> query, beyond it doesn’t.
>>
>> Wouldn't it be cleaner to return INT_MAX or something like that?
>> Right now the interface is inconsistent, it's failing the same way as
>> if userspace passed something invalid, while (IMHO) it should just say
>> "unlimited".
> We need to return an error no. consistent with what ioctl() expects to return. This will enable userspace to take corrective action (or no action at all) depending on the error code.

Well, we could return INT_MAX for the maximum numbers of handles to note 
that there is not limit, but I also prefer returning an error code to 
note that this won't work as expected.

Regarding which error code to return I think that Emil has the right 
idea here.

Returning -EINVAL usually means that userspace provided an invalid 
value, but in this case it doesn't matter which value the UMD provide 
all of them would be invalid because starting with Polaris the 
hardware/firmware simply doesn't work this way any more.

So using -ENODEV or maybe -ENODATA indeed sound like the right think to 
do here.

Regards,
Christian.

>
> Thanks,
> Arindam
>> Gražvydas
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On Thu, Dec 15, 2016 at 4:12 PM, Christian König
<deathsimple@vodafone.de> wrote:
>
> Regarding which error code to return I think that Emil has the right idea
> here.
>
> Returning -EINVAL usually means that userspace provided an invalid value,
> but in this case it doesn't matter which value the UMD provide all of them
> would be invalid because starting with Polaris the hardware/firmware simply
> doesn't work this way any more.
>
> So using -ENODEV or maybe -ENODATA indeed sound like the right think to do
> here.

What about ERANGE then, "Math result not representable" aka infinity?
To me ENODEV is more like "the GPU you are asking about is not there"
and ENODATA "information you ask for is not known", although the later
still somewhat makes sense.

Gražvydas
Am 15.12.2016 um 23:43 schrieb Grazvydas Ignotas:
> On Thu, Dec 15, 2016 at 4:12 PM, Christian König
> <deathsimple@vodafone.de> wrote:
>> Regarding which error code to return I think that Emil has the right idea
>> here.
>>
>> Returning -EINVAL usually means that userspace provided an invalid value,
>> but in this case it doesn't matter which value the UMD provide all of them
>> would be invalid because starting with Polaris the hardware/firmware simply
>> doesn't work this way any more.
>>
>> So using -ENODEV or maybe -ENODATA indeed sound like the right think to do
>> here.
> What about ERANGE then, "Math result not representable" aka infinity?
> To me ENODEV is more like "the GPU you are asking about is not there"
> and ENODATA "information you ask for is not known", although the later
> still somewhat makes sense.

Sorry for the delayed reply.

I think EDATA would still make more sense than ERANGE, because the 
number of open sessions is simply not known any more.

That the maximum number of sessions is now unlimited is just a side 
effect of the new handling if you ask me.

Regards,
Christian.

>
> Gražvydas