Re: [PATCH v2] drm/amd/amdgpu: Use IP discovery data to determine VCN enablement instead of MMSCH

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 15, 2021 at 3:46 AM Peng Ju Zhou <PengJu.Zhou@xxxxxxx> wrote:
>
> From: Bokun Zhang <Bokun.Zhang@xxxxxxx>
>
> In the past, we use MMSCH to determine whether a VCN is enabled or not.
> This is not reliable since after a FLR, MMSCH may report junk data.
>
> It is better to use IP discovery data.
>
> Signed-off-by: Bokun Zhang <Bokun.Zhang@xxxxxxx>
> Signed-off-by: Peng Ju Zhou <PengJu.Zhou@xxxxxxx>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c |  8 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.h |  3 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c       | 23 ++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h       | 13 +++++
>  drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c         | 52 +++++--------------
>  5 files changed, 60 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index f949ed8bfd9e..e02405a24fe3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -373,6 +373,14 @@ int amdgpu_discovery_get_ip_version(struct amdgpu_device *adev, int hw_id, int n
>         return -EINVAL;
>  }
>
> +
> +int amdgpu_discovery_get_vcn_version(struct amdgpu_device *adev, int vcn_instance,
> +                                    int *major, int *minor, int *revision)
> +{
> +       return amdgpu_discovery_get_ip_version(adev, VCN_HWID,
> +                                              vcn_instance, major, minor, revision);
> +}
> +

I think you can drop this wrapper and just call
amdgpu_discovery_get_ip_version() directly from
amdgpu_vcn_is_disabled_vcn().

