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

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

 



[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@xxxxxxxxx>
Sent: February 7, 2022 10:58 AM
To: Dhillon, Jasdeep <Jasdeep.Dhillon@xxxxxxx>
Cc: amd-gfx list <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>; Wang, Chao-kai (Stylon) <Stylon.Wang@xxxxxxx>; Liu, Charlene <Charlene.Liu@xxxxxxx>; Logush, Oliver <Oliver.Logush@xxxxxxx>; Li, Sun peng (Leo) <Sunpeng.Li@xxxxxxx>; Wentland, Harry <Harry.Wentland@xxxxxxx>; Zhuo, Qingqing (Lillian) <Qingqing.Zhuo@xxxxxxx>; Siqueira, Rodrigo <Rodrigo.Siqueira@xxxxxxx>; Li, Roman <Roman.Li@xxxxxxx>; Chiu, Solomon <Solomon.Chiu@xxxxxxx>; Pillai, Aurabindo <Aurabindo.Pillai@xxxxxxx>; Lin, Wayne <Wayne.Lin@xxxxxxx>; Lakha, Bhawanpreet <Bhawanpreet.Lakha@xxxxxxx>; Gutierrez, Agustin <Agustin.Gutierrez@xxxxxxx>; Kotarac, Pavle <Pavle.Kotarac@xxxxxxx>
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@xxxxxxx> wrote:
>
> From: Oliver Logush <oliver.logush@xxxxxxx>
>
> [why]
> To get the the cyan_skillfish check working

NAK.  This is still not correct.

>
> Reviewed-by: Charlene Liu <Charlene.Liu@xxxxxxx>
> Reviewed-by: Charlene Liu <Charlene.Liu@xxxxxxx>
> Acked-by: Jasdeep Dhillon <jdhillon@xxxxxxx>
> Signed-off-by: Oliver Logush <oliver.logush@xxxxxxx>
> ---
>  .../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
>

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

  Powered by Linux