<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> [AMD Official Use Only] <div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Hi Alex, </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> I already merged the branch but I have sent you the revert patch. </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Regards, </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Jasdeep</div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Alex Deucher <alexdeucher@gmail.com> Sent: February 7, 2022 10:58 AM To: Dhillon, Jasdeep <Jasdeep.Dhillon@amd.com> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Wang, Chao-kai (Stylon) <Stylon.Wang@amd.com>; Liu, Charlene <Charlene.Liu@amd.com>; Logush, Oliver <Oliver.Logush@amd.com>; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>; Wentland, Harry <Harry.Wentland@amd.com>; Zhuo, Qingqing (Lillian) <Qingqing.Zhuo@amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; Li, Roman <Roman.Li@amd.com>; Chiu, Solomon <Solomon.Chiu@amd.com>; Pillai, Aurabindo <Aurabindo.Pillai@amd.com>; Lin, Wayne <Wayne.Lin@amd.com>; Lakha, Bhawanpreet <Bhawanpreet.Lakha@amd.com>; Gutierrez, Agustin <Agustin.Gutierrez@amd.com>; Kotarac, Pavle <Pavle.Kotarac@amd.com> Subject: Re: [PATCH 13/13] drm/amd/display: Basic support with device ID <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">On Fri, Feb 4, 2022 at 11:33 PM Jasdeep Dhillon <jdhillon@amd.com> wrote: > > From: Oliver Logush <oliver.logush@amd.com> > > [why] > To get the the cyan_skillfish check working NAK. This is still not correct. > > Reviewed-by: Charlene Liu <Charlene.Liu@amd.com> > Reviewed-by: Charlene Liu <Charlene.Liu@amd.com> > Acked-by: Jasdeep Dhillon <jdhillon@amd.com> > Signed-off-by: Oliver Logush <oliver.logush@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 > </div> </div> </div> </body> </html>