Re: [PATCH] nouveau: fix OpenFirmware support

[Ilia Mirkin]

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.

  -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/