On 07/10/2021 10:08, 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 v1:

• Drop unused fields from msm_display_dsc_config

RFC:

• Drop the DT parsing code
• Port dsc param calculation from downstream

drivers/gpu/drm/msm/dsi/dsi_host.c | 132 +++++++++++++++++++++++++++++ drivers/gpu/drm/msm/msm_drv.h | 18 ++++ 2 files changed, 150 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index e269df285136..ba24458c2e38 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -31,6 +31,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;

@@ -1748,6 +1751,135 @@ 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 8b005d1ac899..367eb1092374 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>

@@ -135,6 +136,20 @@ struct msm_drm_thread { struct kthread_worker *worker; };

+/* DSC config */ +struct msm_display_dsc_config {

• struct drm_dsc_config *drm;
• 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;

+};

• struct msm_drm_private {

  struct drm_device *dev;

@@ -229,6 +244,9 @@ struct msm_drm_private { /* Properties */ struct drm_property *plane_property[PLANE_PROP_MAX_NUM];

• /* DSC configuration */
• struct msm_display_dsc_config *dsc;

I have been looking at this again. I think that we should not put sole msm_display_dsc_config into msm_drm_private. Even if we agree that it is used only for DSI for now, we have two separate DSI channels (and up to two bridges/panels). Do you really need it in the global private data?

Anyway, this field should be moved to the patch where it is actually set or used.

/* VRAM carveout, used when no IOMMU: */ struct { unsigned long size;

On 07/10/2021 10:08, 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 v1:

• remove unused variable lp
• Update copyright year

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 | 210 ++++++++++++++++++ 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, 314 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 904535eda0c4..46c05e401d04 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -60,6 +60,7 @@ msm-y := \ disp/dpu1/dpu_formats.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 d2a945a27cfa..699c378814b1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -553,6 +553,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

@@ -757,6 +767,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..09682c4832ba --- /dev/null +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c @@ -0,0 +1,210 @@ +// SPDX-License-Identifier: GPL-2.0-only +/*

• • Copyright (c) 2020-2021, 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));

DSC_MODE_VIDEO

• 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)
  

I thought that it might make sense to pass just drm_dsc_rc_range_parameters here, but it's a matter of personal preference. I won't insist on doing that.

+{

• struct drm_dsc_rc_range_parameters *rc = dsc->drm->rc_range_params;
• struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
• u32 off;
• int i;
• 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;

+};

+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);
• return c;

+}

+void dpu_hw_dsc_destroy(struct dpu_hw_dsc *dsc) +{

• 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..648c9e4d8749 --- /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-2021, 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 bb9ceadeb0bb..b0ce8cb97d22 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 */

On 07/10/2021 10:08, Vinod Koul wrote:

This adds SDM845 DSC blocks into hw_catalog

Signed-off-by: Vinod Koul vkoul@kernel.org

Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

---

Changes since v1:

• Remove DSC_SDM845_MASK and use 0 as feature mask

.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 20 +++++++++++++++++++ 1 file changed, 20 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 b131fd376192..6423a2fe6698 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -821,6 +821,24 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = { PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, 0, sc7280_pp_sblk, -1, -1), PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1), };

+/*************************************************************

• • DSC sub blocks config
• *************************************************************/

+#define DSC_BLK(_name, _id, _base) \

• {\
• .name = _name, .id = _id, \
• .base = _base, .len = 0x140, \
• .features = 0, \
• }

+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
  *************************************************************/

@@ -1130,6 +1148,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),

On 07/10/2021 10:08, 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()

Signed-off-by: Vinod Koul vkoul@kernel.org

Changes since v1:

• Move this patch from 7 to 6
• Update the changelog
• Make dsc as int and store the DSC indices

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 e7270eb6b84b..fca07ed03317 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -332,6 +332,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 BIT(0) | BIT(1); /* Hardcoding for 2 DSC topology */
  

Please use defined values here rater than just BIT().

• return 0;

+}

