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
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
