[PATCH 13/13] drm/amd/display: Basic support with device ID

Dhillon, Jasdeep Jasdeep.Dhillon at amd.com
Mon Feb 7 16:24:52 UTC 2022


[AMD Official Use Only]

Hi Alex,

I already merged the branch but I have sent you the revert patch.

Regards,
Jasdeep
________________________________
From: Alex Deucher <alexdeucher at gmail.com>
Sent: February 7, 2022 10:58 AM
To: Dhillon, Jasdeep <Jasdeep.Dhillon at amd.com>
Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>; Wang, Chao-kai (Stylon) <Stylon.Wang at amd.com>; Liu, Charlene <Charlene.Liu at amd.com>; Logush, Oliver <Oliver.Logush at amd.com>; Li, Sun peng (Leo) <Sunpeng.Li at amd.com>; Wentland, Harry <Harry.Wentland at amd.com>; Zhuo, Qingqing (Lillian) <Qingqing.Zhuo at amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira at amd.com>; Li, Roman <Roman.Li at amd.com>; Chiu, Solomon <Solomon.Chiu at amd.com>; Pillai, Aurabindo <Aurabindo.Pillai at amd.com>; Lin, Wayne <Wayne.Lin at amd.com>; Lakha, Bhawanpreet <Bhawanpreet.Lakha at amd.com>; Gutierrez, Agustin <Agustin.Gutierrez at amd.com>; Kotarac, Pavle <Pavle.Kotarac at amd.com>
Subject: Re: [PATCH 13/13] drm/amd/display: Basic support with device ID

On Fri, Feb 4, 2022 at 11:33 PM Jasdeep Dhillon <jdhillon at amd.com> wrote:
>
> From: Oliver Logush <oliver.logush at amd.com>
>
> [why]
> To get the the cyan_skillfish check working

NAK.  This is still not correct.

>
> Reviewed-by: Charlene Liu <Charlene.Liu at amd.com>
> Reviewed-by: Charlene Liu <Charlene.Liu at amd.com>
> Acked-by: Jasdeep Dhillon <jdhillon at amd.com>
> Signed-off-by: Oliver Logush <oliver.logush at amd.com>
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 +++++++++++++++++--
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  2 +-
>  .../gpu/drm/amd/display/dc/core/dc_resource.c |  2 +-
>  .../gpu/drm/amd/display/include/dal_asic_id.h |  3 ++-
>  4 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 8f53c9f6b267..f5941e59e5ad 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1014,6 +1014,14 @@ static  void amdgpu_dm_audio_eld_notify(struct amdgpu_device *adev, int pin)
>         }
>  }
>
> +bool is_skillfish_series(struct amdgpu_device *adev)
> +{
> +       if (adev->asic_type == CHIP_CYAN_SKILLFISH || adev->pdev->revision == 0x143F) {
> +               return true;
> +       }
> +       return false;
> +}

I don't see why we need this.

> +
>  static int dm_dmub_hw_init(struct amdgpu_device *adev)
>  {
>         const struct dmcub_firmware_header_v1_0 *hdr;
> @@ -1049,7 +1057,7 @@ static int dm_dmub_hw_init(struct amdgpu_device *adev)
>                 return -EINVAL;
>         }
>
> -       if (!has_hw_support) {
> +       if (is_skillfish_series(adev)) {

Why this change?  won't this break other asics with no hw support?

>                 DRM_INFO("DMUB unsupported on ASIC\n");
>                 return 0;
>         }
> @@ -1471,6 +1479,10 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>                 default:
>                         break;
>                 }
> +               if (is_skillfish_series(adev)) {
> +                       init_data.flags.disable_dmcu = true;
> +                       break;
> +               }

Should not be necessary.

>                 break;
>         }
>
> @@ -1777,7 +1789,6 @@ static int load_dmcu_fw(struct amdgpu_device *adev)
>         case CHIP_VEGA10:
>         case CHIP_VEGA12:
>         case CHIP_VEGA20:
> -               return 0;

This change seems unrelated and may break other asics.

>         case CHIP_NAVI12:
>                 fw_name_dmcu = FIRMWARE_NAVI12_DMCU;
>                 break;
> @@ -1805,6 +1816,9 @@ static int load_dmcu_fw(struct amdgpu_device *adev)
>                 default:
>                         break;
>                 }
> +               if (is_skillfish_series(adev)) {
> +                       return 0;
> +               }

Why do we need this?

>                 DRM_ERROR("Unsupported ASIC type: 0x%X\n", adev->asic_type);
>                 return -EINVAL;
>         }
> @@ -4515,6 +4529,12 @@ static int dm_early_init(void *handle)
>                 adev->mode_info.num_dig = 6;
>                 break;
>         default:
> +       if (is_skillfish_series(adev)) {
> +                       adev->mode_info.num_crtc = 2;
> +                       adev->mode_info.num_hpd = 2;
> +                       adev->mode_info.num_dig = 2;
> +                       break;
> +       }

Same here.

>  #if defined(CONFIG_DRM_AMD_DC_DCN)
>                 switch (adev->ip_versions[DCE_HWIP][0]) {
>                 case IP_VERSION(2, 0, 2):
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index e35977fda5c1..13875d669acd 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -82,7 +82,7 @@ struct common_irq_params {
>         enum dc_irq_source irq_src;
>         atomic64_t previous_timestamp;
>  };
> -
> +bool is_skillfish_series(struct amdgpu_device *adev);
>  /**
>   * struct dm_compressor_info - Buffer info used by frame buffer compression
>   * @cpu_addr: MMIO cpu addr
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> index b36bae4b5bc9..318d381e2910 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> @@ -135,7 +135,7 @@ enum dce_version resource_parse_asic_id(struct hw_asic_id asic_id)
>
>         case FAMILY_NV:
>                 dc_version = DCN_VERSION_2_0;
> -               if (asic_id.chip_id == DEVICE_ID_NV_13FE) {
> +               if (asic_id.chip_id == DEVICE_ID_NV_NAVI10_LITE_P_13FE || asic_id.chip_id == DEVICE_ID_NV_NAVI10_LITE_P_143F) {

I think these last two hunks are the only ones you need.  The rest
should be unnecessary.

>                         dc_version = DCN_VERSION_2_01;
>                         break;
>                 }
> diff --git a/drivers/gpu/drm/amd/display/include/dal_asic_id.h b/drivers/gpu/drm/amd/display/include/dal_asic_id.h
> index e4a2dfacab4c..37ec6343dbd6 100644
> --- a/drivers/gpu/drm/amd/display/include/dal_asic_id.h
> +++ b/drivers/gpu/drm/amd/display/include/dal_asic_id.h
> @@ -211,7 +211,8 @@ enum {
>  #ifndef ASICREV_IS_GREEN_SARDINE
>  #define ASICREV_IS_GREEN_SARDINE(eChipRev) ((eChipRev >= GREEN_SARDINE_A0) && (eChipRev < 0xFF))
>  #endif
> -#define DEVICE_ID_NV_13FE 0x13FE  // CYAN_SKILLFISH
> +#define DEVICE_ID_NV_NAVI10_LITE_P_13FE          0x13FE  // CYAN_SKILLFISH
> +#define DEVICE_ID_NV_NAVI10_LITE_P_143F                        0x143F
>  #define FAMILY_VGH 144
>  #define DEVICE_ID_VGH_163F 0x163F
>  #define VANGOGH_A0 0x01
> --
> 2.25.1
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20220207/ca907d68/attachment-0001.htm>


More information about the amd-gfx mailing list