drivers/gpu/drm/display/drm_bridge_connector.c | 5 +++++ 1 file changed, 5 insertions(+)
It's ok to pass atomic check successfully if an atomic commit tries to
disable the display pipeline which the connector belongs to. That is,
when the crtc or the best_encoder pointers in struct drm_connector_state
are NULL, drm_bridge_connector_atomic_check() should return 0 directly.
Without the check against the NULL pointers, drm_default_rgb_quant_range()
called by drm_atomic_helper_connector_hdmi_check() would dereference
the NULL pointer to_match in drm_match_cea_mode().
[ 46.189903] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[ 46.189906] Mem abort info:
[ 46.189908] ESR = 0x0000000096000004
[ 46.189910] EC = 0x25: DABT (current EL), IL = 32 bits
[ 46.189914] SET = 0, FnV = 0
[ 46.189917] EA = 0, S1PTW = 0
[ 46.189919] FSC = 0x04: level 0 translation fault
[ 46.189922] Data abort info:
[ 46.189923] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[ 46.189926] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[ 46.189929] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[ 46.189932] user pgtable: 4k pages, 48-bit VAs, pgdp=000000010b6b4000
[ 46.189936] [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
[ 46.189945] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
[ 46.719532] Modules linked in: caam_jr caamhash_desc caamalg_desc dw_hdmi_cec crypto_engine snd_soc_hdmi_codec authenc libdes hantro_vpu mwifiex_pcie mwifiex v4l2_vp9 v4l2_h264 imx8
mp_hdmi_tx v4l2_jpeg v4l2_mem2mem dw_hdmi videobuf2_dma_contig videobuf2_memops ite_it6263 videobuf2_v4l2 snd_soc_simple_card drm_display_helper videodev snd_soc_wm8960 snd_soc_simple_
card_utils fsl_ldb samsung_dsim videobuf2_common etnaviv mc fsl_imx8_ddr_perf gpu_sched imx_lcdif adv7511 phy_fsl_samsung_hdmi drm_client_lib snd_soc_fsl_micfil drm_dma_helper cec imx8
mp_hdmi_pvi snd_soc_fsl_sai imx_pcm_dma snd_soc_fsl_utils rtc_snvs pwm_imx27 snvs_pwrkey flexcan caam can_dev error imx8mm_thermal imx_sdma coresight_tmc display_connector drm_kms_help
er snd_soc_bt_sco imx_cpufreq_dt coresight_funnel coresight overlay bluetooth ecdh_generic ecc cfg80211 rfkill drm fuse backlight ipv6
[ 46.795415] CPU: 3 UID: 0 PID: 64 Comm: kworker/3:1 Not tainted 6.13.0-rc6-next-20250107-00009-g045f95f0de2e #1637
[ 46.805767] Hardware name: NXP i.MX8MPlus EVK board (DT)
[ 46.811078] Workqueue: events drm_mode_rmfb_work_fn [drm]
[ 46.816619] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 46.823581] pc : drm_default_rgb_quant_range+0x0/0x4c [drm]
[ 46.829244] lr : drm_atomic_helper_connector_hdmi_check+0xb0/0x6b0 [drm_display_helper]
[ 46.837293] sp : ffff80008364ba00
[ 46.840605] x29: ffff80008364ba10 x28: ffff0000c8955080 x27: ffff0000c2e1c700
[ 46.847743] x26: ffff0000d3887200 x25: ffff0000c2e1c700 x24: ffff0000d3887400
[ 46.854881] x23: 0000000000000001 x22: 0000000000000000 x21: ffff0000c8955080
[ 46.862019] x20: 0000000000000001 x19: ffff0000d3887c00 x18: 00000000ffffffff
[ 46.869157] x17: 0000000000000000 x16: 0000000000000000 x15: ffff800082ce6911
[ 46.876293] x14: 0000000000000001 x13: ffff8000827e4a88 x12: 00000000000015f0
[ 46.883435] x11: 0000000000000750 x10: ffff80008283ca88 x9 : ffff8000827e4a88
[ 46.890573] x8 : 00000000ffffefff x7 : 0000000000000038 x6 : ffff0000db2c3440
[ 46.897711] x5 : 0000000000000001 x4 : 0000000000000038 x3 : ffff0000db2c3440
[ 46.904847] x2 : 0000000000000000 x1 : 0000000000000001 x0 : 0000000000000000
[ 46.911984] Call trace:
[ 46.914429] drm_default_rgb_quant_range+0x0/0x4c [drm] (P)
[ 46.920106] drm_bridge_connector_atomic_check+0x20/0x2c [drm_display_helper]
[ 46.927275] drm_atomic_helper_check_modeset+0x488/0xc78 [drm_kms_helper]
[ 46.934111] drm_atomic_helper_check+0x20/0xa4 [drm_kms_helper]
[ 46.940063] drm_atomic_check_only+0x4b8/0x984 [drm]
[ 46.945136] drm_atomic_commit+0x48/0xc4 [drm]
[ 46.949674] drm_framebuffer_remove+0x44c/0x530 [drm]
[ 46.954809] drm_mode_rmfb_work_fn+0x7c/0xa0 [drm]
[ 46.959687] process_one_work+0x150/0x294
[ 46.963700] worker_thread+0x2dc/0x3dc
[ 46.967450] kthread+0x130/0x204
[ 46.970679] ret_from_fork+0x10/0x20
[ 46.974258] Code: d50323bf d65f03c0 52800041 17ffffe6 (b9400001)
[ 46.980350] ---[ end trace 0000000000000000 ]---
Fixes: 8ec116ff21a9 ("drm/display: bridge_connector: provide atomic_check for HDMI bridges")
Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 32108307de66..587020a3f3e8 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -343,6 +343,11 @@ static int drm_bridge_connector_atomic_check(struct drm_connector *connector,
{
struct drm_bridge_connector *bridge_connector =
to_drm_bridge_connector(connector);
+ struct drm_connector_state *new_conn_state =
+ drm_atomic_get_new_connector_state(state, connector);
+
+ if (!new_conn_state->crtc || !new_conn_state->best_encoder)
+ return 0;
if (bridge_connector->bridge_hdmi)
return drm_atomic_helper_connector_hdmi_check(connector, state);
--
2.34.1On Wed, Jan 08, 2025 at 06:13:51PM +0800, Liu Ying wrote:
> It's ok to pass atomic check successfully if an atomic commit tries to
> disable the display pipeline which the connector belongs to. That is,
> when the crtc or the best_encoder pointers in struct drm_connector_state
> are NULL, drm_bridge_connector_atomic_check() should return 0 directly.
> Without the check against the NULL pointers, drm_default_rgb_quant_range()
> called by drm_atomic_helper_connector_hdmi_check() would dereference
> the NULL pointer to_match in drm_match_cea_mode().
>
[..]
> [ 46.823581] pc : drm_default_rgb_quant_range+0x0/0x4c [drm]
> [ 46.829244] lr : drm_atomic_helper_connector_hdmi_check+0xb0/0x6b0 [drm_display_helper]
> [ 46.837293] sp : ffff80008364ba00
[..]
> [ 46.911984] Call trace:
> [ 46.914429] drm_default_rgb_quant_range+0x0/0x4c [drm] (P)
> [ 46.920106] drm_bridge_connector_atomic_check+0x20/0x2c [drm_display_helper]
> [ 46.927275] drm_atomic_helper_check_modeset+0x488/0xc78 [drm_kms_helper]
> [ 46.934111] drm_atomic_helper_check+0x20/0xa4 [drm_kms_helper]
> [ 46.940063] drm_atomic_check_only+0x4b8/0x984 [drm]
> [ 46.945136] drm_atomic_commit+0x48/0xc4 [drm]
> [ 46.949674] drm_framebuffer_remove+0x44c/0x530 [drm]
> [ 46.954809] drm_mode_rmfb_work_fn+0x7c/0xa0 [drm]
> [ 46.959687] process_one_work+0x150/0x294
> [ 46.963700] worker_thread+0x2dc/0x3dc
> [ 46.967450] kthread+0x130/0x204
> [ 46.970679] ret_from_fork+0x10/0x20
> [ 46.974258] Code: d50323bf d65f03c0 52800041 17ffffe6 (b9400001)
> [ 46.980350] ---[ end trace 0000000000000000 ]---
Please trim the backtrace in a way I trimmed it. Also you can drop the
timestamps, they don't have useful information.
>
> Fixes: 8ec116ff21a9 ("drm/display: bridge_connector: provide atomic_check for HDMI bridges")
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
> drivers/gpu/drm/display/drm_bridge_connector.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> index 32108307de66..587020a3f3e8 100644
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> @@ -343,6 +343,11 @@ static int drm_bridge_connector_atomic_check(struct drm_connector *connector,
> {
> struct drm_bridge_connector *bridge_connector =
> to_drm_bridge_connector(connector);
> + struct drm_connector_state *new_conn_state =
> + drm_atomic_get_new_connector_state(state, connector);
> +
> + if (!new_conn_state->crtc || !new_conn_state->best_encoder)
> + return 0;
Unfortunately, this is not enough. Other drivers (e.g. sun4i) use
drm_atomic_helper_connector_hdmi_check() at connector's atomic_check()
function. Please move necessary checks to the helper itself. Also please
add corresponding KUnit test: attempt to atomic_check() the unconnected
HDMI connector, there is a test suite for the HDMI connector functions,
but the atomic_check() for the disconnected connecor seems to be missing.
>
> if (bridge_connector->bridge_hdmi)
> return drm_atomic_helper_connector_hdmi_check(connector, state);
> --
> 2.34.1
>
--
With best wishes
Dmitry
On 01/08/2025, Dmitry Baryshkov wrote:
> On Wed, Jan 08, 2025 at 06:13:51PM +0800, Liu Ying wrote:
>> It's ok to pass atomic check successfully if an atomic commit tries to
>> disable the display pipeline which the connector belongs to. That is,
>> when the crtc or the best_encoder pointers in struct drm_connector_state
>> are NULL, drm_bridge_connector_atomic_check() should return 0 directly.
>> Without the check against the NULL pointers, drm_default_rgb_quant_range()
>> called by drm_atomic_helper_connector_hdmi_check() would dereference
>> the NULL pointer to_match in drm_match_cea_mode().
>>
>
> [..]
>
>> [ 46.823581] pc : drm_default_rgb_quant_range+0x0/0x4c [drm]
>> [ 46.829244] lr : drm_atomic_helper_connector_hdmi_check+0xb0/0x6b0 [drm_display_helper]
>> [ 46.837293] sp : ffff80008364ba00
>
> [..]
>
>> [ 46.911984] Call trace:
>> [ 46.914429] drm_default_rgb_quant_range+0x0/0x4c [drm] (P)
>> [ 46.920106] drm_bridge_connector_atomic_check+0x20/0x2c [drm_display_helper]
>> [ 46.927275] drm_atomic_helper_check_modeset+0x488/0xc78 [drm_kms_helper]
>> [ 46.934111] drm_atomic_helper_check+0x20/0xa4 [drm_kms_helper]
>> [ 46.940063] drm_atomic_check_only+0x4b8/0x984 [drm]
>> [ 46.945136] drm_atomic_commit+0x48/0xc4 [drm]
>> [ 46.949674] drm_framebuffer_remove+0x44c/0x530 [drm]
>> [ 46.954809] drm_mode_rmfb_work_fn+0x7c/0xa0 [drm]
>> [ 46.959687] process_one_work+0x150/0x294
>> [ 46.963700] worker_thread+0x2dc/0x3dc
>> [ 46.967450] kthread+0x130/0x204
>> [ 46.970679] ret_from_fork+0x10/0x20
>> [ 46.974258] Code: d50323bf d65f03c0 52800041 17ffffe6 (b9400001)
>> [ 46.980350] ---[ end trace 0000000000000000 ]---
>
> Please trim the backtrace in a way I trimmed it. Also you can drop the
> timestamps, they don't have useful information.
Ack.
>
>>
>> Fixes: 8ec116ff21a9 ("drm/display: bridge_connector: provide atomic_check for HDMI bridges")
>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>> ---
>> drivers/gpu/drm/display/drm_bridge_connector.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
>> index 32108307de66..587020a3f3e8 100644
>> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
>> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
>> @@ -343,6 +343,11 @@ static int drm_bridge_connector_atomic_check(struct drm_connector *connector,
>> {
>> struct drm_bridge_connector *bridge_connector =
>> to_drm_bridge_connector(connector);
>> + struct drm_connector_state *new_conn_state =
>> + drm_atomic_get_new_connector_state(state, connector);
>> +
>> + if (!new_conn_state->crtc || !new_conn_state->best_encoder)
>> + return 0;
>
> Unfortunately, this is not enough. Other drivers (e.g. sun4i) use
> drm_atomic_helper_connector_hdmi_check() at connector's atomic_check()
> function. Please move necessary checks to the helper itself. Also please
Will move the checks to drm_atomic_helper_connector_hdmi_check().
> add corresponding KUnit test: attempt to atomic_check() the unconnected
> HDMI connector, there is a test suite for the HDMI connector functions,
> but the atomic_check() for the disconnected connecor seems to be missing.
Will add a KUnit test case to test HDMI connector disablement.
>
>>
>> if (bridge_connector->bridge_hdmi)
>> return drm_atomic_helper_connector_hdmi_check(connector, state);
>> --
>> 2.34.1
>>
>
--
Regards,
Liu Ying
© 2016 - 2025 Red Hat, Inc.