[PATCH] nouveau: fix OpenFirmware support

[PATCH] nouveau: fix OpenFirmware support

[Laurent Vivier]

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).

Before:

    nouveau 0000:0a:00.0: NVIDIA NV43 (043200a4)
    u3msi: allocated virq 0x19 (hw 0x8) addr 0xf8004080
    nouveau 0000:0a:00.0: Invalid ROM contents
    nouveau 0000:0a:00.0: bios: unable to locate usable image
    nouveau 0000:0a:00.0: bios ctor failed, -22
    nouveau: probe of 0000:0a:00.0 failed with error -22

After:

    nouveau 0000:0a:00.0: NVIDIA NV43 (043200a4)
    u3msi: allocated virq 0x19 (hw 0x8) addr 0xf8004080
    nouveau 0000:0a:00.0: bios: version 05.43.02.75.00
    nouveau 0000:0a:00.0: fb: 128 MiB DDR1
    nouveau 0000:0a:00.0: Using 32-bit DMA via iommu
    [TTM] Zone  kernel: Available graphics memory: 5610528 kiB
    [TTM] Zone   dma32: Available graphics memory: 2097152 kiB
    [TTM] Initializing pool allocator
    [TTM] Initializing DMA pool allocator
    nouveau 0000:0a:00.0: DRM: VRAM: 124 MiB
    nouveau 0000:0a:00.0: DRM: GART: 512 MiB
    nouveau 0000:0a:00.0: DRM: TMDS table version 1.1
    nouveau 0000:0a:00.0: DRM: DCB version 3.0
    nouveau 0000:0a:00.0: DRM: DCB outp 00: 01000100 00000028
    nouveau 0000:0a:00.0: DRM: DCB outp 01: 03000102 00000000
    nouveau 0000:0a:00.0: DRM: DCB outp 02: 04011210 00000028
    nouveau 0000:0a:00.0: DRM: DCB outp 03: 02111212 02000100
    nouveau 0000:0a:00.0: DRM: DCB outp 04: 02011211 0020c070
    nouveau 0000:0a:00.0: DRM: DCB conn 00: 1030
    nouveau 0000:0a:00.0: DRM: DCB conn 01: 2130
    [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
    [drm] Driver supports precise vblank timestamp query.
    nouveau 0000:0a:00.0: DRM: 0x14C5: Parsing digital output script table
    nouveau 0000:0a:00.0: DRM: MM: using M2MF for buffer copies
    nouveau 0000:0a:00.0: DRM: Setting dpms mode 3 on TV encoder (output 4)
    nouveau 0000:0a:00.0: DRM: allocated 1680x1050 fb: 0x30000, bo c00000000399d800
    nouveau 0000:0a:00.0: DRM: 0x14C5: Parsing digital output script table
    Console: switching to colour frame buffer device 210x65
    nouveau 0000:0a:00.0: fb0: nouveaufb frame buffer device
    [drm] Initialized nouveau 1.3.0 20120801 for 0000:0a:00.0 on minor 0

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/bios/image.c    | 10 ++++++++--
 drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c |  8 ++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/image.c b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/image.c
index 74b14cf..17ba0c726 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/image.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/image.c
@@ -47,8 +47,14 @@ nvbios_imagen(struct nvkm_bios *bios, struct nvbios_image *image)
 		return false;
 	}
 
