From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from fllvem-ot03.ext.ti.com (fllvem-ot03.ext.ti.com [198.47.19.245]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C57A122094 for ; Tue, 1 Jul 2025 13:13:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.47.19.245 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1751375589; cv=none; b=TcILa++6+OYDxPVy5mSfIL1wxK0HbuZevjy1apL9cnAhKTS/Tbr+k6HlIaUGwsMl19Km4gPVTsn70EWOarOcqIhuzbWs5Hf80GTHhPy4Xg5uXzqiwhCt2XvvjPVvoOSsd1SEvFjJloHJ2iSEPEFjs63jOxUtc6STfOMEkY+o5cg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1751375589; c=relaxed/simple; bh=B/AWvKDjVRGEzVRrtZjjeLe+E6pnKJQl0j9dPHKe7iM=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=kQPvTFKEDTyebrOV89F4hgaQ06ONkf1iqlxy2PAznWLLd9qPc8rzhKfuoJAtiSn+SplNB9ibAANOjPiw1M2xupV6o1ZY+TJiIB0RG9cPg9KWby7kqEokU6P83qNZxY8tonyNjljTD+1oT0GUGoj+okOAtQhxYWxAmaSQ5xkNfZ4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=ti.com; spf=pass smtp.mailfrom=ti.com; dkim=pass (1024-bit key) header.d=ti.com header.i=@ti.com header.b=evMQbb58; arc=none smtp.client-ip=198.47.19.245 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=ti.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ti.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ti.com header.i=@ti.com header.b="evMQbb58" Received: from lelvem-sh01.itg.ti.com ([10.180.77.71]) by fllvem-ot03.ext.ti.com (8.15.2/8.15.2) with ESMTP id 561DCWwO3321452; Tue, 1 Jul 2025 08:12:32 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1751375552; bh=6MPo5B7QzRak+hNKiJwQdJgnEubBotM30t/YWAjUMYU=; h=Date:Subject:To:CC:References:From:In-Reply-To; b=evMQbb58XyXAq1lHsGPkUKQCndcaJFkumRnlMN5pM+vFekdkBCNIynym8mjzrZAoh krsiHJ+PwC1zEfnpRl4Md69cpgp/JNV0f9GSEdgm0G9Ib4/dKFTYSGLu8RZevynrnW 6HDQ6vEvRUPQWuiQ4wYQ+5J0loAaFoJUK4cGO2fA= Received: from DFLE107.ent.ti.com (dfle107.ent.ti.com [10.64.6.28]) by lelvem-sh01.itg.ti.com (8.18.1/8.18.1) with ESMTPS id 561DCWwg247591 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA256 bits=128 verify=FAIL); Tue, 1 Jul 2025 08:12:32 -0500 Received: from DFLE103.ent.ti.com (10.64.6.24) by DFLE107.ent.ti.com (10.64.6.28) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.55; Tue, 1 Jul 2025 08:12:31 -0500 Received: from lelvem-mr05.itg.ti.com (10.180.75.9) by DFLE103.ent.ti.com (10.64.6.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.55 via Frontend Transport; Tue, 1 Jul 2025 08:12:31 -0500 Received: from [172.24.227.193] (devarsh-precision-tower-3620.dhcp.ti.com [172.24.227.193]) by lelvem-mr05.itg.ti.com (8.18.1/8.18.1) with ESMTP id 561DCSQa1607227; Tue, 1 Jul 2025 08:12:28 -0500 Message-ID: Date: Tue, 1 Jul 2025 18:42:27 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 2/3] drm/tidss: Remove max_pclk_khz from tidss display features To: Jayesh Choudhary , , , , , , , CC: , , References: <20250701095541.190422-1-j-choudhary@ti.com> <20250701095541.190422-3-j-choudhary@ti.com> Content-Language: en-US From: Devarsh Thakkar In-Reply-To: <20250701095541.190422-3-j-choudhary@ti.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-C2ProcessedOrg: 333ef613-75bf-4e12-a4b1-8e3623f5dcea 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 > --- > 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 Regards Devarsh > + unsigned long curr_max_pclk[TIDSS_MAX_PORTS]; > > > unsigned int num_crtcs;