Re: [PATCH v5 22/22] drm/sun4i: tv: Convert to the new TV mode property

[Maxime Ripard]

Hi Jernej,

On Thu, Oct 13, 2022 at 08:23:51PM +0200, Jernej Škrabec wrote:
> Dne četrtek, 13. oktober 2022 ob 15:19:06 CEST je Maxime Ripard napisal(a):
> > Now that the core can deal fine with analog TV modes, let's convert the
> > sun4i TV driver to leverage those new features.
> > 
> > Acked-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
> > Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx>
> > 
> > ---
> > Changes in v5:
> > - Removed the count variable in get_modes
> > - Removed spurious vc4 change
> > ---
> >  drivers/gpu/drm/sun4i/sun4i_tv.c | 145
> > +++++++++++++-------------------------- 1 file changed, 48 insertions(+),
> > 97 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c
> > b/drivers/gpu/drm/sun4i/sun4i_tv.c index c65f0a89b6b0..4f07aff11551 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_tv.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
> > @@ -141,23 +141,14 @@ struct resync_parameters {
> >  struct tv_mode {
> >  	char		*name;
> > 
> > +	unsigned int	tv_mode;
> > +
> >  	u32		mode;
> >  	u32		chroma_freq;
> >  	u16		back_porch;
> >  	u16		front_porch;
> > -	u16		line_number;
> >  	u16		vblank_level;
> 
> isn't there a way to get or calculate back_porch, front_porch and vblank_level 
> from mode? From quick glance over removed values below, I would say that at 
> least back_porch can be removed too?

I tried actually, but I'm not sure what the front porch and back porch
parameters actually are. They are called that way by Allwinner, but it
doesn't match the PAL or NTSC timings at all.

For example, back_porch is 118 for NTSC and 138 for PAL. Actual back
porches would be around 12 and 16, respectively. Actually, the entire
blanking area are 138 and 144. This is close enough for PAL, but pretty
far off for NTSC.

Allwinner has the habit of integrating the sync period into one of the
porches, but still there's no obvious match.

front_porch is pretty much in the same case.

Since it affected the display output quite a lot, I chose to keep it for
now unfortunately.

> Otherwise this patch looks ok.

Can I add your Acked-by/Reviewed-by then?

Thanks!
Maxime