Re: [LTP] [PATCH v1 15/16] drm/i915/bios: search for VBT #57 by default

Hi Michał, all,

> Hi Petr & Oliver,

> > in our tests, it's quite persistent and clean on parent:

> I haven't reproduced it myself, but this issue seems somewhat related
> with issue observed earlier by CI [1]. I would expect it to not be a
> random glitch but a true bug related to the series.

> In previous versions of the series, during parsing there was a
> unfortunate snippet of:

> +	const u32 *tables = block->tables;
> +	size_t offset = 0;
> +	size_t row_width;
> +	const u32 *vals;
> +
> +	row_width = block->num_columns * sizeof(*tables);
> +
> +	for (int idx = 0; idx < block->num_tables; idx++) {
> +		for (int row = 0; row < num_rows; row++) {
> +			vals = &tables[offset];
> [...]
> +			offset += row_width;
> +		}
> +	}

> Which caused hopping by 4 times more bytes than it should, because the
> sizeof() wasn't meant to be there. Since parsing takes place at module
> load, looks like it could be the issue. This seems especially convincing
> given that report is from parse_vswing_preemph_snps(). I have to also
> admit that it would be consistent with review comment from Sashiko [2].

> Lastly, sadly due to poorly split commits, 15th patch from v1 turned on
> whole VBT #57 searching. This probably caused bisect to point at 15th
> patch instead of pointing at patch adding parsing code, which would
> clearly show that patch/es introduce/s the bug.

> Anyways, now the bug should be fixed in v2 [3].

Michał, thanks a lot for pointing pointing this out.

Kind regards,
Petr

> [1] https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164196v1/fi-hsw-4770/dmesg0.txt
> [2] https://sashiko.dev/#/patchset/20260331183332.1773886-1-michal.grzelak%40intel.com?part=5
> [3] https://lore.kernel.org/intel-gfx/[email protected]/

> BR,
> Michał