Re: [PATCH 18/25] drm/exynos: fimd: fix dma burst size setting for small plane size

[Tobias Jakobi]

This one looks a bit strange. It only changes the argument list of
fimd_win_set_pixfmt() but the commit message that it actually fixes
something.

The corresponding upstream commit:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=66367461e573321f0fbb0be0391165b5a54d5fe4

With best wishes,
Tobias


Marek Szyprowski wrote:
> 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.
> 
> Signed-off-by: Marek Szyprowski 
> ---
>  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 44226b2b46c7..6c04ff6432d4 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 ARGB then change it to XRGB.
>*/
>   if (ctx->driver_data->has_limited_fmt && !win) {
> - if (fb->pixel_format == DRM_FORMAT_ARGB)
> - fb->pixel_format = DRM_FORMAT_XRGB;
> + if (pixel_format == DRM_FORMAT_ARGB)
> + pixel_format = DRM_FORMAT_XRGB;
>   }
>  
> - 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)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 18/25] drm/exynos: fimd: fix dma burst size setting for small plane size

[Daniel Stone]

Hey Tobias,

On 12 November 2015 at 15:17, Tobias Jakobi
 wrote:
> This one looks a bit strange. It only changes the argument list of
> fimd_win_set_pixfmt() but the commit message that it actually fixes
> something.
>
> The corresponding upstream commit:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=66367461e573321f0fbb0be0391165b5a54d5fe4

It's subtle:

>> - fimd_win_set_pixfmt(ctx, win, fb);
>> + fimd_win_set_pixfmt(ctx, win, fb->pixel_format, state->src.w);

i.e. rather than checking the total width of the allocated fb, we
check the width we are _actually_ scanning out from the fb. So I think
the patch is correct.

Cheers,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html