* [PATCH] drm/mcde: Fix RGB/BGR bug
@ 2020-11-17 17:54 Linus Walleij
2020-11-17 19:24 ` Sam Ravnborg
2020-11-23 21:59 ` Sam Ravnborg
0 siblings, 2 replies; 4+ messages in thread
From: Linus Walleij @ 2020-11-17 17:54 UTC (permalink / raw)
To: dri-devel, Maarten Lankhorst, Maxime Ripard, Sean Paul
Cc: Linus Walleij, phone-devel, Stephan Gerhold, linux-arm-kernel
I was confused when the graphics came out with blue
penguins on the DPI panel.
It turns out that the so-called "packed RGB666" mode
on the DSI formatter is incorrect: this mode is the
actual RGB888 mode, and the mode called RGB888 is
BGR888.
The claims that the MCDE had inverse RGB/BGR buffer
formats was wrong, so correct this and the buggy
register and everything is much more consistent, and
graphics look good on all targets, both DPI and
DSI.
Cc: phone-devel@vger.kernel.org
Cc: Stephan Gerhold <stephan@gerhold.net>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpu/drm/mcde/mcde_display.c | 23 +++++++++++------------
drivers/gpu/drm/mcde/mcde_display_regs.h | 4 ++--
2 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/mcde/mcde_display.c b/drivers/gpu/drm/mcde/mcde_display.c
index 14c76d3a8e5a..192e11c88d72 100644
--- a/drivers/gpu/drm/mcde/mcde_display.c
+++ b/drivers/gpu/drm/mcde/mcde_display.c
@@ -250,73 +250,70 @@ static int mcde_configure_extsrc(struct mcde *mcde, enum mcde_extsrc src,
val = 0 << MCDE_EXTSRCXCONF_BUF_ID_SHIFT;
val |= 1 << MCDE_EXTSRCXCONF_BUF_NB_SHIFT;
val |= 0 << MCDE_EXTSRCXCONF_PRI_OVLID_SHIFT;
- /*
- * MCDE has inverse semantics from DRM on RBG/BGR which is why
- * all the modes are inversed here.
- */
+
switch (format) {
case DRM_FORMAT_ARGB8888:
val |= MCDE_EXTSRCXCONF_BPP_ARGB8888 <<
MCDE_EXTSRCXCONF_BPP_SHIFT;
- val |= MCDE_EXTSRCXCONF_BGR;
break;
case DRM_FORMAT_ABGR8888:
val |= MCDE_EXTSRCXCONF_BPP_ARGB8888 <<
MCDE_EXTSRCXCONF_BPP_SHIFT;
+ val |= MCDE_EXTSRCXCONF_BGR;
break;
case DRM_FORMAT_XRGB8888:
val |= MCDE_EXTSRCXCONF_BPP_XRGB8888 <<
MCDE_EXTSRCXCONF_BPP_SHIFT;
- val |= MCDE_EXTSRCXCONF_BGR;
break;
case DRM_FORMAT_XBGR8888:
val |= MCDE_EXTSRCXCONF_BPP_XRGB8888 <<
MCDE_EXTSRCXCONF_BPP_SHIFT;
+ val |= MCDE_EXTSRCXCONF_BGR;
break;
case DRM_FORMAT_RGB888:
val |= MCDE_EXTSRCXCONF_BPP_RGB888 <<
MCDE_EXTSRCXCONF_BPP_SHIFT;
- val |= MCDE_EXTSRCXCONF_BGR;
break;
case DRM_FORMAT_BGR888:
val |= MCDE_EXTSRCXCONF_BPP_RGB888 <<
MCDE_EXTSRCXCONF_BPP_SHIFT;
+ val |= MCDE_EXTSRCXCONF_BGR;
break;
case DRM_FORMAT_ARGB4444:
val |= MCDE_EXTSRCXCONF_BPP_ARGB4444 <<
MCDE_EXTSRCXCONF_BPP_SHIFT;
- val |= MCDE_EXTSRCXCONF_BGR;
break;
case DRM_FORMAT_ABGR4444:
val |= MCDE_EXTSRCXCONF_BPP_ARGB4444 <<
MCDE_EXTSRCXCONF_BPP_SHIFT;
+ val |= MCDE_EXTSRCXCONF_BGR;
break;
case DRM_FORMAT_XRGB4444:
val |= MCDE_EXTSRCXCONF_BPP_RGB444 <<
MCDE_EXTSRCXCONF_BPP_SHIFT;
- val |= MCDE_EXTSRCXCONF_BGR;
break;
case DRM_FORMAT_XBGR4444:
val |= MCDE_EXTSRCXCONF_BPP_RGB444 <<
MCDE_EXTSRCXCONF_BPP_SHIFT;
+ val |= MCDE_EXTSRCXCONF_BGR;
break;
case DRM_FORMAT_XRGB1555:
val |= MCDE_EXTSRCXCONF_BPP_IRGB1555 <<
MCDE_EXTSRCXCONF_BPP_SHIFT;
- val |= MCDE_EXTSRCXCONF_BGR;
break;
case DRM_FORMAT_XBGR1555:
val |= MCDE_EXTSRCXCONF_BPP_IRGB1555 <<
MCDE_EXTSRCXCONF_BPP_SHIFT;
+ val |= MCDE_EXTSRCXCONF_BGR;
break;
case DRM_FORMAT_RGB565:
val |= MCDE_EXTSRCXCONF_BPP_RGB565 <<
MCDE_EXTSRCXCONF_BPP_SHIFT;
- val |= MCDE_EXTSRCXCONF_BGR;
break;
case DRM_FORMAT_BGR565:
val |= MCDE_EXTSRCXCONF_BPP_RGB565 <<
MCDE_EXTSRCXCONF_BPP_SHIFT;
+ val |= MCDE_EXTSRCXCONF_BGR;
break;
case DRM_FORMAT_YUV422:
val |= MCDE_EXTSRCXCONF_BPP_YCBCR422 <<
@@ -810,7 +807,9 @@ static void mcde_configure_dsi_formatter(struct mcde *mcde,
MCDE_DSICONF0_PACKING_SHIFT;
break;
case MIPI_DSI_FMT_RGB666_PACKED:
- val |= MCDE_DSICONF0_PACKING_RGB666_PACKED <<
+ dev_err(mcde->dev,
+ "we cannot handle the packed RGB666 format\n");
+ val |= MCDE_DSICONF0_PACKING_RGB666 <<
MCDE_DSICONF0_PACKING_SHIFT;
break;
case MIPI_DSI_FMT_RGB565:
diff --git a/drivers/gpu/drm/mcde/mcde_display_regs.h b/drivers/gpu/drm/mcde/mcde_display_regs.h
index ae76da8a2138..2ad78c59d627 100644
--- a/drivers/gpu/drm/mcde/mcde_display_regs.h
+++ b/drivers/gpu/drm/mcde/mcde_display_regs.h
@@ -552,8 +552,8 @@
#define MCDE_DSICONF0_PACKING_MASK 0x00700000
#define MCDE_DSICONF0_PACKING_RGB565 0
#define MCDE_DSICONF0_PACKING_RGB666 1
-#define MCDE_DSICONF0_PACKING_RGB666_PACKED 2
-#define MCDE_DSICONF0_PACKING_RGB888 3
+#define MCDE_DSICONF0_PACKING_RGB888 2
+#define MCDE_DSICONF0_PACKING_BGR888 3
#define MCDE_DSICONF0_PACKING_HDTV 4
#define MCDE_DSIVID0FRAME 0x00000E04
--
2.26.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] drm/mcde: Fix RGB/BGR bug
2020-11-17 17:54 [PATCH] drm/mcde: Fix RGB/BGR bug Linus Walleij
@ 2020-11-17 19:24 ` Sam Ravnborg
2020-11-17 20:04 ` Linus Walleij
2020-11-23 21:59 ` Sam Ravnborg
1 sibling, 1 reply; 4+ messages in thread
From: Sam Ravnborg @ 2020-11-17 19:24 UTC (permalink / raw)
To: Linus Walleij
Cc: Stephan Gerhold, Maarten Lankhorst, Maxime Ripard, dri-devel,
phone-devel, Sean Paul, linux-arm-kernel
Hi Linus
On Tue, Nov 17, 2020 at 06:54:13PM +0100, Linus Walleij wrote:
> I was confused when the graphics came out with blue
> penguins on the DPI panel.
>
> It turns out that the so-called "packed RGB666" mode
> on the DSI formatter is incorrect: this mode is the
> actual RGB888 mode, and the mode called RGB888 is
> BGR888.
>
> The claims that the MCDE had inverse RGB/BGR buffer
> formats was wrong, so correct this and the buggy
> register and everything is much more consistent, and
> graphics look good on all targets, both DPI and
> DSI.
>
> Cc: phone-devel@vger.kernel.org
> Cc: Stephan Gerhold <stephan@gerhold.net>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Fixes: ??
Sam
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/mcde: Fix RGB/BGR bug
2020-11-17 19:24 ` Sam Ravnborg
@ 2020-11-17 20:04 ` Linus Walleij
0 siblings, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2020-11-17 20:04 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Stephan Gerhold, Maarten Lankhorst, Maxime Ripard,
open list:DRM PANEL DRIVERS, phone-devel, Sean Paul, Linux ARM
On Tue, Nov 17, 2020 at 8:24 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> > Cc: phone-devel@vger.kernel.org
> > Cc: Stephan Gerhold <stephan@gerhold.net>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> Fixes: ??
It's no regression actually: for the current hardware we just
invert the buffers twice and it looks OK. Buffers are declared
BGR and then BGR-switched again by the DSI formatter.
But when adding DPI (I have some two other patches on the
list) this needs to be corrected.
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/mcde: Fix RGB/BGR bug
2020-11-17 17:54 [PATCH] drm/mcde: Fix RGB/BGR bug Linus Walleij
2020-11-17 19:24 ` Sam Ravnborg
@ 2020-11-23 21:59 ` Sam Ravnborg
1 sibling, 0 replies; 4+ messages in thread
From: Sam Ravnborg @ 2020-11-23 21:59 UTC (permalink / raw)
To: Linus Walleij
Cc: Stephan Gerhold, Maarten Lankhorst, Maxime Ripard, dri-devel,
phone-devel, Sean Paul, linux-arm-kernel
Hi Linus.
On Tue, Nov 17, 2020 at 06:54:13PM +0100, Linus Walleij wrote:
> I was confused when the graphics came out with blue
> penguins on the DPI panel.
>
> It turns out that the so-called "packed RGB666" mode
> on the DSI formatter is incorrect: this mode is the
> actual RGB888 mode, and the mode called RGB888 is
> BGR888.
>
> The claims that the MCDE had inverse RGB/BGR buffer
> formats was wrong, so correct this and the buggy
> register and everything is much more consistent, and
> graphics look good on all targets, both DPI and
> DSI.
>
> Cc: phone-devel@vger.kernel.org
> Cc: Stephan Gerhold <stephan@gerhold.net>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Looks fine and seems to do what you write it should do.
I do not understand why this part:
> case DRM_FORMAT_BGR888:
> val |= MCDE_EXTSRCXCONF_BPP_RGB888 <<
> MCDE_EXTSRCXCONF_BPP_SHIFT;
> + val |= MCDE_EXTSRCXCONF_BGR;
> break;
does not use MCDE_EXTSRCXCONF_BPP_BGR888
But maybe there is no counterpart to MCDE_DSICONF0_PACKING_BGR888?
Assuming all is good despite my confusion:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-23 22:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-17 17:54 [PATCH] drm/mcde: Fix RGB/BGR bug Linus Walleij
2020-11-17 19:24 ` Sam Ravnborg
2020-11-17 20:04 ` Linus Walleij
2020-11-23 21:59 ` Sam Ravnborg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).