Re: [PATCH v2 14/22] drm/exynos: fimd: fix dma burst size setting for small plane size

[Marek Szyprowski <m.szyprowski@samsung.com>]

Hello,

On 2015-12-10 12:35, Inki Dae wrote:
> Hi Marek,
>
> 2015년 11월 30일 22:53에 Marek Szyprowski 이(가) 쓴 글:
>> This patch fixes trashed display of buffers cropped to very small width.
>> Even if DMA is unstable and causes tearing when changing the burst size,
>> it is still better than displaying a garbage.
> It seems that this patch is different from above description. I think below patch is just cleanup,
> which passes each member necessary instead of passing a drm_framebuffer object.

Please note the last chunk of this patch. After applying it width is
taken from state->src.w instead of fb->width. If you want, I can split
this patch into 2 parts - one cleanup without functional change, and
second, replacement of fb->width with state->src.w.


>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   drivers/gpu/drm/exynos/exynos_drm_fimd.c | 24 +++++++++++-------------
>>   1 file changed, 11 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index 70cd2681e343..2e2247126581 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -487,7 +487,7 @@ static void fimd_commit(struct exynos_drm_crtc *crtc)
>>   
>>   
>>   static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win,
>> -				struct drm_framebuffer *fb)
>> +				uint32_t pixel_format, int width)
>>   {
>>   	unsigned long val;
>>   
>> @@ -498,11 +498,11 @@ static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win,
>>   	 * So the request format is ARGB8888 then change it to XRGB8888.
>>   	 */
>>   	if (ctx->driver_data->has_limited_fmt && !win) {
>> -		if (fb->pixel_format == DRM_FORMAT_ARGB8888)
>> -			fb->pixel_format = DRM_FORMAT_XRGB8888;
>> +		if (pixel_format == DRM_FORMAT_ARGB8888)
>> +			pixel_format = DRM_FORMAT_XRGB8888;
>>   	}
>>   
>> -	switch (fb->pixel_format) {
>> +	switch (pixel_format) {
>>   	case DRM_FORMAT_C8:
>>   		val |= WINCON0_BPPMODE_8BPP_PALETTE;
>>   		val |= WINCONx_BURSTLEN_8WORD;
>> @@ -538,17 +538,15 @@ static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win,
>>   		break;
>>   	}
>>   
>> -	DRM_DEBUG_KMS("bpp = %d\n", fb->bits_per_pixel);
>> -
>>   	/*
>> -	 * In case of exynos, setting dma-burst to 16Word causes permanent
>> -	 * tearing for very small buffers, e.g. cursor buffer. Burst Mode
>> -	 * switching which is based on plane size is not recommended as
>> -	 * plane size varies alot towards the end of the screen and rapid
>> -	 * movement causes unstable DMA which results into iommu crash/tear.
>> +	 * Setting dma-burst to 16Word causes permanent tearing for very small
>> +	 * buffers, e.g. cursor buffer. Burst Mode switching which based on
>> +	 * plane size is not recommended as plane size varies alot towards the
>> +	 * end of the screen and rapid movement causes unstable DMA, but it is
>> +	 * still better to change dma-burst than displaying garbage.
>>   	 */
>>   
>> -	if (fb->width < MIN_FB_WIDTH_FOR_16WORD_BURST) {
>> +	if (width < MIN_FB_WIDTH_FOR_16WORD_BURST) {
>>   		val &= ~WINCONx_BURSTLEN_MASK;
>>   		val |= WINCONx_BURSTLEN_4WORD;
>>   	}
>> @@ -723,7 +721,7 @@ static void fimd_update_plane(struct exynos_drm_crtc *crtc,
>>   		DRM_DEBUG_KMS("osd size = 0x%x\n", (unsigned int)val);
>>   	}
>>   
>> -	fimd_win_set_pixfmt(ctx, win, fb);
>> +	fimd_win_set_pixfmt(ctx, win, fb->pixel_format, state->src.w);
>>   
>>   	/* hardware window 0 doesn't support color key. */
>>   	if (win != 0)
>>

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