Re: [PATCH] nouveau: fix OpenFirmware support

[Ilia Mirkin]

On Sat, Oct 10, 2015 at 7:45 PM, Laurent Vivier <laurent@xxxxxxxxx> wrote:
>
>
> Le 10/10/2015 21:56, Ilia Mirkin a Ãcrit :
>> On Sat, Oct 10, 2015 at 3:29 PM, Laurent Vivier <laurent@xxxxxxxxx> wrote:
>>>
>>>
>>> Le 10/10/2015 20:41, Ilia Mirkin a Ãcrit :
>>>> Hi Laurent,
>>>>
>>>> On Sat, Oct 10, 2015 at 9:27 AM, Laurent Vivier <laurent@xxxxxxxxx> wrote:
>>>>> On PowerMac G5 (and I think on all OpenFirmware platforms), nvbios_pcirTp()
>>>>> returns NULL. But in fact the OpenFirmware has given us the size
>>>>> we can store in image->size.
>>>>>
>>>>> This size is stored in bios->size by of_init() as there is no way
>>>>> to retrieve it otherwise. And as we know the size, copy all data
>>>>> to bios->data.
>>>>>
>>>>> Tested on PowerMac G5 with 64bit kernel and a NV43 card (GeForce 6600 LE).
>>>>
>>>> Can you give this patch a shot instead?
>>>>
>>>> http://cgit.freedesktop.org/~darktama/nouveau/commit/?id=794a63cc75eada9ad6b2a0275c1c8c4d3522864f
>>>
>>> Well, I think mine is also a good solution and it is much more simple.
>>> ;)
>>>
>>> ... because it is useless to add a size() function if we can directly
>>> copy the content/size of the bios in bios->data and bios->size.
>>> We can do that because we have the size of the property, which is not
>>> the case when we discover the BIOS directly from a PCI ROM or from ACPI
>>> (this is why we need a shadow, I think).
>>
>> I'll let Ben rule on this.
>>
>>>
>>> For pcir part, I think we can just ignore the result and take the size
>>> from bios->size, as in the case of non openfirmware bios->size will be 4
>>> (we have only shadowed the first word to read the id, 0xaa55) and then
>>> the checksum and others ID searches will fail. So I think the checksum
>>> should not be ignored.
>>
>> Non-OF will still end up with a NVDA,BMP file? That seems surprising.
>> My understanding is that if OF has it, it should be used. The problem
>> is that e.g. on my GPU I have a perfectly valid PCI ROM, whose
>> checksum matches and everything, but it contains who-knows-what apple
>> happened to leave in there. So I still want OF. Ignoring checksum
>> failures allows nouveau to always select the OF vbios.
>>
>>>
>>> I've tried to restore behavior before commit:
>>>
>>> 7af4dec drm/nouveau/bios: use size/type from pci data structure
>>>
>>> and commit:
>>>
>>> ad4a362 drm/nouveau/bios: split out shadow methods
>>>
>>> Originally, openfirmware content was copied directly into bios->data:
>>
>> Yeah, but then the whole interface was redone.
>>
>>>
>>> 77145f1 drm/nouveau: port remainder of drm code, and rip out compat layer
>>>
>>>> This resolved my issues on a PPC G5 + NV34. I think mine ran into a
>>>> few additional problems that you didn't see -- no PCIR header and
>>>> invalid checksum.
>>>
>>> I have no PCIR header too.
>>
>> Er right. I realized that shortly after I sent the email. However my
>> bios isn't even 0x1000 in size, so the read would fail due to not
>> enough length. (It's also an odd number in size, and your patch chops
>> off the last few bytes.) The read could, of course, be reduced in
>> size, but that whole logic is to deal with multiple parts in a vbios,
>> which on GM20x contain some necessary blobs. I wasn't sure where the
>> 0x1000 came from or whether it was significant.
>>
>>>
>>> Could you send me the content of the file "NVDA,BMP" you can find
>>> somewhere under /proc/device-tree/ ?
>>
>> http://filebin.ca/2Ib4SdDOAQqC/nv34-vbios.rom
>>
>> Note that it's a 2404 byte file as uploaded, but that was from an
>> attempt to do something silly -- in reality it's 2403 bytes.
>>
>>>
>>> Could you try my patch on your system, please ?
>>
>> My G5 is off for now, and the time I do spend with it goes towards
>> working out mesa issues (it should kinda-sorta work with Mesa 11.0.3
>> again btw). If I have time, I'll try it out.
>
> I've checked on my second PowerMac G5 which seems to be the same as
> yours (PowerMac7,3).
>
> It doesn't work but not because of the checksum:
>
> [  140.410535] nouveau 0000:f0:10.0: NVIDIA NV34 (034100a2)
> [  140.476781] nouveau 0000:f0:10.0: bios: version 04.34.20.19.00
> [  140.476993] nouveau 0000:f0:10.0: bios: DCB table not found
> [  140.477186] nouveau 0000:f0:10.0: bios: DCB table not found
> [  140.477283] nouveau 0000:f0:10.0: bios: DCB table not found
> [  140.477289] nouveau 0000:f0:10.0: bios: DCB table not found
> [  140.477664] nouveau 0000:f0:10.0: bios: DCB table not found
> [  140.480949] nouveau 0000:f0:10.0: devinit: 0x1a08[ ]: unknown opcode 0x10
> [  140.480962] nouveau 0000:f0:10.0: preinit failed with -22
> [  140.480978] nouveau: DRM:dddddddd:00000080: init failed with -22
> [  140.487980] nouveau: probe of 0000:f0:10.0 failed with error -22
>

Because the checksum fails on OF but passes on the PCI ROM. But the
PCI ROM contains silliness (in fact I'm fairly sure we should just
remove that method, it never provides anything useful) which passes
the checksum test.

Try it with my patch from Ben's tree.

  -ilia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/