[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <38d6de06-77c0-5c87-86fb-010954e06a63@quicinc.com>
Date: Mon, 1 May 2023 16:12:40 -0700
From: Kuogee Hsieh <quic_khsieh@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
<dri-devel@...ts.freedesktop.org>, <robdclark@...il.com>,
<sean@...rly.run>, <swboyd@...omium.org>, <dianders@...omium.org>,
<vkoul@...nel.org>, <daniel@...ll.ch>, <airlied@...il.com>,
<agross@...nel.org>, <andersson@...nel.org>
CC: <quic_abhinavk@...cinc.com>, <quic_sbillaka@...cinc.com>,
<marijn.suijten@...ainline.org>, <freedreno@...ts.freedesktop.org>,
<linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/7] drm/msm/dpu: add support for DSC encoder v1.2
engine
On 5/1/2023 1:50 PM, Dmitry Baryshkov wrote:
> On 01/05/2023 23:40, Kuogee Hsieh wrote:
>>
>> On 4/28/2023 5:30 PM, Dmitry Baryshkov wrote:
>>> On 29/04/2023 02:45, Kuogee Hsieh wrote:
>>>> Add support for DSC 1.2 by providing the necessary hooks to program
>>>> the DPU DSC 1.2 encoder.
>>>>
>>>> Reported-by: kernel test robot <lkp@...el.com>
>>>
>>> What exactly was reported?
>>>
>>>> Signed-off-by: Kuogee Hsieh <quic_khsieh@...cinc.com>
>>>> ---
>>>> drivers/gpu/drm/msm/Makefile | 1 +
>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 34 ++-
>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 14 +-
>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 335
>>>> +++++++++++++++++++++++++
>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +-
>>>> 5 files changed, 387 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 71584cd..fc87db1 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.
>>>> */
>>>> @@ -241,12 +241,20 @@ 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_1 DSC block supports dsc 1.1 only
>>>> + * @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_1,
>>>> + DPU_DSC_HW_REV_1_2,
>>>> + DPU_DSC_NATIVE_422_EN,
>>>> + DPU_DSC_MAX
>>>> };
>>>> /**
>>>> @@ -311,6 +319,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 {
>>>> @@ -459,6 +475,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 {
>>>> @@ -612,10 +639,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 287ec5f..e11240a 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
>>>> @@ -70,6 +73,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..a777c7b
>>>> --- /dev/null
>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
>>>> @@ -0,0 +1,335 @@
>>>> +// 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"
>>>> +
>>>> +
>>>
>>> Unused empty line
>>>
>>>> +#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_MIN_QP_0 0x78
>>>> +#define DSC_RC_MAX_QP_0 0x84
>>>> +#define DSC_RC_RANGE_BPG_OFFSETS_0 0x90
>>>> +
>>>> +/* 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, bool native_422)
>>>> +{
>>>> + int max_addr = 2400 / num_ss;
>>>> +
>>>> + if ((hw_dsc->caps->features & BIT(DPU_DSC_NATIVE_422_EN)) &&
>>>> native_422)
>>>> + max_addr /= 2;
>>>> +
>>>> + return max_addr - 1;
>>>> +};
>>>> +
>>>> +static 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);
>>>> +}
>>>> +
>>>> +static void dpu_hw_dsc_config_1_2(struct dpu_hw_dsc *hw_dsc,
>>>> + struct drm_dsc_config *dsc,
>>>> + u32 mode,
>>>> + u32 initial_lines)
>>>> +{
>>>> + struct dpu_hw_blk_reg_map *hw;
>>>> + u32 offset;
>>>> + u32 data = 0;
>>>> + u32 det_thresh_flatness;
>>>> + u32 num_active_ss_per_enc;
>>>> + 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,
>>>> dsc->native_422) << 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;
>>>> +
>>>> + DPU_REG_WRITE(hw, offset + DSC_RC_CONFIG, data);
>>>> +
>>>> + /* program the dsc wrapper */
>>>> + offset = hw_dsc->caps->sblk->ctl.base;
>>>> +
>>>> + data = BIT(0); /* encoder enable */
>>>> + if (dsc->native_422)
>>>> + data |= BIT(8);
>>>> + else if (dsc->native_420)
>>>> + data |= BIT(9);
>>>> + if (!dsc->convert_rgb)
>>>> + data |= BIT(10);
>>>> + if (dsc->bits_per_component == 8)
>>>> + data |= BIT(11);
>>>> + if (mode & DSC_MODE_SPLIT_PANEL)
>>>> + data |= BIT(12);
>>>> + if (mode & DSC_MODE_MULTIPLEX)
>>>> + data |= BIT(13);
>>>> + if (!(mode & DSC_MODE_VIDEO))
>>>> + data |= BIT(17);
>>>> +
>>>> + DPU_REG_WRITE(hw, offset + DSC_CFG, data);
>>>> +}
>>>> +
>>>> +static void dpu_hw_dsc_config_thresh_1_2(struct dpu_hw_dsc *hw_dsc,
>>>> + struct drm_dsc_config *dsc)
>>>> +{
>>>> + struct dpu_hw_blk_reg_map *hw;
>>>> + u32 offset, off;
>>>> + int i, j = 0;
>>>> + struct drm_dsc_rc_range_parameters *rc;
>>>> + u32 data = 0, min_qp = 0, max_qp = 0, bpg_off = 0;
>>>> +
>>>> + if (!hw_dsc || !dsc)
>>>> + return;
>>>> +
>>>> + offset = hw_dsc->caps->sblk->enc.base;
>>>> +
>>>> + hw = &hw_dsc->hw;
>>>> +
>>>> + rc = dsc->rc_range_params;
>>>> +
>>>> + /*
>>>> + * With BUF_THRESH -- 14 in total
>>>> + * each register contains 4 thresh values with the last register
>>>> + * containing only 2 thresh values
>>>> + */
>>>> + off = 0;
>>>> + for (i = 0; i < DSC_NUM_BUF_RANGES - 1; i++) {
>>>> + data |= dsc->rc_buf_thresh[i] << (8 * j);
>>>> + j++;
>>>> + if ((j == 4) || (i == DSC_NUM_BUF_RANGES - 2)) {
>>>> + DPU_REG_WRITE(hw, offset + DSC_RC_BUF_THRESH_0 + off,
>>>> data);
>>>> + off += 4;
>>>> + j = 0;
>>>> + data = 0;
>>>> + }
>>>> + }
>>>
>>> This is barely understandable code. The following line is much better:
>>>
>>> DPU_REG_WRITE(hw, offset + DSC_RC_BUF_THRESH_0,
>>> (dsc->rc_buf_thresh[0] << 0) |
>>> (dsc->rc_buf_thresh[1] << 8) |
>>> (dsc->rc_buf_thresh[2] << 16) |
>>> (dsc->rc_buf_thresh[3] << 24));
>>>
>>> etc.
>>>
>>> Please unroll both loops.
>>>
>>>> +
>>>> +
>>>> + /*
>>>> + * with min/max_QP -- 5 bits each
>>>> + * each register contains 5 min_qp or max_qp for total of 15
>>>> + *
>>>> + * With BPG_OFFSET -- 6 bits each
>>>> + * each register contains 5 BPG_offset for total of 15
>>>> + */
>>>> + off = 0;
>>>> + for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
>>>> + min_qp |= rc[i].range_min_qp << (5 * j);
>>>> + max_qp |= rc[i].range_max_qp << (5 * j);
>>>> + bpg_off |= rc[i].range_bpg_offset << (6 * j);
>>>> + j++;
>>>> + if (j == 5) {
>>>> + DPU_REG_WRITE(hw, offset + DSC_RC_MIN_QP_0 + off,
>>>> min_qp);
>>>> + DPU_REG_WRITE(hw, offset + DSC_RC_MAX_QP_0 + off,
>>>> max_qp);
>>>> + DPU_REG_WRITE(hw, offset + DSC_RC_RANGE_BPG_OFFSETS_0
>>>> + off, bpg_off);
>>>> + off += 4;
>>>> + j = 0;
>>>> + min_qp = 0;
>>>> + max_qp = 0;
>>>> + bpg_off = 0;
>>>> + }
>>>> + }
>>>> +}
>>>> +
>>>> +static void dpu_hw_dsc_bind_pingpong_blk_1_2(
>>>> + struct dpu_hw_dsc *hw_dsc,
>>>> + bool enable,
>>>> + const enum dpu_pingpong pp)
>>>> +{
>>>> + struct dpu_hw_blk_reg_map *hw;
>>>> + int offset;
>>>> + int mux_cfg = 0xf; /* Disabled */
>>>> +
>>>> + offset = hw_dsc->caps->sblk->ctl.base;
>>>> +
>>>> + hw = &hw_dsc->hw;
>>>> + if (enable)
>>>> + mux_cfg = (pp - PINGPONG_0) & 0x7;
>>>> +
>>>> + DPU_REG_WRITE(hw, offset + DSC_CTL, mux_cfg);
>>>> +}
>>>
>>> Please refactor the existing bind_pingpong_blk() semantics to accept
>>> either a valid PINGPONG_n, or PINGPONG_NONE to disable the binding.
>>
>> Could you please give me more details for this comment.
>>
>> both dsc_1_1 and dsc_1_2 are required to select and set mux bits of
>> mux_cfg to binding dsc and pingpong together.
>
> Yes. I was refererring to the function interface. You pass both `bool
> enable` and PP index. The index doesn't make sense for 'disable' case.
> So we can use a single argument of `enum dpu_pingpong` type. Pass
> PINGPONG_n for binding it. Pass PINGPONG_NONE for removing the binding.
ok, i will add an patch for this since it will touch dpu_encoder.c and
dpu_hw_dsc.c
>
>>
>
Powered by blists - more mailing lists