On Friday, March 20th, 2026 at 9:36 AM, Pengyu Luo <[email protected]> wrote:
> On Fri, Mar 20, 2026 at 9:18 PM Alexander Koskovich <[email protected]> wrote: > > > > On Friday, March 20th, 2026 at 8:20 AM, Pengyu Luo <[email protected]> > > wrote: > > > > > On Fri, Mar 20, 2026 at 8:14 PM Alexander Koskovich <[email protected]> > > > wrote: > > > > > > > > On Friday, March 20th, 2026 at 8:08 AM, Pengyu Luo > > > > <[email protected]> wrote: > > > > > > > > > On Fri, Mar 20, 2026 at 4:17 PM Alexander Koskovich > > > > > <[email protected]> wrote: > > > > > > > > > > > > > > On Friday, March 20th, 2026 at 3:36 AM, Dmitry Baryshkov > > > > > > > <[email protected]> wrote: > > > > > > > > > > > > > > > On Fri, Mar 20, 2026 at 07:02:54AM +0000, Alexander Koskovich > > > > > > > > wrote: > > > > > > > > > On Friday, March 20th, 2026 at 2:50 AM, Alexander Koskovich > > > > > > > > > <[email protected]> wrote: > > > > > > > > > > > > > > > > > > > On Friday, March 20th, 2026 at 2:38 AM, Dmitry Baryshkov > > > > > > > > > > <[email protected]> wrote: > > > > > > > > > > > > > > > > > > > > > On Fri, Mar 20, 2026 at 04:46:02AM +0000, Alexander > > > > > > > > > > > Koskovich wrote: > > > > > > > > > > > > On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek > > > > > > > > > > > > <[email protected]> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote: > > > > > > > > > > > > > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan > > > > > > > > > > > > > > Marek wrote: > > > > > > > > > > > > > ... > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> That's not how it works. INTF (which feeds DSI) is > > > > > > > > > > > > > >> after DSC compression. > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> INTF timings are always in RGB888 (24-bit) units. > > > > > > > > > > > > > >> Ignoring widebus details, > > > > > > > > > > > > > >> the INTF timing should match what is programmed on > > > > > > > > > > > > > >> the DSI side (hdisplay, > > > > > > > > > > > > > >> which is calculated as bytes per line / 3). > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> (fwiw, the current "timing->width = ..." > > > > > > > > > > > > > >> calculation here blames to me, but > > > > > > > > > > > > > >> what I wrote originally was just "timing->width = > > > > > > > > > > > > > >> timing->width / 3" with a > > > > > > > > > > > > > >> comment about being incomplete.) > > > > > > > > > > > > > >> > > > > > > > > > > > > > > Okay. After reading the docs (sorry, it took a > > > > > > > > > > > > > > while). > > > > > > > > > > > > > > > > > > > > > > > > > > > > - When widebus is not enabled, the transfer is > > > > > > > > > > > > > > always 24 bit of > > > > > > > > > > > > > > compressed data. Thus if it is not in play, pclk > > > > > > > > > > > > > > and timing->width > > > > > > > > > > > > > > should be scaled by source_pixel_depth / > > > > > > > > > > > > > > compression_ratio / 24. In > > > > > > > > > > > > > > case of the code it is 'drm_dsc_get_bpp_int(dsc) > > > > > > > > > > > > > > / 24'. > > > > > > > > > > > > > > > > > > > > > > > > > > > > For RGB101010 / 8bpp DSC this should result in > > > > > > > > > > > > > > the PCLK being lowered > > > > > > > > > > > > > > by the factor of 3 (= 24 / (30 / 3.75)) > > > > > > > > > > > > > > > > > > > > > > > > > > > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), > > > > > > > > > > > > > > the transfer takes > > > > > > > > > > > > > > more than 24 bits. In this case the PCLK and > > > > > > > > > > > > > > timing->width should be > > > > > > > > > > > > > > scaled exactly by the DSC compression ratio, > > > > > > > > > > > > > > which is > > > > > > > > > > > > > > 'drm_dsc_get_bpp_int(dsc) / (3 * > > > > > > > > > > > > > > dsc->bits_per_component). > > > > > > > > > > > > > > > > > > > > > > > > > > > > So, this piece of code needs to be adjusted to > > > > > > > > > > > > > > check for the widebus > > > > > > > > > > > > > > being enabled or not. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The widebus adjustment on the MDP/INTF side is > > > > > > > > > > > > > already in > > > > > > > > > > > > > dpu_hw_intf_setup_timing_engine: the "data width" is > > > > > > > > > > > > > divided by 2 for > > > > > > > > > > > > > 48-bit widebus instead of 24-bit. there shouldn't be > > > > > > > > > > > > > any other > > > > > > > > > > > > > adjustment (downstream doesn't have any other > > > > > > > > > > > > > adjustment). > > > > > > > > > > > > > > > > > > > > > > > > > > a relevant downstream comment: "In DATABUS-WIDEN > > > > > > > > > > > > > mode, MDP always sends > > > > > > > > > > > > > out 48-bit compressed data per pclk and on average, > > > > > > > > > > > > > DSI consumes an > > > > > > > > > > > > > amount of compressed data equivalent to the > > > > > > > > > > > > > uncompressed pixel depth per > > > > > > > > > > > > > pclk." > > > > > > > > > > > > > > > > > > > > > > > > > > Based on that comment, this patch is correct, and the > > > > > > > > > > > > > ''drm_dsc_get_bpp_int(dsc) / (3 * > > > > > > > > > > > > > dsc->bits_per_component)' adjustment > > > > > > > > > > > > > only applies to DSI. > > > > > > > > > > > > > > > > > > > > > > > > If I keep the INTF side at /24 and change the DSI side > > > > > > > > > > > > to: > > > > > > > > > > > > > > > > > > > > > > > > if (wide_bus) > > > > > > > > > > > > new_hdisplay = DIV_ROUND_UP(mode->hdisplay * > > > > > > > > > > > > drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3); > > > > > > > > > > > > else > > > > > > > > > > > > new_hdisplay = DIV_ROUND_UP(mode->hdisplay * > > > > > > > > > > > > drm_dsc_get_bpp_int(dsc), 24); > > > > > > > > > > > > > > > > > > > > > > Please check the actual fps (I'm usually using a > > > > > > > > > > > vblank-synced GL, e.g. > > > > > > > > > > > kmscube). > > > > > > > > > > > > > > > > > > > > > > At least this matches my expectations. > > > > > > > > > > > > > > > > > > > > Hmmm with kmscube I am getting 120FPS with 24 and 100FPS > > > > > > > > > > with 30. So I guess that's a no go > > > > > > > > > > > > > > > > > > Although it was using dsc->bits_per_component * 3 regardless > > > > > > > > > before for > > > > > > > > > dsi_adjust_pclk_for_compression so I guess that's what > > > > > > > > > Jonathan was > > > > > > > > > referring to earlier... > > > > > > > > > > > > > > > > Do you have any of the patches by Marijn or Pengyu? > > > > > > > > > > > > > > > > - > > > > > > > > https://lore.kernel.org/linux-arm-msm/20260311-dsi-dsc-regresses-again-v1-1-6a422141e...@somainline.org/ > > > > > > > > > > > > > > > > - > > > > > > > > https://lore.kernel.org/linux-arm-msm/[email protected]/ > > > > > > > > > > > > > > Nope, I can test with them though if you'd like > > > > > > > > > > > > Tested the following 3 patches on top of this series: > > > > > > drm/msm/dsi: fix hdisplay calculation when programming dsi registers > > > > > > drm/msm/dsi: fix bits_per_pclk > > > > > > drm/msm/dsi: fix hdisplay calculation for CMD mode panel > > > > > > > > > > > > Getting constant FIFO errors with them applied: > > > > > > [ 64.851635] dsi_err_worker: status=4 > > > > > > [ 64.851692] dsi_err_worker: status=4 > > > > > > [ 64.851730] dsi_err_worker: status=4 > > > > > > [ 64.851766] dsi_err_worker: status=4 > > > > > > [ 64.851802] dsi_err_worker: status=4 > > > > > > [ 64.851837] dsi_err_worker: status=4 > > > > > > [ 64.851903] dsi_err_worker: status=4 > > > > > > [ 64.851940] dsi_err_worker: status=4 > > > > > > [ 64.851976] dsi_err_worker: status=4 > > > > > > [ 64.852011] dsi_err_worker: status=4 > > > > > > > > > > Personally, I use > > > > > timing->width = DIV_ROUND_UP(timing->width * drm_dsc_get_bpp_int(dsc), > > > > > dsc->bits_per_component * 3); > > > > > > > > > > DIV_ROUND_UP is magic for me. If not, I got status=4 too. > > > > > > > > > > This works for 8-bit dst bpc with 10-bit src bpc. > > > > > > > > Did a quick test with that (plus the 3 changes listed above), still > > > > getting the > > > > FIFO errors and no display. > > > > > > > > Are you using RGB101010? > > > > > > > > > > RGB101010(dst bpp) downstream, my panel supports RGB101010, and I can > > > use RGB888 for it too. > > > I am testing rgb101010. > > > BTW, have you dumped registers? compared with downstream. > > > > > > I hardcode > > > display_data_hctl = (0xbe << 16) | hsync_data_start_x; > > > in dpu_hw_intf.c > > > downstream uses 0xbe for data_width > > > > Can you try this series with ac47870fd ("drm/msm/dsi: fix hdisplay > > calculation when programming dsi registers") reverted and with the > > following 2 changes? > > > > https://github.com/AKoskovich/linux/commit/af24b0e2ee212153953dfee040da5cc077567363 > > https://github.com/AKoskovich/linux/commit/e334675c0caf47956a838e2eafda22fb689a967d > > > > I don't mind testing it, but some parameters are obviously wrong for me. Did some additional testing, reverted the change in dpu_encoder_phys_vid.c and picked only "drm/msm/dsi: fix hdisplay calculation when programming dsi registers". This results in the FIFO errors. But then when I mirror data_width calculation I see downstream, display then works. Can you try with just this (ignore other change I suggested): https://github.com/AKoskovich/linux/commit/af24b0e2ee212153953dfee040da5cc077567363 That should make the display_data_hctl calculation correct so you don't need to hardcode (I think) > > int new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), > dsc->bits_per_component * 3); > > This gives the correct clkrate with fixes([1]). > > fixed ac47870fd([2]) will give me the right hdisplay register value > @0xae94024 (I compared with downstream) > > [1]: > https://gitlab.freedesktop.org/drm/msm/-/commit/e4eb11b34d6c84f398d8f08d7cb4d6c38e739dd2 > [2]: > https://lore.kernel.org/linux-arm-msm/[email protected]/ > > BTW, I can only do 8-bit dst and 10-bit src test, 10-bit dst and > 10-bit src gave me > > Mar 20 20:46:00 qcom kernel: dsi_mgr_bridge_mode_set: set mode: > "1904x3040": 120 915552 1904 2180 2212 2244 3040 3066 3070 3400 0x48 > 0x0 > Mar 20 20:46:00 qcom kernel: dsi_mgr_bridge_pre_enable: id=0 > Mar 20 20:46:00 qcom kernel: dsi_mgr_bridge_power_on: id=0 > Mar 20 20:46:00 qcom kernel: msm_dsi_host_reset_phy: > Mar 20 20:46:00 qcom kernel: msm_dsi_host_reset_phy: > Mar 20 20:46:00 qcom kernel: dsi_calc_pclk: pclk=172992000, bclk=108120000 > Mar 20 20:46:00 qcom kernel: dsi_calc_pclk: pclk=172992000, bclk=108120000 > Mar 20 20:46:00 qcom kernel: dsi_link_clk_set_rate_6g: Set clk rates: > pclk=172992000, byteclk=108120000 > Mar 20 20:46:00 qcom kernel: dsi_link_clk_set_rate_6g: Failed to set > rate pixel clk, -22 > Mar 20 20:46:00 qcom kernel: msm_dsi_host_power_on: failed to enable > link clocks. ret=-22 > Mar 20 20:46:00 qcom kernel: dsi_mgr_bridge_power_on: power on host 0 > failed, -22 > Mar 20 20:46:00 qcom kernel: msm_dsi ae94000.dsi: Power on failed: -22 > Mar 20 20:46:00 qcom kernel: bias enabled > Mar 20 20:46:00 qcom kernel: panel-nt36536 ae94000.dsi.0: sending dcs > data ff 20 failed: -22 > Mar 20 20:46:00 qcom kernel: panel-nt36536 ae94000.dsi.0: Failed to > initialize panel: -22 > Mar 20 20:46:00 qcom kernel: [drm:dpu_encoder_phys_vid_enable:471] enc33 intf2 > Mar 20 20:46:00 qcom kernel: > [drm:dpu_encoder_phys_vid_setup_timing_engine:292] enc33 intf2 > enabling mode: > Mar 20 20:46:00 qcom kernel: > [drm:dpu_encoder_phys_vid_setup_timing_engine:304] enc33 intf2 > split_role 2, halve horizontal 952 1122 > 1090 1106 0 > Mar 20 20:46:00 qcom kernel: > [drm:dpu_encoder_phys_vid_setup_timing_engine:315] enc33 intf2 > fmt_fourcc 0x34324752 > Mar 20 20:46:00 qcom kernel: > [drm:programmable_fetch_get_num_lines:202] enc33 intf2 prog fetch is > not needed, large vbp+vsw > Mar 20 20:46:00 qcom kernel: > [drm:programmable_fetch_get_num_lines:217] enc33 intf2 v_front_porch > 26 v_back_porch 330 vsync_pulse_wi > dth 4 > Mar 20 20:46:00 qcom kernel: > [drm:programmable_fetch_get_num_lines:221] enc33 intf2 wc_lines 24 > needed_vfp_lines 4294966986 actual_v > fp_lines 0 > Mar 20 20:46:00 qcom kernel: [drm:programmable_fetch_config:261] enc33 > intf2 vfp_fetch_lines 0 vfp_fetch_start_vsync_counter 0 > Mar 20 20:46:00 qcom kernel: [drm:dpu_encoder_phys_vid_enable:507] > enc33 intf2 update pending flush ctl 0 intf 3 > Mar 20 20:46:00 qcom kernel: [drm:dpu_encoder_phys_vid_enable:471] enc33 intf1 > Mar 20 20:46:00 qcom kernel: > [drm:dpu_encoder_phys_vid_setup_timing_engine:292] enc33 intf1 > enabling mode: > Mar 20 20:46:00 qcom kernel: > [drm:dpu_encoder_phys_vid_setup_timing_engine:304] enc33 intf1 > split_role 1, halve horizontal 952 1122 > 1090 1106 0 > Mar 20 20:46:00 qcom kernel: > [drm:dpu_encoder_phys_vid_setup_timing_engine:315] enc33 intf1 > fmt_fourcc 0x34324752 > > > Best wishes, > Pengyu > > > > > > > > > Best wishes, > > > Pengyu > > > > > > > > > > Best wishes, > > > > > Pengyu > > > > > > > > > > > > > Thanks, > > > > Alex > > > > > > > >
