2021-11-28 14:22:30

by kernel test robot

[permalink] [raw]
Subject: [drm] d1af5cd869: BUG:kernel_NULL_pointer_dereference,address



Greeting,

FYI, we noticed the following commit (built with gcc-9):

commit: d1af5cd86997d53c140a5abdced40c5e45d68e34 ("[PATCH] drm: get rid of DRM_DEBUG_* log calls in drm core, files drm_a*.c")
url: https://github.com/0day-ci/linux/commits/Claudio-Suarez/drm-get-rid-of-DRM_DEBUG_-log-calls-in-drm-core-files-drm_a-c/20211126-185054
base: git://anongit.freedesktop.org/drm/drm drm-next
patch link: https://lore.kernel.org/dri-devel/[email protected]

in testcase: boot

on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 4G

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):


+---------------------------------------------+------------+------------+
| | c18c889111 | d1af5cd869 |
+---------------------------------------------+------------+------------+
| boot_successes | 13 | 0 |
| boot_failures | 0 | 15 |
| BUG:kernel_NULL_pointer_dereference,address | 0 | 15 |
| Oops:#[##] | 0 | 15 |
| EIP:drm_atomic_helper_check_plane_state | 0 | 15 |
| Kernel_panic-not_syncing:Fatal_exception | 0 | 15 |
+---------------------------------------------+------------+------------+


If you fix the issue, kindly add following tag
Reported-by: kernel test robot <[email protected]>


