From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from canpmsgout07.his.huawei.com (canpmsgout07.his.huawei.com [113.46.200.222]) (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 B0543394469 for ; Tue, 7 Apr 2026 07:37:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=113.46.200.222 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775547428; cv=none; b=E1meETxpjBg+paHfSWeRmS2cGN8HFVu7Eh73m8wtnjxf42QE1d4MHBIjB+WRfS3ugx/ZRMIY1y1StaRNNK46qkWSFSnBsra1Vlzfqnhw1rOSdpmc0f/LHUcnSqvCqrZXDNUl763lQ6rB9F6ltZNfCWf3VQvMlLSwAw7isVKcD1E= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775547428; c=relaxed/simple; bh=xZz5eSAYPGX6IS9MrpsBpng5yMvm01wp2XklQDhXADM=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=lrDhQ+Ahbg0CknEL0JyTZq1lV2iZTLx2ajLQ9SkrEPfpKgihxQrMmAsq04RHVY/bIUxRQLd7niTu2s2PkCzq6H+1jE61vEB9AnW1FYEOsJn6JKJDEitP2lQt2d77q6ZY2gJmRXoyuOjXLfV9Mr9D2QzTbAMC6GIgtzcIbrVH1Kw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; dkim=pass (1024-bit key) header.d=huawei.com header.i=@huawei.com header.b=zTkbjES6; arc=none smtp.client-ip=113.46.200.222 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=huawei.com header.i=@huawei.com header.b="zTkbjES6" dkim-signature: v=1; a=rsa-sha256; d=huawei.com; s=dkim; c=relaxed/relaxed; q=dns/txt; h=From; bh=ekYJtS41Y1m1v/Uwg6Lhd8rfE0HgzAD0WnjVmoaU93I=; b=zTkbjES6ttxf+T2EkkuA+k6BfWVEeC+rXC9BLnj3nZ0FZTTd9+LIvJskIAWFq1wG30xSyUJ9R /nlkrOmV92MBK23Nf2lBraz7cOGPQI8Qp337/DwD6O10LkeOP7npVfOTQSmcpI2PT0zI8H4zB7D bZ3/NhlIuCNcMe1vDLbnDiY= Received: from mail.maildlp.com (unknown [172.19.162.92]) by canpmsgout07.his.huawei.com (SkyGuard) with ESMTPS id 4fqdCQ5CvpzLlVQ; Tue, 7 Apr 2026 15:30:42 +0800 (CST) Received: from dggemv712-chm.china.huawei.com (unknown [10.1.198.32]) by mail.maildlp.com (Postfix) with ESMTPS id 6251040565; Tue, 7 Apr 2026 15:36:55 +0800 (CST) Received: from kwepemq100007.china.huawei.com (7.202.195.175) by dggemv712-chm.china.huawei.com (10.1.198.32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Tue, 7 Apr 2026 15:36:55 +0800 Received: from [10.159.167.44] (10.159.167.44) by kwepemq100007.china.huawei.com (7.202.195.175) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Tue, 7 Apr 2026 15:36:54 +0800 Message-ID: <4d82b278-3699-4407-94d4-2317e98017f2@huawei.com> Date: Tue, 7 Apr 2026 15:36:54 +0800 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 for drm-misc-fixes v2 4/4] drm/hisilicon/hibmc: use clock to look up the PLL value To: "tiantao (H)" , , , , , , , , CC: , , , , , , , , References: <20260403024828.1131906-1-shiyongbang@huawei.com> <20260403024828.1131906-5-shiyongbang@huawei.com> From: Yongbang Shi In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: kwepems200001.china.huawei.com (7.221.188.67) To kwepemq100007.china.huawei.com (7.202.195.175) >> From: Lin He >> >> In the past, we use width and height to look up our PLL value. >> But actually the actual clock check is also necessnary. There are >> some resolutions that width and height same, but its clock different. >> Add the clock check when using pll_table to determine the PLL value. >> >> Fixes: da52605eea8f ("drm/hisilicon/hibmc: Add support for display >> engine") >> Signed-off-by: Lin He >> Signed-off-by: Yongbang Shi >> --- >> ChangeLog: >> v1 -> v2: >>    - remove tag "Reviewed-by: Tao Tian ", >> witch will >>      be given in public. >>    - add 'drm-misc-fixes' in subject prefix. >> --- >>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c    | 80 +++++++++++-------- >>   1 file changed, 46 insertions(+), 34 deletions(-) >> >> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/ >> gpu/drm/hisilicon/hibmc/hibmc_drm_de.c >> index 89bed78f1466..8561acbbc3c8 100644 >> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c >> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c >> @@ -22,6 +22,8 @@ >>   #include "hibmc_drm_drv.h" >>   #include "hibmc_drm_regs.h" >> +#define CLOCK_TOLERANCE 100 /* kHz tolerance */ >> + > The macro CLOCK_TOLERANCE is introduced in this patch but not referenced. > If it's not required, please drop the definition to avoid dead code. > If it is intended for future use, please add a comment explaining its > purpose. > Ok, CLOCK_TOLERANCE will not be used. It will be removed in v3. Thanks, Yongbang. >>   struct hibmc_display_panel_pll { >>       u64 M; >>       u64 N; >> @@ -32,26 +34,43 @@ struct hibmc_display_panel_pll { >>   struct hibmc_dislay_pll_config { >>       u64 hdisplay; >>       u64 vdisplay; >> +    int clock; >>       u32 pll1_config_value; >>       u32 pll2_config_value; >>   }; >>   static const struct hibmc_dislay_pll_config hibmc_pll_table[] = { >> -    {640, 480, CRT_PLL1_HS_25MHZ, CRT_PLL2_HS_25MHZ}, >> -    {800, 600, CRT_PLL1_HS_40MHZ, CRT_PLL2_HS_40MHZ}, >> -    {1024, 768, CRT_PLL1_HS_65MHZ, CRT_PLL2_HS_65MHZ}, >> -    {1152, 864, CRT_PLL1_HS_80MHZ_1152, CRT_PLL2_HS_80MHZ}, >> -    {1280, 768, CRT_PLL1_HS_80MHZ, CRT_PLL2_HS_80MHZ}, >> -    {1280, 720, CRT_PLL1_HS_74MHZ, CRT_PLL2_HS_74MHZ}, >> -    {1280, 960, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ}, >> -    {1280, 1024, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ}, >> -    {1440, 900, CRT_PLL1_HS_106MHZ, CRT_PLL2_HS_106MHZ}, >> -    {1600, 900, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ}, >> -    {1600, 1200, CRT_PLL1_HS_162MHZ, CRT_PLL2_HS_162MHZ}, >> -    {1920, 1080, CRT_PLL1_HS_148MHZ, CRT_PLL2_HS_148MHZ}, >> -    {1920, 1200, CRT_PLL1_HS_193MHZ, CRT_PLL2_HS_193MHZ}, >> +    {640, 480, 25000, CRT_PLL1_HS_25MHZ, CRT_PLL2_HS_25MHZ}, >> +    {800, 600, 40000, CRT_PLL1_HS_40MHZ, CRT_PLL2_HS_40MHZ}, >> +    {1024, 768, 65000, CRT_PLL1_HS_65MHZ, CRT_PLL2_HS_65MHZ}, >> +    {1152, 864, 78750, CRT_PLL1_HS_80MHZ_1152, CRT_PLL2_HS_80MHZ}, >> +    {1280, 768, 80000, CRT_PLL1_HS_80MHZ, CRT_PLL2_HS_80MHZ}, >> +    {1280, 720, 74375, CRT_PLL1_HS_74MHZ, CRT_PLL2_HS_74MHZ}, >> +    {1280, 960, 108000, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ}, >> +    {1280, 1024, 108000, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ}, >> +    {1440, 900, 105952, CRT_PLL1_HS_106MHZ, CRT_PLL2_HS_106MHZ}, >> +    {1600, 900, 108000, CRT_PLL1_HS_108MHZ, CRT_PLL2_HS_108MHZ}, >> +    {1600, 1200, 162500, CRT_PLL1_HS_162MHZ, CRT_PLL2_HS_162MHZ}, >> +    {1920, 1080, 148750, CRT_PLL1_HS_148MHZ, CRT_PLL2_HS_148MHZ}, >> +    {1920, 1200, 193750, CRT_PLL1_HS_193MHZ, CRT_PLL2_HS_193MHZ}, >>   }; >> +static int hibmc_get_best_clock_idx(const struct drm_display_mode *mode) >> +{ >> +    int i, diff; >> + >> +    for (i = 0; i < ARRAY_SIZE(hibmc_pll_table); i++) { >> +        if (hibmc_pll_table[i].hdisplay == mode->hdisplay && >> +            hibmc_pll_table[i].vdisplay == mode->vdisplay) { >> +            diff = abs(mode->clock - hibmc_pll_table[i].clock); >> +            if (diff < mode->clock / 100) /* tolerance 1/100 */ >> +                return i; >> +        } >> +    } >> + >> +    return -EOPNOTSUPP; >> +} >> + >>   static int hibmc_plane_atomic_check(struct drm_plane *plane, >>                       struct drm_atomic_state *state) >>   { >> @@ -214,17 +233,13 @@ static enum drm_mode_status >>   hibmc_crtc_mode_valid(struct drm_crtc *crtc, >>                 const struct drm_display_mode *mode) >>   { >> -    size_t i = 0; >>       int vrefresh = drm_mode_vrefresh(mode); >>       if (vrefresh < 59 || vrefresh > 61) >>           return MODE_NOCLOCK; >> -    for (i = 0; i < ARRAY_SIZE(hibmc_pll_table); i++) { >> -        if (hibmc_pll_table[i].hdisplay == mode->hdisplay && >> -            hibmc_pll_table[i].vdisplay == mode->vdisplay) >> -            return MODE_OK; >> -    } >> +    if (hibmc_get_best_clock_idx(mode) >= 0) >> +        return MODE_OK; >>       return MODE_BAD; >>   } >> @@ -281,23 +296,20 @@ static void set_vclock_hisilicon(struct >> drm_device *dev, u64 pll) >>       writel(val, priv->mmio + CRT_PLL1_HS); >>   } >> -static void get_pll_config(u64 x, u64 y, u32 *pll1, u32 *pll2) >> +static void get_pll_config(struct drm_display_mode *mode, u32 *pll1, >> u32 *pll2) >>   { >> -    size_t i; >> -    size_t count = ARRAY_SIZE(hibmc_pll_table); >> - >> -    for (i = 0; i < count; i++) { >> -        if (hibmc_pll_table[i].hdisplay == x && >> -            hibmc_pll_table[i].vdisplay == y) { >> -            *pll1 = hibmc_pll_table[i].pll1_config_value; >> -            *pll2 = hibmc_pll_table[i].pll2_config_value; >> -            return; >> -        } >> +    int idx; >> + >> +    idx = hibmc_get_best_clock_idx(mode); >> +    if (idx < 0) { >> +        /* if found none, we use default value */ >> +        *pll1 = CRT_PLL1_HS_25MHZ; >> +        *pll2 = CRT_PLL2_HS_25MHZ; >> +        return; >>       } >> -    /* if found none, we use default value */ >> -    *pll1 = CRT_PLL1_HS_25MHZ; >> -    *pll2 = CRT_PLL2_HS_25MHZ; >> +    *pll1 = hibmc_pll_table[idx].pll1_config_value; >> +    *pll2 = hibmc_pll_table[idx].pll2_config_value; >>   } >>   /* >> @@ -319,7 +331,7 @@ static u32 display_ctrl_adjust(struct drm_device >> *dev, >>       x = mode->hdisplay; >>       y = mode->vdisplay; >> -    get_pll_config(x, y, &pll1, &pll2); >> +    get_pll_config(mode, &pll1, &pll2); >>       writel(pll2, priv->mmio + CRT_PLL2_HS); >>       set_vclock_hisilicon(dev, pll1);