linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] drm/msm: Add Display Stream Compression Support
@ 2021-07-15  6:51 Vinod Koul
  2021-07-15  6:51 ` [PATCH 01/11] drm/msm/dsi: add support for dsc data Vinod Koul
                   ` (10 more replies)
  0 siblings, 11 replies; 41+ messages in thread
From: Vinod Koul @ 2021-07-15  6:51 UTC (permalink / raw)
  To: Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, David Airlie,
	Daniel Vetter, Jonathan Marek, Dmitry Baryshkov, Abhinav Kumar,
	Jeffrey Hugo, Sumit Semwal, linux-kernel, dri-devel, freedreno

Display Stream Compression (DSC) compresses the display stream in host which
is later decoded by panel. This series enables this for Qualcomm msm driver.
This was tested on Google Pixel3 phone which use LGE SW43408 panel.
 
The changes include adding DT properties for DSC then hardware blocks
support required in DPU1 driver and support in encoder. We also add support
in DSI and introduce required topology changes.
 
In order for panel to set the DSC parameters we add dsc in drm_panel and set
it from the msm driver.
 
Complete changes which enable this for Pixel3 along with panel driver (not
part of this series) and DT changes can be found at:
git.linaro.org/people/vinod.koul/kernel.git pixel/dsc_v1
 
Comments welcome!

Changes since RFC:
 - Drop the DT binding patch as we derive the configuration from panel
 - Drop the drm api patch as we no longer need it (use pps drm api)
 - Fix comments raised by Dimitry
 - Add dsc parameters calculation from downstream

Vinod Koul (11):
  drm/msm/dsi: add support for dsc data
  drm/msm/disp/dpu1: Add support for DSC
  drm/msm/disp/dpu1: Add support for DSC in pingpong block
  drm/msm/disp/dpu1: Add DSC support in RM
  drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog
  drm/msm/disp/dpu1: Add DSC support in hw_ctl
  drm/msm/disp/dpu1: Don't use DSC with mode_3d
  drm/msm/disp/dpu1: Add support for DSC in encoder
  drm/msm/disp/dpu1: Add support for DSC in topology
  drm/msm/dsi: Add support for DSC configuration
  drm/msm/dsi: Pass DSC params to drm_panel

 drivers/gpu/drm/msm/Makefile                  |   1 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 158 +++++++++-
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  |  11 +
 .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  |   2 +
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    |  22 ++
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |  13 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c    |  12 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h    |   2 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c    | 221 ++++++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h    |  77 +++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h   |  13 +
 .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c   |  32 ++
 .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h   |  14 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h       |   1 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c        |  32 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h        |   1 +
 drivers/gpu/drm/msm/dsi/dsi.xml.h             |  10 +
 drivers/gpu/drm/msm/dsi/dsi_host.c            | 289 +++++++++++++++++-
 drivers/gpu/drm/msm/msm_drv.h                 |  21 ++
 include/drm/drm_panel.h                       |   7 +
 20 files changed, 925 insertions(+), 14 deletions(-)
 create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
 create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h

-- 
2.31.1


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

* [PATCH 01/11] drm/msm/dsi: add support for dsc data
  2021-07-15  6:51 [PATCH 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
@ 2021-07-15  6:51 ` Vinod Koul
  2021-08-02 22:55   ` [Freedreno] " abhinavk
  2021-07-15  6:51 ` [PATCH 02/11] drm/msm/disp/dpu1: Add support for DSC Vinod Koul
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 41+ messages in thread
From: Vinod Koul @ 2021-07-15  6:51 UTC (permalink / raw)
  To: Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, David Airlie,
	Daniel Vetter, Jonathan Marek, Dmitry Baryshkov, Abhinav Kumar,
	Jeffrey Hugo, Sumit Semwal, linux-kernel, dri-devel, freedreno

Display Stream Compression (DSC) parameters need to be calculated. Add
helpers and struct msm_display_dsc_config in msm_drv for this
msm_display_dsc_config uses drm_dsc_config for DSC parameters.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
Changes since RFC:
 - Drop the DT parsing code
 - Port dsc param calculation from downstream

 drivers/gpu/drm/msm/dsi/dsi_host.c | 133 +++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/msm_drv.h      |  21 +++++
 2 files changed, 154 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 8a10e4343281..e1e5d91809b5 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -30,6 +30,8 @@
 
 #define DSI_RESET_TOGGLE_DELAY_MS 20
 
+static int dsi_populate_dsc_params(struct msm_display_dsc_config *dsc);
+
 static int dsi_get_version(const void __iomem *base, u32 *major, u32 *minor)
 {
 	u32 ver;
@@ -156,6 +158,7 @@ struct msm_dsi_host {
 	struct regmap *sfpb;
 
 	struct drm_display_mode *mode;
+	struct msm_display_dsc_config *dsc;
 
 	/* connected device info */
 	struct device_node *device_node;
@@ -1744,6 +1747,136 @@ static int dsi_host_parse_lane_data(struct msm_dsi_host *msm_host,
 	return -EINVAL;
 }
 
+static u32 dsi_dsc_rc_buf_thresh[DSC_NUM_BUF_RANGES - 1] = {
+	0x0e, 0x1c, 0x2a, 0x38, 0x46, 0x54, 0x62,
+	0x69, 0x70, 0x77, 0x79, 0x7b, 0x7d, 0x7e
+};
+
+/* only 8bpc, 8bpp added */
+static char min_qp[DSC_NUM_BUF_RANGES] = {
+	0, 0, 1, 1, 3, 3, 3, 3, 3, 3, 5, 5, 5, 7, 13
+};
+
+static char max_qp[DSC_NUM_BUF_RANGES] = {
+	4, 4, 5, 6, 7, 7, 7, 8, 9, 10, 11, 12, 13, 13, 15
+};
+
+static char bpg_offset[DSC_NUM_BUF_RANGES] = {
+	2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12
+};
+
+static int dsi_populate_dsc_params(struct msm_display_dsc_config *dsc)
+{
+	int mux_words_size;
+	int groups_per_line, groups_total;
+	int min_rate_buffer_size;
+	int hrd_delay;
+	int pre_num_extra_mux_bits, num_extra_mux_bits;
+	int slice_bits;
+	int target_bpp_x16;
+	int data;
+	int final_value, final_scale;
+	int i;
+
+	dsc->drm->rc_model_size = 8192;
+	dsc->drm->first_line_bpg_offset = 12;
+	dsc->drm->rc_edge_factor = 6;
+	dsc->drm->rc_tgt_offset_high = 3;
+	dsc->drm->rc_tgt_offset_low = 3;
+	dsc->drm->simple_422 = 0;
+	dsc->drm->convert_rgb = 1;
+	dsc->drm->vbr_enable = 0;
+
+	/* handle only bpp = bpc = 8 */
+	for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++)
+		dsc->drm->rc_buf_thresh[i] = dsi_dsc_rc_buf_thresh[i];
+
+	for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
+		dsc->drm->rc_range_params[i].range_min_qp = min_qp[i];
+		dsc->drm->rc_range_params[i].range_max_qp = max_qp[i];
+		dsc->drm->rc_range_params[i].range_bpg_offset = bpg_offset[i];
+	}
+
+	dsc->drm->initial_offset = 6144; /* Not bpp 12 */
+	if (dsc->drm->bits_per_pixel != 8)
+		dsc->drm->initial_offset = 2048;	/* bpp = 12 */
+
+	mux_words_size = 48;		/* bpc == 8/10 */
+	if (dsc->drm->bits_per_component == 12)
+		mux_words_size = 64;
+
+	dsc->drm->initial_xmit_delay = 512;
+	dsc->drm->initial_scale_value = 32;
+	dsc->drm->first_line_bpg_offset = 12;
+	dsc->drm->line_buf_depth = dsc->drm->bits_per_component + 1;
+
+	/* bpc 8 */
+	dsc->drm->flatness_min_qp = 3;
+	dsc->drm->flatness_max_qp = 12;
+	dsc->det_thresh_flatness = 7 + 2 * (dsc->drm->bits_per_component - 8);
+	dsc->drm->rc_quant_incr_limit0 = 11;
+	dsc->drm->rc_quant_incr_limit1 = 11;
+	dsc->drm->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC;
+
+	/* FIXME: need to call drm_dsc_compute_rc_parameters() so that rest of
+	 * params are calculated
+	 */
+	dsc->slice_last_group_size = 3 - (dsc->drm->slice_width % 3);
+	groups_per_line = DIV_ROUND_UP(dsc->drm->slice_width, 3);
+	dsc->drm->slice_chunk_size = dsc->drm->slice_width * dsc->drm->bits_per_pixel / 8;
+	if ((dsc->drm->slice_width * dsc->drm->bits_per_pixel) % 8)
+		dsc->drm->slice_chunk_size++;
+
+	/* rbs-min */
+	min_rate_buffer_size =  dsc->drm->rc_model_size - dsc->drm->initial_offset +
+				dsc->drm->initial_xmit_delay * dsc->drm->bits_per_pixel +
+				groups_per_line * dsc->drm->first_line_bpg_offset;
+
+	hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, dsc->drm->bits_per_pixel);
+
+	dsc->drm->initial_dec_delay = hrd_delay - dsc->drm->initial_xmit_delay;
+
+	dsc->drm->initial_scale_value = 8 * dsc->drm->rc_model_size /
+				       (dsc->drm->rc_model_size - dsc->drm->initial_offset);
+
+	slice_bits = 8 * dsc->drm->slice_chunk_size * dsc->drm->slice_height;
+
+	groups_total = groups_per_line * dsc->drm->slice_height;
+
+	data = dsc->drm->first_line_bpg_offset * 2048;
+
+	dsc->drm->nfl_bpg_offset = DIV_ROUND_UP(data, (dsc->drm->slice_height - 1));
+
+	pre_num_extra_mux_bits = 3 * (mux_words_size + (4 * dsc->drm->bits_per_component + 4) - 2);
+
+	num_extra_mux_bits = pre_num_extra_mux_bits - (mux_words_size -
+			     ((slice_bits - pre_num_extra_mux_bits) % mux_words_size));
+
+	data = 2048 * (dsc->drm->rc_model_size - dsc->drm->initial_offset + num_extra_mux_bits);
+	dsc->drm->slice_bpg_offset = DIV_ROUND_UP(data, groups_total);
+
+	/* bpp * 16 + 0.5 */
+	data = dsc->drm->bits_per_pixel * 16;
+	data *= 2;
+	data++;
+	data /= 2;
+	target_bpp_x16 = data;
+
+	data = (dsc->drm->initial_xmit_delay * target_bpp_x16) / 16;
+	final_value =  dsc->drm->rc_model_size - data + num_extra_mux_bits;
+	dsc->drm->final_offset = final_value;
+
+	final_scale = 8 * dsc->drm->rc_model_size / (dsc->drm->rc_model_size - final_value);
+
+
+	data = (final_scale - 9) * (dsc->drm->nfl_bpg_offset + dsc->drm->slice_bpg_offset);
+	dsc->drm->scale_increment_interval = (2048 * dsc->drm->final_offset) / data;
+
+	dsc->drm->scale_decrement_interval = groups_per_line / (dsc->drm->initial_scale_value - 8);
+
+	return 0;
+}
+
 static int dsi_host_parse_dt(struct msm_dsi_host *msm_host)
 {
 	struct device *dev = &msm_host->pdev->dev;
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 2668941df529..65bff0176b66 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -30,6 +30,7 @@
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_fb_helper.h>
+#include <drm/drm_dsc.h>
 #include <drm/msm_drm.h>
 #include <drm/drm_gem.h>
 
@@ -134,6 +135,23 @@ struct msm_drm_thread {
 	struct kthread_worker *worker;
 };
 
+/* DSC config */
+struct msm_display_dsc_config {
+	struct drm_dsc_config *drm;
+	u8 scr_rev;
+
+	u32 initial_lines;
+	u32 pkt_per_line;
+	u32 bytes_in_slice;
+	u32 bytes_per_pkt;
+	u32 eol_byte_num;
+	u32 pclk_per_line;
+	u32 slice_last_group_size;
+	u32 det_thresh_flatness;
+	u32 extra_width;
+	u32 pps_delay_ms;
+};
+
 struct msm_drm_private {
 
 	struct drm_device *dev;
@@ -227,6 +245,9 @@ struct msm_drm_private {
 	/* Properties */
 	struct drm_property *plane_property[PLANE_PROP_MAX_NUM];
 
+	/* DSC configuration */
+	struct msm_display_dsc_config *dsc;
+
 	/* VRAM carveout, used when no IOMMU: */
 	struct {
 		unsigned long size;
-- 
2.31.1


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

* [PATCH 02/11] drm/msm/disp/dpu1: Add support for DSC
  2021-07-15  6:51 [PATCH 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
  2021-07-15  6:51 ` [PATCH 01/11] drm/msm/dsi: add support for dsc data Vinod Koul
@ 2021-07-15  6:51 ` Vinod Koul
  2021-07-19  8:28   ` kernel test robot
  2021-08-02 23:03   ` [Freedreno] " abhinavk
  2021-07-15  6:51 ` [PATCH 03/11] drm/msm/disp/dpu1: Add support for DSC in pingpong block Vinod Koul
                   ` (8 subsequent siblings)
  10 siblings, 2 replies; 41+ messages in thread
From: Vinod Koul @ 2021-07-15  6:51 UTC (permalink / raw)
  To: Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, David Airlie,
	Daniel Vetter, Jonathan Marek, Dmitry Baryshkov, Abhinav Kumar,
	Jeffrey Hugo, Sumit Semwal, linux-kernel, dri-devel, freedreno

Display Stream Compression (DSC) is one of the hw blocks in dpu, so add
support by adding hw blocks for DSC

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
Changes since RFC:
 - Drop unused enums

 drivers/gpu/drm/msm/Makefile                  |   1 +
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |  13 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c    | 221 ++++++++++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h    |  77 ++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h   |  13 ++
 5 files changed, 325 insertions(+)
 create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
 create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 610d630326bb..fd8fc57f1f58 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -61,6 +61,7 @@ msm-y := \
 	disp/dpu1/dpu_hw_blk.o \
 	disp/dpu1/dpu_hw_catalog.o \
 	disp/dpu1/dpu_hw_ctl.o \
+	disp/dpu1/dpu_hw_dsc.o \
 	disp/dpu1/dpu_hw_interrupts.o \
 	disp/dpu1/dpu_hw_intf.o \
 	disp/dpu1/dpu_hw_lm.o \
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 4dfd8a20ad5c..b8b4dc36880c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -547,6 +547,16 @@ struct dpu_merge_3d_cfg  {
 	const struct dpu_merge_3d_sub_blks *sblk;
 };
 
+/**
+ * struct dpu_dsc_cfg - information of DSC blocks
+ * @id                 enum identifying this block
+ * @base               register offset of this block
+ * @features           bit mask identifying sub-blocks/features
+ */
+struct dpu_dsc_cfg {
+	DPU_HW_BLK_INFO;
+};
+
 /**
  * struct dpu_intf_cfg - information of timing engine blocks
  * @id                 enum identifying this block
@@ -748,6 +758,9 @@ struct dpu_mdss_cfg {
 	u32 merge_3d_count;
 	const struct dpu_merge_3d_cfg *merge_3d;
 
+	u32 dsc_count;
+	struct dpu_dsc_cfg *dsc;
+
 	u32 intf_count;
 	const struct dpu_intf_cfg *intf;
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
new file mode 100644
index 000000000000..e27e67bd42e8
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
@@ -0,0 +1,221 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#include "dpu_kms.h"
+#include "dpu_hw_catalog.h"
+#include "dpu_hwio.h"
+#include "dpu_hw_mdss.h"
+#include "dpu_hw_dsc.h"
+
+#define DSC_COMMON_MODE	                0x000
+#define DSC_ENC                         0X004
+#define DSC_PICTURE                     0x008
+#define DSC_SLICE                       0x00C
+#define DSC_CHUNK_SIZE                  0x010
+#define DSC_DELAY                       0x014
+#define DSC_SCALE_INITIAL               0x018
+#define DSC_SCALE_DEC_INTERVAL          0x01C
+#define DSC_SCALE_INC_INTERVAL          0x020
+#define DSC_FIRST_LINE_BPG_OFFSET       0x024
+#define DSC_BPG_OFFSET                  0x028
+#define DSC_DSC_OFFSET                  0x02C
+#define DSC_FLATNESS                    0x030
+#define DSC_RC_MODEL_SIZE               0x034
+#define DSC_RC                          0x038
+#define DSC_RC_BUF_THRESH               0x03C
+#define DSC_RANGE_MIN_QP                0x074
+#define DSC_RANGE_MAX_QP                0x0B0
+#define DSC_RANGE_BPG_OFFSET            0x0EC
+
+static void dpu_hw_dsc_disable(struct dpu_hw_dsc *dsc)
+{
+	struct dpu_hw_blk_reg_map *c = &dsc->hw;
+
+	DPU_REG_WRITE(c, DSC_COMMON_MODE, 0);
+}
+
+static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc,
+			      struct msm_display_dsc_config *dsc, u32 mode)
+{
+	struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
+	u32 data, lsb, bpp;
+	u32 initial_lines = dsc->initial_lines;
+	bool is_cmd_mode = !(mode & BIT(2));
+
+	DPU_REG_WRITE(c, DSC_COMMON_MODE, mode);
+
+	if (is_cmd_mode)
+		initial_lines += 1;
+
+	data = (initial_lines << 20);
+	data |= ((dsc->slice_last_group_size - 1) << 18);
+	/* bpp is 6.4 format, 4 LSBs bits are for fractional part */
+	data |= dsc->drm->bits_per_pixel << 12;
+	lsb = dsc->drm->bits_per_pixel % 4;
+	bpp = dsc->drm->bits_per_pixel / 4;
+	bpp *= 4;
+	bpp <<= 4;
+	bpp |= lsb;
+
+	data |= bpp << 8;
+	data |= (dsc->drm->block_pred_enable << 7);
+	data |= (dsc->drm->line_buf_depth << 3);
+	data |= (dsc->drm->simple_422 << 2);
+	data |= (dsc->drm->convert_rgb << 1);
+	data |= dsc->drm->bits_per_component;
+
+	DPU_REG_WRITE(c, DSC_ENC, data);
+
+	data = dsc->drm->pic_width << 16;
+	data |= dsc->drm->pic_height;
+	DPU_REG_WRITE(c, DSC_PICTURE, data);
+
+	data = dsc->drm->slice_width << 16;
+	data |= dsc->drm->slice_height;
+	DPU_REG_WRITE(c, DSC_SLICE, data);
+
+	data = dsc->drm->slice_chunk_size << 16;
+	DPU_REG_WRITE(c, DSC_CHUNK_SIZE, data);
+
+	data = dsc->drm->initial_dec_delay << 16;
+	data |= dsc->drm->initial_xmit_delay;
+	DPU_REG_WRITE(c, DSC_DELAY, data);
+
+	data = dsc->drm->initial_scale_value;
+	DPU_REG_WRITE(c, DSC_SCALE_INITIAL, data);
+
+	data = dsc->drm->scale_decrement_interval;
+	DPU_REG_WRITE(c, DSC_SCALE_DEC_INTERVAL, data);
+
+	data = dsc->drm->scale_increment_interval;
+	DPU_REG_WRITE(c, DSC_SCALE_INC_INTERVAL, data);
+
+	data = dsc->drm->first_line_bpg_offset;
+	DPU_REG_WRITE(c, DSC_FIRST_LINE_BPG_OFFSET, data);
+
+	data = dsc->drm->nfl_bpg_offset << 16;
+	data |= dsc->drm->slice_bpg_offset;
+	DPU_REG_WRITE(c, DSC_BPG_OFFSET, data);
+
+	data = dsc->drm->initial_offset << 16;
+	data |= dsc->drm->final_offset;
+	DPU_REG_WRITE(c, DSC_DSC_OFFSET, data);
+
+	data = dsc->det_thresh_flatness << 10;
+	data |= dsc->drm->flatness_max_qp << 5;
+	data |= dsc->drm->flatness_min_qp;
+	DPU_REG_WRITE(c, DSC_FLATNESS, data);
+
+	data = dsc->drm->rc_model_size;
+	DPU_REG_WRITE(c, DSC_RC_MODEL_SIZE, data);
+
+	data = dsc->drm->rc_tgt_offset_low << 18;
+	data |= dsc->drm->rc_tgt_offset_high << 14;
+	data |= dsc->drm->rc_quant_incr_limit1 << 9;
+	data |= dsc->drm->rc_quant_incr_limit0 << 4;
+	data |= dsc->drm->rc_edge_factor;
+	DPU_REG_WRITE(c, DSC_RC, data);
+}
+
+static void dpu_hw_dsc_config_thresh(struct dpu_hw_dsc *hw_dsc,
+				     struct msm_display_dsc_config *dsc)
+{
+	struct drm_dsc_rc_range_parameters *rc = dsc->drm->rc_range_params;
+	struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
+	u32 off;
+	u16 *lp;
+	int i;
+
+	lp = dsc->drm->rc_buf_thresh;
+	off = DSC_RC_BUF_THRESH;
+	for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++) {
+		DPU_REG_WRITE(c, off, dsc->drm->rc_buf_thresh[i]);
+		off += 4;
+	}
+
+	off = DSC_RANGE_MIN_QP;
+	for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
+		DPU_REG_WRITE(c, off, rc[i].range_min_qp);
+		off += 4;
+	}
+
+	off = DSC_RANGE_MAX_QP;
+	for (i = 0; i < 15; i++) {
+		DPU_REG_WRITE(c, off, rc[i].range_max_qp);
+		off += 4;
+	}
+
+	off = DSC_RANGE_BPG_OFFSET;
+	for (i = 0; i < 15; i++) {
+		DPU_REG_WRITE(c, off, rc[i].range_bpg_offset);
+		off += 4;
+	}
+}
+
+static struct dpu_dsc_cfg *_dsc_offset(enum dpu_dsc dsc,
+				       struct dpu_mdss_cfg *m,
+				       void __iomem *addr,
+				       struct dpu_hw_blk_reg_map *b)
+{
+	int i;
+
+	for (i = 0; i < m->dsc_count; i++) {
+		if (dsc == m->dsc[i].id) {
+			b->base_off = addr;
+			b->blk_off = m->dsc[i].base;
+			b->length = m->dsc[i].len;
+			b->hwversion = m->hwversion;
+			b->log_mask = DPU_DBG_MASK_DSC;
+			return &m->dsc[i];
+		}
+	}
+
+	return NULL;
+}
+
+static void _setup_dsc_ops(struct dpu_hw_dsc_ops *ops,
+			   unsigned long cap)
+{
+	ops->dsc_disable = dpu_hw_dsc_disable;
+	ops->dsc_config = dpu_hw_dsc_config;
+	ops->dsc_config_thresh = dpu_hw_dsc_config_thresh;
+};
+
+static struct dpu_hw_blk_ops dpu_hw_ops = {
+	.start = NULL,
+	.stop = NULL,
+};
+
+struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc idx, void __iomem *addr,
+				   struct dpu_mdss_cfg *m)
+{
+	struct dpu_hw_dsc *c;
+	struct dpu_dsc_cfg *cfg;
+
+	c = kzalloc(sizeof(*c), GFP_KERNEL);
+	if (!c)
+		return ERR_PTR(-ENOMEM);
+
+	cfg = _dsc_offset(idx, m, addr, &c->hw);
+	if (IS_ERR_OR_NULL(cfg)) {
+		kfree(c);
+		return ERR_PTR(-EINVAL);
+	}
+
+	c->idx = idx;
+	c->caps = cfg;
+	_setup_dsc_ops(&c->ops, c->caps->features);
+
+	dpu_hw_blk_init(&c->base, DPU_HW_BLK_DSC, idx, &dpu_hw_ops);
+
+	return c;
+}
+
+void dpu_hw_dsc_destroy(struct dpu_hw_dsc *dsc)
+{
+	if (dsc)
+		dpu_hw_blk_destroy(&dsc->base);
+	kfree(dsc);
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
new file mode 100644
index 000000000000..0fb9ffe9f23f
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
@@ -0,0 +1,77 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright (c) 2020, Linaro Limited */
+
+#ifndef _DPU_HW_DSC_H
+#define _DPU_HW_DSC_H
+
+#include <drm/drm_dsc.h>
+
+#define DSC_MODE_SPLIT_PANEL            BIT(0)
+#define DSC_MODE_MULTIPLEX              BIT(1)
+#define DSC_MODE_VIDEO                  BIT(2)
+
+struct dpu_hw_dsc;
+
+/**
+ * struct dpu_hw_dsc_ops - interface to the dsc hardware driver functions
+ * Assumption is these functions will be called after clocks are enabled
+ */
+struct dpu_hw_dsc_ops {
+	/**
+	 * dsc_disable - disable dsc
+	 * @hw_dsc: Pointer to dsc context
+	 */
+	void (*dsc_disable)(struct dpu_hw_dsc *hw_dsc);
+
+	/**
+	 * dsc_config - configures dsc encoder
+	 * @hw_dsc: Pointer to dsc context
+	 * @dsc: panel dsc parameters
+	 * @mode: dsc topology mode to be set
+	 */
+	void (*dsc_config)(struct dpu_hw_dsc *hw_dsc,
+			   struct msm_display_dsc_config *dsc, u32 mode);
+
+	/**
+	 * dsc_config_thresh - programs panel thresholds
+	 * @hw_dsc: Pointer to dsc context
+	 * @dsc: panel dsc parameters
+	 */
+	void (*dsc_config_thresh)(struct dpu_hw_dsc *hw_dsc,
+				  struct msm_display_dsc_config *dsc);
+};
+
+struct dpu_hw_dsc {
+	struct dpu_hw_blk base;
+	struct dpu_hw_blk_reg_map hw;
+
+	/* dsc */
+	enum dpu_dsc idx;
+	const struct dpu_dsc_cfg *caps;
+
+	/* ops */
+	struct dpu_hw_dsc_ops ops;
+};
+
+/**
+ * dpu_hw_dsc_init - initializes the dsc block for the passed dsc idx.
+ * @idx:  DSC index for which driver object is required
+ * @addr: Mapped register io address of MDP
+ * @m:    Pointer to mdss catalog data
+ * Returns: Error code or allocated dpu_hw_dsc context
+ */
+struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc idx, void __iomem *addr,
+				   struct dpu_mdss_cfg *m);
+
+/**
+ * dpu_hw_dsc_destroy - destroys dsc driver context
+ * @dsc:   Pointer to dsc driver context returned by dpu_hw_dsc_init
+ */
+void dpu_hw_dsc_destroy(struct dpu_hw_dsc *dsc);
+
+static inline struct dpu_hw_dsc *to_dpu_hw_dsc(struct dpu_hw_blk *hw)
+{
+	return container_of(hw, struct dpu_hw_dsc, base);
+}
+
+#endif /* _DPU_HW_DSC_H */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index 09a3fb3e89f5..1b72c11090ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -97,6 +97,7 @@ enum dpu_hw_blk_type {
 	DPU_HW_BLK_WB,
 	DPU_HW_BLK_DSPP,
 	DPU_HW_BLK_MERGE_3D,
+	DPU_HW_BLK_DSC,
 	DPU_HW_BLK_MAX,
 };
 
@@ -176,6 +177,17 @@ enum dpu_ctl {
 	CTL_MAX
 };
 
+enum dpu_dsc {
+	DSC_NONE = 0,
+	DSC_0,
+	DSC_1,
+	DSC_2,
+	DSC_3,
+	DSC_4,
+	DSC_5,
+	DSC_MAX
+};
+
 enum dpu_pingpong {
 	PINGPONG_0 = 1,
 	PINGPONG_1,
@@ -437,5 +449,6 @@ struct dpu_mdss_color {
 #define DPU_DBG_MASK_VBIF     (1 << 8)
 #define DPU_DBG_MASK_ROT      (1 << 9)
 #define DPU_DBG_MASK_DSPP     (1 << 10)
+#define DPU_DBG_MASK_DSC      (1 << 11)
 
 #endif  /* _DPU_HW_MDSS_H */
-- 
2.31.1


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

* [PATCH 03/11] drm/msm/disp/dpu1: Add support for DSC in pingpong block
  2021-07-15  6:51 [PATCH 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
  2021-07-15  6:51 ` [PATCH 01/11] drm/msm/dsi: add support for dsc data Vinod Koul
  2021-07-15  6:51 ` [PATCH 02/11] drm/msm/disp/dpu1: Add support for DSC Vinod Koul
@ 2021-07-15  6:51 ` Vinod Koul
  2021-08-02 23:07   ` [Freedreno] " abhinavk
  2021-07-15  6:51 ` [PATCH 04/11] drm/msm/disp/dpu1: Add DSC support in RM Vinod Koul
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 41+ messages in thread
From: Vinod Koul @ 2021-07-15  6:51 UTC (permalink / raw)
  To: Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, David Airlie,
	Daniel Vetter, Jonathan Marek, Dmitry Baryshkov, Abhinav Kumar,
	Jeffrey Hugo, Sumit Semwal, linux-kernel, dri-devel, freedreno

In SDM845, DSC can be enabled by writing to pingpong block registers, so
add support for DSC in hw_pp

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c   | 32 +++++++++++++++++++
 .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h   | 14 ++++++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
index 245a7a62b5c6..07fc131ca9aa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
@@ -28,6 +28,9 @@
 #define PP_FBC_MODE                     0x034
 #define PP_FBC_BUDGET_CTL               0x038
 #define PP_FBC_LOSSY_MODE               0x03C
+#define PP_DSC_MODE                     0x0a0
+#define PP_DCE_DATA_IN_SWAP             0x0ac
+#define PP_DCE_DATA_OUT_SWAP            0x0c8
 
 #define PP_DITHER_EN			0x000
 #define PP_DITHER_BITDEPTH		0x004
@@ -245,6 +248,32 @@ static u32 dpu_hw_pp_get_line_count(struct dpu_hw_pingpong *pp)
 	return line;
 }
 
