[PATCH v3 2/3] drm/tidss: Remove max_pclk_khz from tidss display features

Devarsh Thakkar devarsht at ti.com
Tue Jul 1 13:12:27 UTC 2025


Hi Jayesh,

On 01/07/25 15:25, Jayesh Choudhary wrote:
> TIDSS hardware by itself does not have variable max_pclk for each VP.
> The maximum pixel clock is determined by the limiting factor between
> the functional clock and the PLL

and the pixel clock
.
> 
> The limitation that has been modeled till now comes from the clock
> (PLL can only be programmed to a particular max value). Instead of
> putting it as a constant field in dispc_features, we can query the
> DM to see if requested clock can be set or not and use it in
> "mode_valid()".
> 
> Replace constant "max_pclk_khz" in dispc_features with "curr_max_pclk"
> in tidss_device structure which would be modified in runtime.
> In mode_valid() call, check if a best frequency match for mode clock
> can be found or not using "clk_round_rate()". Based on that, propagate
> "cur_max_pclk" and query DM again only if the requested mode clock
> is greater than cur_max_pclk. (As the preferred display mode is usually
> the max resolution, driver ends up checking the highest clock the first
> time itself which is used in subsequent checks)
> 
> Since TIDSS display controller provides clock tolerance of 5%, we use
> this while checking the curr_max_pclk. Also, move up "dispc_pclk_diff()"
> before it is called.
> 
> This will make the existing compatibles reusable

reusable if DSS features are same across two SoCs and only difference 
being the pixel clock.

