diff mbox series

drm/msm/dpu: Add UBWC support for RGB8888 formats

Message ID 20191106232553.76553-1-frkoenig@google.com (mailing list archive)
State Accepted
Commit cb929b8f5faa2a94c2f28e656e243e3ec173b480
Headers show
Series drm/msm/dpu: Add UBWC support for RGB8888 formats | expand

Commit Message

Fritz Koenig Nov. 6, 2019, 11:25 p.m. UTC
Hardware only natively supports BGR8888 UBWC.
UBWC support for RGB8888 can be had by pretending
that the buffer is BGR.

Signed-off-by: Fritz Koenig <frkoenig@google.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c    | 18 ++++++++++++++++++
 .../drm/msm/disp/dpu1/dpu_hw_catalog_format.h  |  2 ++
 2 files changed, 20 insertions(+)

Comments

Rob Clark Nov. 7, 2019, 4:29 p.m. UTC | #1
On Wed, Nov 6, 2019 at 3:26 PM Fritz Koenig <frkoenig@google.com> wrote:
>
> Hardware only natively supports BGR8888 UBWC.
> UBWC support for RGB8888 can be had by pretending
> that the buffer is BGR.

Just to expand, this aligns with how we handle RGB component order in
mesa for tiled or tiled+ubwc.  If uncompressed to linear the component
order is RGB, but in tiled or tiled+ubwc, the component order is
always the hw "native" order (BGR) regardless of what the outside
world thinks.  But that detail kinda doesn't matter, it's not like
generic code is going to understand the tiled or tiled+ubwc format in
the first place.. and code that does understand it, knows enough to
know that tiled/tiled+ubwc is always in the native component order.

> Signed-off-by: Fritz Koenig <frkoenig@google.com>

