Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <5e18705f-79c1-18a7-57f2-74866abe21e9@samsung.com>
Date:   Thu, 24 Feb 2022 10:41:04 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
        Thunderbird/91.5.0
Subject: Re: [PATCH] drm/exynos: fimd: add BGR support for exynos4/5
Content-Language: en-US
To:     =?UTF-8?Q?Martin_J=c3=bccker?= <martin.juecker@gmail.com>,
        Joonyoung Shim <jy0922.shim@samsung.com>,
        Seung-Woo Kim <sw0312.kim@samsung.com>,
        Kyungmin Park <kyungmin.park@samsung.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
        dri-devel@lists.freedesktop.org,
        linux-arm-kernel@lists.infradead.org,
        linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org
From:   Inki Dae <inki.dae@samsung.com>
In-Reply-To: <20220129220153.GA33165@adroid>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 101P
DLP-Filter: Pass
References: <CGME20220129220203epcas1p25b1704191dd7babfb8d5b8dc6704d566@epcas1p2.samsung.com>
        <20220129220153.GA33165@adroid>
Precedence: bulk

Hi Martin.

I found that exynos4 and 5 data sheet include documented same register.
RGB_ORDER_E field of VIDCONx registers will do same thing.

I'm not sure whether the use of undocumented register is safe or not - maybe some HW bug exists.

Anyway, I'd like to recommend you to use documented register only.

Sorry for late and thanks,
Inki Dae

