Re: [PATCH v1 20/35] drm/vc4: vec: Switch for common modes

[Maxime Ripard]

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

Attachment: signature.asc
Description: PGP signature