On 3/16/2023 9:23 AM, Dmitry Baryshkov wrote:
On 16/03/2023 18:13, Abhinav Kumar wrote:
On 3/16/2023 9:03 AM, Dmitry Baryshkov wrote:
Hi,
[removed previous conversation]
Hi Dmitry and Abhinav,
Just wanted to follow up on this thread. I've gone over the
MSM-specific
DSC params for DP and DSI and have found a few shared calculations and
variables between both DSI and DP paths:
- (as mentioned earlier in the thread) almost all the calculations in
dpu_dsc_populate_dsc_config() match dsi_populate_dsc_params() [1]. The
only difference in the math I'm seeing is initial_scale_value.
The value in dsi code is valid for initial_offset = 6144. Please use
the formula from the standard (= sde_dsc_populate_dsc_config) and add
it to drm_dsc_helper.c
If I remember correctly the last remaining item in
dsi_populate_dsc_params() (except mentioned initial_offset) was
line_buf_depth, see [3]. I'm not sure about setting it to bpc+1.
According to the standard it should come from a DSC decoder spec,
which means it should be set by the DSI panel driver or via
drm_dp_dsc_sink_line_buf_depth() in the case of DP output.
- dsc_extra_pclk_cycle_cnt and dce_bytes_per_line, which were
introduced
in Kuogee's v1 DSC series [2], are used for DSI, DP, and the DPU timing
engine. dsc_extra_pclk_cycle_cnt is calculated based on pclk_per_line
(which is calculated differently between DP and DSI), but
dce_bytes_per_line is calculated the same way between DP and DSI.
To avoid having to duplicate math in 2 different places, I think it
would help to have these calculations in some msm_dsc_helper.c file.
Any
thoughts on this?
dsc_extra_pclk_cycle_cnt and dce_bytes_per_line are used only in DPU
code, so they can stay in DPU driver.
They can stay in the dpu driver is fine but where?
Like Jessica wrote, this is computed and used in 3 places today :
1) DSI video engine computation
2) DP controller computation
3) timing engine programming
Please excuse me if I'm wrong. I checked both vendor techpack and the
Kuogee's patches. I see them being used only in the SDE / DPU driver
code. Could you please point me to the code path that we are discussing?
DSI code :
https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/dsi/dsi_host.c#L868
DP code:
Refer to dp_panel_dsc_pclk_param_calc in
https://patchwork.freedesktop.org/patch/519837/?series=113240&rev=1
Timing engine:
refer to https://patchwork.freedesktop.org/patch/519838/?series=113240&rev=1
Probably confusion is due to the naming. bytes_per_line is nothing but
bytes_per_pkt * pkt_per_line but the concept is common for DP and DSI.
+ if (phys->comp_type == MSM_DISPLAY_COMPRESSION_DSC) {
+ phys->dsc_extra_pclk_cycle_cnt = dsc_info->pclk_per_line;
+ phys->dsc_extra_disp_width = dsc_info->extra_width;
+ phys->dce_bytes_per_line =
+ dsc_info->bytes_per_pkt * dsc_info->pkt_per_line;
So either we have a helper in a common location somewhere so that
these 3 modules can call that helper and use it OR each module
duplicates the computation code.
What should be the common location is the discussion here.
It cannot be dpu_encoder.c as the DSI/DP dont call into the encoder
methods.
Thanks,
Jessica Zhang
[1]
https://elixir.bootlin.com/linux/v6.3-rc2/source/drivers/gpu/drm/msm/dsi/dsi_host.c#L1756
[2] https://patchwork.freedesktop.org/patch/519845/?series=113240&rev=1
[3] https://patchwork.freedesktop.org/patch/525441/?series=114472&rev=2
