[PATCH 2/3] drm/exynos: scaler: Add support for tiled formats

[Marek Szyprowski]

Hi Inki,

On 2018-09-13 10:23, Inki Dae wrote:
> 2018년 09월 13일 17:03에 Marek Szyprowski 이(가) 쓴 글:
>> On 2018-09-13 07:14, Inki Dae wrote:
>>> 2018년 09월 12일 15:59에 Andrzej Pietrasiewicz 이(가) 쓴 글:
>>>> W dniu 12.09.2018 o 01:54, Inki Dae pisze:
>>>>> Hi Marek and Andrzej,
>>>>>
>>>>> 2018년 08월 10일 22:29에 Marek Szyprowski 이(가) 쓴 글:
>>>>>> From: Andrzej Pietrasiewicz <andrzej.p at samsung.com>
>>>>>>
>>>>>> Add support for 16x16 tiled formats: NV12/NV21, YUYV and YUV420.
>>>> <snip>
>>>>
>>>>>> -static u32 scaler_get_format(u32 drm_fmt)
>>>>>> +struct scaler_format {
>>>>>> +	u32	drm_fmt;
>>>>>> +	u32	internal_fmt;
>>>>>> +	u32	chroma_tile_w;
>>>>>> +	u32	chroma_tile_h;
>>>>>> +};
>>>>>> +
>>>>>> +static const struct scaler_format scaler_formats[] = {
>>>>>> +	{ DRM_FORMAT_NV12, SCALER_YUV420_2P_UV, 8, 8 },
>>>>>> +	{ DRM_FORMAT_NV21, SCALER_YUV420_2P_VU, 8, 8 },
>>>>>> +	{ DRM_FORMAT_YUV420, SCALER_YUV420_3P, 8, 8 },
>>>>>> +	{ DRM_FORMAT_YUYV, SCALER_YUV422_1P_YUYV, 16, 16 },
>>>>>> +	{ DRM_FORMAT_UYVY, SCALER_YUV422_1P_UYVY, 16, 16 },
>>>>>> +	{ DRM_FORMAT_YVYU, SCALER_YUV422_1P_YVYU, 16, 16 },
>>>>>> +	{ DRM_FORMAT_NV16, SCALER_YUV422_2P_UV, 8, 16 },
>>>>>> +	{ DRM_FORMAT_NV61, SCALER_YUV422_2P_VU, 8, 16 },
>>>>>> +	{ DRM_FORMAT_YUV422, SCALER_YUV422_3P, 8, 16 },
>>>>>> +	{ DRM_FORMAT_NV24, SCALER_YUV444_2P_UV, 16, 16 },
>>>>>> +	{ DRM_FORMAT_NV42, SCALER_YUV444_2P_VU, 16, 16 },
>>>>>> +	{ DRM_FORMAT_YUV444, SCALER_YUV444_3P, 16, 16 },
>>>>>> +	{ DRM_FORMAT_RGB565, SCALER_RGB_565, 0, 0 },
>>>>>> +	{ DRM_FORMAT_XRGB1555, SCALER_ARGB1555, 0, 0 },
>>>>>> +	{ DRM_FORMAT_ARGB1555, SCALER_ARGB1555, 0, 0 },
>>>>>> +	{ DRM_FORMAT_XRGB4444, SCALER_ARGB4444, 0, 0 },
>>>>>> +	{ DRM_FORMAT_ARGB4444, SCALER_ARGB4444, 0, 0 },
>>>>>> +	{ DRM_FORMAT_XRGB8888, SCALER_ARGB8888, 0, 0 },
>>>>>> +	{ DRM_FORMAT_ARGB8888, SCALER_ARGB8888, 0, 0 },
>>>>>> +	{ DRM_FORMAT_RGBX8888, SCALER_RGBA8888, 0, 0 },
>>>>>> +	{ DRM_FORMAT_RGBA8888, SCALER_RGBA8888, 0, 0 },
>>>>> Seems the tile size of each format you declared above is wrong.
>>>>> According to data sheet for Exynos5420/5422/5433, each plane has different tile size.
>>>>> I.e., SACLE_YUV420_2P_UV/VU has two planes, and Y plane has 16 x 16 tile size but U and V plane have 8 x 8 tile size each other.
>>>>>
>>>>> However, above declaration has only one tile size
>>>> true, but the members of struct scaler_format are called
>>>>
>>>> _____chroma_____tile_w/h
>>>>
>>>>> and it means that each plane has always same tile size.
>>>> this conclusion is not justified
>>>>
>>>>> And also RGB formats have 16 x 16 tile size in tile mode but you declared it as 0.
>>>> The data sheet says, that all formats have the same Y/RGB tile size
>>>> and it is always 16x16. That is why only chroma tile size is remembered,
>>>> because only chroma tile size can be different between formats.
>>> Ok, chroma_tile_h/w are used to calculate only the position of the chroma buffer.
>>>
>>> By the way, user space would want to know the size limit in tiled mode so that they can allocate each buffer for Y(luma) and CbCr(chroma).
>>> However, below code would provide Y/RGB tile size limit - 16 - to user space,
>>> static const struct drm_exynos_ipp_limit scaler_5420_tile_limits[] = {
>>>            { IPP_SIZE_LIMIT(BUFFER, .h = { 16, SZ_8K }, .v = { 16, SZ_8K })},
>>>            { IPP_SIZE_LIMIT(AREA, .h.align = 16, .v.align = 16) },
>>>
>>> It would mean that user space will allocate 16 pixels aligned buffer even for chroma but the actual size limit of it would be 8 pixels in case of SCALE_YUV420_WP_UV/VU foramt.
>>> Is there some code I'm missing?
>> Userspace knows everything needed to allocate proper buffers. Those
> How does user space know everything? Actually, the size of each plane in tiled mode would depend on Hardware, Scaler device in our case. Is there something user space can know it explictly? Or you mean they can know it implictly?

The size of each plane depends on the selected pixel format only. If you 
select DRM_FORMAT_NV12 + DRM_FORMAT_MOD_SAMSUNG_16_16_TILE modifier, 
then this unambiguously defines buffer size for each plane. Hardware has 
nothing to change here. Userspace can even query if given IPP module 
supports such pixel+modifier combo and get alignment requirements for it.

>> limits describes size of the image buffers in pixels. The size of each
>> plane (luma or chroma if exists for given format) in bytes directly
>> comes out of the selected pixel format (fourcc).
> fourcc would say the size not considered for the Hardware limit.

Alignment can be queried by userspace via get EXYNOS_IPP_GET_LIMITS 
ioctl. It provides limits for given pixel format + tile modifier combo, 
although in case of Exynos Scaler, the alignment requirements are simple 
result of the tiled format definition (tiled formats are typically 
defined only for width/height matching multiples of single tile size).

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland