All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Ravnborg <sam@ravnborg.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: dri-devel@lists.freedesktop.org,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>, Sean Paul <sean@poorly.run>,
	phone-devel@vger.kernel.org,
	Stephan Gerhold <stephan@gerhold.net>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] drm/mcde: Fix RGB/BGR bug
Date: Mon, 23 Nov 2020 22:59:04 +0100	[thread overview]
Message-ID: <20201123215904.GA676783@ravnborg.org> (raw)
In-Reply-To: <20201117175413.869871-1-linus.walleij@linaro.org>

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>

WARNING: multiple messages have this Message-ID (diff)
From: Sam Ravnborg <sam@ravnborg.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Stephan Gerhold <stephan@gerhold.net>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	dri-devel@lists.freedesktop.org, phone-devel@vger.kernel.org,
	Sean Paul <sean@poorly.run>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] drm/mcde: Fix RGB/BGR bug
Date: Mon, 23 Nov 2020 22:59:04 +0100	[thread overview]
Message-ID: <20201123215904.GA676783@ravnborg.org> (raw)
In-Reply-To: <20201117175413.869871-1-linus.walleij@linaro.org>

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

WARNING: multiple messages have this Message-ID (diff)
From: Sam Ravnborg <sam@ravnborg.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Stephan Gerhold <stephan@gerhold.net>,
	dri-devel@lists.freedesktop.org, phone-devel@vger.kernel.org,
	Sean Paul <sean@poorly.run>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] drm/mcde: Fix RGB/BGR bug
Date: Mon, 23 Nov 2020 22:59:04 +0100	[thread overview]
Message-ID: <20201123215904.GA676783@ravnborg.org> (raw)
In-Reply-To: <20201117175413.869871-1-linus.walleij@linaro.org>

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>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2020-11-23 21:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-17 17:54 [PATCH] drm/mcde: Fix RGB/BGR bug Linus Walleij
2020-11-17 17:54 ` Linus Walleij
2020-11-17 17:54 ` Linus Walleij
2020-11-17 19:24 ` Sam Ravnborg
2020-11-17 19:24   ` Sam Ravnborg
2020-11-17 19:24   ` Sam Ravnborg
2020-11-17 20:04   ` Linus Walleij
2020-11-17 20:04     ` Linus Walleij
2020-11-17 20:04     ` Linus Walleij
2020-11-23 21:59 ` Sam Ravnborg [this message]
2020-11-23 21:59   ` Sam Ravnborg
2020-11-23 21:59   ` Sam Ravnborg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201123215904.GA676783@ravnborg.org \
    --to=sam@ravnborg.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=phone-devel@vger.kernel.org \
    --cc=sean@poorly.run \
    --cc=stephan@gerhold.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.