1. Patchew
  2. linux
  3. [PATCH drm-dp 0/4] Add dp module in hibmc driver
  4. View patch

[PATCH drm-dp 3/4] drm/hisilicon/hibmc: add dp kapi moduel in hibmc drivers

shiyongbang posted 4 patches 1 month, 4 weeks ago
There is a newer version of this series
[PATCH drm-dp 3/4] drm/hisilicon/hibmc: add dp kapi moduel in hibmc drivers
Posted by shiyongbang 1 month, 4 weeks ago 
From: baihan li <libaihan@huawei.com>

Build a kapi level that hibmc driver can enable dp by
calling these kapi functions.

Signed-off-by: baihan li <libaihan@huawei.com>
---
 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
+
+#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++;
 
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 <linux/types.h>
+#include <drm/drm_device.h>
+#include <drm/drm_encoder.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_print.h>
+#include <linux/delay.h>
+
+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
+};
+
+struct hibmc_dp {
+	struct hibmc_dp_dev *dp_dev;
+	struct drm_device *drm_dev;
+	struct drm_encoder encoder;
+	struct drm_connector connector;
+	void __iomem *mmio;
+};
+
+int hibmc_dp_kapi_init(struct hibmc_dp *dp);
+void hibmc_dp_kapi_uninit(struct hibmc_dp *dp);
+int hibmc_dp_mode_set(struct hibmc_dp *dp, struct dp_mode *mode);
+void hibmc_dp_display_en(struct hibmc_dp *dp, bool enable);
+
+#endif
-- 
2.33.0
Re: [PATCH drm-dp 3/4] drm/hisilicon/hibmc: add dp kapi moduel in hibmc drivers
Posted by Dmitry Baryshkov 1 month, 1 week ago 
On Mon, Sep 30, 2024 at 06:06:09PM +0800, shiyongbang wrote:
> From: baihan li <libaihan@huawei.com>
> 
> Build a kapi level that hibmc driver can enable dp by
> calling these kapi functions.
> 
> Signed-off-by: baihan li <libaihan@huawei.com>
> ---
>  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 <linux/types.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_encoder.h>
> +#include <drm/drm_connector.h>
> +#include <drm/drm_print.h>
> +#include <linux/delay.h>

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?

> +};
> +
> +struct hibmc_dp {
> +	struct hibmc_dp_dev *dp_dev;
> +	struct drm_device *drm_dev;
> +	struct drm_encoder encoder;
> +	struct drm_connector connector;
> +	void __iomem *mmio;
> +};
> +
> +int hibmc_dp_kapi_init(struct hibmc_dp *dp);
> +void hibmc_dp_kapi_uninit(struct hibmc_dp *dp);
> +int hibmc_dp_mode_set(struct hibmc_dp *dp, struct dp_mode *mode);
> +void hibmc_dp_display_en(struct hibmc_dp *dp, bool enable);

It looks like this should also be defined earlier.

> +
> +#endif
> -- 
> 2.33.0
> 

-- 
With best wishes
Dmitry
Re: [PATCH drm-dp 3/4] drm/hisilicon/hibmc: add dp kapi moduel in hibmc drivers
Posted by Yongbang Shi 1 month, 1 week ago 
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 <libaihan@huawei.com>
>>
>> Build a kapi level that hibmc driver can enable dp by
>> calling these kapi functions.
>>
>> Signed-off-by: baihan li <libaihan@huawei.com>
>> ---
>>   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 <linux/types.h>
>> +#include <drm/drm_device.h>
>> +#include <drm/drm_encoder.h>
>> +#include <drm/drm_connector.h>
>> +#include <drm/drm_print.h>
>> +#include <linux/delay.h>
> 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:
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;

}