+static int dpu_hw_pp_dsc_enable(struct dpu_hw_pingpong *pp)
+{
+	struct dpu_hw_blk_reg_map *c = &pp->hw;
+
+	DPU_REG_WRITE(c, PP_DSC_MODE, 1);
+	return 0;
+}
+
+static void dpu_hw_pp_dsc_disable(struct dpu_hw_pingpong *pp)
+{
+	struct dpu_hw_blk_reg_map *c = &pp->hw;
+
+	DPU_REG_WRITE(c, PP_DSC_MODE, 0);
+}
+
+static int dpu_hw_pp_setup_dsc(struct dpu_hw_pingpong *pp)
+{
+	struct dpu_hw_blk_reg_map *pp_c = &pp->hw;
+	int data;
+
+	data = DPU_REG_READ(pp_c, PP_DCE_DATA_OUT_SWAP);
+	data |= BIT(18); /* endian flip */
+	DPU_REG_WRITE(pp_c, PP_DCE_DATA_OUT_SWAP, data);
+	return 0;
+}
+
 static void _setup_pingpong_ops(struct dpu_hw_pingpong *c,
 				unsigned long features)
 {
@@ -256,6 +285,9 @@ static void _setup_pingpong_ops(struct dpu_hw_pingpong *c,
 	c->ops.get_autorefresh = dpu_hw_pp_get_autorefresh_config;
 	c->ops.poll_timeout_wr_ptr = dpu_hw_pp_poll_timeout_wr_ptr;
 	c->ops.get_line_count = dpu_hw_pp_get_line_count;
+	c->ops.setup_dsc = dpu_hw_pp_setup_dsc;
+	c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
+	c->ops.disable_dsc = dpu_hw_pp_dsc_disable;
 
 	if (test_bit(DPU_PINGPONG_DITHER, &features))
 		c->ops.setup_dither = dpu_hw_pp_setup_dither;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
index 845b9ce80e31..5058e41ffbc0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
@@ -124,6 +124,20 @@ struct dpu_hw_pingpong_ops {
 	 */
 	void (*setup_dither)(struct dpu_hw_pingpong *pp,
 			struct dpu_hw_dither_cfg *cfg);
+	/**
+	 * Enable DSC
+	 */
+	int (*enable_dsc)(struct dpu_hw_pingpong *pp);
+
+	/**
+	 * Disable DSC
+	 */
+	void (*disable_dsc)(struct dpu_hw_pingpong *pp);
+
+	/**
+	 * Setup DSC
+	 */
+	int (*setup_dsc)(struct dpu_hw_pingpong *pp);
 };
 
 struct dpu_hw_pingpong {
-- 
2.31.1


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

* [PATCH 04/11] drm/msm/disp/dpu1: Add DSC support in RM
  2021-07-15  6:51 [PATCH 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
                   ` (2 preceding siblings ...)
  2021-07-15  6:51 ` [PATCH 03/11] drm/msm/disp/dpu1: Add support for DSC in pingpong block Vinod Koul
@ 2021-07-15  6:51 ` Vinod Koul
  2021-07-29 20:23   ` Dmitry Baryshkov
  2021-08-02 23:24   ` [Freedreno] " abhinavk
  2021-07-15  6:51 ` [PATCH 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog Vinod Koul
                   ` (6 subsequent siblings)
  10 siblings, 2 replies; 41+ messages in thread
From: Vinod Koul @ 2021-07-15  6:51 UTC (permalink / raw)
  To: Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, David Airlie,
	Daniel Vetter, Jonathan Marek, Dmitry Baryshkov, Abhinav Kumar,
	Jeffrey Hugo, Sumit Semwal, linux-kernel, dri-devel, freedreno

This add the bits in RM to enable the DSC blocks

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  1 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c  | 32 +++++++++++++++++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h  |  1 +
 3 files changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index d6717d6672f7..d56c05146dfe 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -165,6 +165,7 @@ struct dpu_global_state {
 	uint32_t ctl_to_enc_id[CTL_MAX - CTL_0];
 	uint32_t intf_to_enc_id[INTF_MAX - INTF_0];
 	uint32_t dspp_to_enc_id[DSPP_MAX - DSPP_0];
+	uint32_t dsc_to_enc_id[DSC_MAX - DSC_0];
 };
 
 struct dpu_global_state
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index fd2d104f0a91..4da6d72b7996 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -11,6 +11,7 @@
 #include "dpu_hw_intf.h"
 #include "dpu_hw_dspp.h"
 #include "dpu_hw_merge3d.h"
+#include "dpu_hw_dsc.h"
 #include "dpu_encoder.h"
 #include "dpu_trace.h"
 
@@ -75,6 +76,14 @@ int dpu_rm_destroy(struct dpu_rm *rm)
 			dpu_hw_intf_destroy(hw);
 		}
 	}
+	for (i = 0; i < ARRAY_SIZE(rm->dsc_blks); i++) {
+		struct dpu_hw_dsc *hw;
+
+		if (rm->intf_blks[i]) {
+			hw = to_dpu_hw_dsc(rm->dsc_blks[i]);
+			dpu_hw_dsc_destroy(hw);
+		}
+	}
 
 	return 0;
 }
@@ -221,6 +230,19 @@ int dpu_rm_init(struct dpu_rm *rm,
 		rm->dspp_blks[dspp->id - DSPP_0] = &hw->base;
 	}
 
+	for (i = 0; i < cat->dsc_count; i++) {
+		struct dpu_hw_dsc *hw;
+		const struct dpu_dsc_cfg *dsc = &cat->dsc[i];
+
+		hw = dpu_hw_dsc_init(dsc->id, mmio, cat);
+		if (IS_ERR_OR_NULL(hw)) {
+			rc = PTR_ERR(hw);
+			DPU_ERROR("failed dsc object creation: err %d\n", rc);
+			goto fail;
+		}
+		rm->dsc_blks[dsc->id - DSC_0] = &hw->base;
+	}
+
 	return 0;
 
 fail:
@@ -476,6 +498,9 @@ static int _dpu_rm_reserve_intf(
 	}
 
 	global_state->intf_to_enc_id[idx] = enc_id;
+
+	global_state->dsc_to_enc_id[0] = enc_id;
+	global_state->dsc_to_enc_id[1] = enc_id;
 	return 0;
 }
 
@@ -567,6 +592,8 @@ void dpu_rm_release(struct dpu_global_state *global_state,
 		ARRAY_SIZE(global_state->ctl_to_enc_id), enc->base.id);
 	_dpu_rm_clear_mapping(global_state->intf_to_enc_id,
 		ARRAY_SIZE(global_state->intf_to_enc_id), enc->base.id);
+	_dpu_rm_clear_mapping(global_state->dsc_to_enc_id,
+		ARRAY_SIZE(global_state->dsc_to_enc_id), enc->base.id);
 }
 
 int dpu_rm_reserve(
@@ -640,6 +667,11 @@ int dpu_rm_get_assigned_resources(struct dpu_rm *rm,
 		hw_to_enc_id = global_state->dspp_to_enc_id;
 		max_blks = ARRAY_SIZE(rm->dspp_blks);
 		break;
+	case DPU_HW_BLK_DSC:
+		hw_blks = rm->dsc_blks;
+		hw_to_enc_id = global_state->dsc_to_enc_id;
+		max_blks = ARRAY_SIZE(rm->dsc_blks);
+		break;
 	default:
 		DPU_ERROR("blk type %d not managed by rm\n", type);
 		return 0;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index 1f12c8d5b8aa..278d2a510b80 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -30,6 +30,7 @@ struct dpu_rm {
 	struct dpu_hw_blk *intf_blks[INTF_MAX - INTF_0];
 	struct dpu_hw_blk *dspp_blks[DSPP_MAX - DSPP_0];
 	struct dpu_hw_blk *merge_3d_blks[MERGE_3D_MAX - MERGE_3D_0];
+	struct dpu_hw_blk *dsc_blks[DSC_MAX - DSC_0];
 
 	uint32_t lm_max_width;
 };
-- 
2.31.1


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

* [PATCH 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog
  2021-07-15  6:51 [PATCH 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
                   ` (3 preceding siblings ...)
  2021-07-15  6:51 ` [PATCH 04/11] drm/msm/disp/dpu1: Add DSC support in RM Vinod Koul
@ 2021-07-15  6:51 ` Vinod Koul
  2021-07-29 20:25   ` Dmitry Baryshkov
  2021-08-02 23:29   ` [Freedreno] " abhinavk
  2021-07-15  6:51 ` [PATCH 06/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl Vinod Koul
                   ` (5 subsequent siblings)
  10 siblings, 2 replies; 41+ messages in thread
From: Vinod Koul @ 2021-07-15  6:51 UTC (permalink / raw)
  To: Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, David Airlie,
	Daniel Vetter, Jonathan Marek, Dmitry Baryshkov, Abhinav Kumar,
	Jeffrey Hugo, Sumit Semwal, linux-kernel, dri-devel, freedreno

This add SDM845 DSC blocks into hw_catalog

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
Changes since RFC:
 - use BIT values from MASK

 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

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 b569030a0847..b45a08303c99 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -40,6 +40,8 @@
 
 #define PINGPONG_SDM845_MASK BIT(DPU_PINGPONG_DITHER)
 
+#define DSC_SDM845_MASK BIT(1)
+
 #define PINGPONG_SDM845_SPLIT_MASK \
 	(PINGPONG_SDM845_MASK | BIT(DPU_PINGPONG_TE2))
 
@@ -751,6 +753,24 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = {
 	PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, 0, sc7280_pp_sblk),
 	PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk),
 };
+
+/*************************************************************
+ * DSC sub blocks config
+ *************************************************************/
+#define DSC_BLK(_name, _id, _base) \
+	{\
+	.name = _name, .id = _id, \
+	.base = _base, .len = 0x140, \
+	.features = DSC_SDM845_MASK, \
+	}
+
+static struct dpu_dsc_cfg sdm845_dsc[] = {
+	DSC_BLK("dsc_0", DSC_0, 0x80000),
+	DSC_BLK("dsc_1", DSC_1, 0x80400),
+	DSC_BLK("dsc_2", DSC_2, 0x80800),
+	DSC_BLK("dsc_3", DSC_3, 0x80c00),
+};
+
 /*************************************************************
  * INTF sub blocks config
  *************************************************************/
@@ -1053,6 +1073,8 @@ static void sdm845_cfg_init(struct dpu_mdss_cfg *dpu_cfg)
 		.mixer = sdm845_lm,
 		.pingpong_count = ARRAY_SIZE(sdm845_pp),
 		.pingpong = sdm845_pp,
+		.dsc_count = ARRAY_SIZE(sdm845_dsc),
+		.dsc = sdm845_dsc,
 		.intf_count = ARRAY_SIZE(sdm845_intf),
 		.intf = sdm845_intf,
 		.vbif_count = ARRAY_SIZE(sdm845_vbif),
-- 
2.31.1


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

* [PATCH 06/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl
  2021-07-15  6:51 [PATCH 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
                   ` (4 preceding siblings ...)
  2021-07-15  6:51 ` [PATCH 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog Vinod Koul
@ 2021-07-15  6:51 ` Vinod Koul
  2021-07-29 22:15   ` Dmitry Baryshkov
  2021-08-03  0:00   ` [Freedreno] " abhinavk
  2021-07-15  6:51 ` [PATCH 07/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d Vinod Koul
                   ` (4 subsequent siblings)
  10 siblings, 2 replies; 41+ messages in thread
From: Vinod Koul @ 2021-07-15  6:51 UTC (permalink / raw)
  To: Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, David Airlie,
	Daniel Vetter, Jonathan Marek, Dmitry Baryshkov, Abhinav Kumar,
	Jeffrey Hugo, Sumit Semwal, linux-kernel, dri-devel, freedreno

Later gens of hardware have DSC bits moved to hw_ctl, so configure these
bits so that DSC would work there as well

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 2d4645e01ebf..aeea6add61ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -25,6 +25,8 @@
 #define   CTL_MERGE_3D_ACTIVE           0x0E4
 #define   CTL_INTF_ACTIVE               0x0F4
 #define   CTL_MERGE_3D_FLUSH            0x100
+#define   CTL_DSC_ACTIVE                0x0E8
+#define   CTL_DSC_FLUSH                0x104
 #define   CTL_INTF_FLUSH                0x110
 #define   CTL_INTF_MASTER               0x134
 #define   CTL_FETCH_PIPE_ACTIVE         0x0FC
@@ -34,6 +36,7 @@
 
 #define DPU_REG_RESET_TIMEOUT_US        2000
 #define  MERGE_3D_IDX   23
+#define  DSC_IDX        22
 #define  INTF_IDX       31
 #define CTL_INVALID_BIT                 0xffff
 
@@ -120,6 +123,7 @@ static u32 dpu_hw_ctl_get_pending_flush(struct dpu_hw_ctl *ctx)
 
 static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
 {
+	DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | BIT(3));
 
 	if (ctx->pending_flush_mask & BIT(MERGE_3D_IDX))
 		DPU_REG_WRITE(&ctx->hw, CTL_MERGE_3D_FLUSH,
@@ -128,7 +132,7 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
 		DPU_REG_WRITE(&ctx->hw, CTL_INTF_FLUSH,
 				ctx->pending_intf_flush_mask);
 
-	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
+	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask |  BIT(DSC_IDX));
 }
 
 static inline void dpu_hw_ctl_trigger_flush(struct dpu_hw_ctl *ctx)
@@ -507,6 +511,7 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
 	if (cfg->merge_3d)
 		DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
 			      BIT(cfg->merge_3d - MERGE_3D_0));
+	DPU_REG_WRITE(c, CTL_DSC_ACTIVE, BIT(0) | BIT(1) | BIT(2) | BIT(3));
 }
 
 static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
-- 
2.31.1


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

* [PATCH 07/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d
  2021-07-15  6:51 [PATCH 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
                   ` (5 preceding siblings ...)
  2021-07-15  6:51 ` [PATCH 06/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl Vinod Koul
@ 2021-07-15  6:51 ` Vinod Koul
  2021-08-03  0:24   ` [Freedreno] " abhinavk
  2021-07-15  6:52 ` [PATCH 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder Vinod Koul
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 41+ messages in thread
From: Vinod Koul @ 2021-07-15  6:51 UTC (permalink / raw)
  To: Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, David Airlie,
	Daniel Vetter, Jonathan Marek, Dmitry Baryshkov, Abhinav Kumar,
	Jeffrey Hugo, Sumit Semwal, linux-kernel, dri-devel, freedreno

We cannot enable mode_3d when we are using the DSC. So pass
configuration to detect DSC is enabled and not enable mode_3d
when we are using DSC

We add a helper dpu_encoder_helper_get_dsc_mode() to detect dsc
enabled and pass this to .setup_intf_cfg()

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h     | 11 +++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c |  2 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c           |  5 +++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h           |  2 ++
 4 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index ecbc4be98980..d43b804528eb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -336,6 +336,17 @@ static inline enum dpu_3d_blend_mode dpu_encoder_helper_get_3d_blend_mode(
 	return BLEND_3D_NONE;
 }
 
+static inline bool dpu_encoder_helper_get_dsc_mode(struct dpu_encoder_phys *phys_enc)
+{
+	struct drm_encoder *drm_enc = phys_enc->parent;
+	struct msm_drm_private *priv = drm_enc->dev->dev_private;
+
+	if (priv->dsc)
+		return true;
+
+	return false;
+}
+
 /**
  * dpu_encoder_helper_split_config - split display configuration helper function
  *	This helper function may be used by physical encoders to configure
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 b2be39b9144e..5fe87881c30c 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
@@ -69,6 +69,8 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg(
 	intf_cfg.intf_mode_sel = DPU_CTL_MODE_SEL_CMD;
 	intf_cfg.stream_sel = cmd_enc->stream_sel;
 	intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
+	intf_cfg.dsc = dpu_encoder_helper_get_dsc_mode(phys_enc);
+
 	ctl->ops.setup_intf_cfg(ctl, &intf_cfg);
 }
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index aeea6add61ee..f059416311ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -121,7 +121,7 @@ static u32 dpu_hw_ctl_get_pending_flush(struct dpu_hw_ctl *ctx)
 	return ctx->pending_flush_mask;
 }
 
-static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
+static void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
 {
 	DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | BIT(3));
 
@@ -522,7 +522,8 @@ static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
 
 	intf_cfg |= (cfg->intf & 0xF) << 4;
 
-	if (cfg->mode_3d) {
+	/* In DSC we can't set merge, so check for dsc too */
+	if (cfg->mode_3d && !cfg->dsc) {
 		intf_cfg |= BIT(19);
 		intf_cfg |= (cfg->mode_3d - 0x1) << 20;
 	}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
index 806c171e5df2..347a653c1e01 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
@@ -39,6 +39,7 @@ struct dpu_hw_stage_cfg {
  * @mode_3d:               3d mux configuration
  * @merge_3d:              3d merge block used
  * @intf_mode_sel:         Interface mode, cmd / vid
+ * @dsc:                   DSC is enabled
  * @stream_sel:            Stream selection for multi-stream interfaces
  */
 struct dpu_hw_intf_cfg {
@@ -46,6 +47,7 @@ struct dpu_hw_intf_cfg {
 	enum dpu_3d_blend_mode mode_3d;
 	enum dpu_merge_3d merge_3d;
 	enum dpu_ctl_mode_sel intf_mode_sel;
+	bool dsc;
 	int stream_sel;
 };
 
-- 
2.31.1


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

* [PATCH 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder
  2021-07-15  6:51 [PATCH 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
                   ` (6 preceding siblings ...)
  2021-07-15  6:51 ` [PATCH 07/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d Vinod Koul
@ 2021-07-15  6:52 ` Vinod Koul
  2021-07-19  8:54   ` kernel test robot
                     ` (2 more replies)
  2021-07-15  6:52 ` [PATCH 09/11] drm/msm/disp/dpu1: Add support for DSC in topology Vinod Koul
                   ` (2 subsequent siblings)
  10 siblings, 3 replies; 41+ messages in thread
From: Vinod Koul @ 2021-07-15  6:52 UTC (permalink / raw)
  To: Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, David Airlie,
	Daniel Vetter, Jonathan Marek, Dmitry Baryshkov, Abhinav Kumar,
	Jeffrey Hugo, Sumit Semwal, linux-kernel, dri-devel, freedreno

When DSC is enabled in DT, we need to configure the encoder for DSC
configuration, calculate DSC parameters for the given timing.

This patch adds that support by adding dpu_encoder_prep_dsc() which is
invoked when DSC is enabled in DT

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 142 +++++++++++++++++++-
 1 file changed, 141 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 8d942052db8a..41140b781e66 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -21,12 +21,17 @@
 #include "dpu_hw_intf.h"
 #include "dpu_hw_ctl.h"
 #include "dpu_hw_dspp.h"
+#include "dpu_hw_dsc.h"
 #include "dpu_formats.h"
 #include "dpu_encoder_phys.h"
 #include "dpu_crtc.h"
 #include "dpu_trace.h"
 #include "dpu_core_irq.h"
 
+#define DSC_MODE_SPLIT_PANEL		BIT(0)
+#define DSC_MODE_MULTIPLEX		BIT(1)
+#define DSC_MODE_VIDEO			BIT(2)
+
 #define DPU_DEBUG_ENC(e, fmt, ...) DPU_DEBUG("enc%d " fmt,\
 		(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
 
@@ -135,6 +140,7 @@ enum dpu_enc_rc_states {
  * @cur_slave:		As above but for the slave encoder.
  * @hw_pp:		Handle to the pingpong blocks used for the display. No.
  *			pingpong blocks can be different than num_phys_encs.
+ * @hw_dsc		Handle to the DSC blocks used for the display.
  * @intfs_swapped:	Whether or not the phys_enc interfaces have been swapped
  *			for partial update right-only cases, such as pingpong
  *			split where virtual pingpong does not generate IRQs
@@ -180,6 +186,7 @@ struct dpu_encoder_virt {
 	struct dpu_encoder_phys *cur_master;
 	struct dpu_encoder_phys *cur_slave;
 	struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
+	struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
 
 	bool intfs_swapped;
 
@@ -1008,7 +1015,8 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 	struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
 	struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
 	struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC] = { NULL };
-	int num_lm, num_ctl, num_pp;
+	struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
+	int num_lm, num_ctl, num_pp, num_dsc;
 	int i, j;
 
 	if (!drm_enc) {
@@ -1061,11 +1069,16 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 	dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
 		drm_enc->base.id, DPU_HW_BLK_DSPP, hw_dspp,
 		ARRAY_SIZE(hw_dspp));
+	num_dsc = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
+		drm_enc->base.id, DPU_HW_BLK_DSC, hw_dsc, ARRAY_SIZE(hw_dsc));
 
 	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
 		dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i])
 						: NULL;
 
+	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
+		dpu_enc->hw_dsc[i] = i < num_dsc ? to_dpu_hw_dsc(hw_dsc[i]) : NULL;
+
 	cstate = to_dpu_crtc_state(drm_crtc->state);
 
 	for (i = 0; i < num_lm; i++) {
@@ -1810,10 +1823,133 @@ static void dpu_encoder_vsync_event_work_handler(struct kthread_work *work)
 			nsecs_to_jiffies(ktime_to_ns(wakeup_time)));
 }
 
+static void
+dpu_encoder_dsc_pclk_param_calc(struct msm_display_dsc_config *dsc, u32 width)
+{
+	int slice_count, slice_per_intf;
+	int bytes_in_slice, total_bytes_per_intf;
+
+	if (!dsc || !dsc->drm->slice_width || !dsc->drm->slice_count) {
+		DPU_ERROR("Invalid DSC/slices\n");
+		return;
+	}
+
+	slice_count = dsc->drm->slice_count;
+	slice_per_intf = DIV_ROUND_UP(width, dsc->drm->slice_width);
+
+	/*
+	 * If slice_count is greater than slice_per_intf then default to 1.
+	 * This can happen during partial update.
+	 */
+	if (slice_count > slice_per_intf)
+		slice_count = 1;
+
+	bytes_in_slice = DIV_ROUND_UP(dsc->drm->slice_width *
+				      dsc->drm->bits_per_pixel, 8);
+	total_bytes_per_intf = bytes_in_slice * slice_per_intf;
+
+	dsc->eol_byte_num = total_bytes_per_intf % 3;
+	dsc->pclk_per_line =  DIV_ROUND_UP(total_bytes_per_intf, 3);
+	dsc->bytes_in_slice = bytes_in_slice;
+	dsc->bytes_per_pkt = bytes_in_slice * slice_count;
+	dsc->pkt_per_line = slice_per_intf / slice_count;
+}
+
+static void
+dpu_encoder_dsc_initial_line_calc(struct msm_display_dsc_config *dsc,
+				  u32 enc_ip_width)
+{
+	int ssm_delay, total_pixels, soft_slice_per_enc;
+
+	soft_slice_per_enc = enc_ip_width / dsc->drm->slice_width;
+
+	/*
+	 * minimum number of initial line pixels is a sum of:
+	 * 1. sub-stream multiplexer delay (83 groups for 8bpc,
+	 *    91 for 10 bpc) * 3
+	 * 2. for two soft slice cases, add extra sub-stream multiplexer * 3
+	 * 3. the initial xmit delay
+	 * 4. total pipeline delay through the "lock step" of encoder (47)
+	 * 5. 6 additional pixels as the output of the rate buffer is
+	 *    48 bits wide
+	 */
+	ssm_delay = ((dsc->drm->bits_per_component < 10) ? 84 : 92);
+	total_pixels = ssm_delay * 3 + dsc->drm->initial_xmit_delay + 47;
+	if (soft_slice_per_enc > 1)
+		total_pixels += (ssm_delay * 3);
+	dsc->initial_lines = DIV_ROUND_UP(total_pixels, dsc->drm->slice_width);
+}
+
+static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
+				     struct dpu_hw_pingpong *hw_pp,
+				     struct msm_display_dsc_config *dsc,
+				     u32 common_mode)
+{
+	if (hw_dsc->ops.dsc_config)
+		hw_dsc->ops.dsc_config(hw_dsc, dsc, common_mode);
+
+	if (hw_dsc->ops.dsc_config_thresh)
+		hw_dsc->ops.dsc_config_thresh(hw_dsc, dsc);
+
+	if (hw_pp->ops.setup_dsc)
+		hw_pp->ops.setup_dsc(hw_pp);
+
+	if (hw_pp->ops.enable_dsc)
+		hw_pp->ops.enable_dsc(hw_pp);
+}
+
+static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
+				 struct msm_display_dsc_config *dsc)
+{
+	/* coding only for 2LM, 2enc, 1 dsc config */
+	struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
+	struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
+	struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
+	int this_frame_slices;
+	int intf_ip_w, enc_ip_w;
+	int dsc_common_mode;
+	int pic_width, pic_height;
+	int i;
+
+	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
+		hw_pp[i] = dpu_enc->hw_pp[i];
+		hw_dsc[i] = dpu_enc->hw_dsc[i];
+
+		if (!hw_pp[i] || !hw_dsc[i]) {
+			DPU_ERROR_ENC(dpu_enc, "invalid params for DSC\n");
+			return;
+		}
+	}
+
+	dsc_common_mode = 0;
+	pic_width = dsc->drm->pic_width;
+	pic_height = dsc->drm->pic_height;
+
+	dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL;
+	if (enc_master->intf_mode == INTF_MODE_VIDEO)
+		dsc_common_mode |= DSC_MODE_VIDEO;
+
+	this_frame_slices = pic_width / dsc->drm->slice_width;
+	intf_ip_w = this_frame_slices * dsc->drm->slice_width;
+
+	dpu_encoder_dsc_pclk_param_calc(dsc, intf_ip_w);
+
+	/*
+	 * dsc merge case: when using 2 encoders for the same stream,
+	 * no. of slices need to be same on both the encoders.
+	 */
+	enc_ip_w = intf_ip_w / 2;
+	dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
+
+	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
+		dpu_encoder_dsc_pipe_cfg(hw_dsc[i], hw_pp[i], dsc, dsc_common_mode);
+}
+
 void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
 {
 	struct dpu_encoder_virt *dpu_enc;
 	struct dpu_encoder_phys *phys;
+	struct msm_drm_private *priv;
 	bool needs_hw_reset = false;
 	unsigned int i;
 
@@ -1841,6 +1977,10 @@ void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
 			dpu_encoder_helper_hw_reset(dpu_enc->phys_encs[i]);
 		}
 	}
+
+	priv = drm_enc->dev->dev_private;
+	if (priv->dsc)
+		dpu_encoder_prep_dsc(dpu_enc, priv->dsc);
 }
 
 void dpu_encoder_kickoff(struct drm_encoder *drm_enc)
-- 
2.31.1


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

* [PATCH 09/11] drm/msm/disp/dpu1: Add support for DSC in topology
  2021-07-15  6:51 [PATCH 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
                   ` (7 preceding siblings ...)
  2021-07-15  6:52 ` [PATCH 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder Vinod Koul
@ 2021-07-15  6:52 ` Vinod Koul
  2021-08-03  1:05   ` [Freedreno] " abhinavk
  2021-07-15  6:52 ` [PATCH 10/11] drm/msm/dsi: Add support for DSC configuration Vinod Koul
  2021-07-15  6:52 ` [PATCH 11/11] drm/msm/dsi: Pass DSC params to drm_panel Vinod Koul
  10 siblings, 1 reply; 41+ messages in thread
From: Vinod Koul @ 2021-07-15  6:52 UTC (permalink / raw)
  To: Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, David Airlie,
	Daniel Vetter, Jonathan Marek, Dmitry Baryshkov, Abhinav Kumar,
	Jeffrey Hugo, Sumit Semwal, linux-kernel, dri-devel, freedreno

For DSC to work we typically need a 2,2,1 configuration. This should
suffice for resolutions upto 4k. For more resolutions like 8k this won't
work.

The topology information is provided by DTS so we try to deduce the
topology required for DSC.
Furthermore, we can use 1 DSC encoder in lesser resolutions, but that is
not power efficient according to Abhinav, it is better to use 2 mixers
as that will split width/2 and is proven to be power efficient.

Also, the panel has been tested only with 2,2,1 configuration, so for
now we blindly create 2,2,1 topology when DSC is enabled

Co-developed-by: Abhinav Kumar <abhinavk@codeaurora.org>
Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
Changes since RFC:
 - Add more details in changelog

 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 41140b781e66..8f0a8bd9c8ff 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -573,6 +573,8 @@ static struct msm_display_topology dpu_encoder_get_topology(
 			struct drm_display_mode *mode)
 {
 	struct msm_display_topology topology = {0};
+	struct drm_encoder *drm_enc;
+	struct msm_drm_private *priv;
 	int i, intf_count = 0;
 
 	for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
@@ -607,8 +609,22 @@ static struct msm_display_topology dpu_encoder_get_topology(
 	topology.num_enc = 0;
 	topology.num_intf = intf_count;
 
+	drm_enc = &dpu_enc->base;
+	priv = drm_enc->dev->dev_private;
+	if (priv && priv->dsc) {
+		/* In case of Display Stream Compression DSC, we would use
+		 * 2 encoders, 2 line mixers and 1 interface
+		 * this is power optimal and can drive upto (including) 4k
+		 * screens
+		 */
+		topology.num_enc = 2;
+		topology.num_intf = 1;
+		topology.num_lm = 2;
+	}
+
 	return topology;
 }
+
 static int dpu_encoder_virt_atomic_check(
 		struct drm_encoder *drm_enc,
 		struct drm_crtc_state *crtc_state,
-- 
2.31.1


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

* [PATCH 10/11] drm/msm/dsi: Add support for DSC configuration
  2021-07-15  6:51 [PATCH 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
                   ` (8 preceding siblings ...)
  2021-07-15  6:52 ` [PATCH 09/11] drm/msm/disp/dpu1: Add support for DSC in topology Vinod Koul
@ 2021-07-15  6:52 ` Vinod Koul
  2021-07-29 22:10   ` Dmitry Baryshkov
  2021-08-03  1:16   ` [Freedreno] " abhinavk
  2021-07-15  6:52 ` [PATCH 11/11] drm/msm/dsi: Pass DSC params to drm_panel Vinod Koul
  10 siblings, 2 replies; 41+ messages in thread
From: Vinod Koul @ 2021-07-15  6:52 UTC (permalink / raw)
  To: Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, David Airlie,
	Daniel Vetter, Jonathan Marek, Dmitry Baryshkov, Abhinav Kumar,
	Jeffrey Hugo, Sumit Semwal, linux-kernel, dri-devel, freedreno

When DSC is enabled, we need to configure DSI registers accordingly and
configure the respective stream compression registers.

Add support to calculate the register setting based on DSC params and
timing information and configure these registers.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/gpu/drm/msm/dsi/dsi.xml.h  |  10 ++
 drivers/gpu/drm/msm/dsi/dsi_host.c | 142 +++++++++++++++++++++++++++--
 2 files changed, 142 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.xml.h b/drivers/gpu/drm/msm/dsi/dsi.xml.h
index 50eb4d1b8fdd..b8e9e608abfc 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.xml.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.xml.h
@@ -2310,4 +2310,14 @@ static inline uint32_t REG_DSI_7nm_PHY_LN_TX_DCTRL(uint32_t i0) { return 0x00000
 
 #define REG_DSI_7nm_PHY_PLL_PERF_OPTIMIZE			0x00000260
 
+#define REG_DSI_VIDEO_COMPRESSION_MODE_CTRL			0x0000029c
+
+#define REG_DSI_VIDEO_COMPRESSION_MODE_CTRL2			0x000002a0
+
+#define REG_DSI_COMMAND_COMPRESSION_MODE_CTRL			0x000002a4
+
+#define REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2			0x000002a8
+
+#define REG_DSI_COMMAND_COMPRESSION_MODE_CTRL3			0x000002ac
+
 #endif /* DSI_XML */
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index e1e5d91809b5..4e8ab1b1df8b 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -942,6 +942,26 @@ static void dsi_ctrl_config(struct msm_dsi_host *msm_host, bool enable,
 	dsi_write(msm_host, REG_DSI_CTRL, data);
 }
 
+static int dsi_dsc_update_pic_dim(struct msm_display_dsc_config *dsc,
+				  int pic_width, int pic_height)
+{
+	if (!dsc || !pic_width || !pic_height) {
+		pr_err("DSI: invalid input: pic_width: %d pic_height: %d\n", pic_width, pic_height);
+		return -EINVAL;
+	}
+
+	if ((pic_width % dsc->drm->slice_width) || (pic_height % dsc->drm->slice_height)) {
+		pr_err("DSI: pic_dim %dx%d has to be multiple of slice %dx%d\n",
+		       pic_width, pic_height, dsc->drm->slice_width, dsc->drm->slice_height);
+		return -EINVAL;
+	}
+
+	dsc->drm->pic_width = pic_width;
+	dsc->drm->pic_height = pic_height;
+
+	return 0;
+}
+
 static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_dual_dsi)
 {
 	struct drm_display_mode *mode = msm_host->mode;
@@ -956,6 +976,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_dual_dsi)
 	u32 va_end = va_start + mode->vdisplay;
 	u32 hdisplay = mode->hdisplay;
 	u32 wc;
+	u32 data;
 
 	DBG("");
 
@@ -974,7 +995,73 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_dual_dsi)
 		hdisplay /= 2;
 	}
 
+	if (msm_host->dsc) {
+		struct msm_display_dsc_config *dsc = msm_host->dsc;
+
+		/* update dsc params with timing params */
+		dsi_dsc_update_pic_dim(dsc, mode->hdisplay, mode->vdisplay);
+		DBG("Mode Width- %d x Height %d\n", dsc->drm->pic_width, dsc->drm->pic_height);
+
+		/* we do the calculations for dsc parameters here so that
+		 * panel can use these parameters
+		 */
+		dsi_populate_dsc_params(dsc);
+
+		/* Divide the display by 3 but keep back/font porch and
+		 * pulse width same
+		 */
+		h_total -= hdisplay;
+		hdisplay /= 3;
+		h_total += hdisplay;
+		ha_end = ha_start + hdisplay;
+	}
+
 	if (msm_host->mode_flags & MIPI_DSI_MODE_VIDEO) {
+		if (msm_host->dsc) {
+			struct msm_display_dsc_config *dsc = msm_host->dsc;
+			u32 reg, intf_width, slice_per_intf, width;
+			u32 total_bytes_per_intf;
+
+			/* first calculate dsc parameters and then program
+			 * compress mode registers
+			 */
+			intf_width = hdisplay;
+			slice_per_intf = DIV_ROUND_UP(intf_width, dsc->drm->slice_width);
+
+			/* If slice_count > slice_per_intf, then use 1
+			 * This can happen during partial update
+			 */
+				dsc->drm->slice_count = 1;
+
+			dsc->bytes_in_slice = DIV_ROUND_UP(dsc->drm->slice_width * 8, 8);
+			total_bytes_per_intf = dsc->bytes_in_slice * slice_per_intf;
+
+			dsc->eol_byte_num = total_bytes_per_intf % 3;
+			dsc->pclk_per_line =  DIV_ROUND_UP(total_bytes_per_intf, 3);
+			dsc->bytes_per_pkt = dsc->bytes_in_slice * dsc->drm->slice_count;
+			dsc->pkt_per_line = slice_per_intf / dsc->drm->slice_count;
+
+			width = dsc->pclk_per_line;
+			reg = dsc->bytes_per_pkt << 16;
+			reg |= (0x0b << 8);    /* dtype of compressed image */
+
+			/* pkt_per_line:
+			 * 0 == 1 pkt
+			 * 1 == 2 pkt
+			 * 2 == 4 pkt
+			 * 3 pkt is not supported
+			 * above translates to ffs() - 1
+			 */
+			reg |= (ffs(dsc->pkt_per_line) - 1) << 6;
+
+			dsc->eol_byte_num = total_bytes_per_intf % 3;
+			reg |= dsc->eol_byte_num << 4;
+			reg |= 1;
+
+			dsi_write(msm_host,
+				  REG_DSI_VIDEO_COMPRESSION_MODE_CTRL, reg);
+		}
+
 		dsi_write(msm_host, REG_DSI_ACTIVE_H,
 			DSI_ACTIVE_H_START(ha_start) |
 			DSI_ACTIVE_H_END(ha_end));
@@ -993,19 +1080,50 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_dual_dsi)
 			DSI_ACTIVE_VSYNC_VPOS_START(vs_start) |
 			DSI_ACTIVE_VSYNC_VPOS_END(vs_end));
 	} else {		/* command mode */
+		if (msm_host->dsc) {
+			struct msm_display_dsc_config *dsc = msm_host->dsc;
+			u32 reg, reg_ctrl, reg_ctrl2;
+			u32 slice_per_intf, bytes_in_slice, total_bytes_per_intf;
+
+			reg_ctrl = dsi_read(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL);
+			reg_ctrl2 = dsi_read(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2);
+
+			slice_per_intf = DIV_ROUND_UP(hdisplay, dsc->drm->slice_width);
+			bytes_in_slice = DIV_ROUND_UP(dsc->drm->slice_width *
+						      dsc->drm->bits_per_pixel, 8);
+			dsc->drm->slice_chunk_size = bytes_in_slice;
+			total_bytes_per_intf = dsc->bytes_in_slice * slice_per_intf;
+			dsc->pkt_per_line = slice_per_intf / dsc->drm->slice_count;
+
+			reg = 0x39 << 8;
+			reg |= ffs(dsc->pkt_per_line) << 6;
+
+			dsc->eol_byte_num = total_bytes_per_intf % 3;
+			reg |= dsc->eol_byte_num << 4;
+			reg |= 1;
+
+			reg_ctrl |= reg;
+			reg_ctrl2 |= bytes_in_slice;
+
+			dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL, reg);
+			dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2, reg_ctrl2);
+		}
+
 		/* image data and 1 byte write_memory_start cmd */
-		wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
+		if (!msm_host->dsc)
+			wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
+		else
+			wc = mode->hdisplay / 2 + 1;
 
-		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL,
-			DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
-			DSI_CMD_MDP_STREAM0_CTRL_VIRTUAL_CHANNEL(
-					msm_host->channel) |
-			DSI_CMD_MDP_STREAM0_CTRL_DATA_TYPE(
-					MIPI_DSI_DCS_LONG_WRITE));
+		data = DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
+		       DSI_CMD_MDP_STREAM0_CTRL_VIRTUAL_CHANNEL(msm_host->channel) |
+			DSI_CMD_MDP_STREAM0_CTRL_DATA_TYPE(MIPI_DSI_DCS_LONG_WRITE);
 
-		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_TOTAL,
-			DSI_CMD_MDP_STREAM0_TOTAL_H_TOTAL(hdisplay) |
-			DSI_CMD_MDP_STREAM0_TOTAL_V_TOTAL(mode->vdisplay));
+		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL, data);
+
+		data = DSI_CMD_MDP_STREAM0_TOTAL_H_TOTAL(hdisplay) |
+			DSI_CMD_MDP_STREAM0_TOTAL_V_TOTAL(mode->vdisplay);
+		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_TOTAL, data);
 	}
 }
 
@@ -2074,6 +2192,7 @@ int msm_dsi_host_modeset_init(struct mipi_dsi_host *host,
 	struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
 	const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
 	struct platform_device *pdev = msm_host->pdev;
+	struct msm_drm_private *priv;
 	int ret;
 
 	msm_host->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
@@ -2093,6 +2212,9 @@ int msm_dsi_host_modeset_init(struct mipi_dsi_host *host,
 	}
 
 	msm_host->dev = dev;
+	priv = dev->dev_private;
+	priv->dsc = msm_host->dsc;
+
 	ret = cfg_hnd->ops->tx_buf_alloc(msm_host, SZ_4K);
 	if (ret) {
 		pr_err("%s: alloc tx gem obj failed, %d\n", __func__, ret);
-- 
2.31.1


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

* [PATCH 11/11] drm/msm/dsi: Pass DSC params to drm_panel
  2021-07-15  6:51 [PATCH 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
                   ` (9 preceding siblings ...)
  2021-07-15  6:52 ` [PATCH 10/11] drm/msm/dsi: Add support for DSC configuration Vinod Koul
@ 2021-07-15  6:52 ` Vinod Koul
  2021-08-03  1:22   ` [Freedreno] " abhinavk
  10 siblings, 1 reply; 41+ messages in thread
From: Vinod Koul @ 2021-07-15  6:52 UTC (permalink / raw)
  To: Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, David Airlie,
	Daniel Vetter, Jonathan Marek, Dmitry Baryshkov, Abhinav Kumar,
	Jeffrey Hugo, Sumit Semwal, linux-kernel, dri-devel, freedreno

When DSC is enabled, we need to pass the DSC parameters to panel driver
as well, so add a dsc parameter in panel and set it when DSC is enabled

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 16 +++++++++++++++-
 include/drm/drm_panel.h            |  7 +++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 4e8ab1b1df8b..ee21cda243a7 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -2193,6 +2193,7 @@ int msm_dsi_host_modeset_init(struct mipi_dsi_host *host,
 	const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
 	struct platform_device *pdev = msm_host->pdev;
 	struct msm_drm_private *priv;
+	struct drm_panel *panel;
 	int ret;
 
 	msm_host->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
@@ -2212,8 +2213,21 @@ int msm_dsi_host_modeset_init(struct mipi_dsi_host *host,
 	}
 
 	msm_host->dev = dev;
+	panel = msm_dsi_host_get_panel(&msm_host->base);
 	priv = dev->dev_private;
-	priv->dsc = msm_host->dsc;
+
+	if (panel && panel->dsc) {
+		struct msm_display_dsc_config *dsc = priv->dsc;
+
+		if (!dsc) {
+			dsc = kzalloc(sizeof(*dsc), GFP_KERNEL);
+			if (!dsc)
+				return -ENOMEM;
+			dsc->drm = panel->dsc;
+			priv->dsc = dsc;
+			msm_host->dsc = dsc;
+		}
+	}
 
 	ret = cfg_hnd->ops->tx_buf_alloc(msm_host, SZ_4K);
 	if (ret) {
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 33605c3f0eba..27a7808a29f2 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -171,6 +171,13 @@ struct drm_panel {
 	 * Panel entry in registry.
 	 */
 	struct list_head list;
+
+	/**
+	 * @dsc:
+	 *
+	 * Panel DSC pps payload to be sent
+	 */
+	struct drm_dsc_config *dsc;
 };
 
 void drm_panel_init(struct drm_panel *panel, struct device *dev,
-- 
2.31.1


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

* Re: [PATCH 02/11] drm/msm/disp/dpu1: Add support for DSC
  2021-07-15  6:51 ` [PATCH 02/11] drm/msm/disp/dpu1: Add support for DSC Vinod Koul
@ 2021-07-19  8:28   ` kernel test robot
  2021-08-02 23:03   ` [Freedreno] " abhinavk
  1 sibling, 0 replies; 41+ messages in thread
From: kernel test robot @ 2021-07-19  8:28 UTC (permalink / raw)
  To: Vinod Koul, Rob Clark
  Cc: kbuild-all, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Abhinav Kumar, Bjorn Andersson,
	Vinod Koul, dri-devel

[-- Attachment #1: Type: text/plain, Size: 2902 bytes --]

Hi Vinod,

I love your patch! Perhaps something to improve:

[auto build test WARNING on v5.13]
[also build test WARNING on next-20210716]
[cannot apply to linus/master v5.14-rc1]
[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]

url:    https://github.com/0day-ci/linux/commits/Vinod-Koul/drm-msm-Add-Display-Stream-Compression-Support/20210715-145540
base:    62fb9874f5da54fdb243003b386128037319b219
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/032a82b57221a13f65c55870ae3f64d0e5a07390
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Vinod-Koul/drm-msm-Add-Display-Stream-Compression-Support/20210715-145540
        git checkout 032a82b57221a13f65c55870ae3f64d0e5a07390
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c: In function 'dpu_hw_dsc_config_thresh':
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c:128:7: warning: variable 'lp' set but not used [-Wunused-but-set-variable]
     128 |  u16 *lp;
         |       ^~


vim +/lp +128 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c

   121	
   122	static void dpu_hw_dsc_config_thresh(struct dpu_hw_dsc *hw_dsc,
   123					     struct msm_display_dsc_config *dsc)
   124	{
   125		struct drm_dsc_rc_range_parameters *rc = dsc->drm->rc_range_params;
   126		struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
   127		u32 off;
 > 128		u16 *lp;
   129		int i;
   130	
   131		lp = dsc->drm->rc_buf_thresh;
   132		off = DSC_RC_BUF_THRESH;
   133		for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++) {
   134			DPU_REG_WRITE(c, off, dsc->drm->rc_buf_thresh[i]);
   135			off += 4;
   136		}
   137	
   138		off = DSC_RANGE_MIN_QP;
   139		for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
   140			DPU_REG_WRITE(c, off, rc[i].range_min_qp);
   141			off += 4;
   142		}
   143	
   144		off = DSC_RANGE_MAX_QP;
   145		for (i = 0; i < 15; i++) {
   146			DPU_REG_WRITE(c, off, rc[i].range_max_qp);
   147			off += 4;
   148		}
   149	
   150		off = DSC_RANGE_BPG_OFFSET;
   151		for (i = 0; i < 15; i++) {
   152			DPU_REG_WRITE(c, off, rc[i].range_bpg_offset);
   153			off += 4;
   154		}
   155	}
   156	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54713 bytes --]

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

* Re: [PATCH 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder
  2021-07-15  6:52 ` [PATCH 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder Vinod Koul
@ 2021-07-19  8:54   ` kernel test robot
  2021-07-29 20:54   ` Dmitry Baryshkov
  2021-08-03  0:57   ` [Freedreno] " abhinavk
  2 siblings, 0 replies; 41+ messages in thread
From: kernel test robot @ 2021-07-19  8:54 UTC (permalink / raw)
  To: Vinod Koul, Rob Clark
  Cc: kbuild-all, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Abhinav Kumar, Bjorn Andersson,
	Vinod Koul, dri-devel

[-- Attachment #1: Type: text/plain, Size: 3593 bytes --]

Hi Vinod,

I love your patch! Perhaps something to improve:

[auto build test WARNING on v5.13]
[cannot apply to linus/master v5.14-rc1 next-20210716]
[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]

url:    https://github.com/0day-ci/linux/commits/Vinod-Koul/drm-msm-Add-Display-Stream-Compression-Support/20210715-145540
base:    62fb9874f5da54fdb243003b386128037319b219
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/e61efb569c28d8036eb18f53763c195c16d8a396
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Vinod-Koul/drm-msm-Add-Display-Stream-Compression-Support/20210715-145540
        git checkout e61efb569c28d8036eb18f53763c195c16d8a396
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c: In function 'dpu_encoder_prep_dsc':
>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:1911:17: warning: variable 'pic_height' set but not used [-Wunused-but-set-variable]
    1911 |  int pic_width, pic_height;
         |                 ^~~~~~~~~~


vim +/pic_height +1911 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c

  1900	
  1901	static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
  1902					 struct msm_display_dsc_config *dsc)
  1903	{
  1904		/* coding only for 2LM, 2enc, 1 dsc config */
  1905		struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
  1906		struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
  1907		struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
  1908		int this_frame_slices;
  1909		int intf_ip_w, enc_ip_w;
  1910		int dsc_common_mode;
> 1911		int pic_width, pic_height;
  1912		int i;
  1913	
  1914		for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
  1915			hw_pp[i] = dpu_enc->hw_pp[i];
  1916			hw_dsc[i] = dpu_enc->hw_dsc[i];
  1917	
  1918			if (!hw_pp[i] || !hw_dsc[i]) {
  1919				DPU_ERROR_ENC(dpu_enc, "invalid params for DSC\n");
  1920				return;
  1921			}
  1922		}
  1923	
  1924		dsc_common_mode = 0;
  1925		pic_width = dsc->drm->pic_width;
  1926		pic_height = dsc->drm->pic_height;
  1927	
  1928		dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL;
  1929		if (enc_master->intf_mode == INTF_MODE_VIDEO)
  1930			dsc_common_mode |= DSC_MODE_VIDEO;
  1931	
  1932		this_frame_slices = pic_width / dsc->drm->slice_width;
  1933		intf_ip_w = this_frame_slices * dsc->drm->slice_width;
  1934	
  1935		dpu_encoder_dsc_pclk_param_calc(dsc, intf_ip_w);
  1936	
  1937		/*
  1938		 * dsc merge case: when using 2 encoders for the same stream,
  1939		 * no. of slices need to be same on both the encoders.
  1940		 */
  1941		enc_ip_w = intf_ip_w / 2;
  1942		dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
  1943	
  1944		for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
  1945			dpu_encoder_dsc_pipe_cfg(hw_dsc[i], hw_pp[i], dsc, dsc_common_mode);
  1946	}
  1947	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54713 bytes --]

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

* Re: [PATCH 04/11] drm/msm/disp/dpu1: Add DSC support in RM
  2021-07-15  6:51 ` [PATCH 04/11] drm/msm/disp/dpu1: Add DSC support in RM Vinod Koul
@ 2021-07-29 20:23   ` Dmitry Baryshkov
  2021-10-06 10:26     ` Vinod Koul
  2021-08-02 23:24   ` [Freedreno] " abhinavk
  1 sibling, 1 reply; 41+ messages in thread
From: Dmitry Baryshkov @ 2021-07-29 20:23 UTC (permalink / raw)
  To: Vinod Koul, Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, David Airlie, Daniel Vetter,
	Jonathan Marek, Abhinav Kumar, Jeffrey Hugo, Sumit Semwal,
	linux-kernel, dri-devel, freedreno

On 15/07/2021 09:51, Vinod Koul wrote:
> This add the bits in RM to enable the DSC blocks
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  1 +
>   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c  | 32 +++++++++++++++++++++++++
>   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h  |  1 +
>   3 files changed, 34 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index d6717d6672f7..d56c05146dfe 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -165,6 +165,7 @@ struct dpu_global_state {
>   	uint32_t ctl_to_enc_id[CTL_MAX - CTL_0];
>   	uint32_t intf_to_enc_id[INTF_MAX - INTF_0];
>   	uint32_t dspp_to_enc_id[DSPP_MAX - DSPP_0];
> +	uint32_t dsc_to_enc_id[DSC_MAX - DSC_0];
>   };
>   
>   struct dpu_global_state
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index fd2d104f0a91..4da6d72b7996 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -11,6 +11,7 @@
>   #include "dpu_hw_intf.h"
>   #include "dpu_hw_dspp.h"
>   #include "dpu_hw_merge3d.h"
> +#include "dpu_hw_dsc.h"
>   #include "dpu_encoder.h"
>   #include "dpu_trace.h"
>   
> @@ -75,6 +76,14 @@ int dpu_rm_destroy(struct dpu_rm *rm)
>   			dpu_hw_intf_destroy(hw);
>   		}
>   	}
> +	for (i = 0; i < ARRAY_SIZE(rm->dsc_blks); i++) {
> +		struct dpu_hw_dsc *hw;
> +
> +		if (rm->intf_blks[i]) {

rm->dsc_blks[i]

> +			hw = to_dpu_hw_dsc(rm->dsc_blks[i]);
> +			dpu_hw_dsc_destroy(hw);
> +		}
> +	}
>   
>   	return 0;
>   }
> @@ -221,6 +230,19 @@ int dpu_rm_init(struct dpu_rm *rm,
>   		rm->dspp_blks[dspp->id - DSPP_0] = &hw->base;
>   	}
>   
> +	for (i = 0; i < cat->dsc_count; i++) {
> +		struct dpu_hw_dsc *hw;
> +		const struct dpu_dsc_cfg *dsc = &cat->dsc[i];
> +
> +		hw = dpu_hw_dsc_init(dsc->id, mmio, cat);
> +		if (IS_ERR_OR_NULL(hw)) {
> +			rc = PTR_ERR(hw);
> +			DPU_ERROR("failed dsc object creation: err %d\n", rc);
> +			goto fail;
> +		}
> +		rm->dsc_blks[dsc->id - DSC_0] = &hw->base;
> +	}
> +
>   	return 0;
>   
>   fail:
> @@ -476,6 +498,9 @@ static int _dpu_rm_reserve_intf(
>   	}
>   
>   	global_state->intf_to_enc_id[idx] = enc_id;
> +
> +	global_state->dsc_to_enc_id[0] = enc_id;
> +	global_state->dsc_to_enc_id[1] = enc_id;

This is not correct. At least this should be guarded with an if, 
checking that DSC is requested. Also we'd need to check that DSC 0 and 1 
are not allocated.

>   	return 0;
>   }
>   
> @@ -567,6 +592,8 @@ void dpu_rm_release(struct dpu_global_state *global_state,
>   		ARRAY_SIZE(global_state->ctl_to_enc_id), enc->base.id);
>   	_dpu_rm_clear_mapping(global_state->intf_to_enc_id,
>   		ARRAY_SIZE(global_state->intf_to_enc_id), enc->base.id);
> +	_dpu_rm_clear_mapping(global_state->dsc_to_enc_id,
> +		ARRAY_SIZE(global_state->dsc_to_enc_id), enc->base.id);
>   }
>   
>   int dpu_rm_reserve(
> @@ -640,6 +667,11 @@ int dpu_rm_get_assigned_resources(struct dpu_rm *rm,
>   		hw_to_enc_id = global_state->dspp_to_enc_id;
>   		max_blks = ARRAY_SIZE(rm->dspp_blks);
>   		break;
> +	case DPU_HW_BLK_DSC:
> +		hw_blks = rm->dsc_blks;
> +		hw_to_enc_id = global_state->dsc_to_enc_id;
> +		max_blks = ARRAY_SIZE(rm->dsc_blks);
> +		break;
>   	default:
>   		DPU_ERROR("blk type %d not managed by rm\n", type);
>   		return 0;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> index 1f12c8d5b8aa..278d2a510b80 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> @@ -30,6 +30,7 @@ struct dpu_rm {
>   	struct dpu_hw_blk *intf_blks[INTF_MAX - INTF_0];
>   	struct dpu_hw_blk *dspp_blks[DSPP_MAX - DSPP_0];
>   	struct dpu_hw_blk *merge_3d_blks[MERGE_3D_MAX - MERGE_3D_0];
> +	struct dpu_hw_blk *dsc_blks[DSC_MAX - DSC_0];
>   
>   	uint32_t lm_max_width;
>   };
> 


-- 
With best wishes
Dmitry

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

* Re: [PATCH 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog
  2021-07-15  6:51 ` [PATCH 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog Vinod Koul
@ 2021-07-29 20:25   ` Dmitry Baryshkov
  2021-10-06 10:50     ` Vinod Koul
  2021-08-02 23:29   ` [Freedreno] " abhinavk
  1 sibling, 1 reply; 41+ messages in thread
From: Dmitry Baryshkov @ 2021-07-29 20:25 UTC (permalink / raw)
  To: Vinod Koul, Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, David Airlie, Daniel Vetter,
	Jonathan Marek, Abhinav Kumar, Jeffrey Hugo, Sumit Semwal,
	linux-kernel, dri-devel, freedreno

On 15/07/2021 09:51, Vinod Koul wrote:
> This add SDM845 DSC blocks into hw_catalog
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
> Changes since RFC:
>   - use BIT values from MASK
> 
>   .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    | 22 +++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> 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 b569030a0847..b45a08303c99 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -40,6 +40,8 @@
>   
>   #define PINGPONG_SDM845_MASK BIT(DPU_PINGPONG_DITHER)
>   
> +#define DSC_SDM845_MASK BIT(1)
> +

This does not seem used. You can pass (0) as the feature mask.

>   #define PINGPONG_SDM845_SPLIT_MASK \
>   	(PINGPONG_SDM845_MASK | BIT(DPU_PINGPONG_TE2))
>   
> @@ -751,6 +753,24 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = {
>   	PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, 0, sc7280_pp_sblk),
>   	PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk),
>   };
> +
> +/*************************************************************
> + * DSC sub blocks config
> + *************************************************************/
> +#define DSC_BLK(_name, _id, _base) \
> +	{\
> +	.name = _name, .id = _id, \
> +	.base = _base, .len = 0x140, \
> +	.features = DSC_SDM845_MASK, \
> +	}
> +
> +static struct dpu_dsc_cfg sdm845_dsc[] = {
> +	DSC_BLK("dsc_0", DSC_0, 0x80000),
> +	DSC_BLK("dsc_1", DSC_1, 0x80400),
> +	DSC_BLK("dsc_2", DSC_2, 0x80800),
> +	DSC_BLK("dsc_3", DSC_3, 0x80c00),
> +};
> +
>   /*************************************************************
>    * INTF sub blocks config
>    *************************************************************/
> @@ -1053,6 +1073,8 @@ static void sdm845_cfg_init(struct dpu_mdss_cfg *dpu_cfg)
>   		.mixer = sdm845_lm,
>   		.pingpong_count = ARRAY_SIZE(sdm845_pp),
>   		.pingpong = sdm845_pp,
> +		.dsc_count = ARRAY_SIZE(sdm845_dsc),
> +		.dsc = sdm845_dsc,
>   		.intf_count = ARRAY_SIZE(sdm845_intf),
>   		.intf = sdm845_intf,
>   		.vbif_count = ARRAY_SIZE(sdm845_vbif),
> 


-- 
With best wishes
Dmitry

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

* Re: [PATCH 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder
  2021-07-15  6:52 ` [PATCH 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder Vinod Koul
  2021-07-19  8:54   ` kernel test robot
@ 2021-07-29 20:54   ` Dmitry Baryshkov
  2021-10-06 12:43     ` Vinod Koul
  2021-08-03  0:57   ` [Freedreno] " abhinavk
  2 siblings, 1 reply; 41+ messages in thread
From: Dmitry Baryshkov @ 2021-07-29 20:54 UTC (permalink / raw)
  To: Vinod Koul, Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, David Airlie, Daniel Vetter,
	Jonathan Marek, Abhinav Kumar, Jeffrey Hugo, Sumit Semwal,
	linux-kernel, dri-devel, freedreno

On 15/07/2021 09:52, Vinod Koul wrote:
> When DSC is enabled in DT, we need to configure the encoder for DSC
> configuration, calculate DSC parameters for the given timing.
> 
> This patch adds that support by adding dpu_encoder_prep_dsc() which is
> invoked when DSC is enabled in DT
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 142 +++++++++++++++++++-
>   1 file changed, 141 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 8d942052db8a..41140b781e66 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -21,12 +21,17 @@
>   #include "dpu_hw_intf.h"
>   #include "dpu_hw_ctl.h"
>   #include "dpu_hw_dspp.h"
> +#include "dpu_hw_dsc.h"
>   #include "dpu_formats.h"
>   #include "dpu_encoder_phys.h"
>   #include "dpu_crtc.h"
>   #include "dpu_trace.h"
>   #include "dpu_core_irq.h"
>   
> +#define DSC_MODE_SPLIT_PANEL		BIT(0)
> +#define DSC_MODE_MULTIPLEX		BIT(1)
> +#define DSC_MODE_VIDEO			BIT(2)

This should go into dpu_hw_dsc.h. Ah. They are already defined there and 
just redefined there. Remove the defines here.

It might be cleaner to add bool flags to struct msm_display_dsc_config 
and then calculate common mode in the dpu_hw_dsc_config().


> +
>   #define DPU_DEBUG_ENC(e, fmt, ...) DPU_DEBUG("enc%d " fmt,\
>   		(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
>   
> @@ -135,6 +140,7 @@ enum dpu_enc_rc_states {
>    * @cur_slave:		As above but for the slave encoder.
>    * @hw_pp:		Handle to the pingpong blocks used for the display. No.
>    *			pingpong blocks can be different than num_phys_encs.
> + * @hw_dsc		Handle to the DSC blocks used for the display.
>    * @intfs_swapped:	Whether or not the phys_enc interfaces have been swapped
>    *			for partial update right-only cases, such as pingpong
>    *			split where virtual pingpong does not generate IRQs
> @@ -180,6 +186,7 @@ struct dpu_encoder_virt {
>   	struct dpu_encoder_phys *cur_master;
>   	struct dpu_encoder_phys *cur_slave;
>   	struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
> +	struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
>   
>   	bool intfs_swapped;
>   
> @@ -1008,7 +1015,8 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>   	struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
>   	struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
>   	struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC] = { NULL };
> -	int num_lm, num_ctl, num_pp;
> +	struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
> +	int num_lm, num_ctl, num_pp, num_dsc;
>   	int i, j;
>   
>   	if (!drm_enc) {
> @@ -1061,11 +1069,16 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>   	dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
>   		drm_enc->base.id, DPU_HW_BLK_DSPP, hw_dspp,
>   		ARRAY_SIZE(hw_dspp));
> +	num_dsc = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> +		drm_enc->base.id, DPU_HW_BLK_DSC, hw_dsc, ARRAY_SIZE(hw_dsc));
>   
>   	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
>   		dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i])
>   						: NULL;
>   
> +	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
> +		dpu_enc->hw_dsc[i] = i < num_dsc ? to_dpu_hw_dsc(hw_dsc[i]) : NULL;
> +
>   	cstate = to_dpu_crtc_state(drm_crtc->state);
>   
>   	for (i = 0; i < num_lm; i++) {
> @@ -1810,10 +1823,133 @@ static void dpu_encoder_vsync_event_work_handler(struct kthread_work *work)
>   			nsecs_to_jiffies(ktime_to_ns(wakeup_time)));
>   }
>   
> +static void
> +dpu_encoder_dsc_pclk_param_calc(struct msm_display_dsc_config *dsc, u32 width)
> +{
> +	int slice_count, slice_per_intf;
> +	int bytes_in_slice, total_bytes_per_intf;
> +
> +	if (!dsc || !dsc->drm->slice_width || !dsc->drm->slice_count) {
> +		DPU_ERROR("Invalid DSC/slices\n");
> +		return;
> +	}
> +
> +	slice_count = dsc->drm->slice_count;
> +	slice_per_intf = DIV_ROUND_UP(width, dsc->drm->slice_width);
> +
> +	/*
> +	 * If slice_count is greater than slice_per_intf then default to 1.
> +	 * This can happen during partial update.
> +	 */
> +	if (slice_count > slice_per_intf)
> +		slice_count = 1;
> +
> +	bytes_in_slice = DIV_ROUND_UP(dsc->drm->slice_width *
> +				      dsc->drm->bits_per_pixel, 8);
> +	total_bytes_per_intf = bytes_in_slice * slice_per_intf;
> +
> +	dsc->eol_byte_num = total_bytes_per_intf % 3;
> +	dsc->pclk_per_line =  DIV_ROUND_UP(total_bytes_per_intf, 3);
> +	dsc->bytes_in_slice = bytes_in_slice;
> +	dsc->bytes_per_pkt = bytes_in_slice * slice_count;
> +	dsc->pkt_per_line = slice_per_intf / slice_count;
> +}
> +
> +static void
> +dpu_encoder_dsc_initial_line_calc(struct msm_display_dsc_config *dsc,
> +				  u32 enc_ip_width)
> +{
> +	int ssm_delay, total_pixels, soft_slice_per_enc;
> +
> +	soft_slice_per_enc = enc_ip_width / dsc->drm->slice_width;
> +
> +	/*
> +	 * minimum number of initial line pixels is a sum of:
> +	 * 1. sub-stream multiplexer delay (83 groups for 8bpc,
> +	 *    91 for 10 bpc) * 3
> +	 * 2. for two soft slice cases, add extra sub-stream multiplexer * 3
> +	 * 3. the initial xmit delay
> +	 * 4. total pipeline delay through the "lock step" of encoder (47)
> +	 * 5. 6 additional pixels as the output of the rate buffer is
> +	 *    48 bits wide
> +	 */
> +	ssm_delay = ((dsc->drm->bits_per_component < 10) ? 84 : 92);
> +	total_pixels = ssm_delay * 3 + dsc->drm->initial_xmit_delay + 47;
> +	if (soft_slice_per_enc > 1)
> +		total_pixels += (ssm_delay * 3);
> +	dsc->initial_lines = DIV_ROUND_UP(total_pixels, dsc->drm->slice_width);
> +}
> +
> +static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
> +				     struct dpu_hw_pingpong *hw_pp,
> +				     struct msm_display_dsc_config *dsc,
> +				     u32 common_mode)
> +{
> +	if (hw_dsc->ops.dsc_config)
> +		hw_dsc->ops.dsc_config(hw_dsc, dsc, common_mode);
> +
> +	if (hw_dsc->ops.dsc_config_thresh)
> +		hw_dsc->ops.dsc_config_thresh(hw_dsc, dsc);
> +
> +	if (hw_pp->ops.setup_dsc)
> +		hw_pp->ops.setup_dsc(hw_pp);
> +
> +	if (hw_pp->ops.enable_dsc)
> +		hw_pp->ops.enable_dsc(hw_pp);

I think, we do not need to split these operations, I'd suggest having 
just hw_dsc->ops.dsc_config() and hw_pp->ops.enable_dsc(), merging 
dsc_config_thres() and setup_dsc() into respective methods.

> +}
> +
> +static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> +				 struct msm_display_dsc_config *dsc)
> +{
> +	/* coding only for 2LM, 2enc, 1 dsc config */
> +	struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
> +	struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
> +	struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
> +	int this_frame_slices;
> +	int intf_ip_w, enc_ip_w;
> +	int dsc_common_mode;
> +	int pic_width, pic_height;
> +	int i;
> +
> +	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
> +		hw_pp[i] = dpu_enc->hw_pp[i];
> +		hw_dsc[i] = dpu_enc->hw_dsc[i];
> +
> +		if (!hw_pp[i] || !hw_dsc[i]) {
> +			DPU_ERROR_ENC(dpu_enc, "invalid params for DSC\n");
> +			return;
> +		}
> +	}
> +
> +	dsc_common_mode = 0;
> +	pic_width = dsc->drm->pic_width;
> +	pic_height = dsc->drm->pic_height;
> +
> +	dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL;
> +	if (enc_master->intf_mode == INTF_MODE_VIDEO)
> +		dsc_common_mode |= DSC_MODE_VIDEO;
> +
> +	this_frame_slices = pic_width / dsc->drm->slice_width;
> +	intf_ip_w = this_frame_slices * dsc->drm->slice_width;
> +
> +	dpu_encoder_dsc_pclk_param_calc(dsc, intf_ip_w);
> +
> +	/*
> +	 * dsc merge case: when using 2 encoders for the same stream,
> +	 * no. of slices need to be same on both the encoders.
> +	 */
> +	enc_ip_w = intf_ip_w / 2;
> +	dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
> +
> +	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
> +		dpu_encoder_dsc_pipe_cfg(hw_dsc[i], hw_pp[i], dsc, dsc_common_mode);
> +}
> +
>   void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
>   {
>   	struct dpu_encoder_virt *dpu_enc;
>   	struct dpu_encoder_phys *phys;
> +	struct msm_drm_private *priv;
>   	bool needs_hw_reset = false;
>   	unsigned int i;
>   
> @@ -1841,6 +1977,10 @@ void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
>   			dpu_encoder_helper_hw_reset(dpu_enc->phys_encs[i]);
>   		}
>   	}
> +
> +	priv = drm_enc->dev->dev_private;
> +	if (priv->dsc)
> +		dpu_encoder_prep_dsc(dpu_enc, priv->dsc);

Not quite. This makes dsc config global, while we can have several 
encoders enabled at once (think of DSI + DP). So the dsc should be a 
per-encoder setting rather than global.

>   }
>   
>   void dpu_encoder_kickoff(struct drm_encoder *drm_enc)
> 


-- 
With best wishes
Dmitry

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

* Re: [PATCH 10/11] drm/msm/dsi: Add support for DSC configuration
  2021-07-15  6:52 ` [PATCH 10/11] drm/msm/dsi: Add support for DSC configuration Vinod Koul
@ 2021-07-29 22:10   ` Dmitry Baryshkov
  2021-08-03  1:16   ` [Freedreno] " abhinavk
  1 sibling, 0 replies; 41+ messages in thread
From: Dmitry Baryshkov @ 2021-07-29 22:10 UTC (permalink / raw)
  To: Vinod Koul, Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, David Airlie, Daniel Vetter,
	Jonathan Marek, Abhinav Kumar, Jeffrey Hugo, Sumit Semwal,
	linux-kernel, dri-devel, freedreno

On 15/07/2021 09:52, Vinod Koul wrote:
> When DSC is enabled, we need to configure DSI registers accordingly and
> configure the respective stream compression registers.
> 
> Add support to calculate the register setting based on DSC params and
> timing information and configure these registers.
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>   drivers/gpu/drm/msm/dsi/dsi.xml.h  |  10 ++
>   drivers/gpu/drm/msm/dsi/dsi_host.c | 142 +++++++++++++++++++++++++++--
>   2 files changed, 142 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.xml.h b/drivers/gpu/drm/msm/dsi/dsi.xml.h
> index 50eb4d1b8fdd..b8e9e608abfc 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.xml.h
> +++ b/drivers/gpu/drm/msm/dsi/dsi.xml.h
> @@ -2310,4 +2310,14 @@ static inline uint32_t REG_DSI_7nm_PHY_LN_TX_DCTRL(uint32_t i0) { return 0x00000
>   
>   #define REG_DSI_7nm_PHY_PLL_PERF_OPTIMIZE			0x00000260
>   
> +#define REG_DSI_VIDEO_COMPRESSION_MODE_CTRL			0x0000029c
> +
> +#define REG_DSI_VIDEO_COMPRESSION_MODE_CTRL2			0x000002a0
> +
> +#define REG_DSI_COMMAND_COMPRESSION_MODE_CTRL			0x000002a4
> +
> +#define REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2			0x000002a8
> +
> +#define REG_DSI_COMMAND_COMPRESSION_MODE_CTRL3			0x000002ac
> +
>   #endif /* DSI_XML */


Could you please post the patch to mesa3d to add these registers?

> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index e1e5d91809b5..4e8ab1b1df8b 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -942,6 +942,26 @@ static void dsi_ctrl_config(struct msm_dsi_host *msm_host, bool enable,
>   	dsi_write(msm_host, REG_DSI_CTRL, data);
>   }
>   
> +static int dsi_dsc_update_pic_dim(struct msm_display_dsc_config *dsc,
> +				  int pic_width, int pic_height)
> +{
> +	if (!dsc || !pic_width || !pic_height) {
> +		pr_err("DSI: invalid input: pic_width: %d pic_height: %d\n", pic_width, pic_height);
> +		return -EINVAL;
> +	}
> +
> +	if ((pic_width % dsc->drm->slice_width) || (pic_height % dsc->drm->slice_height)) {
> +		pr_err("DSI: pic_dim %dx%d has to be multiple of slice %dx%d\n",
> +		       pic_width, pic_height, dsc->drm->slice_width, dsc->drm->slice_height);
> +		return -EINVAL;
> +	}
> +
> +	dsc->drm->pic_width = pic_width;
> +	dsc->drm->pic_height = pic_height;
> +
> +	return 0;
> +}
> +
>   static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_dual_dsi)
>   {
>   	struct drm_display_mode *mode = msm_host->mode;
> @@ -956,6 +976,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_dual_dsi)
>   	u32 va_end = va_start + mode->vdisplay;
>   	u32 hdisplay = mode->hdisplay;
>   	u32 wc;
> +	u32 data;
>   
>   	DBG("");
>   
> @@ -974,7 +995,73 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_dual_dsi)
>   		hdisplay /= 2;
>   	}
>   
> +	if (msm_host->dsc) {
> +		struct msm_display_dsc_config *dsc = msm_host->dsc;
> +
> +		/* update dsc params with timing params */
> +		dsi_dsc_update_pic_dim(dsc, mode->hdisplay, mode->vdisplay);
> +		DBG("Mode Width- %d x Height %d\n", dsc->drm->pic_width, dsc->drm->pic_height);
> +
> +		/* we do the calculations for dsc parameters here so that
> +		 * panel can use these parameters
> +		 */
> +		dsi_populate_dsc_params(dsc);
> +
> +		/* Divide the display by 3 but keep back/font porch and
> +		 * pulse width same
> +		 */
> +		h_total -= hdisplay;
> +		hdisplay /= 3;
> +		h_total += hdisplay;
> +		ha_end = ha_start + hdisplay;
> +	}
> +
>   	if (msm_host->mode_flags & MIPI_DSI_MODE_VIDEO) {
> +		if (msm_host->dsc) {
> +			struct msm_display_dsc_config *dsc = msm_host->dsc;
> +			u32 reg, intf_width, slice_per_intf, width;
> +			u32 total_bytes_per_intf;
> +
> +			/* first calculate dsc parameters and then program
> +			 * compress mode registers
> +			 */
> +			intf_width = hdisplay;
> +			slice_per_intf = DIV_ROUND_UP(intf_width, dsc->drm->slice_width);
> +
> +			/* If slice_count > slice_per_intf, then use 1
> +			 * This can happen during partial update
> +			 */
> +				dsc->drm->slice_count = 1;
> +
> +			dsc->bytes_in_slice = DIV_ROUND_UP(dsc->drm->slice_width * 8, 8);
> +			total_bytes_per_intf = dsc->bytes_in_slice * slice_per_intf;
> +
> +			dsc->eol_byte_num = total_bytes_per_intf % 3;
> +			dsc->pclk_per_line =  DIV_ROUND_UP(total_bytes_per_intf, 3);
> +			dsc->bytes_per_pkt = dsc->bytes_in_slice * dsc->drm->slice_count;
> +			dsc->pkt_per_line = slice_per_intf / dsc->drm->slice_count;
> +
> +			width = dsc->pclk_per_line;
> +			reg = dsc->bytes_per_pkt << 16;
> +			reg |= (0x0b << 8);    /* dtype of compressed image */
> +
> +			/* pkt_per_line:
> +			 * 0 == 1 pkt
> +			 * 1 == 2 pkt
> +			 * 2 == 4 pkt
> +			 * 3 pkt is not supported
> +			 * above translates to ffs() - 1
> +			 */
> +			reg |= (ffs(dsc->pkt_per_line) - 1) << 6;
> +
> +			dsc->eol_byte_num = total_bytes_per_intf % 3;
> +			reg |= dsc->eol_byte_num << 4;
> +			reg |= 1;
> +
> +			dsi_write(msm_host,
> +				  REG_DSI_VIDEO_COMPRESSION_MODE_CTRL, reg);
> +		}
> +
>   		dsi_write(msm_host, REG_DSI_ACTIVE_H,
>   			DSI_ACTIVE_H_START(ha_start) |
>   			DSI_ACTIVE_H_END(ha_end));
> @@ -993,19 +1080,50 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_dual_dsi)
>   			DSI_ACTIVE_VSYNC_VPOS_START(vs_start) |
>   			DSI_ACTIVE_VSYNC_VPOS_END(vs_end));
>   	} else {		/* command mode */
> +		if (msm_host->dsc) {
> +			struct msm_display_dsc_config *dsc = msm_host->dsc;
> +			u32 reg, reg_ctrl, reg_ctrl2;
> +			u32 slice_per_intf, bytes_in_slice, total_bytes_per_intf;
> +
> +			reg_ctrl = dsi_read(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL);
> +			reg_ctrl2 = dsi_read(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2);
> +
> +			slice_per_intf = DIV_ROUND_UP(hdisplay, dsc->drm->slice_width);
> +			bytes_in_slice = DIV_ROUND_UP(dsc->drm->slice_width *
> +						      dsc->drm->bits_per_pixel, 8);
> +			dsc->drm->slice_chunk_size = bytes_in_slice;
> +			total_bytes_per_intf = dsc->bytes_in_slice * slice_per_intf;
> +			dsc->pkt_per_line = slice_per_intf / dsc->drm->slice_count;
> +
> +			reg = 0x39 << 8;
> +			reg |= ffs(dsc->pkt_per_line) << 6;
> +
> +			dsc->eol_byte_num = total_bytes_per_intf % 3;
> +			reg |= dsc->eol_byte_num << 4;
> +			reg |= 1;
> +
> +			reg_ctrl |= reg;
> +			reg_ctrl2 |= bytes_in_slice;
> +
> +			dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL, reg);
> +			dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2, reg_ctrl2);
> +		}
> +
>   		/* image data and 1 byte write_memory_start cmd */
> -		wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
> +		if (!msm_host->dsc)
> +			wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
> +		else
> +			wc = mode->hdisplay / 2 + 1;
>   
> -		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL,
> -			DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
> -			DSI_CMD_MDP_STREAM0_CTRL_VIRTUAL_CHANNEL(
> -					msm_host->channel) |
> -			DSI_CMD_MDP_STREAM0_CTRL_DATA_TYPE(
> -					MIPI_DSI_DCS_LONG_WRITE));
> +		data = DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
> +		       DSI_CMD_MDP_STREAM0_CTRL_VIRTUAL_CHANNEL(msm_host->channel) |
> +			DSI_CMD_MDP_STREAM0_CTRL_DATA_TYPE(MIPI_DSI_DCS_LONG_WRITE);
>   
> -		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_TOTAL,
> -			DSI_CMD_MDP_STREAM0_TOTAL_H_TOTAL(hdisplay) |
> -			DSI_CMD_MDP_STREAM0_TOTAL_V_TOTAL(mode->vdisplay));
> +		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL, data);
> +
> +		data = DSI_CMD_MDP_STREAM0_TOTAL_H_TOTAL(hdisplay) |
> +			DSI_CMD_MDP_STREAM0_TOTAL_V_TOTAL(mode->vdisplay);
> +		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_TOTAL, data);

