Corruption on dual-DSI dual-DSC CMD-mode panels
As requested by @abhinavk in #34 (comment 2087098), here's an issue to track the progress around supporting DSC on bonded DSI (2:2:2 toplogy).
This is on the Sony Xperia 1 III (sm8350-sony-xperia-sagami-pdx215.dts) with ana6707_amb650yl01 panel. It hosts 4 modes, 1644x3840 and 1096x2560 resolutions, both at 60 and 120Hz refresh rate. 1096x2560@60 is used in the dumps below.
For completeness the Xperia 1 IV (sm8450) has a mostly identical panel, but it has not been tested yet: https://github.com/sonyxperiadev/kernel-copyleft-dts/blob/64.1.A.0.xxx/display-devicetree/display/dsi-panel-ana6707_amb650yl01-uhd-pdx223.dtsi
Together with this devcoredump:
Most notably:
- Only the left encoder seems to emit data, the right side of the panel remains static and corrupted;
- Every 5th-or-so slice seems to be interpreted correctly by the panel and shows readable framebuffer consle.
For this to work many kernel changes are needed that I am working towards upstreaming, as long as we can figure out this last issue which is likely a misconfiguration of the DSC or DSI block(s). They are spread far and wide on a massive development branch, but according to @abhinav there was no need to spend time sorting these out and pointing to a clean branch containing them. Instead, if someone (from Qcom) can help me validate and hopefully pin-point a register issue in the provided devcoredump, I will work towards cleaning, publishing and upstreaming these patches. The current work-in-progress is available at https://github.com/somainline/linux/commits/marijn/panel-exclusives.
Non-exhaustive list of what has already been done to support DSC on dual-DSI:
- Better support for active-CTL (both architectural as well as register programming), which is a combination of my changes on top of those from @lumag;
- Replacement of hardcoded 2:2:1 topology codepaths to also support this 2:2:2 topology (and 1:1:1 for e.g. the FairPhone 5 that @lucaweiss is working on);
- Proper disablement of DSC multiplex mode (while retaining split-panel) when using bonded-DSI with multiple DSC encoders;
- Boilerplate to support passing only half the (DSC) width to both DSI encoders, so that their math for clocks and number of slices/pixels to send remains correct.
For example, over the width of this panel 1096 pixels are sent in two slices. However, because there are two encoders, both panels only see 512 pixels with a slice width of 1 (this has been confirmed with a downstream kernel).
Activity
mentioned in issue #34 (closed)
I've played with Xiaomi Mi Pad 6 's dual-dsi + dsc panel and got similar results lol
The only difference was that I was getting regularly correctly parsed slice data on both halfs of the display. Unfortunately I couldn't get any further either.Edit: This is a video panel BTW, not a command mode one
Edited by Molly Sophia-
0001-test.patch Here are the changes I've made. I don't have much knowledge about the DPU code :P
mentioned in issue #36
hey @MarijnS95 ,
Sorry for the lack of updates -- finally was able to set aside some time this week to take a look at the dumps.
I'm working on confirming the DSC register configuration with downstream. In the mean time, can you share a hex dump of the PPS being sent from your panel driver?
Thanks for your patience,
Jessica Zhang
hi @jesszhan, I have some information for the video-mode panel, maybe it will be useful for cmd-mode as well
pipa_dsc_research.txtI also tried to compare all DSC registers, they are the same as downstream
Hi @adomerle , awesome, thanks! I'll also take a look at your notes.
Hi @jesszhan, glad to hear that this is being picked up! If it wasn't for the somewhat unnecessary "bump" message above I wouldn't have seen this, as there was no notification for your first reply.
I had forgotten to mention that I've pushed all the commits needed to get this far, which I will send upstream once we've found the final issue and confirmed this is all working:
https://github.com/somainline/linux/commits/marijn/panel-exclusives
Downstream PPS looks as follows:
[ 2.011040] DSC:11 00 00 89 30 80 0a 00 04 48 00 20 02 24 02 24 [ 2.011121] DSC:02 00 02 13 00 20 03 1b 00 07 00 0c 03 19 03 1b [ 2.011166] DSC:18 00 10 e0 03 0c 20 00 06 0b 0b 33 0e 1c 2a 38 [ 2.011248] DSC:46 54 62 69 70 77 79 7b 7d 7e 01 02 01 00 09 40 [ 2.011292] DSC:09 be 19 fc 19 fa 19 f8 1a 38 1a 78 1a b6 2a f6 [ 2.011372] DSC:2b 34 2b 74 3b 74 6b f4 00 00 00 00 00 00 00 00 [ 2.011416] DSC:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 2.011496] DSC:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00As you can imagine, I made sure that upstream sends identical data:
[ 1.408086] DSC:11 00 00 89 30 80 0a 00 04 48 00 20 02 24 02 24 [ 1.408103] DSC:02 00 02 13 00 20 03 1b 00 07 00 0c 03 19 03 1b [ 1.408110] DSC:18 00 10 e0 03 0c 20 00 06 0b 0b 33 0e 1c 2a 38 [ 1.408118] DSC:46 54 62 69 70 77 79 7b 7d 7e 01 02 01 00 09 40 [ 1.408125] DSC:09 be 19 fc 19 fa 19 f8 1a 38 1a 78 1a b6 2a f6 [ 1.408131] DSC:2b 34 2b 74 3b 74 6b f4 00 00 00 00 00 00 00 00 [ 1.408139] DSC:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 1.408146] DSC:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00@MarijnS95 , thanks! I'm seeing in
538a11f ("dual-DSI dual-DSC panel "), you're hardcodinginitial_lines = 3. Just wondering, how did you get that value?-
Hi @jesszhan, this was a test that turned out not to make a difference.
The value used downstream (where this topology is fully working) is
3, and mainline (dpu_encoder_dsc_initial_line_calc()) was returning some other value (I don't remember what, maybe it was2). @MarijnS95 , got it. So far I'm not seeing any issues with the register dump, but it looks like you're hardcoding your pclk and byte clk rates based on the uncompressed clock rate from DT.
Can you try using the following compressed clock values and let me know if that fixes anything?
- pclk: 33826960
- byte clk: 25370220
-
@jesszhan with that hardcoding many clock values have been tried, including those calculated downstream and calculated manually. Clocks close to the values you suggest have also been tried, and even exactly the ones you suggest. Unfortunately they make no difference to the output.
-
Found the golden egg during Easter today: after long debugging sessions, this is all that was needed on top of the previous state to get dual-DSI dual-DSC working properly without corruption:
diff --git a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dts b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dts index 384e08cd41c9..01864fe4340a 100644 --- a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dts +++ b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dts @@ -75,8 +75,8 @@ &mdss_dsi0 { &mdss_dsi1 { /* DSI1 is slave, so use DSI0 clocks */ - assigned-clock-parents = <&mdss_dsi0_phy 0>, - <&mdss_dsi0_phy 1>; + // assigned-clock-parents = <&mdss_dsi0_phy 0>, + // <&mdss_dsi0_phy 1>; vdda-supply = <&pm8350_l6>; qcom,dual-dsi-mode; qcom,sync-dual-dsi; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index 83d923699540..211d59d396a9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -438,9 +438,6 @@ static void dpu_encoder_phys_cmd_enable_helper( _dpu_encoder_phys_cmd_pingpong_config(phys_enc); - if (!dpu_encoder_phys_cmd_is_master(phys_enc)) - return; - ctl = phys_enc->hw_ctl; ctl->ops.update_pending_flush_intf(ctl, phys_enc->hw_intf->idx); }
While on a random tour through DPU I found that strangely
INTF_3(the slave DSI interface on my configuration) is never flushed, though it definitely writes registers that should be applied. This fixed all the corruption and timeouts/errors indmesg, but as before only the left side of the panel was working. After a lot of random trial-and-error, leaving DSI1 PHY clocks to use the entire DSI1 PLL tree leading up all the way to DSI1 VCO, instead of parenting off of DSI0 clocks makes the right panel show output too.This is not a proper fix and likely amounts to a missing flag in dispcc, as having the whole DSI1 clock tree generating the same frequency (possibly at a different phase?) as DSI0 is unnecessarily wasting power.
I've already compared the clock trees downstream and upstream (before and after this change) and don't see anything out of the ordinary.
I'll work on pushing the patches for this to the aforementioned branch as soon as possible, as well as clean them up in some soon-ish™ time frame to be upstreamable.
-
Another weird thing that I have been unable to explain (why downstream doesn't suffer). For
1096x2560@60Hzthere is a corrupted line at the top of the screen, and a video recording shows clearly that it's a tear (above the line displays an earlier or later frame than below):This is the only mode where downstream sets
mipi_dsi_dcs_set_tear_scanline()to3840(0xf00):It's a strange number since this mode has nothing to do with the
3840mode. Calling this function with0for every mode immediately gets rid of this corruption -
After analyzing the changes further:
Flushing the slave intf
A downstream kernel clarifies this:
Since PPSPLIT support was stripped out of the upstream DPU driver when it was submitted, this should have become
if (false && !is_master) goto skip;, thus never skipping the flush. However, theifwas erroneously modified by removing thefalse &&bit (as shown in the diff above), thus always skipping the flush for the slave, which is wrong.I'll send a patch shortly to address this.
Reparenting DSI1 clocks to DSI0 tree breaks
Here's a
clk_summarydiff from havingassigned-clock-parents(the case that breaks with colorful corruption) to having a duplicated clocktree:diff --git a/pdx215-assigned-clocks b/pdx215-dsi1-clocks index ed9e0ff13b9c..736d1ceb7171 100644 --- a/pdx215-assigned-clocks +++ b/pdx215-dsi1-clocks @@ -1,11 +1,11 @@ enable prepare protect duty hardware connection clock count count count rate accuracy phase cycle enable consumer id --------------------------------------------------------------------------------------------------------------------------------------------- - qcom_cpufreq2 0 0 0 844800000 0 0 50000 Y cpu7 no_connection_id + qcom_cpufreq2 0 0 0 2764800000 0 0 50000 Y cpu7 no_connection_id deviceless no_connection_id - qcom_cpufreq1 0 0 0 710400000 0 0 50000 Y cpu4 no_connection_id + qcom_cpufreq1 0 0 0 1209600000 0 0 50000 Y cpu4 no_connection_id deviceless no_connection_id - qcom_cpufreq0 0 0 0 1209600000 0 0 50000 Y cpu0 no_connection_id + qcom_cpufreq0 0 0 0 1804800000 0 0 50000 Y cpu0 no_connection_id deviceless no_connection_id gpu_cc_sleep_clk 0 0 0 0 0 0 50000 N deviceless no_connection_id gpu_cc_mnd1x_1_gfx3d_clk 0 0 0 0 0 0 50000 N deviceless no_connection_id @@ -122,32 +122,39 @@ ln_bb_clk2_ao 0 0 0 19200000 0 0 50000 Y deviceless no_connection_id ln_bb_clk2 0 0 0 19200000 0 0 50000 Y deviceless no_connection_id bi_tcxo_ao 0 0 0 19200000 0 0 50000 Y deviceless no_connection_id - bi_tcxo 12 14 0 19200000 0 0 50000 Y 8804000.mmc xo + bi_tcxo 13 15 0 19200000 0 0 50000 Y 8804000.mmc xo 88e3000.phy ref 88e8000.phy ref deviceless no_connection_id - dsi1vco_clk 0 0 0 0 0 0 50000 Y deviceless no_connection_id - dsi1_pll_out_div_clk 0 0 0 0 0 0 50000 Y deviceless no_connection_id - dsi1_pll_post_out_div_clk 0 0 0 0 0 0 50000 Y deviceless no_connection_id - dsi1_pll_bit_clk 0 0 0 0 0 0 50000 Y deviceless no_connection_id - dsi1_pclk_mux 0 0 0 0 0 0 50000 Y deviceless no_connection_id - dsi1_phy_pll_out_dsiclk 0 0 0 0 0 0 50000 Y deviceless no_connection_id - dsi1_pll_by_2_bit_clk 0 0 0 0 0 0 50000 Y deviceless no_connection_id - dsi1_phy_pll_out_byteclk 0 0 0 0 0 0 50000 Y deviceless no_connection_id + dsi1vco_clk 1 1 0 1075507177 0 0 50000 Y deviceless no_connection_id + dsi1_pll_out_div_clk 1 1 0 1075507177 0 0 50000 Y deviceless no_connection_id + dsi1_pll_post_out_div_clk 0 0 0 268876794 0 0 50000 Y deviceless no_connection_id + dsi1_pll_bit_clk 2 2 0 1075507177 0 0 50000 Y deviceless no_connection_id + dsi1_pclk_mux 1 1 0 1075507177 0 0 50000 Y deviceless no_connection_id + dsi1_phy_pll_out_dsiclk 1 1 0 179251197 0 0 50000 Y deviceless no_connection_id + disp_cc_mdss_pclk1_clk_src 1 1 0 179251197 0 0 50000 Y deviceless no_connection_id + disp_cc_mdss_pclk1_clk 1 1 0 179251197 0 0 50000 Y ae96000.dsi pixel + deviceless no_connection_id + dsi1_pll_by_2_bit_clk 0 0 0 537753588 0 0 50000 Y deviceless no_connection_id + dsi1_phy_pll_out_byteclk 1 1 0 134438397 0 0 50000 Y deviceless no_connection_id + disp_cc_mdss_byte1_clk_src 2 2 0 134438397 0 0 50000 Y deviceless no_connection_id + disp_cc_mdss_byte1_div_clk_src 1 1 0 67219199 0 0 50000 Y deviceless no_connection_id + disp_cc_mdss_byte1_intf_clk 1 1 0 67219199 0 0 50000 Y ae96000.dsi byte_intf + deviceless no_connection_id + disp_cc_mdss_byte1_clk 1 1 0 134438397 0 0 50000 Y ae96000.dsi byte + ae96000.dsi byte + deviceless no_connection_id dsi0vco_clk 1 1 0 1075507177 0 0 50000 Y deviceless no_connection_id dsi0_pll_out_div_clk 1 1 0 1075507177 0 0 50000 Y deviceless no_connection_id dsi0_pll_post_out_div_clk 0 0 0 268876794 0 0 50000 Y deviceless no_connection_id dsi0_pll_bit_clk 2 2 0 1075507177 0 0 50000 Y deviceless no_connection_id dsi0_pclk_mux 1 1 0 1075507177 0 0 50000 Y deviceless no_connection_id - dsi0_phy_pll_out_dsiclk 2 2 0 179251197 0 0 50000 Y deviceless no_connection_id + dsi0_phy_pll_out_dsiclk 1 1 0 179251197 0 0 50000 Y deviceless no_connection_id disp_cc_mdss_pclk0_clk_src 1 1 0 179251197 0 0 50000 Y deviceless no_connection_id disp_cc_mdss_pclk0_clk 1 1 0 179251197 0 0 50000 Y ae94000.dsi pixel deviceless no_connection_id - disp_cc_mdss_pclk1_clk_src 1 1 0 179251197 0 0 50000 Y deviceless no_connection_id - disp_cc_mdss_pclk1_clk 1 1 0 179251197 0 0 50000 Y ae96000.dsi pixel - deviceless no_connection_id dsi0_pll_by_2_bit_clk 0 0 0 537753588 0 0 50000 Y deviceless no_connection_id - dsi0_phy_pll_out_byteclk 2 2 0 134438397 0 0 50000 Y deviceless no_connection_id + dsi0_phy_pll_out_byteclk 1 1 0 134438397 0 0 50000 Y deviceless no_connection_id disp_cc_mdss_byte0_clk_src 2 2 0 134438397 0 0 50000 Y deviceless no_connection_id disp_cc_mdss_byte0_div_clk_src 1 1 0 67219199 0 0 50000 Y deviceless no_connection_id disp_cc_mdss_byte0_intf_clk 1 1 0 67219199 0 0 50000 Y ae94000.dsi byte_intf @@ -155,13 +162,6 @@ disp_cc_mdss_byte0_clk 1 1 0 134438397 0 0 50000 Y ae94000.dsi byte ae94000.dsi byte deviceless no_connection_id - disp_cc_mdss_byte1_clk_src 2 2 0 134438397 0 0 50000 Y deviceless no_connection_id - disp_cc_mdss_byte1_div_clk_src 1 1 0 67219199 0 0 50000 Y deviceless no_connection_id - disp_cc_mdss_byte1_intf_clk 1 1 0 67219199 0 0 50000 Y ae96000.dsi byte_intf - deviceless no_connection_id - disp_cc_mdss_byte1_clk 1 1 0 134438397 0 0 50000 Y ae96000.dsi byte - ae96000.dsi byte - deviceless no_connection_id gpu_cc_pll1 0 0 0 499999804 0 0 50000 N deviceless no_connection_id gpu_cc_pll0 0 0 0 468000000 0 0 50000 N deviceless no_connection_id gpu_cc_hub_clk_src 0 3 0 19200000 0 0 50000 Y deviceless no_connection_idAt least all the clock trees are ticking at the right rate, as confirmed by measuring with
debugccfor thedispblock when the panel is broken:sony-pdx215-generic ~ $ sudo ./debugcc -p sm8350 -b disp -a disp_cc_mdss_ahb_clk: 19.199264MHz (19199264Hz) disp_cc_mdss_byte0_clk: 26.841240MHz (26841240Hz) disp_cc_mdss_byte0_intf_clk: 13.419888MHz (13419888Hz) disp_cc_mdss_byte1_clk: 26.841240MHz (26841240Hz) disp_cc_mdss_byte1_intf_clk: 13.419888MHz (13419888Hz) disp_cc_mdss_dp_aux1_clk: off disp_cc_mdss_dp_aux_clk: off disp_cc_mdss_dp_link1_clk: off disp_cc_mdss_dp_link1_intf_clk: off disp_cc_mdss_dp_link_clk: off disp_cc_mdss_dp_link_intf_clk: off disp_cc_mdss_dp_pixel1_clk: off disp_cc_mdss_dp_pixel2_clk: off disp_cc_mdss_dp_pixel_clk: off disp_cc_mdss_edp_aux_clk: off disp_cc_mdss_edp_link_clk: off disp_cc_mdss_edp_link_intf_clk: off disp_cc_mdss_edp_pixel_clk: off disp_cc_mdss_esc0_clk: 19.199264MHz (19199264Hz) disp_cc_mdss_esc1_clk: 19.199264MHz (19199264Hz) disp_cc_mdss_mdp_clk: 199.999376MHz (199999376Hz) disp_cc_mdss_mdp_lut_clk: 199.998784MHz (199998784Hz) disp_cc_mdss_non_gdsc_ahb_clk: 19.199264MHz (19199264Hz) disp_cc_mdss_pclk0_clk: 35.788616MHz (35788616Hz) disp_cc_mdss_pclk1_clk: 35.788024MHz (35788024Hz) disp_cc_mdss_rot_clk: 19.199264MHz (19199264Hz) disp_cc_mdss_rscc_ahb_clk: 19.199264MHz (19199264Hz) disp_cc_mdss_rscc_vsync_clk: 19.199264MHz (19199264Hz) disp_cc_mdss_vsync_clk: 19.199264MHz (19199264Hz) disp_cc_sleep_clk: 0.033096MHz (33096Hz)Maybe a mux is set up incorrectly later in the pipeline?
-
The problem is understood now, but a clean solution hasn't emerged yet. @lumag helpfully pointed me to
dsi_7nm_set_usecase()which after some debugging shows that it was setting theMASTERconfiguration on both PLLs, maybe causing the PLL/MUX to not source from the right parent (even though the dispcc clocks have their own muxes for this).It's not yet understood why
debugccmeasured a clock signal on all children from this PLL.This seems really terrible and I wish we could just use the clock API to tell us what our parent is, to configure this what is effectively a mux? Instead of having
dsi_managerpush about global state :(What seems to happen is that
msm_dsi_host_register()enables and configures the PLL viadsi_7nm_set_usecase(), before we push the global state onto it viamsm_dsi_phy_set_usecase(), so simply flipping that logic is enough:diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c index f7a9554c17d8..2beb742f85a3 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c @@ -81,6 +83,12 @@ static int dsi_mgr_setup_components(int id) msm_dsi_phy_set_usecase(msm_dsi->phy, MSM_DSI_PHY_STANDALONE); msm_dsi_host_set_phy_mode(msm_dsi->host, msm_dsi->phy); } else if (other_dsi) { + /* PLL0 is to drive both 2 DSI link clocks in bonded DSI mode. */ + msm_dsi_phy_set_usecase(clk_master_dsi->phy, + MSM_DSI_PHY_MASTER); + msm_dsi_phy_set_usecase(clk_slave_dsi->phy, + MSM_DSI_PHY_SLAVE); + struct msm_dsi *master_link_dsi = IS_MASTER_DSI_LINK(id) ? msm_dsi : other_dsi; struct msm_dsi *slave_link_dsi = IS_MASTER_DSI_LINK(id) ? @@ -100,10 +109,6 @@ static int dsi_mgr_setup_components(int id) return ret; /* PLL0 is to drive both 2 DSI link clocks in bonded DSI mode. */ - msm_dsi_phy_set_usecase(clk_master_dsi->phy, - MSM_DSI_PHY_MASTER); - msm_dsi_phy_set_usecase(clk_slave_dsi->phy, - MSM_DSI_PHY_SLAVE); msm_dsi_host_set_phy_mode(msm_dsi->host, msm_dsi->phy); msm_dsi_host_set_phy_mode(other_dsi->host, other_dsi->phy); }
-
The first set of patches for this series is now up at https://lore.kernel.org/lkml/20240417-drm-msm-initial-dualpipe-dsc-fixes-v1-0-78ae3ee9a697@somainline.org/. Only Active-CTL changes are still pending, which are the final piece of this puzzle.
Thanks @MarijnS95 for the updates. I will review your series.
Regarding #41 (comment 2375636), I actually checked this flow to make sure one was master and the other was slave. But I didnt follow why you had to move those two lines?
msm_dsi_host_register() is not enabling phy. msm_dsi_phy_enable() does that.
Even from the register dumps,
====================dsi0_phy================
0x0 : 00000024 00000000 00000000 00000000
0x10 : 00000061 00000030 00000000 00000000
reg 0x14 is 30 for DSI0, which means internal PLL
====================dsi1_phy================
0x0 : 00000024 00000000 00000000 00000000
0x10 : 000000f1 00000034 00000000 00000000
for dsi1_phy, reg 0x14 is 0x34 which is external PLL
So this matches the code
Edited by Abhinav Kumar-
Hi @abhinavk, thanks for the quick reply. Looking forward to the review, and hope you notice that the cover letter explains that I'm not sure what is up with the diff proposed above, as it doesn't seem like a very clean solution, and is mostly "RFC". You'll also notice that I point out that perhaps something is wrong with the panel driver and/or path around it.
Regarding #41 (comment 2375636)
Feel free to reply that directly under the right thread to keep conversations segregated :)
I actually checked this flow to make sure one was master and the other was slave. But I didnt follow why you had to move those two lines?
msm_dsi_host_register() is not enabling phy. msm_dsi_phy_enable() does that.
Even from the register dumps,
[..]
for dsi1_phy, reg 0x14 is 0x34 which is external PLL
So this matches the code
Thanks for doing the archaeology, I hadn't known that it was set correctly before. So perhaps, if I had found the missing slave intf flush issue sooner, this might not have been a problem yet, and is the result of more recent changes "somewhere"?
Debug logging pointed out to me that the flow was as follows:
dsi_mgr_setup_components()msm_dsi_host_register()-
dsi_7nm_phy_enable()->dsi_7nm_set_usecase(), with "internal" for both PLLs. -
msm_dsi_phy_set_usecase(), which is too late at this point
But what better way to show that with a backtrace (which I should have included in the patch/cover letter). Here you can clearly see a flow that goes from
msm_dsi_manager_register()all the way down through the panel driver attaching a DSI host, and ultimately resulting in a commit from the framebuffer and callingdsi_7nm_phy_enable(). Again, beforemsm_dsi_phy_set_usecase()was called:[ 2.299743] WARNING: CPU: 0 PID: 65 at drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c:612 dsi_7nm_phy_enable+0x39c/0x890 [ 2.299760] Modules linked in: [ 2.299770] CPU: 0 PID: 65 Comm: kworker/u32:1 Tainted: G U W 6.9.0-rc4-next-20240415-SoMainline-06297-g44b0c0a455c3-dirty #102 [ 2.299783] Hardware name: Sony Xperia 1 III (DT) [ 2.299792] Workqueue: events_unbound deferred_probe_work_func [ 2.299805] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 2.299815] pc : dsi_7nm_phy_enable+0x39c/0x890 [ 2.299825] lr : dsi_7nm_phy_enable+0x270/0x890 [ 2.299833] sp : ffff8000803a2ce0 [ 2.299840] x29: ffff8000803a2ce0 x28: ffffcea6d6da91b0 x27: ffff5f86c470eb00 [ 2.299856] x26: ffff5f86c470bc80 x25: ffff8000803a2e20 x24: ffff5f86c33536e0 [ 2.299872] x23: ffff5f86c3083180 x22: 0000000000000000 x21: ffff5f86c3353680 [ 2.299887] x20: ffff8000816dd400 x19: ffff8000816dd400 x18: ffff80008036d200 [ 2.299902] x17: 00000000000122ff x16: 000000000000007d x15: 000000000000270f [ 2.299916] x14: 0000000000000004 x13: 0000000000000001 x12: 0000000000012300 [ 2.299931] x11: 0000000000000002 x10: 0000000000000084 x9 : 000000000000005c [ 2.299945] x8 : ffffcea6d6da9000 x7 : 0000000000000000 x6 : 0000000000000088 [ 2.299960] x5 : 0000000000000000 x4 : ffffcea6d623ebf0 x3 : 0000000000000000 [ 2.299974] x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000004 [ 2.299989] Call trace: [ 2.299995] dsi_7nm_phy_enable+0x39c/0x890 [ 2.300005] msm_dsi_phy_enable+0x78/0x23c [ 2.300016] dsi_mgr_bridge_pre_enable+0x350/0x3b4 [ 2.300027] drm_atomic_bridge_call_pre_enable+0x70/0x8c [ 2.300038] drm_atomic_bridge_chain_pre_enable+0xc0/0x138 [ 2.300049] drm_atomic_helper_commit_modeset_enables+0x168/0x26c [ 2.300061] msm_atomic_commit_tail+0x1a4/0x510 [ 2.300072] commit_tail+0xa8/0x19c [ 2.300083] drm_atomic_helper_commit+0x188/0x1a0 [ 2.300094] drm_atomic_commit+0xb4/0xf0 [ 2.300104] drm_client_modeset_commit_atomic+0x1fc/0x268 [ 2.300115] drm_client_modeset_commit_locked+0x60/0x178 [ 2.300126] drm_client_modeset_commit+0x30/0x5c [ 2.300137] __drm_fb_helper_restore_fbdev_mode_unlocked+0xac/0xf0 [ 2.300149] drm_fb_helper_set_par+0x34/0x54 [ 2.300160] fbcon_init+0x218/0x500 [ 2.300171] visual_init+0xb8/0x10c [ 2.300181] do_bind_con_driver.isra.0+0x190/0x390 [ 2.300192] do_take_over_console+0x1a8/0x1fc [ 2.300203] do_fbcon_takeover+0x74/0xf4 [ 2.300213] do_fb_registered+0x138/0x150 [ 2.300224] fbcon_fb_registered+0x44/0x108 [ 2.300235] register_framebuffer+0x144/0x224 [ 2.300248] __drm_fb_helper_initial_config_and_unlock+0x34c/0x4b4 [ 2.300261] drm_fb_helper_initial_config+0x4c/0x60 [ 2.300273] msm_fbdev_client_hotplug+0x8c/0xe0 [ 2.300285] drm_client_register+0x5c/0xa4 [ 2.300295] msm_fbdev_setup+0xd8/0x158 [ 2.300306] msm_drm_bind+0x230/0x3d4 [ 2.300315] try_to_bring_up_aggregate_device+0x21c/0x2e4 [ 2.300327] __component_add+0xa8/0x194 [ 2.300338] component_add+0x18/0x28 [ 2.300349] dsi_dev_attach+0x24/0x34 [ 2.300359] dsi_host_attach+0xa0/0x154 [ 2.300370] devm_mipi_dsi_attach+0x38/0x98 [ 2.300381] ana6707_amb650yl01_probe+0x24c/0x39c [ 2.300392] mipi_dsi_drv_probe+0x24/0x34 [ 2.300404] really_probe+0xc4/0x398 [ 2.300414] __driver_probe_device+0x80/0x164 [ 2.300425] driver_probe_device+0x40/0x118 [ 2.300436] __device_attach_driver+0xc8/0x16c [ 2.300446] bus_for_each_drv+0x84/0xe4 [ 2.300455] __device_attach+0xa0/0x1b4 [ 2.300466] device_initial_probe+0x18/0x28 [ 2.300476] bus_probe_device+0xb4/0xb8 [ 2.300485] device_add+0x5a0/0x79c [ 2.300494] mipi_dsi_device_register_full+0x110/0x1c4 [ 2.300507] mipi_dsi_host_register+0xa8/0x1b0 [ 2.300518] msm_dsi_host_register+0x44/0x64 [ 2.300528] msm_dsi_manager_register+0x174/0x230 [ 2.300540] dsi_dev_probe+0x18c/0x248 [ 2.300549] platform_probe+0x6c/0xcc [ 2.300560] really_probe+0xc4/0x398 [ 2.300570] __driver_probe_device+0x80/0x164 [ 2.300580] driver_probe_device+0x40/0x118 [ 2.300590] __device_attach_driver+0xc8/0x16c [ 2.300600] bus_for_each_drv+0x84/0xe4 [ 2.300610] __device_attach+0xa0/0x1b4 [ 2.300620] device_initial_probe+0x18/0x28 [ 2.300631] bus_probe_device+0xb4/0xb8 [ 2.300641] deferred_probe_work_func+0xa4/0xf8 [ 2.300652] process_one_work+0x14c/0x2d4 [ 2.300663] worker_thread+0x280/0x494 [ 2.300674] kthread+0xe4/0xe8 [ 2.300683] ret_from_fork+0x10/0x20Let me know what you think about this.
mentioned in issue #56
-
If I pick Jun Nie's
pic_width /= 2patch at https://lore.kernel.org/linux-arm-msm/20240829-sm8650-v6-11-hmd-pocf-mdss-quad-upstream-8-v1-2-bdb05b4b5a2e@linaro.org/, together with this missing (probably mis-squashed) part from patch 03/21:diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 7a4d9c071be5a..5abade8f26b88 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -953,7 +953,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) return; } - dsc->pic_width = mode->hdisplay; + dsc->pic_width = hdisplay; dsc->pic_height = mode->vdisplay; DBG("Mode %dx%d\n", dsc->pic_width, dsc->pic_height);This bonded DSI panel becomes very unhappy:
mentioned in issue #64