• /**
  • 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 aa01698d6b25..8e5c0911734c 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 @@ -70,6 +70,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 64740ddb983e..3c79bd9c2fe5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c @@ -118,7 +118,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) {

if (ctx->pending_flush_mask & BIT(MERGE_3D_IDX)) @@ -519,7 +519,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) {

The more I think about this hunk, the more I'm unsure about it. Downstream has the following topoligies defined: * @SDE_RM_TOPOLOGY_DUALPIPE_3DMERGE_DSC: 2 LM, 2 PP, 3DMux, 1 DSC, 1 INTF/WB * @SDE_RM_TOPOLOGY_QUADPIPE_3DMERGE_DSC 4 LM, 4 PP, 3DMux, 3 DSC, 2 INTF

While the latter is not supported on sdm845, the former one should be (by the hardware). So in the driver I think we should make sure that mode_3d does not get set rather than disallowing it here.

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..5dfac5994bd4 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 BIT masks
  • @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;

• unsigned int dsc; int stream_sel; };

On 14-10-21, 16:50, Dmitry Baryshkov wrote:

On 14/10/2021 16:41, Dmitry Baryshkov wrote:

...

On 07/10/2021 10:08, 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()

Signed-off-by: Vinod Koul vkoul@kernel.org

Changes since v1:   - Move this patch from 7 to 6   - Update the changelog   - Make dsc as int and store the DSC indices

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 e7270eb6b84b..fca07ed03317 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -332,6 +332,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 BIT(0) | BIT(1); /* Hardcoding for 2 DSC topology */

Please use defined values here rater than just BIT().

Ah, it's a list of DSC blocks used. So the function name is misleading (as it's not a mode). I think we'd better pass DSC_n names here. What about using an array for cfg->dsc?

Yeah I can do better names.

...

...

+    return 0; +}

