Linux ARM-MSM sub-architecture
 help / color / mirror / Atom feed
* [PATCH v3 0/5] Add DSC v1.2 Support for DSI
@ 2023-05-19 21:17 Jessica Zhang
  2023-05-19 21:17 ` [PATCH v3 1/5] msm/drm/dsi: Round up DSC hdisplay calculation Jessica Zhang
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Jessica Zhang @ 2023-05-19 21:17 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	David Airlie, Daniel Vetter, Marijn Suijten
  Cc: Konrad Dybcio, linux-arm-msm, dri-devel, freedreno, linux-kernel,
	Jessica Zhang

This is a series of changes for DSI to enable command mode support
for DSC v1.2.

This includes:

1) Rounding up `hdisplay / 3` in dsc_timing_setup()
2) Adjusting pclk_rate to account for compression
3) Fixing incorrect uses of slice_count in DSI DSC calculations
4) Setting the DATA_COMPRESS bit when DSC is enabled

With these changes (and the dependency below), DSC v1.2 should work over
DSI in command mode.

Note: Changes that add DSC v1.2 support for video mode will be posted
with the DP support changes.

Depends-on: "add DSC 1.2 dpu supports" [1] and "Introduce MSM-specific
DSC helpers" [2]

[1] https://patchwork.freedesktop.org/series/116789/
[2] https://patchwork.freedesktop.org/series/115833/

Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
Changes in v3:
- Added fix to round up hdisplay DSC adjustment
- Fixed inconsistent whitespace in dpu_hw_intf_ops comment doc
- Moved placement of dpu_hw_intf_enable_compression
- Picked up "drm/msm/dsi: Fix calculation for pkt_per_line" patch and
  squashed all slice_count fixes into a single patch
- Use drm_mode_vrefresh() to calculate adjusted pclk rate
- Moved compressed pclk adjustment to dsi_adjust_compressed_pclk() helper
- Rebased changes on top of updated dependencies
- Reworded commit message for "drm/msm/dpu: Set DATA_COMPRESS for
  command mode" for clarity
- Removed revision changelog in commit messages
- Link to v2: https://lore.kernel.org/r/20230405-add-dsc-support-v2-0-1072c70e9786@quicinc.com

Changes in v2:
- Changed has_data_compress dpu_cap to a DATA_COMPRESS INTF feature flag
- Changed pclk math to only divide hdisplay by compression ratio
- Reworded word count TODO comment
- Make DATA_COMPRESS an INTF flag
- Read INTF_CONFIG2 before writing to DATA_COMPRESS bit
- Fixed whitespace issue in macro definition
- Removed `inline` from dpu_hw_intf_enable_compression declaration
- Only set dpu_hw_intf_ops.data_compress if DATA_COMPRESS feature is set
- Reworded commit messages and cover letter for clarity
- Link to v1: https://lore.kernel.org/r/20230405-add-dsc-support-v1-0-6bc6f03ae735@quicinc.com

---
Jessica Zhang (5):
      msm/drm/dsi: Round up DSC hdisplay calculation
      drm/msm/dsi: Adjust pclk rate for compression
      drm/msm/dpu: Add DPU_INTF_DATA_COMPRESS feature flag
      drm/msm/dpu: Set DATA_COMPRESS for command mode
      drm/msm/dsi: Remove incorrect references to slice_count

 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c   |  3 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c     |  2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h     |  2 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c        | 13 ++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h        |  2 +
 drivers/gpu/drm/msm/dsi/dsi_host.c                 | 49 +++++++++++++++-------
 6 files changed, 55 insertions(+), 16 deletions(-)
---
base-commit: 2f0218fa4805d7c7eed8dc072e1bdf9f100492c7
change-id: 20230405-add-dsc-support-fe130ba49841

Best regards,
-- 
Jessica Zhang <quic_jesszhan@quicinc.com>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 1/5] msm/drm/dsi: Round up DSC hdisplay calculation
  2023-05-19 21:17 [PATCH v3 0/5] Add DSC v1.2 Support for DSI Jessica Zhang
@ 2023-05-19 21:17 ` Jessica Zhang
  2023-05-19 21:33   ` Marijn Suijten
  2023-05-20  8:07   ` Marijn Suijten
  2023-05-19 21:17 ` [PATCH v3 2/5] drm/msm/dsi: Adjust pclk rate for compression Jessica Zhang
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Jessica Zhang @ 2023-05-19 21:17 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	David Airlie, Daniel Vetter, Marijn Suijten
  Cc: Konrad Dybcio, linux-arm-msm, dri-devel, freedreno, linux-kernel,
	Jessica Zhang

Currently, when compression is enabled, hdisplay is reduced via integer
division. This causes issues for modes where the original hdisplay is
not a multiple of 3.

To fix this, use DIV_ROUND_UP to divide hdisplay.

Reported-by: Marijn Suijten <marijn.suijten@somainline.org>
Fixes: f3a99460406b ("drm/msm/dsi: update hdisplay calculation for dsi_timing_setup")
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 9223d7ec5a73..18d38b90eb28 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
 		 * pulse width same
 		 */
 		h_total -= hdisplay;
-		hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3;
+		hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
 		h_total += hdisplay;
 		ha_end = ha_start + hdisplay;
 	}

-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 2/5] drm/msm/dsi: Adjust pclk rate for compression
  2023-05-19 21:17 [PATCH v3 0/5] Add DSC v1.2 Support for DSI Jessica Zhang
  2023-05-19 21:17 ` [PATCH v3 1/5] msm/drm/dsi: Round up DSC hdisplay calculation Jessica Zhang
