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