mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
To: Alexey Klimov <alexey.klimov@linaro.org>,
	Rob Clark <robin.clark@oss.qualcomm.com>,
	dri-devel@lists.freedesktop.org
Cc: linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org,
	Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>,
	Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>,
	Konrad Dybcio <konradybcio@kernel.org>,
	Abhinav Kumar <quic_abhinavk@quicinc.com>,
	Dmitry Baryshkov <lumag@kernel.org>,
	Marijn Suijten <marijn.suijten@somainline.org>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 3/3] drm/msm/adreno: Check for recognized GPU before bind
Date: Thu, 12 Jun 2025 18:08:53 +0200	[thread overview]
Message-ID: <1bf920c8-245b-40c3-bce1-ec5194b30fd9@oss.qualcomm.com> (raw)
In-Reply-To: <DAKOKYU9O323.M7OSA1CFHQWX@linaro.org>

On 6/12/25 6:04 PM, Alexey Klimov wrote:
> On Mon Jun 9, 2025 at 7:24 PM BST, Rob Clark wrote:
>> If we have a newer dtb than kernel, we could end up in a situation where
>> the GPU device is present in the dtb, but not in the drivers device
>> table.  We don't want this to prevent the display from probing.  So
>> check that we recognize the GPU before adding the GPU component.
>>
>> v2: use %pOF
>>
>> Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
>> ---
>>  drivers/gpu/drm/msm/adreno/adreno_device.c | 29 ++++++++++++++++++----
>>  drivers/gpu/drm/msm/msm_drv.c              |  2 +-
>>  drivers/gpu/drm/msm/msm_gpu.h              |  1 +
>>  3 files changed, 26 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> index 778e6ae7f137..0d12454b1f2e 100644
>> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
>> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> @@ -178,6 +178,26 @@ static int find_chipid(struct device_node *node, uint32_t *chipid)
>>  	return 0;
>>  }
>>  
>> +bool adreno_has_gpu(struct device_node *node)
>> +{
>> +	const struct adreno_info *info;
>> +	uint32_t chip_id;
>> +	int ret;
>> +
>> +	ret = find_chipid(node, &chip_id);
>> +	if (ret)
>> +		return false;
>> +
>> +	info = adreno_info(chip_id);
>> +	if (!info) {
>> +		pr_warn("%pOF: Unknown GPU revision: %"ADRENO_CHIPID_FMT"\n",
>> +			node, ADRENO_CHIPID_ARGS(chip_id));
>> +		return false;
>> +	}
>> +
>> +	return true;
>> +}
>> +
>>  static int adreno_bind(struct device *dev, struct device *master, void *data)
>>  {
>>  	static struct adreno_platform_config config = {};
>> @@ -188,18 +208,17 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
>>  	int ret;
>>  
>>  	ret = find_chipid(dev->of_node, &config.chip_id);
>> -	if (ret)
>> +	/* We shouldn't have gotten this far if we can't parse the chip_id */
>> +	if (WARN_ON(ret))
>>  		return ret;
> 
> I just hit this with linux-next on qrb2210 RB1 [1].
> 
> Is it expected an warning now or do we miss some device tree updates on
> linux-next for RB1?
> 
> I don't recall seeing such warnings previously.
> 
> Thanks,
> Alexey
> 
> [1]:
> 
>  msm_dpu 5e01000.display-controller: bound 5e94000.dsi (ops dsi_ops [msm])
>  ------------[ cut here ]------------
>  WARNING: CPU: 0 PID: 242 at drivers/gpu/drm/msm/adreno/adreno_device.c:224 adreno_bind+0x90/0x120 [msm]
>  Modules linked in: q6asm_dai q6routing q6afe_dai q6adm q6asm q6afe_clocks snd_q6dsp_common q6afe q6core apr pdr_interface qrtr_smd qcom_pd_mapper qcom_pdr_msg mcp251xfd ath10k_snoc snd_soc_wsa881x_i2c snd_soc_wsa881x_common can_dev lontium_lt9611uxc(+) ath10k_core ath mac80211 hci_uart btqca btbcm libarc4 msm snd_soc_sm8250 qrtr bluetooth drm_exec snd_soc_qcom_sdw qcom_q6v5_pas llcc_qcom snd_soc_qcom_common lmh qcom_wdt ocmem cfg80211 ecdh_generic qcom_pil_info pinctrl_sm6115_lpass_lpi gpu_sched ecc drm_display_helper rfkill qcom_q6v5 pinctrl_lpass_lpi qcom_sysmon pwrseq_core lpasscc_sm6115 dispcc_qcm2290 qcom_common snd_soc_lpass_va_macro cec snd_soc_lpass_rx_macro drm_dp_aux_bus snd_soc_lpass_tx_macro qcom_glink_smem gpucc_qcm2290 snd_soc_pm4125 mdt_loader snd_soc_lpass_macro_common qmi_helpers snd_soc_pm4125_sdw soundwire_qcom regmap_sdw slimbus qcom_pmic_tcpm qcom_usb_vbus_regulator drm_client_lib tcpm rtc_pm8xxx snd_soc_wcd_mbhc aux_hpd_bridge qcom_pon qcrypto soundwire_bus sha256 qcom_stats gpi
>   spi_geni_qcom i2c_qcom_geni rpmsg_ctrl libsha256_generic libsha256 rpmsg_char qcom_rng sha256_arm64 authenc icc_bwmon phy_qcom_qmp_usbc libdes typec phy_qcom_qusb2 display_connector i2c_gpio rmtfs_mem drm_kms_helper socinfo fuse drm backlight dm_mod ip_tables x_tables ipv6
>  CPU: 0 UID: 0 PID: 242 Comm: (udev-worker) Not tainted 6.16.0-rc1-next-20250612-00025-g0ce0d3974333-dirty #2 PREEMPT 
>  Hardware name: Qualcomm Technologies, Inc. Robotics RB1 (DT)
>  pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>  pc : adreno_bind+0x90/0x120 [msm]
>  lr : adreno_bind+0x50/0x120 [msm]
>  sp : ffff8000813f3580
>  x29: ffff8000813f3580 x28: ffff000008ae8800 x27: ffff000007c63700
>  x26: ffffca4a2814b860 x25: ffff000008b26880 x24: ffffca4a24922000
>  x23: ffffca4a249229d8 x22: ffff000009838800 x21: ffff000008b26880
>  x20: ffff000002ce4410 x19: ffffca4a2495a710 x18: 0000000000000006
>  x17: 6f5f697364207370 x16: 6f28206973642e30 x15: 0720072007200720
>  x14: 0000000000000000 x13: 0000000000000000 x12: 0101010101010101
>  x11: 7f7f7f7f7f7f7f7f x10: ffffca4a2866e1b2 x9 : 0000000000000002
>  x8 : 0101010101010101 x7 : 7f7f7f7f7f7f7f7f x6 : fefefeff2f383131
>  x5 : ffff00007fc1b968 x4 : 0000000000000000 x3 : 000000000000001e
>  x2 : 0000000000000001 x1 : 0000000007000200 x0 : ffffca4a2495a710
>  Call trace:
>   adreno_bind+0x90/0x120 [msm] (P)
>   component_bind_all+0x100/0x23c
>   msm_drm_bind+0x148/0x3e0 [msm]
>   try_to_bring_up_aggregate_device+0x16c/0x1e0
>   __component_add+0xa4/0x174
>   component_add+0x14/0x20
>   dsi_dev_attach+0x20/0x34 [msm]
>   dsi_host_attach+0x58/0x98 [msm]
>   devm_mipi_dsi_attach+0x34/0x90
>   lt9611uxc_attach_dsi.isra.0+0x94/0x124 [lontium_lt9611uxc]
>   lt9611uxc_probe+0x568/0x604 [lontium_lt9611uxc]
>   i2c_device_probe+0x158/0x32c
>   really_probe+0xbc/0x2b4
>   __driver_probe_device+0x78/0x120
>   driver_probe_device+0x3c/0x154
>   __driver_attach+0x90/0x1a0
>   bus_for_each_dev+0x68/0xb8
>   driver_attach+0x24/0x30
>   bus_add_driver+0xe4/0x208
>   driver_register+0x68/0x124
>   i2c_register_driver+0x48/0xcc
>   lt9611uxc_driver_init+0x20/0x1000 [lontium_lt9611uxc]
>   do_one_initcall+0x60/0x1d4
>   do_init_module+0x54/0x23c
>   load_module+0x1730/0x1cc0
>   init_module_from_file+0x74/0xa0
>   __arm64_sys_finit_module+0x130/0x2f8
>   invoke_syscall+0x48/0x104
>   el0_svc_common.constprop.0+0xc0/0xe0
>   do_el0_svc+0x1c/0x28
>   el0_svc+0x2c/0x80
>   el0t_64_sync_handler+0x10c/0x138
>   el0t_64_sync+0x198/0x19c
>  ---[ end trace 0000000000000000 ]---
>  adreno 5900000.gpu: supply vdd not found, using dummy regulator
>  adreno 5900000.gpu: supply vddcx not found, using dummy regulator
>  msm_dpu 5e01000.display-controller: bound 5900000.gpu (ops a3xx_ops [msm])
>  [drm:dpu_kms_hw_init:1173] dpu hardware revision:0x60050000
>  dummy 1-0045: No cache used with register defaults set!
>  [drm] Initialized msm 1.12.0 for 5e01000.display-controller on minor 0
>  msm_dpu 5e01000.display-controller: [drm:adreno_request_fw [msm]] loaded qcom/a702_sqe.fw from new location

Looks like we should be doing this instead

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 5e7307567239..16e7ac444efd 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -221,7 +221,7 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
 
        info = adreno_info(config.chip_id);
        /* We shouldn't have gotten this far if we don't recognize the GPU: */
-       if (!WARN_ON(info))
+       if (WARN_ON(!info))
                return -ENXIO;
 
        config.info = info;


Konrad

  reply	other threads:[~2025-06-12 16:09 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-09 18:24 [PATCH v2 0/3] drm/msm: Better unknown GPU handling Rob Clark
2025-06-09 18:24 ` [PATCH v2 1/3] drm/msm: Rename add_components_mdp() Rob Clark
2025-06-09 18:54   ` Dmitry Baryshkov
2025-06-09 18:24 ` [PATCH v2 2/3] drm/msm/adreno: Pass device_node to find_chipid() Rob Clark
2025-06-09 18:54   ` Dmitry Baryshkov
2025-06-10 12:05   ` Konrad Dybcio
2025-06-09 18:24 ` [PATCH v2 3/3] drm/msm/adreno: Check for recognized GPU before bind Rob Clark
2025-06-09 18:54   ` Dmitry Baryshkov
2025-06-10 12:07   ` Konrad Dybcio
2025-06-12 16:04   ` Alexey Klimov
2025-06-12 16:08     ` Konrad Dybcio [this message]
2025-06-12 20:47       ` Rob Clark
2025-06-13  9:24       ` neil.armstrong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1bf920c8-245b-40c3-bce1-ec5194b30fd9@oss.qualcomm.com \
    --to=konrad.dybcio@oss.qualcomm.com \
    --cc=airlied@gmail.com \
    --cc=alexey.klimov@linaro.org \
    --cc=dmitry.baryshkov@oss.qualcomm.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=konradybcio@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lumag@kernel.org \
    --cc=marijn.suijten@somainline.org \
    --cc=quic_abhinavk@quicinc.com \
    --cc=robdclark@gmail.com \
    --cc=robin.clark@oss.qualcomm.com \
    --cc=sean@poorly.run \
    --cc=simona@ffwll.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®