[PATCH] drm/amdgpu: simplify nv and soc21 read_register functions

[Christian König]

Am 05.05.22 um 04:58 schrieb Alex Deucher:
> Check of the base offset for the IP exists rather than
> explicitly checking for how many instances of a particular
> IP there are.  This is what soc15.c already does.  Expand
> this to nv.c and soc21.c.
>
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>

I still think the higher level code should not read registers which 
aren't present in the first place. Could we add a WARN_ONCE() here maybe?

On the other hand I totally see why it is a good idea to not try to 
access something which isn't present. So Acked-by: Christian König 
<christian.koenig at amd.com> for now.

> ---
>   drivers/gpu/drm/amd/amdgpu/nv.c    | 6 +++---
>   drivers/gpu/drm/amd/amdgpu/soc21.c | 5 +++--
>   2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> index 8ecfd66c4cee..d016e3c3e221 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -392,9 +392,9 @@ static int nv_read_register(struct amdgpu_device *adev, u32 se_num,
>   	*value = 0;
>   	for (i = 0; i < ARRAY_SIZE(nv_allowed_read_registers); i++) {
>   		en = &nv_allowed_read_registers[i];
> -		if ((i == 7 && (adev->sdma.num_instances == 1)) || /* some asics don't have SDMA1 */
> -		    reg_offset !=
> -		    (adev->reg_offset[en->hwip][en->inst][en->seg] + en->reg_offset))
> +		if (adev->reg_offset[en->hwip][en->inst] &&
> +		    reg_offset != (adev->reg_offset[en->hwip][en->inst][en->seg]
> +				   + en->reg_offset))
>   			continue;
>   
>   		*value = nv_get_register_value(adev,
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c
> index 9e689a1f2ea4..2f23cb8cd6e2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc21.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
> @@ -252,8 +252,9 @@ static int soc21_read_register(struct amdgpu_device *adev, u32 se_num,
>   	*value = 0;
>   	for (i = 0; i < ARRAY_SIZE(soc21_allowed_read_registers); i++) {
>   		en = &soc21_allowed_read_registers[i];
> -		if (reg_offset !=
> -		    (adev->reg_offset[en->hwip][en->inst][en->seg] + en->reg_offset))
> +		if (adev->reg_offset[en->hwip][en->inst] &&
> +		    reg_offset != (adev->reg_offset[en->hwip][en->inst][en->seg]
> +				   + en->reg_offset))
>   			continue;
>   
>   		*value = soc21_get_register_value(adev,