Hi Thomas, Thanks for the feedback. Let me consider how to achieve your suggestion. Regards, Jammy Huang > > Hi Jamie > > > Am 25.12.24 um 01:32 schrieb Jammy Huang: > > In this patch, 3 new timings are added into support list. > > > > If you want to have new timings, 1280x720 and 1280x960 on DisplayPort, > > your dp-fw should be newer than version, 20240502. > > Thank you for the patch. > > What happens if the firmware is not at the required version? > > As it seems unreasonable to expect everyone to upgrade their DP firmware (or > even be able to), we 'll likely need a test against the firmware version. So that > the additional display modes are optional within the driver. My proposal is to > get the firmware version at the end of > ast_dp_launch() [1]. Then implement drm_encoder_helper_funcs.mode_valid > [2] to filter out unsupported modes on older firmwares. Does that make sense? > > Best regards > Thomas > > [1] > https://elixir.bootlin.com/linux/v6.13-rc3/source/drivers/gpu/drm/ast/ast_dp.c > #L148 > [2] > https://elixir.bootlin.com/linux/v6.13-rc3/source/include/drm/drm_modeset_h > elper_vtables.h#L562 > > > > > Signed-off-by: Jammy Huang <jammy_huang@xxxxxxxxxxxxxx> > > --- > > v2: > > - Fix build errors. > > v3: > > - Fix typo. > > --- > > drivers/gpu/drm/ast/ast_dp.c | 9 ++++++++- > > drivers/gpu/drm/ast/ast_drv.h | 3 +++ > > drivers/gpu/drm/ast/ast_mode.c | 14 ++++++++++++++ > > drivers/gpu/drm/ast/ast_tables.h | 18 ++++++++++++++++++ > > 4 files changed, 43 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c > > index 0e282b7b167c..0b56f0335871 100644 > > --- a/drivers/gpu/drm/ast/ast_dp.c > > +++ b/drivers/gpu/drm/ast/ast_dp.c > > @@ -255,6 +255,10 @@ static void ast_dp_set_mode(struct drm_crtc *crtc, > struct ast_vbios_mode_info *v > > case 1280: > > if (crtc->mode.crtc_vdisplay == 800) > > ModeIdx = (ASTDP_1280x800_60_RB - (u8) > ulRefreshRateIndex); > > + else if (crtc->mode.crtc_vdisplay == 720) > > + ModeIdx = ASTDP_1280x720_60; > > + else if (crtc->mode.crtc_vdisplay == 960) > > + ModeIdx = ASTDP_1280x960_60; > > else // 1024 > > ModeIdx = (ASTDP_1280x1024_60 + (u8) > ulRefreshRateIndex); > > break; > > @@ -267,7 +271,10 @@ static void ast_dp_set_mode(struct drm_crtc *crtc, > struct ast_vbios_mode_info *v > > break; > > case 1600: > > if (crtc->mode.crtc_vdisplay == 900) > > - ModeIdx = (ASTDP_1600x900_60_RB - (u8) > ulRefreshRateIndex); > > + if (ulRefreshRateIndex == 2) > > + ModeIdx = ASTDP_1600x900_60_DMT; > > + else > > + ModeIdx = (ASTDP_1600x900_60_RB - (u8) > ulRefreshRateIndex); > > else //1200 > > ModeIdx = ASTDP_1600x1200_60; > > break; > > diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h > > index 21ce3769bf0d..e7cef334b3ec 100644 > > --- a/drivers/gpu/drm/ast/ast_drv.h > > +++ b/drivers/gpu/drm/ast/ast_drv.h > > @@ -442,6 +442,9 @@ int ast_mode_config_init(struct ast_device *ast); > > #define ASTDP_1600x900_60_RB 0x1D > > #define ASTDP_1366x768_60 0x1E > > #define ASTDP_1152x864_75 0x1F > > +#define ASTDP_1600x900_60_DMT 0x51 > > +#define ASTDP_1280x720_60 0x52 > > +#define ASTDP_1280x960_60 0x53 > > > > int ast_mm_init(struct ast_device *ast); > > > > diff --git a/drivers/gpu/drm/ast/ast_mode.c > b/drivers/gpu/drm/ast/ast_mode.c > > index 9d5321c81e68..48d1065846a2 100644 > > --- a/drivers/gpu/drm/ast/ast_mode.c > > +++ b/drivers/gpu/drm/ast/ast_mode.c > > @@ -147,6 +147,10 @@ static bool ast_get_vbios_mode_info(const struct > drm_format_info *format, > > case 1280: > > if (mode->crtc_vdisplay == 800) > > vbios_mode->enh_table = > &res_1280x800[refresh_rate_index]; > > + else if (mode->crtc_vdisplay == 720) > > + vbios_mode->enh_table = &res_1280x720[refresh_rate_index]; > > + else if (mode->crtc_vdisplay == 960) > > + vbios_mode->enh_table = &res_1280x960[refresh_rate_index]; > > else > > vbios_mode->enh_table = > &res_1280x1024[refresh_rate_index]; > > break; > > @@ -475,6 +479,12 @@ static void ast_set_dclk_reg(struct ast_device *ast, > > ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xbb, 0x0f, > > (clk_info->param3 & 0xc0) | > > ((clk_info->param3 & 0x3) << 4)); > > + > > + /* Set SEQ; Half dclk for this timing */ > > + if (vbios_mode->enh_table->flags & HalfDCLK) > > + ast_set_index_reg_mask(ast, AST_IO_VGASRI, 0x01, 0xff, 0x08); > > + else > > + ast_set_index_reg_mask(ast, AST_IO_VGASRI, 0x01, 0xf7, 0x00); > > } > > > > static void ast_set_color_reg(struct ast_device *ast, > > @@ -1027,8 +1037,12 @@ ast_crtc_helper_mode_valid(struct drm_crtc > *crtc, const struct drm_display_mode > > if (ast->support_wide_screen) { > > if ((mode->hdisplay == 1680) && (mode->vdisplay == 1050)) > > return MODE_OK; > > + if ((mode->hdisplay == 1280) && (mode->vdisplay == 960)) > > + return MODE_OK; > > if ((mode->hdisplay == 1280) && (mode->vdisplay == 800)) > > return MODE_OK; > > + if ((mode->hdisplay == 1280) && (mode->vdisplay == 720)) > > + return MODE_OK; > > if ((mode->hdisplay == 1440) && (mode->vdisplay == 900)) > > return MODE_OK; > > if ((mode->hdisplay == 1360) && (mode->vdisplay == 768)) > > diff --git a/drivers/gpu/drm/ast/ast_tables.h > b/drivers/gpu/drm/ast/ast_tables.h > > index 0378c9bc079b..329d6bac867b 100644 > > --- a/drivers/gpu/drm/ast/ast_tables.h > > +++ b/drivers/gpu/drm/ast/ast_tables.h > > @@ -254,6 +254,13 @@ static const struct ast_vbios_enhtable > res_1024x768[] = { > > (SyncPP | Charx8Dot), 0xFF, 4, 0x31 }, > > }; > > > > +static const struct ast_vbios_enhtable res_1280x960[] = { > > + {1800, 1280, 96, 112, 1000, 960, 1, 3, VCLK108, /* 60Hz */ > > + (SyncPP | Charx8Dot), 60, 1, 0x3E }, > > + {1800, 1280, 96, 112, 1000, 960, 1, 3, VCLK108, /* end */ > > + (SyncPP | Charx8Dot), 0xFF, 1, 0x3E }, > > +}; > > + > > static const struct ast_vbios_enhtable res_1280x1024[] = { > > {1688, 1280, 48, 112, 1066, 1024, 1, 3, VCLK108, /* 60Hz */ > > (SyncPP | Charx8Dot), 60, 1, 0x32 }, > > @@ -280,6 +287,15 @@ static const struct ast_vbios_enhtable > res_1152x864[] = { > > }; > > > > /* 16:9 */ > > +static const struct ast_vbios_enhtable res_1280x720[] = { > > + {1650, 1280, 110, 40, 750, 720, 5, 5, VCLK148_5, /* 60Hz */ > > + (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | > NewModeInfo | > > + HalfDCLK), 60, 1, 0x3D }, > > + {1650, 1280, 110, 40, 750, 720, 5, 5, VCLK148_5, /* end */ > > + (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | > NewModeInfo | > > + HalfDCLK), 0xFF, 1, 0x3D }, > > +}; > > + > > static const struct ast_vbios_enhtable res_1360x768[] = { > > {1792, 1360, 64, 112, 795, 768, 3, 6, VCLK85_5, /* 60Hz */ > > (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | > NewModeInfo), 60, 1, 0x39 }, > > @@ -294,6 +310,8 @@ static const struct ast_vbios_enhtable res_1600x900[] > = { > > AST2500PreCatchCRT), 60, 1, 0x3A }, > > {2112, 1600, 88, 168, 934, 900, 3, 5, VCLK118_25, /* 60Hz CVT > */ > > (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | > NewModeInfo), 60, 2, 0x3A }, > > + {1800, 1600, 24, 80, 1000, 900, 1, 3, VCLK108, /* 60Hz DMT */ > > + (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | > NewModeInfo), 60, 3, 0x3A }, > > {2112, 1600, 88, 168, 934, 900, 3, 5, VCLK118_25, /* 60Hz CVT > */ > > (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | > NewModeInfo), 0xFF, 2, 0x3A }, > > }; > > > > base-commit: 4bbf9020becbfd8fc2c3da790855b7042fad455b > > -- > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Frankenstrasse 146, 90461 Nuernberg, Germany > GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman > HRB 36809 (AG Nuernberg)