Hi,

On Fri, Jul 29, 2022 at 08:12:07PM +0200, Mateusz Kwiatkowski wrote:

I'm just noting that the modelines you defined in drm_modes.c are different
to the ones you're removing here.

The horizontal sync differences probably doesn't matter too much, VC4 uses
those only as a hint anyway and generates sync autonomously, so the slight
differences will only cause the image to slightly shift horizontally.

But you're also changing the 480i vertical sync (vsync_start is now 488
instead of 487, etc.). Are you sure that this won't break anything? This
will probably shift the image by 1 line (which for the 480i might actually
mean going out of spec), and I _think_ it might control the odd vs. even
field first modes on some drivers. I won't be able to test this before
Monday, but I'm just pointing out the potential issue.

I didn't see any difference on both vc4 and sun4i, but you might be
right about this.

I didn't have much confidence in the vc4 modes since they were broken
before your patches, but maybe I should have used yours still.

BTW, I've seen a similar thing in the sun4i driver changes (patch 32/35) and
the differences in vertical sync are even more dramatic. It's entirely
possible that the current timings in sun4i are broken and the new ones are
correct (the new ones certainly look saner to me), but I'd double-check if
that driver does not have any quirks that would _require_ such weird
settings.

The only thing sun4i requires from the new mode is the line number
anyway, which stays the same.

Maxime