Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 16 Mar 2023 18:23:01 +0200 | Subject | Re: [Freedreno] [RFC PATCH 1/2] drm/msm/dpu: add dsc helper functions | From | Dmitry Baryshkov <> |
| |
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?
> 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 >> >> >>
-- With best wishes Dmitry
| ![\](/images/icornerr.gif) |