Messages in this thread | | | Date | Sun, 26 Feb 2023 15:13:39 +0200 | Subject | Re: [RFC PATCH 1/2] drm/msm/dpu: add dsc helper functions | From | Dmitry Baryshkov <> |
| |
On 26/02/2023 02:16, Abhinav Kumar wrote: > Hi Dmitry > > On 2/24/2023 3:57 PM, Dmitry Baryshkov wrote: >> On Sat, 25 Feb 2023 at 01:51, Kuogee Hsieh <quic_khsieh@quicinc.com> >> wrote: >>> >>> >>> On 2/24/2023 1:13 PM, Dmitry Baryshkov wrote: >>>> On 24/02/2023 21:40, Kuogee Hsieh wrote: >>>>> Add DSC helper functions based on DSC configuration profiles to >>>>> produce >>>>> DSC related runtime parameters through both table look up and runtime >>>>> calculation to support DSC on DPU. >>>>> >>>>> There are 6 different DSC configuration profiles are supported >>>>> currently. >>>>> DSC configuration profiles are differiented by 5 keys, DSC version >>>>> (V1.1), >>>>> chroma (444/422/420), colorspace (RGB/YUV), bpc(8/10), >>>>> bpp (6/7/7.5/8/9/10/12/15) and SCR (0/1). >>>>> >>>>> Only DSC version V1.1 added and V1.2 will be added later. >>>> >>>> These helpers should go to drivers/gpu/drm/display/drm_dsc_helper.c >>>> Also please check that they can be used for i915 or for amdgpu >>>> (ideally for both of them). >>>> >>>> I didn't check the tables against the standard (or against the current >>>> source code), will do that later. >>>> >>>>> >>>>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> >>>>> --- >>>>> drivers/gpu/drm/msm/Makefile | 1 + >>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.c | 209 >>>>> +++++++++++++++++++++++++ >>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.h | 34 ++++ >>>>> 3 files changed, 244 insertions(+) >>>>> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.c >>>>> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.h >>>>> >>>>> diff --git a/drivers/gpu/drm/msm/Makefile >>>>> b/drivers/gpu/drm/msm/Makefile >>>>> index 7274c412..28cf52b 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_dsc_helper.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_dsc_helper.c >>>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.c >>>>> new file mode 100644 >>>>> index 00000000..88207e9 >>>>> --- /dev/null >>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_dsc_helper.c >>>>> @@ -0,0 +1,209 @@ >>>>> +// SPDX-License-Identifier: GPL-2.0-only >>>>> +/* >>>>> + * Copyright (c) 2023. Qualcomm Innovation Center, Inc. All rights >>>>> reserved >>>>> + */ >>>>> + >>>>> +#include <drm/display/drm_dsc_helper.h> >>>>> +#include "msm_drv.h" >>>>> +#include "dpu_kms.h" >>>>> +#include "dpu_hw_dsc.h" >>>>> +#include "dpu_dsc_helper.h" >>>>> + >>>>> + >>>> >>>> Extra empty line >>>> >>>>> +#define DPU_DSC_PPS_SIZE 128 >>>>> + >>>>> +enum dpu_dsc_ratio_type { >>>>> + DSC_V11_8BPC_8BPP, >>>>> + DSC_V11_10BPC_8BPP, >>>>> + DSC_V11_10BPC_10BPP, >>>>> + DSC_V11_SCR1_8BPC_8BPP, >>>>> + DSC_V11_SCR1_10BPC_8BPP, >>>>> + DSC_V11_SCR1_10BPC_10BPP, >>>>> + DSC_RATIO_TYPE_MAX >>>>> +}; >>>>> + >>>>> + >>>>> +static u16 dpu_dsc_rc_buf_thresh[DSC_NUM_BUF_RANGES - 1] = { >>>>> + 0x0e, 0x1c, 0x2a, 0x38, 0x46, 0x54, >>>>> + 0x62, 0x69, 0x70, 0x77, 0x79, 0x7b, 0x7d, 0x7e >>>> >>>> Weird indentation >>>> >>>>> +}; >>>>> + >>>>> +/* >>>>> + * Rate control - Min QP values for each ratio type in >>>>> dpu_dsc_ratio_type >>>>> + */ >>>>> +static char >>>>> dpu_dsc_rc_range_min_qp[DSC_RATIO_TYPE_MAX][DSC_NUM_BUF_RANGES] = { >>>>> + /* DSC v1.1 */ >>>>> + {0, 0, 1, 1, 3, 3, 3, 3, 3, 3, 5, 5, 5, 7, 13}, >>>>> + {0, 4, 5, 5, 7, 7, 7, 7, 7, 7, 9, 9, 9, 11, 17}, >>>>> + {0, 4, 5, 6, 7, 7, 7, 7, 7, 7, 9, 9, 9, 11, 15}, >>>>> + /* DSC v1.1 SCR and DSC v1.2 RGB 444 */ >>>> >>>> What is SCR? Is there any reason to use older min/max Qp params >>>> instead of always using the ones from the VESA-DSC-1.1 standard? >>> >>> Standards change request, some vendors may use scr to work with their >>> panel. >>> >>> These table value are provided by system team. >> >> So, what will happen if we use values from 1.2 standard (aka 1.1 SCR >> 1) with the older panel? >> > > Standards change request means fixing errors/errata for the given > standard. Those are typically released as a different spec. > > So I referred the DSC 1.1 SCR spec, and it does have a few differences > in the table compared to DSC 1.1 which will get into DSC 1.2. > > Hence the table entries are same between DSC 1.1 SCR and DSC 1.2 > > You are right, ideally DSC 1.2 should be backwards compatible with DSC > 1.1 in terms of the values (thats what the spec says too) but I am not > sure if we can expect every panel/DP monitor to be forward compatible > without any SW change because it might need some firmware update (for > the panel) or SW update to support that especially during transitions of > the spec revisions (SCR to be precise). > > Typically we do below for DP monitors exactly for the same reason: > > DSC_ver_to_use = min(what_we_support, what_DP_monitor_supports) and use > that table. > > For DSI panels, typically in the panel spec it should say whether the > SCR version needs to be used because we have seen that for some panels ( > I dont remember exactly which one ) based on which panel and which > revision of the panel, it might not.
So, what happens if we use DSC 1.1 SCR (= DSC 1.2) values with older panel? Does it result in the broken image?
I'm asking here, because I think that these parameters tune the _encoder_. The decoder should be able to handle different compressed streams as long as values fit into the required 'profile'.
> > Thats why downstream started adding qcom,mdss-dsc-scr-version to the > devicetree. > >>>>> + {0, 0, 1, 1, 3, 3, 3, 3, 3, 3, 5, 5, 5, 9, 12}, >>>>> + {0, 4, 5, 5, 7, 7, 7, 7, 7, 7, 9, 9, 9, 13, 16}, >>>>> + {0, 4, 5, 6, 7, 7, 7, 7, 7, 7, 9, 9, 9, 11, 15}, >> >>
-- With best wishes Dmitry
| |