Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1244146 > unrolled thread

[PATCH v2] nouveau: fix OpenFirmware support

Started byLaurent Vivier <laurent@vivier.eu>
First post2015-10-11 16:50 +0200
Last post2015-10-14 17:50 +0200
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] nouveau: fix OpenFirmware support Laurent Vivier <laurent@vivier.eu> - 2015-10-11 16:50 +0200
    Re: [PATCH v2] nouveau: fix OpenFirmware support Olaf Hering <olaf@aepfle.de> - 2015-10-14 16:30 +0200
      Re: [PATCH v2] nouveau: fix OpenFirmware support Ilia Mirkin <imirkin@alum.mit.edu> - 2015-10-14 17:00 +0200
        Re: [PATCH v2] nouveau: fix OpenFirmware support Olaf Hering <olaf@aepfle.de> - 2015-10-14 17:30 +0200
          Re: [PATCH v2] nouveau: fix OpenFirmware support Ilia Mirkin <imirkin@alum.mit.edu> - 2015-10-14 17:50 +0200
        Re: [PATCH v2] nouveau: fix OpenFirmware support Laurent Vivier <laurent@vivier.eu> - 2015-10-14 17:30 +0200
          Re: [PATCH v2] nouveau: fix OpenFirmware support Ilia Mirkin <imirkin@alum.mit.edu> - 2015-10-14 17:50 +0200

#1244146 — [PATCH v2] nouveau: fix OpenFirmware support

FromLaurent Vivier <laurent@vivier.eu>
Date2015-10-11 16:50 +0200
Subject[PATCH v2] nouveau: fix OpenFirmware support
Message-ID<qiq6K-2Xq-3@gated-at.bofh.it>
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.

The NV34 has a small image and an invalid checksum, we manage this by
changing the size of the header we try to fetch (reduce from 4096 to 1024),
and by setting the image->type from nvbios_of->init(). A type of "1"
avoid to compute the checksum.

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),
and a NV34 (GeForce FX 5200 Ultra).

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

CC: imirkin@alum.mit.edu
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
v2: manage NV34: fetch a smaller header size to avoid error,
                 disable checksum from nvbios_of->init()

 drivers/gpu/drm/nouveau/include/nvkm/subdev/bios.h  |  1 +
 drivers/gpu/drm/nouveau/nvkm/subdev/bios/image.c    | 13 +++++++++++--
 drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c   |  2 +-
 drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c | 12 ++++++++++--
 4 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios.h
index e39a1fe..785342d 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios.h
@@ -6,6 +6,7 @@ struct nvkm_bios {
 	struct nvkm_subdev subdev;
 	u32 size;
 	u8 *data;
+	u8 type;
 
 	u32 bmp_offset;
 	u32 bit_offset;
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/image.c b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/image.c
index 74b14cf..25588f4 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/image.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/image.c
@@ -47,8 +47,17 @@ 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) {
+		/* in the case of openfirmware, size, data and type have
+		 * already been set and known from the device tree
+		 */
+		image->size = bios->size;
+		image->type = bios->type;
+		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/shadow.c b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
index 792f017..edc4f23 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
@@ -55,7 +55,7 @@ shadow_image(struct nvkm_bios *bios, int idx, u32 offset, struct shadow *mthd)
 	struct nvbios_image image;
 	int score = 1;
 
-	if (!shadow_fetch(bios, mthd, offset + 0x1000)) {
+	if (!shadow_fetch(bios, mthd, offset + 0x400)) {
 		nvkm_debug(subdev, "%08x: header fetch failed\n", offset);
 		return 0;
 	}
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
index bd60d7d..98fde60 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,17 @@ 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) {
+		/* NV34 has an invalid checksum,
+		 * setting type to 1 allows to ignore the checksum
+		 */
+		bios->type = bios->subdev.device->card_type == NV_30;
+		bios->size = (priv->size + 3) & ~3;
+		bios->data = kzalloc(bios->size, GFP_KERNEL);
+		memcpy(bios->data, priv->data, priv->size);
 		return priv;
+	}
 	kfree(priv);
 	return ERR_PTR(-EINVAL);
 }
-- 
2.4.3

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

[toc] | [next] | [standalone]

#1246826

FromOlaf Hering <olaf@aepfle.de>
Date2015-10-14 16:30 +0200
Message-ID<qjve3-17o-35@gated-at.bofh.it>
In reply to#1244146
On Sun, Oct 11, Laurent Vivier 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.

> CC: imirkin@alum.mit.edu
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>

Tested-by: Olaf Hering <olaf@aepfle.de>

Broken since a while already...
https://lists.ozlabs.org/pipermail/linuxppc-dev/2015-March/126737.html

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

[toc] | [prev] | [next] | [standalone]

#1246865

FromIlia Mirkin <imirkin@alum.mit.edu>
Date2015-10-14 17:00 +0200
Message-ID<qjvH4-1Iz-27@gated-at.bofh.it>
In reply to#1246826
On Wed, Oct 14, 2015 at 10:20 AM, Olaf Hering <olaf@aepfle.de> wrote:
> On Sun, Oct 11, Laurent Vivier 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.
>
>> CC: imirkin@alum.mit.edu
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>
> Tested-by: Olaf Hering <olaf@aepfle.de>
>
> Broken since a while already...
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2015-March/126737.html

