Re: [PATCH] drm/msm/dpu: fix vblank IRQ handling for command panels
On Sat, 30 Mar 2024 at 18:49, Marijn Suijten
wrote:
>
> On 2024-03-30 05:52:29, Dmitry Baryshkov wrote:
> > In case of CMD DSI panels, the vblank IRQ can be used outside of
> > irq_enable/irq_disable pair. This results in the following kind of
>
> Can you clarify when exactly that is? Is it via ops.control_vblank_irq in
> dpu_encoder_toggle_vblank_for_crtc()?
Call trace:
dpu_encoder_phys_cmd_control_vblank_irq+0x218/0x294
dpu_encoder_toggle_vblank_for_crtc+0x160/0x194
dpu_crtc_vblank+0xbc/0x228
dpu_kms_enable_vblank+0x18/0x24
vblank_ctrl_worker+0x34/0x6c
process_one_work+0x218/0x620
worker_thread+0x1ac/0x37c
kthread+0x114/0x118
ret_from_fork+0x10/0x20
The vblank_ctrl_work happens when the framework attempts to trigger
the vblank on the CRTC.
>
> > messages. Move assignment of IRQ indices to atomic_enable /
> > atomic_disable callbacks.
> >
> > [dpu error]invalid IRQ=[134217727, 31]
> > [drm:dpu_encoder_phys_cmd_control_vblank_irq] *ERROR* vblank irq err id:31
> > pp:0 ret:-22, enable true/0
> > [drm:dpu_encoder_phys_cmd_control_vblank_irq] *ERROR* vblank irq err id:31
> > pp:0 ret:-22, enable false/0
>
> You are right that such messages are common, both at random but also seemingly
> around toggling the `ACTIVE` property on the CRTC:
>
> [ 45.878300] panel-samsung-souxp ae94000.dsi.0:
> samsung_souxp00_disable
> [ 45.909941] panel-samsung-souxp ae94000.dsi.0:
> samsung_souxp00_unprepare
> [ 46.093234] [drm:dpu_encoder_helper_wait_for_irq] *ERROR* encoder
> is disabled id=31, callback=dpu_encoder_phys_cmd_ctl_start_irq,
> IRQ=[134217727, 31]
> [ 46.130421] panel-samsung-souxp ae94000.dsi.0:
> samsung_souxp00_prepare
> [ 46.340457] panel-samsung-souxp ae94000.dsi.0:
> samsung_souxp00_enable
> [ 65.520323] [dpu error]invalid IRQ=[134217727, 31]
> irq_cb:dpu_encoder_phys_cmd_te_rd_ptr_irq
> [ 65.520463] [drm:dpu_encoder_phys_cmd_control_vblank_irq] *ERROR*
> vblank irq err id:31 pp:0 ret:-22, enable true/0
> [ 65.630199] [drm:dpu_encoder_phys_cmd_control_vblank_irq] *ERROR*
> vblank irq err id:31 pp:0 ret:-22, enable false/0
> [ 166.576465] panel-samsung-souxp ae94000.dsi.0:
> samsung_souxp00_disable
> [ 166.609674] panel-samsung-souxp ae94000.dsi.0:
> samsung_souxp00_unprepare
> [ 166.781967] [drm:dpu_encoder_helper_wait_for_irq] *ERROR* encoder
> is disabled id=31, callback=dpu_encoder_phys_cmd_ctl_start_irq,
> IRQ=[134217727, 31]
> [ 166.829805] panel-samsung-souxp ae94000.dsi.0:
> samsung_souxp00_prepare
> [ 167.040476] panel-samsung-souxp ae94000.dsi.0:
> samsung_souxp00_enable
> [ 337.449827] [dpu error]invalid IRQ=[134217727, 31]
> irq_cb:dpu_encoder_phys_cmd_te_rd_ptr_irq
> [ 337.450434] [drm:dpu_encoder_phys_cmd_control_vblank_irq] *ERROR*
> vblank irq err id:31 pp:0 ret:-22, enable true/0
> [ 337.569526] [drm:dpu_encoder_phys_cmd_control_vblank_irq] *ERROR*
> vblank irq err id:31 pp:0 ret:-22, enable false/0
> [ 354.980357] [dpu error]invalid IRQ=[134217727, 31]
> irq_cb:dpu_encoder_phys_cmd_te_rd_ptr_irq
> [ 354.980495] [drm:dpu_encoder_phys_cmd_control_vblank_irq] *ERROR*
> vblank irq err id:31 pp:0 ret:-22, enable true/0
> [ 355.090460] [drm:dpu_encoder_phys_cmd_control_vblank_irq] *ERROR*
> vblank irq err id:31 pp:0 ret:-22, enable false/0
>
> Unfortunately with this patch, turning the CRTC off via ./modetest -M msm -a
> -w 81:ACTIVE:0 immediately triggers a bunch of WARNs (note that the CRTC turns
> on immediately again when the command returns, that's probably the framebuffer
> console taking over again). Running it a few times in succession this may or
> may not happen, or reboot the phone (Xperia Griffin) entirely:
I could not reproduce it here, on Pixel-3. I'd like to review vblank
IRQs later. For now I think it is easier to revert d13f638c9b88
("drm/msm/dpu: drop dpu_encoder_phys_ops.atomic_mode_set"). I'll send
a patch.
>
> [ 23.423930] panel-samsung-souxp ae94000.dsi.0:
> samsung_souxp00_disable
> [ 23.461013] [dpu error]invalid IRQ=[134217727, 31]
> [ 23.461144] [dpu error]invalid IRQ=[134217727, 31]
> [ 23.461208] [drm:dpu_encoder_phys_cmd_control_vblank_irq] *ERROR*
> vblank irq err id:31 pp:0 ret:-22, enable false/1
> [ 23.461340] [dpu error]invalid IRQ=[134217727, 31]
> [ 23.461406] panel-samsung-souxp ae94000.dsi.0:
> samsung_souxp00_unprepare
> [ 23.641721] [drm:dpu_encoder_helper_wait_for_irq] *ERROR* encoder
> is disabled id=31, callback=dpu_encoder_phys_cmd_ctl_start_irq,
> IRQ=[134217727, 31]
> [ 23.679938] panel-samsung-souxp ae94000.dsi.0:
> samsung_souxp00_prepare
> [ 23.900465] [ cut here ]
> [ 23.900813] WARNING: CPU: 1 PID: 747 at
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c:545
>
Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD
On Sat, 6 Apr 2024 at 06:16, Abhinav Kumar wrote:
>
> From: Kuogee Hsieh
>
> For HPD events coming from external modules using drm_bridge_hpd_notify(),
> the sequence of calls leading to dp_bridge_hpd_notify() is like below:
>
> dp_bridge_hpd_notify+0x18/0x70 [msm]
> drm_bridge_connector_detect+0x60/0xe8 [drm_kms_helper]
> drm_helper_probe_detect+0x94/0xc0 [drm_kms_helper]
> drm_helper_probe_single_connector_modes+0x43c/0x53c [drm_kms_helper]
> drm_client_modeset_probe+0x240/0x1114 [drm]
> drm_fb_helper_hotplug_event.part.26+0x9c/0xe8 [drm_kms_helper]
> drm_fb_helper_hotplug_event+0x24/0x38 [drm_kms_helper]
> msm_fbdev_client_hotplug+0x24/0xd4 [msm]
> drm_client_dev_hotplug+0xd8/0x148 [drm]
> drm_kms_helper_connector_hotplug_event+0x30/0x3c [drm_kms_helper]
> drm_bridge_connector_handle_hpd+0x84/0x94 [drm_kms_helper]
> drm_bridge_connector_hpd_cb+0xc/0x14 [drm_kms_helper]
> drm_bridge_hpd_notify+0x38/0x50 [drm]
> drm_aux_hpd_bridge_notify+0x14/0x20 [aux_hpd_bridge]
> pmic_glink_altmode_worker+0xec/0x27c [pmic_glink_altmode]
> process_scheduled_works+0x17c/0x2cc
> worker_thread+0x2ac/0x2d0
> kthread+0xfc/0x120
>
> There are three notifications delivered to DP driver for each notification
> event.
>
> 1) From the drm_aux_hpd_bridge_notify() itself as shown above
>
> 2) From output_poll_execute() thread which arises due to
> drm_helper_probe_single_connector_modes() call of the above stacktrace
> as shown in more detail here.
>
> dp_bridge_hpd_notify+0x18/0x70 [msm]
> drm_bridge_connector_detect+0x60/0xe8 [drm_kms_helper]
> drm_helper_probe_detect+0x94/0xc0 [drm_kms_helper]
> drm_helper_probe_single_connector_modes+0x43c/0x53c [drm_kms_helper]
> drm_client_modeset_probe+0x240/0x1114 [drm]
> drm_fb_helper_hotplug_event.part.26+0x9c/0xe8 [drm_kms_helper]
> drm_fb_helper_hotplug_event+0x24/0x38 [drm_kms_helper]
> msm_fbdev_client_hotplug+0x24/0xd4 [msm]
> drm_client_dev_hotplug+0xd8/0x148 [drm]
> drm_kms_helper_hotplug_event+0x30/0x3c [drm_kms_helper]
> output_poll_execute+0xe0/0x210 [drm_kms_helper]
>
> 3) From the DP driver as the dp_bridge_hpd_notify() callback today triggers
> the hpd_event_thread for connect and disconnect events respectively via below
> stack
>
> dp_bridge_hpd_notify+0x18/0x70 [msm]
> drm_bridge_connector_detect+0x60/0xe8 [drm_kms_helper]
> drm_helper_probe_detect_ctx+0x98/0x110 [drm_kms_helper]
> check_connector_changed+0x4c/0x20c [drm_kms_helper]
> drm_helper_hpd_irq_event+0x98/0x120 [drm_kms_helper]
> hpd_event_thread+0x478/0x5bc [msm]
>
> dp_bridge_hpd_notify() delivered from output_poll_execute() thread
> returns the incorrect HPD status as the MSM DP driver returns the value
> of link_ready and not the HPD status currently in the .detect() callback.
>
> And because the HPD event thread has not run yet, this results in two
> complementary
> events.
>
> To address this, fix dp_bridge_hpd_notify() to call
> dp_hpd_plug_handle/unplug_handle()
> directly to return consistent values for the above scenarios.
>
> changes in v3:
> - Fix the commit message as per submitting guidelines.
> - remove extra line added
>
> changes in v2:
> - Fix the commit message to explain the scenario
> - Fix the subject a little as well
>
> Fixes: 542b37efc20e ("drm/msm/dp: Implement hpd_notify()")
> Signed-off-by: Kuogee Hsieh
> Signed-off-by: Abhinav Kumar
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
Re: [PATCH 4/6] drm/msm/adreno: Implement SMEM-based speed bin
Hi Konrad,
kernel test robot noticed the following build errors:
[auto build test ERROR on 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd]
url:
https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/soc-qcom-Move-some-socinfo-defines-to-the-header-expand-them/20240405-164231
base: 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd
patch link:
https://lore.kernel.org/r/20240405-topic-smem_speedbin-v1-4-ce2b864251b1%40linaro.org
patch subject: [PATCH 4/6] drm/msm/adreno: Implement SMEM-based speed bin
config: i386-buildonly-randconfig-003-20240406
(https://download.01.org/0day-ci/archive/20240406/202404061841.njuovdv7-...@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240406/202404061841.njuovdv7-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202404061841.njuovdv7-...@intel.com/
All errors (new ones prefixed by >>):
drivers/gpu/drm/msm/adreno/adreno_gpu.c: In function 'adreno_read_speedbin':
>> drivers/gpu/drm/msm/adreno/adreno_gpu.c:1090:14: error: implicit declaration
>> of function 'FIELD_PREP'; did you mean 'NEED_PGE'?
>> [-Werror=implicit-function-declaration]
*speedbin = FIELD_PREP(ADRENO_SKU_ID_PCODE, pcode) |
^~
NEED_PGE
cc1: some warnings being treated as errors
vim +1090 drivers/gpu/drm/msm/adreno/adreno_gpu.c
1062
1063 int adreno_read_speedbin(struct adreno_gpu *adreno_gpu,
1064 struct device *dev, u32 *speedbin)
1065 {
1066 u32 fcode, pcode;
1067 int ret;
1068
1069 /* Try reading the speedbin via a nvmem cell first */
1070 ret = nvmem_cell_read_variable_le_u32(dev, "speed_bin",
speedbin);
1071 if (!ret && ret != -EINVAL)
1072 return ret;
1073
1074 ret = qcom_smem_get_feature_code();
1075 if (ret) {
1076 dev_err(dev, "Couldn't get feature code from SMEM!\n");
1077 return ret;
1078 }
1079
1080 ret = qcom_smem_get_product_code();
1081 if (ret) {
1082 dev_err(dev, "Couldn't get product code from SMEM!\n");
1083 return ret;
1084 }
1085
1086 /* Don't consider fcode for external feature codes */
1087 if (fcode <= SOCINFO_FC_EXT_RESERVE)
1088 fcode = SOCINFO_FC_UNKNOWN;
1089
> 1090 *speedbin = FIELD_PREP(ADRENO_SKU_ID_PCODE, pcode) |
1091 FIELD_PREP(ADRENO_SKU_ID_FCODE, fcode);
1092
1093 return ret;
1094 }
1095
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH 4/6] drm/msm/adreno: Implement SMEM-based speed bin
Hi Konrad,
kernel test robot noticed the following build errors:
[auto build test ERROR on 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd]
url:
https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/soc-qcom-Move-some-socinfo-defines-to-the-header-expand-them/20240405-164231
base: 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd
patch link:
https://lore.kernel.org/r/20240405-topic-smem_speedbin-v1-4-ce2b864251b1%40linaro.org
patch subject: [PATCH 4/6] drm/msm/adreno: Implement SMEM-based speed bin
config: i386-buildonly-randconfig-001-20240406
(https://download.01.org/0day-ci/archive/20240406/202404061839.0wagfxwj-...@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project
6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240406/202404061839.0wagfxwj-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202404061839.0wagfxwj-...@intel.com/
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/msm/adreno/adreno_gpu.c:1090:14: error: call to undeclared
>> function 'FIELD_PREP'; ISO C99 and later do not support implicit function
>> declarations [-Wimplicit-function-declaration]
1090 | *speedbin = FIELD_PREP(ADRENO_SKU_ID_PCODE, pcode) |
| ^
1 error generated.
vim +/FIELD_PREP +1090 drivers/gpu/drm/msm/adreno/adreno_gpu.c
1062
1063 int adreno_read_speedbin(struct adreno_gpu *adreno_gpu,
1064 struct device *dev, u32 *speedbin)
1065 {
1066 u32 fcode, pcode;
1067 int ret;
1068
1069 /* Try reading the speedbin via a nvmem cell first */
1070 ret = nvmem_cell_read_variable_le_u32(dev, "speed_bin",
speedbin);
1071 if (!ret && ret != -EINVAL)
1072 return ret;
1073
1074 ret = qcom_smem_get_feature_code();
1075 if (ret) {
1076 dev_err(dev, "Couldn't get feature code from SMEM!\n");
1077 return ret;
1078 }
1079
1080 ret = qcom_smem_get_product_code();
1081 if (ret) {
1082 dev_err(dev, "Couldn't get product code from SMEM!\n");
1083 return ret;
1084 }
1085
1086 /* Don't consider fcode for external feature codes */
1087 if (fcode <= SOCINFO_FC_EXT_RESERVE)
1088 fcode = SOCINFO_FC_UNKNOWN;
1089
> 1090 *speedbin = FIELD_PREP(ADRENO_SKU_ID_PCODE, pcode) |
1091 FIELD_PREP(ADRENO_SKU_ID_FCODE, fcode);
1092
1093 return ret;
1094 }
1095
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] phy: qcom: qmp-combo: Fix register base for QSERDES_DP_PHY_MODE
On Thu, 04 Apr 2024 17:01:03 -0700, Stephen Boyd wrote:
> The register base that was used to write to the QSERDES_DP_PHY_MODE
> register was 'dp_dp_phy' before commit 815891eee668 ("phy:
> qcom-qmp-combo: Introduce orientation variable"). There isn't any
> explanation in the commit why this is changed, so I suspect it was an
> oversight or happened while being extracted from some other series.
> Oddly the value being 0x4c or 0x5c doesn't seem to matter for me, so I
> suspect this is dead code, but that can be fixed in another patch. It's
> not good to write to the wrong register space, and maybe some other
> version of this phy relies on this.
>
> [...]
Applied, thanks!
[1/1] phy: qcom: qmp-combo: Fix register base for QSERDES_DP_PHY_MODE
commit: ee13e1f3c72b9464a4d73017c060ab503eed653a
Best regards,
--
~Vinod
Re: [PATCH] phy: qcom: qmp-combo: Fix VCO div offset on v3
On Thu, 04 Apr 2024 16:43:44 -0700, Stephen Boyd wrote:
> Commit ec17373aebd0 ("phy: qcom: qmp-combo: extract common function to
> setup clocks") changed the offset that is used to write to
> DP_PHY_VCO_DIV from QSERDES_V3_DP_PHY_VCO_DIV to
> QSERDES_V4_DP_PHY_VCO_DIV. Unfortunately, this offset is different
> between v3 and v4 phys:
>
> #define QSERDES_V3_DP_PHY_VCO_DIV 0x064
> #define QSERDES_V4_DP_PHY_VCO_DIV 0x070
>
> [...]
Applied, thanks!
[1/1] phy: qcom: qmp-combo: Fix VCO div offset on v3
commit: 5abed58a8bde6d349bde364a160510b5bb904d18
Best regards,
--
~Vinod