Reviewed-by: Rob Clark <robdclark@gmail.com>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c    | 18 ++++++++++++++++++
>  .../drm/msm/disp/dpu1/dpu_hw_catalog_format.h  |  2 ++
>  2 files changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> index 24ab6249083a..528632690f1e 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
> @@ -489,12 +489,28 @@ static const struct dpu_format dpu_format_map_ubwc[] = {
>                 true, 4, DPU_FORMAT_FLAG_COMPRESSED,
>                 DPU_FETCH_UBWC, 2, DPU_TILE_HEIGHT_UBWC),
>
> +       /* ARGB8888 and ABGR8888 purposely have the same color
> +        * ordering.  The hardware only supports ABGR8888 UBWC
> +        * natively.
> +        */
> +       INTERLEAVED_RGB_FMT_TILED(ARGB8888,
> +               COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
> +               C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4,
> +               true, 4, DPU_FORMAT_FLAG_COMPRESSED,
> +               DPU_FETCH_UBWC, 2, DPU_TILE_HEIGHT_UBWC),
> +
>         INTERLEAVED_RGB_FMT_TILED(XBGR8888,
>                 COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
>                 C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4,
>                 false, 4, DPU_FORMAT_FLAG_COMPRESSED,
>                 DPU_FETCH_UBWC, 2, DPU_TILE_HEIGHT_UBWC),
>
> +       INTERLEAVED_RGB_FMT_TILED(XRGB8888,
> +               COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
> +               C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4,
> +               false, 4, DPU_FORMAT_FLAG_COMPRESSED,
> +               DPU_FETCH_UBWC, 2, DPU_TILE_HEIGHT_UBWC),
> +
>         INTERLEAVED_RGB_FMT_TILED(ABGR2101010,
>                 COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
>                 C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4,
> @@ -550,7 +566,9 @@ static int _dpu_format_get_media_color_ubwc(const struct dpu_format *fmt)
>  {
>         static const struct dpu_media_color_map dpu_media_ubwc_map[] = {
>                 {DRM_FORMAT_ABGR8888, COLOR_FMT_RGBA8888_UBWC},
> +               {DRM_FORMAT_ARGB8888, COLOR_FMT_RGBA8888_UBWC},
>                 {DRM_FORMAT_XBGR8888, COLOR_FMT_RGBA8888_UBWC},
> +               {DRM_FORMAT_XRGB8888, COLOR_FMT_RGBA8888_UBWC},
>                 {DRM_FORMAT_ABGR2101010, COLOR_FMT_RGBA1010102_UBWC},
>                 {DRM_FORMAT_XBGR2101010, COLOR_FMT_RGBA1010102_UBWC},
>                 {DRM_FORMAT_BGR565, COLOR_FMT_RGB565_UBWC},
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h
> index bb6112c949ae..fbcb3c4bbfee 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h
> @@ -6,7 +6,9 @@
>
>  static const uint32_t qcom_compressed_supported_formats[] = {
>         DRM_FORMAT_ABGR8888,
> +       DRM_FORMAT_ARGB8888,
>         DRM_FORMAT_XBGR8888,
> +       DRM_FORMAT_XRGB8888,
>         DRM_FORMAT_BGR565,
>  };
>
> --
> 2.24.0.432.g9d3f5f5b63-goog
>
Eric Anholt Nov. 7, 2019, 5:30 p.m. UTC | #2
Rob Clark <robdclark@gmail.com> writes:

> On Wed, Nov 6, 2019 at 3:26 PM Fritz Koenig <frkoenig@google.com> wrote:
>>
>> Hardware only natively supports BGR8888 UBWC.
>> UBWC support for RGB8888 can be had by pretending
>> that the buffer is BGR.
>
> Just to expand, this aligns with how we handle RGB component order in
> mesa for tiled or tiled+ubwc.  If uncompressed to linear the component
> order is RGB, but in tiled or tiled+ubwc, the component order is
> always the hw "native" order (BGR) regardless of what the outside
> world thinks.  But that detail kinda doesn't matter, it's not like
> generic code is going to understand the tiled or tiled+ubwc format in
> the first place.. and code that does understand it, knows enough to
> know that tiled/tiled+ubwc is always in the native component order.
>
>> Signed-off-by: Fritz Koenig <frkoenig@google.com>
>
> Reviewed-by: Rob Clark <robdclark@gmail.com>

Seems like a reasonable workaround to me, and permissible by our fourcc
modifier rules ("you just have to have one way to address the pixels
given a fourcc and a modifier").

Reviewed-by: Eric Anholt <eric@anholt.net>
Daniel Vetter Nov. 7, 2019, 5:43 p.m. UTC | #3
On Thu, Nov 07, 2019 at 09:30:50AM -0800, Eric Anholt wrote:
> Rob Clark <robdclark@gmail.com> writes:
> > On Wed, Nov 6, 2019 at 3:26 PM Fritz Koenig <frkoenig@google.com> wrote:
> >>
> >> Hardware only natively supports BGR8888 UBWC.
> >> UBWC support for RGB8888 can be had by pretending
> >> that the buffer is BGR.
> >
> > Just to expand, this aligns with how we handle RGB component order in
> > mesa for tiled or tiled+ubwc.  If uncompressed to linear the component
> > order is RGB, but in tiled or tiled+ubwc, the component order is
> > always the hw "native" order (BGR) regardless of what the outside
> > world thinks.  But that detail kinda doesn't matter, it's not like
> > generic code is going to understand the tiled or tiled+ubwc format in
> > the first place.. and code that does understand it, knows enough to
> > know that tiled/tiled+ubwc is always in the native component order.
> >
> >> Signed-off-by: Fritz Koenig <frkoenig@google.com>
> >
> > Reviewed-by: Rob Clark <robdclark@gmail.com>
> 
> Seems like a reasonable workaround to me, and permissible by our fourcc
> modifier rules ("you just have to have one way to address the pixels
> given a fourcc and a modifier").

Yeah we have some other aliasing going on already I think. And since for
interopt you just need to pick matching (fourcc, modifier) pairs worst
case that means drivers need to add a bunch of dummies/duplicates. Like we
do here.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Cheers, Daniel

> 
> Reviewed-by: Eric Anholt <eric@anholt.net>



> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index 24ab6249083a..528632690f1e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -489,12 +489,28 @@  static const struct dpu_format dpu_format_map_ubwc[] = {
 		true, 4, DPU_FORMAT_FLAG_COMPRESSED,
 		DPU_FETCH_UBWC, 2, DPU_TILE_HEIGHT_UBWC),
 
+	/* ARGB8888 and ABGR8888 purposely have the same color
+	 * ordering.  The hardware only supports ABGR8888 UBWC
+	 * natively.
+	 */
+	INTERLEAVED_RGB_FMT_TILED(ARGB8888,
+		COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
+		C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4,
+		true, 4, DPU_FORMAT_FLAG_COMPRESSED,
+		DPU_FETCH_UBWC, 2, DPU_TILE_HEIGHT_UBWC),
+
 	INTERLEAVED_RGB_FMT_TILED(XBGR8888,
 		COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
 		C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4,
 		false, 4, DPU_FORMAT_FLAG_COMPRESSED,
 		DPU_FETCH_UBWC, 2, DPU_TILE_HEIGHT_UBWC),
 
+	INTERLEAVED_RGB_FMT_TILED(XRGB8888,
+		COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
+		C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4,
+		false, 4, DPU_FORMAT_FLAG_COMPRESSED,
+		DPU_FETCH_UBWC, 2, DPU_TILE_HEIGHT_UBWC),
+
 	INTERLEAVED_RGB_FMT_TILED(ABGR2101010,
 		COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT,
 		C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4,
@@ -550,7 +566,9 @@  static int _dpu_format_get_media_color_ubwc(const struct dpu_format *fmt)
 {
 	static const struct dpu_media_color_map dpu_media_ubwc_map[] = {
 		{DRM_FORMAT_ABGR8888, COLOR_FMT_RGBA8888_UBWC},
+		{DRM_FORMAT_ARGB8888, COLOR_FMT_RGBA8888_UBWC},
 		{DRM_FORMAT_XBGR8888, COLOR_FMT_RGBA8888_UBWC},
+		{DRM_FORMAT_XRGB8888, COLOR_FMT_RGBA8888_UBWC},
 		{DRM_FORMAT_ABGR2101010, COLOR_FMT_RGBA1010102_UBWC},
 		{DRM_FORMAT_XBGR2101010, COLOR_FMT_RGBA1010102_UBWC},
 		{DRM_FORMAT_BGR565, COLOR_FMT_RGB565_UBWC},
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h
index bb6112c949ae..fbcb3c4bbfee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h
@@ -6,7 +6,9 @@ 
 
 static const uint32_t qcom_compressed_supported_formats[] = {
 	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_ARGB8888,
 	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_BGR565,
 };