FTR, looks like my version is upstream now:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=25d295882a1adfcdaaad85369289677b87c7c8f0

I agree that my of_size thing was a bit of a hack, if you want to redo
it by removing of_size/of_read from shadowof.c and copying it in at
init time (also a hack, IMHO), I certainly wouldn't object. As before,
it's Ben's call though.

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

[toc] | [prev] | [next] | [standalone]

#1246891

FromOlaf Hering <olaf@aepfle.de>
Date2015-10-14 17:30 +0200
Message-ID<qjwa6-2yi-9@gated-at.bofh.it>
In reply to#1246865
On Wed, Oct 14, Ilia Mirkin wrote:

> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=25d295882a1adfcdaaad85369289677b87c7c8f0

Thanks.
Almost perfect, lacks a stable tag.

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

[toc] | [prev] | [next] | [standalone]

#1246920

FromIlia Mirkin <imirkin@alum.mit.edu>
Date2015-10-14 17:50 +0200
Message-ID<qjwtr-2XK-9@gated-at.bofh.it>
In reply to#1246891
On Wed, Oct 14, 2015 at 11:20 AM, Olaf Hering <olaf@aepfle.de> wrote:
> On Wed, Oct 14, Ilia Mirkin wrote:
>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=25d295882a1adfcdaaad85369289677b87c7c8f0
>
> Thanks.
> Almost perfect, lacks a stable tag.

Code shifted around a lot, I believe. One could create a separate
patch to be backported which would be rather similar, but... meh. Feel
free to do the backport :)

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

[toc] | [prev] | [next] | [standalone]

#1246899

FromLaurent Vivier <laurent@vivier.eu>
Date2015-10-14 17:30 +0200
Message-ID<qjwa7-2yi-41@gated-at.bofh.it>
In reply to#1246865

Le 14/10/2015 16:52, Ilia Mirkin a écrit :
> On Wed, Oct 14, 2015 at 10:20 AM, Olaf Hering <olaf@aepfle.de> wrote:
>> On Sun, Oct 11, Laurent Vivier 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.
>>
>>> CC: imirkin@alum.mit.edu
>>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>>
>> Tested-by: Olaf Hering <olaf@aepfle.de>
>>
>> Broken since a while already...
>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2015-March/126737.html
> 
> FTR, looks like my version is upstream now:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=25d295882a1adfcdaaad85369289677b87c7c8f0
> 
> I agree that my of_size thing was a bit of a hack, if you want to redo
> it by removing of_size/of_read from shadowof.c and copying it in at
> init time (also a hack, IMHO), I certainly wouldn't object. As before,
> it's Ben's call though.

It's nice to have a fix upstream.

I'll send a patch to remove the of_size.

Rereading your patch and mine, I think we don't need no_pcir, we can do
instead:

	if (!nvbios_rd16(bios, base + 0x18)) {
		/* no PCIr */
                image.base = 0;
                image.type = 0;
                image.size = bios->size;
                image.last = 1;
	} else {
		if (!shadow_fetch(bios, mthd, offset + 0x1000)) {

...
What is you opinion ?

Laurent

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

[toc] | [prev] | [next] | [standalone]

#1246925

FromIlia Mirkin <imirkin@alum.mit.edu>
Date2015-10-14 17:50 +0200
Message-ID<qjwtr-2XK-13@gated-at.bofh.it>
In reply to#1246899
On Wed, Oct 14, 2015 at 11:20 AM, Laurent Vivier <laurent@vivier.eu> wrote:
>
>
> Le 14/10/2015 16:52, Ilia Mirkin a écrit :
>> On Wed, Oct 14, 2015 at 10:20 AM, Olaf Hering <olaf@aepfle.de> wrote:
>>> On Sun, Oct 11, Laurent Vivier 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.
>>>
>>>> CC: imirkin@alum.mit.edu
>>>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>>>
>>> Tested-by: Olaf Hering <olaf@aepfle.de>
>>>
>>> Broken since a while already...
>>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2015-March/126737.html
>>
>> FTR, looks like my version is upstream now:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=25d295882a1adfcdaaad85369289677b87c7c8f0
>>
>> I agree that my of_size thing was a bit of a hack, if you want to redo
>> it by removing of_size/of_read from shadowof.c and copying it in at
>> init time (also a hack, IMHO), I certainly wouldn't object. As before,
>> it's Ben's call though.
>
> It's nice to have a fix upstream.
>
> I'll send a patch to remove the of_size.
>
> Rereading your patch and mine, I think we don't need no_pcir, we can do
> instead:
>
>         if (!nvbios_rd16(bios, base + 0x18)) {
>                 /* no PCIr */
>                 image.base = 0;
>                 image.type = 0;
>                 image.size = bios->size;
>                 image.last = 1;
>         } else {
>                 if (!shadow_fetch(bios, mthd, offset + 0x1000)) {
>
> ...
> What is you opinion ?

What if we hit on a real VBIOS without a PCIR? Like, say, some NV4, or
who-knows-what. Seems less safe. My way is very explicit and allows OF
some leeway that "regular" methods don't get.

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web