@ 2023-05-19 21:17 ` Jessica Zhang
  2023-05-19 21:17 ` [PATCH v3 3/5] drm/msm/dpu: Add DPU_INTF_DATA_COMPRESS feature flag Jessica Zhang
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Jessica Zhang @ 2023-05-19 21:17 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	David Airlie, Daniel Vetter, Marijn Suijten
  Cc: Konrad Dybcio, linux-arm-msm, dri-devel, freedreno, linux-kernel,
	Jessica Zhang

Adjust the pclk rate to divide hdisplay by the compression ratio when DSC
is enabled.

Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 18d38b90eb28..d04f8bbd707d 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -561,7 +561,18 @@ void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host)
 	clk_disable_unprepare(msm_host->byte_clk);
 }
 
-static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool is_bonded_dsi)
+static unsigned long dsi_adjust_compressed_pclk(const struct drm_display_mode *mode,
+		const struct drm_dsc_config *dsc)
+{
+	int new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc),
+			dsc->bits_per_component * 3);
+
+	return (new_hdisplay + (mode->htotal - mode->hdisplay))
+			* mode->vtotal * drm_mode_vrefresh(mode);
+}
+
+static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode,
+		const struct drm_dsc_config *dsc, bool is_bonded_dsi)
 {
 	unsigned long pclk_rate;
 
@@ -576,6 +587,10 @@ static unsigned long dsi_get_pclk_rate(const struct drm_display_mode *mode, bool
 	if (is_bonded_dsi)
 		pclk_rate /= 2;
 
+	/* If DSC is enabled, divide hdisplay by compression ratio */
+	if (dsc)
+		pclk_rate = dsi_adjust_compressed_pclk(mode, dsc);
+
 	return pclk_rate;
 }
 
@@ -585,7 +600,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d
 	struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
 	u8 lanes = msm_host->lanes;
 	u32 bpp = dsi_get_bpp(msm_host->format);
-	unsigned long pclk_rate = dsi_get_pclk_rate(mode, is_bonded_dsi);
+	unsigned long pclk_rate = dsi_get_pclk_rate(mode, msm_host->dsc, is_bonded_dsi);
 	u64 pclk_bpp = (u64)pclk_rate * bpp;
 
 	if (lanes == 0) {
@@ -604,7 +619,7 @@ unsigned long dsi_byte_clk_get_rate(struct mipi_dsi_host *host, bool is_bonded_d
 
 static void dsi_calc_pclk(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
 {
-	msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi);
+	msm_host->pixel_clk_rate = dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi);
 	msm_host->byte_clk_rate = dsi_byte_clk_get_rate(&msm_host->base, is_bonded_dsi,
 							msm_host->mode);
 
@@ -634,7 +649,7 @@ int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
 
 	dsi_calc_pclk(msm_host, is_bonded_dsi);
 
-	pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi) * bpp;
+	pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, msm_host->dsc, is_bonded_dsi) * bpp;
 	do_div(pclk_bpp, 8);
 	msm_host->src_clk_rate = pclk_bpp;
 

-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 3/5] drm/msm/dpu: Add DPU_INTF_DATA_COMPRESS feature flag
  2023-05-19 21:17 [PATCH v3 0/5] Add DSC v1.2 Support for DSI Jessica Zhang
  2023-05-19 21:17 ` [PATCH v3 1/5] msm/drm/dsi: Round up DSC hdisplay calculation Jessica Zhang
  2023-05-19 21:17 ` [PATCH v3 2/5] drm/msm/dsi: Adjust pclk rate for compression Jessica Zhang
@ 2023-05-19 21:17 ` Jessica Zhang
  2023-05-19 21:34   ` Marijn Suijten
  2023-05-19 21:17 ` [PATCH v3 4/5] drm/msm/dpu: Set DATA_COMPRESS for command mode Jessica Zhang
  2023-05-19 21:17 ` [PATCH v3 5/5] drm/msm/dsi: Remove incorrect references to slice_count Jessica Zhang
  4 siblings, 1 reply; 15+ messages in thread
From: Jessica Zhang @ 2023-05-19 21:17 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	David Airlie, Daniel Vetter, Marijn Suijten
  Cc: Konrad Dybcio, linux-arm-msm, dri-devel, freedreno, linux-kernel,
	Jessica Zhang

Add DATA_COMPRESS feature flag to DPU INTF block.

In DPU 7.x and later, DSC/DCE enablement registers have been moved from
PINGPONG to INTF.

Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 7944481d0a33..c74051906d05 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -104,7 +104,7 @@
 #define INTF_SC7180_MASK \
 	(BIT(DPU_INTF_INPUT_CTRL) | BIT(DPU_INTF_TE) | BIT(DPU_INTF_STATUS_SUPPORTED))
 