/**    * 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 aa01698d6b25..8e5c0911734c 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 @@ -70,6 +70,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 64740ddb983e..3c79bd9c2fe5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c @@ -118,7 +118,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)   {       if (ctx->pending_flush_mask & BIT(MERGE_3D_IDX)) @@ -519,7 +519,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) {

The more I think about this hunk, the more I'm unsure about it. Downstream has the following topoligies defined:  * @SDE_RM_TOPOLOGY_DUALPIPE_3DMERGE_DSC: 2 LM, 2 PP, 3DMux, 1 DSC, 1 INTF/WB  * @SDE_RM_TOPOLOGY_QUADPIPE_3DMERGE_DSC  4 LM, 4 PP, 3DMux, 3 DSC, 2 INTF

While the latter is not supported on sdm845, the former one should be (by the hardware). So in the driver I think we should make sure that mode_3d does not get set rather than disallowing it here.

...

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..5dfac5994bd4 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 BIT masks

* @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; +    unsigned int dsc;

I think this should be: enum dpu_dsc dsc[MAX_DSCS]; unsigned int num_dsc;

hmmm, how do we go about getting the num_dsc value here. dpu_encoder_phys does not know about that..

On 25-10-21, 17:40, Dmitry Baryshkov wrote:

On 20/10/2021 09:57, Vinod Koul wrote:

...

On 14-10-21, 16:50, Dmitry Baryshkov wrote:

...

On 14/10/2021 16:41, Dmitry Baryshkov wrote:

...

On 07/10/2021 10:08, Vinod Koul wrote:

...

...

...

...

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..5dfac5994bd4 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 BIT masks

* @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; +    unsigned int dsc;

I think this should be: enum dpu_dsc dsc[MAX_DSCS]; unsigned int num_dsc;

hmmm, how do we go about getting the num_dsc value here. dpu_encoder_phys does not know about that..

dpu_encoder_get_topology() can decide whether to use DSC or not and then set num_dsc. For now it will always set 2 if we are using DSC at all, but let's keep the decision in a single place rather than having it scattered all over the driver.

Yes agree, but dpu_encoder_get_topology() is private to encoder. Am not sure how best to propagate the info into the hw_intf_cfg?

On 14-10-21, 17:06, Dmitry Baryshkov wrote:

On 07/10/2021 10:08, 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

Changes since v1:

• Move this patch from 6 to 7 due to dependency on 6th one
• Use DSC indices for programming DSC registers and program only on non null indices

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)

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 3c79bd9c2fe5..8ea9d8dce3f7 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,7 +123,6 @@ static u32 dpu_hw_ctl_get_pending_flush(struct dpu_hw_ctl *ctx) static void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx) {

• if (ctx->pending_flush_mask & BIT(MERGE_3D_IDX)) DPU_REG_WRITE(&ctx->hw, CTL_MERGE_3D_FLUSH, ctx->pending_merge_3d_flush_mask);

@@ -128,7 +130,6 @@ static 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);

This would break non-DSC case.

This is a mistake, I have fixed it up now..

...

} static inline void dpu_hw_ctl_trigger_flush(struct dpu_hw_ctl *ctx) @@ -498,6 +499,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx, u32 intf_active = 0; u32 mode_sel = 0;

• if (cfg->dsc)

• DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, cfg->dsc);
  

• if (cfg->intf_mode_sel == DPU_CTL_MODE_SEL_CMD) mode_sel |= BIT(17);

@@ -509,6 +513,10 @@ 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));

• if (cfg->dsc) {

• DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask |  BIT(DSC_IDX));
  

Why?

I have fixed it up to write only DSC_IDX

On 07/10/2021 10:08, Vinod Koul wrote:

We need to configure the encoder for DSC configuration and calculate DSC parameters for the given timing so this patch adds that support by adding dpu_encoder_prep_dsc() which is invoked when DSC is enabled.

Signed-off-by: Vinod Koul vkoul@kernel.org

Changes since v1:

• Remove duplicate defines
• Update changelog

drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 139 +++++++++++++++++++- 1 file changed, 138 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 0e9d3fa1544b..aac51c1bdf94 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -21,6 +21,7 @@ #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" @@ -136,6 +137,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
    

@@ -181,6 +183,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;

@@ -977,7 +980,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) {

@@ -1035,6 +1039,13 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc, dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i]) : NULL;

• if (priv->dsc) {

• 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_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++) {

@@ -1778,10 +1789,132 @@ 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;
• 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;
• 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;

@@ -1809,6 +1942,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);
  

Again, DP + DSI case would be broken here. We'd need to tie DSC config to the encoder itself rather than having a single global DSC config.

}

void dpu_encoder_kickoff(struct drm_encoder *drm_enc)

On 07/10/2021 10:08, 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.

Also, it is better to use 2 LMs and DSC instances as half width results in lesser power consumption as compared to single LM, DSC at full width.

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 aac51c1bdf94..70f57a071165 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -538,6 +538,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++)

@@ -572,8 +574,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;
  

So, here you'd set the topology.num_rm.

• }
• return topology; }
• static int dpu_encoder_virt_atomic_check( struct drm_encoder *drm_enc, struct drm_crtc_state *crtc_state,

On 25-10-21, 17:37, Dmitry Baryshkov wrote:

On 14/10/2021 17:13, Dmitry Baryshkov wrote:

...

On 07/10/2021 10:08, Vinod Koul wrote:

...

...

@@ -572,8 +574,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;

So, here you'd set the topology.num_rm.

I meant 'num_dsc', please excuse the typo.

No worries, i had assumed such and made it num_dsc

On 07/10/2021 10:08, 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 | 123 ++++++++++++++++++++++++++++- 2 files changed, 132 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.xml.h b/drivers/gpu/drm/msm/dsi/dsi.xml.h index 49b551ad1bff..c1c85df58c4b 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.xml.h +++ b/drivers/gpu/drm/msm/dsi/dsi.xml.h @@ -706,4 +706,14 @@ static inline uint32_t DSI_VERSION_MAJOR(uint32_t val) #define REG_DSI_CPHY_MODE_CTRL 0x000002d4

+#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 ba24458c2e38..86e36a3e97b6 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -946,6 +946,26 @@ static void dsi_ctrl_config(struct msm_dsi_host *msm_host, bool enable, dsi_write(msm_host, REG_DSI_CPHY_MODE_CTRL, BIT(0)); }

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

• }

This should go to the mode_valid() callback for the dsi_bridge.

• 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_bonded_dsi) { struct drm_display_mode *mode = msm_host->mode;

@@ -978,7 +998,72 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_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;
  

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

Is the if() missing here? The indentpation and the comment seems unclear about that.

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

• 	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));

@@ -997,8 +1082,40 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_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) |

@@ -2074,6 +2191,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 +2211,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);