On 5/23/22 8:14 AM, Icenowy Zheng wrote: > 在 2022-05-22星期日的 10:36 +0200,Jernej Škrabec写道: >> Hi! >> >> Dne sobota, 21. maj 2022 ob 15:34:43 CEST je Genfu Pan napisal(a): >>> Accrording the SDK from Allwinner, the scanline value of yuv and >>> rgb for >>> V3s are both 1024. >> >> s/scanline value/scanline length/ >> >> Which SDK? All SDKs that I have or found on internet don't mention >> YUV nor RGB >> scanline limit. That doesn't mean there is none, I'm just unable to >> verify >> your claim. Did you test this by yourself? Also, please make YUV >> scanline >> change separate patch with fixes tag. > > BTW I think chip manuals all say that the chip supports NxN resolution > in DE2 chapter, e.g. the V3 datasheet says DE2 "Output size up to > 1024x1024". > > However there's no information about D1's second mixer. My information comes from the BSP driver[0]: static const int sun8iw20_de_scale_line_buffer[] = { /* DISP0 */ 2048, /* DISP1 */ 1024, }; It looks like the value returned from de_feat_get_scale_linebuf() may be used for RGB as well, if scaling is enabled. This appears to be the "format == 3" case in de_rtmx_get_coarse_fac[1]. On the other hand, the code for V3 has specific code for the RGB limit[2]. Is there some test I can do on D1 to see what the right value for RGB is? Regards, Samuel [0]: https://github.com/Tina-Linux/tina-d1x-linux-5.4/blob/master/drivers/video/fbdev/sunxi/disp2/disp/de/lowlevel_v2x/de_feat.c#L182 [1]: https://github.com/Tina-Linux/tina-d1x-linux-5.4/blob/master/drivers/video/fbdev/sunxi/disp2/disp/de/lowlevel_v2x/de_rtmx.c#L1588 [2]: https://github.com/Tina-Linux/tina-d1x-linux-5.4/blob/master/drivers/video/fbdev/sunxi/disp2/disp/de/lowlevel_sun8iw8/de_rtmx.c#L1211 >>> The is also the same for mixer 1 of D1. Currently the >>> scanline value of rgb is hardcoded to 2048 for all SOCs. >>> >>> Change the scanline_yuv property of V3s to 1024. > Add the >>> scanline_rgb >>> property to the mixer config and replace the hardcoded value with >>> it before >>> scaling. >> >> I guess RGB scanline patch would also need fixes tag, since it fixes >> existing >> bug. >> >>> >>> Signed-off-by: Genfu Pan <benlypan@xxxxxxxxx> >>> --- >>> drivers/gpu/drm/sun4i/sun8i_mixer.c | 13 ++++++++++++- >>> drivers/gpu/drm/sun4i/sun8i_mixer.h | 1 + >>> drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 3 +-- >>> 3 files changed, 14 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c >>> b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 875a1156c..e64e08207 >>> 100644 >>> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c >>> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c >>> @@ -567,6 +567,7 @@ static const struct sun8i_mixer_cfg >>> sun8i_a83t_mixer0_cfg = { .ccsc = CCSC_MIXER0_LAYOUT, >>> .scaler_mask = 0xf, >>> .scanline_yuv = 2048, >>> + .scanline_rgb = 2048, >>> .ui_num = 3, >>> .vi_num = 1, >>> }; >>> @@ -575,6 +576,7 @@ static const struct sun8i_mixer_cfg >>> sun8i_a83t_mixer1_cfg = { .ccsc = CCSC_MIXER1_LAYOUT, >>> .scaler_mask = 0x3, >>> .scanline_yuv = 2048, >>> + .scanline_rgb = 2048, >>> .ui_num = 1, >>> .vi_num = 1, >>> }; >>> @@ -584,6 +586,7 @@ static const struct sun8i_mixer_cfg >>> sun8i_h3_mixer0_cfg >>> = { .mod_rate = 432000000, >>> .scaler_mask = 0xf, >>> .scanline_yuv = 2048, >>> + .scanline_rgb = 2048, >>> .ui_num = 3, >>> .vi_num = 1, >>> }; >>> @@ -593,6 +596,7 @@ static const struct sun8i_mixer_cfg >>> sun8i_r40_mixer0_cfg >>> = { .mod_rate = 297000000, >>> .scaler_mask = 0xf, >>> .scanline_yuv = 2048, >>> + .scanline_rgb = 2048, >>> .ui_num = 3, >>> .vi_num = 1, >>> }; >>> @@ -602,6 +606,7 @@ static const struct sun8i_mixer_cfg >>> sun8i_r40_mixer1_cfg >>> = { .mod_rate = 297000000, >>> .scaler_mask = 0x3, >>> .scanline_yuv = 2048, >>> + .scanline_rgb = 2048, >>> .ui_num = 1, >>> .vi_num = 1, >>> }; >>> @@ -610,7 +615,8 @@ static const struct sun8i_mixer_cfg >>> sun8i_v3s_mixer_cfg >>> = { .vi_num = 2, >>> .ui_num = 1, >>> .scaler_mask = 0x3, >>> - .scanline_yuv = 2048, >>> + .scanline_yuv = 1024, >>> + .scanline_rgb = 1024, >>> .ccsc = CCSC_MIXER0_LAYOUT, >>> .mod_rate = 150000000, >>> }; >>> @@ -620,6 +626,7 @@ static const struct sun8i_mixer_cfg >>> sun20i_d1_mixer0_cfg >>> = { .mod_rate = 297000000, >>> .scaler_mask = 0x3, >>> .scanline_yuv = 2048, >>> + .scanline_rgb = 2048, >>> .ui_num = 1, >>> .vi_num = 1, >>> }; >>> @@ -629,6 +636,7 @@ static const struct sun8i_mixer_cfg >>> sun20i_d1_mixer1_cfg >>> = { .mod_rate = 297000000, >>> .scaler_mask = 0x1, >>> .scanline_yuv = 1024, >>> + .scanline_rgb = 1024, >>> .ui_num = 0, >>> .vi_num = 1, >>> }; >>> @@ -638,6 +646,7 @@ static const struct sun8i_mixer_cfg >>> sun50i_a64_mixer0_cfg = { .mod_rate = 297000000, >>> .scaler_mask = 0xf, >>> .scanline_yuv = 4096, >>> + .scanline_rgb = 2048, >>> .ui_num = 3, >>> .vi_num = 1, >>> }; >>> @@ -647,6 +656,7 @@ static const struct sun8i_mixer_cfg >>> sun50i_a64_mixer1_cfg = { .mod_rate = 297000000, >>> .scaler_mask = 0x3, >>> .scanline_yuv = 2048, >>> + .scanline_rgb = 2048, >>> .ui_num = 1, >>> .vi_num = 1, >>> }; >>> @@ -657,6 +667,7 @@ static const struct sun8i_mixer_cfg >>> sun50i_h6_mixer0_cfg >>> = { .mod_rate = 600000000, >>> .scaler_mask = 0xf, >>> .scanline_yuv = 4096, >>> + .scanline_rgb = 2048, >>> .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 85c94884f..c01b3e9d6 >>> 100644 >>> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h >>> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h >>> @@ -172,6 +172,7 @@ struct sun8i_mixer_cfg { >>> unsigned long mod_rate; >>> unsigned int is_de3 : 1; >>> unsigned int scanline_yuv; >>> + unsigned int scanline_rgb; >> >> This quirk needs to be documented above in the comment. >> >> Best regards, >> Jernej >> >>> }; >>> >>> struct sun8i_mixer { >>> diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c >>> b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index f7d0b082d..30e6bde92 >>> 100644 >>> --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c >>> +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c >>> @@ -188,8 +188,7 @@ static int sun8i_vi_layer_update_coord(struct >>> sun8i_mixer *mixer, int channel, src_h = vn; >>> } >>> >>> - /* it seems that every RGB scaler has buffer for >>> 2048 >> pixels */ >>> - scanline = subsampled ? mixer->cfg->scanline_yuv : >> 2048; >>> + scanline = subsampled ? mixer->cfg->scanline_yuv : >>> mixer->cfg->scanline_rgb; >>> >>> if (src_w > scanline) { >>> DRM_DEBUG_DRIVER("Using horizontal coarse >> scaling\n"); >> >> >> >> >> > >