On Fri, Nov 01, 2024 at 06:50:27PM +0800, Yongbang Shi wrote: > From: baihan li <libaihan@xxxxxxxxxx> > > Build a dp level that hibmc driver can enable dp by > calling their functions. > > Signed-off-by: baihan li <libaihan@xxxxxxxxxx> > Signed-off-by: yongbang shi <shiyongbang@xxxxxxxxxx> > --- > ChangeLog: > v2 -> v3: > - fix build errors reported by kernel test robot <lkp@xxxxxxxxx> > Closes: https://lore.kernel.org/oe-kbuild-all/202410250931.UDQ9s66H-lkp@xxxxxxxxx/ > 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@xxxxxxxxx> > Closes: https://lore.kernel.org/oe-kbuild-all/202410040328.VeVxM9yB-lkp@xxxxxxxxx/ > v1:https://lore.kernel.org/all/20240930100610.782363-1-shiyongbang@xxxxxxxxxx/ > --- > 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