On Sat, Feb 16, 2019 at 10:02 AM Colin Ian King colin.king@canonical.com wrote:

Hi,

Static Analysis with CoverityScan as detected an issue with the setting of the RON pull value in function nvkm_gddr3_calc in drm/nouveau/bios/ramcfg.c

This was introduced by commit: c25bf7b6155cb ("drm/nouveau/bios/ramcfg: Separate out RON pull value")

CoverityScan reports the issue as follows:

84 case 0x20: 85 CWL = (ram->next->bios.timing[1] & 0x00000f80) >> 7; 86 CL = (ram->next->bios.timing[1] & 0x0000001f) >> 0; 87 WR = (ram->next->bios.timing[2] & 0x007f0000) >> 16; 88 /* XXX: Get these values from the VBIOS instead */ 89 DLL = !(ram->mr[1] & 0x1);

CID 1324005 (#1 of 1): Operands don't affect result (CONSTANT_EXPRESSION_RESULT)

result_independent_of_operands: !(ram->mr[1] & 768) >> 8 is 0 regardless of the values of its operands. This occurs as the operand of assignment.

90 RON = !(ram->mr[1] & 0x300) >> 8; 91 break;

Looking at this, I believe perhaps the correct setting could be:

RON = !((ram->mr[1] & 0x300) >> 8);

..however I don't have the datasheet available for the H/W so I'm not sure if this a correct fix.

Actually looking at the code a bit, I suspect it should just be

RON = (ram->mr[1] & 0x300) >> 8;

since later on, when we recompose the MR (memory register) value, we do:

ram->mr[1] |= (RON & 0x03) << 8;

(And the whole point here is that we don't know how to get the proper RON value for that timing table version, so we just copy whatever used to be there in that case.)

-ilia