Re: [PATCH] drm/exynos: fimd: add BGR support for exynos4/5

["Martin Jücker" <martin.juecker@gmail.com>]

Hello Inki,

On Thu, Feb 24, 2022 at 10:41:04AM +0900, Inki Dae wrote:
> 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.

If I read the manual correctly, this register combined with the
RGB_ORDER_O makes it possible to map the whole RGB interface output to a
different order. What my patch provides is a way to configure each
hardware plane separately while maintaining a consistent output on the
RGB interface.

Implementing the RGB_ORDER_O and E would need some logic to make sure
that all planes are always using the same RGB order.

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

I see, that makes sense. Would it be possible then to introduce a new
compatible, e.g. samsung,exynos4210-fimd-ext which can be used on tested
devices? I know that some other Galaxy Note and S devices with the
exynos4 chip have the same problem (and solution).

> 
> Anyway, I'd like to recommend you to use documented register only.
> 
> Sorry for late and thanks,
> Inki Dae

Kind Regards
Martin

> 
> 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel