* [PATCH v2 00/11] drm/msm: Add Display Stream Compression Support
@ 2021-10-07 7:08 Vinod Koul
2021-10-07 7:08 ` [PATCH v2 01/11] drm/msm/dsi: add support for dsc data Vinod Koul
` (10 more replies)
0 siblings, 11 replies; 35+ messages in thread
From: Vinod Koul @ 2021-10-07 7:08 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 DSC data and hardware block enabling for DPU1 then
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_v2
Comments welcome!
Changes since v1:
- Fix various issues spotted by kbuildbot
- Rebase to v5.15-rc3
- Remove unused fields and duplicate defines
- Enable DSC blocks only when DSC is enabled
- remove sdm845 feature mask, use 0
- Check for DSC in hw_ctl
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: Don't use DSC with mode_3d
drm/msm/disp/dpu1: Add DSC support in hw_ctl
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 | 155 +++++++++-
.../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 | 20 ++
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 17 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 2 +
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 +
.../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 | 61 ++++
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 | 269 +++++++++++++++++-
drivers/gpu/drm/msm/msm_drv.h | 18 ++
include/drm/drm_panel.h | 7 +
20 files changed, 928 insertions(+), 6 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] 35+ messages in thread
* [PATCH v2 01/11] drm/msm/dsi: add support for dsc data
2021-10-07 7:08 [PATCH v2 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
@ 2021-10-07 7:08 ` Vinod Koul
2021-10-25 14:43 ` Dmitry Baryshkov
2021-10-07 7:08 ` [PATCH v2 02/11] drm/msm/disp/dpu1: Add support for DSC Vinod Koul
` (9 subsequent siblings)
10 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2021-10-07 7:08 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
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;
+
/* VRAM carveout, used when no IOMMU: */
struct {
unsigned long size;
--
2.31.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 02/11] drm/msm/disp/dpu1: Add support for DSC
2021-10-07 7:08 [PATCH v2 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
2021-10-07 7:08 ` [PATCH v2 01/11] drm/msm/dsi: add support for dsc data Vinod Koul
@ 2021-10-07 7:08 ` Vinod Koul
2021-10-14 14:40 ` Dmitry Baryshkov
2021-10-07 7:08 ` [PATCH v2 03/11] drm/msm/disp/dpu1: Add support for DSC in pingpong block Vinod Koul
` (8 subsequent siblings)
10 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2021-10-07 7:08 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
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));
+
+ 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;
+ 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 */
--
2.31.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 03/11] drm/msm/disp/dpu1: Add support for DSC in pingpong block
2021-10-07 7:08 [PATCH v2 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
2021-10-07 7:08 ` [PATCH v2 01/11] drm/msm/dsi: add support for dsc data Vinod Koul
2021-10-07 7:08 ` [PATCH v2 02/11] drm/msm/disp/dpu1: Add support for DSC Vinod Koul
@ 2021-10-07 7:08 ` Vinod Koul
2021-10-14 13:30 ` Dmitry Baryshkov
2021-10-07 7:08 ` [PATCH v2 04/11] drm/msm/disp/dpu1: Add DSC support in RM Vinod Koul
` (7 subsequent siblings)
10 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2021-10-07 7:08 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
Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>
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 55766c97c4c8..47c6ab6caf95 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 89d08a715c16..12758468d9ca 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_merge_3d;
--
2.31.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 04/11] drm/msm/disp/dpu1: Add DSC support in RM
2021-10-07 7:08 [PATCH v2 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
` (2 preceding siblings ...)
2021-10-07 7:08 ` [PATCH v2 03/11] drm/msm/disp/dpu1: Add support for DSC in pingpong block Vinod Koul
@ 2021-10-07 7:08 ` Vinod Koul
2021-10-14 14:11 ` Dmitry Baryshkov
2021-10-07 7:08 ` [PATCH v2 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog Vinod Koul
` (6 subsequent siblings)
10 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2021-10-07 7:08 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>
---
Changes since
v1:
- Add _dpu_rm_reserve_dsc() function which checks if DSC is enabled
- Fix to use dsc_blks
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 61 +++++++++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 1 +
3 files changed, 63 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 323a6bce9e64..da646817585d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -159,6 +159,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 f9c83d6e427a..95bdabc16280 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->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,7 @@ static int _dpu_rm_reserve_intf(
}
global_state->intf_to_enc_id[idx] = enc_id;
+
return 0;
}
@@ -500,6 +523,33 @@ static int _dpu_rm_reserve_intf_related_hw(
return ret;
}
+static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
+ struct dpu_global_state *global_state,
+ struct drm_encoder *enc)
+{
+ struct msm_drm_private *priv;
+
+ priv = enc->dev->dev_private;
+
+ if (!priv)
+ return -EIO;
+
+ /* check if DSC is supported */
+ if (!priv->dsc)
+ return 0;
+
+ /* check if DSC 0 & 1 and allocated or not */
+ if (global_state->dsc_to_enc_id[0] || global_state->dsc_to_enc_id[1]) {
+ DPU_ERROR("DSC 0|1 is already allocated\n");
+ return -EIO;
+ }
+
+ global_state->dsc_to_enc_id[0] = enc->base.id;
+ global_state->dsc_to_enc_id[1] = enc->base.id;
+
+ return 0;
+}
+
static int _dpu_rm_make_reservation(
struct dpu_rm *rm,
struct dpu_global_state *global_state,
@@ -526,6 +576,10 @@ static int _dpu_rm_make_reservation(
if (ret)
return ret;
+ ret = _dpu_rm_reserve_dsc(rm, global_state, enc);
+ if (ret)
+ return ret;
+
return ret;
}
@@ -567,6 +621,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 +696,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 related [flat|nested] 35+ messages in thread
* [PATCH v2 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog
2021-10-07 7:08 [PATCH v2 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
` (3 preceding siblings ...)
2021-10-07 7:08 ` [PATCH v2 04/11] drm/msm/disp/dpu1: Add DSC support in RM Vinod Koul
@ 2021-10-07 7:08 ` Vinod Koul
2021-10-14 13:33 ` Dmitry Baryshkov
2021-10-07 7:08 ` [PATCH v2 06/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d Vinod Koul
` (5 subsequent siblings)
10 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2021-10-07 7:08 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 adds SDM845 DSC blocks into hw_catalog
Signed-off-by: Vinod Koul <vkoul@kernel.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),
--
2.31.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 06/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d
2021-10-07 7:08 [PATCH v2 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
` (4 preceding siblings ...)
2021-10-07 7:08 ` [PATCH v2 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog Vinod Koul
@ 2021-10-07 7:08 ` Vinod Koul
2021-10-14 13:41 ` Dmitry Baryshkov
2021-10-07 7:08 ` [PATCH v2 07/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl Vinod Koul
` (4 subsequent siblings)
10 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2021-10-07 7:08 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>
---
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 */
+
+ 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) {
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;
};
--
2.31.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 07/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl
2021-10-07 7:08 [PATCH v2 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
` (5 preceding siblings ...)
2021-10-07 7:08 ` [PATCH v2 06/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d Vinod Koul
@ 2021-10-07 7:08 ` Vinod Koul
2021-10-14 14:06 ` Dmitry Baryshkov
2021-10-07 7:08 ` [PATCH v2 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder Vinod Koul
` (3 subsequent siblings)
10 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2021-10-07 7:08 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>
---
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);
}
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));
+ DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
+ }
}
static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
--
2.31.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder
2021-10-07 7:08 [PATCH v2 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
` (6 preceding siblings ...)
2021-10-07 7:08 ` [PATCH v2 07/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl Vinod Koul
@ 2021-10-07 7:08 ` Vinod Koul
2021-10-25 14:44 ` Dmitry Baryshkov
2021-10-07 7:08 ` [PATCH v2 09/11] drm/msm/disp/dpu1: Add support for DSC in topology Vinod Koul
` (2 subsequent siblings)
10 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2021-10-07 7:08 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 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);
}
void dpu_encoder_kickoff(struct drm_encoder *drm_enc)
--
2.31.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 09/11] drm/msm/disp/dpu1: Add support for DSC in topology
2021-10-07 7:08 [PATCH v2 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
` (7 preceding siblings ...)
2021-10-07 7:08 ` [PATCH v2 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder Vinod Koul
@ 2021-10-07 7:08 ` Vinod Koul
2021-10-14 14:13 ` Dmitry Baryshkov
2021-10-07 7:08 ` [PATCH v2 10/11] drm/msm/dsi: Add support for DSC configuration Vinod Koul
2021-10-07 7:09 ` [PATCH v2 11/11] drm/msm/dsi: Pass DSC params to drm_panel Vinod Koul
10 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2021-10-07 7:08 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.
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;
+ }
+
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 related [flat|nested] 35+ messages in thread
* [PATCH v2 10/11] drm/msm/dsi: Add support for DSC configuration
2021-10-07 7:08 [PATCH v2 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
` (8 preceding siblings ...)
2021-10-07 7:08 ` [PATCH v2 09/11] drm/msm/disp/dpu1: Add support for DSC in topology Vinod Koul
@ 2021-10-07 7:08 ` Vinod Koul
2021-10-14 23:18 ` Dmitry Baryshkov
2021-10-07 7:09 ` [PATCH v2 11/11] drm/msm/dsi: Pass DSC params to drm_panel Vinod Koul
10 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2021-10-07 7:08 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 | 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;
+ }
+
+ 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;
+
+ 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);
--
2.31.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 11/11] drm/msm/dsi: Pass DSC params to drm_panel
2021-10-07 7:08 [PATCH v2 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
` (9 preceding siblings ...)
2021-10-07 7:08 ` [PATCH v2 10/11] drm/msm/dsi: Add support for DSC configuration Vinod Koul
@ 2021-10-07 7:09 ` Vinod Koul
10 siblings, 0 replies; 35+ messages in thread
From: Vinod Koul @ 2021-10-07 7:09 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 86e36a3e97b6..097827a6149c 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -2192,6 +2192,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);
@@ -2211,8 +2212,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 4602f833eb51..eb8ae9bf32ed 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 related [flat|nested] 35+ messages in thread
* Re: [PATCH v2 03/11] drm/msm/disp/dpu1: Add support for DSC in pingpong block
2021-10-07 7:08 ` [PATCH v2 03/11] drm/msm/disp/dpu1: Add support for DSC in pingpong block Vinod Koul
@ 2021-10-14 13:30 ` Dmitry Baryshkov
0 siblings, 0 replies; 35+ messages in thread
From: Dmitry Baryshkov @ 2021-10-14 13:30 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 07/10/2021 10:08, Vinod Koul wrote:
> In SDM845, DSC can be enabled by writing to pingpong block registers, so
> add support for DSC in hw_pp
>
> Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.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 55766c97c4c8..47c6ab6caf95 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 89d08a715c16..12758468d9ca 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_merge_3d;
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog
2021-10-07 7:08 ` [PATCH v2 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog Vinod Koul
@ 2021-10-14 13:33 ` Dmitry Baryshkov
0 siblings, 0 replies; 35+ messages in thread
From: Dmitry Baryshkov @ 2021-10-14 13:33 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 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),
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 06/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d
2021-10-07 7:08 ` [PATCH v2 06/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d Vinod Koul
@ 2021-10-14 13:41 ` Dmitry Baryshkov
2021-10-14 13:50 ` Dmitry Baryshkov
0 siblings, 1 reply; 35+ messages in thread
From: Dmitry Baryshkov @ 2021-10-14 13:41 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 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;
> };
>
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 06/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d
2021-10-14 13:41 ` Dmitry Baryshkov
@ 2021-10-14 13:50 ` Dmitry Baryshkov
2021-10-20 6:57 ` Vinod Koul
0 siblings, 1 reply; 35+ messages in thread
From: Dmitry Baryshkov @ 2021-10-14 13:50 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 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?
>
>> +
>> + 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;
>> int stream_sel;
>> };
>>
>
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 07/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl
2021-10-07 7:08 ` [PATCH v2 07/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl Vinod Koul
@ 2021-10-14 14:06 ` Dmitry Baryshkov
2021-10-20 9:19 ` Vinod Koul
0 siblings, 1 reply; 35+ messages in thread
From: Dmitry Baryshkov @ 2021-10-14 14:06 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 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.
> }
>
> 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?
> + DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
> + }
> }
>
> static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 04/11] drm/msm/disp/dpu1: Add DSC support in RM
2021-10-07 7:08 ` [PATCH v2 04/11] drm/msm/disp/dpu1: Add DSC support in RM Vinod Koul
@ 2021-10-14 14:11 ` Dmitry Baryshkov
2021-10-19 15:30 ` Vinod Koul
0 siblings, 1 reply; 35+ messages in thread
From: Dmitry Baryshkov @ 2021-10-14 14:11 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 07/10/2021 10:08, Vinod Koul wrote:
> This add the bits in RM to enable the DSC blocks
>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
> Changes since
> v1:
> - Add _dpu_rm_reserve_dsc() function which checks if DSC is enabled
> - Fix to use dsc_blks
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 61 +++++++++++++++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 1 +
> 3 files changed, 63 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index 323a6bce9e64..da646817585d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -159,6 +159,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 f9c83d6e427a..95bdabc16280 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->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,7 @@ static int _dpu_rm_reserve_intf(
> }
>
> global_state->intf_to_enc_id[idx] = enc_id;
> +
> return 0;
> }
>
> @@ -500,6 +523,33 @@ static int _dpu_rm_reserve_intf_related_hw(
> return ret;
> }
>
> +static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
> + struct dpu_global_state *global_state,
> + struct drm_encoder *enc)
> +{
> + struct msm_drm_private *priv;
> +
> + priv = enc->dev->dev_private;
> +
> + if (!priv)
> + return -EIO;
> +
> + /* check if DSC is supported */
> + if (!priv->dsc)
> + return 0;
> +
> + /* check if DSC 0 & 1 and allocated or not */
> + if (global_state->dsc_to_enc_id[0] || global_state->dsc_to_enc_id[1]) {
> + DPU_ERROR("DSC 0|1 is already allocated\n");
> + return -EIO;
> + }
> +
> + global_state->dsc_to_enc_id[0] = enc->base.id;
> + global_state->dsc_to_enc_id[1] = enc->base.id;
Still hardcoding DSC_0 and DSC_1.
Could you please add num_dsc to the topology and allocate the requested
amount of DSC blocks? Otherwise this would break for the DSI + DP case.
> +
> + return 0;
> +}
> +
> static int _dpu_rm_make_reservation(
> struct dpu_rm *rm,
> struct dpu_global_state *global_state,
> @@ -526,6 +576,10 @@ static int _dpu_rm_make_reservation(
> if (ret)
> return ret;
>
> + ret = _dpu_rm_reserve_dsc(rm, global_state, enc);
> + if (ret)
> + return ret;
> +
> return ret;
> }
>
> @@ -567,6 +621,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 +696,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] 35+ messages in thread
* Re: [PATCH v2 09/11] drm/msm/disp/dpu1: Add support for DSC in topology
2021-10-07 7:08 ` [PATCH v2 09/11] drm/msm/disp/dpu1: Add support for DSC in topology Vinod Koul
@ 2021-10-14 14:13 ` Dmitry Baryshkov
2021-10-25 14:37 ` Dmitry Baryshkov
0 siblings, 1 reply; 35+ messages in thread
From: Dmitry Baryshkov @ 2021-10-14 14:13 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 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,
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 02/11] drm/msm/disp/dpu1: Add support for DSC
2021-10-07 7:08 ` [PATCH v2 02/11] drm/msm/disp/dpu1: Add support for DSC Vinod Koul
@ 2021-10-14 14:40 ` Dmitry Baryshkov
2021-10-19 14:07 ` Vinod Koul
0 siblings, 1 reply; 35+ messages in thread
From: Dmitry Baryshkov @ 2021-10-14 14:40 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 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 */
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 10/11] drm/msm/dsi: Add support for DSC configuration
2021-10-07 7:08 ` [PATCH v2 10/11] drm/msm/dsi: Add support for DSC configuration Vinod Koul
@ 2021-10-14 23:18 ` Dmitry Baryshkov
2021-10-20 11:53 ` Vinod Koul
0 siblings, 1 reply; 35+ messages in thread
From: Dmitry Baryshkov @ 2021-10-14 23:18 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 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);
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 02/11] drm/msm/disp/dpu1: Add support for DSC
2021-10-14 14:40 ` Dmitry Baryshkov
@ 2021-10-19 14:07 ` Vinod Koul
0 siblings, 0 replies; 35+ messages in thread
From: Vinod Koul @ 2021-10-19 14:07 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 14-10-21, 17:40, Dmitry Baryshkov wrote:
> On 07/10/2021 10:08, Vinod Koul wrote:
> > +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
Updated
> > +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.
This is called from encoder, so prefer not to have encoder invoke
dsc->drm->rc_range_params
So will keep this.
--
~Vinod
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 04/11] drm/msm/disp/dpu1: Add DSC support in RM
2021-10-14 14:11 ` Dmitry Baryshkov
@ 2021-10-19 15:30 ` Vinod Koul
2021-10-19 15:52 ` Dmitry Baryshkov
0 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2021-10-19 15:30 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 14-10-21, 17:11, Dmitry Baryshkov wrote:
> On 07/10/2021 10:08, Vinod Koul wrote:
> > +static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
> > + struct dpu_global_state *global_state,
> > + struct drm_encoder *enc)
> > +{
> > + struct msm_drm_private *priv;
> > +
> > + priv = enc->dev->dev_private;
> > +
> > + if (!priv)
> > + return -EIO;
> > +
> > + /* check if DSC is supported */
> > + if (!priv->dsc)
> > + return 0;
> > +
> > + /* check if DSC 0 & 1 and allocated or not */
> > + if (global_state->dsc_to_enc_id[0] || global_state->dsc_to_enc_id[1]) {
> > + DPU_ERROR("DSC 0|1 is already allocated\n");
> > + return -EIO;
> > + }
> > +
> > + global_state->dsc_to_enc_id[0] = enc->base.id;
> > + global_state->dsc_to_enc_id[1] = enc->base.id;
>
> Still hardcoding DSC_0 and DSC_1.
Yes!
> Could you please add num_dsc to the topology and allocate the requested
> amount of DSC blocks? Otherwise this would break for the DSI + DP case.
It wont as we check for dsc and dont proceed, so it cant make an impact
in non dsc case.
Nevertheless I agree with you, so I am making it based on dsc defined in
topology. Do we need additional field for num_dsc in topology, num_enc
should be it, right?
--
~Vinod
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 04/11] drm/msm/disp/dpu1: Add DSC support in RM
2021-10-19 15:30 ` Vinod Koul
@ 2021-10-19 15:52 ` Dmitry Baryshkov
2021-10-19 16:25 ` Vinod Koul
0 siblings, 1 reply; 35+ messages in thread
From: Dmitry Baryshkov @ 2021-10-19 15:52 UTC (permalink / raw)
To: Vinod Koul
Cc: Rob Clark, open list:DRM DRIVER FOR MSM ADRENO GPU,
Bjorn Andersson, David Airlie, Daniel Vetter, Jonathan Marek,
Abhinav Kumar, Jeffrey Hugo, Sumit Semwal, open list,
open list:DRM DRIVER FOR MSM ADRENO GPU, freedreno
On Tue, 19 Oct 2021 at 18:30, Vinod Koul <vkoul@kernel.org> wrote:
>
> On 14-10-21, 17:11, Dmitry Baryshkov wrote:
> > On 07/10/2021 10:08, Vinod Koul wrote:
>
> > > +static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
> > > + struct dpu_global_state *global_state,
> > > + struct drm_encoder *enc)
> > > +{
> > > + struct msm_drm_private *priv;
> > > +
> > > + priv = enc->dev->dev_private;
> > > +
> > > + if (!priv)
> > > + return -EIO;
> > > +
> > > + /* check if DSC is supported */
> > > + if (!priv->dsc)
> > > + return 0;
> > > +
> > > + /* check if DSC 0 & 1 and allocated or not */
> > > + if (global_state->dsc_to_enc_id[0] || global_state->dsc_to_enc_id[1]) {
> > > + DPU_ERROR("DSC 0|1 is already allocated\n");
> > > + return -EIO;
> > > + }
> > > +
> > > + global_state->dsc_to_enc_id[0] = enc->base.id;
> > > + global_state->dsc_to_enc_id[1] = enc->base.id;
> >
> > Still hardcoding DSC_0 and DSC_1.
>
> Yes!
>
> > Could you please add num_dsc to the topology and allocate the requested
> > amount of DSC blocks? Otherwise this would break for the DSI + DP case.
>
> It wont as we check for dsc and dont proceed, so it cant make an impact
> in non dsc case.
>
> Nevertheless I agree with you, so I am making it based on dsc defined in
> topology. Do we need additional field for num_dsc in topology, num_enc
> should be it, right?
I'd vote for the separate num_dsc.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 04/11] drm/msm/disp/dpu1: Add DSC support in RM
2021-10-19 15:52 ` Dmitry Baryshkov
@ 2021-10-19 16:25 ` Vinod Koul
0 siblings, 0 replies; 35+ messages in thread
From: Vinod Koul @ 2021-10-19 16:25 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, open list:DRM DRIVER FOR MSM ADRENO GPU,
Bjorn Andersson, David Airlie, Daniel Vetter, Jonathan Marek,
Abhinav Kumar, Jeffrey Hugo, Sumit Semwal, open list,
open list:DRM DRIVER FOR MSM ADRENO GPU, freedreno
On 19-10-21, 18:52, Dmitry Baryshkov wrote:
> On Tue, 19 Oct 2021 at 18:30, Vinod Koul <vkoul@kernel.org> wrote:
> >
> > On 14-10-21, 17:11, Dmitry Baryshkov wrote:
> > > On 07/10/2021 10:08, Vinod Koul wrote:
> >
> > > > +static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
> > > > + struct dpu_global_state *global_state,
> > > > + struct drm_encoder *enc)
> > > > +{
> > > > + struct msm_drm_private *priv;
> > > > +
> > > > + priv = enc->dev->dev_private;
> > > > +
> > > > + if (!priv)
> > > > + return -EIO;
> > > > +
> > > > + /* check if DSC is supported */
> > > > + if (!priv->dsc)
> > > > + return 0;
> > > > +
> > > > + /* check if DSC 0 & 1 and allocated or not */
> > > > + if (global_state->dsc_to_enc_id[0] || global_state->dsc_to_enc_id[1]) {
> > > > + DPU_ERROR("DSC 0|1 is already allocated\n");
> > > > + return -EIO;
> > > > + }
> > > > +
> > > > + global_state->dsc_to_enc_id[0] = enc->base.id;
> > > > + global_state->dsc_to_enc_id[1] = enc->base.id;
> > >
> > > Still hardcoding DSC_0 and DSC_1.
> >
> > Yes!
> >
> > > Could you please add num_dsc to the topology and allocate the requested
> > > amount of DSC blocks? Otherwise this would break for the DSI + DP case.
> >
> > It wont as we check for dsc and dont proceed, so it cant make an impact
> > in non dsc case.
> >
> > Nevertheless I agree with you, so I am making it based on dsc defined in
> > topology. Do we need additional field for num_dsc in topology, num_enc
> > should be it, right?
>
> I'd vote for the separate num_dsc.
Okay will update... will move up topology patch up in the order for that
as well
--
~Vinod
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 06/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d
2021-10-14 13:50 ` Dmitry Baryshkov
@ 2021-10-20 6:57 ` Vinod Koul
2021-10-25 14:40 ` Dmitry Baryshkov
0 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2021-10-20 6:57 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 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..
--
~Vinod
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 07/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl
2021-10-14 14:06 ` Dmitry Baryshkov
@ 2021-10-20 9:19 ` Vinod Koul
0 siblings, 0 replies; 35+ messages in thread
From: Vinod Koul @ 2021-10-20 9:19 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 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
--
~Vinod
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 10/11] drm/msm/dsi: Add support for DSC configuration
2021-10-14 23:18 ` Dmitry Baryshkov
@ 2021-10-20 11:53 ` Vinod Koul
0 siblings, 0 replies; 35+ messages in thread
From: Vinod Koul @ 2021-10-20 11:53 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 15-10-21, 02:18, Dmitry Baryshkov wrote:
> 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.
Done added a new callback for dsi_bridge to check the mode valid if DSC
is enabled
>
> > +
> > + 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.
yes, fixed that
--
~Vinod
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 09/11] drm/msm/disp/dpu1: Add support for DSC in topology
2021-10-14 14:13 ` Dmitry Baryshkov
@ 2021-10-25 14:37 ` Dmitry Baryshkov
2021-10-25 16:11 ` Vinod Koul
0 siblings, 1 reply; 35+ messages in thread
From: Dmitry Baryshkov @ 2021-10-25 14:37 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 14/10/2021 17:13, Dmitry Baryshkov wrote:
> 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.
I meant 'num_dsc', please excuse the typo.
>
>> + }
>> +
>> return topology;
>> }
>> +
>> static int dpu_encoder_virt_atomic_check(
>> struct drm_encoder *drm_enc,
>> struct drm_crtc_state *crtc_state,
>>
>
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 06/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d
2021-10-20 6:57 ` Vinod Koul
@ 2021-10-25 14:40 ` Dmitry Baryshkov
2021-10-25 16:10 ` Vinod Koul
0 siblings, 1 reply; 35+ messages in thread
From: Dmitry Baryshkov @ 2021-10-25 14:40 UTC (permalink / raw)
To: Vinod Koul
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 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:
>>>> 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..
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.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 01/11] drm/msm/dsi: add support for dsc data
2021-10-07 7:08 ` [PATCH v2 01/11] drm/msm/dsi: add support for dsc data Vinod Koul
@ 2021-10-25 14:43 ` Dmitry Baryshkov
0 siblings, 0 replies; 35+ messages in thread
From: Dmitry Baryshkov @ 2021-10-25 14:43 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 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;
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder
2021-10-07 7:08 ` [PATCH v2 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder Vinod Koul
@ 2021-10-25 14:44 ` Dmitry Baryshkov
0 siblings, 0 replies; 35+ messages in thread
From: Dmitry Baryshkov @ 2021-10-25 14:44 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 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)
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 06/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d
2021-10-25 14:40 ` Dmitry Baryshkov
@ 2021-10-25 16:10 ` Vinod Koul
2021-10-25 18:33 ` Dmitry Baryshkov
0 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2021-10-25 16:10 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 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?
--
~Vinod
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 09/11] drm/msm/disp/dpu1: Add support for DSC in topology
2021-10-25 14:37 ` Dmitry Baryshkov
@ 2021-10-25 16:11 ` Vinod Koul
0 siblings, 0 replies; 35+ messages in thread
From: Vinod Koul @ 2021-10-25 16:11 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 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
--
~Vinod
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 06/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d
2021-10-25 16:10 ` Vinod Koul
@ 2021-10-25 18:33 ` Dmitry Baryshkov
0 siblings, 0 replies; 35+ messages in thread
From: Dmitry Baryshkov @ 2021-10-25 18:33 UTC (permalink / raw)
To: Vinod Koul
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 25/10/2021 19:10, Vinod Koul wrote:
> 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?
Let dpu_encoder_get_topology() set num_dscs to 2 and merge_3d to 0 if
the encoder has DSC information and to 0 otherwise. This will cover all
topologies that we care about for now.
Regarding getting the DSC config. Currently you use single priv->dsc
pointer, which works for the simple case of single DSI output, but will
break as soon as somebody has DSC DSI + DP config. Either we can
introduce the array of DSC configs, or we can add a DSI-specific
msm_dsi_get_dsc_config(), which will be later paired with the
corresponding displayport function.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2021-10-25 18:33 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-07 7:08 [PATCH v2 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
2021-10-07 7:08 ` [PATCH v2 01/11] drm/msm/dsi: add support for dsc data Vinod Koul
2021-10-25 14:43 ` Dmitry Baryshkov
2021-10-07 7:08 ` [PATCH v2 02/11] drm/msm/disp/dpu1: Add support for DSC Vinod Koul
2021-10-14 14:40 ` Dmitry Baryshkov
2021-10-19 14:07 ` Vinod Koul
2021-10-07 7:08 ` [PATCH v2 03/11] drm/msm/disp/dpu1: Add support for DSC in pingpong block Vinod Koul
2021-10-14 13:30 ` Dmitry Baryshkov
2021-10-07 7:08 ` [PATCH v2 04/11] drm/msm/disp/dpu1: Add DSC support in RM Vinod Koul
2021-10-14 14:11 ` Dmitry Baryshkov
2021-10-19 15:30 ` Vinod Koul
2021-10-19 15:52 ` Dmitry Baryshkov
2021-10-19 16:25 ` Vinod Koul
2021-10-07 7:08 ` [PATCH v2 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog Vinod Koul
2021-10-14 13:33 ` Dmitry Baryshkov
2021-10-07 7:08 ` [PATCH v2 06/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d Vinod Koul
2021-10-14 13:41 ` Dmitry Baryshkov
2021-10-14 13:50 ` Dmitry Baryshkov
2021-10-20 6:57 ` Vinod Koul
2021-10-25 14:40 ` Dmitry Baryshkov
2021-10-25 16:10 ` Vinod Koul
2021-10-25 18:33 ` Dmitry Baryshkov
2021-10-07 7:08 ` [PATCH v2 07/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl Vinod Koul
2021-10-14 14:06 ` Dmitry Baryshkov
2021-10-20 9:19 ` Vinod Koul
2021-10-07 7:08 ` [PATCH v2 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder Vinod Koul
2021-10-25 14:44 ` Dmitry Baryshkov
2021-10-07 7:08 ` [PATCH v2 09/11] drm/msm/disp/dpu1: Add support for DSC in topology Vinod Koul
2021-10-14 14:13 ` Dmitry Baryshkov
2021-10-25 14:37 ` Dmitry Baryshkov
2021-10-25 16:11 ` Vinod Koul
2021-10-07 7:08 ` [PATCH v2 10/11] drm/msm/dsi: Add support for DSC configuration Vinod Koul
2021-10-14 23:18 ` Dmitry Baryshkov
2021-10-20 11:53 ` Vinod Koul
2021-10-07 7:09 ` [PATCH v2 11/11] drm/msm/dsi: Pass DSC params to drm_panel Vinod Koul
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).