>> +};
>> +
>> +struct hibmc_dp {
>> +	struct hibmc_dp_dev *dp_dev;
>> +	struct drm_device *drm_dev;
>> +	struct drm_encoder encoder;
>> +	struct drm_connector connector;
>> +	void __iomem *mmio;
>> +};
>> +
>> +int hibmc_dp_kapi_init(struct hibmc_dp *dp);
>> +void hibmc_dp_kapi_uninit(struct hibmc_dp *dp);
>> +int hibmc_dp_mode_set(struct hibmc_dp *dp, struct dp_mode *mode);
>> +void hibmc_dp_display_en(struct hibmc_dp *dp, bool enable);
> It looks like this should also be defined earlier.
>
>> +
>> +#endif
>> -- 
>> 2.33.0
>>
Re: [PATCH drm-dp 3/4] drm/hisilicon/hibmc: add dp kapi moduel in hibmc drivers
Posted by Dmitry Baryshkov 1 month, 1 week ago 
On Mon, 21 Oct 2024 at 15:22, Yongbang Shi <shiyongbang@huawei.com> 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 <libaihan@huawei.com>
> >>
> >> Build a kapi level that hibmc driver can enable dp by
> >> calling these kapi functions.
> >>
> >> Signed-off-by: baihan li <libaihan@huawei.com>
> >> ---
> >>   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 <linux/types.h>
> >> +#include <drm/drm_device.h>
> >> +#include <drm/drm_encoder.h>
> >> +#include <drm/drm_connector.h>
> >> +#include <drm/drm_print.h>
> >> +#include <linux/delay.h>
> > 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.

> 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;
> }
>

-- 
With best wishes
Dmitry
Re: [PATCH drm-dp 3/4] drm/hisilicon/hibmc: add dp kapi moduel in hibmc drivers
Posted by Yongbang Shi 1 month, 1 week ago 
> On Mon, 21 Oct 2024 at 15:22, Yongbang Shi <shiyongbang@huawei.com> 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 <libaihan@huawei.com>
>>>>
>>>> Build a kapi level that hibmc driver can enable dp by
>>>> calling these kapi functions.
>>>>
>>>> Signed-off-by: baihan li <libaihan@huawei.com>
>>>> ---
>>>>    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 <linux/types.h>
>>>> +#include <drm/drm_device.h>
>>>> +#include <drm/drm_encoder.h>
>>>> +#include <drm/drm_connector.h>
>>>> +#include <drm/drm_print.h>
>>>> +#include <linux/delay.h>
>>> 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;
>> }
>>
Re: [PATCH drm-dp 3/4] drm/hisilicon/hibmc: add dp kapi moduel in hibmc drivers
Posted by s00452708 1 month, 1 week ago 
Hi Dmitry,
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 <libaihan@huawei.com>
>>
>> Build a kapi level that hibmc driver can enable dp by
>> calling these kapi functions.
>>
>> Signed-off-by: baihan li <libaihan@huawei.com>
>> ---
>>   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 <linux/types.h>
>> +#include <drm/drm_device.h>
>> +#include <drm/drm_encoder.h>
>> +#include <drm/drm_connector.h>
>> +#include <drm/drm_print.h>
>> +#include <linux/delay.h>
> 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 ourscenarios.Here's change template bellow: 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; }
>> +};
>> +
>> +struct hibmc_dp {
>> +	struct hibmc_dp_dev *dp_dev;
>> +	struct drm_device *drm_dev;
>> +	struct drm_encoder encoder;
>> +	struct drm_connector connector;
>> +	void __iomem *mmio;
>> +};
>> +
>> +int hibmc_dp_kapi_init(struct hibmc_dp *dp);
>> +void hibmc_dp_kapi_uninit(struct hibmc_dp *dp);
>> +int hibmc_dp_mode_set(struct hibmc_dp *dp, struct dp_mode *mode);
>> +void hibmc_dp_display_en(struct hibmc_dp *dp, bool enable);
> It looks like this should also be defined earlier.
>
>> +
>> +#endif
>> -- 
>> 2.33.0
>>
Re: [PATCH drm-dp 3/4] drm/hisilicon/hibmc: add dp kapi moduel in hibmc drivers
Posted by kernel test robot 1 month, 3 weeks ago 
Hi shiyongbang,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v6.12-rc1 next-20241003]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/shiyongbang/drm-hisilicon-hibmc-add-dp-aux-in-hibmc-drivers/20240930-181437
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20240930100610.782363-4-shiyongbang%40huawei.com
patch subject: [PATCH drm-dp 3/4] drm/hisilicon/hibmc: add dp kapi moduel in hibmc drivers
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20241004/202410040328.VeVxM9yB-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241004/202410040328.VeVxM9yB-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410040328.VeVxM9yB-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: drivers/gpu/drm/hisilicon/hibmc/dp/dp_kapi.o: in function `hibmc_dp_link_cfg':
>> dp_kapi.c:(.text+0x233): undefined reference to `__udivdi3'
>> ld: dp_kapi.c:(.text+0x35a): undefined reference to `__udivdi3'
   ld: dp_kapi.c:(.text+0x3cd): undefined reference to `__udivdi3'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.