RE: [PATCH 2/5] drm/amdgpu: Init pcie_index/data address as fallback

2024-01-02 Thread Zhang, Hawking 
[AMD Official Use Only - General]

Sure, will do in v2

Regards,
Hawking

-Original Message-
From: Alex Deucher 
Sent: Wednesday, January 3, 2024 03:44
To: Zhang, Hawking 
Cc: amd-gfx@lists.freedesktop.org; Zhou1, Tao ; Yang, 
Stanley ; Wang, Yang(Kevin) ; 
Chai, Thomas ; Li, Candice ; Deucher, 
Alexander ; Ma, Le ; Lazar, Lijo 

Subject: Re: [PATCH 2/5] drm/amdgpu: Init pcie_index/data address as fallback

On Mon, Jan 1, 2024 at 10:50 PM Hawking Zhang  wrote:
>
> To allow using this helper for indirect access when nbio funcs is not
> available. For instance, in ip discovery phase.
>
> Signed-off-by: Hawking Zhang 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 +++-
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 001a35fa0f19..873419a5b9aa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -781,12 +781,22 @@ u32 amdgpu_device_indirect_rreg_ext(struct 
> amdgpu_device *adev,
> void __iomem *pcie_index_hi_offset;
> void __iomem *pcie_data_offset;
>
> -   pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
> -   pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
> -   if ((reg_addr >> 32) && (adev->nbio.funcs->get_pcie_index_hi_offset))
> -   pcie_index_hi = 
> adev->nbio.funcs->get_pcie_index_hi_offset(adev);
> -   else
> +   if (unlikely(!adev->nbio.funcs)) {
> +   pcie_index = (0x38 >> 2);
> +   pcie_data = (0x3C >> 2);
> +   } else {
> +   pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
> +   pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
> +   }
> +
> +   if (reg_addr >> 32) {
> +   if (unlikely(!adev->nbio.funcs))
> +   pcie_index_hi = (0x44 >> 2);

I'd still use a define for these, E.g.,

#define AMDGPU_PCIE_INDEX_FALLBACK (0x38 >> 2) etc.
Or something similar.

Alex

> +   else
> +   pcie_index_hi = 
> adev->nbio.funcs->get_pcie_index_hi_offset(adev);
> +   } else {
> pcie_index_hi = 0;
> +   }
>
> spin_lock_irqsave(>pcie_idx_lock, flags);
> pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index *
> 4;
> --
> 2.17.1
>

Re: [PATCH 2/5] drm/amdgpu: Init pcie_index/data address as fallback

2024-01-02 Thread Alex Deucher 
On Mon, Jan 1, 2024 at 10:50 PM Hawking Zhang  wrote:
>
> To allow using this helper for indirect access when
> nbio funcs is not available. For instance, in ip
> discovery phase.
>
> Signed-off-by: Hawking Zhang 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 +++-
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 001a35fa0f19..873419a5b9aa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -781,12 +781,22 @@ u32 amdgpu_device_indirect_rreg_ext(struct 
> amdgpu_device *adev,
> void __iomem *pcie_index_hi_offset;
> void __iomem *pcie_data_offset;
>
> -   pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
> -   pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
> -   if ((reg_addr >> 32) && (adev->nbio.funcs->get_pcie_index_hi_offset))
> -   pcie_index_hi = 
> adev->nbio.funcs->get_pcie_index_hi_offset(adev);
> -   else
> +   if (unlikely(!adev->nbio.funcs)) {
> +   pcie_index = (0x38 >> 2);
> +   pcie_data = (0x3C >> 2);
> +   } else {
> +   pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
> +   pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
> +   }
> +
> +   if (reg_addr >> 32) {
> +   if (unlikely(!adev->nbio.funcs))
> +   pcie_index_hi = (0x44 >> 2);

I'd still use a define for these, E.g.,

#define AMDGPU_PCIE_INDEX_FALLBACK (0x38 >> 2)
etc.
Or something similar.

Alex

> +   else
> +   pcie_index_hi = 
> adev->nbio.funcs->get_pcie_index_hi_offset(adev);
> +   } else {
> pcie_index_hi = 0;
> +   }
>
> spin_lock_irqsave(>pcie_idx_lock, flags);
> pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
> --
> 2.17.1
>

[PATCH 2/5] drm/amdgpu: Init pcie_index/data address as fallback

2024-01-02 Thread Hawking Zhang 
To allow using this helper for indirect access when
nbio funcs is not available. For instance, in ip
discovery phase.

Signed-off-by: Hawking Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 001a35fa0f19..873419a5b9aa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -781,12 +781,22 @@ u32 amdgpu_device_indirect_rreg_ext(struct amdgpu_device 
*adev,
void __iomem *pcie_index_hi_offset;
void __iomem *pcie_data_offset;
 
-   pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
-   pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
-   if ((reg_addr >> 32) && (adev->nbio.funcs->get_pcie_index_hi_offset))
-   pcie_index_hi = 
adev->nbio.funcs->get_pcie_index_hi_offset(adev);
-   else
+   if (unlikely(!adev->nbio.funcs)) {
+   pcie_index = (0x38 >> 2);
+   pcie_data = (0x3C >> 2);
+   } else {
+   pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
+   pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
+   }
+
+   if (reg_addr >> 32) {
+   if (unlikely(!adev->nbio.funcs))
+   pcie_index_hi = (0x44 >> 2);
+   else
+   pcie_index_hi = 
adev->nbio.funcs->get_pcie_index_hi_offset(adev);
+   } else {
pcie_index_hi = 0;
+   }
 
spin_lock_irqsave(>pcie_idx_lock, flags);
pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
-- 
2.17.1

[PATCH 2/5] drm/amdgpu: Init pcie_index/data address as fallback

2024-01-01 Thread Hawking Zhang 
To allow using this helper for indirect access when
nbio funcs is not available. For instance, in ip
discovery phase.

Signed-off-by: Hawking Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 001a35fa0f19..873419a5b9aa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -781,12 +781,22 @@ u32 amdgpu_device_indirect_rreg_ext(struct amdgpu_device 
*adev,
void __iomem *pcie_index_hi_offset;
void __iomem *pcie_data_offset;
 
-   pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
-   pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
-   if ((reg_addr >> 32) && (adev->nbio.funcs->get_pcie_index_hi_offset))
-   pcie_index_hi = 
adev->nbio.funcs->get_pcie_index_hi_offset(adev);
-   else
+   if (unlikely(!adev->nbio.funcs)) {
+   pcie_index = (0x38 >> 2);
+   pcie_data = (0x3C >> 2);
+   } else {
+   pcie_index = adev->nbio.funcs->get_pcie_index_offset(adev);
+   pcie_data = adev->nbio.funcs->get_pcie_data_offset(adev);
+   }
+
+   if (reg_addr >> 32) {
+   if (unlikely(!adev->nbio.funcs))
+   pcie_index_hi = (0x44 >> 2);
+   else
+   pcie_index_hi = 
adev->nbio.funcs->get_pcie_index_hi_offset(adev);
+   } else {
pcie_index_hi = 0;
+   }
 
spin_lock_irqsave(>pcie_idx_lock, flags);
pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
-- 
2.17.1