>  void amdgpu_discovery_harvest_ip(struct amdgpu_device *adev)
>  {
>         struct binary_header *bhdr;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.h
> index 02e340cd3a38..48e6b88cfdfe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.h
> @@ -32,6 +32,9 @@ int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev);
>  void amdgpu_discovery_harvest_ip(struct amdgpu_device *adev);
>  int amdgpu_discovery_get_ip_version(struct amdgpu_device *adev, int hw_id, int number_instance,
>                                      int *major, int *minor, int *revision);
> +
> +int amdgpu_discovery_get_vcn_version(struct amdgpu_device *adev, int vcn_instance,
> +                                    int *major, int *minor, int *revision);
>  int amdgpu_discovery_get_gfx_info(struct amdgpu_device *adev);
>
>  #endif /* __AMDGPU_DISCOVERY__ */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 9492b505e69b..84b025405578 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -287,6 +287,29 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
>         return 0;
>  }
>
> +bool amdgpu_vcn_is_disabled_vcn(struct amdgpu_device *adev, enum vcn_ring_type type, uint32_t vcn_instance)
> +{
> +       bool ret = false;
> +
> +       int major;
> +       int minor;
> +       int revision;
> +
> +       /* if cannot find IP data, then this VCN does not exist */
> +       if (amdgpu_discovery_get_vcn_version(adev, vcn_instance, &major, &minor, &revision) != 0)

Just call amdgpu_discovery_get_ip_version() directly here.

> +               return true;
> +
> +       if ((type == VCN_ENCODE_RING) && (revision & VCN_BLOCK_ENCODE_DISABLE_MASK)) {
> +               ret = true;
> +       } else if ((type == VCN_DECODE_RING) && (revision & VCN_BLOCK_DECODE_DISABLE_MASK)) {
> +               ret = true;
> +       } else if ((type == VCN_UNIFIED_RING) && (revision & VCN_BLOCK_QUEUE_DISABLE_MASK)) {
> +               ret = true;
> +       }
> +
> +       return ret;
> +}
> +
>  int amdgpu_vcn_suspend(struct amdgpu_device *adev)
>  {
>         unsigned size;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index bc76cab67697..d74c62b49795 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -280,6 +280,16 @@ struct amdgpu_vcn_decode_buffer {
>         uint32_t pad[30];
>  };
>
> +#define VCN_BLOCK_ENCODE_DISABLE_MASK 0x80
> +#define VCN_BLOCK_DECODE_DISABLE_MASK 0x40
> +#define VCN_BLOCK_QUEUE_DISABLE_MASK 0xC0
> +
> +enum vcn_ring_type {
> +       VCN_ENCODE_RING,
> +       VCN_DECODE_RING,
> +       VCN_UNIFIED_RING,
> +};
> +
>  int amdgpu_vcn_sw_init(struct amdgpu_device *adev);
>  int amdgpu_vcn_sw_fini(struct amdgpu_device *adev);
>  int amdgpu_vcn_suspend(struct amdgpu_device *adev);
> @@ -287,6 +297,9 @@ int amdgpu_vcn_resume(struct amdgpu_device *adev);
>  void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring);
>  void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring);
>
> +bool amdgpu_vcn_is_disabled_vcn(struct amdgpu_device *adev,
> +                               enum vcn_ring_type type, uint32_t vcn_instance);
> +
>  int amdgpu_vcn_dec_ring_test_ring(struct amdgpu_ring *ring);
>  int amdgpu_vcn_dec_ring_test_ib(struct amdgpu_ring *ring, long timeout);
>  int amdgpu_vcn_dec_sw_ring_test_ring(struct amdgpu_ring *ring);
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> index ce3c794c176f..a79ae86bc752 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> @@ -85,9 +85,12 @@ static void vcn_v3_0_enc_ring_set_wptr(struct amdgpu_ring *ring);
>  static int vcn_v3_0_early_init(void *handle)
>  {
>         struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +       int i;
>
>         if (amdgpu_sriov_vf(adev)) {
> -               adev->vcn.num_vcn_inst = VCN_INSTANCES_SIENNA_CICHLID;
> +               for (i = 0; i < VCN_INSTANCES_SIENNA_CICHLID; i++)
> +                       if (amdgpu_vcn_is_disabled_vcn(adev, VCN_DECODE_RING, i))
> +                               adev->vcn.num_vcn_inst++;
>                 adev->vcn.harvest_config = 0;
>                 adev->vcn.num_enc_rings = 1;
>
> @@ -99,7 +102,6 @@ static int vcn_v3_0_early_init(void *handle)
>         } else {
>                 if (adev->asic_type == CHIP_SIENNA_CICHLID) {
>                         u32 harvest;
> -                       int i;
>
>                         adev->vcn.num_vcn_inst = VCN_INSTANCES_SIENNA_CICHLID;
>                         for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
> @@ -154,7 +156,7 @@ static int vcn_v3_0_sw_init(void *handle)
>                 adev->firmware.fw_size +=
>                         ALIGN(le32_to_cpu(hdr->ucode_size_bytes), PAGE_SIZE);
>
> -               if (adev->vcn.num_vcn_inst == VCN_INSTANCES_SIENNA_CICHLID) {
> +               if (adev->asic_type == CHIP_SIENNA_CICHLID) {

Is this change safe?  Will this break anything on harvested boards?
Other than these comments the patch looks good.

Alex

>                         adev->firmware.ucode[AMDGPU_UCODE_ID_VCN1].ucode_id = AMDGPU_UCODE_ID_VCN1;
>                         adev->firmware.ucode[AMDGPU_UCODE_ID_VCN1].fw = adev->vcn.fw;
>                         adev->firmware.fw_size +=
> @@ -324,19 +326,17 @@ static int vcn_v3_0_hw_init(void *handle)
>                                 continue;
>
>                         ring = &adev->vcn.inst[i].ring_dec;
> -                       if (ring->sched.ready) {
> -                               ring->wptr = 0;
> -                               ring->wptr_old = 0;
> -                               vcn_v3_0_dec_ring_set_wptr(ring);
> -                       }
> +                       ring->wptr = 0;
> +                       ring->wptr_old = 0;
> +                       vcn_v3_0_dec_ring_set_wptr(ring);
> +                       ring->sched.ready = true;
>
>                         for (j = 0; j < adev->vcn.num_enc_rings; ++j) {
>                                 ring = &adev->vcn.inst[i].ring_enc[j];
> -                               if (ring->sched.ready) {
> -                                       ring->wptr = 0;
> -                                       ring->wptr_old = 0;
> -                                       vcn_v3_0_enc_ring_set_wptr(ring);
> -                               }
> +                               ring->wptr = 0;
> +                               ring->wptr_old = 0;
> +                               vcn_v3_0_enc_ring_set_wptr(ring);
> +                               ring->sched.ready = true;
>                         }
>                 }
>         } else {
> @@ -1303,8 +1303,6 @@ static int vcn_v3_0_start_sriov(struct amdgpu_device *adev)
>         uint32_t table_size;
>         uint32_t size, size_dw;
>
> -       bool is_vcn_ready;
> -
>         struct mmsch_v3_0_cmd_direct_write
>                 direct_wt = { {0} };
>         struct mmsch_v3_0_cmd_direct_read_modify_write
> @@ -1496,30 +1494,6 @@ static int vcn_v3_0_start_sriov(struct amdgpu_device *adev)
>                 }
>         }
>
> -       /* 6, check each VCN's init_status
> -        * if it remains as 0, then this VCN is not assigned to current VF
> -        * do not start ring for this VCN
> -        */
> -       size = sizeof(struct mmsch_v3_0_init_header);
> -       table_loc = (uint32_t *)table->cpu_addr;
> -       memcpy(&header, (void *)table_loc, size);
> -
> -       for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
> -               if (adev->vcn.harvest_config & (1 << i))
> -                       continue;
> -
> -               is_vcn_ready = (header.inst[i].init_status == 1);
> -               if (!is_vcn_ready)
> -                       DRM_INFO("VCN(%d) engine is disabled by hypervisor\n", i);
> -
> -               ring = &adev->vcn.inst[i].ring_dec;
> -               ring->sched.ready = is_vcn_ready;
> -               for (j = 0; j < adev->vcn.num_enc_rings; ++j) {
> -                       ring = &adev->vcn.inst[i].ring_enc[j];
> -                       ring->sched.ready = is_vcn_ready;
> -               }
> -       }
> -
>         return 0;
>  }
>
> --
> 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



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux