[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <86a208c1-9277-32de-3f8f-8976eab15524@sholland.org>
Date: Tue, 24 May 2022 12:07:27 -0500
From: Samuel Holland <samuel@...lland.org>
To: Icenowy Zheng <icenowy@...c.io>,
Jernej Škrabec <jernej.skrabec@...il.com>,
mripard@...nel.org, wens@...e.org, Genfu Pan <benlypan@...il.com>
Cc: airlied@...ux.ie, daniel@...ll.ch, dri-devel@...ts.freedesktop.org,
linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/sun4i: mixer: fix scanline for V3s and D1
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@...il.com>
>>> ---
>>> 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");
>>
>>
>>
>>
>>
>
>
Powered by blists - more mailing lists