-#define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN)
+#define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN) | BIT(DPU_INTF_DATA_COMPRESS)
 
 #define WB_SM8250_MASK (BIT(DPU_WB_LINE_MODE) | \
 			 BIT(DPU_WB_UBWC) | \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 4eda2cc847ef..01c65f940f2a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -185,6 +185,7 @@ enum {
  * @DPU_DATA_HCTL_EN                Allows data to be transferred at different rate
  *                                  than video timing
  * @DPU_INTF_STATUS_SUPPORTED       INTF block has INTF_STATUS register
+ * @DPU_INTF_DATA_COMPRESS          INTF block has DATA_COMPRESS register
  * @DPU_INTF_MAX
  */
 enum {
@@ -192,6 +193,7 @@ enum {
 	DPU_INTF_TE,
 	DPU_DATA_HCTL_EN,
 	DPU_INTF_STATUS_SUPPORTED,
+	DPU_INTF_DATA_COMPRESS,
 	DPU_INTF_MAX
 };
 

-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 4/5] drm/msm/dpu: Set DATA_COMPRESS for command mode
  2023-05-19 21:17 [PATCH v3 0/5] Add DSC v1.2 Support for DSI Jessica Zhang
                   ` (2 preceding siblings ...)
  2023-05-19 21:17 ` [PATCH v3 3/5] drm/msm/dpu: Add DPU_INTF_DATA_COMPRESS feature flag Jessica Zhang
@ 2023-05-19 21:17 ` Jessica Zhang
  2023-05-19 21:17 ` [PATCH v3 5/5] drm/msm/dsi: Remove incorrect references to slice_count Jessica Zhang
  4 siblings, 0 replies; 15+ messages in thread
From: Jessica Zhang @ 2023-05-19 21:17 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	David Airlie, Daniel Vetter, Marijn Suijten
  Cc: Konrad Dybcio, linux-arm-msm, dri-devel, freedreno, linux-kernel,
	Jessica Zhang

Add a DPU INTF op to set DATA_COMPRESS register if the
DPU_INTF_DATA_COMPRESS feature is enabled. This bit needs to be set in
order for DSC v1.2 to work.

Note: For now, this op is called for command mode encoders only. Changes to
set DATA_COMPRESS for video mode encoders will be posted along with DSC
v1.2 support for DP.

Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c |  3 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c          | 13 +++++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h          |  2 ++
 3 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index d8ed85a238af..1a4c20f02312 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -68,6 +68,9 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg(
 				phys_enc->hw_intf,
 				true,
 				phys_enc->hw_pp->idx);
+
+	if (phys_enc->hw_intf->ops.enable_compression)
+		phys_enc->hw_intf->ops.enable_compression(phys_enc->hw_intf);
 }
 
 static void dpu_encoder_phys_cmd_pp_tx_done_irq(void *arg, int irq_idx)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
index 6485500eedb8..a462c6780e6e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
@@ -91,6 +91,7 @@
 
 #define INTF_CFG2_DATABUS_WIDEN	BIT(0)
 #define INTF_CFG2_DATA_HCTL_EN	BIT(4)
+#define INTF_CFG2_DCE_DATA_COMPRESS     BIT(12)
 
 static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,
 		const struct intf_timing_params *p,
@@ -522,6 +523,15 @@ static void dpu_hw_intf_disable_autorefresh(struct dpu_hw_intf *intf,
 
 }
 
