[PATCH v6 5/8] drm/msm/dpu: add support for DSC encoder v1.2 engine
Dmitry Baryshkov
dmitry.baryshkov at linaro.org
Thu May 11 17:24:38 UTC 2023
On 11/05/2023 20:00, Kuogee Hsieh wrote:
>
> On 5/10/2023 9:52 PM, Dmitry Baryshkov wrote:
>> On 11/05/2023 01:07, Kuogee Hsieh wrote:
>>> Add support for DSC 1.2 by providing the necessary hooks to program
>>> the DPU DSC 1.2 encoder.
>>>
>>> Changes in v3:
>>> -- fixed kernel test rebot report that "__iomem *off" is declared but
>>> not
>>> used at dpu_hw_dsc_config_1_2()
>>> -- unrolling thresh loops
>>>
>>> Changes in v4:
>>> -- delete DPU_DSC_HW_REV_1_1
>>> -- delete off and used real register name directly
>>>
>>> Reported-by: kernel test robot <lkp at intel.com>
>>> Signed-off-by: Kuogee Hsieh <quic_khsieh at quicinc.com>
>>> ---
>>> drivers/gpu/drm/msm/Makefile | 1 +
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 32 +-
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 14 +-
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 385
>>> +++++++++++++++++++++++++
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +-
>>> 5 files changed, 435 insertions(+), 4 deletions(-)
>>> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
>>>
>>> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
>>> index b814fc8..b9af5e4 100644
>>> --- a/drivers/gpu/drm/msm/Makefile
>>> +++ b/drivers/gpu/drm/msm/Makefile
>>> @@ -65,6 +65,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
>>> disp/dpu1/dpu_hw_catalog.o \
>>> disp/dpu1/dpu_hw_ctl.o \
>>> disp/dpu1/dpu_hw_dsc.o \
>>> + disp/dpu1/dpu_hw_dsc_1_2.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 dc0a4da..4eda2cc 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
>>> @@ -1,6 +1,6 @@
>>> /* SPDX-License-Identifier: GPL-2.0-only */
>>> /*
>>> - * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights
>>> reserved.
>>> + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All
>>> rights reserved.
>>> * Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights
>>> reserved.
>>> */
>>> @@ -244,12 +244,18 @@ enum {
>>> };
>>> /**
>>> - * DSC features
>>> + * DSC sub-blocks/features
>>> * @DPU_DSC_OUTPUT_CTRL Configure which PINGPONG block gets
>>> * the pixel output from this DSC.
>>> + * @DPU_DSC_HW_REV_1_2 DSC block supports dsc 1.1 and 1.2
>>> + * @DPU_DSC_NATIVE_422_EN Supports native422 and native420 encoding
>>> + * @DPU_DSC_MAX
>>> */
>>> enum {
>>> DPU_DSC_OUTPUT_CTRL = 0x1,
>>> + DPU_DSC_HW_REV_1_2,
>>> + DPU_DSC_NATIVE_422_EN,
>>> + DPU_DSC_MAX
>>> };
>>> /**
>>> @@ -306,6 +312,14 @@ struct dpu_pp_blk {
>>> };
>>> /**
>>> + * struct dpu_dsc_blk - DSC Encoder sub-blk information
>>> + * @info: HW register and features supported by this sub-blk
>>> + */
>>> +struct dpu_dsc_blk {
>>> + DPU_HW_SUBBLK_INFO;
>>> +};
>>> +
>>> +/**
>>> * enum dpu_qos_lut_usage - define QoS LUT use cases
>>> */
>>> enum dpu_qos_lut_usage {
>>> @@ -452,6 +466,17 @@ struct dpu_pingpong_sub_blks {
>>> };
>>> /**
>>> + * struct dpu_dsc_sub_blks - DSC sub-blks
>>> + * @enc: DSC encoder sub block
>>> + * @ctl: DSC controller sub block
>>> + *
>>> + */
>>> +struct dpu_dsc_sub_blks {
>>> + struct dpu_dsc_blk enc;
>>> + struct dpu_dsc_blk ctl;
>>> +};
>>> +
>>> +/**
>>> * dpu_clk_ctrl_type - Defines top level clock control signals
>>> */
>>> enum dpu_clk_ctrl_type {
>>> @@ -605,10 +630,13 @@ struct dpu_merge_3d_cfg {
>>> * struct dpu_dsc_cfg - information of DSC blocks
>>> * @id enum identifying this block
>>> * @base register offset of this block
>>> + * @len: length of hardware block
>>> * @features bit mask identifying sub-blocks/features
>>> + * @sblk sub-blocks information
>>> */
>>> struct dpu_dsc_cfg {
>>> DPU_HW_BLK_INFO;
>>> + const struct dpu_dsc_sub_blks *sblk;
>>> };
>>> /**
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
>>> index 138080a..bdff74d 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
>>> @@ -1,5 +1,8 @@
>>> /* SPDX-License-Identifier: GPL-2.0-only */
>>> -/* Copyright (c) 2020-2022, Linaro Limited */
>>> +/*
>>> + * Copyright (c) 2020-2022, Linaro Limited
>>> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights
>>> reserved
>>> + */
>>> #ifndef _DPU_HW_DSC_H
>>> #define _DPU_HW_DSC_H
>>> @@ -69,6 +72,15 @@ struct dpu_hw_dsc *dpu_hw_dsc_init(const struct
>>> dpu_dsc_cfg *cfg,
>>> void __iomem *addr);
>>> /**
>>> + * dpu_hw_dsc_init_1_2 - initializes the v1.2 DSC hw driver block
>>> + * @cfg: DSC catalog entry for which driver object is required
>>> + * @addr: Mapped register io address of MDP
>>> + * Returns: Error code or allocated dpu_hw_dsc context
>>> + */
>>> +struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(const struct dpu_dsc_cfg *cfg,
>>> + void __iomem *addr);
>>> +
>>> +/**
>>> * dpu_hw_dsc_destroy - destroys dsc driver context
>>> * @dsc: Pointer to dsc driver context returned by dpu_hw_dsc_init
>>> */
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
>>> new file mode 100644
>>> index 00000000..0c77c85
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
>>> @@ -0,0 +1,385 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
>>> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights
>>> reserved
>>> + */
>>> +
>>> +#include <drm/display/drm_dsc_helper.h>
>>> +
>>> +#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_CMN_MAIN_CNF 0x00
>>> +
>>> +/* DPU_DSC_ENC register offsets */
>>> +#define ENC_DF_CTRL 0x00
>>> +#define ENC_GENERAL_STATUS 0x04
>>> +#define ENC_HSLICE_STATUS 0x08
>>> +#define ENC_OUT_STATUS 0x0C
>>> +#define ENC_INT_STAT 0x10
>>> +#define ENC_INT_CLR 0x14
>>> +#define ENC_INT_MASK 0x18
>>> +#define DSC_MAIN_CONF 0x30
>>> +#define DSC_PICTURE_SIZE 0x34
>>> +#define DSC_SLICE_SIZE 0x38
>>> +#define DSC_MISC_SIZE 0x3C
>>> +#define DSC_HRD_DELAYS 0x40
>>> +#define DSC_RC_SCALE 0x44
>>> +#define DSC_RC_SCALE_INC_DEC 0x48
>>> +#define DSC_RC_OFFSETS_1 0x4C
>>> +#define DSC_RC_OFFSETS_2 0x50
>>> +#define DSC_RC_OFFSETS_3 0x54
>>> +#define DSC_RC_OFFSETS_4 0x58
>>> +#define DSC_FLATNESS_QP 0x5C
>>> +#define DSC_RC_MODEL_SIZE 0x60
>>> +#define DSC_RC_CONFIG 0x64
>>> +#define DSC_RC_BUF_THRESH_0 0x68
>>> +#define DSC_RC_BUF_THRESH_1 0x6C
>>> +#define DSC_RC_BUF_THRESH_2 0x70
>>> +#define DSC_RC_BUF_THRESH_3 0x74
>>> +#define DSC_RC_MIN_QP_0 0x78
>>> +#define DSC_RC_MIN_QP_1 0x7C
>>> +#define DSC_RC_MIN_QP_2 0x80
>>> +#define DSC_RC_MAX_QP_0 0x84
>>> +#define DSC_RC_MAX_QP_1 0x88
>>> +#define DSC_RC_MAX_QP_2 0x8C
>>> +#define DSC_RC_RANGE_BPG_OFFSETS_0 0x90
>>> +#define DSC_RC_RANGE_BPG_OFFSETS_1 0x94
>>> +#define DSC_RC_RANGE_BPG_OFFSETS_2 0x98
>>> +
>>> +/* DPU_DSC_CTL register offsets */
>>> +#define DSC_CTL 0x00
>>> +#define DSC_CFG 0x04
>>> +#define DSC_DATA_IN_SWAP 0x08
>>> +#define DSC_CLK_CTRL 0x0C
>>> +
>>> +static inline int _dsc_calc_ob_max_addr(struct dpu_hw_dsc *hw_dsc,
>>> int num_ss)
>>> +{
>>> + int max_addr = 2400 / num_ss;
>>> +
>>> + if (hw_dsc->caps->features & BIT(DPU_DSC_NATIVE_422_EN))
>>> + max_addr /= 2;
>>> +
>>> + return max_addr - 1;
>>> +};
>>> +
>>> +static inline void dpu_hw_dsc_disable_1_2(struct dpu_hw_dsc *hw_dsc)
>>> +{
>>> + struct dpu_hw_blk_reg_map *hw;
>>> + u32 offset;
>>> +
>>> + if (!hw_dsc)
>>> + return;
>>> +
>>> + hw = &hw_dsc->hw;
>>> + offset = hw_dsc->caps->sblk->ctl.base;
>>> + DPU_REG_WRITE(hw, offset + DSC_CFG, 0);
>>> +
>>> + offset = hw_dsc->caps->sblk->enc.base;
>>> + DPU_REG_WRITE(hw, offset + ENC_DF_CTRL, 0);
>>> + DPU_REG_WRITE(hw, offset + DSC_MAIN_CONF, 0);
>>
>> This has the same problem as before. By looking at this line you can
>> not check where this write is targeted. Instead I'd suggest storing
>> sblk in the local variable and then using something like:
>>
>> DPU_REG_WRITE(hw, sblk->ctl.base + DSC_CFG, 0);
>>
>> Same applies to other functions in this patch.
>>
>>> +}
>>> +
>>> +static inline void dpu_hw_dsc_config_1_2(struct dpu_hw_dsc *hw_dsc,
>>> + struct drm_dsc_config *dsc,
>>> + u32 mode,
>>> + u32 initial_lines)
>>
>> Please indent to the opening brace. If using Vim, this is the "set
>> cino=(0" setting
>>
>>> +{
>>> + struct dpu_hw_blk_reg_map *hw;
>>> + u32 offset;
>>> + u32 data = 0;
>>> + u32 det_thresh_flatness;
>>> + u32 num_active_ss_per_enc;
>>
>> s/_ss_/_slice_/g
>>
>>> + u32 bpp;
>>> +
>>> + if (!hw_dsc || !dsc)
>>> + return;
>>> +
>>> + hw = &hw_dsc->hw;
>>> +
>>> + offset = hw_dsc->caps->sblk->enc.base;
>>> +
>>> + if (mode & DSC_MODE_SPLIT_PANEL)
>>> + data |= BIT(0);
>>> +
>>> + if (mode & DSC_MODE_MULTIPLEX)
>>> + data |= BIT(1);
>>> +
>>> + num_active_ss_per_enc = dsc->slice_count;
>>> + if (mode & DSC_MODE_MULTIPLEX)
>>> + num_active_ss_per_enc = dsc->slice_count >> 1;
>>> +
>>> + data |= (num_active_ss_per_enc & 0x3) << 7;
>>> +
>>> + DPU_REG_WRITE(hw, DSC_CMN_MAIN_CNF, data);
>>> +
>>> + data = (initial_lines & 0xff);
>>> +
>>> + if (mode & DSC_MODE_VIDEO)
>>> + data |= BIT(9);
>>> +
>>> + data |= (_dsc_calc_ob_max_addr(hw_dsc, num_active_ss_per_enc) <<
>>> 18);
>>> +
>>> + DPU_REG_WRITE(hw, offset + ENC_DF_CTRL, data);
>>> +
>>> + data = (dsc->dsc_version_minor & 0xf) << 28;
>>> + if (dsc->dsc_version_minor == 0x2) {
>>> + if (dsc->native_422)
>>> + data |= BIT(22);
>>> + if (dsc->native_420)
>>> + data |= BIT(21);
>>> + }
>>> +
>>> + bpp = dsc->bits_per_pixel;
>>> + /* as per hw requirement bpp should be programmed
>>> + * twice the actual value in case of 420 or 422 encoding
>>> + */
>>> + if (dsc->native_422 || dsc->native_420)
>>> + bpp = 2 * bpp;
>>> + data |= (dsc->block_pred_enable ? 1 : 0) << 20;
>>> + data |= bpp << 10;
>>> + data |= (dsc->line_buf_depth & 0xf) << 6;
>>> + data |= dsc->convert_rgb << 4;
>>> + data |= dsc->bits_per_component & 0xf;
>>> +
>>> + DPU_REG_WRITE(hw, offset + DSC_MAIN_CONF, data);
>>> +
>>> + data = (dsc->pic_width & 0xffff) |
>>> + ((dsc->pic_height & 0xffff) << 16);
>>> +
>>> + DPU_REG_WRITE(hw, offset + DSC_PICTURE_SIZE, data);
>>> +
>>> + data = (dsc->slice_width & 0xffff) |
>>> + ((dsc->slice_height & 0xffff) << 16);
>>> +
>>> + DPU_REG_WRITE(hw, offset + DSC_SLICE_SIZE, data);
>>> +
>>> + DPU_REG_WRITE(hw, offset + DSC_MISC_SIZE,
>>> + (dsc->slice_chunk_size) & 0xffff);
>>> +
>>> + data = (dsc->initial_xmit_delay & 0xffff) |
>>> + ((dsc->initial_dec_delay & 0xffff) << 16);
>>> +
>>> + DPU_REG_WRITE(hw, offset + DSC_HRD_DELAYS, data);
>>> +
>>> + DPU_REG_WRITE(hw, offset + DSC_RC_SCALE,
>>> + dsc->initial_scale_value & 0x3f);
>>> +
>>> + data = (dsc->scale_increment_interval & 0xffff) |
>>> + ((dsc->scale_decrement_interval & 0x7ff) << 16);
>>> +
>>> + DPU_REG_WRITE(hw, offset + DSC_RC_SCALE_INC_DEC, data);
>>> +
>>> + data = (dsc->first_line_bpg_offset & 0x1f) |
>>> + ((dsc->second_line_bpg_offset & 0x1f) << 5);
>>> +
>>> + DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_1, data);
>>> +
>>> + data = (dsc->nfl_bpg_offset & 0xffff) |
>>> + ((dsc->slice_bpg_offset & 0xffff) << 16);
>>> +
>>> + DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_2, data);
>>> +
>>> + data = (dsc->initial_offset & 0xffff) |
>>> + ((dsc->final_offset & 0xffff) << 16);
>>> +
>>> + DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_3, data);
>>> +
>>> + data = (dsc->nsl_bpg_offset & 0xffff) |
>>> + ((dsc->second_line_offset_adj & 0xffff) << 16);
>>> +
>>> + DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_4, data);
>>> +
>>> + data = (dsc->flatness_min_qp & 0x1f);
>>> + data |= (dsc->flatness_max_qp & 0x1f) << 5;
>>> +
>>> + det_thresh_flatness = drm_dsc_calculate_flatness_det_thresh(dsc);
>>> + data |= (det_thresh_flatness & 0xff) << 10;
>>> +
>>> + DPU_REG_WRITE(hw, offset + DSC_FLATNESS_QP, data);
>>> +
>>> + DPU_REG_WRITE(hw, offset + DSC_RC_MODEL_SIZE,
>>> + (dsc->rc_model_size) & 0xffff);
>>> +
>>> + data = dsc->rc_edge_factor & 0xf;
>>> + data |= (dsc->rc_quant_incr_limit0 & 0x1f) << 8;
>>> + data |= (dsc->rc_quant_incr_limit1 & 0x1f) << 13;
>>> + data |= (dsc->rc_tgt_offset_high & 0xf) << 20;
>>> + data |= (dsc->rc_tgt_offset_low & 0xf) << 24;
>>
>> FIELD_PREP should become our friend. As the DPU driver does not use
>> generated headers/helpers, we should probably enforce its usage all
>> over the code.
>
> Agree, since this patch series have almost complete the review process.
>
> can we start use FIELD_PREP at next?
Yes
--
With best wishes
Dmitry
More information about the dri-devel
mailing list