[v2,2/8] video: Provide screen_info_get_pci_dev() to find screen_info's PCI device
Thomas Zimmermann
tzimmermann at suse.de
Mon Feb 5 12:32:40 UTC 2024
Hi
Am 05.02.24 um 11:05 schrieb Sui Jingfeng:
> Hi,
>
> On 2024/2/5 16:17, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 02.02.24 um 18:03 schrieb Sui Jingfeng:
>>> Hi,
>>>
>>>
>>> On 2024/2/2 19:58, Thomas Zimmermann wrote:
>>>> +
>>>> +/**
>>>> + * screen_info_pci_dev() - Return PCI parent device that contains
>>>> screen_info's framebuffer
>>>> + * @si: the screen_info
>>>> + *
>>>> + * Returns:
>>>> + * The screen_info's parent device on success, or NULL otherwise.
>>>> + */
>>>> +struct pci_dev *screen_info_pci_dev(const struct screen_info *si)
>>>> +{
>>>> + struct resource res[SCREEN_INFO_MAX_RESOURCES];
>>>> + ssize_t i, numres;
>>>> +
>>>> + numres = screen_info_resources(si, res, ARRAY_SIZE(res));
>>>> + if (numres < 0)
>>>> + return ERR_PTR(numres);
>>>
>>>
>>> Please return NULL at here, otherwise we have to use the IS_ERR or
>>> IS_ERR_OR_NULL()
>>> in the caller function to check the returned value. Meanwhile, I
>>> noticed that you
>>> didn't actually call IS_ERR() in the sysfb_parent_dev() function
>>> (introduced by the
>>> 3/8 patch), so I think we probably should return NULL at here.
>>>
>>> Please also consider that the comments of this function says that it
>>> return NULL on
>>> the otherwise cases.
>>
>> Right. The idea is to return NULL is there is no parent device.
>
>
> return NULL is more easier and clear, it stand for "None" or "don't exist".
> There is another reason that I want to tell you:
>
> Some systems which don't have a good UEFI firmware support for uncommon
> GPUs.
> the word "uncommon" means "not very popular GPU" or "extremely new GPU" or
> "just refer to the GPUs that UEFI firmware don't know(recognize) about"
>
> On such cases, there is no firmware framebuffer support. I means it is
> possible
> that screen_info_resources() return -EINVAL because of not support yet.
> Then,
> the screen_info_pci_dev(si) returns ERR_PTR(-EINVAL) and
> sysfb_pci_dev_is_enabled()
> will take this error code as a pointer and de-reference it, cause the
> following
> problem:
Right, that's part of the bug you already pointed out. As I said, I need
to review the callers of screen_info_pci_dev() and fix them.
But returning an errno pointer is useful in cases where a possible
parent device would have been detected, but something did not work out.
For example, screen_info_resources() does not support all VIDEO_TYPE_
constants. In such a case, it's better to tell the caller about the
problem than to silently return NULL.
Best regards
Thomas
>
> And even the x86-64 motherboard will not likely support all GPU(for
> example the one
> with a old UEFI BIOS). And for an example, The intel Xe is the
> "extremely new GPU".
>
>
> [ 5.031966] CPU 4 Unable to handle kernel paging request at virtual
> address 000000000000081a, era == 900000000329b448, ra == 900000000329b440
> [ 5.044587] Oops[#1]:
> [ 5.046837] CPU: 4 PID: 1 Comm: swapper/0 Not tainted 6.8.0-rc1+ #7
> [ 5.053062] Hardware name: Loongson
> Loongson-3A6000-HV-7A2000-XA61200/Loongson-3A6000-HV-7A2000-XA61200,
> BIOS Loongson-UDK2018-V4.0.05636-stable202311 12/
> [ 5.066803] pc 900000000329b448 ra 900000000329b440 tp
> 90000001000d0000 sp 90000001000d3d40
> [ 5.075100] a0 ffffffffffffffea a1 90000001000d3c38 a2
> 0000000000000003 a3 9000000003867ce8
> [ 5.083398] a4 9000000003867ce0 a5 90000001000d3a80 a6
> 0000000000000001 a7 0000000000000001
> [ 5.091695] t0 ac81f55e34713962 t1 ac81f55e34713962 t2
> 0000000000000000 t3 0000000000000001
> [ 5.099992] t4 0000000000000004 t5 0000000000000000 t6
> 0000000000000030 t7 0000000000000000
> [ 5.108290] t8 00000000000070b1 u0 0000000000000000 s9
> 0000000000000008 s0 9000000003d58b48
> [ 5.116587] s1 9000000003c0b4a8 s2 9000000003787000 s3
> 9000000003778000 s4 90000000032c0578
> [ 5.124884] s5 ffffffffffffffea s6 90000000032c0560 s7
> 90000000032df900 s8 ffffffffccccc000
> [ 5.133182] ra: 900000000329b440 sysfb_init+0x80/0x1f0
> [ 5.138545] ERA: 900000000329b448 sysfb_init+0x88/0x1f0
> [ 5.143905] CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
> [ 5.150048] PRMD: 00000004 (PPLV0 +PIE -PWE)
> [ 5.154373] EUEN: 00000000 (-FPE -SXE -ASXE -BTE)
> [ 5.159131] ECFG: 00071c1c (LIE=2-4,10-12 VS=7)
> [ 5.163717] ESTAT: 00010000 [PIL] (IS= ECode=1 EsubCode=0)
> [ 5.169164] BADV: 000000000000081a
> [ 5.172623] PRID: 0014d000 (Loongson-64bit, Loongson-3A6000-HV)
> [ 5.178587] Modules linked in:
> [ 5.181614] Process swapper/0 (pid: 1, threadinfo=(____ptrval____),
> task=(____ptrval____))
> [ 5.189827] Stack : 0000000000000000 0000000000000000
> 0000000000000000 0000000000000000
> [ 5.197782] 0000000000000000 ac81f55e34713962
> 90000000032c0000 9000000003778000
> [ 5.205736] 90000000032c0578 0000000000000000
> 900000000329b3c0 9000000003c54000
> [ 5.213691] 90000001000d3db8 9000000002260154
> 0000000000000006 0000000000000000
> [ 5.221645] 0000000000000000 0000000000000000
> 0000000000000000 0000000000000000
> [ 5.229599] 0000000000000000 0000000000000000
> 0000000000000000 ac81f55e34713962
> [ 5.237553] 90000000037468f8 90000000037468f8
> 90000000032c0578 90000000036a7658
> [ 5.245508] 0000000000000006 9000000100041e00
> 0000000000000a55 9000000003260ff4
> [ 5.251549] ata3: SATA link down (SStatus 0 SControl 300)
> [ 5.253463] 0000000000000000 90000000032600b0
> 0000000000000000 0000000000000000
> [ 5.266777] 0000000000000000 0000000000000000
> 0000000000000000 0000000000000000
> [ 5.274731] ...
> [ 5.277154] Call Trace:
> [ 5.277155] [<900000000329b448>] sysfb_init+0x88/0x1f0
> [ 5.284678] [<9000000002260154>] do_one_initcall+0x78/0x1cc
> [ 5.290213] [<9000000003260ff4>] kernel_init_freeable+0x228/0x298
> [ 5.296267] [<900000000324d104>] kernel_init+0x20/0x110
> [ 5.301455] [<90000000022611e8>] ret_from_kernel_thread+0xc/0xa4
> [ 5.307421]
> [ 5.308892] Code: 561667fe 0015009c 40007080 <2408308c> 29403860
> 02c2e09b 0040818c 6400180c 1a007d45
> [ 5.318579]
> [ 5.320053] ---[ end trace 0000000000000000 ]---
> [ 5.324640] Kernel panic - not syncing: Attempted to kill init!
> exitcode=0x0000000b
> [ 5.332247] Kernel relocated by 0x2040000
> [ 5.336226] .text @ 0x9000000002240000
> [ 5.340031] .data @ 0x90000000032f0000
> [ 5.343835] .bss @ 0x9000000003c3f200
> [ 5.347640] ---[ end Kernel panic - not syncing: Attempted to kill
> init! exitcode=0x0000000b ]---
>
>
>> Best regards
>> Thomas
>>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20240205/d9aa22c4/attachment.sig>
More information about the dri-devel
mailing list