[ 125.561383][ T1] BUG: kernel NULL pointer dereference, address: 00000010
[ 125.562724][ T1] #PF: supervisor read access in kernel mode
[ 125.563784][ T1] #PF: error_code(0x0000) - not-present page
[ 125.564418][ T1] *pde = 00000000
[ 125.564418][ T1] Oops: 0000 [#1] PREEMPT SMP
[ 125.564418][ T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.16.0-rc2-00259-gd1af5cd86997 #1
[ 125.564418][ T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
[ 125.564418][ T1] EIP: drm_atomic_helper_check_plane_state (drivers/gpu/drm/drm_atomic_helper.c:867)
[ 125.564418][ T1] Code: 45 d4 50 89 f0 e8 c6 8a 00 00 5a 80 7b 6c 00 74 6f 80 7d cc 00 75 69 8b 45 e4 39 43 5c 75 08 8b 45 ec 39 43 64 74 49 8b 43 04 <8b> 00 85 c0 74 03 8b 40 08 68 d3 54 71 c2 6a 04 50 e8 50 0d 04 00
All code
========
0: 45 d4 rex.RB (bad)
2: 50 push %rax
3: 89 f0 mov %esi,%eax
5: e8 c6 8a 00 00 callq 0x8ad0
a: 5a pop %rdx
b: 80 7b 6c 00 cmpb $0x0,0x6c(%rbx)
f: 74 6f je 0x80
11: 80 7d cc 00 cmpb $0x0,-0x34(%rbp)
15: 75 69 jne 0x80
17: 8b 45 e4 mov -0x1c(%rbp),%eax
1a: 39 43 5c cmp %eax,0x5c(%rbx)
1d: 75 08 jne 0x27
1f: 8b 45 ec mov -0x14(%rbp),%eax
22: 39 43 64 cmp %eax,0x64(%rbx)
25: 74 49 je 0x70
27: 8b 43 04 mov 0x4(%rbx),%eax
2a:* 8b 00 mov (%rax),%eax <-- trapping instruction
2c: 85 c0 test %eax,%eax
2e: 74 03 je 0x33
30: 8b 40 08 mov 0x8(%rax),%eax
33: 68 d3 54 71 c2 pushq $0xffffffffc27154d3
38: 6a 04 pushq $0x4
3a: 50 push %rax
3b: e8 50 0d 04 00 callq 0x40d90

Code starting with the faulting instruction
===========================================
0: 8b 00 mov (%rax),%eax
2: 85 c0 test %eax,%eax
4: 74 03 je 0x9
6: 8b 40 08 mov 0x8(%rax),%eax
9: 68 d3 54 71 c2 pushq $0xffffffffc27154d3
e: 6a 04 pushq $0x4
10: 50 push %rax
11: e8 50 0d 04 00 callq 0x40d66
[ 125.564418][ T1] EAX: 00000010 EBX: c036fce8 ECX: 08000000 EDX: 00000001
[ 125.564418][ T1] ESI: c036fd34 EDI: 00010000 EBP: c036fcd4 ESP: c036fca0
[ 125.564418][ T1] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010287
[ 125.564418][ T1] CR0: 80050033 CR2: 00000010 CR3: 02f31000 CR4: 000406d0
[ 125.564418][ T1] Call Trace:
[ 125.564418][ T1] igt_check_plane_state (drivers/gpu/drm/selftests/test-drm_plane_helper.c:131 (discriminator 2))
[ 125.564418][ T1] ? test_drm_mm_init (drivers/gpu/drm/selftests/test-drm_modeset_common.c:16)
[ 125.564418][ T1] test_drm_modeset_init (drivers/gpu/drm/selftests/drm_selftest.c:77 drivers/gpu/drm/selftests/test-drm_modeset_common.c:19)
[ 125.564418][ T1] do_one_initcall (init/main.c:1297)
[ 125.564418][ T1] ? __this_cpu_preempt_check (lib/smp_processor_id.c:67)
[ 125.564418][ T1] ? lock_is_held_type (kernel/locking/lockdep.c:438 kernel/locking/lockdep.c:5681)
[ 125.564418][ T1] kernel_init_freeable (init/main.c:1369 init/main.c:1386 init/main.c:1405 init/main.c:1610)
[ 125.564418][ T1] ? rest_init (init/main.c:1491)
[ 125.564418][ T1] kernel_init (init/main.c:1501)
[ 125.564418][ T1] ret_from_fork (arch/x86/entry/entry_32.S:775)
[ 125.564418][ T1] Modules linked in:
[ 125.564418][ T1] CR2: 0000000000000010
[ 125.564418][ T1] random: get_random_bytes called from print_oops_end_marker+0x2c/0x80 with crng_init=0
[ 125.564418][ T1] ---[ end trace 9f868d4c92c9c57f ]---
[ 125.564418][ T1] EIP: drm_atomic_helper_check_plane_state (drivers/gpu/drm/drm_atomic_helper.c:867)
[ 125.564418][ T1] Code: 45 d4 50 89 f0 e8 c6 8a 00 00 5a 80 7b 6c 00 74 6f 80 7d cc 00 75 69 8b 45 e4 39 43 5c 75 08 8b 45 ec 39 43 64 74 49 8b 43 04 <8b> 00 85 c0 74 03 8b 40 08 68 d3 54 71 c2 6a 04 50 e8 50 0d 04 00
All code
========
0: 45 d4 rex.RB (bad)
2: 50 push %rax
3: 89 f0 mov %esi,%eax
5: e8 c6 8a 00 00 callq 0x8ad0
a: 5a pop %rdx
b: 80 7b 6c 00 cmpb $0x0,0x6c(%rbx)
f: 74 6f je 0x80
11: 80 7d cc 00 cmpb $0x0,-0x34(%rbp)
15: 75 69 jne 0x80
17: 8b 45 e4 mov -0x1c(%rbp),%eax
1a: 39 43 5c cmp %eax,0x5c(%rbx)
1d: 75 08 jne 0x27
1f: 8b 45 ec mov -0x14(%rbp),%eax
22: 39 43 64 cmp %eax,0x64(%rbx)
25: 74 49 je 0x70
27: 8b 43 04 mov 0x4(%rbx),%eax
2a:* 8b 00 mov (%rax),%eax <-- trapping instruction
2c: 85 c0 test %eax,%eax
2e: 74 03 je 0x33
30: 8b 40 08 mov 0x8(%rax),%eax
33: 68 d3 54 71 c2 pushq $0xffffffffc27154d3
38: 6a 04 pushq $0x4
3a: 50 push %rax
3b: e8 50 0d 04 00 callq 0x40d90

Code starting with the faulting instruction
===========================================
0: 8b 00 mov (%rax),%eax
2: 85 c0 test %eax,%eax
4: 74 03 je 0x9
6: 8b 40 08 mov 0x8(%rax),%eax
9: 68 d3 54 71 c2 pushq $0xffffffffc27154d3
e: 6a 04 pushq $0x4
10: 50 push %rax
11: e8 50 0d 04 00 callq 0x40d66


To reproduce:

# build kernel
cd linux
cp config-5.16.0-rc2-00259-gd1af5cd86997 .config
make HOSTCC=gcc-9 CC=gcc-9 ARCH=i386 olddefconfig prepare modules_prepare bzImage modules
make HOSTCC=gcc-9 CC=gcc-9 ARCH=i386 INSTALL_MOD_PATH=<mod-install-dir> modules_install
cd <mod-install-dir>
find lib/ | cpio -o -H newc --quiet | gzip > modules.cgz


git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> -m modules.cgz job-script # job-script is attached in this email

# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.



---
0DAY/LKP+ Test Infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation

Thanks,
Oliver Sang


Attachments:
(No filename) (7.95 kB)
config-5.16.0-rc2-00259-gd1af5cd86997 (146.26 kB)
job-script (4.83 kB)
dmesg.xz (10.34 kB)
Download all attachments

2021-11-29 19:51:03

by Claudio Suarez

[permalink] [raw]
Subject: [PATCH] drm: fix error found in some cases after the patch d1af5cd86997

The patch d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log
calls in drm core, files drm_a*.c") fails when the drm_device
cannot be found in the parameter plane_state. Fix it.

Reported-by: kernel test robot <[email protected]>
Fixes: d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log calls in drm core, files drm_a*.c")
Signed-off-by: Claudio Suarez <[email protected]>
---
drivers/gpu/drm/drm_atomic_helper.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index aef2fbd676e5..8bd4472d7949 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -312,7 +312,7 @@ update_connector_routing(struct drm_atomic_state *state,

if (!new_connector_state->crtc) {
drm_dbg_atomic(connector->dev, "Disabling [CONNECTOR:%d:%s]\n",
- connector->base.id, connector->name);
+ connector->base.id, connector->name);

set_best_encoder(state, new_connector_state, NULL);

@@ -805,6 +805,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
bool can_update_disabled)
{
struct drm_framebuffer *fb = plane_state->fb;
+ struct drm_device *dev = plane_state->plane ? plane_state->plane->dev : NULL;
struct drm_rect *src = &plane_state->src;
struct drm_rect *dst = &plane_state->dst;
unsigned int rotation = plane_state->rotation;
@@ -828,8 +829,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
}

if (!crtc_state->enable && !can_update_disabled) {
- drm_dbg_kms(plane_state->crtc->dev,
- "Cannot update plane of a disabled CRTC.\n");
+ drm_dbg_kms(dev, "Cannot update plane of a disabled CRTC.\n");
return -EINVAL;
}

@@ -839,8 +839,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
if (hscale < 0 || vscale < 0) {
- drm_dbg_kms(plane_state->crtc->dev,
- "Invalid scaling of plane\n");
+ drm_dbg_kms(dev, "Invalid scaling of plane\n");
drm_rect_debug_print("src: ", &plane_state->src, true);
drm_rect_debug_print("dst: ", &plane_state->dst, false);
return -ERANGE;
@@ -864,8 +863,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
return 0;

if (!can_position && !drm_rect_equals(dst, &clip)) {
- drm_dbg_kms(plane_state->crtc->dev,
- "Plane must cover entire CRTC\n");
+ drm_dbg_kms(dev, "Plane must cover entire CRTC\n");
drm_rect_debug_print("dst: ", dst, false);
drm_rect_debug_print("clip: ", &clip, false);
return -EINVAL;
--
2.33.0




2021-11-30 08:38:17

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH] drm: fix error found in some cases after the patch d1af5cd86997

On Mon, Nov 29, 2021 at 08:27:45PM +0100, Claudio Suarez wrote:
> The patch d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log
> calls in drm core, files drm_a*.c") fails when the drm_device
> cannot be found in the parameter plane_state. Fix it.
>
> Reported-by: kernel test robot <[email protected]>
> Fixes: d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log calls in drm core, files drm_a*.c")
> Signed-off-by: Claudio Suarez <[email protected]>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index aef2fbd676e5..8bd4472d7949 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -312,7 +312,7 @@ update_connector_routing(struct drm_atomic_state *state,
>
> if (!new_connector_state->crtc) {
> drm_dbg_atomic(connector->dev, "Disabling [CONNECTOR:%d:%s]\n",
> - connector->base.id, connector->name);
> + connector->base.id, connector->name);

Can you pls split this checkpatch fix out?

>
> set_best_encoder(state, new_connector_state, NULL);
>
> @@ -805,6 +805,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> bool can_update_disabled)
> {
> struct drm_framebuffer *fb = plane_state->fb;
> + struct drm_device *dev = plane_state->plane ? plane_state->plane->dev : NULL;

For real drivers plane_state->plane really should never be NULL, and
looking at the test report we blow up in an selftest. I think the right
fix here is to make the selftest more robust and also mock a drm_plane
(with a NULL plane->dev pointer, that should be fine).

Can you pls spin that and test it with the selftests enabled in the
config?

Thanks, Daniel

> struct drm_rect *src = &plane_state->src;
> struct drm_rect *dst = &plane_state->dst;
> unsigned int rotation = plane_state->rotation;
> @@ -828,8 +829,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> }
>
> if (!crtc_state->enable && !can_update_disabled) {
> - drm_dbg_kms(plane_state->crtc->dev,
> - "Cannot update plane of a disabled CRTC.\n");
> + drm_dbg_kms(dev, "Cannot update plane of a disabled CRTC.\n");
> return -EINVAL;
> }
>
> @@ -839,8 +839,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> if (hscale < 0 || vscale < 0) {
> - drm_dbg_kms(plane_state->crtc->dev,
> - "Invalid scaling of plane\n");
> + drm_dbg_kms(dev, "Invalid scaling of plane\n");
> drm_rect_debug_print("src: ", &plane_state->src, true);
> drm_rect_debug_print("dst: ", &plane_state->dst, false);
> return -ERANGE;
> @@ -864,8 +863,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> return 0;
>
> if (!can_position && !drm_rect_equals(dst, &clip)) {
> - drm_dbg_kms(plane_state->crtc->dev,
> - "Plane must cover entire CRTC\n");
> + drm_dbg_kms(dev, "Plane must cover entire CRTC\n");
> drm_rect_debug_print("dst: ", dst, false);
> drm_rect_debug_print("clip: ", &clip, false);
> return -EINVAL;
> --
> 2.33.0
>
>
>

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2021-12-02 09:44:53

by Claudio Suarez

[permalink] [raw]
Subject: Re: [PATCH] drm: fix error found in some cases after the patch d1af5cd86997

On Tue, Nov 30, 2021 at 09:38:11AM +0100, Daniel Vetter wrote:
> On Mon, Nov 29, 2021 at 08:27:45PM +0100, Claudio Suarez wrote:
> > The patch d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log
> > calls in drm core, files drm_a*.c") fails when the drm_device
> > cannot be found in the parameter plane_state. Fix it.
> >
> > Reported-by: kernel test robot <[email protected]>
> > Fixes: d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log calls in drm core, files drm_a*.c")
> > Signed-off-by: Claudio Suarez <[email protected]>
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 12 +++++-------
> > 1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index aef2fbd676e5..8bd4472d7949 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -312,7 +312,7 @@ update_connector_routing(struct drm_atomic_state *state,
> >
> > if (!new_connector_state->crtc) {
> > drm_dbg_atomic(connector->dev, "Disabling [CONNECTOR:%d:%s]\n",
> > - connector->base.id, connector->name);
> > + connector->base.id, connector->name);
>
> Can you pls split this checkpatch fix out?

Of course.

>
> >
> > set_best_encoder(state, new_connector_state, NULL);
> >
> > @@ -805,6 +805,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > bool can_update_disabled)
> > {
> > struct drm_framebuffer *fb = plane_state->fb;
> > + struct drm_device *dev = plane_state->plane ? plane_state->plane->dev : NULL;
>
> For real drivers plane_state->plane really should never be NULL, and
> looking at the test report we blow up in an selftest. I think the right
> fix here is to make the selftest more robust and also mock a drm_plane
> (with a NULL plane->dev pointer, that should be fine).
>
> Can you pls spin that and test it with the selftests enabled in the
> config?

You are right. I made this change in the test and the error was gone.
Output before and after follow.
The code in the kernel should be fixed though. Currently plane_state->crtc
is used. It should be plane_state->plane
I going to send two patches: one for the test and a v2 for the drm code.

Output before changing the test:
[ 38.161315][ T1] drm_mm: bottom-up fragmented insert of 10000 and 20000 insertions took 7047892 and 14424064 nsecs
[ 38.191061][ T1] drm_mm: top-down fragmented insert of 10000 and 20000 insertions took 7114502 and 14582794 nsecs
[ 40.260837][ T1] BUG: kernel NULL pointer dereference, address: 00000000
[ 40.261381][ T1] #PF: supervisor read access in kernel mode
[ 40.261845][ T1] #PF: error_code(0x0000) - not-present page
[ 40.262346][ T1] *pde = 00000000
[ 40.262692][ T1] Oops: 0000 [#1] PREEMPT SMP
[ 40.263061][ T1] CPU: 1 PID: 1 Comm: swapper/0 Tainted: G S W 5.16.0-rc2-00002-g58f3ee4dc1e9-dirty #1
[ 40.264036][ T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-2 04/01/2014
[ 40.264036][ T1] EIP: drm_atomic_helper_check_plane_state+0x15e/0x280
[ 40.264036][ T1] Code: 8b 45 d4 50 89 f0 e8 c1 0c 03 00 5a 80 7b 6c 00 74 6a 80 7d cc 00 75 64 8b 45 e4 39 43 5c 75 08 8b 45 ec 39 43 64 74 44 8b 03 <8b> 00 85 c0 74 03 8b 40 08 68 33 c2 70 c2 6a 04 50 e8 4c 14 04 00
[ 40.264036][ T1] EAX: 00000000 EBX: c036dce8 ECX: 08000000 EDX: 00000001

Output after:
[ 38.230609][ T1] drm_mm: bottom-up fragmented insert of 10000 and 20000 insertions took 7235202 and 14675224 nsecs
[ 38.259900][ T1] drm_mm: top-down fragmented insert of 10000 and 20000 insertions took 7078062 and 14642524 nsecs
[ 40.347207][ T1] random: get_random_bytes called from igt_dp_mst_sideband_msg_req_decode+0x25f/0x300 with crng_init=0
[ 40.347248][ T1] ACPI: bus type drm_connector registered
[ 40.350980][ T1] [drm] Initialized vgem 1.0.0 20120112 for vgem on minor 0
[ 40.351670][ T2] random: get_random_u32 called from copy_process+0x21c/0x1a00 with crng_init=0
[ 40.353310][ T1] [drm] Initialized vkms 1.0.0 20180514 for vkms on minor 1


BR
Claudio Suarez

>
> Thanks, Daniel
>
> > struct drm_rect *src = &plane_state->src;
> > struct drm_rect *dst = &plane_state->dst;
> > unsigned int rotation = plane_state->rotation;
> > @@ -828,8 +829,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > }
> >
> > if (!crtc_state->enable && !can_update_disabled) {
> > - drm_dbg_kms(plane_state->crtc->dev,
> > - "Cannot update plane of a disabled CRTC.\n");
> > + drm_dbg_kms(dev, "Cannot update plane of a disabled CRTC.\n");
> > return -EINVAL;
> > }
> >
> > @@ -839,8 +839,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> > vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> > if (hscale < 0 || vscale < 0) {
> > - drm_dbg_kms(plane_state->crtc->dev,
> > - "Invalid scaling of plane\n");
> > + drm_dbg_kms(dev, "Invalid scaling of plane\n");
> > drm_rect_debug_print("src: ", &plane_state->src, true);
> > drm_rect_debug_print("dst: ", &plane_state->dst, false);
> > return -ERANGE;
> > @@ -864,8 +863,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > return 0;
> >
> > if (!can_position && !drm_rect_equals(dst, &clip)) {
> > - drm_dbg_kms(plane_state->crtc->dev,
> > - "Plane must cover entire CRTC\n");
> > + drm_dbg_kms(dev, "Plane must cover entire CRTC\n");
> > drm_rect_debug_print("dst: ", dst, false);
> > drm_rect_debug_print("clip: ", &clip, false);
> > return -EINVAL;
> > --
> > 2.33.0
> >
> >
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



2021-12-02 09:50:04

by Claudio Suarez

[permalink] [raw]
Subject: [PATCH] mock a drm_plane in igt_check_plane_state to make the test more robust

igt_check_plane_state test crashes in drm_atomic_helper_check_plane_state
when trying to de-reference drm_plane_state->plane->dev
due to the lack of a struct drm_plane in the mock struct drm_plane_state.
Since drm_plane_state always should contain a plane, the mock also
needs a plane to be the test more robust and realistic. Add it.

Signed-off-by: Claudio Suarez <[email protected]>
---
drivers/gpu/drm/selftests/test-drm_plane_helper.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/selftests/test-drm_plane_helper.c b/drivers/gpu/drm/selftests/test-drm_plane_helper.c
index 0a9553f51796..ceebeede55ea 100644
--- a/drivers/gpu/drm/selftests/test-drm_plane_helper.c
+++ b/drivers/gpu/drm/selftests/test-drm_plane_helper.c
@@ -87,11 +87,15 @@ int igt_check_plane_state(void *ignored)
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC)
},
};
+ struct drm_plane plane = {
+ .dev = NULL
+ };
struct drm_framebuffer fb = {
.width = 2048,
.height = 2048
};
struct drm_plane_state plane_state = {
+ .plane = &plane,
.crtc = ZERO_SIZE_PTR,
.fb = &fb,
.rotation = DRM_MODE_ROTATE_0
--
2.33.0




2021-12-02 09:52:33

by Claudio Suarez

[permalink] [raw]
Subject: Re: [PATCH v2] drm: fix error found in some cases after the patch d1af5cd86997

The patch d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log
calls in drm core, files drm_a*.c") fails when the drm_device
cannot be found in the parameter plane_state->crtc.
Fix it using plane_state->plane.

Reported-by: kernel test robot <[email protected]>
Fixes: d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log calls in drm core, files drm_a*.c")
Signed-off-by: Claudio Suarez <[email protected]>
---
drivers/gpu/drm/drm_atomic_helper.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index aef2fbd676e5..a7a05e1e26bb 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -828,8 +828,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
}

if (!crtc_state->enable && !can_update_disabled) {
- drm_dbg_kms(plane_state->crtc->dev,
- "Cannot update plane of a disabled CRTC.\n");
+ drm_dbg_kms(plane_state->plane->dev,
+ "Cannot update plane of a disabled CRTC.\n");
return -EINVAL;
}

@@ -839,8 +839,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
if (hscale < 0 || vscale < 0) {
- drm_dbg_kms(plane_state->crtc->dev,
- "Invalid scaling of plane\n");
+ drm_dbg_kms(plane_state->plane->dev,
+ "Invalid scaling of plane\n");
drm_rect_debug_print("src: ", &plane_state->src, true);
drm_rect_debug_print("dst: ", &plane_state->dst, false);
return -ERANGE;
@@ -864,8 +864,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
return 0;

if (!can_position && !drm_rect_equals(dst, &clip)) {
- drm_dbg_kms(plane_state->crtc->dev,
- "Plane must cover entire CRTC\n");
+ drm_dbg_kms(plane_state->plane->dev,
+ "Plane must cover entire CRTC\n");
drm_rect_debug_print("dst: ", dst, false);
drm_rect_debug_print("clip: ", &clip, false);
return -EINVAL;
--
2.33.0




2021-12-20 09:18:45

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH v2] drm: fix error found in some cases after the patch d1af5cd86997

On Thu, Dec 02, 2021 at 10:51:12AM +0100, Claudio Suarez wrote:
> The patch d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log
> calls in drm core, files drm_a*.c") fails when the drm_device
> cannot be found in the parameter plane_state->crtc.
> Fix it using plane_state->plane.
>
> Reported-by: kernel test robot <[email protected]>
> Fixes: d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log calls in drm core, files drm_a*.c")
> Signed-off-by: Claudio Suarez <[email protected]>

Sorry I missed these two patches, but both applied now, thanks.
-Daniel

> ---
> drivers/gpu/drm/drm_atomic_helper.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index aef2fbd676e5..a7a05e1e26bb 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -828,8 +828,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> }
>
> if (!crtc_state->enable && !can_update_disabled) {
> - drm_dbg_kms(plane_state->crtc->dev,
> - "Cannot update plane of a disabled CRTC.\n");
> + drm_dbg_kms(plane_state->plane->dev,
> + "Cannot update plane of a disabled CRTC.\n");
> return -EINVAL;
> }
>
> @@ -839,8 +839,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> if (hscale < 0 || vscale < 0) {
> - drm_dbg_kms(plane_state->crtc->dev,
> - "Invalid scaling of plane\n");
> + drm_dbg_kms(plane_state->plane->dev,
> + "Invalid scaling of plane\n");
> drm_rect_debug_print("src: ", &plane_state->src, true);
> drm_rect_debug_print("dst: ", &plane_state->dst, false);
> return -ERANGE;
> @@ -864,8 +864,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> return 0;
>
> if (!can_position && !drm_rect_equals(dst, &clip)) {
> - drm_dbg_kms(plane_state->crtc->dev,
> - "Plane must cover entire CRTC\n");
> + drm_dbg_kms(plane_state->plane->dev,
> + "Plane must cover entire CRTC\n");
> drm_rect_debug_print("dst: ", dst, false);
> drm_rect_debug_print("clip: ", &clip, false);
> return -EINVAL;
> --
> 2.33.0
>
>
>

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2021-12-20 17:11:43

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH v2] drm: fix error found in some cases after the patch d1af5cd86997

On Mon, Dec 20, 2021 at 10:18:38AM +0100, Daniel Vetter wrote:
> On Thu, Dec 02, 2021 at 10:51:12AM +0100, Claudio Suarez wrote:
> > The patch d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log
> > calls in drm core, files drm_a*.c") fails when the drm_device
> > cannot be found in the parameter plane_state->crtc.
> > Fix it using plane_state->plane.
> >
> > Reported-by: kernel test robot <[email protected]>
> > Fixes: d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log calls in drm core, files drm_a*.c")

My scrip complained about the fixes line, so I fixed it. I guess you've
used the sha1 from your tree, not from upstream? Please always use
upstream sha1 when referencing commits.

Anyway patches are now pushed.

Cheers, Daniel

> > Signed-off-by: Claudio Suarez <[email protected]>
>
> Sorry I missed these two patches, but both applied now, thanks.
> -Daniel
>
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index aef2fbd676e5..a7a05e1e26bb 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -828,8 +828,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > }
> >
> > if (!crtc_state->enable && !can_update_disabled) {
> > - drm_dbg_kms(plane_state->crtc->dev,
> > - "Cannot update plane of a disabled CRTC.\n");
> > + drm_dbg_kms(plane_state->plane->dev,
> > + "Cannot update plane of a disabled CRTC.\n");
> > return -EINVAL;
> > }
> >
> > @@ -839,8 +839,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> > vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> > if (hscale < 0 || vscale < 0) {
> > - drm_dbg_kms(plane_state->crtc->dev,
> > - "Invalid scaling of plane\n");
> > + drm_dbg_kms(plane_state->plane->dev,
> > + "Invalid scaling of plane\n");
> > drm_rect_debug_print("src: ", &plane_state->src, true);
> > drm_rect_debug_print("dst: ", &plane_state->dst, false);
> > return -ERANGE;
> > @@ -864,8 +864,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > return 0;
> >
> > if (!can_position && !drm_rect_equals(dst, &clip)) {
> > - drm_dbg_kms(plane_state->crtc->dev,
> > - "Plane must cover entire CRTC\n");
> > + drm_dbg_kms(plane_state->plane->dev,
> > + "Plane must cover entire CRTC\n");
> > drm_rect_debug_print("dst: ", dst, false);
> > drm_rect_debug_print("clip: ", &clip, false);
> > return -EINVAL;
> > --
> > 2.33.0
> >
> >
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2021-12-21 22:55:39

by Claudio Suarez

[permalink] [raw]
Subject: Re: [PATCH v2] drm: fix error found in some cases after the patch d1af5cd86997

On Mon, Dec 20, 2021 at 06:11:31PM +0100, Daniel Vetter wrote:
> On Mon, Dec 20, 2021 at 10:18:38AM +0100, Daniel Vetter wrote:
> > On Thu, Dec 02, 2021 at 10:51:12AM +0100, Claudio Suarez wrote:
> > > The patch d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log
> > > calls in drm core, files drm_a*.c") fails when the drm_device
> > > cannot be found in the parameter plane_state->crtc.
> > > Fix it using plane_state->plane.
> > >
> > > Reported-by: kernel test robot <[email protected]>
> > > Fixes: d1af5cd86997 ("drm: get rid of DRM_DEBUG_* log calls in drm core, files drm_a*.c")
>
> My scrip complained about the fixes line, so I fixed it. I guess you've
> used the sha1 from your tree, not from upstream?

Yes, my bad, sorry. Thanks for the advice.

> Please always use
> upstream sha1 when referencing commits.
>
> Anyway patches are now pushed.

Thank you!

Best regards.
Claudio Suarez.