+static void dpu_hw_intf_enable_compression(struct dpu_hw_intf *ctx)
+{
+	u32 intf_cfg2 = DPU_REG_READ(&ctx->hw, INTF_CONFIG2);
+
+	intf_cfg2 |= INTF_CFG2_DCE_DATA_COMPRESS;
+
+	DPU_REG_WRITE(&ctx->hw, INTF_CONFIG2, intf_cfg2);
+}
+
 static void _setup_intf_ops(struct dpu_hw_intf_ops *ops,
 		unsigned long cap)
 {
@@ -542,6 +552,9 @@ static void _setup_intf_ops(struct dpu_hw_intf_ops *ops,
 		ops->vsync_sel = dpu_hw_intf_vsync_sel;
 		ops->disable_autorefresh = dpu_hw_intf_disable_autorefresh;
 	}
+
+	if (cap & BIT(DPU_INTF_DATA_COMPRESS))
+		ops->enable_compression = dpu_hw_intf_enable_compression;
 }
 
 struct dpu_hw_intf *dpu_hw_intf_init(const struct dpu_intf_cfg *cfg,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
index 73b0885918f8..72fe907729f1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
@@ -70,6 +70,7 @@ struct intf_status {
  * @get_autorefresh:            Retrieve autorefresh config from hardware
  *                              Return: 0 on success, -ETIMEDOUT on timeout
  * @vsync_sel:                  Select vsync signal for tear-effect configuration
+ * @enable_compression:         Enable data compression
  */
 struct dpu_hw_intf_ops {
 	void (*setup_timing_gen)(struct dpu_hw_intf *intf,
@@ -107,6 +108,7 @@ struct dpu_hw_intf_ops {
 	 * Disable autorefresh if enabled
 	 */
 	void (*disable_autorefresh)(struct dpu_hw_intf *intf, uint32_t encoder_id, u16 vdisplay);
+	void (*enable_compression)(struct dpu_hw_intf *intf);
 };
 
 struct dpu_hw_intf {

-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 5/5] drm/msm/dsi: Remove incorrect references to slice_count
  2023-05-19 21:17 [PATCH v3 0/5] Add DSC v1.2 Support for DSI Jessica Zhang
                   ` (3 preceding siblings ...)
  2023-05-19 21:17 ` [PATCH v3 4/5] drm/msm/dpu: Set DATA_COMPRESS for command mode Jessica Zhang
@ 2023-05-19 21:17 ` Jessica Zhang
  2023-05-19 21:24   ` Marijn Suijten
  2023-05-21  0:32   ` Dmitry Baryshkov
  4 siblings, 2 replies; 15+ messages in thread
From: Jessica Zhang @ 2023-05-19 21:17 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	David Airlie, Daniel Vetter, Marijn Suijten
  Cc: Konrad Dybcio, linux-arm-msm, dri-devel, freedreno, linux-kernel,
	Jessica Zhang

Currently, slice_count is being used to calculate word count and
pkt_per_line. In downstream, these values are calculated using slice per
packet, which is not the same as slice_count.

Slice count represents the number of soft slices per interface, and its
value will not always match that of slice per packet. For example, it is
possible to have cases where there are multiple soft slices per interface
but the panel specifies only one slice per packet.

Thus, use the default value of one slice per packet and remove slice_count
from the aforementioned calculations.

Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
Fixes: bc6b6ff8135c ("drm/msm/dsi: Use DSC slice(s) packet size to compute word count")
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index d04f8bbd707d..8c8858ee59ec 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -866,18 +866,15 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
 	 */
 	slice_per_intf = msm_dsc_get_slices_per_intf(dsc, hdisplay);
 
-	/*
-	 * If slice_count is greater than slice_per_intf
-	 * then default to 1. This can happen during partial
-	 * update.
-	 */
-	if (dsc->slice_count > slice_per_intf)
-		dsc->slice_count = 1;
-
 	total_bytes_per_intf = dsc->slice_chunk_size * slice_per_intf;
 
 	eol_byte_num = total_bytes_per_intf % 3;
-	pkt_per_line = slice_per_intf / dsc->slice_count;
+
+	/*
+	 * Default to 1 slice_per_pkt, so pkt_per_line will be equal to
+	 * slice per intf.
+	 */
+	pkt_per_line = slice_per_intf;
 
 	if (is_cmd_mode) /* packet data type */
 		reg = DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(MIPI_DSI_DCS_LONG_WRITE);
@@ -1001,7 +998,14 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
 		if (!msm_host->dsc)
 			wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
 		else
-			wc = msm_host->dsc->slice_chunk_size * msm_host->dsc->slice_count + 1;
+			/*
+			 * When DSC is enabled, WC = slice_chunk_size * slice_per_packet + 1.
+			 * Currently, the driver only supports default value of slice_per_packet = 1
+			 *
+			 * TODO: Expand mipi_dsi_device struct to hold slice_per_packet info
+			 *       and adjust DSC math to account for slice_per_packet.
+			 */
+			wc = msm_host->dsc->slice_chunk_size + 1;
 
 		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL,
 			DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |

-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/5] drm/msm/dsi: Remove incorrect references to slice_count
  2023-05-19 21:17 ` [PATCH v3 5/5] drm/msm/dsi: Remove incorrect references to slice_count Jessica Zhang
@ 2023-05-19 21:24   ` Marijn Suijten
  2023-05-19 23:02     ` Jessica Zhang
  2023-05-21  0:32   ` Dmitry Baryshkov
  1 sibling, 1 reply; 15+ messages in thread
From: Marijn Suijten @ 2023-05-19 21:24 UTC (permalink / raw)
  To: Jessica Zhang
  Cc: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	David Airlie, Daniel Vetter, Konrad Dybcio, linux-arm-msm,
	dri-devel, freedreno, linux-kernel

On 2023-05-19 14:17:30, Jessica Zhang wrote:
> Currently, slice_count is being used to calculate word count and
> pkt_per_line. In downstream, these values are calculated using slice per
> packet, which is not the same as slice_count.
> 
> Slice count represents the number of soft slices per interface, and its
> value will not always match that of slice per packet. For example, it is
> possible to have cases where there are multiple soft slices per interface
> but the panel specifies only one slice per packet.
> 
> Thus, use the default value of one slice per packet and remove slice_count
> from the aforementioned calculations.
> 
> Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
> Fixes: bc6b6ff8135c ("drm/msm/dsi: Use DSC slice(s) packet size to compute word count")
> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index d04f8bbd707d..8c8858ee59ec 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -866,18 +866,15 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
>  	 */
>  	slice_per_intf = msm_dsc_get_slices_per_intf(dsc, hdisplay);
>  
> -	/*
> -	 * If slice_count is greater than slice_per_intf
> -	 * then default to 1. This can happen during partial
> -	 * update.
> -	 */
> -	if (dsc->slice_count > slice_per_intf)
> -		dsc->slice_count = 1;
> -
>  	total_bytes_per_intf = dsc->slice_chunk_size * slice_per_intf;
>  
>  	eol_byte_num = total_bytes_per_intf % 3;
> -	pkt_per_line = slice_per_intf / dsc->slice_count;
> +
> +	/*
> +	 * Default to 1 slice_per_pkt, so pkt_per_line will be equal to
> +	 * slice per intf.
> +	 */
> +	pkt_per_line = slice_per_intf;

Same as the comment below, perhaps it is a good idea to clarify in the
comment here that the original value wa *multiplied by* slice_per_pkt?
Otherwise this default of 1 is "invisible".

Alternatively we could have a `const slice_per_pkt = 1;` at the top, and
have the comment (the elaborate one from below) there?

- Marijn

>  
>  	if (is_cmd_mode) /* packet data type */
>  		reg = DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(MIPI_DSI_DCS_LONG_WRITE);
> @@ -1001,7 +998,14 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
>  		if (!msm_host->dsc)
>  			wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
>  		else
> -			wc = msm_host->dsc->slice_chunk_size * msm_host->dsc->slice_count + 1;
> +			/*
> +			 * When DSC is enabled, WC = slice_chunk_size * slice_per_packet + 1.
> +			 * Currently, the driver only supports default value of slice_per_packet = 1
> +			 *
> +			 * TODO: Expand mipi_dsi_device struct to hold slice_per_packet info
> +			 *       and adjust DSC math to account for slice_per_packet.
> +			 */
> +			wc = msm_host->dsc->slice_chunk_size + 1;
>  
>  		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL,
>  			DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
> 
> -- 
> 2.40.1
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/5] msm/drm/dsi: Round up DSC hdisplay calculation
  2023-05-19 21:17 ` [PATCH v3 1/5] msm/drm/dsi: Round up DSC hdisplay calculation Jessica Zhang
@ 2023-05-19 21:33   ` Marijn Suijten
  2023-05-20  8:07   ` Marijn Suijten
  1 sibling, 0 replies; 15+ messages in thread
From: Marijn Suijten @ 2023-05-19 21:33 UTC (permalink / raw)
  To: Jessica Zhang
  Cc: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	David Airlie, Daniel Vetter, Konrad Dybcio, linux-arm-msm,
	dri-devel, freedreno, linux-kernel

On 2023-05-19 14:17:26, Jessica Zhang wrote:
> Currently, when compression is enabled, hdisplay is reduced via integer
> division. This causes issues for modes where the original hdisplay is
> not a multiple of 3.

The "issue" probably being some kind of underflow, because the stream
size is too small compared to how much data we actually send?

> To fix this, use DIV_ROUND_UP to divide hdisplay.
> 
> Reported-by: Marijn Suijten <marijn.suijten@somainline.org>
> Fixes: f3a99460406b ("drm/msm/dsi: update hdisplay calculation for dsi_timing_setup")
> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>

Yes, downstream has as very clear:

    dsc->pclk_per_line =  DIV_ROUND_UP(total_bytes_per_intf, 3);

which is used for width_final in SDE, and this is one of the mandatory
fixes on a 1096-pixels-wide panel, which is not a multiple of 3.

Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>

> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 9223d7ec5a73..18d38b90eb28 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
>  		 * pulse width same
>  		 */
>  		h_total -= hdisplay;
> -		hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3;
> +		hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
>  		h_total += hdisplay;
>  		ha_end = ha_start + hdisplay;
>  	}
> 
> -- 
> 2.40.1
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/5] drm/msm/dpu: Add DPU_INTF_DATA_COMPRESS feature flag
  2023-05-19 21:17 ` [PATCH v3 3/5] drm/msm/dpu: Add DPU_INTF_DATA_COMPRESS feature flag Jessica Zhang
@ 2023-05-19 21:34   ` Marijn Suijten
  2023-05-22 18:03     ` Jessica Zhang
  0 siblings, 1 reply; 15+ messages in thread
From: Marijn Suijten @ 2023-05-19 21:34 UTC (permalink / raw)
  To: Jessica Zhang
  Cc: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	David Airlie, Daniel Vetter, Konrad Dybcio, linux-arm-msm,
	dri-devel, freedreno, linux-kernel

On 2023-05-19 14:17:28, Jessica Zhang wrote:
> Add DATA_COMPRESS feature flag to DPU INTF block.
> 
> In DPU 7.x and later, DSC/DCE enablement registers have been moved from
> PINGPONG to INTF.
> 
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 2 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> index 7944481d0a33..c74051906d05 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -104,7 +104,7 @@
>  #define INTF_SC7180_MASK \
>  	(BIT(DPU_INTF_INPUT_CTRL) | BIT(DPU_INTF_TE) | BIT(DPU_INTF_STATUS_SUPPORTED))
>  
> -#define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN)
> +#define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN) | BIT(DPU_INTF_DATA_COMPRESS)

We should really wrap these in parenthesis at some point.

>  
>  #define WB_SM8250_MASK (BIT(DPU_WB_LINE_MODE) | \
>  			 BIT(DPU_WB_UBWC) | \
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> index 4eda2cc847ef..01c65f940f2a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -185,6 +185,7 @@ enum {
>   * @DPU_DATA_HCTL_EN                Allows data to be transferred at different rate
>   *                                  than video timing
>   * @DPU_INTF_STATUS_SUPPORTED       INTF block has INTF_STATUS register
> + * @DPU_INTF_DATA_COMPRESS          INTF block has DATA_COMPRESS register
>   * @DPU_INTF_MAX
>   */
>  enum {
> @@ -192,6 +193,7 @@ enum {
>  	DPU_INTF_TE,
>  	DPU_DATA_HCTL_EN,
>  	DPU_INTF_STATUS_SUPPORTED,
> +	DPU_INTF_DATA_COMPRESS,
>  	DPU_INTF_MAX
>  };
>  
> 
> -- 
> 2.40.1
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/5] drm/msm/dsi: Remove incorrect references to slice_count
  2023-05-19 21:24   ` Marijn Suijten
@ 2023-05-19 23:02     ` Jessica Zhang
  0 siblings, 0 replies; 15+ messages in thread
From: Jessica Zhang @ 2023-05-19 23:02 UTC (permalink / raw)
  To: Marijn Suijten
  Cc: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	David Airlie, Daniel Vetter, Konrad Dybcio, linux-arm-msm,
	dri-devel, freedreno, linux-kernel



On 5/19/2023 2:24 PM, Marijn Suijten wrote:
> On 2023-05-19 14:17:30, Jessica Zhang wrote:
>> Currently, slice_count is being used to calculate word count and
>> pkt_per_line. In downstream, these values are calculated using slice per
>> packet, which is not the same as slice_count.
>>
>> Slice count represents the number of soft slices per interface, and its
>> value will not always match that of slice per packet. For example, it is
>> possible to have cases where there are multiple soft slices per interface
>> but the panel specifies only one slice per packet.
>>
>> Thus, use the default value of one slice per packet and remove slice_count
>> from the aforementioned calculations.
>>
>> Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
>> Fixes: bc6b6ff8135c ("drm/msm/dsi: Use DSC slice(s) packet size to compute word count")
>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/dsi/dsi_host.c | 24 ++++++++++++++----------
>>   1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> index d04f8bbd707d..8c8858ee59ec 100644
>> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
>> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> @@ -866,18 +866,15 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
>>   	 */
>>   	slice_per_intf = msm_dsc_get_slices_per_intf(dsc, hdisplay);
>>   
>> -	/*
>> -	 * If slice_count is greater than slice_per_intf
>> -	 * then default to 1. This can happen during partial
>> -	 * update.
>> -	 */
>> -	if (dsc->slice_count > slice_per_intf)
>> -		dsc->slice_count = 1;
>> -
>>   	total_bytes_per_intf = dsc->slice_chunk_size * slice_per_intf;
>>   
>>   	eol_byte_num = total_bytes_per_intf % 3;
>> -	pkt_per_line = slice_per_intf / dsc->slice_count;
>> +
>> +	/*
>> +	 * Default to 1 slice_per_pkt, so pkt_per_line will be equal to
>> +	 * slice per intf.
>> +	 */
>> +	pkt_per_line = slice_per_intf;
> 
> Same as the comment below, perhaps it is a good idea to clarify in the
> comment here that the original value wa *multiplied by* slice_per_pkt?
> Otherwise this default of 1 is "invisible".

Hi Marijn,

Acked.

> 
> Alternatively we could have a `const slice_per_pkt = 1;` at the top, and
> have the comment (the elaborate one from below) there?

Since the default slice_per_pkt = 1 and there's only 2 places where I 
have to add a clarifying comment, I think having a separate 
slice_per_pkt variable is a bit redundant.

Plus, if support for multiple slice_per_pkt values is added in the 
future, we'd also have to drop this variable anyways.

Thanks,

Jessica Zhang

> 
> - Marijn
> 
>>   
>>   	if (is_cmd_mode) /* packet data type */
>>   		reg = DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(MIPI_DSI_DCS_LONG_WRITE);
>> @@ -1001,7 +998,14 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
>>   		if (!msm_host->dsc)
>>   			wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
>>   		else
>> -			wc = msm_host->dsc->slice_chunk_size * msm_host->dsc->slice_count + 1;
>> +			/*
>> +			 * When DSC is enabled, WC = slice_chunk_size * slice_per_packet + 1.
>> +			 * Currently, the driver only supports default value of slice_per_packet = 1
>> +			 *
>> +			 * TODO: Expand mipi_dsi_device struct to hold slice_per_packet info
>> +			 *       and adjust DSC math to account for slice_per_packet.
>> +			 */
>> +			wc = msm_host->dsc->slice_chunk_size + 1;
>>   
>>   		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL,
>>   			DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
>>
>> -- 
>> 2.40.1
>>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/5] msm/drm/dsi: Round up DSC hdisplay calculation
  2023-05-19 21:17 ` [PATCH v3 1/5] msm/drm/dsi: Round up DSC hdisplay calculation Jessica Zhang
  2023-05-19 21:33   ` Marijn Suijten
@ 2023-05-20  8:07   ` Marijn Suijten
  2023-05-22 18:03     ` Jessica Zhang
  1 sibling, 1 reply; 15+ messages in thread
From: Marijn Suijten @ 2023-05-20  8:07 UTC (permalink / raw)
  To: Jessica Zhang
  Cc: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	David Airlie, Daniel Vetter, Konrad Dybcio, linux-arm-msm,
	dri-devel, freedreno, linux-kernel

On 2023-05-19 14:17:26, Jessica Zhang wrote:
> Currently, when compression is enabled, hdisplay is reduced via integer
> division. This causes issues for modes where the original hdisplay is
> not a multiple of 3.
> 
> To fix this, use DIV_ROUND_UP to divide hdisplay.
> 
> Reported-by: Marijn Suijten <marijn.suijten@somainline.org>

This should have been:

Suggested-by: Marijn Suijten <marijn.suijten@somainline.org>

> Fixes: f3a99460406b ("drm/msm/dsi: update hdisplay calculation for dsi_timing_setup")

This hash is not valid (and checkpatch points it out...), as it is your
local commit from the MSM DSC helper methods series.  The original issue
was introduced in:

Fixes: 08802f515c3cf ("drm/msm/dsi: Add support for DSC configuration")

- Marijn

> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 9223d7ec5a73..18d38b90eb28 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
>  		 * pulse width same
>  		 */
>  		h_total -= hdisplay;
> -		hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3;
> +		hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
>  		h_total += hdisplay;
>  		ha_end = ha_start + hdisplay;
>  	}
> 
> -- 
> 2.40.1
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/5] drm/msm/dsi: Remove incorrect references to slice_count
  2023-05-19 21:17 ` [PATCH v3 5/5] drm/msm/dsi: Remove incorrect references to slice_count Jessica Zhang
  2023-05-19 21:24   ` Marijn Suijten
@ 2023-05-21  0:32   ` Dmitry Baryshkov
  2023-05-22 18:38     ` Jessica Zhang
  1 sibling, 1 reply; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-05-21  0:32 UTC (permalink / raw)
  To: Jessica Zhang, Rob Clark, Abhinav Kumar, Sean Paul, David Airlie,
	Daniel Vetter, Marijn Suijten
  Cc: Konrad Dybcio, linux-arm-msm, dri-devel, freedreno, linux-kernel

On 20/05/2023 00:17, Jessica Zhang wrote:
> Currently, slice_count is being used to calculate word count and
> pkt_per_line. In downstream, these values are calculated using slice per
> packet, which is not the same as slice_count.

I'd say the reference to downstream is not correct. We have seen cases 
where the vendor kernel contained errors. So it should be something like 
"Instead these values should be calculated using ...."

> 
> Slice count represents the number of soft slices per interface, and its
> value will not always match that of slice per packet. For example, it is
> possible to have cases where there are multiple soft slices per interface
> but the panel specifies only one slice per packet.
> 
> Thus, use the default value of one slice per packet and remove slice_count
> from the aforementioned calculations.
> 
> Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
> Fixes: bc6b6ff8135c ("drm/msm/dsi: Use DSC slice(s) packet size to compute word count")
> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dsi/dsi_host.c | 24 ++++++++++++++----------
>   1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index d04f8bbd707d..8c8858ee59ec 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -866,18 +866,15 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
>   	 */
>   	slice_per_intf = msm_dsc_get_slices_per_intf(dsc, hdisplay);
>   
> -	/*
> -	 * If slice_count is greater than slice_per_intf
> -	 * then default to 1. This can happen during partial
> -	 * update.
> -	 */
> -	if (dsc->slice_count > slice_per_intf)
> -		dsc->slice_count = 1;
> -
>   	total_bytes_per_intf = dsc->slice_chunk_size * slice_per_intf;
>   
>   	eol_byte_num = total_bytes_per_intf % 3;
> -	pkt_per_line = slice_per_intf / dsc->slice_count;
> +
> +	/*
> +	 * Default to 1 slice_per_pkt, so pkt_per_line will be equal to
> +	 * slice per intf.
> +	 */
> +	pkt_per_line = slice_per_intf;
>   
>   	if (is_cmd_mode) /* packet data type */
>   		reg = DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(MIPI_DSI_DCS_LONG_WRITE);
> @@ -1001,7 +998,14 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
>   		if (!msm_host->dsc)
>   			wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
>   		else
> -			wc = msm_host->dsc->slice_chunk_size * msm_host->dsc->slice_count + 1;
> +			/*
> +			 * When DSC is enabled, WC = slice_chunk_size * slice_per_packet + 1.
> +			 * Currently, the driver only supports default value of slice_per_packet = 1
> +			 *
> +			 * TODO: Expand mipi_dsi_device struct to hold slice_per_packet info
> +			 *       and adjust DSC math to account for slice_per_packet.
> +			 */
> +			wc = msm_host->dsc->slice_chunk_size + 1;
>   
>   		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL,
>   			DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
> 

-- 
With best wishes
Dmitry


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/5] drm/msm/dpu: Add DPU_INTF_DATA_COMPRESS feature flag
  2023-05-19 21:34   ` Marijn Suijten
@ 2023-05-22 18:03     ` Jessica Zhang
  0 siblings, 0 replies; 15+ messages in thread
From: Jessica Zhang @ 2023-05-22 18:03 UTC (permalink / raw)
  To: Marijn Suijten
  Cc: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	David Airlie, Daniel Vetter, Konrad Dybcio, linux-arm-msm,
	dri-devel, freedreno, linux-kernel



On 5/19/2023 2:34 PM, Marijn Suijten wrote:
> On 2023-05-19 14:17:28, Jessica Zhang wrote:
>> Add DATA_COMPRESS feature flag to DPU INTF block.
>>
>> In DPU 7.x and later, DSC/DCE enablement registers have been moved from
>> PINGPONG to INTF.
>>
>> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 2 +-
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++
>>   2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> index 7944481d0a33..c74051906d05 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> @@ -104,7 +104,7 @@
>>   #define INTF_SC7180_MASK \
>>   	(BIT(DPU_INTF_INPUT_CTRL) | BIT(DPU_INTF_TE) | BIT(DPU_INTF_STATUS_SUPPORTED))
>>   
>> -#define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN)
>> +#define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN) | BIT(DPU_INTF_DATA_COMPRESS)
> 
> We should really wrap these in parenthesis at some point.

Hi Marijn,

Acked.

Thanks,

Jessica Zhang

> 
>>   
>>   #define WB_SM8250_MASK (BIT(DPU_WB_LINE_MODE) | \
>>   			 BIT(DPU_WB_UBWC) | \
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
>> index 4eda2cc847ef..01c65f940f2a 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
>> @@ -185,6 +185,7 @@ enum {
>>    * @DPU_DATA_HCTL_EN                Allows data to be transferred at different rate
>>    *                                  than video timing
>>    * @DPU_INTF_STATUS_SUPPORTED       INTF block has INTF_STATUS register
>> + * @DPU_INTF_DATA_COMPRESS          INTF block has DATA_COMPRESS register
>>    * @DPU_INTF_MAX
>>    */
>>   enum {
>> @@ -192,6 +193,7 @@ enum {
>>   	DPU_INTF_TE,
>>   	DPU_DATA_HCTL_EN,
>>   	DPU_INTF_STATUS_SUPPORTED,
>> +	DPU_INTF_DATA_COMPRESS,
>>   	DPU_INTF_MAX
>>   };
>>   
>>
>> -- 
>> 2.40.1
>>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/5] msm/drm/dsi: Round up DSC hdisplay calculation
  2023-05-20  8:07   ` Marijn Suijten
@ 2023-05-22 18:03     ` Jessica Zhang
  0 siblings, 0 replies; 15+ messages in thread
From: Jessica Zhang @ 2023-05-22 18:03 UTC (permalink / raw)
  To: Marijn Suijten
  Cc: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	David Airlie, Daniel Vetter, Konrad Dybcio, linux-arm-msm,
	dri-devel, freedreno, linux-kernel



On 5/20/2023 1:07 AM, Marijn Suijten wrote:
> On 2023-05-19 14:17:26, Jessica Zhang wrote:
>> Currently, when compression is enabled, hdisplay is reduced via integer
>> division. This causes issues for modes where the original hdisplay is
>> not a multiple of 3.
>>
>> To fix this, use DIV_ROUND_UP to divide hdisplay.
>>
>> Reported-by: Marijn Suijten <marijn.suijten@somainline.org>
> 
> This should have been:
> 
> Suggested-by: Marijn Suijten <marijn.suijten@somainline.org>
> 
>> Fixes: f3a99460406b ("drm/msm/dsi: update hdisplay calculation for dsi_timing_setup")
> 
> This hash is not valid (and checkpatch points it out...), as it is your
> local commit from the MSM DSC helper methods series.  The original issue
> was introduced in:
> 
> Fixes: 08802f515c3cf ("drm/msm/dsi: Add support for DSC configuration")

Hi Marijn,

Acked.

Thanks,

Jessica Zhang

> 
> - Marijn
> 
>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> index 9223d7ec5a73..18d38b90eb28 100644
>> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
>> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> @@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
>>   		 * pulse width same
>>   		 */
>>   		h_total -= hdisplay;
>> -		hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3;
>> +		hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3);
>>   		h_total += hdisplay;
>>   		ha_end = ha_start + hdisplay;
>>   	}
>>
>> -- 
>> 2.40.1
>>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/5] drm/msm/dsi: Remove incorrect references to slice_count
  2023-05-21  0:32   ` Dmitry Baryshkov
@ 2023-05-22 18:38     ` Jessica Zhang
  0 siblings, 0 replies; 15+ messages in thread
From: Jessica Zhang @ 2023-05-22 18:38 UTC (permalink / raw)
  To: Dmitry Baryshkov, Rob Clark, Abhinav Kumar, Sean Paul,
	David Airlie, Daniel Vetter, Marijn Suijten
  Cc: Konrad Dybcio, linux-arm-msm, dri-devel, freedreno, linux-kernel



On 5/20/2023 5:32 PM, Dmitry Baryshkov wrote:
> On 20/05/2023 00:17, Jessica Zhang wrote:
>> Currently, slice_count is being used to calculate word count and
>> pkt_per_line. In downstream, these values are calculated using slice per
>> packet, which is not the same as slice_count.
> 
> I'd say the reference to downstream is not correct. We have seen cases 
> where the vendor kernel contained errors. So it should be something like 
> "Instead these values should be calculated using ...."

Hi Dmitry,

Acked.

Thanks,

Jessica Zhang

> 
>>
>> Slice count represents the number of soft slices per interface, and its
>> value will not always match that of slice per packet. For example, it is
>> possible to have cases where there are multiple soft slices per interface
>> but the panel specifies only one slice per packet.
>>
>> Thus, use the default value of one slice per packet and remove 
>> slice_count
>> from the aforementioned calculations.
>>
>> Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
>> Fixes: bc6b6ff8135c ("drm/msm/dsi: Use DSC slice(s) packet size to 
>> compute word count")
>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/dsi/dsi_host.c | 24 ++++++++++++++----------
>>   1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
>> b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> index d04f8bbd707d..8c8858ee59ec 100644
>> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
>> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> @@ -866,18 +866,15 @@ static void dsi_update_dsc_timing(struct 
>> msm_dsi_host *msm_host, bool is_cmd_mod
>>        */
>>       slice_per_intf = msm_dsc_get_slices_per_intf(dsc, hdisplay);
>> -    /*
>> -     * If slice_count is greater than slice_per_intf
>> -     * then default to 1. This can happen during partial
>> -     * update.
>> -     */
>> -    if (dsc->slice_count > slice_per_intf)
>> -        dsc->slice_count = 1;
>> -
>>       total_bytes_per_intf = dsc->slice_chunk_size * slice_per_intf;
>>       eol_byte_num = total_bytes_per_intf % 3;
>> -    pkt_per_line = slice_per_intf / dsc->slice_count;
>> +
>> +    /*
>> +     * Default to 1 slice_per_pkt, so pkt_per_line will be equal to
>> +     * slice per intf.
>> +     */
>> +    pkt_per_line = slice_per_intf;
>>       if (is_cmd_mode) /* packet data type */
>>           reg = 
>> DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(MIPI_DSI_DCS_LONG_WRITE);
>> @@ -1001,7 +998,14 @@ static void dsi_timing_setup(struct msm_dsi_host 
>> *msm_host, bool is_bonded_dsi)
>>           if (!msm_host->dsc)
>>               wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
>>           else
>> -            wc = msm_host->dsc->slice_chunk_size * 
>> msm_host->dsc->slice_count + 1;
>> +            /*
>> +             * When DSC is enabled, WC = slice_chunk_size * 
>> slice_per_packet + 1.
>> +             * Currently, the driver only supports default value of 
>> slice_per_packet = 1
>> +             *
>> +             * TODO: Expand mipi_dsi_device struct to hold 
>> slice_per_packet info
>> +             *       and adjust DSC math to account for 
>> slice_per_packet.
>> +             */
>> +            wc = msm_host->dsc->slice_chunk_size + 1;
>>           dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL,
>>               DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
>>
> 
> -- 
> With best wishes
> Dmitry
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-05-22 18:39 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-19 21:17 [PATCH v3 0/5] Add DSC v1.2 Support for DSI Jessica Zhang
2023-05-19 21:17 ` [PATCH v3 1/5] msm/drm/dsi: Round up DSC hdisplay calculation Jessica Zhang
2023-05-19 21:33   ` Marijn Suijten
2023-05-20  8:07   ` Marijn Suijten
2023-05-22 18:03     ` Jessica Zhang
2023-05-19 21:17 ` [PATCH v3 2/5] drm/msm/dsi: Adjust pclk rate for compression Jessica Zhang
2023-05-19 21:17 ` [PATCH v3 3/5] drm/msm/dpu: Add DPU_INTF_DATA_COMPRESS feature flag Jessica Zhang
2023-05-19 21:34   ` Marijn Suijten
2023-05-22 18:03     ` Jessica Zhang
2023-05-19 21:17 ` [PATCH v3 4/5] drm/msm/dpu: Set DATA_COMPRESS for command mode Jessica Zhang
2023-05-19 21:17 ` [PATCH v3 5/5] drm/msm/dsi: Remove incorrect references to slice_count Jessica Zhang
2023-05-19 21:24   ` Marijn Suijten
2023-05-19 23:02     ` Jessica Zhang
2023-05-21  0:32   ` Dmitry Baryshkov
2023-05-22 18:38     ` Jessica Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox