On Fri, Nov 01, 2024 at 06:50:27PM +0800, Yongbang Shi wrote:
> From: baihan li <libai...@huawei.com>
>
> Build a dp level that hibmc driver can enable dp by
> calling their functions.
>
> Signed-off-by: baihan li <libai...@huawei.com>
> Signed-off-by: yongbang shi <shiyongb...@huawei.com>
> ---
> ChangeLog:
> v2 -> v3:
> - fix build errors reported by kernel test robot <l...@intel.com>
> Closes:
> https://lore.kernel.org/oe-kbuild-all/202410250931.udq9s66h-...@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 <l...@intel.com>
> Closes:
> https://lore.kernel.org/oe-kbuild-all/202410040328.vevxm9yb-...@intel.com/
>
> v1:https://lore.kernel.org/all/20240930100610.782363-1-shiyongb...@huawei.com/
> ---
> drivers/gpu/drm/hisilicon/hibmc/Makefile | 2 +-
> drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c | 237 ++++++++++++++++++++
> drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h | 31 +++
> drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h | 41 ++++
> 4 files changed, 310 insertions(+), 1 deletion(-)
> 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_hw.c
> b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> new file mode 100644
> index 000000000000..214897798bdb
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> @@ -0,0 +1,237 @@
> +// 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"
> +#include "dp_link.h"
> +#include "dp_aux.h"
> +
> +static int hibmc_dp_link_init(struct dp_dev *dp)
> +{
> + dp->link.cap.lanes = 2;
> + dp->link.train_set = devm_kzalloc(dp->dev->dev,
> + dp->link.cap.lanes * sizeof(u8),
> GFP_KERNEL);
Can you replace it just with an array, removing a need for an additional
allocation?
> + if (!dp->link.train_set)
> + return -ENOMEM;
> +
> + dp->link.cap.link_rate = 1;
Ok, this is why I don't link using indices for link rates. Which rate is
this? Unlike cap.lanes this is pure magic number. I think it should be
handled other way around: store actual link rate and convert to the
register value when required.
> +
> + return 0;
> +}
> +
> +static void hibmc_dp_set_tu(struct 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 = DP_BPP;
Where is this defined? Is it hibmc-specific or a generic value?
> + rate_ks = hibmc_dp_get_link_rate(dp->link.cap.link_rate) *
> DP_LINK_RATE_CAL;
same question
> + 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);
> +
> + dp_reg_write_field(dp->base + DP_VIDEO_PACKET,
> + DP_CFG_STREAM_TU_SYMBOL_SIZE, tu_symbol_size);
> + dp_reg_write_field(dp->base + DP_VIDEO_PACKET,
> + DP_CFG_STREAM_TU_SYMBOL_FRAC_SIZE,
> tu_symbol_frac_size);
> +}
> +
> +static void hibmc_dp_set_sst(struct 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 = hibmc_dp_get_link_rate(dp->link.cap.link_rate) *
> DP_LINK_RATE_CAL;
> +
> + /* ssc: 9947 / 10000 = 0.9947 */
This is obvious. More interesting question might be what exactly is
0.9947 (or 0.53 %).
> + htotal_int = mode->htotal * 9947 / 10000;
> + htotal_size = (u32)(htotal_int * fclk / (DP_SYMBOL_PER_FCLK *
> (mode->clock / 1000)));
Drop u32 ?
> +
> + /* ssc: max effect bandwidth 53 / 10000 = 0.53% */
> + hblank_int = (mode->htotal - mode->hdisplay) - mode->hdisplay * 53 /
> 10000;
> + hblank_size = hblank_int * fclk * 9947 /
> + (mode->clock * 10 * 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));
> +
> + dp_reg_write_field(dp->base + DP_VIDEO_HORIZONTAL_SIZE,
> + DP_CFG_STREAM_HTOTAL_SIZE, htotal_size);
> + dp_reg_write_field(dp->base + DP_VIDEO_HORIZONTAL_SIZE,
> + DP_CFG_STREAM_HBLANK_SIZE, hblank_size);
> +}
> +
> +static void hibmc_dp_link_cfg(struct dp_dev *dp, struct drm_display_mode
> *mode)
> +{
> + u32 timing_delay;
> + u32 vblank;
> + u32 hstart;
> + u32 vstart;
> +
> + vblank = mode->vtotal - mode->vdisplay;
> + timing_delay = mode->htotal - mode->hsync_start;
> + hstart = mode->htotal - mode->hsync_start;
> + vstart = mode->vtotal - mode->vsync_start;
> +
> + dp_reg_write_field(dp->base + DP_TIMING_GEN_CONFIG0,
> + DP_CFG_TIMING_GEN0_HBLANK, (mode->htotal -
> mode->hdisplay));
> + dp_reg_write_field(dp->base + DP_TIMING_GEN_CONFIG0,
> + DP_CFG_TIMING_GEN0_HACTIVE, mode->hdisplay);
> +
> + dp_reg_write_field(dp->base + DP_TIMING_GEN_CONFIG2,
> + DP_CFG_TIMING_GEN0_VBLANK, vblank);
> + dp_reg_write_field(dp->base + DP_TIMING_GEN_CONFIG2,
> + DP_CFG_TIMING_GEN0_VACTIVE, mode->vdisplay);
> + dp_reg_write_field(dp->base + DP_TIMING_GEN_CONFIG3,
> + DP_CFG_TIMING_GEN0_VFRONT_PORCH, (mode->vsync_start
> - mode->vdisplay));
> +
> + dp_reg_write_field(dp->base + DP_VIDEO_CONFIG0,
> + DP_CFG_STREAM_HACTIVE, mode->hdisplay);
> + dp_reg_write_field(dp->base + DP_VIDEO_CONFIG0,
> + DP_CFG_STREAM_HBLANK, (mode->htotal -
> mode->hdisplay));
> + dp_reg_write_field(dp->base + DP_VIDEO_CONFIG2,
> + DP_CFG_STREAM_HSYNC_WIDTH, (mode->hsync_end -
> mode->hsync_start));
> +
> + dp_reg_write_field(dp->base + DP_VIDEO_CONFIG1,
> + DP_CFG_STREAM_VACTIVE, mode->vdisplay);
> + dp_reg_write_field(dp->base + DP_VIDEO_CONFIG1,
> + DP_CFG_STREAM_VBLANK, vblank);
> + dp_reg_write_field(dp->base + DP_VIDEO_CONFIG3,
> + DP_CFG_STREAM_VFRONT_PORCH, (mode->vsync_start -
> mode->vdisplay));
> + dp_reg_write_field(dp->base + DP_VIDEO_CONFIG3,
> + DP_CFG_STREAM_VSYNC_WIDTH, (mode->vsync_end -
> mode->vsync_start));
> +
> + dp_reg_write_field(dp->base + DP_VIDEO_MSA0,
> + DP_CFG_STREAM_VSTART, vstart);
> + dp_reg_write_field(dp->base + DP_VIDEO_MSA0,
> + DP_CFG_STREAM_HSTART, hstart);
> +
> + dp_reg_write_field(dp->base + DP_VIDEO_CTRL,
> DP_CFG_STREAM_VSYNC_POLARITY,
> + mode->flags & DRM_MODE_FLAG_PVSYNC ? 1 : 0);
> + dp_reg_write_field(dp->base + DP_VIDEO_CTRL,
> DP_CFG_STREAM_HSYNC_POLARITY,
> + mode->flags & DRM_MODE_FLAG_PHSYNC ? 1 : 0);
> +
> + /* MSA mic 0 and 1 */
> + writel(DP_MSA1, dp->base + DP_VIDEO_MSA1);
> + writel(DP_MSA2, dp->base + DP_VIDEO_MSA2);
> +
> + hibmc_dp_set_tu(dp, mode);
> +
> + dp_reg_write_field(dp->base + DP_VIDEO_CTRL, DP_CFG_STREAM_RGB_ENABLE,
> 0x1);
> + dp_reg_write_field(dp->base + DP_VIDEO_CTRL,
> DP_CFG_STREAM_VIDEO_MAPPING, 0);
> +
> + /* divide 2: up even */
> + if (timing_delay % 2)
> + timing_delay++;
> +
> + dp_reg_write_field(dp->base + DP_TIMING_MODEL_CTRL,
> + DP_CFG_PIXEL_NUM_TIMING_MODE_SEL1, timing_delay);
> +
> + hibmc_dp_set_sst(dp, mode);
> +}
> +
> +int hibmc_dp_hw_init(struct hibmc_dp *dp)
> +{
> + struct drm_device *drm_dev = dp->drm_dev;
> + struct dp_dev *dp_dev;
> + int ret;
> +
> + dp_dev = devm_kzalloc(drm_dev->dev, sizeof(struct dp_dev), GFP_KERNEL);
> + if (!dp_dev)
> + return -ENOMEM;
> +
> + dp->dp_dev = dp_dev;
> +
> + dp_dev->dev = drm_dev;
> + dp_dev->base = dp->mmio + DP_OFFSET;
> +
> + hibmc_dp_aux_init(dp_dev);
> +
> + ret = hibmc_dp_link_init(dp_dev);
> + if (ret) {
> + drm_err(drm_dev, "dp link init failed\n");
> + return ret;
> + }
> +
> + /* hdcp data */
> + writel(DP_HDCP, dp_dev->base + DP_HDCP_CFG);
> + /* int init */
> + writel(0, dp_dev->base + DP_INTR_ENABLE);
> + writel(DP_INT_RST, dp_dev->base + DP_INTR_ORIGINAL_STATUS);
> + /* rst */
> + writel(DP_DPTX_RST, dp_dev->base + DP_DPTX_RST_CTRL);
> + /* clock enable */
> + writel(DP_CLK_EN, dp_dev->base + DP_DPTX_CLK_CTRL);
> +
> + return 0;
> +}
> +
> +void hibmc_dp_hw_uninit(struct hibmc_dp *dp)
> +{
> + // keep this uninit interface in the future use
no reason to, introduce it when required, not the other way around
> +}
> +
> +void hibmc_dp_display_en(struct hibmc_dp *dp, bool enable)
> +{
> + struct dp_dev *dp_dev = dp->dp_dev;
> +
> + if (enable) {
> + dp_reg_write_field(dp_dev->base + DP_VIDEO_CTRL, BIT(0), 0x1);
> + writel(DP_SYNC_EN_MASK, dp_dev->base + DP_TIMING_SYNC_CTRL);
> + dp_reg_write_field(dp_dev->base + DP_DPTX_GCTL0, BIT(10), 0x1);
> + writel(DP_SYNC_EN_MASK, dp_dev->base + DP_TIMING_SYNC_CTRL);
> + } else {
> + dp_reg_write_field(dp_dev->base + DP_DPTX_GCTL0, BIT(10), 0);
> + writel(DP_SYNC_EN_MASK, dp_dev->base + DP_TIMING_SYNC_CTRL);
> + dp_reg_write_field(dp_dev->base + DP_VIDEO_CTRL, BIT(0), 0);
> + writel(DP_SYNC_EN_MASK, dp_dev->base + DP_TIMING_SYNC_CTRL);
> + }
> +
> + msleep(50);
> +}
> +
> +int hibmc_dp_mode_set(struct hibmc_dp *dp, struct drm_display_mode *mode)
> +{
> + struct dp_dev *dp_dev = dp->dp_dev;
> + int ret;
> +
> + if (!dp_dev->link.status.channel_equalized) {
> + ret = hibmc_dp_link_training(dp_dev);
> + if (ret) {
> + drm_err(dp->drm_dev, "dp link training failed, ret:
> %d\n", ret);
> + return ret;
> + }
> + }
> +
> + hibmc_dp_display_en(dp, false);
> + hibmc_dp_link_cfg(dp_dev, mode);
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> new file mode 100644
> index 000000000000..de802aaa8b4a
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/* Copyright (c) 2024 Hisilicon Limited. */
> +
> +#ifndef DP_KAPI_H
> +#define DP_KAPI_H
> +
> +#include <linux/types.h>
> +#include <linux/delay.h>
> +
> +#include <drm/drm_device.h>
> +#include <drm/drm_encoder.h>
> +#include <drm/drm_connector.h>
> +#include <drm/drm_print.h>
> +#include <video/videomode.h>
> +
> +struct dp_dev;
hibmc_dp_dev
> +
> +struct hibmc_dp {
> + struct dp_dev *dp_dev;
> + struct drm_device *drm_dev;
> + struct drm_encoder encoder;
> + struct drm_connector connector;
> + void __iomem *mmio;
> +};
> +
> +int hibmc_dp_hw_init(struct hibmc_dp *dp);
> +void hibmc_dp_hw_uninit(struct hibmc_dp *dp);
> +int hibmc_dp_mode_set(struct hibmc_dp *dp, struct drm_display_mode *mode);
> +void hibmc_dp_display_en(struct hibmc_dp *dp, bool enable);
> +
> +#endif
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> index 1032f6cde761..3dcb847057a4 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> @@ -14,8 +14,26 @@
> #define DP_AUX_STATUS 0x78
> #define DP_PHYIF_CTRL0 0xa0
> #define DP_VIDEO_CTRL 0x100
> +#define DP_VIDEO_CONFIG0 0x104
> +#define DP_VIDEO_CONFIG1 0x108
> +#define DP_VIDEO_CONFIG2 0x10c
> +#define DP_VIDEO_CONFIG3 0x110
> +#define DP_VIDEO_PACKET 0x114
> +#define DP_VIDEO_MSA0 0x118
> +#define DP_VIDEO_MSA1 0x11c
> +#define DP_VIDEO_MSA2 0x120
> +#define DP_VIDEO_HORIZONTAL_SIZE 0X124
> +#define DP_TIMING_GEN_CONFIG0 0x26c
> +#define DP_TIMING_GEN_CONFIG2 0x274
> +#define DP_TIMING_GEN_CONFIG3 0x278
> +#define DP_HDCP_CFG 0x600
> +#define DP_INTR_ENABLE 0x720
> +#define DP_INTR_ORIGINAL_STATUS 0x728
> #define DP_DPTX_RST_CTRL 0x700
> +#define DP_DPTX_CLK_CTRL 0x704
> #define DP_DPTX_GCTL0 0x708
> +#define DP_TIMING_MODEL_CTRL 0x884
> +#define DP_TIMING_SYNC_CTRL 0xFF0
>
> #define DP_CFG_AUX_SYNC_LEN_SEL BIT(1)
> #define DP_CFG_AUX_TIMER_TIMEOUT BIT(2)
> @@ -31,5 +49,28 @@
> #define DP_CFG_AUX_STATUS GENMASK(11, 4)
> #define DP_CFG_SCRAMBLE_EN BIT(0)
> #define DP_CFG_PAT_SEL GENMASK(7, 4)
> +#define DP_CFG_TIMING_GEN0_HACTIVE GENMASK(31, 16)
> +#define DP_CFG_TIMING_GEN0_HBLANK GENMASK(15, 0)
> +#define DP_CFG_TIMING_GEN0_VACTIVE GENMASK(31, 16)
> +#define DP_CFG_TIMING_GEN0_VBLANK GENMASK(15, 0)
> +#define DP_CFG_TIMING_GEN0_VFRONT_PORCH GENMASK(31, 16)
> +#define DP_CFG_STREAM_HACTIVE GENMASK(31, 16)
> +#define DP_CFG_STREAM_HBLANK GENMASK(15, 0)
> +#define DP_CFG_STREAM_HSYNC_WIDTH GENMASK(15, 0)
> +#define DP_CFG_STREAM_VACTIVE GENMASK(31, 16)
> +#define DP_CFG_STREAM_VBLANK GENMASK(15, 0)
> +#define DP_CFG_STREAM_VFRONT_PORCH GENMASK(31, 16)
> +#define DP_CFG_STREAM_VSYNC_WIDTH GENMASK(15, 0)
> +#define DP_CFG_STREAM_VSTART GENMASK(31, 16)
> +#define DP_CFG_STREAM_HSTART GENMASK(15, 0)
> +#define DP_CFG_STREAM_VSYNC_POLARITY BIT(8)
> +#define DP_CFG_STREAM_HSYNC_POLARITY BIT(7)
> +#define DP_CFG_STREAM_RGB_ENABLE BIT(1)
> +#define DP_CFG_STREAM_VIDEO_MAPPING GENMASK(5, 2)
> +#define DP_CFG_PIXEL_NUM_TIMING_MODE_SEL1 GENMASK(31, 16)
> +#define DP_CFG_STREAM_TU_SYMBOL_SIZE GENMASK(5, 0)
> +#define DP_CFG_STREAM_TU_SYMBOL_FRAC_SIZE GENMASK(9, 6)
> +#define DP_CFG_STREAM_HTOTAL_SIZE GENMASK(31, 16)
> +#define DP_CFG_STREAM_HBLANK_SIZE GENMASK(15, 0)
>
> #endif
> --
> 2.33.0
>
--
With best wishes
Dmitry