Could you please separate this cleanup away.

>   	}
>   }
>   
> @@ -2074,6 +2192,7 @@ int msm_dsi_host_modeset_init(struct mipi_dsi_host *host,
>   	struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
>   	const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
>   	struct platform_device *pdev = msm_host->pdev;
> +	struct msm_drm_private *priv;
>   	int ret;
>   
>   	msm_host->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> @@ -2093,6 +2212,9 @@ int msm_dsi_host_modeset_init(struct mipi_dsi_host *host,
>   	}
>   
>   	msm_host->dev = dev;
> +	priv = dev->dev_private;
> +	priv->dsc = msm_host->dsc;

I'd prefer not to push dsc config into msm_drm_private and to get it as 
necessary using msm_dsi function calls.

> +
>   	ret = cfg_hnd->ops->tx_buf_alloc(msm_host, SZ_4K);
>   	if (ret) {
>   		pr_err("%s: alloc tx gem obj failed, %d\n", __func__, ret);
> 


-- 
With best wishes
Dmitry

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

* Re: [PATCH 06/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl
  2021-07-15  6:51 ` [PATCH 06/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl Vinod Koul
@ 2021-07-29 22:15   ` Dmitry Baryshkov
  2021-10-06 12:21     ` Vinod Koul
  2021-08-03  0:00   ` [Freedreno] " abhinavk
  1 sibling, 1 reply; 41+ messages in thread
From: Dmitry Baryshkov @ 2021-07-29 22:15 UTC (permalink / raw)
  To: Vinod Koul, Rob Clark
  Cc: linux-arm-msm, Bjorn Andersson, David Airlie, Daniel Vetter,
	Jonathan Marek, Abhinav Kumar, Jeffrey Hugo, Sumit Semwal,
	linux-kernel, dri-devel, freedreno

On 15/07/2021 09:51, Vinod Koul wrote:
> Later gens of hardware have DSC bits moved to hw_ctl, so configure these
> bits so that DSC would work there as well
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index 2d4645e01ebf..aeea6add61ee 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -25,6 +25,8 @@
>   #define   CTL_MERGE_3D_ACTIVE           0x0E4
>   #define   CTL_INTF_ACTIVE               0x0F4
>   #define   CTL_MERGE_3D_FLUSH            0x100
> +#define   CTL_DSC_ACTIVE                0x0E8
> +#define   CTL_DSC_FLUSH                0x104
>   #define   CTL_INTF_FLUSH                0x110
>   #define   CTL_INTF_MASTER               0x134
>   #define   CTL_FETCH_PIPE_ACTIVE         0x0FC
> @@ -34,6 +36,7 @@
>   
>   #define DPU_REG_RESET_TIMEOUT_US        2000
>   #define  MERGE_3D_IDX   23
> +#define  DSC_IDX        22
>   #define  INTF_IDX       31
>   #define CTL_INVALID_BIT                 0xffff
>   
> @@ -120,6 +123,7 @@ static u32 dpu_hw_ctl_get_pending_flush(struct dpu_hw_ctl *ctx)
>   
>   static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
>   {
> +	DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | BIT(3));

Please pass DSC indices using intf cfg and use them to configure 
register writes.

>   
>   	if (ctx->pending_flush_mask & BIT(MERGE_3D_IDX))
>   		DPU_REG_WRITE(&ctx->hw, CTL_MERGE_3D_FLUSH,
> @@ -128,7 +132,7 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
>   		DPU_REG_WRITE(&ctx->hw, CTL_INTF_FLUSH,
>   				ctx->pending_intf_flush_mask);
>   
> -	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
> +	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask |  BIT(DSC_IDX));

Only if DSCs are used

>   }
>   
>   static inline void dpu_hw_ctl_trigger_flush(struct dpu_hw_ctl *ctx)
> @@ -507,6 +511,7 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
>   	if (cfg->merge_3d)
>   		DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
>   			      BIT(cfg->merge_3d - MERGE_3D_0));
> +	DPU_REG_WRITE(c, CTL_DSC_ACTIVE, BIT(0) | BIT(1) | BIT(2) | BIT(3));

And here

>   }
>   
>   static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
> 


