From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id A02FFD2C547 for ; Tue, 22 Oct 2024 12:26:06 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2057910E670; Tue, 22 Oct 2024 12:26:06 +0000 (UTC) Received: from szxga06-in.huawei.com (szxga06-in.huawei.com [45.249.212.32]) by gabe.freedesktop.org (Postfix) with ESMTPS id DAADA10E670 for ; Tue, 22 Oct 2024 12:26:03 +0000 (UTC) Received: from mail.maildlp.com (unknown [172.19.162.112]) by szxga06-in.huawei.com (SkyGuard) with ESMTP id 4XXrxq02PCz1ynMP; Tue, 22 Oct 2024 20:26:07 +0800 (CST) Received: from kwepemd500013.china.huawei.com (unknown [7.221.188.12]) by mail.maildlp.com (Postfix) with ESMTPS id DF8D2140361; Tue, 22 Oct 2024 20:25:59 +0800 (CST) Received: from [10.159.166.136] (10.159.166.136) by kwepemd500013.china.huawei.com (7.221.188.12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1258.34; Tue, 22 Oct 2024 20:25:58 +0800 Message-ID: Date: Tue, 22 Oct 2024 20:25:57 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH drm-dp 3/4] drm/hisilicon/hibmc: add dp kapi moduel in hibmc drivers To: Dmitry Baryshkov CC: , , , , , , , , , , , , , , , References: <20240930100610.782363-1-shiyongbang@huawei.com> <20240930100610.782363-4-shiyongbang@huawei.com> From: Yongbang Shi In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.159.166.136] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To kwepemd500013.china.huawei.com (7.221.188.12) X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" > On Mon, 21 Oct 2024 at 15:22, Yongbang Shi wrote: >> Hi Dmitry, >> There're some format problems with the previous replies. Send it again here. >> Thanks for your advices, I'll resolve the problems you mentioned. >> >>> On Mon, Sep 30, 2024 at 06:06:09PM +0800, shiyongbang wrote: >>>> From: baihan li >>>> >>>> Build a kapi level that hibmc driver can enable dp by >>>> calling these kapi functions. >>>> >>>> Signed-off-by: baihan li >>>> --- >>>> drivers/gpu/drm/hisilicon/hibmc/Makefile | 2 +- >>>> .../gpu/drm/hisilicon/hibmc/dp/dp_config.h | 20 ++++++++ >>>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_kapi.c | 12 ++--- >>>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_kapi.h | 48 +++++++++++++++++++ >>>> 4 files changed, 75 insertions(+), 7 deletions(-) >>>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h >>>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_kapi.h >>>> >>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile >>>> index 94d77da88bbf..693036dfab52 100644 >>>> --- a/drivers/gpu/drm/hisilicon/hibmc/Makefile >>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile >>>> @@ -1,5 +1,5 @@ >>>> # SPDX-License-Identifier: GPL-2.0-only >>>> hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_drm_i2c.o \ >>>> - dp/dp_aux.o dp/dp_link.o >>>> + dp/dp_aux.o dp/dp_link.o dp/dp_kapi.o >>>> >>>> obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o >>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h >>>> new file mode 100644 >>>> index 000000000000..a6353a808cc4 >>>> --- /dev/null >>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h >>>> @@ -0,0 +1,20 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >>>> +/* Copyright (c) 2024 Hisilicon Limited. */ >>>> + >>>> +#ifndef DP_CONFIG_H >>>> +#define DP_CONFIG_H >>>> + >>>> +#define DP_BPP 24 >>>> +#define DP_SYMBOL_PER_FCLK 4 >>>> +#define DP_MIN_PULSE_NUM 0x9 >>>> +#define DP_MSA1 0x20 >>>> +#define DP_MSA2 0x845c00 >>>> +#define DP_OFFSET 0x1e0000 >>>> +#define DP_HDCP 0x2 >>>> +#define DP_INT_RST 0xffff >>>> +#define DP_DPTX_RST 0x3ff >>>> +#define DP_CLK_EN 0x7 >>>> +#define DP_SYNC_EN_MASK 0x3 >>>> +#define DP_LINK_RATE_CAL 27 >>> I think some of these defines were used in previous patches. Please make >>> sure that at each step the code builds without errors. >>> >>>> + >>>> +#endif >>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_kapi.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_kapi.c >>>> index 4091723473ad..ca7edc69427c 100644 >>>> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_kapi.c >>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_kapi.c >>>> @@ -64,12 +64,12 @@ static void hibmc_dp_set_tu(struct hibmc_dp_dev *dp, struct dp_mode *mode) >>>> rate_ks = dp->link.cap.link_rate * DP_LINK_RATE_CAL; >>>> value = (pixel_clock * bpp * 5000) / (61 * lane_num * rate_ks); >>>> >>>> - if (value % 10 == 9) { /* 10: div, 9: carry */ >>>> - tu_symbol_size = value / 10 + 1; /* 10: div */ >>>> + if (value % 10 == 9) { /* 9 carry */ >>>> + tu_symbol_size = value / 10 + 1; >>>> tu_symbol_frac_size = 0; >>>> } else { >>>> - tu_symbol_size = value / 10; /* 10: div */ >>>> - tu_symbol_frac_size = value % 10 + 1; /* 10: div */ >>>> + tu_symbol_size = value / 10; >>>> + tu_symbol_frac_size = value % 10 + 1; >>>> } >>>> >>>> drm_info(dp->dev, "tu value: %u.%u value: %u\n", >>>> @@ -158,7 +158,7 @@ static void hibmc_dp_link_cfg(struct hibmc_dp_dev *dp, struct dp_mode *mode) >>>> dp_write_bits(dp->base + DP_VIDEO_CTRL, >>>> DP_CFG_STREAM_HSYNC_POLARITY, mode->h_pol); >>>> >>>> - /* MSA mic 0 and 1*/ >>>> + /* MSA mic 0 and 1 */ >>>> writel(DP_MSA1, dp->base + DP_VIDEO_MSA1); >>>> writel(DP_MSA2, dp->base + DP_VIDEO_MSA2); >>>> >>>> @@ -167,7 +167,7 @@ static void hibmc_dp_link_cfg(struct hibmc_dp_dev *dp, struct dp_mode *mode) >>>> dp_write_bits(dp->base + DP_VIDEO_CTRL, DP_CFG_STREAM_RGB_ENABLE, 0x1); >>>> dp_write_bits(dp->base + DP_VIDEO_CTRL, DP_CFG_STREAM_VIDEO_MAPPING, 0); >>>> >>>> - /*divide 2: up even */ >>>> + /* divide 2: up even */ >>>> if (timing_delay % 2) >>>> timing_delay++; >>>> >>> This should be squashed into the previous commits. >>> >>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_kapi.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_kapi.h >>>> new file mode 100644 >>>> index 000000000000..6b07642d55b8 >>>> --- /dev/null >>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_kapi.h >>>> @@ -0,0 +1,48 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >>>> +/* Copyright (c) 2024 Hisilicon Limited. */ >>>> + >>>> +#ifndef DP_KAPI_H >>>> +#define DP_KAPI_H >>>> + >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>> Sort the headers, please. >>> >>>> + >>>> +struct hibmc_dp_dev; >>>> + >>>> +struct dp_mode { >>>> + u32 h_total; >>>> + u32 h_active; >>>> + u32 h_blank; >>>> + u32 h_front; >>>> + u32 h_sync; >>>> + u32 h_back; >>>> + bool h_pol; >>>> + u32 v_total; >>>> + u32 v_active; >>>> + u32 v_blank; >>>> + u32 v_front; >>>> + u32 v_sync; >>>> + u32 v_back; >>>> + bool v_pol; >>>> + u32 field_rate; >>>> + u32 pixel_clock; // khz >>> Why do you need a separate struct for this? >> I can try to use drm_mode function and refactor this struct, but they're insufficient for our scenarios. >> Here's change template bellow: > But you are generating the data from struct drm_display_mode. Please > use the existing struct instead and generate the blank and porch > timings when you have to program them. > There is really no need to define another struct just to temporarily > hold the same data. I got it! I'll directly use drm_mode values in dp config. Thanks, Baihan >> struct dp_mode { >> sturct videomode mode; >> u32 h_total; >> u32 h_blank; >> u32 v_total; >> u32 v_blank; >> u32 field_rate; >> }; >> static void dp_mode_cfg(struct dp_mode *dp_mode, struct drm_display_mode *mode) >> { >> dp_mode->field_rate = drm_mode_vrefresh(mode); >> drm_display_mode_to_videomode(mode, &dp_mode->vmode); >> dp_mode->h_total = mode->htotal; >> dp_mode->h_blank = mode->htotal - mode->hdisplay; >> dp_mode->v_total = mode->vtotal; >> dp_mode->v_blank = mode->vtotal - mode->vdisplay; >> } >>