[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <yqloscxeflxj6lvq5pmktpznmp2iv4p4ddavcvlscbtmytqt33@k5n66gh5axxq>
Date: Fri, 22 Nov 2024 04:18:16 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Yongbang Shi <shiyongbang@...wei.com>
Cc: xinliang.liu@...aro.org, tiantao6@...ilicon.com,
maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de, airlied@...il.com,
daniel@...ll.ch, kong.kongxinwei@...ilicon.com, liangjian010@...wei.com,
chenjianmin@...wei.com, lidongming5@...wei.com, libaihan@...wei.com,
shenjian15@...wei.com, shaojijie@...wei.com, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 drm-dp 3/5] drm/hisilicon/hibmc: add dp hw moduel in
hibmc
On Mon, Nov 18, 2024 at 10:28:03PM +0800, Yongbang Shi wrote:
> From: baihan li <libaihan@...wei.com>
>
> Build a dp level that hibmc driver can enable dp by
> calling their functions.
>
> Signed-off-by: Baihan Li <libaihan@...wei.com>
> Signed-off-by: Yongbang Shi <shiyongbang@...wei.com>
> ---
> ChangeLog:
> v3 -> v4:
> - changed the type of train_set to array, suggested by Dmitry Baryshkov.
> - using actual link rate instead of magic num, suggested by Dmitry Baryshkov.
> - deleting hibmc_dp_hw_uninit(), suggested by Dmitry Baryshkov.
> v2 -> v3:
> - fix build errors reported by kernel test robot <lkp@...el.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202410250931.UDQ9s66H-lkp@intel.com/
> v1 -> v2:
> - changed some defines and functions to former patch, suggested by Dmitry Baryshkov.
> - sorting the headers including in dp_hw.h and hibmc_drm_drv.c files, suggested by Dmitry Baryshkov.
> - deleting struct dp_mode and dp_mode_cfg function, suggested by Dmitry Baryshkov.
> - fix build errors reported by kernel test robot <lkp@...el.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202410040328.VeVxM9yB-lkp@intel.com/
> v1:https://lore.kernel.org/all/20240930100610.782363-1-shiyongbang@huawei.com/
> ---
> drivers/gpu/drm/hisilicon/hibmc/Makefile | 2 +-
> .../gpu/drm/hisilicon/hibmc/dp/dp_config.h | 19 ++
> drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c | 217 ++++++++++++++++++
> drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h | 28 +++
> drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h | 41 ++++
> 5 files changed, 306 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> index 94d77da88bbf..214228052ccf 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_hw.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..74dd9956144e
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/* Copyright (c) 2024 Hisilicon Limited. */
> +
> +#ifndef DP_CONFIG_H
> +#define DP_CONFIG_H
> +
> +#define HIBMC_DP_BPP 24
> +#define HIBMC_DP_SYMBOL_PER_FCLK 4
> +#define HIBMC_DP_MSA1 0x20
> +#define HIBMC_DP_MSA2 0x845c00
> +#define HIBMC_DP_OFFSET 0x1e0000
> +#define HIBMC_DP_HDCP 0x2
> +#define HIBMC_DP_INT_RST 0xffff
> +#define HIBMC_DP_DPTX_RST 0x3ff
> +#define HIBMC_DP_CLK_EN 0x7
> +#define HIBMC_DP_SYNC_EN_MASK 0x3
> +#define HIBMC_DP_LINK_RATE_CAL 27
> +
> +#endif
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> new file mode 100644
> index 000000000000..9d7337cd9309
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> @@ -0,0 +1,217 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// Copyright (c) 2024 Hisilicon Limited.
> +
> +#include <linux/io.h>
> +#include <linux/delay.h>
> +#include "dp_config.h"
> +#include "dp_comm.h"
> +#include "dp_reg.h"
> +#include "dp_hw.h"
> +
> +static void hibmc_dp_set_tu(struct hibmc_dp_dev *dp, struct drm_display_mode *mode)
> +{
> + u32 tu_symbol_frac_size;
> + u32 tu_symbol_size;
> + u32 rate_ks;
> + u8 lane_num;
> + u32 value;
> + u32 bpp;
> +
> + lane_num = dp->link.cap.lanes;
> + if (lane_num == 0) {
> + drm_err(dp->dev, "set tu failed, lane num cannot be 0!\n");
> + return;
> + }
> +
> + bpp = HIBMC_DP_BPP;
> + rate_ks = dp->link.cap.link_rate * HIBMC_DP_LINK_RATE_CAL;
> + value = (mode->clock * bpp * 5) / (61 * lane_num * rate_ks);
> +
> + if (value % 10 == 9) { /* 9 carry */
> + tu_symbol_size = value / 10 + 1;
> + tu_symbol_frac_size = 0;
> + } else {
> + tu_symbol_size = value / 10;
> + tu_symbol_frac_size = value % 10 + 1;
> + }
> +
> + drm_info(dp->dev, "tu value: %u.%u value: %u\n",
> + tu_symbol_size, tu_symbol_frac_size, value);
drm_dbg_driver()
> +
> + hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_PACKET,
> + HIBMC_DP_CFG_STREAM_TU_SYMBOL_SIZE, tu_symbol_size);
> + hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_PACKET,
> + HIBMC_DP_CFG_STREAM_TU_SYMBOL_FRAC_SIZE, tu_symbol_frac_size);
> +}
> +
> +static void hibmc_dp_set_sst(struct hibmc_dp_dev *dp, struct drm_display_mode *mode)
> +{
> + u32 hblank_size;
> + u32 htotal_size;
> + u32 htotal_int;
> + u32 hblank_int;
> + u32 fclk; /* flink_clock */
> +
> + fclk = dp->link.cap.link_rate * HIBMC_DP_LINK_RATE_CAL;
> +
> + htotal_int = mode->htotal * 9947 / 10000;
> + htotal_size = htotal_int * fclk / (HIBMC_DP_SYMBOL_PER_FCLK * (mode->clock / 1000));
> +
> + hblank_int = mode->htotal - mode->hdisplay - mode->hdisplay * 53 / 10000;
> + hblank_size = hblank_int * fclk * 9947 /
Still no idea, what 0.9947 is.
> + (mode->clock * 10 * HIBMC_DP_SYMBOL_PER_FCLK);
> +
> + drm_info(dp->dev, "h_active %u v_active %u htotal_size %u hblank_size %u",
> + mode->hdisplay, mode->vdisplay, htotal_size, hblank_size);
> + drm_info(dp->dev, "flink_clock %u pixel_clock %d", fclk, mode->clock / 1000);
And here. Please review the driver not to output any debug information
by default.
> +
> + hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_HORIZONTAL_SIZE,
> + HIBMC_DP_CFG_STREAM_HTOTAL_SIZE, htotal_size);
> + hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_HORIZONTAL_SIZE,
> + HIBMC_DP_CFG_STREAM_HBLANK_SIZE, hblank_size);
> +}
> +
Other than that LGTM
--
With best wishes
Dmitry
Powered by blists - more mailing lists