> 
> Signed-off-by: Jayesh Choudhary <j-choudhary at ti.com>
> ---
>   drivers/gpu/drm/tidss/tidss_dispc.c | 77 +++++++++++------------------
>   drivers/gpu/drm/tidss/tidss_dispc.h |  1 -
>   drivers/gpu/drm/tidss/tidss_drv.h   |  2 +
>   3 files changed, 31 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
> index 3f6cff2ab1b2..fb59a6a0f86a 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -58,10 +58,6 @@ static const u16 tidss_k2g_common_regs[DISPC_COMMON_REG_TABLE_LEN] = {
>   const struct dispc_features dispc_k2g_feats = {
>   	.min_pclk_khz = 4375,
>   
> -	.max_pclk_khz = {
> -		[DISPC_VP_DPI] = 150000,
> -	},
> -
>   	/*
>   	 * XXX According TRM the RGB input buffer width up to 2560 should
>   	 *     work on 3 taps, but in practice it only works up to 1280.
> @@ -144,11 +140,6 @@ static const u16 tidss_am65x_common_regs[DISPC_COMMON_REG_TABLE_LEN] = {
>   };
>   
>   const struct dispc_features dispc_am65x_feats = {
> -	.max_pclk_khz = {
> -		[DISPC_VP_DPI] = 165000,
> -		[DISPC_VP_OLDI_AM65X] = 165000,
> -	},
> -
>   	.scaling = {
>   		.in_width_max_5tap_rgb = 1280,
>   		.in_width_max_3tap_rgb = 2560,
> @@ -244,11 +235,6 @@ static const u16 tidss_j721e_common_regs[DISPC_COMMON_REG_TABLE_LEN] = {
>   };
>   
>   const struct dispc_features dispc_j721e_feats = {
> -	.max_pclk_khz = {
> -		[DISPC_VP_DPI] = 170000,
> -		[DISPC_VP_INTERNAL] = 600000,
> -	},
> -
>   	.scaling = {
>   		.in_width_max_5tap_rgb = 2048,
>   		.in_width_max_3tap_rgb = 4096,
> @@ -315,11 +301,6 @@ const struct dispc_features dispc_j721e_feats = {
>   };
>   
>   const struct dispc_features dispc_am625_feats = {
> -	.max_pclk_khz = {
> -		[DISPC_VP_DPI] = 165000,
> -		[DISPC_VP_INTERNAL] = 170000,
> -	},
> -
>   	.scaling = {
>   		.in_width_max_5tap_rgb = 1280,
>   		.in_width_max_3tap_rgb = 2560,
> @@ -376,15 +357,6 @@ const struct dispc_features dispc_am625_feats = {
>   };
>   
>   const struct dispc_features dispc_am62a7_feats = {
> -	/*
> -	 * if the code reaches dispc_mode_valid with VP1,
> -	 * it should return MODE_BAD.
> -	 */
> -	.max_pclk_khz = {
> -		[DISPC_VP_TIED_OFF] = 0,
> -		[DISPC_VP_DPI] = 165000,
> -	},
> -
>   	.scaling = {
>   		.in_width_max_5tap_rgb = 1280,
>   		.in_width_max_3tap_rgb = 2560,
> @@ -441,10 +413,6 @@ const struct dispc_features dispc_am62a7_feats = {
>   };
>   
>   const struct dispc_features dispc_am62l_feats = {
> -	.max_pclk_khz = {
> -		[DISPC_VP_DPI] = 165000,
> -	},
> -
>   	.subrev = DISPC_AM62L,
>   
>   	.common = "common",
> @@ -1347,25 +1315,49 @@ static void dispc_vp_set_default_color(struct dispc_device *dispc,
>   			DISPC_OVR_DEFAULT_COLOR2, (v >> 32) & 0xffff);
>   }
>   
> +/*
> + * Calculate the percentage difference between the requested pixel clock rate
> + * and the effective rate resulting from calculating the clock divider value.
> + */
> +unsigned int dispc_pclk_diff(unsigned long rate, unsigned long real_rate)
> +{
> +	int r = rate / 100, rr = real_rate / 100;
> +
> +	return (unsigned int)(abs(((rr - r) * 100) / r));
> +}
> +
> +static int check_pixel_clock(struct dispc_device *dispc,
> +			     u32 hw_videoport, unsigned long clock)
> +{
> +	if (clock > dispc->tidss->curr_max_pclk[hw_videoport] &&
> +	    !dispc->tidss->is_oldi_vp[hw_videoport]) {
> +		unsigned long round_clock = clk_round_rate(dispc->vp_clk[hw_videoport], clock);
> +
> +		if (dispc_pclk_diff(clock, round_clock) > 5)
> +			return -EINVAL;
> +
> +		dispc->tidss->curr_max_pclk[hw_videoport] = round_clock;
> +	}
> +
> +	return 0;
> +}
> +
>   enum drm_mode_status dispc_vp_mode_valid(struct dispc_device *dispc,
>   					 u32 hw_videoport,
>   					 const struct drm_display_mode *mode)
>   {
>   	u32 hsw, hfp, hbp, vsw, vfp, vbp;
>   	enum dispc_vp_bus_type bus_type;
> -	int max_pclk;
>   
>   	bus_type = dispc->feat->vp_bus_type[hw_videoport];
>   
> -	max_pclk = dispc->feat->max_pclk_khz[bus_type];
> -
> -	if (WARN_ON(max_pclk == 0))
> +	if (WARN_ON(bus_type == DISPC_VP_TIED_OFF))
>   		return MODE_BAD;
>   
>   	if (mode->clock < dispc->feat->min_pclk_khz)
>   		return MODE_CLOCK_LOW;
>   
> -	if (mode->clock > max_pclk)
> +	if (check_pixel_clock(dispc, hw_videoport, mode->clock * 1000))
>   		return MODE_CLOCK_HIGH;
>   
>   	if (mode->hdisplay > 4096)
> @@ -1437,17 +1429,6 @@ void dispc_vp_disable_clk(struct dispc_device *dispc, u32 hw_videoport)
>   	clk_disable_unprepare(dispc->vp_clk[hw_videoport]);
>   }
>   
> -/*
> - * Calculate the percentage difference between the requested pixel clock rate
> - * and the effective rate resulting from calculating the clock divider value.
> - */
> -unsigned int dispc_pclk_diff(unsigned long rate, unsigned long real_rate)
> -{
> -	int r = rate / 100, rr = real_rate / 100;
> -
> -	return (unsigned int)(abs(((rr - r) * 100) / r));
> -}
> -
>   int dispc_vp_set_clk_rate(struct dispc_device *dispc, u32 hw_videoport,
>   			  unsigned long rate)
>   {
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
> index 60c1b400eb89..fbfe6e304ac8 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.h
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.h
> @@ -78,7 +78,6 @@ enum dispc_dss_subrevision {
>   
>   struct dispc_features {
>   	int min_pclk_khz;
> -	int max_pclk_khz[DISPC_VP_MAX_BUS_TYPE];
>   
>   	struct dispc_features_scaling scaling;
>   
> diff --git a/drivers/gpu/drm/tidss/tidss_drv.h b/drivers/gpu/drm/tidss/tidss_drv.h
> index 82beaaceadb3..e89c38a386f7 100644
> --- a/drivers/gpu/drm/tidss/tidss_drv.h
> +++ b/drivers/gpu/drm/tidss/tidss_drv.h
> @@ -25,6 +25,8 @@ struct tidss_device {
>   	const struct dispc_features *feat;
>   	struct dispc_device *dispc;
>   	bool is_oldi_vp[TIDSS_MAX_PORTS];
> +	/* stores max supported pixel clock requested during checking modes */

Stores highest pixel clock value found to be valid while checking 
supported modes for connected display

With suggested changes,
Reviewed-by: Devarsh Thakkar <devarsht at ti.com>

Regards
Devarsh

> +	unsigned long curr_max_pclk[TIDSS_MAX_PORTS];
>   
>   
>   	unsigned int num_crtcs;


More information about the dri-devel mailing list