Hey Ben,
--- original message --- From: Ben Skeggs <[email protected]> Date: 05:05:34 30-09-2015 To: Roy Spliet <[email protected]>, Nouveau Mailinglist <[email protected]> Subject: Re: [PATCH 7/9] fb/ramnv50: Script changes for G94 and up > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA256 > > On 09/30/2015 09:23 AM, Roy Spliet wrote: > > 10053c is not even read on some cards, and I have no idea exactly > > what the criteria are. Likely NVIDIA pre-scans the VBIOS and in > > their driver disables all features that are never used. The > > practical effect should be the same as this implementation though. > I haven't fully reviewed the series properly yet, but wanted to put my > two cents worth in here. > > If this works like it does on Kepler, you're going to get bitten here > if you don't do this correctly. > [snip] > Depending on what DEVINIT did, this can leave something either enabled > or disabled, you don't know which, you're just supposed to just not > touch it. Yes, this is precisely what I have observed, but thanks for restating. I realise I have been living on the edge by ignoring this for now. Quite honestly though, I haven't come across an example where this would become a problem *yet*. I reckon this is something you could expect a follow-up patch for once I have time for it. > > I tried to ignore this on Kepler initially, but eventually got bitten > by it and had to implement it properly (gk104_ram_ctor_data() handles > this for the most part). > > I'd *strongly* recommend you verify if the behaviour is the same on > these boards, and implement it if it is. Agreed, but given I am probably running out of time quite soon, I do recommend taking the current work upstream as is or suggest minor fixes, and not first wait for this proposed change. That's purely from a practical stance: what I pushed to the ML are improvements. Some people benefit from it and it'd be a shame to let that bit-rot. Roy. > > Ben. > > > > > Signed-off-by: Roy Spliet <[email protected]> Tested-by: Pierre > > > Moreau <[email protected]> --- > > drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramnv50.c | 36 > > ++++++++++++++++++++---- 1 file changed, 30 insertions(+), 6 > > deletions(-) > > > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramnv50.c > > b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramnv50.c index > > 1c6ae6b..651b74a 100644 --- > > a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramnv50.c +++ > > b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramnv50.c @@ -302,6 +302,9 > > > @@ nv50_ram_calc(struct nvkm_ram *base, u32 freq) return ret; } > > > > + if (subdev->device->chipset <= 0x96 && > > !next->bios.ramcfg_00_03_02) + ram_mask(hwsq, 0x100710, > > 0x00000200, 0x00000000); + /* Always disable this bit during > > reclock */ ram_mask(hwsq, 0x100200, 0x00000800, 0x00000000); > > > > @@ -353,8 +356,11 @@ nv50_ram_calc(struct nvkm_ram *base, u32 > > freq) next->bios.rammap_00_16_40 << 14); ram_mask(hwsq, 0x00400c, > > > 0x0000ffff, (N1 << 8) | M1); ram_mask(hwsq, 0x004008, 0x91ff0000, > > > r004008); - if (subdev->device->chipset >= 0x96) + + /* XXX: > GDDR3 > > only? */ + if (subdev->device->chipset >= 0x92) ram_wr32(hwsq, > > > 0x100da0, r100da0); + nv50_ram_gpio(hwsq, 0x18, > > !next->bios.ramcfg_FBVDDQ); ram_nsec(hwsq, 64000); /*XXX*/ > > ram_nsec(hwsq, 32000); /*XXX*/ @@ -397,19 +403,33 @@ > > nv50_ram_calc(struct nvkm_ram *base, u32 freq) ram_mask(hwsq, > > 0x100200, 0x00001000, !next->bios.ramcfg_00_04_02 << 12); > > > > /* XXX: A lot of this could be "chipset"/"ram type" > specific stuff > > */ - unk710 = ram_rd32(hwsq, 0x100710) & ~0x00000101; + unk710 > = > > ram_rd32(hwsq, 0x100710) & ~0x00000100; unk714 = ram_rd32(hwsq, > > > 0x100714) & ~0xf0000020; unk718 = ram_rd32(hwsq, 0x100718) & > > > ~0x00000100; unk71c = ram_rd32(hwsq, 0x10071c) & ~0x00000100; + > if > > (subdev->device->chipset <= 0x96) { + unk710 &= ~0x0000006e; > + > > unk714 &= ~0x00000100; + + if (!next->bios.ramcfg_00_03_08) > + > > unk710 |= 0x00000060; + if (!next->bios.ramcfg_FBVDDQ) + > > unk714 > > > |= 0x00000100; + if ( next->bios.ramcfg_00_04_04) + > > unk710 |= > > > 0x0000000e; + } else { + unk710 &= ~0x00000001; + + > > if > > (!next->bios.ramcfg_00_03_08) + unk710 |= 0x00000001; + > > } > > > > if ( next->bios.ramcfg_00_03_01) unk71c |= 0x00000100; if ( > > next->bios.ramcfg_00_03_02) unk710 |= 0x00000100; - if > > (!next->bios.ramcfg_00_03_08) { - unk710 |= 0x1; - > > unk714 |= > > 0x20; - } + if (!next->bios.ramcfg_00_03_08) + unk714 > > |= > > 0x00000020; if ( next->bios.ramcfg_00_04_04) unk714 |= 0x70000000; > > > if ( next->bios.ramcfg_00_04_20) @@ -420,6 +440,8 @@ > > nv50_ram_calc(struct nvkm_ram *base, u32 freq) ram_mask(hwsq, > > 0x100718, 0xffffffff, unk718); ram_mask(hwsq, 0x100710, 0xffffffff, > > > unk710); > > > > + /* XXX: G94 does not even test these regs in trace. Harmless we > > do it, + * but why is it omitted? */ if > > (next->bios.rammap_00_16_20) { ram_wr32(hwsq, 0x1005a0, > > next->bios.ramcfg_00_07 << 16 | next->bios.ramcfg_00_06 > << 8 | @@ > > -450,6 +472,8 @@ nv50_ram_calc(struct nvkm_ram *base, u32 freq) > > ram_mask(hwsq, 0x004008, 0x00004000, 0x00000000); if > > (next->bios.ramcfg_00_03_02) ram_mask(hwsq, 0x10021c, 0x00010000, > > > 0x00010000); + if (subdev->device->chipset <= 0x96 && > > > next->bios.ramcfg_00_03_02) + ram_mask(hwsq, 0x100710, > > 0x00000200, > > > 0x00000200); > > > > return 0; } > > > _______________________________________________ Nouveau mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/nouveau
