| Message ID | 1444574882-3565-1-git-send-email-laurent@vivier.eu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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
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
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
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); }
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(-)