22. 1. 30. 07:01에 Martin Jücker 이(가) 쓴 글:
> In the downstream kernels for exynos4 and exynos5 devices, there is an
> undocumented register that controls the order of the RGB output. It can
> be set to either normal order or reversed, which enables BGR support for
> those SoCs.
> 
> This patch enables the BGR support for all the SoCs that were found to
> have at least one device with this logic in the corresponding downstream
> kernels.
> 
> Signed-off-by: Martin Jücker <martin.juecker@gmail.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 42 ++++++++++++++++++++++--
>  include/video/samsung_fimd.h             |  4 +++
>  2 files changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index c735e53939d8..cb632360c968 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -109,6 +109,7 @@ struct fimd_driver_data {
>  	unsigned int has_dp_clk:1;
>  	unsigned int has_hw_trigger:1;
>  	unsigned int has_trigger_per_te:1;
> +	unsigned int has_bgr_support:1;
>  };
>  
>  static struct fimd_driver_data s3c64xx_fimd_driver_data = {
> @@ -138,6 +139,7 @@ static struct fimd_driver_data exynos4_fimd_driver_data = {
>  	.lcdblk_bypass_shift = 1,
>  	.has_shadowcon = 1,
>  	.has_vtsel = 1,
> +	.has_bgr_support = 1,
>  };
>  
>  static struct fimd_driver_data exynos5_fimd_driver_data = {
> @@ -149,6 +151,7 @@ static struct fimd_driver_data exynos5_fimd_driver_data = {
>  	.has_vidoutcon = 1,
>  	.has_vtsel = 1,
>  	.has_dp_clk = 1,
> +	.has_bgr_support = 1,
>  };
>  
>  static struct fimd_driver_data exynos5420_fimd_driver_data = {
> @@ -162,6 +165,7 @@ static struct fimd_driver_data exynos5420_fimd_driver_data = {
>  	.has_vtsel = 1,
>  	.has_mic_bypass = 1,
>  	.has_dp_clk = 1,
> +	.has_bgr_support = 1,
>  };
>  
>  struct fimd_context {
> @@ -226,6 +230,18 @@ static const uint32_t fimd_formats[] = {
>  	DRM_FORMAT_ARGB8888,
>  };
>  
> +static const uint32_t fimd_extended_formats[] = {
> +	DRM_FORMAT_C8,
> +	DRM_FORMAT_XRGB1555,
> +	DRM_FORMAT_XBGR1555,
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_BGR565,
> +	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_XBGR8888,
> +	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_ABGR8888,
> +};
> +
>  static const unsigned int capabilities[WINDOWS_NR] = {
>  	0,
>  	EXYNOS_DRM_PLANE_CAP_WIN_BLEND | EXYNOS_DRM_PLANE_CAP_PIX_BLEND,
> @@ -673,21 +689,25 @@ static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win,
>  		val |= WINCONx_BYTSWP;
>  		break;
>  	case DRM_FORMAT_XRGB1555:
> +	case DRM_FORMAT_XBGR1555:
>  		val |= WINCON0_BPPMODE_16BPP_1555;
>  		val |= WINCONx_HAWSWP;
>  		val |= WINCONx_BURSTLEN_16WORD;
>  		break;
>  	case DRM_FORMAT_RGB565:
> +	case DRM_FORMAT_BGR565:
>  		val |= WINCON0_BPPMODE_16BPP_565;
>  		val |= WINCONx_HAWSWP;
>  		val |= WINCONx_BURSTLEN_16WORD;
>  		break;
>  	case DRM_FORMAT_XRGB8888:
> +	case DRM_FORMAT_XBGR8888:
>  		val |= WINCON0_BPPMODE_24BPP_888;
>  		val |= WINCONx_WSWP;
>  		val |= WINCONx_BURSTLEN_16WORD;
>  		break;
>  	case DRM_FORMAT_ARGB8888:
> +	case DRM_FORMAT_ABGR8888:
>  	default:
>  		val |= WINCON1_BPPMODE_25BPP_A1888;
>  		val |= WINCONx_WSWP;
> @@ -695,6 +715,18 @@ static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win,
>  		break;
>  	}
>  
> +	switch (pixel_format) {
> +	case DRM_FORMAT_XBGR1555:
> +	case DRM_FORMAT_XBGR8888:
> +	case DRM_FORMAT_ABGR8888:
> +	case DRM_FORMAT_BGR565:
> +		writel(WIN_RGB_ORDER_REVERSE, ctx->regs + WIN_RGB_ORDER(win));
> +		break;
> +	default:
> +		writel(WIN_RGB_ORDER_FORWARD, ctx->regs + WIN_RGB_ORDER(win));
> +		break;
> +	}
> +
>  	/*
>  	 * Setting dma-burst to 16Word causes permanent tearing for very small
>  	 * buffers, e.g. cursor buffer. Burst Mode switching which based on
> @@ -1074,8 +1106,14 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
>  	ctx->drm_dev = drm_dev;
>  
>  	for (i = 0; i < WINDOWS_NR; i++) {
> -		ctx->configs[i].pixel_formats = fimd_formats;
> -		ctx->configs[i].num_pixel_formats = ARRAY_SIZE(fimd_formats);
> +		if (ctx->driver_data->has_bgr_support) {
> +			ctx->configs[i].pixel_formats = fimd_extended_formats;
> +			ctx->configs[i].num_pixel_formats = ARRAY_SIZE(fimd_extended_formats);
> +		} else {
> +			ctx->configs[i].pixel_formats = fimd_formats;
> +			ctx->configs[i].num_pixel_formats = ARRAY_SIZE(fimd_formats);
> +		}
> +
>  		ctx->configs[i].zpos = i;
>  		ctx->configs[i].type = fimd_win_types[i];
>  		ctx->configs[i].capabilities = capabilities[i];
> diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h
> index c4a93ce1de48..e6966d187591 100644
> --- a/include/video/samsung_fimd.h
> +++ b/include/video/samsung_fimd.h
> @@ -476,6 +476,10 @@
>   * 1111		-none-	 -none-   -none-   -none-    -none-
>  */
>  
> +#define WIN_RGB_ORDER(_win)			(0x2020 + ((_win) * 4))
> +#define WIN_RGB_ORDER_FORWARD			(0 << 11)
> +#define WIN_RGB_ORDER_REVERSE			(1 << 11)
> +
>  /* FIMD Version 8 register offset definitions */
>  #define FIMD_V8_VIDTCON0	0x20010
>  #define FIMD_V8_VIDTCON1	0x20014