On 2023-01-23 10:24:34, Kuogee Hsieh wrote:
> This patch add DSC block and sub block to support new DSC v1.2 hardware
> encoder. Also sc7280 DSC related hardware information are added to allow
> sc7280 DSC feature be enabled at sc7280 platform.
You're not adding support (that happened in previous patches), you're
only describing SC7280 DSC blocks. Drop the first sentence.
> Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 50 +++++++++++++++++++-------
> 1 file changed, 38 insertions(+), 12 deletions(-)
>
> 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 7deffc9f9..2c78a46 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0-only
> /* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
> - * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
> */
>
> #define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__
> @@ -476,6 +476,7 @@ static const struct dpu_caps sc7280_dpu_caps = {
> .has_idle_pc = true,
> .max_linewidth = 2400,
> .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
> + .max_dsc_width = 2560,
> };
>
> static const struct dpu_mdp_cfg msm8998_mdp[] = {
> @@ -1707,7 +1708,7 @@ static const struct dpu_pingpong_cfg sm8350_pp[] = {
> };
>
> static const struct dpu_pingpong_cfg sc7280_pp[] = {
> - PP_BLK("pingpong_0", PINGPONG_0, 0x59000, 0, sc7280_pp_sblk, -1, -1),
> + PP_BLK("pingpong_0", PINGPONG_0, 0x69000, 0, sc7280_pp_sblk, -1, -1),
This should go in a separate Fixes: patch.
> PP_BLK("pingpong_1", PINGPONG_1, 0x6a000, 0, sc7280_pp_sblk, -1, -1),
> PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, 0, sc7280_pp_sblk, -1, -1),
> PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1),
> @@ -1814,25 +1815,48 @@ static const struct dpu_merge_3d_cfg sm8550_merge_3d[] = {
> /*************************************************************
> * DSC sub blocks config
> *************************************************************/
> -#define DSC_BLK(_name, _id, _base, _features) \
> +#define DSC_BLK_HW_1_1(_name, _id, _base, _features) \
Not sure if HW_ is necessary here, DSC_BLK_1_1 seems cleaner.
> {\
> .name = _name, .id = _id, \
> .base = _base, .len = 0x140, \
> - .features = _features, \
> + .features = BIT(DPU_DSC_HW_REV_1_1) | _features, \
> + }
> +
> +#define DSC_BLK_HW_1_2(_name, _id, _base, _features, _sblk) \
> + {\
> + .name = _name, .id = _id, \
> + .base = _base, .len = 0x140, \
> + .features = BIT(DPU_DSC_HW_REV_1_2) | _features, \
> + .sblk = &_sblk, \
> }
>
> static struct dpu_dsc_cfg sdm845_dsc[] = {
> - DSC_BLK("dsc_0", DSC_0, 0x80000, 0),
> - DSC_BLK("dsc_1", DSC_1, 0x80400, 0),
> - DSC_BLK("dsc_2", DSC_2, 0x80800, 0),
> - DSC_BLK("dsc_3", DSC_3, 0x80c00, 0),
> + DSC_BLK_HW_1_1("dsc_0", DSC_0, 0x80000, 0),
> + DSC_BLK_HW_1_1("dsc_1", DSC_1, 0x80400, 0),
> + DSC_BLK_HW_1_1("dsc_2", DSC_2, 0x80800, 0),
> + DSC_BLK_HW_1_1("dsc_3", DSC_3, 0x80c00, 0),
> };
>
> static struct dpu_dsc_cfg sm8150_dsc[] = {
> - DSC_BLK("dsc_0", DSC_0, 0x80000, BIT(DPU_DSC_OUTPUT_CTRL)),
> - DSC_BLK("dsc_1", DSC_1, 0x80400, BIT(DPU_DSC_OUTPUT_CTRL)),
> - DSC_BLK("dsc_2", DSC_2, 0x80800, BIT(DPU_DSC_OUTPUT_CTRL)),
> - DSC_BLK("dsc_3", DSC_3, 0x80c00, BIT(DPU_DSC_OUTPUT_CTRL)),
> + DSC_BLK_HW_1_1("dsc_0", DSC_0, 0x80000, BIT(DPU_DSC_OUTPUT_CTRL)),
> + DSC_BLK_HW_1_1("dsc_1", DSC_1, 0x80400, BIT(DPU_DSC_OUTPUT_CTRL)),
> + DSC_BLK_HW_1_1("dsc_2", DSC_2, 0x80800, BIT(DPU_DSC_OUTPUT_CTRL)),
> + DSC_BLK_HW_1_1("dsc_3", DSC_3, 0x80c00, BIT(DPU_DSC_OUTPUT_CTRL)),
> +};
> +
> +static struct dpu_dsc_sub_blks sc7280_dsc_sblk_0 = {
> + .enc = {.base = 0x100, .len = 0x100},
> + .ctl = {.base = 0xF00, .len = 0x10},
> +};
> +
> +static struct dpu_dsc_sub_blks sc7280_dsc_sblk_1 = {
> + .enc = {.base = 0x200, .len = 0x100},
> + .ctl = {.base = 0xF80, .len = 0x10},
> +};
> +
> +static struct dpu_dsc_cfg sc7280_dsc[] = {
> + DSC_BLK_HW_1_2("dsc_0", DSC_0, 0x80000, BIT(DPU_DSC_NATIVE_422_EN), sc7280_dsc_sblk_0),
> + DSC_BLK_HW_1_2("dsc_1", DSC_1, 0x80000, BIT(DPU_DSC_NATIVE_422_EN), sc7280_dsc_sblk_1),
Bit scary that the blocks have the same address, because it is purely
driven by the sub-blocks with non-overlapping offsets/ranges. Should we
clarify that with a comment?
While at it, in v1.1 we use a hacky DSC_CTL() macro to bind a pingpong
block via a register in this magical "CTL" range at (0x1800 - 0x3FC * (m
- DSC_0)), can and/or should we represent that CTL sub-block explicitly
in the catalog for v1.1 hardware as well?
- Marijn
> };
>
> /*************************************************************
> @@ -2809,6 +2833,8 @@ static const struct dpu_mdss_cfg sc7280_dpu_cfg = {
> .pingpong_count = ARRAY_SIZE(sc7280_pp),
> .pingpong = sc7280_pp,
> .intf_count = ARRAY_SIZE(sc7280_intf),
> + .dsc_count = ARRAY_SIZE(sc7280_dsc),
> + .dsc = sc7280_dsc,
> .intf = sc7280_intf,
> .vbif_count = ARRAY_SIZE(sdm845_vbif),
> .vbif = sdm845_vbif,
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
