Re: [PATCH v1 12/14] drm/msm/disp/dpu1: revise timing engine programming to work for DSC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 23/01/2023 20:24, Kuogee Hsieh wrote:
Current implementation timing engine programming does not consider
compression factors. This patch add consideration of DSC factors
while programming timing engine.

Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
---
  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   |   2 +
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h     |  14 ++-
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c        | 132 +++++++++++++--------
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h        |  10 +-
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h         |   6 +-
  5 files changed, 110 insertions(+), 54 deletions(-)


[skipped]

@@ -113,82 +124,96 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,
  	/* read interface_cfg */
  	intf_cfg = DPU_REG_READ(c, INTF_CONFIG);
- if (ctx->cap->type == INTF_DP)
+	if (ctx->cap->type == INTF_EDP || ctx->cap->type == INTF_DP)
  		dp_intf = true;
hsync_period = p->hsync_pulse_width + p->h_back_porch + p->width +
-	p->h_front_porch;
+			p->h_front_porch;
  	vsync_period = p->vsync_pulse_width + p->v_back_porch + p->height +
-	p->v_front_porch;
+			p->v_front_porch;

Actually I went on through the history and found the previous submission, https://patchwork.freedesktop.org/patch/471505/. Exactly the same piece of code. Did you expect that the comments will be different this time?

I really hoped that at that time we already went through this. But it seems I was wrong. That series went through v10 or v12 before being accepted. And it was just adding wide_bus_en. Back at that time we lightly discussed that the code will receive compression support. But I never expected to see the original submission again.

It might sound bad, but could you please find somebody who can do internal review for you? Good internal review.

That said, I really do not expect to see v2 before the whole series is reworked, restructured and prepared for the review on your side.

display_v_start = ((p->vsync_pulse_width + p->v_back_porch) *
-	hsync_period) + p->hsync_skew;
+			hsync_period) + p->hsync_skew;
  	display_v_end = ((vsync_period - p->v_front_porch) * hsync_period) +
-	p->hsync_skew - 1;
+			p->hsync_skew - 1;
+
+	hsync_ctl = (hsync_period << 16) | p->hsync_pulse_width;
hsync_start_x = p->h_back_porch + p->hsync_pulse_width;
  	hsync_end_x = hsync_period - p->h_front_porch - 1;
- if (p->width != p->xres) { /* border fill added */
-		active_h_start = hsync_start_x;
-		active_h_end = active_h_start + p->xres - 1;
-	} else {
-		active_h_start = 0;
-		active_h_end = 0;
-	}
-
-	if (p->height != p->yres) { /* border fill added */
-		active_v_start = display_v_start;
-		active_v_end = active_v_start + (p->yres * hsync_period) - 1;
-	} else {
-		active_v_start = 0;
-		active_v_end = 0;
-	}
-
-	if (active_h_end) {
-		active_hctl = (active_h_end << 16) | active_h_start;
-		intf_cfg |= INTF_CFG_ACTIVE_H_EN;
-	} else {
-		active_hctl = 0;
-	}
-
-	if (active_v_end)
-		intf_cfg |= INTF_CFG_ACTIVE_V_EN;
-
-	hsync_ctl = (hsync_period << 16) | p->hsync_pulse_width;
-	display_hctl = (hsync_end_x << 16) | hsync_start_x;
-
  	/*
  	 * DATA_HCTL_EN controls data timing which can be different from
  	 * video timing. It is recommended to enable it for all cases, except
  	 * if compression is enabled in 1 pixel per clock mode
  	 */
+	if (!p->compression_en || p->wide_bus_en)
+		intf_cfg2 |= INTF_CFG2_DATA_HCTL_EN;
+
  	if (p->wide_bus_en)
-		intf_cfg2 |= INTF_CFG2_DATABUS_WIDEN | INTF_CFG2_DATA_HCTL_EN;
+		intf_cfg2 |= INTF_CFG2_DATABUS_WIDEN;
+ /*
+	 * If widebus is disabled:
+	 * For uncompressed stream, the data is valid for the entire active
+	 * window period.
+	 * For compressed stream, data is valid for a shorter time period
+	 * inside the active window depending on the compression ratio.
+	 *
+	 * If widebus is enabled:
+	 * For uncompressed stream, data is valid for only half the active
+	 * window, since the data rate is doubled in this mode.
+	 * p->width holds the adjusted width for DP but unadjusted width for DSI
+	 * For compressed stream, data validity window needs to be adjusted for
+	 * compression ratio and then further halved.
+	 */
  	data_width = p->width;
+ if (p->compression_en) {
+		if (p->wide_bus_en)
+			data_width = DIV_ROUND_UP(p->dce_bytes_per_line, 6);
+		else
+			data_width = DIV_ROUND_UP(p->dce_bytes_per_line, 3);
+	} else if (!dp_intf && p->wide_bus_en) {
+		data_width = p->width >> 1;
+	} else {
+		data_width = p->width;
+	}
+
  	hsync_data_start_x = hsync_start_x;
  	hsync_data_end_x =  hsync_start_x + data_width - 1;
+ display_hctl = (hsync_end_x << 16) | hsync_start_x;
  	display_data_hctl = (hsync_data_end_x << 16) | hsync_data_start_x;
if (dp_intf) {
  		/* DP timing adjustment */
  		display_v_start += p->hsync_pulse_width + p->h_back_porch;
  		display_v_end   -= p->h_front_porch;
+	}
+
+	intf_cfg |= INTF_CFG_ACTIVE_H_EN;
+	intf_cfg |= INTF_CFG_ACTIVE_V_EN;
+	active_h_start = hsync_start_x;
+	active_h_end = active_h_start + p->xres - 1;
+	active_v_start = display_v_start;
+	active_v_end = active_v_start + (p->yres * hsync_period) - 1;
- active_h_start = hsync_start_x;
-		active_h_end = active_h_start + p->xres - 1;
-		active_v_start = display_v_start;
-		active_v_end = active_v_start + (p->yres * hsync_period) - 1;
+	active_hctl = (active_h_end << 16) | active_h_start;
- active_hctl = (active_h_end << 16) | active_h_start;
+	if (dp_intf) {
  		display_hctl = active_hctl;
- intf_cfg |= INTF_CFG_ACTIVE_H_EN | INTF_CFG_ACTIVE_V_EN;
+		if (p->compression_en) {
+			active_data_hctl = (hsync_start_x + p->extra_dto_cycles) << 16;
+			active_data_hctl += hsync_start_x;
+
+			display_data_hctl = active_data_hctl;
+		}
  	}
+ _check_and_set_comp_bit(ctx, p->dsc_4hs_merge, p->compression_en, &intf_cfg2);
+
  	den_polarity = 0;
  	if (ctx->cap->type == INTF_HDMI) {
  		hsync_polarity = p->yres >= 720 ? 0 : 1;
@@ -202,7 +227,7 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,

--
With best wishes
Dmitry




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux