This series is fine. Please feel free to add my RB.
But it reminds me another thing: it's a little weird to enable all RAS features and disable those not needed afterwards.
Is it possible to only enable those which are really needed(no disablement will be needed then)?
Regards,
Evan
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Pan,
> Xinhui
> Sent: 2019年4月8日 15:08
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>
> Subject: [PATCH 1/5] drm/amdgpu: Make default ras error type to none
>
> Unless IP has implemented its own ras, use ERROR_NONE as the default type.
>
> Signed-off-by: xinhui pan <xinhui.pan@xxxxxxx>
> Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 24 +++++++++++++++------
> ---
> 1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index 2b7a420d5a1f..6cbf1d34c5b4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -664,11 +664,13 @@ static int amdgpu_ras_enable_all_features(struct
> amdgpu_device *adev,
> struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
> int ras_block_count = AMDGPU_RAS_BLOCK_COUNT;
> int i;
> + const enum amdgpu_ras_error_type default_ras_type =
> + AMDGPU_RAS_ERROR__NONE;
>
> for (i = 0; i < ras_block_count; i++) {
> struct ras_common_if head = {
> .block = i,
> - .type =
> AMDGPU_RAS_ERROR__MULTI_UNCORRECTABLE,
> + .type = default_ras_type,
> .sub_block_index = 0,
> };
> strcpy(head.name, ras_block_str(i));
> @@ -1474,9 +1476,6 @@ int amdgpu_ras_init(struct amdgpu_device *adev)
>
> amdgpu_ras_mask &= AMDGPU_RAS_BLOCK_MASK;
>
> - if (con->flags & AMDGPU_RAS_FLAG_INIT_BY_VBIOS)
> - amdgpu_ras_enable_all_features(adev, 1);
> -
> if (amdgpu_ras_fs_init(adev))
> goto fs_out;
>
> @@ -1504,18 +1503,25 @@ void amdgpu_ras_post_init(struct
> amdgpu_device *adev)
> if (!con)
> return;
>
> - /* We enable ras on all hw_supported block, but as boot parameter
> might
> - * disable some of them and one or more IP has not implemented
> yet.
> - * So we disable them on behalf.
> - */
> if (con->flags & AMDGPU_RAS_FLAG_INIT_BY_VBIOS) {
> + /* Set up all other IPs which are not implemented. There is a
> + * tricky thing that IP's actual ras error type should be
> + * MULTI_UNCORRECTABLE, but as driver does not handle it,
> so
> + * ERROR_NONE make sense anyway.
> + */
> + amdgpu_ras_enable_all_features(adev, 1);
> +
> + /* We enable ras on all hw_supported block, but as boot
> + * parameter might disable some of them and one or more IP
> has
> + * not implemented yet. So we disable them on behalf.
> + */
> list_for_each_entry_safe(obj, tmp, &con->head, node) {
> if (!amdgpu_ras_is_supported(adev, obj-
> >head.block)) {
> amdgpu_ras_feature_enable(adev, &obj-
> >head, 0);
> /* there should be no any reference. */
> WARN_ON(alive_obj(obj));
> }
> - };
> + }
> }
> }
>
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
