[PATCH] drm/amdgpu: simplify amdgpu_ras_eeprom.c

[Luben Tuikov]

On 2023-03-27 20:11, Alex Deucher wrote:
> All chips that support RAS also support IP discovery, so
> use the IP versions rather than a mix of IP versions and
> asic types.
> 
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> Cc: Luben Tuikov <luben.tuikov at amd.com>
> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c    | 72 ++++++-------------
>  1 file changed, 20 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> index 3106fa8a15ef..c2ef2b1456bc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> @@ -106,48 +106,13 @@
>  #define to_amdgpu_device(x) (container_of(x, struct amdgpu_ras, eeprom_control))->adev
>  
>  static bool __is_ras_eeprom_supported(struct amdgpu_device *adev)
> -{
> -	if (adev->asic_type == CHIP_IP_DISCOVERY) {
> -		switch (adev->ip_versions[MP1_HWIP][0]) {
> -		case IP_VERSION(13, 0, 0):
> -		case IP_VERSION(13, 0, 10):
> -			return true;
> -		default:
> -			return false;
> -		}
> -	}
> -
> -	return  adev->asic_type == CHIP_VEGA20 ||
> -		adev->asic_type == CHIP_ARCTURUS ||
> -		adev->asic_type == CHIP_SIENNA_CICHLID ||
> -		adev->asic_type == CHIP_ALDEBARAN;
> -}
> -
> -static bool __get_eeprom_i2c_addr_arct(struct amdgpu_device *adev,
> -				       struct amdgpu_ras_eeprom_control *control)
> -{
> -	struct atom_context *atom_ctx = adev->mode_info.atom_context;
> -
> -	if (!control || !atom_ctx)
> -		return false;
> -
> -	if (strnstr(atom_ctx->vbios_version,
> -	            "D342",
> -		    sizeof(atom_ctx->vbios_version)))
> -		control->i2c_address = EEPROM_I2C_MADDR_0;
> -	else
> -		control->i2c_address = EEPROM_I2C_MADDR_4;
> -
> -	return true;
> -}
> -
> -static bool __get_eeprom_i2c_addr_ip_discovery(struct amdgpu_device *adev,
> -				       struct amdgpu_ras_eeprom_control *control)
>  {
>  	switch (adev->ip_versions[MP1_HWIP][0]) {
> +	case IP_VERSION(11, 0, 2): /* VEGA20 and ARCTURUS */
> +	case IP_VERSION(11, 0, 7):
>  	case IP_VERSION(13, 0, 0):
> +	case IP_VERSION(13, 0, 2):
>  	case IP_VERSION(13, 0, 10):

I'd add the rest of the proper names here which are being deleted by this change,
so as to not lose this information by this commit: Sienna Cichlid and Aldebaran,
the rest can be left blank as per the current state of the code.

> -		control->i2c_address = EEPROM_I2C_MADDR_4;
>  		return true;
>  	default:
>  		return false;
> @@ -178,29 +143,32 @@ static bool __get_eeprom_i2c_addr(struct amdgpu_device *adev,
>  		return true;
>  	}
>  
> -	switch (adev->asic_type) {
> -	case CHIP_VEGA20:
> -		control->i2c_address = EEPROM_I2C_MADDR_0;
> +	switch (adev->ip_versions[MP1_HWIP][0]) {
> +	case IP_VERSION(11, 0, 2):
> +		/* VEGA20 and ARCTURUS */
> +		if (adev->asic_type == CHIP_VEGA20)
> +			control->i2c_address = EEPROM_I2C_MADDR_0;
> +		else if (strnstr(atom_ctx->vbios_version,

In the code this is qualified with atom_ctx != NULL; and if it is,
then we return false. So, this is fine, iff we can guarantee that
"atom_ctx" will never be NULL. If, OTOH, we cannot guarantee that,
then we need to add,
		else if (!atom_ctx)
			return false;
		else if (strnstr(...

Although, I do recognize that for Aldebaran below, we do not qualify
atom_ctx, so we should probably qualify there too.

> +				 "D342",
> +				 sizeof(atom_ctx->vbios_version)))
> +			control->i2c_address = EEPROM_I2C_MADDR_0;
> +		else
> +			control->i2c_address = EEPROM_I2C_MADDR_4;
>  		return true;
> -
> -	case CHIP_ARCTURUS:
> -		return __get_eeprom_i2c_addr_arct(adev, control);
> -
> -	case CHIP_SIENNA_CICHLID:
> +	case IP_VERSION(11, 0, 7):
>  		control->i2c_address = EEPROM_I2C_MADDR_0;
>  		return true;
> -
> -	case CHIP_ALDEBARAN:
> +	case IP_VERSION(13, 0, 2):
>  		if (strnstr(atom_ctx->vbios_version, "D673",
>  			    sizeof(atom_ctx->vbios_version)))
>  			control->i2c_address = EEPROM_I2C_MADDR_4;
>  		else
>  			control->i2c_address = EEPROM_I2C_MADDR_0;
>  		return true;
> -
> -	case CHIP_IP_DISCOVERY:
> -		return __get_eeprom_i2c_addr_ip_discovery(adev, control);
> -
> +	case IP_VERSION(13, 0, 0):
> +	case IP_VERSION(13, 0, 10):
> +		control->i2c_address = EEPROM_I2C_MADDR_4;
> +		return true;
>  	default:
>  		return false;
>  	}

-- 
Regards,
Luben