-- 
With best wishes
Dmitry

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

* Re: [Freedreno] [PATCH 01/11] drm/msm/dsi: add support for dsc data
  2021-07-15  6:51 ` [PATCH 01/11] drm/msm/dsi: add support for dsc data Vinod Koul
@ 2021-08-02 22:55   ` abhinavk
  2021-10-06  5:24     ` Vinod Koul
  0 siblings, 1 reply; 41+ messages in thread
From: abhinavk @ 2021-08-02 22:55 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 2021-07-14 23:51, Vinod Koul wrote:
> Display Stream Compression (DSC) parameters need to be calculated. Add
> helpers and struct msm_display_dsc_config in msm_drv for this
> msm_display_dsc_config uses drm_dsc_config for DSC parameters.
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
> Changes since RFC:
>  - Drop the DT parsing code
>  - Port dsc param calculation from downstream
> 
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 133 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/msm/msm_drv.h      |  21 +++++
>  2 files changed, 154 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 8a10e4343281..e1e5d91809b5 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -30,6 +30,8 @@
> 
>  #define DSI_RESET_TOGGLE_DELAY_MS 20
> 
> +static int dsi_populate_dsc_params(struct msm_display_dsc_config 
> *dsc);
> +
>  static int dsi_get_version(const void __iomem *base, u32 *major, u32 
> *minor)
>  {
>  	u32 ver;
> @@ -156,6 +158,7 @@ struct msm_dsi_host {
>  	struct regmap *sfpb;
> 
>  	struct drm_display_mode *mode;
> +	struct msm_display_dsc_config *dsc;
> 
>  	/* connected device info */
>  	struct device_node *device_node;
> @@ -1744,6 +1747,136 @@ static int dsi_host_parse_lane_data(struct
> msm_dsi_host *msm_host,
>  	return -EINVAL;
>  }
> 
> +static u32 dsi_dsc_rc_buf_thresh[DSC_NUM_BUF_RANGES - 1] = {
> +	0x0e, 0x1c, 0x2a, 0x38, 0x46, 0x54, 0x62,
> +	0x69, 0x70, 0x77, 0x79, 0x7b, 0x7d, 0x7e
> +};
> +
> +/* only 8bpc, 8bpp added */
> +static char min_qp[DSC_NUM_BUF_RANGES] = {
> +	0, 0, 1, 1, 3, 3, 3, 3, 3, 3, 5, 5, 5, 7, 13
> +};
> +
> +static char max_qp[DSC_NUM_BUF_RANGES] = {
> +	4, 4, 5, 6, 7, 7, 7, 8, 9, 10, 11, 12, 13, 13, 15
> +};
> +
> +static char bpg_offset[DSC_NUM_BUF_RANGES] = {
> +	2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12
> +};
> +
> +static int dsi_populate_dsc_params(struct msm_display_dsc_config *dsc)
> +{
> +	int mux_words_size;
> +	int groups_per_line, groups_total;
> +	int min_rate_buffer_size;
> +	int hrd_delay;
> +	int pre_num_extra_mux_bits, num_extra_mux_bits;
> +	int slice_bits;
> +	int target_bpp_x16;
> +	int data;
> +	int final_value, final_scale;
> +	int i;
> +
> +	dsc->drm->rc_model_size = 8192;
> +	dsc->drm->first_line_bpg_offset = 12;
> +	dsc->drm->rc_edge_factor = 6;
> +	dsc->drm->rc_tgt_offset_high = 3;
> +	dsc->drm->rc_tgt_offset_low = 3;
> +	dsc->drm->simple_422 = 0;
> +	dsc->drm->convert_rgb = 1;
> +	dsc->drm->vbr_enable = 0;
> +
> +	/* handle only bpp = bpc = 8 */
> +	for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++)
> +		dsc->drm->rc_buf_thresh[i] = dsi_dsc_rc_buf_thresh[i];
> +
> +	for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
> +		dsc->drm->rc_range_params[i].range_min_qp = min_qp[i];
> +		dsc->drm->rc_range_params[i].range_max_qp = max_qp[i];
> +		dsc->drm->rc_range_params[i].range_bpg_offset = bpg_offset[i];
> +	}
> +
> +	dsc->drm->initial_offset = 6144; /* Not bpp 12 */
> +	if (dsc->drm->bits_per_pixel != 8)
> +		dsc->drm->initial_offset = 2048;	/* bpp = 12 */
> +
> +	mux_words_size = 48;		/* bpc == 8/10 */
> +	if (dsc->drm->bits_per_component == 12)
> +		mux_words_size = 64;
> +
> +	dsc->drm->initial_xmit_delay = 512;
> +	dsc->drm->initial_scale_value = 32;
> +	dsc->drm->first_line_bpg_offset = 12;
> +	dsc->drm->line_buf_depth = dsc->drm->bits_per_component + 1;
> +
> +	/* bpc 8 */
> +	dsc->drm->flatness_min_qp = 3;
> +	dsc->drm->flatness_max_qp = 12;
> +	dsc->det_thresh_flatness = 7 + 2 * (dsc->drm->bits_per_component - 
> 8);
> +	dsc->drm->rc_quant_incr_limit0 = 11;
> +	dsc->drm->rc_quant_incr_limit1 = 11;
> +	dsc->drm->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC;
> +
> +	/* FIXME: need to call drm_dsc_compute_rc_parameters() so that rest 
> of
> +	 * params are calculated
> +	 */
> +	dsc->slice_last_group_size = 3 - (dsc->drm->slice_width % 3);
> +	groups_per_line = DIV_ROUND_UP(dsc->drm->slice_width, 3);
> +	dsc->drm->slice_chunk_size = dsc->drm->slice_width *
> dsc->drm->bits_per_pixel / 8;
> +	if ((dsc->drm->slice_width * dsc->drm->bits_per_pixel) % 8)
> +		dsc->drm->slice_chunk_size++;
> +
> +	/* rbs-min */
> +	min_rate_buffer_size =  dsc->drm->rc_model_size - 
> dsc->drm->initial_offset +
> +				dsc->drm->initial_xmit_delay * dsc->drm->bits_per_pixel +
> +				groups_per_line * dsc->drm->first_line_bpg_offset;
> +
> +	hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, 
> dsc->drm->bits_per_pixel);
> +
> +	dsc->drm->initial_dec_delay = hrd_delay - 
> dsc->drm->initial_xmit_delay;
> +
> +	dsc->drm->initial_scale_value = 8 * dsc->drm->rc_model_size /
> +				       (dsc->drm->rc_model_size - dsc->drm->initial_offset);
> +
> +	slice_bits = 8 * dsc->drm->slice_chunk_size * dsc->drm->slice_height;
> +
> +	groups_total = groups_per_line * dsc->drm->slice_height;
> +
> +	data = dsc->drm->first_line_bpg_offset * 2048;
> +
> +	dsc->drm->nfl_bpg_offset = DIV_ROUND_UP(data, (dsc->drm->slice_height 
> - 1));
> +
> +	pre_num_extra_mux_bits = 3 * (mux_words_size + (4 *
> dsc->drm->bits_per_component + 4) - 2);
> +
> +	num_extra_mux_bits = pre_num_extra_mux_bits - (mux_words_size -
> +			     ((slice_bits - pre_num_extra_mux_bits) % mux_words_size));
> +
> +	data = 2048 * (dsc->drm->rc_model_size - dsc->drm->initial_offset +
> num_extra_mux_bits);
> +	dsc->drm->slice_bpg_offset = DIV_ROUND_UP(data, groups_total);
> +
> +	/* bpp * 16 + 0.5 */
> +	data = dsc->drm->bits_per_pixel * 16;
> +	data *= 2;
> +	data++;
> +	data /= 2;
> +	target_bpp_x16 = data;
> +
> +	data = (dsc->drm->initial_xmit_delay * target_bpp_x16) / 16;
> +	final_value =  dsc->drm->rc_model_size - data + num_extra_mux_bits;
As we discussed, can you please check why there is an additional + 8 and 
/16 in the upstream final_offset calculation?
If we can eliminate or root-cause the difference in the calculations, 
either this patch can be substantially reduced or
we will atleast know for future reference what was the delta and can 
leave a comment.
> +	dsc->drm->final_offset = final_value;
> +
> +	final_scale = 8 * dsc->drm->rc_model_size / (dsc->drm->rc_model_size
> - final_value);
> +
> +
> +	data = (final_scale - 9) * (dsc->drm->nfl_bpg_offset +
> dsc->drm->slice_bpg_offset);
> +	dsc->drm->scale_increment_interval = (2048 * dsc->drm->final_offset) 
> / data;
> +
> +	dsc->drm->scale_decrement_interval = groups_per_line /
> (dsc->drm->initial_scale_value - 8);
> +
> +	return 0;
> +}
> +
>  static int dsi_host_parse_dt(struct msm_dsi_host *msm_host)
>  {
>  	struct device *dev = &msm_host->pdev->dev;
> diff --git a/drivers/gpu/drm/msm/msm_drv.h 
> b/drivers/gpu/drm/msm/msm_drv.h
> index 2668941df529..65bff0176b66 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -30,6 +30,7 @@
>  #include <drm/drm_plane_helper.h>
>  #include <drm/drm_probe_helper.h>
>  #include <drm/drm_fb_helper.h>
> +#include <drm/drm_dsc.h>
>  #include <drm/msm_drm.h>
>  #include <drm/drm_gem.h>
> 
> @@ -134,6 +135,23 @@ struct msm_drm_thread {
>  	struct kthread_worker *worker;
>  };
> 
> +/* DSC config */
> +struct msm_display_dsc_config {
> +	struct drm_dsc_config *drm;
> +	u8 scr_rev;
Can scr_rev also move into drm_dsc_config? SCR itself is not QC 
specific. Its just telling there was a change request
for that DSC revision.
In QC side, we only use this scr_rev to have some different tables. This 
can even be true for other vendors.
So moving this to drm_dsc_config will only help.
> +
> +	u32 initial_lines;
> +	u32 pkt_per_line;
> +	u32 bytes_in_slice;
> +	u32 bytes_per_pkt;
> +	u32 eol_byte_num;
> +	u32 pclk_per_line;
> +	u32 slice_last_group_size;
> +	u32 det_thresh_flatness;
> +	u32 extra_width;
> +	u32 pps_delay_ms;
> +};
> +
>  struct msm_drm_private {
> 
>  	struct drm_device *dev;
> @@ -227,6 +245,9 @@ struct msm_drm_private {
>  	/* Properties */
>  	struct drm_property *plane_property[PLANE_PROP_MAX_NUM];
> 
> +	/* DSC configuration */
> +	struct msm_display_dsc_config *dsc;
> +
>  	/* VRAM carveout, used when no IOMMU: */
>  	struct {
>  		unsigned long size;

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

* Re: [Freedreno] [PATCH 02/11] drm/msm/disp/dpu1: Add support for DSC
  2021-07-15  6:51 ` [PATCH 02/11] drm/msm/disp/dpu1: Add support for DSC Vinod Koul
  2021-07-19  8:28   ` kernel test robot
@ 2021-08-02 23:03   ` abhinavk
  2021-10-06  5:36     ` Vinod Koul
  1 sibling, 1 reply; 41+ messages in thread
From: abhinavk @ 2021-08-02 23:03 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 2021-07-14 23:51, Vinod Koul wrote:
> Display Stream Compression (DSC) is one of the hw blocks in dpu, so add
> support by adding hw blocks for DSC
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
> Changes since RFC:
>  - Drop unused enums
> 
>  drivers/gpu/drm/msm/Makefile                  |   1 +
>  .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |  13 ++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c    | 221 ++++++++++++++++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h    |  77 ++++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h   |  13 ++
>  5 files changed, 325 insertions(+)
>  create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
>  create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
> 
> diff --git a/drivers/gpu/drm/msm/Makefile 
> b/drivers/gpu/drm/msm/Makefile
> index 610d630326bb..fd8fc57f1f58 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -61,6 +61,7 @@ msm-y := \
>  	disp/dpu1/dpu_hw_blk.o \
>  	disp/dpu1/dpu_hw_catalog.o \
>  	disp/dpu1/dpu_hw_ctl.o \
> +	disp/dpu1/dpu_hw_dsc.o \
>  	disp/dpu1/dpu_hw_interrupts.o \
>  	disp/dpu1/dpu_hw_intf.o \
>  	disp/dpu1/dpu_hw_lm.o \
> 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 4dfd8a20ad5c..b8b4dc36880c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -547,6 +547,16 @@ struct dpu_merge_3d_cfg  {
>  	const struct dpu_merge_3d_sub_blks *sblk;
>  };
> 
> +/**
> + * struct dpu_dsc_cfg - information of DSC blocks
> + * @id                 enum identifying this block
> + * @base               register offset of this block
> + * @features           bit mask identifying sub-blocks/features
> + */
> +struct dpu_dsc_cfg {
> +	DPU_HW_BLK_INFO;
> +};
> +
>  /**
>   * struct dpu_intf_cfg - information of timing engine blocks
>   * @id                 enum identifying this block
> @@ -748,6 +758,9 @@ struct dpu_mdss_cfg {
>  	u32 merge_3d_count;
>  	const struct dpu_merge_3d_cfg *merge_3d;
> 
> +	u32 dsc_count;
> +	struct dpu_dsc_cfg *dsc;
> +
>  	u32 intf_count;
>  	const struct dpu_intf_cfg *intf;
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
> new file mode 100644
> index 000000000000..e27e67bd42e8
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
> @@ -0,0 +1,221 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2020, Linaro Limited
Copyright year needs an update : 2020-2021?
> + */
> +
> +#include "dpu_kms.h"
> +#include "dpu_hw_catalog.h"
> +#include "dpu_hwio.h"
> +#include "dpu_hw_mdss.h"
> +#include "dpu_hw_dsc.h"
> +
> +#define DSC_COMMON_MODE	                0x000
> +#define DSC_ENC                         0X004
> +#define DSC_PICTURE                     0x008
> +#define DSC_SLICE                       0x00C
> +#define DSC_CHUNK_SIZE                  0x010
> +#define DSC_DELAY                       0x014
> +#define DSC_SCALE_INITIAL               0x018
> +#define DSC_SCALE_DEC_INTERVAL          0x01C
> +#define DSC_SCALE_INC_INTERVAL          0x020
> +#define DSC_FIRST_LINE_BPG_OFFSET       0x024
> +#define DSC_BPG_OFFSET                  0x028
> +#define DSC_DSC_OFFSET                  0x02C
> +#define DSC_FLATNESS                    0x030
> +#define DSC_RC_MODEL_SIZE               0x034
> +#define DSC_RC                          0x038
> +#define DSC_RC_BUF_THRESH               0x03C
> +#define DSC_RANGE_MIN_QP                0x074
> +#define DSC_RANGE_MAX_QP                0x0B0
> +#define DSC_RANGE_BPG_OFFSET            0x0EC
> +
> +static void dpu_hw_dsc_disable(struct dpu_hw_dsc *dsc)
> +{
> +	struct dpu_hw_blk_reg_map *c = &dsc->hw;
> +
> +	DPU_REG_WRITE(c, DSC_COMMON_MODE, 0);
> +}
> +
> +static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc,
> +			      struct msm_display_dsc_config *dsc, u32 mode)
> +{
> +	struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
> +	u32 data, lsb, bpp;
> +	u32 initial_lines = dsc->initial_lines;
> +	bool is_cmd_mode = !(mode & BIT(2));
> +
> +	DPU_REG_WRITE(c, DSC_COMMON_MODE, mode);
> +
> +	if (is_cmd_mode)
> +		initial_lines += 1;
> +
> +	data = (initial_lines << 20);
> +	data |= ((dsc->slice_last_group_size - 1) << 18);
> +	/* bpp is 6.4 format, 4 LSBs bits are for fractional part */
> +	data |= dsc->drm->bits_per_pixel << 12;
> +	lsb = dsc->drm->bits_per_pixel % 4;
> +	bpp = dsc->drm->bits_per_pixel / 4;
> +	bpp *= 4;
> +	bpp <<= 4;
> +	bpp |= lsb;
> +
> +	data |= bpp << 8;
> +	data |= (dsc->drm->block_pred_enable << 7);
> +	data |= (dsc->drm->line_buf_depth << 3);
> +	data |= (dsc->drm->simple_422 << 2);
> +	data |= (dsc->drm->convert_rgb << 1);
> +	data |= dsc->drm->bits_per_component;
> +
> +	DPU_REG_WRITE(c, DSC_ENC, data);
> +
> +	data = dsc->drm->pic_width << 16;
> +	data |= dsc->drm->pic_height;
> +	DPU_REG_WRITE(c, DSC_PICTURE, data);
> +
> +	data = dsc->drm->slice_width << 16;
> +	data |= dsc->drm->slice_height;
> +	DPU_REG_WRITE(c, DSC_SLICE, data);
> +
> +	data = dsc->drm->slice_chunk_size << 16;
> +	DPU_REG_WRITE(c, DSC_CHUNK_SIZE, data);
> +
> +	data = dsc->drm->initial_dec_delay << 16;
> +	data |= dsc->drm->initial_xmit_delay;
> +	DPU_REG_WRITE(c, DSC_DELAY, data);
> +
> +	data = dsc->drm->initial_scale_value;
> +	DPU_REG_WRITE(c, DSC_SCALE_INITIAL, data);
> +
> +	data = dsc->drm->scale_decrement_interval;
> +	DPU_REG_WRITE(c, DSC_SCALE_DEC_INTERVAL, data);
> +
> +	data = dsc->drm->scale_increment_interval;
> +	DPU_REG_WRITE(c, DSC_SCALE_INC_INTERVAL, data);
> +
> +	data = dsc->drm->first_line_bpg_offset;
> +	DPU_REG_WRITE(c, DSC_FIRST_LINE_BPG_OFFSET, data);
> +
> +	data = dsc->drm->nfl_bpg_offset << 16;
> +	data |= dsc->drm->slice_bpg_offset;
> +	DPU_REG_WRITE(c, DSC_BPG_OFFSET, data);
> +
> +	data = dsc->drm->initial_offset << 16;
> +	data |= dsc->drm->final_offset;
> +	DPU_REG_WRITE(c, DSC_DSC_OFFSET, data);
> +
> +	data = dsc->det_thresh_flatness << 10;
> +	data |= dsc->drm->flatness_max_qp << 5;
> +	data |= dsc->drm->flatness_min_qp;
> +	DPU_REG_WRITE(c, DSC_FLATNESS, data);
> +
> +	data = dsc->drm->rc_model_size;
> +	DPU_REG_WRITE(c, DSC_RC_MODEL_SIZE, data);
> +
> +	data = dsc->drm->rc_tgt_offset_low << 18;
> +	data |= dsc->drm->rc_tgt_offset_high << 14;
> +	data |= dsc->drm->rc_quant_incr_limit1 << 9;
> +	data |= dsc->drm->rc_quant_incr_limit0 << 4;
> +	data |= dsc->drm->rc_edge_factor;
> +	DPU_REG_WRITE(c, DSC_RC, data);
> +}
> +
> +static void dpu_hw_dsc_config_thresh(struct dpu_hw_dsc *hw_dsc,
> +				     struct msm_display_dsc_config *dsc)
> +{
> +	struct drm_dsc_rc_range_parameters *rc = dsc->drm->rc_range_params;
> +	struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
> +	u32 off;
> +	u16 *lp;
> +	int i;
> +
> +	lp = dsc->drm->rc_buf_thresh;
> +	off = DSC_RC_BUF_THRESH;
> +	for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++) {
> +		DPU_REG_WRITE(c, off, dsc->drm->rc_buf_thresh[i]);
> +		off += 4;
> +	}
> +
> +	off = DSC_RANGE_MIN_QP;
> +	for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
> +		DPU_REG_WRITE(c, off, rc[i].range_min_qp);
> +		off += 4;
> +	}
> +
> +	off = DSC_RANGE_MAX_QP;
> +	for (i = 0; i < 15; i++) {
> +		DPU_REG_WRITE(c, off, rc[i].range_max_qp);
> +		off += 4;
> +	}
> +
> +	off = DSC_RANGE_BPG_OFFSET;
> +	for (i = 0; i < 15; i++) {
> +		DPU_REG_WRITE(c, off, rc[i].range_bpg_offset);
> +		off += 4;
> +	}
> +}
> +
> +static struct dpu_dsc_cfg *_dsc_offset(enum dpu_dsc dsc,
> +				       struct dpu_mdss_cfg *m,
> +				       void __iomem *addr,
> +				       struct dpu_hw_blk_reg_map *b)
> +{
> +	int i;
> +
> +	for (i = 0; i < m->dsc_count; i++) {
> +		if (dsc == m->dsc[i].id) {
> +			b->base_off = addr;
> +			b->blk_off = m->dsc[i].base;
> +			b->length = m->dsc[i].len;
> +			b->hwversion = m->hwversion;
> +			b->log_mask = DPU_DBG_MASK_DSC;
> +			return &m->dsc[i];
> +		}
> +	}
> +
> +	return NULL;
> +}
> +
> +static void _setup_dsc_ops(struct dpu_hw_dsc_ops *ops,
> +			   unsigned long cap)
> +{
> +	ops->dsc_disable = dpu_hw_dsc_disable;
> +	ops->dsc_config = dpu_hw_dsc_config;
> +	ops->dsc_config_thresh = dpu_hw_dsc_config_thresh;
> +};
> +
> +static struct dpu_hw_blk_ops dpu_hw_ops = {
> +	.start = NULL,
> +	.stop = NULL,
> +};
> +
> +struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc idx, void __iomem 
> *addr,
> +				   struct dpu_mdss_cfg *m)
> +{
> +	struct dpu_hw_dsc *c;
> +	struct dpu_dsc_cfg *cfg;
> +
> +	c = kzalloc(sizeof(*c), GFP_KERNEL);
> +	if (!c)
> +		return ERR_PTR(-ENOMEM);
> +
> +	cfg = _dsc_offset(idx, m, addr, &c->hw);
> +	if (IS_ERR_OR_NULL(cfg)) {
> +		kfree(c);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	c->idx = idx;
> +	c->caps = cfg;
> +	_setup_dsc_ops(&c->ops, c->caps->features);
> +
> +	dpu_hw_blk_init(&c->base, DPU_HW_BLK_DSC, idx, &dpu_hw_ops);
> +
> +	return c;
> +}
> +
> +void dpu_hw_dsc_destroy(struct dpu_hw_dsc *dsc)
> +{
> +	if (dsc)
> +		dpu_hw_blk_destroy(&dsc->base);
> +	kfree(dsc);
> +}
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
> new file mode 100644
> index 000000000000..0fb9ffe9f23f
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
> @@ -0,0 +1,77 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright (c) 2020, Linaro Limited */

Copyright year needs an update : 2020-2021?
> +
> +#ifndef _DPU_HW_DSC_H
> +#define _DPU_HW_DSC_H
> +
> +#include <drm/drm_dsc.h>
> +
> +#define DSC_MODE_SPLIT_PANEL            BIT(0)
> +#define DSC_MODE_MULTIPLEX              BIT(1)
> +#define DSC_MODE_VIDEO                  BIT(2)
> +
> +struct dpu_hw_dsc;
> +
> +/**
> + * struct dpu_hw_dsc_ops - interface to the dsc hardware driver 
> functions
> + * Assumption is these functions will be called after clocks are 
> enabled
> + */
> +struct dpu_hw_dsc_ops {
> +	/**
> +	 * dsc_disable - disable dsc
> +	 * @hw_dsc: Pointer to dsc context
> +	 */
> +	void (*dsc_disable)(struct dpu_hw_dsc *hw_dsc);
> +
> +	/**
> +	 * dsc_config - configures dsc encoder
> +	 * @hw_dsc: Pointer to dsc context
> +	 * @dsc: panel dsc parameters
> +	 * @mode: dsc topology mode to be set
> +	 */
> +	void (*dsc_config)(struct dpu_hw_dsc *hw_dsc,
> +			   struct msm_display_dsc_config *dsc, u32 mode);
> +
> +	/**
> +	 * dsc_config_thresh - programs panel thresholds
> +	 * @hw_dsc: Pointer to dsc context
> +	 * @dsc: panel dsc parameters
> +	 */
> +	void (*dsc_config_thresh)(struct dpu_hw_dsc *hw_dsc,
> +				  struct msm_display_dsc_config *dsc);
> +};
> +
> +struct dpu_hw_dsc {
> +	struct dpu_hw_blk base;
> +	struct dpu_hw_blk_reg_map hw;
> +
> +	/* dsc */
> +	enum dpu_dsc idx;
> +	const struct dpu_dsc_cfg *caps;
> +
> +	/* ops */
> +	struct dpu_hw_dsc_ops ops;
> +};
> +
> +/**
> + * dpu_hw_dsc_init - initializes the dsc block for the passed dsc idx.
> + * @idx:  DSC index for which driver object is required
> + * @addr: Mapped register io address of MDP
> + * @m:    Pointer to mdss catalog data
> + * Returns: Error code or allocated dpu_hw_dsc context
> + */
> +struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc idx, void __iomem 
> *addr,
> +				   struct dpu_mdss_cfg *m);
> +
> +/**
> + * dpu_hw_dsc_destroy - destroys dsc driver context
> + * @dsc:   Pointer to dsc driver context returned by dpu_hw_dsc_init
> + */
> +void dpu_hw_dsc_destroy(struct dpu_hw_dsc *dsc);
> +
> +static inline struct dpu_hw_dsc *to_dpu_hw_dsc(struct dpu_hw_blk *hw)
> +{
> +	return container_of(hw, struct dpu_hw_dsc, base);
> +}
> +
> +#endif /* _DPU_HW_DSC_H */
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> index 09a3fb3e89f5..1b72c11090ee 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> @@ -97,6 +97,7 @@ enum dpu_hw_blk_type {
>  	DPU_HW_BLK_WB,
>  	DPU_HW_BLK_DSPP,
>  	DPU_HW_BLK_MERGE_3D,
> +	DPU_HW_BLK_DSC,
>  	DPU_HW_BLK_MAX,
>  };
> 
> @@ -176,6 +177,17 @@ enum dpu_ctl {
>  	CTL_MAX
>  };
> 
> +enum dpu_dsc {
> +	DSC_NONE = 0,
> +	DSC_0,
> +	DSC_1,
> +	DSC_2,
> +	DSC_3,
> +	DSC_4,
> +	DSC_5,
> +	DSC_MAX
> +};
> +
>  enum dpu_pingpong {
>  	PINGPONG_0 = 1,
>  	PINGPONG_1,
> @@ -437,5 +449,6 @@ struct dpu_mdss_color {
>  #define DPU_DBG_MASK_VBIF     (1 << 8)
>  #define DPU_DBG_MASK_ROT      (1 << 9)
>  #define DPU_DBG_MASK_DSPP     (1 << 10)
> +#define DPU_DBG_MASK_DSC      (1 << 11)
> 
>  #endif  /* _DPU_HW_MDSS_H */

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

* Re: [Freedreno] [PATCH 03/11] drm/msm/disp/dpu1: Add support for DSC in pingpong block
  2021-07-15  6:51 ` [PATCH 03/11] drm/msm/disp/dpu1: Add support for DSC in pingpong block Vinod Koul
