Display Stream Compression (DSC) compresses the display stream in host which
is later decoded by panel. This series enables this for Qualcomm msm driver.
This was tested on Google Pixel3 phone which use LGE SW43408 panel.
The changes include adding DT properties for DSC then hardware blocks support
required in DPU1 driver and support in encoder. We also add support in DSI
and introduce required topology changes.
In order for panel to set the DSC parameters we add dsc in drm_panel and set
it from the msm driver.
Complete changes which enable this for Pixel3 along with panel driver (not
part of this series) and DT changes can be found at:
git.linaro.org/people/vinod.koul/kernel.git pixel/dsc_rfc
Comments welcome!
Vinod Koul (13):
drm/dsc: Add dsc pps header init function
dt-bindings: msm/dsi: Document Display Stream Compression (DSC)
parameters
drm/msm/dsi: add support for dsc data
drm/msm/disp/dpu1: Add support for DSC
drm/msm/disp/dpu1: Add support for DSC in pingpong block
drm/msm/disp/dpu1: Add DSC support in RM
drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog
drm/msm/disp/dpu1: Add DSC support in hw_ctl
drm/msm/disp/dpu1: Don't use DSC with mode_3d
drm/msm/disp/dpu1: Add support for DSC in encoder
drm/msm/disp/dpu1: Add support for DSC in topology
drm/msm/dsi: Add support for DSC configuration
drm/msm/dsi: Pass DSC params to drm_panel
.../devicetree/bindings/display/msm/dsi.txt | 15 +
drivers/gpu/drm/drm_dsc.c | 11 +
drivers/gpu/drm/msm/Makefile | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 204 +++++++++++-
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 11 +
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 2 +
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 22 ++
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 26 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 12 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 2 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 221 +++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 79 +++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 13 +
.../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 32 ++
.../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h | 14 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 32 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 1 +
drivers/gpu/drm/msm/dsi/dsi.xml.h | 10 +
drivers/gpu/drm/msm/dsi/dsi_host.c | 293 +++++++++++++++++-
drivers/gpu/drm/msm/msm_drv.h | 32 ++
include/drm/drm_dsc.h | 16 +
include/drm/drm_panel.h | 7 +
23 files changed, 1043 insertions(+), 14 deletions(-)
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
--
2.26.3
DSC needs some configuration from device tree, add support to read and
store these params and add DSC structures in msm_drv
Signed-off-by: Vinod Koul <[email protected]>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 170 +++++++++++++++++++++++++++++
drivers/gpu/drm/msm/msm_drv.h | 32 ++++++
2 files changed, 202 insertions(+)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 8a10e4343281..864d3c655e73 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -156,6 +156,7 @@ struct msm_dsi_host {
struct regmap *sfpb;
struct drm_display_mode *mode;
+ struct msm_display_dsc_config *dsc;
/* connected device info */
struct device_node *device_node;
@@ -1744,6 +1745,168 @@ 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 i;
+
+ dsc->drm.rc_model_size = 8192;
+ dsc->drm.first_line_bpg_offset = 15;
+ 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;
+ 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;
+ 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
+ */
+
+ i = dsc->drm.slice_width % 3;
+ switch (i) {
+ case 0:
+ dsc->slice_last_group_size = 2;
+ break;
+
+ case 1:
+ dsc->slice_last_group_size = 0;
+ break;
+
+ case 2:
+ dsc->slice_last_group_size = 0;
+ break;
+
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int dsi_host_parse_dsc(struct msm_dsi_host *msm_host,
+ struct device_node *np)
+{
+ struct device *dev = &msm_host->pdev->dev;
+ struct msm_display_dsc_config *dsc;
+ bool is_dsc_enabled;
+ u32 data;
+ int ret;
+
+ is_dsc_enabled = of_property_read_bool(np, "qcom,mdss-dsc-enabled");
+
+ if (!is_dsc_enabled)
+ return 0;
+
+ dsc = kzalloc(sizeof(*dsc), GFP_KERNEL);
+ if (!dsc)
+ return -ENOMEM;
+
+ ret = of_property_read_u32(np, "qcom,mdss-dsc-version", &data);
+ if (ret) {
+ dsc->drm.dsc_version_major = 0x1;
+ dsc->drm.dsc_version_minor = 0x1;
+ } else {
+ dsc->drm.dsc_version_major = (data >> 4) & 0xf;
+ dsc->drm.dsc_version_minor = data & 0xf;
+ }
+
+ ret = of_property_read_u32(np, "qcom,mdss-scr-version", &data);
+ if (ret)
+ dsc->scr_rev = 0;
+ else
+ dsc->scr_rev = data & 0xff;
+
+ ret = of_property_read_u32(np, "qcom,mdss-slice-height", &data);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "failed to read dsc slice height\n");
+ goto err;
+ }
+ dsc->drm.slice_height = data;
+
+ ret = of_property_read_u32(np, "qcom,mdss-slice-width", &data);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "failed to read dsc slice width\n");
+ goto err;
+ }
+ dsc->drm.slice_width = data;
+
+ ret = of_property_read_u32(np, "qcom,mdss-slice-per-pkt", &data);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "failed to read mdss-slice-per-pkt\n");
+ goto err;
+ }
+ dsc->slice_per_pkt = data;
+
+ ret = of_property_read_u32(np, "qcom,mdss-bit-per-component", &data);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "failed to read mdss-bit-per-component\n");
+ goto err;
+ }
+ dsc->drm.bits_per_component = data;
+
+ ret = of_property_read_u32(np, "qcom,mdss-bit-per-pixel", &data);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "failed to read bit-per-pixel\n");
+ goto err;
+ }
+ dsc->drm.bits_per_pixel = data;
+
+ dsc->drm.block_pred_enable = of_property_read_bool(np,
+ "qcom,mdss-block-prediction-enable");
+
+ dsi_populate_dsc_params(dsc);
+
+ msm_host->dsc = dsc;
+
+ return 0;
+
+err:
+ kfree(dsc);
+ return ret;
+}
+
static int dsi_host_parse_dt(struct msm_dsi_host *msm_host)
{
struct device *dev = &msm_host->pdev->dev;
@@ -1763,6 +1926,13 @@ static int dsi_host_parse_dt(struct msm_dsi_host *msm_host)
return 0;
}
+ ret = dsi_host_parse_dsc(msm_host, np);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "%s: invalid dsc configuration %d\n", __func__, ret);
+ ret = -EINVAL;
+ goto err;
+ }
+
ret = dsi_host_parse_lane_data(msm_host, endpoint);
if (ret) {
DRM_DEV_ERROR(dev, "%s: invalid lane configuration %d\n",
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 2668941df529..26661dd43936 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>
@@ -70,6 +71,16 @@ enum msm_mdp_plane_property {
#define MSM_GPU_MAX_RINGS 4
#define MAX_H_TILES_PER_DISPLAY 2
+/**
+ * enum msm_display_compression_type - compression method used for pixel stream
+ * @MSM_DISPLAY_COMPRESSION_NONE: Pixel data is not compressed
+ * @MSM_DISPLAY_COMPRESSION_DSC: DSC compresison is used
+ */
+enum msm_display_compression_type {
+ MSM_DISPLAY_COMPRESSION_NONE,
+ MSM_DISPLAY_COMPRESSION_DSC,
+};
+
/**
* enum msm_display_caps - features/capabilities supported by displays
* @MSM_DISPLAY_CAP_VID_MODE: Video or "active" mode supported
@@ -134,6 +145,24 @@ struct msm_drm_thread {
struct kthread_worker *worker;
};
+/* DSC config */
+struct msm_display_dsc_config {
+ struct drm_dsc_config drm;
+ u8 scr_rev;
+
+ u32 initial_lines;
+ u32 pkt_per_line;
+ u32 bytes_in_slice;
+ u32 bytes_per_pkt;
+ u32 eol_byte_num;
+ u32 pclk_per_line;
+ u32 slice_last_group_size;
+ u32 slice_per_pkt;
+ u32 det_thresh_flatness;
+ u32 extra_width;
+ u32 pps_delay_ms;
+};
+
struct msm_drm_private {
struct drm_device *dev;
@@ -227,6 +256,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.26.3
In SDM845, DSC can be enabled by writing to pingpong block registers, so
add support for DSC in hw_pp
Signed-off-by: Vinod Koul <[email protected]>
---
.../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 32 +++++++++++++++++++
.../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h | 14 ++++++++
2 files changed, 46 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
index 245a7a62b5c6..07fc131ca9aa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
@@ -28,6 +28,9 @@
#define PP_FBC_MODE 0x034
#define PP_FBC_BUDGET_CTL 0x038
#define PP_FBC_LOSSY_MODE 0x03C
+#define PP_DSC_MODE 0x0a0
+#define PP_DCE_DATA_IN_SWAP 0x0ac
+#define PP_DCE_DATA_OUT_SWAP 0x0c8
#define PP_DITHER_EN 0x000
#define PP_DITHER_BITDEPTH 0x004
@@ -245,6 +248,32 @@ static u32 dpu_hw_pp_get_line_count(struct dpu_hw_pingpong *pp)
return line;
}
+static int dpu_hw_pp_dsc_enable(struct dpu_hw_pingpong *pp)
+{
+ struct dpu_hw_blk_reg_map *c = &pp->hw;
+
+ DPU_REG_WRITE(c, PP_DSC_MODE, 1);
+ return 0;
+}
+
+static void dpu_hw_pp_dsc_disable(struct dpu_hw_pingpong *pp)
+{
+ struct dpu_hw_blk_reg_map *c = &pp->hw;
+
+ DPU_REG_WRITE(c, PP_DSC_MODE, 0);
+}
+
+static int dpu_hw_pp_setup_dsc(struct dpu_hw_pingpong *pp)
+{
+ struct dpu_hw_blk_reg_map *pp_c = &pp->hw;
+ int data;
+
+ data = DPU_REG_READ(pp_c, PP_DCE_DATA_OUT_SWAP);
+ data |= BIT(18); /* endian flip */
+ DPU_REG_WRITE(pp_c, PP_DCE_DATA_OUT_SWAP, data);
+ return 0;
+}
+
static void _setup_pingpong_ops(struct dpu_hw_pingpong *c,
unsigned long features)
{
@@ -256,6 +285,9 @@ static void _setup_pingpong_ops(struct dpu_hw_pingpong *c,
c->ops.get_autorefresh = dpu_hw_pp_get_autorefresh_config;
c->ops.poll_timeout_wr_ptr = dpu_hw_pp_poll_timeout_wr_ptr;
c->ops.get_line_count = dpu_hw_pp_get_line_count;
+ c->ops.setup_dsc = dpu_hw_pp_setup_dsc;
+ c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
+ c->ops.disable_dsc = dpu_hw_pp_dsc_disable;
if (test_bit(DPU_PINGPONG_DITHER, &features))
c->ops.setup_dither = dpu_hw_pp_setup_dither;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
index 845b9ce80e31..5058e41ffbc0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
@@ -124,6 +124,20 @@ struct dpu_hw_pingpong_ops {
*/
void (*setup_dither)(struct dpu_hw_pingpong *pp,
struct dpu_hw_dither_cfg *cfg);
+ /**
+ * Enable DSC
+ */
+ int (*enable_dsc)(struct dpu_hw_pingpong *pp);
+
+ /**
+ * Disable DSC
+ */
+ void (*disable_dsc)(struct dpu_hw_pingpong *pp);
+
+ /**
+ * Setup DSC
+ */
+ int (*setup_dsc)(struct dpu_hw_pingpong *pp);
};
struct dpu_hw_pingpong {
--
2.26.3
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 <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 11 +++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 2 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 5 +++--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 2 ++
4 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index ecbc4be98980..d43b804528eb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -336,6 +336,17 @@ static inline enum dpu_3d_blend_mode dpu_encoder_helper_get_3d_blend_mode(
return BLEND_3D_NONE;
}
+static inline bool dpu_encoder_helper_get_dsc_mode(struct dpu_encoder_phys *phys_enc)
+{
+ struct drm_encoder *drm_enc = phys_enc->parent;
+ struct msm_drm_private *priv = drm_enc->dev->dev_private;
+
+ if (priv->dsc)
+ return true;
+
+ return false;
+}
+
/**
* dpu_encoder_helper_split_config - split display configuration helper function
* This helper function may be used by physical encoders to configure
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index b2be39b9144e..5fe87881c30c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -69,6 +69,8 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg(
intf_cfg.intf_mode_sel = DPU_CTL_MODE_SEL_CMD;
intf_cfg.stream_sel = cmd_enc->stream_sel;
intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
+ intf_cfg.dsc = dpu_encoder_helper_get_dsc_mode(phys_enc);
+
ctl->ops.setup_intf_cfg(ctl, &intf_cfg);
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index aeea6add61ee..f059416311ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -121,7 +121,7 @@ static u32 dpu_hw_ctl_get_pending_flush(struct dpu_hw_ctl *ctx)
return ctx->pending_flush_mask;
}
-static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
+static void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
{
DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | BIT(3));
@@ -522,7 +522,8 @@ static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
intf_cfg |= (cfg->intf & 0xF) << 4;
- if (cfg->mode_3d) {
+ /* In DSC we can't set merge, so check for dsc too */
+ if (cfg->mode_3d && !cfg->dsc) {
intf_cfg |= BIT(19);
intf_cfg |= (cfg->mode_3d - 0x1) << 20;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
index 806c171e5df2..347a653c1e01 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
@@ -39,6 +39,7 @@ struct dpu_hw_stage_cfg {
* @mode_3d: 3d mux configuration
* @merge_3d: 3d merge block used
* @intf_mode_sel: Interface mode, cmd / vid
+ * @dsc: DSC is enabled
* @stream_sel: Stream selection for multi-stream interfaces
*/
struct dpu_hw_intf_cfg {
@@ -46,6 +47,7 @@ struct dpu_hw_intf_cfg {
enum dpu_3d_blend_mode mode_3d;
enum dpu_merge_3d merge_3d;
enum dpu_ctl_mode_sel intf_mode_sel;
+ bool dsc;
int stream_sel;
};
--
2.26.3
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.
Furthermore, we can use 1 DSC encoder in lesser resulutions, but that is
not power efficient according to Abhinav, so it is recommended to always
use 2 encoders.
So for now we blindly create 2,2,1 topology when DSC is enabled
Co-developed-by: Abhinav Kumar <[email protected]>
Signed-off-by: Abhinav Kumar <[email protected]>
Signed-off-by: Vinod Koul <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 18cb1274a8bb..bffb40085c67 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -609,8 +609,22 @@ static struct msm_display_topology dpu_encoder_get_topology(
topology.num_enc = 0;
topology.num_intf = intf_count;
+ drm_enc = &dpu_enc->base;
+ priv = drm_enc->dev->dev_private;
+ if (priv && priv->dsc) {
+ /* In case of Display Stream Compression DSC, we would use
+ * 2 encoders, 2 line mixers and 1 interface
+ * this is power optimal and can drive upto (including) 4k
+ * screens
+ */
+ topology.num_enc = 2;
+ topology.num_intf = 1;
+ topology.num_lm = 2;
+ }
+
return topology;
}
+
static int dpu_encoder_virt_atomic_check(
struct drm_encoder *drm_enc,
struct drm_crtc_state *crtc_state,
--
2.26.3
When DSC is enabled in DT, we need to configure the encoder for DSC
configuration, calculate DSC parameters for the given timing.
This patch adds that support by adding dpu_encoder_prep_dsc() which is
invoked when DSC is enabled in DT
Signed-off-by: Vinod Koul <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 190 +++++++++++++++++++-
1 file changed, 189 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 8d942052db8a..18cb1274a8bb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -21,12 +21,17 @@
#include "dpu_hw_intf.h"
#include "dpu_hw_ctl.h"
#include "dpu_hw_dspp.h"
+#include "dpu_hw_dsc.h"
#include "dpu_formats.h"
#include "dpu_encoder_phys.h"
#include "dpu_crtc.h"
#include "dpu_trace.h"
#include "dpu_core_irq.h"
+#define DSC_MODE_SPLIT_PANEL BIT(0)
+#define DSC_MODE_MULTIPLEX BIT(1)
+#define DSC_MODE_VIDEO BIT(2)
+
#define DPU_DEBUG_ENC(e, fmt, ...) DPU_DEBUG("enc%d " fmt,\
(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
@@ -135,6 +140,7 @@ enum dpu_enc_rc_states {
* @cur_slave: As above but for the slave encoder.
* @hw_pp: Handle to the pingpong blocks used for the display. No.
* pingpong blocks can be different than num_phys_encs.
+ * @hw_dsc Handle to the DSC blocks used for the display.
* @intfs_swapped: Whether or not the phys_enc interfaces have been swapped
* for partial update right-only cases, such as pingpong
* split where virtual pingpong does not generate IRQs
@@ -180,6 +186,7 @@ struct dpu_encoder_virt {
struct dpu_encoder_phys *cur_master;
struct dpu_encoder_phys *cur_slave;
struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
+ struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
bool intfs_swapped;
@@ -566,6 +573,8 @@ static struct msm_display_topology dpu_encoder_get_topology(
struct drm_display_mode *mode)
{
struct msm_display_topology topology = {0};
+ struct drm_encoder *drm_enc;
+ struct msm_drm_private *priv;
int i, intf_count = 0;
for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
@@ -1008,7 +1017,8 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC] = { NULL };
- int num_lm, num_ctl, num_pp;
+ struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
+ int num_lm, num_ctl, num_pp, num_dsc;
int i, j;
if (!drm_enc) {
@@ -1061,11 +1071,16 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
drm_enc->base.id, DPU_HW_BLK_DSPP, hw_dspp,
ARRAY_SIZE(hw_dspp));
+ num_dsc = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
+ drm_enc->base.id, DPU_HW_BLK_DSC, hw_dsc, ARRAY_SIZE(hw_dsc));
for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i])
: NULL;
+ for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
+ dpu_enc->hw_dsc[i] = i < num_dsc ? to_dpu_hw_dsc(hw_dsc[i]) : NULL;
+
cstate = to_dpu_crtc_state(drm_crtc->state);
for (i = 0; i < num_lm; i++) {
@@ -1810,10 +1825,179 @@ static void dpu_encoder_vsync_event_work_handler(struct kthread_work *work)
nsecs_to_jiffies(ktime_to_ns(wakeup_time)));
}
+static int dpu_encoder_dsc_update_pic_dim(struct msm_display_dsc_config *dsc,
+ int pic_width, int pic_height)
+{
+ if (!dsc || !pic_width || !pic_height) {
+ DPU_ERROR("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)) {
+ DPU_ERROR("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
+dpu_encoder_dsc_pclk_param_calc(struct msm_display_dsc_config *dsc, u32 width)
+{
+ int slice_per_pkt, slice_per_intf;
+ int bytes_in_slice, total_bytes_per_intf;
+
+ if (!dsc || !dsc->drm.slice_width || !dsc->slice_per_pkt) {
+ DPU_ERROR("Invalid DSC/slices\n");
+ return;
+ }
+
+ slice_per_pkt = dsc->slice_per_pkt;
+ slice_per_intf = DIV_ROUND_UP(width, dsc->drm.slice_width);
+
+ /*
+ * If slice_per_pkt is greater than slice_per_intf then default to 1.
+ * This can happen during partial update.
+ */
+ if (slice_per_pkt > slice_per_intf)
+ slice_per_pkt = 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_per_pkt;
+ dsc->pkt_per_line = slice_per_intf / slice_per_pkt;
+}
+
+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 bool
+dpu_encoder_dsc_ich_reset_override_needed(struct msm_display_dsc_config *dsc, bool pu_en)
+{
+ /* As per the DSC spec, ICH_RESET can be either end of the slice line
+ * or at the end of the slice. HW internally generates ich_reset at
+ * end of the slice line if DSC_MERGE is used or encoder has two
+ * soft slices. However, if encoder has only 1 soft slice and DSC_MERGE
+ * is not used then it will generate ich_reset at the end of slice.
+ *
+ * Now as per the spec, during one PPS session, position where
+ * ich_reset is generated should not change. Now if full-screen frame
+ * has more than 1 soft slice then HW will automatically generate
+ * ich_reset at the end of slice_line. But for the same panel, if
+ * partial frame is enabled and only 1 encoder is used with 1 slice,
+ * then HW will generate ich_reset at end of the slice. This is a
+ * mismatch. Prevent this by overriding HW's decision.
+ */
+ return pu_en && dsc && (dsc->drm.slice_count > 1) &&
+ (dsc->drm.slice_width == dsc->drm.pic_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, bool ich_reset)
+{
+ if (hw_dsc->ops.dsc_config)
+ hw_dsc->ops.dsc_config(hw_dsc, dsc, common_mode, ich_reset);
+
+ 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 ich_res, dsc_common_mode;
+ int pic_width, pic_height;
+ int i;
+
+ for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
+ hw_pp[i] = dpu_enc->hw_pp[i];
+ hw_dsc[i] = dpu_enc->hw_dsc[i];
+
+ if (!hw_pp[i] || !hw_dsc[i]) {
+ DPU_ERROR_ENC(dpu_enc, "invalid params for DSC\n");
+ return;
+ }
+ }
+
+ dsc_common_mode = 0;
+ pic_width = dsc->drm.pic_width;
+ pic_height = dsc->drm.pic_height;
+
+ dpu_encoder_dsc_update_pic_dim(dsc, pic_width, pic_height);
+
+ dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL;
+ if (enc_master->intf_mode == INTF_MODE_VIDEO)
+ dsc_common_mode |= DSC_MODE_VIDEO;
+
+ this_frame_slices = pic_width / dsc->drm.slice_width;
+ intf_ip_w = this_frame_slices * dsc->drm.slice_width;
+
+ dpu_encoder_dsc_pclk_param_calc(dsc, intf_ip_w);
+
+ /*
+ * dsc merge case: when using 2 encoders for the same stream,
+ * no. of slices need to be same on both the encoders.
+ */
+ enc_ip_w = intf_ip_w / 2;
+ dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
+
+ ich_res = dpu_encoder_dsc_ich_reset_override_needed(dsc, false);
+
+ for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
+ dpu_encoder_dsc_pipe_cfg(hw_dsc[i], hw_pp[i], dsc, dsc_common_mode, ich_res);
+}
+
void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
{
struct dpu_encoder_virt *dpu_enc;
struct dpu_encoder_phys *phys;
+ struct msm_drm_private *priv;
bool needs_hw_reset = false;
unsigned int i;
@@ -1841,6 +2025,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.26.3
This add SDM845 DSC blocks into hw_catalog
Signed-off-by: Vinod Koul <[email protected]>
---
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 22 +++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index b569030a0847..1bf599e8ffe0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -40,6 +40,8 @@
#define PINGPONG_SDM845_MASK BIT(DPU_PINGPONG_DITHER)
+#define DSC_SDM845_MASK BIT(DPU_DSC)
+
#define PINGPONG_SDM845_SPLIT_MASK \
(PINGPONG_SDM845_MASK | BIT(DPU_PINGPONG_TE2))
@@ -751,6 +753,24 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = {
PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, 0, sc7280_pp_sblk),
PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk),
};
+
+/*************************************************************
+ * DSC sub blocks config
+ *************************************************************/
+#define DSC_BLK(_name, _id, _base) \
+ {\
+ .name = _name, .id = _id, \
+ .base = _base, .len = 0x140, \
+ .features = DSC_SDM845_MASK, \
+ }
+
+static struct dpu_dsc_cfg sdm845_dsc[] = {
+ DSC_BLK("dsc_0", DSC_0, 0x80000),
+ DSC_BLK("dsc_1", DSC_1, 0x80400),
+ DSC_BLK("dsc_2", DSC_2, 0x80800),
+ DSC_BLK("dsc_3", DSC_3, 0x80c00),
+};
+
/*************************************************************
* INTF sub blocks config
*************************************************************/
@@ -1053,6 +1073,8 @@ static void sdm845_cfg_init(struct dpu_mdss_cfg *dpu_cfg)
.mixer = sdm845_lm,
.pingpong_count = ARRAY_SIZE(sdm845_pp),
.pingpong = sdm845_pp,
+ .dsc_count = ARRAY_SIZE(sdm845_dsc),
+ .dsc = sdm845_dsc,
.intf_count = ARRAY_SIZE(sdm845_intf),
.intf = sdm845_intf,
.vbif_count = ARRAY_SIZE(sdm845_vbif),
--
2.26.3
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 <[email protected]>
---
drivers/gpu/drm/msm/dsi/dsi.xml.h | 10 +++
drivers/gpu/drm/msm/dsi/dsi_host.c | 118 ++++++++++++++++++++++++++---
2 files changed, 118 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.xml.h b/drivers/gpu/drm/msm/dsi/dsi.xml.h
index 50eb4d1b8fdd..b8e9e608abfc 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.xml.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.xml.h
@@ -2310,4 +2310,14 @@ static inline uint32_t REG_DSI_7nm_PHY_LN_TX_DCTRL(uint32_t i0) { return 0x00000
#define REG_DSI_7nm_PHY_PLL_PERF_OPTIMIZE 0x00000260
+#define REG_DSI_VIDEO_COMPRESSION_MODE_CTRL 0x0000029c
+
+#define REG_DSI_VIDEO_COMPRESSION_MODE_CTRL2 0x000002a0
+
+#define REG_DSI_COMMAND_COMPRESSION_MODE_CTRL 0x000002a4
+
+#define REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2 0x000002a8
+
+#define REG_DSI_COMMAND_COMPRESSION_MODE_CTRL3 0x000002ac
+
#endif /* DSI_XML */
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 864d3c655e73..e26545fc82e0 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -954,6 +954,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_dual_dsi)
u32 va_end = va_start + mode->vdisplay;
u32 hdisplay = mode->hdisplay;
u32 wc;
+ u32 data;
DBG("");
@@ -972,7 +973,69 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_dual_dsi)
hdisplay /= 2;
}
+ if (msm_host->dsc) {
+ struct msm_display_dsc_config *dsc = msm_host->dsc;
+
+ /* update dsc params with timing params */
+ dsc->drm.pic_width = mode->hdisplay;
+ dsc->drm.pic_height = mode->vdisplay;
+
+ /* Divide the display by 3 but keep back/font porch and
+ * pulse width same
+ */
+ h_total -= hdisplay;
+ hdisplay /= 3;
+ h_total += hdisplay;
+ ha_end = ha_start + hdisplay;
+ }
+
if (msm_host->mode_flags & MIPI_DSI_MODE_VIDEO) {
+ if (msm_host->dsc) {
+ struct msm_display_dsc_config *dsc = msm_host->dsc;
+ u32 reg, intf_width, slice_per_intf, width;
+ u32 total_bytes_per_intf;
+
+ /* first calculate dsc parameters and then program
+ * compress mode registers
+ */
+ intf_width = hdisplay;
+ slice_per_intf = DIV_ROUND_UP(intf_width, dsc->drm.slice_width);
+
+ /* If slice_per_pkt > slice_per_intf, then use 1
+ * This can happen during partial update
+ */
+ if (dsc->slice_per_pkt > slice_per_intf)
+ dsc->slice_per_pkt = 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->slice_per_pkt;
+ dsc->pkt_per_line = slice_per_intf / dsc->slice_per_pkt;
+
+ width = dsc->pclk_per_line;
+ reg = dsc->bytes_per_pkt << 16;
+ reg |= (0x0b << 8); /* dtype of compressed image */
+
+ /* pkt_per_line:
+ * 0 == 1 pkt
+ * 1 == 2 pkt
+ * 2 == 4 pkt
+ * 3 pkt is not supported
+ * above translates to ffs() - 1
+ */
+ reg |= (ffs(dsc->pkt_per_line) - 1) << 6;
+
+ dsc->eol_byte_num = total_bytes_per_intf % 3;
+ reg |= dsc->eol_byte_num << 4;
+ reg |= 1;
+
+ dsi_write(msm_host,
+ REG_DSI_VIDEO_COMPRESSION_MODE_CTRL, reg);
+ }
+
dsi_write(msm_host, REG_DSI_ACTIVE_H,
DSI_ACTIVE_H_START(ha_start) |
DSI_ACTIVE_H_END(ha_end));
@@ -991,19 +1054,50 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_dual_dsi)
DSI_ACTIVE_VSYNC_VPOS_START(vs_start) |
DSI_ACTIVE_VSYNC_VPOS_END(vs_end));
} else { /* command mode */
+ if (msm_host->dsc) {
+ struct msm_display_dsc_config *dsc = msm_host->dsc;
+ u32 reg, reg_ctrl, reg_ctrl2;
+ u32 slice_per_intf, bytes_in_slice, total_bytes_per_intf;
+
+ reg_ctrl = dsi_read(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL);
+ reg_ctrl2 = dsi_read(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2);
+
+ slice_per_intf = DIV_ROUND_UP(hdisplay, dsc->drm.slice_width);
+ bytes_in_slice = DIV_ROUND_UP(dsc->drm.slice_width *
+ dsc->drm.bits_per_pixel, 8);
+ dsc->drm.slice_chunk_size = bytes_in_slice;
+ total_bytes_per_intf = dsc->bytes_in_slice * slice_per_intf;
+ dsc->pkt_per_line = slice_per_intf / dsc->slice_per_pkt;
+
+ 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;
+
+ data = DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
+ DSI_CMD_MDP_STREAM0_CTRL_VIRTUAL_CHANNEL(msm_host->channel) |
+ DSI_CMD_MDP_STREAM0_CTRL_DATA_TYPE(MIPI_DSI_DCS_LONG_WRITE);
- dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL,
- DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
- DSI_CMD_MDP_STREAM0_CTRL_VIRTUAL_CHANNEL(
- msm_host->channel) |
- DSI_CMD_MDP_STREAM0_CTRL_DATA_TYPE(
- MIPI_DSI_DCS_LONG_WRITE));
+ dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL, data);
- dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_TOTAL,
- DSI_CMD_MDP_STREAM0_TOTAL_H_TOTAL(hdisplay) |
- DSI_CMD_MDP_STREAM0_TOTAL_V_TOTAL(mode->vdisplay));
+ data = DSI_CMD_MDP_STREAM0_TOTAL_H_TOTAL(hdisplay) |
+ DSI_CMD_MDP_STREAM0_TOTAL_V_TOTAL(mode->vdisplay);
+ dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_TOTAL, data);
}
}
@@ -2111,6 +2205,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);
@@ -2130,6 +2225,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.26.3
In SDM845, DSC can be enabled by writing to pingpong block registers, so
add support for DSC in hw_pp
Signed-off-by: Vinod Koul <[email protected]>
---
.../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 32 +++++++++++++++++++
.../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h | 14 ++++++++
2 files changed, 46 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
index 245a7a62b5c6..07fc131ca9aa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
@@ -28,6 +28,9 @@
#define PP_FBC_MODE 0x034
#define PP_FBC_BUDGET_CTL 0x038
#define PP_FBC_LOSSY_MODE 0x03C
+#define PP_DSC_MODE 0x0a0
+#define PP_DCE_DATA_IN_SWAP 0x0ac
+#define PP_DCE_DATA_OUT_SWAP 0x0c8
#define PP_DITHER_EN 0x000
#define PP_DITHER_BITDEPTH 0x004
@@ -245,6 +248,32 @@ static u32 dpu_hw_pp_get_line_count(struct dpu_hw_pingpong *pp)
return line;
}
+static int dpu_hw_pp_dsc_enable(struct dpu_hw_pingpong *pp)
+{
+ struct dpu_hw_blk_reg_map *c = &pp->hw;
+
+ DPU_REG_WRITE(c, PP_DSC_MODE, 1);
+ return 0;
+}
+
+static void dpu_hw_pp_dsc_disable(struct dpu_hw_pingpong *pp)
+{
+ struct dpu_hw_blk_reg_map *c = &pp->hw;
+
+ DPU_REG_WRITE(c, PP_DSC_MODE, 0);
+}
+
+static int dpu_hw_pp_setup_dsc(struct dpu_hw_pingpong *pp)
+{
+ struct dpu_hw_blk_reg_map *pp_c = &pp->hw;
+ int data;
+
+ data = DPU_REG_READ(pp_c, PP_DCE_DATA_OUT_SWAP);
+ data |= BIT(18); /* endian flip */
+ DPU_REG_WRITE(pp_c, PP_DCE_DATA_OUT_SWAP, data);
+ return 0;
+}
+
static void _setup_pingpong_ops(struct dpu_hw_pingpong *c,
unsigned long features)
{
@@ -256,6 +285,9 @@ static void _setup_pingpong_ops(struct dpu_hw_pingpong *c,
c->ops.get_autorefresh = dpu_hw_pp_get_autorefresh_config;
c->ops.poll_timeout_wr_ptr = dpu_hw_pp_poll_timeout_wr_ptr;
c->ops.get_line_count = dpu_hw_pp_get_line_count;
+ c->ops.setup_dsc = dpu_hw_pp_setup_dsc;
+ c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
+ c->ops.disable_dsc = dpu_hw_pp_dsc_disable;
if (test_bit(DPU_PINGPONG_DITHER, &features))
c->ops.setup_dither = dpu_hw_pp_setup_dither;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
index 845b9ce80e31..5058e41ffbc0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
@@ -124,6 +124,20 @@ struct dpu_hw_pingpong_ops {
*/
void (*setup_dither)(struct dpu_hw_pingpong *pp,
struct dpu_hw_dither_cfg *cfg);
+ /**
+ * Enable DSC
+ */
+ int (*enable_dsc)(struct dpu_hw_pingpong *pp);
+
+ /**
+ * Disable DSC
+ */
+ void (*disable_dsc)(struct dpu_hw_pingpong *pp);
+
+ /**
+ * Setup DSC
+ */
+ int (*setup_dsc)(struct dpu_hw_pingpong *pp);
};
struct dpu_hw_pingpong {
--
2.26.3
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 <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 11 +++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 2 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 5 +++--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 2 ++
4 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index ecbc4be98980..d43b804528eb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -336,6 +336,17 @@ static inline enum dpu_3d_blend_mode dpu_encoder_helper_get_3d_blend_mode(
return BLEND_3D_NONE;
}
+static inline bool dpu_encoder_helper_get_dsc_mode(struct dpu_encoder_phys *phys_enc)
+{
+ struct drm_encoder *drm_enc = phys_enc->parent;
+ struct msm_drm_private *priv = drm_enc->dev->dev_private;
+
+ if (priv->dsc)
+ return true;
+
+ return false;
+}
+
/**
* dpu_encoder_helper_split_config - split display configuration helper function
* This helper function may be used by physical encoders to configure
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index b2be39b9144e..5fe87881c30c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -69,6 +69,8 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg(
intf_cfg.intf_mode_sel = DPU_CTL_MODE_SEL_CMD;
intf_cfg.stream_sel = cmd_enc->stream_sel;
intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
+ intf_cfg.dsc = dpu_encoder_helper_get_dsc_mode(phys_enc);
+
ctl->ops.setup_intf_cfg(ctl, &intf_cfg);
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index aeea6add61ee..f059416311ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -121,7 +121,7 @@ static u32 dpu_hw_ctl_get_pending_flush(struct dpu_hw_ctl *ctx)
return ctx->pending_flush_mask;
}
-static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
+static void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
{
DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | BIT(3));
@@ -522,7 +522,8 @@ static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
intf_cfg |= (cfg->intf & 0xF) << 4;
- if (cfg->mode_3d) {
+ /* In DSC we can't set merge, so check for dsc too */
+ if (cfg->mode_3d && !cfg->dsc) {
intf_cfg |= BIT(19);
intf_cfg |= (cfg->mode_3d - 0x1) << 20;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
index 806c171e5df2..347a653c1e01 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
@@ -39,6 +39,7 @@ struct dpu_hw_stage_cfg {
* @mode_3d: 3d mux configuration
* @merge_3d: 3d merge block used
* @intf_mode_sel: Interface mode, cmd / vid
+ * @dsc: DSC is enabled
* @stream_sel: Stream selection for multi-stream interfaces
*/
struct dpu_hw_intf_cfg {
@@ -46,6 +47,7 @@ struct dpu_hw_intf_cfg {
enum dpu_3d_blend_mode mode_3d;
enum dpu_merge_3d merge_3d;
enum dpu_ctl_mode_sel intf_mode_sel;
+ bool dsc;
int stream_sel;
};
--
2.26.3
DSC enables streams to be compressed before we send to panel. This
requires DSC enabled encoder and a panel to be present. So we add this
information in board DTS and find if DSC can be enabled and the
parameters required to configure DSC are added to binding document along
with example
Signed-off-by: Vinod Koul <[email protected]>
---
.../devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt
index b9a64d3ff184..83d2fb92267e 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi.txt
+++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
@@ -48,6 +48,13 @@ Optional properties:
- pinctrl-n: the "sleep" pinctrl state
- ports: contains DSI controller input and output ports as children, each
containing one endpoint subnode.
+- qcom,mdss-dsc-enabled: Display Stream Compression (DSC) is enabled
+- qcom,mdss-slice-height: DSC slice height in pixels
+- qcom,mdss-slice-width: DSC slice width in pixels
+- qcom,mdss-slice-per-pkt: DSC slices per packet
+- qcom,mdss-bit-per-component: DSC bits per component
+- qcom,mdss-bit-per-pixel: DSC bits per pixel
+- qcom,mdss-block-prediction-enable: Block prediction mode of DSC enabled
DSI Endpoint properties:
- remote-endpoint: For port@0, set to phandle of the connected panel/bridge's
@@ -188,6 +195,14 @@ Example:
qcom,master-dsi;
qcom,sync-dual-dsi;
+ qcom,mdss-dsc-enabled;
+ qcom,mdss-slice-height = <16>;
+ qcom,mdss-slice-width = <540>;
+ qcom,mdss-slice-per-pkt = <1>;
+ qcom,mdss-bit-per-component = <8>;
+ qcom,mdss-bit-per-pixel = <8>;
+ qcom,mdss-block-prediction-enable;
+
qcom,mdss-mdp-transfer-time-us = <12000>;
pinctrl-names = "default", "sleep";
--
2.26.3
This add the bits in RM to enable the DSC blocks
Signed-off-by: Vinod Koul <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 32 +++++++++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 1 +
3 files changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index d6717d6672f7..d56c05146dfe 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -165,6 +165,7 @@ struct dpu_global_state {
uint32_t ctl_to_enc_id[CTL_MAX - CTL_0];
uint32_t intf_to_enc_id[INTF_MAX - INTF_0];
uint32_t dspp_to_enc_id[DSPP_MAX - DSPP_0];
+ uint32_t dsc_to_enc_id[DSC_MAX - DSC_0];
};
struct dpu_global_state
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index fd2d104f0a91..4da6d72b7996 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -11,6 +11,7 @@
#include "dpu_hw_intf.h"
#include "dpu_hw_dspp.h"
#include "dpu_hw_merge3d.h"
+#include "dpu_hw_dsc.h"
#include "dpu_encoder.h"
#include "dpu_trace.h"
@@ -75,6 +76,14 @@ int dpu_rm_destroy(struct dpu_rm *rm)
dpu_hw_intf_destroy(hw);
}
}
+ for (i = 0; i < ARRAY_SIZE(rm->dsc_blks); i++) {
+ struct dpu_hw_dsc *hw;
+
+ if (rm->intf_blks[i]) {
+ hw = to_dpu_hw_dsc(rm->dsc_blks[i]);
+ dpu_hw_dsc_destroy(hw);
+ }
+ }
return 0;
}
@@ -221,6 +230,19 @@ int dpu_rm_init(struct dpu_rm *rm,
rm->dspp_blks[dspp->id - DSPP_0] = &hw->base;
}
+ for (i = 0; i < cat->dsc_count; i++) {
+ struct dpu_hw_dsc *hw;
+ const struct dpu_dsc_cfg *dsc = &cat->dsc[i];
+
+ hw = dpu_hw_dsc_init(dsc->id, mmio, cat);
+ if (IS_ERR_OR_NULL(hw)) {
+ rc = PTR_ERR(hw);
+ DPU_ERROR("failed dsc object creation: err %d\n", rc);
+ goto fail;
+ }
+ rm->dsc_blks[dsc->id - DSC_0] = &hw->base;
+ }
+
return 0;
fail:
@@ -476,6 +498,9 @@ static int _dpu_rm_reserve_intf(
}
global_state->intf_to_enc_id[idx] = enc_id;
+
+ global_state->dsc_to_enc_id[0] = enc_id;
+ global_state->dsc_to_enc_id[1] = enc_id;
return 0;
}
@@ -567,6 +592,8 @@ void dpu_rm_release(struct dpu_global_state *global_state,
ARRAY_SIZE(global_state->ctl_to_enc_id), enc->base.id);
_dpu_rm_clear_mapping(global_state->intf_to_enc_id,
ARRAY_SIZE(global_state->intf_to_enc_id), enc->base.id);
+ _dpu_rm_clear_mapping(global_state->dsc_to_enc_id,
+ ARRAY_SIZE(global_state->dsc_to_enc_id), enc->base.id);
}
int dpu_rm_reserve(
@@ -640,6 +667,11 @@ int dpu_rm_get_assigned_resources(struct dpu_rm *rm,
hw_to_enc_id = global_state->dspp_to_enc_id;
max_blks = ARRAY_SIZE(rm->dspp_blks);
break;
+ case DPU_HW_BLK_DSC:
+ hw_blks = rm->dsc_blks;
+ hw_to_enc_id = global_state->dsc_to_enc_id;
+ max_blks = ARRAY_SIZE(rm->dsc_blks);
+ break;
default:
DPU_ERROR("blk type %d not managed by rm\n", type);
return 0;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index 1f12c8d5b8aa..278d2a510b80 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -30,6 +30,7 @@ struct dpu_rm {
struct dpu_hw_blk *intf_blks[INTF_MAX - INTF_0];
struct dpu_hw_blk *dspp_blks[DSPP_MAX - DSPP_0];
struct dpu_hw_blk *merge_3d_blks[MERGE_3D_MAX - MERGE_3D_0];
+ struct dpu_hw_blk *dsc_blks[DSC_MAX - DSC_0];
uint32_t lm_max_width;
};
--
2.26.3
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 <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 2d4645e01ebf..aeea6add61ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -25,6 +25,8 @@
#define CTL_MERGE_3D_ACTIVE 0x0E4
#define CTL_INTF_ACTIVE 0x0F4
#define CTL_MERGE_3D_FLUSH 0x100
+#define CTL_DSC_ACTIVE 0x0E8
+#define CTL_DSC_FLUSH 0x104
#define CTL_INTF_FLUSH 0x110
#define CTL_INTF_MASTER 0x134
#define CTL_FETCH_PIPE_ACTIVE 0x0FC
@@ -34,6 +36,7 @@
#define DPU_REG_RESET_TIMEOUT_US 2000
#define MERGE_3D_IDX 23
+#define DSC_IDX 22
#define INTF_IDX 31
#define CTL_INVALID_BIT 0xffff
@@ -120,6 +123,7 @@ static u32 dpu_hw_ctl_get_pending_flush(struct dpu_hw_ctl *ctx)
static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
{
+ DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | BIT(3));
if (ctx->pending_flush_mask & BIT(MERGE_3D_IDX))
DPU_REG_WRITE(&ctx->hw, CTL_MERGE_3D_FLUSH,
@@ -128,7 +132,7 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
DPU_REG_WRITE(&ctx->hw, CTL_INTF_FLUSH,
ctx->pending_intf_flush_mask);
- DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
+ DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask | BIT(DSC_IDX));
}
static inline void dpu_hw_ctl_trigger_flush(struct dpu_hw_ctl *ctx)
@@ -507,6 +511,7 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
if (cfg->merge_3d)
DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
BIT(cfg->merge_3d - MERGE_3D_0));
+ DPU_REG_WRITE(c, CTL_DSC_ACTIVE, BIT(0) | BIT(1) | BIT(2) | BIT(3));
}
static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
--
2.26.3
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 <[email protected]>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 5 +++++
include/drm/drm_panel.h | 7 +++++++
2 files changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index e26545fc82e0..7fc7002eda78 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1700,6 +1700,7 @@ static int dsi_host_attach(struct mipi_dsi_host *host,
struct mipi_dsi_device *dsi)
{
struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
+ struct drm_panel *panel;
int ret;
if (dsi->lanes > msm_host->num_data_lanes)
@@ -1719,6 +1720,10 @@ static int dsi_host_attach(struct mipi_dsi_host *host,
if (msm_host->dev)
queue_work(msm_host->workqueue, &msm_host->hpd_work);
+ panel = msm_dsi_host_get_panel(host);
+ if (panel)
+ panel->dsc = &msm_host->dsc->drm;
+
return 0;
}
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 33605c3f0eba..27a7808a29f2 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -171,6 +171,13 @@ struct drm_panel {
* Panel entry in registry.
*/
struct list_head list;
+
+ /**
+ * @dsc:
+ *
+ * Panel DSC pps payload to be sent
+ */
+ struct drm_dsc_config *dsc;
};
void drm_panel_init(struct drm_panel *panel, struct device *dev,
--
2.26.3
On Fri, May 21, 2021 at 7:50 AM Vinod Koul <[email protected]> wrote:
>
> DSC enables streams to be compressed before we send to panel. This
> requires DSC enabled encoder and a panel to be present. So we add this
> information in board DTS and find if DSC can be enabled and the
> parameters required to configure DSC are added to binding document along
> with example
>
> Signed-off-by: Vinod Koul <[email protected]>
> ---
> .../devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
This is getting converted to schema. Hopefully, v17 will be it. Sigh.
Rob
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 <[email protected]>
---
drivers/gpu/drm/msm/Makefile | 1 +
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 26 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 221 ++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 79 +++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 13 ++
5 files changed, 340 insertions(+)
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 610d630326bb..fd8fc57f1f58 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -61,6 +61,7 @@ msm-y := \
disp/dpu1/dpu_hw_blk.o \
disp/dpu1/dpu_hw_catalog.o \
disp/dpu1/dpu_hw_ctl.o \
+ disp/dpu1/dpu_hw_dsc.o \
disp/dpu1/dpu_hw_interrupts.o \
disp/dpu1/dpu_hw_intf.o \
disp/dpu1/dpu_hw_lm.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 4dfd8a20ad5c..a699633f7013 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -165,6 +165,7 @@ enum {
* @DPU_PINGPONG_TE2 Additional tear check block for split pipes
* @DPU_PINGPONG_SPLIT PP block supports split fifo
* @DPU_PINGPONG_SLAVE PP block is a suitable slave for split fifo
+ * @DPU_PINGPONG_DSC Display stream compression blocks
* @DPU_PINGPONG_DITHER, Dither blocks
* @DPU_PINGPONG_MAX
*/
@@ -173,10 +174,21 @@ enum {
DPU_PINGPONG_TE2,
DPU_PINGPONG_SPLIT,
DPU_PINGPONG_SLAVE,
+ DPU_PINGPONG_DSC,
DPU_PINGPONG_DITHER,
DPU_PINGPONG_MAX
};
+/**
+ * DSC sub-blocks
+ * @DPU_DSC DSC sub block
+ * @DPU_DSC_MAX
+ */
+enum {
+ DPU_DSC = 0x1,
+ DPU_DSC_MAX
+};
+
/**
* CTL sub-blocks
* @DPU_CTL_SPLIT_DISPLAY CTL supports video mode split display
@@ -413,6 +425,7 @@ struct dpu_dspp_sub_blks {
struct dpu_pingpong_sub_blks {
struct dpu_pp_blk te;
struct dpu_pp_blk te2;
+ struct dpu_pp_blk dsc;
struct dpu_pp_blk dither;
};
@@ -547,6 +560,16 @@ struct dpu_merge_3d_cfg {
const struct dpu_merge_3d_sub_blks *sblk;
};
+/**
+ * struct dpu_dsc_cfg - information of DSC blocks
+ * @id enum identifying this block
+ * @base register offset of this block
+ * @features bit mask identifying sub-blocks/features
+ */
+struct dpu_dsc_cfg {
+ DPU_HW_BLK_INFO;
+};
+
/**
* struct dpu_intf_cfg - information of timing engine blocks
* @id enum identifying this block
@@ -748,6 +771,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..8b8d0553709d
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
@@ -0,0 +1,221 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#include "dpu_kms.h"
+#include "dpu_hw_catalog.h"
+#include "dpu_hwio.h"
+#include "dpu_hw_mdss.h"
+#include "dpu_hw_dsc.h"
+
+#define DSC_COMMON_MODE 0x000
+#define DSC_ENC 0X004
+#define DSC_PICTURE 0x008
+#define DSC_SLICE 0x00C
+#define DSC_CHUNK_SIZE 0x010
+#define DSC_DELAY 0x014
+#define DSC_SCALE_INITIAL 0x018
+#define DSC_SCALE_DEC_INTERVAL 0x01C
+#define DSC_SCALE_INC_INTERVAL 0x020
+#define DSC_FIRST_LINE_BPG_OFFSET 0x024
+#define DSC_BPG_OFFSET 0x028
+#define DSC_DSC_OFFSET 0x02C
+#define DSC_FLATNESS 0x030
+#define DSC_RC_MODEL_SIZE 0x034
+#define DSC_RC 0x038
+#define DSC_RC_BUF_THRESH 0x03C
+#define DSC_RANGE_MIN_QP 0x074
+#define DSC_RANGE_MAX_QP 0x0B0
+#define DSC_RANGE_BPG_OFFSET 0x0EC
+
+static void dpu_hw_dsc_disable(struct dpu_hw_dsc *dsc)
+{
+ struct dpu_hw_blk_reg_map *c = &dsc->hw;
+
+ DPU_REG_WRITE(c, DSC_COMMON_MODE, 0);
+}
+
+static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc,
+ struct msm_display_dsc_config *dsc,
+ u32 mode, bool ich_reset_override)
+{
+ struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
+ u32 data;
+ u32 initial_lines = dsc->initial_lines;
+ bool is_cmd_mode = !(mode & BIT(2));
+
+ DPU_REG_WRITE(c, DSC_COMMON_MODE, mode);
+
+ data = 0;
+ if (ich_reset_override)
+ data = 3 << 28;
+
+ if (is_cmd_mode)
+ initial_lines += 1;
+
+ data |= (initial_lines << 20);
+ data |= ((dsc->slice_last_group_size) << 18);
+ /* bpp is 6.4 format, 4 LSBs bits are for fractional part */
+ data |= dsc->drm.bits_per_pixel << 12;
+ 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);
+ if (dsc->drm.bits_per_component == 10)
+ data |= BIT(0);
+
+ 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 = DIV_ROUND_UP(dsc->drm.slice_width * dsc->drm.bits_per_pixel, 8) << 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 = 0x00000184; /* only this value works */
+ 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 = 0x0;
+ u16 *lp;
+ int i;
+
+ lp = dsc->drm.rc_buf_thresh;
+ off = DSC_RC_BUF_THRESH;
+ for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++) {
+ DPU_REG_WRITE(c, off, *lp++);
+ off += 4;
+ }
+
+ off = DSC_RANGE_MIN_QP;
+ for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
+ DPU_REG_WRITE(c, off, rc[i].range_min_qp);
+ off += 4;
+ }
+
+ off = DSC_RANGE_MAX_QP;
+ for (i = 0; i < 15; i++) {
+ DPU_REG_WRITE(c, off, rc[i].range_max_qp);
+ off += 4;
+ }
+
+ off = DSC_RANGE_BPG_OFFSET;
+ for (i = 0; i < 15; i++) {
+ DPU_REG_WRITE(c, off, rc[i].range_bpg_offset);
+ off += 4;
+ }
+}
+
+static struct dpu_dsc_cfg *_dsc_offset(enum dpu_dsc dsc,
+ struct dpu_mdss_cfg *m,
+ void __iomem *addr,
+ struct dpu_hw_blk_reg_map *b)
+{
+ int i;
+
+ for (i = 0; i < m->dsc_count; i++) {
+ if (dsc == m->dsc[i].id) {
+ b->base_off = addr;
+ b->blk_off = m->dsc[i].base;
+ b->length = m->dsc[i].len;
+ b->hwversion = m->hwversion;
+ b->log_mask = DPU_DBG_MASK_DSC;
+ return &m->dsc[i];
+ }
+ }
+
+ return NULL;
+}
+
+static void _setup_dsc_ops(struct dpu_hw_dsc_ops *ops,
+ unsigned long cap)
+{
+ ops->dsc_disable = dpu_hw_dsc_disable;
+ ops->dsc_config = dpu_hw_dsc_config;
+ ops->dsc_config_thresh = dpu_hw_dsc_config_thresh;
+};
+
+static struct dpu_hw_blk_ops dpu_hw_ops = {
+ .start = NULL,
+ .stop = NULL,
+};
+
+struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc idx, void __iomem *addr,
+ struct dpu_mdss_cfg *m)
+{
+ struct dpu_hw_dsc *c;
+ struct dpu_dsc_cfg *cfg;
+
+ c = kzalloc(sizeof(*c), GFP_KERNEL);
+ if (!c)
+ return ERR_PTR(-ENOMEM);
+
+ cfg = _dsc_offset(idx, m, addr, &c->hw);
+ if (IS_ERR_OR_NULL(cfg)) {
+ kfree(c);
+ return ERR_PTR(-EINVAL);
+ }
+
+ c->idx = idx;
+ c->caps = cfg;
+ _setup_dsc_ops(&c->ops, c->caps->features);
+
+ dpu_hw_blk_init(&c->base, DPU_HW_BLK_DSC, idx, &dpu_hw_ops);
+
+ return c;
+}
+
+void dpu_hw_dsc_destroy(struct dpu_hw_dsc *dsc)
+{
+ if (dsc)
+ dpu_hw_blk_destroy(&dsc->base);
+ kfree(dsc);
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
new file mode 100644
index 000000000000..c680fd948865
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
@@ -0,0 +1,79 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright (c) 2020, Linaro Limited */
+
+#ifndef _DPU_HW_DSC_H
+#define _DPU_HW_DSC_H
+
+#include <drm/drm_dsc.h>
+
+#define DSC_MODE_SPLIT_PANEL BIT(0)
+#define DSC_MODE_MULTIPLEX BIT(1)
+#define DSC_MODE_VIDEO BIT(2)
+
+struct dpu_hw_dsc;
+
+/**
+ * struct dpu_hw_dsc_ops - interface to the dsc hardware driver functions
+ * Assumption is these functions will be called after clocks are enabled
+ */
+struct dpu_hw_dsc_ops {
+ /**
+ * dsc_disable - disable dsc
+ * @hw_dsc: Pointer to dsc context
+ */
+ void (*dsc_disable)(struct dpu_hw_dsc *hw_dsc);
+
+ /**
+ * dsc_config - configures dsc encoder
+ * @hw_dsc: Pointer to dsc context
+ * @dsc: panel dsc parameters
+ * @mode: dsc topology mode to be set
+ * @ich_reset_override: option to reset ich
+ */
+ void (*dsc_config)(struct dpu_hw_dsc *hw_dsc,
+ struct msm_display_dsc_config *dsc,
+ u32 mode, bool ich_reset_override);
+
+ /**
+ * dsc_config_thresh - programs panel thresholds
+ * @hw_dsc: Pointer to dsc context
+ * @dsc: panel dsc parameters
+ */
+ void (*dsc_config_thresh)(struct dpu_hw_dsc *hw_dsc,
+ struct msm_display_dsc_config *dsc);
+};
+
+struct dpu_hw_dsc {
+ struct dpu_hw_blk base;
+ struct dpu_hw_blk_reg_map hw;
+
+ /* dsc */
+ enum dpu_dsc idx;
+ const struct dpu_dsc_cfg *caps;
+
+ /* ops */
+ struct dpu_hw_dsc_ops ops;
+};
+
+/**
+ * dpu_hw_dsc_init - initializes the dsc block for the passed dsc idx.
+ * @idx: DSC index for which driver object is required
+ * @addr: Mapped register io address of MDP
+ * @m: Pointer to mdss catalog data
+ * Returns: Error code or allocated dpu_hw_dsc context
+ */
+struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc idx, void __iomem *addr,
+ struct dpu_mdss_cfg *m);
+
+/**
+ * dpu_hw_dsc_destroy - destroys dsc driver context
+ * @dsc: Pointer to dsc driver context returned by dpu_hw_dsc_init
+ */
+void dpu_hw_dsc_destroy(struct dpu_hw_dsc *dsc);
+
+static inline struct dpu_hw_dsc *to_dpu_hw_dsc(struct dpu_hw_blk *hw)
+{
+ return container_of(hw, struct dpu_hw_dsc, base);
+}
+
+#endif /* _DPU_HW_DSC_H */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index 09a3fb3e89f5..1b72c11090ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -97,6 +97,7 @@ enum dpu_hw_blk_type {
DPU_HW_BLK_WB,
DPU_HW_BLK_DSPP,
DPU_HW_BLK_MERGE_3D,
+ DPU_HW_BLK_DSC,
DPU_HW_BLK_MAX,
};
@@ -176,6 +177,17 @@ enum dpu_ctl {
CTL_MAX
};
+enum dpu_dsc {
+ DSC_NONE = 0,
+ DSC_0,
+ DSC_1,
+ DSC_2,
+ DSC_3,
+ DSC_4,
+ DSC_5,
+ DSC_MAX
+};
+
enum dpu_pingpong {
PINGPONG_0 = 1,
PINGPONG_1,
@@ -437,5 +449,6 @@ struct dpu_mdss_color {
#define DPU_DBG_MASK_VBIF (1 << 8)
#define DPU_DBG_MASK_ROT (1 << 9)
#define DPU_DBG_MASK_DSPP (1 << 10)
+#define DPU_DBG_MASK_DSC (1 << 11)
#endif /* _DPU_HW_MDSS_H */
--
2.26.3
DSC needs some configuration from device tree, add support to read and
store these params and add DSC structures in msm_drv
Signed-off-by: Vinod Koul <[email protected]>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 171 +++++++++++++++++++++++++++++
drivers/gpu/drm/msm/msm_drv.h | 32 ++++++
2 files changed, 203 insertions(+)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 8a10e4343281..e0c0f627d15e 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -156,6 +156,7 @@ struct msm_dsi_host {
struct regmap *sfpb;
struct drm_display_mode *mode;
+ struct msm_display_dsc_config *dsc;
/* connected device info */
struct device_node *device_node;
@@ -1744,6 +1745,168 @@ 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 i;
+
+ dsc->drm.rc_model_size = 8192;
+ dsc->drm.first_line_bpg_offset = 15;
+ 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;
+ 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;
+ 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
+ */
+
+ i = dsc->drm.slice_width % 3;
+ switch (i) {
+ case 0:
+ dsc->slice_last_group_size = 2;
+ break;
+
+ case 1:
+ dsc->slice_last_group_size = 0;
+ break;
+
+ case 2:
+ dsc->slice_last_group_size = 0;
+ break;
+
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int dsi_host_parse_dsc(struct msm_dsi_host *msm_host,
+ struct device_node *np)
+{
+ struct device *dev = &msm_host->pdev->dev;
+ struct msm_display_dsc_config *dsc;
+ bool is_dsc_enabled;
+ u32 data;
+ int ret;
+
+ is_dsc_enabled = of_property_read_bool(np, "qcom,mdss-dsc-enabled");
+
+ if (!is_dsc_enabled)
+ return 0;
+
+ dsc = kzalloc(sizeof(*dsc), GFP_KERNEL);
+ if (!dsc)
+ return -ENOMEM;
+
+ ret = of_property_read_u32(np, "qcom,mdss-dsc-version", &data);
+ if (ret) {
+ dsc->drm.dsc_version_major = 0x1;
+ dsc->drm.dsc_version_minor = 0x1;
+ } else {
+ dsc->drm.dsc_version_major = (data >> 4) & 0xf;
+ dsc->drm.dsc_version_minor = data & 0xf;
+ }
+
+ ret = of_property_read_u32(np, "qcom,mdss-scr-version", &data);
+ if (ret)
+ dsc->scr_rev = 0;
+ else
+ dsc->scr_rev = data & 0xff;
+
+ ret = of_property_read_u32(np, "qcom,mdss-slice-height", &data);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "failed to read dsc slice height\n");
+ goto err;
+ }
+ dsc->drm.slice_height = data;
+
+ ret = of_property_read_u32(np, "qcom,mdss-slice-width", &data);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "failed to read dsc slice width\n");
+ goto err;
+ }
+ dsc->drm.slice_width = data;
+
+ ret = of_property_read_u32(np, "qcom,mdss-slice-per-pkt", &data);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "failed to read mdss-slice-per-pkt\n");
+ goto err;
+ }
+ dsc->slice_per_pkt = data;
+
+ ret = of_property_read_u32(np, "qcom,mdss-bit-per-component", &data);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "failed to read mdss-bit-per-component\n");
+ goto err;
+ }
+ dsc->drm.bits_per_component = data;
+
+ ret = of_property_read_u32(np, "qcom,mdss-bit-per-pixel", &data);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "failed to read bit-per-pixel\n");
+ goto err;
+ }
+ dsc->drm.bits_per_pixel = data;
+
+ dsc->drm.block_pred_enable = of_property_read_bool(np,
+ "qcom,mdss-block-prediction-enable");
+
+ dsi_populate_dsc_params(dsc);
+
+ msm_host->dsc = dsc;
+
+ return 0;
+
+err:
+ kfree(dsc);
+ return ret;
+}
+
static int dsi_host_parse_dt(struct msm_dsi_host *msm_host)
{
struct device *dev = &msm_host->pdev->dev;
@@ -1763,6 +1926,14 @@ static int dsi_host_parse_dt(struct msm_dsi_host *msm_host)
return 0;
}
+ ret = dsi_host_parse_dsc(msm_host, np);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "%s: invalid dsc configuration %d\n",
+ __func__, ret);
+ ret = -EINVAL;
+ goto err;
+ }
+
ret = dsi_host_parse_lane_data(msm_host, endpoint);
if (ret) {
DRM_DEV_ERROR(dev, "%s: invalid lane configuration %d\n",
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 2668941df529..26661dd43936 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>
@@ -70,6 +71,16 @@ enum msm_mdp_plane_property {
#define MSM_GPU_MAX_RINGS 4
#define MAX_H_TILES_PER_DISPLAY 2
+/**
+ * enum msm_display_compression_type - compression method used for pixel stream
+ * @MSM_DISPLAY_COMPRESSION_NONE: Pixel data is not compressed
+ * @MSM_DISPLAY_COMPRESSION_DSC: DSC compresison is used
+ */
+enum msm_display_compression_type {
+ MSM_DISPLAY_COMPRESSION_NONE,
+ MSM_DISPLAY_COMPRESSION_DSC,
+};
+
/**
* enum msm_display_caps - features/capabilities supported by displays
* @MSM_DISPLAY_CAP_VID_MODE: Video or "active" mode supported
@@ -134,6 +145,24 @@ struct msm_drm_thread {
struct kthread_worker *worker;
};
+/* DSC config */
+struct msm_display_dsc_config {
+ struct drm_dsc_config drm;
+ u8 scr_rev;
+
+ u32 initial_lines;
+ u32 pkt_per_line;
+ u32 bytes_in_slice;
+ u32 bytes_per_pkt;
+ u32 eol_byte_num;
+ u32 pclk_per_line;
+ u32 slice_last_group_size;
+ u32 slice_per_pkt;
+ u32 det_thresh_flatness;
+ u32 extra_width;
+ u32 pps_delay_ms;
+};
+
struct msm_drm_private {
struct drm_device *dev;
@@ -227,6 +256,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.26.3
On 21-05-21, 08:18, Rob Herring wrote:
> On Fri, May 21, 2021 at 7:50 AM Vinod Koul <[email protected]> wrote:
> >
> > DSC enables streams to be compressed before we send to panel. This
> > requires DSC enabled encoder and a panel to be present. So we add this
> > information in board DTS and find if DSC can be enabled and the
> > parameters required to configure DSC are added to binding document along
> > with example
> >
> > Signed-off-by: Vinod Koul <[email protected]>
> > ---
> > .../devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
>
> This is getting converted to schema. Hopefully, v17 will be it. Sigh.
I will update these on top, whenever that one gets merged... Any comments
on the parameters added here?
--
~Vinod
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 <[email protected]>
---
drivers/gpu/drm/msm/Makefile | 1 +
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 26 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 221 ++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 79 +++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 13 ++
5 files changed, 340 insertions(+)
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 610d630326bb..fd8fc57f1f58 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -61,6 +61,7 @@ msm-y := \
disp/dpu1/dpu_hw_blk.o \
disp/dpu1/dpu_hw_catalog.o \
disp/dpu1/dpu_hw_ctl.o \
+ disp/dpu1/dpu_hw_dsc.o \
disp/dpu1/dpu_hw_interrupts.o \
disp/dpu1/dpu_hw_intf.o \
disp/dpu1/dpu_hw_lm.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 4dfd8a20ad5c..a699633f7013 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -165,6 +165,7 @@ enum {
* @DPU_PINGPONG_TE2 Additional tear check block for split pipes
* @DPU_PINGPONG_SPLIT PP block supports split fifo
* @DPU_PINGPONG_SLAVE PP block is a suitable slave for split fifo
+ * @DPU_PINGPONG_DSC Display stream compression blocks
* @DPU_PINGPONG_DITHER, Dither blocks
* @DPU_PINGPONG_MAX
*/
@@ -173,10 +174,21 @@ enum {
DPU_PINGPONG_TE2,
DPU_PINGPONG_SPLIT,
DPU_PINGPONG_SLAVE,
+ DPU_PINGPONG_DSC,
DPU_PINGPONG_DITHER,
DPU_PINGPONG_MAX
};
+/**
+ * DSC sub-blocks
+ * @DPU_DSC DSC sub block
+ * @DPU_DSC_MAX
+ */
+enum {
+ DPU_DSC = 0x1,
+ DPU_DSC_MAX
+};
+
/**
* CTL sub-blocks
* @DPU_CTL_SPLIT_DISPLAY CTL supports video mode split display
@@ -413,6 +425,7 @@ struct dpu_dspp_sub_blks {
struct dpu_pingpong_sub_blks {
struct dpu_pp_blk te;
struct dpu_pp_blk te2;
+ struct dpu_pp_blk dsc;
struct dpu_pp_blk dither;
};
@@ -547,6 +560,16 @@ struct dpu_merge_3d_cfg {
const struct dpu_merge_3d_sub_blks *sblk;
};
+/**
+ * struct dpu_dsc_cfg - information of DSC blocks
+ * @id enum identifying this block
+ * @base register offset of this block
+ * @features bit mask identifying sub-blocks/features
+ */
+struct dpu_dsc_cfg {
+ DPU_HW_BLK_INFO;
+};
+
/**
* struct dpu_intf_cfg - information of timing engine blocks
* @id enum identifying this block
@@ -748,6 +771,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..8b8d0553709d
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
@@ -0,0 +1,221 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#include "dpu_kms.h"
+#include "dpu_hw_catalog.h"
+#include "dpu_hwio.h"
+#include "dpu_hw_mdss.h"
+#include "dpu_hw_dsc.h"
+
+#define DSC_COMMON_MODE 0x000
+#define DSC_ENC 0X004
+#define DSC_PICTURE 0x008
+#define DSC_SLICE 0x00C
+#define DSC_CHUNK_SIZE 0x010
+#define DSC_DELAY 0x014
+#define DSC_SCALE_INITIAL 0x018
+#define DSC_SCALE_DEC_INTERVAL 0x01C
+#define DSC_SCALE_INC_INTERVAL 0x020
+#define DSC_FIRST_LINE_BPG_OFFSET 0x024
+#define DSC_BPG_OFFSET 0x028
+#define DSC_DSC_OFFSET 0x02C
+#define DSC_FLATNESS 0x030
+#define DSC_RC_MODEL_SIZE 0x034
+#define DSC_RC 0x038
+#define DSC_RC_BUF_THRESH 0x03C
+#define DSC_RANGE_MIN_QP 0x074
+#define DSC_RANGE_MAX_QP 0x0B0
+#define DSC_RANGE_BPG_OFFSET 0x0EC
+
+static void dpu_hw_dsc_disable(struct dpu_hw_dsc *dsc)
+{
+ struct dpu_hw_blk_reg_map *c = &dsc->hw;
+
+ DPU_REG_WRITE(c, DSC_COMMON_MODE, 0);
+}
+
+static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc,
+ struct msm_display_dsc_config *dsc,
+ u32 mode, bool ich_reset_override)
+{
+ struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
+ u32 data;
+ u32 initial_lines = dsc->initial_lines;
+ bool is_cmd_mode = !(mode & BIT(2));
+
+ DPU_REG_WRITE(c, DSC_COMMON_MODE, mode);
+
+ data = 0;
+ if (ich_reset_override)
+ data = 3 << 28;
+
+ if (is_cmd_mode)
+ initial_lines += 1;
+
+ data |= (initial_lines << 20);
+ data |= ((dsc->slice_last_group_size) << 18);
+ /* bpp is 6.4 format, 4 LSBs bits are for fractional part */
+ data |= dsc->drm.bits_per_pixel << 12;
+ 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);
+ if (dsc->drm.bits_per_component == 10)
+ data |= BIT(0);
+
+ 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 = DIV_ROUND_UP(dsc->drm.slice_width * dsc->drm.bits_per_pixel, 8) << 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 = 0x00000184; /* only this value works */
+ 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 = 0x0;
+ u16 *lp;
+ int i;
+
+ lp = dsc->drm.rc_buf_thresh;
+ off = DSC_RC_BUF_THRESH;
+ for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++) {
+ DPU_REG_WRITE(c, off, *lp++);
+ off += 4;
+ }
+
+ off = DSC_RANGE_MIN_QP;
+ for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
+ DPU_REG_WRITE(c, off, rc[i].range_min_qp);
+ off += 4;
+ }
+
+ off = DSC_RANGE_MAX_QP;
+ for (i = 0; i < 15; i++) {
+ DPU_REG_WRITE(c, off, rc[i].range_max_qp);
+ off += 4;
+ }
+
+ off = DSC_RANGE_BPG_OFFSET;
+ for (i = 0; i < 15; i++) {
+ DPU_REG_WRITE(c, off, rc[i].range_bpg_offset);
+ off += 4;
+ }
+}
+
+static struct dpu_dsc_cfg *_dsc_offset(enum dpu_dsc dsc,
+ struct dpu_mdss_cfg *m,
+ void __iomem *addr,
+ struct dpu_hw_blk_reg_map *b)
+{
+ int i;
+
+ for (i = 0; i < m->dsc_count; i++) {
+ if (dsc == m->dsc[i].id) {
+ b->base_off = addr;
+ b->blk_off = m->dsc[i].base;
+ b->length = m->dsc[i].len;
+ b->hwversion = m->hwversion;
+ b->log_mask = DPU_DBG_MASK_DSC;
+ return &m->dsc[i];
+ }
+ }
+
+ return NULL;
+}
+
+static void _setup_dsc_ops(struct dpu_hw_dsc_ops *ops,
+ unsigned long cap)
+{
+ ops->dsc_disable = dpu_hw_dsc_disable;
+ ops->dsc_config = dpu_hw_dsc_config;
+ ops->dsc_config_thresh = dpu_hw_dsc_config_thresh;
+};
+
+static struct dpu_hw_blk_ops dpu_hw_ops = {
+ .start = NULL,
+ .stop = NULL,
+};
+
+struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc idx, void __iomem *addr,
+ struct dpu_mdss_cfg *m)
+{
+ struct dpu_hw_dsc *c;
+ struct dpu_dsc_cfg *cfg;
+
+ c = kzalloc(sizeof(*c), GFP_KERNEL);
+ if (!c)
+ return ERR_PTR(-ENOMEM);
+
+ cfg = _dsc_offset(idx, m, addr, &c->hw);
+ if (IS_ERR_OR_NULL(cfg)) {
+ kfree(c);
+ return ERR_PTR(-EINVAL);
+ }
+
+ c->idx = idx;
+ c->caps = cfg;
+ _setup_dsc_ops(&c->ops, c->caps->features);
+
+ dpu_hw_blk_init(&c->base, DPU_HW_BLK_DSC, idx, &dpu_hw_ops);
+
+ return c;
+}
+
+void dpu_hw_dsc_destroy(struct dpu_hw_dsc *dsc)
+{
+ if (dsc)
+ dpu_hw_blk_destroy(&dsc->base);
+ kfree(dsc);
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
new file mode 100644
index 000000000000..c680fd948865
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
@@ -0,0 +1,79 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright (c) 2020, Linaro Limited */
+
+#ifndef _DPU_HW_DSC_H
+#define _DPU_HW_DSC_H
+
+#include <drm/drm_dsc.h>
+
+#define DSC_MODE_SPLIT_PANEL BIT(0)
+#define DSC_MODE_MULTIPLEX BIT(1)
+#define DSC_MODE_VIDEO BIT(2)
+
+struct dpu_hw_dsc;
+
+/**
+ * struct dpu_hw_dsc_ops - interface to the dsc hardware driver functions
+ * Assumption is these functions will be called after clocks are enabled
+ */
+struct dpu_hw_dsc_ops {
+ /**
+ * dsc_disable - disable dsc
+ * @hw_dsc: Pointer to dsc context
+ */
+ void (*dsc_disable)(struct dpu_hw_dsc *hw_dsc);
+
+ /**
+ * dsc_config - configures dsc encoder
+ * @hw_dsc: Pointer to dsc context
+ * @dsc: panel dsc parameters
+ * @mode: dsc topology mode to be set
+ * @ich_reset_override: option to reset ich
+ */
+ void (*dsc_config)(struct dpu_hw_dsc *hw_dsc,
+ struct msm_display_dsc_config *dsc,
+ u32 mode, bool ich_reset_override);
+
+ /**
+ * dsc_config_thresh - programs panel thresholds
+ * @hw_dsc: Pointer to dsc context
+ * @dsc: panel dsc parameters
+ */
+ void (*dsc_config_thresh)(struct dpu_hw_dsc *hw_dsc,
+ struct msm_display_dsc_config *dsc);
+};
+
+struct dpu_hw_dsc {
+ struct dpu_hw_blk base;
+ struct dpu_hw_blk_reg_map hw;
+
+ /* dsc */
+ enum dpu_dsc idx;
+ const struct dpu_dsc_cfg *caps;
+
+ /* ops */
+ struct dpu_hw_dsc_ops ops;
+};
+
+/**
+ * dpu_hw_dsc_init - initializes the dsc block for the passed dsc idx.
+ * @idx: DSC index for which driver object is required
+ * @addr: Mapped register io address of MDP
+ * @m: Pointer to mdss catalog data
+ * Returns: Error code or allocated dpu_hw_dsc context
+ */
+struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc idx, void __iomem *addr,
+ struct dpu_mdss_cfg *m);
+
+/**
+ * dpu_hw_dsc_destroy - destroys dsc driver context
+ * @dsc: Pointer to dsc driver context returned by dpu_hw_dsc_init
+ */
+void dpu_hw_dsc_destroy(struct dpu_hw_dsc *dsc);
+
+static inline struct dpu_hw_dsc *to_dpu_hw_dsc(struct dpu_hw_blk *hw)
+{
+ return container_of(hw, struct dpu_hw_dsc, base);
+}
+
+#endif /* _DPU_HW_DSC_H */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index 09a3fb3e89f5..1b72c11090ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -97,6 +97,7 @@ enum dpu_hw_blk_type {
DPU_HW_BLK_WB,
DPU_HW_BLK_DSPP,
DPU_HW_BLK_MERGE_3D,
+ DPU_HW_BLK_DSC,
DPU_HW_BLK_MAX,
};
@@ -176,6 +177,17 @@ enum dpu_ctl {
CTL_MAX
};
+enum dpu_dsc {
+ DSC_NONE = 0,
+ DSC_0,
+ DSC_1,
+ DSC_2,
+ DSC_3,
+ DSC_4,
+ DSC_5,
+ DSC_MAX
+};
+
enum dpu_pingpong {
PINGPONG_0 = 1,
PINGPONG_1,
@@ -437,5 +449,6 @@ struct dpu_mdss_color {
#define DPU_DBG_MASK_VBIF (1 << 8)
#define DPU_DBG_MASK_ROT (1 << 9)
#define DPU_DBG_MASK_DSPP (1 << 10)
+#define DPU_DBG_MASK_DSC (1 << 11)
#endif /* _DPU_HW_MDSS_H */
--
2.26.3
On Fri, May 21, 2021 at 6:50 AM Vinod Koul <[email protected]> wrote:
>
> Display Stream Compression (DSC) compresses the display stream in host which
> is later decoded by panel. This series enables this for Qualcomm msm driver.
> This was tested on Google Pixel3 phone which use LGE SW43408 panel.
>
> The changes include adding DT properties for DSC then hardware blocks support
> required in DPU1 driver and support in encoder. We also add support in DSI
> and introduce required topology changes.
>
> In order for panel to set the DSC parameters we add dsc in drm_panel and set
> it from the msm driver.
>
> Complete changes which enable this for Pixel3 along with panel driver (not
> part of this series) and DT changes can be found at:
> git.linaro.org/people/vinod.koul/kernel.git pixel/dsc_rfc
>
> Comments welcome!
This feels backwards to me. I've only skimmed this series, and the DT
changes didn't come through for me, so perhaps I have an incomplete
view.
DSC is not MSM specific. There is a standard for it. Yet it looks
like everything is implemented in a MSM specific way, and then pushed
to the panel. So, every vendor needs to implement their vendor
specific way to get the DSC info, and then push it to the panel?
Seems wrong, given there is an actual standard for this feature.
Additionally, we define panel properties (resolution, BPP, etc) at the
panel, and have the display drivers pull it from the panel. However,
for DSC, you do the reverse (define it in the display driver, and push
it to the panel). If the argument is that DSC properties can be
dynamic, well, so can resolution. Every panel for MSM MTPs supports
multiple resolutions, yet we define that with the panel in Linux.
Finally, I haven't seen the DT bits, but I'm concerned about using DT
for this. It inherently excludes ACPI systems. You appear to have
sdm845 support in this series, but what about ACPI boot on the Lenovo
C630 for example? Or any of the 8cx laptops? We don't read the panel
resolution, etc from DT, so why the DSC?
I'm glad that work is being done to add DSC to Linux, it's something I
struggled with when working on the 8998 mtp, and I realize this is a
bit of a drive-by review. However, it seems like there should be a
better way.
>
> Vinod Koul (13):
> drm/dsc: Add dsc pps header init function
> dt-bindings: msm/dsi: Document Display Stream Compression (DSC)
> parameters
> drm/msm/dsi: add support for dsc data
> drm/msm/disp/dpu1: Add support for DSC
> drm/msm/disp/dpu1: Add support for DSC in pingpong block
> drm/msm/disp/dpu1: Add DSC support in RM
> drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog
> drm/msm/disp/dpu1: Add DSC support in hw_ctl
> drm/msm/disp/dpu1: Don't use DSC with mode_3d
> drm/msm/disp/dpu1: Add support for DSC in encoder
> drm/msm/disp/dpu1: Add support for DSC in topology
> drm/msm/dsi: Add support for DSC configuration
> drm/msm/dsi: Pass DSC params to drm_panel
>
> .../devicetree/bindings/display/msm/dsi.txt | 15 +
> drivers/gpu/drm/drm_dsc.c | 11 +
> drivers/gpu/drm/msm/Makefile | 1 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 204 +++++++++++-
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 11 +
> .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 2 +
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 22 ++
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 26 ++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 12 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 2 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 221 +++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 79 +++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 13 +
> .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 32 ++
> .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h | 14 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 32 ++
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 1 +
> drivers/gpu/drm/msm/dsi/dsi.xml.h | 10 +
> drivers/gpu/drm/msm/dsi/dsi_host.c | 293 +++++++++++++++++-
> drivers/gpu/drm/msm/msm_drv.h | 32 ++
> include/drm/drm_dsc.h | 16 +
> include/drm/drm_panel.h | 7 +
> 23 files changed, 1043 insertions(+), 14 deletions(-)
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
>
> --
> 2.26.3
>
> _______________________________________________
> Freedreno mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/freedreno
On Fri 21 May 07:49 CDT 2021, Vinod Koul wrote:
> DSC enables streams to be compressed before we send to panel. This
> requires DSC enabled encoder and a panel to be present. So we add this
> information in board DTS and find if DSC can be enabled and the
> parameters required to configure DSC are added to binding document along
> with example
>
> Signed-off-by: Vinod Koul <[email protected]>
> ---
> .../devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt
> index b9a64d3ff184..83d2fb92267e 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi.txt
> +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
> @@ -48,6 +48,13 @@ Optional properties:
> - pinctrl-n: the "sleep" pinctrl state
> - ports: contains DSI controller input and output ports as children, each
> containing one endpoint subnode.
> +- qcom,mdss-dsc-enabled: Display Stream Compression (DSC) is enabled
> +- qcom,mdss-slice-height: DSC slice height in pixels
> +- qcom,mdss-slice-width: DSC slice width in pixels
> +- qcom,mdss-slice-per-pkt: DSC slices per packet
> +- qcom,mdss-bit-per-component: DSC bits per component
> +- qcom,mdss-bit-per-pixel: DSC bits per pixel
> +- qcom,mdss-block-prediction-enable: Block prediction mode of DSC enabled
>
> DSI Endpoint properties:
> - remote-endpoint: For port@0, set to phandle of the connected panel/bridge's
> @@ -188,6 +195,14 @@ Example:
> qcom,master-dsi;
> qcom,sync-dual-dsi;
>
> + qcom,mdss-dsc-enabled;
To me the activation of DSC seems to be a property of the panel.
> + qcom,mdss-slice-height = <16>;
> + qcom,mdss-slice-width = <540>;
> + qcom,mdss-slice-per-pkt = <1>;
> + qcom,mdss-bit-per-component = <8>;
> + qcom,mdss-bit-per-pixel = <8>;
> + qcom,mdss-block-prediction-enable;
Which of these properties relates to the DSC encoder and what needs to
be agreed with the sink? Can't we derive e.g. bpp from the information
we have from the attached panel already?
Regards,
Bjorn
> +
> qcom,mdss-mdp-transfer-time-us = <12000>;
>
> pinctrl-names = "default", "sleep";
> --
> 2.26.3
>
On 21-05-21, 09:42, Bjorn Andersson wrote:
> On Fri 21 May 07:49 CDT 2021, Vinod Koul wrote:
>
> > DSC enables streams to be compressed before we send to panel. This
> > requires DSC enabled encoder and a panel to be present. So we add this
> > information in board DTS and find if DSC can be enabled and the
> > parameters required to configure DSC are added to binding document along
> > with example
> >
> > Signed-off-by: Vinod Koul <[email protected]>
> > ---
> > .../devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt
> > index b9a64d3ff184..83d2fb92267e 100644
> > --- a/Documentation/devicetree/bindings/display/msm/dsi.txt
> > +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
> > @@ -48,6 +48,13 @@ Optional properties:
> > - pinctrl-n: the "sleep" pinctrl state
> > - ports: contains DSI controller input and output ports as children, each
> > containing one endpoint subnode.
> > +- qcom,mdss-dsc-enabled: Display Stream Compression (DSC) is enabled
> > +- qcom,mdss-slice-height: DSC slice height in pixels
> > +- qcom,mdss-slice-width: DSC slice width in pixels
> > +- qcom,mdss-slice-per-pkt: DSC slices per packet
> > +- qcom,mdss-bit-per-component: DSC bits per component
> > +- qcom,mdss-bit-per-pixel: DSC bits per pixel
> > +- qcom,mdss-block-prediction-enable: Block prediction mode of DSC enabled
> >
> > DSI Endpoint properties:
> > - remote-endpoint: For port@0, set to phandle of the connected panel/bridge's
> > @@ -188,6 +195,14 @@ Example:
> > qcom,master-dsi;
> > qcom,sync-dual-dsi;
> >
> > + qcom,mdss-dsc-enabled;
>
> To me the activation of DSC seems to be a property of the panel.
I think there are three parts to the problem
1. Panel needs to support it
2. Host needs to support it
3. Someone needs to decide to use when both the above conditions are
met.
There are cases where above 1, 2 will be satisfied, but we might be okay
without DSC too.. so how to decide when to do DSC :)
I feel it is more of a system property. And I also think that these
parameters here are host configuration and not really for panel...
>
> > + qcom,mdss-slice-height = <16>;
> > + qcom,mdss-slice-width = <540>;
> > + qcom,mdss-slice-per-pkt = <1>;
> > + qcom,mdss-bit-per-component = <8>;
> > + qcom,mdss-bit-per-pixel = <8>;
> > + qcom,mdss-block-prediction-enable;
>
> Which of these properties relates to the DSC encoder and what needs to
> be agreed with the sink? Can't we derive e.g. bpp from the information
> we have from the attached panel already?
Let me go back and check on this a bit more
Thanks
--
~Vinod
On Mon 24 May 02:30 CDT 2021, Vinod Koul wrote:
> On 21-05-21, 09:42, Bjorn Andersson wrote:
> > On Fri 21 May 07:49 CDT 2021, Vinod Koul wrote:
> >
> > > DSC enables streams to be compressed before we send to panel. This
> > > requires DSC enabled encoder and a panel to be present. So we add this
> > > information in board DTS and find if DSC can be enabled and the
> > > parameters required to configure DSC are added to binding document along
> > > with example
> > >
> > > Signed-off-by: Vinod Koul <[email protected]>
> > > ---
> > > .../devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++
> > > 1 file changed, 15 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt
> > > index b9a64d3ff184..83d2fb92267e 100644
> > > --- a/Documentation/devicetree/bindings/display/msm/dsi.txt
> > > +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
> > > @@ -48,6 +48,13 @@ Optional properties:
> > > - pinctrl-n: the "sleep" pinctrl state
> > > - ports: contains DSI controller input and output ports as children, each
> > > containing one endpoint subnode.
> > > +- qcom,mdss-dsc-enabled: Display Stream Compression (DSC) is enabled
> > > +- qcom,mdss-slice-height: DSC slice height in pixels
> > > +- qcom,mdss-slice-width: DSC slice width in pixels
> > > +- qcom,mdss-slice-per-pkt: DSC slices per packet
> > > +- qcom,mdss-bit-per-component: DSC bits per component
> > > +- qcom,mdss-bit-per-pixel: DSC bits per pixel
> > > +- qcom,mdss-block-prediction-enable: Block prediction mode of DSC enabled
> > >
> > > DSI Endpoint properties:
> > > - remote-endpoint: For port@0, set to phandle of the connected panel/bridge's
> > > @@ -188,6 +195,14 @@ Example:
> > > qcom,master-dsi;
> > > qcom,sync-dual-dsi;
> > >
> > > + qcom,mdss-dsc-enabled;
> >
> > To me the activation of DSC seems to be a property of the panel.
>
> I think there are three parts to the problem
> 1. Panel needs to support it
In the case of DP there's bits to be read in the panel to figure this
out, for DSI panels this seems like a property that the panel (driver)
should know about.
> 2. Host needs to support it
Right, so this needs to be known by the driver. My suggestion is that we
derive it from the compatible or from the HW version.
> 3. Someone needs to decide to use when both the above conditions are
> met.
>
> There are cases where above 1, 2 will be satisfied, but we might be okay
> without DSC too.. so how to decide when to do DSC :)
>
Can we describe those cases? E.g. is it because enabling DSC would not
cause a reduction in clock speed that's worth the effort? Or do we only
use DSC for DSI when it allows us to squeeze everything into a single
link?
Regards,
Bjorn
> I feel it is more of a system property. And I also think that these
> parameters here are host configuration and not really for panel...
>
> >
> > > + qcom,mdss-slice-height = <16>;
> > > + qcom,mdss-slice-width = <540>;
> > > + qcom,mdss-slice-per-pkt = <1>;
> > > + qcom,mdss-bit-per-component = <8>;
> > > + qcom,mdss-bit-per-pixel = <8>;
> > > + qcom,mdss-block-prediction-enable;
> >
> > Which of these properties relates to the DSC encoder and what needs to
> > be agreed with the sink? Can't we derive e.g. bpp from the information
> > we have from the attached panel already?
>
> Let me go back and check on this a bit more
>
> Thanks
> --
> ~Vinod
Hello Jeff,
On 21-05-21, 08:09, Jeffrey Hugo wrote:
> On Fri, May 21, 2021 at 6:50 AM Vinod Koul <[email protected]> wrote:
> >
> > Display Stream Compression (DSC) compresses the display stream in host which
> > is later decoded by panel. This series enables this for Qualcomm msm driver.
> > This was tested on Google Pixel3 phone which use LGE SW43408 panel.
> >
> > The changes include adding DT properties for DSC then hardware blocks support
> > required in DPU1 driver and support in encoder. We also add support in DSI
> > and introduce required topology changes.
> >
> > In order for panel to set the DSC parameters we add dsc in drm_panel and set
> > it from the msm driver.
> >
> > Complete changes which enable this for Pixel3 along with panel driver (not
> > part of this series) and DT changes can be found at:
> > git.linaro.org/people/vinod.koul/kernel.git pixel/dsc_rfc
> >
> > Comments welcome!
>
> This feels backwards to me. I've only skimmed this series, and the DT
> changes didn't come through for me, so perhaps I have an incomplete
> view.
Not sure why, I see it on lore:
https://lore.kernel.org/dri-devel/[email protected]/
> DSC is not MSM specific. There is a standard for it. Yet it looks
> like everything is implemented in a MSM specific way, and then pushed
> to the panel. So, every vendor needs to implement their vendor
> specific way to get the DSC info, and then push it to the panel?
> Seems wrong, given there is an actual standard for this feature.
I have added slice and bpp info in the DT here under the host and then
pass the generic struct drm_dsc_config to panel which allows panel to
write the pps cmd
Nothing above is MSM specific.. It can very well work with non MSM
controllers too.
I didn't envision DSC to be a specific thing, most of
the patches here are hardware enabling ones for DSC bits for MSM
hardware.
> Additionally, we define panel properties (resolution, BPP, etc) at the
> panel, and have the display drivers pull it from the panel. However,
> for DSC, you do the reverse (define it in the display driver, and push
> it to the panel). If the argument is that DSC properties can be
> dynamic, well, so can resolution. Every panel for MSM MTPs supports
> multiple resolutions, yet we define that with the panel in Linux.
I dont have an answer for that right now, to start with yes the
properties are in host but I am okay to discuss this and put wherever we
feel is most correct thing. I somehow dont like that we should pull
from panel DT and program host with that. Here using struct
drm_dsc_config allows me to configure panel based on resolution passed
> Finally, I haven't seen the DT bits, but I'm concerned about using DT
> for this. It inherently excludes ACPI systems. You appear to have
> sdm845 support in this series, but what about ACPI boot on the Lenovo
> C630 for example? Or any of the 8cx laptops? We don't read the panel
> resolution, etc from DT, so why the DSC?
But you must read from somewhere like ACPI tables. I think ACPI systems
would have some ACPI table info out there which would help on this.
Yes that is another task which we need to start with once we enable OF
systems.
Thanks
--
~Vinod
On 24-05-21, 10:08, Bjorn Andersson wrote:
> On Mon 24 May 02:30 CDT 2021, Vinod Koul wrote:
>
> > On 21-05-21, 09:42, Bjorn Andersson wrote:
> > > On Fri 21 May 07:49 CDT 2021, Vinod Koul wrote:
> > >
> > > > DSC enables streams to be compressed before we send to panel. This
> > > > requires DSC enabled encoder and a panel to be present. So we add this
> > > > information in board DTS and find if DSC can be enabled and the
> > > > parameters required to configure DSC are added to binding document along
> > > > with example
> > > >
> > > > Signed-off-by: Vinod Koul <[email protected]>
> > > > ---
> > > > .../devicetree/bindings/display/msm/dsi.txt | 15 +++++++++++++++
> > > > 1 file changed, 15 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt
> > > > index b9a64d3ff184..83d2fb92267e 100644
> > > > --- a/Documentation/devicetree/bindings/display/msm/dsi.txt
> > > > +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
> > > > @@ -48,6 +48,13 @@ Optional properties:
> > > > - pinctrl-n: the "sleep" pinctrl state
> > > > - ports: contains DSI controller input and output ports as children, each
> > > > containing one endpoint subnode.
> > > > +- qcom,mdss-dsc-enabled: Display Stream Compression (DSC) is enabled
> > > > +- qcom,mdss-slice-height: DSC slice height in pixels
> > > > +- qcom,mdss-slice-width: DSC slice width in pixels
> > > > +- qcom,mdss-slice-per-pkt: DSC slices per packet
> > > > +- qcom,mdss-bit-per-component: DSC bits per component
> > > > +- qcom,mdss-bit-per-pixel: DSC bits per pixel
> > > > +- qcom,mdss-block-prediction-enable: Block prediction mode of DSC enabled
> > > >
> > > > DSI Endpoint properties:
> > > > - remote-endpoint: For port@0, set to phandle of the connected panel/bridge's
> > > > @@ -188,6 +195,14 @@ Example:
> > > > qcom,master-dsi;
> > > > qcom,sync-dual-dsi;
> > > >
> > > > + qcom,mdss-dsc-enabled;
> > >
> > > To me the activation of DSC seems to be a property of the panel.
> >
> > I think there are three parts to the problem
> > 1. Panel needs to support it
>
> In the case of DP there's bits to be read in the panel to figure this
> out, for DSI panels this seems like a property that the panel (driver)
> should know about.
Yes panel should know..
>
> > 2. Host needs to support it
>
> Right, so this needs to be known by the driver. My suggestion is that we
> derive it from the compatible or from the HW version.
Okay
>
> > 3. Someone needs to decide to use when both the above conditions are
> > met.
> >
> > There are cases where above 1, 2 will be satisfied, but we might be okay
> > without DSC too.. so how to decide when to do DSC :)
> >
>
> Can we describe those cases? E.g. is it because enabling DSC would not
> cause a reduction in clock speed that's worth the effort? Or do we only
> use DSC for DSI when it allows us to squeeze everything into a single
> link?
I really dont know how and when we should decide that :-|
One thing we can do is that if both support then always enable and get
benefit of getting power and clock speed reduction.
With this, who should have slice and bpp settings, panel or host?
Thanks
--
~Vinod
On Tue, May 25, 2021 at 11:46 PM Vinod Koul <[email protected]> wrote:
>
> Hello Jeff,
>
> On 21-05-21, 08:09, Jeffrey Hugo wrote:
> > On Fri, May 21, 2021 at 6:50 AM Vinod Koul <[email protected]> wrote:
> > >
> > > Display Stream Compression (DSC) compresses the display stream in host which
> > > is later decoded by panel. This series enables this for Qualcomm msm driver.
> > > This was tested on Google Pixel3 phone which use LGE SW43408 panel.
> > >
> > > The changes include adding DT properties for DSC then hardware blocks support
> > > required in DPU1 driver and support in encoder. We also add support in DSI
> > > and introduce required topology changes.
> > >
> > > In order for panel to set the DSC parameters we add dsc in drm_panel and set
> > > it from the msm driver.
> > >
> > > Complete changes which enable this for Pixel3 along with panel driver (not
> > > part of this series) and DT changes can be found at:
> > > git.linaro.org/people/vinod.koul/kernel.git pixel/dsc_rfc
> > >
> > > Comments welcome!
> >
> > This feels backwards to me. I've only skimmed this series, and the DT
> > changes didn't come through for me, so perhaps I have an incomplete
> > view.
>
> Not sure why, I see it on lore:
> https://lore.kernel.org/dri-devel/[email protected]/
>
> > DSC is not MSM specific. There is a standard for it. Yet it looks
> > like everything is implemented in a MSM specific way, and then pushed
> > to the panel. So, every vendor needs to implement their vendor
> > specific way to get the DSC info, and then push it to the panel?
> > Seems wrong, given there is an actual standard for this feature.
>
> I have added slice and bpp info in the DT here under the host and then
> pass the generic struct drm_dsc_config to panel which allows panel to
> write the pps cmd
>
> Nothing above is MSM specific.. It can very well work with non MSM
> controllers too.
I disagree.
The DT bindings you defined (thanks for the direct link) are MSM
specific. I'm not talking (yet) about the properties you defined, but
purely from the stand point that you defined the binding within the
scope of the MSM dsi binding. No other vendor can use those bindings.
Of course, if we look at the properties themselves, they are prefixed
with "qcom", which is vendor specific.
So, purely on the face of it, this is MSM specific.
Assuming we want a DT solution for DSC, I think it should be something
like Documentation/devicetree/bindings/clock/clock-bindings.txt (the
first example that comes to mind), which is a non-vendor specific
generic set of properties that each vendor/device specific binding can
inherit. Panel has similar things.
Specific to the properties, I don't much like that you duplicate BPP,
which is already associated with the panel (although perhaps not in
the scope of DT). What if the panel and your DSC bindings disagree?
Also, I guess I need to ask, have you read the DSC spec? Last I
looked, there were something like 3 dozen properties that could be
configured. You have five in your proposed binding. To me, this is
not a generic DSC solution, this is MSM specific (and frankly I don't
think this supports all the configuration the MSM hardware can do,
either).
I'm surprised Rob Herring didn't have more to say on this.
> I didn't envision DSC to be a specific thing, most of
> the patches here are hardware enabling ones for DSC bits for MSM
> hardware.
>
> > Additionally, we define panel properties (resolution, BPP, etc) at the
> > panel, and have the display drivers pull it from the panel. However,
> > for DSC, you do the reverse (define it in the display driver, and push
> > it to the panel). If the argument is that DSC properties can be
> > dynamic, well, so can resolution. Every panel for MSM MTPs supports
> > multiple resolutions, yet we define that with the panel in Linux.
>
> I dont have an answer for that right now, to start with yes the
> properties are in host but I am okay to discuss this and put wherever we
> feel is most correct thing. I somehow dont like that we should pull
> from panel DT and program host with that. Here using struct
> drm_dsc_config allows me to configure panel based on resolution passed
I somewhat agree that pulling from the panel and programing the host
based on that is an odd solution, but we have it currently. Have a
look at Documentation/devicetree/bindings/display/panel in particular
panel-timing. All of that ends up informing the mdss programing
anyways (particularly the dsi and its phy). So my problem is that we
currently have a solution that seems to just need to be extended, and
instead you have proposed a completely different solution which is
arguably contradictory.
However, I'd like to see thoughts from Rob Clark, David, and any
others that typically handle this stuff (maybe Sam Ravenborg from the
panel side?). I consider them to be the experts, and if they think
your solution is the way to go, I'll shut up. I consider myself to be
a novice that has dabbled in this area, and while this currently
doesn't make sense to me, maybe I need some education here to see the
light.
> > Finally, I haven't seen the DT bits, but I'm concerned about using DT
> > for this. It inherently excludes ACPI systems. You appear to have
> > sdm845 support in this series, but what about ACPI boot on the Lenovo
> > C630 for example? Or any of the 8cx laptops? We don't read the panel
> > resolution, etc from DT, so why the DSC?
>
> But you must read from somewhere like ACPI tables. I think ACPI systems
> would have some ACPI table info out there which would help on this.
> Yes that is another task which we need to start with once we enable OF
> systems.
Frankly, I don't like the MSM ACPI solution that I've seen on the laptops.
The ACPI assumes the entire MDSS (including DSI parts) and GPU is one
device, and ultimately handled by one driver. That driver needs to
get a value from UEFI (set by the bootloader) that is the "panel id".
Then the driver calls into ACPI (I think its _ROM, but I might be
mistaken, doing this from memory) with that id. It gets back a binary
blob which is mostly an xml file (format is publicly documented) that
contains the panel timings and such.
Generally we've defined simple-panel entities for these, with the
timings in code (you can see what Bjorn and I have upstreamed), and
just match on the compatible.
In summary, I don't mean to be difficult, I just think this solution
needs more "baking".
On Wed, May 26, 2021 at 8:00 AM Jeffrey Hugo <[email protected]> wrote:
>
> On Tue, May 25, 2021 at 11:46 PM Vinod Koul <[email protected]> wrote:
> >
> > Hello Jeff,
> >
> > On 21-05-21, 08:09, Jeffrey Hugo wrote:
> > > On Fri, May 21, 2021 at 6:50 AM Vinod Koul <[email protected]> wrote:
> > > >
> > > > Display Stream Compression (DSC) compresses the display stream in host which
> > > > is later decoded by panel. This series enables this for Qualcomm msm driver.
> > > > This was tested on Google Pixel3 phone which use LGE SW43408 panel.
> > > >
> > > > The changes include adding DT properties for DSC then hardware blocks support
> > > > required in DPU1 driver and support in encoder. We also add support in DSI
> > > > and introduce required topology changes.
> > > >
> > > > In order for panel to set the DSC parameters we add dsc in drm_panel and set
> > > > it from the msm driver.
> > > >
> > > > Complete changes which enable this for Pixel3 along with panel driver (not
> > > > part of this series) and DT changes can be found at:
> > > > git.linaro.org/people/vinod.koul/kernel.git pixel/dsc_rfc
> > > >
> > > > Comments welcome!
> > >
> > > This feels backwards to me. I've only skimmed this series, and the DT
> > > changes didn't come through for me, so perhaps I have an incomplete
> > > view.
> >
> > Not sure why, I see it on lore:
> > https://lore.kernel.org/dri-devel/[email protected]/
> >
> > > DSC is not MSM specific. There is a standard for it. Yet it looks
> > > like everything is implemented in a MSM specific way, and then pushed
> > > to the panel. So, every vendor needs to implement their vendor
> > > specific way to get the DSC info, and then push it to the panel?
> > > Seems wrong, given there is an actual standard for this feature.
> >
> > I have added slice and bpp info in the DT here under the host and then
> > pass the generic struct drm_dsc_config to panel which allows panel to
> > write the pps cmd
> >
> > Nothing above is MSM specific.. It can very well work with non MSM
> > controllers too.
>
> I disagree.
>
> The DT bindings you defined (thanks for the direct link) are MSM
> specific. I'm not talking (yet) about the properties you defined, but
> purely from the stand point that you defined the binding within the
> scope of the MSM dsi binding. No other vendor can use those bindings.
> Of course, if we look at the properties themselves, they are prefixed
> with "qcom", which is vendor specific.
>
> So, purely on the face of it, this is MSM specific.
>
> Assuming we want a DT solution for DSC, I think it should be something
> like Documentation/devicetree/bindings/clock/clock-bindings.txt (the
> first example that comes to mind), which is a non-vendor specific
> generic set of properties that each vendor/device specific binding can
> inherit. Panel has similar things.
>
> Specific to the properties, I don't much like that you duplicate BPP,
> which is already associated with the panel (although perhaps not in
> the scope of DT). What if the panel and your DSC bindings disagree?
> Also, I guess I need to ask, have you read the DSC spec? Last I
> looked, there were something like 3 dozen properties that could be
> configured. You have five in your proposed binding. To me, this is
> not a generic DSC solution, this is MSM specific (and frankly I don't
> think this supports all the configuration the MSM hardware can do,
> either).
>
> I'm surprised Rob Herring didn't have more to say on this.
>
> > I didn't envision DSC to be a specific thing, most of
> > the patches here are hardware enabling ones for DSC bits for MSM
> > hardware.
> >
> > > Additionally, we define panel properties (resolution, BPP, etc) at the
> > > panel, and have the display drivers pull it from the panel. However,
> > > for DSC, you do the reverse (define it in the display driver, and push
> > > it to the panel). If the argument is that DSC properties can be
> > > dynamic, well, so can resolution. Every panel for MSM MTPs supports
> > > multiple resolutions, yet we define that with the panel in Linux.
> >
> > I dont have an answer for that right now, to start with yes the
> > properties are in host but I am okay to discuss this and put wherever we
> > feel is most correct thing. I somehow dont like that we should pull
> > from panel DT and program host with that. Here using struct
> > drm_dsc_config allows me to configure panel based on resolution passed
>
> I somewhat agree that pulling from the panel and programing the host
> based on that is an odd solution, but we have it currently. Have a
> look at Documentation/devicetree/bindings/display/panel in particular
> panel-timing. All of that ends up informing the mdss programing
> anyways (particularly the dsi and its phy). So my problem is that we
> currently have a solution that seems to just need to be extended, and
> instead you have proposed a completely different solution which is
> arguably contradictory.
>
> However, I'd like to see thoughts from Rob Clark, David, and any
> others that typically handle this stuff (maybe Sam Ravenborg from the
> panel side?). I consider them to be the experts, and if they think
> your solution is the way to go, I'll shut up. I consider myself to be
> a novice that has dabbled in this area, and while this currently
> doesn't make sense to me, maybe I need some education here to see the
> light.
>
> > > Finally, I haven't seen the DT bits, but I'm concerned about using DT
> > > for this. It inherently excludes ACPI systems. You appear to have
> > > sdm845 support in this series, but what about ACPI boot on the Lenovo
> > > C630 for example? Or any of the 8cx laptops? We don't read the panel
> > > resolution, etc from DT, so why the DSC?
> >
> > But you must read from somewhere like ACPI tables. I think ACPI systems
> > would have some ACPI table info out there which would help on this.
> > Yes that is another task which we need to start with once we enable OF
> > systems.
>
> Frankly, I don't like the MSM ACPI solution that I've seen on the laptops.
> The ACPI assumes the entire MDSS (including DSI parts) and GPU is one
> device, and ultimately handled by one driver. That driver needs to
> get a value from UEFI (set by the bootloader) that is the "panel id".
> Then the driver calls into ACPI (I think its _ROM, but I might be
> mistaken, doing this from memory) with that id. It gets back a binary
> blob which is mostly an xml file (format is publicly documented) that
> contains the panel timings and such.
tbh, I kinda suspect that having a single "gpu" device (which also
includes venus, in addition to display, IIRC) in the ACPI tables is a
windowsism, trying to make things look to userspace like a single "GPU
card" in the x86 world.. but either way, I think the ACPI tables on
the windows arm laptops which use dsi->bridge->edp is too much of a
lost cause to even consider here. Possibly ACPI boot on these devices
would be more feasible on newer devices which have direct eDP out of
the SoC without requiring external bridge/panel glue.
I'd worry more about what makes sense in a DT world, when it comes to
DT bindings.
BR,
-R
> Generally we've defined simple-panel entities for these, with the
> timings in code (you can see what Bjorn and I have upstreamed), and
> just match on the compatible.
>
> In summary, I don't mean to be difficult, I just think this solution
> needs more "baking".
On 21/05/2021 15:49, Vinod Koul wrote:
> DSC needs some configuration from device tree, add support to read and
> store these params and add DSC structures in msm_drv
>
> Signed-off-by: Vinod Koul <[email protected]>
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 170 +++++++++++++++++++++++++++++
> drivers/gpu/drm/msm/msm_drv.h | 32 ++++++
> 2 files changed, 202 insertions(+)
>
[skipped]
> DRM_DEV_ERROR(dev, "%s: invalid lane configuration %d\n",
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 2668941df529..26661dd43936 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>
>
> @@ -70,6 +71,16 @@ enum msm_mdp_plane_property {
> #define MSM_GPU_MAX_RINGS 4
> #define MAX_H_TILES_PER_DISPLAY 2
>
> +/**
> + * enum msm_display_compression_type - compression method used for pixel stream
> + * @MSM_DISPLAY_COMPRESSION_NONE: Pixel data is not compressed
> + * @MSM_DISPLAY_COMPRESSION_DSC: DSC compresison is used
> + */
> +enum msm_display_compression_type {
> + MSM_DISPLAY_COMPRESSION_NONE,
> + MSM_DISPLAY_COMPRESSION_DSC,
> +};
> +
Seems to be unused
> /**
> * enum msm_display_caps - features/capabilities supported by displays
> * @MSM_DISPLAY_CAP_VID_MODE: Video or "active" mode supported
--
With best wishes
Dmitry
On 21/05/2021 15:49, 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 <[email protected]>
> ---
> drivers/gpu/drm/msm/Makefile | 1 +
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 26 +++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 221 ++++++++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 79 +++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 13 ++
> 5 files changed, 340 insertions(+)
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
>
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index 610d630326bb..fd8fc57f1f58 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -61,6 +61,7 @@ msm-y := \
> disp/dpu1/dpu_hw_blk.o \
> disp/dpu1/dpu_hw_catalog.o \
> disp/dpu1/dpu_hw_ctl.o \
> + disp/dpu1/dpu_hw_dsc.o \
> disp/dpu1/dpu_hw_interrupts.o \
> disp/dpu1/dpu_hw_intf.o \
> disp/dpu1/dpu_hw_lm.o \
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> index 4dfd8a20ad5c..a699633f7013 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -165,6 +165,7 @@ enum {
> * @DPU_PINGPONG_TE2 Additional tear check block for split pipes
> * @DPU_PINGPONG_SPLIT PP block supports split fifo
> * @DPU_PINGPONG_SLAVE PP block is a suitable slave for split fifo
> + * @DPU_PINGPONG_DSC Display stream compression blocks
PP block supports DSC compression?
Also you don't seem to set it anywhere. Do we have hardware w/o DSC support?
> * @DPU_PINGPONG_DITHER, Dither blocks
> * @DPU_PINGPONG_MAX
> */
> @@ -173,10 +174,21 @@ enum {
> DPU_PINGPONG_TE2,
> DPU_PINGPONG_SPLIT,
> DPU_PINGPONG_SLAVE,
> + DPU_PINGPONG_DSC,
> DPU_PINGPONG_DITHER,
> DPU_PINGPONG_MAX
> };
>
> +/**
> + * DSC sub-blocks
> + * @DPU_DSC DSC sub block
> + * @DPU_DSC_MAX
> + */
> +enum {
> + DPU_DSC = 0x1,
> + DPU_DSC_MAX
> +};
> +
Unused
> /**
> * CTL sub-blocks
> * @DPU_CTL_SPLIT_DISPLAY CTL supports video mode split display
> @@ -413,6 +425,7 @@ struct dpu_dspp_sub_blks {
> struct dpu_pingpong_sub_blks {
> struct dpu_pp_blk te;
> struct dpu_pp_blk te2;
> + struct dpu_pp_blk dsc;
> struct dpu_pp_blk dither;
> };
Unused
>
> @@ -547,6 +560,16 @@ struct dpu_merge_3d_cfg {
> const struct dpu_merge_3d_sub_blks *sblk;
> };
>
> +/**
> + * struct dpu_dsc_cfg - information of DSC blocks
> + * @id enum identifying this block
> + * @base register offset of this block
> + * @features bit mask identifying sub-blocks/features
> + */
> +struct dpu_dsc_cfg {
> + DPU_HW_BLK_INFO;
> +};
> +
> /**
> * struct dpu_intf_cfg - information of timing engine blocks
> * @id enum identifying this block
> @@ -748,6 +771,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..8b8d0553709d
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
> @@ -0,0 +1,221 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2020, Linaro Limited
> + */
> +
> +#include "dpu_kms.h"
> +#include "dpu_hw_catalog.h"
> +#include "dpu_hwio.h"
> +#include "dpu_hw_mdss.h"
> +#include "dpu_hw_dsc.h"
> +
> +#define DSC_COMMON_MODE 0x000
> +#define DSC_ENC 0X004
> +#define DSC_PICTURE 0x008
> +#define DSC_SLICE 0x00C
> +#define DSC_CHUNK_SIZE 0x010
> +#define DSC_DELAY 0x014
> +#define DSC_SCALE_INITIAL 0x018
> +#define DSC_SCALE_DEC_INTERVAL 0x01C
> +#define DSC_SCALE_INC_INTERVAL 0x020
> +#define DSC_FIRST_LINE_BPG_OFFSET 0x024
> +#define DSC_BPG_OFFSET 0x028
> +#define DSC_DSC_OFFSET 0x02C
> +#define DSC_FLATNESS 0x030
> +#define DSC_RC_MODEL_SIZE 0x034
> +#define DSC_RC 0x038
> +#define DSC_RC_BUF_THRESH 0x03C
> +#define DSC_RANGE_MIN_QP 0x074
> +#define DSC_RANGE_MAX_QP 0x0B0
> +#define DSC_RANGE_BPG_OFFSET 0x0EC
> +
> +static void dpu_hw_dsc_disable(struct dpu_hw_dsc *dsc)
> +{
> + struct dpu_hw_blk_reg_map *c = &dsc->hw;
> +
> + DPU_REG_WRITE(c, DSC_COMMON_MODE, 0);
> +}
> +
> +static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc,
> + struct msm_display_dsc_config *dsc,
> + u32 mode, bool ich_reset_override)
> +{
> + struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
> + u32 data;
> + u32 initial_lines = dsc->initial_lines;
> + bool is_cmd_mode = !(mode & BIT(2));
> +
> + DPU_REG_WRITE(c, DSC_COMMON_MODE, mode);
> +
> + data = 0;
> + if (ich_reset_override)
> + data = 3 << 28;
> +
> + if (is_cmd_mode)
> + initial_lines += 1;
> +
> + data |= (initial_lines << 20);
> + data |= ((dsc->slice_last_group_size) << 18);
> + /* bpp is 6.4 format, 4 LSBs bits are for fractional part */
> + data |= dsc->drm.bits_per_pixel << 12;
> + 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);
> + if (dsc->drm.bits_per_component == 10)
> + data |= BIT(0);
> +
> + 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 = DIV_ROUND_UP(dsc->drm.slice_width * dsc->drm.bits_per_pixel, 8) << 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 = 0x00000184; /* only this value works */
> + 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 = 0x0;
> + u16 *lp;
> + int i;
> +
> + lp = dsc->drm.rc_buf_thresh;
> + off = DSC_RC_BUF_THRESH;
> + for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++) {
> + DPU_REG_WRITE(c, off, *lp++);
> + off += 4;
> + }
> +
> + off = DSC_RANGE_MIN_QP;
> + for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
> + DPU_REG_WRITE(c, off, rc[i].range_min_qp);
> + off += 4;
> + }
> +
> + off = DSC_RANGE_MAX_QP;
> + for (i = 0; i < 15; i++) {
> + DPU_REG_WRITE(c, off, rc[i].range_max_qp);
> + off += 4;
> + }
> +
> + off = DSC_RANGE_BPG_OFFSET;
> + for (i = 0; i < 15; i++) {
> + DPU_REG_WRITE(c, off, rc[i].range_bpg_offset);
> + off += 4;
> + }
> +}
> +
> +static struct dpu_dsc_cfg *_dsc_offset(enum dpu_dsc dsc,
> + struct dpu_mdss_cfg *m,
> + void __iomem *addr,
> + struct dpu_hw_blk_reg_map *b)
> +{
> + int i;
> +
> + for (i = 0; i < m->dsc_count; i++) {
> + if (dsc == m->dsc[i].id) {
> + b->base_off = addr;
> + b->blk_off = m->dsc[i].base;
> + b->length = m->dsc[i].len;
> + b->hwversion = m->hwversion;
> + b->log_mask = DPU_DBG_MASK_DSC;
> + return &m->dsc[i];
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static void _setup_dsc_ops(struct dpu_hw_dsc_ops *ops,
> + unsigned long cap)
> +{
> + ops->dsc_disable = dpu_hw_dsc_disable;
> + ops->dsc_config = dpu_hw_dsc_config;
> + ops->dsc_config_thresh = dpu_hw_dsc_config_thresh;
> +};
> +
> +static struct dpu_hw_blk_ops dpu_hw_ops = {
> + .start = NULL,
> + .stop = NULL,
> +};
> +
> +struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc idx, void __iomem *addr,
> + struct dpu_mdss_cfg *m)
> +{
> + struct dpu_hw_dsc *c;
> + struct dpu_dsc_cfg *cfg;
> +
> + c = kzalloc(sizeof(*c), GFP_KERNEL);
> + if (!c)
> + return ERR_PTR(-ENOMEM);
> +
> + cfg = _dsc_offset(idx, m, addr, &c->hw);
> + if (IS_ERR_OR_NULL(cfg)) {
> + kfree(c);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + c->idx = idx;
> + c->caps = cfg;
> + _setup_dsc_ops(&c->ops, c->caps->features);
> +
> + dpu_hw_blk_init(&c->base, DPU_HW_BLK_DSC, idx, &dpu_hw_ops);
> +
> + return c;
> +}
> +
> +void dpu_hw_dsc_destroy(struct dpu_hw_dsc *dsc)
> +{
> + if (dsc)
> + dpu_hw_blk_destroy(&dsc->base);
> + kfree(dsc);
> +}
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
> new file mode 100644
> index 000000000000..c680fd948865
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
> @@ -0,0 +1,79 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright (c) 2020, Linaro Limited */
> +
> +#ifndef _DPU_HW_DSC_H
> +#define _DPU_HW_DSC_H
> +
> +#include <drm/drm_dsc.h>
> +
> +#define DSC_MODE_SPLIT_PANEL BIT(0)
> +#define DSC_MODE_MULTIPLEX BIT(1)
> +#define DSC_MODE_VIDEO BIT(2)
> +
> +struct dpu_hw_dsc;
> +
> +/**
> + * struct dpu_hw_dsc_ops - interface to the dsc hardware driver functions
> + * Assumption is these functions will be called after clocks are enabled
> + */
> +struct dpu_hw_dsc_ops {
> + /**
> + * dsc_disable - disable dsc
> + * @hw_dsc: Pointer to dsc context
> + */
> + void (*dsc_disable)(struct dpu_hw_dsc *hw_dsc);
> +
> + /**
> + * dsc_config - configures dsc encoder
> + * @hw_dsc: Pointer to dsc context
> + * @dsc: panel dsc parameters
> + * @mode: dsc topology mode to be set
> + * @ich_reset_override: option to reset ich
> + */
> + void (*dsc_config)(struct dpu_hw_dsc *hw_dsc,
> + struct msm_display_dsc_config *dsc,
> + u32 mode, bool ich_reset_override);
> +
> + /**
> + * dsc_config_thresh - programs panel thresholds
> + * @hw_dsc: Pointer to dsc context
> + * @dsc: panel dsc parameters
> + */
> + void (*dsc_config_thresh)(struct dpu_hw_dsc *hw_dsc,
> + struct msm_display_dsc_config *dsc);
> +};
> +
> +struct dpu_hw_dsc {
> + struct dpu_hw_blk base;
> + struct dpu_hw_blk_reg_map hw;
> +
> + /* dsc */
> + enum dpu_dsc idx;
> + const struct dpu_dsc_cfg *caps;
> +
> + /* ops */
> + struct dpu_hw_dsc_ops ops;
> +};
> +
> +/**
> + * dpu_hw_dsc_init - initializes the dsc block for the passed dsc idx.
> + * @idx: DSC index for which driver object is required
> + * @addr: Mapped register io address of MDP
> + * @m: Pointer to mdss catalog data
> + * Returns: Error code or allocated dpu_hw_dsc context
> + */
> +struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc idx, void __iomem *addr,
> + struct dpu_mdss_cfg *m);
> +
> +/**
> + * dpu_hw_dsc_destroy - destroys dsc driver context
> + * @dsc: Pointer to dsc driver context returned by dpu_hw_dsc_init
> + */
> +void dpu_hw_dsc_destroy(struct dpu_hw_dsc *dsc);
> +
> +static inline struct dpu_hw_dsc *to_dpu_hw_dsc(struct dpu_hw_blk *hw)
> +{
> + return container_of(hw, struct dpu_hw_dsc, base);
> +}
> +
> +#endif /* _DPU_HW_DSC_H */
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> index 09a3fb3e89f5..1b72c11090ee 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> @@ -97,6 +97,7 @@ enum dpu_hw_blk_type {
> DPU_HW_BLK_WB,
> DPU_HW_BLK_DSPP,
> DPU_HW_BLK_MERGE_3D,
> + DPU_HW_BLK_DSC,
> DPU_HW_BLK_MAX,
> };
>
> @@ -176,6 +177,17 @@ enum dpu_ctl {
> CTL_MAX
> };
>
> +enum dpu_dsc {
> + DSC_NONE = 0,
> + DSC_0,
> + DSC_1,
> + DSC_2,
> + DSC_3,
> + DSC_4,
> + DSC_5,
> + DSC_MAX
> +};
> +
> enum dpu_pingpong {
> PINGPONG_0 = 1,
> PINGPONG_1,
> @@ -437,5 +449,6 @@ struct dpu_mdss_color {
> #define DPU_DBG_MASK_VBIF (1 << 8)
> #define DPU_DBG_MASK_ROT (1 << 9)
> #define DPU_DBG_MASK_DSPP (1 << 10)
> +#define DPU_DBG_MASK_DSC (1 << 11)
>
> #endif /* _DPU_HW_MDSS_H */
>
--
With best wishes
Dmitry
On 21/05/2021 15:49, 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 <[email protected]>
> ---
> drivers/gpu/drm/msm/Makefile | 1 +
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 26 +++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 221 ++++++++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 79 +++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 13 ++
> 5 files changed, 340 insertions(+)
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
>
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index 610d630326bb..fd8fc57f1f58 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -61,6 +61,7 @@ msm-y := \
> disp/dpu1/dpu_hw_blk.o \
> disp/dpu1/dpu_hw_catalog.o \
> disp/dpu1/dpu_hw_ctl.o \
> + disp/dpu1/dpu_hw_dsc.o \
> disp/dpu1/dpu_hw_interrupts.o \
> disp/dpu1/dpu_hw_intf.o \
> disp/dpu1/dpu_hw_lm.o \
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> index 4dfd8a20ad5c..a699633f7013 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -165,6 +165,7 @@ enum {
> * @DPU_PINGPONG_TE2 Additional tear check block for split pipes
> * @DPU_PINGPONG_SPLIT PP block supports split fifo
> * @DPU_PINGPONG_SLAVE PP block is a suitable slave for split fifo
> + * @DPU_PINGPONG_DSC Display stream compression blocks
> * @DPU_PINGPONG_DITHER, Dither blocks
> * @DPU_PINGPONG_MAX
> */
> @@ -173,10 +174,21 @@ enum {
> DPU_PINGPONG_TE2,
> DPU_PINGPONG_SPLIT,
> DPU_PINGPONG_SLAVE,
> + DPU_PINGPONG_DSC,
> DPU_PINGPONG_DITHER,
> DPU_PINGPONG_MAX
> };
>
> +/**
> + * DSC sub-blocks
> + * @DPU_DSC DSC sub block
> + * @DPU_DSC_MAX
> + */
> +enum {
> + DPU_DSC = 0x1,
> + DPU_DSC_MAX
> +};
I don't think we need this. DSC is always a DSC. You can safely pass (0)
as a features mask.
Once we'd have to add 1.2 block revision support, we might have to add
it here.
> +
> /**
> * CTL sub-blocks
> * @DPU_CTL_SPLIT_DISPLAY CTL supports video mode split display
> @@ -413,6 +425,7 @@ struct dpu_dspp_sub_blks {
> struct dpu_pingpong_sub_blks {
> struct dpu_pp_blk te;
> struct dpu_pp_blk te2;
> + struct dpu_pp_blk dsc;
Is this used?
> struct dpu_pp_blk dither;
> };
>
> @@ -547,6 +560,16 @@ struct dpu_merge_3d_cfg {
> const struct dpu_merge_3d_sub_blks *sblk;
> };
>
> +/**
> + * struct dpu_dsc_cfg - information of DSC blocks
> + * @id enum identifying this block
> + * @base register offset of this block
> + * @features bit mask identifying sub-blocks/features
> + */
> +struct dpu_dsc_cfg {
> + DPU_HW_BLK_INFO;
> +};
> +
> /**
> * struct dpu_intf_cfg - information of timing engine blocks
> * @id enum identifying this block
> @@ -748,6 +771,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..8b8d0553709d
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
> @@ -0,0 +1,221 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2020, Linaro Limited
> + */
> +
> +#include "dpu_kms.h"
> +#include "dpu_hw_catalog.h"
> +#include "dpu_hwio.h"
> +#include "dpu_hw_mdss.h"
> +#include "dpu_hw_dsc.h"
> +
> +#define DSC_COMMON_MODE 0x000
> +#define DSC_ENC 0X004
> +#define DSC_PICTURE 0x008
> +#define DSC_SLICE 0x00C
> +#define DSC_CHUNK_SIZE 0x010
> +#define DSC_DELAY 0x014
> +#define DSC_SCALE_INITIAL 0x018
> +#define DSC_SCALE_DEC_INTERVAL 0x01C
> +#define DSC_SCALE_INC_INTERVAL 0x020
> +#define DSC_FIRST_LINE_BPG_OFFSET 0x024
> +#define DSC_BPG_OFFSET 0x028
> +#define DSC_DSC_OFFSET 0x02C
> +#define DSC_FLATNESS 0x030
> +#define DSC_RC_MODEL_SIZE 0x034
> +#define DSC_RC 0x038
> +#define DSC_RC_BUF_THRESH 0x03C
> +#define DSC_RANGE_MIN_QP 0x074
> +#define DSC_RANGE_MAX_QP 0x0B0
> +#define DSC_RANGE_BPG_OFFSET 0x0EC
> +
> +static void dpu_hw_dsc_disable(struct dpu_hw_dsc *dsc)
> +{
> + struct dpu_hw_blk_reg_map *c = &dsc->hw;
> +
> + DPU_REG_WRITE(c, DSC_COMMON_MODE, 0);
> +}
> +
> +static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc,
> + struct msm_display_dsc_config *dsc,
> + u32 mode, bool ich_reset_override)
> +{
> + struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
> + u32 data;
> + u32 initial_lines = dsc->initial_lines;
> + bool is_cmd_mode = !(mode & BIT(2));
> +
> + DPU_REG_WRITE(c, DSC_COMMON_MODE, mode);
> +
> + data = 0;
> + if (ich_reset_override)
> + data = 3 << 28;
Do we need this? It seems this is only necessary for the half panel
partial updates, which is not enabled for now. Do you know if this is
really handy at this point or if it's an odd case? We might drop the
ich_reset logic alltogether and add it later if/when the need arises.
> +
> + if (is_cmd_mode)
> + initial_lines += 1;
> +
> + data |= (initial_lines << 20);
> + data |= ((dsc->slice_last_group_size) << 18);
> + /* bpp is 6.4 format, 4 LSBs bits are for fractional part */
> + data |= dsc->drm.bits_per_pixel << 12;
> + 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);
> + if (dsc->drm.bits_per_component == 10)
> + data |= BIT(0);
> +
> + 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 = DIV_ROUND_UP(dsc->drm.slice_width * dsc->drm.bits_per_pixel, 8) << 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 = 0x00000184; /* only this value works */
> + 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 = 0x0;
> + u16 *lp;
> + int i;
> +
> + lp = dsc->drm.rc_buf_thresh;
> + off = DSC_RC_BUF_THRESH;
> + for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++) {
> + DPU_REG_WRITE(c, off, *lp++);
Nit: no need for lp here. Use array access like you do below.
> + off += 4;
> + }
> +
> + off = DSC_RANGE_MIN_QP;
> + for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
> + DPU_REG_WRITE(c, off, rc[i].range_min_qp);
> + off += 4;
> + }
> +
> + off = DSC_RANGE_MAX_QP;
> + for (i = 0; i < 15; i++) {
> + DPU_REG_WRITE(c, off, rc[i].range_max_qp);
> + off += 4;
> + }
> +
> + off = DSC_RANGE_BPG_OFFSET;
> + for (i = 0; i < 15; i++) {
> + DPU_REG_WRITE(c, off, rc[i].range_bpg_offset);
> + off += 4;
> + }
> +}
> +
> +static struct dpu_dsc_cfg *_dsc_offset(enum dpu_dsc dsc,
> + struct dpu_mdss_cfg *m,
> + void __iomem *addr,
> + struct dpu_hw_blk_reg_map *b)
> +{
> + int i;
> +
> + for (i = 0; i < m->dsc_count; i++) {
> + if (dsc == m->dsc[i].id) {
> + b->base_off = addr;
> + b->blk_off = m->dsc[i].base;
> + b->length = m->dsc[i].len;
> + b->hwversion = m->hwversion;
> + b->log_mask = DPU_DBG_MASK_DSC;
> + return &m->dsc[i];
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static void _setup_dsc_ops(struct dpu_hw_dsc_ops *ops,
> + unsigned long cap)
> +{
> + ops->dsc_disable = dpu_hw_dsc_disable;
> + ops->dsc_config = dpu_hw_dsc_config;
> + ops->dsc_config_thresh = dpu_hw_dsc_config_thresh;
> +};
> +
> +static struct dpu_hw_blk_ops dpu_hw_ops = {
> + .start = NULL,
> + .stop = NULL,
> +};
> +
> +struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc idx, void __iomem *addr,
> + struct dpu_mdss_cfg *m)
> +{
> + struct dpu_hw_dsc *c;
> + struct dpu_dsc_cfg *cfg;
> +
> + c = kzalloc(sizeof(*c), GFP_KERNEL);
> + if (!c)
> + return ERR_PTR(-ENOMEM);
> +
> + cfg = _dsc_offset(idx, m, addr, &c->hw);
> + if (IS_ERR_OR_NULL(cfg)) {
> + kfree(c);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + c->idx = idx;
> + c->caps = cfg;
> + _setup_dsc_ops(&c->ops, c->caps->features);
> +
> + dpu_hw_blk_init(&c->base, DPU_HW_BLK_DSC, idx, &dpu_hw_ops);
> +
> + return c;
> +}
> +
> +void dpu_hw_dsc_destroy(struct dpu_hw_dsc *dsc)
> +{
> + if (dsc)
> + dpu_hw_blk_destroy(&dsc->base);
> + kfree(dsc);
> +}
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
> new file mode 100644
> index 000000000000..c680fd948865
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
> @@ -0,0 +1,79 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright (c) 2020, Linaro Limited */
> +
> +#ifndef _DPU_HW_DSC_H
> +#define _DPU_HW_DSC_H
> +
> +#include <drm/drm_dsc.h>
> +
> +#define DSC_MODE_SPLIT_PANEL BIT(0)
> +#define DSC_MODE_MULTIPLEX BIT(1)
> +#define DSC_MODE_VIDEO BIT(2)
> +
> +struct dpu_hw_dsc;
> +
> +/**
> + * struct dpu_hw_dsc_ops - interface to the dsc hardware driver functions
> + * Assumption is these functions will be called after clocks are enabled
> + */
> +struct dpu_hw_dsc_ops {
> + /**
> + * dsc_disable - disable dsc
> + * @hw_dsc: Pointer to dsc context
> + */
> + void (*dsc_disable)(struct dpu_hw_dsc *hw_dsc);
> +
> + /**
> + * dsc_config - configures dsc encoder
> + * @hw_dsc: Pointer to dsc context
> + * @dsc: panel dsc parameters
> + * @mode: dsc topology mode to be set
> + * @ich_reset_override: option to reset ich
> + */
> + void (*dsc_config)(struct dpu_hw_dsc *hw_dsc,
> + struct msm_display_dsc_config *dsc,
> + u32 mode, bool ich_reset_override);
> +
> + /**
> + * dsc_config_thresh - programs panel thresholds
> + * @hw_dsc: Pointer to dsc context
> + * @dsc: panel dsc parameters
> + */
> + void (*dsc_config_thresh)(struct dpu_hw_dsc *hw_dsc,
> + struct msm_display_dsc_config *dsc);
> +};
> +
> +struct dpu_hw_dsc {
> + struct dpu_hw_blk base;
> + struct dpu_hw_blk_reg_map hw;
> +
> + /* dsc */
> + enum dpu_dsc idx;
> + const struct dpu_dsc_cfg *caps;
> +
> + /* ops */
> + struct dpu_hw_dsc_ops ops;
> +};
> +
> +/**
> + * dpu_hw_dsc_init - initializes the dsc block for the passed dsc idx.
> + * @idx: DSC index for which driver object is required
> + * @addr: Mapped register io address of MDP
> + * @m: Pointer to mdss catalog data
> + * Returns: Error code or allocated dpu_hw_dsc context
> + */
> +struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc idx, void __iomem *addr,
> + struct dpu_mdss_cfg *m);
> +
> +/**
> + * dpu_hw_dsc_destroy - destroys dsc driver context
> + * @dsc: Pointer to dsc driver context returned by dpu_hw_dsc_init
> + */
> +void dpu_hw_dsc_destroy(struct dpu_hw_dsc *dsc);
> +
> +static inline struct dpu_hw_dsc *to_dpu_hw_dsc(struct dpu_hw_blk *hw)
> +{
> + return container_of(hw, struct dpu_hw_dsc, base);
> +}
> +
> +#endif /* _DPU_HW_DSC_H */
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> index 09a3fb3e89f5..1b72c11090ee 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> @@ -97,6 +97,7 @@ enum dpu_hw_blk_type {
> DPU_HW_BLK_WB,
> DPU_HW_BLK_DSPP,
> DPU_HW_BLK_MERGE_3D,
> + DPU_HW_BLK_DSC,
> DPU_HW_BLK_MAX,
> };
>
> @@ -176,6 +177,17 @@ enum dpu_ctl {
> CTL_MAX
> };
>
> +enum dpu_dsc {
> + DSC_NONE = 0,
> + DSC_0,
> + DSC_1,
> + DSC_2,
> + DSC_3,
> + DSC_4,
> + DSC_5,
> + DSC_MAX
> +};
> +
> enum dpu_pingpong {
> PINGPONG_0 = 1,
> PINGPONG_1,
> @@ -437,5 +449,6 @@ struct dpu_mdss_color {
> #define DPU_DBG_MASK_VBIF (1 << 8)
> #define DPU_DBG_MASK_ROT (1 << 9)
> #define DPU_DBG_MASK_DSPP (1 << 10)
> +#define DPU_DBG_MASK_DSC (1 << 11)
>
> #endif /* _DPU_HW_MDSS_H */
>
--
With best wishes
Dmitry
On 21/05/2021 15:49, Vinod Koul wrote:
> This add the bits in RM to enable the DSC blocks
>
> Signed-off-by: Vinod Koul <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 32 +++++++++++++++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 1 +
> 3 files changed, 34 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index d6717d6672f7..d56c05146dfe 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -165,6 +165,7 @@ struct dpu_global_state {
> uint32_t ctl_to_enc_id[CTL_MAX - CTL_0];
> uint32_t intf_to_enc_id[INTF_MAX - INTF_0];
> uint32_t dspp_to_enc_id[DSPP_MAX - DSPP_0];
> + uint32_t dsc_to_enc_id[DSC_MAX - DSC_0];
> };
>
> struct dpu_global_state
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index fd2d104f0a91..4da6d72b7996 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -11,6 +11,7 @@
> #include "dpu_hw_intf.h"
> #include "dpu_hw_dspp.h"
> #include "dpu_hw_merge3d.h"
> +#include "dpu_hw_dsc.h"
> #include "dpu_encoder.h"
> #include "dpu_trace.h"
>
> @@ -75,6 +76,14 @@ int dpu_rm_destroy(struct dpu_rm *rm)
> dpu_hw_intf_destroy(hw);
> }
> }
> + for (i = 0; i < ARRAY_SIZE(rm->dsc_blks); i++) {
> + struct dpu_hw_dsc *hw;
> +
> + if (rm->intf_blks[i]) {
> + hw = to_dpu_hw_dsc(rm->dsc_blks[i]);
> + dpu_hw_dsc_destroy(hw);
> + }
> + }
>
> return 0;
> }
> @@ -221,6 +230,19 @@ int dpu_rm_init(struct dpu_rm *rm,
> rm->dspp_blks[dspp->id - DSPP_0] = &hw->base;
> }
>
> + for (i = 0; i < cat->dsc_count; i++) {
> + struct dpu_hw_dsc *hw;
> + const struct dpu_dsc_cfg *dsc = &cat->dsc[i];
> +
> + hw = dpu_hw_dsc_init(dsc->id, mmio, cat);
> + if (IS_ERR_OR_NULL(hw)) {
> + rc = PTR_ERR(hw);
> + DPU_ERROR("failed dsc object creation: err %d\n", rc);
> + goto fail;
> + }
> + rm->dsc_blks[dsc->id - DSC_0] = &hw->base;
> + }
> +
> return 0;
>
> fail:
> @@ -476,6 +498,9 @@ static int _dpu_rm_reserve_intf(
> }
>
> global_state->intf_to_enc_id[idx] = enc_id;
> +
> + global_state->dsc_to_enc_id[0] = enc_id;
> + global_state->dsc_to_enc_id[1] = enc_id;
This should bear at least the FIXME.
> return 0;
> }
>
> @@ -567,6 +592,8 @@ void dpu_rm_release(struct dpu_global_state *global_state,
> ARRAY_SIZE(global_state->ctl_to_enc_id), enc->base.id);
> _dpu_rm_clear_mapping(global_state->intf_to_enc_id,
> ARRAY_SIZE(global_state->intf_to_enc_id), enc->base.id);
> + _dpu_rm_clear_mapping(global_state->dsc_to_enc_id,
> + ARRAY_SIZE(global_state->dsc_to_enc_id), enc->base.id);
> }
>
> int dpu_rm_reserve(
> @@ -640,6 +667,11 @@ int dpu_rm_get_assigned_resources(struct dpu_rm *rm,
> hw_to_enc_id = global_state->dspp_to_enc_id;
> max_blks = ARRAY_SIZE(rm->dspp_blks);
> break;
> + case DPU_HW_BLK_DSC:
> + hw_blks = rm->dsc_blks;
> + hw_to_enc_id = global_state->dsc_to_enc_id;
> + max_blks = ARRAY_SIZE(rm->dsc_blks);
> + break;
> default:
> DPU_ERROR("blk type %d not managed by rm\n", type);
> return 0;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> index 1f12c8d5b8aa..278d2a510b80 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> @@ -30,6 +30,7 @@ struct dpu_rm {
> struct dpu_hw_blk *intf_blks[INTF_MAX - INTF_0];
> struct dpu_hw_blk *dspp_blks[DSPP_MAX - DSPP_0];
> struct dpu_hw_blk *merge_3d_blks[MERGE_3D_MAX - MERGE_3D_0];
> + struct dpu_hw_blk *dsc_blks[DSC_MAX - DSC_0];
>
> uint32_t lm_max_width;
> };
>
--
With best wishes
Dmitry
On 21/05/2021 15:49, 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.
>
> Furthermore, we can use 1 DSC encoder in lesser resulutions, but that is
> not power efficient according to Abhinav, so it is recommended to always
> use 2 encoders.
Not power efficient because the second DSC would also be powered on or
because single DSC enc would consume more power than two DSCs?
>
> So for now we blindly create 2,2,1 topology when DSC is enabled
>
> Co-developed-by: Abhinav Kumar <[email protected]>
> Signed-off-by: Abhinav Kumar <[email protected]>
> Signed-off-by: Vinod Koul <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 18cb1274a8bb..bffb40085c67 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -609,8 +609,22 @@ static struct msm_display_topology dpu_encoder_get_topology(
> topology.num_enc = 0;
> topology.num_intf = intf_count;
>
> + drm_enc = &dpu_enc->base;
> + priv = drm_enc->dev->dev_private;
> + if (priv && priv->dsc) {
> + /* In case of Display Stream Compression DSC, we would use
> + * 2 encoders, 2 line mixers and 1 interface
> + * this is power optimal and can drive upto (including) 4k
> + * screens
> + */
> + topology.num_enc = 2;
> + topology.num_intf = 1;
> + topology.num_lm = 2;
> + }
> +
> return topology;
> }
> +
> static int dpu_encoder_virt_atomic_check(
> struct drm_encoder *drm_enc,
> struct drm_crtc_state *crtc_state,
>
--
With best wishes
Dmitry
On 21/05/2021 15:49, Vinod Koul wrote:
> DSC needs some configuration from device tree, add support to read and
> store these params and add DSC structures in msm_drv
>
> Signed-off-by: Vinod Koul <[email protected]>
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 170 +++++++++++++++++++++++++++++
> drivers/gpu/drm/msm/msm_drv.h | 32 ++++++
> 2 files changed, 202 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 8a10e4343281..864d3c655e73 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -156,6 +156,7 @@ struct msm_dsi_host {
> struct regmap *sfpb;
>
> struct drm_display_mode *mode;
> + struct msm_display_dsc_config *dsc;
>
> /* connected device info */
> struct device_node *device_node;
> @@ -1744,6 +1745,168 @@ 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
> +};
I think we should move this table to a generic place. AMD and Intel DSC
code uses the same table, shifted by 6 (and both of those drivers shift
it before writing to the HW). Intel modifies this table for 6bpp case.
AMD seems to use it as is.
> +
> +/* 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
> +};
And these parameters seem to be generic too. Intel DSC code contains
them in a bit different form. Should we probably move them to the
drm_dsc.c and use the tables the generic location?
AMD drivers uses a bit different values at the first glance, so let's
stick with Intel version.
> +
> +static int dsi_populate_dsc_params(struct msm_display_dsc_config *dsc)
> +{
> + int i;
> +
> + dsc->drm.rc_model_size = 8192;
> + dsc->drm.first_line_bpg_offset = 15;
> + 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;
> + 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;
> + 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
> + */
> +
> + i = dsc->drm.slice_width % 3;
> + switch (i) {
> + case 0:
> + dsc->slice_last_group_size = 2;
> + break;
> +
> + case 1:
> + dsc->slice_last_group_size = 0;
> + break;
> +
> + case 2:
> + dsc->slice_last_group_size = 0;
> + break;
> +
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
--
With best wishes
Dmitry
On 21/05/2021 15:49, 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 <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 11 +++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 2 ++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 5 +++--
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 2 ++
> 4 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> index ecbc4be98980..d43b804528eb 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -336,6 +336,17 @@ static inline enum dpu_3d_blend_mode dpu_encoder_helper_get_3d_blend_mode(
> return BLEND_3D_NONE;
> }
>
> +static inline bool dpu_encoder_helper_get_dsc_mode(struct dpu_encoder_phys *phys_enc)
> +{
> + struct drm_encoder *drm_enc = phys_enc->parent;
> + struct msm_drm_private *priv = drm_enc->dev->dev_private;
> +
> + if (priv->dsc)
> + return true;
> +
> + return false;
> +}
> +
> /**
> * dpu_encoder_helper_split_config - split display configuration helper function
> * This helper function may be used by physical encoders to configure
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> index b2be39b9144e..5fe87881c30c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> @@ -69,6 +69,8 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg(
> intf_cfg.intf_mode_sel = DPU_CTL_MODE_SEL_CMD;
> intf_cfg.stream_sel = cmd_enc->stream_sel;
> intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
> + intf_cfg.dsc = dpu_encoder_helper_get_dsc_mode(phys_enc);
> +
> ctl->ops.setup_intf_cfg(ctl, &intf_cfg);
> }
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index aeea6add61ee..f059416311ee 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -121,7 +121,7 @@ static u32 dpu_hw_ctl_get_pending_flush(struct dpu_hw_ctl *ctx)
> return ctx->pending_flush_mask;
> }
>
> -static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
> +static void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
> {
> DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | BIT(3));
>
> @@ -522,7 +522,8 @@ static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
>
> intf_cfg |= (cfg->intf & 0xF) << 4;
>
> - if (cfg->mode_3d) {
> + /* In DSC we can't set merge, so check for dsc too */
> + if (cfg->mode_3d && !cfg->dsc) {
I think we should catch this earlier (in atomic_check?), so that we
won't enable modes that would require merge_3d with DSC enabled (until
we support DSC merge?)
> intf_cfg |= BIT(19);
> intf_cfg |= (cfg->mode_3d - 0x1) << 20;
> }
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> index 806c171e5df2..347a653c1e01 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> @@ -39,6 +39,7 @@ struct dpu_hw_stage_cfg {
> * @mode_3d: 3d mux configuration
> * @merge_3d: 3d merge block used
> * @intf_mode_sel: Interface mode, cmd / vid
> + * @dsc: DSC is enabled
> * @stream_sel: Stream selection for multi-stream interfaces
> */
> struct dpu_hw_intf_cfg {
> @@ -46,6 +47,7 @@ struct dpu_hw_intf_cfg {
> enum dpu_3d_blend_mode mode_3d;
> enum dpu_merge_3d merge_3d;
> enum dpu_ctl_mode_sel intf_mode_sel;
> + bool dsc;
> int stream_sel;
> };
>
>
--
With best wishes
Dmitry
On 2021-05-28 03:39, Dmitry Baryshkov wrote:
> On 21/05/2021 15:49, 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.
>>
>> Furthermore, we can use 1 DSC encoder in lesser resulutions, but that
>> is
>> not power efficient according to Abhinav, so it is recommended to
>> always
>> use 2 encoders.
>
> Not power efficient because the second DSC would also be powered on or
> because single DSC enc would consume more power than two DSCs?
I havent got through the series yet but just thought of answering this,
So before coming to the power aspects of this, hard-coding was done for
the foll reasons:
-> We do not have a topology DTSI property in upstream and will probably
not have as well till
other features are added which support all the topologies
-> The DSC panel which is being upstreamed as part of this series is
working with this 2,2,1 topology
downstream ( dual lm, dual DSC encoders, single DSI ). Other topologies
have not been tried on it yet
-> There needs to be a better approach to handle all topologies once we
have added support for them.
It can be either a DTSI property if others agree OR some helper API
which will determine the best topology
based on various factors. Till then, since this will be the only DSC
panel we are adding support for
I thought we can start with a fixed topology for now.
Coming to the power aspect, I only recommended 2-2-1 here because using
two mixers is better power wise
as it will split the width/2. We can also do 2-1-1 by enabling 3D mux
but this panel has not been validated
with a single DSC. So to keep things simple with what has been
validated, I thought we can go ahead with
2-2-1 for now.
So rather than giving too much importance to the power aspect of it, the
other reasons should also
be highlighted here as the main reason and the commit text should give
these details as well.
>>
>> So for now we blindly create 2,2,1 topology when DSC is enabled
>>
>> Co-developed-by: Abhinav Kumar <[email protected]>
>> Signed-off-by: Abhinav Kumar <[email protected]>
>> Signed-off-by: Vinod Koul <[email protected]>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index 18cb1274a8bb..bffb40085c67 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -609,8 +609,22 @@ static struct msm_display_topology
>> dpu_encoder_get_topology(
>> topology.num_enc = 0;
>> topology.num_intf = intf_count;
>> + drm_enc = &dpu_enc->base;
>> + priv = drm_enc->dev->dev_private;
>> + if (priv && priv->dsc) {
>> + /* In case of Display Stream Compression DSC, we would use
>> + * 2 encoders, 2 line mixers and 1 interface
>> + * this is power optimal and can drive upto (including) 4k
>> + * screens
>> + */
>> + topology.num_enc = 2;
>> + topology.num_intf = 1;
>> + topology.num_lm = 2;
>> + }
>> +
>> return topology;
>> }
>> +
>> static int dpu_encoder_virt_atomic_check(
>> struct drm_encoder *drm_enc,
>> struct drm_crtc_state *crtc_state,
>>
On 29/05/2021 01:23, [email protected] wrote:
> On 2021-05-28 03:39, Dmitry Baryshkov wrote:
>> On 21/05/2021 15:49, 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.
>>>
>>> Furthermore, we can use 1 DSC encoder in lesser resulutions, but that is
>>> not power efficient according to Abhinav, so it is recommended to always
>>> use 2 encoders.
>>
>> Not power efficient because the second DSC would also be powered on or
>> because single DSC enc would consume more power than two DSCs?
>
> I havent got through the series yet but just thought of answering this,
>
> So before coming to the power aspects of this, hard-coding was done for
> the foll reasons:
>
> -> We do not have a topology DTSI property in upstream and will probably
> not have as well till
> other features are added which support all the topologies
> -> The DSC panel which is being upstreamed as part of this series is
> working with this 2,2,1 topology
> downstream ( dual lm, dual DSC encoders, single DSI ). Other topologies
> have not been tried on it yet
> -> There needs to be a better approach to handle all topologies once we
> have added support for them.
> It can be either a DTSI property if others agree OR some helper API
> which will determine the best topology
> based on various factors. Till then, since this will be the only DSC
> panel we are adding support for
> I thought we can start with a fixed topology for now.
>
> Coming to the power aspect, I only recommended 2-2-1 here because using
> two mixers is better power wise
> as it will split the width/2. We can also do 2-1-1 by enabling 3D mux
> but this panel has not been validated
> with a single DSC. So to keep things simple with what has been
> validated, I thought we can go ahead with
> 2-2-1 for now.
>
> So rather than giving too much importance to the power aspect of it, the
> other reasons should also
> be highlighted here as the main reason and the commit text should give
> these details as well.
Sounds reasonable now, thank you!
>
>>>
>>> So for now we blindly create 2,2,1 topology when DSC is enabled
>>>
>>> Co-developed-by: Abhinav Kumar <[email protected]>
>>> Signed-off-by: Abhinav Kumar <[email protected]>
>>> Signed-off-by: Vinod Koul <[email protected]>
>>> ---
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 14 ++++++++++++++
>>> 1 file changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> index 18cb1274a8bb..bffb40085c67 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> @@ -609,8 +609,22 @@ static struct msm_display_topology
>>> dpu_encoder_get_topology(
>>> topology.num_enc = 0;
>>> topology.num_intf = intf_count;
>>> + drm_enc = &dpu_enc->base;
>>> + priv = drm_enc->dev->dev_private;
>>> + if (priv && priv->dsc) {
>>> + /* In case of Display Stream Compression DSC, we would use
>>> + * 2 encoders, 2 line mixers and 1 interface
>>> + * this is power optimal and can drive upto (including) 4k
>>> + * screens
>>> + */
>>> + topology.num_enc = 2;
>>> + topology.num_intf = 1;
>>> + topology.num_lm = 2;
>>> + }
>>> +
>>> return topology;
>>> }
>>> +
>>> static int dpu_encoder_virt_atomic_check(
>>> struct drm_encoder *drm_enc,
>>> struct drm_crtc_state *crtc_state,
>>>
--
With best wishes
Dmitry
On 26-05-21, 09:00, Jeffrey Hugo wrote:
> On Tue, May 25, 2021 at 11:46 PM Vinod Koul <[email protected]> wrote:
> > On 21-05-21, 08:09, Jeffrey Hugo wrote:
> > > On Fri, May 21, 2021 at 6:50 AM Vinod Koul <[email protected]> wrote:
> > > >
> > > > Display Stream Compression (DSC) compresses the display stream in host which
> > > > is later decoded by panel. This series enables this for Qualcomm msm driver.
> > > > This was tested on Google Pixel3 phone which use LGE SW43408 panel.
> > > >
> > > > The changes include adding DT properties for DSC then hardware blocks support
> > > > required in DPU1 driver and support in encoder. We also add support in DSI
> > > > and introduce required topology changes.
> > > >
> > > > In order for panel to set the DSC parameters we add dsc in drm_panel and set
> > > > it from the msm driver.
> > > >
> > > > Complete changes which enable this for Pixel3 along with panel driver (not
> > > > part of this series) and DT changes can be found at:
> > > > git.linaro.org/people/vinod.koul/kernel.git pixel/dsc_rfc
> > > >
> > > > Comments welcome!
> > >
> > > This feels backwards to me. I've only skimmed this series, and the DT
> > > changes didn't come through for me, so perhaps I have an incomplete
> > > view.
> >
> > Not sure why, I see it on lore:
> > https://lore.kernel.org/dri-devel/[email protected]/
> >
> > > DSC is not MSM specific. There is a standard for it. Yet it looks
> > > like everything is implemented in a MSM specific way, and then pushed
> > > to the panel. So, every vendor needs to implement their vendor
> > > specific way to get the DSC info, and then push it to the panel?
> > > Seems wrong, given there is an actual standard for this feature.
> >
> > I have added slice and bpp info in the DT here under the host and then
> > pass the generic struct drm_dsc_config to panel which allows panel to
> > write the pps cmd
> >
> > Nothing above is MSM specific.. It can very well work with non MSM
> > controllers too.
>
> I disagree.
>
> The DT bindings you defined (thanks for the direct link) are MSM
> specific. I'm not talking (yet) about the properties you defined, but
> purely from the stand point that you defined the binding within the
> scope of the MSM dsi binding. No other vendor can use those bindings.
> Of course, if we look at the properties themselves, they are prefixed
> with "qcom", which is vendor specific.
>
> So, purely on the face of it, this is MSM specific.
>
> Assuming we want a DT solution for DSC, I think it should be something
> like Documentation/devicetree/bindings/clock/clock-bindings.txt (the
> first example that comes to mind), which is a non-vendor specific
> generic set of properties that each vendor/device specific binding can
> inherit. Panel has similar things.
>
> Specific to the properties, I don't much like that you duplicate BPP,
> which is already associated with the panel (although perhaps not in
> the scope of DT). What if the panel and your DSC bindings disagree?
> Also, I guess I need to ask, have you read the DSC spec? Last I
> looked, there were something like 3 dozen properties that could be
> configured. You have five in your proposed binding. To me, this is
> not a generic DSC solution, this is MSM specific (and frankly I don't
> think this supports all the configuration the MSM hardware can do,
> either).
I would concede the point that DT is msm specific. I dont disagree on
making them a common dsc biding which anyone can use. I think your idea
does have merits...
I am still not sure who should include these properties, would it be the
panel or host. Either would work and rest of the system can use these
properties...
--
~Vinod
On 27-05-21, 16:30, Rob Clark wrote:
> On Wed, May 26, 2021 at 8:00 AM Jeffrey Hugo <[email protected]> wrote:
> > On Tue, May 25, 2021 at 11:46 PM Vinod Koul <[email protected]> wrote:
> > Frankly, I don't like the MSM ACPI solution that I've seen on the laptops.
> > The ACPI assumes the entire MDSS (including DSI parts) and GPU is one
> > device, and ultimately handled by one driver. That driver needs to
> > get a value from UEFI (set by the bootloader) that is the "panel id".
> > Then the driver calls into ACPI (I think its _ROM, but I might be
> > mistaken, doing this from memory) with that id. It gets back a binary
> > blob which is mostly an xml file (format is publicly documented) that
> > contains the panel timings and such.
>
> tbh, I kinda suspect that having a single "gpu" device (which also
> includes venus, in addition to display, IIRC) in the ACPI tables is a
> windowsism, trying to make things look to userspace like a single "GPU
> card" in the x86 world.. but either way, I think the ACPI tables on
> the windows arm laptops which use dsi->bridge->edp is too much of a
> lost cause to even consider here. Possibly ACPI boot on these devices
> would be more feasible on newer devices which have direct eDP out of
> the SoC without requiring external bridge/panel glue.
yeah that is always a very different world. although it might make sense
to use information in tables and try to deduce information about the
system can be helpful...
> I'd worry more about what makes sense in a DT world, when it comes to
> DT bindings.
And do you have thoughts on that..?
--
~Vinod
On 28-05-21, 02:45, Dmitry Baryshkov wrote:
> On 21/05/2021 15:49, Vinod Koul wrote:
> > DSC needs some configuration from device tree, add support to read and
> > store these params and add DSC structures in msm_drv
> >
> > Signed-off-by: Vinod Koul <[email protected]>
> > ---
> > drivers/gpu/drm/msm/dsi/dsi_host.c | 170 +++++++++++++++++++++++++++++
> > drivers/gpu/drm/msm/msm_drv.h | 32 ++++++
> > 2 files changed, 202 insertions(+)
> >
>
>
> [skipped]
>
>
> > DRM_DEV_ERROR(dev, "%s: invalid lane configuration %d\n",
> > diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> > index 2668941df529..26661dd43936 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>
> > @@ -70,6 +71,16 @@ enum msm_mdp_plane_property {
> > #define MSM_GPU_MAX_RINGS 4
> > #define MAX_H_TILES_PER_DISPLAY 2
> > +/**
> > + * enum msm_display_compression_type - compression method used for pixel stream
> > + * @MSM_DISPLAY_COMPRESSION_NONE: Pixel data is not compressed
> > + * @MSM_DISPLAY_COMPRESSION_DSC: DSC compresison is used
> > + */
> > +enum msm_display_compression_type {
> > + MSM_DISPLAY_COMPRESSION_NONE,
> > + MSM_DISPLAY_COMPRESSION_DSC,
> > +};
> > +
>
> Seems to be unused
Yeah this was started from downstream which used this and I seem to have
not cleared this up, thanks for pointing. Will remove..
--
~Vinod
On 28-05-21, 13:29, Dmitry Baryshkov wrote:
> On 21/05/2021 15:49, Vinod Koul wrote:
> > DSC needs some configuration from device tree, add support to read and
> > store these params and add DSC structures in msm_drv
> >
> > Signed-off-by: Vinod Koul <[email protected]>
> > ---
> > drivers/gpu/drm/msm/dsi/dsi_host.c | 170 +++++++++++++++++++++++++++++
> > drivers/gpu/drm/msm/msm_drv.h | 32 ++++++
> > 2 files changed, 202 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > index 8a10e4343281..864d3c655e73 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > @@ -156,6 +156,7 @@ struct msm_dsi_host {
> > struct regmap *sfpb;
> > struct drm_display_mode *mode;
> > + struct msm_display_dsc_config *dsc;
> > /* connected device info */
> > struct device_node *device_node;
> > @@ -1744,6 +1745,168 @@ 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
> > +};
>
> I think we should move this table to a generic place. AMD and Intel DSC code
> uses the same table, shifted by 6 (and both of those drivers shift it before
> writing to the HW). Intel modifies this table for 6bpp case. AMD seems to
> use it as is.
>
> > +
> > +/* 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
> > +};
>
> And these parameters seem to be generic too. Intel DSC code contains them in
> a bit different form. Should we probably move them to the drm_dsc.c and use
> the tables the generic location?
>
> AMD drivers uses a bit different values at the first glance, so let's stick
> with Intel version.
Yeah I think this is a good suggestion. I did look into and had this in
my todo. Yes drm_dsc.c would be apt for the move..
--
~Vinod
On 2021-06-02 04:01, Vinod Koul wrote:
> On 27-05-21, 16:30, Rob Clark wrote:
>> On Wed, May 26, 2021 at 8:00 AM Jeffrey Hugo
>> <[email protected]> wrote:
>> > On Tue, May 25, 2021 at 11:46 PM Vinod Koul <[email protected]> wrote:
>
>> > Frankly, I don't like the MSM ACPI solution that I've seen on the laptops.
>> > The ACPI assumes the entire MDSS (including DSI parts) and GPU is one
>> > device, and ultimately handled by one driver. That driver needs to
>> > get a value from UEFI (set by the bootloader) that is the "panel id".
>> > Then the driver calls into ACPI (I think its _ROM, but I might be
>> > mistaken, doing this from memory) with that id. It gets back a binary
>> > blob which is mostly an xml file (format is publicly documented) that
>> > contains the panel timings and such.
>>
>> tbh, I kinda suspect that having a single "gpu" device (which also
>> includes venus, in addition to display, IIRC) in the ACPI tables is a
>> windowsism, trying to make things look to userspace like a single "GPU
>> card" in the x86 world.. but either way, I think the ACPI tables on
>> the windows arm laptops which use dsi->bridge->edp is too much of a
>> lost cause to even consider here. Possibly ACPI boot on these devices
>> would be more feasible on newer devices which have direct eDP out of
>> the SoC without requiring external bridge/panel glue.
>
> yeah that is always a very different world. although it might make
> sense
> to use information in tables and try to deduce information about the
> system can be helpful...
>
>> I'd worry more about what makes sense in a DT world, when it comes to
>> DT bindings.
>
> And do you have thoughts on that..?
At the moment, I will comment on the bindings first and my idea on how
to proceed.
The bindings mentioned here:
https://lore.kernel.org/dri-devel/[email protected]/
seem to be just
taken directly from downstream which was not the plan.
I think all of these should be part of the generic panel bindings as
none of these are QC specific:
@@ -188,6 +195,14 @@ Example:
qcom,master-dsi;
qcom,sync-dual-dsi;
+ qcom,mdss-dsc-enabled;
+ qcom,mdss-slice-height = <16>;
+ qcom,mdss-slice-width = <540>;
+ qcom,mdss-slice-per-pkt = <1>;
+ qcom,mdss-bit-per-component = <8>;
+ qcom,mdss-bit-per-pixel = <8>;
+ qcom,mdss-block-prediction-enable;
+
How about having a panel-dsc.yaml which will have these properties and
have a panel-dsc node to have this information?
I would like to hear the feedback on this proposal then the series can
be reworked.
Thanks
Abhinav
On Wed, Jun 2, 2021 at 4:01 AM Vinod Koul <[email protected]> wrote:
>
> On 27-05-21, 16:30, Rob Clark wrote:
> > On Wed, May 26, 2021 at 8:00 AM Jeffrey Hugo <[email protected]> wrote:
> > > On Tue, May 25, 2021 at 11:46 PM Vinod Koul <[email protected]> wrote:
>
> > > Frankly, I don't like the MSM ACPI solution that I've seen on the laptops.
> > > The ACPI assumes the entire MDSS (including DSI parts) and GPU is one
> > > device, and ultimately handled by one driver. That driver needs to
> > > get a value from UEFI (set by the bootloader) that is the "panel id".
> > > Then the driver calls into ACPI (I think its _ROM, but I might be
> > > mistaken, doing this from memory) with that id. It gets back a binary
> > > blob which is mostly an xml file (format is publicly documented) that
> > > contains the panel timings and such.
> >
> > tbh, I kinda suspect that having a single "gpu" device (which also
> > includes venus, in addition to display, IIRC) in the ACPI tables is a
> > windowsism, trying to make things look to userspace like a single "GPU
> > card" in the x86 world.. but either way, I think the ACPI tables on
> > the windows arm laptops which use dsi->bridge->edp is too much of a
> > lost cause to even consider here. Possibly ACPI boot on these devices
> > would be more feasible on newer devices which have direct eDP out of
> > the SoC without requiring external bridge/panel glue.
>
> yeah that is always a very different world. although it might make sense
> to use information in tables and try to deduce information about the
> system can be helpful...
>
> > I'd worry more about what makes sense in a DT world, when it comes to
> > DT bindings.
>
> And do you have thoughts on that..?
Only that I wouldn't get too hung up on existing snapdragon ACPI
tables.. not sure if there is prior art as far as ACPI tables for this
on x86 systems, if so that *might* be a thing to consider, but
otherwise it does sound a bit like we want less qcom specific bindings
here. But other than that I'll leave it to folks who spend more time
thinking about bindings.. left to my own devices I'd come up with a
point solution and go back to working on mesa, so that probably isn't
the opinion you want to follow ;-)
BR,
-R
On 03-06-21, 16:40, [email protected] wrote:
> On 2021-06-02 04:01, Vinod Koul wrote:
> > On 27-05-21, 16:30, Rob Clark wrote:
> >
> > yeah that is always a very different world. although it might make sense
> > to use information in tables and try to deduce information about the
> > system can be helpful...
> >
> > > I'd worry more about what makes sense in a DT world, when it comes to
> > > DT bindings.
> >
> > And do you have thoughts on that..?
>
> At the moment, I will comment on the bindings first and my idea on how to
> proceed.
> The bindings mentioned here:
> https://lore.kernel.org/dri-devel/[email protected]/
> seem to be just
> taken directly from downstream which was not the plan.
>
> I think all of these should be part of the generic panel bindings as none of
> these are QC specific:
Okay so we have discussed this w/ Bjorn and Abhinav and here are the
conclusions and recommendations for binding
1. the properties are generic and not msm specific
2. The host supports multiple formats but the one we choose depends
mostly upon panel. Notably host runs the config which the panel supports.
So the recommendations is to add a table of dsc properties in the panel
driver. No DT binding here.
I should also note that for DP we should be able to calculate these
values from EDID like the i915 driver seems to do
With this I will drop the binding patch and move dsc properties to panel
driver
Thanks
--
~Vinod