-	if (!(data = nvbios_pcirTp(bios, image->base, &ver, &hdr, &pcir)))
-		return false;
+	data = nvbios_pcirTp(bios, image->base, &ver, &hdr, &pcir);
+	if (!data) {
+		image->size = bios->size;
+		image->type = 0x00;
+		image->last = true;
+
+		return true;
+	}
 	image->size = pcir.image_size;
 	image->type = pcir.image_type;
 	image->last = pcir.last;
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
index bd60d7d..d4c8801 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
@@ -34,7 +34,6 @@ of_read(void *data, u32 offset, u32 length, struct nvkm_bios *bios)
 {
 	struct priv *priv = data;
 	if (offset + length <= priv->size) {
-		memcpy_fromio(bios->data + offset, priv->data + offset, length);
 		return length;
 	}
 	return 0;
@@ -50,8 +49,13 @@ of_init(struct nvkm_bios *bios, const char *name)
 		return ERR_PTR(-ENODEV);
 	if (!(priv = kzalloc(sizeof(*priv), GFP_KERNEL)))
 		return ERR_PTR(-ENOMEM);
-	if ((priv->data = of_get_property(dn, "NVDA,BMP", &priv->size)))
+	priv->data = of_get_property(dn, "NVDA,BMP", &priv->size);
+	if (priv->data) {
+		bios->size = (priv->size + 3) & ~3;
+		bios->data = kmalloc(bios->size, GFP_KERNEL);
+		memcpy(bios->data, priv->data, priv->size);
 		return priv;
+	}
 	kfree(priv);
 	return ERR_PTR(-EINVAL);
 }
-- 
2.4.3

Re: [PATCH] nouveau: fix OpenFirmware support

[Ilia Mirkin]

Hi Laurent,

On Sat, Oct 10, 2015 at 9:27 AM, Laurent Vivier <laurent@vivier.eu> 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

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.

  -ilia

Re: [PATCH] nouveau: fix OpenFirmware support

[Laurent Vivier]

Le 10/10/2015 20:41, Ilia Mirkin a écrit :
> Hi Laurent,
> 
> On Sat, Oct 10, 2015 at 9:27 AM, Laurent Vivier <laurent@vivier.eu> 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).

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.

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:

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.

Could you send me the content of the file "NVDA,BMP" you can find
somewhere under /proc/device-tree/ ?

Could you try my patch on your system, please ?

Laurent

Re: [PATCH] nouveau: fix OpenFirmware support

[Ilia Mirkin]

On Sat, Oct 10, 2015 at 3:29 PM, Laurent Vivier <laurent@vivier.eu> 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@vivier.eu> 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

Re: [PATCH] nouveau: fix OpenFirmware support

[Laurent Vivier]

Le 10/10/2015 21:56, Ilia Mirkin a écrit :
> On Sat, Oct 10, 2015 at 3:29 PM, Laurent Vivier <laurent@vivier.eu> 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@vivier.eu> 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

Laurent

Re: [PATCH] nouveau: fix OpenFirmware support

[Ilia Mirkin]

On Sat, Oct 10, 2015 at 7:45 PM, Laurent Vivier <laurent@vivier.eu> wrote:
>
>
> Le 10/10/2015 21:56, Ilia Mirkin a écrit :
>> On Sat, Oct 10, 2015 at 3:29 PM, Laurent Vivier <laurent@vivier.eu> 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@vivier.eu> 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

Re: [PATCH] nouveau: fix OpenFirmware support

[Laurent Vivier]

Le 11/10/2015 01:49, Ilia Mirkin a écrit :
> On Sat, Oct 10, 2015 at 7:45 PM, Laurent Vivier <laurent@vivier.eu> wrote:
>>
>>
>> Le 10/10/2015 21:56, Ilia Mirkin a écrit :
>>> On Sat, Oct 10, 2015 at 3:29 PM, Laurent Vivier <laurent@vivier.eu> 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@vivier.eu> 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.

On NV34, yours works fine.

Mine fails because:
1- we try to fetch 0x1000 bytes whereas BIOS is smaller,
2- we compute checksum whereas it is no valid

I'm going to send a v2 of my patch which fixes that, not because I think
it will be better, just because I like to finish what I start (and to share)

We can fix (1) by reducing the size of the shadow_fetch from 0x1000 to
0x200 (there is no reason to keep 0x1000 if we know an existing smaller
size), we can fix (2) by setting the image->type from nvbios_of to
disable the checksum.

Thank you for your help,
Laurent