@ 2021-08-02 23:07   ` abhinavk
  0 siblings, 0 replies; 41+ messages in thread
From: abhinavk @ 2021-08-02 23:07 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 2021-07-14 23:51, Vinod Koul wrote:
> In SDM845, DSC can be enabled by writing to pingpong block registers, 
> so
> add support for DSC in hw_pp
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>
> ---
>  .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c   | 32 +++++++++++++++++++
>  .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h   | 14 ++++++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> index 245a7a62b5c6..07fc131ca9aa 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> @@ -28,6 +28,9 @@
>  #define PP_FBC_MODE                     0x034
>  #define PP_FBC_BUDGET_CTL               0x038
>  #define PP_FBC_LOSSY_MODE               0x03C
> +#define PP_DSC_MODE                     0x0a0
> +#define PP_DCE_DATA_IN_SWAP             0x0ac
> +#define PP_DCE_DATA_OUT_SWAP            0x0c8
> 
>  #define PP_DITHER_EN			0x000
>  #define PP_DITHER_BITDEPTH		0x004
> @@ -245,6 +248,32 @@ static u32 dpu_hw_pp_get_line_count(struct
> dpu_hw_pingpong *pp)
>  	return line;
>  }
> 
> +static int dpu_hw_pp_dsc_enable(struct dpu_hw_pingpong *pp)
> +{
> +	struct dpu_hw_blk_reg_map *c = &pp->hw;
> +
> +	DPU_REG_WRITE(c, PP_DSC_MODE, 1);
> +	return 0;
> +}
> +
> +static void dpu_hw_pp_dsc_disable(struct dpu_hw_pingpong *pp)
> +{
> +	struct dpu_hw_blk_reg_map *c = &pp->hw;
> +
> +	DPU_REG_WRITE(c, PP_DSC_MODE, 0);
> +}
> +
> +static int dpu_hw_pp_setup_dsc(struct dpu_hw_pingpong *pp)
> +{
> +	struct dpu_hw_blk_reg_map *pp_c = &pp->hw;
> +	int data;
> +
> +	data = DPU_REG_READ(pp_c, PP_DCE_DATA_OUT_SWAP);
> +	data |= BIT(18); /* endian flip */
> +	DPU_REG_WRITE(pp_c, PP_DCE_DATA_OUT_SWAP, data);
> +	return 0;
> +}
> +
>  static void _setup_pingpong_ops(struct dpu_hw_pingpong *c,
>  				unsigned long features)
>  {
> @@ -256,6 +285,9 @@ static void _setup_pingpong_ops(struct 
> dpu_hw_pingpong *c,
>  	c->ops.get_autorefresh = dpu_hw_pp_get_autorefresh_config;
>  	c->ops.poll_timeout_wr_ptr = dpu_hw_pp_poll_timeout_wr_ptr;
>  	c->ops.get_line_count = dpu_hw_pp_get_line_count;
> +	c->ops.setup_dsc = dpu_hw_pp_setup_dsc;
> +	c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
> +	c->ops.disable_dsc = dpu_hw_pp_dsc_disable;
> 
>  	if (test_bit(DPU_PINGPONG_DITHER, &features))
>  		c->ops.setup_dither = dpu_hw_pp_setup_dither;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
> index 845b9ce80e31..5058e41ffbc0 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
> @@ -124,6 +124,20 @@ struct dpu_hw_pingpong_ops {
>  	 */
>  	void (*setup_dither)(struct dpu_hw_pingpong *pp,
>  			struct dpu_hw_dither_cfg *cfg);
> +	/**
> +	 * Enable DSC
> +	 */
> +	int (*enable_dsc)(struct dpu_hw_pingpong *pp);
> +
> +	/**
> +	 * Disable DSC
> +	 */
> +	void (*disable_dsc)(struct dpu_hw_pingpong *pp);
> +
> +	/**
> +	 * Setup DSC
> +	 */
> +	int (*setup_dsc)(struct dpu_hw_pingpong *pp);
>  };
> 
>  struct dpu_hw_pingpong {

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

* Re: [Freedreno] [PATCH 04/11] drm/msm/disp/dpu1: Add DSC support in RM
  2021-07-15  6:51 ` [PATCH 04/11] drm/msm/disp/dpu1: Add DSC support in RM Vinod Koul
  2021-07-29 20:23   ` Dmitry Baryshkov
@ 2021-08-02 23:24   ` abhinavk
  2021-10-06 10:27     ` Vinod Koul
  1 sibling, 1 reply; 41+ messages in thread
From: abhinavk @ 2021-08-02 23:24 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 2021-07-14 23:51, Vinod Koul wrote:
> This add the bits in RM to enable the DSC blocks
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  1 +
>  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c  | 32 +++++++++++++++++++++++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h  |  1 +
>  3 files changed, 34 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index d6717d6672f7..d56c05146dfe 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -165,6 +165,7 @@ struct dpu_global_state {
>  	uint32_t ctl_to_enc_id[CTL_MAX - CTL_0];
>  	uint32_t intf_to_enc_id[INTF_MAX - INTF_0];
>  	uint32_t dspp_to_enc_id[DSPP_MAX - DSPP_0];
> +	uint32_t dsc_to_enc_id[DSC_MAX - DSC_0];
>  };
> 
>  struct dpu_global_state
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index fd2d104f0a91..4da6d72b7996 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -11,6 +11,7 @@
>  #include "dpu_hw_intf.h"
>  #include "dpu_hw_dspp.h"
>  #include "dpu_hw_merge3d.h"
> +#include "dpu_hw_dsc.h"
>  #include "dpu_encoder.h"
>  #include "dpu_trace.h"
> 
> @@ -75,6 +76,14 @@ int dpu_rm_destroy(struct dpu_rm *rm)
>  			dpu_hw_intf_destroy(hw);
>  		}
>  	}
> +	for (i = 0; i < ARRAY_SIZE(rm->dsc_blks); i++) {
> +		struct dpu_hw_dsc *hw;
> +
> +		if (rm->intf_blks[i]) {
same comment as dmitry on this 
https://patchwork.freedesktop.org/patch/444070/?series=90413&rev=2
> +			hw = to_dpu_hw_dsc(rm->dsc_blks[i]);
> +			dpu_hw_dsc_destroy(hw);
> +		}
> +	}
> 
>  	return 0;
>  }
> @@ -221,6 +230,19 @@ int dpu_rm_init(struct dpu_rm *rm,
>  		rm->dspp_blks[dspp->id - DSPP_0] = &hw->base;
>  	}
> 
> +	for (i = 0; i < cat->dsc_count; i++) {
> +		struct dpu_hw_dsc *hw;
> +		const struct dpu_dsc_cfg *dsc = &cat->dsc[i];
> +
> +		hw = dpu_hw_dsc_init(dsc->id, mmio, cat);
> +		if (IS_ERR_OR_NULL(hw)) {
> +			rc = PTR_ERR(hw);
> +			DPU_ERROR("failed dsc object creation: err %d\n", rc);
> +			goto fail;
> +		}
> +		rm->dsc_blks[dsc->id - DSC_0] = &hw->base;
> +	}
> +
>  	return 0;
> 
>  fail:
> @@ -476,6 +498,9 @@ static int _dpu_rm_reserve_intf(
>  	}
> 
>  	global_state->intf_to_enc_id[idx] = enc_id;
> +
> +	global_state->dsc_to_enc_id[0] = enc_id;
> +	global_state->dsc_to_enc_id[1] = enc_id;
>  	return 0;
>  }
agree with dmitry again here, why are DSCs being reserved in the 
_dpu_rm_reserve_intf function?
First, for clarity, they should be in a function of their own.
Allocating the DSCs has to also account for the PP availability of that 
DSC and other factors need to
be considered as well.
I suggest checking _sde_rm_reserve_dsc() from downstream to improve the 
DSC reservation logic.
> 
> @@ -567,6 +592,8 @@ void dpu_rm_release(struct dpu_global_state 
> *global_state,
>  		ARRAY_SIZE(global_state->ctl_to_enc_id), enc->base.id);
>  	_dpu_rm_clear_mapping(global_state->intf_to_enc_id,
>  		ARRAY_SIZE(global_state->intf_to_enc_id), enc->base.id);
> +	_dpu_rm_clear_mapping(global_state->dsc_to_enc_id,
> +		ARRAY_SIZE(global_state->dsc_to_enc_id), enc->base.id);
>  }
> 
>  int dpu_rm_reserve(
> @@ -640,6 +667,11 @@ int dpu_rm_get_assigned_resources(struct dpu_rm 
> *rm,
>  		hw_to_enc_id = global_state->dspp_to_enc_id;
>  		max_blks = ARRAY_SIZE(rm->dspp_blks);
>  		break;
> +	case DPU_HW_BLK_DSC:
> +		hw_blks = rm->dsc_blks;
> +		hw_to_enc_id = global_state->dsc_to_enc_id;
> +		max_blks = ARRAY_SIZE(rm->dsc_blks);
> +		break;
>  	default:
>  		DPU_ERROR("blk type %d not managed by rm\n", type);
>  		return 0;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> index 1f12c8d5b8aa..278d2a510b80 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> @@ -30,6 +30,7 @@ struct dpu_rm {
>  	struct dpu_hw_blk *intf_blks[INTF_MAX - INTF_0];
>  	struct dpu_hw_blk *dspp_blks[DSPP_MAX - DSPP_0];
>  	struct dpu_hw_blk *merge_3d_blks[MERGE_3D_MAX - MERGE_3D_0];
> +	struct dpu_hw_blk *dsc_blks[DSC_MAX - DSC_0];
> 
>  	uint32_t lm_max_width;
>  };

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

* Re: [Freedreno] [PATCH 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog
  2021-07-15  6:51 ` [PATCH 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog Vinod Koul
  2021-07-29 20:25   ` Dmitry Baryshkov
@ 2021-08-02 23:29   ` abhinavk
  2021-10-06 10:52     ` Vinod Koul
  1 sibling, 1 reply; 41+ messages in thread
From: abhinavk @ 2021-08-02 23:29 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 2021-07-14 23:51, Vinod Koul wrote:
> This add SDM845 DSC blocks into hw_catalog
/add --> adds
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
> Changes since RFC:
>  - use BIT values from MASK
> 
>  .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> 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 b569030a0847..b45a08303c99 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -40,6 +40,8 @@
> 
>  #define PINGPONG_SDM845_MASK BIT(DPU_PINGPONG_DITHER)
> 
> +#define DSC_SDM845_MASK BIT(1)
agree with dmitry again : 
https://patchwork.freedesktop.org/patch/444072/?series=90413&rev=2
this is unused. you can use .features = 0
> +
>  #define PINGPONG_SDM845_SPLIT_MASK \
>  	(PINGPONG_SDM845_MASK | BIT(DPU_PINGPONG_TE2))
> 
> @@ -751,6 +753,24 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = 
> {
>  	PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, 0, sc7280_pp_sblk),
>  	PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk),
>  };
> +
> +/*************************************************************
> + * DSC sub blocks config
> + *************************************************************/
> +#define DSC_BLK(_name, _id, _base) \
> +	{\
> +	.name = _name, .id = _id, \
> +	.base = _base, .len = 0x140, \
> +	.features = DSC_SDM845_MASK, \
> +	}
> +
> +static struct dpu_dsc_cfg sdm845_dsc[] = {
> +	DSC_BLK("dsc_0", DSC_0, 0x80000),
> +	DSC_BLK("dsc_1", DSC_1, 0x80400),
> +	DSC_BLK("dsc_2", DSC_2, 0x80800),
> +	DSC_BLK("dsc_3", DSC_3, 0x80c00),
> +};
> +
>  /*************************************************************
>   * INTF sub blocks config
>   *************************************************************/
> @@ -1053,6 +1073,8 @@ static void sdm845_cfg_init(struct dpu_mdss_cfg 
> *dpu_cfg)
>  		.mixer = sdm845_lm,
>  		.pingpong_count = ARRAY_SIZE(sdm845_pp),
>  		.pingpong = sdm845_pp,
> +		.dsc_count = ARRAY_SIZE(sdm845_dsc),
> +		.dsc = sdm845_dsc,
>  		.intf_count = ARRAY_SIZE(sdm845_intf),
>  		.intf = sdm845_intf,
>  		.vbif_count = ARRAY_SIZE(sdm845_vbif),

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

* Re: [Freedreno] [PATCH 06/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl
  2021-07-15  6:51 ` [PATCH 06/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl Vinod Koul
  2021-07-29 22:15   ` Dmitry Baryshkov
@ 2021-08-03  0:00   ` abhinavk
  2021-10-06 12:21     ` Vinod Koul
  1 sibling, 1 reply; 41+ messages in thread
From: abhinavk @ 2021-08-03  0:00 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 2021-07-14 23:51, Vinod Koul wrote:
> Later gens of hardware have DSC bits moved to hw_ctl, so configure 
> these
> bits so that DSC would work there as well
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
Please correct me if wrong but here you seem to be flushing all the DSC 
bits
even the unused ones. This will end-up enabling DSC even when DSC is 
unused on
the newer targets.
If so, thats wrong.
We need to implement bit-mask based approach to avoid this change and 
only enable
those DSCs which are used.

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index 2d4645e01ebf..aeea6add61ee 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -25,6 +25,8 @@
>  #define   CTL_MERGE_3D_ACTIVE           0x0E4
>  #define   CTL_INTF_ACTIVE               0x0F4
>  #define   CTL_MERGE_3D_FLUSH            0x100
> +#define   CTL_DSC_ACTIVE                0x0E8
> +#define   CTL_DSC_FLUSH                0x104
>  #define   CTL_INTF_FLUSH                0x110
>  #define   CTL_INTF_MASTER               0x134
>  #define   CTL_FETCH_PIPE_ACTIVE         0x0FC
> @@ -34,6 +36,7 @@
> 
>  #define DPU_REG_RESET_TIMEOUT_US        2000
>  #define  MERGE_3D_IDX   23
> +#define  DSC_IDX        22
>  #define  INTF_IDX       31
>  #define CTL_INVALID_BIT                 0xffff
> 
> @@ -120,6 +123,7 @@ static u32 dpu_hw_ctl_get_pending_flush(struct
> dpu_hw_ctl *ctx)
> 
>  static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
>  {
> +	DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | 
> BIT(3));
> 
>  	if (ctx->pending_flush_mask & BIT(MERGE_3D_IDX))
>  		DPU_REG_WRITE(&ctx->hw, CTL_MERGE_3D_FLUSH,
> @@ -128,7 +132,7 @@ static inline void
> dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
>  		DPU_REG_WRITE(&ctx->hw, CTL_INTF_FLUSH,
>  				ctx->pending_intf_flush_mask);
> 
> -	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
> +	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask |  
> BIT(DSC_IDX));
>  }
> 
>  static inline void dpu_hw_ctl_trigger_flush(struct dpu_hw_ctl *ctx)
> @@ -507,6 +511,7 @@ static void dpu_hw_ctl_intf_cfg_v1(struct 
> dpu_hw_ctl *ctx,
>  	if (cfg->merge_3d)
>  		DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
>  			      BIT(cfg->merge_3d - MERGE_3D_0));
> +	DPU_REG_WRITE(c, CTL_DSC_ACTIVE, BIT(0) | BIT(1) | BIT(2) | BIT(3));
>  }
> 
>  static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,

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

* Re: [Freedreno] [PATCH 07/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d
  2021-07-15  6:51 ` [PATCH 07/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d Vinod Koul
@ 2021-08-03  0:24   ` abhinavk
  2021-10-06 12:22     ` Vinod Koul
  0 siblings, 1 reply; 41+ messages in thread
From: abhinavk @ 2021-08-03  0:24 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 2021-07-14 23:51, Vinod Koul wrote:
> We cannot enable mode_3d when we are using the DSC. So pass
> configuration to detect DSC is enabled and not enable mode_3d
> when we are using DSC
> 
> We add a helper dpu_encoder_helper_get_dsc_mode() to detect dsc
> enabled and pass this to .setup_intf_cfg()
> 
This is not entirely correct. This is true only for the 2-2-1 topology 
you are using
on this panel.

When you are using 2-2-1, you are using 2 LMs, 2 DSCs and 1 DSI.
So 3D mux shouldnt be used.

If you are using something like 4-2-1 or 4-2-2, then you have 4LMs,
2 DSCs and 2/1 DSI.

Here you need the 3D mux to convert the data from 4LMs to 2 DSCs.

So please correct the commit text here and also add a check for the 
topology.

> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h     | 11 +++++++++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c |  2 ++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c           |  5 +++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h           |  2 ++
>  4 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> index ecbc4be98980..d43b804528eb 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -336,6 +336,17 @@ static inline enum dpu_3d_blend_mode
> dpu_encoder_helper_get_3d_blend_mode(
>  	return BLEND_3D_NONE;
>  }
> 
> +static inline bool dpu_encoder_helper_get_dsc_mode(struct
> dpu_encoder_phys *phys_enc)
> +{
> +	struct drm_encoder *drm_enc = phys_enc->parent;
> +	struct msm_drm_private *priv = drm_enc->dev->dev_private;
> +
> +	if (priv->dsc)
> +		return true;
> +
> +	return false;
> +}
Check whether DSC is enabled and only if its 2-2-1 topology.
This needs to be reworked when other topologies are supported.

> +
>  /**
>   * dpu_encoder_helper_split_config - split display configuration
> helper function
>   *	This helper function may be used by physical encoders to configure
> 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 b2be39b9144e..5fe87881c30c 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
> @@ -69,6 +69,8 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg(
>  	intf_cfg.intf_mode_sel = DPU_CTL_MODE_SEL_CMD;
>  	intf_cfg.stream_sel = cmd_enc->stream_sel;
>  	intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
> +	intf_cfg.dsc = dpu_encoder_helper_get_dsc_mode(phys_enc);
> +
>  	ctl->ops.setup_intf_cfg(ctl, &intf_cfg);
>  }
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index aeea6add61ee..f059416311ee 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -121,7 +121,7 @@ static u32 dpu_hw_ctl_get_pending_flush(struct
> dpu_hw_ctl *ctx)
>  	return ctx->pending_flush_mask;
>  }
> 
> -static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
> +static void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
>  {
>  	DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | 
> BIT(3));
> 
> @@ -522,7 +522,8 @@ static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl 
> *ctx,
> 
>  	intf_cfg |= (cfg->intf & 0xF) << 4;
> 
> -	if (cfg->mode_3d) {
> +	/* In DSC we can't set merge, so check for dsc too */
> +	if (cfg->mode_3d && !cfg->dsc) {
>  		intf_cfg |= BIT(19);
>  		intf_cfg |= (cfg->mode_3d - 0x1) << 20;
>  	}
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> index 806c171e5df2..347a653c1e01 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> @@ -39,6 +39,7 @@ struct dpu_hw_stage_cfg {
>   * @mode_3d:               3d mux configuration
>   * @merge_3d:              3d merge block used
>   * @intf_mode_sel:         Interface mode, cmd / vid
> + * @dsc:                   DSC is enabled
>   * @stream_sel:            Stream selection for multi-stream 
> interfaces
>   */
>  struct dpu_hw_intf_cfg {
> @@ -46,6 +47,7 @@ struct dpu_hw_intf_cfg {
>  	enum dpu_3d_blend_mode mode_3d;
>  	enum dpu_merge_3d merge_3d;
>  	enum dpu_ctl_mode_sel intf_mode_sel;
> +	bool dsc;
>  	int stream_sel;
>  };

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

* Re: [Freedreno] [PATCH 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder
  2021-07-15  6:52 ` [PATCH 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder Vinod Koul
  2021-07-19  8:54   ` kernel test robot
  2021-07-29 20:54   ` Dmitry Baryshkov
@ 2021-08-03  0:57   ` abhinavk
  2021-10-06 12:47     ` Vinod Koul
  2 siblings, 1 reply; 41+ messages in thread
From: abhinavk @ 2021-08-03  0:57 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 2021-07-14 23:52, Vinod Koul wrote:
> When DSC is enabled in DT, we need to configure the encoder for DSC
> configuration, calculate DSC parameters for the given timing.
> 
> This patch adds that support by adding dpu_encoder_prep_dsc() which is
> invoked when DSC is enabled in DT
correct me if wrong but this commit text is not valid anymore in my 
opinion.
are there any params you are getting from DT now? I thought its all 
coming from the panel
driver directly.
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
agree with dmitry's comment's 
https://patchwork.freedesktop.org/patch/444078/?series=90413&rev=2

instead of dsc being part of priv->dsc it should be per encoder.

On top of his comment, I also think that like on the newer chipsets, 
moving the dsc related
encoder configuration to a dpu_encoder_dce.c will help for future 
expansion of other topologies
and also for other compression algorithms.

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 142 +++++++++++++++++++-
>  1 file changed, 141 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 8d942052db8a..41140b781e66 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -21,12 +21,17 @@
>  #include "dpu_hw_intf.h"
>  #include "dpu_hw_ctl.h"
>  #include "dpu_hw_dspp.h"
> +#include "dpu_hw_dsc.h"
>  #include "dpu_formats.h"
>  #include "dpu_encoder_phys.h"
>  #include "dpu_crtc.h"
>  #include "dpu_trace.h"
>  #include "dpu_core_irq.h"
> 
> +#define DSC_MODE_SPLIT_PANEL		BIT(0)
> +#define DSC_MODE_MULTIPLEX		BIT(1)
> +#define DSC_MODE_VIDEO			BIT(2)
> +
>  #define DPU_DEBUG_ENC(e, fmt, ...) DPU_DEBUG("enc%d " fmt,\
>  		(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
> 
> @@ -135,6 +140,7 @@ enum dpu_enc_rc_states {
>   * @cur_slave:		As above but for the slave encoder.
>   * @hw_pp:		Handle to the pingpong blocks used for the display. No.
>   *			pingpong blocks can be different than num_phys_encs.
> + * @hw_dsc		Handle to the DSC blocks used for the display.
>   * @intfs_swapped:	Whether or not the phys_enc interfaces have been 
> swapped
>   *			for partial update right-only cases, such as pingpong
>   *			split where virtual pingpong does not generate IRQs
> @@ -180,6 +186,7 @@ struct dpu_encoder_virt {
>  	struct dpu_encoder_phys *cur_master;
>  	struct dpu_encoder_phys *cur_slave;
>  	struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
> +	struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
> 
>  	bool intfs_swapped;
> 
> @@ -1008,7 +1015,8 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
>  	struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
>  	struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
>  	struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC] = { NULL };
> -	int num_lm, num_ctl, num_pp;
> +	struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
> +	int num_lm, num_ctl, num_pp, num_dsc;
>  	int i, j;
> 
>  	if (!drm_enc) {
> @@ -1061,11 +1069,16 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
>  	dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
>  		drm_enc->base.id, DPU_HW_BLK_DSPP, hw_dspp,
>  		ARRAY_SIZE(hw_dspp));
> +	num_dsc = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> +		drm_enc->base.id, DPU_HW_BLK_DSC, hw_dsc, ARRAY_SIZE(hw_dsc));
> 
>  	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
>  		dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i])
>  						: NULL;
> 
> +	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
> +		dpu_enc->hw_dsc[i] = i < num_dsc ? to_dpu_hw_dsc(hw_dsc[i]) : NULL;
> +
>  	cstate = to_dpu_crtc_state(drm_crtc->state);
> 
>  	for (i = 0; i < num_lm; i++) {
> @@ -1810,10 +1823,133 @@ static void
> dpu_encoder_vsync_event_work_handler(struct kthread_work *work)
>  			nsecs_to_jiffies(ktime_to_ns(wakeup_time)));
>  }
> 
> +static void
> +dpu_encoder_dsc_pclk_param_calc(struct msm_display_dsc_config *dsc, 
> u32 width)
> +{
> +	int slice_count, slice_per_intf;
> +	int bytes_in_slice, total_bytes_per_intf;
> +
> +	if (!dsc || !dsc->drm->slice_width || !dsc->drm->slice_count) {
> +		DPU_ERROR("Invalid DSC/slices\n");
> +		return;
> +	}
> +
> +	slice_count = dsc->drm->slice_count;
> +	slice_per_intf = DIV_ROUND_UP(width, dsc->drm->slice_width);
> +
> +	/*
> +	 * If slice_count is greater than slice_per_intf then default to 1.
> +	 * This can happen during partial update.
> +	 */
> +	if (slice_count > slice_per_intf)
> +		slice_count = 1;
> +
> +	bytes_in_slice = DIV_ROUND_UP(dsc->drm->slice_width *
> +				      dsc->drm->bits_per_pixel, 8);
> +	total_bytes_per_intf = bytes_in_slice * slice_per_intf;
> +
> +	dsc->eol_byte_num = total_bytes_per_intf % 3;
> +	dsc->pclk_per_line =  DIV_ROUND_UP(total_bytes_per_intf, 3);
> +	dsc->bytes_in_slice = bytes_in_slice;
> +	dsc->bytes_per_pkt = bytes_in_slice * slice_count;
> +	dsc->pkt_per_line = slice_per_intf / slice_count;
> +}
> +
> +static void
> +dpu_encoder_dsc_initial_line_calc(struct msm_display_dsc_config *dsc,
> +				  u32 enc_ip_width)
> +{
> +	int ssm_delay, total_pixels, soft_slice_per_enc;
> +
> +	soft_slice_per_enc = enc_ip_width / dsc->drm->slice_width;
> +
> +	/*
> +	 * minimum number of initial line pixels is a sum of:
> +	 * 1. sub-stream multiplexer delay (83 groups for 8bpc,
> +	 *    91 for 10 bpc) * 3
> +	 * 2. for two soft slice cases, add extra sub-stream multiplexer * 3
> +	 * 3. the initial xmit delay
> +	 * 4. total pipeline delay through the "lock step" of encoder (47)
> +	 * 5. 6 additional pixels as the output of the rate buffer is
> +	 *    48 bits wide
> +	 */
> +	ssm_delay = ((dsc->drm->bits_per_component < 10) ? 84 : 92);
> +	total_pixels = ssm_delay * 3 + dsc->drm->initial_xmit_delay + 47;
> +	if (soft_slice_per_enc > 1)
> +		total_pixels += (ssm_delay * 3);
> +	dsc->initial_lines = DIV_ROUND_UP(total_pixels, 
> dsc->drm->slice_width);
> +}
> +
> +static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
> +				     struct dpu_hw_pingpong *hw_pp,
> +				     struct msm_display_dsc_config *dsc,
> +				     u32 common_mode)
> +{
> +	if (hw_dsc->ops.dsc_config)
> +		hw_dsc->ops.dsc_config(hw_dsc, dsc, common_mode);
> +
> +	if (hw_dsc->ops.dsc_config_thresh)
> +		hw_dsc->ops.dsc_config_thresh(hw_dsc, dsc);
> +
> +	if (hw_pp->ops.setup_dsc)
> +		hw_pp->ops.setup_dsc(hw_pp);
> +
> +	if (hw_pp->ops.enable_dsc)
> +		hw_pp->ops.enable_dsc(hw_pp);
> +}
> +
> +static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> +				 struct msm_display_dsc_config *dsc)
> +{
> +	/* coding only for 2LM, 2enc, 1 dsc config */
> +	struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
> +	struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
> +	struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
> +	int this_frame_slices;
> +	int intf_ip_w, enc_ip_w;
> +	int dsc_common_mode;
> +	int pic_width, pic_height;
> +	int i;
> +
> +	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
> +		hw_pp[i] = dpu_enc->hw_pp[i];
> +		hw_dsc[i] = dpu_enc->hw_dsc[i];
> +
> +		if (!hw_pp[i] || !hw_dsc[i]) {
> +			DPU_ERROR_ENC(dpu_enc, "invalid params for DSC\n");
> +			return;
> +		}
> +	}
> +
> +	dsc_common_mode = 0;
> +	pic_width = dsc->drm->pic_width;
> +	pic_height = dsc->drm->pic_height;
> +
> +	dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL;
> +	if (enc_master->intf_mode == INTF_MODE_VIDEO)
> +		dsc_common_mode |= DSC_MODE_VIDEO;
> +
> +	this_frame_slices = pic_width / dsc->drm->slice_width;
> +	intf_ip_w = this_frame_slices * dsc->drm->slice_width;
> +
> +	dpu_encoder_dsc_pclk_param_calc(dsc, intf_ip_w);
> +
> +	/*
> +	 * dsc merge case: when using 2 encoders for the same stream,
> +	 * no. of slices need to be same on both the encoders.
> +	 */
> +	enc_ip_w = intf_ip_w / 2;
> +	dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
> +
> +	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
> +		dpu_encoder_dsc_pipe_cfg(hw_dsc[i], hw_pp[i], dsc, dsc_common_mode);
> +}
> +
>  void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
>  {
>  	struct dpu_encoder_virt *dpu_enc;
>  	struct dpu_encoder_phys *phys;
> +	struct msm_drm_private *priv;
>  	bool needs_hw_reset = false;
>  	unsigned int i;
> 
> @@ -1841,6 +1977,10 @@ void dpu_encoder_prepare_for_kickoff(struct
> drm_encoder *drm_enc)
>  			dpu_encoder_helper_hw_reset(dpu_enc->phys_encs[i]);
>  		}
>  	}
> +
> +	priv = drm_enc->dev->dev_private;
> +	if (priv->dsc)
> +		dpu_encoder_prep_dsc(dpu_enc, priv->dsc);
>  }
> 
>  void dpu_encoder_kickoff(struct drm_encoder *drm_enc)

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

* Re: [Freedreno] [PATCH 09/11] drm/msm/disp/dpu1: Add support for DSC in topology
  2021-07-15  6:52 ` [PATCH 09/11] drm/msm/disp/dpu1: Add support for DSC in topology Vinod Koul
@ 2021-08-03  1:05   ` abhinavk
  0 siblings, 0 replies; 41+ messages in thread
From: abhinavk @ 2021-08-03  1:05 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 2021-07-14 23:52, Vinod Koul wrote:
> For DSC to work we typically need a 2,2,1 configuration. This should
> suffice for resolutions upto 4k. For more resolutions like 8k this 
> won't
> work.
> 
> The topology information is provided by DTS so we try to deduce the
> topology required for DSC.
> Furthermore, we can use 1 DSC encoder in lesser resolutions, but that 
> is
> not power efficient according to Abhinav, it is better to use 2 mixers
> as that will split width/2 and is proven to be power efficient.

I think now that we have added the technical reason of why it is better 
to use
2-2-1 ( using 2 LMs is better than one as it will half layer width), we 
can drop my name from the commit text
as it holds less value than the actual reason itself :)
You can still keep my signed-off and co-developed by tag

> 
> Also, the panel has been tested only with 2,2,1 configuration, so for
> now we blindly create 2,2,1 topology when DSC is enabled
> 
> Co-developed-by: Abhinav Kumar <abhinavk@codeaurora.org>
> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
> Changes since RFC:
>  - Add more details in changelog
> 
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 41140b781e66..8f0a8bd9c8ff 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -573,6 +573,8 @@ static struct msm_display_topology 
> dpu_encoder_get_topology(
>  			struct drm_display_mode *mode)
>  {
>  	struct msm_display_topology topology = {0};
> +	struct drm_encoder *drm_enc;
> +	struct msm_drm_private *priv;
>  	int i, intf_count = 0;
> 
>  	for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
> @@ -607,8 +609,22 @@ static struct msm_display_topology
> dpu_encoder_get_topology(
>  	topology.num_enc = 0;
>  	topology.num_intf = intf_count;
> 
> +	drm_enc = &dpu_enc->base;
> +	priv = drm_enc->dev->dev_private;
> +	if (priv && priv->dsc) {
if dsc is moved to the encoder, this will need to be changed too
> +		/* In case of Display Stream Compression DSC, we would use
> +		 * 2 encoders, 2 line mixers and 1 interface
> +		 * this is power optimal and can drive upto (including) 4k
> +		 * screens
> +		 */
> +		topology.num_enc = 2;
> +		topology.num_intf = 1;
> +		topology.num_lm = 2;
> +	}
> +
>  	return topology;
>  }
> +
>  static int dpu_encoder_virt_atomic_check(
>  		struct drm_encoder *drm_enc,
>  		struct drm_crtc_state *crtc_state,

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

* Re: [Freedreno] [PATCH 10/11] drm/msm/dsi: Add support for DSC configuration
  2021-07-15  6:52 ` [PATCH 10/11] drm/msm/dsi: Add support for DSC configuration Vinod Koul
  2021-07-29 22:10   ` Dmitry Baryshkov
@ 2021-08-03  1:16   ` abhinavk
  1 sibling, 0 replies; 41+ messages in thread
From: abhinavk @ 2021-08-03  1:16 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 2021-07-14 23:52, Vinod Koul wrote:
> When DSC is enabled, we need to configure DSI registers accordingly and
> configure the respective stream compression registers.
> 
> Add support to calculate the register setting based on DSC params and
> timing information and configure these registers.
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>

same comments as dmitry on this one: 
https://patchwork.freedesktop.org/patch/444082/?series=90413&rev=2
nothing more to add.

> ---
>  drivers/gpu/drm/msm/dsi/dsi.xml.h  |  10 ++
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 142 +++++++++++++++++++++++++++--
>  2 files changed, 142 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.xml.h
> b/drivers/gpu/drm/msm/dsi/dsi.xml.h
> index 50eb4d1b8fdd..b8e9e608abfc 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.xml.h
> +++ b/drivers/gpu/drm/msm/dsi/dsi.xml.h
> @@ -2310,4 +2310,14 @@ static inline uint32_t
> REG_DSI_7nm_PHY_LN_TX_DCTRL(uint32_t i0) { return 0x00000
> 
>  #define REG_DSI_7nm_PHY_PLL_PERF_OPTIMIZE			0x00000260
> 
> +#define REG_DSI_VIDEO_COMPRESSION_MODE_CTRL			0x0000029c
> +
> +#define REG_DSI_VIDEO_COMPRESSION_MODE_CTRL2			0x000002a0
> +
> +#define REG_DSI_COMMAND_COMPRESSION_MODE_CTRL			0x000002a4
> +
> +#define REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2			0x000002a8
> +
> +#define REG_DSI_COMMAND_COMPRESSION_MODE_CTRL3			0x000002ac
> +
>  #endif /* DSI_XML */
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index e1e5d91809b5..4e8ab1b1df8b 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -942,6 +942,26 @@ static void dsi_ctrl_config(struct msm_dsi_host
> *msm_host, bool enable,
>  	dsi_write(msm_host, REG_DSI_CTRL, data);
>  }
> 
> +static int dsi_dsc_update_pic_dim(struct msm_display_dsc_config *dsc,
> +				  int pic_width, int pic_height)
> +{
> +	if (!dsc || !pic_width || !pic_height) {
> +		pr_err("DSI: invalid input: pic_width: %d pic_height: %d\n",
> pic_width, pic_height);
> +		return -EINVAL;
> +	}
> +
> +	if ((pic_width % dsc->drm->slice_width) || (pic_height %
> dsc->drm->slice_height)) {
> +		pr_err("DSI: pic_dim %dx%d has to be multiple of slice %dx%d\n",
> +		       pic_width, pic_height, dsc->drm->slice_width, 
> dsc->drm->slice_height);
> +		return -EINVAL;
> +	}
> +
> +	dsc->drm->pic_width = pic_width;
> +	dsc->drm->pic_height = pic_height;
> +
> +	return 0;
> +}
> +
>  static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool 
> is_dual_dsi)
>  {
>  	struct drm_display_mode *mode = msm_host->mode;
> @@ -956,6 +976,7 @@ static void dsi_timing_setup(struct msm_dsi_host
> *msm_host, bool is_dual_dsi)
>  	u32 va_end = va_start + mode->vdisplay;
>  	u32 hdisplay = mode->hdisplay;
>  	u32 wc;
> +	u32 data;
> 
>  	DBG("");
> 
> @@ -974,7 +995,73 @@ static void dsi_timing_setup(struct msm_dsi_host
> *msm_host, bool is_dual_dsi)
>  		hdisplay /= 2;
>  	}
> 
> +	if (msm_host->dsc) {
> +		struct msm_display_dsc_config *dsc = msm_host->dsc;
> +
> +		/* update dsc params with timing params */
> +		dsi_dsc_update_pic_dim(dsc, mode->hdisplay, mode->vdisplay);
> +		DBG("Mode Width- %d x Height %d\n", dsc->drm->pic_width,
> dsc->drm->pic_height);
> +
> +		/* we do the calculations for dsc parameters here so that
> +		 * panel can use these parameters
> +		 */
> +		dsi_populate_dsc_params(dsc);
> +
> +		/* Divide the display by 3 but keep back/font porch and
> +		 * pulse width same
> +		 */
> +		h_total -= hdisplay;
> +		hdisplay /= 3;
> +		h_total += hdisplay;
> +		ha_end = ha_start + hdisplay;
> +	}
> +
>  	if (msm_host->mode_flags & MIPI_DSI_MODE_VIDEO) {
> +		if (msm_host->dsc) {
> +			struct msm_display_dsc_config *dsc = msm_host->dsc;
> +			u32 reg, intf_width, slice_per_intf, width;
> +			u32 total_bytes_per_intf;
> +
> +			/* first calculate dsc parameters and then program
> +			 * compress mode registers
> +			 */
> +			intf_width = hdisplay;
> +			slice_per_intf = DIV_ROUND_UP(intf_width, dsc->drm->slice_width);
> +
> +			/* If slice_count > slice_per_intf, then use 1
> +			 * This can happen during partial update
> +			 */
> +				dsc->drm->slice_count = 1;
> +
> +			dsc->bytes_in_slice = DIV_ROUND_UP(dsc->drm->slice_width * 8, 8);
> +			total_bytes_per_intf = dsc->bytes_in_slice * slice_per_intf;
> +
> +			dsc->eol_byte_num = total_bytes_per_intf % 3;
> +			dsc->pclk_per_line =  DIV_ROUND_UP(total_bytes_per_intf, 3);
> +			dsc->bytes_per_pkt = dsc->bytes_in_slice * dsc->drm->slice_count;
> +			dsc->pkt_per_line = slice_per_intf / dsc->drm->slice_count;
> +
> +			width = dsc->pclk_per_line;
> +			reg = dsc->bytes_per_pkt << 16;
> +			reg |= (0x0b << 8);    /* dtype of compressed image */
> +
> +			/* pkt_per_line:
> +			 * 0 == 1 pkt
> +			 * 1 == 2 pkt
> +			 * 2 == 4 pkt
> +			 * 3 pkt is not supported
> +			 * above translates to ffs() - 1
> +			 */
> +			reg |= (ffs(dsc->pkt_per_line) - 1) << 6;
> +
> +			dsc->eol_byte_num = total_bytes_per_intf % 3;
> +			reg |= dsc->eol_byte_num << 4;
> +			reg |= 1;
> +
> +			dsi_write(msm_host,
> +				  REG_DSI_VIDEO_COMPRESSION_MODE_CTRL, reg);
> +		}
> +
>  		dsi_write(msm_host, REG_DSI_ACTIVE_H,
>  			DSI_ACTIVE_H_START(ha_start) |
>  			DSI_ACTIVE_H_END(ha_end));
> @@ -993,19 +1080,50 @@ static void dsi_timing_setup(struct
> msm_dsi_host *msm_host, bool is_dual_dsi)
>  			DSI_ACTIVE_VSYNC_VPOS_START(vs_start) |
>  			DSI_ACTIVE_VSYNC_VPOS_END(vs_end));
>  	} else {		/* command mode */
> +		if (msm_host->dsc) {
> +			struct msm_display_dsc_config *dsc = msm_host->dsc;
> +			u32 reg, reg_ctrl, reg_ctrl2;
> +			u32 slice_per_intf, bytes_in_slice, total_bytes_per_intf;
> +
> +			reg_ctrl = dsi_read(msm_host, 
> REG_DSI_COMMAND_COMPRESSION_MODE_CTRL);
> +			reg_ctrl2 = dsi_read(msm_host, 
> REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2);
> +
> +			slice_per_intf = DIV_ROUND_UP(hdisplay, dsc->drm->slice_width);
> +			bytes_in_slice = DIV_ROUND_UP(dsc->drm->slice_width *
> +						      dsc->drm->bits_per_pixel, 8);
> +			dsc->drm->slice_chunk_size = bytes_in_slice;
> +			total_bytes_per_intf = dsc->bytes_in_slice * slice_per_intf;
> +			dsc->pkt_per_line = slice_per_intf / dsc->drm->slice_count;
> +
> +			reg = 0x39 << 8;
> +			reg |= ffs(dsc->pkt_per_line) << 6;
> +
> +			dsc->eol_byte_num = total_bytes_per_intf % 3;
> +			reg |= dsc->eol_byte_num << 4;
> +			reg |= 1;
> +
> +			reg_ctrl |= reg;
> +			reg_ctrl2 |= bytes_in_slice;
> +
> +			dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL, reg);
> +			dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2, 
> reg_ctrl2);
> +		}
> +
>  		/* image data and 1 byte write_memory_start cmd */
> -		wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
> +		if (!msm_host->dsc)
> +			wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
> +		else
> +			wc = mode->hdisplay / 2 + 1;
> 
> -		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL,
> -			DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
> -			DSI_CMD_MDP_STREAM0_CTRL_VIRTUAL_CHANNEL(
> -					msm_host->channel) |
> -			DSI_CMD_MDP_STREAM0_CTRL_DATA_TYPE(
> -					MIPI_DSI_DCS_LONG_WRITE));
> +		data = DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
> +		       DSI_CMD_MDP_STREAM0_CTRL_VIRTUAL_CHANNEL(msm_host->channel) |
> +			DSI_CMD_MDP_STREAM0_CTRL_DATA_TYPE(MIPI_DSI_DCS_LONG_WRITE);
> 
> -		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_TOTAL,
> -			DSI_CMD_MDP_STREAM0_TOTAL_H_TOTAL(hdisplay) |
> -			DSI_CMD_MDP_STREAM0_TOTAL_V_TOTAL(mode->vdisplay));
> +		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL, data);
> +
> +		data = DSI_CMD_MDP_STREAM0_TOTAL_H_TOTAL(hdisplay) |
> +			DSI_CMD_MDP_STREAM0_TOTAL_V_TOTAL(mode->vdisplay);
> +		dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_TOTAL, data);
>  	}
>  }
> 
> @@ -2074,6 +2192,7 @@ int msm_dsi_host_modeset_init(struct 
> mipi_dsi_host *host,
>  	struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
>  	const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
>  	struct platform_device *pdev = msm_host->pdev;
> +	struct msm_drm_private *priv;
>  	int ret;
> 
>  	msm_host->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> @@ -2093,6 +2212,9 @@ int msm_dsi_host_modeset_init(struct 
> mipi_dsi_host *host,
>  	}
> 
>  	msm_host->dev = dev;
> +	priv = dev->dev_private;
> +	priv->dsc = msm_host->dsc;
> +
>  	ret = cfg_hnd->ops->tx_buf_alloc(msm_host, SZ_4K);
>  	if (ret) {
>  		pr_err("%s: alloc tx gem obj failed, %d\n", __func__, ret);

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

* Re: [Freedreno] [PATCH 11/11] drm/msm/dsi: Pass DSC params to drm_panel
  2021-07-15  6:52 ` [PATCH 11/11] drm/msm/dsi: Pass DSC params to drm_panel Vinod Koul
@ 2021-08-03  1:22   ` abhinavk
  0 siblings, 0 replies; 41+ messages in thread
From: abhinavk @ 2021-08-03  1:22 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 2021-07-14 23:52, Vinod Koul wrote:
> When DSC is enabled, we need to pass the DSC parameters to panel driver
> as well, so add a dsc parameter in panel and set it when DSC is enabled
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>

based on the comments on prev patches in the series, this will need to 
be reworked
too as there wont be any priv->dsc anymore.

Also, can you also post the panel changes? Would like to see how you 
will use the
dsc param there.

> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 16 +++++++++++++++-
>  include/drm/drm_panel.h            |  7 +++++++
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 4e8ab1b1df8b..ee21cda243a7 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -2193,6 +2193,7 @@ int msm_dsi_host_modeset_init(struct 
> mipi_dsi_host *host,
>  	const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
>  	struct platform_device *pdev = msm_host->pdev;
>  	struct msm_drm_private *priv;
> +	struct drm_panel *panel;
>  	int ret;
> 
>  	msm_host->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> @@ -2212,8 +2213,21 @@ int msm_dsi_host_modeset_init(struct 
> mipi_dsi_host *host,
>  	}
> 
>  	msm_host->dev = dev;
> +	panel = msm_dsi_host_get_panel(&msm_host->base);
>  	priv = dev->dev_private;
> -	priv->dsc = msm_host->dsc;
> +
> +	if (panel && panel->dsc) {
> +		struct msm_display_dsc_config *dsc = priv->dsc;
> +
> +		if (!dsc) {
> +			dsc = kzalloc(sizeof(*dsc), GFP_KERNEL);
> +			if (!dsc)
> +				return -ENOMEM;
> +			dsc->drm = panel->dsc;
> +			priv->dsc = dsc;
> +			msm_host->dsc = dsc;
> +		}
> +	}
> 
>  	ret = cfg_hnd->ops->tx_buf_alloc(msm_host, SZ_4K);
>  	if (ret) {
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 33605c3f0eba..27a7808a29f2 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -171,6 +171,13 @@ struct drm_panel {
>  	 * Panel entry in registry.
>  	 */
>  	struct list_head list;
> +
> +	/**
> +	 * @dsc:
> +	 *
> +	 * Panel DSC pps payload to be sent
> +	 */
> +	struct drm_dsc_config *dsc;
>  };
> 
>  void drm_panel_init(struct drm_panel *panel, struct device *dev,

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

* Re: [Freedreno] [PATCH 01/11] drm/msm/dsi: add support for dsc data
  2021-08-02 22:55   ` [Freedreno] " abhinavk
@ 2021-10-06  5:24     ` Vinod Koul
  0 siblings, 0 replies; 41+ messages in thread
From: Vinod Koul @ 2021-10-06  5:24 UTC (permalink / raw)
  To: abhinavk
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

Hi Abhinav,

On 02-08-21, 15:55, abhinavk@codeaurora.org wrote:

> > +static int dsi_populate_dsc_params(struct msm_display_dsc_config *dsc)
> > +{
> > +	int mux_words_size;
> > +	int groups_per_line, groups_total;
> > +	int min_rate_buffer_size;
> > +	int hrd_delay;
> > +	int pre_num_extra_mux_bits, num_extra_mux_bits;
> > +	int slice_bits;
> > +	int target_bpp_x16;
> > +	int data;
> > +	int final_value, final_scale;
> > +	int i;
> > +
> > +	dsc->drm->rc_model_size = 8192;
> > +	dsc->drm->first_line_bpg_offset = 12;
> > +	dsc->drm->rc_edge_factor = 6;
> > +	dsc->drm->rc_tgt_offset_high = 3;
> > +	dsc->drm->rc_tgt_offset_low = 3;
> > +	dsc->drm->simple_422 = 0;
> > +	dsc->drm->convert_rgb = 1;
> > +	dsc->drm->vbr_enable = 0;
> > +
> > +	/* handle only bpp = bpc = 8 */
> > +	for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++)
> > +		dsc->drm->rc_buf_thresh[i] = dsi_dsc_rc_buf_thresh[i];
> > +
> > +	for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
> > +		dsc->drm->rc_range_params[i].range_min_qp = min_qp[i];
> > +		dsc->drm->rc_range_params[i].range_max_qp = max_qp[i];
> > +		dsc->drm->rc_range_params[i].range_bpg_offset = bpg_offset[i];
> > +	}
> > +
> > +	dsc->drm->initial_offset = 6144; /* Not bpp 12 */
> > +	if (dsc->drm->bits_per_pixel != 8)
> > +		dsc->drm->initial_offset = 2048;	/* bpp = 12 */
> > +
> > +	mux_words_size = 48;		/* bpc == 8/10 */
> > +	if (dsc->drm->bits_per_component == 12)
> > +		mux_words_size = 64;
> > +
> > +	dsc->drm->initial_xmit_delay = 512;
> > +	dsc->drm->initial_scale_value = 32;
> > +	dsc->drm->first_line_bpg_offset = 12;
> > +	dsc->drm->line_buf_depth = dsc->drm->bits_per_component + 1;
> > +
> > +	/* bpc 8 */
> > +	dsc->drm->flatness_min_qp = 3;
> > +	dsc->drm->flatness_max_qp = 12;
> > +	dsc->det_thresh_flatness = 7 + 2 * (dsc->drm->bits_per_component - 8);
> > +	dsc->drm->rc_quant_incr_limit0 = 11;
> > +	dsc->drm->rc_quant_incr_limit1 = 11;
> > +	dsc->drm->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC;
> > +
> > +	/* FIXME: need to call drm_dsc_compute_rc_parameters() so that rest of
> > +	 * params are calculated
> > +	 */
> > +	dsc->slice_last_group_size = 3 - (dsc->drm->slice_width % 3);
> > +	groups_per_line = DIV_ROUND_UP(dsc->drm->slice_width, 3);
> > +	dsc->drm->slice_chunk_size = dsc->drm->slice_width *
> > dsc->drm->bits_per_pixel / 8;
> > +	if ((dsc->drm->slice_width * dsc->drm->bits_per_pixel) % 8)
> > +		dsc->drm->slice_chunk_size++;
> > +
> > +	/* rbs-min */
> > +	min_rate_buffer_size =  dsc->drm->rc_model_size -
> > dsc->drm->initial_offset +
> > +				dsc->drm->initial_xmit_delay * dsc->drm->bits_per_pixel +
> > +				groups_per_line * dsc->drm->first_line_bpg_offset;
> > +
> > +	hrd_delay = DIV_ROUND_UP(min_rate_buffer_size,
> > dsc->drm->bits_per_pixel);
> > +
> > +	dsc->drm->initial_dec_delay = hrd_delay -
> > dsc->drm->initial_xmit_delay;
> > +
> > +	dsc->drm->initial_scale_value = 8 * dsc->drm->rc_model_size /
> > +				       (dsc->drm->rc_model_size - dsc->drm->initial_offset);
> > +
> > +	slice_bits = 8 * dsc->drm->slice_chunk_size * dsc->drm->slice_height;
> > +
> > +	groups_total = groups_per_line * dsc->drm->slice_height;
> > +
> > +	data = dsc->drm->first_line_bpg_offset * 2048;
> > +
> > +	dsc->drm->nfl_bpg_offset = DIV_ROUND_UP(data, (dsc->drm->slice_height
> > - 1));
> > +
> > +	pre_num_extra_mux_bits = 3 * (mux_words_size + (4 *
> > dsc->drm->bits_per_component + 4) - 2);
> > +
> > +	num_extra_mux_bits = pre_num_extra_mux_bits - (mux_words_size -
> > +			     ((slice_bits - pre_num_extra_mux_bits) % mux_words_size));
> > +
> > +	data = 2048 * (dsc->drm->rc_model_size - dsc->drm->initial_offset +
> > num_extra_mux_bits);
> > +	dsc->drm->slice_bpg_offset = DIV_ROUND_UP(data, groups_total);
> > +
> > +	/* bpp * 16 + 0.5 */
> > +	data = dsc->drm->bits_per_pixel * 16;
> > +	data *= 2;
> > +	data++;
> > +	data /= 2;
> > +	target_bpp_x16 = data;
> > +
> > +	data = (dsc->drm->initial_xmit_delay * target_bpp_x16) / 16;
> > +	final_value =  dsc->drm->rc_model_size - data + num_extra_mux_bits;
> As we discussed, can you please check why there is an additional + 8 and /16
> in the upstream final_offset calculation?
> If we can eliminate or root-cause the difference in the calculations, either
> this patch can be substantially reduced or
> we will atleast know for future reference what was the delta and can leave a
> comment.

I am checking this as well, I think there is something more, so will
continue to debug on that.

Meanwhile I propose we continue this and then switch once we have
concluded.

> > +/* DSC config */
> > +struct msm_display_dsc_config {
> > +	struct drm_dsc_config *drm;
> > +	u8 scr_rev;
> Can scr_rev also move into drm_dsc_config? SCR itself is not QC specific.
> Its just telling there was a change request
> for that DSC revision.
> In QC side, we only use this scr_rev to have some different tables. This can
> even be true for other vendors.
> So moving this to drm_dsc_config will only help.

So I checked and looks like this is not used here in the code, so will
drop it for now. Once we add support for this, we can add this back in
drm_dsc_config

-- 
~Vinod

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

* Re: [Freedreno] [PATCH 02/11] drm/msm/disp/dpu1: Add support for DSC
  2021-08-02 23:03   ` [Freedreno] " abhinavk
@ 2021-10-06  5:36     ` Vinod Koul
  0 siblings, 0 replies; 41+ messages in thread
From: Vinod Koul @ 2021-10-06  5:36 UTC (permalink / raw)
  To: abhinavk
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 02-08-21, 16:03, abhinavk@codeaurora.org wrote:
> On 2021-07-14 23:51, Vinod Koul wrote:
> > Display Stream Compression (DSC) is one of the hw blocks in dpu, so add
> > support by adding hw blocks for DSC
> > 
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > ---
> > Changes since RFC:
> >  - Drop unused enums
> > 
> >  drivers/gpu/drm/msm/Makefile                  |   1 +
> >  .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |  13 ++
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c    | 221 ++++++++++++++++++
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h    |  77 ++++++
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h   |  13 ++
> >  5 files changed, 325 insertions(+)
> >  create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
> >  create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
> > 
> > diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> > index 610d630326bb..fd8fc57f1f58 100644
> > --- a/drivers/gpu/drm/msm/Makefile
> > +++ b/drivers/gpu/drm/msm/Makefile
> > @@ -61,6 +61,7 @@ msm-y := \
> >  	disp/dpu1/dpu_hw_blk.o \
> >  	disp/dpu1/dpu_hw_catalog.o \
> >  	disp/dpu1/dpu_hw_ctl.o \
> > +	disp/dpu1/dpu_hw_dsc.o \
> >  	disp/dpu1/dpu_hw_interrupts.o \
> >  	disp/dpu1/dpu_hw_intf.o \
> >  	disp/dpu1/dpu_hw_lm.o \
> > 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 4dfd8a20ad5c..b8b4dc36880c 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> > @@ -547,6 +547,16 @@ struct dpu_merge_3d_cfg  {
> >  	const struct dpu_merge_3d_sub_blks *sblk;
> >  };
> > 
> > +/**
> > + * struct dpu_dsc_cfg - information of DSC blocks
> > + * @id                 enum identifying this block
> > + * @base               register offset of this block
> > + * @features           bit mask identifying sub-blocks/features
> > + */
> > +struct dpu_dsc_cfg {
> > +	DPU_HW_BLK_INFO;
> > +};
> > +
> >  /**
> >   * struct dpu_intf_cfg - information of timing engine blocks
> >   * @id                 enum identifying this block
> > @@ -748,6 +758,9 @@ struct dpu_mdss_cfg {
> >  	u32 merge_3d_count;
> >  	const struct dpu_merge_3d_cfg *merge_3d;
> > 
> > +	u32 dsc_count;
> > +	struct dpu_dsc_cfg *dsc;
> > +
> >  	u32 intf_count;
> >  	const struct dpu_intf_cfg *intf;
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
> > new file mode 100644
> > index 000000000000..e27e67bd42e8
> > --- /dev/null
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
> > @@ -0,0 +1,221 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2020, Linaro Limited
> Copyright year needs an update : 2020-2021?

Thanks for spotting, fixed up

-- 
~Vinod

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

* Re: [PATCH 04/11] drm/msm/disp/dpu1: Add DSC support in RM
  2021-07-29 20:23   ` Dmitry Baryshkov
@ 2021-10-06 10:26     ` Vinod Koul
  0 siblings, 0 replies; 41+ messages in thread
From: Vinod Koul @ 2021-10-06 10:26 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, linux-arm-msm, Bjorn Andersson, David Airlie,
	Daniel Vetter, Jonathan Marek, Abhinav Kumar, Jeffrey Hugo,
	Sumit Semwal, linux-kernel, dri-devel, freedreno

On 29-07-21, 23:23, Dmitry Baryshkov wrote:
> On 15/07/2021 09:51, Vinod Koul wrote:

> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> > index fd2d104f0a91..4da6d72b7996 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> > @@ -11,6 +11,7 @@
> >   #include "dpu_hw_intf.h"
> >   #include "dpu_hw_dspp.h"
> >   #include "dpu_hw_merge3d.h"
> > +#include "dpu_hw_dsc.h"
> >   #include "dpu_encoder.h"
> >   #include "dpu_trace.h"
> > @@ -75,6 +76,14 @@ int dpu_rm_destroy(struct dpu_rm *rm)
> >   			dpu_hw_intf_destroy(hw);
> >   		}
> >   	}
> > +	for (i = 0; i < ARRAY_SIZE(rm->dsc_blks); i++) {
> > +		struct dpu_hw_dsc *hw;
> > +
> > +		if (rm->intf_blks[i]) {
> 
> rm->dsc_blks[i]

Thanks for spotting, fixed!

> 
> > +			hw = to_dpu_hw_dsc(rm->dsc_blks[i]);
> > +			dpu_hw_dsc_destroy(hw);
> > +		}
> > +	}
> >   	return 0;
> >   }
> > @@ -221,6 +230,19 @@ int dpu_rm_init(struct dpu_rm *rm,
> >   		rm->dspp_blks[dspp->id - DSPP_0] = &hw->base;
> >   	}
> > +	for (i = 0; i < cat->dsc_count; i++) {
> > +		struct dpu_hw_dsc *hw;
> > +		const struct dpu_dsc_cfg *dsc = &cat->dsc[i];
> > +
> > +		hw = dpu_hw_dsc_init(dsc->id, mmio, cat);
> > +		if (IS_ERR_OR_NULL(hw)) {
> > +			rc = PTR_ERR(hw);
> > +			DPU_ERROR("failed dsc object creation: err %d\n", rc);
> > +			goto fail;
> > +		}
> > +		rm->dsc_blks[dsc->id - DSC_0] = &hw->base;
> > +	}
> > +
> >   	return 0;
> >   fail:
> > @@ -476,6 +498,9 @@ static int _dpu_rm_reserve_intf(
> >   	}
> >   	global_state->intf_to_enc_id[idx] = enc_id;
> > +
> > +	global_state->dsc_to_enc_id[0] = enc_id;
> > +	global_state->dsc_to_enc_id[1] = enc_id;
> 
> This is not correct. At least this should be guarded with an if, checking
> that DSC is requested. Also we'd need to check that DSC 0 and 1 are not
> allocated.

Correct, so I have done few changes here and for DSC block reservation..
- Calling dpu_rm_get_assigned_resources() for DSC only when DSC is
  required from dpu encoder
- moved the above code to dsc helper: _dpu_rm_reserve_dsc() as suggested
  by Abhinav as well
- Check if DSC is supported and then check if DSC 0|1 are not allocated
  and then assign as above

-- 
~Vinod

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

* Re: [Freedreno] [PATCH 04/11] drm/msm/disp/dpu1: Add DSC support in RM
  2021-08-02 23:24   ` [Freedreno] " abhinavk
@ 2021-10-06 10:27     ` Vinod Koul
  0 siblings, 0 replies; 41+ messages in thread
From: Vinod Koul @ 2021-10-06 10:27 UTC (permalink / raw)
  To: abhinavk
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 02-08-21, 16:24, abhinavk@codeaurora.org wrote:
> On 2021-07-14 23:51, Vinod Koul wrote:

> > @@ -476,6 +498,9 @@ static int _dpu_rm_reserve_intf(
> >  	}
> > 
> >  	global_state->intf_to_enc_id[idx] = enc_id;
> > +
> > +	global_state->dsc_to_enc_id[0] = enc_id;
> > +	global_state->dsc_to_enc_id[1] = enc_id;
> >  	return 0;
> >  }
> agree with dmitry again here, why are DSCs being reserved in the
> _dpu_rm_reserve_intf function?
> First, for clarity, they should be in a function of their own.
> Allocating the DSCs has to also account for the PP availability of that DSC
> and other factors need to
> be considered as well.
> I suggest checking _sde_rm_reserve_dsc() from downstream to improve the DSC
> reservation logic.

Yes I have moved to a new helper _dpu_rm_reserve_dsc(). PP availability
is already checked so no need to do that here as well

-- 
~Vinod

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

* Re: [PATCH 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog
  2021-07-29 20:25   ` Dmitry Baryshkov
@ 2021-10-06 10:50     ` Vinod Koul
  0 siblings, 0 replies; 41+ messages in thread
From: Vinod Koul @ 2021-10-06 10:50 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, linux-arm-msm, Bjorn Andersson, David Airlie,
	Daniel Vetter, Jonathan Marek, Abhinav Kumar, Jeffrey Hugo,
	Sumit Semwal, linux-kernel, dri-devel, freedreno

On 29-07-21, 23:25, Dmitry Baryshkov wrote:
> On 15/07/2021 09:51, Vinod Koul wrote:
> > This add SDM845 DSC blocks into hw_catalog
> > 
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > ---
> > Changes since RFC:
> >   - use BIT values from MASK
> > 
> >   .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    | 22 +++++++++++++++++++
> >   1 file changed, 22 insertions(+)
> > 
> > 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 b569030a0847..b45a08303c99 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > @@ -40,6 +40,8 @@
> >   #define PINGPONG_SDM845_MASK BIT(DPU_PINGPONG_DITHER)
> > +#define DSC_SDM845_MASK BIT(1)
> > +
> 
> This does not seem used. You can pass (0) as the feature mask.

Yes fixed

-- 
~Vinod

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

* Re: [Freedreno] [PATCH 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog
  2021-08-02 23:29   ` [Freedreno] " abhinavk
@ 2021-10-06 10:52     ` Vinod Koul
  0 siblings, 0 replies; 41+ messages in thread
From: Vinod Koul @ 2021-10-06 10:52 UTC (permalink / raw)
  To: abhinavk
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 02-08-21, 16:29, abhinavk@codeaurora.org wrote:
> On 2021-07-14 23:51, Vinod Koul wrote:
> > This add SDM845 DSC blocks into hw_catalog
> /add --> adds
> > 
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > ---
> > Changes since RFC:
> >  - use BIT values from MASK
> > 
> >  .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    | 22 +++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > 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 b569030a0847..b45a08303c99 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > @@ -40,6 +40,8 @@
> > 
> >  #define PINGPONG_SDM845_MASK BIT(DPU_PINGPONG_DITHER)
> > 
> > +#define DSC_SDM845_MASK BIT(1)
> agree with dmitry again :
> https://patchwork.freedesktop.org/patch/444072/?series=90413&rev=2
> this is unused. you can use .features = 0

Yes I have updated that

-- 
~Vinod

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

* Re: [PATCH 06/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl
  2021-07-29 22:15   ` Dmitry Baryshkov
@ 2021-10-06 12:21     ` Vinod Koul
  0 siblings, 0 replies; 41+ messages in thread
From: Vinod Koul @ 2021-10-06 12:21 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, linux-arm-msm, Bjorn Andersson, David Airlie,
	Daniel Vetter, Jonathan Marek, Abhinav Kumar, Jeffrey Hugo,
	Sumit Semwal, linux-kernel, dri-devel, freedreno

On 30-07-21, 01:15, Dmitry Baryshkov wrote:
> On 15/07/2021 09:51, Vinod Koul wrote:
> > Later gens of hardware have DSC bits moved to hw_ctl, so configure these
> > bits so that DSC would work there as well
> > 
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 ++++++-
> >   1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > index 2d4645e01ebf..aeea6add61ee 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > @@ -25,6 +25,8 @@
> >   #define   CTL_MERGE_3D_ACTIVE           0x0E4
> >   #define   CTL_INTF_ACTIVE               0x0F4
> >   #define   CTL_MERGE_3D_FLUSH            0x100
> > +#define   CTL_DSC_ACTIVE                0x0E8
> > +#define   CTL_DSC_FLUSH                0x104
> >   #define   CTL_INTF_FLUSH                0x110
> >   #define   CTL_INTF_MASTER               0x134
> >   #define   CTL_FETCH_PIPE_ACTIVE         0x0FC
> > @@ -34,6 +36,7 @@
> >   #define DPU_REG_RESET_TIMEOUT_US        2000
> >   #define  MERGE_3D_IDX   23
> > +#define  DSC_IDX        22
> >   #define  INTF_IDX       31
> >   #define CTL_INVALID_BIT                 0xffff
> > @@ -120,6 +123,7 @@ static u32 dpu_hw_ctl_get_pending_flush(struct dpu_hw_ctl *ctx)
> >   static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
> >   {
> > +	DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | BIT(3));
> 
> Please pass DSC indices using intf cfg and use them to configure register
> writes.

Yes I have modified the intf cfg dsc from bool to pass actual indices.
So this patch goes next (as a dependency reorder) and we use this only
when DSC is enabled and use the indices set. Thanks for the suggestion

-- 
~Vinod

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

* Re: [Freedreno] [PATCH 06/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl
  2021-08-03  0:00   ` [Freedreno] " abhinavk
@ 2021-10-06 12:21     ` Vinod Koul
  0 siblings, 0 replies; 41+ messages in thread
From: Vinod Koul @ 2021-10-06 12:21 UTC (permalink / raw)
  To: abhinavk
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 02-08-21, 17:00, abhinavk@codeaurora.org wrote:
> On 2021-07-14 23:51, Vinod Koul wrote:
> > Later gens of hardware have DSC bits moved to hw_ctl, so configure these
> > bits so that DSC would work there as well
> > 
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> Please correct me if wrong but here you seem to be flushing all the DSC bits
> even the unused ones. This will end-up enabling DSC even when DSC is unused
> on
> the newer targets.
> If so, thats wrong.
> We need to implement bit-mask based approach to avoid this change and only
> enable
> those DSCs which are used.

Yes as Dimitry suggested I have done that by passing indices

-- 
~Vinod

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

* Re: [Freedreno] [PATCH 07/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d
  2021-08-03  0:24   ` [Freedreno] " abhinavk
@ 2021-10-06 12:22     ` Vinod Koul
  0 siblings, 0 replies; 41+ messages in thread
From: Vinod Koul @ 2021-10-06 12:22 UTC (permalink / raw)
  To: abhinavk
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 02-08-21, 17:24, abhinavk@codeaurora.org wrote:
> On 2021-07-14 23:51, Vinod Koul wrote:
> > We cannot enable mode_3d when we are using the DSC. So pass
> > configuration to detect DSC is enabled and not enable mode_3d
> > when we are using DSC
> > 
> > We add a helper dpu_encoder_helper_get_dsc_mode() to detect dsc
> > enabled and pass this to .setup_intf_cfg()
> > 
> This is not entirely correct. This is true only for the 2-2-1 topology you
> are using
> on this panel.
> 
> When you are using 2-2-1, you are using 2 LMs, 2 DSCs and 1 DSI.
> So 3D mux shouldnt be used.
> 
> If you are using something like 4-2-1 or 4-2-2, then you have 4LMs,
> 2 DSCs and 2/1 DSI.
> 
> Here you need the 3D mux to convert the data from 4LMs to 2 DSCs.
> 
> So please correct the commit text here and also add a check for the
> topology.

Ack, we should mention this and modify it in future when more topology
support is added.

-- 
~Vinod

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

* Re: [PATCH 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder
  2021-07-29 20:54   ` Dmitry Baryshkov
@ 2021-10-06 12:43     ` Vinod Koul
  0 siblings, 0 replies; 41+ messages in thread
From: Vinod Koul @ 2021-10-06 12:43 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, linux-arm-msm, Bjorn Andersson, David Airlie,
	Daniel Vetter, Jonathan Marek, Abhinav Kumar, Jeffrey Hugo,
	Sumit Semwal, linux-kernel, dri-devel, freedreno

On 29-07-21, 23:54, Dmitry Baryshkov wrote:
> On 15/07/2021 09:52, Vinod Koul wrote:

> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index 8d942052db8a..41140b781e66 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -21,12 +21,17 @@
> >   #include "dpu_hw_intf.h"
> >   #include "dpu_hw_ctl.h"
> >   #include "dpu_hw_dspp.h"
> > +#include "dpu_hw_dsc.h"
> >   #include "dpu_formats.h"
> >   #include "dpu_encoder_phys.h"
> >   #include "dpu_crtc.h"
> >   #include "dpu_trace.h"
> >   #include "dpu_core_irq.h"
> > +#define DSC_MODE_SPLIT_PANEL		BIT(0)
> > +#define DSC_MODE_MULTIPLEX		BIT(1)
> > +#define DSC_MODE_VIDEO			BIT(2)
> 
> This should go into dpu_hw_dsc.h. Ah. They are already defined there and
> just redefined there. Remove the defines here.

Sure, updated

> It might be cleaner to add bool flags to struct msm_display_dsc_config and
> then calculate common mode in the dpu_hw_dsc_config().

How would that be better than calculating here? I dont see much of an
advantage.

> > +static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
> > +				     struct dpu_hw_pingpong *hw_pp,
> > +				     struct msm_display_dsc_config *dsc,
> > +				     u32 common_mode)
> > +{
> > +	if (hw_dsc->ops.dsc_config)
> > +		hw_dsc->ops.dsc_config(hw_dsc, dsc, common_mode);
> > +
> > +	if (hw_dsc->ops.dsc_config_thresh)
> > +		hw_dsc->ops.dsc_config_thresh(hw_dsc, dsc);
> > +
> > +	if (hw_pp->ops.setup_dsc)
> > +		hw_pp->ops.setup_dsc(hw_pp);
> > +
> > +	if (hw_pp->ops.enable_dsc)
> > +		hw_pp->ops.enable_dsc(hw_pp);
> 
> I think, we do not need to split these operations, I'd suggest having just
> hw_dsc->ops.dsc_config() and hw_pp->ops.enable_dsc(), merging
> dsc_config_thres() and setup_dsc() into respective methods.

Merging hw_dsc->ops.dsc_config() and hw_dsc->ops.dsc_config_thresh() would make
it from L to XL size, so lets keep them split.

We could merge the small hw_pp->ops.setup_dsc() and
hw_pp->ops.enable_dsc() though.

> >   void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
> >   {
> >   	struct dpu_encoder_virt *dpu_enc;
> >   	struct dpu_encoder_phys *phys;
> > +	struct msm_drm_private *priv;
> >   	bool needs_hw_reset = false;
> >   	unsigned int i;
> > @@ -1841,6 +1977,10 @@ void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
> >   			dpu_encoder_helper_hw_reset(dpu_enc->phys_encs[i]);
> >   		}
> >   	}
> > +
> > +	priv = drm_enc->dev->dev_private;
> > +	if (priv->dsc)
> > +		dpu_encoder_prep_dsc(dpu_enc, priv->dsc);
> 
> Not quite. This makes dsc config global, while we can have several encoders
> enabled at once (think of DSI + DP). So the dsc should be a per-encoder
> setting rather than global.

I agree it would make sense to have per-encoder. The DP part needs to be
comprehended for DSC and would need more changes. I think updating this
for DP then and making it generic as required for DP would be better,
right? In that case I will skip moving to encoder for now.

-- 
~Vinod

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

* Re: [Freedreno] [PATCH 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder
  2021-08-03  0:57   ` [Freedreno] " abhinavk
@ 2021-10-06 12:47     ` Vinod Koul
  0 siblings, 0 replies; 41+ messages in thread
From: Vinod Koul @ 2021-10-06 12:47 UTC (permalink / raw)
  To: abhinavk
  Cc: Rob Clark, Jonathan Marek, Jeffrey Hugo, David Airlie,
	linux-arm-msm, linux-kernel, Bjorn Andersson, dri-devel,
	Daniel Vetter, Dmitry Baryshkov, freedreno, Sumit Semwal

On 02-08-21, 17:57, abhinavk@codeaurora.org wrote:
> On 2021-07-14 23:52, Vinod Koul wrote:
> > When DSC is enabled in DT, we need to configure the encoder for DSC
> > configuration, calculate DSC parameters for the given timing.
> > 
> > This patch adds that support by adding dpu_encoder_prep_dsc() which is
> > invoked when DSC is enabled in DT
> correct me if wrong but this commit text is not valid anymore in my opinion.
> are there any params you are getting from DT now? I thought its all coming
> from the panel
> driver directly.

Yes thanks for spotting this, updated!

> > 
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> agree with dmitry's comment's
> https://patchwork.freedesktop.org/patch/444078/?series=90413&rev=2
> 
> instead of dsc being part of priv->dsc it should be per encoder.
> 
> On top of his comment, I also think that like on the newer chipsets, moving
> the dsc related
> encoder configuration to a dpu_encoder_dce.c will help for future expansion
> of other topologies
> and also for other compression algorithms.

As replied to Dimitry, the DP and other topology support needs to be
comprehended so this should be done when we know how DP, other
compression algorithms and other topologies would be modeled here :)

-- 
~Vinod

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

end of thread, other threads:[~2021-10-06 12:47 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15  6:51 [PATCH 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
2021-07-15  6:51 ` [PATCH 01/11] drm/msm/dsi: add support for dsc data Vinod Koul
2021-08-02 22:55   ` [Freedreno] " abhinavk
2021-10-06  5:24     ` Vinod Koul
2021-07-15  6:51 ` [PATCH 02/11] drm/msm/disp/dpu1: Add support for DSC Vinod Koul
2021-07-19  8:28   ` kernel test robot
2021-08-02 23:03   ` [Freedreno] " abhinavk
2021-10-06  5:36     ` Vinod Koul
2021-07-15  6:51 ` [PATCH 03/11] drm/msm/disp/dpu1: Add support for DSC in pingpong block Vinod Koul
2021-08-02 23:07   ` [Freedreno] " abhinavk
2021-07-15  6:51 ` [PATCH 04/11] drm/msm/disp/dpu1: Add DSC support in RM Vinod Koul
2021-07-29 20:23   ` Dmitry Baryshkov
2021-10-06 10:26     ` Vinod Koul
2021-08-02 23:24   ` [Freedreno] " abhinavk
2021-10-06 10:27     ` Vinod Koul
2021-07-15  6:51 ` [PATCH 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog Vinod Koul
2021-07-29 20:25   ` Dmitry Baryshkov
2021-10-06 10:50     ` Vinod Koul
2021-08-02 23:29   ` [Freedreno] " abhinavk
2021-10-06 10:52     ` Vinod Koul
2021-07-15  6:51 ` [PATCH 06/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl Vinod Koul
2021-07-29 22:15   ` Dmitry Baryshkov
2021-10-06 12:21     ` Vinod Koul
2021-08-03  0:00   ` [Freedreno] " abhinavk
2021-10-06 12:21     ` Vinod Koul
2021-07-15  6:51 ` [PATCH 07/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d Vinod Koul
2021-08-03  0:24   ` [Freedreno] " abhinavk
2021-10-06 12:22     ` Vinod Koul
2021-07-15  6:52 ` [PATCH 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder Vinod Koul
2021-07-19  8:54   ` kernel test robot
2021-07-29 20:54   ` Dmitry Baryshkov
2021-10-06 12:43     ` Vinod Koul
2021-08-03  0:57   ` [Freedreno] " abhinavk
2021-10-06 12:47     ` Vinod Koul
2021-07-15  6:52 ` [PATCH 09/11] drm/msm/disp/dpu1: Add support for DSC in topology Vinod Koul
2021-08-03  1:05   ` [Freedreno] " abhinavk
2021-07-15  6:52 ` [PATCH 10/11] drm/msm/dsi: Add support for DSC configuration Vinod Koul
2021-07-29 22:10   ` Dmitry Baryshkov
2021-08-03  1:16   ` [Freedreno] " abhinavk
2021-07-15  6:52 ` [PATCH 11/11] drm/msm/dsi: Pass DSC params to drm_panel Vinod Koul
2021-08-03  1:22   ` [Freedreno] " abhinavk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).