Re: [PATCH 26/30] drm/sun4i: mixer: Add quirk for number of VI scalers

["Jernej Škrabec" <jernej.skrabec@gmail.com>]

Hi Chen-Yu,

Dne ponedeljek, 3. november 2025 ob 18:11:07 Srednjeevropski standardni čas je Chen-Yu Tsai napisal(a):
> On Mon, Oct 13, 2025 at 3:24 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
> >
> > On DE2 and DE3, UI scalers are located right after VI scalers. So in
> > order to calculate proper UI scaler base address, number of VI scalers
> > must be known. In practice, it is same as number of VI channels, but it
> > doesn't need to be.
> >
> > Let's make a quirk for this number. Code for configuring channels and
> > associated functions won't have access to vi_num quirk anymore after
> > rework for independent planes.
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > ---
> >  drivers/gpu/drm/sun4i/sun8i_mixer.c     | 11 +++++++++++
> >  drivers/gpu/drm/sun4i/sun8i_mixer.h     |  2 ++
> >  drivers/gpu/drm/sun4i/sun8i_ui_scaler.c | 10 +++++-----
> >  3 files changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > index 78bbfbe62833..f9131396f22f 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > @@ -708,6 +708,7 @@ static const struct sun8i_mixer_cfg sun8i_a83t_mixer0_cfg = {
> >         .scaler_mask    = 0xf,
> >         .scanline_yuv   = 2048,
> >         .de2_fcc_alpha  = 1,
> > +       .vi_scaler_num  = 1,
> >         .ui_num         = 3,
> >         .vi_num         = 1,
> >  };
> > @@ -718,6 +719,7 @@ static const struct sun8i_mixer_cfg sun8i_a83t_mixer1_cfg = {
> >         .scaler_mask    = 0x3,
> >         .scanline_yuv   = 2048,
> >         .de2_fcc_alpha  = 1,
> > +       .vi_scaler_num  = 1,
> >         .ui_num         = 1,
> >         .vi_num         = 1,
> >  };
> > @@ -729,6 +731,7 @@ static const struct sun8i_mixer_cfg sun8i_h3_mixer0_cfg = {
> >         .scaler_mask    = 0xf,
> >         .scanline_yuv   = 2048,
> >         .de2_fcc_alpha  = 1,
> > +       .vi_scaler_num  = 1,
> >         .ui_num         = 3,
> >         .vi_num         = 1,
> >  };
> > @@ -740,6 +743,7 @@ static const struct sun8i_mixer_cfg sun8i_r40_mixer0_cfg = {
> >         .scaler_mask    = 0xf,
> >         .scanline_yuv   = 2048,
> >         .de2_fcc_alpha  = 1,
> > +       .vi_scaler_num  = 1,
> >         .ui_num         = 3,
> >         .vi_num         = 1,
> >  };
> > @@ -751,6 +755,7 @@ static const struct sun8i_mixer_cfg sun8i_r40_mixer1_cfg = {
> >         .scaler_mask    = 0x3,
> >         .scanline_yuv   = 2048,
> >         .de2_fcc_alpha  = 1,
> > +       .vi_scaler_num  = 1,
> >         .ui_num         = 1,
> >         .vi_num         = 1,
> >  };
> > @@ -761,6 +766,7 @@ static const struct sun8i_mixer_cfg sun8i_v3s_mixer_cfg = {
> >         .ui_num = 1,
> >         .scaler_mask = 0x3,
> >         .scanline_yuv = 2048,
> > +       .vi_scaler_num  = 2,
> >         .ccsc = CCSC_MIXER0_LAYOUT,
> >         .mod_rate = 150000000,
> >  };
> > @@ -772,6 +778,7 @@ static const struct sun8i_mixer_cfg sun20i_d1_mixer0_cfg = {
> >         .scaler_mask    = 0x3,
> >         .scanline_yuv   = 2048,
> >         .de2_fcc_alpha  = 1,
> > +       .vi_scaler_num  = 1,
> >         .ui_num         = 1,
> >         .vi_num         = 1,
> >  };
> > @@ -783,6 +790,7 @@ static const struct sun8i_mixer_cfg sun20i_d1_mixer1_cfg = {
> >         .scaler_mask    = 0x1,
> >         .scanline_yuv   = 1024,
> >         .de2_fcc_alpha  = 1,
> > +       .vi_scaler_num  = 1,
> >         .ui_num         = 0,
> >         .vi_num         = 1,
> >  };
> > @@ -794,6 +802,7 @@ static const struct sun8i_mixer_cfg sun50i_a64_mixer0_cfg = {
> >         .scaler_mask    = 0xf,
> >         .scanline_yuv   = 4096,
> >         .de2_fcc_alpha  = 1,
> > +       .vi_scaler_num  = 1,
> >         .ui_num         = 3,
> >         .vi_num         = 1,
> >  };
> > @@ -805,6 +814,7 @@ static const struct sun8i_mixer_cfg sun50i_a64_mixer1_cfg = {
> >         .scaler_mask    = 0x3,
> >         .scanline_yuv   = 2048,
> >         .de2_fcc_alpha  = 1,
> > +       .vi_scaler_num  = 1,
> >         .ui_num         = 1,
> >         .vi_num         = 1,
> >  };
> > @@ -814,6 +824,7 @@ static const struct sun8i_mixer_cfg sun50i_h6_mixer0_cfg = {
> >         .mod_rate       = 600000000,
> >         .scaler_mask    = 0xf,
> >         .scanline_yuv   = 4096,
> > +       .vi_scaler_num  = 1,
> >         .ui_num         = 3,
> >         .vi_num         = 1,
> >  };
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > index def07afd37e1..40b800022237 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > @@ -178,6 +178,7 @@ enum sun8i_mixer_type {
> >   * @scaline_yuv: size of a scanline for VI scaler for YUV formats.
> >   * @de2_fcc_alpha: use FCC for missing DE2 VI alpha capability
> >   *     Most DE2 cores has FCC. If number of VI planes is one, enable this.
> > + * @vi_scaler_num: Number of VI scalers. Used on DE2 and DE3.
> >   * @map: channel map for DE variants processing YUV separately (DE33)
> >   */
> >  struct sun8i_mixer_cfg {
> > @@ -189,6 +190,7 @@ struct sun8i_mixer_cfg {
> >         unsigned int    de_type;
> >         unsigned int    scanline_yuv;
> >         unsigned int    de2_fcc_alpha : 1;
> > +       unsigned int    vi_scaler_num;
> 
> This could be a smaller type. Please do a sweep of the struct after the
> refactoring is done and see if any of the types could be shrunk.

A lot of things can be stored in smaller type. However, making things smaller
may be contraproductive. Structs are usually aligned for performance reasons,
so it won't save any memory and accessing them will use extra asm instructions
for zeroing out parts of CPU registers since registers are larger than used
data type.

> 
> And just a nitpick, but I would probably insert it above scaler_mask.
> 

Will do.

> 
> Reviewed-by: Chen-Yu Tsai <wens@kernel.org>
> 

Thanks.

Best regards,
Jernej